From 078ce6a28955891cd517ceeaf9d14791a59d669a Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Mon, 27 Jan 2025 13:37:38 -0700 Subject: [PATCH] Library elements: Delete orphaned connections with the dashboard service (#99612) --- .../dashboards/service/dashboard_service.go | 4 +- pkg/services/libraryelements/database.go | 40 +++++++++++++------ .../libraryelements_delete_test.go | 10 +++++ 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index 3a46f051a7d..0c90a37542c 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -1074,7 +1074,9 @@ func (dr *DashboardServiceImpl) GetDashboardUIDByID(ctx context.Context, query * return nil, err } - if len(result) != 1 { + if len(result) == 0 { + return nil, dashboards.ErrDashboardNotFound + } else if len(result) > 1 { return nil, fmt.Errorf("unexpected number of dashboards found: %d. desired: 1", len(result)) } diff --git a/pkg/services/libraryelements/database.go b/pkg/services/libraryelements/database.go index c68ee55c2a1..057a7e3783c 100644 --- a/pkg/services/libraryelements/database.go +++ b/pkg/services/libraryelements/database.go @@ -36,16 +36,6 @@ SELECT DISTINCT , (SELECT COUNT(connection_id) FROM ` + model.LibraryElementConnectionTableName + ` WHERE element_id = le.id AND kind=1) AS connected_dashboards` ) -// redundant SELECT to trick mysql's optimizer -const deleteInvalidConnections = ` -DELETE FROM library_element_connection -WHERE connection_id IN ( - SELECT connection_id FROM ( - SELECT connection_id as id FROM library_element_connection - WHERE element_id=? AND connection_id NOT IN (SELECT id as connection_id from dashboard) - ) as dummy -)` - func getFromLibraryElementDTOWithMeta(dialect migrator.Dialect) string { user := dialect.Quote("user") userJoin := ` @@ -243,11 +233,37 @@ func (l *LibraryElementService) deleteLibraryElement(c context.Context, signedIn } } - // Delete any hanging/invalid connections - if _, err = session.Exec(deleteInvalidConnections, element.ID); err != nil { + dashboardIDs := []int64{} + // get all connections for this element + if err := session.SQL("SELECT connection_id FROM library_element_connection where element_id = ?", element.ID).Find(&dashboardIDs); err != nil { return err } + // then find the dashboards that were supposed to be connected to this element + _, requester := identity.WithServiceIdentitiy(c, signedInUser.GetOrgID()) + dashs, err := l.dashboardsService.FindDashboards(c, &dashboards.FindPersistedDashboardsQuery{ + OrgId: signedInUser.GetOrgID(), + DashboardIds: dashboardIDs, + SignedInUser: requester, // a user may be able to delete a library element but not read all dashboards. We still need to run this check, so we don't allow deleting elements if dashboards are connected + }) + if err != nil { + return err + } + + foundDashes := make([]int64, len(dashs)) + for i, d := range dashs { + foundDashes[i] = d.ID + } + + // delete any connections that are orphaned for this element (i.e. the dashboard was deleted) + session.Table("library_element_connection") + session.Where("element_id = ?", element.ID) + session.NotIn("connection_id", foundDashes) + if _, err = session.Delete(model.LibraryElementConnectionWithMeta{}); err != nil { + return err + } + + // now try to delete the element, but fail if it is connected to any dashboards var connectionIDs []struct { ConnectionID int64 `xorm:"connection_id"` } diff --git a/pkg/services/libraryelements/libraryelements_delete_test.go b/pkg/services/libraryelements/libraryelements_delete_test.go index 597787d6ee0..4246b0ac66d 100644 --- a/pkg/services/libraryelements/libraryelements_delete_test.go +++ b/pkg/services/libraryelements/libraryelements_delete_test.go @@ -82,4 +82,14 @@ func TestDeleteLibraryElement(t *testing.T) { resp := sc.service.deleteHandler(sc.reqContext) require.Equal(t, 403, resp.Status()) }) + + scenarioWithPanel(t, "When an admin tries to delete a library panel that is connected to a non-existent dashboard, it should succeed", + func(t *testing.T, sc scenarioContext) { + err := sc.service.ConnectElementsToDashboard(sc.reqContext.Req.Context(), sc.reqContext.SignedInUser, []string{sc.initialResult.Result.UID}, 9999999) + require.NoError(t, err) + + sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": sc.initialResult.Result.UID}) + resp := sc.service.deleteHandler(sc.reqContext) + require.Equal(t, 200, resp.Status()) + }) }