From ef1dfed0d8a81cc5c388df54bcd50b169738c947 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 7 Jun 2017 11:50:09 +0200 Subject: [PATCH] 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 --- pkg/api/api.go | 5 +- pkg/api/dashboard.go | 109 +++----------- pkg/api/dtos/dashboard.go | 12 ++ pkg/components/dashdiffs/compare.go | 53 ++++--- .../features/dashboard/history/history.html | 57 ++++---- .../app/features/dashboard/history/history.ts | 138 ++++++++---------- .../features/dashboard/history/history_srv.ts | 35 ++++- .../app/features/dashboard/history/models.ts | 15 -- public/app/features/dashboard/model.ts | 2 +- .../dashboard/specs/history_ctrl_specs.ts | 89 ++++++----- .../dashboard/specs/history_srv_specs.ts | 22 --- 11 files changed, 235 insertions(+), 302 deletions(-) delete mode 100644 public/app/features/dashboard/history/models.ts diff --git a/pkg/api/api.go b/pkg/api/api.go index 636d61e14d3..c3a3728338d 100644 --- a/pkg/api/api.go +++ b/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)) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 4b4f6fe8597..129faa3d01e 100644 --- a/pkg/api/dashboard.go +++ b/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") + 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, + }, } - 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) + result, err := dashdiffs.CalculateDiff(&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) + 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 Json(200, util.DynMap{ - "meta": util.DynMap{ - "baseVersion": options.BaseVersion, - "newVersion": options.NewVersion, - }, - "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) - - 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") } // RestoreDashboardVersion restores a dashboard to the given version. diff --git a/pkg/api/dtos/dashboard.go b/pkg/api/dtos/dashboard.go index fd8a7115de2..6a11199bb1c 100644 --- a/pkg/api/dtos/dashboard.go +++ b/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"` } diff --git a/pkg/components/dashdiffs/compare.go b/pkg/components/dashdiffs/compare.go index a559c5b16f9..51c8a947ce1 100644 --- a/pkg/components/dashdiffs/compare.go +++ b/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 { diff --git a/public/app/features/dashboard/history/history.html b/public/app/features/dashboard/history/history.html index 53d960fd620..ab64b48d342 100644 --- a/public/app/features/dashboard/history/history.html +++ b/public/app/features/dashboard/history/history.html @@ -5,16 +5,13 @@ @@ -47,15 +44,15 @@ - + + on-change="ctrl.revisionSelectionChanged()" + ng-disabled="!revision.checked && ctrl.canCompare"> {{revision.version}} - {{ctrl.formatDate(revision.created)}} + {{revision.createdDateString}} {{revision.createdBy}} {{revision.message}} @@ -63,7 +60,7 @@   Restore -   Current +   Latest @@ -80,9 +77,9 @@ + bs-tooltip="ctrl.canCompare ? '' : 'Select 2 versions to start comparing'" data-placement="bottom">   Compare versions @@ -113,36 +110,36 @@ Fetching changes… -
+
-   Restore to version {{new}} + ng-click="ctrl.restore(ctrl.baseInfo.version)" + ng-if="ctrl.isNewLatest"> +   Restore to version {{ctrl.baseInfo.version}}

- Comparing Version {{ctrl.selected[0]}} + Comparing Version {{ctrl.baseInfo.version}} - Version {{ctrl.selected[1]}} - (Current) + Version {{ctrl.newInfo.version}} + (Latest)

- Version {{new}} updated by - {{ctrl.getMeta(new, 'createdBy')}} - {{ctrl.formatBasicDate(ctrl.getMeta(new, 'created'))}} - - {{ctrl.getMeta(new, 'message')}} + Version {{ctrl.newInfo.version}} updated by + {{ctrl.newInfo.createdBy}} + {{ctrl.newInfo.ageString}} + - {{ctrl.newInfo.message}}

- Version {{original}} updated by - {{ctrl.getMeta(original, 'createdBy')}} - {{ctrl.formatBasicDate(ctrl.getMeta(original, 'created'))}} - - {{ctrl.getMeta(original, 'message')}} + Version {{ctrl.baseInfo.version}} updated by + {{ctrl.baseInfo.createdBy}} + {{ctrl.baseInfo.ageString}} + - {{ctrl.baseInfo.message}}

-
+
diff --git a/public/app/features/dashboard/history/history.ts b/public/app/features/dashboard/history/history.ts index 44d8a0f2226..5bd8ef7c895 100644 --- a/public/app/features/dashboard/history/history.ts +++ b/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]) { + return this.$q.when(this.delta[this.diff]); + } + + 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, }; - if (this.delta[this.diff]) { + return this.historySrv.calculateDiff(options).then(response => { + this.delta[this.diff] = response; + }).catch(() => { + this.mode = 'list'; + }).finally(() => { 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; }); - } + }); } 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; }); } diff --git a/public/app/features/dashboard/history/history_srv.ts b/public/app/features/dashboard/history/history_srv.ts index ba9ad3975de..6517c36e1a7 100644 --- a/public/app/features/dashboard/history/history_srv.ts +++ b/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) { diff --git a/public/app/features/dashboard/history/models.ts b/public/app/features/dashboard/history/models.ts deleted file mode 100644 index b3cc2829511..00000000000 --- a/public/app/features/dashboard/history/models.ts +++ /dev/null @@ -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; -} diff --git a/public/app/features/dashboard/model.ts b/public/app/features/dashboard/model.ts index e62c98b8598..cdb61ac4ad7 100644 --- a/public/app/features/dashboard/model.ts +++ b/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(); diff --git a/public/app/features/dashboard/specs/history_ctrl_specs.ts b/public/app/features/dashboard/specs/history_ctrl_specs.ts index f8e4a3d25f1..0bb09546a34 100644 --- a/public/app/features/dashboard/specs/history_ctrl_specs.ts +++ b/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('
'); - 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('
'); + expect(ctx.ctrl.delta.json).to.be('
'); }); 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: '' }); }); }); }); diff --git a/public/app/features/dashboard/specs/history_srv_specs.ts b/public/app/features/dashboard/specs/history_srv_specs.ts index 2d01f323d35..4678759c438 100644 --- a/public/app/features/dashboard/specs/history_srv_specs.ts +++ b/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;