HTTP: Pass along the api version for all request methods

[blowery]

Previously, we would only pass it along for GET requests. This allows POST to also use api versions other than v1.

[matticbot]

Test live: https://calypso.live/?branch=fix/http/allow-apiversion-on-non-get

[samouri]

Why does the check for GET exist at all? Can't all requests (GET/POST/ etc ) use query params?

[blowery]

Why does the check for GET exist at all? Can't all request (GET/POST/ etc ) use query params?

They can indeed. I was trying to make the smallest diff, as I wasn't sure why we were excluding query params from non-GET requests.

[samouri]

I'm okay with shipping this as-is because it solves a current bug, but I'd like to ideally figure out the answer to #12839 (comment) from @dmsnell

[blowery]

I'm happy to wait on it, I have this patch on the PR I'm working on.

[dmsnell]

Looks good to me @blowery!

I tested the /plans data (it's a GET anyway) and it worked fine. I don't think we have any POSTs yet passing the apiVersion. note, however, that this will soon change as we need to also allow apiNamespace

[blowery]

@dmsnell there's one other quirk I uncovered while working on #12703; for a post, you must pass along a truthy body for the apiVersion to take. If you don't, compact ends up changing the parameter order to wpcom.js and things don't work. so

http( {
  method: 'POST',
  path: '/foo'
  apiVersion: '1.2',
} );

needs to be

http( {
  method: 'POST',
  path: '/foo'
  apiVersion: '1.2',
  body: {},
} );

to work as expected. Might want to address that when we add apiNamespace support.

[dmsnell]

Might want to address that when we add apiNamespace support.

thanks @blowery - it's possible that @donnapep might have addressed this too in one of her PRs but I can't remember for sure

[donnapep]

@blowery The change @dmsnell is referring to is PR #12135. It involved the removal of the passing of an empty object in the body param. POST wasn't working at all before that change. Thx.