Feature/rest api reordering post #74
Conversation
Bump WP tested-up-to version 5.8
jeffpaul
requested review from
a team and
cadic
and removed request for
jeffpaul and
a team
simple-page-ordering.php
Outdated
|
|
||
| // check and make sure we have what we need | ||
| if ( empty( $post_id ) || ( ! isset( $previd ) && ! isset( $nextid ) ) ) { | ||
| wp_send_json_error(); |
There was a problem hiding this comment.
REST Callback expected to return the result of the action instead of error processing or output inside the callback function. If an error happens, we need to return new WP_Error( ... )
Importantly, a REST API route’s callback should always return data; it shouldn’t attempt to send the response body itself.
simple-page-ordering.php
Outdated
|
|
||
| self::page_ordering( $post_id, $previd, $nextid, $start, $excluded ); | ||
|
|
||
| wp_send_json_error(); |
There was a problem hiding this comment.
After successful processing, REST endpoint callback expected to return WP_REST_Response
simple-page-ordering.php
Outdated
| $previd = empty( $previd ) ? false : (int) $previd; | ||
| $nextid = empty( $nextid ) ? false : (int) $nextid; | ||
| $start = empty( $start ) ? 1 : (int) $start; | ||
| $excluded = empty( $excluded ) ? array( $post_id ) : array_filter( (array) json_decode( $excluded ), 'intval' ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized |
There was a problem hiding this comment.
Is this second validation really necessary after validating $_POST arguments in ajax_simple_page_ordering()?
simple-page-ordering.php
Outdated
| $previd = empty( $_GET['previd'] ) ? false : (int) $_GET['previd']; | ||
| $nextid = empty( $_GET['nextid'] ) ? false : (int) $_GET['nextid']; | ||
| $start = empty( $_GET['start'] ) ? 1 : (int) $_GET['start']; | ||
| $excluded = empty( $_GET['excluded'] ) ? array( $_GET['id'] ) : array_filter( (array) json_decode( $_GET['excluded'] ), 'intval' ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized |
There was a problem hiding this comment.
REST arguments are passed to the callback as a single argument rest_page_ordering( WP_REST_Request $request ) and should be accessed as incoming array values, not from $_GET or $_POST.
There was a problem hiding this comment.
Also, many PHPCS rulesets advise using filter_input() instead of accessing request globals directly.
- Added PHPCS via composer - Added 10UP PHPCS ruleset - Clean up - page_ordering returns object or WP_Error - added REST responses
There was a problem hiding this comment.
@ciprianimike thank you for the update. There are couple minor changes left and maybe a merge conflict (version bumb and readme files)
readme.txt
Outdated
| @@ -3,7 +3,7 @@ Contributors: 10up, jakemgold, welcher, helen, thinkoomph | |||
| Donate link: http://10up.com/plugins/simple-page-ordering-wordpress/ | |||
| Tags: order, re-order, ordering, pages, page, manage, menu_order, hierarchical, ajax, drag-and-drop, admin | |||
| Requires at least: 3.8 | |||
| Tested up to: 5.8 | |||
| Tested up to: 5.7 | |||
There was a problem hiding this comment.
Do we really downshift this from 5.8? Also includes README.md, CREDITS.md and CHANGELOG.md
There was a problem hiding this comment.
merged develop into the feature branch, resolved conflicts and added documentation
simple-page-ordering.php
Outdated
| * @param int $previd The previous post ID. | ||
| * @param int $nextid The next post ID. | ||
| * @param int $start The start index. | ||
| * @param string $excluded Array of post IDs. |
There was a problem hiding this comment.
Doc type for $excluded seems to be array
|
@cadic got it, added parameters |
Description of the Change
The function
ajax_simple_page_ordering()is the callback function of the AJAX call, it was also responsible for saving the position of the posts.A refactor has been done, moving the sorting and posts updating part into a new
page_ordering()function.The REST endpoint has been added:
wp-json/simplepageordering/v1/page_ordering/The
rest_page_ordering()function has been added as callback for the new REST endpoint and it calls thepage_ordering()function.page_ordering()is therefore the part of the code where the sorting of post takes place and this is now called via AJAX or REST.Verification Process
after taking some of the code out of the ajax_simple_page_ordering () function, I made sure that the operation remained intact, verifying the parameters sent in the request before the refactor and replicating them after the refactor.
I then sent the same parameters through the new REST endpoint and verified that the reordering is consistent, the same parameters via AJAX or REST must lead to the usual sorting results.
Checklist:
Applicable Issues
#67