Skip to content

Commit

Permalink
[DDO-3208] Record authors upon v3 version creation (#325)
Browse files Browse the repository at this point in the history
  • Loading branch information
jack-r-warren authored Oct 10, 2023
1 parent 04bb47e commit e3a8906
Show file tree
Hide file tree
Showing 18 changed files with 243 additions and 7 deletions.
11 changes: 11 additions & 0 deletions sherlock/db/migrations/000059_version_user.down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
alter table v2_chart_versions
drop constraint fk_v2_chart_versions_authored_by;

alter table v2_chart_versions
drop if exists authored_by_id;

alter table v2_app_versions
drop constraint fk_v2_app_versions_authored_by;

alter table v2_app_versions
drop if exists authored_by_id;
13 changes: 13 additions & 0 deletions sherlock/db/migrations/000059_version_user.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
alter table v2_app_versions
add if not exists authored_by_id bigint;

alter table v2_app_versions
add constraint fk_v2_app_versions_authored_by
foreign key (authored_by_id) references v2_users;

alter table v2_chart_versions
add if not exists authored_by_id bigint;

alter table v2_chart_versions
add constraint fk_v2_chart_versions_authored_by
foreign key (authored_by_id) references v2_users;
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func (s *handlerSuite) TestAppVersionsProceduresV3Changelog_badParentSelector()
}

func (s *handlerSuite) TestAppVersionsProceduresV3Changelog_childNotFound() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{Name: "my-chart", ChartRepo: utils.PointerTo("some-repo")}
s.NoError(s.DB.Create(&chart).Error)
appVersion := models.AppVersion{ChartID: chart.ID, AppVersion: "1"}
Expand All @@ -41,6 +42,7 @@ func (s *handlerSuite) TestAppVersionsProceduresV3Changelog_childNotFound() {
}

func (s *handlerSuite) TestAppVersionsProceduresV3Changelog_parentNotFound() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{Name: "my-chart", ChartRepo: utils.PointerTo("some-repo")}
s.NoError(s.DB.Create(&chart).Error)
appVersion := models.AppVersion{ChartID: chart.ID, AppVersion: "1"}
Expand All @@ -56,6 +58,7 @@ func (s *handlerSuite) TestAppVersionsProceduresV3Changelog_parentNotFound() {
}

func (s *handlerSuite) TestAppVersionsProceduresV3Changelog_sameChildAndParent() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{Name: "my-chart", ChartRepo: utils.PointerTo("some-repo")}
s.NoError(s.DB.Create(&chart).Error)
appVersion := models.AppVersion{ChartID: chart.ID, AppVersion: "1"}
Expand All @@ -71,6 +74,7 @@ func (s *handlerSuite) TestAppVersionsProceduresV3Changelog_sameChildAndParent()
}

func (s *handlerSuite) TestAppVersionsProceduresV3Changelog_noPathFound() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{Name: "my-chart", ChartRepo: utils.PointerTo("some-repo")}
s.NoError(s.DB.Create(&chart).Error)
appVersion1 := models.AppVersion{ChartID: chart.ID, AppVersion: "1"}
Expand Down Expand Up @@ -99,6 +103,7 @@ func (s *handlerSuite) TestAppVersionsProceduresV3Changelog_noPathFound() {
}

func (s *handlerSuite) TestAppVersionsProceduresV3Changelog_findsPath() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{Name: "my-chart", ChartRepo: utils.PointerTo("some-repo")}
s.NoError(s.DB.Create(&chart).Error)
appVersion1 := models.AppVersion{ChartID: chart.ID, AppVersion: "1"}
Expand Down
27 changes: 27 additions & 0 deletions sherlock/internal/api/sherlock/app_version_v3.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ type AppVersionV3 struct {
CiIdentifier *CiIdentifierV3 `json:"ciIdentifier,omitempty" form:"-"`
ChartInfo *ChartV3 `json:"chartInfo,omitempty" form:"-"`
ParentAppVersionInfo *AppVersionV3 `json:"parentAppVersionInfo,omitempty" swaggertype:"object" form:"-"`
AuthoredBy string `json:"authoredBy,omitempty" form:"authoredBy"`
AuthoredByInfo *UserV3 `json:"authoredByInfo,omitempty" form:"-"`
AppVersionV3Create
}

Expand Down Expand Up @@ -53,6 +55,20 @@ func (v AppVersionV3) toModel(db *gorm.DB, failIfParentInvalid bool) (models.App
parentAppVersionID = &parentAppVersionResult.ID
}
}

var authoredByID *uint
if v.AuthoredBy != "" {
userQuery, err := userModelFromSelector(v.AuthoredBy)
if err != nil {
return models.AppVersion{}, err
}
var userResult models.User
if err = db.Where(&userQuery).First(&userResult).Error; err != nil {
return models.AppVersion{}, err
} else {
authoredByID = &userResult.ID
}
}
return models.AppVersion{
Model: v.toGormModel(),
ChartID: chartResult.ID,
Expand All @@ -61,6 +77,7 @@ func (v AppVersionV3) toModel(db *gorm.DB, failIfParentInvalid bool) (models.App
GitBranch: v.GitBranch,
Description: v.Description,
ParentAppVersionID: parentAppVersionID,
AuthoredByID: authoredByID,
}, nil
}

Expand Down Expand Up @@ -97,11 +114,21 @@ func appVersionFromModel(model models.AppVersion) AppVersionV3 {
parentAppVersionString = utils.UintToString(parentAppVersion.ID)
}
}
var authoredBy *UserV3
var authoredByString string
if model.AuthoredBy != nil {
authoredBy = utils.PointerTo(userFromModel(*model.AuthoredBy))
authoredByString = model.AuthoredBy.Email
} else if model.AuthoredByID != nil {
authoredByString = utils.UintToString(*model.AuthoredByID)
}
return AppVersionV3{
CommonFields: commonFieldsFromGormModel(model.Model),
CiIdentifier: ciIdentifier,
ChartInfo: chart,
ParentAppVersionInfo: parentAppVersion,
AuthoredBy: authoredByString,
AuthoredByInfo: authoredBy,
AppVersionV3Create: AppVersionV3Create{
Chart: chartName,
AppVersion: model.AppVersion,
Expand Down
1 change: 1 addition & 0 deletions sherlock/internal/api/sherlock/app_version_v3_edit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func (s *handlerSuite) TestAppVersionsV3Edit_notFound() {
}

func (s *handlerSuite) TestAppVersionsV3Edit() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{
Name: "name",
ChartRepo: utils.PointerTo("terra-helm"),
Expand Down
5 changes: 5 additions & 0 deletions sherlock/internal/api/sherlock/app_version_v3_get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package sherlock

import (
"github.com/broadinstitute/sherlock/go-shared/pkg/utils"
"github.com/broadinstitute/sherlock/sherlock/internal/authentication/test_users"
"github.com/broadinstitute/sherlock/sherlock/internal/errors"
"github.com/broadinstitute/sherlock/sherlock/internal/models"
"net/http"
Expand All @@ -26,6 +27,7 @@ func (s *handlerSuite) TestAppVersionV3Get_notFound() {
}

func (s *handlerSuite) TestAppVersionV3Get() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{Name: "my-chart", ChartRepo: utils.PointerTo("some-repo")}
s.NoError(s.DB.Create(&chart).Error)
appVersion := models.AppVersion{ChartID: chart.ID, AppVersion: "1"}
Expand All @@ -39,6 +41,9 @@ func (s *handlerSuite) TestAppVersionV3Get() {
if s.NotNil(got.ChartInfo) {
s.NotZero(got.ChartInfo.ID)
}
if s.NotNil(got.AuthoredByInfo) {
s.Equal(test_users.NonSuitableTestUserEmail, got.AuthoredByInfo.Email)
}
s.Equal("1", got.AppVersion)
}

Expand Down
11 changes: 11 additions & 0 deletions sherlock/internal/api/sherlock/app_version_v3_list_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package sherlock

import (
"fmt"
"github.com/broadinstitute/sherlock/go-shared/pkg/utils"
"github.com/broadinstitute/sherlock/sherlock/internal/authentication/test_users"
"github.com/broadinstitute/sherlock/sherlock/internal/errors"
"github.com/broadinstitute/sherlock/sherlock/internal/models"
"net/http"
Expand Down Expand Up @@ -44,6 +46,7 @@ func (s *handlerSuite) TestAppVersionsV3List_badOffset() {
}

func (s *handlerSuite) TestAppVersionsV3List() {
s.SetNonSuitableTestUserForDB()
chart1 := models.Chart{
Name: "name1",
ChartRepo: utils.PointerTo("terra-helm"),
Expand Down Expand Up @@ -80,6 +83,14 @@ func (s *handlerSuite) TestAppVersionsV3List() {
s.Equal(http.StatusOK, code)
s.Len(got, 3)
})
s.Run("all via user filter", func() {
var got []AppVersionV3
code := s.HandleRequest(
s.NewRequest("GET", fmt.Sprintf("/api/app-versions/v3?authoredBy=%s", test_users.NonSuitableTestUserEmail), nil),
&got)
s.Equal(http.StatusOK, code)
s.Len(got, 3)
})
s.Run("none", func() {
var got []AppVersionV3
code := s.HandleRequest(
Expand Down
4 changes: 4 additions & 0 deletions sherlock/internal/api/sherlock/app_version_v3_upsert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package sherlock

import (
"github.com/broadinstitute/sherlock/go-shared/pkg/utils"
"github.com/broadinstitute/sherlock/sherlock/internal/authentication/test_users"
"github.com/broadinstitute/sherlock/sherlock/internal/errors"
"github.com/broadinstitute/sherlock/sherlock/internal/models"
"github.com/gin-gonic/gin"
Expand Down Expand Up @@ -76,6 +77,9 @@ func (s *handlerSuite) TestAppVersionsV3Upsert() {
if s.NotNil(got.Description) {
s.Equal("original description", got.Description)
}
if s.NotNil(got.AuthoredByInfo) {
s.Equal(test_users.SuitableTestUserEmail, got.AuthoredByInfo.Email)
}

var got2 AppVersionV3
code = s.HandleRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func (s *handlerSuite) TestChartVersionsProceduresV3Changelog_badParentSelector(
}

func (s *handlerSuite) TestChartVersionsProceduresV3Changelog_childNotFound() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{Name: "my-chart", ChartRepo: utils.PointerTo("some-repo")}
s.NoError(s.DB.Create(&chart).Error)
chartVersion := models.ChartVersion{ChartID: chart.ID, ChartVersion: "1"}
Expand All @@ -41,6 +42,7 @@ func (s *handlerSuite) TestChartVersionsProceduresV3Changelog_childNotFound() {
}

func (s *handlerSuite) TestChartVersionsProceduresV3Changelog_parentNotFound() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{Name: "my-chart", ChartRepo: utils.PointerTo("some-repo")}
s.NoError(s.DB.Create(&chart).Error)
chartVersion := models.ChartVersion{ChartID: chart.ID, ChartVersion: "1"}
Expand All @@ -56,6 +58,7 @@ func (s *handlerSuite) TestChartVersionsProceduresV3Changelog_parentNotFound() {
}

func (s *handlerSuite) TestChartVersionsProceduresV3Changelog_sameChildAndParent() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{Name: "my-chart", ChartRepo: utils.PointerTo("some-repo")}
s.NoError(s.DB.Create(&chart).Error)
chartVersion := models.ChartVersion{ChartID: chart.ID, ChartVersion: "1"}
Expand All @@ -71,6 +74,7 @@ func (s *handlerSuite) TestChartVersionsProceduresV3Changelog_sameChildAndParent
}

func (s *handlerSuite) TestChartVersionsProceduresV3Changelog_noPathFound() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{Name: "my-chart", ChartRepo: utils.PointerTo("some-repo")}
s.NoError(s.DB.Create(&chart).Error)
chartVersion1 := models.ChartVersion{ChartID: chart.ID, ChartVersion: "1"}
Expand Down Expand Up @@ -99,6 +103,7 @@ func (s *handlerSuite) TestChartVersionsProceduresV3Changelog_noPathFound() {
}

func (s *handlerSuite) TestChartVersionsProceduresV3Changelog_findsPath() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{Name: "my-chart", ChartRepo: utils.PointerTo("some-repo")}
s.NoError(s.DB.Create(&chart).Error)
chartVersion1 := models.ChartVersion{ChartID: chart.ID, ChartVersion: "1"}
Expand Down
27 changes: 27 additions & 0 deletions sherlock/internal/api/sherlock/chart_version_v3.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ type ChartVersionV3 struct {
CiIdentifier *CiIdentifierV3 `json:"ciIdentifier,omitempty" form:"-"`
ChartInfo *ChartV3 `json:"chartInfo,omitempty" form:"-"`
ParentChartVersionInfo *ChartVersionV3 `json:"parentChartVersionInfo,omitempty" swaggertype:"object" form:"-"`
AuthoredBy string `json:"authoredBy,omitempty" form:"authoredBy"`
AuthoredByInfo *UserV3 `json:"authoredByInfo,omitempty" form:"-"`
ChartVersionV3Create
}

Expand Down Expand Up @@ -51,12 +53,27 @@ func (v ChartVersionV3) toModel(db *gorm.DB, failIfParentInvalid bool) (models.C
parentChartVersionID = &parentChartVersionResult.ID
}
}

var authoredByID *uint
if v.AuthoredBy != "" {
userQuery, err := userModelFromSelector(v.AuthoredBy)
if err != nil {
return models.ChartVersion{}, err
}
var userResult models.User
if err = db.Where(&userQuery).First(&userResult).Error; err != nil {
return models.ChartVersion{}, err
} else {
authoredByID = &userResult.ID
}
}
return models.ChartVersion{
Model: v.toGormModel(),
ChartID: chartResult.ID,
ChartVersion: v.ChartVersion,
Description: v.Description,
ParentChartVersionID: parentChartVersionID,
AuthoredByID: authoredByID,
}, nil
}

Expand Down Expand Up @@ -91,11 +108,21 @@ func chartVersionFromModel(model models.ChartVersion) ChartVersionV3 {
parentChartVersionString = utils.UintToString(parentChartVersion.ID)
}
}
var authoredBy *UserV3
var authoredByString string
if model.AuthoredBy != nil {
authoredBy = utils.PointerTo(userFromModel(*model.AuthoredBy))
authoredByString = model.AuthoredBy.Email
} else if model.AuthoredByID != nil {
authoredByString = utils.UintToString(*model.AuthoredByID)
}
return ChartVersionV3{
CommonFields: commonFieldsFromGormModel(model.Model),
CiIdentifier: ciIdentifier,
ChartInfo: chart,
ParentChartVersionInfo: parentChartVersion,
AuthoredBy: authoredByString,
AuthoredByInfo: authoredBy,
ChartVersionV3Create: ChartVersionV3Create{
Chart: chartName,
ChartVersion: model.ChartVersion,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func (s *handlerSuite) TestChartVersionsV3Edit_notFound() {
}

func (s *handlerSuite) TestChartVersionsV3Edit() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{
Name: "name",
ChartRepo: utils.PointerTo("terra-helm"),
Expand Down
5 changes: 5 additions & 0 deletions sherlock/internal/api/sherlock/chart_version_v3_get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package sherlock

import (
"github.com/broadinstitute/sherlock/go-shared/pkg/utils"
"github.com/broadinstitute/sherlock/sherlock/internal/authentication/test_users"
"github.com/broadinstitute/sherlock/sherlock/internal/errors"
"github.com/broadinstitute/sherlock/sherlock/internal/models"
"net/http"
Expand All @@ -26,6 +27,7 @@ func (s *handlerSuite) TestChartVersionV3Get_notFound() {
}

func (s *handlerSuite) TestChartVersionV3Get() {
s.SetNonSuitableTestUserForDB()
chart := models.Chart{Name: "my-chart", ChartRepo: utils.PointerTo("some-repo")}
s.NoError(s.DB.Create(&chart).Error)
chartVersion := models.ChartVersion{ChartID: chart.ID, ChartVersion: "1"}
Expand All @@ -39,6 +41,9 @@ func (s *handlerSuite) TestChartVersionV3Get() {
if s.NotNil(got.ChartInfo) {
s.NotZero(got.ChartInfo.ID)
}
if s.NotNil(got.AuthoredByInfo) {
s.Equal(test_users.NonSuitableTestUserEmail, got.AuthoredByInfo.Email)
}
s.Equal("1", got.ChartVersion)
}

Expand Down
12 changes: 11 additions & 1 deletion sherlock/internal/api/sherlock/chart_version_v3_list_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package sherlock

import (
"fmt"
"github.com/broadinstitute/sherlock/go-shared/pkg/utils"
"github.com/broadinstitute/sherlock/sherlock/internal/authentication/test_users"
"github.com/broadinstitute/sherlock/sherlock/internal/errors"
"github.com/broadinstitute/sherlock/sherlock/internal/models"
"net/http"
Expand Down Expand Up @@ -44,6 +46,7 @@ func (s *handlerSuite) TestChartVersionsV3List_badOffset() {
}

func (s *handlerSuite) TestChartVersionsV3List() {
s.SetNonSuitableTestUserForDB()
chart1 := models.Chart{
Name: "name1",
ChartRepo: utils.PointerTo("terra-helm"),
Expand All @@ -62,7 +65,6 @@ func (s *handlerSuite) TestChartVersionsV3List() {
s.NotZero(chart.ID)
}

println("Chart1", chart1.ID)
chartVersion1 := models.ChartVersion{ChartID: chart1.ID, ChartVersion: "1"}
chartVersion2 := models.ChartVersion{ChartID: chart2.ID, ChartVersion: "2"}
chartVersion3 := models.ChartVersion{ChartID: chart3.ID, ChartVersion: "3"}
Expand All @@ -80,6 +82,14 @@ func (s *handlerSuite) TestChartVersionsV3List() {
s.Equal(http.StatusOK, code)
s.Len(got, 3)
})
s.Run("all via user filter", func() {
var got []ChartVersionV3
code := s.HandleRequest(
s.NewRequest("GET", fmt.Sprintf("/api/chart-versions/v3?authoredBy=%s", test_users.NonSuitableTestUserEmail), nil),
&got)
s.Equal(http.StatusOK, code)
s.Len(got, 3)
})
s.Run("none", func() {
var got []ChartVersionV3
code := s.HandleRequest(
Expand Down
Loading

0 comments on commit e3a8906

Please sign in to comment.