IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-16 10:24:54 -06:00
Code Issues Packages Projects Releases Wiki Activity

Dashboards: Fix regression when deleting folder (#88311)

Browse Source 
* Fix regression when deleting folder

* Apply suggestion from code review
This commit is contained in:
Sofia Papagiannaki 2024-05-30 14:21:34 +03:00 committed by GitHub
parent f6a1ed3581
commit 4f999f2b6c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 25 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
pkg/services/dashboards/database/database.go
View File

@ -594,24 +594,31 @@ func (d *dashboardStore) deleteDashboard(cmd *dashboards.DeleteDashboardCommand,
return dashboards.ErrDashboardNotFound
}
deletes := []string{
"DELETE FROM dashboard_tag WHERE dashboard_id = ? ",
"DELETE FROM star WHERE dashboard_id = ? ",
"DELETE FROM dashboard WHERE id = ?",
"DELETE FROM playlist_item WHERE type = 'dashboard_by_id' AND value = ?",
"DELETE FROM dashboard_version WHERE dashboard_id = ?",
"DELETE FROM dashboard_provisioning WHERE dashboard_id = ?",
"DELETE FROM dashboard_acl WHERE dashboard_id = ?",
type statement struct {
SQL string
args []any
}
sqlStatements := []statement{
{SQL: "DELETE FROM dashboard_tag WHERE dashboard_id = ? ", args: []any{dashboard.ID}},
{SQL: "DELETE FROM star WHERE dashboard_id = ? ", args: []any{dashboard.ID}},
{SQL: "DELETE FROM dashboard WHERE id = ?", args: []any{dashboard.ID}},
{SQL: "DELETE FROM playlist_item WHERE type = 'dashboard_by_id' AND value = ?", args: []any{dashboard.ID}},
{SQL: "DELETE FROM dashboard_version WHERE dashboard_id = ?", args: []any{dashboard.ID}},
{SQL: "DELETE FROM dashboard_provisioning WHERE dashboard_id = ?", args: []any{dashboard.ID}},
{SQL: "DELETE FROM dashboard_acl WHERE dashboard_id = ?", args: []any{dashboard.ID}},
}
if dashboard.IsFolder {
// if this is a soft delete, we need to skip children deletion.
if !d.features.IsEnabledGlobally(featuremgmt.FlagDashboardRestore) && !cmd.SkipSoftDeletedDashboards {
deletes = append(deletes, "DELETE FROM dashboard WHERE folder_id = ? AND deleted IS NULL")
if !d.features.IsEnabledGlobally(featuremgmt.FlagDashboardRestore) {
sqlStatements = append(sqlStatements, statement{SQL: "DELETE FROM dashboard WHERE org_id = ? AND folder_uid = ? AND is_folder = ? AND deleted IS NULL", args: []any{dashboard.OrgID, dashboard.UID, d.store.GetDialect().BooleanStr(false)}})
if err := d.deleteChildrenDashboardAssociations(sess, &dashboard); err != nil {
return err
}
} else {
// soft delete all dashboards in the folder
sqlStatements = append(sqlStatements, statement{SQL: "UPDATE dashboard SET deleted = ? WHERE org_id = ? AND folder_uid = ? AND is_folder = ? ", args: []any{time.Now(), dashboard.OrgID, dashboard.UID, d.store.GetDialect().BooleanStr(false)}})
}
// remove all access control permission with folder scope
@ -630,8 +637,8 @@ func (d *dashboardStore) deleteDashboard(cmd *dashboards.DeleteDashboardCommand,
return err
}
for _, sql := range deletes {
_, err := sess.Exec(sql, dashboard.ID)
for _, stmnt := range sqlStatements {
_, err := sess.Exec(append([]any{stmnt.SQL}, stmnt.args...)...)
if err != nil {
return err
}

9
pkg/services/dashboards/models.go
View File

@ -224,11 +224,10 @@ type DashboardProvisioning struct {
}
type DeleteDashboardCommand struct {
ID int64
UID string
OrgID int64
ForceDeleteFolderRules bool
SkipSoftDeletedDashboards bool
ID int64
UID string
OrgID int64
ForceDeleteFolderRules bool
}
type DeleteOrphanedProvisionedDashboardsCommand struct {

8
pkg/services/folder/folderimpl/folder.go
View File

@ -852,16 +852,10 @@ func (s *Service) deleteChildrenInFolder(ctx context.Context, orgID int64, folde
}
func (s *Service) legacyDelete(ctx context.Context, cmd *folder.DeleteFolderCommand, folderUIDs []string) error {
if s.features.IsEnabledGlobally(featuremgmt.FlagDashboardRestore) {
if err := s.dashboardStore.SoftDeleteDashboardsInFolders(ctx, cmd.OrgID, folderUIDs); err != nil {
return toFolderError(err)
}
}
// TODO use bulk delete
for _, folderUID := range folderUIDs {
// only hard delete the folder representation in the dashboard store
// nolint:staticcheck
deleteCmd := dashboards.DeleteDashboardCommand{OrgID: cmd.OrgID, UID: folderUID, ForceDeleteFolderRules: cmd.ForceDeleteRules, SkipSoftDeletedDashboards: true}
deleteCmd := dashboards.DeleteDashboardCommand{OrgID: cmd.OrgID, UID: folderUID, ForceDeleteFolderRules: cmd.ForceDeleteRules}
if err := s.dashboardStore.DeleteDashboard(ctx, &deleteCmd); err != nil {
return toFolderError(err)
}

12
pkg/services/folder/folderimpl/folder_test.go
View File

@ -291,11 +291,6 @@ func TestIntegrationFolderService(t *testing.T) {
}).Return(nil).Once()
service.features = featuremgmt.WithFeatures(featuremgmt.FlagDashboardRestore)
var folderUids []string
dashStore.On("SoftDeleteDashboardsInFolders", mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
folderUids = args.Get(2).([]string)
}).Return(nil).Once()
expectedForceDeleteRules := false
err := service.Delete(context.Background(), &folder.DeleteFolderCommand{
UID: f.UID,
@ -307,7 +302,6 @@ func TestIntegrationFolderService(t *testing.T) {
require.NotNil(t, actualCmd)
require.Equal(t, orgID, actualCmd.OrgID)
require.Equal(t, expectedForceDeleteRules, actualCmd.ForceDeleteFolderRules)
require.Equal(t, f.UID, folderUids[0])
})
t.Run("When deleting folder by uid, expectedForceDeleteRules as true, and dashboard Restore turned on should not return access denied error", func(t *testing.T) {
@ -321,11 +315,6 @@ func TestIntegrationFolderService(t *testing.T) {
}).Return(nil).Once()
service.features = featuremgmt.WithFeatures(featuremgmt.FlagDashboardRestore)
var folderUids []string
dashStore.On("SoftDeleteDashboardsInFolders", mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
folderUids = args.Get(2).([]string)
}).Return(nil).Once()
expectedForceDeleteRules := true
err := service.Delete(context.Background(), &folder.DeleteFolderCommand{
UID: f.UID,
@ -337,7 +326,6 @@ func TestIntegrationFolderService(t *testing.T) {
require.NotNil(t, actualCmd)
require.Equal(t, orgID, actualCmd.OrgID)
require.Equal(t, expectedForceDeleteRules, actualCmd.ForceDeleteFolderRules)
require.Equal(t, f.UID, folderUids[0])
})
t.Cleanup(func() {