#51722 closed enhancement (fixed)
Allow embedded links in preloaded api request
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | 5.0 |
Component: | REST API | Keywords: | good-first-bug has-patch |
Focuses: | rest-api | Cc: |
Description
Currently the rest_preload_api_request
function doesn't embed links / respect the ?_embed=
url param. This means that embedded links in the REST API can not be prefetched.
Change History (18)
#1
@
4 months ago
- Keywords good-first-bug added
- Milestone changed from Awaiting Review to Future Release
#2
@
3 months ago
I'm looking to jump in on this ticket but I'm trying to grok how/where this method is used for testing reasons. But as an FYI... I am looking to pick this ticket up.
#3
@
3 months ago
@technosailor great! In WordPress Core it is used to bootstrap the Block Editor. You can see that in action here: https://github.com/WordPress/wordpress-develop/blob/c869ef617e03677279ff64b68630da823275988e/src/wp-admin/edit-form-blocks.php#L79
This ticket was mentioned in PR #788 on WordPress/wordpress-develop by lukaspawlik.
3 months ago
- Keywords has-patch added; needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/51722
@spacedmonkey please find attached implementation of your idea from mentioned Trac ticket.
Please let me know if you would like me to modify this in any different way.
cc @TimothyBJacobs
#5
@
2 months ago
- Milestone changed from Future Release to 5.7
Thanks for picking this up @lukaspawlik! The approach looks right to me, would you be interested in adding tests for this?
#6
@
7 weeks ago
lukaspawlik commented on PR #788:
@TimothyBJacobs Accepted your suggestion. This seems to be ready.
#7
@
7 weeks ago
TimothyBJacobs commented on PR #788:
This looks great, thanks @lukaspawlik! Would you be interested in writing a test for this?
This ticket was mentioned in PR #879 on WordPress/wordpress-develop by spacedmonkey.
7 weeks ago
Adding a simple fix for 51722. The response_to_data
method to resolve the links and embed fields.
Trac ticket: https://core.trac.wordpress.org/ticket/51722
#9
@
7 weeks ago
@TimothyBlynJacobs I have created a simpler patch. If you seem to think that valid course, I will add some simple tests.
#10
@
7 weeks ago
Thanks for picking it up @spacedmonkey. Both approaches look pretty similar to me. Missing in the second PR is a call to rest_parse_embed_param
. I do like that in @lpawlik's patch we work off of the already parsed request object.
#11
@
7 weeks ago
For what is worth, we are currently using my patch on a very popular plugin called web stories. See our plugin. https://github.com/google/web-stories-wp/pull/5149
This was test and code reviewed by me and @swissspidy . There is a benefit to using existing code that others are using.
#12
@
7 weeks ago
spacedmonkey commented on PR #879:
@TimothyBJacobs I added unit tests now.
#13
@
7 weeks ago
- Keywords needs-testing-info added
Scheduling this ticket for Testing Scrub, but need more information for the testers to QA it:
- What are the steps to test the PR?
- Are there any testing dependencies such as a plugin or script?
- What is the expected behavior after applying the patch?
This ticket was mentioned in Slack in #core-restapi by spacedmonkey. View the logs.
7 weeks ago
#15
@
6 weeks ago
- Owner set to TimothyBlynJacobs
- Resolution set to fixed
- Status changed from new to closed
In 50005:
#16
@
6 weeks ago
TimothyBJacobs commented on PR #788:
Merged in https://github.com/WordPress/wordpress-develop/commit/bb395706f4ebe6efeca2a951fad69ba4d98fb799. Thanks again for the patch @lukaspawlik!
#17
@
6 weeks ago
TimothyBJacobs commented on PR #879:
Thanks for working on this, I combined this with @lukaspawlik's previous PR in https://github.com/WordPress/wordpress-develop/commit/bb395706f4ebe6efeca2a951fad69ba4d98fb799.
#18
@
6 weeks ago
- Keywords needs-testing needs-testing-info removed
@hellofromTonya I've removed needs-testing-info
since this is purely a developer facing change that wouldn't have any user-facing impact and I think we have good test coverage on the change. Feel free to re-add though if you feel differently.
This will require changing
rest_preload_api_request
to instead call$server->response_to_data()
instead of just calling$response->get_data()
directly. And pull_embed
from$query_params
and applyrest_parse_embed_param()
to it.