refactoring: Renamed dashboard version queries that wrongly had Command suffix, added missing OrgId to dashboard history commands and queries

pkg/api/dashboard.go

@@ -116,13 +116,9 @@ func DeleteDashboard(c *middleware.Context) {
 }
 
 func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response {
-	cmd.OrgId = c.OrgId
 
-	if !c.IsSignedIn {
+	cmd.OrgId = c.OrgId
-		cmd.UserId = -1
+	cmd.UserId = c.UserId
-	} else {
-		cmd.UserId = c.UserId
-	}
 
 	dash := cmd.GetDashboardModel()
 	// Check if Title is empty
@@ -261,19 +257,16 @@ func GetDashboardFromJsonFile(c *middleware.Context) {
 // GetDashboardVersions returns all dashboard versions as JSON
 func GetDashboardVersions(c *middleware.Context) Response {
 	dashboardId := c.ParamsInt64(":dashboardId")
-	orderBy := c.Query("orderBy")
 	limit := c.QueryInt("limit")
 	start := c.QueryInt("start")
-	if orderBy == "" {
+
-		orderBy = "version"
-	}
 	if limit == 0 {
 		limit = 1000
 	}
 
-	query := m.GetDashboardVersionsCommand{
+	query := m.GetDashboardVersionsQuery{
+		OrgId:       c.OrgId,
 		DashboardId: dashboardId,
-		OrderBy:     orderBy,
 		Limit:       limit,
 		Start:       start,
 	}
@@ -282,26 +275,6 @@ func GetDashboardVersions(c *middleware.Context) Response {
 		return ApiError(404, fmt.Sprintf("No versions found for dashboardId %d", dashboardId), err)
 	}
 
-	// // TODO(ben) use inner join with DTO
-	// dashboardVersions := make([]*m.DashboardVersionDTO, len(query.Result))
-	// for i, dashboardVersion := range query.Result {
-	// 	creator := "Anonymous"
-	// 	if dashboardVersion.CreatedBy > 0 {
-	// 		creator = getUserLogin(dashboardVersion.CreatedBy)
-	// 	}
-
-	// 	dashboardVersions[i] = &m.DashboardVersionDTO{
-	// 		Id:            dashboardVersion.Id,
-	// 		DashboardId:   dashboardVersion.DashboardId,
-	// 		ParentVersion: dashboardVersion.ParentVersion,
-	// 		RestoredFrom:  dashboardVersion.RestoredFrom,
-	// 		Version:       dashboardVersion.Version,
-	// 		Created:       dashboardVersion.Created,
-	// 		CreatedBy:     creator,
-	// 		Message:       dashboardVersion.Message,
-	// 	}
-	// }
-
 	return Json(200, query.Result)
 }
 
@@ -310,16 +283,14 @@ func GetDashboardVersion(c *middleware.Context) Response {
 	dashboardId := c.ParamsInt64(":dashboardId")
 	version := c.ParamsInt(":id")
 
-	query := m.GetDashboardVersionCommand{
+	query := m.GetDashboardVersionQuery{
+		OrgId:       c.OrgId,
 		DashboardId: dashboardId,
 		Version:     version,
 	}
+
 	if err := bus.Dispatch(&query); err != nil {
-		return ApiError(
+		return ApiError(500, fmt.Sprintf("Dashboard version %d not found for dashboardId %d", version, dashboardId), err)
-			500,
-			fmt.Sprintf("Dashboard version %d not found for dashboardId %d", version, dashboardId),
-			err,
-		)
 	}
 
 	creator := "Anonymous"
@@ -360,6 +331,7 @@ func createCompareDashboardVersionCommand(c *middleware.Context) (m.CompareDashb
 	}
 
 	cmd.DashboardId = int64(dashboardId)
+	cmd.OrgId = c.OrgId
 	cmd.Original = originalDash
 	cmd.New = newDash
 	return cmd, nil
@@ -428,12 +400,9 @@ func CompareDashboardVersionsBasic(c *middleware.Context) Response {
 
 // RestoreDashboardVersion restores a dashboard to the given version.
 func RestoreDashboardVersion(c *middleware.Context, cmd m.RestoreDashboardVersionCommand) Response {
-	if !c.IsSignedIn {
-		return ApiError(401, "Must be signed in to restore a version", nil)
-	}
-
-	cmd.UserId = c.UserId
 	cmd.DashboardId = c.ParamsInt64(":dashboardId")
+	cmd.UserId = c.UserId
+	cmd.OrgId = c.OrgId
 
 	if err := bus.Dispatch(&cmd); err != nil {
 		return ApiError(500, "Cannot restore version", err)

pkg/models/dashboard_version.go

@@ -29,9 +29,8 @@ type DashboardVersion struct {
 	RestoredFrom  int   `json:"restoredFrom"`
 	Version       int   `json:"version"`
 
-	Created time.Time `json:"created"`
+	Created   time.Time `json:"created"`
-
+	CreatedBy int64     `json:"createdBy"`
-	CreatedBy int64 `json:"createdBy"`
 
 	Message string           `json:"message"`
 	Data    *simplejson.Json `json:"data"`
@@ -59,45 +58,47 @@ type DashboardVersionDTO struct {
 }
 
 //
-// COMMANDS
+// Queries
 //
 
-// GetDashboardVersionCommand contains the data required to execute the
+type GetDashboardVersionQuery struct {
-// sqlstore.GetDashboardVersionCommand, which returns the DashboardVersion for
+	DashboardId int64
-// the given Version.
+	OrgId       int64
-type GetDashboardVersionCommand struct {
+	Version     int
-	DashboardId int64 `json:"dashboardId" binding:"Required"`
-	Version     int   `json:"version" binding:"Required"`
 
 	Result *DashboardVersion
 }
 
-// GetDashboardVersionsCommand contains the data required to execute the
+type GetDashboardVersionsQuery struct {
-// sqlstore.GetDashboardVersionsCommand, which returns all dashboard versions.
+	DashboardId int64
-type GetDashboardVersionsCommand struct {
+	OrgId       int64
-	DashboardId int64  `json:"dashboardId" binding:"Required"`
+	Limit       int
-	OrderBy     string `json:"orderBy"`
+	Start       int
-	Limit       int    `json:"limit"`
-	Start       int    `json:"start"`
 
 	Result []*DashboardVersionDTO
 }
 
+//
+// Commands
+//
+
 // RestoreDashboardVersionCommand creates a new dashboard version.
 type RestoreDashboardVersionCommand struct {
 	DashboardId int64 `json:"dashboardId"`
 	Version     int   `json:"version" binding:"Required"`
 	UserId      int64 `json:"-"`
+	OrgId       int64 `json:"-"`
 
 	Result *Dashboard
 }
 
 // CompareDashboardVersionsCommand is used to compare two versions.
 type CompareDashboardVersionsCommand struct {
-	DashboardId int64    `json:"dashboardId"`
+	OrgId       int64
-	Original    int      `json:"original" binding:"Required"`
+	DashboardId int64
-	New         int      `json:"new" binding:"Required"`
+	Original    int
-	DiffType    DiffType `json:"-"`
+	New         int
+	DiffType    DiffType
 
 	Delta []byte `json:"delta"`
 }

pkg/models/dashboards.go

@@ -98,12 +98,17 @@ func NewDashboardFromJson(data *simplejson.Json) *Dashboard {
 // GetDashboardModel turns the command into the savable model
 func (cmd *SaveDashboardCommand) GetDashboardModel() *Dashboard {
 	dash := NewDashboardFromJson(cmd.Dashboard)
+	userId := cmd.UserId
 
-	if dash.Data.Get("version").MustInt(0) == 0 {
+	if userId == 0 {
-		dash.CreatedBy = cmd.UserId
+		userId = -1
 	}
 
-	dash.UpdatedBy = cmd.UserId
+	if dash.Data.Get("version").MustInt(0) == 0 {
+		dash.CreatedBy = userId
+	}
+
+	dash.UpdatedBy = userId
 	dash.OrgId = cmd.OrgId
 	dash.PluginId = cmd.PluginId
 	dash.UpdateSlug()

pkg/services/sqlstore/dashboard.go

@@ -70,24 +70,22 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error {
 		}
 
 		parentVersion := dash.Version
-		version, err := getMaxVersion(sess, dash.Id)
-		if err != nil {
-			return err
-		}
-		dash.Version = version
-
 		affectedRows := int64(0)
+
 		if dash.Id == 0 {
 			metrics.M_Models_Dashboard_Insert.Inc(1)
 			dash.Data.Set("version", dash.Version)
 			affectedRows, err = sess.Insert(dash)
 		} else {
+			dash.Version += 1
 			dash.Data.Set("version", dash.Version)
 			affectedRows, err = sess.Id(dash.Id).Update(dash)
 		}
+
 		if err != nil {
 			return err
 		}
+
 		if affectedRows == 0 {
 			return m.ErrDashboardNotFound
 		}
@@ -102,11 +100,11 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error {
 			Message:       cmd.Message,
 			Data:          dash.Data,
 		}
-		affectedRows, err = sess.Insert(dashVersion)
+
-		if err != nil {
+		// insert version entry
+		if affectedRows, err = sess.Insert(dashVersion); err != nil {
 			return err
-		}
+		} else if affectedRows == 0 {
-		if affectedRows == 0 {
 			return m.ErrDashboardNotFound
 		}
 

pkg/services/sqlstore/dashboard_version.go

@@ -32,12 +32,12 @@ func init() {
 // CompareDashboardVersionsCommand computes the JSON diff of two versions,
 // assigning the delta of the diff to the `Delta` field.
 func CompareDashboardVersionsCommand(cmd *m.CompareDashboardVersionsCommand) error {
-	original, err := getDashboardVersion(cmd.DashboardId, cmd.Original)
+	original, err := getDashboardVersion(cmd.DashboardId, cmd.Original, cmd.OrgId)
 	if err != nil {
 		return err
 	}
 
-	newDashboard, err := getDashboardVersion(cmd.DashboardId, cmd.New)
+	newDashboard, err := getDashboardVersion(cmd.DashboardId, cmd.New, cmd.OrgId)
 	if err != nil {
 		return err
 	}
@@ -79,8 +79,8 @@ func CompareDashboardVersionsCommand(cmd *m.CompareDashboardVersionsCommand) err
 
 // GetDashboardVersion gets the dashboard version for the given dashboard ID
 // and version number.
-func GetDashboardVersion(query *m.GetDashboardVersionCommand) error {
+func GetDashboardVersion(query *m.GetDashboardVersionQuery) error {
-	result, err := getDashboardVersion(query.DashboardId, query.Version)
+	result, err := getDashboardVersion(query.DashboardId, query.Version, query.OrgId)
 	if err != nil {
 		return err
 	}
@@ -90,12 +90,7 @@ func GetDashboardVersion(query *m.GetDashboardVersionCommand) error {
 }
 
 // GetDashboardVersions gets all dashboard versions for the given dashboard ID.
-func GetDashboardVersions(query *m.GetDashboardVersionsCommand) error {
+func GetDashboardVersions(query *m.GetDashboardVersionsQuery) error {
-	if query.OrderBy == "" {
-		query.OrderBy = "version"
-	}
-	query.OrderBy += " desc"
-
 	err := x.Table("dashboard_version").
 		Select(`dashboard_version.id,
 				dashboard_version.dashboard_id,
@@ -108,8 +103,9 @@ func GetDashboardVersions(query *m.GetDashboardVersionsCommand) error {
 				dashboard_version.data,
 				"user".login as created_by`).
 		Join("LEFT", "user", `dashboard_version.created_by = "user".id`).
-		Where("dashboard_version.dashboard_id=?", query.DashboardId).
+		Join("LEFT", "dashboard", `dashboard.id = "dashboard_version".dashboard_id`).
-		OrderBy("dashboard_version."+query.OrderBy).
+		Where("dashboard_version.dashboard_id=? AND dashboard.org_id=?", query.DashboardId, query.OrgId).
+		OrderBy("dashboard_version.version DESC").
 		Limit(query.Limit, query.Start).
 		Find(&query.Result)
 	if err != nil {
@@ -132,26 +128,21 @@ func RestoreDashboardVersion(cmd *m.RestoreDashboardVersionCommand) error {
 		// session instead of using the global `x`, so we copy those functions
 		// here, replacing `x` with `sess`
 		dashboardVersion := m.DashboardVersion{}
-		has, err := sess.Where(
+		has, err := sess.Where("dashboard_id=? AND version=? AND org_id=?", cmd.DashboardId, cmd.Version, cmd.OrgId).Get(&dashboardVersion)
-			"dashboard_id=? AND version=?",
-			cmd.DashboardId,
-			cmd.Version,
-		).Get(&dashboardVersion)
 		if err != nil {
 			return err
 		}
 		if !has {
 			return m.ErrDashboardVersionNotFound
 		}
+
 		dashboardVersion.Data.Set("id", dashboardVersion.DashboardId)
 
-		// get the dashboard version
 		dashboard := m.Dashboard{Id: cmd.DashboardId}
-		has, err = sess.Get(&dashboard)
+		// Get the dashboard version
-		if err != nil {
+		if has, err = sess.Get(&dashboard); err != nil {
 			return err
-		}
+		} else if !has {
-		if has == false {
 			return m.ErrDashboardNotFound
 		}
 
@@ -166,11 +157,11 @@ func RestoreDashboardVersion(cmd *m.RestoreDashboardVersionCommand) error {
 		dashboard.UpdatedBy = cmd.UserId
 		dashboard.Version = version
 		dashboard.Data.Set("version", dashboard.Version)
-		affectedRows, err := sess.Id(dashboard.Id).Update(dashboard)
+
-		if err != nil {
+		// Update dashboard
+		if affectedRows, err := sess.Id(dashboard.Id).Update(dashboard); err != nil {
 			return err
-		}
+		} else if affectedRows == 0 {
-		if affectedRows == 0 {
 			return m.ErrDashboardNotFound
 		}
 
@@ -185,11 +176,10 @@ func RestoreDashboardVersion(cmd *m.RestoreDashboardVersionCommand) error {
 			Message:       "",
 			Data:          dashboard.Data,
 		}
-		affectedRows, err = sess.Insert(dashVersion)
+
-		if err != nil {
+		if affectedRows, err := sess.Insert(dashVersion); err != nil {
 			return err
-		}
+		} else if affectedRows == 0 {
-		if affectedRows == 0 {
 			return m.ErrDashboardNotFound
 		}
 
@@ -200,9 +190,9 @@ func RestoreDashboardVersion(cmd *m.RestoreDashboardVersionCommand) error {
 
 // getDashboardVersion is a helper function that gets the dashboard version for
 // the given dashboard ID and version ID.
-func getDashboardVersion(dashboardId int64, version int) (*m.DashboardVersion, error) {
+func getDashboardVersion(dashboardId int64, version int, orgId int64) (*m.DashboardVersion, error) {
 	dashboardVersion := m.DashboardVersion{}
-	has, err := x.Where("dashboard_id=? AND version=?", dashboardId, version).Get(&dashboardVersion)
+	has, err := x.Where("dashboard_id=? AND version=? AND org_id=?", dashboardId, version, orgId).Get(&dashboardVersion)
 	if err != nil {
 		return nil, err
 	}

public/app/features/dashboard/dashboard_srv.ts

@@ -34,7 +34,7 @@ export class DashboardSrv {
         yesText: "Save & Overwrite",
         icon: "fa-warning",
         onConfirm: () => {
-          this.saveDashboard({overwrite: true}, clone);
+          this.save(clone, {overwrite: true});
         }
       });
     }
@@ -49,7 +49,7 @@ export class DashboardSrv {
         yesText: "Save & Overwrite",
         icon: "fa-warning",
         onConfirm: () => {
-          this.saveDashboard({overwrite: true}, clone);
+          this.save(clone, {overwrite: true});
         }
       });
     }
@@ -68,7 +68,7 @@ export class DashboardSrv {
           this.showSaveAsModal();
         },
         onConfirm: () => {
-          this.saveDashboard({overwrite: true}, clone);
+          this.save(clone, {overwrite: true});
         }
       });
     }

public/app/features/dashboard/history/history.html

@@ -1,6 +1,6 @@
 <div class="tabbed-view-header">
 	<h2 class="tabbed-view-title">
-		Changelog
+		Version History
 	</h2>
 
 	<ul class="gf-tabs">

public/app/features/dashboard/history/history.ts

@@ -18,7 +18,6 @@ export class HistoryListCtrl {
   loading: boolean;
   max: number;
   mode: string;
-  orderBy: string;
   revisions: RevisionsModel[];
   selected: number[];
   start: number;
@@ -38,7 +37,6 @@ export class HistoryListCtrl {
     this.loading = false;
     this.max = 2;
     this.mode = 'list';
-    this.orderBy = 'version';
     this.selected = [];
     this.start = 0;
 
@@ -124,7 +122,6 @@ export class HistoryListCtrl {
     const options: HistoryListOpts = {
       limit: this.limit,
       start: this.start,
-      orderBy: this.orderBy,
     };
     return this.historySrv.getHistoryList(this.dashboard, options).then(revisions => {
       const formattedRevisions =  _.flow(

public/app/features/dashboard/history/models.ts

@@ -1,7 +1,6 @@
 export interface HistoryListOpts {
   limit: number;
   start: number;
-  orderBy: string;
 }
 
 export interface RevisionsModel {

public/app/features/dashboard/save_modal.ts

@@ -7,7 +7,7 @@ const  template = `
 	<div class="modal-header">
 		<h2 class="modal-header-title">
 			<i class="fa fa-save"></i>
-			<span class="p-l-1">Save Dashboard</span>
+			<span class="p-l-1">Save changes</span>
 		</h2>
 
 		<a class="modal-header-close" ng-click="ctrl.dismiss();">