diff --git a/sherlock/db/migrations/000059_version_user.down.sql b/sherlock/db/migrations/000059_version_user.down.sql new file mode 100644 index 000000000..76b1fe9c6 --- /dev/null +++ b/sherlock/db/migrations/000059_version_user.down.sql @@ -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; diff --git a/sherlock/db/migrations/000059_version_user.up.sql b/sherlock/db/migrations/000059_version_user.up.sql new file mode 100644 index 000000000..d69c3d029 --- /dev/null +++ b/sherlock/db/migrations/000059_version_user.up.sql @@ -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; diff --git a/sherlock/internal/api/sherlock/app_version_procedures_v3_changelog_test.go b/sherlock/internal/api/sherlock/app_version_procedures_v3_changelog_test.go index cd9485681..b24491891 100644 --- a/sherlock/internal/api/sherlock/app_version_procedures_v3_changelog_test.go +++ b/sherlock/internal/api/sherlock/app_version_procedures_v3_changelog_test.go @@ -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"} @@ -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"} @@ -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"} @@ -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"} @@ -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"} diff --git a/sherlock/internal/api/sherlock/app_version_v3.go b/sherlock/internal/api/sherlock/app_version_v3.go index 2ee0a398a..f7de41b75 100644 --- a/sherlock/internal/api/sherlock/app_version_v3.go +++ b/sherlock/internal/api/sherlock/app_version_v3.go @@ -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 } @@ -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, @@ -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 } @@ -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, diff --git a/sherlock/internal/api/sherlock/app_version_v3_edit_test.go b/sherlock/internal/api/sherlock/app_version_v3_edit_test.go index 6e9f10c39..fc141f586 100644 --- a/sherlock/internal/api/sherlock/app_version_v3_edit_test.go +++ b/sherlock/internal/api/sherlock/app_version_v3_edit_test.go @@ -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"), diff --git a/sherlock/internal/api/sherlock/app_version_v3_get_test.go b/sherlock/internal/api/sherlock/app_version_v3_get_test.go index 9e2604f89..a63409013 100644 --- a/sherlock/internal/api/sherlock/app_version_v3_get_test.go +++ b/sherlock/internal/api/sherlock/app_version_v3_get_test.go @@ -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" @@ -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"} @@ -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) } diff --git a/sherlock/internal/api/sherlock/app_version_v3_list_test.go b/sherlock/internal/api/sherlock/app_version_v3_list_test.go index 7096c6131..5c8bbdf2a 100644 --- a/sherlock/internal/api/sherlock/app_version_v3_list_test.go +++ b/sherlock/internal/api/sherlock/app_version_v3_list_test.go @@ -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" @@ -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"), @@ -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( diff --git a/sherlock/internal/api/sherlock/app_version_v3_upsert_test.go b/sherlock/internal/api/sherlock/app_version_v3_upsert_test.go index 8f7e942a1..d0a2ad910 100644 --- a/sherlock/internal/api/sherlock/app_version_v3_upsert_test.go +++ b/sherlock/internal/api/sherlock/app_version_v3_upsert_test.go @@ -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" @@ -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( diff --git a/sherlock/internal/api/sherlock/chart_version_procedures_v3_changelog_test.go b/sherlock/internal/api/sherlock/chart_version_procedures_v3_changelog_test.go index cf83c2190..afcbf5e52 100644 --- a/sherlock/internal/api/sherlock/chart_version_procedures_v3_changelog_test.go +++ b/sherlock/internal/api/sherlock/chart_version_procedures_v3_changelog_test.go @@ -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"} @@ -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"} @@ -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"} @@ -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"} @@ -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"} diff --git a/sherlock/internal/api/sherlock/chart_version_v3.go b/sherlock/internal/api/sherlock/chart_version_v3.go index 275cee05c..21e31ba89 100644 --- a/sherlock/internal/api/sherlock/chart_version_v3.go +++ b/sherlock/internal/api/sherlock/chart_version_v3.go @@ -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 } @@ -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 } @@ -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, diff --git a/sherlock/internal/api/sherlock/chart_version_v3_edit_test.go b/sherlock/internal/api/sherlock/chart_version_v3_edit_test.go index c26464b54..a55239b46 100644 --- a/sherlock/internal/api/sherlock/chart_version_v3_edit_test.go +++ b/sherlock/internal/api/sherlock/chart_version_v3_edit_test.go @@ -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"), diff --git a/sherlock/internal/api/sherlock/chart_version_v3_get_test.go b/sherlock/internal/api/sherlock/chart_version_v3_get_test.go index fe20c0582..890394f45 100644 --- a/sherlock/internal/api/sherlock/chart_version_v3_get_test.go +++ b/sherlock/internal/api/sherlock/chart_version_v3_get_test.go @@ -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" @@ -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"} @@ -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) } diff --git a/sherlock/internal/api/sherlock/chart_version_v3_list_test.go b/sherlock/internal/api/sherlock/chart_version_v3_list_test.go index a5119bbb7..0ee342221 100644 --- a/sherlock/internal/api/sherlock/chart_version_v3_list_test.go +++ b/sherlock/internal/api/sherlock/chart_version_v3_list_test.go @@ -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" @@ -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"), @@ -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"} @@ -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( diff --git a/sherlock/internal/api/sherlock/chart_version_v3_upsert_test.go b/sherlock/internal/api/sherlock/chart_version_v3_upsert_test.go index 4640989c5..95cf458b8 100644 --- a/sherlock/internal/api/sherlock/chart_version_v3_upsert_test.go +++ b/sherlock/internal/api/sherlock/chart_version_v3_upsert_test.go @@ -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" @@ -40,6 +41,7 @@ func (s *handlerSuite) TestChartVersionsV3Upsert_error() { } func (s *handlerSuite) TestChartVersionsV3Upsert() { + s.SetNonSuitableTestUserForDB() chart := models.Chart{ Name: "chart-name", ChartRepo: utils.PointerTo("terra-helm"), @@ -70,6 +72,9 @@ func (s *handlerSuite) TestChartVersionsV3Upsert() { 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 ChartVersionV3 code = s.HandleRequest( diff --git a/sherlock/internal/models/app_version.go b/sherlock/internal/models/app_version.go index 32bc8ff48..8b92e7c6b 100644 --- a/sherlock/internal/models/app_version.go +++ b/sherlock/internal/models/app_version.go @@ -16,13 +16,15 @@ type AppVersion struct { Description string ParentAppVersion *AppVersion ParentAppVersionID *uint + AuthoredBy *User + AuthoredByID *uint } -func (a AppVersion) TableName() string { +func (a *AppVersion) TableName() string { return "v2_app_versions" } -func (a AppVersion) GetCiIdentifier() CiIdentifier { +func (a *AppVersion) GetCiIdentifier() CiIdentifier { if a.CiIdentifier != nil { return *a.CiIdentifier } else { @@ -30,6 +32,15 @@ func (a AppVersion) GetCiIdentifier() CiIdentifier { } } +func (a *AppVersion) BeforeCreate(tx *gorm.DB) error { + if user, err := GetCurrentUserForDB(tx); err != nil { + return err + } else { + a.AuthoredByID = &user.ID + } + return nil +} + // GetAppVersionPathIDs iterates from one AppVersion to another, treating each one's parent like a linked list. // If a path is found in few enough iterations, it'll be returned according to these rules: // - It will always be exclusive of the end AppVersion diff --git a/sherlock/internal/models/app_version_test.go b/sherlock/internal/models/app_version_test.go index ed38ac5b2..5326dc722 100644 --- a/sherlock/internal/models/app_version_test.go +++ b/sherlock/internal/models/app_version_test.go @@ -1,13 +1,18 @@ package models -import "github.com/broadinstitute/sherlock/go-shared/pkg/utils" +import ( + "github.com/broadinstitute/sherlock/go-shared/pkg/utils" + "github.com/broadinstitute/sherlock/sherlock/internal/authentication/test_users" +) func (s *modelSuite) TestAppVersionChartIdValidationSqlMissing() { + s.SetNonSuitableTestUserForDB() err := s.DB.Create(&AppVersion{AppVersion: "version"}).Error s.ErrorContains(err, "fk_v2_app_versions_chart") } func (s *modelSuite) TestAppVersionVersionValidationSqlMissing() { + s.SetNonSuitableTestUserForDB() chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} err := s.DB.Create(&chart).Error s.NoError(err) @@ -16,6 +21,7 @@ func (s *modelSuite) TestAppVersionVersionValidationSqlMissing() { } func (s *modelSuite) TestAppVersionVersionValidationSqlEmpty() { + s.SetNonSuitableTestUserForDB() chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} err := s.DB.Create(&chart).Error s.NoError(err) @@ -24,6 +30,7 @@ func (s *modelSuite) TestAppVersionVersionValidationSqlEmpty() { } func (s *modelSuite) TestAppVersionUniquenessSql() { + s.SetNonSuitableTestUserForDB() chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} err := s.DB.Create(&chart).Error s.NoError(err) @@ -34,6 +41,7 @@ func (s *modelSuite) TestAppVersionUniquenessSql() { } func (s *modelSuite) TestAppVersionValid() { + s.SetNonSuitableTestUserForDB() chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} err := s.DB.Create(&chart).Error s.NoError(err) @@ -42,6 +50,7 @@ func (s *modelSuite) TestAppVersionValid() { } func (s *modelSuite) TestAppVersionCiIdentifiers() { + s.SetNonSuitableTestUserForDB() chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} s.NoError(s.DB.Create(&chart).Error) appVersion := AppVersion{ChartID: chart.ID, AppVersion: "version"} @@ -62,6 +71,7 @@ func (s *modelSuite) TestAppVersionCiIdentifiers() { } func (s *modelSuite) TestGetAppVersionPathIDs() { + s.SetNonSuitableTestUserForDB() /* Here's the layout of the app versions we're creating for this test. A, B, C, D are linear. B and E both point at C. Nothing points at F. @@ -176,3 +186,35 @@ func (s *modelSuite) TestGetAppVersionPathIDs() { s.NoError(err) }) } + +func (s *modelSuite) TestAppVersionRecordsUser() { + s.SetNonSuitableTestUserForDB() + chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} + s.NoError(s.DB.Create(&chart).Error) + s.Run("via db.Create", func() { + version := AppVersion{ChartID: chart.ID, AppVersion: "a"} + s.NoError(s.DB.Create(&version).Error) + s.NotNil(version.AuthoredByID) + s.NoError(s.DB.Preload("AuthoredBy").First(&version, version.ID).Error) + if s.NotNil(version.AuthoredBy) { + s.Equal(test_users.NonSuitableTestUserEmail, version.AuthoredBy.Email) + } + }) + s.Run("via db.FirstOrCreate", func() { + version := AppVersion{ChartID: chart.ID, AppVersion: "b"} + s.NoError(s.DB.FirstOrCreate(&version).Error) + s.NotNil(version.AuthoredByID) + s.NoError(s.DB.Preload("AuthoredBy").First(&version, version.ID).Error) + if s.NotNil(version.AuthoredBy) { + s.Equal(test_users.NonSuitableTestUserEmail, version.AuthoredBy.Email) + } + }) +} + +func (s *modelSuite) TestAppVersionErrorsWithoutUser() { + chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} + err := s.DB.Create(&chart).Error + s.NoError(err) + err = s.DB.Create(&AppVersion{ChartID: chart.ID, AppVersion: "version"}).Error + s.ErrorContains(err, "database user") +} diff --git a/sherlock/internal/models/chart_version.go b/sherlock/internal/models/chart_version.go index 5836b84b8..afa141ce5 100644 --- a/sherlock/internal/models/chart_version.go +++ b/sherlock/internal/models/chart_version.go @@ -14,13 +14,15 @@ type ChartVersion struct { Description string ParentChartVersion *ChartVersion ParentChartVersionID *uint + AuthoredBy *User + AuthoredByID *uint } -func (c ChartVersion) TableName() string { +func (c *ChartVersion) TableName() string { return "v2_chart_versions" } -func (c ChartVersion) GetCiIdentifier() CiIdentifier { +func (c *ChartVersion) GetCiIdentifier() CiIdentifier { if c.CiIdentifier != nil { return *c.CiIdentifier } else { @@ -28,6 +30,15 @@ func (c ChartVersion) GetCiIdentifier() CiIdentifier { } } +func (c *ChartVersion) BeforeCreate(tx *gorm.DB) error { + if user, err := GetCurrentUserForDB(tx); err != nil { + return err + } else { + c.AuthoredByID = &user.ID + } + return nil +} + // GetChartVersionPathIDs iterates from one ChartVersion to another, treating each one's parent like a linked list. // If a path is found in few enough iterations, it'll be returned according to these rules: // - It will always be exclusive of the end ChartVersion diff --git a/sherlock/internal/models/chart_version_test.go b/sherlock/internal/models/chart_version_test.go index 745836a9c..c668d42a2 100644 --- a/sherlock/internal/models/chart_version_test.go +++ b/sherlock/internal/models/chart_version_test.go @@ -1,13 +1,18 @@ package models -import "github.com/broadinstitute/sherlock/go-shared/pkg/utils" +import ( + "github.com/broadinstitute/sherlock/go-shared/pkg/utils" + "github.com/broadinstitute/sherlock/sherlock/internal/authentication/test_users" +) func (s *modelSuite) TestChartVersionChartIdValidationSqlMissing() { + s.SetNonSuitableTestUserForDB() err := s.DB.Create(&ChartVersion{ChartVersion: "version"}).Error s.ErrorContains(err, "fk_v2_chart_versions_chart") } func (s *modelSuite) TestChartVersionVersionValidationSqlMissing() { + s.SetNonSuitableTestUserForDB() chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} err := s.DB.Create(&chart).Error s.NoError(err) @@ -16,6 +21,7 @@ func (s *modelSuite) TestChartVersionVersionValidationSqlMissing() { } func (s *modelSuite) TestChartVersionVersionValidationSqlEmpty() { + s.SetNonSuitableTestUserForDB() chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} err := s.DB.Create(&chart).Error s.NoError(err) @@ -24,6 +30,7 @@ func (s *modelSuite) TestChartVersionVersionValidationSqlEmpty() { } func (s *modelSuite) TestChartVersionUniquenessSql() { + s.SetNonSuitableTestUserForDB() chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} err := s.DB.Create(&chart).Error s.NoError(err) @@ -34,6 +41,7 @@ func (s *modelSuite) TestChartVersionUniquenessSql() { } func (s *modelSuite) TestChartVersionValid() { + s.SetNonSuitableTestUserForDB() chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} err := s.DB.Create(&chart).Error s.NoError(err) @@ -42,6 +50,7 @@ func (s *modelSuite) TestChartVersionValid() { } func (s *modelSuite) TestChartVersionCiIdentifiers() { + s.SetNonSuitableTestUserForDB() chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} s.NoError(s.DB.Create(&chart).Error) chartVersion := ChartVersion{ChartID: chart.ID, ChartVersion: "version"} @@ -62,6 +71,7 @@ func (s *modelSuite) TestChartVersionCiIdentifiers() { } func (s *modelSuite) TestGetChartVersionPathIDs() { + s.SetNonSuitableTestUserForDB() /* Here's the layout of the chart versions we're creating for this test. A, B, C, D are linear. B and E both point at C. Nothing points at F. @@ -174,3 +184,35 @@ func (s *modelSuite) TestGetChartVersionPathIDs() { s.NoError(err) }) } + +func (s *modelSuite) TestChartVersionRecordsUser() { + s.SetNonSuitableTestUserForDB() + chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} + s.NoError(s.DB.Create(&chart).Error) + s.Run("via db.Create", func() { + version := ChartVersion{ChartID: chart.ID, ChartVersion: "a"} + s.NoError(s.DB.Create(&version).Error) + s.NotNil(version.AuthoredByID) + s.NoError(s.DB.Preload("AuthoredBy").First(&version, version.ID).Error) + if s.NotNil(version.AuthoredBy) { + s.Equal(test_users.NonSuitableTestUserEmail, version.AuthoredBy.Email) + } + }) + s.Run("via db.FirstOrCreate", func() { + version := ChartVersion{ChartID: chart.ID, ChartVersion: "b"} + s.NoError(s.DB.FirstOrCreate(&version).Error) + s.NotNil(version.AuthoredByID) + s.NoError(s.DB.Preload("AuthoredBy").First(&version, version.ID).Error) + if s.NotNil(version.AuthoredBy) { + s.Equal(test_users.NonSuitableTestUserEmail, version.AuthoredBy.Email) + } + }) +} + +func (s *modelSuite) TestChartVersionErrorsWithoutUser() { + chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")} + err := s.DB.Create(&chart).Error + s.NoError(err) + err = s.DB.Create(&ChartVersion{ChartID: chart.ID, ChartVersion: "version"}).Error + s.ErrorContains(err, "database user") +}