Ver Fonte

dasboard_history: big refactoring of how the compare api looked, now a POST with a model for new & base version, refactored a simplified UI code as well, this was done to lay foundation to make it possible to do diff against current in browser unsaved version

Torkel Ödegaard há 8 anos atrás
pai
commit
ef1dfed0d8

+ 2 - 3
pkg/api/api.go

@@ -226,11 +226,10 @@ func (hs *HttpServer) registerRoutes() {
 
 			r.Get("/id/:dashboardId/versions", wrap(GetDashboardVersions))
 			r.Get("/id/:dashboardId/versions/:id", wrap(GetDashboardVersion))
-			r.Get("/id/:dashboardId/compare/:versions", wrap(CompareDashboardVersions))
-			r.Get("/id/:dashboardId/compare/:versions/html", wrap(CompareDashboardVersionsJSON))
-			r.Get("/id/:dashboardId/compare/:versions/basic", wrap(CompareDashboardVersionsBasic))
 			r.Post("/id/:dashboardId/restore", reqEditorRole, bind(dtos.RestoreDashboardVersionCommand{}), wrap(RestoreDashboardVersion))
 
+			r.Post("/calculate-diff", bind(dtos.CalculateDiffOptions{}), wrap(CalculateDashboardDiff))
+
 			r.Post("/db", reqEditorRole, bind(m.SaveDashboardCommand{}), wrap(PostDashboard))
 			r.Get("/file/:file", GetDashboardFromJsonFile)
 			r.Get("/home", wrap(GetHomeDashboard))

+ 20 - 89
pkg/api/dashboard.go

@@ -2,11 +2,9 @@ package api
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"os"
 	"path"
-	"strconv"
 	"strings"
 
 	"github.com/grafana/grafana/pkg/api/dtos"
@@ -328,101 +326,34 @@ func GetDashboardVersion(c *middleware.Context) Response {
 	return Json(200, dashVersionMeta)
 }
 
-func getDashboardVersionDiffOptions(c *middleware.Context, diffType dashdiffs.DiffType) (*dashdiffs.Options, error) {
+// POST /api/dashboards/calculate-diff performs diffs on two dashboards
+func CalculateDashboardDiff(c *middleware.Context, apiOptions dtos.CalculateDiffOptions) Response {
 
-	dashId := c.ParamsInt64(":dashboardId")
-	if dashId == 0 {
-		return nil, errors.New("Missing dashboardId")
-	}
-
-	versionStrings := strings.Split(c.Params(":versions"), "...")
-	if len(versionStrings) != 2 {
-		return nil, fmt.Errorf("bad format: urls should be in the format /versions/0...1")
-	}
-
-	BaseVersion, err := strconv.Atoi(versionStrings[0])
-	if err != nil {
-		return nil, fmt.Errorf("bad format: first argument is not of type int")
-	}
-
-	newVersion, err := strconv.Atoi(versionStrings[1])
-	if err != nil {
-		return nil, fmt.Errorf("bad format: second argument is not of type int")
-	}
-
-	options := &dashdiffs.Options{}
-	options.DashboardId = dashId
-	options.OrgId = c.OrgId
-	options.BaseVersion = BaseVersion
-	options.NewVersion = newVersion
-	options.DiffType = diffType
-
-	return options, nil
-}
-
-// CompareDashboardVersions compares dashboards the way the GitHub API does.
-func CompareDashboardVersions(c *middleware.Context) Response {
-	options, err := getDashboardVersionDiffOptions(c, dashdiffs.DiffDelta)
-
-	if err != nil {
-		return ApiError(500, err.Error(), err)
-	}
-
-	result, err := dashdiffs.GetVersionDiff(options)
-	if err != nil {
-		return ApiError(500, "Unable to compute diff", err)
-	}
-
-	// here the output is already JSON, so we need to unmarshal it into a
-	// map before marshaling the entire response
-
-	deltaMap := make(map[string]interface{})
-	err = json.Unmarshal(result.Delta, &deltaMap)
-	if err != nil {
-		return ApiError(500, err.Error(), err)
-	}
-
-	return Json(200, util.DynMap{
-		"meta": util.DynMap{
-			"baseVersion": options.BaseVersion,
-			"newVersion":  options.NewVersion,
+	options := dashdiffs.Options{
+		OrgId:    c.OrgId,
+		DiffType: dashdiffs.ParseDiffType(apiOptions.DiffType),
+		Base: dashdiffs.DiffTarget{
+			DashboardId:      apiOptions.Base.DashboardId,
+			Version:          apiOptions.Base.Version,
+			UnsavedDashboard: apiOptions.Base.UnsavedDashboard,
+		},
+		New: dashdiffs.DiffTarget{
+			DashboardId:      apiOptions.New.DashboardId,
+			Version:          apiOptions.New.Version,
+			UnsavedDashboard: apiOptions.New.UnsavedDashboard,
 		},
-		"delta": deltaMap,
-	})
-}
-
-// CompareDashboardVersionsJSON compares dashboards the way the GitHub API does,
-// returning a human-readable JSON diff.
-func CompareDashboardVersionsJSON(c *middleware.Context) Response {
-	options, err := getDashboardVersionDiffOptions(c, dashdiffs.DiffJSON)
-
-	if err != nil {
-		return ApiError(500, err.Error(), err)
-	}
-
-	result, err := dashdiffs.GetVersionDiff(options)
-	if err != nil {
-		return ApiError(500, err.Error(), err)
 	}
 
-	return Respond(200, result.Delta).Header("Content-Type", "text/html")
-}
-
-// CompareDashboardVersionsBasic compares dashboards the way the GitHub API does,
-// returning a human-readable diff.
-func CompareDashboardVersionsBasic(c *middleware.Context) Response {
-	options, err := getDashboardVersionDiffOptions(c, dashdiffs.DiffBasic)
-
+	result, err := dashdiffs.CalculateDiff(&options)
 	if err != nil {
-		return ApiError(500, err.Error(), err)
+		return ApiError(500, "Unable to compute diff", err)
 	}
 
-	result, err := dashdiffs.GetVersionDiff(options)
-	if err != nil {
-		return ApiError(500, err.Error(), err)
+	if options.DiffType == dashdiffs.DiffDelta {
+		return Respond(200, result.Delta).Header("Content-Type", "application/json")
+	} else {
+		return Respond(200, result.Delta).Header("Content-Type", "text/html")
 	}
-
-	return Respond(200, result.Delta).Header("Content-Type", "text/html")
 }
 
 // RestoreDashboardVersion restores a dashboard to the given version.

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

@@ -32,6 +32,18 @@ type DashboardRedirect struct {
 	RedirectUri string `json:"redirectUri"`
 }
 
+type CalculateDiffOptions struct {
+	Base     CalculateDiffTarget `json:"base" binding:"Required"`
+	New      CalculateDiffTarget `json:"new" binding:"Required"`
+	DiffType string              `json:"DiffType" binding:"Required"`
+}
+
+type CalculateDiffTarget struct {
+	DashboardId      int64            `json:"dashboardId"`
+	Version          int              `json:"version"`
+	UnsavedDashboard *simplejson.Json `json:"unsavedDashboard"`
+}
+
 type RestoreDashboardVersionCommand struct {
 	Version int `json:"version" binding:"Required"`
 }

+ 35 - 18
pkg/components/dashdiffs/compare.go

@@ -11,14 +11,6 @@ import (
 	deltaFormatter "github.com/yudai/gojsondiff/formatter"
 )
 
-type DiffType int
-
-const (
-	DiffJSON DiffType = iota
-	DiffBasic
-	DiffDelta
-)
-
 var (
 	// ErrUnsupportedDiffType occurs when an invalid diff type is used.
 	ErrUnsupportedDiffType = errors.New("dashdiff: unsupported diff type")
@@ -27,24 +19,49 @@ var (
 	ErrNilDiff = errors.New("dashdiff: diff is nil")
 )
 
+type DiffType int
+
+const (
+	DiffJSON DiffType = iota
+	DiffBasic
+	DiffDelta
+)
+
 type Options struct {
-	OrgId       int64
-	DashboardId int64
-	BaseVersion int
-	NewVersion  int
-	DiffType    DiffType
+	OrgId    int64
+	Base     DiffTarget
+	New      DiffTarget
+	DiffType DiffType
+}
+
+type DiffTarget struct {
+	DashboardId      int64
+	Version          int
+	UnsavedDashboard *simplejson.Json
 }
 
 type Result struct {
 	Delta []byte `json:"delta"`
 }
 
+func ParseDiffType(diff string) DiffType {
+	switch diff {
+	case "json":
+		return DiffJSON
+	case "basic":
+		return DiffBasic
+	case "delta":
+		return DiffDelta
+	}
+	return DiffBasic
+}
+
 // CompareDashboardVersionsCommand computes the JSON diff of two versions,
 // assigning the delta of the diff to the `Delta` field.
-func GetVersionDiff(options *Options) (*Result, error) {
+func CalculateDiff(options *Options) (*Result, error) {
 	baseVersionQuery := models.GetDashboardVersionQuery{
-		DashboardId: options.DashboardId,
-		Version:     options.BaseVersion,
+		DashboardId: options.Base.DashboardId,
+		Version:     options.Base.Version,
 	}
 
 	if err := bus.Dispatch(&baseVersionQuery); err != nil {
@@ -52,8 +69,8 @@ func GetVersionDiff(options *Options) (*Result, error) {
 	}
 
 	newVersionQuery := models.GetDashboardVersionQuery{
-		DashboardId: options.DashboardId,
-		Version:     options.NewVersion,
+		DashboardId: options.New.DashboardId,
+		Version:     options.New.Version,
 	}
 
 	if err := bus.Dispatch(&newVersionQuery); err != nil {

+ 27 - 30
public/app/features/dashboard/history/history.html

@@ -5,16 +5,13 @@
 
 	<ul class="gf-tabs">
 		<li class="gf-tabs-item" >
-			<a class="gf-tabs-link" ng-click="ctrl.mode = 'list';" ng-class="{active: ctrl.mode === 'list'}">
+			<a class="gf-tabs-link" ng-click="ctrl.switchMode('list');" ng-class="{active: ctrl.mode === 'list'}">
 				List
 			</a>
 		</li>
 		<li class="gf-tabs-item" ng-show="ctrl.mode === 'compare'">
-			<span ng-if="ctrl.isOriginalCurrent()" class="active gf-tabs-link">
-				Version {{ctrl.selected[0]}} <i class="fa fa-arrows-h"></i> Current
-			</span>
-			<span ng-if="!ctrl.isOriginalCurrent()" class="active gf-tabs-link">
-				Version {{ctrl.selected[0]}} <i class="fa fa-arrows-h"></i> Version {{ctrl.selected[1]}}
+			<span class="active gf-tabs-link">
+				Version {{ctrl.baseInfo.version}} <i class="fa fa-arrows-h"></i> Version {{ctrl.newInfo.version}}
 			</span>
 		</li>
 	</ul>
@@ -47,15 +44,15 @@
           </thead>
           <tbody>
             <tr ng-repeat="revision in ctrl.revisions">
-              <td class="filter-table__switch-cell" bs-tooltip="ctrl.compareRevisionDisabled(revision.checked) ? 'You can only compare 2 versions at a time' : ''">
+              <td class="filter-table__switch-cell" bs-tooltip="!revision.checked && ctrl.canCompare ? 'You can only compare 2 versions at a time' : ''" data-placement="right">
                 <gf-form-switch switch-class="gf-form-switch--table-cell"
                                 checked="revision.checked"
-                                on-change="ctrl.compareRevisionStateChanged(revision)"
-                                ng-disabled="ctrl.compareRevisionDisabled(revision.checked)">
+                                on-change="ctrl.revisionSelectionChanged()"
+																ng-disabled="!revision.checked && ctrl.canCompare">
                 </gf-form-switch>
               </td>
               <td class="text-center">{{revision.version}}</td>
-              <td>{{ctrl.formatDate(revision.created)}}</td>
+              <td>{{revision.createdDateString}}</td>
               <td>{{revision.createdBy}}</td>
               <td>{{revision.message}}</td>
               <td class="text-right">
@@ -63,7 +60,7 @@
                   <i class="fa fa-history"></i>&nbsp;&nbsp;Restore
                 </a>
                 <a class="btn btn-outline-disabled btn-small" ng-show="revision.version === ctrl.dashboard.version">
-                  <i class="fa fa-check"></i>&nbsp;&nbsp;Current
+                  <i class="fa fa-check"></i>&nbsp;&nbsp;Latest
                 </a>
               </td>
             </tr>
@@ -80,9 +77,9 @@
             <a	type="button"
                 class="btn gf-form-button btn-secondary"
                 ng-if="ctrl.revisions.length > 1"
-                ng-class="{disabled: !ctrl.isComparable()}"
+                ng-class="{disabled: !ctrl.canCompare}"
                 ng-click="ctrl.getDiff(ctrl.diff)"
-                bs-tooltip="ctrl.isComparable() ? '' : 'Select 2 versions to start comparing'">
+								bs-tooltip="ctrl.canCompare ? '' : 'Select 2 versions to start comparing'" data-placement="bottom">
               <i class="fa fa-code-fork" ></i>&nbsp;&nbsp;Compare versions
             </a>
             <a  type="button"
@@ -103,7 +100,7 @@
     <aside class="edit-sidemenu-aside">
       <ul class="edit-sidemenu">
         <li ng-class="{active: ctrl.diff === 'basic'}"><a ng-click="ctrl.getDiff('basic')" href="">Change Summary</a></li>
-        <li ng-class="{active: ctrl.diff === 'html'}"><a ng-click="ctrl.getDiff('html')" href="">JSON Code View</a></li>
+        <li ng-class="{active: ctrl.diff === 'html'}"><a ng-click="ctrl.getDiff('json')" href="">JSON Code View</a></li>
       </ul>
     </aside>
 
@@ -113,36 +110,36 @@
         <em>Fetching changes&hellip;</em>
       </div>
 
-      <div ng-if="!ctrl.loading" ng-init="new = ctrl.selected[0]; original = ctrl.selected[1]">
+      <div ng-if="!ctrl.loading">
         <a  type="button"
             class="btn gf-form-button btn-secondary pull-right"
-            ng-click="ctrl.restore(new)"
-            ng-if="ctrl.isOriginalCurrent()">
-          <i class="fa fa-rotate-right" ></i>&nbsp;&nbsp;Restore to version {{new}}
+            ng-click="ctrl.restore(ctrl.baseInfo.version)"
+            ng-if="ctrl.isNewLatest">
+          <i class="fa fa-history" ></i>&nbsp;&nbsp;Restore to version {{ctrl.baseInfo.version}}
         </a>
         <h4>
-          Comparing Version {{ctrl.selected[0]}}
+          Comparing Version {{ctrl.baseInfo.version}}
           <i class="fa fa-arrows-h"></i>
-          Version {{ctrl.selected[1]}}
-          <cite class="muted" ng-if="ctrl.isOriginalCurrent()">(Current)</cite>
+					Version {{ctrl.newInfo.version}}
+          <cite class="muted" ng-if="ctrl.isNewLatest">(Latest)</cite>
         </h4>
         <section ng-if="ctrl.diff === 'basic'">
           <p class="small muted">
-          <strong>Version {{new}}</strong> updated by
-          <span>{{ctrl.getMeta(new, 'createdBy')}} </span>
-          <span>{{ctrl.formatBasicDate(ctrl.getMeta(new, 'created'))}}</span>
-          <span> - {{ctrl.getMeta(new, 'message')}}</span>
+          <strong>Version {{ctrl.newInfo.version}}</strong> updated by
+          <span>{{ctrl.newInfo.createdBy}} </span>
+          <span>{{ctrl.newInfo.ageString}}</span>
+          <span> - {{ctrl.newInfo.message}}</span>
           </p>
           <p class="small muted">
-          <strong>Version {{original}}</strong> updated by
-          <span>{{ctrl.getMeta(original, 'createdBy')}} </span>
-          <span>{{ctrl.formatBasicDate(ctrl.getMeta(original, 'created'))}}</span>
-          <span> - {{ctrl.getMeta(original, 'message')}}</span>
+          <strong>Version {{ctrl.baseInfo.version}}</strong> updated by
+          <span>{{ctrl.baseInfo.createdBy}} </span>
+          <span>{{ctrl.baseInfo.ageString}}</span>
+          <span> - {{ctrl.baseInfo.message}}</span>
           </p>
         </section>
         <div id="delta" diff-delta>
           <div class="delta-basic" ng-show="ctrl.diff === 'basic'" compile="ctrl.delta.basic"></div>
-          <div class="delta-html" ng-show="ctrl.diff === 'html'" compile="ctrl.delta.html"></div>
+          <div class="delta-html" ng-show="ctrl.diff === 'json'" compile="ctrl.delta.json"></div>
         </div>
       </div>
     </div>

+ 64 - 74
public/app/features/dashboard/history/history.ts

@@ -7,20 +7,23 @@ import angular from 'angular';
 import moment from 'moment';
 
 import {DashboardModel} from '../model';
-import {HistoryListOpts, RevisionsModel} from './models';
+import {HistoryListOpts, RevisionsModel, CalculateDiffOptions, HistorySrv} from './history_srv';
 
 export class HistoryListCtrl {
   appending: boolean;
   dashboard: DashboardModel;
-  delta: { basic: string; html: string; };
+  delta: { basic: string; json: string; };
   diff: string;
   limit: number;
   loading: boolean;
   max: number;
   mode: string;
   revisions: RevisionsModel[];
-  selected: number[];
   start: number;
+  newInfo: RevisionsModel;
+  baseInfo: RevisionsModel;
+  canCompare: boolean;
+  isNewLatest: boolean;
 
   /** @ngInject */
   constructor(private $scope,
@@ -29,9 +32,7 @@ export class HistoryListCtrl {
               private $window,
               private $timeout,
               private $q,
-              private contextSrv,
-              private historySrv) {
-    $scope.ctrl = this;
+              private historySrv: HistorySrv) {
 
     this.appending = false;
     this.diff = 'basic';
@@ -39,19 +40,22 @@ export class HistoryListCtrl {
     this.loading = false;
     this.max = 2;
     this.mode = 'list';
-    this.selected = [];
     this.start = 0;
+    this.canCompare = false;
 
+    this.$rootScope.onAppEvent('dashboard-saved', this.onDashboardSaved.bind(this), $scope);
     this.resetFromSource();
+  }
 
-    $scope.$watch('ctrl.mode', newVal => {
-      $window.scrollTo(0, 0);
-      if (newVal === 'list') {
-        this.reset();
-      }
-    });
+  onDashboardSaved() {
+    this.resetFromSource();
+  }
 
-    $rootScope.onAppEvent('dashboard-saved', this.onDashboardSaved.bind(this));
+  switchMode(mode: string) {
+    this.mode = mode;
+    if (this.mode === 'list') {
+      this.reset();
+    }
   }
 
   dismiss() {
@@ -63,26 +67,13 @@ export class HistoryListCtrl {
     this.getLog(true);
   }
 
-  compareRevisionStateChanged(revision: any) {
-    if (revision.checked) {
-      this.selected.push(revision.version);
-    } else {
-      _.remove(this.selected, version => version === revision.version);
-    }
-    this.selected = _.sortBy(this.selected);
-  }
-
-  compareRevisionDisabled(checked: boolean) {
-    return (this.selected.length === this.max && !checked) || this.revisions.length === 1;
+  revisionSelectionChanged() {
+    let selected = _.filter(this.revisions, {checked: true}).length;
+    this.canCompare = selected === 2;
   }
 
   formatDate(date) {
-    date = moment.isMoment(date) ? date : moment(date);
-    const format = 'YYYY-MM-DD HH:mm:ss';
-
-    return this.dashboard.timezone === 'browser' ?
-      moment(date).format(format) :
-      moment.utc(date).format(format);
+    return this.dashboard.formatDate(date);
   }
 
   formatBasicDate(date) {
@@ -92,29 +83,40 @@ export class HistoryListCtrl {
   }
 
   getDiff(diff: string) {
-    if (!this.isComparable()) { return; } // disable button but not tooltip
-
     this.diff = diff;
     this.mode = 'compare';
-    this.loading = true;
-
-    // instead of using lodash to find min/max we use the index
-    // due to the array being sorted in ascending order
-    const compare = {
-      new: this.selected[1],
-      original: this.selected[0],
-    };
 
+    // have it already been fetched?
     if (this.delta[this.diff]) {
-      this.loading = false;
       return this.$q.when(this.delta[this.diff]);
-    } else {
-      return this.historySrv.compareVersions(this.dashboard, compare, diff).then(response => {
-        this.delta[this.diff] = response;
-      }).catch(err => {
-        this.mode = 'list';
-      }).finally(() => { this.loading = false; });
     }
+
+    const selected = _.filter(this.revisions, {checked: true});
+
+    this.newInfo = selected[0];
+    this.baseInfo = selected[1];
+    this.isNewLatest = this.newInfo.version === this.dashboard.version;
+
+    this.loading = true;
+    const options: CalculateDiffOptions = {
+      new: {
+        dashboardId: this.dashboard.id,
+        version: this.newInfo.version,
+      },
+      base: {
+        dashboardId: this.dashboard.id,
+        version: this.baseInfo.version,
+      },
+      diffType: diff,
+    };
+
+    return this.historySrv.calculateDiff(options).then(response => {
+      this.delta[this.diff] = response;
+    }).catch(() => {
+      this.mode = 'list';
+    }).finally(() => {
+      this.loading = false;
+    });
   }
 
   getLog(append = false) {
@@ -126,48 +128,35 @@ export class HistoryListCtrl {
     };
 
     return this.historySrv.getHistoryList(this.dashboard, options).then(revisions => {
+      // set formated dates & default values
+      for (let rev of revisions) {
+        rev.createdDateString = this.formatDate(rev.created);
+        rev.ageString = this.formatBasicDate(rev.created);
+        rev.checked = false;
+      }
+
       this.revisions = append ? this.revisions.concat(revisions) : revisions;
+
     }).catch(err => {
-      this.$rootScope.appEvent('alert-error', ['There was an error fetching the history list', (err.message || err)]);
+      this.loading = false;
     }).finally(() => {
       this.loading = false;
       this.appending = false;
     });
   }
 
-  getMeta(version: number, property: string) {
-    const revision = _.find(this.revisions, rev => rev.version === version);
-    return revision[property];
-  }
-
-  isOriginalCurrent() {
-    return this.selected[1] === this.dashboard.version;
-  }
-
-  isComparable() {
-    const isParamLength = this.selected.length === 2;
-    const areNumbers = this.selected.every(version => _.isNumber(version));
-    const areValidVersions = _.filter(this.revisions, revision => {
-      return revision.version === this.selected[0] || revision.version === this.selected[1];
-    }).length === 2;
-    return isParamLength && areNumbers && areValidVersions;
-  }
-
   isLastPage() {
     return _.find(this.revisions, rev => rev.version === 1);
   }
 
-  onDashboardSaved() {
-    this.$rootScope.appEvent('hide-dash-editor');
-  }
-
   reset() {
-    this.delta = { basic: '', html: '' };
+    this.delta = { basic: '', json: '' };
     this.diff = 'basic';
     this.mode = 'list';
     this.revisions = _.map(this.revisions, rev => _.extend({}, rev, { checked: false }));
-    this.selected = [];
+    this.canCompare = false;
     this.start = 0;
+    this.isNewLatest = false;
   }
 
   resetFromSource() {
@@ -191,7 +180,8 @@ export class HistoryListCtrl {
     return this.historySrv.restoreDashboard(this.dashboard, version).then(response => {
       this.$route.reload();
       this.$rootScope.appEvent('alert-success', ['Dashboard restored', 'Restored from version ' + version]);
-    }).finally(() => {
+    }).catch(() => {
+      this.mode = 'list';
       this.loading = false;
     });
   }

+ 30 - 5
public/app/features/dashboard/history/history_srv.ts

@@ -3,7 +3,34 @@
 import _ from 'lodash';
 import coreModule from 'app/core/core_module';
 import {DashboardModel} from '../model';
-import {HistoryListOpts} from './models';
+
+export interface HistoryListOpts {
+  limit: number;
+  start: number;
+}
+
+export interface RevisionsModel {
+  id: number;
+  checked: boolean;
+  dashboardId: number;
+  parentVersion: number;
+  version: number;
+  created: Date;
+  createdBy: string;
+  message: string;
+}
+
+export interface CalculateDiffOptions {
+  new: DiffTarget;
+  base: DiffTarget;
+  diffType: string;
+}
+
+export interface DiffTarget {
+  dashboardId: number;
+  version: number;
+  unsavedDashboard?: DashboardModel; // when doing diffs against unsaved dashboard version
+}
 
 export class HistorySrv {
   /** @ngInject */
@@ -14,10 +41,8 @@ export class HistorySrv {
     return id ? this.backendSrv.get(`api/dashboards/id/${id}/versions`, options) : this.$q.when([]);
   }
 
-  compareVersions(dashboard: DashboardModel, compare: { new: number, original: number }, view = 'html') {
-    const id = dashboard && dashboard.id ? dashboard.id : void 0;
-    const url = `api/dashboards/id/${id}/compare/${compare.original}...${compare.new}/${view}`;
-    return id ? this.backendSrv.get(url) : this.$q.when({});
+  calculateDiff(options: CalculateDiffOptions) {
+    return this.backendSrv.post('api/dashboards/calculate-diff', options);
   }
 
   restoreDashboard(dashboard: DashboardModel, version: number) {

+ 0 - 15
public/app/features/dashboard/history/models.ts

@@ -1,15 +0,0 @@
-export interface HistoryListOpts {
-  limit: number;
-  start: number;
-}
-
-export interface RevisionsModel {
-  id: number;
-  checked: boolean;
-  dashboardId: number;
-  parentVersion: number;
-  version: number;
-  created: Date;
-  createdBy: string;
-  message: string;
-}

+ 1 - 1
public/app/features/dashboard/model.ts

@@ -244,7 +244,7 @@ export class DashboardModel {
     return newPanel;
   }
 
-  formatDate(date, format) {
+  formatDate(date, format?) {
     date = moment.isMoment(date) ? date : moment(date);
     format = format || 'YYYY-MM-DD HH:mm:ss';
     this.timezone = this.getTimezone();

+ 44 - 45
public/app/features/dashboard/specs/history_ctrl_specs.ts

@@ -23,7 +23,7 @@ describe('HistoryListCtrl', function() {
   beforeEach(function() {
     historySrv = {
       getHistoryList: sinon.stub(),
-      compareVersions: sinon.stub(),
+      calculateDiff: sinon.stub(),
       restoreDashboard: sinon.stub(),
     };
     $rootScope = {
@@ -42,6 +42,12 @@ describe('HistoryListCtrl', function() {
         historySrv,
         $rootScope,
         $scope: ctx.scope,
+      }, {
+        dashboard: {
+          id: 2,
+          version: 3,
+          formatDate: sinon.stub().returns('date'),
+        }
       });
     }));
 
@@ -57,9 +63,8 @@ describe('HistoryListCtrl', function() {
 
       it('should reset the controller\'s state', function() {
         expect(ctx.ctrl.mode).to.be('list');
-        expect(ctx.ctrl.delta).to.eql({ basic: '', html: '' });
-        expect(ctx.ctrl.selected.length).to.be(0);
-        expect(ctx.ctrl.selected).to.eql([]);
+        expect(ctx.ctrl.delta).to.eql({ basic: '', json: '' });
+        expect(ctx.ctrl.canCompare).to.be(false);
         expect(_.find(ctx.ctrl.revisions, rev => rev.checked)).to.be.undefined;
       });
 
@@ -82,11 +87,9 @@ describe('HistoryListCtrl', function() {
       it('should set all checked properties to false on reset', function() {
         ctx.ctrl.revisions[0].checked = true;
         ctx.ctrl.revisions[2].checked = true;
-        ctx.ctrl.selected = [0, 2];
         ctx.ctrl.reset();
         var actual = _.filter(ctx.ctrl.revisions, rev => !rev.checked);
         expect(actual.length).to.be(4);
-        expect(ctx.ctrl.selected).to.eql([]);
       });
 
     });
@@ -99,9 +102,7 @@ describe('HistoryListCtrl', function() {
 
       it('should reset the controller\'s state', function() {
         expect(ctx.ctrl.mode).to.be('list');
-        expect(ctx.ctrl.delta).to.eql({ basic: '', html: '' });
-        expect(ctx.ctrl.selected.length).to.be(0);
-        expect(ctx.ctrl.selected).to.eql([]);
+        expect(ctx.ctrl.delta).to.eql({ basic: '', json: '' });
         expect(_.find(ctx.ctrl.revisions, rev => rev.checked)).to.be.undefined;
       });
 
@@ -109,11 +110,6 @@ describe('HistoryListCtrl', function() {
         expect(ctx.ctrl.loading).to.be(false);
       });
 
-      it('should broadcast an event indicating the failure', function() {
-        expect($rootScope.appEvent.calledOnce).to.be(true);
-        expect($rootScope.appEvent.calledWith('alert-error')).to.be(true);
-      });
-
       it('should have an empty revisions list', function() {
         expect(ctx.ctrl.revisions).to.eql([]);
       });
@@ -121,7 +117,7 @@ describe('HistoryListCtrl', function() {
 
     describe('should update the history list when the dashboard is saved', function() {
       beforeEach(function() {
-        ctx.ctrl.dashboard = { version: 3 };
+        ctx.ctrl.dashboard = {version: 3 };
         ctx.ctrl.resetFromSource = sinon.spy();
       });
 
@@ -134,12 +130,6 @@ describe('HistoryListCtrl', function() {
         expect($rootScope.onAppEvent.getCall(0).args[1]).to.not.be(ctx.ctrl.onDashboardSaved);
         expect($rootScope.onAppEvent.getCall(0).args[1].toString).to.be(ctx.ctrl.onDashboardSaved.toString);
       });
-
-      it('should emit an appEvent to hide the changelog', function() {
-        ctx.ctrl.onDashboardSaved();
-        expect($rootScope.appEvent.calledOnce).to.be(true);
-        expect($rootScope.appEvent.getCall(0).args[0]).to.be('hide-dash-editor');
-      });
     });
   });
 
@@ -149,12 +139,19 @@ describe('HistoryListCtrl', function() {
     beforeEach(angularMocks.inject(($controller, $q) => {
       deferred = $q.defer();
       historySrv.getHistoryList.returns($q.when(versionsResponse));
-      historySrv.compareVersions.returns(deferred.promise);
+      historySrv.calculateDiff.returns(deferred.promise);
       ctx.ctrl = $controller(HistoryListCtrl, {
         historySrv,
         $rootScope,
         $scope: ctx.scope,
+      }, {
+        dashboard: {
+          id: 2,
+          version: 3,
+          formatDate: sinon.stub().returns('date'),
+        }
       });
+
       ctx.ctrl.$scope.onDashboardSaved = sinon.spy();
       ctx.ctrl.$scope.$apply();
     }));
@@ -166,33 +163,32 @@ describe('HistoryListCtrl', function() {
 
     it('should check that two valid versions are selected', function() {
       // []
-      expect(ctx.ctrl.isComparable()).to.be(false);
+      expect(ctx.ctrl.canCompare).to.be(false);
 
       // single value
-      ctx.ctrl.selected = [4];
-      expect(ctx.ctrl.isComparable()).to.be(false);
+      ctx.ctrl.revisions = [{checked: true}];
+      ctx.ctrl.revisionSelectionChanged();
+      expect(ctx.ctrl.canCompare).to.be(false);
 
       // both values in range
-      ctx.ctrl.selected = [4, 2];
-      expect(ctx.ctrl.isComparable()).to.be(true);
-
-      // values out of range
-      ctx.ctrl.selected = [7, 4];
-      expect(ctx.ctrl.isComparable()).to.be(false);
+      ctx.ctrl.revisions = [{checked: true}, {checked: true}];
+      ctx.ctrl.revisionSelectionChanged();
+      expect(ctx.ctrl.canCompare).to.be(true);
     });
 
     describe('and the basic diff is successfully fetched', function() {
       beforeEach(function() {
         deferred.resolve(compare('basic'));
-        ctx.ctrl.selected = [3, 1];
+        ctx.ctrl.revisions[1].checked = true;
+        ctx.ctrl.revisions[3].checked = true;
         ctx.ctrl.getDiff('basic');
         ctx.ctrl.$scope.$apply();
       });
 
       it('should fetch the basic diff if two valid versions are selected', function() {
-        expect(historySrv.compareVersions.calledOnce).to.be(true);
+        expect(historySrv.calculateDiff.calledOnce).to.be(true);
         expect(ctx.ctrl.delta.basic).to.be('<div></div>');
-        expect(ctx.ctrl.delta.html).to.be('');
+        expect(ctx.ctrl.delta.json).to.be('');
       });
 
       it('should set the basic diff view as active', function() {
@@ -207,21 +203,22 @@ describe('HistoryListCtrl', function() {
 
     describe('and the json diff is successfully fetched', function() {
       beforeEach(function() {
-        deferred.resolve(compare('html'));
-        ctx.ctrl.selected = [3, 1];
-        ctx.ctrl.getDiff('html');
+        deferred.resolve(compare('json'));
+        ctx.ctrl.revisions[1].checked = true;
+        ctx.ctrl.revisions[3].checked = true;
+        ctx.ctrl.getDiff('json');
         ctx.ctrl.$scope.$apply();
       });
 
       it('should fetch the json diff if two valid versions are selected', function() {
-        expect(historySrv.compareVersions.calledOnce).to.be(true);
+        expect(historySrv.calculateDiff.calledOnce).to.be(true);
         expect(ctx.ctrl.delta.basic).to.be('');
-        expect(ctx.ctrl.delta.html).to.be('<pre><code></code></pre>');
+        expect(ctx.ctrl.delta.json).to.be('<pre><code></code></pre>');
       });
 
       it('should set the json diff view as active', function() {
         expect(ctx.ctrl.mode).to.be('compare');
-        expect(ctx.ctrl.diff).to.be('html');
+        expect(ctx.ctrl.diff).to.be('json');
       });
 
       it('should indicate loading has finished', function() {
@@ -232,14 +229,15 @@ describe('HistoryListCtrl', function() {
     describe('and diffs have already been fetched', function() {
       beforeEach(function() {
         deferred.resolve(compare('basic'));
-        ctx.ctrl.selected = [3, 1];
+        ctx.ctrl.revisions[3].checked = true;
+        ctx.ctrl.revisions[1].checked = true;
         ctx.ctrl.delta.basic = 'cached basic';
         ctx.ctrl.getDiff('basic');
         ctx.ctrl.$scope.$apply();
       });
 
       it('should use the cached diffs instead of fetching', function() {
-        expect(historySrv.compareVersions.calledOnce).to.be(false);
+        expect(historySrv.calculateDiff.calledOnce).to.be(false);
         expect(ctx.ctrl.delta.basic).to.be('cached basic');
       });
 
@@ -251,13 +249,14 @@ describe('HistoryListCtrl', function() {
     describe('and fetching the diff fails', function() {
       beforeEach(function() {
         deferred.reject(new Error('DiffError'));
-        ctx.ctrl.selected = [4, 2];
+        ctx.ctrl.revisions[3].checked = true;
+        ctx.ctrl.revisions[1].checked = true;
         ctx.ctrl.getDiff('basic');
         ctx.ctrl.$scope.$apply();
       });
 
       it('should fetch the diff if two valid versions are selected', function() {
-        expect(historySrv.compareVersions.calledOnce).to.be(true);
+        expect(historySrv.calculateDiff.calledOnce).to.be(true);
       });
 
       it('should return to the history list view', function() {
@@ -269,7 +268,7 @@ describe('HistoryListCtrl', function() {
       });
 
       it('should have an empty delta/changeset', function() {
-        expect(ctx.ctrl.delta).to.eql({ basic: '', html: '' });
+        expect(ctx.ctrl.delta).to.eql({ basic: '', json: '' });
       });
     });
   });

+ 0 - 22
public/app/features/dashboard/specs/history_srv_specs.ts

@@ -8,7 +8,6 @@ describe('historySrv', function() {
   var ctx = new helpers.ServiceTestContext();
 
   var versionsResponse = versions();
-  var compareResponse = compare();
   var restoreResponse = restore;
 
   beforeEach(angularMocks.module('grafana.core'));
@@ -16,7 +15,6 @@ describe('historySrv', function() {
   beforeEach(angularMocks.inject(function($httpBackend) {
     ctx.$httpBackend = $httpBackend;
     $httpBackend.whenRoute('GET', 'api/dashboards/id/:id/versions').respond(versionsResponse);
-    $httpBackend.whenRoute('GET', 'api/dashboards/id/:id/compare/:original...:new').respond(compareResponse);
     $httpBackend.whenRoute('POST', 'api/dashboards/id/:id/restore')
       .respond(function(method, url, data, headers, params) {
         const parsedData = JSON.parse(data);
@@ -51,26 +49,6 @@ describe('historySrv', function() {
     });
   });
 
-  describe('compareVersions', function() {
-    it('should return a diff object for the given dashboard revisions', function(done) {
-      var compare = { original: 6, new: 4 };
-      ctx.service.compareVersions({ id: 1 }, compare).then(function(response) {
-        expect(response).to.eql(compareResponse);
-        done();
-      });
-      ctx.$httpBackend.flush();
-    });
-
-    it('should return an empty object when not given an id', function(done) {
-      var compare = { original: 6, new: 4 };
-      ctx.service.compareVersions({ }, compare).then(function(response) {
-        expect(response).to.eql({});
-        done();
-      });
-      ctx.$httpBackend.flush();
-    });
-  });
-
   describe('restoreDashboard', function() {
     it('should return a success response given valid parameters', function(done) {
       var version = 6;