Merge pull request #4774 from akolson/fix-undo-on-copy

Fix undo on copy
learningequality · Sep 30, 2024 · 16a3d4f · 16a3d4f
2 parents 681f20a + 9a2afbb
commit 16a3d4f
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 7 deletions.
diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue b/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue
@@ -124,7 +124,7 @@
             },
             {
               label: this.$tr('makeACopy'),
-              onClick: this.duplicateNode,
+              onClick: () => this.duplicateNode(this.nodeId),
               condition: this.canEdit,
             },
             {
@@ -347,7 +347,7 @@
           }
         );
       }),
-      duplicateNode: withChangeTracker(async function(changeTracker) {
+      duplicateNode: withChangeTracker(async function(nodeId, changeTracker) {
         this.trackAction('Copy');
         this.showSnackbar({
           duration: null,
@@ -358,8 +358,8 @@
           // actionCallback: () => changeTracker.revert(),
         });
         const copiedContentNode = await this.copyContentNode({
-          id: this.nodeId,
-          target: this.nodeId,
+          id: nodeId,
+          target: nodeId,
           position: RELATIVE_TREE_POSITIONS.RIGHT,
         });
 

diff --git a/...tcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/actions.spec.js b/...tcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/actions.spec.js
@@ -36,6 +36,9 @@ describe('contentNode actions', () => {
       jest
         .spyOn(ContentNode, 'fetchModel')
         .mockImplementation(() => Promise.resolve(contentNodeDatum));
+      jest
+        .spyOn(ContentNode, 'getAncestors')
+        .mockImplementation(() => Promise.resolve([contentNodeDatum]));
       return ContentNode._add({ title: 'notatest', parent: newId, lft: 2 }).then(() => {
         store = storeFactory({
           modules: {

diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js
@@ -1774,9 +1774,14 @@ export const ContentNode = new TreeResource({
    * @param {Function} updateCallback
    * @return {Promise<void>}
    */
-  updateAncestors({ id, includeSelf = false, ignoreChanges = false }, updateCallback) {
-    return this.transaction({ mode: 'rw' }, async () => {
-      const ancestors = await this.getAncestors(id);
+  async updateAncestors({ id, includeSelf = false, ignoreChanges = false }, updateCallback) {
+    // getAncestors invokes a non-Dexie API, so it must be called outside the transaction.
+    // Invoking it within a transaction can lead to transaction-related issues, including premature
+    // commit errors, which are a common problem when mixing non-Dexie API calls with transactions.
+    // See: https://dexie.org/docs/DexieErrors/Dexie.PrematureCommitError
+    const ancestors = await this.getAncestors(id);
+
+    return await this.transaction({ mode: 'rw' }, async () => {
       for (const ancestor of ancestors) {
         if (ancestor.id === id && !includeSelf) {
           continue;