From 67cdae4b7d80b9fd2d5fabf67172297f7ba56cfe Mon Sep 17 00:00:00 2001 From: Kat Yang <69819079+yangkb09@users.noreply.github.com> Date: Thu, 29 Jun 2023 16:15:38 -0400 Subject: [PATCH] Fix: Change getExistingDashboardByTitleAndFolder to get dashboard by title, not slug (#70723) * Fix: Change getExistingDashboardByTitleAndFolder to get dashboard by title, not slug * test: add tests for get dashboard with existing name, get dashboard with non existing name, get dashboard with existing name in a folder * Update pkg/services/dashboards/database/database_test.go Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com> * require specific error for Should be able to get dashboard with existing name * Update pkg/services/dashboards/database/database_test.go Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com> * implement sofia suggestions to check for specific err, remove logs * give test more specific name * implement daniel suggestion of formatting documentation comment in safe way * fix test title to refer to root directory not specific folder --------- Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com> --- pkg/services/dashboards/database/database.go | 4 +-- .../dashboards/database/database_test.go | 35 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/pkg/services/dashboards/database/database.go b/pkg/services/dashboards/database/database.go index 28874d01085..05bc5362fbd 100644 --- a/pkg/services/dashboards/database/database.go +++ b/pkg/services/dashboards/database/database.go @@ -385,15 +385,15 @@ 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, dialect migrator.Dialect, overwrite, isParentFolderChanged bool) (bool, error) { var existing dashboards.Dashboard - exists, err := sess.Where("org_id=? AND slug=? AND (is_folder=? OR folder_id=?)", dash.OrgID, dash.Slug, + 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 { 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 diff --git a/pkg/services/dashboards/database/database_test.go b/pkg/services/dashboards/database/database_test.go index 5ada585b6ee..3f83e06c8f9 100644 --- a/pkg/services/dashboards/database/database_test.go +++ b/pkg/services/dashboards/database/database_test.go @@ -639,6 +639,41 @@ 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() + cfg.IsFeatureToggleEnabled = func(key string) bool { return false } + quotaService := quotatest.New(false, nil) + dashboardStore, err := ProvideDashboardStore(sqlStore, cfg, testFeatureToggles, tagimpl.ProvideService(sqlStore, cfg), 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}, sqlStore.GetDialect(), 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}, sqlStore.GetDialect(), 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, false, "prod", "webapp") + err = sqlStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + _, err = getExistingDashboardByTitleAndFolder(sess, &dashboards.Dashboard{Title: savedDash.Title, FolderID: savedFolder.ID, OrgID: 1}, sqlStore.GetDialect(), false, false) + return err + }) + require.ErrorIs(t, err, dashboards.ErrDashboardWithSameNameInFolderExists) + }) +} + func insertTestRule(t *testing.T, sqlStore db.DB, foderOrgID int64, folderUID string) { err := sqlStore.WithDbSession(context.Background(), func(sess *db.Session) error { type alertQuery struct {