From 7b207fbf89b97a8bc9be3469a2a4a442f6c2c2e2 Mon Sep 17 00:00:00 2001 From: Lajos Katona Date: Mon, 14 Aug 2017 14:59:12 +0200 Subject: [PATCH] Remove initScope from trunk delete.action.service The initScope method is deprecated, the action service should be stateless. Also refactor to simplify delete action logic. Some code got carried over from the images panel, what we originally used as an example. But much of that code was doing checks useless for trunks. Trunks don't have complicated 'public' and 'shared' attributes controlling who can operate on them. A simple policy check will suffice. Co-Authored-By: Bence Romsics Change-Id: I689a98697d997780af42eb31a4b5eeee2ddf9b0f Partially-Implements: blueprint neutron-trunk-ui Related-Bug: #1640049 --- .../trunks/actions/delete.action.service.js | 124 ++++----- .../actions/delete.action.service.spec.js | 245 ++++++++---------- 2 files changed, 154 insertions(+), 215 deletions(-) diff --git a/openstack_dashboard/static/app/core/trunks/actions/delete.action.service.js b/openstack_dashboard/static/app/core/trunks/actions/delete.action.service.js index e5d39dfa10..076818b006 100644 --- a/openstack_dashboard/static/app/core/trunks/actions/delete.action.service.js +++ b/openstack_dashboard/static/app/core/trunks/actions/delete.action.service.js @@ -1,17 +1,17 @@ /* * Copyright 2017 Ericsson * - * Licensed under the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. You may obtain - * a copy of the License at + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations - * under the License. + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. */ (function() { @@ -19,104 +19,70 @@ angular .module('horizon.app.core.trunks') - .factory( - 'horizon.app.core.trunks.actions.delete.service', - deleteTrunkService - ); + .factory('horizon.app.core.trunks.actions.delete.service', deleteService); - deleteTrunkService.$inject = [ + deleteService.$inject = [ '$q', 'horizon.app.core.openstack-service-api.neutron', - 'horizon.app.core.openstack-service-api.userSession', 'horizon.app.core.openstack-service-api.policy', + 'horizon.app.core.trunks.resourceType', 'horizon.framework.util.actions.action-result.service', - 'horizon.framework.util.i18n.gettext', - 'horizon.framework.util.q.extensions', - 'horizon.framework.widgets.modal.deleteModalService', - 'horizon.framework.widgets.toast.service', - 'horizon.app.core.trunks.resourceType' + 'horizon.framework.widgets.modal.deleteModalService' ]; - function deleteTrunkService( + function deleteService( $q, neutron, - userSessionService, policy, + resourceType, actionResultService, - gettext, - $qExtensions, - deleteModal, - toast, - trunkResourceType + deleteModal ) { - var scope, context, deleteTrunkPromise; - var service = { - initScope: initScope, allowed: allowed, perform: perform }; return service; - function initScope(newScope) { - scope = newScope; - context = {}; - deleteTrunkPromise = policy.ifAllowed( - {rules: [['trunk', 'delete_trunk']]}); + //////////// + + function allowed() { + return policy.ifAllowed( + {rules: [ + ['network', 'delete_trunk'] + ]} + ); } - function perform(items) { - var Trunks = angular.isArray(items) ? items : [items]; - context.labels = labelize(Trunks.length); - context.deleteEntity = neutron.deleteTrunk; - return $qExtensions.allSettled(Trunks.map(checkPermission)) - .then(afterCheck); - } + function perform(items, scope) { + var trunks = angular.isArray(items) ? items : [items]; - function allowed(trunk) { - if (trunk) { - return $q.all([ - deleteTrunkPromise, - userSessionService.isCurrentProject(trunk.project_id) - ]); - } else { - return deleteTrunkPromise; + return openModal().then(onComplete); + + function openModal() { + return deleteModal.open( + scope, + trunks, + { + labels: labelize(trunks.length), + deleteEntity: neutron.deleteTrunk + } + ); } - } - function checkPermission(trunk) { - return {promise: allowed(trunk), context: trunk}; - } + function onComplete(result) { + var actionResult = actionResultService.getActionResult(); - function afterCheck(result) { - var outcome = $q.reject(); - if (result.fail.length > 0) { - var msg = interpolate( - gettext("You are not allowed to delete trunks: %s"), - [result.fail.map(function (x) {return x.context.name;}).join(", ")]); - toast.add('error', msg, result.fail); - outcome = $q.reject(result.fail); + result.pass.forEach(function markDeleted(item) { + actionResult.deleted(resourceType, item.context.id); + }); + result.fail.forEach(function markFailed(item) { + actionResult.failed(resourceType, item.context.id); + }); + + return actionResult.result; } - if (result.pass.length > 0) { - outcome = deleteModal.open( - scope, - result.pass.map(function (x) {return x.context;}), - context) - .then(createResult); - } - return outcome; - } - - function createResult(deleteModalResult) { - var actionResult = actionResultService.getActionResult(); - deleteModalResult.pass.forEach(function markDeleted(item) { - actionResult.deleted(trunkResourceType, item.context.id); - }); - deleteModalResult.fail.forEach(function markFailed(item) { - actionResult.failed(trunkResourceType, item.context.id); - }); - return actionResult.result; } function labelize(count) { diff --git a/openstack_dashboard/static/app/core/trunks/actions/delete.action.service.spec.js b/openstack_dashboard/static/app/core/trunks/actions/delete.action.service.spec.js index 2e71e9ed00..5d8aa997ce 100644 --- a/openstack_dashboard/static/app/core/trunks/actions/delete.action.service.spec.js +++ b/openstack_dashboard/static/app/core/trunks/actions/delete.action.service.spec.js @@ -1,17 +1,17 @@ /* * Copyright 2017 Ericsson * - * Licensed under the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. You may obtain - * a copy of the License at + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations - * under the License. + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. */ (function() { @@ -40,21 +40,17 @@ return { success: function(callback) { callback({allowed: true}); + }, + failure: function(callback) { + callback({allowed: false}); } }; } }; - var userSession = { - isCurrentProject: function() { - deferred.resolve(); - return deferred.promise; - } - }; - var deferred, service, $scope, deferredModal; - /////////////////////// + //////////// beforeEach(module('horizon.app.core')); beforeEach(module('horizon.app.core.trunks')); @@ -67,9 +63,7 @@ beforeEach(module('horizon.app.core.openstack-service-api', function($provide) { $provide.value('horizon.app.core.openstack-service-api.neutron', neutronAPI); $provide.value('horizon.app.core.openstack-service-api.policy', policyAPI); - $provide.value('horizon.app.core.openstack-service-api.userSession', userSession); spyOn(policyAPI, 'ifAllowed').and.callThrough(); - spyOn(userSession, 'isCurrentProject').and.callThrough(); })); beforeEach(inject(function($injector, _$rootScope_, $q) { @@ -101,113 +95,95 @@ beforeEach(function() { spyOn(deleteModalService, 'open').and.callThrough(); - service.initScope($scope, labelize); }); - function labelize(count) { - return { - title: ngettext('title', 'titles', count), - message: ngettext('message', 'messages', count), - submit: ngettext('submit', 'submits', count), - success: ngettext('success', 'successes', count), - error: ngettext('error', 'errors', count) - }; - } - //////////// - it('should open the delete modal and show correct singular labels', testSingleLabels); - it('should open the delete modal and show correct labels, one object', testSingleObject); - it('should open the delete modal and show correct plural labels', testpluralLabels); - it('should open the delete modal with correct entities', testEntities); - it('should only delete trunks that are valid', testValids); - it('should fail if this project is not owner', testOwner); - it('should pass in a function that deletes a trunk', testNeutron); + it('should open the delete modal and show correct labels, one object', + function testSingleObject() { + var trunks = generateTrunk(1); + service.perform(trunks[0]); + $scope.$apply(); - //////////// + var labels = deleteModalService.open.calls.argsFor(0)[2].labels; + expect(deleteModalService.open).toHaveBeenCalled(); + angular.forEach(labels, function eachLabel(label) { + expect(label.toLowerCase()).toContain('trunk'); + }); + } + ); - function testSingleObject() { - var trunks = generateTrunk(1); - service.perform(trunks[0]); - $scope.$apply(); + it('should open the delete modal and show correct singular labels', + function testSingleLabels() { + var trunks = generateTrunk(1); + service.perform(trunks); + $scope.$apply(); - var labels = deleteModalService.open.calls.argsFor(0)[2].labels; - expect(deleteModalService.open).toHaveBeenCalled(); - angular.forEach(labels, function eachLabel(label) { - expect(label.toLowerCase()).toContain('trunk'); - }); - } + var labels = deleteModalService.open.calls.argsFor(0)[2].labels; + expect(deleteModalService.open).toHaveBeenCalled(); + angular.forEach(labels, function eachLabel(label) { + expect(label.toLowerCase()).toContain('trunk'); + }); + } + ); - function testSingleLabels() { - var trunks = generateTrunk(1); - service.perform(trunks); - $scope.$apply(); + it('should open the delete modal and show correct plural labels', + function testpluralLabels() { + var trunks = generateTrunk(2); + service.perform(trunks); + $scope.$apply(); - var labels = deleteModalService.open.calls.argsFor(0)[2].labels; - expect(deleteModalService.open).toHaveBeenCalled(); - angular.forEach(labels, function eachLabel(label) { - expect(label.toLowerCase()).toContain('trunk'); - }); - } + var labels = deleteModalService.open.calls.argsFor(0)[2].labels; + expect(deleteModalService.open).toHaveBeenCalled(); + angular.forEach(labels, function eachLabel(label) { + expect(label.toLowerCase()).toContain('trunks'); + }); + } + ); - function testpluralLabels() { - var trunks = generateTrunk(2); - service.perform(trunks); - $scope.$apply(); + it('should open the delete modal with correct entities', + function testEntities() { + var count = 3; + var trunks = generateTrunk(count); + service.perform(trunks); + $scope.$apply(); - var labels = deleteModalService.open.calls.argsFor(0)[2].labels; - expect(deleteModalService.open).toHaveBeenCalled(); - angular.forEach(labels, function eachLabel(label) { - expect(label.toLowerCase()).toContain('trunks'); - }); - } + var entities = deleteModalService.open.calls.argsFor(0)[1]; + expect(deleteModalService.open).toHaveBeenCalled(); + expect(entities.length).toEqual(count); + } + ); - function testEntities() { - var count = 3; - var trunks = generateTrunk(count); - service.perform(trunks); - $scope.$apply(); + it('should only delete trunks that are valid', + function testValids() { + var count = 2; + var trunks = generateTrunk(count); + service.perform(trunks); + $scope.$apply(); - var entities = deleteModalService.open.calls.argsFor(0)[1]; - expect(deleteModalService.open).toHaveBeenCalled(); - expect(entities.length).toEqual(count); - } + var entities = deleteModalService.open.calls.argsFor(0)[1]; + expect(deleteModalService.open).toHaveBeenCalled(); + expect(entities.length).toBe(count); + expect(entities[0].name).toEqual('trunk0'); + expect(entities[1].name).toEqual('trunk1'); + } + ); - function testValids() { - var count = 2; - var trunks = generateTrunk(count); - service.perform(trunks); - $scope.$apply(); + it('should pass in a function that deletes a trunk', + function testNeutron() { + spyOn(neutronAPI, 'deleteTrunk'); + var count = 1; + var trunks = generateTrunk(count); + var trunk = trunks[0]; + service.perform(trunks); + $scope.$apply(); - var entities = deleteModalService.open.calls.argsFor(0)[1]; - expect(deleteModalService.open).toHaveBeenCalled(); - expect(entities.length).toBe(count); - expect(entities[0].name).toEqual('trunk0'); - expect(entities[1].name).toEqual('trunk1'); - } - - function testOwner() { - var trunks = generateTrunk(1); - deferred.reject(); - service.perform(trunks); - $scope.$apply(); - - expect(deleteModalService.open).not.toHaveBeenCalled(); - } - - function testNeutron() { - spyOn(neutronAPI, 'deleteTrunk'); - var count = 1; - var trunks = generateTrunk(count); - var trunk = trunks[0]; - service.perform(trunks); - $scope.$apply(); - - var contextArg = deleteModalService.open.calls.argsFor(0)[2]; - var deleteFunction = contextArg.deleteEntity; - deleteFunction(trunk.id); - expect(neutronAPI.deleteTrunk).toHaveBeenCalledWith(trunk.id); - } + var contextArg = deleteModalService.open.calls.argsFor(0)[2]; + var deleteFunction = contextArg.deleteEntity; + deleteFunction(trunk.id); + expect(neutronAPI.deleteTrunk).toHaveBeenCalledWith(trunk.id); + } + ); }); // end of delete modal @@ -221,39 +197,36 @@ beforeEach(function() { spyOn(resolver, 'success'); spyOn(resolver, 'error'); - service.initScope($scope); }); //////////// - it('should use default policy if batch action', testBatch); - it('allows delete if trunk can be deleted', testValid); - it('disallows delete if trunk is not owned by user', testOwner); + it('should use default policy if batch action', + function testBatch() { + service.allowed(); + $scope.$apply(); + expect(policyAPI.ifAllowed).toHaveBeenCalled(); + expect(resolver.success).not.toHaveBeenCalled(); + expect(resolver.error).not.toHaveBeenCalled(); + } + ); - //////////// + it('allows delete if trunk can be deleted', + function testValid() { + service.allowed().success(resolver.success); + $scope.$apply(); + expect(resolver.success).toHaveBeenCalled(); + } + ); - function testBatch() { - service.allowed(); - $scope.$apply(); - expect(policyAPI.ifAllowed).toHaveBeenCalled(); - expect(resolver.success).not.toHaveBeenCalled(); - expect(resolver.error).not.toHaveBeenCalled(); - } - - function testValid() { - var trunk = generateTrunk(1)[0]; - service.allowed(trunk).then(resolver.success, resolver.error); - $scope.$apply(); - expect(resolver.success).toHaveBeenCalled(); - } - - function testOwner() { - var trunk = generateTrunk(1)[0]; - deferred.reject(); - service.allowed(trunk).then(resolver.success, resolver.error); - $scope.$apply(); - expect(resolver.error).toHaveBeenCalled(); - } + it('disallows delete if trunk is not owned by user', + function testOwner() { + deferred.reject(); + service.allowed().failure(resolver.error); + $scope.$apply(); + expect(resolver.error).toHaveBeenCalled(); + } + ); }); // end of allow method