Data Layer: Allow setting apiNamespace in requests
#13022
Conversation
@see #10942 Replaces #11280 Certain API requests need to specify an `apiNamespace` query parameter which is used to dispatch the request on the WordPress.com servers to the right kind of API: WordPress.com API, WordPress wp-api, etc... This change introduces the new parameter and checks to make sure that we don't accidentally set both values. Previously in #10942 this duplication happened and all requests in the HTTP layer failed. This PR is much less ambitious than #11280 which builds a request validation service into the HTTP layer itself. This merely adds the parameter and safety check in `actions#http()` but that seems sufficient for now and it leaves further validation up to `wpcom.js` down the line. cc: @retrofox @gwwar @jake
dmsnell
added
the
[Status] Needs Review
| it( 'should set the apiVersion', () => { | ||
| const request = http( { apiVersion: 'v1' } ); | ||
|
|
||
| expect( request ).to.have.deep.property( version, 'v1' ); |
There was a problem hiding this comment.
i've never seen .deep.property before. very cool
There was a problem hiding this comment.
I'm preferring this in order to keep the test focused. only check for the thing that's under test and not everything else, also handle cases easily when the properties don't exist ("prop doesn't exist" rather than "threw an error")
There was a problem hiding this comment.
v1 isn't a valid version. should be just plain old 1. I know it doesn't really matter for the test, but it's misled me more than twice.
There was a problem hiding this comment.
thanks. I saw your comment then forgot about it. I'll try to update here in the tests before merging. it's as good of a PR as any I suppose to fix that.
| body, | ||
| method, | ||
| path, | ||
| query: method === 'GET' ? { ...query, ...version } : version, |
There was a problem hiding this comment.
this line isn't immediately obvious to me.
Cant a non GET request have a query?
matticbot
removed
the
[Status] Needs Review
See #10942
Replaces #11280
Certain API requests need to specify an
apiNamespacequery parameterwhich is used to dispatch the request on the WordPress.com servers to
the right kind of API: WordPress.com API, WordPress wp-api, etc...
This change introduces the new parameter and checks to make sure that we
don't accidentally set both values. Previously in #10942 this
duplication happened and all requests in the HTTP layer failed.
This PR is much less ambitious than #11280 which builds a request
validation service into the HTTP layer itself. This merely adds the
parameter and safety check in
actions#http()but that seems sufficientfor now and it leaves further validation up to
wpcom.jsdown the line.Testing
Nothing currently uses the
apiNamespacein the data layer (because this introduces it) but to test we should smoke-test the existing stuff. Reader tag lists and the plans page is a good test.cc: @retrofox @gwwar @samouri
P.S. It was my intention to build on the work @retrofox did in #11280 but I felt like the changes were much bigger than we needed and this is a nice incremental approach without introducing a lot more.