From 9d6ac2c3d41a7303b3d3fd4aa9f53ec088909798 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Mon, 21 Jan 2019 08:47:41 +0100 Subject: [PATCH 1/3] fix: Viewers can edit means that viewers have acces to Explore #14281 --- pkg/api/frontendsettings.go | 1 + pkg/api/index.go | 2 +- public/app/core/config.ts | 2 ++ public/app/core/services/keybindingSrv.ts | 5 +++-- public/app/features/panel/metrics_panel_ctrl.ts | 2 +- .../panel/specs/metrics_panel_ctrl.test.ts | 15 +++++++++++++++ public/app/routes/routes.ts | 3 ++- 7 files changed, 25 insertions(+), 5 deletions(-) diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 8f5457cf271..6d6cc708496 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -165,6 +165,7 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *m.ReqContext) (map[string]interf "externalUserMngInfo": setting.ExternalUserMngInfo, "externalUserMngLinkUrl": setting.ExternalUserMngLinkUrl, "externalUserMngLinkName": setting.ExternalUserMngLinkName, + "viewersCanEdit": setting.ViewersCanEdit, "buildInfo": map[string]interface{}{ "version": setting.BuildVersion, "commit": setting.BuildCommit, diff --git a/pkg/api/index.go b/pkg/api/index.go index 2980d8a5c6b..e90db84016d 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -140,7 +140,7 @@ func (hs *HTTPServer) setIndexViewData(c *m.ReqContext) (*dtos.IndexViewData, er Children: dashboardChildNavs, }) - if setting.ExploreEnabled && (c.OrgRole == m.ROLE_ADMIN || c.OrgRole == m.ROLE_EDITOR) { + if setting.ExploreEnabled && (c.OrgRole == m.ROLE_ADMIN || c.OrgRole == m.ROLE_EDITOR || setting.ViewersCanEdit) { data.NavTree = append(data.NavTree, &dtos.NavLink{ Text: "Explore", Id: "explore", diff --git a/public/app/core/config.ts b/public/app/core/config.ts index 0aa159af84d..26f31ffcf54 100644 --- a/public/app/core/config.ts +++ b/public/app/core/config.ts @@ -34,6 +34,7 @@ export class Settings { disableUserSignUp: boolean; loginHint: any; loginError: any; + viewersCanEdit: boolean; constructor(options) { const defaults = { @@ -50,6 +51,7 @@ export class Settings { env: 'production', isEnterprise: false, }, + viewersCanEdit: false, }; _.extend(this, defaults, options); diff --git a/public/app/core/services/keybindingSrv.ts b/public/app/core/services/keybindingSrv.ts index c02f6850e8b..a85d9b5a500 100644 --- a/public/app/core/services/keybindingSrv.ts +++ b/public/app/core/services/keybindingSrv.ts @@ -8,6 +8,7 @@ import { getExploreUrl } from 'app/core/utils/explore'; import Mousetrap from 'mousetrap'; import 'mousetrap-global-bind'; +import { ContextSrv } from './context_srv'; export class KeybindingSrv { helpModal: boolean; @@ -21,7 +22,7 @@ export class KeybindingSrv { private $timeout, private datasourceSrv, private timeSrv, - private contextSrv + private contextSrv: ContextSrv ) { // clear out all shortcuts on route change $rootScope.$on('$routeChangeSuccess', () => { @@ -196,7 +197,7 @@ export class KeybindingSrv { }); // jump to explore if permissions allow - if (this.contextSrv.isEditor && config.exploreEnabled) { + if ((this.contextSrv.isEditor || config.viewersCanEdit) && config.exploreEnabled) { this.bind('x', async () => { if (dashboard.meta.focusPanelId) { const panel = dashboard.getPanelById(dashboard.meta.focusPanelId); diff --git a/public/app/features/panel/metrics_panel_ctrl.ts b/public/app/features/panel/metrics_panel_ctrl.ts index 5557b477b8f..576bde63114 100644 --- a/public/app/features/panel/metrics_panel_ctrl.ts +++ b/public/app/features/panel/metrics_panel_ctrl.ts @@ -231,7 +231,7 @@ class MetricsPanelCtrl extends PanelCtrl { getAdditionalMenuItems() { const items = []; - if (config.exploreEnabled && this.contextSrv.isEditor && this.datasource) { + if (config.exploreEnabled && (this.contextSrv.isEditor || config.viewersCanEdit) && this.datasource) { items.push({ text: 'Explore', click: 'ctrl.explore();', diff --git a/public/app/features/panel/specs/metrics_panel_ctrl.test.ts b/public/app/features/panel/specs/metrics_panel_ctrl.test.ts index 913a2461fd0..d4ce5cda7e2 100644 --- a/public/app/features/panel/specs/metrics_panel_ctrl.test.ts +++ b/public/app/features/panel/specs/metrics_panel_ctrl.test.ts @@ -2,6 +2,7 @@ jest.mock('app/core/core', () => ({})); jest.mock('app/core/config', () => { return { exploreEnabled: true, + viewersCanEdit: false, panels: { test: { id: 'test', @@ -14,6 +15,7 @@ jest.mock('app/core/config', () => { import q from 'q'; import { PanelModel } from 'app/features/dashboard/panel_model'; import { MetricsPanelCtrl } from '../metrics_panel_ctrl'; +import config from 'app/core/config'; describe('MetricsPanelCtrl', () => { let ctrl; @@ -46,6 +48,19 @@ describe('MetricsPanelCtrl', () => { expect(additionalItems.length).toBe(1); }); }); + + describe('and has datasource set that supports explore and viewersCanEdit is true', () => { + beforeEach(() => { + config.viewersCanEdit = true; + ctrl.contextSrv = { isEditor: false }; + ctrl.datasource = { meta: { explore: true } }; + additionalItems = ctrl.getAdditionalMenuItems(); + }); + + it('should not return any items', () => { + expect(additionalItems.length).toBe(1); + }); + }); }); }); diff --git a/public/app/routes/routes.ts b/public/app/routes/routes.ts index 4b24f69b74e..8552d0510a9 100644 --- a/public/app/routes/routes.ts +++ b/public/app/routes/routes.ts @@ -16,6 +16,7 @@ import UsersListPage from 'app/features/users/UsersListPage'; import DataSourceDashboards from 'app/features/datasources/DataSourceDashboards'; import DataSourceSettingsPage from '../features/datasources/settings/DataSourceSettingsPage'; import OrgDetailsPage from '../features/org/OrgDetailsPage'; +import config from 'app/core/config'; /** @ngInject */ export function setupAngularRoutes($routeProvider, $locationProvider) { @@ -129,7 +130,7 @@ export function setupAngularRoutes($routeProvider, $locationProvider) { template: '', reloadOnSearch: false, resolve: { - roles: () => ['Editor', 'Admin'], + roles: () => (config.viewersCanEdit ? [] : ['Editor', 'Admin']), component: () => import(/* webpackChunkName: "explore" */ 'app/features/explore/Wrapper'), }, }) From b95650d19cc437d72da32c0fe5519e1973ae12ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Mon, 21 Jan 2019 10:50:34 +0100 Subject: [PATCH 2/3] Added function hasAccessToExplore in ContextSrv and refactored tests --- public/app/core/services/context_srv.ts | 4 ++ public/app/core/services/keybindingSrv.ts | 3 +- .../app/features/panel/metrics_panel_ctrl.ts | 6 +-- .../panel/specs/metrics_panel_ctrl.test.ts | 51 +++++++------------ 4 files changed, 25 insertions(+), 39 deletions(-) diff --git a/public/app/core/services/context_srv.ts b/public/app/core/services/context_srv.ts index c4134598175..7bb753e6f71 100644 --- a/public/app/core/services/context_srv.ts +++ b/public/app/core/services/context_srv.ts @@ -59,6 +59,10 @@ export class ContextSrv { this.sidemenu = !this.sidemenu; store.set('grafana.sidemenu', this.sidemenu); } + + hasAccessToExplore() { + return (this.isEditor || config.viewersCanEdit) && config.exploreEnabled; + } } const contextSrv = new ContextSrv(); diff --git a/public/app/core/services/keybindingSrv.ts b/public/app/core/services/keybindingSrv.ts index a85d9b5a500..9e128c449a6 100644 --- a/public/app/core/services/keybindingSrv.ts +++ b/public/app/core/services/keybindingSrv.ts @@ -1,7 +1,6 @@ import $ from 'jquery'; import _ from 'lodash'; -import config from 'app/core/config'; import coreModule from 'app/core/core_module'; import appEvents from 'app/core/app_events'; import { getExploreUrl } from 'app/core/utils/explore'; @@ -197,7 +196,7 @@ export class KeybindingSrv { }); // jump to explore if permissions allow - if ((this.contextSrv.isEditor || config.viewersCanEdit) && config.exploreEnabled) { + if (this.contextSrv.hasAccessToExplore()) { this.bind('x', async () => { if (dashboard.meta.focusPanelId) { const panel = dashboard.getPanelById(dashboard.meta.focusPanelId); diff --git a/public/app/features/panel/metrics_panel_ctrl.ts b/public/app/features/panel/metrics_panel_ctrl.ts index 576bde63114..0b3bbc3080f 100644 --- a/public/app/features/panel/metrics_panel_ctrl.ts +++ b/public/app/features/panel/metrics_panel_ctrl.ts @@ -1,18 +1,18 @@ import _ from 'lodash'; import kbn from 'app/core/utils/kbn'; -import config from 'app/core/config'; import { PanelCtrl } from 'app/features/panel/panel_ctrl'; import { getExploreUrl } from 'app/core/utils/explore'; import { applyPanelTimeOverrides, getResolution } from 'app/features/dashboard/utils/panel'; +import { ContextSrv } from 'app/core/services/context_srv'; class MetricsPanelCtrl extends PanelCtrl { scope: any; datasource: any; $q: any; $timeout: any; - contextSrv: any; + contextSrv: ContextSrv; datasourceSrv: any; timeSrv: any; templateSrv: any; @@ -231,7 +231,7 @@ class MetricsPanelCtrl extends PanelCtrl { getAdditionalMenuItems() { const items = []; - if (config.exploreEnabled && (this.contextSrv.isEditor || config.viewersCanEdit) && this.datasource) { + if (this.contextSrv.hasAccessToExplore() && this.datasource) { items.push({ text: 'Explore', click: 'ctrl.explore();', diff --git a/public/app/features/panel/specs/metrics_panel_ctrl.test.ts b/public/app/features/panel/specs/metrics_panel_ctrl.test.ts index d4ce5cda7e2..8b9607d39ad 100644 --- a/public/app/features/panel/specs/metrics_panel_ctrl.test.ts +++ b/public/app/features/panel/specs/metrics_panel_ctrl.test.ts @@ -1,8 +1,6 @@ jest.mock('app/core/core', () => ({})); jest.mock('app/core/config', () => { return { - exploreEnabled: true, - viewersCanEdit: false, panels: { test: { id: 'test', @@ -15,62 +13,47 @@ jest.mock('app/core/config', () => { import q from 'q'; import { PanelModel } from 'app/features/dashboard/panel_model'; import { MetricsPanelCtrl } from '../metrics_panel_ctrl'; -import config from 'app/core/config'; describe('MetricsPanelCtrl', () => { - let ctrl; - - beforeEach(() => { - ctrl = setupController(); - }); - describe('when getting additional menu items', () => { - let additionalItems; - - describe('and has no datasource set', () => { - beforeEach(() => { - additionalItems = ctrl.getAdditionalMenuItems(); - }); - + describe('and has no datasource set but user has access to explore', () => { it('should not return any items', () => { - expect(additionalItems.length).toBe(0); + const ctrl = setupController({ hasAccessToExplore: true }); + + expect(ctrl.getAdditionalMenuItems().length).toBe(0); }); }); - describe('and has datasource set that supports explore and user has powers', () => { - beforeEach(() => { - ctrl.contextSrv = { isEditor: true }; - ctrl.datasource = { meta: { explore: true } }; - additionalItems = ctrl.getAdditionalMenuItems(); - }); - + describe('and has datasource set that supports explore and user does not have access to explore', () => { it('should not return any items', () => { - expect(additionalItems.length).toBe(1); + const ctrl = setupController({ hasAccessToExplore: false }); + ctrl.datasource = { meta: { explore: true } }; + + expect(ctrl.getAdditionalMenuItems().length).toBe(0); }); }); - describe('and has datasource set that supports explore and viewersCanEdit is true', () => { - beforeEach(() => { - config.viewersCanEdit = true; - ctrl.contextSrv = { isEditor: false }; + describe('and has datasource set that supports explore and user has access to explore', () => { + it('should return one item', () => { + const ctrl = setupController({ hasAccessToExplore: true }); ctrl.datasource = { meta: { explore: true } }; - additionalItems = ctrl.getAdditionalMenuItems(); - }); - it('should not return any items', () => { - expect(additionalItems.length).toBe(1); + expect(ctrl.getAdditionalMenuItems().length).toBe(1); }); }); }); }); -function setupController() { +function setupController({ hasAccessToExplore } = { hasAccessToExplore: false }) { const injectorStub = { get: type => { switch (type) { case '$q': { return q; } + case 'contextSrv': { + return { hasAccessToExplore: () => hasAccessToExplore }; + } default: { return jest.fn(); } From 721e40db90630a645723397e2fae9e81be4475ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Mon, 21 Jan 2019 13:51:33 +0100 Subject: [PATCH 3/3] fixed trailing whitespace --- public/app/core/services/context_srv.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/app/core/services/context_srv.ts b/public/app/core/services/context_srv.ts index c52bbdd775d..05985aae999 100644 --- a/public/app/core/services/context_srv.ts +++ b/public/app/core/services/context_srv.ts @@ -64,7 +64,7 @@ export class ContextSrv { hasAccessToExplore() { return (this.isEditor || config.viewersCanEdit) && config.exploreEnabled; } - + getTheme(): ThemeName { return this.user.lightTheme ? ThemeNames.Light : ThemeNames.Dark; }