unit tests passing for commit

lib/api.js

@@ -165,6 +165,54 @@ module.exports = function(self, options) {
     }
   };
 
+  // Repeat the most recent move operation in the page tree
+  // that took place for `from` in the locale of `to`
+  self.repeatMove = function(req, from, to, callback) {
+    if (!from.workflowMoved) {
+      return callback(null);
+    }
+    let targetId;
+    return async.series([
+      getTargetId,
+      move
+    ], function(err) {
+      if (err === 'notfound') {
+        self.apos.notify(req, 'The most recent move of the page in the tree could not be duplicated as part of the commit.');
+        err = null;
+      }
+      return callback(null);
+    });
+    function getTargetId(callback) {
+      return self.apos.docs.db.findOne({ workflowGuid: from.workflowMoved.target, workflowLocale: to.workflowLocale }, { _id: 1 }, function(err, info) {
+        if (err) {
+          return callback(err);
+        }
+        targetId = info._id;
+        if (!targetId) {
+          return callback('notfound');
+        }
+        return callback(null);
+      });
+    }
+    function move(callback) {
+      console.log(to._id, targetId, from.workflowMoved.position);
+      self.apos.pages.move(req,
+        to._id,
+        targetId,
+        from.workflowMoved.position,
+        {
+          criteria: {
+            workflowLocale: to.workflowLocale
+          },
+          filters: {
+            workflowLocale: to.workflowLocale
+          }
+        },
+        callback
+      );
+    }
+  };
+
   // Invokes resolveRelationships for all docs that have the
   // workflowResolveDeferred: true property, and removes the property.
   //
@@ -217,7 +265,8 @@ module.exports = function(self, options) {
       _.partial(self.resolveRelationships, req, to, to.workflowLocale),
       insertCommit,
       _.partial(self.updateViaManager, req, to),
-      clearSubmitted
+      move,
+      clearSubmittedAndMoved
     ], function(err) {
       return callback(err, commitId);
     });
@@ -230,15 +279,22 @@ module.exports = function(self, options) {
         return callback(null);
       });
     }
-    function clearSubmitted(callback) {
+    function clearSubmittedAndMoved(callback) {
       return self.apos.docs.db.update({
         _id: from._id
       }, {
         $unset: {
-          workflowSubmitted: 1
+          workflowSubmitted: 1,
+          workflowMovedIsNew: 1
         }
       }, callback);
     }
+    function move(callback) {
+      if (!from.workflowMovedIsNew) {
+        return callback(null);
+      }
+      return self.repeatMove(req, from, to, callback);
+    }
   };
 
   // Fetch the draft version of a doc, whose id is `id`, and also the live version of the same
@@ -725,6 +781,10 @@ module.exports = function(self, options) {
           }
           var _draft = self.apos.utils.clonePermanent(draft);
           var _live = self.apos.utils.clonePermanent(live);
+          if (_draft.workflowMovedIsNew) {
+            relatedModified.push(draft);
+            return callback(null);            
+          }
           self.deleteExcludedProperties(_draft);
           self.deleteExcludedProperties(_live);
           // Normalize false vs. undefined to prevent false positives
@@ -1667,7 +1727,7 @@ module.exports = function(self, options) {
         from = _.cloneDeep(commit.from);
         to = _.cloneDeep(commit.to);
 
-        return async.series([ getDraft, resolveToSource, applyPatch, resolveToDestination, update ], callback);
+        return async.series([ getDraft, resolveToSource, applyPatch, resolveToDestination, update, move ], callback);
 
         function getDraft(callback) {
           return self.findDocs(req, { workflowGuid: commit.workflowGuid }, locale).toObject(function(err, _draft) {
@@ -1723,8 +1783,17 @@ module.exports = function(self, options) {
           draft.workflowSubmitted = self.getWorkflowSubmittedProperty(req, { type: 'exported' });
           return self.apos.docs.update(req, draft, callback);
         }
+
+        function move(callback) {
+          if (!from.workflowMovedIsNew) {
+            return callback(null);
+          }
+          return self.repeatMove(req, from, draft, callback);
+        }
+
       }, callback);
     }
+
   };
 
   // Force export the doc with the given id to the given locales.
@@ -1837,6 +1906,10 @@ module.exports = function(self, options) {
           return self.apos.docs.update(req, draft, callback);
         }
 
+        function move(callback) {
+          return self.repeatMove(req, original, draft, callback);
+        }
+
       }, callback);
     }
   };

lib/implementation.js

@@ -133,7 +133,10 @@ module.exports = function(self, options) {
       'viewGroupsRemovedIds',
       'editUsersRemovedIds',
       'editGroupsRemovedIds',
-      'advisoryLock'
+      'advisoryLock',
+      // Handled separately, as it is an action to carry out
+      // at commit time
+      'workflowMoved'
     ];
 
     // Attachment fields themselves are not directly localized (they are not docs)

lib/modules/apostrophe-workflow-pages/index.js

@@ -200,7 +200,7 @@ module.exports = {
       });
     };
 
-    // On the initial invocation of `apos.pages.move`, modify the criteria and filters
+    // On invocation of `apos.pages.move`, modify the criteria and filters
     // to ensure only the relevant locale is in play
 
     var superBeforeMove = self.beforeMove;
@@ -209,9 +209,6 @@ module.exports = {
         if (err) {
           return callback(err);
         }
-        if (options.workflowRecursing) {
-          return callback(null);
-        }
         if (moved.workflowLocale) {
           options.criteria = _.assign({}, options.criteria || {}, { workflowLocale: moved.workflowLocale });
           options.filters = _.assign({}, options.filters || {}, { workflowLocale: moved.workflowLocale });
@@ -220,64 +217,29 @@ module.exports = {
       });
     };
 
-    // After a page is moved in one locale, with all of the ripple effects that go with it,
-    // make the same move in all other locales. Note that we already verified we have permissions
-    // across all locales.
+    // After a page is moved in one locale, record the action that
+    // was taken so it can be repeated on a commit or export
+    // without attempting to reconcile differences in where the
+    // destination parent page happens to be at the start
+    // of the operation.
 
     var superAfterMove = self.afterMove;
     self.afterMove = function(req, moved, info, callback) {
       return superAfterMove(req, moved, info, function(err) {
         if (err) {
           return callback(err);
         }
-        if (info.options.workflowRecursing) {
-          return callback(null);
-        }
-        if (!moved.workflowGuid) {
-          return callback(null);
-        }
-        var locales = {};
-        return async.series([
-          get,
-          invoke
-        ], function(err) {
-          return callback(err);
-        });
-        function get(callback) {
-          // Locate the doc moved and the doc it is moved relative to (target) in all locales other than
-          // the original one
-          var workflow = self.apos.modules['apostrophe-workflow'];
-          return self.apos.docs.db.findWithProjection({ workflowGuid: { $in: [ moved.workflowGuid, info.target.workflowGuid ] }, workflowLocale: { $ne: moved.workflowLocale } }, { workflowGuid: 1, _id: 1, workflowLocale: 1 }).toArray(function(err, docs) {
-            if (err) {
-              return callback(err);
-            }
-            _.each(docs, function(doc) {
-              if (!workflow.locales[doc.workflowLocale]) {
-                // Ignore orphan locales no longer in the configuration, they might
-                // not exist for all of the documents
-                return;
-              }
-              locales[doc.workflowLocale] = locales[doc.workflowLocale] || {};
-              if (doc.workflowGuid === moved.workflowGuid) {
-                locales[doc.workflowLocale].movedId = doc._id;
-              }
-              if (doc.workflowGuid === info.target.workflowGuid) {
-                locales[doc.workflowLocale].targetId = doc._id;
-              }
-            });
-            return callback(null);
-          });
-        }
-        function invoke(callback) {
-          // Reinvoke apos.pages.move
-          return async.eachSeries(_.keys(locales), function(locale, callback) {
-            var _options = _.clone(info.options);
-            _options.criteria = _.assign({}, info.options.criteria || {}, { workflowLocale: locale });
-            _options.filters = _.assign({}, info.options.filters || {}, { workflowLocale: locale });
-            _options.workflowRecursing = true;
-            return self.move(req, locales[locale].movedId, locales[locale].targetId, info.position, _options, callback);
-          }, callback);
-        }
+        return self.apos.docs.db.update({
+          _id: moved._id
+        }, {
+          $set: {
+            workflowMoved: {
+              target: info.target.workflowGuid,
+              position: info.position
+            },
+            workflowMovedIsNew: true
+          }
+        }, callback);
       });
     };