Library elements: remove usage of dashboard table on get (#99619)

This commit is contained in:
Stephanie Hingtgen
2025-01-27 16:39:18 -07:00
committed by GitHub
parent fd85ddf647
commit 61c5b4a25e
3 changed files with 98 additions and 12 deletions

View File

@ -79,15 +79,12 @@ func syncFieldsWithModel(libraryElement *model.LibraryElement) error {
return nil
}
func GetLibraryElement(dialect migrator.Dialect, session *db.Session, uid string, orgID int64) (model.LibraryElementWithMeta, error) {
func (l *LibraryElementService) GetLibraryElement(c context.Context, signedInUser identity.Requester, session *db.Session, uid string) (model.LibraryElementWithMeta, error) {
elements := make([]model.LibraryElementWithMeta, 0)
sql := selectLibraryElementDTOWithMeta +
", coalesce(dashboard.title, 'General') AS folder_name" +
", coalesce(dashboard.uid, '') AS folder_uid" +
getFromLibraryElementDTOWithMeta(dialect) +
" LEFT JOIN dashboard AS dashboard ON dashboard.id = le.folder_id" +
getFromLibraryElementDTOWithMeta(l.SQLStore.GetDialect()) +
" WHERE le.uid=? AND le.org_id=?"
sess := session.SQL(sql, uid, orgID)
sess := session.SQL(sql, uid, signedInUser.GetOrgID())
err := sess.Find(&elements)
if err != nil {
return model.LibraryElementWithMeta{}, err
@ -99,6 +96,19 @@ func GetLibraryElement(dialect migrator.Dialect, session *db.Session, uid string
return model.LibraryElementWithMeta{}, fmt.Errorf("found %d elements, while expecting at most one", len(elements))
}
// get the folder title
f, err := l.folderService.Get(c, &folder.GetFolderQuery{
OrgID: elements[0].OrgID,
UID: &elements[0].FolderUID,
SignedInUser: signedInUser,
})
if err == nil {
elements[0].FolderName = f.Title
} else {
// default to General if we cannot find the folder
elements[0].FolderName = "General"
}
return elements[0], nil
}
@ -220,7 +230,7 @@ func (l *LibraryElementService) createLibraryElement(c context.Context, signedIn
func (l *LibraryElementService) deleteLibraryElement(c context.Context, signedInUser identity.Requester, uid string) (int64, error) {
var elementID int64
err := l.SQLStore.WithTransactionalDbSession(c, func(session *db.Session) error {
element, err := GetLibraryElement(l.SQLStore.GetDialect(), session, uid, signedInUser.GetOrgID())
element, err := l.GetLibraryElement(c, signedInUser, session, uid)
if err != nil {
return err
}
@ -593,7 +603,7 @@ func (l *LibraryElementService) patchLibraryElement(c context.Context, signedInU
return model.LibraryElementDTO{}, err
}
err := l.SQLStore.WithTransactionalDbSession(c, func(session *db.Session) error {
elementInDB, err := GetLibraryElement(l.SQLStore.GetDialect(), session, uid, signedInUser.GetOrgID())
elementInDB, err := l.GetLibraryElement(c, signedInUser, session, uid)
if err != nil {
return err
}
@ -610,7 +620,7 @@ func (l *LibraryElementService) patchLibraryElement(c context.Context, signedInU
return model.ErrLibraryElementUIDTooLong
}
_, err := GetLibraryElement(l.SQLStore.GetDialect(), session, updateUID, signedInUser.GetOrgID())
_, err := l.GetLibraryElement(c, signedInUser, session, updateUID)
if !errors.Is(err, model.ErrLibraryElementNotFound) {
return model.ErrLibraryElementAlreadyExists
}
@ -705,7 +715,7 @@ func (l *LibraryElementService) getConnections(c context.Context, signedInUser i
}
err = l.SQLStore.WithDbSession(c, func(session *db.Session) error {
element, err := GetLibraryElement(l.SQLStore.GetDialect(), session, uid, signedInUser.GetOrgID())
element, err := l.GetLibraryElement(c, signedInUser, session, uid)
if err != nil {
return err
}
@ -832,7 +842,7 @@ func (l *LibraryElementService) connectElementsToDashboardID(c context.Context,
return err
}
for _, elementUID := range elementUIDs {
element, err := GetLibraryElement(l.SQLStore.GetDialect(), session, elementUID, signedInUser.GetOrgID())
element, err := l.GetLibraryElement(c, signedInUser, session, elementUID)
if err != nil {
return err
}

View File

@ -1,12 +1,15 @@
package libraryelements
import (
"encoding/json"
"testing"
"github.com/google/go-cmp/cmp"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/kinds/librarypanel"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/libraryelements/model"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/web"
@ -91,6 +94,37 @@ func TestGetLibraryElement(t *testing.T) {
}
})
scenarioWithPanel(t, "When an admin tries to get a library panel that exists, but the original folder does not, it should succeed and return correct result",
func(t *testing.T, sc scenarioContext) {
b, err := json.Marshal(map[string]string{"test": "test"})
require.NoError(t, err)
newFolder := createFolder(t, sc, "NewFolder", nil)
sc.reqContext.SignedInUser.Permissions[sc.reqContext.OrgID][dashboards.ActionFoldersRead] = []string{dashboards.ScopeFoldersAll}
sc.reqContext.SignedInUser.Permissions[sc.reqContext.OrgID][dashboards.ActionFoldersDelete] = []string{dashboards.ScopeFoldersAll}
result, err := sc.service.createLibraryElement(sc.reqContext.Req.Context(), sc.reqContext.SignedInUser, model.CreateLibraryElementCommand{
FolderID: newFolder.ID, // nolint:staticcheck
FolderUID: &newFolder.UID,
Name: "Testing Library Panel With Deleted Folder",
Kind: 1,
Model: b,
UID: "panel-with-deleted-folder",
})
require.NoError(t, err)
err = sc.service.folderService.Delete(sc.reqContext.Req.Context(), &folder.DeleteFolderCommand{
UID: newFolder.UID,
OrgID: sc.reqContext.OrgID,
SignedInUser: sc.reqContext.SignedInUser,
})
require.NoError(t, err)
err = sc.sqlStore.WithDbSession(sc.reqContext.Req.Context(), func(session *db.Session) error {
elem, err := sc.service.GetLibraryElement(sc.reqContext.Req.Context(), sc.reqContext.SignedInUser, session, result.UID)
require.NoError(t, err)
require.Equal(t, elem.FolderName, "General")
return nil
})
require.NoError(t, err)
})
scenarioWithPanel(t, "When an admin tries to get a connected library panel, it should succeed and return correct connected dashboards",
func(t *testing.T, sc scenarioContext) {
dashJSON := map[string]any{

View File

@ -32,10 +32,13 @@ import (
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/folder/folderimpl"
"github.com/grafana/grafana/pkg/services/folder/foldertest"
"github.com/grafana/grafana/pkg/services/guardian"
"github.com/grafana/grafana/pkg/services/libraryelements/model"
ngstore "github.com/grafana/grafana/pkg/services/ngalert/store"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgimpl"
"github.com/grafana/grafana/pkg/services/publicdashboards"
"github.com/grafana/grafana/pkg/services/quota/quotatest"
"github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest"
"github.com/grafana/grafana/pkg/services/tag/tagimpl"
@ -198,6 +201,39 @@ func TestGetLibraryPanelConnections(t *testing.T) {
t.Fatalf("Result mismatch (-want +got):\n%s", diff)
}
})
scenarioWithPanel(t, "When an admin tries to create a connection with an element that exists, but the original folder does not, it should still succeed",
func(t *testing.T, sc scenarioContext) {
b, err := json.Marshal(map[string]string{"test": "test"})
require.NoError(t, err)
newFolder := createFolder(t, sc, "NewFolder", nil)
sc.reqContext.SignedInUser.Permissions[sc.reqContext.OrgID][dashboards.ActionFoldersRead] = []string{dashboards.ScopeFoldersAll}
sc.reqContext.SignedInUser.Permissions[sc.reqContext.OrgID][dashboards.ActionFoldersDelete] = []string{dashboards.ScopeFoldersAll}
_, err = sc.service.createLibraryElement(sc.reqContext.Req.Context(), sc.reqContext.SignedInUser, model.CreateLibraryElementCommand{
FolderID: newFolder.ID, // nolint:staticcheck
FolderUID: &newFolder.UID,
Name: "Testing Library Panel With Deleted Folder",
Kind: 1,
Model: b,
UID: "panel-with-deleted-folder",
})
require.NoError(t, err)
err = sc.service.folderService.Delete(sc.reqContext.Req.Context(), &folder.DeleteFolderCommand{
UID: newFolder.UID,
OrgID: sc.reqContext.OrgID,
SignedInUser: sc.reqContext.SignedInUser,
})
require.NoError(t, err)
dash := dashboards.Dashboard{
Title: "Testing create element",
Data: simplejson.NewFromAny(map[string]any{}),
}
// nolint:staticcheck
dashInDB := createDashboard(t, sc.sqlStore, sc.user, &dash, sc.folder.ID, sc.folder.UID)
err = sc.service.ConnectElementsToDashboard(sc.reqContext.Req.Context(), sc.reqContext.SignedInUser, []string{sc.initialResult.Result.UID}, dashInDB.ID)
require.NoError(t, err)
})
}
type libraryElement struct {
@ -477,9 +513,15 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo
dashboardPermissions := acmock.NewMockedPermissionsService()
folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
fStore := folderimpl.ProvideStore(sqlStore)
publicDash := &publicdashboards.FakePublicDashboardServiceWrapper{}
publicDash.On("DeleteByDashboardUIDs", mock.Anything, mock.Anything, mock.Anything).Return(nil)
folderSvc := folderimpl.ProvideService(
fStore, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore,
nil, sqlStore, features, supportbundlestest.NewFakeBundleService(), nil, cfg, nil, tracing.InitializeTracerForTest())
nil, sqlStore, features, supportbundlestest.NewFakeBundleService(), publicDash, cfg, nil, tracing.InitializeTracerForTest())
alertStore, err := ngstore.ProvideDBStore(cfg, features, sqlStore, &foldertest.FakeService{}, &dashboards.FakeDashboardService{}, ac, bus.ProvideBus(tracing.InitializeTracerForTest()))
require.NoError(t, err)
err = folderSvc.RegisterService(alertStore)
require.NoError(t, err)
dashService, dashSvcErr := dashboardservice.ProvideDashboardServiceImpl(
cfg, dashboardStore, folderStore,
features, folderPermissions, ac,