Merge pull request #4619 from AlexVelezLl/fix-undo-remove-last-child

Fix undo remove last child
learningequality · Aug 9, 2024 · a4a3c8f · a4a3c8f
2 parents 838fb3b + 1a73271
commit a4a3c8f
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 12 deletions.
diff --git a/contentcuration/contentcuration/frontend/shared/data/changes.js b/contentcuration/contentcuration/frontend/shared/data/changes.js
@@ -140,7 +140,7 @@ export class ChangeTracker {
     // We'll go through each change one by one and revert each.
     //
     // R. Tibbles: I think this could be done in two queries (TODO)
-    return promiseChunk(this._changes.reverse(), 1, ([change]) => {
+    return promiseChunk(this._changes.reverse(), 1, async ([change]) => {
       const resource = INDEXEDDB_RESOURCES[change.table];
       if (!resource) {
         if (process.env.NODE_ENV !== 'production') {
@@ -150,6 +150,16 @@ export class ChangeTracker {
         }
         return Promise.resolve();
       }
+
+      // Query siblings before starting the transaction
+      // to avoid any potential API call inside the transaction
+      let siblings;
+      if (change.type === CHANGE_TYPES.MOVED && change.oldObj) {
+        const { parent } = change.oldObj;
+        siblings = await resource.where({ parent }, false);
+        siblings = siblings.filter(sibling => sibling.id !== change.key);
+      }
+
       return resource.transaction({}, CHANGES_TABLE, () => {
         // If we had created something, we'll delete it
         // Special MOVED case here comes from the operation of COPY then MOVE for duplicating
@@ -170,17 +180,15 @@ export class ChangeTracker {
           const { parent, lft } = change.oldObj;
 
           // Collect the affected node's siblings prior to the change
-          return resource.where({ parent }, false).then(siblings => {
-            // Search the siblings ordered by `lft` to find the first a sibling
-            // where we should move the node, positioned before that sibling
-            const relativeSibling = sortBy(siblings, 'lft').find(sibling => sibling.lft >= lft);
-            if (relativeSibling) {
-              return resource.move(change.key, relativeSibling.id, RELATIVE_TREE_POSITIONS.LEFT);
-            }
-
-            // this handles if there were no siblings OR if the deleted node was at the end
-            return resource.move(change.key, parent, RELATIVE_TREE_POSITIONS.LAST_CHILD);
-          });
+          // Search the siblings ordered by `lft` to find the first a sibling
+          // where we should move the node, positioned before that sibling
+          const relativeSibling = sortBy(siblings, 'lft').find(sibling => sibling.lft >= lft);
+          if (relativeSibling) {
+            return resource.move(change.key, relativeSibling.id, RELATIVE_TREE_POSITIONS.LEFT);
+          }
+
+          // this handles if there were no siblings OR if the deleted node was at the end
+          return resource.move(change.key, parent, RELATIVE_TREE_POSITIONS.LAST_CHILD);
         } else {
           if (process.env.NODE_ENV !== 'production') {
             /* eslint-disable no-console */

diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js
@@ -1357,6 +1357,7 @@ export const ContentNode = new TreeResource({
         return Promise.all([getNode, this.where({ parent: parent.id }, false)]).then(
           ([node, siblings]) => {
             let lft = 1;
+            siblings = siblings.filter(s => s.id !== id);
             if (siblings.length) {
               // If we're creating, we don't need to worry about passing the ID
               lft = this.getNewSortOrder(isCreate ? null : id, target, position, siblings);