-
Notifications
You must be signed in to change notification settings - Fork 3.3k
wp_die() updates for the REST API component #10015
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
base: trunk
Are you sure you want to change the base?
Conversation
…s related to the REST API component
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
…op into 64032-rest_api/wp_die
|
Just noting that I've eyeballed these changes, but I'm setting up to test to ensure no ajax calls are broken with the new response codes. I probably won't have this completed until tomorrow. |
…nd, as they were backwards
mindctrl
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.
Logging this first pass of results as we work through them together in Zoom.
|
|
||
| if ( ! current_user_can( 'delete_term', $tag_id ) ) { | ||
| wp_die( -1 ); | ||
| wp_die( -1, 403 ); |
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.
Confirmed this works.
|
|
||
| if ( wp_delete_term( $tag_id, $taxonomy ) ) { | ||
| wp_die( 1 ); | ||
| wp_die( 1, 200 ); |
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.
Confirmed this works.
|
|
||
| if ( ! $wp_list_table->ajax_user_can() ) { | ||
| wp_die( -1 ); | ||
| wp_die( -1, 403 ); |
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.
Confirmed this works in the Comments list as expected.
I see references to the fetch-list action in WP_Themes_List_Table and WP_Theme_Install_List_Table, but I don't see any AJAX calls being made to these classes, but maybe I'm missing it?
| function wp_ajax_wp_compression_test() { | ||
| if ( ! current_user_can( 'manage_options' ) ) { | ||
| wp_die( -1 ); | ||
| wp_die( -1, 403 ); |
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.
✅ works as expected
| update_option( 'can_compress_scripts', 0, true ); | ||
| } | ||
| wp_die( 0 ); | ||
| wp_die( 0, 200 ); |
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.
✅ works as expected
| $post_id = (int) $_GET['postid']; | ||
| if ( empty( $post_id ) || ! current_user_can( 'edit_post', $post_id ) ) { | ||
| wp_die( -1 ); | ||
| wp_die( -1, 403 ); |
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.
✅ works as expected
|
|
||
| if ( ! stream_preview_image( $post_id ) ) { | ||
| wp_die( -1 ); | ||
| wp_die( -1, 403 ); |
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.
✅ works as expected
| } | ||
|
|
||
| wp_die(); | ||
| wp_die( '', 200 ); |
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.
✅ works as expected
|
|
||
| if ( ! empty( $_POST['add_new'] ) ) { | ||
| wp_die(); | ||
| wp_die( '', 400 ); |
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.
This breaks adding a widget to a widget area. It results in a spinning save icon.
|
|
||
| wp_set_sidebars_widgets( $sidebars ); | ||
| wp_die( 1 ); | ||
| wp_die( 1, 400 ); |
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.
This causes console errors when sorting widgets in a widget area.
| function wp_ajax_menu_locations_save() { | ||
| if ( ! current_user_can( 'edit_theme_options' ) ) { | ||
| wp_die( -1 ); | ||
| wp_die( -1, 403 ); |
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'm unable to test this manually, because I don't see where WP core uses this for anything.
I see the JS tries to work on elements nested within #nav-menu-theme-locations, but I don't see that ID anywhere in WP, or any other element/item with that value. I also looked for elements that might have been constructed in JS, but was unable to find any there either. It seems like old code.
Note: when grabbing the URL to the nav-menu.js code referenced in the previous paragraph, I noticed a code comment, from two weeks ago about #nav-menu-theme-locations no longer existing and that it was possibly removed here in June 2015.
With that in mind, I'll skip the changes in this function. Maybe we need another ticket to remove the old code, or figure out if that change introduced a bug of some kind because the JS wasn't updated - though I didn't notice anything while looking all through Appearance > Menus.
Update wp_die() to include status codes where appropriate within files related to the REST API component
Trac ticket: #64032
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.