diff --git a/horizon/static/framework/util/actions/action-result.service.js b/horizon/static/framework/util/actions/action-result.service.js new file mode 100644 index 0000000000..c7da5fc1be --- /dev/null +++ b/horizon/static/framework/util/actions/action-result.service.js @@ -0,0 +1,147 @@ +/** + * (c) Copyright 2016 Hewlett Packard Enterprise Development Company LP + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use self 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. + */ + +(function() { + 'use strict'; + + angular + .module('horizon.framework.util.actions') + .factory('horizon.framework.util.actions.action-result.service', factory); + + /** + * @ngdoc factory + * @name factory + * @description + * The purpose of this service is to conveniently create meaningful return + * values from an action. For example, if you perform an action that deletes + * three items, it may be useful for the action to return information + * that indicates which three items were deleted. + * + * The ActionResult object allows an action's code to easily indicate what + * items were affected. + * + * For example, let's say our action deleted three items. We would + * resolve the action's promise by appending three 'deleted' items, then + * conclude by returning the bare result object. + * @example + ``` + return actionResultService.getActionResult() + .deleted('OS::Glance::Image', id1) + .deleted('OS::Glance::Image', id2) + .deleted('OS::Glance::Image', id3) + .result; + ``` + * As an example of how this is consumed, imagine a situation where there is + * a display with a list of instances, each having actions. A user performs + * one action, let's say Edit Instance; then after the action completes, the + * user's expectation is that the list of instances is reloaded. The + * controller that is managing that display needs to have a hook into the + * user's action. This is achieved through returning a promise from the + * initiation of the action. In the case of the actions directive, the + * promise is handled through assigning a result-handler in the table row + * markup: + ``` + + ``` + * The controller places a handler (above, ctrl.actionResultHandler) on this + * promise which, when the promise is resolved, analyzes that resolution + * to figure out logically what to do. We want to make this logic simple and + * also capable of handling 'unknown' actions; that is, we want to generically + * handle any action that a third-party could add. The action result + * feature provides this generic mechanism. The Edit Instance action would + * resolve with {updated: [{type: 'OS::Nova::Server', id: 'some-uuid'}]}, + * which then can be handled by the controller as required. In a controller: + ``` + ctrl.actionResultHandler = function resultHandler(returnValue) { + return $q.when(returnValue, actionSuccessHandler); + }; + + function actionSuccessHandler(result) { + // simple logic to just reload any time there are updated results. + if (result.updated.length > 0) { + reloadTheList(); + } + } + ``` + * This logic of course should probably be more fine-grained than the example, + * but this demonstrates the basics of how you use action promises and provide + * appropriate behaviors. + */ + function factory() { + + return { + getActionResult: getActionResult, + getIdsOfType: getIdsOfType + }; + + // Given a list of objects (items) that each have an 'id' property, + // return a list of those id values for objects whose 'type' property + // matches the 'type' parameter. + // This is a convenience method used for extracting IDs from action + // result objects. For example, if you wanted to know the IDs of + // the deleted images (but didn't want to know about other deleted types), + // you'd use this function. + function getIdsOfType(items, type) { + return items ? items.reduce(typeIdReduce, []) : []; + + function typeIdReduce(accumulator, item) { + if (item.type === type) { + accumulator.push(item.id); + } + return accumulator; + } + } + + function getActionResult() { + return new ActionResult(); + } + + function ActionResult() { + this.result = { + created: [], + updated: [], + deleted: [], + failed: [] + }; + this.created = created; + this.updated = updated; + this.deleted = deleted; + this.failed = failed; + + function created(type, id) { + this.result.created.push({type: type, id: id}); + return this; + } + + function updated(type, id) { + this.result.updated.push({type: type, id: id}); + return this; + } + + function deleted(type, id) { + this.result.deleted.push({type: type, id: id}); + return this; + } + + function failed(type, id) { + this.result.failed.push({type: type, id: id}); + return this; + } + } + + } +})(); diff --git a/horizon/static/framework/util/actions/action-result.service.spec.js b/horizon/static/framework/util/actions/action-result.service.spec.js new file mode 100644 index 0000000000..e5b1be7087 --- /dev/null +++ b/horizon/static/framework/util/actions/action-result.service.spec.js @@ -0,0 +1,87 @@ +/* + * (c) Copyright 2016 Hewlett Packard Enterprise Development Company LP + * + * 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. + */ + +(function() { + 'use strict'; + + describe('horizon.framework.util.actions.action-result.service', function() { + var service; + + beforeEach(module('horizon.framework.util.actions')); + + beforeEach(inject(function($injector) { + service = $injector.get('horizon.framework.util.actions.action-result.service'); + })); + + describe('getIdsOfType', function() { + + it('returns an empty array if no items', function() { + expect(service.getIdsOfType([], 'OS::Glance::Image')).toEqual([]); + }); + + it('returns an empty array if items is falsy', function() { + expect(service.getIdsOfType(false, 'OS::Glance::Image')).toEqual([]); + }); + + it('returns items with matching type', function() { + var items = [{type: 'No::Match'}, {type: 'OS::Glance::Image', id: 1}, + {type: 'OS::Glance::Image', id: 2}]; + expect(service.getIdsOfType(items, 'OS::Glance::Image')).toEqual([1, 2]); + }); + }); + + it('has getActionResult', function() { + expect(service.getActionResult).toBeDefined(); + }); + + describe('ActionResult', function() { + var actionResult; + var type = 'OS::Nova::Server'; + var id = 'the-id-value'; + + beforeEach(function() { + actionResult = service.getActionResult(); + }); + + it('.updated() adds updated items', function() { + actionResult.updated(type, id); + expect(actionResult.result.updated) + .toEqual([{type: 'OS::Nova::Server', id: 'the-id-value'}]); + }); + + it('.created() adds created items', function() { + actionResult.created(type, id); + expect(actionResult.result.created) + .toEqual([{type: 'OS::Nova::Server', id: 'the-id-value'}]); + }); + + it('.deleted() adds deleted items', function() { + actionResult.deleted(type, id); + expect(actionResult.result.deleted) + .toEqual([{type: 'OS::Nova::Server', id: 'the-id-value'}]); + }); + + it('.failed() adds failed items', function() { + actionResult.failed(type, id); + expect(actionResult.result.failed) + .toEqual([{type: 'OS::Nova::Server', id: 'the-id-value'}]); + }); + + }); + + }); + +})(); diff --git a/horizon/static/framework/util/actions/actions.module.js b/horizon/static/framework/util/actions/actions.module.js new file mode 100644 index 0000000000..90687a6908 --- /dev/null +++ b/horizon/static/framework/util/actions/actions.module.js @@ -0,0 +1,21 @@ +/* + * (c) Copyright 2016 Hewlett Packard Enterprise Development Company LP + * + * 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. + */ +(function() { + 'use strict'; + + angular.module('horizon.framework.util.actions', []); + +})(); diff --git a/horizon/static/framework/util/util.module.js b/horizon/static/framework/util/util.module.js index 5bf2dbe1ac..6d95727f44 100644 --- a/horizon/static/framework/util/util.module.js +++ b/horizon/static/framework/util/util.module.js @@ -3,6 +3,7 @@ angular .module('horizon.framework.util', [ + 'horizon.framework.util.actions', 'horizon.framework.util.bind-scope', 'horizon.framework.util.file', 'horizon.framework.util.filters', diff --git a/horizon/static/framework/widgets/modal/delete-modal.service.js b/horizon/static/framework/widgets/modal/delete-modal.service.js index 0cbdbfc03f..a13ed5cdb0 100644 --- a/horizon/static/framework/widgets/modal/delete-modal.service.js +++ b/horizon/static/framework/widgets/modal/delete-modal.service.js @@ -115,12 +115,8 @@ scope.$emit(context.failedEvent, failEntities.map(getId)); toast.add('error', getMessage(context.labels.error, failEntities)); } - - return { - // Object intentionally left blank. This data is passed to - // code that holds this action's promise. In the future, it may - // contain entity IDs and types that were modified by this action. - }; + // Return the passed and failed entities as part of resolving the promise + return result; } } diff --git a/openstack_dashboard/static/app/core/core.module.js b/openstack_dashboard/static/app/core/core.module.js index 61b06cd474..2656594b02 100644 --- a/openstack_dashboard/static/app/core/core.module.js +++ b/openstack_dashboard/static/app/core/core.module.js @@ -16,6 +16,9 @@ (function () { 'use strict'; + // Constants used within this file + var VOLUME_RESOURCE_TYPE = 'OS::Cinder::Volume'; + /** * @ngdoc overview * @name horizon.app.core @@ -38,6 +41,9 @@ 'horizon.framework.widgets', 'horizon.dashboard.project.workflow' ], config) + // NOTE: this will move into the correct module as that resource type + // becomes available. For now there is no volumes module. + .constant('horizon.app.core.volumes.resourceType', VOLUME_RESOURCE_TYPE) .run([ 'horizon.framework.conf.resource-type-registry.service', performRegistrations @@ -79,7 +85,7 @@ registry.getResourceType('OS::Cinder::Snapshot', { names: [gettext('Volume Snapshot'), gettext('Volume Snapshots')] }); - registry.getResourceType('OS::Cinder::Volume', { + registry.getResourceType(VOLUME_RESOURCE_TYPE, { names: [gettext('Volume'), gettext('Volumes')] }); registry.getResourceType('OS::Nova::Flavor', { diff --git a/openstack_dashboard/static/app/core/images/actions/create-volume.service.js b/openstack_dashboard/static/app/core/images/actions/create-volume.service.js index 139b988ab9..e6f751df48 100644 --- a/openstack_dashboard/static/app/core/images/actions/create-volume.service.js +++ b/openstack_dashboard/static/app/core/images/actions/create-volume.service.js @@ -27,9 +27,11 @@ 'horizon.app.core.openstack-service-api.serviceCatalog', 'horizon.app.core.images.workflows.create-volume.service', 'horizon.app.core.images.events', + 'horizon.framework.util.actions.action-result.service', 'horizon.framework.util.q.extensions', 'horizon.framework.widgets.modal.wizard-modal.service', - 'horizon.framework.widgets.toast.service' + 'horizon.framework.widgets.toast.service', + 'horizon.app.core.volumes.resourceType' ]; /** @@ -46,9 +48,11 @@ serviceCatalog, createVolumeWorkflowService, events, + actionResultService, $qExtensions, wizardModalService, - toast + toast, + volumeResourceType ) { var scope, createVolumePromise, volumeServiceEnabledPromise; var NON_BOOTABLE_IMAGE_TYPES = ['aki', 'ari']; @@ -105,11 +109,12 @@ function showSuccessMessage(response) { var volume = response.data; toast.add('success', interpolate(message.success, [volume.name])); - return { - // Object intentionally left blank. This data is passed to - // code that holds this action's promise. In the future, it may - // contain entity IDs and types that were modified by this action. - }; + + // To make the result of this action generically useful, reformat the return + // from the deleteModal into a standard form + return actionResultService.getActionResult() + .created(volumeResourceType, volume.id) + .result; } function imageBootable(image) { diff --git a/openstack_dashboard/static/app/core/images/actions/delete-image.service.js b/openstack_dashboard/static/app/core/images/actions/delete-image.service.js index c2e08a3b01..fbb1430d62 100644 --- a/openstack_dashboard/static/app/core/images/actions/delete-image.service.js +++ b/openstack_dashboard/static/app/core/images/actions/delete-image.service.js @@ -24,11 +24,12 @@ 'horizon.app.core.openstack-service-api.glance', 'horizon.app.core.openstack-service-api.userSession', 'horizon.app.core.openstack-service-api.policy', + '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.images.events' + 'horizon.app.core.images.resourceType' ]; /** @@ -46,11 +47,12 @@ glance, userSessionService, policy, + actionResultService, gettext, $qExtensions, deleteModal, toast, - events + imagesResourceType ) { var scope, context, deleteImagePromise; var notAllowedMessage = gettext("You are not allowed to delete images: %s"); @@ -67,7 +69,7 @@ function initScope(newScope) { scope = newScope; - context = { successEvent: events.DELETE_SUCCESS }; + context = { }; deleteImagePromise = policy.ifAllowed({rules: [['image', 'delete_image']]}); } @@ -105,11 +107,24 @@ outcome = $q.reject(result.fail); } if (result.pass.length > 0) { - outcome = deleteModal.open(scope, result.pass.map(getEntity), context); + outcome = deleteModal.open(scope, result.pass.map(getEntity), context).then(createResult); } return outcome; } + function createResult(deleteModalResult) { + // To make the result of this action generically useful, reformat the return + // from the deleteModal into a standard form + var actionResult = actionResultService.getActionResult(); + deleteModalResult.pass.forEach(function markDeleted(item) { + actionResult.deleted(imagesResourceType, getEntity(item).id); + }); + deleteModalResult.fail.forEach(function markFailed(item) { + actionResult.failed(imagesResourceType, getEntity(item).id); + }); + return actionResult.result; + } + function labelize(count) { return { diff --git a/openstack_dashboard/static/app/core/images/actions/delete-image.service.spec.js b/openstack_dashboard/static/app/core/images/actions/delete-image.service.spec.js index 8011cad530..35944dad63 100644 --- a/openstack_dashboard/static/app/core/images/actions/delete-image.service.spec.js +++ b/openstack_dashboard/static/app/core/images/actions/delete-image.service.spec.js @@ -19,7 +19,11 @@ var deleteModalService = { open: function () { - return; + deferredModal.resolve({ + pass: [{context: {id: 'a'}}], + fail: [{context: {id: 'b'}}] + }); + return deferredModal.promise; } }; @@ -46,7 +50,7 @@ } }; - var deferred, service, $scope; + var deferred, service, $scope, deferredModal; /////////////////////// @@ -70,6 +74,7 @@ $scope = _$rootScope_.$new(); service = $injector.get('horizon.app.core.images.actions.delete-image.service'); deferred = $q.defer(); + deferredModal = $q.defer(); })); function generateImage(imageCount) { @@ -95,7 +100,7 @@ describe('perform method', function() { beforeEach(function() { - spyOn(deleteModalService, 'open'); + spyOn(deleteModalService, 'open').and.callThrough(); service.initScope($scope, labelize); }); @@ -114,7 +119,6 @@ it('should open the delete modal and show correct labels', testSingleLabels); it('should open the delete modal and show correct labels', testpluralLabels); it('should open the delete modal with correct entities', testEntities); - it('should pass the success and error events to be thrown', testEvents); it('should only delete images that are valid', testValids); it('should fail if this project is not owner', testOwner); it('should fail if images is protected', testProtected); @@ -154,16 +158,6 @@ expect(entities.length).toEqual(imageCount); } - function testEvents() { - var images = generateImage(1); - service.perform(images); - $scope.$apply(); - - var context = deleteModalService.open.calls.argsFor(0)[2]; - expect(deleteModalService.open).toHaveBeenCalled(); - expect(context.successEvent).toEqual('horizon.app.core.images.DELETE_SUCCESS'); - } - function testValids() { var imageCount = 2; var images = generateImage(imageCount); diff --git a/openstack_dashboard/static/app/core/images/actions/edit.action.service.js b/openstack_dashboard/static/app/core/images/actions/edit.action.service.js index 641e315642..e6b3e1b9ad 100644 --- a/openstack_dashboard/static/app/core/images/actions/edit.action.service.js +++ b/openstack_dashboard/static/app/core/images/actions/edit.action.service.js @@ -21,12 +21,14 @@ editService.$inject = [ '$q', + 'horizon.app.core.images.resourceType', 'horizon.app.core.images.events', 'horizon.app.core.images.actions.editWorkflow', 'horizon.app.core.metadata.service', 'horizon.app.core.openstack-service-api.glance', 'horizon.app.core.openstack-service-api.policy', 'horizon.app.core.openstack-service-api.userSession', + 'horizon.framework.util.actions.action-result.service', 'horizon.framework.util.q.extensions', 'horizon.framework.widgets.modal.wizard-modal.service', 'horizon.framework.widgets.toast.service' @@ -39,12 +41,14 @@ */ function editService( $q, + imageResourceType, events, editWorkflow, metadataService, glance, policy, userSessionService, + actionResultService, $qExtensions, wizardModalService, toast @@ -53,7 +57,7 @@ success: gettext('Image %s was successfully updated.'), successMetadata: gettext('Image metadata %s was successfully updated.') }; - var modifyImagePolicyCheck, scope; + var modifyImagePolicyCheck, scope, saveDeferred; var model = { image: {}, @@ -70,31 +74,9 @@ ////////////// - // include this function in your service - // if you plan to emit events to the parent controller function initScope($scope) { - var watchImageChange = $scope.$on(events.IMAGE_CHANGED, onImageChange); - var watchMetadataChange = $scope.$on(events.IMAGE_METADATA_CHANGED, onMetadataChange); - scope = $scope; modifyImagePolicyCheck = policy.ifAllowed({rules: [['image', 'modify_image']]}); - - $scope.$on('$destroy', destroy); - - function destroy() { - watchImageChange(); - watchMetadataChange(); - } - } - - function onImageChange(e, image) { - model.image = image; - e.stopPropagation(); - } - - function onMetadataChange(e, metadata) { - model.metadata = metadata; - e.stopPropagation(); } function allowed(image) { @@ -115,11 +97,14 @@ model.image = localImage; } - return wizardModalService.modal({ + wizardModalService.modal({ scope: scope, workflow: editWorkflow, submit: submit - }).result; + }); + + saveDeferred = $q.defer(); + return saveDeferred.promise; } function submit() { @@ -137,15 +122,15 @@ function onUpdateImageSuccess() { toast.add('success', interpolate(message.success, [model.image.name])); - scope.$emit(events.UPDATE_SUCCESS, model.image); - return { - // This will be filled out with useful information as it is - // decided upon. - }; + saveDeferred.resolve(actionResultService.getActionResult() + .updated(imageResourceType, model.image.id) + .result); } function onUpdateImageFail() { - scope.$emit(events.UPDATE_SUCCESS, model.image); + saveDeferred.reject(actionResultService.getActionResult() + .failed(imageResourceType, model.image.id) + .result); } function saveMetadata() { diff --git a/openstack_dashboard/static/app/core/images/actions/edit.action.service.spec.js b/openstack_dashboard/static/app/core/images/actions/edit.action.service.spec.js index 576896d049..c0d2a9580f 100644 --- a/openstack_dashboard/static/app/core/images/actions/edit.action.service.spec.js +++ b/openstack_dashboard/static/app/core/images/actions/edit.action.service.spec.js @@ -75,7 +75,7 @@ } }; - var service, events, $scope, $q, toast, deferred, testImage, $timeout; + var service, $scope, $q, toast, deferred, testImage, $timeout; /////////////////////// @@ -94,7 +94,6 @@ $scope = _$rootScope_.$new(); $q = _$q_; service = $injector.get('horizon.app.core.images.actions.edit.service'); - events = $injector.get('horizon.app.core.images.events'); toast = $injector.get('horizon.framework.widgets.toast.service'); service.initScope($scope); deferred = $q.defer(); @@ -118,42 +117,8 @@ expect(modalArgs.workflow).toBeDefined(); }); - it('should update image in glance, update metadata and raise event', function() { - testImage = { name: 'Test', id: '2' }; - var newImage = { name: 'Test2', id: '2' }; - var newMetadata = {p1: '11', p3: '3'}; - - spyOn($scope, '$emit').and.callThrough(); - spyOn(glanceAPI, 'updateImage').and.callThrough(); - spyOn(metadataService, 'editMetadata').and.callThrough(); - spyOn(toast, 'add').and.callThrough(); - spyOn(wizardModalService, 'modal').and.callThrough(); - - service.initScope($scope); - service.perform(testImage); - $timeout.flush(); - - $scope.$emit(events.IMAGE_CHANGED, newImage); - $scope.$emit(events.IMAGE_METADATA_CHANGED, newMetadata); - - var modalArgs = wizardModalService.modal.calls.argsFor(0)[0]; - modalArgs.submit(); - $scope.$apply(); - - expect(glanceAPI.updateImage).toHaveBeenCalledWith(newImage); - expect(metadataService.editMetadata) - .toHaveBeenCalledWith('image', '2', newMetadata, ['p2']); - expect(toast.add) - .toHaveBeenCalledWith('success', 'Image Test2 was successfully updated.'); - expect(toast.add.calls.count()).toBe(2); - expect($scope.$emit) - .toHaveBeenCalledWith('horizon.app.core.images.UPDATE_SUCCESS', newImage); - }); - it('should raise event even if update meta data fails', function() { var image = { name: 'Test', id: '2' }; - var newImage = { name: 'Test2', id: '2' }; - var newMetadata = {prop1: '11', prop3: '3'}; var failedPromise = function() { return { @@ -166,51 +131,17 @@ spyOn(wizardModalService, 'modal').and.callThrough(); spyOn(glanceAPI, 'updateImage').and.callThrough(); spyOn(metadataService, 'editMetadata').and.callFake(failedPromise); - spyOn($scope, '$emit').and.callThrough(); spyOn(toast, 'add').and.callThrough(); service.initScope($scope); service.perform(image); $scope.$apply(); - $scope.$emit(events.IMAGE_CHANGED, newImage); - $scope.$emit(events.IMAGE_METADATA_CHANGED, newMetadata); - var modalArgs = wizardModalService.modal.calls.argsFor(0)[0]; modalArgs.submit(); $scope.$apply(); expect(toast.add.calls.count()).toBe(1); - expect($scope.$emit) - .toHaveBeenCalledWith('horizon.app.core.images.UPDATE_SUCCESS', newImage); - }); - - it('should destroy the event watchers', function() { - testImage = { name: 'Test', id: '2' }; - var newImage = { name: 'Test2', id: '2' }; - var newMetadata = {p1: '11', p3: '3'}; - - spyOn(wizardModalService, 'modal').and.callThrough(); - spyOn(glanceAPI, 'updateImage').and.callThrough(); - spyOn(metadataService, 'editMetadata').and.callThrough(); - spyOn(toast, 'add').and.callThrough(); - - service.initScope($scope); - service.perform(testImage); - $scope.$apply(); - - $scope.$emit('$destroy'); - $scope.$emit(events.IMAGE_CHANGED, newImage); - $scope.$emit(events.IMAGE_METADATA_CHANGED, newMetadata); - - var modalArgs = wizardModalService.modal.calls.argsFor(0)[0]; - modalArgs.submit(); - $scope.$apply(); - - expect(glanceAPI.updateImage).toHaveBeenCalledWith(testImage); - expect(metadataService.editMetadata) - .toHaveBeenCalledWith('image', testImage.id, {}, ['p1', 'p2']); - expect(toast.add.calls.count()).toBe(2); }); }); diff --git a/openstack_dashboard/static/app/core/images/actions/launch-instance.service.js b/openstack_dashboard/static/app/core/images/actions/launch-instance.service.js index e2edd5df0f..1d17fbfe99 100644 --- a/openstack_dashboard/static/app/core/images/actions/launch-instance.service.js +++ b/openstack_dashboard/static/app/core/images/actions/launch-instance.service.js @@ -52,8 +52,10 @@ ////////////// function perform(image) { + // Previous uses of this relocated the display using the successUrl; + // in this case we leave the post-action behavior up to the result + // handler. return launchInstanceModal.open({ - successUrl: '/project/instances', 'imageId': image.id }); } diff --git a/openstack_dashboard/static/app/core/images/actions/launch-instance.service.spec.js b/openstack_dashboard/static/app/core/images/actions/launch-instance.service.spec.js index abf86665d9..91f8dbab71 100644 --- a/openstack_dashboard/static/app/core/images/actions/launch-instance.service.spec.js +++ b/openstack_dashboard/static/app/core/images/actions/launch-instance.service.spec.js @@ -45,7 +45,6 @@ expect(launchInstanceModalMock.open).toHaveBeenCalled(); expect(launchInstanceModalMock.open.calls.argsFor(0)).toEqual([{ - successUrl: '/project/instances', imageId: '1' }]); }); diff --git a/openstack_dashboard/static/app/core/images/actions/update-metadata.action.service.js b/openstack_dashboard/static/app/core/images/actions/update-metadata.action.service.js index 02b7479d95..9e34e47912 100644 --- a/openstack_dashboard/static/app/core/images/actions/update-metadata.action.service.js +++ b/openstack_dashboard/static/app/core/images/actions/update-metadata.action.service.js @@ -25,7 +25,9 @@ 'horizon.app.core.images.events', 'horizon.app.core.metadata.modal.service', 'horizon.app.core.openstack-service-api.userSession', - 'horizon.framework.util.q.extensions' + 'horizon.framework.util.actions.action-result.service', + 'horizon.framework.util.q.extensions', + 'horizon.app.core.images.resourceType' ]; /** @@ -42,12 +44,12 @@ events, metadataModalService, userSessionService, - $qExtensions + actionResultService, + $qExtensions, + imageResourceType ) { - var scope; var service = { - initScope: initScope, perform: perform, allowed: allowed }; @@ -56,22 +58,17 @@ ////////////// - function initScope(newScope) { - scope = newScope; - } - function perform(image) { return metadataModalService.open('image', image.id) .result .then(onSuccess); function onSuccess() { - scope.$emit(events.UPDATE_METADATA_SUCCESS, [image.id]); - return { - // Object intentionally left blank. This data is passed to - // code that holds this action's promise. In the future, it may - // contain entity IDs and types that were modified by this action. - }; + // To make the result of this action generically useful, reformat the return + // from the deleteModal into a standard form + return actionResultService.getActionResult() + .updated(imageResourceType, image.id) + .result; } } diff --git a/openstack_dashboard/static/app/core/images/actions/update-metadata.action.service.spec.js b/openstack_dashboard/static/app/core/images/actions/update-metadata.action.service.spec.js index 62913ec136..ca15412a7c 100644 --- a/openstack_dashboard/static/app/core/images/actions/update-metadata.action.service.spec.js +++ b/openstack_dashboard/static/app/core/images/actions/update-metadata.action.service.spec.js @@ -57,24 +57,17 @@ } }; - spyOn($scope, '$emit').and.callThrough(); spyOn(metadataModalMock, 'open').and.returnValue(fakeModalService); - service.initScope($scope); service.perform({id: '1', name: 'image1'}); expect(metadataModalMock.open).toHaveBeenCalled(); expect(metadataModalMock.open.calls.argsFor(0)).toEqual(['image', '1']); - expect($scope.$emit).toHaveBeenCalledWith( - 'horizon.app.core.images.UPDATE_METADATA_SUCCESS', - ['1'] - ); }); describe('Update Metadata', function() { it('should allow Update Metadata if image can be deleted', function() { var image = {owner: 'project', status: 'active'}; - service.initScope($scope); permissionShouldPass(service.allowed(image)); $scope.$apply(); }); @@ -82,14 +75,12 @@ it('should not allow Update Metadata if service call is rejected', function() { var image = {owner: 'doesnt_matter', status: 'active'}; deferred.reject(); - service.initScope($scope); permissionShouldFail(service.allowed(image)); $scope.$apply(); }); it('should not allow Update Metadata if image status is not active', function() { var image = {owner: 'project', status: 'not_active'}; - service.initScope($scope); permissionShouldFail(service.allowed(image)); $scope.$apply(); }); diff --git a/openstack_dashboard/static/app/core/images/table/images-table.html b/openstack_dashboard/static/app/core/images/table/images-table.html index e9a93752f7..c097cc6e65 100644 --- a/openstack_dashboard/static/app/core/images/table/images-table.html +++ b/openstack_dashboard/static/app/core/images/table/images-table.html @@ -13,7 +13,8 @@ --> - + @@ -55,7 +56,7 @@ Include action-col if you want to perform actions. rsp-p1 rsp-p2 are responsive priority as user resizes window. --> - +
@@ -85,7 +86,8 @@ Table-row-action-column: Actions taken here applies to a single item/row. --> - + diff --git a/openstack_dashboard/static/app/core/images/table/images.controller.js b/openstack_dashboard/static/app/core/images/table/images.controller.js index 2338c416d2..a1a43052b6 100644 --- a/openstack_dashboard/static/app/core/images/table/images.controller.js +++ b/openstack_dashboard/static/app/core/images/table/images.controller.js @@ -30,6 +30,7 @@ 'horizon.app.core.openstack-service-api.glance', 'horizon.app.core.openstack-service-api.userSession', 'horizon.framework.conf.resource-type-registry.service', + 'horizon.framework.util.actions.action-result.service', 'imageVisibilityFilter' ]; @@ -50,6 +51,7 @@ glance, userSession, typeRegistry, + actionResultService, imageVisibilityFilter ) { var ctrl = this; @@ -58,23 +60,19 @@ ctrl.checked = {}; - ctrl.images = []; - ctrl.imagesSrc = []; ctrl.metadataDefs = null; ctrl.imageResourceType = typeRegistry.getResourceType(imageResourceType); + ctrl.actionResultHandler = actionResultHandler; - var deleteWatcher = $scope.$on(events.DELETE_SUCCESS, onDeleteSuccess); - var updateWatcher = $scope.$on(events.UPDATE_SUCCESS, onUpdateSuccess); - - $scope.$on('$destroy', destroy); - - init(); + typeRegistry.initActions(imageResourceType, $scope); + loadImages(); //////////////////////////////// - function init() { - typeRegistry.initActions(imageResourceType, $scope); + function loadImages() { + ctrl.images = []; + ctrl.imagesSrc = []; $q.all( { images: glance.getImages(), @@ -96,21 +94,6 @@ applyMetadataDefinitions(); } - function onUpdateSuccess(e, image) { - e.stopPropagation(); - ctrl.imagesSrc = difference(ctrl.imagesSrc, [image.id], 'id'); - ctrl.imagesSrc.push(image); - } - - function onDeleteSuccess(e, removedImageIds) { - ctrl.imagesSrc = difference(ctrl.imagesSrc, removedImageIds, 'id'); - e.stopPropagation(); - - // after deleting the items - // we need to clear selected items from table controller - $scope.$emit('hzTable:clearSelected'); - } - function difference(currentList, otherList, key) { return currentList.filter(filter); @@ -121,11 +104,6 @@ } } - function destroy() { - updateWatcher(); - deleteWatcher(); - } - function applyMetadataDefinitions() { glance.getNamespaces({resource_type: imageResourceType}, true) .then(function setMetadefs(data) { @@ -133,6 +111,56 @@ }); } + function actionResultHandler(returnValue) { + return $q.when(returnValue, actionSuccessHandler); + } + + function actionSuccessHandler(result) { // eslint-disable-line no-unused-vars + + // The action has completed (for whatever "complete" means to that + // action. Notice the view doesn't really need to know the semantics of the + // particular action because the actions return data in a standard form. + // That return includes the id and type of each created, updated, deleted + // and failed item. + // + // This handler is also careful to check the type of each item. This + // is important because actions which create non-images are launched from + // the images page (like create "volume" from image). + var deletedIds, updatedIds, createdIds, failedIds; + + if ( result ) { + // Reduce the results to just image ids ignoring other types the action + // may have produced + deletedIds = actionResultService.getIdsOfType(result.deleted, imageResourceType); + updatedIds = actionResultService.getIdsOfType(result.updated, imageResourceType); + createdIds = actionResultService.getIdsOfType(result.created, imageResourceType); + failedIds = actionResultService.getIdsOfType(result.failed, imageResourceType); + + // Handle deleted images + if (deletedIds.length) { + ctrl.imagesSrc = difference(ctrl.imagesSrc, deletedIds,'id'); + } + + // Handle updated and created images + if ( updatedIds.length || createdIds.length ) { + // Ideally, get each created image individually, but + // this is simple and robust for the common use case. + // TODO: If we want more detailed updates, we could do so here. + loadImages(); + } + + // Handle failed images + if ( failedIds.length ) { + // Do nothing for now + } + + } else { + // promise resolved, but no result returned. Because the action didn't + // tell us what happened...reload the displayed images just in case. + loadImages(); + } + } + } })(); diff --git a/openstack_dashboard/static/app/core/images/table/images.controller.spec.js b/openstack_dashboard/static/app/core/images/table/images.controller.spec.js index 9200dc5747..dca9cc8c25 100644 --- a/openstack_dashboard/static/app/core/images/table/images.controller.spec.js +++ b/openstack_dashboard/static/app/core/images/table/images.controller.spec.js @@ -57,6 +57,9 @@ callback(input); } }; + }, + when: function (input, callback) { + return callback(input); } }; @@ -65,7 +68,7 @@ 2: {id: '2', is_public: false, owner: 'not_me', filtered_visibility: 'Shared with Me'} }; - var $scope, controller, events, detailsRoute; + var $scope, controller, detailsRoute; beforeEach(module('ui.bootstrap')); beforeEach(module('horizon.framework')); @@ -85,7 +88,6 @@ beforeEach(inject(function ($injector, _$rootScope_) { $scope = _$rootScope_.$new(); - events = $injector.get('horizon.app.core.images.events'); controller = $injector.get('$controller'); detailsRoute = $injector.get('horizon.app.core.images.detailsRoute'); @@ -121,37 +123,62 @@ expect(glanceAPI.getNamespaces).toHaveBeenCalled(); }); - it('should refresh images after delete', function() { + it('re-queries if no result', function() { + var ctrl = createController(); + glanceAPI.getImages.calls.reset(); + ctrl.actionResultHandler(); + expect(glanceAPI.getImages).toHaveBeenCalled(); + }); + + it('re-queries if updated', function() { + var ctrl = createController(); + glanceAPI.getImages.calls.reset(); + ctrl.actionResultHandler({updated: [{type: 'OS::Glance::Image', id: 'b'}]}); + expect(glanceAPI.getImages).toHaveBeenCalled(); + }); + + it('re-queries if created', function() { + var ctrl = createController(); + glanceAPI.getImages.calls.reset(); + ctrl.actionResultHandler({created: [{type: 'OS::Glance::Image', id: 'b'}]}); + expect(glanceAPI.getImages).toHaveBeenCalled(); + }); + + it('does not re-query if only failed', function() { + var ctrl = createController(); + glanceAPI.getImages.calls.reset(); + ctrl.actionResultHandler({failed: [{type: 'OS::Glance::Image', id: 'b'}]}); + expect(glanceAPI.getImages).not.toHaveBeenCalled(); + }); + + it('should remove deleted images', function() { var ctrl = createController(); expect(ctrl.imagesSrc).toEqual([ expectedImages['1'], expectedImages['2'] ]); - spyOn($scope, '$emit').and.callThrough(); - $scope.$emit(events.DELETE_SUCCESS, ['1']); + var result = { + deleted: [ {type: "OS::Glance::Image", id: '1'} ] + }; + ctrl.actionResultHandler(result); expect(ctrl.imagesSrc).toEqual([ expectedImages['2'] ]); - - expect($scope.$emit).toHaveBeenCalledWith('hzTable:clearSelected'); }); - it('should refresh images after update', function() { + it('should not remove deleted volumes', function() { var ctrl = createController(); - expect(ctrl.imagesSrc).toEqual(images); + expect(ctrl.imagesSrc).toEqual([ + expectedImages['1'], + expectedImages['2'] + ]); - $scope.$emit(events.UPDATE_SUCCESS, {id: '1', name: 'name_new'}); - - expect(ctrl.imagesSrc.filter(function (x) { return x.id === '1'; })[0].name).toBe('name_new'); - }); - - it('should destroy the event watcher for delete', function() { - var ctrl = createController(); - - $scope.$emit('$destroy'); - $scope.$emit(events.DELETE_SUCCESS, ['1']); + var result = { + deleted: [ {type: "OS::Cinder::Values", id: '1'} ] + }; + ctrl.actionResultHandler(result); expect(ctrl.imagesSrc).toEqual([ expectedImages['1'], @@ -159,23 +186,5 @@ ]); }); - it('should destroy the event watcher for update', function() { - var ctrl = createController(); - - $scope.$emit('$destroy'); - $scope.$emit(events.UPDATE_SUCCESS, {id: '1', name: 'name_new'}); - - expect(ctrl.imagesSrc).toEqual(images); - }); - - it('should destroy the event watcher for create', function() { - var ctrl = createController(); - - $scope.$emit('$destroy'); - $scope.$emit(events.createSuccess, {id: '3'}); - - expect(ctrl.imagesSrc).toEqual(images); - }); - }); })(); diff --git a/releasenotes/notes/action-results-303c282165b60f47.yaml b/releasenotes/notes/action-results-303c282165b60f47.yaml new file mode 100644 index 0000000000..f33fed94e3 --- /dev/null +++ b/releasenotes/notes/action-results-303c282165b60f47.yaml @@ -0,0 +1,12 @@ +--- +prelude: > + Angular actions now should return a promise + that resolves with an object structured in a way + to indicate what the action did (or didn't do). +features: + - An action-result service provides convenience methods + for construction of the result, and for parsing of + a resolved object +upgrade: + - Although it's not required, it's best to make your + actions return promises with the expected structure.