From 3b1a8d45ed3d98064eb14cde18fe9b05a3bfef0f Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Tue, 4 Oct 2022 08:11:18 -0700 Subject: [PATCH] Playlist: update service so it supports both read+write (#55959) --- .../src/raw/playlist/x/playlist.gen.ts | 22 +--- pkg/api/playlist.go | 71 +++--------- pkg/api/playlist_play.go | 31 ++++-- pkg/coremodel/playlist/coremodel.cue | 21 ++-- pkg/coremodel/playlist/playlist_gen.go | 18 +--- pkg/models/playlist.go | 101 ------------------ pkg/services/export/export_sys_playlists.go | 12 ++- pkg/services/playlist/model.go | 34 +++--- pkg/services/playlist/playlist.go | 4 +- .../playlist/playlistimpl/playlist.go | 27 ++++- .../playlist/playlistimpl/sqlx_store.go | 10 +- .../playlist/playlistimpl/store_test.go | 6 +- .../playlist/playlistimpl/xorm_store.go | 24 ++--- pkg/services/sqlstore/mockstore/mockstore.go | 24 ----- 14 files changed, 118 insertions(+), 287 deletions(-) delete mode 100644 pkg/models/playlist.go diff --git a/packages/grafana-schema/src/raw/playlist/x/playlist.gen.ts b/packages/grafana-schema/src/raw/playlist/x/playlist.gen.ts index bfd1c192b1b..73afa9d350f 100644 --- a/packages/grafana-schema/src/raw/playlist/x/playlist.gen.ts +++ b/packages/grafana-schema/src/raw/playlist/x/playlist.gen.ts @@ -7,10 +7,6 @@ // Run `make gen-cue` from repository root to regenerate. export interface Playlist { - /** - * Unique playlist identifier for internal use, set by Grafana. - */ - id: number; /** * Interval sets the time between switching views in a playlist. * FIXME: Is this based on a standardized format or what options are available? Can datemath be used? @@ -20,15 +16,6 @@ export interface Playlist { * The ordered list of items that the playlist will iterate over. */ items?: Array<{ - /** - * FIXME: The prefixDropper removes playlist from playlist_id, that doesn't work for us since it'll mean we'll have Id twice. - * ID of the playlist item for internal use by Grafana. Deprecated. - */ - id: number; - /** - * PlaylistID for the playlist containing the item. Deprecated. - */ - playlistid: number; /** * Type of the item. */ @@ -38,19 +25,16 @@ export interface Playlist { * * - dashboard_by_id: The value is an internal numerical identifier set by Grafana. This * is not portable as the numerical identifier is non-deterministic between different instances. - * Will be replaced by dashboard_by_uid in the future. + * Will be replaced by dashboard_by_uid in the future. (deprecated) * - dashboard_by_tag: The value is a tag which is set on any number of dashboards. All * dashboards behind the tag will be added to the playlist. + * - dashboard_by_uid: The value is the dashboard UID */ value: string; /** - * Title is the human-readable identifier for the playlist item. + * Title is an unused property -- it will be removed in the future */ title: string; - /** - * Order is the position in the list for the item. Deprecated. - */ - order: number; }>; /** * Name of the playlist. diff --git a/pkg/api/playlist.go b/pkg/api/playlist.go index 3909929a7ca..e37f4d5c577 100644 --- a/pkg/api/playlist.go +++ b/pkg/api/playlist.go @@ -1,12 +1,10 @@ package api import ( - "context" "net/http" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" - cmplaylist "github.com/grafana/grafana/pkg/coremodel/playlist" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/playlist" "github.com/grafana/grafana/pkg/web" @@ -15,7 +13,7 @@ import ( func (hs *HTTPServer) ValidateOrgPlaylist(c *models.ReqContext) { uid := web.Params(c.Req)[":uid"] query := playlist.GetPlaylistByUidQuery{UID: uid, OrgId: c.OrgID} - p, err := hs.playlistService.Get(c.Req.Context(), &query) + p, err := hs.playlistService.GetWithoutItems(c.Req.Context(), &query) if err != nil { c.JsonApiErr(404, "Playlist not found", err) @@ -76,58 +74,14 @@ func (hs *HTTPServer) GetPlaylist(c *models.ReqContext) response.Response { uid := web.Params(c.Req)[":uid"] cmd := playlist.GetPlaylistByUidQuery{UID: uid, OrgId: c.OrgID} - p, err := hs.playlistService.Get(c.Req.Context(), &cmd) + dto, err := hs.playlistService.Get(c.Req.Context(), &cmd) if err != nil { return response.Error(500, "Playlist not found", err) } - playlistDTOs, _ := hs.LoadPlaylistItemDTOs(c.Req.Context(), uid, c.OrgID) - - dto := &playlist.PlaylistDTO{ - OrgId: p.OrgId, - } - dto.Id = p.Id - dto.Uid = p.UID - dto.Name = p.Name - dto.Interval = p.Interval - dto.Items = &playlistDTOs - return response.JSON(http.StatusOK, dto) } -func (hs *HTTPServer) LoadPlaylistItemDTOs(ctx context.Context, uid string, orgId int64) ([]playlist.PlaylistItemDTO, error) { - playlistitems, err := hs.LoadPlaylistItems(ctx, uid, orgId) - - if err != nil { - return nil, err - } - - playlistDTOs := make([]playlist.PlaylistItemDTO, 0) - - for _, item := range playlistitems { - playlistDTOs = append(playlistDTOs, playlist.PlaylistItemDTO{ - Id: item.Id, - Playlistid: item.PlaylistId, - Type: cmplaylist.PlaylistItemType(item.Type), - Value: item.Value, - Order: item.Order, - Title: item.Title, - }) - } - - return playlistDTOs, nil -} - -func (hs *HTTPServer) LoadPlaylistItems(ctx context.Context, uid string, orgId int64) ([]playlist.PlaylistItem, error) { - itemQuery := playlist.GetPlaylistItemsByUidQuery{PlaylistUID: uid, OrgId: orgId} - items, err := hs.playlistService.GetItems(ctx, &itemQuery) - if err != nil { - return nil, err - } - - return items, nil -} - // swagger:route GET /playlists/{uid}/items playlists getPlaylistItems // // Get playlist items. @@ -140,14 +94,14 @@ func (hs *HTTPServer) LoadPlaylistItems(ctx context.Context, uid string, orgId i // 500: internalServerError func (hs *HTTPServer) GetPlaylistItems(c *models.ReqContext) response.Response { uid := web.Params(c.Req)[":uid"] + cmd := playlist.GetPlaylistByUidQuery{UID: uid, OrgId: c.OrgID} - playlistDTOs, err := hs.LoadPlaylistItemDTOs(c.Req.Context(), uid, c.OrgID) - + dto, err := hs.playlistService.Get(c.Req.Context(), &cmd) if err != nil { - return response.Error(500, "Could not load playlist items", err) + return response.Error(500, "Playlist not found", err) } - return response.JSON(http.StatusOK, playlistDTOs) + return response.JSON(http.StatusOK, dto.Items) } // swagger:route GET /playlists/{uid}/dashboards playlists getPlaylistDashboards @@ -235,18 +189,19 @@ func (hs *HTTPServer) UpdatePlaylist(c *models.ReqContext) response.Response { cmd.OrgId = c.OrgID cmd.UID = web.Params(c.Req)[":uid"] - p, err := hs.playlistService.Update(c.Req.Context(), &cmd) + _, err := hs.playlistService.Update(c.Req.Context(), &cmd) if err != nil { return response.Error(500, "Failed to save playlist", err) } - playlistDTOs, err := hs.LoadPlaylistItemDTOs(c.Req.Context(), cmd.UID, c.OrgID) + dto, err := hs.playlistService.Get(c.Req.Context(), &playlist.GetPlaylistByUidQuery{ + UID: cmd.UID, + OrgId: c.OrgID, + }) if err != nil { - return response.Error(500, "Failed to save playlist", err) + return response.Error(500, "Failed to load playlist", err) } - - p.Items = &playlistDTOs - return response.JSON(http.StatusOK, p) + return response.JSON(http.StatusOK, dto) } // swagger:parameters searchPlaylists diff --git a/pkg/api/playlist_play.go b/pkg/api/playlist_play.go index 530a8238deb..156bd28d81e 100644 --- a/pkg/api/playlist_play.go +++ b/pkg/api/playlist_play.go @@ -2,12 +2,14 @@ package api import ( "context" + "fmt" "sort" "strconv" "github.com/grafana/grafana/pkg/api/dtos" _ "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/playlist" "github.com/grafana/grafana/pkg/services/search" "github.com/grafana/grafana/pkg/services/user" ) @@ -68,28 +70,37 @@ func (hs *HTTPServer) populateDashboardsByTag(ctx context.Context, orgID int64, // Deprecated -- the frontend can do this better func (hs *HTTPServer) LoadPlaylistDashboards(ctx context.Context, orgID int64, signedInUser *user.SignedInUser, playlistUID string) (dtos.PlaylistDashboardsSlice, error) { - playlistItems, _ := hs.LoadPlaylistItems(ctx, playlistUID, orgID) + result := make(dtos.PlaylistDashboardsSlice, 0) + dto, err := hs.playlistService.Get(ctx, + &playlist.GetPlaylistByUidQuery{UID: playlistUID, OrgId: orgID}) + if err != nil || dto == nil || dto.Items == nil { + return result, err + } + + playlistItems := *dto.Items dashboardByIDs := make([]int64, 0) dashboardByTag := make([]string, 0) dashboardIDOrder := make(map[int64]int) dashboardTagOrder := make(map[string]int) - for _, i := range playlistItems { - if i.Type == "dashboard_by_id" { - dashboardID, _ := strconv.ParseInt(i.Value, 10, 64) + for i, item := range playlistItems { + if item.Type == "dashboard_by_id" { + dashboardID, _ := strconv.ParseInt(item.Value, 10, 64) dashboardByIDs = append(dashboardByIDs, dashboardID) - dashboardIDOrder[dashboardID] = i.Order + dashboardIDOrder[dashboardID] = i } - if i.Type == "dashboard_by_tag" { - dashboardByTag = append(dashboardByTag, i.Value) - dashboardTagOrder[i.Value] = i.Order + if item.Type == "dashboard_by_tag" { + dashboardByTag = append(dashboardByTag, item.Value) + dashboardTagOrder[item.Value] = i + } + + if item.Type == "dashboard_by_uid" { + return nil, fmt.Errorf("dashboard_by_uid not supported by this deprecated API") } } - result := make(dtos.PlaylistDashboardsSlice, 0) - var k, _ = hs.populateDashboardsByID(ctx, dashboardByIDs, dashboardIDOrder) result = append(result, k...) result = append(result, hs.populateDashboardsByTag(ctx, orgID, signedInUser, dashboardByTag, dashboardTagOrder)...) diff --git a/pkg/coremodel/playlist/coremodel.cue b/pkg/coremodel/playlist/coremodel.cue index 15a1a3c2422..f881798bade 100644 --- a/pkg/coremodel/playlist/coremodel.cue +++ b/pkg/coremodel/playlist/coremodel.cue @@ -10,16 +10,17 @@ seqs: [ { schemas: [ {//0.0 - // Unique playlist identifier for internal use, set by Grafana. - id: int64 @grafana(decisionNeeded) // Unique playlist identifier. Generated on creation, either by the // creator of the playlist of by the application. uid: string + // Name of the playlist. name: string + // Interval sets the time between switching views in a playlist. // FIXME: Is this based on a standardized format or what options are available? Can datemath be used? interval: string | *"5m" + // The ordered list of items that the playlist will iterate over. items?: [...#PlaylistItem] @@ -27,26 +28,20 @@ seqs: [ // Definitions (referenced above) are declared below #PlaylistItem: { - // FIXME: The prefixDropper removes playlist from playlist_id, that doesn't work for us since it'll mean we'll have Id twice. - // ID of the playlist item for internal use by Grafana. Deprecated. - id: int64 @grafana(decisionNeeded) - // PlaylistID for the playlist containing the item. Deprecated. - playlistid: int64 @grafana(decisionNeeded) - // Type of the item. type: "dashboard_by_uid" | "dashboard_by_id" | "dashboard_by_tag" // Value depends on type and describes the playlist item. // // - dashboard_by_id: The value is an internal numerical identifier set by Grafana. This // is not portable as the numerical identifier is non-deterministic between different instances. - // Will be replaced by dashboard_by_uid in the future. + // Will be replaced by dashboard_by_uid in the future. (deprecated) // - dashboard_by_tag: The value is a tag which is set on any number of dashboards. All // dashboards behind the tag will be added to the playlist. + // - dashboard_by_uid: The value is the dashboard UID value: string - // Title is the human-readable identifier for the playlist item. - title: string @grafana(decisionNeeded) - // Order is the position in the list for the item. Deprecated. - order: int64 | *0 @grafana(decisionNeeded) + + // Title is an unused property -- it will be removed in the future + title: string } } ] diff --git a/pkg/coremodel/playlist/playlist_gen.go b/pkg/coremodel/playlist/playlist_gen.go index d885aef5c8b..a742e99ac88 100644 --- a/pkg/coremodel/playlist/playlist_gen.go +++ b/pkg/coremodel/playlist/playlist_gen.go @@ -31,9 +31,6 @@ const ( // THIS TYPE IS INTENDED FOR INTERNAL USE BY THE GRAFANA BACKEND, AND IS SUBJECT TO BREAKING CHANGES. // Equivalent Go types at stable import paths are provided in https://github.com/grafana/grok. type Model struct { - // Unique playlist identifier for internal use, set by Grafana. - Id int64 `json:"id"` - // Interval sets the time between switching views in a playlist. // FIXME: Is this based on a standardized format or what options are available? Can datemath be used? Interval string `json:"interval"` @@ -54,17 +51,7 @@ type Model struct { // THIS TYPE IS INTENDED FOR INTERNAL USE BY THE GRAFANA BACKEND, AND IS SUBJECT TO BREAKING CHANGES. // Equivalent Go types at stable import paths are provided in https://github.com/grafana/grok. type PlaylistItem struct { - // FIXME: The prefixDropper removes playlist from playlist_id, that doesn't work for us since it'll mean we'll have Id twice. - // ID of the playlist item for internal use by Grafana. Deprecated. - Id int64 `json:"id"` - - // Order is the position in the list for the item. Deprecated. - Order int `json:"order"` - - // ID for the playlist containing the item. Deprecated. - Playlistid int64 `json:"playlistid"` - - // Title is the human-readable identifier for the playlist item. + // Title is an unused property -- it will be removed in the future Title string `json:"title"` // Type of the item. @@ -74,9 +61,10 @@ type PlaylistItem struct { // // - dashboard_by_id: The value is an internal numerical identifier set by Grafana. This // is not portable as the numerical identifier is non-deterministic between different instances. - // Will be replaced by dashboard_by_uid in the future. + // Will be replaced by dashboard_by_uid in the future. (deprecated) // - dashboard_by_tag: The value is a tag which is set on any number of dashboards. All // dashboards behind the tag will be added to the playlist. + // - dashboard_by_uid: The value is the dashboard UID Value string `json:"value"` } diff --git a/pkg/models/playlist.go b/pkg/models/playlist.go deleted file mode 100644 index 7be6cba05d0..00000000000 --- a/pkg/models/playlist.go +++ /dev/null @@ -1,101 +0,0 @@ -package models - -import ( - "errors" -) - -// Typed errors -var ( - 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:"-"` -} - -type PlaylistDTO struct { - Id int64 `json:"id"` - UID string `json:"uid"` - Name string `json:"name"` - Interval string `json:"interval"` - OrgId int64 `json:"-"` - Items []PlaylistItemDTO `json:"items"` -} - -type PlaylistItemDTO struct { - Id int64 `json:"id"` - PlaylistId int64 `json:"playlistid"` - Type string `json:"type"` - Title string `json:"title"` - Value string `json:"value"` - Order int `json:"order"` -} - -type PlaylistItem struct { - Id int64 - PlaylistId int64 - Type string - Value string - Order int - Title string -} - -type Playlists []*Playlist - -// -// COMMANDS -// - -type UpdatePlaylistCommand struct { - OrgId int64 `json:"-"` - UID string `json:"uid"` - Name string `json:"name" binding:"Required"` - Interval string `json:"interval"` - Items []PlaylistItemDTO `json:"items"` - - Result *PlaylistDTO -} - -type CreatePlaylistCommand struct { - Name string `json:"name" binding:"Required"` - Interval string `json:"interval"` - Items []PlaylistItemDTO `json:"items"` - - OrgId int64 `json:"-"` - Result *Playlist -} - -type DeletePlaylistCommand struct { - UID string - OrgId int64 -} - -// -// QUERIES -// - -type GetPlaylistsQuery struct { - Name string - Limit int - OrgId int64 - - Result Playlists -} - -type GetPlaylistByUidQuery struct { - UID string - OrgId int64 - Result *Playlist -} - -type GetPlaylistItemsByUidQuery struct { - PlaylistUID string - OrgId int64 - Result *[]PlaylistItem -} diff --git a/pkg/services/export/export_sys_playlists.go b/pkg/services/export/export_sys_playlists.go index fa81ee37a62..97578dadc83 100644 --- a/pkg/services/export/export_sys_playlists.go +++ b/pkg/services/export/export_sys_playlists.go @@ -27,11 +27,17 @@ func exportSystemPlaylists(helper *commitHelper, job *gitExportJob) error { comment: "Export playlists", } - for _, playlist := range res { - // TODO: fix the playlist API so it returns the json we need :) + for _, item := range res { + playlist, err := job.playlistService.Get(helper.ctx, &playlist.GetPlaylistByUidQuery{ + UID: item.UID, + OrgId: helper.orgID, + }) + if err != nil { + return err + } gitcmd.body = append(gitcmd.body, commitBody{ - fpath: filepath.Join(helper.orgDir, "system", "playlists", fmt.Sprintf("%s-playlist.json", playlist.UID)), + fpath: filepath.Join(helper.orgDir, "system", "playlists", fmt.Sprintf("%s-playlist.json", playlist.Uid)), body: prettyJSON(playlist), }) } diff --git a/pkg/services/playlist/model.go b/pkg/services/playlist/model.go index 40e7305dd93..3c0f4b6ad64 100644 --- a/pkg/services/playlist/model.go +++ b/pkg/services/playlist/model.go @@ -22,20 +22,17 @@ type Playlist struct { OrgId int64 `json:"-" db:"org_id"` } -type PlaylistDTO struct { - playlist.Model - OrgId int64 `json:"-"` -} - +type PlaylistDTO = playlist.Model type PlaylistItemDTO = playlist.PlaylistItem +type PlaylistItemType = playlist.PlaylistItemType type PlaylistItem struct { Id int64 `db:"id"` PlaylistId int64 `db:"playlist_id"` - Type string `db:"type"` - Value string `db:"value"` - Order int `db:"order"` - Title string `db:"title"` + Type string `json:"type" db:"type"` + Value string `json:"value" db:"value"` + Order int `json:"order" db:"order"` + Title string `json:"title" db:"title"` } type Playlists []*Playlist @@ -45,18 +42,18 @@ type Playlists []*Playlist // type UpdatePlaylistCommand struct { - OrgId int64 `json:"-"` - UID string `json:"uid"` - Name string `json:"name" binding:"Required"` - Interval string `json:"interval"` - Items []PlaylistItemDTO `json:"items"` + OrgId int64 `json:"-"` + UID string `json:"uid"` + Name string `json:"name" binding:"Required"` + Interval string `json:"interval"` + Items []PlaylistItem `json:"items"` } type CreatePlaylistCommand struct { - Name string `json:"name" binding:"Required"` - Interval string `json:"interval"` - Items []PlaylistItemDTO `json:"items"` - OrgId int64 `json:"-"` + Name string `json:"name" binding:"Required"` + Interval string `json:"interval"` + Items []PlaylistItem `json:"items"` + OrgId int64 `json:"-"` } type DeletePlaylistCommand struct { @@ -69,6 +66,7 @@ type DeletePlaylistCommand struct { // type GetPlaylistsQuery struct { + // NOTE: the frontend never sends this query Name string Limit int OrgId int64 diff --git a/pkg/services/playlist/playlist.go b/pkg/services/playlist/playlist.go index fbea7a84817..d7fabd62719 100644 --- a/pkg/services/playlist/playlist.go +++ b/pkg/services/playlist/playlist.go @@ -7,8 +7,8 @@ import ( type Service interface { Create(context.Context, *CreatePlaylistCommand) (*Playlist, error) Update(context.Context, *UpdatePlaylistCommand) (*PlaylistDTO, error) - Get(context.Context, *GetPlaylistByUidQuery) (*Playlist, error) - GetItems(context.Context, *GetPlaylistItemsByUidQuery) ([]PlaylistItem, error) + GetWithoutItems(context.Context, *GetPlaylistByUidQuery) (*Playlist, error) + Get(context.Context, *GetPlaylistByUidQuery) (*PlaylistDTO, error) Search(context.Context, *GetPlaylistsQuery) (Playlists, error) Delete(ctx context.Context, cmd *DeletePlaylistCommand) error } diff --git a/pkg/services/playlist/playlistimpl/playlist.go b/pkg/services/playlist/playlistimpl/playlist.go index 6646679005d..14854213b7a 100644 --- a/pkg/services/playlist/playlistimpl/playlist.go +++ b/pkg/services/playlist/playlistimpl/playlist.go @@ -35,12 +35,33 @@ func (s *Service) Update(ctx context.Context, cmd *playlist.UpdatePlaylistComman return s.store.Update(ctx, cmd) } -func (s *Service) Get(ctx context.Context, q *playlist.GetPlaylistByUidQuery) (*playlist.Playlist, error) { +func (s *Service) GetWithoutItems(ctx context.Context, q *playlist.GetPlaylistByUidQuery) (*playlist.Playlist, error) { return s.store.Get(ctx, q) } -func (s *Service) GetItems(ctx context.Context, q *playlist.GetPlaylistItemsByUidQuery) ([]playlist.PlaylistItem, error) { - return s.store.GetItems(ctx, q) +func (s *Service) Get(ctx context.Context, q *playlist.GetPlaylistByUidQuery) (*playlist.PlaylistDTO, error) { + v, err := s.store.Get(ctx, q) + if err != nil { + return nil, err + } + rawItems, err := s.store.GetItems(ctx, &playlist.GetPlaylistItemsByUidQuery{ + PlaylistUID: v.UID, + OrgId: q.OrgId, + }) + if err != nil { + return nil, err + } + items := make([]playlist.PlaylistItemDTO, len(rawItems)) + for i := 0; i < len(rawItems); i++ { + items[i].Type = playlist.PlaylistItemType(rawItems[i].Type) + items[i].Value = rawItems[i].Value + } + return &playlist.PlaylistDTO{ + Uid: v.UID, + Name: v.Name, + Interval: v.Interval, + Items: &items, + }, nil } func (s *Service) Search(ctx context.Context, q *playlist.GetPlaylistsQuery) (playlist.Playlists, error) { diff --git a/pkg/services/playlist/playlistimpl/sqlx_store.go b/pkg/services/playlist/playlistimpl/sqlx_store.go index 34a4d1fe882..4def99ad6b9 100644 --- a/pkg/services/playlist/playlistimpl/sqlx_store.go +++ b/pkg/services/playlist/playlistimpl/sqlx_store.go @@ -39,12 +39,12 @@ func (s *sqlxStore) Insert(ctx context.Context, cmd *playlist.CreatePlaylistComm if len(cmd.Items) > 0 { playlistItems := make([]playlist.PlaylistItem, 0) - for _, item := range cmd.Items { + for order, item := range cmd.Items { playlistItems = append(playlistItems, playlist.PlaylistItem{ PlaylistId: p.Id, - Type: string(item.Type), + Type: item.Type, Value: item.Value, - Order: item.Order, + Order: order + 1, Title: item.Title, }) } @@ -94,7 +94,7 @@ func (s *sqlxStore) Update(ctx context.Context, cmd *playlist.UpdatePlaylistComm for index, item := range cmd.Items { playlistItems = append(playlistItems, playlist.PlaylistItem{ PlaylistId: p.Id, - Type: string(item.Type), + Type: item.Type, Value: item.Value, Order: index + 1, Title: item.Title, @@ -197,5 +197,5 @@ func newGenerateAndValidateNewPlaylistUid(ctx context.Context, sess *session.Ses } } - return "", models.ErrPlaylistFailedGenerateUniqueUid + return "", playlist.ErrPlaylistFailedGenerateUniqueUid } diff --git a/pkg/services/playlist/playlistimpl/store_test.go b/pkg/services/playlist/playlistimpl/store_test.go index 16d2f9a5ff8..136dab2b623 100644 --- a/pkg/services/playlist/playlistimpl/store_test.go +++ b/pkg/services/playlist/playlistimpl/store_test.go @@ -20,7 +20,7 @@ func testIntegrationPlaylistDataAccess(t *testing.T, fn getStore) { playlistStore := fn(ss) t.Run("Can create playlist", func(t *testing.T) { - items := []playlist.PlaylistItemDTO{ + items := []playlist.PlaylistItem{ {Title: "graphite", Value: "graphite", Type: "dashboard_by_tag"}, {Title: "Backend response times", Value: "3", Type: "dashboard_by_id"}, } @@ -44,7 +44,7 @@ func testIntegrationPlaylistDataAccess(t *testing.T, fn getStore) { }) t.Run("Can update playlist", func(t *testing.T) { - items := []playlist.PlaylistItemDTO{ + items := []playlist.PlaylistItem{ {Title: "influxdb", Value: "influxdb", Type: "dashboard_by_tag"}, {Title: "Backend response times", Value: "2", Type: "dashboard_by_id"}, } @@ -66,7 +66,7 @@ func testIntegrationPlaylistDataAccess(t *testing.T, fn getStore) { }) t.Run("Search playlist", func(t *testing.T) { - items := []playlist.PlaylistItemDTO{ + items := []playlist.PlaylistItem{ {Title: "graphite", Value: "graphite", Type: "dashboard_by_tag"}, {Title: "Backend response times", Value: "3", Type: "dashboard_by_id"}, } diff --git a/pkg/services/playlist/playlistimpl/xorm_store.go b/pkg/services/playlist/playlistimpl/xorm_store.go index bb75c1b9900..5e44205c2e1 100644 --- a/pkg/services/playlist/playlistimpl/xorm_store.go +++ b/pkg/services/playlist/playlistimpl/xorm_store.go @@ -35,12 +35,12 @@ func (s *sqlStore) Insert(ctx context.Context, cmd *playlist.CreatePlaylistComma } playlistItems := make([]playlist.PlaylistItem, 0) - for _, item := range cmd.Items { + for order, item := range cmd.Items { playlistItems = append(playlistItems, playlist.PlaylistItem{ PlaylistId: p.Id, - Type: string(item.Type), + Type: item.Type, Value: item.Value, - Order: item.Order, + Order: order + 1, Title: item.Title, }) } @@ -70,12 +70,10 @@ func (s *sqlStore) Update(ctx context.Context, cmd *playlist.UpdatePlaylistComma p.Id = existingPlaylist.Id dto = playlist.PlaylistDTO{ - OrgId: p.OrgId, + Uid: p.UID, + Name: p.Name, + Interval: p.Interval, } - dto.Id = p.Id - dto.Uid = p.UID - dto.Name = p.Name - dto.Interval = p.Interval _, err = sess.Where("id=?", p.Id).Cols("name", "interval").Update(&p) if err != nil { @@ -89,12 +87,12 @@ func (s *sqlStore) Update(ctx context.Context, cmd *playlist.UpdatePlaylistComma return err } - playlistItems := make([]models.PlaylistItem, 0) + playlistItems := make([]playlist.PlaylistItem, 0) for index, item := range cmd.Items { - playlistItems = append(playlistItems, models.PlaylistItem{ + playlistItems = append(playlistItems, playlist.PlaylistItem{ PlaylistId: p.Id, - Type: string(item.Type), + Type: item.Type, Value: item.Value, Order: index + 1, Title: item.Title, @@ -198,7 +196,7 @@ func generateAndValidateNewPlaylistUid(sess *sqlstore.DBSession, orgId int64) (s for i := 0; i < 3; i++ { uid := generateNewUid() - playlist := models.Playlist{OrgId: orgId, UID: uid} + playlist := playlist.Playlist{OrgId: orgId, UID: uid} exists, err := sess.Get(&playlist) if err != nil { return "", err @@ -209,7 +207,7 @@ func generateAndValidateNewPlaylistUid(sess *sqlstore.DBSession, orgId int64) (s } } - return "", models.ErrPlaylistFailedGenerateUniqueUid + return "", playlist.ErrPlaylistFailedGenerateUniqueUid } var generateNewUid func() string = util.GenerateShortUID diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index c493bfc8046..1504cb4de1a 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/pkg/services/sqlstore/mockstore/mockstore.go @@ -267,30 +267,6 @@ func (m SQLStoreMock) GetDashboardACLInfoList(ctx context.Context, query *models return m.ExpectedError } -func (m *SQLStoreMock) CreatePlaylist(ctx context.Context, cmd *models.CreatePlaylistCommand) error { - return m.ExpectedError -} - -func (m *SQLStoreMock) UpdatePlaylist(ctx context.Context, cmd *models.UpdatePlaylistCommand) error { - return m.ExpectedError -} - -func (m *SQLStoreMock) GetPlaylist(ctx context.Context, query *models.GetPlaylistByUidQuery) error { - return m.ExpectedError -} - -func (m *SQLStoreMock) DeletePlaylist(ctx context.Context, cmd *models.DeletePlaylistCommand) error { - return m.ExpectedError -} - -func (m *SQLStoreMock) SearchPlaylists(ctx context.Context, query *models.GetPlaylistsQuery) error { - return m.ExpectedError -} - -func (m *SQLStoreMock) GetPlaylistItem(ctx context.Context, query *models.GetPlaylistItemsByUidQuery) error { - return m.ExpectedError -} - func (m *SQLStoreMock) GetAlertById(ctx context.Context, query *models.GetAlertByIdQuery) error { query.Result = m.ExpectedAlert return m.ExpectedError