From 4c42d4de287c44189803e2e86c64278860318cfe Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 27 Jan 2015 13:00:40 -0800 Subject: [PATCH 1/2] [Actions] Restrict mutating actions Restrict actions which cause mutation of domain objects to types of domain objects which can be created. (This does not include Packet or Telemetry Point, WTD-723.) --- platform/commonUI/edit/src/actions/EditAction.js | 6 +++++- .../commonUI/edit/src/actions/PropertiesAction.js | 11 ++++++++--- platform/commonUI/edit/src/actions/RemoveAction.js | 9 +++++++-- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/platform/commonUI/edit/src/actions/EditAction.js b/platform/commonUI/edit/src/actions/EditAction.js index 0798c603a4..1c5a6878a5 100644 --- a/platform/commonUI/edit/src/actions/EditAction.js +++ b/platform/commonUI/edit/src/actions/EditAction.js @@ -57,7 +57,11 @@ define( * will be performed; should contain a `domainObject` property */ EditAction.appliesTo = function (context) { - return (context || {}).domainObject !== undefined; + var domainObject = (context || {}).domainObject, + type = domainObject && domainObject.getCapability('type'); + + // Only allow creatable types to be edited + return type && type.hasFeature('creation'); }; return EditAction; diff --git a/platform/commonUI/edit/src/actions/PropertiesAction.js b/platform/commonUI/edit/src/actions/PropertiesAction.js index 18d2f57586..dd82774d69 100644 --- a/platform/commonUI/edit/src/actions/PropertiesAction.js +++ b/platform/commonUI/edit/src/actions/PropertiesAction.js @@ -70,9 +70,14 @@ define( * context. */ PropertiesAction.appliesTo = function (context) { - return (context || {}).domainObject && - (context || {}).domainObject.hasCapability("type") && - (context || {}).domainObject.hasCapability("persistence"); + var domainObject = (context || {}).domainObject, + type = domainObject && domainObject.getCapability('type'), + creatable = type && type.hasFeature('creation'); + + // Only allow creatable types to be edited + return domainObject && + domainObject.hasCapability("persistence") && + creatable; }; return PropertiesAction; diff --git a/platform/commonUI/edit/src/actions/RemoveAction.js b/platform/commonUI/edit/src/actions/RemoveAction.js index 9ff67c7a48..1a7d5f8ea1 100644 --- a/platform/commonUI/edit/src/actions/RemoveAction.js +++ b/platform/commonUI/edit/src/actions/RemoveAction.js @@ -80,9 +80,14 @@ define( RemoveAction.appliesTo = function (context) { var object = (context || {}).domainObject, contextCapability = object && object.getCapability("context"), - parent = contextCapability && contextCapability.getParent(); + parent = contextCapability && contextCapability.getParent(), + parentType = parent.getCapability('type'), + parentCreatable = parentType && parentType.hasFeature('creation'); + + // Only creatable types should be modifiable return parent !== undefined && - Array.isArray(parent.getModel().composition); + Array.isArray(parent.getModel().composition) && + parentCreatable; }; return RemoveAction; From aa9ceeb2be703025b71a20d9be1d1c8a6eed55b6 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 27 Jan 2015 13:09:23 -0800 Subject: [PATCH 2/2] [Actions] Update specs Update specs for actions which change domain objects to reflect their restriction to types that are creatable, WTD-723. --- platform/commonUI/edit/src/actions/RemoveAction.js | 2 +- platform/commonUI/edit/test/actions/EditActionSpec.js | 10 ++++++++++ .../commonUI/edit/test/actions/PropertiesActionSpec.js | 8 +++++++- .../commonUI/edit/test/actions/RemoveActionSpec.js | 10 ++++++++-- 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/platform/commonUI/edit/src/actions/RemoveAction.js b/platform/commonUI/edit/src/actions/RemoveAction.js index 1a7d5f8ea1..68d3296112 100644 --- a/platform/commonUI/edit/src/actions/RemoveAction.js +++ b/platform/commonUI/edit/src/actions/RemoveAction.js @@ -81,7 +81,7 @@ define( var object = (context || {}).domainObject, contextCapability = object && object.getCapability("context"), parent = contextCapability && contextCapability.getParent(), - parentType = parent.getCapability('type'), + parentType = parent && parent.getCapability('type'), parentCreatable = parentType && parentType.hasFeature('creation'); // Only creatable types should be modifiable diff --git a/platform/commonUI/edit/test/actions/EditActionSpec.js b/platform/commonUI/edit/test/actions/EditActionSpec.js index e0a47feb18..b0fb481b35 100644 --- a/platform/commonUI/edit/test/actions/EditActionSpec.js +++ b/platform/commonUI/edit/test/actions/EditActionSpec.js @@ -10,6 +10,7 @@ define( mockNavigationService, mockLog, mockDomainObject, + mockType, actionContext, action; @@ -30,6 +31,13 @@ define( "domainObject", [ "getId", "getModel", "getCapability" ] ); + mockType = jasmine.createSpyObj( + "type", + [ "hasFeature" ] + ); + + mockDomainObject.getCapability.andReturn(mockType); + mockType.hasFeature.andReturn(true); actionContext = { domainObject: mockDomainObject }; @@ -44,6 +52,8 @@ define( it("is only applicable when a domain object is present", function () { expect(EditAction.appliesTo(actionContext)).toBeTruthy(); expect(EditAction.appliesTo({})).toBeFalsy(); + // Should have checked for creatability + expect(mockType.hasFeature).toHaveBeenCalledWith('creation'); }); it("changes URL path to edit mode when performed", function () { diff --git a/platform/commonUI/edit/test/actions/PropertiesActionSpec.js b/platform/commonUI/edit/test/actions/PropertiesActionSpec.js index ad0f6ad895..604da4bc69 100644 --- a/platform/commonUI/edit/test/actions/PropertiesActionSpec.js +++ b/platform/commonUI/edit/test/actions/PropertiesActionSpec.js @@ -18,7 +18,10 @@ define( beforeEach(function () { capabilities = { - type: { getProperties: function () { return []; } }, + type: { + getProperties: function () { return []; }, + hasFeature: jasmine.createSpy('hasFeature') + }, persistence: jasmine.createSpyObj("persistence", ["persist"]), mutation: jasmine.createSpy("mutation") }; @@ -38,6 +41,7 @@ define( } }; + capabilities.type.hasFeature.andReturn(true); capabilities.mutation.andReturn(true); action = new PropertiesAction(dialogService, context); @@ -65,6 +69,8 @@ define( it("is only applicable when a domain object is in context", function () { expect(PropertiesAction.appliesTo(context)).toBeTruthy(); expect(PropertiesAction.appliesTo({})).toBeFalsy(); + // Make sure it checked for creatability + expect(capabilities.type.hasFeature).toHaveBeenCalledWith('creation'); }); }); } diff --git a/platform/commonUI/edit/test/actions/RemoveActionSpec.js b/platform/commonUI/edit/test/actions/RemoveActionSpec.js index 459c14a31e..e6f73e06b1 100644 --- a/platform/commonUI/edit/test/actions/RemoveActionSpec.js +++ b/platform/commonUI/edit/test/actions/RemoveActionSpec.js @@ -12,6 +12,7 @@ define( mockContext, mockMutation, mockPersistence, + mockType, actionContext, model, capabilities, @@ -47,16 +48,18 @@ define( mockContext = jasmine.createSpyObj("context", [ "getParent" ]); mockMutation = jasmine.createSpyObj("mutation", [ "invoke" ]); mockPersistence = jasmine.createSpyObj("persistence", [ "persist" ]); + mockType = jasmine.createSpyObj("type", [ "hasFeature" ]); mockDomainObject.getId.andReturn("test"); mockDomainObject.getCapability.andReturn(mockContext); mockContext.getParent.andReturn(mockParent); - + mockType.hasFeature.andReturn(true); capabilities = { mutation: mockMutation, - persistence: mockPersistence + persistence: mockPersistence, + type: mockType }; model = { composition: [ "a", "test", "b", "c" ] @@ -73,6 +76,9 @@ define( mockContext.getParent.andReturn(undefined); expect(RemoveAction.appliesTo(actionContext)).toBeFalsy(); + + // Also verify that creatability was checked + expect(mockType.hasFeature).toHaveBeenCalledWith('creation'); }); it("mutates the parent when performed", function () {