From bdc6e2fe0a97256161258c7a5319bb201423db9d Mon Sep 17 00:00:00 2001 From: Emil Hessman Date: Sat, 14 Nov 2020 09:49:07 +0100 Subject: [PATCH] Chore: Require OrgId to be specified in delete playlist command (#29117) --- pkg/services/sqlstore/playlist.go | 2 +- pkg/services/sqlstore/playlist_test.go | 32 ++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/pkg/services/sqlstore/playlist.go b/pkg/services/sqlstore/playlist.go index 5fb38202180..9abfc9f3eb2 100644 --- a/pkg/services/sqlstore/playlist.go +++ b/pkg/services/sqlstore/playlist.go @@ -108,7 +108,7 @@ func GetPlaylist(query *models.GetPlaylistByIdQuery) error { } func DeletePlaylist(cmd *models.DeletePlaylistCommand) error { - if cmd.Id == 0 { + if cmd.Id == 0 || cmd.OrgId == 0 { return models.ErrCommandValidationFailed } diff --git a/pkg/services/sqlstore/playlist_test.go b/pkg/services/sqlstore/playlist_test.go index 45c951de2de..fb0a7463faf 100644 --- a/pkg/services/sqlstore/playlist_test.go +++ b/pkg/services/sqlstore/playlist_test.go @@ -3,6 +3,7 @@ package sqlstore import ( + "fmt" "testing" "github.com/grafana/grafana/pkg/models" @@ -32,9 +33,36 @@ func TestPlaylistDataAccess(t *testing.T) { }) t.Run("Can remove playlist", func(t *testing.T) { - query := models.DeletePlaylistCommand{Id: 1} - err = DeletePlaylist(&query) + deleteQuery := models.DeletePlaylistCommand{Id: 1, OrgId: 1} + err = DeletePlaylist(&deleteQuery) require.NoError(t, err) + + getQuery := models.GetPlaylistByIdQuery{Id: 1} + err = GetPlaylist(&getQuery) + require.NoError(t, err) + require.Equal(t, int64(0), getQuery.Result.Id, "playlist should've been removed") }) }) + + t.Run("Delete playlist that doesn't exist", func(t *testing.T) { + deleteQuery := models.DeletePlaylistCommand{Id: 1, OrgId: 1} + err := DeletePlaylist(&deleteQuery) + require.NoError(t, err) + }) + + t.Run("Delete playlist with invalid command yields error", func(t *testing.T) { + testCases := []struct { + desc string + cmd models.DeletePlaylistCommand + }{ + {desc: "none", cmd: models.DeletePlaylistCommand{}}, + {desc: "no OrgId", cmd: models.DeletePlaylistCommand{Id: 1}}, + {desc: "no Id", cmd: models.DeletePlaylistCommand{OrgId: 1}}, + } + + for _, tc := range testCases { + err := DeletePlaylist(&tc.cmd) + require.EqualError(t, err, models.ErrCommandValidationFailed.Error(), fmt.Sprintf("expected command validation error for %q", tc.desc)) + } + }) }