Dashboards: Remove unique name constraints (#90687)

This commit is contained in:
Ryan McKinley 2024-10-29 08:58:39 +03:00 committed by GitHub
parent c8238c7914
commit 2f40fd6741
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 51 additions and 174 deletions

View File

@ -35,8 +35,7 @@ func ToFolderErrorResponse(err error) response.Response {
return response.JSON(http.StatusNotFound, util.DynMap{"status": "not-found", "message": dashboards.ErrFolderNotFound.Error()}) return response.JSON(http.StatusNotFound, util.DynMap{"status": "not-found", "message": dashboards.ErrFolderNotFound.Error()})
} }
if errors.Is(err, dashboards.ErrFolderSameNameExists) || if errors.Is(err, dashboards.ErrFolderWithSameUIDExists) {
errors.Is(err, dashboards.ErrFolderWithSameUIDExists) {
return response.Error(http.StatusConflict, err.Error(), nil) return response.Error(http.StatusConflict, err.Error(), nil)
} }

View File

@ -476,13 +476,10 @@ func TestDashboardAPIEndpoint(t *testing.T) {
{SaveError: dashboards.ErrDashboardNotFound, ExpectedStatusCode: http.StatusNotFound}, {SaveError: dashboards.ErrDashboardNotFound, ExpectedStatusCode: http.StatusNotFound},
{SaveError: dashboards.ErrFolderNotFound, ExpectedStatusCode: http.StatusBadRequest}, {SaveError: dashboards.ErrFolderNotFound, ExpectedStatusCode: http.StatusBadRequest},
{SaveError: dashboards.ErrDashboardWithSameUIDExists, ExpectedStatusCode: http.StatusBadRequest}, {SaveError: dashboards.ErrDashboardWithSameUIDExists, ExpectedStatusCode: http.StatusBadRequest},
{SaveError: dashboards.ErrDashboardWithSameNameInFolderExists, ExpectedStatusCode: http.StatusPreconditionFailed},
{SaveError: dashboards.ErrDashboardVersionMismatch, ExpectedStatusCode: http.StatusPreconditionFailed}, {SaveError: dashboards.ErrDashboardVersionMismatch, ExpectedStatusCode: http.StatusPreconditionFailed},
{SaveError: dashboards.ErrDashboardTitleEmpty, ExpectedStatusCode: http.StatusBadRequest}, {SaveError: dashboards.ErrDashboardTitleEmpty, ExpectedStatusCode: http.StatusBadRequest},
{SaveError: dashboards.ErrDashboardFolderCannotHaveParent, ExpectedStatusCode: http.StatusBadRequest}, {SaveError: dashboards.ErrDashboardFolderCannotHaveParent, ExpectedStatusCode: http.StatusBadRequest},
{SaveError: dashboards.ErrDashboardTypeMismatch, 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.ErrDashboardFolderNameExists, ExpectedStatusCode: http.StatusBadRequest},
{SaveError: dashboards.ErrDashboardUpdateAccessDenied, ExpectedStatusCode: http.StatusForbidden}, {SaveError: dashboards.ErrDashboardUpdateAccessDenied, ExpectedStatusCode: http.StatusForbidden},
{SaveError: dashboards.ErrDashboardInvalidUid, ExpectedStatusCode: http.StatusBadRequest}, {SaveError: dashboards.ErrDashboardInvalidUid, ExpectedStatusCode: http.StatusBadRequest},

View File

@ -88,13 +88,6 @@ func TestFoldersCreateAPIEndpoint(t *testing.T) {
expectedFolderSvcError: dashboards.ErrDashboardUidTooLong, expectedFolderSvcError: dashboards.ErrDashboardUidTooLong,
permissions: []accesscontrol.Permission{{Action: dashboards.ActionFoldersCreate}}, 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", description: "folder creation fails given folder service error %s",
input: folderWithoutParentInput, input: folderWithoutParentInput,
@ -203,12 +196,6 @@ func TestFoldersUpdateAPIEndpoint(t *testing.T) {
expectedFolderSvcError: dashboards.ErrDashboardUidTooLong, expectedFolderSvcError: dashboards.ErrDashboardUidTooLong,
permissions: []accesscontrol.Permission{{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}}, 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", description: "folder updating fails given folder service error %s",
expectedCode: http.StatusForbidden, expectedCode: http.StatusForbidden,

View File

@ -84,12 +84,6 @@ func (d *dashboardStore) ValidateDashboardBeforeSave(ctx context.Context, dashbo
return err return err
} }
isParentFolderChanged, err = getExistingDashboardByTitleAndFolder(sess, dashboard, overwrite,
isParentFolderChanged)
if err != nil {
return err
}
return nil return nil
}) })
if err != nil { if err != nil {
@ -353,49 +347,6 @@ func getExistingDashboardByIDOrUIDForUpdate(sess *db.Session, dash *dashboards.D
return isParentFolderChanged, nil 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) { func saveDashboard(sess *db.Session, cmd *dashboards.SaveDashboardCommand, emitEntityEvent bool) (*dashboards.Dashboard, error) {
dash := cmd.GetDashboardModel() 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} dashboard := dashboards.Dashboard{OrgID: query.OrgID, ID: query.ID, UID: query.UID}
mustCols := []string{} mustCols := []string{}
if query.Title != nil { if query.Title != nil { // nolint:staticcheck
dashboard.Title = *query.Title dashboard.Title = *query.Title // nolint:staticcheck
mustCols = append(mustCols, "title") mustCols = append(mustCols, "title")
} }
@ -806,8 +757,7 @@ func (d *dashboardStore) GetDashboard(ctx context.Context, query *dashboards.Get
dashboard.FolderUID = *query.FolderUID dashboard.FolderUID = *query.FolderUID
mustCols = append(mustCols, "folder_uid") mustCols = append(mustCols, "folder_uid")
} else if query.FolderID != nil { // nolint:staticcheck } else if query.FolderID != nil { // nolint:staticcheck
// nolint:staticcheck dashboard.FolderID = *query.FolderID // nolint:staticcheck
dashboard.FolderID = *query.FolderID
mustCols = append(mustCols, "folder_id") mustCols = append(mustCols, "folder_id")
metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Dashboard).Inc() metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Dashboard).Inc()
} }

View File

@ -781,40 +781,6 @@ func TestIntegrationDashboard_Filter(t *testing.T) {
assert.Equal(t, dashB.ID, results[0].ID) 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) { func TestIntegrationFindDashboardsByTitle(t *testing.T) {
if testing.Short() { if testing.Short() {
t.Skip("skipping integration test") t.Skip("skipping integration test")

View File

@ -98,6 +98,7 @@ func AddDashboardFolderMigrations(mg *migrator.Migrator) {
mg.AddMigration("Delete unique index for dashboard_org_id_folder_uid_title", &DummyMigration{}) 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{ 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, 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{ 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"}, 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,
}))
} }

View File

@ -32,11 +32,6 @@ var (
Reason: "A dashboard with the same uid already exists", Reason: "A dashboard with the same uid already exists",
StatusCode: 400, StatusCode: 400,
} }
ErrDashboardWithSameNameInFolderExists = DashboardErr{
Reason: "A dashboard with the same name in the folder already exists",
StatusCode: 412,
Status: "name-exists",
}
ErrDashboardVersionMismatch = DashboardErr{ ErrDashboardVersionMismatch = DashboardErr{
Reason: "The dashboard has been changed by someone else", Reason: "The dashboard has been changed by someone else",
StatusCode: 412, StatusCode: 412,
@ -59,15 +54,6 @@ var (
Reason: "Dashboard cannot be changed to a folder", Reason: "Dashboard cannot be changed to a folder",
StatusCode: 400, 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{ ErrDashboardFolderNameExists = DashboardErr{
Reason: "A folder with that name already exists", Reason: "A folder with that name already exists",
StatusCode: 400, StatusCode: 400,
@ -128,7 +114,6 @@ var (
ErrFolderTitleEmpty = errors.New("folder title cannot be empty") ErrFolderTitleEmpty = errors.New("folder title cannot be empty")
ErrFolderWithSameUIDExists = errors.New("a folder/dashboard with the same uid already exists") ErrFolderWithSameUIDExists = errors.New("a folder/dashboard with the same uid already exists")
ErrFolderInvalidUID = errors.New("invalid uid for folder provided") 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") ErrFolderAccessDenied = errors.New("access denied to folder")
ErrUserIsNotSignedInToOrg = errors.New("user is not signed in to organization") ErrUserIsNotSignedInToOrg = errors.New("user is not signed in to organization")
ErrMoveAccessDenied = errutil.Forbidden("folders.forbiddenMove", errutil.WithPublicMessage("Access denied to the destination folder")) ErrMoveAccessDenied = errutil.Forbidden("folders.forbiddenMove", errutil.WithPublicMessage("Access denied to the destination folder"))

View File

@ -252,8 +252,9 @@ type DeleteOrphanedProvisionedDashboardsCommand struct {
// //
// Multiple constraints can be combined. // Multiple constraints can be combined.
type GetDashboardQuery struct { type GetDashboardQuery struct {
ID int64 ID int64
UID string UID string
// Deprecated: this is no-longer a unique constraint and should not be used
Title *string Title *string
// Deprecated: use FolderUID instead // Deprecated: use FolderUID instead
FolderID *int64 FolderID *int64

View File

@ -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", 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) { canSave, func(t *testing.T, sc *permissionScenarioContext) {
t.Skip()
cmd := dashboards.SaveDashboardCommand{ cmd := dashboards.SaveDashboardCommand{
OrgID: testOrgID, OrgID: testOrgID,
Dashboard: simplejson.NewFromAny(map[string]any{ Dashboard: simplejson.NewFromAny(map[string]any{
@ -568,7 +570,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) {
} }
err := callSaveWithError(t, cmd, sc.sqlStore) 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", 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) 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, 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) 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, permissionScenario(t, "When creating a dashboard with same name as dashboard in other folder", canSave,
func(t *testing.T, sc *permissionScenarioContext) { func(t *testing.T, sc *permissionScenarioContext) {
t.Skip()
cmd := dashboards.SaveDashboardCommand{ cmd := dashboards.SaveDashboardCommand{
OrgID: testOrgID, OrgID: testOrgID,
Dashboard: simplejson.NewFromAny(map[string]any{ 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, permissionScenario(t, "When creating a dashboard with same name as dashboard in General folder", canSave,
func(t *testing.T, sc *permissionScenarioContext) { func(t *testing.T, sc *permissionScenarioContext) {
t.Skip()
cmd := dashboards.SaveDashboardCommand{ cmd := dashboards.SaveDashboardCommand{
OrgID: testOrgID, OrgID: testOrgID,
Dashboard: simplejson.NewFromAny(map[string]any{ Dashboard: simplejson.NewFromAny(map[string]any{
@ -815,7 +821,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) {
} }
err := callSaveWithError(t, cmd, sc.sqlStore) 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, 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) err := callSaveWithError(t, cmd, sc.sqlStore)
assert.Equal(t, dashboards.ErrDashboardFolderWithSameNameAsDashboard, err) require.NoError(t, err)
}) })
}) })
}) })

View File

@ -1009,9 +1009,6 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
NewParentUID: &cmd.NewParentUID, NewParentUID: &cmd.NewParentUID,
SignedInUser: cmd.SignedInUser, SignedInUser: cmd.SignedInUser,
}); err != nil { }); 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) 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 // bypass optimistic locking used for dashboards
Overwrite: true, Overwrite: true,
}); err != nil { }); 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) return folder.ErrInternal.Errorf("failed to move legacy folder: %w", err)
} }
@ -1393,10 +1387,6 @@ func toFolderError(err error) error {
return dashboards.ErrFolderAccessDenied return dashboards.ErrFolderAccessDenied
} }
if errors.Is(err, dashboards.ErrDashboardWithSameNameInFolderExists) {
return dashboards.ErrFolderSameNameExists
}
if errors.Is(err, dashboards.ErrDashboardWithSameUIDExists) { if errors.Is(err, dashboards.ErrDashboardWithSameUIDExists) {
return dashboards.ErrFolderWithSameUIDExists return dashboards.ErrFolderWithSameUIDExists
} }

View File

@ -387,7 +387,6 @@ func TestIntegrationFolderService(t *testing.T) {
}{ }{
{ActualError: dashboards.ErrDashboardTitleEmpty, ExpectedError: dashboards.ErrFolderTitleEmpty}, {ActualError: dashboards.ErrDashboardTitleEmpty, ExpectedError: dashboards.ErrFolderTitleEmpty},
{ActualError: dashboards.ErrDashboardUpdateAccessDenied, ExpectedError: dashboards.ErrFolderAccessDenied}, {ActualError: dashboards.ErrDashboardUpdateAccessDenied, ExpectedError: dashboards.ErrFolderAccessDenied},
{ActualError: dashboards.ErrDashboardWithSameNameInFolderExists, ExpectedError: dashboards.ErrFolderSameNameExists},
{ActualError: dashboards.ErrDashboardWithSameUIDExists, ExpectedError: dashboards.ErrFolderWithSameUIDExists}, {ActualError: dashboards.ErrDashboardWithSameUIDExists, ExpectedError: dashboards.ErrFolderWithSameUIDExists},
{ActualError: dashboards.ErrDashboardVersionMismatch, ExpectedError: dashboards.ErrFolderVersionMismatch}, {ActualError: dashboards.ErrDashboardVersionMismatch, ExpectedError: dashboards.ErrFolderVersionMismatch},
{ActualError: dashboards.ErrDashboardNotFound, ExpectedError: dashboards.ErrFolderNotFound}, {ActualError: dashboards.ErrDashboardNotFound, ExpectedError: dashboards.ErrFolderNotFound},

View File

@ -262,8 +262,11 @@ func (fr *FileReader) saveDashboard(ctx context.Context, path string, folderID i
&dashboards.GetDashboardQuery{ &dashboards.GetDashboardQuery{
OrgID: jsonFile.dashboard.OrgID, OrgID: jsonFile.dashboard.OrgID,
UID: jsonFile.dashboard.Dashboard.UID, UID: jsonFile.dashboard.Dashboard.UID,
Title: &jsonFile.dashboard.Dashboard.Title,
FolderUID: util.Pointer(""), FolderUID: util.Pointer(""),
// provisioning depends on unique names
//nolint:staticcheck
Title: &jsonFile.dashboard.Dashboard.Title,
}, },
) )
if err != nil { if err != nil {
@ -349,6 +352,8 @@ func (fr *FileReader) getOrCreateFolder(ctx context.Context, cfg *config, servic
if cfg.FolderUID != "" { if cfg.FolderUID != "" {
cmd.UID = cfg.FolderUID cmd.UID = cfg.FolderUID
} else { } else {
// provisioning depends on unique names
//nolint:staticcheck
cmd.Title = &folderName cmd.Title = &folderName
} }

View File

@ -172,6 +172,7 @@ func addDashboardMigration(mg *Migrator) {
{Name: "title", Type: DB_NVarchar, Length: 189, Nullable: false}, {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{ mg.AddMigration("Add unique index for dashboard_org_id_title_folder_id", NewAddIndexMigration(dashboardV2, &Index{
Cols: []string{"org_id", "folder_id", "title"}, Type: UniqueIndex, Cols: []string{"org_id", "folder_id", "title"}, Type: UniqueIndex,
})) }))

View File

@ -80,6 +80,12 @@ func addFolderMigrations(mg *migrator.Migrator) {
mg.AddMigration("Remove index IDX_folder_parent_uid_org_id", migrator.NewDropIndexMigration(folderv1(), &migrator.Index{ mg.AddMigration("Remove index IDX_folder_parent_uid_org_id", migrator.NewDropIndexMigration(folderv1(), &migrator.Index{
Cols: []string{"parent_uid", "org_id"}, 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 { func folderv1() migrator.Table {

View File

@ -2,12 +2,9 @@ package folders
import ( import (
"context" "context"
"errors"
"net/http" "net/http"
"testing" "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-openapi-client-go/models"
"github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/tracing"
@ -92,14 +89,12 @@ func TestIntegrationCreateFolder(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.Code()) 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{ _, err := adminClient.Folders.CreateFolder(&models.CreateFolderCommand{
Title: "folder", Title: "folder",
}) })
require.Error(t, err) require.NoError(t, err)
var conflict *folders.CreateFolderConflict require.Equal(t, http.StatusOK, resp.Code())
assert.True(t, errors.As(err, &conflict))
assert.Equal(t, http.StatusConflict, conflict.Code())
}) })
}) })
} }
@ -133,14 +128,12 @@ func TestIntegrationNestedFoldersOn(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.Code()) 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{ _, err := adminClient.Folders.CreateFolder(&models.CreateFolderCommand{
Title: "folder", Title: "folder",
}) })
require.Error(t, err) require.NoError(t, err)
var conflict *folders.CreateFolderConflict require.Equal(t, http.StatusOK, resp.Code())
assert.True(t, errors.As(err, &conflict))
assert.Equal(t, http.StatusConflict, conflict.Code())
}) })
}) })
@ -159,15 +152,13 @@ func TestIntegrationNestedFoldersOn(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.Code()) 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{ resp, err = adminClient.Folders.CreateFolder(&models.CreateFolderCommand{
Title: "subfolder", Title: "subfolder",
ParentUID: parentUID, ParentUID: parentUID,
}) })
require.Error(t, err) require.NoError(t, err)
var conflict *folders.CreateFolderConflict require.Equal(t, http.StatusOK, resp.Code())
assert.True(t, errors.As(err, &conflict))
assert.Equal(t, http.StatusConflict, conflict.Code())
}) })
t.Run("create subfolder with same name under other folder should succeed", func(t *testing.T) { 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) assert.Equal(t, other, resp.Payload.ParentUID)
subfolderUnderOther := resp.Payload.UID subfolderUnderOther := resp.Payload.UID
t.Run("move subfolder to other folder containing folder with that name should fail", func(t *testing.T) { t.Run("move subfolder to other folder containing folder with the same name should be ok", func(t *testing.T) {
_, err := adminClient.Folders.MoveFolder(subfolderUnderOther, &models.MoveFolderCommand{ resp, err := adminClient.Folders.MoveFolder(subfolderUnderOther, &models.MoveFolderCommand{
ParentUID: parentUID, ParentUID: parentUID,
}) })
require.Error(t, err) require.NoError(t, err)
var apiError *runtime.APIError assert.Equal(t, http.StatusOK, resp.Code())
assert.True(t, errors.As(err, &apiError)) assert.Equal(t, parentUID, resp.Payload.ParentUID)
assert.Equal(t, http.StatusConflict, apiError.Code)
}) })
t.Run("move subfolder to root should succeed", func(t *testing.T) { t.Run("move subfolder to root should succeed", func(t *testing.T) {

View File

@ -564,7 +564,7 @@ func doCreateDuplicateFolderTest(t *testing.T, helper *apis.K8sTestHelper) {
Body: []byte(payload), Body: []byte(payload),
}, &folder.Folder{}) }, &folder.Folder{})
require.NotEmpty(t, create2.Response) 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) { func doCreateEnsureTitleIsTrimmedTest(t *testing.T, helper *apis.K8sTestHelper) {
@ -699,7 +699,6 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) {
folderWithTitleEmpty := "{ \"title\": \"\"}" folderWithTitleEmpty := "{ \"title\": \"\"}"
folderWithInvalidUid := "{ \"uid\": \"::::::::::::\", \"title\": \"Another folder\"}" folderWithInvalidUid := "{ \"uid\": \"::::::::::::\", \"title\": \"Another folder\"}"
folderWithUIDTooLong := "{ \"uid\": \"asdfghjklqwertyuiopzxcvbnmasdfghjklqwertyuiopzxcvbnmasdfghjklqwertyuiopzxcvbnm\", \"title\": \"Third folder\"}" folderWithUIDTooLong := "{ \"uid\": \"asdfghjklqwertyuiopzxcvbnmasdfghjklqwertyuiopzxcvbnmasdfghjklqwertyuiopzxcvbnm\", \"title\": \"Third folder\"}"
folderWithSameName := "{\"title\": \"same name\"}"
type testCase struct { type testCase struct {
description string description string
@ -770,15 +769,6 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) {
expectedFolderSvcError: dashboards.ErrDashboardUidTooLong, expectedFolderSvcError: dashboards.ErrDashboardUidTooLong,
permissions: folderCreatePermission, 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", description: "folder creation fails given folder service error %s",
input: folderWithoutParentInput, input: folderWithoutParentInput,