wp_die() updates for the Posts component #9997
Conversation
…s related to the media 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. |
|
|
||
| if ( ! $tax ) { | ||
| wp_die( __( 'Invalid taxonomy.' ) ); | ||
| wp_die( __( 'Invalid taxonomy.' ), 400 ); |
There was a problem hiding this comment.
I wonder if this should be 404. 🤷♂️ Also, there's an instance of wp_die() below that's in your list on Trac but not the PR. (Line 23)
There was a problem hiding this comment.
Thanks I have put line 23 in to the PR now 😄
My reasoning for a 400 is because of how we are obtaining the taxonomy through the request params in src/wp-admin/admin.php
on line 149:
$taxnow = $_REQUEST['taxonomy'];
However I can see an argument for both 400 and 404 as I am not sure if this is just a case of "doing it wrong" or a missing taxonomy?
There was a problem hiding this comment.
I think either is fine and definitely better than the default 500.
src/wp-admin/edit-tags.php
Outdated
|
|
||
| if ( ! $term instanceof WP_Term ) { | ||
| wp_die( __( 'You attempted to edit an item that does not exist. Perhaps it was deleted?' ) ); | ||
| wp_die( __( 'You attempted to edit an item that does not exist. Perhaps it was deleted?' ), 403 ); |
There was a problem hiding this comment.
Is this a 403, 404, or 400 situation? The request for a non-existent term, coupled with the "does not exist" message, makes me think 404. (Edit: 404 would also match what you used below on line 180)
Co-authored-by: John Parris <public@johnparris.com>
…evelop into 64025-post/wp_die
| $post_type = $_GET['post_type']; | ||
| } else { | ||
| wp_die( __( 'Invalid post type.' ) ); | ||
| wp_die( __( 'Invalid post type.' ), 400 ); |
There was a problem hiding this comment.
This text is confusing
Essentially is the same as this other one
https://github.com/WordPress/wordpress-develop/pull/9997/files#diff-e16824dc0b95c8da5a7b198cac7bf89280d06180cefaf6da296ccf211671af77R22
Missing in the array search when calling get_*
Or both are 400 or both are 403.
There was a problem hiding this comment.
I think the text is fine for what's happening here. Even still, text changes would be out of scope for this ticket.
Not sure what you mean by array search @SirLouen. There's an in_array check above that compares to post types with show_ui set to true. Can you elaborate?
There was a problem hiding this comment.
I'm checking this better now, and I'm noticing that I reviewed this out of context.
CallumBW95
changed the title
| $post_type = $_GET['post_type']; | ||
| } else { | ||
| wp_die( __( 'Invalid post type.' ) ); | ||
| wp_die( __( 'Invalid post type.' ), 400 ); |
There was a problem hiding this comment.
I'm checking this better now, and I'm noticing that I reviewed this out of context.
3 participants
Update wp_die() to include status codes where appropriate within files related to the Posts component
Trac ticket: 64025
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.