Merge pull request #272 from nasa/open120

[Actions] Avoid suppression of context menus

platform/core/bundle.json

@@ -97,7 +97,7 @@
                 "provides": "actionService",
                 "type": "provider",
                 "implementation": "actions/ActionProvider.js",
-                "depends": [ "actions[]" ]
+                "depends": [ "actions[]", "$log" ]
             },
             {
                 "provides": "actionService",

platform/core/src/actions/ActionProvider.js

@@ -39,9 +39,11 @@ define(
          * @imeplements {ActionService}
          * @constructor
          */
-        function ActionProvider(actions) {
+        function ActionProvider(actions, $log) {
             var self = this;
 
+            this.$log = $log;
+
             // Build up look-up tables
             this.actions = actions;
             this.actionsByKey = {};
@@ -74,6 +76,7 @@ define(
             var context = (actionContext || {}),
                 category = context.category,
                 key = context.key,
+                $log = this.$log,
                 candidates;
 
             // Instantiate an action; invokes the constructor and
@@ -103,12 +106,31 @@ define(
             // appliesTo method of given actions (if defined), and
             // instantiate those applicable actions.
             function createIfApplicable(actions, context) {
-                return (actions || []).filter(function (Action) {
-                    return Action.appliesTo ?
-                        Action.appliesTo(context) : true;
-                }).map(function (Action) {
-                    return instantiateAction(Action, context);
-                });
+                function isApplicable(Action) {
+                    return Action.appliesTo ? Action.appliesTo(context) : true;
+                }
+
+                function instantiate(Action) {
+                    try {
+                        return instantiateAction(Action, context);
+                    } catch (e) {
+                        $log.error([
+                            "Could not instantiate action",
+                            Action.key,
+                            "due to:",
+                            e.message
+                        ].join(" "));
+                        return undefined;
+                    }
+                }
+
+                function isDefined(action) {
+                    return action !== undefined;
+                }
+
+                return (actions || []).filter(isApplicable)
+                    .map(instantiate)
+                    .filter(isDefined);
             }
 
             // Match actions to the provided context by comparing "key"

platform/core/test/actions/ActionProviderSpec.js

@@ -30,7 +30,8 @@ define(
         "use strict";
 
         describe("The action provider", function () {
-            var actions,
+            var mockLog,
+                actions,
                 actionProvider;
 
             function SimpleAction() {
@@ -62,6 +63,10 @@ define(
             MetadataAction.key = "metadata";
 
             beforeEach(function () {
+                mockLog = jasmine.createSpyObj(
+                    '$log',
+                    ['error', 'warn', 'info', 'debug']
+                );
                 actions = [
                     SimpleAction,
                     CategorizedAction,
@@ -137,6 +142,42 @@ define(
                 expect(provided[0].getMetadata()).toEqual("custom metadata");
             });
 
+            describe("when actions throw errors during instantiation", function () {
+                var errorText,
+                    provided;
+
+                beforeEach(function () {
+                    errorText = "some error text";
+
+                    function BadAction() {
+                        throw new Error(errorText);
+                    }
+
+                    provided = new ActionProvider(
+                        [ SimpleAction, BadAction ],
+                        mockLog
+                    ).getActions();
+                });
+
+                it("logs an error", function () {
+                    expect(mockLog.error)
+                        .toHaveBeenCalledWith(jasmine.any(String));
+                });
+
+                it("reports the error's message", function () {
+                    expect(
+                        mockLog.error.mostRecentCall.args[0].indexOf(errorText)
+                    ).not.toEqual(-1);
+                });
+
+                it("still provides valid actions", function () {
+                    expect(provided.length).toEqual(1);
+                    expect(provided[0].perform()).toEqual("simple");
+                });
+
+            });
+
+
         });
     }
-);
+);

platform/entanglement/src/actions/AbstractComposeAction.js

@@ -122,6 +122,14 @@ define(
             });
         };
 
+        AbstractComposeAction.appliesTo = function (context) {
+            var applicableObject =
+                context.selectedObject || context.domainObject;
+
+            return !!(applicableObject &&
+                applicableObject.hasCapability('context'));
+        };
+
         return AbstractComposeAction;
     }
 );

platform/entanglement/src/actions/CopyAction.js

@@ -34,7 +34,7 @@ define(
          * @constructor
          * @memberof platform/entanglement
          */
-        function CopyAction($log, locationService, copyService, dialogService, 
+        function CopyAction($log, locationService, copyService, dialogService,
                             notificationService, context) {
             this.dialog = undefined;
             this.notification = undefined;
@@ -42,7 +42,7 @@ define(
             this.notificationService = notificationService;
             this.$log = $log;
             //Extend the behaviour of the Abstract Compose Action
-            AbstractComposeAction.call(this, locationService, copyService, 
+            AbstractComposeAction.call(this, locationService, copyService,
                 context, "Duplicate", "to a location");
         }
 
@@ -87,8 +87,8 @@ define(
         };
 
         /**
-         * Executes the CopyAction. The CopyAction uses the default behaviour of 
-         * the AbstractComposeAction, but extends it to support notification 
+         * Executes the CopyAction. The CopyAction uses the default behaviour of
+         * the AbstractComposeAction, but extends it to support notification
          * updates of progress on copy.
          */
         CopyAction.prototype.perform = function() {
@@ -131,6 +131,9 @@ define(
             return AbstractComposeAction.prototype.perform.call(this)
                 .then(success, error, notification);
         };
+
+        CopyAction.appliesTo = AbstractComposeAction.appliesTo;
+
         return CopyAction;
     }
 );

platform/entanglement/src/actions/LinkAction.js

@@ -35,14 +35,15 @@ define(
          * @memberof platform/entanglement
          */
         function LinkAction(locationService, linkService, context) {
-            return new AbstractComposeAction(
-                locationService,
-                linkService,
-                context,
-                "Link"
+            AbstractComposeAction.apply(
+                this,
+                [locationService, linkService, context, "Link"]
             );
         }
 
+        LinkAction.prototype = Object.create(AbstractComposeAction.prototype);
+        LinkAction.appliesTo = AbstractComposeAction.appliesTo;
+
         return LinkAction;
     }
 );

platform/entanglement/src/actions/MoveAction.js

@@ -35,14 +35,16 @@ define(
          * @memberof platform/entanglement
          */
         function MoveAction(locationService, moveService, context) {
-            return new AbstractComposeAction(
-                locationService,
-                moveService,
-                context,
-                "Move"
+            AbstractComposeAction.apply(
+                this,
+                [locationService, moveService, context, "Move"]
             );
+
         }
 
+        MoveAction.prototype = Object.create(AbstractComposeAction.prototype);
+        MoveAction.appliesTo = AbstractComposeAction.appliesTo;
+
         return MoveAction;
     }
 );

platform/entanglement/test/actions/AbstractComposeActionSpec.js

@@ -94,6 +94,28 @@ define(
                 composeService = new MockCopyService();
             });
 
+            it("are only applicable to domain objects with a context", function () {
+                var noContextObject = domainObjectFactory({
+                    name: 'selectedObject',
+                    model: { name: 'selectedObject' },
+                    capabilities: {}
+                });
+
+                expect(AbstractComposeAction.appliesTo({
+                    selectedObject: selectedObject
+                })).toBe(true);
+                expect(AbstractComposeAction.appliesTo({
+                    domainObject: selectedObject
+                })).toBe(true);
+
+                expect(AbstractComposeAction.appliesTo({
+                    selectedObject: noContextObject
+                })).toBe(false);
+                expect(AbstractComposeAction.appliesTo({
+                    domainObject: noContextObject
+                })).toBe(false);
+            });
+
 
             describe("with context from context-action", function () {
                 beforeEach(function () {