From 087cea1445a87149379476a74c038c17ca233944 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 1 Apr 2015 15:33:18 -0700 Subject: [PATCH 1/9] [Core] Allow actions to have multiple categories Allow actions to have multiple categories; this will allow the properties action to be applicable as both a menu option and a button in the view-control category, WTD-1062. --- platform/core/src/actions/ActionProvider.js | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/platform/core/src/actions/ActionProvider.js b/platform/core/src/actions/ActionProvider.js index 995dd68bc3..19b5504ad1 100644 --- a/platform/core/src/actions/ActionProvider.js +++ b/platform/core/src/actions/ActionProvider.js @@ -84,11 +84,21 @@ define( // Build up look-up tables actions.forEach(function (Action) { - if (Action.category) { - actionsByCategory[Action.category] = - actionsByCategory[Action.category] || []; - actionsByCategory[Action.category].push(Action); - } + // Get an action's category or categories + var categories = Action.category || []; + + // Convert to an array if necessary + categories = Array.isArray(categories) ? + categories : [categories]; + + // Store action under all relevant categories + categories.forEach(function (category) { + actionsByCategory[category] = + actionsByCategory[category] || []; + actionsByCategory[category].push(Action); + }); + + // Store action by ekey as well if (Action.key) { actionsByKey[Action.key] = actionsByKey[Action.key] || []; From e3ec9c6130cc6b7b82a6694833f08cf9f54c4586 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 1 Apr 2015 15:34:51 -0700 Subject: [PATCH 2/9] [Edit] Allow Edit Properties as a view-control action Allow Edit Properties as an action of the view-control category, WTD-1062. --- platform/commonUI/edit/bundle.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/commonUI/edit/bundle.json b/platform/commonUI/edit/bundle.json index eccaeaf787..6a1f299747 100644 --- a/platform/commonUI/edit/bundle.json +++ b/platform/commonUI/edit/bundle.json @@ -34,7 +34,7 @@ }, { "key": "properties", - "category": "contextual", + "category": ["contextual", "view-control"], "implementation": "actions/PropertiesAction.js", "glyph": "p", "name": "Edit Properties...", From ccf2ccc4c6a8da6b5b114ec6d7f779ae1e624233 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 1 Apr 2015 15:45:43 -0700 Subject: [PATCH 3/9] [Edit] Add policy for the Edit action Allow Edit mode only when editable views exist, WTD-1062. --- platform/commonUI/browse/bundle.json | 3 +- platform/commonUI/edit/bundle.json | 6 ++ .../edit/src/policies/EditActionPolicy.js | 61 +++++++++++++++++++ platform/core/src/actions/ActionProvider.js | 2 +- 4 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 platform/commonUI/edit/src/policies/EditActionPolicy.js diff --git a/platform/commonUI/browse/bundle.json b/platform/commonUI/browse/bundle.json index d4e7a85036..40b215fa2b 100644 --- a/platform/commonUI/browse/bundle.json +++ b/platform/commonUI/browse/bundle.json @@ -108,7 +108,8 @@ "templateUrl": "templates/items/items.html", "uses": [ "composition" ], "gestures": [ "drop" ], - "type": "folder" + "type": "folder", + "editable": false } ], "components": [ diff --git a/platform/commonUI/edit/bundle.json b/platform/commonUI/edit/bundle.json index 6a1f299747..9bb964f419 100644 --- a/platform/commonUI/edit/bundle.json +++ b/platform/commonUI/edit/bundle.json @@ -68,6 +68,12 @@ "depends": [ "$location" ] } ], + "policies": [ + { + "category": "action", + "implementation": "policies/EditActionPolicy.js" + } + ], "templates": [ { "key": "edit-library", diff --git a/platform/commonUI/edit/src/policies/EditActionPolicy.js b/platform/commonUI/edit/src/policies/EditActionPolicy.js new file mode 100644 index 0000000000..7467a01e63 --- /dev/null +++ b/platform/commonUI/edit/src/policies/EditActionPolicy.js @@ -0,0 +1,61 @@ +/*global define*/ + +define( + [], + function () { + "use strict"; + + /** + * Policy controlling when the `edit` and/or `properties` actions + * can appear as applicable actions of the `view-control` category + * (shown as buttons in the top-right of browse mode.) + * @constructor + */ + function EditActionPolicy() { + // Get a count of views which are not flagged as non-editable. + function countEditableViews(context) { + var domainObject = (context || {}).domainObject, + views = domainObject && domainObject.useCapability('view'), + count = 0; + + // A view is editable unless explicitly flagged as not + (views || []).forEach(function (view) { + count += (view.editable !== false) ? 1 : 0; + }); + + return count; + } + + return { + /** + * Check whether or not a given action is allowed by this + * policy. + * @param {Action} action the action + * @param context the context + * @returns {boolean} true if not disallowed + */ + allow: function (action, context) { + var key = action.getMetadata().key, + category = (context || {}).category; + + // Only worry about actions in the view-control category + if (category === 'view-control') { + // Restrict 'edit' to cases where there are editable + // views (similarly, restrict 'properties' to when + // the converse is true) + if (key === 'edit') { + return countEditableViews(context) > 0; + } else if (key === 'properties') { + return countEditableViews(context) < 1; + } + } + + // Like all policies, allow by default. + return true; + } + }; + } + + return EditActionPolicy; + } +); \ No newline at end of file diff --git a/platform/core/src/actions/ActionProvider.js b/platform/core/src/actions/ActionProvider.js index 19b5504ad1..16976b51c8 100644 --- a/platform/core/src/actions/ActionProvider.js +++ b/platform/core/src/actions/ActionProvider.js @@ -89,7 +89,7 @@ define( // Convert to an array if necessary categories = Array.isArray(categories) ? - categories : [categories]; + categories : [categories]; // Store action under all relevant categories categories.forEach(function (category) { From facf350648a13fc6728b65070755b16c548af5b6 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 1 Apr 2015 15:50:15 -0700 Subject: [PATCH 4/9] [Edit] Add policy to restrict views in Edit mode Add policy to disallow views in Edit mode which have been explicitly flagged as non-editable, WTD-1062. --- .../edit/src/policies/EditableViewPolicy.js | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 platform/commonUI/edit/src/policies/EditableViewPolicy.js diff --git a/platform/commonUI/edit/src/policies/EditableViewPolicy.js b/platform/commonUI/edit/src/policies/EditableViewPolicy.js new file mode 100644 index 0000000000..283ad4f485 --- /dev/null +++ b/platform/commonUI/edit/src/policies/EditableViewPolicy.js @@ -0,0 +1,36 @@ +/*global define*/ + +define( + [], + function () { + "use strict"; + + /** + * Policy controlling which views should be visible in Edit mode. + * @constructor + */ + function EditableViewPolicy() { + return { + /** + * Check whether or not a given action is allowed by this + * policy. + * @param {Action} action the action + * @param domainObject the domain object which will be viewed + * @returns {boolean} true if not disallowed + */ + allow: function (view, domainObject) { + // If a view is flagged as non-editable, only allow it + // while we're not in Edit mode. + if ((view || {}).editable === false) { + return !domainObject.hasCapability('editor'); + } + + // Like all policies, allow by default. + return true; + } + }; + } + + return EditableViewPolicy; + } +); \ No newline at end of file From 7915074b1090e6ba0a0d81515f0b10778fbb2a7e Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 1 Apr 2015 15:54:59 -0700 Subject: [PATCH 5/9] [Policy] Add spec for policy-driven view decorator Add spec to allow the applicability of views to be determined by policy decisions, WTD-1062. --- .../policy/test/PolicyViewDecoratorSpec.js | 83 +++++++++++++++++++ platform/policy/test/suite.json | 1 + 2 files changed, 84 insertions(+) create mode 100644 platform/policy/test/PolicyViewDecoratorSpec.js diff --git a/platform/policy/test/PolicyViewDecoratorSpec.js b/platform/policy/test/PolicyViewDecoratorSpec.js new file mode 100644 index 0000000000..60007fb73b --- /dev/null +++ b/platform/policy/test/PolicyViewDecoratorSpec.js @@ -0,0 +1,83 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + +define( + ["../src/PolicyViewDecorator"], + function (PolicyViewDecorator) { + "use strict"; + + describe("The policy view decorator", function () { + var mockPolicyService, + mockViewService, + mockDomainObject, + testViews, + decorator; + + beforeEach(function () { + mockPolicyService = jasmine.createSpyObj( + 'policyService', + ['allow'] + ); + mockViewService = jasmine.createSpyObj( + 'viewService', + ['getViews'] + ); + mockDomainObject = jasmine.createSpyObj( + 'domainObject', + ['getId'] + ); + + // Content of actions should be irrelevant to this + // decorator, so just give it some objects to pass + // around. + testViews = [ + { someKey: "a" }, + { someKey: "b" }, + { someKey: "c" } + ]; + + mockDomainObject.getId.andReturn('xyz'); + mockViewService.getViews.andReturn(testViews); + mockPolicyService.allow.andReturn(true); + + decorator = new PolicyViewDecorator( + mockPolicyService, + mockViewService + ); + }); + + it("delegates to its decorated view service", function () { + decorator.getViews(mockDomainObject); + expect(mockViewService.getViews) + .toHaveBeenCalledWith(mockDomainObject); + }); + + it("provides views from its decorated view service", function () { + // Mock policy service allows everything by default, + // so everything should be returned + expect(decorator.getActions(mockDomainObject)) + .toEqual(testViews); + }); + + it("consults the policy service for each candidate view", function () { + decorator.getViews(mockDomainObject); + testViews.forEach(function (testView) { + expect(mockPolicyService.allow).toHaveBeenCalledWith( + 'view', + testView, + mockDomainObject + ); + }); + }); + + it("filters out policy-disallowed views", function () { + // Disallow the second action + mockPolicyService.allow.andCallFake(function (cat, candidate, ctxt) { + return candidate.someKey !== 'b'; + }); + expect(decorator.getViews(mockDomainObject)) + .toEqual([ testViews[0], testViews[2] ]); + }); + + }); + } +); \ No newline at end of file diff --git a/platform/policy/test/suite.json b/platform/policy/test/suite.json index 8706198dc6..c695797527 100644 --- a/platform/policy/test/suite.json +++ b/platform/policy/test/suite.json @@ -1,4 +1,5 @@ [ "PolicyActionDecorator", + "PolicyViewDecorator", "PolicyProvider" ] \ No newline at end of file From 892e2c9dd4f2f7d6abe64395e5bb66df8b9da306 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 1 Apr 2015 15:58:19 -0700 Subject: [PATCH 6/9] [Policy] Implement view decorator Implement policy-driven view decorator, sufficient to satisfy specs. WTD-1062. --- platform/policy/bundle.json | 6 +++ platform/policy/src/PolicyActionDecorator.js | 2 +- platform/policy/src/PolicyViewDecorator.js | 37 +++++++++++++++++++ .../policy/test/PolicyViewDecoratorSpec.js | 2 +- 4 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 platform/policy/src/PolicyViewDecorator.js diff --git a/platform/policy/bundle.json b/platform/policy/bundle.json index ea836ca732..0f27b51136 100644 --- a/platform/policy/bundle.json +++ b/platform/policy/bundle.json @@ -10,6 +10,12 @@ "implementation": "PolicyActionDecorator.js", "depends": [ "policyService" ] }, + { + "type": "decorator", + "provides": "viewService", + "implementation": "PolicyViewDecorator.js", + "depends": [ "policyService" ] + }, { "type": "provider", "provides": "policyService", diff --git a/platform/policy/src/PolicyActionDecorator.js b/platform/policy/src/PolicyActionDecorator.js index 1057dca905..08cadd5423 100644 --- a/platform/policy/src/PolicyActionDecorator.js +++ b/platform/policy/src/PolicyActionDecorator.js @@ -15,7 +15,7 @@ define( return { /** * Get actions which are applicable in this context. - * These will be filters to remove any actions which + * These will be filtered to remove any actions which * are deemed inapplicable by policy. * @param context the context in which the action will occur * @returns {Action[]} applicable actions diff --git a/platform/policy/src/PolicyViewDecorator.js b/platform/policy/src/PolicyViewDecorator.js new file mode 100644 index 0000000000..e236ba6066 --- /dev/null +++ b/platform/policy/src/PolicyViewDecorator.js @@ -0,0 +1,37 @@ +/*global define*/ + +define( + [], + function () { + "use strict"; + + /** + * Filters out views based on policy. + * @param {PolicyService} policyService the service which provides + * policy decisions + * @param {ViewService} viewService the service to decorate + */ + function PolicyActionDecorator(policyService, viewService) { + return { + /** + * Get views which are applicable to this domain object. + * These will be filtered to remove any views which + * are deemed inapplicable by policy. + * @param {DomainObject} the domain object to view + * @returns {View[]} applicable views + */ + getViews: function (domainObject) { + // Check if an action is allowed by policy. + function allow(view) { + return policyService.allow('view', view, domainObject); + } + + // Look up actions, filter out the disallowed ones. + return viewService.getViews(domainObject).filter(allow); + } + }; + } + + return PolicyActionDecorator; + } +); \ No newline at end of file diff --git a/platform/policy/test/PolicyViewDecoratorSpec.js b/platform/policy/test/PolicyViewDecoratorSpec.js index 60007fb73b..3931a23507 100644 --- a/platform/policy/test/PolicyViewDecoratorSpec.js +++ b/platform/policy/test/PolicyViewDecoratorSpec.js @@ -54,7 +54,7 @@ define( it("provides views from its decorated view service", function () { // Mock policy service allows everything by default, // so everything should be returned - expect(decorator.getActions(mockDomainObject)) + expect(decorator.getViews(mockDomainObject)) .toEqual(testViews); }); From 0ceb8d30cf2291e03148b9d80a242f5ebb87702e Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 1 Apr 2015 16:00:20 -0700 Subject: [PATCH 7/9] [Edit] Expose EditableViewPolicy Expose EditableViewPolicy as an active extension in Edit mode, WTD-1062. --- platform/commonUI/edit/bundle.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/platform/commonUI/edit/bundle.json b/platform/commonUI/edit/bundle.json index 9bb964f419..9c76818e31 100644 --- a/platform/commonUI/edit/bundle.json +++ b/platform/commonUI/edit/bundle.json @@ -72,6 +72,10 @@ { "category": "action", "implementation": "policies/EditActionPolicy.js" + }, + { + "category": "view", + "implementation": "policies/EditableViewPolicy.js" } ], "templates": [ From e2c6db82597ef0cf9dc45170aaf5d54f671906e6 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 1 Apr 2015 16:08:15 -0700 Subject: [PATCH 8/9] [Edit] Stub in specs for Edit policies Stub in specs for policies added for Edit mode which control the availability of the Edit and/or Edit Properties actions in the view-control area, WTD-1062. --- .../test/policies/EditActionPolicySpec.js | 30 +++++++++++++++++++ .../test/policies/EditableViewPolicySpec.js | 20 +++++++++++++ platform/commonUI/edit/test/suite.json | 2 ++ 3 files changed, 52 insertions(+) create mode 100644 platform/commonUI/edit/test/policies/EditActionPolicySpec.js create mode 100644 platform/commonUI/edit/test/policies/EditableViewPolicySpec.js diff --git a/platform/commonUI/edit/test/policies/EditActionPolicySpec.js b/platform/commonUI/edit/test/policies/EditActionPolicySpec.js new file mode 100644 index 0000000000..79b7d585c8 --- /dev/null +++ b/platform/commonUI/edit/test/policies/EditActionPolicySpec.js @@ -0,0 +1,30 @@ +/*global define,describe,it,expect,beforeEach,jasmine*/ + +define( + ["../../src/policies/EditActionPolicy"], + function (EditActionPolicy) { + "use strict"; + + describe("The Edit action policy", function () { + it("allows the edit action when there are editable views", function () { + + }); + + it("allows the edit properties action when there are no editable views", function () { + + }); + + it("disallows the edit action when there are no editable views", function () { + + }); + + it("disallows the edit properties action when there are editable views", function () { + + }); + + it("allows the edit properties outside of the 'view-control' category", function () { + + }); + }); + } +); \ No newline at end of file diff --git a/platform/commonUI/edit/test/policies/EditableViewPolicySpec.js b/platform/commonUI/edit/test/policies/EditableViewPolicySpec.js new file mode 100644 index 0000000000..0883f7636a --- /dev/null +++ b/platform/commonUI/edit/test/policies/EditableViewPolicySpec.js @@ -0,0 +1,20 @@ +/*global define,describe,it,expect,beforeEach,jasmine*/ + +define( + ["../../src/policies/EditableViewPolicy"], + function (EditableViewPolicy) { + "use strict"; + + describe("The editable view policy", function () { + it("disallows views in edit mode that are flagged as non-editable", function () { + + }); + it("allows any view outside of edit mode", function () { + + }); + it("treats views with no defined 'editable' property as editable", function () { + + }); + }); + } +); \ No newline at end of file diff --git a/platform/commonUI/edit/test/suite.json b/platform/commonUI/edit/test/suite.json index 49fffec4a7..17c671b06a 100644 --- a/platform/commonUI/edit/test/suite.json +++ b/platform/commonUI/edit/test/suite.json @@ -17,6 +17,8 @@ "objects/EditableDomainObject", "objects/EditableDomainObjectCache", "objects/EditableModelCache", + "policies/EditableViewPolicy", + "policies/EditActionPolicy", "representers/EditRepresenter", "representers/EditToolbar", "representers/EditToolbarRepresenter", From 9be40d6d2ad17f0f472e391af0e4088f44a048ac Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 1 Apr 2015 16:21:03 -0700 Subject: [PATCH 9/9] [Edit] Fill in tests for Edit policies Complete tests for policies for Edit actions, WTD-1062. --- .../test/policies/EditActionPolicySpec.js | 58 +++++++++++++++++-- .../test/policies/EditableViewPolicySpec.js | 40 ++++++++++++- 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/platform/commonUI/edit/test/policies/EditActionPolicySpec.js b/platform/commonUI/edit/test/policies/EditActionPolicySpec.js index 79b7d585c8..1a2be2bad1 100644 --- a/platform/commonUI/edit/test/policies/EditActionPolicySpec.js +++ b/platform/commonUI/edit/test/policies/EditActionPolicySpec.js @@ -6,24 +6,72 @@ define( "use strict"; describe("The Edit action policy", function () { - it("allows the edit action when there are editable views", function () { + var editableView, + nonEditableView, + undefinedView, + testViews, + testContext, + mockDomainObject, + mockEditAction, + mockPropertiesAction, + policy; + beforeEach(function () { + mockDomainObject = jasmine.createSpyObj( + 'domainObject', + [ 'useCapability' ] + ); + mockEditAction = jasmine.createSpyObj('edit', ['getMetadata']); + mockPropertiesAction = jasmine.createSpyObj('edit', ['getMetadata']); + + editableView = { editable: true }; + nonEditableView = { editable: false }; + undefinedView = { someKey: "some value" }; + testViews = []; + + mockDomainObject.useCapability.andCallFake(function (c) { + // Provide test views, only for the view capability + return c === 'view' && testViews; + }); + + mockEditAction.getMetadata.andReturn({ key: 'edit' }); + mockPropertiesAction.getMetadata.andReturn({ key: 'properties' }); + + testContext = { + domainObject: mockDomainObject, + category: 'view-control' + }; + + policy = new EditActionPolicy(); + }); + + it("allows the edit action when there are editable views", function () { + testViews = [ editableView ]; + expect(policy.allow(mockEditAction, testContext)).toBeTruthy(); + // No edit flag defined; should be treated as editable + testViews = [ undefinedView, undefinedView ]; + expect(policy.allow(mockEditAction, testContext)).toBeTruthy(); }); it("allows the edit properties action when there are no editable views", function () { - + testViews = [ nonEditableView, nonEditableView ]; + expect(policy.allow(mockPropertiesAction, testContext)).toBeTruthy(); }); it("disallows the edit action when there are no editable views", function () { - + testViews = [ nonEditableView, nonEditableView ]; + expect(policy.allow(mockEditAction, testContext)).toBeFalsy(); }); it("disallows the edit properties action when there are editable views", function () { - + testViews = [ editableView ]; + expect(policy.allow(mockPropertiesAction, testContext)).toBeFalsy(); }); it("allows the edit properties outside of the 'view-control' category", function () { - + testViews = [ nonEditableView ]; + testContext.category = "something-else"; + expect(policy.allow(mockPropertiesAction, testContext)).toBeTruthy(); }); }); } diff --git a/platform/commonUI/edit/test/policies/EditableViewPolicySpec.js b/platform/commonUI/edit/test/policies/EditableViewPolicySpec.js index 0883f7636a..b4a4730125 100644 --- a/platform/commonUI/edit/test/policies/EditableViewPolicySpec.js +++ b/platform/commonUI/edit/test/policies/EditableViewPolicySpec.js @@ -6,14 +6,50 @@ define( "use strict"; describe("The editable view policy", function () { + var testView, + mockDomainObject, + testMode, + policy; + + beforeEach(function () { + testMode = true; // Act as if we're in Edit mode by default + mockDomainObject = jasmine.createSpyObj( + 'domainObject', + ['hasCapability'] + ); + mockDomainObject.hasCapability.andCallFake(function (c) { + return (c === 'editor') && testMode; + }); + + policy = new EditableViewPolicy(); + }); + it("disallows views in edit mode that are flagged as non-editable", function () { - + expect(policy.allow({ editable: false }, mockDomainObject)) + .toBeFalsy(); }); + + it("allows views in edit mode that are flagged as editable", function () { + expect(policy.allow({ editable: true }, mockDomainObject)) + .toBeTruthy(); + }); + it("allows any view outside of edit mode", function () { + var testViews = [ + { editable: false }, + { editable: true }, + { someKey: "some value" } + ]; + testMode = false; // Act as if we're not in Edit mode + testViews.forEach(function (testView) { + expect(policy.allow(testView, mockDomainObject)).toBeTruthy(); + }); }); - it("treats views with no defined 'editable' property as editable", function () { + it("treats views with no defined 'editable' property as editable", function () { + expect(policy.allow({ someKey: "some value" }, mockDomainObject)) + .toBeTruthy(); }); }); }