From e42b8d22f72a08a17f6fb45044439d2f5787df26 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 22 Jun 2016 15:32:37 -0700 Subject: [PATCH 01/17] [Edit] Undirty objects on refresh Remove domain objects from the active transaction when they are refreshed, and use this from the SaveAsAction to prevent saving unintended changes. Fixes #1046 --- .../commonUI/edit/src/actions/SaveAsAction.js | 22 ++++++++++++++++--- .../TransactionalPersistenceCapability.js | 10 ++++++++- .../edit/src/services/TransactionService.js | 9 ++++++++ 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/platform/commonUI/edit/src/actions/SaveAsAction.js b/platform/commonUI/edit/src/actions/SaveAsAction.js index cee67ebbfe..d13f723bb8 100644 --- a/platform/commonUI/edit/src/actions/SaveAsAction.js +++ b/platform/commonUI/edit/src/actions/SaveAsAction.js @@ -105,7 +105,8 @@ define( SaveAsAction.prototype.save = function () { var self = this, domainObject = this.domainObject, - copyService = this.copyService; + copyService = this.copyService, + toUndirty = []; function doWizardSave(parent) { var wizard = self.createWizard(parent); @@ -127,14 +128,28 @@ define( } function allowClone(objectToClone) { - return (objectToClone.getId() === domainObject.getId()) || - objectToClone.getCapability('location').isOriginal(); + var allowed = + (objectToClone.getId() === domainObject.getId()) || + objectToClone.getCapability('location').isOriginal(); + if (allowed) { + toUndirty.push(objectToClone); + } + return allowed; } function cloneIntoParent(parent) { return copyService.perform(domainObject, parent, allowClone); } + function undirty(domainObject) { + return domainObject.getCapability('persistence').refresh(); + } + + function undirtyOriginals(object) { + return Promise.all(toUndirty.map(undirty)) + .then(resolveWith(object)); + } + function commitEditingAfterClone(clonedObject) { return domainObject.getCapability("editor").save() .then(resolveWith(clonedObject)); @@ -144,6 +159,7 @@ define( .then(doWizardSave) .then(getParent) .then(cloneIntoParent) + .then(undirtyOriginals) .then(commitEditingAfterClone) .catch(resolveWith(false)); }; diff --git a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js index 9bcf19c002..7bb4486b4f 100644 --- a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js +++ b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js @@ -49,6 +49,7 @@ define( this.domainObject = domainObject; this.$q = $q; this.persistPending = false; + this.removeFromTransaction = undefined; } /** @@ -75,6 +76,7 @@ define( }); } else { self.persistPending = false; + self.removeFromTransaction = undefined; //Model is undefined in persistence, so return undefined. return self.$q.when(undefined); } @@ -82,7 +84,8 @@ define( if (this.transactionService.isActive()) { if (!this.persistPending) { - this.transactionService.addToTransaction(onCommit, onCancel); + this.removeFromTransaction = this.transactionService + .addToTransaction(onCommit, onCancel); this.persistPending = true; } //Need to return a promise from this function @@ -93,6 +96,11 @@ define( }; TransactionalPersistenceCapability.prototype.refresh = function () { + if (this.persistPending) { + this.persistPending = false; + this.removeFromTransaction(); + this.removeFromTransaction = undefined; + } return this.persistenceCapability.refresh(); }; diff --git a/platform/commonUI/edit/src/services/TransactionService.js b/platform/commonUI/edit/src/services/TransactionService.js index 8d6c465959..59f196cf4b 100644 --- a/platform/commonUI/edit/src/services/TransactionService.js +++ b/platform/commonUI/edit/src/services/TransactionService.js @@ -81,6 +81,15 @@ define( //Log error because this is a programming error if it occurs. this.$log.error("No transaction in progress"); } + + return function () { + this.onCommits = this.onCommits.filter(function (callback) { + return callback !== onCommit; + }); + this.onCancels = this.onCancels.filter(function (callback) { + return callback !== onCancel; + }); + }.bind(this); }; /** From a3a0f003f09fe37409ade270cc23eb509d612410 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 22 Jun 2016 15:40:38 -0700 Subject: [PATCH 02/17] [Edit] Don't refresh unpersisted objects --- platform/core/src/capabilities/PersistenceCapability.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/platform/core/src/capabilities/PersistenceCapability.js b/platform/core/src/capabilities/PersistenceCapability.js index aacf48f2e1..d985d394dc 100644 --- a/platform/core/src/capabilities/PersistenceCapability.js +++ b/platform/core/src/capabilities/PersistenceCapability.js @@ -152,6 +152,10 @@ define( }, modified); } + if (domainObject.getModel().persisted === undefined) { + return this.$q.when(true); + } + return this.persistenceService.readObject( this.getSpace(), this.getKey() From 8244e435bf9d3b26a26acf6c148d94b2497a4477 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 22 Jun 2016 16:13:26 -0700 Subject: [PATCH 03/17] [Edit] Update failing spec --- platform/core/test/capabilities/PersistenceCapabilitySpec.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/platform/core/test/capabilities/PersistenceCapabilitySpec.js b/platform/core/test/capabilities/PersistenceCapabilitySpec.js index be8c181cbd..2456e0ba19 100644 --- a/platform/core/test/capabilities/PersistenceCapabilitySpec.js +++ b/platform/core/test/capabilities/PersistenceCapabilitySpec.js @@ -74,7 +74,7 @@ define( ); mockQ = jasmine.createSpyObj( "$q", - ["reject"] + ["reject", "when"] ); mockNofificationService = jasmine.createSpyObj( "notificationService", @@ -103,6 +103,7 @@ define( mockIdentifierService.parse.andReturn(mockIdentifier); mockIdentifier.getSpace.andReturn(SPACE); mockIdentifier.getKey.andReturn(key); + mockQ.when.andCallFake(asPromise); persistence = new PersistenceCapability( mockCacheService, mockPersistenceService, @@ -156,6 +157,7 @@ define( }); it("refreshes the domain object model from persistence", function () { var refreshModel = {someOtherKey: "some other value"}; + model.persisted = 1; mockPersistenceService.readObject.andReturn(asPromise(refreshModel)); persistence.refresh(); expect(model).toEqual(refreshModel); From 48b271b7ca3e7bf4dfd608205fc6252341438cb8 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 22 Jun 2016 16:14:49 -0700 Subject: [PATCH 04/17] [Edit] Unshadow variable name ...to satisfy JSHint --- platform/commonUI/edit/src/actions/SaveAsAction.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/platform/commonUI/edit/src/actions/SaveAsAction.js b/platform/commonUI/edit/src/actions/SaveAsAction.js index d13f723bb8..059adbfe57 100644 --- a/platform/commonUI/edit/src/actions/SaveAsAction.js +++ b/platform/commonUI/edit/src/actions/SaveAsAction.js @@ -141,8 +141,8 @@ define( return copyService.perform(domainObject, parent, allowClone); } - function undirty(domainObject) { - return domainObject.getCapability('persistence').refresh(); + function undirty(object) { + return object.getCapability('persistence').refresh(); } function undirtyOriginals(object) { From eb5493e37b1370284bb3bf43bb13deef0d7f00aa Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 14 Jul 2016 11:51:59 -0700 Subject: [PATCH 05/17] [Persistence] Revert quick-fix for persistence clearing Revert "Squashed commit of the following:" This reverts commit d1c01d3c8681bf335ca6b1ceb1529bc3e389816d. --- platform/commonUI/edit/bundle.js | 3 +- .../commonUI/edit/src/actions/SaveAsAction.js | 16 ---------- .../edit/src/services/TransactionService.js | 31 +------------------ .../edit/test/actions/SaveAsActionSpec.js | 18 +---------- 4 files changed, 3 insertions(+), 65 deletions(-) diff --git a/platform/commonUI/edit/bundle.js b/platform/commonUI/edit/bundle.js index 880e61f5da..6b1723c5d9 100644 --- a/platform/commonUI/edit/bundle.js +++ b/platform/commonUI/edit/bundle.js @@ -222,8 +222,7 @@ define([ "policyService", "dialogService", "creationService", - "copyService", - "transactionService" + "copyService" ], "priority": "mandatory" }, diff --git a/platform/commonUI/edit/src/actions/SaveAsAction.js b/platform/commonUI/edit/src/actions/SaveAsAction.js index 4e2c5d870b..03022be049 100644 --- a/platform/commonUI/edit/src/actions/SaveAsAction.js +++ b/platform/commonUI/edit/src/actions/SaveAsAction.js @@ -44,7 +44,6 @@ define([ dialogService, creationService, copyService, - transactionService, context ) { this.domainObject = (context || {}).domainObject; @@ -55,7 +54,6 @@ define([ this.dialogService = dialogService; this.creationService = creationService; this.copyService = copyService; - this.transactionService = transactionService; } /** @@ -113,8 +111,6 @@ define([ var self = this, domainObject = this.domainObject, copyService = this.copyService, - transactionService = this.transactionService, - cancelOldTransaction, dialog = new SaveInProgressDialog(this.dialogService); function doWizardSave(parent) { @@ -160,16 +156,6 @@ define([ .then(resolveWith(clonedObject)); } - function restartTransaction(object) { - cancelOldTransaction = transactionService.restartTransaction(); - return object; - } - - function doCancelOldTransaction(object) { - cancelOldTransaction(); - return object; - } - function onFailure() { hideBlockingDialog(); return false; @@ -179,10 +165,8 @@ define([ .then(doWizardSave) .then(showBlockingDialog) .then(getParent) - .then(restartTransaction) .then(cloneIntoParent) .then(commitEditingAfterClone) - .then(doCancelOldTransaction) .then(hideBlockingDialog) .catch(onFailure); }; diff --git a/platform/commonUI/edit/src/services/TransactionService.js b/platform/commonUI/edit/src/services/TransactionService.js index b5439d4491..0a7f148853 100644 --- a/platform/commonUI/edit/src/services/TransactionService.js +++ b/platform/commonUI/edit/src/services/TransactionService.js @@ -140,38 +140,9 @@ define( }); }; - /** - * Clear and restart the active transaction. - * - * This neither cancels nor commits the active transaction; - * instead, it returns a function that can be used to cancel that - * transaction. - * - * @returns {Function} a function to cancel the prior transaction - * @private - */ - TransactionService.prototype.restartTransaction = function () { - var oldOnCancels = this.onCancels; - - this.onCommits = []; - this.onCancels = []; - - return function () { - while (oldOnCancels.length > 0) { - var onCancel = oldOnCancels.pop(); - try { - onCancel(); - } catch (error) { - this.$log.error("Error cancelling transaction."); - } - } - }; - }; - TransactionService.prototype.size = function () { return this.onCommits.length; }; return TransactionService; - } -); + }); diff --git a/platform/commonUI/edit/test/actions/SaveAsActionSpec.js b/platform/commonUI/edit/test/actions/SaveAsActionSpec.js index ec649571ee..dbdd1f9692 100644 --- a/platform/commonUI/edit/test/actions/SaveAsActionSpec.js +++ b/platform/commonUI/edit/test/actions/SaveAsActionSpec.js @@ -34,7 +34,6 @@ define( mockCopyService, mockParent, mockUrlService, - mockTransactionService, actionContext, capabilities = {}, action; @@ -120,26 +119,11 @@ define( ["urlForLocation"] ); - mockTransactionService = jasmine.createSpyObj( - "transactionService", - ["restartTransaction"] - ); - mockTransactionService.restartTransaction - .andReturn(jasmine.createSpy()); - actionContext = { domainObject: mockDomainObject }; - action = new SaveAsAction( - undefined, - undefined, - mockDialogService, - undefined, - mockCopyService, - mockTransactionService, - actionContext - ); + action = new SaveAsAction(undefined, undefined, mockDialogService, undefined, mockCopyService, actionContext); spyOn(action, "getObjectService"); action.getObjectService.andReturn(mockObjectService); From 00fff525296c0b3630cbcd4211c7c552cd6b7b33 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 14 Jul 2016 12:50:36 -0700 Subject: [PATCH 06/17] [Persistence] Track persist-pending state globally --- .../TransactionalPersistenceCapability.js | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js index 492cd636b1..583f277d49 100644 --- a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js +++ b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js @@ -24,6 +24,7 @@ define( [], function () { + var PERSIST_PENDING_SET = {}; /** * Wraps persistence capability to enable transactions. Transactions @@ -48,10 +49,17 @@ define( this.persistenceCapability = persistenceCapability; this.domainObject = domainObject; this.$q = $q; - this.persistPending = false; this.removeFromTransaction = undefined; } + TransactionalPersistenceCapability.prototype.persistPending = function (state) { + var id = this.domainObject.getId(); + if (arguments.length > 0) { + PERSIST_PENDING_SET[id] = state; + } + return PERSIST_PENDING_SET[id]; + }; + /** * The wrapped persist function. If a transaction is active, persist * will be queued until the transaction is committed or cancelled. @@ -61,8 +69,11 @@ define( var self = this; function onCommit() { + if (!self.persistPending()) { + return Promise.resolve(true); + } return self.persistenceCapability.persist().then(function (result) { - self.persistPending = false; + self.persistPending(false); return result; }); } @@ -71,11 +82,11 @@ define( if (self.domainObject.getModel().persisted !== undefined) { //Fetch clean model from persistence return self.persistenceCapability.refresh().then(function (result) { - self.persistPending = false; + self.persistPending(false); return result; }); } else { - self.persistPending = false; + self.persistPending(false); self.removeFromTransaction = undefined; //Model is undefined in persistence, so return undefined. return self.$q.when(undefined); @@ -83,10 +94,10 @@ define( } if (this.transactionService.isActive()) { - if (!this.persistPending) { + if (!self.persistPending()) { this.removeFromTransaction = this.transactionService .addToTransaction(onCommit, onCancel); - this.persistPending = true; + this.persistPending(true); } //Need to return a promise from this function return this.$q.when(true); @@ -96,8 +107,8 @@ define( }; TransactionalPersistenceCapability.prototype.refresh = function () { - if (this.persistPending) { - this.persistPending = false; + if (this.persistPending()) { + this.persistPending(false); this.removeFromTransaction(); this.removeFromTransaction = undefined; } From c7529dd56bc4fdb72b618695470b4912cbcc6efb Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 14 Jul 2016 12:53:58 -0700 Subject: [PATCH 07/17] [Persistence] Share transaction clearing functions --- .../TransactionalPersistenceCapability.js | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js index 583f277d49..5071dc040a 100644 --- a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js +++ b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js @@ -24,7 +24,7 @@ define( [], function () { - var PERSIST_PENDING_SET = {}; + var TRANSACTION_SET = {}; /** * Wraps persistence capability to enable transactions. Transactions @@ -49,15 +49,14 @@ define( this.persistenceCapability = persistenceCapability; this.domainObject = domainObject; this.$q = $q; - this.removeFromTransaction = undefined; } - TransactionalPersistenceCapability.prototype.persistPending = function (state) { + TransactionalPersistenceCapability.prototype.transactionClearer = function (state) { var id = this.domainObject.getId(); if (arguments.length > 0) { - PERSIST_PENDING_SET[id] = state; + TRANSACTION_SET[id] = state; } - return PERSIST_PENDING_SET[id]; + return TRANSACTION_SET[id]; }; /** @@ -69,11 +68,11 @@ define( var self = this; function onCommit() { - if (!self.persistPending()) { + if (!self.transactionClearer()) { return Promise.resolve(true); } return self.persistenceCapability.persist().then(function (result) { - self.persistPending(false); + self.transactionClearer(undefined); return result; }); } @@ -82,11 +81,11 @@ define( if (self.domainObject.getModel().persisted !== undefined) { //Fetch clean model from persistence return self.persistenceCapability.refresh().then(function (result) { - self.persistPending(false); + self.transactionClearer(undefined); return result; }); } else { - self.persistPending(false); + self.transactionClearer(undefined); self.removeFromTransaction = undefined; //Model is undefined in persistence, so return undefined. return self.$q.when(undefined); @@ -94,10 +93,9 @@ define( } if (this.transactionService.isActive()) { - if (!self.persistPending()) { - this.removeFromTransaction = this.transactionService - .addToTransaction(onCommit, onCancel); - this.persistPending(true); + if (!self.transactionClearer()) { + this.transactionClearer(this.transactionService + .addToTransaction(onCommit, onCancel)); } //Need to return a promise from this function return this.$q.when(true); @@ -107,10 +105,9 @@ define( }; TransactionalPersistenceCapability.prototype.refresh = function () { - if (this.persistPending()) { - this.persistPending(false); - this.removeFromTransaction(); - this.removeFromTransaction = undefined; + if (this.transactionClearer()) { + this.transactionClearer()(); + this.transactionClearer(undefined); } return this.persistenceCapability.refresh(); }; From a731c35ad610652d0250298168e3744ece920d2e Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 14 Jul 2016 13:43:55 -0700 Subject: [PATCH 08/17] [Persistence] Refactor out transaction management --- platform/commonUI/edit/bundle.js | 13 +++- .../TransactionalPersistenceCapability.js | 57 +++----------- .../edit/src/services/TransactionManager.js | 74 +++++++++++++++++++ 3 files changed, 97 insertions(+), 47 deletions(-) create mode 100644 platform/commonUI/edit/src/services/TransactionManager.js diff --git a/platform/commonUI/edit/bundle.js b/platform/commonUI/edit/bundle.js index 6b1723c5d9..2141110409 100644 --- a/platform/commonUI/edit/bundle.js +++ b/platform/commonUI/edit/bundle.js @@ -42,6 +42,7 @@ define([ "./src/representers/EditToolbarRepresenter", "./src/capabilities/EditorCapability", "./src/capabilities/TransactionCapabilityDecorator", + "./src/services/TransactionManager", "./src/services/TransactionService", "./src/creation/CreateMenuController", "./src/creation/LocatorController", @@ -80,6 +81,7 @@ define([ EditToolbarRepresenter, EditorCapability, TransactionCapabilityDecorator, + TransactionManager, TransactionService, CreateMenuController, LocatorController, @@ -320,7 +322,7 @@ define([ "implementation": TransactionCapabilityDecorator, "depends": [ "$q", - "transactionService" + "transactionManager" ], "priority": "fallback" }, @@ -405,6 +407,15 @@ define([ "key": "locator", "template": locatorTemplate } + ], + "services": [ + { + "key": "transactionManager", + "implementation": TransactionManager, + "depends": [ + "transactionService" + ] + } ] } }); diff --git a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js index 5071dc040a..f9cdc837a3 100644 --- a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js +++ b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js @@ -34,69 +34,37 @@ define( * called. * @memberof platform/commonUI/edit/capabilities * @param $q - * @param transactionService + * @param transactionManager * @param persistenceCapability * @param domainObject * @constructor */ function TransactionalPersistenceCapability( $q, - transactionService, + transactionManager, persistenceCapability, domainObject ) { - this.transactionService = transactionService; + this.transactionManager = transactionManager; this.persistenceCapability = persistenceCapability; this.domainObject = domainObject; this.$q = $q; } - TransactionalPersistenceCapability.prototype.transactionClearer = function (state) { - var id = this.domainObject.getId(); - if (arguments.length > 0) { - TRANSACTION_SET[id] = state; - } - return TRANSACTION_SET[id]; - }; - /** * The wrapped persist function. If a transaction is active, persist * will be queued until the transaction is committed or cancelled. * @returns {*} */ TransactionalPersistenceCapability.prototype.persist = function () { - var self = this; + var wrappedPersistence = this.persistenceCapability; - function onCommit() { - if (!self.transactionClearer()) { - return Promise.resolve(true); - } - return self.persistenceCapability.persist().then(function (result) { - self.transactionClearer(undefined); - return result; - }); - } - - function onCancel() { - if (self.domainObject.getModel().persisted !== undefined) { - //Fetch clean model from persistence - return self.persistenceCapability.refresh().then(function (result) { - self.transactionClearer(undefined); - return result; - }); - } else { - self.transactionClearer(undefined); - self.removeFromTransaction = undefined; - //Model is undefined in persistence, so return undefined. - return self.$q.when(undefined); - } - } - - if (this.transactionService.isActive()) { - if (!self.transactionClearer()) { - this.transactionClearer(this.transactionService - .addToTransaction(onCommit, onCancel)); - } + if (this.transactionManager.isActive()) { + this.transactionManager.addToTransaction( + this.domainObject, + wrappedPersistence.persist.bind(wrappedPersistence), + wrappedPersistence.refresh.bind(wrappedPersistence) + ); //Need to return a promise from this function return this.$q.when(true); } else { @@ -105,10 +73,7 @@ define( }; TransactionalPersistenceCapability.prototype.refresh = function () { - if (this.transactionClearer()) { - this.transactionClearer()(); - this.transactionClearer(undefined); - } + this.transactionManager.clearTransactionsFor(this.domainObject); return this.persistenceCapability.refresh(); }; diff --git a/platform/commonUI/edit/src/services/TransactionManager.js b/platform/commonUI/edit/src/services/TransactionManager.js new file mode 100644 index 0000000000..1855c438be --- /dev/null +++ b/platform/commonUI/edit/src/services/TransactionManager.js @@ -0,0 +1,74 @@ +/***************************************************************************** + * Open MCT, Copyright (c) 2014-2016, United States Government + * as represented by the Administrator of the National Aeronautics and Space + * Administration. All rights reserved. + * + * Open MCT is licensed under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0. + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + * Open MCT includes source code licensed under additional open source + * licenses. See the Open Source Licenses file (LICENSES.md) included with + * this source code distribution or the Licensing information page available + * at runtime from the About dialog for additional information. + *****************************************************************************/ + +define([], function () { + /** + * Manages transactions to support the TransactionalPersistenceCapability. + */ + function TransactionManager(transactionService) { + this.transactionService = transactionService; + this.clearTransactionFns = {}; + } + + TransactionManager.prototype.isActive = function () { + return this.transactionService.isActive(); + }; + + TransactionManager.prototype.isScheduled = function (domainObject) { + return !!this.clearTransactionFns[domainObject.getId()]; + }; + + TransactionManager.prototype.addToTransaction = function ( + domainObject, + onCommit, + onCancel + ) { + var release = this.releaseClearFn.bind(this, domainObject); + + function chain(promiseFn, nextFn) { + return function () { + return promiseFn().then(nextFn); + }; + } + + if (!this.isScheduled(domainObject)) { + this.clearTransactionFns[domainObject.getId()] = + this.transactionService.addToTransaction( + chain(onCommit, release), + chain(onCancel, release) + ); + } + }; + + TransactionManager.prototype.clearTransactionsFor = function (domainObject) { + if (this.isScheduled(domainObject)) { + this.clearTransactionFns[domainObject.getId()](); + this.releaseClearFn(domainObject); + } + }; + + TransactionManager.prototype.releaseClearFn = function (domainObject) { + delete this.clearTransactionFns[domainObject.getId()]; + }; + + return TransactionManager; +}); From 1f7cece8ec35427b07d04ef643d5ed63faddb7c1 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 14 Jul 2016 15:05:43 -0700 Subject: [PATCH 09/17] [Persistence] Remove unused variable --- .../edit/src/capabilities/TransactionalPersistenceCapability.js | 1 - 1 file changed, 1 deletion(-) diff --git a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js index f9cdc837a3..8445f2f467 100644 --- a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js +++ b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js @@ -24,7 +24,6 @@ define( [], function () { - var TRANSACTION_SET = {}; /** * Wraps persistence capability to enable transactions. Transactions From 86fcf190660bfd0bbd2d965b031c05c09bfe43e6 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 14 Jul 2016 16:14:20 -0700 Subject: [PATCH 10/17] [Persistence] Update Save As spec ...to account for asynchrony in test case due to usage of Promise.all --- .../commonUI/edit/test/actions/SaveAsActionSpec.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/platform/commonUI/edit/test/actions/SaveAsActionSpec.js b/platform/commonUI/edit/test/actions/SaveAsActionSpec.js index dbdd1f9692..4ad8ffa215 100644 --- a/platform/commonUI/edit/test/actions/SaveAsActionSpec.js +++ b/platform/commonUI/edit/test/actions/SaveAsActionSpec.js @@ -179,9 +179,15 @@ define( }); it("hides the blocking dialog after saving", function () { - action.perform(); + var mockCallback = jasmine.createSpy(); + action.perform().then(mockCallback); expect(mockDialogService.showBlockingMessage).toHaveBeenCalled(); - expect(mockDialogService.dismiss).toHaveBeenCalled(); + waitsFor(function () { + return mockCallback.calls.length > 0; + }); + runs(function () { + expect(mockDialogService.dismiss).toHaveBeenCalled(); + }); }); }); From 7c865f87be109c07fecb38100a8794239f784707 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 14 Jul 2016 16:20:29 -0700 Subject: [PATCH 11/17] [Persistence] Update transactional persistence spec ...to account for changes to separate out transaction management, including removal of specs made obsolete (as tested behavior has been moved to TransactionManager or was already redundant to behavior from the undecorated PersistenceCapability) --- .../TransactionalPersistenceCapabilitySpec.js | 47 ++++++------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js b/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js index 1157ba5892..b1dc24a718 100644 --- a/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js +++ b/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js @@ -37,7 +37,7 @@ define( describe("The transactional persistence decorator", function () { var mockQ, - mockTransactionService, + mockTransactionManager, mockPersistence, mockDomainObject, capability; @@ -47,13 +47,13 @@ define( mockQ.when.andCallFake(function (val) { return fastPromise(val); }); - mockTransactionService = jasmine.createSpyObj( + mockTransactionManager = jasmine.createSpyObj( "transactionService", - ["isActive", "addToTransaction"] + ["isActive", "addToTransaction", "clearTransactionsFor"] ); mockPersistence = jasmine.createSpyObj( "persistenceCapability", - ["persist", "refresh"] + ["persist", "refresh", "getSpace"] ); mockPersistence.persist.andReturn(fastPromise()); mockPersistence.refresh.andReturn(fastPromise()); @@ -66,49 +66,32 @@ define( ); mockDomainObject.getModel.andReturn({persisted: 1}); - capability = new TransactionalPersistenceCapability(mockQ, mockTransactionService, mockPersistence, mockDomainObject); + capability = new TransactionalPersistenceCapability( + mockQ, + mockTransactionManager, + mockPersistence, + mockDomainObject + ); }); it("if no transaction is active, passes through to persistence" + " provider", function () { - mockTransactionService.isActive.andReturn(false); + mockTransactionManager.isActive.andReturn(false); capability.persist(); expect(mockPersistence.persist).toHaveBeenCalled(); }); it("if transaction is active, persist and cancel calls are" + " queued", function () { - mockTransactionService.isActive.andReturn(true); + mockTransactionManager.isActive.andReturn(true); capability.persist(); - expect(mockTransactionService.addToTransaction).toHaveBeenCalled(); - mockTransactionService.addToTransaction.mostRecentCall.args[0](); + expect(mockTransactionManager.addToTransaction).toHaveBeenCalled(); + mockTransactionManager.addToTransaction.mostRecentCall.args[1](); expect(mockPersistence.persist).toHaveBeenCalled(); - mockTransactionService.addToTransaction.mostRecentCall.args[1](); + mockTransactionManager.addToTransaction.mostRecentCall.args[2](); expect(mockPersistence.refresh).toHaveBeenCalled(); }); - it("if transaction is active, cancel call is queued that refreshes model when appropriate", function () { - mockTransactionService.isActive.andReturn(true); - capability.persist(); - expect(mockTransactionService.addToTransaction).toHaveBeenCalled(); - - mockDomainObject.getModel.andReturn({}); - mockTransactionService.addToTransaction.mostRecentCall.args[1](); - expect(mockPersistence.refresh).not.toHaveBeenCalled(); - - mockDomainObject.getModel.andReturn({persisted: 1}); - mockTransactionService.addToTransaction.mostRecentCall.args[1](); - expect(mockPersistence.refresh).toHaveBeenCalled(); - }); - - it("persist call is only added to transaction once", function () { - mockTransactionService.isActive.andReturn(true); - capability.persist(); - expect(mockTransactionService.addToTransaction).toHaveBeenCalled(); - capability.persist(); - expect(mockTransactionService.addToTransaction.calls.length).toBe(1); - - }); }); } From 11a2fbacb49cb7fb1fa3ce3c6a7c590b019b4b9b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 14 Jul 2016 16:22:25 -0700 Subject: [PATCH 12/17] [Persistence] Add test cases Adds test cases for TransactionalPersistenceCapability which brings coverage up to 100%. --- .../TransactionalPersistenceCapabilitySpec.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js b/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js index b1dc24a718..b8d06b3abf 100644 --- a/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js +++ b/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js @@ -92,6 +92,18 @@ define( expect(mockPersistence.refresh).toHaveBeenCalled(); }); + it("wraps getSpace", function () { + mockPersistence.getSpace.andReturn('foo'); + expect(capability.getSpace()).toEqual('foo'); + }); + + it("clears transactions and delegates refresh calls", function () { + capability.refresh(); + expect(mockTransactionManager.clearTransactionsFor) + .toHaveBeenCalledWith(mockDomainObject); + expect(mockPersistence.refresh) + .toHaveBeenCalled(); + }); }); } From 31264aaddabfd65828101c48b0140cbc4947bcd0 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 14 Jul 2016 16:28:27 -0700 Subject: [PATCH 13/17] [Persistence] Begin testing TransactionManager --- .../test/services/TransactionManagerSpec.js | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 platform/commonUI/edit/test/services/TransactionManagerSpec.js diff --git a/platform/commonUI/edit/test/services/TransactionManagerSpec.js b/platform/commonUI/edit/test/services/TransactionManagerSpec.js new file mode 100644 index 0000000000..17ecf68ccf --- /dev/null +++ b/platform/commonUI/edit/test/services/TransactionManagerSpec.js @@ -0,0 +1,61 @@ +/***************************************************************************** + * Open MCT, Copyright (c) 2014-2016, United States Government + * as represented by the Administrator of the National Aeronautics and Space + * Administration. All rights reserved. + * + * Open MCT is licensed under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0. + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + * Open MCT includes source code licensed under additional open source + * licenses. See the Open Source Licenses file (LICENSES.md) included with + * this source code distribution or the Licensing information page available + * at runtime from the About dialog for additional information. + *****************************************************************************/ +/*global define,describe,it,expect,beforeEach,jasmine*/ + +define( + ["../../src/services/TransactionManager"], + function (TransactionManager) { + describe("TransactionManager", function () { + var mockTransactionService, + mockOnCommit, + mockOnCancel, + mockRemoves, + manager; + + beforeEach(function () { + mockRemoves = []; + mockTransactionService = jasmine.createSpyObj( + "transactionService", + [ "addToTransaction", "isActive" ] + ); + mockOnCommit = jasmine.createSpy('commit'); + mockOnCancel = jasmine.createSpy('cancel'); + + mockTransactionService.addToTransaction.andCallFake(function () { + var mockRemove = + jasmine.createSpy('remove-' + mockRemoves.length); + mockRemoves.push(mockRemove); + return mockRemove; + }); + + manager = new TransactionManager(mockTransactionService); + }); + + it("delegates isActive calls", function () { + [false, true].forEach(function (state) { + mockTransactionService.isActive.andReturn(state); + expect(manager.isActive()).toBe(state); + }); + }); + }); + } +); From 99227d2e42fde52bfa0b54ab6144e503311f163b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 14 Jul 2016 16:39:50 -0700 Subject: [PATCH 14/17] [Persistence] Finish testing TransactionManager --- .../test/services/TransactionManagerSpec.js | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/platform/commonUI/edit/test/services/TransactionManagerSpec.js b/platform/commonUI/edit/test/services/TransactionManagerSpec.js index 17ecf68ccf..628596c390 100644 --- a/platform/commonUI/edit/test/services/TransactionManagerSpec.js +++ b/platform/commonUI/edit/test/services/TransactionManagerSpec.js @@ -26,9 +26,11 @@ define( function (TransactionManager) { describe("TransactionManager", function () { var mockTransactionService, + mockDomainObject, mockOnCommit, mockOnCancel, mockRemoves, + mockPromise, manager; beforeEach(function () { @@ -39,6 +41,15 @@ define( ); mockOnCommit = jasmine.createSpy('commit'); mockOnCancel = jasmine.createSpy('cancel'); + mockDomainObject = jasmine.createSpyObj( + 'domainObject', + [ 'getId', 'getModel', 'getCapability' ] + ); + mockDomainObject.getId.andReturn('testId'); + mockPromise = jasmine.createSpyObj('promise', ['then']); + + mockOnCommit.andReturn(mockPromise); + mockOnCancel.andReturn(mockPromise); mockTransactionService.addToTransaction.andCallFake(function () { var mockRemove = @@ -56,6 +67,71 @@ define( expect(manager.isActive()).toBe(state); }); }); + + describe("when addToTransaction is called", function () { + beforeEach(function () { + manager.addToTransaction( + mockDomainObject, + mockOnCommit, + mockOnCancel + ); + }); + + it("adds callbacks to the active transaction", function () { + expect(mockTransactionService.addToTransaction) + .toHaveBeenCalledWith( + jasmine.any(Function), + jasmine.any(Function) + ); + }); + + it("invokes passed-in callbacks from its own callbacks", function () { + expect(mockOnCommit).not.toHaveBeenCalled(); + mockTransactionService.addToTransaction + .mostRecentCall.args[0](); + expect(mockOnCommit).toHaveBeenCalled(); + + expect(mockOnCancel).not.toHaveBeenCalled(); + mockTransactionService.addToTransaction + .mostRecentCall.args[1](); + expect(mockOnCancel).toHaveBeenCalled(); + }); + + it("ignores subsequent calls for the same object", function () { + manager.addToTransaction( + mockDomainObject, + jasmine.createSpy(), + jasmine.createSpy() + ); + expect(mockTransactionService.addToTransaction.calls.length) + .toEqual(1); + }); + + it("accepts subsequent calls for other objects", function () { + mockDomainObject.getId.andReturn('otherId'); + manager.addToTransaction( + mockDomainObject, + jasmine.createSpy(), + jasmine.createSpy() + ); + expect(mockTransactionService.addToTransaction.calls.length) + .toEqual(2); + }); + + it("does not remove callbacks from the transaction", function () { + expect(mockRemoves[0]).not.toHaveBeenCalled(); + }); + + describe("and clearTransactionsFor is subsequently called", function () { + beforeEach(function () { + manager.clearTransactionsFor(mockDomainObject); + }); + + it("removes callbacks from the transaction", function () { + expect(mockRemoves[0]).toHaveBeenCalled(); + }); + }); + }); }); } ); From d263b808101dba4a485a4eb770a2d4eb397ccf4d Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 14 Jul 2016 16:47:36 -0700 Subject: [PATCH 15/17] [Persistence] Add TransactionManager JSDoc Fixes #1059 (or, rather, concludes work on said fix) --- .../edit/src/services/TransactionManager.js | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/platform/commonUI/edit/src/services/TransactionManager.js b/platform/commonUI/edit/src/services/TransactionManager.js index 1855c438be..94914b9541 100644 --- a/platform/commonUI/edit/src/services/TransactionManager.js +++ b/platform/commonUI/edit/src/services/TransactionManager.js @@ -23,20 +23,48 @@ define([], function () { /** * Manages transactions to support the TransactionalPersistenceCapability. + * This assumes that all commit/cancel callbacks for a given domain + * object are equivalent, and only need to be added once to any active + * transaction. Violating this assumption may cause unexpected behavior. + * @constructor + * @memberof platform/commonUI/edit */ function TransactionManager(transactionService) { this.transactionService = transactionService; this.clearTransactionFns = {}; } + /** + * Check if a transaction is currently active. + * @returns {boolean} true if there is a transaction active + */ TransactionManager.prototype.isActive = function () { return this.transactionService.isActive(); }; + /** + * Check if callbacks associated with this domain object have already + * been added to the active transaction. + * @private + * @param {DomainObject} domainObject the object to check + * @returns {boolean} true if callbacks have been added + */ TransactionManager.prototype.isScheduled = function (domainObject) { return !!this.clearTransactionFns[domainObject.getId()]; }; + /** + * Add callbacks associated with this domain object to the active + * transaction. Both callbacks are expected to return promises that + * resolve when their associated behavior is complete. + * + * If callbacks associated with this domain object have already been + * added to the active transaction, this call will be ignored. + * + * @param {DomainObject} domainObject the associated domain object + * @param {Function} onCommit behavior to invoke when committing transaction + * @param {Function} onCancel behavior to invoke when cancelling transaction + */ TransactionManager.prototype.addToTransaction = function ( domainObject, onCommit, @@ -59,6 +87,11 @@ define([], function () { } }; + /** + * Remove any callbacks associated with this domain object from the + * active transaction. + * @param {DomainObject} domainObject the domain object + */ TransactionManager.prototype.clearTransactionsFor = function (domainObject) { if (this.isScheduled(domainObject)) { this.clearTransactionFns[domainObject.getId()](); @@ -66,6 +99,12 @@ define([], function () { } }; + /** + * Release the cached "remove from transaction" function that has been + * stored in association with this domain object. + * @param {DomainObject} domainObject the domain object + * @private + */ TransactionManager.prototype.releaseClearFn = function (domainObject) { delete this.clearTransactionFns[domainObject.getId()]; }; From 550e60455bea1ee531bdb867c282e71c0df42feb Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 14 Jul 2016 16:49:31 -0700 Subject: [PATCH 16/17] [Persistence] Fix code style --- .../commonUI/edit/test/services/TransactionManagerSpec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/platform/commonUI/edit/test/services/TransactionManagerSpec.js b/platform/commonUI/edit/test/services/TransactionManagerSpec.js index 628596c390..1fc22cc9d1 100644 --- a/platform/commonUI/edit/test/services/TransactionManagerSpec.js +++ b/platform/commonUI/edit/test/services/TransactionManagerSpec.js @@ -37,13 +37,13 @@ define( mockRemoves = []; mockTransactionService = jasmine.createSpyObj( "transactionService", - [ "addToTransaction", "isActive" ] + ["addToTransaction", "isActive"] ); mockOnCommit = jasmine.createSpy('commit'); mockOnCancel = jasmine.createSpy('cancel'); mockDomainObject = jasmine.createSpyObj( 'domainObject', - [ 'getId', 'getModel', 'getCapability' ] + ['getId', 'getModel', 'getCapability'] ); mockDomainObject.getId.andReturn('testId'); mockPromise = jasmine.createSpyObj('promise', ['then']); From eb6ddb5e45c734d346c9edfb9bff01927b0da03c Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 15 Jul 2016 12:23:33 -0700 Subject: [PATCH 17/17] [Persistence] Use ids from TransactionManager API Recommended during code review, https://github.com/nasa/openmct/pull/1084#discussion_r71021889 --- .../TransactionalPersistenceCapability.js | 5 +-- .../edit/src/services/TransactionManager.js | 32 +++++++++---------- .../TransactionalPersistenceCapabilitySpec.js | 10 +++--- .../test/services/TransactionManagerSpec.js | 17 ++++------ 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js index 8445f2f467..bcda009c56 100644 --- a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js +++ b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js @@ -60,7 +60,7 @@ define( if (this.transactionManager.isActive()) { this.transactionManager.addToTransaction( - this.domainObject, + this.domainObject.getId(), wrappedPersistence.persist.bind(wrappedPersistence), wrappedPersistence.refresh.bind(wrappedPersistence) ); @@ -72,7 +72,8 @@ define( }; TransactionalPersistenceCapability.prototype.refresh = function () { - this.transactionManager.clearTransactionsFor(this.domainObject); + this.transactionManager + .clearTransactionsFor(this.domainObject.getId()); return this.persistenceCapability.refresh(); }; diff --git a/platform/commonUI/edit/src/services/TransactionManager.js b/platform/commonUI/edit/src/services/TransactionManager.js index 94914b9541..d27ba95ced 100644 --- a/platform/commonUI/edit/src/services/TransactionManager.js +++ b/platform/commonUI/edit/src/services/TransactionManager.js @@ -46,11 +46,11 @@ define([], function () { * Check if callbacks associated with this domain object have already * been added to the active transaction. * @private - * @param {DomainObject} domainObject the object to check + * @param {string} id the identifier of the domain object to check * @returns {boolean} true if callbacks have been added */ - TransactionManager.prototype.isScheduled = function (domainObject) { - return !!this.clearTransactionFns[domainObject.getId()]; + TransactionManager.prototype.isScheduled = function (id) { + return !!this.clearTransactionFns[id]; }; /** @@ -61,16 +61,16 @@ define([], function () { * If callbacks associated with this domain object have already been * added to the active transaction, this call will be ignored. * - * @param {DomainObject} domainObject the associated domain object + * @param {string} id the identifier of the associated domain object * @param {Function} onCommit behavior to invoke when committing transaction * @param {Function} onCancel behavior to invoke when cancelling transaction */ TransactionManager.prototype.addToTransaction = function ( - domainObject, + id, onCommit, onCancel ) { - var release = this.releaseClearFn.bind(this, domainObject); + var release = this.releaseClearFn.bind(this, id); function chain(promiseFn, nextFn) { return function () { @@ -78,8 +78,8 @@ define([], function () { }; } - if (!this.isScheduled(domainObject)) { - this.clearTransactionFns[domainObject.getId()] = + if (!this.isScheduled(id)) { + this.clearTransactionFns[id] = this.transactionService.addToTransaction( chain(onCommit, release), chain(onCancel, release) @@ -90,23 +90,23 @@ define([], function () { /** * Remove any callbacks associated with this domain object from the * active transaction. - * @param {DomainObject} domainObject the domain object + * @param {string} id the identifier for the domain object */ - TransactionManager.prototype.clearTransactionsFor = function (domainObject) { - if (this.isScheduled(domainObject)) { - this.clearTransactionFns[domainObject.getId()](); - this.releaseClearFn(domainObject); + TransactionManager.prototype.clearTransactionsFor = function (id) { + if (this.isScheduled(id)) { + this.clearTransactionFns[id](); + this.releaseClearFn(id); } }; /** * Release the cached "remove from transaction" function that has been * stored in association with this domain object. - * @param {DomainObject} domainObject the domain object + * @param {string} id the identifier for the domain object * @private */ - TransactionManager.prototype.releaseClearFn = function (domainObject) { - delete this.clearTransactionFns[domainObject.getId()]; + TransactionManager.prototype.releaseClearFn = function (id) { + delete this.clearTransactionFns[id]; }; return TransactionManager; diff --git a/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js b/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js index b8d06b3abf..e5129f0055 100644 --- a/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js +++ b/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js @@ -40,9 +40,12 @@ define( mockTransactionManager, mockPersistence, mockDomainObject, + testId, capability; beforeEach(function () { + testId = "test-id"; + mockQ = jasmine.createSpyObj("$q", ["when"]); mockQ.when.andCallFake(function (val) { return fastPromise(val); @@ -60,11 +63,10 @@ define( mockDomainObject = jasmine.createSpyObj( "domainObject", - [ - "getModel" - ] + ["getModel", "getId"] ); mockDomainObject.getModel.andReturn({persisted: 1}); + mockDomainObject.getId.andReturn(testId); capability = new TransactionalPersistenceCapability( mockQ, @@ -100,7 +102,7 @@ define( it("clears transactions and delegates refresh calls", function () { capability.refresh(); expect(mockTransactionManager.clearTransactionsFor) - .toHaveBeenCalledWith(mockDomainObject); + .toHaveBeenCalledWith(testId); expect(mockPersistence.refresh) .toHaveBeenCalled(); }); diff --git a/platform/commonUI/edit/test/services/TransactionManagerSpec.js b/platform/commonUI/edit/test/services/TransactionManagerSpec.js index 1fc22cc9d1..1b616fc44b 100644 --- a/platform/commonUI/edit/test/services/TransactionManagerSpec.js +++ b/platform/commonUI/edit/test/services/TransactionManagerSpec.js @@ -26,7 +26,7 @@ define( function (TransactionManager) { describe("TransactionManager", function () { var mockTransactionService, - mockDomainObject, + testId, mockOnCommit, mockOnCancel, mockRemoves, @@ -41,11 +41,7 @@ define( ); mockOnCommit = jasmine.createSpy('commit'); mockOnCancel = jasmine.createSpy('cancel'); - mockDomainObject = jasmine.createSpyObj( - 'domainObject', - ['getId', 'getModel', 'getCapability'] - ); - mockDomainObject.getId.andReturn('testId'); + testId = 'test-id'; mockPromise = jasmine.createSpyObj('promise', ['then']); mockOnCommit.andReturn(mockPromise); @@ -71,7 +67,7 @@ define( describe("when addToTransaction is called", function () { beforeEach(function () { manager.addToTransaction( - mockDomainObject, + testId, mockOnCommit, mockOnCancel ); @@ -99,7 +95,7 @@ define( it("ignores subsequent calls for the same object", function () { manager.addToTransaction( - mockDomainObject, + testId, jasmine.createSpy(), jasmine.createSpy() ); @@ -108,9 +104,8 @@ define( }); it("accepts subsequent calls for other objects", function () { - mockDomainObject.getId.andReturn('otherId'); manager.addToTransaction( - mockDomainObject, + 'other-id', jasmine.createSpy(), jasmine.createSpy() ); @@ -124,7 +119,7 @@ define( describe("and clearTransactionsFor is subsequently called", function () { beforeEach(function () { - manager.clearTransactionsFor(mockDomainObject); + manager.clearTransactionsFor(testId); }); it("removes callbacks from the transaction", function () {