diff --git a/public/app/core/services/context_srv.ts b/public/app/core/services/context_srv.ts index 243c7bb69da..2f8b524a8c4 100644 --- a/public/app/core/services/context_srv.ts +++ b/public/app/core/services/context_srv.ts @@ -110,9 +110,16 @@ export class ContextSrv { } } -const contextSrv = new ContextSrv(); +let contextSrv = new ContextSrv(); export { contextSrv }; +export const setContextSrv = (override: ContextSrv) => { + if (process.env.NODE_ENV !== 'test') { + throw new Error('contextSrv can be only overriden in test environment'); + } + contextSrv = override; +}; + coreModule.factory('contextSrv', () => { return contextSrv; }); diff --git a/public/app/features/dashboard/services/ChangeTracker.test.ts b/public/app/features/dashboard/services/ChangeTracker.test.ts index 8b52ac78a88..a7175184df9 100644 --- a/public/app/features/dashboard/services/ChangeTracker.test.ts +++ b/public/app/features/dashboard/services/ChangeTracker.test.ts @@ -1,56 +1,57 @@ import { ChangeTracker } from './ChangeTracker'; import { DashboardModel } from '../state/DashboardModel'; import { PanelModel } from '../state/PanelModel'; +import { setContextSrv } from '../../../core/services/context_srv'; -jest.mock('app/core/services/context_srv', () => ({ - contextSrv: { - user: { orgId: 1 }, - }, -})); +function getDefaultDashboardModel(): DashboardModel { + return new DashboardModel({ + refresh: false, + panels: [ + { + id: 1, + type: 'graph', + gridPos: { x: 0, y: 0, w: 24, h: 6 }, + legend: { sortDesc: false }, + }, + { + id: 2, + type: 'row', + gridPos: { x: 0, y: 6, w: 24, h: 2 }, + collapsed: true, + panels: [ + { id: 3, type: 'graph', gridPos: { x: 0, y: 6, w: 12, h: 2 } }, + { id: 4, type: 'graph', gridPos: { x: 12, y: 6, w: 12, h: 2 } }, + ], + }, + { id: 5, type: 'row', gridPos: { x: 0, y: 6, w: 1, h: 1 } }, + ], + }); +} + +function getTestContext() { + const contextSrv: any = { isSignedIn: true, isEditor: true }; + setContextSrv(contextSrv); + const dash: any = getDefaultDashboardModel(); + const tracker = new ChangeTracker(); + const original: any = dash.getSaveModelClone(); + + return { dash, tracker, original, contextSrv }; +} describe('ChangeTracker', () => { - let tracker: ChangeTracker; - let dash: any; - let original: any; - - beforeEach(() => { - dash = new DashboardModel({ - refresh: false, - panels: [ - { - id: 1, - type: 'graph', - gridPos: { x: 0, y: 0, w: 24, h: 6 }, - legend: { sortDesc: false }, - }, - { - id: 2, - type: 'row', - gridPos: { x: 0, y: 6, w: 24, h: 2 }, - collapsed: true, - panels: [ - { id: 3, type: 'graph', gridPos: { x: 0, y: 6, w: 12, h: 2 } }, - { id: 4, type: 'graph', gridPos: { x: 12, y: 6, w: 12, h: 2 } }, - ], - }, - { id: 5, type: 'row', gridPos: { x: 0, y: 6, w: 1, h: 1 } }, - ], - }); - - tracker = new ChangeTracker(); - original = dash.getSaveModelClone(); - }); - it('No changes should not have changes', () => { + const { tracker, original, dash } = getTestContext(); expect(tracker.hasChanges(dash, original)).toBe(false); }); it('Simple change should be registered', () => { + const { tracker, original, dash } = getTestContext(); dash.title = 'google'; expect(tracker.hasChanges(dash, original)).toBe(true); }); it('Should ignore a lot of changes', () => { + const { tracker, original, dash } = getTestContext(); dash.time = { from: '1h' }; dash.refresh = true; dash.schemaVersion = 10; @@ -58,23 +59,104 @@ describe('ChangeTracker', () => { }); it('Should ignore .iteration changes', () => { + const { tracker, original, dash } = getTestContext(); dash.iteration = new Date().getTime() + 1; expect(tracker.hasChanges(dash, original)).toBe(false); }); it('Should ignore row collapse change', () => { + const { tracker, original, dash } = getTestContext(); dash.toggleRow(dash.panels[1]); expect(tracker.hasChanges(dash, original)).toBe(false); }); it('Should ignore panel legend changes', () => { + const { tracker, original, dash } = getTestContext(); dash.panels[0].legend.sortDesc = true; dash.panels[0].legend.sort = 'avg'; expect(tracker.hasChanges(dash, original)).toBe(false); }); it('Should ignore panel repeats', () => { + const { tracker, original, dash } = getTestContext(); dash.panels.push(new PanelModel({ repeatPanelId: 10 })); expect(tracker.hasChanges(dash, original)).toBe(false); }); + + describe('ignoreChanges', () => { + describe('when called without original dashboard', () => { + it('then it should return true', () => { + const { tracker, dash } = getTestContext(); + expect(tracker.ignoreChanges(dash, null)).toBe(true); + }); + }); + + describe('when called without current dashboard', () => { + it('then it should return true', () => { + const { tracker, original } = getTestContext(); + expect(tracker.ignoreChanges((null as unknown) as DashboardModel, original)).toBe(true); + }); + }); + + describe('when called without meta in current dashboard', () => { + it('then it should return true', () => { + const { tracker, original, dash } = getTestContext(); + expect(tracker.ignoreChanges({ ...dash, meta: undefined }, original)).toBe(true); + }); + }); + + describe('when called for a viewer without save permissions', () => { + it('then it should return true', () => { + const { tracker, original, dash, contextSrv } = getTestContext(); + contextSrv.isEditor = false; + expect(tracker.ignoreChanges({ ...dash, meta: { canSave: false } }, original)).toBe(true); + }); + }); + + describe('when called for a viewer with save permissions', () => { + it('then it should return undefined', () => { + const { tracker, original, dash, contextSrv } = getTestContext(); + contextSrv.isEditor = false; + expect(tracker.ignoreChanges({ ...dash, meta: { canSave: true } }, original)).toBe(undefined); + }); + }); + + describe('when called for an user that is not signed in', () => { + it('then it should return true', () => { + const { tracker, original, dash, contextSrv } = getTestContext(); + contextSrv.isSignedIn = false; + expect(tracker.ignoreChanges({ ...dash, meta: { canSave: true } }, original)).toBe(true); + }); + }); + + describe('when called with fromScript', () => { + it('then it should return true', () => { + const { tracker, original, dash } = getTestContext(); + expect( + tracker.ignoreChanges({ ...dash, meta: { canSave: true, fromScript: true, fromFile: undefined } }, original) + ).toBe(true); + }); + }); + + describe('when called with fromFile', () => { + it('then it should return true', () => { + const { tracker, original, dash } = getTestContext(); + expect( + tracker.ignoreChanges({ ...dash, meta: { canSave: true, fromScript: undefined, fromFile: true } }, original) + ).toBe(true); + }); + }); + + describe('when called with canSave but without fromScript and fromFile', () => { + it('then it should return false', () => { + const { tracker, original, dash } = getTestContext(); + expect( + tracker.ignoreChanges( + { ...dash, meta: { canSave: true, fromScript: undefined, fromFile: undefined } }, + original + ) + ).toBe(undefined); + }); + }); + }); }); diff --git a/public/app/features/dashboard/services/ChangeTracker.ts b/public/app/features/dashboard/services/ChangeTracker.ts index eb20557ba9b..1f4436f133f 100644 --- a/public/app/features/dashboard/services/ChangeTracker.ts +++ b/public/app/features/dashboard/services/ChangeTracker.ts @@ -78,7 +78,8 @@ export class ChangeTracker { return true; } - if (!contextSrv.isEditor) { + // Ignore changes if the user has been signed out + if (!contextSrv.isSignedIn) { return true; } @@ -86,13 +87,12 @@ export class ChangeTracker { return true; } - // Ignore changes if the user has been signed out - if (!contextSrv.isSignedIn) { + const { canSave, fromScript, fromFile } = current.meta; + if (!contextSrv.isEditor && !canSave) { return true; } - const meta = current.meta; - return !meta.canSave || meta.fromScript || meta.fromFile; + return !canSave || fromScript || fromFile; } // remove stuff that should not count in diff