-
Notifications
You must be signed in to change notification settings - Fork 185
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.
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
Fix undo remove last child #4619
Fix undo remove last child #4619
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @AlexVelezLl!. Although the fix seems to work sometimes, the bug can still be reproduced.
Screen.Recording.2024-07-29.at.14.43.58.mov
@AlexVelezLl, if its useful, I picked up this in the terminal!
|
d7c55ad
to
ba275eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code changes here make sense to me, as they mostly amount to never including the node itself among its own siblings, which seems semantically correct!
If @akolson can confirm he is no long replicating the error he saw, then we should be good to merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can confirm that the issue has now been fully resolved. Manual QA checks out. Thanks @AlexVelezLl 🚀 . cc @rtibbles
Summary
Description of the change(s) you made
When removing the last child of a folder, the folder was left without nodes, and within the undo transaction there was a resource.where() filter to find the siblings of the recovered node, but as it did not find siblings, it was making an API call, which caused the transaction to end before the api call returned, while there were still operations to be done within the transaction.
This PR just relocates the siblings query so that it is done before the transaction.
Manual verification steps performed
Screenshots (if applicable)
Compartir.pantalla.-.2024-07-24.09_11_14.mp4
Reviewer guidance
How can a reviewer test these changes?
References
Closes #4468
Contributor's Checklist
PR process:
CHANGELOG
label been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocs
label has been added if this introduces a change that needs to be updated in the user docs?requirements.txt
files also included in this PRStudio-specifc:
notranslate
class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages
,components
, andlayouts
directories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarn
andpip
)