Skip to content

Fix undo on copy #4774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rtibbles merged 2 commits into from
Sep 30, 2024
Merged

Fix undo on copy #4774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
962a7e3
Fixes undo copy snackbar button
akolson Sep 30, 2024
9a2afbb
Fixes failing tests
akolson Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
},
{
label: this.$tr('makeACopy'),
onClick: this.duplicateNode,
onClick: () => this.duplicateNode(this.nodeId),
condition: this.canEdit,
},
{
Expand Down Expand Up @@ -347,7 +347,7 @@
}
);
}),
duplicateNode: withChangeTracker(async function(changeTracker) {
duplicateNode: withChangeTracker(async function(nodeId, changeTracker) {
this.trackAction('Copy');
this.showSnackbar({
duration: null,
Expand All @@ -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,
});

Expand Down
3 changes: 3 additions & 0 deletions ...tcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/actions.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
11 changes: 8 additions & 3 deletions contentcuration/contentcuration/frontend/shared/data/resources.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down