backend: add PlaylistUIDs to Playlist; remove playlist IDs from API (#49609)

* backend/api: refactor PlaylistId to PlaylistUid
* Add org_id to Get and Update playlist functions
Fix migration - no longer pad the uid; fix mysql syntax

The relevant tests are passing using postgres, mysql and the default sqllite backends, but there are a number of other failing tests when using postgres and myself so I'm not entirely confident with those results.

* fix bad query in GetPlaylistItem and add a test that would have caught the mistake in the first place. Reverted the playlist_uid column addition in playlist_item; it became unnecessary after this PR.

Added default value to the new UID column based on PR feedback.

* break this PRs migration into its own function

* Playlists: Update UI to use the updated API

Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com>
This commit is contained in:
Kristin Laemmert 2022-06-14 15:32:52 -04:00 committed by GitHub
parent 39b467b46d
commit a33a023629
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
26 changed files with 231 additions and 163 deletions

View File

@ -42,7 +42,7 @@ HTTP/1.1 200
Content-Type: application/json
[
{
"id": 1,
"uid": "1",
"name": "my playlist",
"interval": "5m"
}
@ -51,7 +51,7 @@ Content-Type: application/json
## Get one playlist
`GET /api/playlists/:id`
`GET /api/playlists/:uid`
**Example Request**:
@ -67,14 +67,14 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
HTTP/1.1 200
Content-Type: application/json
{
"id" : 1,
"uid" : "1",
"name": "my playlist",
"interval": "5m",
"orgId": "my org",
"items": [
{
"id": 1,
"playlistId": 1,
"playlistUid": "1",
"type": "dashboard_by_id",
"value": "3",
"order": 1,
@ -82,7 +82,7 @@ Content-Type: application/json
},
{
"id": 2,
"playlistId": 1,
"playlistUid": "1",
"type": "dashboard_by_tag",
"value": "myTag",
"order": 2,
@ -94,7 +94,7 @@ Content-Type: application/json
## Get Playlist items
`GET /api/playlists/:id/items`
`GET /api/playlists/:uid/items`
**Example Request**:
@ -112,7 +112,7 @@ Content-Type: application/json
[
{
"id": 1,
"playlistId": 1,
"playlistUid": "1",
"type": "dashboard_by_id",
"value": "3",
"order": 1,
@ -120,7 +120,7 @@ Content-Type: application/json
},
{
"id": 2,
"playlistId": 1,
"playlistUid": "1",
"type": "dashboard_by_tag",
"value": "myTag",
"order": 2,
@ -131,7 +131,7 @@ Content-Type: application/json
## Get Playlist dashboards
`GET /api/playlists/:id/dashboards`
`GET /api/playlists/:uid/dashboards`
**Example Request**:
@ -198,7 +198,7 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
HTTP/1.1 200
Content-Type: application/json
{
"id": 1,
"uid": "1",
"name": "my playlist",
"interval": "5m"
}
@ -206,7 +206,7 @@ Content-Type: application/json
## Update a playlist
`PUT /api/playlists/:id`
`PUT /api/playlists/:uid`
**Example Request**:
@ -220,14 +220,14 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
"interval": "5m",
"items": [
{
"playlistId": 1,
"playlistUid": "1",
"type": "dashboard_by_id",
"value": "3",
"order": 1,
"title":"my third dashboard"
},
{
"playlistId": 1,
"playlistUid": "1",
"type": "dashboard_by_tag",
"value": "myTag",
"order": 2,
@ -243,14 +243,14 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
HTTP/1.1 200
Content-Type: application/json
{
"id" : 1,
"uid" : "1",
"name": "my playlist",
"interval": "5m",
"orgId": "my org",
"items": [
{
"id": 1,
"playlistId": 1,
"playlistUid": "1",
"type": "dashboard_by_id",
"value": "3",
"order": 1,
@ -258,7 +258,7 @@ Content-Type: application/json
},
{
"id": 2,
"playlistId": 1,
"playlistUid": "1",
"type": "dashboard_by_tag",
"value": "myTag",
"order": 2,
@ -270,7 +270,7 @@ Content-Type: application/json
## Delete a playlist
`DELETE /api/playlists/:id`
`DELETE /api/playlists/:uid`
**Example Request**:

View File

@ -438,11 +438,11 @@ func (hs *HTTPServer) registerRoutes() {
// Playlist
apiRoute.Group("/playlists", func(playlistRoute routing.RouteRegister) {
playlistRoute.Get("/", routing.Wrap(hs.SearchPlaylists))
playlistRoute.Get("/:id", hs.ValidateOrgPlaylist, routing.Wrap(hs.GetPlaylist))
playlistRoute.Get("/:id/items", hs.ValidateOrgPlaylist, routing.Wrap(hs.GetPlaylistItems))
playlistRoute.Get("/:id/dashboards", hs.ValidateOrgPlaylist, routing.Wrap(hs.GetPlaylistDashboards))
playlistRoute.Delete("/:id", reqEditorRole, hs.ValidateOrgPlaylist, routing.Wrap(hs.DeletePlaylist))
playlistRoute.Put("/:id", reqEditorRole, hs.ValidateOrgPlaylist, routing.Wrap(hs.UpdatePlaylist))
playlistRoute.Get("/:uid", hs.ValidateOrgPlaylist, routing.Wrap(hs.GetPlaylist))
playlistRoute.Get("/:uid/items", hs.ValidateOrgPlaylist, routing.Wrap(hs.GetPlaylistItems))
playlistRoute.Get("/:uid/dashboards", hs.ValidateOrgPlaylist, routing.Wrap(hs.GetPlaylistDashboards))
playlistRoute.Delete("/:uid", reqEditorRole, hs.ValidateOrgPlaylist, routing.Wrap(hs.DeletePlaylist))
playlistRoute.Put("/:uid", reqEditorRole, hs.ValidateOrgPlaylist, routing.Wrap(hs.UpdatePlaylist))
playlistRoute.Post("/", reqEditorRole, routing.Wrap(hs.CreatePlaylist))
})

View File

@ -3,7 +3,6 @@ package api
import (
"context"
"net/http"
"strconv"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/models"
@ -11,13 +10,9 @@ import (
)
func (hs *HTTPServer) ValidateOrgPlaylist(c *models.ReqContext) {
id, err := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64)
if err != nil {
c.JsonApiErr(http.StatusBadRequest, "id is invalid", nil)
return
}
query := models.GetPlaylistByIdQuery{Id: id}
err = hs.SQLStore.GetPlaylist(c.Req.Context(), &query)
uid := web.Params(c.Req)[":uid"]
query := models.GetPlaylistByUidQuery{UID: uid, OrgId: c.OrgId}
err := hs.SQLStore.GetPlaylist(c.Req.Context(), &query)
if err != nil {
c.JsonApiErr(404, "Playlist not found", err)
@ -58,20 +53,18 @@ func (hs *HTTPServer) SearchPlaylists(c *models.ReqContext) response.Response {
}
func (hs *HTTPServer) GetPlaylist(c *models.ReqContext) response.Response {
id, err := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "id is invalid", err)
}
cmd := models.GetPlaylistByIdQuery{Id: id}
uid := web.Params(c.Req)[":uid"]
cmd := models.GetPlaylistByUidQuery{UID: uid, OrgId: c.OrgId}
if err := hs.SQLStore.GetPlaylist(c.Req.Context(), &cmd); err != nil {
return response.Error(500, "Playlist not found", err)
}
playlistDTOs, _ := hs.LoadPlaylistItemDTOs(c.Req.Context(), id)
playlistDTOs, _ := hs.LoadPlaylistItemDTOs(c.Req.Context(), uid, c.OrgId)
dto := &models.PlaylistDTO{
Id: cmd.Result.Id,
UID: cmd.Result.UID,
Name: cmd.Result.Name,
Interval: cmd.Result.Interval,
OrgId: cmd.Result.OrgId,
@ -81,8 +74,8 @@ func (hs *HTTPServer) GetPlaylist(c *models.ReqContext) response.Response {
return response.JSON(http.StatusOK, dto)
}
func (hs *HTTPServer) LoadPlaylistItemDTOs(ctx context.Context, id int64) ([]models.PlaylistItemDTO, error) {
playlistitems, err := hs.LoadPlaylistItems(ctx, id)
func (hs *HTTPServer) LoadPlaylistItemDTOs(ctx context.Context, uid string, orgId int64) ([]models.PlaylistItemDTO, error) {
playlistitems, err := hs.LoadPlaylistItems(ctx, uid, orgId)
if err != nil {
return nil, err
@ -104,8 +97,8 @@ func (hs *HTTPServer) LoadPlaylistItemDTOs(ctx context.Context, id int64) ([]mod
return playlistDTOs, nil
}
func (hs *HTTPServer) LoadPlaylistItems(ctx context.Context, id int64) ([]models.PlaylistItem, error) {
itemQuery := models.GetPlaylistItemsByIdQuery{PlaylistId: id}
func (hs *HTTPServer) LoadPlaylistItems(ctx context.Context, uid string, orgId int64) ([]models.PlaylistItem, error) {
itemQuery := models.GetPlaylistItemsByUidQuery{PlaylistUID: uid, OrgId: orgId}
if err := hs.SQLStore.GetPlaylistItem(ctx, &itemQuery); err != nil {
return nil, err
}
@ -114,12 +107,9 @@ func (hs *HTTPServer) LoadPlaylistItems(ctx context.Context, id int64) ([]models
}
func (hs *HTTPServer) GetPlaylistItems(c *models.ReqContext) response.Response {
id, err := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "id is invalid", err)
}
uid := web.Params(c.Req)[":uid"]
playlistDTOs, err := hs.LoadPlaylistItemDTOs(c.Req.Context(), id)
playlistDTOs, err := hs.LoadPlaylistItemDTOs(c.Req.Context(), uid, c.OrgId)
if err != nil {
return response.Error(500, "Could not load playlist items", err)
@ -129,12 +119,9 @@ func (hs *HTTPServer) GetPlaylistItems(c *models.ReqContext) response.Response {
}
func (hs *HTTPServer) GetPlaylistDashboards(c *models.ReqContext) response.Response {
playlistID, err := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "id is invalid", err)
}
playlistUID := web.Params(c.Req)[":uid"]
playlists, err := hs.LoadPlaylistDashboards(c.Req.Context(), c.OrgId, c.SignedInUser, playlistID)
playlists, err := hs.LoadPlaylistDashboards(c.Req.Context(), c.OrgId, c.SignedInUser, playlistUID)
if err != nil {
return response.Error(500, "Could not load dashboards", err)
}
@ -143,12 +130,9 @@ func (hs *HTTPServer) GetPlaylistDashboards(c *models.ReqContext) response.Respo
}
func (hs *HTTPServer) DeletePlaylist(c *models.ReqContext) response.Response {
id, err := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "id is invalid", err)
}
uid := web.Params(c.Req)[":uid"]
cmd := models.DeletePlaylistCommand{Id: id, OrgId: c.OrgId}
cmd := models.DeletePlaylistCommand{UID: uid, OrgId: c.OrgId}
if err := hs.SQLStore.DeletePlaylist(c.Req.Context(), &cmd); err != nil {
return response.Error(500, "Failed to delete playlist", err)
}
@ -176,17 +160,13 @@ func (hs *HTTPServer) UpdatePlaylist(c *models.ReqContext) response.Response {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
cmd.OrgId = c.OrgId
var err error
cmd.Id, err = strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "id is invalid", err)
}
cmd.UID = web.Params(c.Req)[":uid"]
if err := hs.SQLStore.UpdatePlaylist(c.Req.Context(), &cmd); err != nil {
return response.Error(500, "Failed to save playlist", err)
}
playlistDTOs, err := hs.LoadPlaylistItemDTOs(c.Req.Context(), cmd.Id)
playlistDTOs, err := hs.LoadPlaylistItemDTOs(c.Req.Context(), cmd.UID, c.OrgId)
if err != nil {
return response.Error(500, "Failed to save playlist", err)
}

View File

@ -65,8 +65,8 @@ func (hs *HTTPServer) populateDashboardsByTag(ctx context.Context, orgID int64,
return result
}
func (hs *HTTPServer) LoadPlaylistDashboards(ctx context.Context, orgID int64, signedInUser *models.SignedInUser, playlistID int64) (dtos.PlaylistDashboardsSlice, error) {
playlistItems, _ := hs.LoadPlaylistItems(ctx, playlistID)
func (hs *HTTPServer) LoadPlaylistDashboards(ctx context.Context, orgID int64, signedInUser *models.SignedInUser, playlistUID string) (dtos.PlaylistDashboardsSlice, error) {
playlistItems, _ := hs.LoadPlaylistItems(ctx, playlistUID, orgID)
dashboardByIDs := make([]int64, 0)
dashboardByTag := make([]string, 0)

View File

@ -6,12 +6,14 @@ import (
// Typed errors
var (
ErrPlaylistNotFound = errors.New("Playlist not found")
ErrPlaylistNotFound = errors.New("Playlist not found")
ErrPlaylistFailedGenerateUniqueUid = errors.New("failed to generate unique playlist UID")
)
// Playlist model
type Playlist struct {
Id int64 `json:"id"`
UID string `json:"uid" xorm:"uid"`
Name string `json:"name"`
Interval string `json:"interval"`
OrgId int64 `json:"-"`
@ -19,6 +21,7 @@ type Playlist struct {
type PlaylistDTO struct {
Id int64 `json:"id"`
UID string `json:"uid"`
Name string `json:"name"`
Interval string `json:"interval"`
OrgId int64 `json:"-"`
@ -51,7 +54,7 @@ type Playlists []*Playlist
type UpdatePlaylistCommand struct {
OrgId int64 `json:"-"`
Id int64 `json:"id"`
UID string `json:"uid"`
Name string `json:"name" binding:"Required"`
Interval string `json:"interval"`
Items []PlaylistItemDTO `json:"items"`
@ -69,7 +72,7 @@ type CreatePlaylistCommand struct {
}
type DeletePlaylistCommand struct {
Id int64
UID string
OrgId int64
}
@ -85,12 +88,14 @@ type GetPlaylistsQuery struct {
Result Playlists
}
type GetPlaylistByIdQuery struct {
Id int64
type GetPlaylistByUidQuery struct {
UID string
OrgId int64
Result *Playlist
}
type GetPlaylistItemsByIdQuery struct {
PlaylistId int64
Result *[]PlaylistItem
type GetPlaylistItemsByUidQuery struct {
PlaylistUID string
OrgId int64
Result *[]PlaylistItem
}

View File

@ -91,6 +91,7 @@ func (*OSSMigrations) AddMigration(mg *Migrator) {
accesscontrol.AddManagedPermissionsMigration(mg, accesscontrol.ManagedPermissionsMigrationID)
accesscontrol.AddManagedFolderAlertActionsMigration(mg)
accesscontrol.AddActionNameMigrator(mg)
addPlaylistUIDMigration(mg)
}
func addMigrationLogMigrations(mg *Migrator) {

View File

@ -6,18 +6,8 @@ func addPlaylistMigrations(mg *Migrator) {
mg.AddMigration("Drop old table playlist table", NewDropTableMigration("playlist"))
mg.AddMigration("Drop old table playlist_item table", NewDropTableMigration("playlist_item"))
playlistV2 := Table{
Name: "playlist",
Columns: []*Column{
{Name: "id", Type: DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true},
{Name: "name", Type: DB_NVarchar, Length: 255, Nullable: false},
{Name: "interval", Type: DB_NVarchar, Length: 255, Nullable: false},
{Name: "org_id", Type: DB_BigInt, Nullable: false},
},
}
// create table
mg.AddMigration("create playlist table v2", NewAddTableMigration(playlistV2))
mg.AddMigration("create playlist table v2", NewAddTableMigration(playlistV2()))
playlistItemV2 := Table{
Name: "playlist_item",
@ -44,3 +34,32 @@ func addPlaylistMigrations(mg *Migrator) {
{Name: "title", Type: DB_Text, Nullable: false},
}))
}
func addPlaylistUIDMigration(mg *Migrator) {
// Replacing auto-incremented playlistIDs with string UIDs
mg.AddMigration("Add UID column to playlist", NewAddColumnMigration(playlistV2(), &Column{
Name: "uid", Type: DB_NVarchar, Length: 80, Nullable: false, Default: "0",
}))
// copy the (string representation of) existing IDs into the new uid column.
mg.AddMigration("Update uid column values in playlist", NewRawSQLMigration("").
SQLite("UPDATE playlist SET uid=printf('%d',id);").
Postgres("UPDATE playlist SET uid=id::text;").
Mysql("UPDATE playlist SET uid=id;"))
mg.AddMigration("Add index for uid in playlist", NewAddIndexMigration(playlistV2(), &Index{
Cols: []string{"org_id", "uid"}, Type: UniqueIndex,
}))
}
func playlistV2() Table {
return Table{
Name: "playlist",
Columns: []*Column{
{Name: "id", Type: DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true},
{Name: "name", Type: DB_NVarchar, Length: 255, Nullable: false},
{Name: "interval", Type: DB_NVarchar, Length: 255, Nullable: false},
{Name: "org_id", Type: DB_BigInt, Nullable: false},
},
}
}

View File

@ -355,7 +355,7 @@ func (m *SQLStoreMock) UpdatePlaylist(ctx context.Context, cmd *models.UpdatePla
return m.ExpectedError
}
func (m *SQLStoreMock) GetPlaylist(ctx context.Context, query *models.GetPlaylistByIdQuery) error {
func (m *SQLStoreMock) GetPlaylist(ctx context.Context, query *models.GetPlaylistByUidQuery) error {
return m.ExpectedError
}
@ -367,7 +367,7 @@ func (m *SQLStoreMock) SearchPlaylists(ctx context.Context, query *models.GetPla
return m.ExpectedError
}
func (m *SQLStoreMock) GetPlaylistItem(ctx context.Context, query *models.GetPlaylistItemsByIdQuery) error {
func (m *SQLStoreMock) GetPlaylistItem(ctx context.Context, query *models.GetPlaylistItemsByUidQuery) error {
return m.ExpectedError
}

View File

@ -8,13 +8,19 @@ import (
func (ss *SQLStore) CreatePlaylist(ctx context.Context, cmd *models.CreatePlaylistCommand) error {
return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
uid, err := generateAndValidateNewPlaylistUid(sess, cmd.OrgId)
if err != nil {
return err
}
playlist := models.Playlist{
Name: cmd.Name,
Interval: cmd.Interval,
OrgId: cmd.OrgId,
UID: uid,
}
_, err := sess.Insert(&playlist)
_, err = sess.Insert(&playlist)
if err != nil {
return err
}
@ -40,33 +46,34 @@ func (ss *SQLStore) CreatePlaylist(ctx context.Context, cmd *models.CreatePlayli
func (ss *SQLStore) UpdatePlaylist(ctx context.Context, cmd *models.UpdatePlaylistCommand) error {
return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
playlist := models.Playlist{
Id: cmd.Id,
UID: cmd.UID,
OrgId: cmd.OrgId,
Name: cmd.Name,
Interval: cmd.Interval,
}
existingPlaylist := sess.Where("id = ? AND org_id = ?", cmd.Id, cmd.OrgId).Find(models.Playlist{})
if existingPlaylist == nil {
return models.ErrPlaylistNotFound
existingPlaylist := models.Playlist{UID: cmd.UID, OrgId: cmd.OrgId}
_, err := sess.Get(&existingPlaylist)
if err != nil {
return err
}
playlist.Id = existingPlaylist.Id
cmd.Result = &models.PlaylistDTO{
Id: playlist.Id,
UID: playlist.UID,
OrgId: playlist.OrgId,
Name: playlist.Name,
Interval: playlist.Interval,
}
_, err := sess.ID(cmd.Id).Cols("name", "interval").Update(&playlist)
_, err = sess.Where("id=?", playlist.Id).Cols("name", "interval").Update(&playlist)
if err != nil {
return err
}
rawSQL := "DELETE FROM playlist_item WHERE playlist_id = ?"
_, err = sess.Exec(rawSQL, cmd.Id)
_, err = sess.Exec(rawSQL, playlist.Id)
if err != nil {
return err
@ -85,20 +92,18 @@ func (ss *SQLStore) UpdatePlaylist(ctx context.Context, cmd *models.UpdatePlayli
}
_, err = sess.Insert(&playlistItems)
return err
})
}
func (ss *SQLStore) GetPlaylist(ctx context.Context, query *models.GetPlaylistByIdQuery) error {
if query.Id == 0 {
func (ss *SQLStore) GetPlaylist(ctx context.Context, query *models.GetPlaylistByUidQuery) error {
if query.UID == "" || query.OrgId == 0 {
return models.ErrCommandValidationFailed
}
return ss.WithDbSession(ctx, func(sess *DBSession) error {
playlist := models.Playlist{}
_, err := sess.ID(query.Id).Get(&playlist)
playlist := models.Playlist{UID: query.UID, OrgId: query.OrgId}
_, err := sess.Get(&playlist)
query.Result = &playlist
return err
@ -106,26 +111,35 @@ func (ss *SQLStore) GetPlaylist(ctx context.Context, query *models.GetPlaylistBy
}
func (ss *SQLStore) DeletePlaylist(ctx context.Context, cmd *models.DeletePlaylistCommand) error {
if cmd.Id == 0 || cmd.OrgId == 0 {
if cmd.UID == "" || cmd.OrgId == 0 {
return models.ErrCommandValidationFailed
}
return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
var rawPlaylistSQL = "DELETE FROM playlist WHERE id = ? and org_id = ?"
_, err := sess.Exec(rawPlaylistSQL, cmd.Id, cmd.OrgId)
playlist := models.Playlist{UID: cmd.UID, OrgId: cmd.OrgId}
_, err := sess.Get(&playlist)
if err != nil {
return err
}
var rawPlaylistSQL = "DELETE FROM playlist WHERE uid = ? and org_id = ?"
_, err = sess.Exec(rawPlaylistSQL, cmd.UID, cmd.OrgId)
if err != nil {
return err
}
var rawItemSQL = "DELETE FROM playlist_item WHERE playlist_id = ?"
_, err2 := sess.Exec(rawItemSQL, cmd.Id)
_, err = sess.Exec(rawItemSQL, playlist.Id)
return err2
return err
})
}
func (ss *SQLStore) SearchPlaylists(ctx context.Context, query *models.GetPlaylistsQuery) error {
if query.OrgId == 0 {
return models.ErrCommandValidationFailed
}
return ss.WithDbSession(ctx, func(dbSess *DBSession) error {
var playlists = make(models.Playlists, 0)
@ -143,17 +157,44 @@ func (ss *SQLStore) SearchPlaylists(ctx context.Context, query *models.GetPlayli
})
}
func (ss *SQLStore) GetPlaylistItem(ctx context.Context, query *models.GetPlaylistItemsByIdQuery) error {
func (ss *SQLStore) GetPlaylistItem(ctx context.Context, query *models.GetPlaylistItemsByUidQuery) error {
return ss.WithDbSession(ctx, func(sess *DBSession) error {
if query.PlaylistId == 0 {
if query.PlaylistUID == "" || query.OrgId == 0 {
return models.ErrCommandValidationFailed
}
var playlistItems = make([]models.PlaylistItem, 0)
err := sess.Where("playlist_id=?", query.PlaylistId).Find(&playlistItems)
// get the playlist Id
get := &models.GetPlaylistByUidQuery{UID: query.PlaylistUID, OrgId: query.OrgId}
err := ss.GetPlaylist(ctx, get)
if err != nil {
return err
}
var playlistItems = make([]models.PlaylistItem, 0)
err = sess.Where("playlist_id=?", get.Result.Id).Find(&playlistItems)
query.Result = &playlistItems
return err
})
}
// generateAndValidateNewPlaylistUid generates a playlistUID and verifies that
// the uid isn't already in use. This is deliberately overly cautious, since users
// can also specify playlist uids during provisioning.
func generateAndValidateNewPlaylistUid(sess *DBSession, orgId int64) (string, error) {
for i := 0; i < 3; i++ {
uid := generateNewUid()
playlist := models.Playlist{OrgId: orgId, UID: uid}
exists, err := sess.Get(&playlist)
if err != nil {
return "", err
}
if !exists {
return uid, nil
}
}
return "", models.ErrPlaylistFailedGenerateUniqueUid
}

View File

@ -4,8 +4,9 @@ import (
"context"
"testing"
"github.com/grafana/grafana/pkg/models"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/models"
)
func TestIntegrationPlaylistDataAccess(t *testing.T) {
@ -22,31 +23,39 @@ func TestIntegrationPlaylistDataAccess(t *testing.T) {
cmd := models.CreatePlaylistCommand{Name: "NYC office", Interval: "10m", OrgId: 1, Items: items}
err := ss.CreatePlaylist(context.Background(), &cmd)
require.NoError(t, err)
uid := cmd.Result.UID
t.Run("Can get playlist items", func(t *testing.T) {
get := &models.GetPlaylistItemsByUidQuery{PlaylistUID: uid, OrgId: 1}
err = ss.GetPlaylistItem(context.Background(), get)
require.NoError(t, err)
require.Equal(t, len(*get.Result), len(items))
})
t.Run("Can update playlist", func(t *testing.T) {
items := []models.PlaylistItemDTO{
{Title: "influxdb", Value: "influxdb", Type: "dashboard_by_tag"},
{Title: "Backend response times", Value: "2", Type: "dashboard_by_id"},
}
query := models.UpdatePlaylistCommand{Name: "NYC office ", OrgId: 1, Id: 1, Interval: "10s", Items: items}
query := models.UpdatePlaylistCommand{Name: "NYC office ", OrgId: 1, UID: uid, Interval: "10s", Items: items}
err = ss.UpdatePlaylist(context.Background(), &query)
require.NoError(t, err)
})
t.Run("Can remove playlist", func(t *testing.T) {
deleteQuery := models.DeletePlaylistCommand{Id: 1, OrgId: 1}
deleteQuery := models.DeletePlaylistCommand{UID: uid, OrgId: 1}
err = ss.DeletePlaylist(context.Background(), &deleteQuery)
require.NoError(t, err)
getQuery := models.GetPlaylistByIdQuery{Id: 1}
getQuery := models.GetPlaylistByUidQuery{UID: uid, OrgId: 1}
err = ss.GetPlaylist(context.Background(), &getQuery)
require.NoError(t, err)
require.Equal(t, int64(0), getQuery.Result.Id, "playlist should've been removed")
require.Equal(t, uid, getQuery.Result.UID, "playlist should've been removed")
})
})
t.Run("Delete playlist that doesn't exist", func(t *testing.T) {
deleteQuery := models.DeletePlaylistCommand{Id: 1, OrgId: 1}
deleteQuery := models.DeletePlaylistCommand{UID: "654312", OrgId: 1}
err := ss.DeletePlaylist(context.Background(), &deleteQuery)
require.NoError(t, err)
})
@ -57,8 +66,8 @@ func TestIntegrationPlaylistDataAccess(t *testing.T) {
cmd models.DeletePlaylistCommand
}{
{desc: "none", cmd: models.DeletePlaylistCommand{}},
{desc: "no OrgId", cmd: models.DeletePlaylistCommand{Id: 1}},
{desc: "no Id", cmd: models.DeletePlaylistCommand{OrgId: 1}},
{desc: "no OrgId", cmd: models.DeletePlaylistCommand{UID: "1"}},
{desc: "no Uid", cmd: models.DeletePlaylistCommand{OrgId: 1}},
}
for _, tc := range testCases {

View File

@ -70,10 +70,10 @@ type Store interface {
InTransaction(ctx context.Context, fn func(ctx context.Context) error) error
CreatePlaylist(ctx context.Context, cmd *models.CreatePlaylistCommand) error
UpdatePlaylist(ctx context.Context, cmd *models.UpdatePlaylistCommand) error
GetPlaylist(ctx context.Context, query *models.GetPlaylistByIdQuery) error
GetPlaylist(ctx context.Context, query *models.GetPlaylistByUidQuery) error
DeletePlaylist(ctx context.Context, cmd *models.DeletePlaylistCommand) error
SearchPlaylists(ctx context.Context, query *models.GetPlaylistsQuery) error
GetPlaylistItem(ctx context.Context, query *models.GetPlaylistItemsByIdQuery) error
GetPlaylistItem(ctx context.Context, query *models.GetPlaylistItemsByUidQuery) error
GetAlertById(ctx context.Context, query *models.GetAlertByIdQuery) error
GetAllAlertQueryHandler(ctx context.Context, query *models.GetAllAlertsQuery) error
HandleAlertsQuery(ctx context.Context, query *models.GetAlertsQuery) error

View File

@ -19,12 +19,12 @@ jest.mock('../../core/components/TagFilter/TagFilter', () => ({
},
}));
async function getTestContext({ name, interval, items }: Partial<Playlist> = {}) {
async function getTestContext({ name, interval, items, uid }: Partial<Playlist> = {}) {
jest.clearAllMocks();
const playlist = { name, items, interval } as unknown as Playlist;
const playlist = { name, items, interval, uid } as unknown as Playlist;
const queryParams = {};
const route: any = {};
const match: any = { params: { id: 1 } };
const match: any = { params: { uid: 'foo' } };
const location: any = {};
const history: any = {};
const navModel: any = {
@ -37,6 +37,7 @@ async function getTestContext({ name, interval, items }: Partial<Playlist> = {})
name: 'Test Playlist',
interval: '5s',
items: [{ title: 'First item', type: 'dashboard_by_id', order: 1, value: '1' }],
uid: 'foo',
});
const { rerender } = render(
<PlaylistEditPage
@ -76,7 +77,7 @@ describe('PlaylistEditPage', () => {
await userEvent.type(screen.getByRole('textbox', { name: /playlist interval/i }), '10s');
fireEvent.submit(screen.getByRole('button', { name: /save/i }));
await waitFor(() => expect(putMock).toHaveBeenCalledTimes(1));
expect(putMock).toHaveBeenCalledWith('/api/playlists/1', {
expect(putMock).toHaveBeenCalledWith('/api/playlists/foo', {
name: 'A Name',
interval: '10s',
items: [{ title: 'First item', type: 'dashboard_by_id', order: 1, value: '1' }],

View File

@ -21,16 +21,16 @@ interface ConnectedProps {
}
export interface RouteParams {
id: number;
uid: string;
}
interface Props extends ConnectedProps, GrafanaRouteComponentProps<RouteParams> {}
export const PlaylistEditPage: FC<Props> = ({ navModel, match }) => {
const styles = useStyles2(getPlaylistStyles);
const { playlist, loading } = usePlaylist(match.params.id);
const { playlist, loading } = usePlaylist(match.params.uid);
const onSubmit = async (playlist: Playlist) => {
await updatePlaylist(match.params.id, playlist);
await updatePlaylist(match.params.uid, playlist);
locationService.push('/playlists');
};

View File

@ -12,9 +12,9 @@ jest.mock('../../core/components/TagFilter/TagFilter', () => ({
},
}));
function getTestContext({ name, interval, items }: Partial<Playlist> = {}) {
function getTestContext({ name, interval, items, uid }: Partial<Playlist> = {}) {
const onSubmitMock = jest.fn();
const playlist = { name, items, interval } as unknown as Playlist;
const playlist = { name, items, interval, uid } as unknown as Playlist;
const { rerender } = render(<PlaylistForm onSubmit={onSubmitMock} playlist={playlist} />);
return { onSubmitMock, playlist, rerender };
@ -28,6 +28,7 @@ const playlist: Playlist = {
{ title: 'Middle item', type: 'dashboard_by_id', order: 2, value: '2' },
{ title: 'Last item', type: 'dashboard_by_tag', order: 2, value: 'Last item' },
],
uid: 'foo',
};
function rows() {
@ -135,7 +136,15 @@ describe('PlaylistForm', () => {
await userEvent.click(screen.getByRole('button', { name: /save/i }));
expect(onSubmitMock).toHaveBeenCalledTimes(1);
expect(onSubmitMock).toHaveBeenCalledWith(playlist);
expect(onSubmitMock).toHaveBeenCalledWith({
name: 'A test playlist',
interval: '10m',
items: [
{ title: 'First item', type: 'dashboard_by_id', order: 1, value: '1' },
{ title: 'Middle item', type: 'dashboard_by_id', order: 2, value: '2' },
{ title: 'Last item', type: 'dashboard_by_tag', order: 2, value: 'Last item' },
],
});
});
describe('and name is missing', () => {

View File

@ -69,6 +69,7 @@ describe('PlaylistPage', () => {
{ title: 'Middle item', type: 'dashboard_by_id', order: 2, value: '2' },
{ title: 'Last item', type: 'dashboard_by_tag', order: 2, value: 'Last item' },
],
uid: 'playlist-0',
},
]);
const { getByText } = getTestContext();

View File

@ -52,7 +52,7 @@ export const PlaylistPage: FC<PlaylistPageProps> = ({ navModel }) => {
if (!playlistToDelete) {
return;
}
deletePlaylist(playlistToDelete.id).finally(() => {
deletePlaylist(playlistToDelete.uid).finally(() => {
setForcePlaylistsFetch(forcePlaylistsFetch + 1);
setPlaylistToDelete(undefined);
});

View File

@ -21,7 +21,7 @@ export const PlaylistPageList = ({ playlists, setStartPlaylist, setPlaylistToDel
return (
<ul className={styles.list}>
{playlists!.map((playlist: PlaylistDTO) => (
<li className={styles.listItem} key={playlist.id.toString()}>
<li className={styles.listItem} key={playlist.uid}>
<Card>
<Card.Heading>
{playlist.name}
@ -33,7 +33,7 @@ export const PlaylistPageList = ({ playlists, setStartPlaylist, setPlaylistToDel
iconSize="lg"
onClick={() => {
showModal(ShareModal, {
playlistId: playlist.id,
playlistUid: playlist.uid,
onDismiss: hideModal,
});
}}
@ -47,12 +47,12 @@ export const PlaylistPageList = ({ playlists, setStartPlaylist, setPlaylistToDel
</Button>
{contextSrv.isEditor && (
<>
<LinkButton key="edit" variant="secondary" href={`/playlists/edit/${playlist.id}`} icon="cog">
<LinkButton key="edit" variant="secondary" href={`/playlists/edit/${playlist.uid}`} icon="cog">
Edit playlist
</LinkButton>
<Button
disabled={false}
onClick={() => setPlaylistToDelete({ id: playlist.id, name: playlist.name })}
onClick={() => setPlaylistToDelete({ id: playlist.id, uid: playlist.uid, name: playlist.name })}
icon="trash-alt"
variant="destructive"
>

View File

@ -29,7 +29,7 @@ setStore(
const dashboards = [{ url: '/dash1' }, { url: '/dash2' }];
function createPlaylistSrv(): PlaylistSrv {
locationService.push('/playlists/1');
locationService.push('/playlists/foo');
return new PlaylistSrv();
}
@ -66,9 +66,9 @@ describe('PlaylistSrv', () => {
getMock.mockImplementation(
jest.fn((url) => {
switch (url) {
case '/api/playlists/1':
case '/api/playlists/foo':
return Promise.resolve({ interval: '1s' });
case '/api/playlists/1/dashboards':
case '/api/playlists/foo/dashboards':
return Promise.resolve(dashboards);
default:
throw new Error(`Unexpected url=${url}`);
@ -88,7 +88,7 @@ describe('PlaylistSrv', () => {
});
it('runs all dashboards in cycle and reloads page after 3 cycles', async () => {
await srv.start(1);
await srv.start('foo');
for (let i = 0; i < 6; i++) {
srv.next();
@ -99,7 +99,7 @@ describe('PlaylistSrv', () => {
});
it('keeps the refresh counter value after restarting', async () => {
await srv.start(1);
await srv.start('foo');
// 1 complete loop
for (let i = 0; i < 3; i++) {
@ -107,7 +107,7 @@ describe('PlaylistSrv', () => {
}
srv.stop();
await srv.start(1);
await srv.start('foo');
// Another 2 loops
for (let i = 0; i < 4; i++) {
@ -119,7 +119,7 @@ describe('PlaylistSrv', () => {
});
it('Should stop playlist when navigating away', async () => {
await srv.start(1);
await srv.start('foo');
locationService.push('/datasources');
@ -127,7 +127,7 @@ describe('PlaylistSrv', () => {
});
it('storeUpdated should not stop playlist when navigating to next dashboard', async () => {
await srv.start(1);
await srv.start('foo');
srv.next();

View File

@ -68,7 +68,7 @@ export class PlaylistSrv {
}
}
start(playlistId: number) {
start(playlistUid: string) {
this.stop();
this.startUrl = window.location.href;
@ -79,10 +79,10 @@ export class PlaylistSrv {
this.locationListenerUnsub = locationService.getHistory().listen(this.locationUpdated);
return getBackendSrv()
.get(`/api/playlists/${playlistId}`)
.get(`/api/playlists/${playlistUid}`)
.then((playlist: any) => {
return getBackendSrv()
.get(`/api/playlists/${playlistId}/dashboards`)
.get(`/api/playlists/${playlistUid}/dashboards`)
.then((dashboards: any) => {
this.dashboards = dashboards;
this.interval = rangeUtil.intervalToMs(playlist.interval);

View File

@ -4,10 +4,10 @@ import { GrafanaRouteComponentProps } from '../../core/navigation/types';
import { playlistSrv } from './PlaylistSrv';
interface Props extends GrafanaRouteComponentProps<{ id: string }> {}
interface Props extends GrafanaRouteComponentProps<{ uid: string }> {}
export const PlaylistStartPage: FC<Props> = ({ match }) => {
playlistSrv.start(parseInt(match.params.id, 10));
playlistSrv.start(match.params.uid);
return null;
};

View File

@ -9,11 +9,11 @@ import { buildBaseUrl } from '../dashboard/components/ShareModal/utils';
import { PlaylistMode } from './types';
interface ShareModalProps {
playlistId: number;
playlistUid: string;
onDismiss: () => void;
}
export const ShareModal = ({ playlistId, onDismiss }: ShareModalProps) => {
export const ShareModal = ({ playlistUid, onDismiss }: ShareModalProps) => {
const [mode, setMode] = useState<PlaylistMode>(false);
const [autoFit, setAutofit] = useState(false);
@ -35,7 +35,7 @@ export const ShareModal = ({ playlistId, onDismiss }: ShareModalProps) => {
params.autofitpanels = true;
}
const shareUrl = urlUtil.renderUrl(`${buildBaseUrl()}/play/${playlistId}`, params);
const shareUrl = urlUtil.renderUrl(`${buildBaseUrl()}/play/${playlistUid}`, params);
return (
<Modal isOpen={true} title="Share playlist" onDismiss={onDismiss}>

View File

@ -29,7 +29,7 @@ export const StartModal: FC<StartModalProps> = ({ playlist, onDismiss }) => {
if (autoFit) {
params.autofitpanels = true;
}
locationService.push(urlUtil.renderUrl(`/playlists/play/${playlist.id}`, params));
locationService.push(urlUtil.renderUrl(`/playlists/play/${playlist.uid}`, params));
};
return (

View File

@ -10,16 +10,16 @@ export async function createPlaylist(playlist: Playlist) {
await withErrorHandling(() => getBackendSrv().post('/api/playlists', playlist));
}
export async function updatePlaylist(id: number, playlist: Playlist) {
await withErrorHandling(() => getBackendSrv().put(`/api/playlists/${id}`, playlist));
export async function updatePlaylist(uid: string, playlist: Playlist) {
await withErrorHandling(() => getBackendSrv().put(`/api/playlists/${uid}`, playlist));
}
export async function deletePlaylist(id: number) {
await withErrorHandling(() => getBackendSrv().delete(`/api/playlists/${id}`), 'Playlist deleted');
export async function deletePlaylist(uid: string) {
await withErrorHandling(() => getBackendSrv().delete(`/api/playlists/${uid}`), 'Playlist deleted');
}
export async function getPlaylist(id: number): Promise<Playlist> {
const result: Playlist = await getBackendSrv().get(`/api/playlists/${id}`);
export async function getPlaylist(uid: string): Promise<Playlist> {
const result: Playlist = await getBackendSrv().get(`/api/playlists/${uid}`);
return result;
}

View File

@ -2,6 +2,7 @@ export interface PlaylistDTO {
id: number;
name: string;
startUrl?: string;
uid: string;
}
export type PlaylistMode = boolean | 'tv';
@ -17,6 +18,7 @@ export interface Playlist {
name: string;
interval: string;
items?: PlaylistItem[];
uid: string;
}
export interface PlaylistItem {

View File

@ -3,22 +3,22 @@ import { useEffect, useState } from 'react';
import { getPlaylist } from './api';
import { Playlist } from './types';
export function usePlaylist(playlistId?: number) {
const [playlist, setPlaylist] = useState<Playlist>({ items: [], interval: '5m', name: '' });
export function usePlaylist(playlistUid?: string) {
const [playlist, setPlaylist] = useState<Playlist>({ items: [], interval: '5m', name: '', uid: '' });
const [loading, setLoading] = useState<boolean>(true);
useEffect(() => {
const initPlaylist = async () => {
if (!playlistId) {
if (!playlistUid) {
setLoading(false);
return;
}
const list = await getPlaylist(playlistId);
const list = await getPlaylist(playlistUid);
setPlaylist(list);
setLoading(false);
};
initPlaylist();
}, [playlistId]);
}, [playlistUid]);
return { playlist, loading };
}

View File

@ -371,7 +371,7 @@ export function getAppRoutes(): RouteDescriptor[] {
),
},
{
path: '/playlists/play/:id',
path: '/playlists/play/:uid',
component: SafeDynamicImport(
() => import(/* webpackChunkName: "PlaylistStartPage"*/ 'app/features/playlist/PlaylistStartPage')
),
@ -383,7 +383,7 @@ export function getAppRoutes(): RouteDescriptor[] {
),
},
{
path: '/playlists/edit/:id',
path: '/playlists/edit/:uid',
component: SafeDynamicImport(
() => import(/* webpackChunkName: "PlaylistEditPage"*/ 'app/features/playlist/PlaylistEditPage')
),