refactor(playlist): refactor of playlist feature, and PR #3776

pkg/api/api.go

@@ -189,8 +189,8 @@ func Register(r *macaron.Macaron) {
 			r.Get("/:id/items", ValidateOrgPlaylist, wrap(GetPlaylistItems))
 			r.Get("/:id/dashboards", ValidateOrgPlaylist, wrap(GetPlaylistDashboards))
 			r.Delete("/:id", reqEditorRole, ValidateOrgPlaylist, wrap(DeletePlaylist))
-			r.Put("/:id", reqEditorRole, bind(m.UpdatePlaylistQuery{}), ValidateOrgPlaylist, wrap(UpdatePlaylist))
+			r.Put("/:id", reqEditorRole, bind(m.UpdatePlaylistCommand{}), ValidateOrgPlaylist, wrap(UpdatePlaylist))
-			r.Post("/", reqEditorRole, bind(m.CreatePlaylistQuery{}), wrap(CreatePlaylist))
+			r.Post("/", reqEditorRole, bind(m.CreatePlaylistCommand{}), wrap(CreatePlaylist))
 		})
 
 		// Search

pkg/api/playlist.go

@@ -2,11 +2,12 @@ package api
 
 import (
 	"errors"
+	"strconv"
+
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/log"
 	"github.com/grafana/grafana/pkg/middleware"
 	m "github.com/grafana/grafana/pkg/models"
-	"strconv"
 )
 
 func ValidateOrgPlaylist(c *middleware.Context) {
@@ -33,8 +34,8 @@ func SearchPlaylists(c *middleware.Context) Response {
 		limit = 1000
 	}
 
-	searchQuery := m.PlaylistQuery{
+	searchQuery := m.GetPlaylistsQuery{
-		Title: query,
+		Name:  query,
 		Limit: limit,
 		OrgId: c.OrgId,
 	}
@@ -59,7 +60,7 @@ func GetPlaylist(c *middleware.Context) Response {
 
 	dto := &m.PlaylistDTO{
 		Id:       cmd.Result.Id,
-		Title:    cmd.Result.Title,
+		Name:     cmd.Result.Name,
 		Interval: cmd.Result.Interval,
 		OrgId:    cmd.Result.OrgId,
 		Items:    playlistDTOs,
@@ -159,7 +160,7 @@ func GetPlaylistDashboards(c *middleware.Context) Response {
 func DeletePlaylist(c *middleware.Context) Response {
 	id := c.ParamsInt64(":id")
 
-	cmd := m.DeletePlaylistQuery{Id: id}
+	cmd := m.DeletePlaylistCommand{Id: id, OrgId: c.OrgId}
 	if err := bus.Dispatch(&cmd); err != nil {
 		return ApiError(500, "Failed to delete playlist", err)
 	}
@@ -167,28 +168,27 @@ func DeletePlaylist(c *middleware.Context) Response {
 	return Json(200, "")
 }
 
-func CreatePlaylist(c *middleware.Context, query m.CreatePlaylistQuery) Response {
+func CreatePlaylist(c *middleware.Context, cmd m.CreatePlaylistCommand) Response {
-	query.OrgId = c.OrgId
+	cmd.OrgId = c.OrgId
-	err := bus.Dispatch(&query)
+
-	if err != nil {
+	if err := bus.Dispatch(&cmd); err != nil {
 		return ApiError(500, "Failed to create playlist", err)
 	}
 
-	return Json(200, query.Result)
+	return Json(200, cmd.Result)
 }
 
-func UpdatePlaylist(c *middleware.Context, query m.UpdatePlaylistQuery) Response {
+func UpdatePlaylist(c *middleware.Context, cmd m.UpdatePlaylistCommand) Response {
-	err := bus.Dispatch(&query)
+	cmd.OrgId = c.OrgId
+	if err := bus.Dispatch(&cmd); err != nil {
+		return ApiError(500, "Failed to save playlist", err)
+	}
+
+	playlistDTOs, err := LoadPlaylistItemDTOs(cmd.Id)
 	if err != nil {
 		return ApiError(500, "Failed to save playlist", err)
 	}
 
-	playlistDTOs, err := LoadPlaylistItemDTOs(query.Id)
+	cmd.Result.Items = playlistDTOs
-	if err != nil {
+	return Json(200, cmd.Result)
-		return ApiError(500, "Failed to save playlist", err)
-	}
-
-	query.Result.Items = playlistDTOs
-
-	return Json(200, query.Result)
 }

pkg/models/playlist.go

@@ -13,14 +13,14 @@ var (
 // Playlist model
 type Playlist struct {
 	Id       int64  `json:"id"`
-	Title    string `json:"title"`
+	Name     string `json:"name"`
 	Interval string `json:"interval"`
 	OrgId    int64  `json:"-"`
 }
 
 type PlaylistDTO struct {
 	Id       int64             `json:"id"`
-	Title    string            `json:"title"`
+	Name     string            `json:"name"`
 	Interval string            `json:"interval"`
 	OrgId    int64             `json:"-"`
 	Items    []PlaylistItemDTO `json:"items"`
@@ -71,35 +71,47 @@ type PlaylistDashboardDto struct {
 //
 // COMMANDS
 //
-type PlaylistQuery struct {
-	Title string
-	Limit int
-	OrgId int64
 
-	Result Playlists
+type UpdatePlaylistCommand struct {
-}
+	OrgId    int64             `json:"-"`
-
+	Id       int64             `json:"id" binding:"Required"`
-type UpdatePlaylistQuery struct {
+	Name     string            `json:"name" binding:"Required"`
-	Id       int64
+	Type     string            `json:"type"`
-	Title    string
+	Interval string            `json:"interval"`
-	Type     string
+	Data     []int64           `json:"data"`
-	Interval string
+	Items    []PlaylistItemDTO `json:"items"`
-	Items    []PlaylistItemDTO
 
 	Result *PlaylistDTO
 }
 
-type CreatePlaylistQuery struct {
+type CreatePlaylistCommand struct {
-	Title    string
+	Name     string            `json:"name" binding:"Required"`
-	Type     string
+	Type     string            `json:"type"`
-	Interval string
+	Interval string            `json:"interval"`
-	Data     []int64
+	Data     []int64           `json:"data"`
-	OrgId    int64
+	Items    []PlaylistItemDTO `json:"items"`
-	Items    []PlaylistItemDTO
 
+	OrgId  int64 `json:"-"`
 	Result *Playlist
 }
 
+type DeletePlaylistCommand struct {
+	Id    int64
+	OrgId int64
+}
+
+//
+// QUERIES
+//
+
+type GetPlaylistsQuery struct {
+	Name  string
+	Limit int
+	OrgId int64
+
+	Result Playlists
+}
+
 type GetPlaylistByIdQuery struct {
 	Id     int64
 	Result *Playlist
@@ -114,7 +126,3 @@ type GetPlaylistDashboardsQuery struct {
 	DashboardIds []int64
 	Result       *PlaylistDashboards
 }
-
-type DeletePlaylistQuery struct {
-	Id int64
-}

pkg/services/sqlstore/migrations/playlist_mig.go

@@ -3,20 +3,23 @@ package migrations
 import . "github.com/grafana/grafana/pkg/services/sqlstore/migrator"
 
 func addPlaylistMigrations(mg *Migrator) {
-	playlistV1 := Table{
+	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: "title", Type: DB_NVarchar, Length: 255, Nullable: false},
+			{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 v1", NewAddTableMigration(playlistV1))
+	mg.AddMigration("create playlist table v2", NewAddTableMigration(playlistV2))
 
-	playlistItemV1 := Table{
+	playlistItemV2 := Table{
 		Name: "playlist_item",
 		Columns: []*Column{
 			{Name: "id", Type: DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true},
@@ -28,5 +31,5 @@ func addPlaylistMigrations(mg *Migrator) {
 		},
 	}
 
-	mg.AddMigration("create playlist item table v1", NewAddTableMigration(playlistItemV1))
+	mg.AddMigration("create playlist item table v2", NewAddTableMigration(playlistItemV2))
 }

pkg/services/sqlstore/playlist.go

@@ -2,6 +2,7 @@ package sqlstore
 
 import (
 	"fmt"
+
 	"github.com/go-xorm/xorm"
 
 	"github.com/grafana/grafana/pkg/bus"
@@ -18,13 +19,13 @@ func init() {
 	bus.AddHandler("sql", GetPlaylistItem)
 }
 
-func CreatePlaylist(query *m.CreatePlaylistQuery) error {
+func CreatePlaylist(cmd *m.CreatePlaylistCommand) error {
 	var err error
 
 	playlist := m.Playlist{
-		Title:    query.Title,
+		Name:     cmd.Name,
-		Interval: query.Interval,
+		Interval: cmd.Interval,
-		OrgId:    query.OrgId,
+		OrgId:    cmd.OrgId,
 	}
 
 	_, err = x.Insert(&playlist)
@@ -32,7 +33,7 @@ func CreatePlaylist(query *m.CreatePlaylistQuery) error {
 	fmt.Printf("%v", playlist.Id)
 
 	playlistItems := make([]m.PlaylistItem, 0)
-	for _, item := range query.Items {
+	for _, item := range cmd.Items {
 		playlistItems = append(playlistItems, m.PlaylistItem{
 			PlaylistId: playlist.Id,
 			Type:       item.Type,
@@ -44,40 +45,40 @@ func CreatePlaylist(query *m.CreatePlaylistQuery) error {
 
 	_, err = x.Insert(&playlistItems)
 
-	query.Result = &playlist
+	cmd.Result = &playlist
 	return err
 }
 
-func UpdatePlaylist(query *m.UpdatePlaylistQuery) error {
+func UpdatePlaylist(cmd *m.UpdatePlaylistCommand) error {
 	var err error
-	x.Logger.SetLevel(5)
 	playlist := m.Playlist{
-		Id:       query.Id,
+		Id:       cmd.Id,
-		Title:    query.Title,
+		OrgId:    cmd.OrgId,
-		Interval: query.Interval,
+		Name:     cmd.Name,
+		Interval: cmd.Interval,
 	}
 
-	existingPlaylist := x.Where("id = ?", query.Id).Find(m.Playlist{})
+	existingPlaylist := x.Where("id = ? AND org_id = ?", cmd.Id, cmd.OrgId).Find(m.Playlist{})
 
 	if existingPlaylist == nil {
 		return m.ErrPlaylistNotFound
 	}
 
-	query.Result = &m.PlaylistDTO{
+	cmd.Result = &m.PlaylistDTO{
 		Id:       playlist.Id,
 		OrgId:    playlist.OrgId,
-		Title:    playlist.Title,
+		Name:     playlist.Name,
 		Interval: playlist.Interval,
 	}
 
-	_, err = x.Id(query.Id).Cols("id", "title", "timespan").Update(&playlist)
+	_, err = x.Id(cmd.Id).Cols("id", "name", "interval").Update(&playlist)
 
 	if err != nil {
 		return err
 	}
 
 	rawSql := "DELETE FROM playlist_item WHERE playlist_id = ?"
-	_, err = x.Exec(rawSql, query.Id)
+	_, err = x.Exec(rawSql, cmd.Id)
 
 	if err != nil {
 		return err
@@ -85,7 +86,7 @@ func UpdatePlaylist(query *m.UpdatePlaylistQuery) error {
 
 	playlistItems := make([]m.PlaylistItem, 0)
 
-	for _, item := range query.Items {
+	for _, item := range cmd.Items {
 		playlistItems = append(playlistItems, m.PlaylistItem{
 			PlaylistId: playlist.Id,
 			Type:       item.Type,
@@ -113,33 +114,33 @@ func GetPlaylist(query *m.GetPlaylistByIdQuery) error {
 	return err
 }
 
-func DeletePlaylist(query *m.DeletePlaylistQuery) error {
+func DeletePlaylist(cmd *m.DeletePlaylistCommand) error {
-	if query.Id == 0 {
+	if cmd.Id == 0 {
 		return m.ErrCommandValidationFailed
 	}
 
 	return inTransaction(func(sess *xorm.Session) error {
-		var rawPlaylistSql = "DELETE FROM playlist WHERE id = ?"
+		var rawPlaylistSql = "DELETE FROM playlist WHERE id = ? and org_id = ?"
-		_, err := sess.Exec(rawPlaylistSql, query.Id)
+		_, err := sess.Exec(rawPlaylistSql, cmd.Id, cmd.OrgId)
 
 		if err != nil {
 			return err
 		}
 
 		var rawItemSql = "DELETE FROM playlist_item WHERE playlist_id = ?"
-		_, err2 := sess.Exec(rawItemSql, query.Id)
+		_, err2 := sess.Exec(rawItemSql, cmd.Id)
 
 		return err2
 	})
 }
 
-func SearchPlaylists(query *m.PlaylistQuery) error {
+func SearchPlaylists(query *m.GetPlaylistsQuery) error {
 	var playlists = make(m.Playlists, 0)
 
 	sess := x.Limit(query.Limit)
 
-	if query.Title != "" {
+	if query.Name != "" {
-		sess.Where("title LIKE ?", query.Title)
+		sess.Where("name LIKE ?", query.Name)
 	}
 
 	sess.Where("org_id = ?", query.OrgId)

public/app/core/components/grafana_app.ts

@@ -185,7 +185,6 @@ export function grafanaAppDirective() {
 
         // hide popovers
         var popover = elem.find('.popover');
-        console.log(target.parents('.graph-legend').length);
         if (popover.length > 0 && target.parents('.graph-legend').length === 0) {
           popover.hide();
         }

public/app/features/playlist/all.js

@@ -1,6 +1,6 @@
 define([
   './playlists_ctrl',
-  './playlistSrv',
+  './playlist_srv',
   './playlist_edit_ctrl',
   './playlist_routes'
 ], function () {});

public/app/features/playlist/partials/playlist.html

@@ -1,7 +1,7 @@
 <navbar title="Playlists" title-url="playlists" icon="fa fa-fw fa-list" subnav="true">
 	<ul class="nav">
 		<li ng-class="{active: isNew()}" ng-show="isNew()"><a href="datasources/create">New</a></li>
-		<li class="active" ng-show="!isNew()"><a href="playlists/edit/{{playlist.id}}">{{playlist.title}}</a></li>
+		<li class="active" ng-show="!isNew()"><a href="playlists/edit/{{playlist.id}}">{{playlist.name}}</a></li>
 	</ul>
 </navbar>
 
@@ -20,7 +20,7 @@
               Name
             </li>
             <li>
-              <input type="text" required ng-model="playlist.title" class="input-xlarge tight-form-input">
+              <input type="text" required ng-model="playlist.name" class="input-xlarge tight-form-input">
             </li>
           </ul>
           <div class="clearfix"></div>

public/app/features/playlist/partials/playlists.html

@@ -1,12 +1,12 @@
-<topnav icon="fa fa-fw fa-list" title="Playlists"></topnav>
+<navbar icon="fa fa-fw fa-list" title="Playlists"></navbar>
 
 <div class="page-container">
   <div class="page-wide">
 
-		<button type="submit" class="btn btn-inverse pull-right" ng-click="createPlaylist()">
+		<a class="btn btn-inverse pull-right" href="playlists/create">
 			<i class="fa fa-plus"></i>
 			New playlist
-		</button>
+		</a>
 
     <h2>Saved playlists</h2>
 
@@ -21,7 +21,7 @@
       </thead>
       <tr ng-repeat="playlist in playlists">
         <td>
-					<a href="playlists/edit/{{playlist.id}}">{{playlist.title}}</a>
+					<a href="playlists/edit/{{playlist.id}}">{{playlist.name}}</a>
         </td>
         <td >
 					<a href="playlists/play/{{playlist.id}}">playlists/play/{{playlist.id}}</a>

public/app/features/playlist/playlist_edit_ctrl.js

@@ -13,7 +13,9 @@ function (angular, config, _) {
     $scope.foundPlaylistItems = [];
     $scope.searchQuery = '';
     $scope.loading = false;
-    $scope.playlist = {};
+    $scope.playlist = {
+      interval: '10m',
+    };
     $scope.playlistItems = [];
 
     $scope.init = function() {
@@ -68,7 +70,6 @@ function (angular, config, _) {
 
       $scope.playlistItems.push(playlistItem);
       $scope.filterFoundPlaylistItems();
-
     };
 
     $scope.removePlaylistItem = function(playlistItem) {

public/app/features/playlist/playlist_routes.js

@@ -23,12 +23,9 @@ function (angular) {
         controller : 'PlaylistEditCtrl'
       })
       .when('/playlists/play/:id', {
-        templateUrl: 'app/partials/dashboard.html',
-        controller : 'LoadDashboardCtrl',
         resolve: {
           init: function(playlistSrv, $route) {
             var playlistId = $route.current.params.id;
-
             playlistSrv.start(playlistId);
           }
         }

public/app/features/playlist/playlist_srv.ts

@@ -17,9 +17,7 @@ class PlaylistSrv {
   next() {
     this.$timeout.cancel(this.cancelPromise);
 
-    angular.element(window).unbind('resize');
+    if (this.index > this.dashboards.length - 1) {
-
-    if (this.index > this.dashboards.length -1) {
       this.start(this.playlistId);
     } else {
       var dash = this.dashboards[this.index];
@@ -27,11 +25,11 @@ class PlaylistSrv {
       this.$location.url('dashboard/' + dash.uri);
 
       this.index++;
-      this.cancelPromise = this.$timeout(() => { this.next(); }, this.interval);
+      this.cancelPromise = this.$timeout(() => this.next(), this.interval);
     }
   }
 
-  prevfunction() {
+  prev() {
     this.index = Math.max(this.index - 2, 0);
     this.next();
   }
@@ -41,20 +39,15 @@ class PlaylistSrv {
 
     this.index = 0;
     this.playlistId = playlistId;
-
     this.$rootScope.playlistSrv = this;
 
-    this.backendSrv.get('/api/playlists/' + playlistId)
+    this.backendSrv.get(`/api/playlists/${playlistId}`).then(playlist => {
-      .then((playlist) => {
+      this.backendSrv.get(`/api/playlists/${playlistId}/dashboards`).then(dashboards => {
-        this.backendSrv.get('/api/playlists/' + playlistId + '/dashboards')
+        this.dashboards = dashboards;
-          .then((dashboards) => {
+        this.interval = kbn.interval_to_ms(playlist.interval);
-            this.dashboards = dashboards;
+        this.next();
-            this.interval = kbn.interval_to_ms(playlist.interval);
-            this.cancelPromise = this.$timeout(() => { this.next(); }, this.interval);
-
-            this.next();
-          });
       });
+    });
   }
 
   stop() {
@@ -62,7 +55,7 @@ class PlaylistSrv {
     this.playlistId = 0;
 
     if (this.cancelPromise) {
-        this.$timeout.cancel(this.cancelPromise);
+      this.$timeout.cancel(this.cancelPromise);
     }
 
     this.$rootScope.playlistSrv = null;

public/app/features/playlist/playlists_ctrl.js

@@ -13,31 +13,31 @@ function (angular, _) {
         $scope.playlists = result;
       });
 
-    $scope.removePlaylist = function(playlist) {
+    $scope.removePlaylistConfirmed = function(playlist) {
-      var modalScope = $scope.$new(true);
+      _.remove($scope.playlists, {id: playlist.id});
 
-      modalScope.playlist = playlist;
+      backendSrv.delete('/api/playlists/' + playlist.id)
-      modalScope.removePlaylist = function() {
+      .then(function() {
-        modalScope.dismiss();
+        $scope.appEvent('alert-success', ['Playlist deleted', '']);
-        _.remove($scope.playlists, {id: playlist.id});
+      }, function() {
-
+        $scope.appEvent('alert-error', ['Unable to delete playlist', '']);
-        backendSrv.delete('/api/playlists/' + playlist.id)
+        $scope.playlists.push(playlist);
-          .then(function() {
-            $scope.appEvent('alert-success', ['Playlist deleted', '']);
-          }, function() {
-            $scope.appEvent('alert-error', ['Unable to delete playlist', '']);
-            $scope.playlists.push(playlist);
-          });
-      };
-
-      $scope.appEvent('show-modal', {
-        src: './app/features/playlist/partials/playlist-remove.html',
-        scope: modalScope
       });
     };
 
-    $scope.createPlaylist = function() {
+    $scope.removePlaylist = function(playlist) {
-      $location.path('/playlists/create');
+
+      $scope.appEvent('confirm-modal', {
+        title: 'Confirm delete playlist',
+        text: 'Are you sure you want to delete playlist ' + playlist.name + '?',
+        yesText: "Delete",
+        icon: "fa-warning",
+        onConfirm: function() {
+          $scope.removePlaylistConfirmed(playlist);
+        }
+      });
+
     };
+
   });
 });