diff --git a/platform/commonUI/edit/src/actions/RemoveAction.js b/platform/commonUI/edit/src/actions/RemoveAction.js index e89ad3ed2d..21c290cd08 100644 --- a/platform/commonUI/edit/src/actions/RemoveAction.js +++ b/platform/commonUI/edit/src/actions/RemoveAction.js @@ -37,23 +37,37 @@ define( * * @param {DomainObject} object the object to be removed * @param {ActionContext} context the context in which this action is performed + * @memberof platform/commonUI/edit * @constructor - * @memberof module:editor/actions/remove-action + * @implements {Action} */ function RemoveAction($q, navigationService, context) { - var object = (context || {}).domainObject, + this.domainObject = (context || {}).domainObject; + this.$q = $q; + this.navigationService = navigationService; + } + + /** + * Perform this action. + * @return {Promise} a promise which will be + * fulfilled when the action has completed. + */ + RemoveAction.prototype.perform = function () { + var $q = this.$q, + navigationService = this.navigationService, + domainObject = this.domainObject, ROOT_ID = "ROOT"; - /** + /* * Check whether an object ID matches the ID of the object being * removed (used to filter a parent's composition to handle the * removal.) */ function isNotObject(otherObjectId) { - return otherObjectId !== object.getId(); + return otherObjectId !== domainObject.getId(); } - /** + /* * Mutate a parent object such that it no longer contains the object * which is being removed. */ @@ -61,7 +75,7 @@ define( model.composition = model.composition.filter(isNotObject); } - /** + /* * Invoke persistence on a domain object. This will be called upon * the removed object's parent (as its composition will have changed.) */ @@ -76,10 +90,10 @@ define( // navigate back to parent of removed object. function checkObjectNavigation(object, parentObject) { // Traverse object starts at current location - var traverseObject = navigationService.getNavigation(); + var traverseObject = (navigationService).getNavigation(); // Stop at ROOT of folder path - while(traverseObject.getId() !== ROOT_ID) { + while (traverseObject.getId() !== ROOT_ID) { // If traverse object is object being removed // navigate to parent of removed object if (traverseObject.getId() === object.getId()) { @@ -90,37 +104,25 @@ define( traverseObject = traverseObject.getCapability('context').getParent(); } } - - /** + + /* * Remove the object from its parent, as identified by its context * capability. - * @param {object} domain object being removed contextCapability - gotten from the "context" capability of this object */ function removeFromContext(object) { var contextCapability = object.getCapability('context'), parent = contextCapability.getParent(); - // Navigates through/ascendant if deleting current object - checkObjectNavigation(object, parent) - $q.when( + checkObjectNavigation(object, parent); + return $q.when( parent.useCapability('mutation', doMutate) ).then(function () { return doPersist(parent); }); } - return { - /** - * Perform this action. - * @return {module:core/promises.Promise} a promise which will be - * fulfilled when the action has completed. - */ - perform: function () { - return $q.when(object) - .then(removeFromContext); - } - }; - } + return $q.when(domainObject) + .then(removeFromContext); + }; // Object needs to have a parent for Remove to be applicable RemoveAction.appliesTo = function (context) { @@ -138,4 +140,4 @@ define( return RemoveAction; } -); \ No newline at end of file +); diff --git a/platform/commonUI/edit/test/actions/RemoveActionSpec.js b/platform/commonUI/edit/test/actions/RemoveActionSpec.js index 072094ed1d..0a93ffa180 100644 --- a/platform/commonUI/edit/test/actions/RemoveActionSpec.js +++ b/platform/commonUI/edit/test/actions/RemoveActionSpec.js @@ -31,8 +31,11 @@ define( mockNavigationService, mockDomainObject, mockParent, - mockGrandparent, + mockChildObject, + mockGrandchildObject, mockContext, + mockChildContext, + mockGrandchildContext, mockMutation, mockPersistence, mockType, @@ -56,21 +59,15 @@ define( "domainObject", [ "getId", "getCapability" ] ); + mockChildObject = jasmine.createSpyObj( + "domainObject", + [ "getId", "getCapability" ] + ); + mockGrandchildObject = jasmine.createSpyObj( + "domainObject", + [ "getId", "getCapability" ] + ); mockQ = { when: mockPromise }; - mockGrandparent = { - getModel: function () { - return model; - }, - getCapability: function (k) { - return capabilities[k]; - }, - useCapability: function (k, v) { - return capabilities[k].invoke(v); - }, - getId: function () { - return "test"; - } - }; mockParent = { getModel: function () { return model; @@ -80,12 +77,11 @@ define( }, useCapability: function (k, v) { return capabilities[k].invoke(v); - }, - getParent: function () { - return mockGrandparent; } }; mockContext = jasmine.createSpyObj("context", [ "getParent" ]); + mockChildContext = jasmine.createSpyObj("context", [ "getParent" ]); + mockGrandchildContext = jasmine.createSpyObj("context", [ "getParent" ]); mockMutation = jasmine.createSpyObj("mutation", [ "invoke" ]); mockPersistence = jasmine.createSpyObj("persistence", [ "persist" ]); mockType = jasmine.createSpyObj("type", [ "hasFeature" ]); @@ -112,7 +108,7 @@ define( type: mockType }; model = { - composition: [ "a", "b", "test", "c" ] + composition: [ "a", "test", "b" ] }; actionContext = { domainObject: mockDomainObject }; @@ -152,11 +148,66 @@ define( // Should have removed "test" - that was our // mock domain object's id. - expect(result.composition).toEqual(["a", "b", "c"]); + expect(result.composition).toEqual(["a", "b"]); // Finally, should have persisted expect(mockPersistence.persist).toHaveBeenCalled(); }); + + it("removes parent of object currently navigated to", function () { + var mutator, result; + + // Navigates to child object + mockNavigationService.getNavigation.andReturn(mockChildObject); + + // Test is id of object being removed + // Child object has different id + mockDomainObject.getId.andReturn("test"); + mockChildObject.getId.andReturn("not test"); + + // Sets context for the child and domainObject + mockDomainObject.getCapability.andReturn(mockContext); + mockChildObject.getCapability.andReturn(mockChildContext); + + // Parents of child and domainObject are set + mockContext.getParent.andReturn(mockParent); + mockChildContext.getParent.andReturn(mockDomainObject); + + mockType.hasFeature.andReturn(true); + + action.perform(); + + // Expects navigation to parent of domainObject (removed object) + expect(mockNavigationService.setNavigation).toHaveBeenCalledWith(mockParent); + }); + + it("checks if removing object not in ascendent path (reaches ROOT)", function () { + // Navigates to grandchild of ROOT + mockNavigationService.getNavigation.andReturn(mockGrandchildObject); + + // domainObject (grandparent) is set as ROOT, child and grandchild + // are set objects not being removed + mockDomainObject.getId.andReturn("ROOT"); + mockChildObject.getId.andReturn("not test"); + mockGrandchildObject.getId.andReturn("not test"); + + // Sets context for the grandchild, child, and domainObject + mockDomainObject.getCapability.andReturn(mockContext); + mockChildObject.getCapability.andReturn(mockChildContext); + mockGrandchildObject.getCapability.andReturn(mockGrandchildContext); + + // Parents of grandchild, child, and domainObject are set + mockContext.getParent.andReturn(mockParent); + mockChildContext.getParent.andReturn(mockDomainObject); + mockGrandchildContext.getParent.andReturn(mockChildObject); + + mockType.hasFeature.andReturn(true); + + action.perform(); + + // Expects no navigation to occur + expect(mockNavigationService.setNavigation).not.toHaveBeenCalled(); + }); }); }