From cb419e799b769d79239eaf0f046dec72bba3eed9 Mon Sep 17 00:00:00 2001 From: idafurjes <36131195+idafurjes@users.noreply.github.com> Date: Fri, 12 Jan 2024 16:43:39 +0100 Subject: [PATCH] Remove folderid service test (#80433) * Remove FolderID from service tests * Add models * Add folderID pack to publicdashboard tests * Remove folderID from dashboard tests * Remove folderID from folders * Remove folderID from ngalert tests * Remove nolint comment * Add back some tests after rebase --- pkg/services/alerting/store_test.go | 5 +- .../dashboardimport/service/service_test.go | 53 +++--- pkg/services/dashboards/accesscontrol_test.go | 12 +- .../database/database_provisioning_test.go | 2 - .../dashboards/database/database_test.go | 154 ++++-------------- pkg/services/dashboards/models_test.go | 6 +- .../service/dashboard_service_test.go | 11 +- .../folderimpl/dashboard_folder_store_test.go | 9 +- pkg/services/folder/folderimpl/folder_test.go | 39 +---- .../folder/folderimpl/sqlstore_test.go | 23 +-- .../ngalert/api/api_ruler_validation_test.go | 1 - .../ngalert/migration/migration_test.go | 10 +- pkg/services/ngalert/ngalert_test.go | 2 - pkg/services/ngalert/store/deltas_test.go | 1 - .../publicdashboards/api/query_test.go | 1 - .../database/database_test.go | 10 +- pkg/services/searchV2/service_bench_test.go | 18 +- .../permissions/dashboards_bench_test.go | 21 ++- 18 files changed, 105 insertions(+), 273 deletions(-) diff --git a/pkg/services/alerting/store_test.go b/pkg/services/alerting/store_test.go index 23745c1ad84..ddae8e6dc30 100644 --- a/pkg/services/alerting/store_test.go +++ b/pkg/services/alerting/store_test.go @@ -434,12 +434,11 @@ func (ss *sqlStore) pauseAllAlerts(t *testing.T, pauseState bool) error { return err } -func insertTestDashboard(t *testing.T, store db.DB, title string, orgId int64, +func insertTestDashboard(t *testing.T, store db.DB, title string, orgID int64, folderId int64, folderUID string, isFolder bool, tags ...any) *dashboards.Dashboard { t.Helper() cmd := dashboards.SaveDashboardCommand{ - OrgID: orgId, - FolderID: folderId, // nolint:staticcheck + OrgID: orgID, FolderUID: folderUID, IsFolder: isFolder, Dashboard: simplejson.NewFromAny(map[string]any{ diff --git a/pkg/services/dashboardimport/service/service_test.go b/pkg/services/dashboardimport/service/service_test.go index c9b0ae8a113..19e0dba8937 100644 --- a/pkg/services/dashboardimport/service/service_test.go +++ b/pkg/services/dashboardimport/service/service_test.go @@ -31,15 +31,15 @@ func TestImportDashboardService(t *testing.T) { importDashboardFunc: func(ctx context.Context, dto *dashboards.SaveDashboardDTO) (*dashboards.Dashboard, error) { importDashboardArg = dto return &dashboards.Dashboard{ - ID: 4, - UID: dto.Dashboard.UID, - Slug: dto.Dashboard.Slug, - OrgID: 3, - Version: dto.Dashboard.Version, - PluginID: "prometheus", - FolderID: dto.Dashboard.FolderID, // nolint:staticcheck - Title: dto.Dashboard.Title, - Data: dto.Dashboard.Data, + ID: 4, + UID: dto.Dashboard.UID, + Slug: dto.Dashboard.Slug, + OrgID: 3, + Version: dto.Dashboard.Version, + PluginID: "prometheus", + FolderUID: dto.Dashboard.FolderUID, + Title: dto.Dashboard.Title, + Data: dto.Dashboard.Data, }, nil }, } @@ -58,7 +58,6 @@ func TestImportDashboardService(t *testing.T) { } folderService := &foldertest.FakeService{ ExpectedFolder: &folder.Folder{ - ID: 5, // nolint:staticcheck UID: "123", }, } @@ -76,8 +75,9 @@ func TestImportDashboardService(t *testing.T) { Inputs: []dashboardimport.ImportDashboardInput{ {Name: "*", Type: "datasource", Value: "prom"}, }, - User: &user.SignedInUser{UserID: 2, OrgRole: org.RoleAdmin, OrgID: 3}, - FolderId: 5, // nolint:staticcheck + User: &user.SignedInUser{UserID: 2, OrgRole: org.RoleAdmin, OrgID: 3}, + // FolderId: 5, + FolderUid: "123", } resp, err := s.ImportDashboard(context.Background(), req) require.NoError(t, err) @@ -91,8 +91,7 @@ func TestImportDashboardService(t *testing.T) { require.Equal(t, int64(3), importDashboardArg.OrgID) require.Equal(t, int64(2), userID) require.Equal(t, "prometheus", importDashboardArg.Dashboard.PluginID) - // nolint:staticcheck - require.Equal(t, int64(5), importDashboardArg.Dashboard.FolderID) + require.Equal(t, "123", importDashboardArg.Dashboard.FolderUID) panel := importDashboardArg.Dashboard.Data.Get("panels").GetIndex(0) require.Equal(t, "prom", panel.Get("datasource").MustString()) @@ -107,22 +106,21 @@ func TestImportDashboardService(t *testing.T) { importDashboardFunc: func(ctx context.Context, dto *dashboards.SaveDashboardDTO) (*dashboards.Dashboard, error) { importDashboardArg = dto return &dashboards.Dashboard{ - ID: 4, - UID: dto.Dashboard.UID, - Slug: dto.Dashboard.Slug, - OrgID: 3, - Version: dto.Dashboard.Version, - PluginID: "prometheus", - FolderID: dto.Dashboard.FolderID, // nolint:staticcheck - Title: dto.Dashboard.Title, - Data: dto.Dashboard.Data, + ID: 4, + UID: dto.Dashboard.UID, + Slug: dto.Dashboard.Slug, + OrgID: 3, + Version: dto.Dashboard.Version, + PluginID: "prometheus", + FolderUID: dto.Dashboard.FolderUID, + Title: dto.Dashboard.Title, + Data: dto.Dashboard.Data, }, nil }, } libraryPanelService := &libraryPanelServiceMock{} folderService := &foldertest.FakeService{ ExpectedFolder: &folder.Folder{ - ID: 5, // nolint:staticcheck UID: "123", }, } @@ -144,8 +142,8 @@ func TestImportDashboardService(t *testing.T) { Inputs: []dashboardimport.ImportDashboardInput{ {Name: "*", Type: "datasource", Value: "prom"}, }, - User: &user.SignedInUser{UserID: 2, OrgRole: org.RoleAdmin, OrgID: 3}, - FolderId: 5, // nolint:staticcheck + User: &user.SignedInUser{UserID: 2, OrgRole: org.RoleAdmin, OrgID: 3}, + FolderUid: "123", } resp, err := s.ImportDashboard(context.Background(), req) require.NoError(t, err) @@ -159,8 +157,7 @@ func TestImportDashboardService(t *testing.T) { require.Equal(t, int64(3), importDashboardArg.OrgID) require.Equal(t, int64(2), userID) require.Equal(t, "", importDashboardArg.Dashboard.PluginID) - // nolint:staticcheck - require.Equal(t, int64(5), importDashboardArg.Dashboard.FolderID) + require.Equal(t, "123", importDashboardArg.Dashboard.FolderUID) panel := importDashboardArg.Dashboard.Data.Get("panels").GetIndex(0) require.Equal(t, "prom", panel.Get("datasource").MustString()) diff --git a/pkg/services/dashboards/accesscontrol_test.go b/pkg/services/dashboards/accesscontrol_test.go index d20cb8c2c73..d1b2d9f584b 100644 --- a/pkg/services/dashboards/accesscontrol_test.go +++ b/pkg/services/dashboards/accesscontrol_test.go @@ -26,8 +26,7 @@ func TestNewFolderNameScopeResolver(t *testing.T) { t.Run("resolver should convert to uid scope", func(t *testing.T) { orgId := rand.Int63() title := "Very complex :title with: and /" + util.GenerateShortUID() - // nolint:staticcheck - db := &folder.Folder{Title: title, ID: rand.Int63(), UID: util.GenerateShortUID()} + db := &folder.Folder{Title: title, UID: util.GenerateShortUID()} folderStore := foldertest.NewFakeFolderStore(t) folderStore.On("GetFolderByTitle", mock.Anything, mock.Anything, mock.Anything).Return(db, nil).Once() @@ -44,8 +43,7 @@ func TestNewFolderNameScopeResolver(t *testing.T) { orgId := rand.Int63() title := "Very complex :title with: and /" + util.GenerateShortUID() - // nolint:staticcheck - db := &folder.Folder{Title: title, ID: rand.Int63(), UID: util.GenerateShortUID()} + db := &folder.Folder{Title: title, UID: util.GenerateShortUID()} folderStore := foldertest.NewFakeFolderStore(t) folderStore.On("GetFolderByTitle", mock.Anything, mock.Anything, mock.Anything).Return(db, nil).Once() @@ -114,7 +112,7 @@ func TestNewFolderIDScopeResolver(t *testing.T) { folderStore := foldertest.NewFakeFolderStore(t) _, resolver := NewFolderIDScopeResolver(folderStore, foldertest.NewFakeService()) - orgId := rand.Int63() + orgID := rand.Int63() uid := util.GenerateShortUID() // nolint:staticcheck db := &folder.Folder{ID: rand.Int63(), UID: uid} @@ -122,13 +120,13 @@ func TestNewFolderIDScopeResolver(t *testing.T) { // nolint:staticcheck scope := "folders:id:" + strconv.FormatInt(db.ID, 10) - resolvedScopes, err := resolver.Resolve(context.Background(), orgId, scope) + resolvedScopes, err := resolver.Resolve(context.Background(), orgID, scope) require.NoError(t, err) require.Len(t, resolvedScopes, 1) require.Equal(t, fmt.Sprintf("folders:uid:%v", db.UID), resolvedScopes[0]) // nolint:staticcheck - folderStore.AssertCalled(t, "GetFolderByID", mock.Anything, orgId, db.ID) + folderStore.AssertCalled(t, "GetFolderByID", mock.Anything, orgID, db.ID) }) t.Run("resolver should should include inherited scopes if any", func(t *testing.T) { folderStore := foldertest.NewFakeFolderStore(t) diff --git a/pkg/services/dashboards/database/database_provisioning_test.go b/pkg/services/dashboards/database/database_provisioning_test.go index 6578eafa500..f794f0883a4 100644 --- a/pkg/services/dashboards/database/database_provisioning_test.go +++ b/pkg/services/dashboards/database/database_provisioning_test.go @@ -25,7 +25,6 @@ func TestIntegrationDashboardProvisioningTest(t *testing.T) { folderCmd := dashboards.SaveDashboardCommand{ OrgID: 1, - FolderID: 0, // nolint:staticcheck FolderUID: "", IsFolder: true, Dashboard: simplejson.NewFromAny(map[string]any{ @@ -67,7 +66,6 @@ func TestIntegrationDashboardProvisioningTest(t *testing.T) { saveCmd := dashboards.SaveDashboardCommand{ OrgID: 1, IsFolder: false, - FolderID: dash.ID, // nolint:staticcheck FolderUID: dash.UID, Dashboard: simplejson.NewFromAny(map[string]any{ "id": nil, diff --git a/pkg/services/dashboards/database/database_test.go b/pkg/services/dashboards/database/database_test.go index e61ee8c5d5e..9d3931021ac 100644 --- a/pkg/services/dashboards/database/database_test.go +++ b/pkg/services/dashboards/database/database_test.go @@ -60,16 +60,14 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { require.Equal(t, savedDash.Slug, "test-dash-23") require.NotEqual(t, savedDash.ID, 0) require.False(t, savedDash.IsFolder) - // nolint:staticcheck - require.Positive(t, savedDash.FolderID) + require.NotEmpty(t, savedDash.FolderUID) require.Positive(t, len(savedDash.UID)) require.Equal(t, savedFolder.Title, "1 test dash folder") require.Equal(t, savedFolder.Slug, "1-test-dash-folder") require.NotEqual(t, savedFolder.ID, 0) require.True(t, savedFolder.IsFolder) - // nolint:staticcheck - require.EqualValues(t, savedFolder.FolderID, 0) + require.Empty(t, savedFolder.FolderUID) require.Positive(t, len(savedFolder.UID)) }) @@ -90,41 +88,6 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { require.False(t, queryResult.IsFolder) }) - t.Run("Should be able to get dashboard by title and folderID", func(t *testing.T) { - setup() - query := dashboards.GetDashboardQuery{ - Title: util.Pointer("test dash 23"), - FolderID: &savedFolder.ID, // nolint:staticcheck - OrgID: 1, - } - - queryResult, err := dashboardStore.GetDashboard(context.Background(), &query) - require.NoError(t, err) - - require.Equal(t, queryResult.Title, "test dash 23") - require.Equal(t, queryResult.Slug, "test-dash-23") - require.Equal(t, queryResult.ID, savedDash.ID) - require.Equal(t, queryResult.UID, savedDash.UID) - require.False(t, queryResult.IsFolder) - }) - - t.Run("Should be able to get dashboard by title and folderUID", func(t *testing.T) { - setup() - query := dashboards.GetDashboardQuery{ - Title: util.Pointer("test dash 23"), - FolderUID: savedFolder.UID, - OrgID: 1, - } - queryResult, err := dashboardStore.GetDashboard(context.Background(), &query) - require.NoError(t, err) - - require.Equal(t, queryResult.Title, "test dash 23") - require.Equal(t, queryResult.Slug, "test-dash-23") - require.Equal(t, queryResult.ID, savedDash.ID) - require.Equal(t, queryResult.UID, savedDash.UID) - require.False(t, queryResult.IsFolder) - }) - t.Run("Should not be able to get dashboard by title alone", func(t *testing.T) { setup() query := dashboards.GetDashboardQuery{ @@ -136,16 +99,21 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { require.ErrorIs(t, err, dashboards.ErrDashboardIdentifierNotSet) }) - t.Run("Folder=0 should not be able to get a dashboard in a folder", func(t *testing.T) { + t.Run("Should be able to get dashboard by title and folderUID", func(t *testing.T) { setup() query := dashboards.GetDashboardQuery{ - Title: util.Pointer("test dash 23"), - FolderID: util.Pointer(int64(0)), // nolint:staticcheck - OrgID: 1, + Title: util.Pointer("test dash 23"), + FolderUID: savedFolder.UID, + OrgID: 1, } + queryResult, err := dashboardStore.GetDashboard(context.Background(), &query) + require.NoError(t, err) - _, err := dashboardStore.GetDashboard(context.Background(), &query) - require.ErrorIs(t, err, dashboards.ErrDashboardNotFound) + require.Equal(t, queryResult.Title, "test dash 23") + require.Equal(t, queryResult.Slug, "test-dash-23") + require.Equal(t, queryResult.ID, savedDash.ID) + require.Equal(t, queryResult.UID, savedDash.UID) + require.False(t, queryResult.IsFolder) }) t.Run("Should be able to get dashboard by uid", func(t *testing.T) { @@ -183,6 +151,18 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { require.Equal(t, err, dashboards.ErrDashboardIdentifierNotSet) }) + t.Run("Folder=0 should not be able to get a dashboard in a folder", func(t *testing.T) { + setup() + query := dashboards.GetDashboardQuery{ + Title: util.Pointer("test dash 23"), + FolderUID: "", + OrgID: 1, + } + + _, err := dashboardStore.GetDashboard(context.Background(), &query) + require.Error(t, err, dashboards.ErrDashboardNotFound) + }) + t.Run("Should be able to get dashboards by IDs & UIDs", func(t *testing.T) { setup() query := dashboards.GetDashboardsQuery{DashboardIDs: []int64{savedDash.ID, savedDash2.ID}} @@ -235,13 +215,12 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { "tags": []interface{}{}, }), Overwrite: true, - FolderID: 2, // nolint:staticcheck + FolderUID: "2", UserID: 100, } dash, err := dashboardStore.SaveDashboard(context.Background(), cmd) require.NoError(t, err) - // nolint:staticcheck - require.EqualValues(t, dash.FolderID, 2) + require.EqualValues(t, dash.FolderUID, "2") cmd = dashboards.SaveDashboardCommand{ OrgID: 1, @@ -250,7 +229,7 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { "title": "folderId", "tags": []interface{}{}, }), - FolderID: 0, // nolint:staticcheck + FolderUID: "", Overwrite: true, UserID: 100, } @@ -264,8 +243,7 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { queryResult, err := dashboardStore.GetDashboard(context.Background(), &query) require.NoError(t, err) - // nolint:staticcheck - require.Equal(t, queryResult.FolderID, int64(0)) + require.Equal(t, queryResult.FolderUID, "") require.Equal(t, queryResult.CreatedBy, savedDash.CreatedBy) require.WithinDuration(t, queryResult.Created, savedDash.Created, 3*time.Second) require.Equal(t, queryResult.UpdatedBy, int64(100)) @@ -415,8 +393,8 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { t.Run("Should be able to find a dashboard folder's children", func(t *testing.T) { setup() query := dashboards.FindPersistedDashboardsQuery{ - OrgId: 1, - FolderIds: []int64{savedFolder.ID}, // nolint:staticcheck + OrgId: 1, + FolderUIDs: []string{savedFolder.UID}, SignedInUser: &user.SignedInUser{ OrgID: 1, OrgRole: org.RoleEditor, @@ -433,8 +411,6 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { hit := hits[0] require.Equal(t, hit.ID, savedDash.ID) require.Equal(t, hit.URL, fmt.Sprintf("/d/%s/%s", savedDash.UID, savedDash.Slug)) - // nolint:staticcheck - require.Equal(t, hit.FolderID, savedFolder.ID) require.Equal(t, hit.FolderUID, savedFolder.UID) require.Equal(t, hit.FolderTitle, savedFolder.Title) require.Equal(t, hit.FolderURL, fmt.Sprintf("/dashboards/f/%s/%s", savedFolder.UID, savedFolder.Slug)) @@ -461,8 +437,6 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { hit := hits[0] require.Equal(t, hit.ID, savedDash.ID) require.Equal(t, hit.URL, fmt.Sprintf("/d/%s/%s", savedDash.UID, savedDash.Slug)) - // nolint:staticcheck - require.Equal(t, hit.FolderID, savedFolder.ID) require.Equal(t, hit.FolderUID, savedFolder.UID) require.Equal(t, hit.FolderTitle, savedFolder.Title) require.Equal(t, hit.FolderURL, fmt.Sprintf("/dashboards/f/%s/%s", savedFolder.UID, savedFolder.Slug)) @@ -888,43 +862,6 @@ func TestIntegrationFindDashboardsByFolder(t *testing.T) { expectedResult map[string][]res typ string }{ - { - desc: "find dashboard under general using folder id", - folderIDs: []int64{0}, // nolint:staticcheck - typ: searchstore.TypeDashboard, - expectedResult: map[string][]res{ - "": {{title: "dashboard under general"}}, - featuremgmt.FlagNestedFolders: {{title: "dashboard under general"}}, - }, - }, - { - desc: "find dashboard under general using folder id", - folderIDs: []int64{0}, // nolint:staticcheck - typ: searchstore.TypeDashboard, - expectedResult: map[string][]res{ - "": {{title: "dashboard under general"}}, - featuremgmt.FlagNestedFolders: {{title: "dashboard under general"}}, - }, - }, - { - desc: "find dashboard under f0 using folder id", - folderIDs: []int64{f0.ID}, // nolint:staticcheck - typ: searchstore.TypeDashboard, - expectedResult: map[string][]res{ - "": {{title: "dashboard under f0", folderUID: f0.UID, folderTitle: f0.Title}}, - }, - }, - { - desc: "find dashboard under f0 or f1 using folder id", - folderIDs: []int64{f0.ID, f1.ID}, // nolint:staticcheck - typ: searchstore.TypeDashboard, - expectedResult: map[string][]res{ - "": {{title: "dashboard under f0", folderUID: f0.UID, folderTitle: f0.Title}, - {title: "dashboard under f1", folderUID: f1.UID, folderTitle: f1.Title}}, - featuremgmt.FlagNestedFolders: {{title: "dashboard under f0", folderUID: f0.UID, folderTitle: f0.Title}, - {title: "dashboard under f1", folderUID: f1.UID, folderTitle: f1.Title}}, - }, - }, { desc: "find dashboard under general using folder UID", folderUIDs: []string{folder.GeneralFolderUID}, @@ -963,30 +900,6 @@ func TestIntegrationFindDashboardsByFolder(t *testing.T) { {title: "dashboard under f1", folderUID: f1.UID, folderTitle: f1.Title}}, }, }, - { - desc: "find dashboard under general or f0 using folder id", - folderIDs: []int64{0, f0.ID}, // nolint:staticcheck - typ: searchstore.TypeDashboard, - expectedResult: map[string][]res{ - "": {{title: "dashboard under f0", folderUID: f0.UID, folderTitle: f0.Title}, - {title: "dashboard under general"}}, - featuremgmt.FlagNestedFolders: {{title: "dashboard under f0", folderUID: f0.UID, folderTitle: f0.Title}, - {title: "dashboard under general"}}, - }, - }, - { - desc: "find dashboard under general or f0 or f1 using folder id", - folderIDs: []int64{0, f0.ID, f1.ID}, // nolint:staticcheck - typ: searchstore.TypeDashboard, - expectedResult: map[string][]res{ - "": {{title: "dashboard under f0", folderUID: f0.UID, folderTitle: f0.Title}, - {title: "dashboard under f1", folderUID: f1.UID, folderTitle: f1.Title}, - {title: "dashboard under general"}}, - featuremgmt.FlagNestedFolders: {{title: "dashboard under f0", folderUID: f0.UID, folderTitle: f0.Title}, - {title: "dashboard under f1", folderUID: f1.UID, folderTitle: f1.Title}, - {title: "dashboard under general"}}, - }, - }, { desc: "find dashboard under general or f0 using folder UID", folderUIDs: []string{folder.GeneralFolderUID, f0.UID}, @@ -1030,7 +943,6 @@ func TestIntegrationFindDashboardsByFolder(t *testing.T) { res, err := dashboardStore.FindDashboards(context.Background(), &dashboards.FindPersistedDashboardsQuery{ SignedInUser: user, Type: tc.typ, - FolderIds: tc.folderIDs, // nolint:staticcheck FolderUIDs: tc.folderUIDs, }) require.NoError(t, err) @@ -1199,14 +1111,12 @@ func makeQueryResult(query *dashboards.FindPersistedDashboardsQuery, res []dashb URI: "db/" + item.Slug, URL: dashboards.GetDashboardFolderURL(item.IsFolder, item.UID, item.Slug), Type: hitType, - FolderID: item.FolderID, // nolint:staticcheck FolderUID: item.FolderUID, FolderTitle: item.FolderTitle, Tags: []string{}, } - // nolint:staticcheck - if item.FolderID > 0 { + if item.FolderUID != "" { hit.FolderURL = dashboards.GetFolderURL(item.FolderUID, item.FolderSlug) } diff --git a/pkg/services/dashboards/models_test.go b/pkg/services/dashboards/models_test.go index 87bb644069d..06aeb1eba60 100644 --- a/pkg/services/dashboards/models_test.go +++ b/pkg/services/dashboards/models_test.go @@ -63,12 +63,10 @@ func TestSaveDashboardCommand_GetDashboardModel(t *testing.T) { json := simplejson.New() json.Set("title", "test dash") - // nolint:staticcheck - cmd := &SaveDashboardCommand{Dashboard: json, FolderID: 1, FolderUID: "1"} + cmd := &SaveDashboardCommand{Dashboard: json, FolderUID: "1"} dash := cmd.GetDashboardModel() - // nolint:staticcheck - assert.Equal(t, int64(1), dash.FolderID) + assert.Equal(t, "1", dash.FolderUID) }) } diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index 8291f8474ea..b4316dd9fcb 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -52,14 +52,6 @@ func TestDashboardService(t *testing.T) { } }) - t.Run("Should return validation error if it's a folder and have a folder id", func(t *testing.T) { - dto.Dashboard = dashboards.NewDashboardFolder("Folder") - // nolint:staticcheck - dto.Dashboard.FolderID = 1 - _, err := service.SaveDashboard(context.Background(), dto, false) - require.Equal(t, err, dashboards.ErrDashboardFolderCannotHaveParent) - }) - t.Run("Should return validation error if folder is named General", func(t *testing.T) { dto.Dashboard = dashboards.NewDashboardFolder("General") _, err := service.SaveDashboard(context.Background(), dto, false) @@ -234,8 +226,7 @@ func TestDashboardService(t *testing.T) { t.Run("Count dashboards in folder", func(t *testing.T) { fakeStore.On("CountDashboardsInFolder", mock.Anything, mock.AnythingOfType("*dashboards.CountDashboardsInFolderRequest")).Return(int64(3), nil) - // nolint:staticcheck - folderSvc.ExpectedFolder = &folder.Folder{ID: 1} + folderSvc.ExpectedFolder = &folder.Folder{UID: "i am a folder"} // set up a ctx with signed in user usr := &user.SignedInUser{UserID: 1} ctx := appcontext.WithUser(context.Background(), usr) diff --git a/pkg/services/folder/folderimpl/dashboard_folder_store_test.go b/pkg/services/folder/folderimpl/dashboard_folder_store_test.go index 997714c1a23..4f433e6af99 100644 --- a/pkg/services/folder/folderimpl/dashboard_folder_store_test.go +++ b/pkg/services/folder/folderimpl/dashboard_folder_store_test.go @@ -44,8 +44,7 @@ func TestIntegrationDashboardFolderStore(t *testing.T) { t.Run("GetFolderByTitle should find the folder", func(t *testing.T) { result, err := folderStore.GetFolderByTitle(context.Background(), orgId, title) require.NoError(t, err) - // nolint:staticcheck - require.Equal(t, folder1.ID, result.ID) + require.Equal(t, folder1.UID, result.UID) }) }) @@ -58,8 +57,7 @@ func TestIntegrationDashboardFolderStore(t *testing.T) { t.Run("should return folder by UID", func(t *testing.T) { d, err := folderStore.GetFolderByUID(context.Background(), orgId, folder.UID) - // nolint:staticcheck - require.Equal(t, folder.ID, d.ID) + require.Equal(t, folder.UID, d.UID) require.NoError(t, err) }) t.Run("should not find dashboard", func(t *testing.T) { @@ -83,8 +81,7 @@ func TestIntegrationDashboardFolderStore(t *testing.T) { t.Run("should return folder by ID", func(t *testing.T) { d, err := folderStore.GetFolderByID(context.Background(), orgId, folder.ID) - // nolint:staticcheck - require.Equal(t, folder.ID, d.ID) + require.Equal(t, folder.UID, d.UID) require.NoError(t, err) }) t.Run("should not find dashboard", func(t *testing.T) { diff --git a/pkg/services/folder/folderimpl/folder_test.go b/pkg/services/folder/folderimpl/folder_test.go index ff4c643002f..28f2c3d9120 100644 --- a/pkg/services/folder/folderimpl/folder_test.go +++ b/pkg/services/folder/folderimpl/folder_test.go @@ -106,38 +106,22 @@ func TestIntegrationFolderService(t *testing.T) { origNewGuardian := guardian.New guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{}) - folderId := rand.Int63() folderUID := util.GenerateShortUID() f := folder.NewFolder("Folder", "") - // nolint:staticcheck - f.ID = folderId f.UID = folderUID - folderStore.On("GetFolderByID", mock.Anything, orgID, folderId).Return(f, nil) folderStore.On("GetFolderByUID", mock.Anything, orgID, folderUID).Return(f, nil) t.Run("When get folder by id should return access denied error", func(t *testing.T) { _, err := service.Get(context.Background(), &folder.GetFolderQuery{ - ID: &folderId, // nolint:staticcheck + UID: &folderUID, OrgID: orgID, SignedInUser: usr, }) require.Equal(t, err, dashboards.ErrFolderAccessDenied) }) - var zeroInt int64 = 0 - t.Run("When get folder by id, with id = 0 should return default folder", func(t *testing.T) { - foldr, err := service.Get(context.Background(), &folder.GetFolderQuery{ - ID: &zeroInt, // nolint:staticcheck - OrgID: orgID, - SignedInUser: usr, - }) - require.NoError(t, err) - // nolint:staticcheck - require.Equal(t, foldr, &folder.Folder{ID: 0, Title: "General"}) - }) - t.Run("When get folder by uid should return access denied error", func(t *testing.T) { _, err := service.Get(context.Background(), &folder.GetFolderQuery{ UID: &folderUID, @@ -176,7 +160,6 @@ func TestIntegrationFolderService(t *testing.T) { newFolder := folder.NewFolder("Folder", "") newFolder.UID = folderUID - folderStore.On("GetFolderByID", mock.Anything, orgID, folderId).Return(newFolder, nil) folderStore.On("GetFolderByUID", mock.Anything, orgID, folderUID).Return(newFolder, nil) err := service.Delete(context.Background(), &folder.DeleteFolderCommand{ @@ -267,8 +250,6 @@ func TestIntegrationFolderService(t *testing.T) { t.Run("When deleting folder by uid should not return access denied error", func(t *testing.T) { f := folder.NewFolder(util.GenerateShortUID(), "") - // nolint:staticcheck - f.ID = rand.Int63() f.UID = util.GenerateShortUID() folderStore.On("GetFolders", mock.Anything, orgID, []string{f.UID}).Return(map[string]*folder.Folder{f.UID: f}, nil) folderStore.On("GetFolderByUID", mock.Anything, orgID, f.UID).Return(f, nil) @@ -287,8 +268,6 @@ func TestIntegrationFolderService(t *testing.T) { }) require.NoError(t, err) require.NotNil(t, actualCmd) - // nolint:staticcheck - require.Equal(t, f.ID, actualCmd.ID) require.Equal(t, orgID, actualCmd.OrgID) require.Equal(t, expectedForceDeleteRules, actualCmd.ForceDeleteFolderRules) }) @@ -302,20 +281,6 @@ func TestIntegrationFolderService(t *testing.T) { origNewGuardian := guardian.New guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanViewValue: true}) - t.Run("When get folder by id should return folder", func(t *testing.T) { - expected := folder.NewFolder(util.GenerateShortUID(), "") - // nolint:staticcheck - expected.ID = rand.Int63() - - // nolint:staticcheck - folderStore.On("GetFolderByID", mock.Anything, orgID, expected.ID).Return(expected, nil) - - // nolint:staticcheck - actual, err := service.getFolderByID(context.Background(), expected.ID, orgID) - require.Equal(t, expected, actual) - require.NoError(t, err) - }) - t.Run("When get folder by uid should return folder", func(t *testing.T) { expected := folder.NewFolder(util.GenerateShortUID(), "") expected.UID = util.GenerateShortUID() @@ -1384,8 +1349,6 @@ func CreateSubtreeInStore(t *testing.T, store *sqlStore, service *Service, depth f, err := service.Create(context.Background(), &cmd) require.NoError(t, err) require.Equal(t, title, f.Title) - // nolint:staticcheck - require.NotEmpty(t, f.ID) require.NotEmpty(t, f.UID) parents, err := store.GetParents(context.Background(), folder.GetParentsQuery{ diff --git a/pkg/services/folder/folderimpl/sqlstore_test.go b/pkg/services/folder/folderimpl/sqlstore_test.go index bd711630f78..9b81f92df00 100644 --- a/pkg/services/folder/folderimpl/sqlstore_test.go +++ b/pkg/services/folder/folderimpl/sqlstore_test.go @@ -70,8 +70,7 @@ func TestIntegrationCreate(t *testing.T) { assert.Equal(t, folderTitle, f.Title) assert.Equal(t, folderDsc, f.Description) - // nolint:staticcheck - assert.NotEmpty(t, f.ID) + assert.NotEmpty(t, f.UID) assert.Equal(t, uid, f.UID) assert.Empty(t, f.ParentUID) assert.NotEmpty(t, f.URL) @@ -98,8 +97,7 @@ func TestIntegrationCreate(t *testing.T) { }) require.NoError(t, err) require.Equal(t, "parent", parent.Title) - // nolint:staticcheck - require.NotEmpty(t, parent.ID) + require.NotEmpty(t, parent.UID) assert.Equal(t, parentUID, parent.UID) assert.NotEmpty(t, parent.URL) @@ -125,8 +123,7 @@ func TestIntegrationCreate(t *testing.T) { assert.Equal(t, folderTitle, f.Title) assert.Equal(t, folderDsc, f.Description) - // nolint:staticcheck - assert.NotEmpty(t, f.ID) + assert.NotEmpty(t, f.UID) assert.Equal(t, uid, f.UID) assert.Equal(t, parentUID, f.ParentUID) assert.NotEmpty(t, f.URL) @@ -404,8 +401,6 @@ func TestIntegrationGet(t *testing.T) { OrgID: orgID, }) require.NoError(t, err) - // nolint:staticcheck - assert.Equal(t, f.ID, ff.ID) assert.Equal(t, f.UID, ff.UID) assert.Equal(t, f.OrgID, ff.OrgID) assert.Equal(t, f.Title, ff.Title) @@ -422,8 +417,6 @@ func TestIntegrationGet(t *testing.T) { OrgID: orgID, }) require.NoError(t, err) - // nolint:staticcheck - assert.Equal(t, f.ID, ff.ID) assert.Equal(t, f.UID, ff.UID) assert.Equal(t, f.OrgID, ff.OrgID) assert.Equal(t, f.Title, ff.Title) @@ -436,11 +429,10 @@ func TestIntegrationGet(t *testing.T) { t.Run("get folder by title should succeed", func(t *testing.T) { ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{ - ID: &f.ID, // nolint:staticcheck + UID: &f.UID, + OrgID: orgID, }) require.NoError(t, err) - // nolint:staticcheck - assert.Equal(t, f.ID, ff.ID) assert.Equal(t, f.UID, ff.UID) assert.Equal(t, f.OrgID, ff.OrgID) assert.Equal(t, f.Title, ff.Title) @@ -777,8 +769,7 @@ func TestIntegrationGetFolders(t *testing.T) { }) assert.NotEqual(t, -1, folderInResponseIdx) rf := ff[folderInResponseIdx] - // nolint:staticcheck - assert.Equal(t, f.ID, rf.ID) + assert.Equal(t, f.UID, rf.UID) assert.Equal(t, f.OrgID, rf.OrgID) assert.Equal(t, f.Title, rf.Title) assert.Equal(t, f.Description, rf.Description) @@ -822,8 +813,6 @@ func CreateSubtree(t *testing.T, store *sqlStore, orgID int64, parentUID string, f, err := store.Create(context.Background(), cmd) require.NoError(t, err) require.Equal(t, title, f.Title) - // nolint:staticcheck - require.NotEmpty(t, f.ID) require.NotEmpty(t, f.UID) parents, err := store.GetParents(context.Background(), folder.GetParentsQuery{ diff --git a/pkg/services/ngalert/api/api_ruler_validation_test.go b/pkg/services/ngalert/api/api_ruler_validation_test.go index 5c7a1fefa60..cba6b194d70 100644 --- a/pkg/services/ngalert/api/api_ruler_validation_test.go +++ b/pkg/services/ngalert/api/api_ruler_validation_test.go @@ -85,7 +85,6 @@ func validGroup(cfg *setting.UnifiedAlertingSettings, rules ...apimodels.Postabl func randFolder() *folder.Folder { return &folder.Folder{ - ID: rand.Int63(), // nolint:staticcheck UID: util.GenerateShortUID(), Title: "TEST-FOLDER-" + util.GenerateShortUID(), // URL: "", diff --git a/pkg/services/ngalert/migration/migration_test.go b/pkg/services/ngalert/migration/migration_test.go index 68f60863584..af4abae4dd6 100644 --- a/pkg/services/ngalert/migration/migration_test.go +++ b/pkg/services/ngalert/migration/migration_test.go @@ -1110,10 +1110,9 @@ func TestDashAlertQueryMigration(t *testing.T) { }), }, expectedFolder: &dashboards.Dashboard{ - OrgID: 2, - Title: "General Alerting", - FolderID: 0, // nolint:staticcheck - Slug: "general-alerting", + OrgID: 2, + Title: "General Alerting", + Slug: "general-alerting", }, expected: map[int64][]*ngModels.AlertRule{ int64(2): { @@ -1227,8 +1226,7 @@ func TestDashAlertQueryMigration(t *testing.T) { folder := getDashboard(t, x, orgId, r.NamespaceUID) require.Equal(t, tt.expectedFolder.Title, folder.Title) require.Equal(t, tt.expectedFolder.OrgID, folder.OrgID) - // nolint:staticcheck - require.Equal(t, tt.expectedFolder.FolderID, folder.FolderID) + require.Equal(t, tt.expectedFolder.FolderUID, folder.FolderUID) } } diff --git a/pkg/services/ngalert/ngalert_test.go b/pkg/services/ngalert/ngalert_test.go index b7844f1fe18..b717f562cb1 100644 --- a/pkg/services/ngalert/ngalert_test.go +++ b/pkg/services/ngalert/ngalert_test.go @@ -26,7 +26,6 @@ import ( func Test_subscribeToFolderChanges(t *testing.T) { orgID := rand.Int63() folder := &folder.Folder{ - ID: 0, // nolint:staticcheck UID: util.GenerateShortUID(), Title: "Folder" + util.GenerateShortUID(), } @@ -42,7 +41,6 @@ func Test_subscribeToFolderChanges(t *testing.T) { err := bus.Publish(context.Background(), &events.FolderTitleUpdated{ Timestamp: time.Now(), Title: "Folder" + util.GenerateShortUID(), - ID: folder.ID, // nolint:staticcheck UID: folder.UID, OrgID: orgID, }) diff --git a/pkg/services/ngalert/store/deltas_test.go b/pkg/services/ngalert/store/deltas_test.go index cba46b399c2..8d29c5db099 100644 --- a/pkg/services/ngalert/store/deltas_test.go +++ b/pkg/services/ngalert/store/deltas_test.go @@ -457,7 +457,6 @@ func withUIDs(uids map[string]*models.AlertRule) func(rule *models.AlertRule) { func randFolder() *folder.Folder { return &folder.Folder{ - ID: rand.Int63(), // nolint:staticcheck UID: util.GenerateShortUID(), Title: "TEST-FOLDER-" + util.GenerateShortUID(), URL: "", diff --git a/pkg/services/publicdashboards/api/query_test.go b/pkg/services/publicdashboards/api/query_test.go index d6efc69b738..7371d036591 100644 --- a/pkg/services/publicdashboards/api/query_test.go +++ b/pkg/services/publicdashboards/api/query_test.go @@ -276,7 +276,6 @@ func TestIntegrationUnauthenticatedUserCanGetPubdashPanelQueryData(t *testing.T) // Create Dashboard saveDashboardCmd := dashboards.SaveDashboardCommand{ OrgID: 1, - FolderID: 1, // nolint:staticcheck FolderUID: "1", IsFolder: false, Dashboard: simplejson.NewFromAny(map[string]any{ diff --git a/pkg/services/publicdashboards/database/database_test.go b/pkg/services/publicdashboards/database/database_test.go index 03f826afa4f..43f43bcf9d4 100644 --- a/pkg/services/publicdashboards/database/database_test.go +++ b/pkg/services/publicdashboards/database/database_test.go @@ -763,7 +763,7 @@ func TestGetDashboardByFolder(t *testing.T) { pubdashStore := ProvideStore(sqlStore, cfg, featuremgmt.WithFeatures()) dashboard := insertTestDashboard(t, dashboardStore, "title", 1, 1, "1", true, PublicShareType) pubdash := insertPublicDashboard(t, pubdashStore, dashboard.UID, dashboard.OrgID, true, PublicShareType) - dashboard2 := insertTestDashboard(t, dashboardStore, "title", 1, 2, "2", true, PublicShareType) + dashboard2 := insertTestDashboard(t, dashboardStore, "title2", 1, 2, "2", true, PublicShareType) _ = insertPublicDashboard(t, pubdashStore, dashboard2.UID, dashboard2.OrgID, true, PublicShareType) pubdashes, err := pubdashStore.FindByDashboardFolder(context.Background(), dashboard) @@ -838,12 +838,12 @@ func TestGetMetrics(t *testing.T) { } // helper function to insert a dashboard -func insertTestDashboard(t *testing.T, dashboardStore dashboards.Store, title string, orgId int64, - folderId int64, folderUID string, isFolder bool, tags ...any) *dashboards.Dashboard { +func insertTestDashboard(t *testing.T, dashboardStore dashboards.Store, title string, orgID int64, + folderID int64, folderUID string, isFolder bool, tags ...any) *dashboards.Dashboard { t.Helper() cmd := dashboards.SaveDashboardCommand{ - OrgID: orgId, - FolderID: folderId, // nolint:staticcheck + OrgID: orgID, + FolderID: folderID, // nolint:staticcheck FolderUID: folderUID, IsFolder: isFolder, Dashboard: simplejson.NewFromAny(map[string]any{ diff --git a/pkg/services/searchV2/service_bench_test.go b/pkg/services/searchV2/service_bench_test.go index bd5d95df44d..a6b503c1bcd 100644 --- a/pkg/services/searchV2/service_bench_test.go +++ b/pkg/services/searchV2/service_bench_test.go @@ -134,16 +134,16 @@ func populateDB(folderCount, dashboardsPerFolder int, sqlStore *sqlstore.SQLStor for u := start; u < end; u++ { dashID := int64(u + offset) - folderID := int64((u+offset)%folderCount + 1) + folderUID := fmt.Sprintf("folder%v", int64((u+offset)%folderCount+1)) dbs = append(dbs, dashboards.Dashboard{ - ID: dashID, - UID: fmt.Sprintf("dashboard%v", dashID), - Title: fmt.Sprintf("dashboard%v", dashID), - IsFolder: false, - FolderID: folderID, // nolint:staticcheck - OrgID: 1, - Created: now, - Updated: now, + ID: dashID, + UID: fmt.Sprintf("dashboard%v", dashID), + Title: fmt.Sprintf("dashboard%v", dashID), + IsFolder: false, + FolderUID: folderUID, + OrgID: 1, + Created: now, + Updated: now, }) } diff --git a/pkg/services/sqlstore/permissions/dashboards_bench_test.go b/pkg/services/sqlstore/permissions/dashboards_bench_test.go index dd53cbf51b1..28083e5cdd8 100644 --- a/pkg/services/sqlstore/permissions/dashboards_bench_test.go +++ b/pkg/services/sqlstore/permissions/dashboards_bench_test.go @@ -125,15 +125,15 @@ func setupBenchMark(b *testing.B, usr user.SignedInUser, features featuremgmt.Fe str := fmt.Sprintf("dashboard under folder %s", leaf.Title) now := time.Now() dashes = append(dashes, dashboards.Dashboard{ - OrgID: usr.OrgID, - IsFolder: false, - UID: str, - Slug: str, - Title: str, - Data: simplejson.New(), - Created: now, - Updated: now, - FolderID: leaf.ID, // nolint:staticcheck + OrgID: usr.OrgID, + IsFolder: false, + UID: str, + Slug: str, + Title: str, + Data: simplejson.New(), + Created: now, + Updated: now, + FolderUID: leaf.UID, }) } @@ -177,8 +177,7 @@ func setupBenchMark(b *testing.B, usr user.SignedInUser, features featuremgmt.Fe }) for _, dash := range dashes { // add permission to read dashboards under the general - // nolint:staticcheck - if dash.FolderID == 0 { + if dash.FolderUID == "" { permissions = append(permissions, accesscontrol.Permission{ RoleID: int64(i), Action: dashboards.ActionDashboardsRead,