Allow specifying item to use for actions in the actions directive

Currently, the item passed to the 'allowed-actions' is aware of
what the HTML referred to. This patch allows for passing in the
item to use in the actions directive. This removes
the need to pass in the item key as part of the
allowed actions list thus removing the implicit binding
of the controller/service/html.

This also changes the list type attribute for the directive
from the action-list-type to type for readability.

Co-Authored-By: Kyle Olivo<keolivo@thoughtworks.com>

Partially-Implements: blueprint angularize-images-table
Change-Id: Idc344153aa7582c485a5d9c6b3cccbf8ff788d7c

horizon/static/framework/widgets/action-list/actions.batch.mock.html

@@ -1,2 +1,2 @@
-<actions allowed-actions="actions" action-list-type="batch">
+<actions allowed="actions" type="batch">
 </actions>

horizon/static/framework/widgets/action-list/actions.directive.js

@@ -35,71 +35,111 @@
    *
    * Attributes:
    *
-   * allowedActions: actions allowed that can be displayed
+   * @param {string} type
-   * actionListType: allow the buttons to be shown as a list or doropdown
+   * Type can be only be 'row' or 'batch'.
+   * 'batch' actions are rendered as a button group, 'row' is rendered as a button dropdown menu.
+   * 'batch' actions are typically used for actions across multiple items while
+   * 'row' actions are used per item.
    *
-   * `allowedActions` is a list of allowed actions on the service.
+   * @param {string=} item
-   * It's an array of objects of the form:
+   * The item to pass to the callback when using 'row' type.
-   * { template: {}, permissions: <promise to determine permissions>, callback: 'callback'}
+   * The variable is evaluated and passed as an argument when evaluating 'allowed'.
+   * 'item' is not used when row type is 'batch'.
    *
-   * `template` is an object that can be
+   * @param {function} allowed
+   * Returns an array of actions that can be performed on the item(s).
+   * When using 'row' type, the current 'item' will be passed to the function.
+   * When using 'batch' type, no arguments are provided.
    *
-   * {url: 'template.html'} the location of the template for the action button.
+   * This is an array that should contain objects with the following properties:
+   * {
+   *   template: <template object - described below>,
+   *   permissions: <a promise to determine if action is allowed>,
+   *   callback: 'callback for the action'
+   * }
+   *
+   *   template: is an object that can be any of
+   *   1. url: <full_path_to_template.html>
+   *      This specifies the location of the template for the action button.
    *      Use this for complete extensibility and control over what is rendered.
-   * The template will be responsible for binding the callback and styling.
+   *      The template will be responsible for binding the callback and styling the button.
+   *
+   *   2. type: '<action_button_type>'
+   *      This uses a known action button type.
+   *      Currently supported values are
+   *      1. 'delete' - Delete a single row. Only for 'row' type.
+   *      2. 'delete-selected' - Delete multiple rows. Only for 'batch' type.
+   *      3. 'create' - Create a new entity. Only for 'batch' type.
    *
-   * {type: 'type', item: 'item'} use a known action button type.
-   * Currently supported values are 'delete', 'delete-selected' and 'create'.
-   * `item` is optional and if provided will be used in the callback.
    *      The styling and binding of the callback is done by the template.
    *
-   * {text: 'text', item: 'item'} use an unstyled button with given text.
+   *   3. text: 'text', actionClasses: 'custom-classes'
-   * `item` is optional and if provided will be used in the callback.
+   *      This creates a button with the given text.
-   * The styling of the button will vary per the `actionListType` chosen.
+   *      For custom styling of the button, `actionClasses` can be optionally included.
-   * For custom styling of the button, `actionClasses` can be included in
-   * the template.
    *
-   * `permissions` is expected to be a promise that resolves
+   *   permissions: is expected to be a promise that resolves
-   * if permitted and is rejected if not.
+   *   if the action is permitted and is rejected if not. If there are multiple promises that
-   * `callback` is the method to call when the button is clicked.
+   *   need to be resolved, you can $q.all to combine multiple promises into a single promise.
    *
-   * `actionListType` can be only be `row` or `batch`
+   *   callback: is the method to call when the button is clicked.
-   * `batch` is rendered as buttons, `row` is rendered as dropdown menu.
+   *   When using 'row' type, the current 'item' is evaluated and passed to the function.
+   *   When using 'batch' type, 'item' is not passed.
+   *   When using 'delete-selected' for 'batch' type, all selected rows are passed.
    *
    * @restrict E
    * @scope
    * @example
    *
-   * $scope.actions = [{
+   * batch:
+   *
+   * function actions() {
+   *   return [{
+   *     callback: 'table.batchActions.delete.open',
+   *     template: {
+   *       type: 'delete-selected',
+   *       text: gettext('Delete Images')
+   *     },
+   *     permissions: policy.ifAllowed({ rules: [['image', 'delete_image']] })
+   *     }, {
+   *     callback: 'table.batchActions.create.open',
+   *     template: {
+   *       type: 'create',
+   *       text: gettext('Create Image')
+   *     },
+   *     permissions: policy.ifAllowed({ rules: [['image', 'add_image']] })
+   *   }];
+   * }
+   *
+   * ```
+   * <actions allowed="actions" type="batch">
+   * </actions>
+   * ```
+   *
+   * row:
+   *
+   * function actions(image) {
+   *   return [{
+   *     callback: 'table.rowActions.deleteImage.open',
    *     template: {
    *       text: gettext('Delete Image'),
-   *     type: 'delete',
+   *       type: 'delete'
-   *     item: 'image'
    *     },
-   *   permissions: policy.ifAllowed({ rules: [['image', 'delete_image']] }),
+   *     permissions: imageDeletePermitted(image)
-   *   callback: deleteModalService.open
    *   }, {
+   *     callback: 'table.rowActions.createVolume.open',
    *     template: {
-   *     text: gettext('Create Volume'),
+   *       text: gettext('Create Volume')
-   *     item: 'image'
    *     },
-   *   permissions: policy.ifAllowed({rules: [['volume', 'volume:create']]}),
+   *     permissions: createVolumeFromImagePermitted(image)
-   *   callback: createVolumeModalService.open
+   *   }];
-   *  }, {
+   * }
-   *   template: {
-   *     url: basePath + 'actions/my-custom-action.html'
-   *   },
-   *   permissions: policy.ifAllowed({ rules: [['image', 'custom']] }),
-   *   callback: customModalService.open
-   * }]
    *
    * ```
-   * <actions allowed-actions="actions" action-list-type="row">
+   * <actions allowed="actions" type="row" item="image">
    * </actions>
    *
-   * <actions allowed-actions="actions" action-list-type="batch">
-   * </actions>
    * ```
+   *
    */
   function actions(
     $parse,
@@ -114,11 +154,21 @@
     return directive;
 
     function link(scope, element, attrs) {
-      var listType = attrs.actionListType;
+      var listType = attrs.type;
-      var allowedActions = $parse(attrs.allowedActions)(scope);
+      var item = attrs.item;
-
+      var service = actionsService({
-      actionsService({scope: scope, element: element, listType: listType})
+        scope: scope,
-        .renderActions(allowedActions);
+        element: element,
+        listType: listType,
+        item: item
+      });
+      var allowedActions = $parse(attrs.allowed)(scope);
+      if (listType === 'row') {
+        var itemVal = $parse(item)(scope);
+        service.renderActions(allowedActions(itemVal));
+      } else {
+        service.renderActions(allowedActions());
+      }
     }
   }
 })();

horizon/static/framework/widgets/action-list/actions.directive.spec.js

@@ -17,6 +17,8 @@
   describe('actions directive', function () {
     var $scope, $compile, $q, $templateCache, basePath;
 
+    var rowItem = {id: 1};
+
     beforeEach(module('templates'));
     beforeEach(module('horizon.framework'));
 
@@ -29,18 +31,12 @@
     }));
 
     it('should have no buttons if there are no actions', function () {
-      var element = angular.element(getTemplate('actions.batch'));
+      var element = batchElementFor([]);
-      $scope.actions = [];
-      $compile(element)($scope);
-      $scope.$apply();
       expect(element.children().length).toBe(0);
     });
 
     it('should allow for specifying action text', function () {
-      var element = angular.element(getTemplate('actions.batch'));
+      var element = batchElementFor([permittedActionWithText('Create Image')]);
-      $scope.actions = [permittedActionWithText('Create Image', 'image')];
-      $compile(element)($scope);
-      $scope.$apply();
 
       expect(element.children().length).toBe(1);
       var actionList = element.find('action-list');
@@ -52,10 +48,7 @@
     });
 
     it('should allow for specifying by template for create', function () {
-      var element = angular.element(getTemplate('actions.batch'));
+      var element = batchElementFor([permittedActionWithType('create', 'Create Image')]);
-      $scope.actions = [permittedActionWithType('create', 'Create Image')];
-      $compile(element)($scope);
-      $scope.$apply();
 
       expect(element.children().length).toBe(1);
       var actionList = element.find('action-list');
@@ -67,10 +60,7 @@
     });
 
     it('should allow for specifying by template for delete-selected', function () {
-      var element = angular.element(getTemplate('actions.batch'));
+      var element = batchElementFor([permittedActionWithType('delete-selected', 'Delete Images')]);
-      $scope.actions = [permittedActionWithType('delete-selected', 'Delete Images')];
-      $compile(element)($scope);
-      $scope.$apply();
 
       expect(element.children().length).toBe(1);
       var actionList = element.find('action-list');
@@ -83,10 +73,12 @@
     });
 
     it('should allow for specifying by template for delete', function () {
-      var element = angular.element(getTemplate('actions.row'));
+      $scope.callback = function(item) {
-      $scope.actions = [permittedActionWithType('delete', 'Delete Image')];
+        expect(item).toEqual(rowItem);
-      $compile(element)($scope);
+      };
-      $scope.$apply();
+      spyOn($scope, 'callback').and.callThrough();
+
+      var element = rowElementFor([permittedActionWithType('delete', 'Delete Image')]);
 
       expect(element.children().length).toBe(1);
       var actionList = element.find('action-list');
@@ -95,15 +87,14 @@
       expect(actionList.find('button').attr('class')).toEqual('text-danger');
       expect(actionList.find('button').attr('ng-click')).toEqual('disabled || callback(item)');
       expect(actionList.text().trim()).toEqual('Delete Image');
+
+      actionList.find('button').click();
+      expect($scope.callback).toHaveBeenCalled();
     });
 
     it('should have one button if there is one action', function () {
       var action = getTemplatePath('action-create', getTemplate());
-
+      var element = batchElementFor([permittedActionWithUrl(action)]);
-      var element = angular.element(getTemplate('actions.batch'));
-      $scope.actions = [permittedActionWithUrl(action)];
-      $compile(element)($scope);
-      $scope.$apply();
 
       expect(element.children().length).toBe(1);
       var actionList = element.find('action-list');
@@ -115,10 +106,7 @@
     });
 
     it('should have no buttons if not permitted', function () {
-      var element = angular.element(getTemplate('actions.batch'));
+      var element = batchElementFor([notPermittedAction()]);
-      $scope.actions = [notPermittedAction()];
-      $compile(element)($scope);
-      $scope.$apply();
 
       expect(element.children().length).toBe(0);
     });
@@ -126,11 +114,10 @@
     it('should have multiple buttons for multiple actions as a list', function () {
       var action1 = getTemplatePath('action-create');
       var action2 = getTemplatePath('action-delete');
-
+      var element = batchElementFor([
-      var element = angular.element(getTemplate('actions.batch'));
+        permittedActionWithUrl(action1),
-      $scope.actions = [permittedActionWithUrl(action1), permittedActionWithUrl(action2)];
+        permittedActionWithUrl(action2)
-      $compile(element)($scope);
+      ]);
-      $scope.$apply();
 
       expect(element.children().length).toBe(2);
       var actionList = element.find('action-list');
@@ -142,11 +129,10 @@
 
     it('should have as many buttons as permitted', function () {
       var actionTemplate1 = getTemplatePath('action-create');
-
+      var element = batchElementFor([
-      var element = angular.element(getTemplate('actions.batch'));
+        permittedActionWithUrl(actionTemplate1),
-      $scope.actions = [permittedActionWithUrl(actionTemplate1), notPermittedAction()];
+        notPermittedAction()
-      $compile(element)($scope);
+      ]);
-      $scope.$apply();
 
       expect(element.children().length).toBe(1);
       var actionList = element.find('action-list');
@@ -159,10 +145,10 @@
       var action1 = getTemplatePath('action-create');
       var action2 = getTemplatePath('action-delete');
 
-      var element = angular.element(getTemplate('actions.row'));
+      var element = rowElementFor([
-      $scope.actions = [permittedActionWithUrl(action1), permittedActionWithUrl(action2)];
+        permittedActionWithUrl(action1),
-      $compile(element)($scope);
+        permittedActionWithUrl(action2)
-      $scope.$apply();
+      ]);
 
       expect(element.children().length).toBe(1);
       var actionList = element.find('action-list');
@@ -173,13 +159,10 @@
     });
 
     it('should have multiple buttons as a dropdown for actions text', function () {
-      var element = angular.element(getTemplate('actions.row'));
+      var element = rowElementFor([
-      $scope.actions = [
+        permittedActionWithText('Create Image'),
-        permittedActionWithText('Create Image', 'image'),
+        permittedActionWithText('Delete Image', 'text-danger')
-        permittedActionWithText('Delete Image', 'image', 'text-danger')
+      ]);
-      ];
-      $compile(element)($scope);
-      $scope.$apply();
 
       expect(element.children().length).toBe(1);
       var actionList = element.find('action-list');
@@ -190,13 +173,10 @@
     });
 
     it('should have one button if only one permitted for dropdown', function () {
-      var element = angular.element(getTemplate('actions.row'));
+      var element = rowElementFor([
-      $scope.actions = [
         permittedActionWithUrl(getTemplatePath('action-create')),
         notPermittedAction()
-      ];
+      ]);
-      $compile(element)($scope);
-      $scope.$apply();
 
       expect(element.children().length).toBe(1);
       var actionList = element.find('action-list');
@@ -213,11 +193,10 @@
       };
     }
 
-    function permittedActionWithText(text, item, actionClasses) {
+    function permittedActionWithText(text, actionClasses) {
       return {
         template: {
           text: text,
-          item: item,
           actionClasses: actionClasses
         },
         permissions: getPermission(true),
@@ -225,12 +204,11 @@
       };
     }
 
-    function permittedActionWithType(templateType, text, item) {
+    function permittedActionWithType(templateType, text) {
       return {
         template: {
           type: templateType,
-          text: text,
+          text: text
-          item: item
         },
         permissions: getPermission(true),
         callback: 'callback'
@@ -261,5 +239,33 @@
       return deferred.promise;
     }
 
+    function batchElementFor(actions) {
+      $scope.actions = function() {
+        return actions;
+      };
+
+      var element = angular.element(getTemplate('actions.batch'));
+
+      $compile(element)($scope);
+      $scope.$apply();
+
+      return element;
+    }
+
+    function rowElementFor(actions) {
+      $scope.rowItem = rowItem;
+      $scope.actions = function(item) {
+        expect(item).toEqual(rowItem);
+        return actions;
+      };
+
+      var element = angular.element(getTemplate('actions.row'));
+
+      $compile(element)($scope);
+      $scope.$apply();
+
+      return element;
+    }
+
   });
 })();

horizon/static/framework/widgets/action-list/actions.row.mock.html

@@ -1,2 +1,2 @@
-<actions allowed-actions="actions" action-list-type="row">
+<actions allowed="actions" type="row" item="rowItem">
 </actions>

horizon/static/framework/widgets/action-list/actions.service.js

@@ -30,12 +30,12 @@
 
   function actionsService($compile, $http, $q, $templateCache, basePath, $qExtensions) {
     return function(spec) {
-      return createService(spec.scope, spec.element, spec.listType);
+      return createService(spec.scope, spec.element, spec.listType, spec.item);
     };
 
     ///////////////
 
-    function createService(scope, element, listType) {
+    function createService(scope, element, listType, item) {
       var service = {
         renderActions: renderActions
       };
@@ -157,9 +157,9 @@
       /**
        * Fetch the HTML Template for the Action
        */
-      function getTemplate(permittedActionResponse) {
+      function getTemplate(permittedAction) {
         var defered = $q.defer();
-        var action = permittedActionResponse.context;
+        var action = permittedAction.context;
         $http.get(getTemplateUrl(action), {cache: $templateCache}).then(onTemplateGet);
         return defered.promise;
 
@@ -167,7 +167,7 @@
           var template = response.data
                 .replace('$action-classes$', action.template.actionClasses || '')
                 .replace('$text$', action.template.text)
-                .replace('$item$', action.template.item);
+                .replace('$item$', item);
           defered.resolve({template: template, callback: action.callback});
         }
       }