Only persist latest mutated model. (#2295)

* Only persist latest mutated model. Fixes #2277

* Updated tests

* Fixed style issues

platform/commonUI/edit/src/services/TransactionManager.js

@@ -77,14 +77,19 @@ define([], function () {
                 return promiseFn().then(nextFn);
             };
         }
+        /**
+         * Clear any existing persistence calls for object with given ID. This ensures only the most recent persistence
+         * call is executed. This should prevent stale objects being persisted and overwriting fresh ones.
+         */
+        if (this.isScheduled(id)) {
+            this.clearTransactionsFor(id);
+        }
 
-        if (!this.isScheduled(id)) {
         this.clearTransactionFns[id] =
             this.transactionService.addToTransaction(
                 chain(onCommit, release),
                 chain(onCancel, release)
             );
-        }
     };
 
     /**

platform/commonUI/edit/test/services/TransactionManagerSpec.js

@@ -93,24 +93,33 @@ define(
                     expect(mockOnCancel).toHaveBeenCalled();
                 });
 
-                it("ignores subsequent calls for the same object", function () {
+                describe("Adds callbacks to transaction", function () {
+                    beforeEach(function () {
+                        spyOn(manager, 'clearTransactionsFor');
+                        manager.clearTransactionsFor.and.callThrough();
+                    });
+
+                    it("and clears pending calls if same object", function () {
                         manager.addToTransaction(
                             testId,
                             jasmine.createSpy(),
                             jasmine.createSpy()
                         );
-                    expect(mockTransactionService.addToTransaction.calls.count())
+                        expect(manager.clearTransactionsFor).toHaveBeenCalledWith(testId);
-                        .toEqual(1);
                     });
 
-                it("accepts subsequent calls for other objects", function () {
+                    it("and does not clear pending calls if different object", function () {
                         manager.addToTransaction(
                             'other-id',
                             jasmine.createSpy(),
                             jasmine.createSpy()
                         );
-                    expect(mockTransactionService.addToTransaction.calls.count())
+                        expect(manager.clearTransactionsFor).not.toHaveBeenCalled();
-                        .toEqual(2);
+                    });
+
+                    afterEach(function () {
+                        expect(mockTransactionService.addToTransaction.calls.count()).toEqual(2);
+                    });
                 });
 
                 it("does not remove callbacks from the transaction", function () {

platform/core/src/runs/TransactingMutationListener.js

@@ -43,20 +43,10 @@ define([], function () {
         var mutationTopic = topic('mutation');
         mutationTopic.listen(function (domainObject) {
             var persistence = domainObject.getCapability('persistence');
-            var wasActive = transactionService.isActive();
             cacheService.put(domainObject.getId(), domainObject.getModel());
 
             if (hasChanged(domainObject)) {
-
-                if (!wasActive) {
-                    transactionService.startTransaction();
-                }
-
                 persistence.persist();
-
-                if (!wasActive) {
-                    transactionService.commit();
-                }
             }
         });
     }

platform/core/test/runs/TransactingMutationListenerSpec.js

@@ -83,44 +83,27 @@ define(
                     .toHaveBeenCalledWith(jasmine.any(Function));
             });
 
-            [false, true].forEach(function (isActive) {
+            it("calls persist if the model has changed", function () {
-                var verb = isActive ? "is" : "isn't";
+                mockModel.persisted = Date.now();
 
-                function onlyWhenInactive(expectation) {
+                //Mark the model dirty by setting the mutated date later than the last persisted date.
-                    return isActive ? expectation.not : expectation;
+                mockModel.modified = mockModel.persisted + 1;
-                }
 
-                describe("when a transaction " + verb + " active", function () {
-                    var innerVerb = isActive ? "does" : "doesn't";
-
-                    beforeEach(function () {
-                        mockTransactionService.isActive.and.returnValue(isActive);
-                    });
-
-                    describe("and mutation occurs", function () {
-                        beforeEach(function () {
                 mockMutationTopic.listen.calls.mostRecent()
                     .args[0](mockDomainObject);
-                        });
 
-
-                        it(innerVerb + " start a new transaction", function () {
-                            onlyWhenInactive(
-                                expect(mockTransactionService.startTransaction)
-                            ).toHaveBeenCalled();
-                        });
-
-                        it("calls persist", function () {
                 expect(mockPersistence.persist).toHaveBeenCalled();
             });
 
-                        it(innerVerb + " immediately commit", function () {
+            it("does not call persist if the model has not changed", function () {
-                            onlyWhenInactive(
+                mockModel.persisted = Date.now();
-                                expect(mockTransactionService.commit)
+
-                            ).toHaveBeenCalled();
+                mockModel.modified = mockModel.persisted;
-                        });
+
-                    });
+                mockMutationTopic.listen.calls.mostRecent()
-                });
+                    .args[0](mockDomainObject);
+
+                expect(mockPersistence.persist).not.toHaveBeenCalled();
             });
         });
     }