-
Notifications
You must be signed in to change notification settings - Fork 53
Studio: Ensure the site can't be deleted when syncing #2254
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
Studio: Ensure the site can't be deleted when syncing #2254
Conversation
📊 Performance Test ResultsComparing d8fe9dc vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change |
ivan-ottinger
left a comment
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.
Thank you for the fix, Kat! I have observed one inconsistency where the Delete site... button in the Settings tab is disabled for the whole process, while the similar option in the sidebar dropdown gets disabled only for a small part of the process:
CleanShot.2025-12-17.at.13.42.06.mp4
👀 |
|
Great catch @ivan-ottinger - it should be fixed now. Would you be able to give it another try? Thank you! |
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.
Great catch @ivan-ottinger - it should be fixed now. Would you be able to give it another try? Thank you!
Thanks for the fix, Kat. The changes work as expected (bot delete buttons are now disabled in the same way). The code looks generally good to me. I have just shared one suggestion and noticed that tests are failing.
gcsecsey
left a comment
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.
Thanks @katinthehatsite for working on this! I tested both pushing and pulling and now I confirm not being able to delete the site for the whole duration, either from the sidebar or from the Settings tab.
CleanShot.2025-12-17.at.17.24.18.mp4
…local-site-cannot-be-deleted-during-sync
ivan-ottinger
left a comment
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.
Thanks for the updates, Kat! All is looking good to me now. ✅
Related issues
Fixes STU-1127
Proposed Changes
This PR ensures that the local site can not be deleted when syncing to WP.com (either pulling or pushing).
Testing Instructions
SynctabSettingstabDeletesite button is disabled and the site can not be deletedDelete sitebutton is disabledPre-merge Checklist