Skip to content
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

Jump to bottom

Fix undo remove last child #4619

Merged
merged 2 commits into from
Aug 9, 2024
Merged

Fix undo remove last child #4619

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba275eb
Fix undo remove last child
AlexVelezLl Jul 24, 2024
1a73271
Fix siblings fetch
AlexVelezLl Aug 5, 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
31 changes: 19 additions & 12 deletions contentcuration/contentcuration/frontend/shared/data/changes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand All @@ -150,6 +150,15 @@ 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);
}

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
Expand All @@ -170,17 +179,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 */
Expand Down