-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Edit Post: Prevent locking users in saving state when saving meta boxes fails #32485
Conversation
Size Change: +6 B (0%) Total Size: 1.03 MB
ℹ️ View Unchanged
|
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 fix 👍 This tests well for me!
One note though: I wonder if the fact that we need to dispatch a success action in the event of a failure isn't a signal that we need to address this at a separate level.
46bdafa
to
a6410ee
Compare
Thanks for the review, @tyxla
I was thinking the same while working on PR. But since we don't currently have a different state for failed meta box saving, all the new So decided to keep it simple for now. Happy to change the logic if there's a need for a separated |
a6410ee
to
3d97884
Compare
Thanks for the fix!
Maybe we could expand the semantics in the reducer: export function isSavingMetaBoxes( state = false, action ) {
switch ( action.type ) {
case 'REQUEST_META_BOX_UPDATES':
return true;
case 'META_BOX_UPDATES_SUCCESS':
+ case 'META_BOX_UPDATES_FAILURE':
return false;
default:
return state;
}
} And adjust the error handling accordingly: try {
yield apiFetch( ... ); // Save the metaboxes
yield controls.dispatch( editPostStore.name, 'metaBoxUpdatesSuccess' );
} catch ( e ) {
e; // Maybe inspect the error
yield controls.dispatch( editPostStore.name, 'metaBoxUpdatesFailure' );
} cc @youknowriad |
Thanks for the feedback, @mcsf. I can create a new PR for the update. |
Description
Changes logic to update the
isSavingMetaBoxes
state even if the saving meta boxes request fails.Fixes #6966.
Fixes #19780.
How has this been tested?
.maintenance
file in the WP root directory, with contents:Screenshots
CleanShot.2021-06-07.at.17.19.41.mp4
Types of changes
Bugfix
Checklist:
*.native.js
files for terms that need renaming or removal).