Browse Source

refactoring: Dashboard history restore operation is now reusing existing
operations instead of duplicating a bunch of get & save logic.

Torkel Ödegaard 8 years ago
parent
commit
c87418d060

+ 1 - 1
pkg/api/api.go

@@ -229,7 +229,7 @@ func (hs *HttpServer) registerRoutes() {
 			r.Get("/db/:dashboardId/compare/:versions", wrap(CompareDashboardVersions))
 			r.Get("/db/:dashboardId/compare/:versions/html", wrap(CompareDashboardVersionsJSON))
 			r.Get("/db/:dashboardId/compare/:versions/basic", wrap(CompareDashboardVersionsBasic))
-			r.Post("/db/:dashboardId/restore", reqEditorRole, bind(m.RestoreDashboardVersionCommand{}), wrap(RestoreDashboardVersion))
+			r.Post("/db/:dashboardId/restore", reqEditorRole, bind(dtos.RestoreDashboardVersionCommand{}), wrap(RestoreDashboardVersion))
 
 			r.Post("/db", reqEditorRole, bind(m.SaveDashboardCommand{}), wrap(PostDashboard))
 			r.Get("/file/:file", GetDashboardFromJsonFile)

+ 19 - 39
pkg/api/dashboard.go

@@ -116,15 +116,16 @@ func DeleteDashboard(c *middleware.Context) {
 }
 
 func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response {
-
 	cmd.OrgId = c.OrgId
 	cmd.UserId = c.UserId
 
 	dash := cmd.GetDashboardModel()
+
 	// Check if Title is empty
 	if dash.Title == "" {
 		return ApiError(400, m.ErrDashboardTitleEmpty.Error(), nil)
 	}
+
 	if dash.Id == 0 {
 		limitReached, err := middleware.QuotaReached(c, "dashboard")
 		if err != nil {
@@ -399,51 +400,30 @@ func CompareDashboardVersionsBasic(c *middleware.Context) Response {
 }
 
 // RestoreDashboardVersion restores a dashboard to the given version.
-func RestoreDashboardVersion(c *middleware.Context, cmd m.RestoreDashboardVersionCommand) Response {
-	cmd.DashboardId = c.ParamsInt64(":dashboardId")
-	cmd.UserId = c.UserId
-	cmd.OrgId = c.OrgId
+func RestoreDashboardVersion(c *middleware.Context, apiCmd dtos.RestoreDashboardVersionCommand) Response {
+	dashboardId := c.ParamsInt64(":dashboardId")
 
-	if err := bus.Dispatch(&cmd); err != nil {
-		return ApiError(500, "Cannot restore version", err)
+	dashQuery := m.GetDashboardQuery{Id: dashboardId, OrgId: c.OrgId}
+	if err := bus.Dispatch(&dashQuery); err != nil {
+		return ApiError(404, "Dashboard not found", nil)
 	}
 
-	isStarred, err := isDashboardStarredByUser(c, cmd.Result.Id)
-	if err != nil {
-		return ApiError(500, "Error checking if dashboard is starred by user", err)
+	versionQuery := m.GetDashboardVersionQuery{DashboardId: dashboardId, Version: apiCmd.Version, OrgId: c.OrgId}
+	if err := bus.Dispatch(&versionQuery); err != nil {
+		return ApiError(404, "Dashboard version not found", nil)
 	}
 
-	// Finding creator and last updater of the dashboard
-	updater, creator := "Anonymous", "Anonymous"
-	if cmd.Result.UpdatedBy > 0 {
-		updater = getUserLogin(cmd.Result.UpdatedBy)
-	}
-	if cmd.Result.CreatedBy > 0 {
-		creator = getUserLogin(cmd.Result.CreatedBy)
-	}
+	dashboard := dashQuery.Result
+	version := versionQuery.Result
 
-	dto := dtos.DashboardFullWithMeta{
-		Dashboard: cmd.Result.Data,
-		Meta: dtos.DashboardMeta{
-			IsStarred: isStarred,
-			Slug:      cmd.Result.Slug,
-			Type:      m.DashTypeDB,
-			CanStar:   c.IsSignedIn,
-			CanSave:   c.OrgRole == m.ROLE_ADMIN || c.OrgRole == m.ROLE_EDITOR,
-			CanEdit:   canEditDashboard(c.OrgRole),
-			Created:   cmd.Result.Created,
-			Updated:   cmd.Result.Updated,
-			UpdatedBy: updater,
-			CreatedBy: creator,
-			Version:   cmd.Result.Version,
-		},
-	}
+	saveCmd := m.SaveDashboardCommand{}
+	saveCmd.OrgId = c.OrgId
+	saveCmd.UserId = c.UserId
+	saveCmd.Dashboard = version.Data
+	saveCmd.Dashboard.Set("version", dashboard.Version)
+	saveCmd.Message = fmt.Sprintf("Dashboard restored from version %d", version.Version)
 
-	return Json(200, util.DynMap{
-		"message":   fmt.Sprintf("Dashboard restored to version %d", cmd.Result.Version),
-		"version":   cmd.Result.Version,
-		"dashboard": dto,
-	})
+	return PostDashboard(c, saveCmd)
 }
 
 func GetDashboardTags(c *middleware.Context) {

+ 37 - 0
pkg/api/dtos/dashboard.go

@@ -0,0 +1,37 @@
+package dtos
+
+import (
+	"time"
+
+	"github.com/grafana/grafana/pkg/components/simplejson"
+)
+
+type DashboardMeta struct {
+	IsStarred  bool      `json:"isStarred,omitempty"`
+	IsHome     bool      `json:"isHome,omitempty"`
+	IsSnapshot bool      `json:"isSnapshot,omitempty"`
+	Type       string    `json:"type,omitempty"`
+	CanSave    bool      `json:"canSave"`
+	CanEdit    bool      `json:"canEdit"`
+	CanStar    bool      `json:"canStar"`
+	Slug       string    `json:"slug"`
+	Expires    time.Time `json:"expires"`
+	Created    time.Time `json:"created"`
+	Updated    time.Time `json:"updated"`
+	UpdatedBy  string    `json:"updatedBy"`
+	CreatedBy  string    `json:"createdBy"`
+	Version    int       `json:"version"`
+}
+
+type DashboardFullWithMeta struct {
+	Meta      DashboardMeta    `json:"meta"`
+	Dashboard *simplejson.Json `json:"dashboard"`
+}
+
+type DashboardRedirect struct {
+	RedirectUri string `json:"redirectUri"`
+}
+
+type RestoreDashboardVersionCommand struct {
+	Version int `json:"version" binding:"Required"`
+}

+ 0 - 27
pkg/api/dtos/models.go

@@ -4,7 +4,6 @@ import (
 	"crypto/md5"
 	"fmt"
 	"strings"
-	"time"
 
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
@@ -38,32 +37,6 @@ type CurrentUser struct {
 	HelpFlags1     m.HelpFlags1 `json:"helpFlags1"`
 }
 
-type DashboardMeta struct {
-	IsStarred  bool      `json:"isStarred,omitempty"`
-	IsHome     bool      `json:"isHome,omitempty"`
-	IsSnapshot bool      `json:"isSnapshot,omitempty"`
-	Type       string    `json:"type,omitempty"`
-	CanSave    bool      `json:"canSave"`
-	CanEdit    bool      `json:"canEdit"`
-	CanStar    bool      `json:"canStar"`
-	Slug       string    `json:"slug"`
-	Expires    time.Time `json:"expires"`
-	Created    time.Time `json:"created"`
-	Updated    time.Time `json:"updated"`
-	UpdatedBy  string    `json:"updatedBy"`
-	CreatedBy  string    `json:"createdBy"`
-	Version    int       `json:"version"`
-}
-
-type DashboardFullWithMeta struct {
-	Meta      DashboardMeta    `json:"meta"`
-	Dashboard *simplejson.Json `json:"dashboard"`
-}
-
-type DashboardRedirect struct {
-	RedirectUri string `json:"redirectUri"`
-}
-
 type DataSource struct {
 	Id                int64            `json:"id"`
 	OrgId             int64            `json:"orgId"`

+ 0 - 10
pkg/models/dashboard_version.go

@@ -82,16 +82,6 @@ type GetDashboardVersionsQuery struct {
 // 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 {
 	OrgId       int64

+ 2 - 1
pkg/models/dashboards.go

@@ -151,7 +151,8 @@ type DeleteDashboardCommand struct {
 //
 
 type GetDashboardQuery struct {
-	Slug  string
+	Slug  string // required if no Id is specified
+	Id    int64  // optional if slug is set
 	OrgId int64
 
 	Result *Dashboard

+ 2 - 1
pkg/services/sqlstore/dashboard.go

@@ -131,8 +131,9 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error {
 }
 
 func GetDashboard(query *m.GetDashboardQuery) error {
-	dashboard := m.Dashboard{Slug: query.Slug, OrgId: query.OrgId}
+	dashboard := m.Dashboard{Slug: query.Slug, OrgId: query.OrgId, Id: query.Id}
 	has, err := x.Get(&dashboard)
+
 	if err != nil {
 		return err
 	} else if has == false {

+ 0 - 100
pkg/services/sqlstore/dashboard_version.go

@@ -3,7 +3,6 @@ package sqlstore
 import (
 	"encoding/json"
 	"errors"
-	"time"
 
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/components/formatter"
@@ -26,7 +25,6 @@ func init() {
 	bus.AddHandler("sql", CompareDashboardVersionsCommand)
 	bus.AddHandler("sql", GetDashboardVersion)
 	bus.AddHandler("sql", GetDashboardVersions)
-	bus.AddHandler("sql", RestoreDashboardVersion)
 }
 
 // CompareDashboardVersionsCommand computes the JSON diff of two versions,
@@ -118,76 +116,6 @@ func GetDashboardVersions(query *m.GetDashboardVersionsQuery) error {
 	return nil
 }
 
-// RestoreDashboardVersion restores the dashboard data to the given version.
-func RestoreDashboardVersion(cmd *m.RestoreDashboardVersionCommand) error {
-	return inTransaction(func(sess *DBSession) error {
-		// check if dashboard version exists in dashboard_version table
-		//
-		// normally we could use the getDashboardVersion func here, but since
-		// we're in a transaction, we need to run the queries using the
-		// session instead of using the global `x`, so we copy those functions
-		// here, replacing `x` with `sess`
-		dashboardVersion := m.DashboardVersion{}
-		has, err := sess.Where("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)
-
-		dashboard := m.Dashboard{Id: cmd.DashboardId}
-		// Get the dashboard version
-		if has, err = sess.Get(&dashboard); err != nil {
-			return err
-		} else if !has {
-			return m.ErrDashboardNotFound
-		}
-
-		version, err := getMaxVersion(sess, dashboard.Id)
-		if err != nil {
-			return err
-		}
-
-		// revert and save to a new dashboard version
-		dashboard.Data = dashboardVersion.Data
-		dashboard.Updated = time.Now()
-		dashboard.UpdatedBy = cmd.UserId
-		dashboard.Version = version
-		dashboard.Data.Set("version", dashboard.Version)
-
-		// Update dashboard
-		if affectedRows, err := sess.Id(dashboard.Id).Update(dashboard); err != nil {
-			return err
-		} else if affectedRows == 0 {
-			return m.ErrDashboardNotFound
-		}
-
-		// save that version a new version
-		dashVersion := &m.DashboardVersion{
-			DashboardId:   dashboard.Id,
-			ParentVersion: cmd.Version,
-			RestoredFrom:  cmd.Version,
-			Version:       dashboard.Version,
-			Created:       time.Now(),
-			CreatedBy:     dashboard.UpdatedBy,
-			Message:       "",
-			Data:          dashboard.Data,
-		}
-
-		if affectedRows, err := sess.Insert(dashVersion); err != nil {
-			return err
-		} else if affectedRows == 0 {
-			return m.ErrDashboardNotFound
-		}
-
-		cmd.Result = &dashboard
-		return nil
-	})
-}
-
 // 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) {
@@ -243,31 +171,3 @@ func getDiff(originalDash, newDash *m.DashboardVersion) (interface{}, diff.Diff,
 	err = json.Unmarshal(leftBytes, &left)
 	return left, jsonDiff, nil
 }
-
-type version struct {
-	Max int
-}
-
-// getMaxVersion returns the highest version number in the `dashboard_version`
-// table.
-//
-// This is necessary because sqlite3 doesn't support autoincrement in the same
-// way that Postgres or MySQL do, so we use this to get around that. Since it's
-// impossible to delete a version in Grafana, this is believed to be a
-// safe-enough alternative.
-func getMaxVersion(sess *DBSession, dashboardId int64) (int, error) {
-	v := version{}
-	has, err := sess.Table("dashboard_version").
-		Select("MAX(version) AS max").
-		Where("dashboard_id = ?", dashboardId).
-		Get(&v)
-	if !has {
-		return 0, m.ErrDashboardNotFound
-	}
-	if err != nil {
-		return 0, err
-	}
-
-	v.Max++
-	return v.Max, nil
-}

+ 8 - 20
public/app/features/dashboard/history/history.ts

@@ -25,7 +25,9 @@ export class HistoryListCtrl {
   /** @ngInject */
   constructor(private $scope,
               private $rootScope,
+              private $route,
               private $window,
+              private $timeout,
               private $q,
               private contextSrv,
               private historySrv) {
@@ -201,7 +203,7 @@ export class HistoryListCtrl {
       title: 'Restore version',
       text: '',
       text2: `Are you sure you want to restore the dashboard to version ${version}? All unsaved changes will be lost.`,
-      icon: 'fa-rotate-right',
+      icon: 'fa-history',
       yesText: `Yes, restore to version ${version}`,
       onConfirm: this.restoreConfirm.bind(this, version),
     });
@@ -210,25 +212,11 @@ export class HistoryListCtrl {
   restoreConfirm(version: number) {
     this.loading = true;
     return this.historySrv.restoreDashboard(this.dashboard, version).then(response => {
-      this.revisions.unshift({
-        id: this.revisions[0].id + 1,
-        checked: false,
-        dashboardId: this.dashboard.id,
-        parentVersion: version,
-        version: this.revisions[0].version + 1,
-        created: new Date(),
-        createdBy: this.contextSrv.user.name,
-        message: `Restored from version ${version}`,
-      });
-
-      this.reset();
-      const restoredData = response.dashboard;
-      this.dashboard = restoredData.dashboard;
-      this.dashboard.meta = restoredData.meta;
-      this.$scope.setupDashboard(restoredData);
-    }).catch(err => {
-      this.$rootScope.appEvent('alert-error', ['There was an error restoring the dashboard', (err.message || err)]);
-    }).finally(() => { this.loading = false; });
+      this.$route.reload();
+      this.$rootScope.appEvent('alert-success', ['Dashboard restored', 'Restored from version ' + version]);
+    }).finally(() => {
+      this.loading = false;
+    });
   }
 }