From 2f40fd674110a8a22f7ed149ed481fb7b3a1f7af Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Tue, 29 Oct 2024 08:58:39 +0300 Subject: [PATCH] Dashboards: Remove unique name constraints (#90687) --- pkg/api/apierrors/folder.go | 3 +- pkg/api/dashboard_test.go | 3 - pkg/api/folder_test.go | 13 ----- pkg/services/dashboards/database/database.go | 56 +------------------ .../dashboards/database/database_test.go | 34 ----------- .../migrations/folder_uid_migrator.go | 5 ++ pkg/services/dashboards/errors.go | 15 ----- pkg/services/dashboards/models.go | 5 +- .../dashboard_service_integration_test.go | 16 ++++-- pkg/services/folder/folderimpl/folder.go | 10 ---- pkg/services/folder/folderimpl/folder_test.go | 1 - .../provisioning/dashboards/file_reader.go | 7 ++- .../sqlstore/migrations/dashboard_mig.go | 1 + .../sqlstore/migrations/folder_mig.go | 6 ++ pkg/tests/api/folders/api_folder_test.go | 38 +++++-------- pkg/tests/apis/folder/folders_test.go | 12 +--- 16 files changed, 51 insertions(+), 174 deletions(-) diff --git a/pkg/api/apierrors/folder.go b/pkg/api/apierrors/folder.go index a5d544f1a4a..8006001356c 100644 --- a/pkg/api/apierrors/folder.go +++ b/pkg/api/apierrors/folder.go @@ -35,8 +35,7 @@ func ToFolderErrorResponse(err error) response.Response { return response.JSON(http.StatusNotFound, util.DynMap{"status": "not-found", "message": dashboards.ErrFolderNotFound.Error()}) } - if errors.Is(err, dashboards.ErrFolderSameNameExists) || - errors.Is(err, dashboards.ErrFolderWithSameUIDExists) { + if errors.Is(err, dashboards.ErrFolderWithSameUIDExists) { return response.Error(http.StatusConflict, err.Error(), nil) } diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 2a561916821..f78b1fc5f24 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -476,13 +476,10 @@ func TestDashboardAPIEndpoint(t *testing.T) { {SaveError: dashboards.ErrDashboardNotFound, ExpectedStatusCode: http.StatusNotFound}, {SaveError: dashboards.ErrFolderNotFound, ExpectedStatusCode: http.StatusBadRequest}, {SaveError: dashboards.ErrDashboardWithSameUIDExists, ExpectedStatusCode: http.StatusBadRequest}, - {SaveError: dashboards.ErrDashboardWithSameNameInFolderExists, ExpectedStatusCode: http.StatusPreconditionFailed}, {SaveError: dashboards.ErrDashboardVersionMismatch, ExpectedStatusCode: http.StatusPreconditionFailed}, {SaveError: dashboards.ErrDashboardTitleEmpty, ExpectedStatusCode: http.StatusBadRequest}, {SaveError: dashboards.ErrDashboardFolderCannotHaveParent, ExpectedStatusCode: http.StatusBadRequest}, {SaveError: dashboards.ErrDashboardTypeMismatch, ExpectedStatusCode: http.StatusBadRequest}, - {SaveError: dashboards.ErrDashboardFolderWithSameNameAsDashboard, ExpectedStatusCode: http.StatusBadRequest}, - {SaveError: dashboards.ErrDashboardWithSameNameAsFolder, ExpectedStatusCode: http.StatusBadRequest}, {SaveError: dashboards.ErrDashboardFolderNameExists, ExpectedStatusCode: http.StatusBadRequest}, {SaveError: dashboards.ErrDashboardUpdateAccessDenied, ExpectedStatusCode: http.StatusForbidden}, {SaveError: dashboards.ErrDashboardInvalidUid, ExpectedStatusCode: http.StatusBadRequest}, diff --git a/pkg/api/folder_test.go b/pkg/api/folder_test.go index c5086968880..35d5af8390f 100644 --- a/pkg/api/folder_test.go +++ b/pkg/api/folder_test.go @@ -88,13 +88,6 @@ func TestFoldersCreateAPIEndpoint(t *testing.T) { expectedFolderSvcError: dashboards.ErrDashboardUidTooLong, permissions: []accesscontrol.Permission{{Action: dashboards.ActionFoldersCreate}}, }, - { - description: "folder creation fails given folder service error %s", - input: folderWithoutParentInput, - expectedCode: http.StatusConflict, - expectedFolderSvcError: dashboards.ErrFolderSameNameExists, - permissions: []accesscontrol.Permission{{Action: dashboards.ActionFoldersCreate}}, - }, { description: "folder creation fails given folder service error %s", input: folderWithoutParentInput, @@ -203,12 +196,6 @@ func TestFoldersUpdateAPIEndpoint(t *testing.T) { expectedFolderSvcError: dashboards.ErrDashboardUidTooLong, permissions: []accesscontrol.Permission{{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}}, }, - { - description: "folder updating fails given folder service error %s", - expectedCode: http.StatusConflict, - expectedFolderSvcError: dashboards.ErrFolderSameNameExists, - permissions: []accesscontrol.Permission{{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}}, - }, { description: "folder updating fails given folder service error %s", expectedCode: http.StatusForbidden, diff --git a/pkg/services/dashboards/database/database.go b/pkg/services/dashboards/database/database.go index 5e33ddcca73..5ed1db5160e 100644 --- a/pkg/services/dashboards/database/database.go +++ b/pkg/services/dashboards/database/database.go @@ -84,12 +84,6 @@ func (d *dashboardStore) ValidateDashboardBeforeSave(ctx context.Context, dashbo return err } - isParentFolderChanged, err = getExistingDashboardByTitleAndFolder(sess, dashboard, overwrite, - isParentFolderChanged) - if err != nil { - return err - } - return nil }) if err != nil { @@ -353,49 +347,6 @@ func getExistingDashboardByIDOrUIDForUpdate(sess *db.Session, dash *dashboards.D return isParentFolderChanged, nil } -// getExistingDashboardByTitleAndFolder returns a boolean (on whether the parent folder changed) and an error for if the dashboard already exists. -func getExistingDashboardByTitleAndFolder(sess *db.Session, dash *dashboards.Dashboard, overwrite, - isParentFolderChanged bool) (bool, error) { - var existing dashboards.Dashboard - condition := "org_id=? AND title=?" - args := []any{dash.OrgID, dash.Title} - if dash.FolderUID != "" { - condition += " AND folder_uid=?" - args = append(args, dash.FolderUID) - } else { - condition += " AND folder_uid IS NULL" - } - exists, err := sess.Where(condition, args...).Get(&existing) - if err != nil { - return isParentFolderChanged, fmt.Errorf("SQL query for existing dashboard by org ID or folder ID failed: %w", err) - } - if exists && dash.ID != existing.ID { - if existing.IsFolder && !dash.IsFolder { - return isParentFolderChanged, dashboards.ErrDashboardWithSameNameAsFolder - } - - if !existing.IsFolder && dash.IsFolder { - return isParentFolderChanged, dashboards.ErrDashboardFolderWithSameNameAsDashboard - } - - metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Dashboard).Inc() - // nolint:staticcheck - if !dash.IsFolder && (dash.FolderID != existing.FolderID || dash.ID == 0) { - isParentFolderChanged = true - } - - if overwrite { - dash.SetID(existing.ID) - dash.SetUID(existing.UID) - dash.SetVersion(existing.Version) - } else { - return isParentFolderChanged, dashboards.ErrDashboardWithSameNameInFolderExists - } - } - - return isParentFolderChanged, nil -} - func saveDashboard(sess *db.Session, cmd *dashboards.SaveDashboardCommand, emitEntityEvent bool) (*dashboards.Dashboard, error) { dash := cmd.GetDashboardModel() @@ -797,8 +748,8 @@ func (d *dashboardStore) GetDashboard(ctx context.Context, query *dashboards.Get dashboard := dashboards.Dashboard{OrgID: query.OrgID, ID: query.ID, UID: query.UID} mustCols := []string{} - if query.Title != nil { - dashboard.Title = *query.Title + if query.Title != nil { // nolint:staticcheck + dashboard.Title = *query.Title // nolint:staticcheck mustCols = append(mustCols, "title") } @@ -806,8 +757,7 @@ func (d *dashboardStore) GetDashboard(ctx context.Context, query *dashboards.Get dashboard.FolderUID = *query.FolderUID mustCols = append(mustCols, "folder_uid") } else if query.FolderID != nil { // nolint:staticcheck - // nolint:staticcheck - dashboard.FolderID = *query.FolderID + dashboard.FolderID = *query.FolderID // nolint:staticcheck mustCols = append(mustCols, "folder_id") metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Dashboard).Inc() } diff --git a/pkg/services/dashboards/database/database_test.go b/pkg/services/dashboards/database/database_test.go index 83259785a69..67487b1226b 100644 --- a/pkg/services/dashboards/database/database_test.go +++ b/pkg/services/dashboards/database/database_test.go @@ -781,40 +781,6 @@ func TestIntegrationDashboard_Filter(t *testing.T) { assert.Equal(t, dashB.ID, results[0].ID) } -func TestGetExistingDashboardByTitleAndFolder(t *testing.T) { - sqlStore := db.InitTestDB(t) - cfg := setting.NewCfg() - quotaService := quotatest.New(false, nil) - dashboardStore, err := ProvideDashboardStore(sqlStore, cfg, testFeatureToggles, tagimpl.ProvideService(sqlStore), quotaService) - require.NoError(t, err) - insertTestDashboard(t, dashboardStore, "Apple", 1, 0, "", false) - t.Run("Finds a dashboard with existing name in root directory and throws DashboardWithSameNameInFolderExists error", func(t *testing.T) { - err = sqlStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { - _, err = getExistingDashboardByTitleAndFolder(sess, &dashboards.Dashboard{Title: "Apple", OrgID: 1}, false, false) - return err - }) - require.ErrorIs(t, err, dashboards.ErrDashboardWithSameNameInFolderExists) - }) - - t.Run("Returns no error when dashboard does not exist in root folder", func(t *testing.T) { - err = sqlStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { - _, err = getExistingDashboardByTitleAndFolder(sess, &dashboards.Dashboard{Title: "Beta", OrgID: 1}, false, false) - return err - }) - require.NoError(t, err) - }) - - t.Run("Finds a dashboard with existing name in specific folder and throws DashboardWithSameNameInFolderExists error", func(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 { - _, err = getExistingDashboardByTitleAndFolder(sess, &dashboards.Dashboard{Title: savedDash.Title, FolderUID: savedFolder.UID, OrgID: 1}, false, false) - return err - }) - require.ErrorIs(t, err, dashboards.ErrDashboardWithSameNameInFolderExists) - }) -} - func TestIntegrationFindDashboardsByTitle(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") diff --git a/pkg/services/dashboards/database/migrations/folder_uid_migrator.go b/pkg/services/dashboards/database/migrations/folder_uid_migrator.go index fd1a075c82f..703377f62e0 100644 --- a/pkg/services/dashboards/database/migrations/folder_uid_migrator.go +++ b/pkg/services/dashboards/database/migrations/folder_uid_migrator.go @@ -98,6 +98,7 @@ func AddDashboardFolderMigrations(mg *migrator.Migrator) { mg.AddMigration("Delete unique index for dashboard_org_id_folder_uid_title", &DummyMigration{}) + // Removed a few lines below mg.AddMigration("Add unique index for dashboard_org_id_folder_uid_title_is_folder", migrator.NewAddIndexMigration(migrator.Table{Name: "dashboard"}, &migrator.Index{ Cols: []string{"org_id", "folder_uid", "title", "is_folder"}, Type: migrator.UniqueIndex, })) @@ -106,4 +107,8 @@ func AddDashboardFolderMigrations(mg *migrator.Migrator) { mg.AddMigration("Restore index for dashboard_org_id_folder_id_title", migrator.NewAddIndexMigration(migrator.Table{Name: "dashboard"}, &migrator.Index{ Cols: []string{"org_id", "folder_id", "title"}, })) + + mg.AddMigration("Remove unique index for dashboard_org_id_folder_uid_title_is_folder", migrator.NewDropIndexMigration(migrator.Table{Name: "dashboard"}, &migrator.Index{ + Cols: []string{"org_id", "folder_uid", "title", "is_folder"}, Type: migrator.UniqueIndex, + })) } diff --git a/pkg/services/dashboards/errors.go b/pkg/services/dashboards/errors.go index 0dafc5aebc0..af11c84f652 100644 --- a/pkg/services/dashboards/errors.go +++ b/pkg/services/dashboards/errors.go @@ -32,11 +32,6 @@ var ( Reason: "A dashboard with the same uid already exists", StatusCode: 400, } - ErrDashboardWithSameNameInFolderExists = DashboardErr{ - Reason: "A dashboard with the same name in the folder already exists", - StatusCode: 412, - Status: "name-exists", - } ErrDashboardVersionMismatch = DashboardErr{ Reason: "The dashboard has been changed by someone else", StatusCode: 412, @@ -59,15 +54,6 @@ var ( Reason: "Dashboard cannot be changed to a folder", StatusCode: 400, } - ErrDashboardFolderWithSameNameAsDashboard = DashboardErr{ - Reason: "Folder name cannot be the same as one of its dashboards", - StatusCode: 400, - } - ErrDashboardWithSameNameAsFolder = DashboardErr{ - Reason: "Dashboard name cannot be the same as folder", - StatusCode: 400, - Status: "name-match", - } ErrDashboardFolderNameExists = DashboardErr{ Reason: "A folder with that name already exists", StatusCode: 400, @@ -128,7 +114,6 @@ var ( ErrFolderTitleEmpty = errors.New("folder title cannot be empty") ErrFolderWithSameUIDExists = errors.New("a folder/dashboard with the same uid already exists") ErrFolderInvalidUID = errors.New("invalid uid for folder provided") - ErrFolderSameNameExists = errors.New("a folder with the same name already exists in the current location") ErrFolderAccessDenied = errors.New("access denied to folder") ErrUserIsNotSignedInToOrg = errors.New("user is not signed in to organization") ErrMoveAccessDenied = errutil.Forbidden("folders.forbiddenMove", errutil.WithPublicMessage("Access denied to the destination folder")) diff --git a/pkg/services/dashboards/models.go b/pkg/services/dashboards/models.go index 48e10536d55..2458c64305c 100644 --- a/pkg/services/dashboards/models.go +++ b/pkg/services/dashboards/models.go @@ -252,8 +252,9 @@ type DeleteOrphanedProvisionedDashboardsCommand struct { // // Multiple constraints can be combined. type GetDashboardQuery struct { - ID int64 - UID string + ID int64 + UID string + // Deprecated: this is no-longer a unique constraint and should not be used Title *string // Deprecated: use FolderUID instead FolderID *int64 diff --git a/pkg/services/dashboards/service/dashboard_service_integration_test.go b/pkg/services/dashboards/service/dashboard_service_integration_test.go index 775ae923aef..8333cdf0d4d 100644 --- a/pkg/services/dashboards/service/dashboard_service_integration_test.go +++ b/pkg/services/dashboards/service/dashboard_service_integration_test.go @@ -150,6 +150,8 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { permissionScenario(t, "When creating a new dashboard by existing title in folder, it should create dashboard guardian for dashboard with correct arguments and result in access denied error", canSave, func(t *testing.T, sc *permissionScenarioContext) { + t.Skip() + cmd := dashboards.SaveDashboardCommand{ OrgID: testOrgID, Dashboard: simplejson.NewFromAny(map[string]any{ @@ -568,7 +570,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { } err := callSaveWithError(t, cmd, sc.sqlStore) - assert.Equal(t, dashboards.ErrDashboardWithSameNameInFolderExists, err) + require.NoError(t, err) }) permissionScenario(t, "When creating a dashboard with same name as dashboard in General folder", @@ -584,7 +586,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { } err := callSaveWithError(t, cmd, sc.sqlStore) - assert.Equal(t, dashboards.ErrDashboardWithSameNameInFolderExists, err) + require.NoError(t, err) }) permissionScenario(t, "When creating a folder with same name as existing folder", canSave, @@ -600,7 +602,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { } err := callSaveWithError(t, cmd, sc.sqlStore) - assert.Equal(t, dashboards.ErrDashboardWithSameNameInFolderExists, err) + require.NoError(t, err) }) }) @@ -693,6 +695,8 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { permissionScenario(t, "When creating a dashboard with same name as dashboard in other folder", canSave, func(t *testing.T, sc *permissionScenarioContext) { + t.Skip() + cmd := dashboards.SaveDashboardCommand{ OrgID: testOrgID, Dashboard: simplejson.NewFromAny(map[string]any{ @@ -717,6 +721,8 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { permissionScenario(t, "When creating a dashboard with same name as dashboard in General folder", canSave, func(t *testing.T, sc *permissionScenarioContext) { + t.Skip() + cmd := dashboards.SaveDashboardCommand{ OrgID: testOrgID, Dashboard: simplejson.NewFromAny(map[string]any{ @@ -815,7 +821,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { } err := callSaveWithError(t, cmd, sc.sqlStore) - assert.Equal(t, dashboards.ErrDashboardWithSameNameAsFolder, err) + require.NoError(t, err) }) permissionScenario(t, "When updating existing dashboard to a folder using title", canSave, @@ -830,7 +836,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) { } err := callSaveWithError(t, cmd, sc.sqlStore) - assert.Equal(t, dashboards.ErrDashboardFolderWithSameNameAsDashboard, err) + require.NoError(t, err) }) }) }) diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index 8498a082908..0c56002d1f4 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -1009,9 +1009,6 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol NewParentUID: &cmd.NewParentUID, SignedInUser: cmd.SignedInUser, }); err != nil { - if s.db.GetDialect().IsUniqueConstraintViolation(err) { - return folder.ErrConflict.Errorf("%w", dashboards.ErrFolderSameNameExists) - } return folder.ErrInternal.Errorf("failed to move folder: %w", err) } @@ -1023,9 +1020,6 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol // bypass optimistic locking used for dashboards Overwrite: true, }); err != nil { - if s.db.GetDialect().IsUniqueConstraintViolation(err) { - return folder.ErrConflict.Errorf("%w", dashboards.ErrFolderSameNameExists) - } return folder.ErrInternal.Errorf("failed to move legacy folder: %w", err) } @@ -1393,10 +1387,6 @@ func toFolderError(err error) error { return dashboards.ErrFolderAccessDenied } - if errors.Is(err, dashboards.ErrDashboardWithSameNameInFolderExists) { - return dashboards.ErrFolderSameNameExists - } - if errors.Is(err, dashboards.ErrDashboardWithSameUIDExists) { return dashboards.ErrFolderWithSameUIDExists } diff --git a/pkg/services/folder/folderimpl/folder_test.go b/pkg/services/folder/folderimpl/folder_test.go index 32bf156e55c..0ddab9f39fd 100644 --- a/pkg/services/folder/folderimpl/folder_test.go +++ b/pkg/services/folder/folderimpl/folder_test.go @@ -387,7 +387,6 @@ func TestIntegrationFolderService(t *testing.T) { }{ {ActualError: dashboards.ErrDashboardTitleEmpty, ExpectedError: dashboards.ErrFolderTitleEmpty}, {ActualError: dashboards.ErrDashboardUpdateAccessDenied, ExpectedError: dashboards.ErrFolderAccessDenied}, - {ActualError: dashboards.ErrDashboardWithSameNameInFolderExists, ExpectedError: dashboards.ErrFolderSameNameExists}, {ActualError: dashboards.ErrDashboardWithSameUIDExists, ExpectedError: dashboards.ErrFolderWithSameUIDExists}, {ActualError: dashboards.ErrDashboardVersionMismatch, ExpectedError: dashboards.ErrFolderVersionMismatch}, {ActualError: dashboards.ErrDashboardNotFound, ExpectedError: dashboards.ErrFolderNotFound}, diff --git a/pkg/services/provisioning/dashboards/file_reader.go b/pkg/services/provisioning/dashboards/file_reader.go index a061558e82a..e36bedda79e 100644 --- a/pkg/services/provisioning/dashboards/file_reader.go +++ b/pkg/services/provisioning/dashboards/file_reader.go @@ -262,8 +262,11 @@ func (fr *FileReader) saveDashboard(ctx context.Context, path string, folderID i &dashboards.GetDashboardQuery{ OrgID: jsonFile.dashboard.OrgID, UID: jsonFile.dashboard.Dashboard.UID, - Title: &jsonFile.dashboard.Dashboard.Title, FolderUID: util.Pointer(""), + + // provisioning depends on unique names + //nolint:staticcheck + Title: &jsonFile.dashboard.Dashboard.Title, }, ) if err != nil { @@ -349,6 +352,8 @@ func (fr *FileReader) getOrCreateFolder(ctx context.Context, cfg *config, servic if cfg.FolderUID != "" { cmd.UID = cfg.FolderUID } else { + // provisioning depends on unique names + //nolint:staticcheck cmd.Title = &folderName } diff --git a/pkg/services/sqlstore/migrations/dashboard_mig.go b/pkg/services/sqlstore/migrations/dashboard_mig.go index bf19b56277d..46bdd1c886a 100644 --- a/pkg/services/sqlstore/migrations/dashboard_mig.go +++ b/pkg/services/sqlstore/migrations/dashboard_mig.go @@ -172,6 +172,7 @@ func addDashboardMigration(mg *Migrator) { {Name: "title", Type: DB_NVarchar, Length: 189, Nullable: false}, })) + // This gets removed later in AddDashboardFolderMigrations mg.AddMigration("Add unique index for dashboard_org_id_title_folder_id", NewAddIndexMigration(dashboardV2, &Index{ Cols: []string{"org_id", "folder_id", "title"}, Type: UniqueIndex, })) diff --git a/pkg/services/sqlstore/migrations/folder_mig.go b/pkg/services/sqlstore/migrations/folder_mig.go index b107afea972..7cc9c451a8c 100644 --- a/pkg/services/sqlstore/migrations/folder_mig.go +++ b/pkg/services/sqlstore/migrations/folder_mig.go @@ -80,6 +80,12 @@ func addFolderMigrations(mg *migrator.Migrator) { mg.AddMigration("Remove index IDX_folder_parent_uid_org_id", migrator.NewDropIndexMigration(folderv1(), &migrator.Index{ Cols: []string{"parent_uid", "org_id"}, })) + + // Remove the unique name constraint + mg.AddMigration("Remove unique index UQE_folder_org_id_parent_uid_title", migrator.NewDropIndexMigration(folderv1(), &migrator.Index{ + Type: migrator.UniqueIndex, + Cols: []string{"org_id", "parent_uid", "title"}, + })) } func folderv1() migrator.Table { diff --git a/pkg/tests/api/folders/api_folder_test.go b/pkg/tests/api/folders/api_folder_test.go index c61e94b0cd9..72534347445 100644 --- a/pkg/tests/api/folders/api_folder_test.go +++ b/pkg/tests/api/folders/api_folder_test.go @@ -2,12 +2,9 @@ package folders import ( "context" - "errors" "net/http" "testing" - "github.com/go-openapi/runtime" - "github.com/grafana/grafana-openapi-client-go/client/folders" "github.com/grafana/grafana-openapi-client-go/models" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/tracing" @@ -92,14 +89,12 @@ func TestIntegrationCreateFolder(t *testing.T) { require.NoError(t, err) require.Equal(t, http.StatusOK, resp.Code()) - t.Run("create folder with same name under root should fail", func(t *testing.T) { + t.Run("create folder with same name under root should succeed", func(t *testing.T) { _, err := adminClient.Folders.CreateFolder(&models.CreateFolderCommand{ Title: "folder", }) - require.Error(t, err) - var conflict *folders.CreateFolderConflict - assert.True(t, errors.As(err, &conflict)) - assert.Equal(t, http.StatusConflict, conflict.Code()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.Code()) }) }) } @@ -133,14 +128,12 @@ func TestIntegrationNestedFoldersOn(t *testing.T) { require.NoError(t, err) require.Equal(t, http.StatusOK, resp.Code()) - t.Run("create folder with same name under root should fail", func(t *testing.T) { + t.Run("create folder with same name under root should succeed", func(t *testing.T) { _, err := adminClient.Folders.CreateFolder(&models.CreateFolderCommand{ Title: "folder", }) - require.Error(t, err) - var conflict *folders.CreateFolderConflict - assert.True(t, errors.As(err, &conflict)) - assert.Equal(t, http.StatusConflict, conflict.Code()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.Code()) }) }) @@ -159,15 +152,13 @@ func TestIntegrationNestedFoldersOn(t *testing.T) { require.NoError(t, err) require.Equal(t, http.StatusOK, resp.Code()) - t.Run("create subfolder with same name should fail", func(t *testing.T) { + t.Run("create subfolder with same name should succeed", func(t *testing.T) { resp, err = adminClient.Folders.CreateFolder(&models.CreateFolderCommand{ Title: "subfolder", ParentUID: parentUID, }) - require.Error(t, err) - var conflict *folders.CreateFolderConflict - assert.True(t, errors.As(err, &conflict)) - assert.Equal(t, http.StatusConflict, conflict.Code()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.Code()) }) t.Run("create subfolder with same name under other folder should succeed", func(t *testing.T) { @@ -187,14 +178,13 @@ func TestIntegrationNestedFoldersOn(t *testing.T) { assert.Equal(t, other, resp.Payload.ParentUID) subfolderUnderOther := resp.Payload.UID - t.Run("move subfolder to other folder containing folder with that name should fail", func(t *testing.T) { - _, err := adminClient.Folders.MoveFolder(subfolderUnderOther, &models.MoveFolderCommand{ + t.Run("move subfolder to other folder containing folder with the same name should be ok", func(t *testing.T) { + resp, err := adminClient.Folders.MoveFolder(subfolderUnderOther, &models.MoveFolderCommand{ ParentUID: parentUID, }) - require.Error(t, err) - var apiError *runtime.APIError - assert.True(t, errors.As(err, &apiError)) - assert.Equal(t, http.StatusConflict, apiError.Code) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.Code()) + assert.Equal(t, parentUID, resp.Payload.ParentUID) }) t.Run("move subfolder to root should succeed", func(t *testing.T) { diff --git a/pkg/tests/apis/folder/folders_test.go b/pkg/tests/apis/folder/folders_test.go index a5704b179fc..1f0bf2a9d1b 100644 --- a/pkg/tests/apis/folder/folders_test.go +++ b/pkg/tests/apis/folder/folders_test.go @@ -564,7 +564,7 @@ func doCreateDuplicateFolderTest(t *testing.T, helper *apis.K8sTestHelper) { Body: []byte(payload), }, &folder.Folder{}) require.NotEmpty(t, create2.Response) - require.Equal(t, 409, create2.Response.StatusCode) + require.Equal(t, 200, create2.Response.StatusCode) // it is OK } func doCreateEnsureTitleIsTrimmedTest(t *testing.T, helper *apis.K8sTestHelper) { @@ -699,7 +699,6 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) { folderWithTitleEmpty := "{ \"title\": \"\"}" folderWithInvalidUid := "{ \"uid\": \"::::::::::::\", \"title\": \"Another folder\"}" folderWithUIDTooLong := "{ \"uid\": \"asdfghjklqwertyuiopzxcvbnmasdfghjklqwertyuiopzxcvbnmasdfghjklqwertyuiopzxcvbnm\", \"title\": \"Third folder\"}" - folderWithSameName := "{\"title\": \"same name\"}" type testCase struct { description string @@ -770,15 +769,6 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) { expectedFolderSvcError: dashboards.ErrDashboardUidTooLong, permissions: folderCreatePermission, }, - { - description: "folder creation fails given folder service error %s", - input: folderWithSameName, - expectedCode: http.StatusConflict, - expectedMessage: dashboards.ErrFolderSameNameExists.Error(), - expectedFolderSvcError: dashboards.ErrFolderSameNameExists, - createSecondRecord: true, - permissions: folderCreatePermission, - }, { description: "folder creation fails given folder service error %s", input: folderWithoutParentInput,