#40919 closed enhancement (fixed)
REST API: Allow overriding `jQuery.ajax` calls from within wp-admin
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | 4.8 |
Component: | REST API | Keywords: | has-patch has-unit-tests commit |
Focuses: | javascript, administration, rest-api | Cc: |
Description
In WP 4.8, there are now 3 places where we call out to the REST API from within wp-admin (see attached patch for details):
- When viewing the settings for the video widget, to load a video preview.
- When editing the settings for a video widget in the media modal, to load a video preview.
- When embedding from a URL in the post editor, to load an oEmbed preview.
These all call jQuery.ajax
directly. We need to avoid this going forward because it is impossible to override. I am aware of one large, customized WordPress installation cough where we will need to put this change in place as a core hack. (jQuery has functionality to override AJAX request options but this is not sufficient due to the way cross-domain API requests work on WP.com.)
There are other benefits as well, such as testability (this wrapper will provide a cleaner way to mock these API responses) and extensibility (this will be a natural place to add a filter when #21170 lands).
Attachments (5)
Change History (24)
#1
@
8 years ago
- Keywords needs-refresh dev-feedback added
- Owner set to jnylen0
- Status changed from new to assigned
This ticket was mentioned in Slack in #core-customize by timmyc. View the logs.
8 years ago
#4
@
8 years ago
Right now, this patch is pretty useless, and is basically just abstraction for abstraction's sake (I get the need behind it, but there's already ways to do that).
That said, if it incorporated the nonce header for you automatically, I'd be happy to use it.
Not a big fan of the naming here though; RESTRequest
sounds like a class name to me. wp.api.request()
I think would be a better name, and fits with the existing wp.api
for the Backbone library. That said, it's also slightly confusing in that you'd have two scripts with an overlapping namespace. wp.api_request
or wp.rest_request
might be better there?
(We could also consider switching to use fetch()
with the polyfill for this...)
This ticket was mentioned in Slack in #core-restapi by jnylen. View the logs.
8 years ago
#6
@
8 years ago
- Keywords needs-refresh removed
40919.2.diff changes the name of the method to wp.restRequest
and adds in _wpnonce
to the options.data
that is passed along to the $.ajax
call.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
@
8 years ago
Pass nonce as header; use main wpApiSettings
variable; allow requests via path
/namespace
/endpoint
; add unit tests
#8
@
8 years ago
In 40919.3.diff:
- Rename to
wp.apiRequest
to match the existingwp.api
. - Use the main
window.wpApiSettings
variable to get the nonce and the root URL. - Pass the nonce as a header instead of a query string argument.
- Allow sending requests via
path
parameter (ornamespace
andendpoint
parameters) instead of requiring full URLs. This eliminates the need for plugins and themes to callrest_url
themselves. - Add QUnit tests.
I would still like to get this into 4.8.1 if possible. @westonruter @adamsilverstein are either of you available to take a look?
Future follow-up steps:
#9
@
8 years ago
@jnylen0 Instead of adding wp.apiRequest
via a newly-enqueued script, what about adding this as wp.api.request
and make it part of the existing wp-api.js
? This will be required for this to be part of 4.8.1 since minor releases can't introduce new files (does not apply to tests) due to how automatic updates work.
#10
@
8 years ago
The idea is that we can load wp.apiRequest
without loading all of wp.api
, for example in the existing wp-admin uses of the API, and probably in Gutenberg as well since we're not using Backbone otherwise.
I forgot about the new file rule. In that case I'd say this can wait until 4.9.
#11
@
8 years ago
- Keywords has-unit-tests needs-refresh added; dev-feedback removed
- Milestone changed from 4.8.1 to 4.9
Punting to 4.9 to enable adding a new file. Patch needs refresh for at the very least changing 4.8.1
to 4.9.0
. If there are no blockers, I plan to make that change and land this within the next week or two.
Edit: this could also use a test to ensure that pre-existing values for the X-WP-Nonce
header are preserved.
This ticket was mentioned in Slack in #core-restapi by jnylen. View the logs.
8 years ago
#14
@
8 years ago
- Keywords commit added
I plan to land this tomorrow. A further change for 4.9 will be to make wp-api.js
use this function. I'll create a separate ticket for that.
40919.diff is incomplete:
media-views.js
file but not the source filewp-includes/js/media/views/embed/link.js
.wp.RESTRequest
should depend on thewp-rest-request
script; however, we probably want to avoid clobbering an existingwp.RESTRequest
function.