Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pubdash: Email sharing handle dashboard deleted #64247

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
490bcdc
pubdash service handles dashboard delete WIP
owensmallwood Feb 28, 2023
cf1e9a5
Merge branch 'main' into owensmallwood/pubdash-email-sharing-handle-d…
owensmallwood Mar 6, 2023
a2b0a7f
Merge branch 'main' into owensmallwood/pubdash-email-sharing-handle-d…
owensmallwood Mar 6, 2023
ce8c069
updates function name
owensmallwood Mar 6, 2023
27f2875
regenerates store mock
owensmallwood Mar 6, 2023
e9b3fc4
adds service tests
owensmallwood Mar 6, 2023
ca50c4c
adds database tests for OSS
owensmallwood Mar 6, 2023
22d2dc7
regenerates service and service wrapper mocks
owensmallwood Mar 6, 2023
b82640b
adds log when call fails to delete pubdash using pubdash service
owensmallwood Mar 6, 2023
35c635e
Merge branch 'main' into owensmallwood/pubdash-email-sharing-handle-d…
owensmallwood Mar 6, 2023
bb1461c
Merge branch 'main' into owensmallwood/pubdash-email-sharing-handle-d…
owensmallwood Mar 7, 2023
4e7bfaa
renames function to DeleteByDashboard
owensmallwood Mar 7, 2023
92bf3ef
refactors the Delete() method on the pubdash service to call DeleteBy…
owensmallwood Mar 7, 2023
b09b8d6
Merge branch 'main' into owensmallwood/pubdash-email-sharing-handle-d…
owensmallwood Mar 7, 2023
940f63d
fixes failing tests
owensmallwood Mar 7, 2023
b80ebfc
regenerates mock
owensmallwood Mar 7, 2023
e096505
renames a few functions, fixes query, removes pubdash deletes from da…
owensmallwood Mar 8, 2023
a3763bf
removes tests from dashboard service for making sure pubdash gets del…
owensmallwood Mar 8, 2023
1cd592e
fixes issue for the case where pubdash is not found. Adds test
owensmallwood Mar 8, 2023
726feb5
checks error in test setup
owensmallwood Mar 8, 2023
065f9da
Merge branch 'main' into owensmallwood/pubdash-email-sharing-handle-d…
owensmallwood Mar 8, 2023
106208e
refactors delete method to only accept pubdash uid
owensmallwood Mar 8, 2023
17e2ed7
returns nil instead of empty slice
owensmallwood Mar 8, 2023
ff66e83
use error plane error in service
owensmallwood Mar 8, 2023
3f1029c
deleting a pubdash returns nil when nothing is deleted instead of error
owensmallwood Mar 8, 2023
6d6b9b7
updates query to get all pubdashes for dashboard folder
owensmallwood Mar 8, 2023
5d7ae1e
formats imports
owensmallwood Mar 8, 2023
795cffc
updates query and test
owensmallwood Mar 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions pkg/api/dashboard.go
Expand Up @@ -326,6 +326,12 @@ func (hs *HTTPServer) deleteDashboard(c *contextmodel.ReqContext) response.Respo
hs.log.Error("Failed to disconnect library elements", "dashboard", dash.ID, "user", c.SignedInUser.UserID, "error", err)
}

// deletes all related public dashboard entities
err = hs.PublicDashboardsApi.PublicDashboardService.DeleteByDashboard(c.Req.Context(), dash)
evictorero marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
hs.log.Error("Failed to delete public dashboard")
}
juanicabanas marked this conversation as resolved.
Show resolved Hide resolved

err = hs.DashboardService.DeleteDashboard(c.Req.Context(), dash.ID, c.OrgID)
if err != nil {
var dashboardErr dashboards.DashboardErr
Expand Down
38 changes: 15 additions & 23 deletions pkg/api/dashboard_test.go
Expand Up @@ -9,6 +9,8 @@ import (
"os"
"testing"

"github.com/grafana/grafana/pkg/services/publicdashboards"
"github.com/grafana/grafana/pkg/services/publicdashboards/api"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -304,7 +306,7 @@ func TestDashboardAPIEndpoint(t *testing.T) {
"/api/dashboards/uid/:uid", role, func(sc *scenarioContext) {
setUp()
sc.sqlStore = mockSQLStore
hs.callDeleteDashboardByUID(t, sc, dashboardService)
hs.callDeleteDashboardByUID(t, sc, dashboardService, nil)

assert.Equal(t, 403, sc.resp.Code)
}, mockSQLStore)
Expand Down Expand Up @@ -342,7 +344,7 @@ func TestDashboardAPIEndpoint(t *testing.T) {
loggedInUserScenarioWithRole(t, "When calling DELETE on", "DELETE", "/api/dashboards/uid/abcdefghi",
"/api/dashboards/uid/:uid", role, func(sc *scenarioContext) {
setUp()
hs.callDeleteDashboardByUID(t, sc, dashboardService)
hs.callDeleteDashboardByUID(t, sc, dashboardService, nil)

assert.Equal(t, 403, sc.resp.Code)
}, mockSQLStore)
Expand Down Expand Up @@ -400,23 +402,9 @@ func TestDashboardAPIEndpoint(t *testing.T) {
dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(qResult, nil)
dashboardService.On("DeleteDashboard", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(nil)

hs.callDeleteDashboardByUID(t, sc, dashboardService)

assert.Equal(t, 200, sc.resp.Code)
}, mockSQLStore)

loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) {
setUpInner()
sc.sqlStore = mockSQLStore
sc.dashboardVersionService = fakeDashboardVersionService
hs.callGetDashboardVersion(sc)

assert.Equal(t, 200, sc.resp.Code)
}, mockSQLStore)

loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/dashboards/id/2/versions", "/api/dashboards/id/:dashboardId/versions", role, func(sc *scenarioContext) {
setUpInner()
hs.callGetDashboardVersions(sc)
pubdashService := publicdashboards.NewFakePublicDashboardService(t)
pubdashService.On("DeleteByDashboard", mock.Anything, mock.Anything).Return(nil)
hs.callDeleteDashboardByUID(t, sc, dashboardService, pubdashService)

assert.Equal(t, 200, sc.resp.Code)
}, mockSQLStore)
Expand Down Expand Up @@ -455,7 +443,7 @@ func TestDashboardAPIEndpoint(t *testing.T) {
loggedInUserScenarioWithRole(t, "When calling DELETE on", "DELETE", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) {
setUpInner()

hs.callDeleteDashboardByUID(t, sc, dashboardService)
hs.callDeleteDashboardByUID(t, sc, dashboardService, nil)
assert.Equal(t, 403, sc.resp.Code)
}, mockSQLStore)
})
Expand Down Expand Up @@ -495,7 +483,9 @@ func TestDashboardAPIEndpoint(t *testing.T) {
qResult := dashboards.NewDashboard("test")
dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(qResult, nil)
dashboardService.On("DeleteDashboard", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(nil)
hs.callDeleteDashboardByUID(t, sc, dashboardService)
pubdashService := publicdashboards.NewFakePublicDashboardService(t)
pubdashService.On("DeleteByDashboard", mock.Anything, mock.Anything).Return(nil)
hs.callDeleteDashboardByUID(t, sc, dashboardService, pubdashService)

assert.Equal(t, 200, sc.resp.Code)
}, mockSQLStore)
Expand Down Expand Up @@ -538,7 +528,7 @@ func TestDashboardAPIEndpoint(t *testing.T) {

loggedInUserScenarioWithRole(t, "When calling DELETE on", "DELETE", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) {
setUpInner()
hs.callDeleteDashboardByUID(t, sc, dashboardService)
hs.callDeleteDashboardByUID(t, sc, dashboardService, nil)

assert.Equal(t, 403, sc.resp.Code)
}, mockSQLStore)
Expand Down Expand Up @@ -1035,8 +1025,10 @@ func (hs *HTTPServer) callGetDashboardVersions(sc *scenarioContext) {
}

func (hs *HTTPServer) callDeleteDashboardByUID(t *testing.T,
sc *scenarioContext, mockDashboard *dashboards.FakeDashboardService) {
sc *scenarioContext, mockDashboard *dashboards.FakeDashboardService, mockPubdashService *publicdashboards.FakePublicDashboardService) {
hs.DashboardService = mockDashboard
pubdashApi := api.ProvideApi(mockPubdashService, nil, nil, featuremgmt.WithFeatures())
hs.PublicDashboardsApi = pubdashApi
sc.handlerFunc = hs.DeleteDashboardByUID
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/services/dashboards/database/database.go
Expand Up @@ -702,7 +702,6 @@ func (d *dashboardStore) deleteDashboard(cmd *dashboards.DeleteDashboardCommand,
deletes := []string{
"DELETE FROM dashboard_tag WHERE dashboard_id = ? ",
"DELETE FROM star WHERE dashboard_id = ? ",
"DELETE FROM dashboard_public WHERE dashboard_uid = (SELECT uid FROM dashboard WHERE id = ?)",
"DELETE FROM dashboard WHERE id = ?",
"DELETE FROM playlist_item WHERE type = 'dashboard_by_id' AND value = ?",
"DELETE FROM dashboard_version WHERE dashboard_id = ?",
Expand Down Expand Up @@ -751,7 +750,6 @@ func (d *dashboardStore) deleteDashboard(cmd *dashboards.DeleteDashboardCommand,
"DELETE FROM annotation WHERE dashboard_id IN (SELECT id FROM dashboard WHERE org_id = ? AND folder_id = ?)",
"DELETE FROM dashboard_provisioning WHERE dashboard_id IN (SELECT id FROM dashboard WHERE org_id = ? AND folder_id = ?)",
"DELETE FROM dashboard_acl WHERE dashboard_id IN (SELECT id FROM dashboard WHERE org_id = ? AND folder_id = ?)",
"DELETE FROM dashboard_public WHERE dashboard_uid IN (SELECT uid FROM dashboard WHERE org_id = ? AND folder_id = ?)",
}
for _, sql := range childrenDeletes {
_, err := sess.Exec(sql, dashboard.OrgID, dashboard.ID)
Expand Down
78 changes: 0 additions & 78 deletions pkg/services/dashboards/database/database_test.go
Expand Up @@ -16,8 +16,6 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/publicdashboards/database"
publicDashboardModels "github.com/grafana/grafana/pkg/services/publicdashboards/models"
"github.com/grafana/grafana/pkg/services/quota/quotatest"
"github.com/grafana/grafana/pkg/services/search/model"
"github.com/grafana/grafana/pkg/services/sqlstore"
Expand All @@ -27,7 +25,6 @@ import (
"github.com/grafana/grafana/pkg/services/tag/tagimpl"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
)

func TestIntegrationDashboardDataAccess(t *testing.T) {
Expand All @@ -39,7 +36,6 @@ func TestIntegrationDashboardDataAccess(t *testing.T) {
var savedFolder, savedDash, savedDash2 *dashboards.Dashboard
var dashboardStore dashboards.Store
var starService star.Service
var publicDashboardStore *database.PublicDashboardStoreImpl

setup := func() {
sqlStore, cfg = db.InitTestDBwithCfg(t)
Expand All @@ -53,8 +49,6 @@ func TestIntegrationDashboardDataAccess(t *testing.T) {
insertTestDashboard(t, dashboardStore, "test dash 45", 1, savedFolder.ID, false, "prod")
savedDash2 = insertTestDashboard(t, dashboardStore, "test dash 67", 1, 0, false, "prod")
insertTestRule(t, sqlStore, savedFolder.OrgID, savedFolder.UID)

publicDashboardStore = database.ProvideStore(sqlStore)
}

t.Run("Should return dashboard model", func(t *testing.T) {
Expand Down Expand Up @@ -246,78 +240,6 @@ func TestIntegrationDashboardDataAccess(t *testing.T) {
require.True(t, errors.Is(err, dashboards.ErrFolderContainsAlertRules))
})

t.Run("Should be able to delete dashboard and related public dashboard", func(t *testing.T) {
setup()

uid := util.GenerateShortUID()
cmd := publicDashboardModels.SavePublicDashboardCommand{
PublicDashboard: publicDashboardModels.PublicDashboard{
Uid: uid,
DashboardUid: savedDash.UID,
OrgId: savedDash.OrgID,
IsEnabled: true,
TimeSettings: &publicDashboardModels.TimeSettings{},
CreatedBy: 1,
CreatedAt: time.Now(),
AccessToken: "an-access-token",
},
}
_, err := publicDashboardStore.Create(context.Background(), cmd)
require.NoError(t, err)
pubdashConfig, _ := publicDashboardStore.FindByAccessToken(context.Background(), "an-access-token")
require.NotNil(t, pubdashConfig)

deleteCmd := &dashboards.DeleteDashboardCommand{ID: savedDash.ID, OrgID: savedDash.OrgID}
err = dashboardStore.DeleteDashboard(context.Background(), deleteCmd)
require.NoError(t, err)

query := dashboards.GetDashboardQuery{UID: savedDash.UID, OrgID: savedDash.OrgID}
dash, getErr := dashboardStore.GetDashboard(context.Background(), &query)
require.Equal(t, getErr, dashboards.ErrDashboardNotFound)
assert.Nil(t, dash)

pubdashConfig, err = publicDashboardStore.FindByAccessToken(context.Background(), "an-access-token")
require.Nil(t, err)
require.Nil(t, pubdashConfig)
})

t.Run("Should be able to delete a dashboard folder, with its dashboard and related public dashboard", func(t *testing.T) {
setup()

uid := util.GenerateShortUID()
cmd := publicDashboardModels.SavePublicDashboardCommand{
PublicDashboard: publicDashboardModels.PublicDashboard{
Uid: uid,
DashboardUid: savedDash.UID,
OrgId: savedDash.OrgID,
IsEnabled: true,
TimeSettings: &publicDashboardModels.TimeSettings{},
CreatedBy: 1,
CreatedAt: time.Now(),
AccessToken: "an-access-token",
},
}
_, err := publicDashboardStore.Create(context.Background(), cmd)
require.NoError(t, err)
pubdashConfig, _ := publicDashboardStore.FindByAccessToken(context.Background(), "an-access-token")
require.NotNil(t, pubdashConfig)

deleteCmd := &dashboards.DeleteDashboardCommand{ID: savedFolder.ID, ForceDeleteFolderRules: true}
err = dashboardStore.DeleteDashboard(context.Background(), deleteCmd)
require.NoError(t, err)

query := dashboards.GetDashboardsQuery{
DashboardIDs: []int64{savedFolder.ID, savedDash.ID},
}
queryResult, err := dashboardStore.GetDashboards(context.Background(), &query)
require.NoError(t, err)
require.Equal(t, len(queryResult), 0)

pubdashConfig, err = publicDashboardStore.FindByAccessToken(context.Background(), "an-access-token")
require.Nil(t, err)
require.Nil(t, pubdashConfig)
})

t.Run("Should be able to delete a dashboard folder and its children if force delete rules is enabled", func(t *testing.T) {
setup()
deleteCmd := &dashboards.DeleteDashboardCommand{ID: savedFolder.ID, ForceDeleteFolderRules: true}
Expand Down
2 changes: 1 addition & 1 deletion pkg/services/publicdashboards/api/api.go
Expand Up @@ -196,7 +196,7 @@ func (api *Api) DeletePublicDashboard(c *contextmodel.ReqContext) response.Respo
return response.Err(ErrInvalidUid.Errorf("UpdatePublicDashboard: invalid Uid %s", uid))
}

err := api.PublicDashboardService.Delete(c.Req.Context(), c.OrgID, uid)
err := api.PublicDashboardService.Delete(c.Req.Context(), uid)
if err != nil {
return response.Err(err)
}
Expand Down
22 changes: 20 additions & 2 deletions pkg/services/publicdashboards/database/database.go
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/publicdashboards"
. "github.com/grafana/grafana/pkg/services/publicdashboards/models"
"github.com/grafana/grafana/pkg/services/sqlstore"
)

// Define the storage implementation. We're generating the mock implementation
Expand Down Expand Up @@ -255,8 +256,8 @@ func (d *PublicDashboardStoreImpl) Update(ctx context.Context, cmd SavePublicDas
}

// Deletes a public dashboard
func (d *PublicDashboardStoreImpl) Delete(ctx context.Context, orgId int64, uid string) (int64, error) {
dashboard := &PublicDashboard{OrgId: orgId, Uid: uid}
func (d *PublicDashboardStoreImpl) Delete(ctx context.Context, uid string) (int64, error) {
dashboard := &PublicDashboard{Uid: uid}
var affectedRows int64
err := d.sqlStore.WithDbSession(ctx, func(sess *db.Session) error {
var err error
Expand All @@ -267,3 +268,20 @@ func (d *PublicDashboardStoreImpl) Delete(ctx context.Context, orgId int64, uid

return affectedRows, err
}

func (d *PublicDashboardStoreImpl) FindByDashboardFolder(ctx context.Context, dashboard *dashboards.Dashboard) ([]*PublicDashboard, error) {
if dashboard == nil || !dashboard.IsFolder {
return nil, nil
}

var pubdashes []*PublicDashboard

err := d.sqlStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
return sess.SQL("SELECT * from dashboard_public WHERE (dashboard_uid, org_id) IN (SELECT uid, org_id FROM dashboard WHERE folder_id = ?)", dashboard.ID).Find(&pubdashes)
})
if err != nil {
return nil, err
}

return pubdashes, nil
}
43 changes: 41 additions & 2 deletions pkg/services/publicdashboards/database/database_test.go
Expand Up @@ -652,7 +652,7 @@ func TestIntegrationDelete(t *testing.T) {
t.Run("Delete success", func(t *testing.T) {
setup()
// Do the deletion
affectedRows, err := publicdashboardStore.Delete(context.Background(), savedPublicDashboard.OrgId, savedPublicDashboard.Uid)
affectedRows, err := publicdashboardStore.Delete(context.Background(), savedPublicDashboard.Uid)
require.NoError(t, err)
assert.EqualValues(t, affectedRows, 1)

Expand All @@ -665,12 +665,51 @@ func TestIntegrationDelete(t *testing.T) {
t.Run("Non-existent public dashboard deletion doesn't throw an error", func(t *testing.T) {
setup()

affectedRows, err := publicdashboardStore.Delete(context.Background(), 15, "non-existent-uid")
affectedRows, err := publicdashboardStore.Delete(context.Background(), "non-existent-uid")
require.NoError(t, err)
assert.EqualValues(t, affectedRows, 0)
})
}

func TestGetDashboardByFolder(t *testing.T) {
t.Run("returns nil when dashboard is not a folder", func(t *testing.T) {
sqlStore, _ := db.InitTestDBwithCfg(t)
dashboard := &dashboards.Dashboard{IsFolder: false}
store := ProvideStore(sqlStore)
pubdashes, err := store.FindByDashboardFolder(context.Background(), dashboard)

require.NoError(t, err)
assert.Nil(t, pubdashes)
})

t.Run("returns nil when dashboard is nil", func(t *testing.T) {
sqlStore, _ := db.InitTestDBwithCfg(t)
store := ProvideStore(sqlStore)
pubdashes, err := store.FindByDashboardFolder(context.Background(), nil)

require.NoError(t, err)
assert.Nil(t, pubdashes)
})

t.Run("can get all pubdashes for dashboard folder and org", func(t *testing.T) {
sqlStore, cfg := db.InitTestDBwithCfg(t)
quotaService := quotatest.New(false, nil)
dashboardStore, err := dashboardsDB.ProvideDashboardStore(sqlStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore, cfg), quotaService)
require.NoError(t, err)
pubdashStore := ProvideStore(sqlStore)
dashboard := insertTestDashboard(t, dashboardStore, "title", 1, 1, true)
pubdash := insertPublicDashboard(t, pubdashStore, dashboard.UID, dashboard.OrgID, true)
dashboard2 := insertTestDashboard(t, dashboardStore, "title", 1, 2, true)
_ = insertPublicDashboard(t, pubdashStore, dashboard2.UID, dashboard2.OrgID, true)

pubdashes, err := pubdashStore.FindByDashboardFolder(context.Background(), dashboard)

require.NoError(t, err)
assert.Len(t, pubdashes, 1)
assert.Equal(t, pubdash, pubdashes[0])
})
}

// helper function to insert a dashboard
func insertTestDashboard(t *testing.T, dashboardStore dashboards.Store, title string, orgId int64,
folderId int64, isFolder bool, tags ...interface{}) *dashboards.Dashboard {
Expand Down
26 changes: 20 additions & 6 deletions pkg/services/publicdashboards/public_dashboard_service_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.