From 97ff245ad0a46fd2024b38edbbb3190a158ab8e5 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Fri, 23 Jun 2017 17:45:37 +0200 Subject: [PATCH 1/2] dashfolders: validation for duplicates in acl modal --- public/app/features/dashboard/acl/acl.html | 37 ++++++---- public/app/features/dashboard/acl/acl.ts | 24 +++++++ .../features/dashboard/acl/specs/acl_specs.ts | 70 +++++++++++++++++++ 3 files changed, 116 insertions(+), 15 deletions(-) diff --git a/public/app/features/dashboard/acl/acl.html b/public/app/features/dashboard/acl/acl.html index 96ce10176c3..8dfa86f958a 100644 --- a/public/app/features/dashboard/acl/acl.html +++ b/public/app/features/dashboard/acl/acl.html @@ -39,22 +39,30 @@ -
-
Add Permission For
-
-
-
- +
+ +
Add Permission For
+
+
+
+ +
+
+
+ +
+
+
-
- -
-
- -
+ +
+ + + {{ctrl.error}} +
- +
Close
- -
+
diff --git a/public/app/features/dashboard/acl/acl.ts b/public/app/features/dashboard/acl/acl.ts index 8316f1f30e7..492320fce80 100644 --- a/public/app/features/dashboard/acl/acl.ts +++ b/public/app/features/dashboard/acl/acl.ts @@ -22,6 +22,8 @@ export class AclCtrl { dismiss: () => void; newType: string; canUpdate: boolean; + error: string; + readonly duplicateError = 'This permission exists already.'; /** @ngInject */ constructor(private backendSrv, private dashboardSrv, private $sce, private $scope) { @@ -111,6 +113,11 @@ export class AclCtrl { } addNewItem(item) { + if (!this.isValid(item)) { + return; + } + this.error = ''; + item.dashboardId = this.dashboard.id; this.items.push(this.prepareViewModel(item)); @@ -119,6 +126,23 @@ export class AclCtrl { this.canUpdate = true; } + isValid(item) { + const dupe = _.find(this.items, (it) => { return this.isDuplicate(it, item); }); + + if (dupe) { + this.error = this.duplicateError; + return false; + } + + return true; + } + + isDuplicate(origItem, newItem) { + return (origItem.role && newItem.role && origItem.role === newItem.role) || + (origItem.userId && newItem.userId && origItem.userId === newItem.userId) || + (origItem.userGroupId && newItem.userGroupId && origItem.userGroupId === newItem.userGroupId); + } + userPicked(user) { this.addNewItem({userId: user.id, userLogin: user.login, permission: 1,}); this.$scope.$broadcast('user-picker-reset'); diff --git a/public/app/features/dashboard/acl/specs/acl_specs.ts b/public/app/features/dashboard/acl/specs/acl_specs.ts index 24f3e3eb61c..99a5cfd2339 100644 --- a/public/app/features/dashboard/acl/specs/acl_specs.ts +++ b/public/app/features/dashboard/acl/specs/acl_specs.ts @@ -77,4 +77,74 @@ describe('AclCtrl', () => { expect(backendSrv.post.getCall(0).args[1].items[3].permission).to.eql(1); }); }); + + describe('when duplicate role permissions are added', () => { + beforeEach(() => { + backendSrv.get.reset(); + backendSrv.post.reset(); + ctx.ctrl.items = []; + + ctx.ctrl.newType = 'Editor'; + ctx.ctrl.typeChanged(); + + ctx.ctrl.newType = 'Editor'; + ctx.ctrl.typeChanged(); + }); + + it('should throw a validation error', () => { + expect(ctx.ctrl.error).to.eql(ctx.ctrl.duplicateError); + }); + + it('should not add the duplicate permission', () => { + expect(ctx.ctrl.items.length).to.eql(1); + }); + }); + + describe('when duplicate user permissions are added', () => { + beforeEach(() => { + backendSrv.get.reset(); + backendSrv.post.reset(); + ctx.ctrl.items = []; + + const userItem = { + id: 2, + login: 'user2', + }; + + ctx.ctrl.userPicked(userItem); + ctx.ctrl.userPicked(userItem); + }); + + it('should throw a validation error', () => { + expect(ctx.ctrl.error).to.eql(ctx.ctrl.duplicateError); + }); + + it('should not add the duplicate permission', () => { + expect(ctx.ctrl.items.length).to.eql(1); + }); + }); + + describe('when duplicate user group permissions are added', () => { + beforeEach(() => { + backendSrv.get.reset(); + backendSrv.post.reset(); + ctx.ctrl.items = []; + + const userGroupItem = { + id: 2, + name: 'ug1', + }; + + ctx.ctrl.groupPicked(userGroupItem); + ctx.ctrl.groupPicked(userGroupItem); + }); + + it('should throw a validation error', () => { + expect(ctx.ctrl.error).to.eql(ctx.ctrl.duplicateError); + }); + + it('should not add the duplicate permission', () => { + expect(ctx.ctrl.items.length).to.eql(1); + }); + }); }); From c6965ce6f49c6710c67361819e55723854be4fb7 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Fri, 23 Jun 2017 18:00:49 +0200 Subject: [PATCH 2/2] dashfolders: rename refactor --- public/app/core/routes/routes.ts | 2 +- .../partials/{edit_user_group.html => user_group_details.html} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename public/app/features/org/partials/{edit_user_group.html => user_group_details.html} (100%) diff --git a/public/app/core/routes/routes.ts b/public/app/core/routes/routes.ts index 50f76c6af79..0bf0a03bf7a 100644 --- a/public/app/core/routes/routes.ts +++ b/public/app/core/routes/routes.ts @@ -90,7 +90,7 @@ function setupAngularRoutes($routeProvider, $locationProvider) { resolve: loadOrgBundle, }) .when('/org/user-groups/edit/:id', { - templateUrl: 'public/app/features/org/partials/edit_user_group.html', + templateUrl: 'public/app/features/org/partials/user_group_details.html', controller : 'UserGroupDetailsCtrl', controllerAs: 'ctrl', resolve: loadOrgBundle, diff --git a/public/app/features/org/partials/edit_user_group.html b/public/app/features/org/partials/user_group_details.html similarity index 100% rename from public/app/features/org/partials/edit_user_group.html rename to public/app/features/org/partials/user_group_details.html