Images tables uses action promises

This patch demonstrates how promises returned by actions are
used by the images table. In particular, notice how specific
action event handlers do not need to be registered. This
allows a view to support a wide variety of actions without
needing to setup a specific event listener for each type of
event the action might emit.

One could argue that if action events were standardized, they
could still be used, which is true. Consider a generic SUCCESS
event. However, promises have additional benefits and avoid
some of the problems of events.

One advantage is that promises make it easy to "chain" multiple
handlers to a single success or failure. For example, once ALL
of these actions have completed successfully, then update a status
icon. We see another example of this in the delete-action.service
which uses a promise chain to convert the data returned by the
delete-modal.service into a standardized form used by actions.

Also, promises don't require that the caller have a parent scope.
In angular, events "bubble" from child scope to the parent
scope. These scopes are effectively the view hierarchy, where
a panel contains a table, which contains rows, which contain
buttons. However, actions are not view elements lke a table or
input box. Actions, like "delete image" are a behavior
that may be invoked independently, and the code that cares about
the success or failure of that action may not be a scope parent.

Added an action-result service that provides a convenience object
for creating such results.

Co-Authored-By: Matt Borland <matt.borland@hpe.com>

Partially-Implements: blueprint angular-registry
Change-Id: Id6725664e5654a4f75508993b9640a0de80c6884

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:
+   ```
+   <actions allowed="ctrl.itemActions" type="row" item="currInstance"
+            result-handler="ctrl.actionResultHandler"></actions>
+   ```
+   * 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;
+      }
+    }
+
+  }
+})();

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'}]);
+      });
+
+    });
+
+  });
+
+})();

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', []);
+
+})();

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',

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;
       }
     }
 

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', {

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) {

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 {
 

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);

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() {

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);
       });
     });
 

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
       });
     }

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'
       }]);
     });

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;
       }
     }
 

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();
       });

openstack_dashboard/static/app/core/images/table/images-table.html

@@ -13,7 +13,8 @@
       -->
       <th colspan="100" class="search-header">
         <hz-search-bar group-classes="input-group" icon-classes="fa-search">
-          <actions allowed="table.imageResourceType.batchActions" type="batch"></actions>
+          <actions allowed="table.imageResourceType.batchActions" type="batch"
+              result-handler="table.actionResultHandler"></actions>
         </hz-search-bar>
       </th>
     </tr>
@@ -55,7 +56,7 @@
       Include action-col if you want to perform actions.
       rsp-p1 rsp-p2 are responsive priority as user resizes window.
     -->
-    <tr ng-repeat-start="image in table.images track by image.id">
+    <tr ng-repeat-start="image in table.images">
 
       <td class="multi_select_column">
         <div class="themable-checkbox">
@@ -85,7 +86,8 @@
         Table-row-action-column:
         Actions taken here applies to a single item/row.
         -->
-        <actions allowed="table.imageResourceType.itemActions" type="row" item="image">
+        <actions allowed="table.imageResourceType.itemActions" type="row" item="image"
+            result-handler="table.actionResultHandler">
         </actions>
       </td>
     </tr>

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();
+      }
+    }
+
   }
 
 })();

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);
-    });
-
   });
 })();

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.