This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Migrate REST API usage to the Store API #2282
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Conflict on initial commit 😆 |
|
@senadir Not marked for review yet, fixing tests. |
mikejolley
force-pushed
the
update/rest-api-overrides
branch
from
1d6d251 to
be3eadd
Compare
|
@senadir I'm going to do some more testing tomorrow and I had to comment out the tests for with-attributes - I redid the HOC with hooks and it isn't compatible with existing tests. Marking for review though. |
|
Size Change: -12.7 kB (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
senadir
approved these changes
Apr 24, 2020
| @@ -24,23 +20,21 @@ const getProductsRequests = ( { | |||
| queryArgs = [], | |||
| } ) => { | |||
| const defaultArgs = { | |||
| per_page: IS_LARGE_CATALOG ? 100 : -1, | |||
| per_page: IS_LARGE_CATALOG ? 100 : 0, | |||
There was a problem hiding this comment.
Nope. I noticed some functions treat 0 as "unlimited" and adopted that instead of -1 which also makes validation easier.
| /** | ||
| * ProductCategories class. | ||
| */ | ||
| class ProductCategories extends AbstractTermsRoute { |
There was a problem hiding this comment.
This class is not implementing the count property right.
in Master:
In this branch:
senadir
reviewed
Apr 24, 2020
| @@ -31,17 +31,36 @@ public function prepare_objects_query( $request ) { | |||
| 'paged' => $request['page'], | |||
| 'post__in' => $request['include'], | |||
| 'post__not_in' => $request['exclude'], | |||
| 'posts_per_page' => $request['per_page'], | |||
| 'posts_per_page' => $request['per_page'] ? $request['per_page'] : -1, | |||
There was a problem hiding this comment.
I know that if per_page is set to 0, this will evaluate to -1, but why does that? wouldn't make sense to let the client use the same logic as WP?
There was a problem hiding this comment.
it's easier to check ! per_page then set to -1 than handle 0 and -1 separately. The schema validates from 0 and has a message than leaving blank, or using 0, maps to unlimited.
Some functions in WP use 0 instead of -1 - I think the terms query object.
|
Good catch @senadir I am using the count from WP, but some counts enhance to include children terms. Will revise. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Upon reviewing the REST API overrides in blocks, to see what needed to be ported to the REST API itself, I found that most endpoints simply needed to expand access capabilities to authors/non-admin users.
As well as expanding access rights, some of the endpoints were then needing to shape data to hide private things, or return data in a specific opinionated format. This didn't seem like a good solution to me, so I worked through each endpoint to see if the Store API could be used instead. Store API is public, so there should be no auth issues in doing this.
I had success with the existing Store endpoints (products, attributes) so decided to port over some of the other endpoints too, creating new Store API endpoints for tags, categories, reviews all following similar practices and only allowing READ access to public data.
This PR contains that work :)
How to test the changes in this Pull Request: