From 3a2e96b0dbe6a26ed340764957c141834bb6aef0 Mon Sep 17 00:00:00 2001 From: Kat Yang <69819079+yangkb09@users.noreply.github.com> Date: Wed, 15 Nov 2023 10:28:50 -0500 Subject: [PATCH] Chore: Deprecate FolderID from Dashboard (#77823) * Chore: Deprecate FolderID from Dashboard * chore: add two missing nolint comments --- pkg/api/dashboard.go | 2 ++ pkg/api/dashboard_permission.go | 2 +- pkg/api/dashboard_test.go | 5 +++++ pkg/api/folder_bench_test.go | 6 +++--- .../ossaccesscontrol/permissions_services.go | 1 + .../dashboardimport/service/service_test.go | 6 ++++-- pkg/services/dashboards/accesscontrol.go | 2 ++ pkg/services/dashboards/accesscontrol_test.go | 6 ++++++ pkg/services/dashboards/database/database.go | 5 +++++ pkg/services/dashboards/database/database_test.go | 5 +++++ pkg/services/dashboards/models.go | 3 +++ pkg/services/dashboards/models_test.go | 2 ++ .../dashboards/service/dashboard_service.go | 7 ++++++- .../service/dashboard_service_integration_test.go | 14 ++++++++------ .../dashboards/service/dashboard_service_test.go | 1 + .../folder/folderimpl/dashboard_folder_store.go | 3 +++ pkg/services/folder/folderimpl/folder.go | 5 ++++- .../guardian/accesscontrol_guardian_test.go | 15 ++++++++------- .../libraryelements/libraryelements_test.go | 1 + pkg/services/librarypanels/librarypanels_test.go | 1 + pkg/services/ngalert/migration/migration_test.go | 7 ++++--- pkg/services/ngalert/migration/permissions.go | 8 ++++++++ .../provisioning/dashboards/file_reader.go | 3 +++ pkg/services/provisioning/dashboards/types.go | 1 + .../publicdashboards/service/service_test.go | 1 + pkg/services/searchV2/service_bench_test.go | 2 +- .../sqlstore/permissions/dashboard_test.go | 2 +- .../sqlstore/permissions/dashboards_bench_test.go | 3 ++- 28 files changed, 92 insertions(+), 27 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 3567411c464..1d76407e7cb 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -172,7 +172,9 @@ func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response } // lookup folder title + // nolint:staticcheck if dash.FolderID > 0 { + // nolint:staticcheck query := dashboards.GetDashboardQuery{ID: dash.FolderID, OrgID: c.SignedInUser.GetOrgID()} queryResult, err := hs.DashboardService.GetDashboard(c.Req.Context(), &query) if err != nil { diff --git a/pkg/api/dashboard_permission.go b/pkg/api/dashboard_permission.go index adf739148c8..22756e12430 100644 --- a/pkg/api/dashboard_permission.go +++ b/pkg/api/dashboard_permission.go @@ -195,7 +195,7 @@ func (hs *HTTPServer) getDashboardACL(ctx context.Context, user identity.Request acl = append(acl, &dashboards.DashboardACLInfoDTO{ OrgID: dashboard.OrgID, DashboardID: dashboard.ID, - FolderID: dashboard.FolderID, + FolderID: dashboard.FolderID, // nolint:staticcheck Created: p.Created, Updated: p.Updated, UserID: p.UserId, diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index a19b2b519c7..fc41a360cf2 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -379,11 +379,13 @@ func TestDashboardAPIEndpoint(t *testing.T) { t.Run("Given two dashboards with the same title in different folders", func(t *testing.T) { dashOne := dashboards.NewDashboard("dash") dashOne.ID = 2 + // nolint:staticcheck dashOne.FolderID = 1 dashOne.HasACL = false dashTwo := dashboards.NewDashboard("dash") dashTwo.ID = 4 + // nolint:staticcheck dashTwo.FolderID = 3 dashTwo.HasACL = false }) @@ -411,6 +413,7 @@ func TestDashboardAPIEndpoint(t *testing.T) { } dashboardService := dashboards.NewFakeDashboardService(t) + // nolint:staticcheck dashboardService.On("SaveDashboard", mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"), mock.AnythingOfType("bool")). Return(&dashboards.Dashboard{ID: dashID, UID: "uid", Title: "Dash", Slug: "dash", Version: 2, FolderUID: folderUID, FolderID: folderID}, nil) mockFolderService := &foldertest.FakeService{ @@ -646,6 +649,7 @@ func TestDashboardAPIEndpoint(t *testing.T) { const folderID int64 = 1 fakeDash := dashboards.NewDashboard("Child dash") fakeDash.ID = 2 + // nolint:staticcheck fakeDash.FolderID = folderID fakeDash.HasACL = false @@ -780,6 +784,7 @@ func TestDashboardAPIEndpoint(t *testing.T) { func TestDashboardVersionsAPIEndpoint(t *testing.T) { fakeDash := dashboards.NewDashboard("Child dash") fakeDash.ID = 1 + // nolint:staticcheck fakeDash.FolderID = 1 fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake() diff --git a/pkg/api/folder_bench_test.go b/pkg/api/folder_bench_test.go index 56ffb87d614..02b1137855c 100644 --- a/pkg/api/folder_bench_test.go +++ b/pkg/api/folder_bench_test.go @@ -346,7 +346,7 @@ func setupDB(b testing.TB) benchScenario { OrgID: signedInUser.OrgID, IsFolder: false, UID: str, - FolderID: f0.ID, + FolderID: f0.ID, // nolint:staticcheck Slug: str, Title: str, Data: simplejson.New(), @@ -373,7 +373,7 @@ func setupDB(b testing.TB) benchScenario { OrgID: signedInUser.OrgID, IsFolder: false, UID: str, - FolderID: f1.ID, + FolderID: f1.ID, // nolint:staticcheck Slug: str, Title: str, Data: simplejson.New(), @@ -400,7 +400,7 @@ func setupDB(b testing.TB) benchScenario { OrgID: signedInUser.OrgID, IsFolder: false, UID: str, - FolderID: f2.ID, + FolderID: f2.ID, // nolint:staticcheck Slug: str, Title: str, Data: simplejson.New(), diff --git a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go index feb864e6d91..7841365e725 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go @@ -149,6 +149,7 @@ func ProvideDashboardPermissions( if err != nil { return nil, err } + // nolint:staticcheck if dashboard.FolderID > 0 { query := &dashboards.GetDashboardQuery{ID: dashboard.FolderID, OrgID: orgID} queryResult, err := dashboardStore.GetDashboard(ctx, query) diff --git a/pkg/services/dashboardimport/service/service_test.go b/pkg/services/dashboardimport/service/service_test.go index 13e9de19735..bdd5c48a14c 100644 --- a/pkg/services/dashboardimport/service/service_test.go +++ b/pkg/services/dashboardimport/service/service_test.go @@ -37,7 +37,7 @@ func TestImportDashboardService(t *testing.T) { OrgID: 3, Version: dto.Dashboard.Version, PluginID: "prometheus", - FolderID: dto.Dashboard.FolderID, + FolderID: dto.Dashboard.FolderID, // nolint:staticcheck Title: dto.Dashboard.Title, Data: dto.Dashboard.Data, }, nil @@ -91,6 +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) panel := importDashboardArg.Dashboard.Data.Get("panels").GetIndex(0) @@ -112,7 +113,7 @@ func TestImportDashboardService(t *testing.T) { OrgID: 3, Version: dto.Dashboard.Version, PluginID: "prometheus", - FolderID: dto.Dashboard.FolderID, + FolderID: dto.Dashboard.FolderID, // nolint:staticcheck Title: dto.Dashboard.Title, Data: dto.Dashboard.Data, }, nil @@ -158,6 +159,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) panel := importDashboardArg.Dashboard.Data.Get("panels").GetIndex(0) diff --git a/pkg/services/dashboards/accesscontrol.go b/pkg/services/dashboards/accesscontrol.go index 3b798fddb4d..61c19267e53 100644 --- a/pkg/services/dashboards/accesscontrol.go +++ b/pkg/services/dashboards/accesscontrol.go @@ -166,10 +166,12 @@ func NewDashboardUIDScopeResolver(folderDB folder.FolderStore, ds DashboardServi func resolveDashboardScope(ctx context.Context, folderDB folder.FolderStore, orgID int64, dashboard *Dashboard, folderSvc folder.Service) ([]string, error) { var folderUID string + // nolint:staticcheck if dashboard.FolderID < 0 { return []string{ScopeDashboardsProvider.GetResourceScopeUID(dashboard.UID)}, nil } + // nolint:staticcheck if dashboard.FolderID == 0 { folderUID = ac.GeneralFolderUID } else { diff --git a/pkg/services/dashboards/accesscontrol_test.go b/pkg/services/dashboards/accesscontrol_test.go index 267778d0cbd..c7463e62c4c 100644 --- a/pkg/services/dashboards/accesscontrol_test.go +++ b/pkg/services/dashboards/accesscontrol_test.go @@ -212,6 +212,7 @@ func TestNewDashboardIDScopeResolver(t *testing.T) { orgID := rand.Int63() folder := &folder.Folder{ID: 2, UID: "2"} + // nolint:staticcheck dashboard := &Dashboard{ID: 1, FolderID: folder.ID, UID: "1"} dashSvc.On("G") folderStore.On("GetFolderByID", mock.Anything, orgID, folder.ID).Return(folder, nil).Once() @@ -240,6 +241,7 @@ func TestNewDashboardIDScopeResolver(t *testing.T) { orgID := rand.Int63() folder := &folder.Folder{ID: 2, UID: "2"} + // nolint:staticcheck dashboard := &Dashboard{ID: 1, FolderID: folder.ID, UID: "1"} dashSvc.On("GetDashboard", mock.Anything, mock.Anything).Return(dashboard, nil).Once() @@ -270,6 +272,7 @@ func TestNewDashboardIDScopeResolver(t *testing.T) { dashSvc := &FakeDashboardService{} _, resolver := NewDashboardIDScopeResolver(foldertest.NewFakeFolderStore(t), dashSvc, foldertest.NewFakeService()) + // nolint:staticcheck dashboard := &Dashboard{ID: 1, FolderID: 0, UID: "1"} dashSvc.On("GetDashboard", mock.Anything, mock.Anything).Return(dashboard, nil) resolved, err := resolver.Resolve(context.Background(), 1, ac.Scope("dashboards", "id", "1")) @@ -294,6 +297,7 @@ func TestNewDashboardUIDScopeResolver(t *testing.T) { orgID := rand.Int63() folder := &folder.Folder{ID: 2, UID: "2"} + // nolint:staticcheck dashboard := &Dashboard{ID: 1, FolderID: folder.ID, UID: "1"} dashSvc.On("GetDashboard", mock.Anything, mock.Anything).Return(dashboard, nil).Once() @@ -322,6 +326,7 @@ func TestNewDashboardUIDScopeResolver(t *testing.T) { orgID := rand.Int63() folder := &folder.Folder{ID: 2, UID: "2"} + // nolint:staticcheck dashboard := &Dashboard{ID: 1, FolderID: folder.ID, UID: "1"} dashSvc.On("GetDashboard", mock.Anything, mock.Anything).Return(dashboard, nil).Once() folderStore.On("GetFolderByID", mock.Anything, mock.Anything, mock.Anything).Return(folder, nil).Once() @@ -351,6 +356,7 @@ func TestNewDashboardUIDScopeResolver(t *testing.T) { service := &FakeDashboardService{} _, resolver := NewDashboardUIDScopeResolver(foldertest.NewFakeFolderStore(t), service, foldertest.NewFakeService()) + // nolint:staticcheck dashboard := &Dashboard{ID: 1, FolderID: 0, UID: "1"} service.On("GetDashboard", mock.Anything, mock.Anything).Return(dashboard, nil) resolved, err := resolver.Resolve(context.Background(), 1, ac.Scope("dashboards", "uid", "1")) diff --git a/pkg/services/dashboards/database/database.go b/pkg/services/dashboards/database/database.go index 2b52dae49fa..f7d237eeb19 100644 --- a/pkg/services/dashboards/database/database.go +++ b/pkg/services/dashboards/database/database.go @@ -330,6 +330,7 @@ func getExistingDashboardByIDOrUIDForUpdate(sess *db.Session, dash *dashboards.D } } + // nolint:staticcheck if dash.FolderID > 0 { var existingFolder dashboards.Dashboard folderExists, err := sess.Where("org_id=? AND id=? AND is_folder=?", dash.OrgID, dash.FolderID, @@ -364,6 +365,7 @@ func getExistingDashboardByIDOrUIDForUpdate(sess *db.Session, dash *dashboards.D return isParentFolderChanged, dashboards.ErrDashboardTypeMismatch } + // nolint:staticcheck if !dash.IsFolder && dash.FolderID != existing.FolderID { isParentFolderChanged = true } @@ -389,6 +391,7 @@ func getExistingDashboardByIDOrUIDForUpdate(sess *db.Session, dash *dashboards.D func getExistingDashboardByTitleAndFolder(sess *db.Session, dash *dashboards.Dashboard, dialect migrator.Dialect, overwrite, isParentFolderChanged bool) (bool, error) { var existing dashboards.Dashboard + // nolint:staticcheck exists, err := sess.Where("org_id=? AND title=? AND (is_folder=? OR folder_id=?)", dash.OrgID, dash.Title, dialect.BooleanStr(true), dash.FolderID).Get(&existing) if err != nil { @@ -403,6 +406,7 @@ func getExistingDashboardByTitleAndFolder(sess *db.Session, dash *dashboards.Das return isParentFolderChanged, dashboards.ErrDashboardFolderWithSameNameAsDashboard } + // nolint:staticcheck if !dash.IsFolder && (dash.FolderID != existing.FolderID || dash.ID == 0) { isParentFolderChanged = true } @@ -897,6 +901,7 @@ func (d *dashboardStore) GetDashboard(ctx context.Context, query *dashboards.Get } // nolint:staticcheck if query.FolderID != nil { + // nolint:staticcheck dashboard.FolderID = *query.FolderID mustCols = append(mustCols, "folder_id") } diff --git a/pkg/services/dashboards/database/database_test.go b/pkg/services/dashboards/database/database_test.go index af7238567d6..f0db79ba1e7 100644 --- a/pkg/services/dashboards/database/database_test.go +++ b/pkg/services/dashboards/database/database_test.go @@ -61,6 +61,7 @@ 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.Positive(t, len(savedDash.UID)) @@ -68,6 +69,7 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { 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.Positive(t, len(savedFolder.UID)) }) @@ -222,6 +224,7 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { } dash, err := dashboardStore.SaveDashboard(context.Background(), cmd) require.NoError(t, err) + // nolint:staticcheck require.EqualValues(t, dash.FolderID, 2) cmd = dashboards.SaveDashboardCommand{ @@ -245,6 +248,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.CreatedBy, savedDash.CreatedBy) require.WithinDuration(t, queryResult.Created, savedDash.Created, 3*time.Second) @@ -700,6 +704,7 @@ func TestGetExistingDashboardByTitleAndFolder(t *testing.T) { savedFolder := insertTestDashboard(t, dashboardStore, "test dash folder", 1, 0, "", true, "prod", "webapp") savedDash := insertTestDashboard(t, dashboardStore, "test dash", 1, savedFolder.ID, savedFolder.UID, false, "prod", "webapp") err = sqlStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + // nolint:staticcheck _, err = getExistingDashboardByTitleAndFolder(sess, &dashboards.Dashboard{Title: savedDash.Title, FolderID: savedFolder.ID, OrgID: 1}, sqlStore.GetDialect(), false, false) return err }) diff --git a/pkg/services/dashboards/models.go b/pkg/services/dashboards/models.go index 98c2a59eacd..ec2561524d9 100644 --- a/pkg/services/dashboards/models.go +++ b/pkg/services/dashboards/models.go @@ -41,6 +41,7 @@ type Dashboard struct { UpdatedBy int64 CreatedBy int64 + // Deprecated: use FolderUID instead FolderID int64 `xorm:"folder_id"` FolderUID string `xorm:"folder_uid"` IsFolder bool @@ -104,6 +105,7 @@ func (d *Dashboard) ToResource() kinds.GrafanaResource[simplejson.Json, any] { Key: d.PluginID, }) } + // nolint:staticcheck if d.FolderID > 0 { res.Metadata.SetFolder(fmt.Sprintf("folder:%d", d.FolderID)) } @@ -186,6 +188,7 @@ func (cmd *SaveDashboardCommand) GetDashboardModel() *Dashboard { dash.OrgID = cmd.OrgID dash.PluginID = cmd.PluginID dash.IsFolder = cmd.IsFolder + // nolint:staticcheck dash.FolderID = cmd.FolderID dash.FolderUID = cmd.FolderUID dash.UpdateSlug() diff --git a/pkg/services/dashboards/models_test.go b/pkg/services/dashboards/models_test.go index 3ab908c0039..fd49152dfdb 100644 --- a/pkg/services/dashboards/models_test.go +++ b/pkg/services/dashboards/models_test.go @@ -69,6 +69,7 @@ func TestSaveDashboardCommand_GetDashboardModel(t *testing.T) { cmd := &SaveDashboardCommand{Dashboard: json, FolderID: 1, FolderUID: "1"} dash := cmd.GetDashboardModel() + // nolint:staticcheck assert.Equal(t, int64(1), dash.FolderID) }) } @@ -103,6 +104,7 @@ func TestResourceConversion(t *testing.T) { dash.CreatedBy = 10 dash.UpdatedBy = 11 dash.PluginID = "plugin-xyz" + // nolint:staticcheck dash.FolderID = 1234 dash.SetID(12345) // should be removed in resource version diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index 9e480d7d88e..3eb825392db 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -106,6 +106,7 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d return nil, dashboards.ErrDashboardTitleEmpty } + // nolint:staticcheck if dash.IsFolder && dash.FolderID > 0 { return nil, dashboards.ErrDashboardFolderCannotHaveParent } @@ -142,6 +143,7 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d if err != nil { return nil, err } + // nolint:staticcheck if canSave, err := guardian.CanCreate(dash.FolderID, dash.IsFolder); err != nil || !canSave { if err != nil { return nil, err @@ -167,6 +169,7 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d } if dash.ID == 0 { + // nolint:staticcheck if canCreate, err := guard.CanCreate(dash.FolderID, dash.IsFolder); err != nil || !canCreate { if err != nil { return nil, err @@ -193,7 +196,7 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d OrgID: dto.OrgID, Overwrite: dto.Overwrite, UserID: userID, - FolderID: dash.FolderID, + FolderID: dash.FolderID, // nolint:staticcheck FolderUID: dash.FolderUID, IsFolder: dash.IsFolder, PluginID: dash.PluginID, @@ -236,6 +239,7 @@ func getGuardianForSavePermissionCheck(ctx context.Context, d *dashboards.Dashbo if newDashboard { // if it's a new dashboard/folder check the parent folder permissions + // nolint:staticcheck guard, err := guardian.New(ctx, d.FolderID, d.OrgID, user) if err != nil { return nil, err @@ -473,6 +477,7 @@ func (dr *DashboardServiceImpl) GetDashboardsByPluginID(ctx context.Context, que } func (dr *DashboardServiceImpl) setDefaultPermissions(ctx context.Context, dto *dashboards.SaveDashboardDTO, dash *dashboards.Dashboard, provisioned bool) { + // nolint:staticcheck inFolder := dash.FolderID > 0 var permissions []accesscontrol.SetResourcePermissionCommand diff --git a/pkg/services/dashboards/service/dashboard_service_integration_test.go b/pkg/services/dashboards/service/dashboard_service_integration_test.go index dadd20da48b..be9df62ccd1 100644 --- a/pkg/services/dashboards/service/dashboard_service_integration_test.go +++ b/pkg/services/dashboards/service/dashboard_service_integration_test.go @@ -202,7 +202,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { "id": sc.savedDashInGeneralFolder.ID, "title": "Dash", }), - FolderID: sc.savedDashInGeneralFolder.FolderID, + FolderID: sc.savedDashInGeneralFolder.FolderID, // nolint:staticcheck FolderUID: sc.savedDashInGeneralFolder.FolderUID, UserID: 10000, Overwrite: true, @@ -227,7 +227,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { "id": sc.savedDashInFolder.ID, "title": "Dash", }), - FolderID: sc.savedDashInFolder.FolderID, + FolderID: sc.savedDashInFolder.FolderID, // nolint:staticcheck FolderUID: sc.savedDashInFolder.FolderUID, UserID: 10000, Overwrite: true, @@ -575,7 +575,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { "id": nil, "title": sc.savedDashInFolder.Title, }), - FolderID: sc.savedDashInFolder.FolderID, + FolderID: sc.savedDashInFolder.FolderID, // nolint:staticcheck FolderUID: sc.savedDashInFolder.FolderUID, Overwrite: shouldOverwrite, } @@ -592,7 +592,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { "id": nil, "title": sc.savedDashInGeneralFolder.Title, }), - FolderID: sc.savedDashInGeneralFolder.FolderID, + FolderID: sc.savedDashInGeneralFolder.FolderID, // nolint:staticcheck FolderUID: sc.savedDashInGeneralFolder.FolderUID, Overwrite: shouldOverwrite, } @@ -715,7 +715,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { "id": nil, "title": sc.savedDashInFolder.Title, }), - FolderID: sc.savedDashInFolder.FolderID, + FolderID: sc.savedDashInFolder.FolderID, // nolint:staticcheck FolderUID: sc.savedDashInFolder.FolderUID, Overwrite: shouldOverwrite, } @@ -740,7 +740,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { "id": nil, "title": sc.savedDashInGeneralFolder.Title, }), - FolderID: sc.savedDashInGeneralFolder.FolderID, + FolderID: sc.savedDashInGeneralFolder.FolderID, // nolint:staticcheck FolderUID: sc.savedDashInGeneralFolder.FolderUID, Overwrite: shouldOverwrite, } @@ -907,6 +907,7 @@ func permissionScenario(t *testing.T, desc string, canSave bool, fn permissionSc require.Equal(t, "saved-folder", savedFolder.Slug) require.NotEqual(t, int64(0), savedFolder.ID) require.True(t, savedFolder.IsFolder) + // nolint:staticcheck require.Equal(t, int64(0), savedFolder.FolderID) require.NotEmpty(t, savedFolder.UID) @@ -914,6 +915,7 @@ func permissionScenario(t *testing.T, desc string, canSave bool, fn permissionSc require.Equal(t, "saved-dash-in-folder", savedDashInFolder.Slug) require.NotEqual(t, int64(0), savedDashInFolder.ID) require.False(t, savedDashInFolder.IsFolder) + // nolint:staticcheck require.Equal(t, savedFolder.ID, savedDashInFolder.FolderID) require.NotEmpty(t, savedDashInFolder.UID) diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index 5a907f3716e..12ca3232475 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -54,6 +54,7 @@ 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) diff --git a/pkg/services/folder/folderimpl/dashboard_folder_store.go b/pkg/services/folder/folderimpl/dashboard_folder_store.go index 8a98b2adcb7..636582b431b 100644 --- a/pkg/services/folder/folderimpl/dashboard_folder_store.go +++ b/pkg/services/folder/folderimpl/dashboard_folder_store.go @@ -26,6 +26,7 @@ func (d *DashboardFolderStoreImpl) GetFolderByTitle(ctx context.Context, orgID i // there is a unique constraint on org_id, folder_id, title // there are no nested folders so the parent folder id is always 0 + // nolint:staticcheck dashboard := dashboards.Dashboard{OrgID: orgID, FolderID: 0, Title: title} err := d.store.WithTransactionalDbSession(ctx, func(sess *db.Session) error { has, err := sess.Table(&dashboards.Dashboard{}).Where("is_folder = " + d.store.GetDialect().BooleanStr(true)).Where("folder_id=0").Get(&dashboard) @@ -43,6 +44,7 @@ func (d *DashboardFolderStoreImpl) GetFolderByTitle(ctx context.Context, orgID i } func (d *DashboardFolderStoreImpl) GetFolderByID(ctx context.Context, orgID int64, id int64) (*folder.Folder, error) { + // nolint:staticcheck dashboard := dashboards.Dashboard{OrgID: orgID, FolderID: 0, ID: id} err := d.store.WithTransactionalDbSession(ctx, func(sess *db.Session) error { has, err := sess.Table(&dashboards.Dashboard{}).Where("is_folder = " + d.store.GetDialect().BooleanStr(true)).Where("folder_id=0").Get(&dashboard) @@ -67,6 +69,7 @@ func (d *DashboardFolderStoreImpl) GetFolderByUID(ctx context.Context, orgID int return nil, dashboards.ErrDashboardIdentifierNotSet } + // nolint:staticcheck dashboard := dashboards.Dashboard{OrgID: orgID, FolderID: 0, UID: uid} err := d.store.WithTransactionalDbSession(ctx, func(sess *db.Session) error { has, err := sess.Table(&dashboards.Dashboard{}).Where("is_folder = " + d.store.GetDialect().BooleanStr(true)).Where("folder_id=0").Get(&dashboard) diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index d136b83c495..10516fdd12c 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -856,6 +856,7 @@ func (s *Service) buildSaveDashboardCommand(ctx context.Context, dto *dashboards return nil, dashboards.ErrDashboardTitleEmpty } + // nolint:staticcheck if dash.IsFolder && dash.FolderID > 0 { return nil, dashboards.ErrDashboardFolderCannotHaveParent } @@ -881,6 +882,7 @@ func (s *Service) buildSaveDashboardCommand(ctx context.Context, dto *dashboards } if dash.ID == 0 { + // nolint:staticcheck if canCreate, err := guard.CanCreate(dash.FolderID, dash.IsFolder); err != nil || !canCreate { if err != nil { return nil, err @@ -913,7 +915,7 @@ func (s *Service) buildSaveDashboardCommand(ctx context.Context, dto *dashboards OrgID: dto.OrgID, Overwrite: dto.Overwrite, UserID: userID, - FolderID: dash.FolderID, + FolderID: dash.FolderID, // nolint:staticcheck FolderUID: dash.FolderUID, IsFolder: dash.IsFolder, PluginID: dash.PluginID, @@ -933,6 +935,7 @@ func getGuardianForSavePermissionCheck(ctx context.Context, d *dashboards.Dashbo if newDashboard { // if it's a new dashboard/folder check the parent folder permissions + // nolint:staticcheck guard, err := guardian.New(ctx, d.FolderID, d.OrgID, user) if err != nil { return nil, err diff --git a/pkg/services/guardian/accesscontrol_guardian_test.go b/pkg/services/guardian/accesscontrol_guardian_test.go index 666b67e3f12..c48c44a0ed5 100644 --- a/pkg/services/guardian/accesscontrol_guardian_test.go +++ b/pkg/services/guardian/accesscontrol_guardian_test.go @@ -30,8 +30,9 @@ const ( var ( folderUIDScope = fmt.Sprintf("folders:uid:%s", folderUID) invalidFolderUIDScope = fmt.Sprintf("folders:uid:%s", invalidFolderUID) - dashboard = &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: folderID} - fldr = &dashboards.Dashboard{OrgID: orgID, UID: folderUID, IsFolder: true} + // nolint:staticcheck + dashboard = &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: folderID} + fldr = &dashboards.Dashboard{OrgID: orgID, UID: folderUID, IsFolder: true} ) type accessControlGuardianTestCase struct { @@ -79,7 +80,7 @@ func TestAccessControlDashboardGuardian_CanSave(t *testing.T) { }, { desc: "should be able to save dashboard under root with general folder scope", - dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, + dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, // nolint:staticcheck permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, @@ -236,7 +237,7 @@ func TestAccessControlDashboardGuardian_CanEdit(t *testing.T) { }, { desc: "should be able to edit dashboard under root with general folder scope", - dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, + dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, // nolint:staticcheck permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, @@ -409,7 +410,7 @@ func TestAccessControlDashboardGuardian_CanView(t *testing.T) { }, { desc: "should be able to view dashboard under root with general folder scope", - dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, + dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, // nolint:staticcheck permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsRead, @@ -578,7 +579,7 @@ func TestAccessControlDashboardGuardian_CanAdmin(t *testing.T) { }, { desc: "should be able to admin dashboard under root with general folder scope", - dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, + dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, // nolint:staticcheck permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsPermissionsRead, @@ -820,7 +821,7 @@ func TestAccessControlDashboardGuardian_CanDelete(t *testing.T) { }, { desc: "should be able to delete dashboard under root with general folder scope", - dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, + dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, // nolint:staticcheck permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsDelete, diff --git a/pkg/services/libraryelements/libraryelements_test.go b/pkg/services/libraryelements/libraryelements_test.go index bfcf0671b4d..36541777197 100644 --- a/pkg/services/libraryelements/libraryelements_test.go +++ b/pkg/services/libraryelements/libraryelements_test.go @@ -274,6 +274,7 @@ type scenarioContext struct { } func createDashboard(t *testing.T, sqlStore db.DB, user user.SignedInUser, dash *dashboards.Dashboard, folderID int64) *dashboards.Dashboard { + // nolint:staticcheck dash.FolderID = folderID dashItem := &dashboards.SaveDashboardDTO{ Dashboard: dash, diff --git a/pkg/services/librarypanels/librarypanels_test.go b/pkg/services/librarypanels/librarypanels_test.go index e370d20f1eb..4465f5d95b9 100644 --- a/pkg/services/librarypanels/librarypanels_test.go +++ b/pkg/services/librarypanels/librarypanels_test.go @@ -705,6 +705,7 @@ func getExpected(t *testing.T, res model.LibraryElementDTO, UID string, name str } func createDashboard(t *testing.T, sqlStore db.DB, user *user.SignedInUser, dash *dashboards.Dashboard, folderID int64) *dashboards.Dashboard { + // nolint:staticcheck dash.FolderID = folderID dashItem := &dashboards.SaveDashboardDTO{ Dashboard: dash, diff --git a/pkg/services/ngalert/migration/migration_test.go b/pkg/services/ngalert/migration/migration_test.go index 408f05b3f7c..7acb4ec380d 100644 --- a/pkg/services/ngalert/migration/migration_test.go +++ b/pkg/services/ngalert/migration/migration_test.go @@ -984,7 +984,7 @@ func TestDashAlertQueryMigration(t *testing.T) { expectedFolder: &dashboards.Dashboard{ OrgID: 2, Title: "General Alerting", - FolderID: 0, + FolderID: 0, // nolint:staticcheck Slug: "general-alerting", }, expected: map[int64][]*ngModels.AlertRule{ @@ -1041,6 +1041,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) } } @@ -1190,8 +1191,8 @@ func createDashboard(t *testing.T, id int64, orgId int64, uid string, folderId i UID: uid, Created: now, Updated: now, - Title: uid, // Not tested, needed to satisfy constraint. - FolderID: folderId, + Title: uid, // Not tested, needed to satisfy constraint. + FolderID: folderId, // nolint:staticcheck Data: simplejson.New(), Version: 1, } diff --git a/pkg/services/ngalert/migration/permissions.go b/pkg/services/ngalert/migration/permissions.go index 48894c9617e..5d8686370ab 100644 --- a/pkg/services/ngalert/migration/permissions.go +++ b/pkg/services/ngalert/migration/permissions.go @@ -65,6 +65,7 @@ func (om *OrgMigration) migratedFolder(ctx context.Context, log log.Logger, dash dashFolder, err := om.getFolder(ctx, dash) if err != nil { + // nolint:staticcheck l.Warn("Failed to find folder for dashboard", "missing_folder_id", dash.FolderID, "error", err) } if dashFolder != nil { @@ -105,6 +106,7 @@ func (om *OrgMigration) getOrCreateMigratedFolder(ctx context.Context, l log.Log permissionsToFolder, ok := om.permissionsMap[parentFolder.ID] if !ok { permissionsToFolder = make(map[permissionHash]*folder.Folder) + // nolint:staticcheck om.permissionsMap[dash.FolderID] = permissionsToFolder folderPerms, err := om.getFolderPermissions(ctx, parentFolder) @@ -328,10 +330,12 @@ func (om *OrgMigration) getDashboardPermissions(ctx context.Context, d *dashboar // getFolder returns the parent folder for the given dashboard. If the dashboard is in the general folder, it returns the general alerting folder. func (om *OrgMigration) getFolder(ctx context.Context, dash *dashboards.Dashboard) (*folder.Folder, error) { + // nolint:staticcheck if f, ok := om.folderCache[dash.FolderID]; ok { return f, nil } + // nolint:staticcheck if dash.FolderID <= 0 { // Don't use general folder since it has no uid, instead we use a new "General Alerting" folder. migratedFolder, err := om.getOrCreateGeneralAlertingFolder(ctx, om.orgID) @@ -341,13 +345,17 @@ func (om *OrgMigration) getFolder(ctx context.Context, dash *dashboards.Dashboar return migratedFolder, err } + // nolint:staticcheck f, err := om.migrationStore.GetFolder(ctx, &folder.GetFolderQuery{ID: &dash.FolderID, OrgID: om.orgID, SignedInUser: getMigrationUser(om.orgID)}) if err != nil { if errors.Is(err, dashboards.ErrFolderNotFound) { + // nolint:staticcheck return nil, fmt.Errorf("folder with id %v not found", dash.FolderID) } + // nolint:staticcheck return nil, fmt.Errorf("get folder %d: %w", dash.FolderID, err) } + // nolint:staticcheck om.folderCache[dash.FolderID] = f return f, nil } diff --git a/pkg/services/provisioning/dashboards/file_reader.go b/pkg/services/provisioning/dashboards/file_reader.go index c3afe0f0560..b57dbe247f8 100644 --- a/pkg/services/provisioning/dashboards/file_reader.go +++ b/pkg/services/provisioning/dashboards/file_reader.go @@ -264,6 +264,7 @@ func (fr *FileReader) saveDashboard(ctx context.Context, path string, folderID i } if upToDate { + // nolint:staticcheck fr.log.Debug("provisioned dashboard is up to date", "provisioner", fr.Cfg.Name, "file", path, "folderId", dash.Dashboard.FolderID, "folderUid", dash.Dashboard.FolderUID) return provisioningMetadata, nil } @@ -278,6 +279,7 @@ func (fr *FileReader) saveDashboard(ctx context.Context, path string, folderID i } if !fr.isDatabaseAccessRestricted() { + // nolint:staticcheck fr.log.Debug("saving new dashboard", "provisioner", fr.Cfg.Name, "file", path, "folderId", dash.Dashboard.FolderID, "folderUid", dash.Dashboard.FolderUID) dp := &dashboards.DashboardProvisioning{ ExternalID: path, @@ -290,6 +292,7 @@ func (fr *FileReader) saveDashboard(ctx context.Context, path string, folderID i return provisioningMetadata, err } } else { + // nolint:staticcheck fr.log.Warn("Not saving new dashboard due to restricted database access", "provisioner", fr.Cfg.Name, "file", path, "folderId", dash.Dashboard.FolderID) } diff --git a/pkg/services/provisioning/dashboards/types.go b/pkg/services/provisioning/dashboards/types.go index dbe11a0aa84..87b365a8ffb 100644 --- a/pkg/services/provisioning/dashboards/types.go +++ b/pkg/services/provisioning/dashboards/types.go @@ -63,6 +63,7 @@ func createDashboardJSON(data *simplejson.Json, lastModified time.Time, cfg *con dash.Overwrite = true dash.OrgID = cfg.OrgID dash.Dashboard.OrgID = cfg.OrgID + // nolint:staticcheck dash.Dashboard.FolderID = folderID dash.Dashboard.FolderUID = folderUID diff --git a/pkg/services/publicdashboards/service/service_test.go b/pkg/services/publicdashboards/service/service_test.go index 188d386080d..5f8352f438e 100644 --- a/pkg/services/publicdashboards/service/service_test.go +++ b/pkg/services/publicdashboards/service/service_test.go @@ -312,6 +312,7 @@ func TestGetPublicDashboardForView(t *testing.T) { // #nosec G101 -- This is dummy/test token accessToken := "c54b1c4dd2b143a1a7a43005264d256d" + // nolint:staticcheck d := &dashboards.Dashboard{UID: "mydashboard", Data: data, Slug: "dashboardSlug", Created: now, Updated: now, Version: 1, FolderID: 1} testCases := []struct { diff --git a/pkg/services/searchV2/service_bench_test.go b/pkg/services/searchV2/service_bench_test.go index 01e4ca4be4d..bd5d95df44d 100644 --- a/pkg/services/searchV2/service_bench_test.go +++ b/pkg/services/searchV2/service_bench_test.go @@ -140,7 +140,7 @@ func populateDB(folderCount, dashboardsPerFolder int, sqlStore *sqlstore.SQLStor UID: fmt.Sprintf("dashboard%v", dashID), Title: fmt.Sprintf("dashboard%v", dashID), IsFolder: false, - FolderID: folderID, + FolderID: folderID, // nolint:staticcheck OrgID: 1, Created: now, Updated: now, diff --git a/pkg/services/sqlstore/permissions/dashboard_test.go b/pkg/services/sqlstore/permissions/dashboard_test.go index 9f04d6f7a08..f800b264e18 100644 --- a/pkg/services/sqlstore/permissions/dashboard_test.go +++ b/pkg/services/sqlstore/permissions/dashboard_test.go @@ -777,7 +777,7 @@ func setupTest(t *testing.T, numFolders, numDashboards int, permissions []access dashes = append(dashes, dashboards.Dashboard{ OrgID: 1, IsFolder: false, - FolderID: int64(folderID), + FolderID: int64(folderID), // nolint:staticcheck UID: str, Slug: str, Title: str, diff --git a/pkg/services/sqlstore/permissions/dashboards_bench_test.go b/pkg/services/sqlstore/permissions/dashboards_bench_test.go index c071ba3e6c4..33f3733c8aa 100644 --- a/pkg/services/sqlstore/permissions/dashboards_bench_test.go +++ b/pkg/services/sqlstore/permissions/dashboards_bench_test.go @@ -132,7 +132,7 @@ func setupBenchMark(b *testing.B, usr user.SignedInUser, features featuremgmt.Fe Data: simplejson.New(), Created: now, Updated: now, - FolderID: leaf.ID, + FolderID: leaf.ID, // nolint:staticcheck }) } @@ -176,6 +176,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 { permissions = append(permissions, accesscontrol.Permission{ RoleID: int64(i),