Templates Endpoint: Add resolved query arg to return only relevant templates
#21981
Conversation
ockham
changed the title
resolved query arg to return only relevant templates
|
Size Change: 0 B Total Size: 827 kB ℹ️ View Unchanged
|
ockham
force-pushed
the
add/templates-endpoint-resolved-arg
branch
5 times, most recently
from
772cff9 to
301ec43
Compare
lib/template-loader.php
Outdated
| // TODO: We could consider moving the following logic to a hook like `posts_results` that is run | ||
| // after query results have been fetched. That might allow us to absorb filtering logic | ||
| // (as e.g. found in `filter_rest_wp_template_query`) into the generic querying logic. |
There was a problem hiding this comment.
This comment is confusing. How would moving this into a hook allow for filter_rest_wp_template_query to be in the "generic querying logic."?
There was a problem hiding this comment.
What I meant is to move the auto-draft creation logic into a post_results filter, and to filter the results returned by the query to only return the template(s) that are relevant for the requested template hierarchy -- similar to what filter_rest_wp_template_query does in this PR.
There was a problem hiding this comment.
The auto-drafts are not created for the templates that are returned by the query. They are created for spots in the template hierarchy that don't have a returned template but have a theme file. You would have to create the auto-drafts before making the query so that they show up, but give preference to non-auto-drafts. That could work, but I doubt it will be more straightforward.
There was a problem hiding this comment.
Yeah, I think we're basically on the same page. <y thinking was basically:
- We tap into the
post_resultshook to watch out for a query for one (or many)wp_template(s), possibly with apost_nameorpost_name__inparam, akin to this:gutenberg/lib/template-loader.php
Lines 212 to 222 in 8676302
- In our filter, we create the auto-drafts from theme files, and if there's one that matches the template hierarchy better than our query result, we return that -- that's basically
gutenberg/lib/template-loader.php
Lines 224 to 280 in 8676302
While it mangles the query logic quite a bit, it's arguable that we'd pretty much always like to perform these steps whenever a template is queried (whether for rendering or through the REST API). I think it might be worth experimenting with.
There was a problem hiding this comment.
We could also just call the new functions directly for rendering.
lib/templates.php
Outdated
| // TODO: gutenberg_find_template_post_and_parts() internally performs a query. | ||
| // This means that there's room for optimization, and we might be able to | ||
| // generalize the logic found here into something that runs upon every `wp_template` | ||
| // query (not just those coming from the REST API). |
There was a problem hiding this comment.
Why would we want that? We still need a way to use the regular querying logic.
There was a problem hiding this comment.
I was thinking of adding some kind of flag, similar to the resolved bool we're adding to the REST API endpoint in this PR. The same technique might not carry over 1:1 to WP_Query, but maybe we could add e.g. a dummy meta field.
I think that we could consider manipulating all queries for wp_template. If you look at the querying and auto-draft creation logic in gutenberg_find_template_post_and_parts, and where it's used, it's arguable that we'll always want to create those auto-drafts when querying a wp_template.
There was a problem hiding this comment.
But, you need to query the templates to figure out which ones you need to create auto-drafts for. The only way I see to get around that is to generate auto-drafts for all files before queries are made. Still, then we have to make sure we always update them when the underlying files change and that we give priority to actual customized non-auto-draft counterparts. All in all, I don't see it being much more straightforward, and we introduce a lot of magic into the very standardized functionality of WP_Query.
There was a problem hiding this comment.
But, you need to query the templates to figure out which ones you need to create auto-drafts for. The only way I see to get around that is to generate auto-drafts for all files before queries are made.
That depends on the query, doesn't it? If there's no post_name or post_name__in param, then that's true -- but that means it'd be pretty similar to what we have now in edit-site-page.php or in the new REST API filter I'm adding here -- where we just loop over template hierarchies.
Still, then we have to make sure we always update them when the underlying files change and that we give priority to actual customized non-auto-draft counterparts.
That should be feasible by splitting what we now have in gutenberg_find_template_post_and_parts along sensible lines (like I sketched here), no?
All in all, I don't see it being much more straightforward, and we introduce a lot of magic into the very standardized functionality of
WP_Query.
Yeah, I think the magic vs. expected behavior is the biggest concern.
There was a problem hiding this comment.
Yeah, I don't see it as a definite net improvement.
We can explore it, but I wouldn't prioritize it.
|
Addressed feedback. If the TODO comments are the main concern, I'm happy to revert them. Also happy to demo what I meant with them in a separate PR (since I'd rather not block this one on it). |
Co-authored-by: Enrique Piqueras <[email protected]>
This reverts commit 6ca25eac8c060c69e32ff946fc586a2570aae749.
ockham
force-pushed
the
add/templates-endpoint-resolved-arg
branch
from
ed88f8c to
b0ea6f4
Compare
Description
Implements https://github.com/WordPress/gutenberg/pull/21877/files#r416075231 and unblocks #21877.
This PR changes
gutenberg_find_template_post_and_parts()to accept a$template_typearg, and make the$template_hierarchyarg optional.$template_hierarchywill be used as a hint if it's present (e.g. when called from a{$type}_templatehook); if it isn't, the template hierarchy will be looked up for$template_type.This allows writing the code in
edit-site-page.phpin a more succinct way. More importantly, it allows us to add arest_wp_template_queryfilter for thewp_templatecollection endpoint to accept aresolvedbool arg that will cause the endpoint to reflect the behavior of the template resolution algorithm that's used to populate the$settings[ 'templateIds' ](see):How has this been tested?
As always, make sure that the Full-Site Editing experiment and demo templates are enabled.
We'll compare the results the
wp_templatecollections REST API endpoint gives to$settings['templateIds']. To that end, apply the following patch and re-build the app (npm run build):Next, in wp-admin, navigate to the Appearance > Templates section, and create a few dummy templates whose names are not actually part of the template hierarchy (e.g.
some-templateetc).Now go to Full-Site Editing and open your browser console. You should see the output of the
settingsvariable there. Compare thetemplateIdsproperty to the output of the following commands you run in the console (twice each!). Icons ✔️ and ❌ indicate whether the output is expected to matchsettings.templateIds, or not.wp.data.select('core').getEntityRecords('postType', 'wp_template' )should return the list of allwp_templateCPTs, including those that are not part of the template hierarchy.wp.data.select('core').getEntityRecords('postType', 'wp_template', { resolved: true } )should return the list of templates that match the template hierarchy.wp.data.select('core').getEntityRecords('postType', 'wp_template', { slug: 'some-template' } )should return thewp_templatepart whose slug issome-template.wp.data.select('core').getEntityRecords('postType', 'wp_template', { slug: 'some-template', resolved: true } )should return an empty array, assome-templateis not part of the template hierarchy.Types of changes
Add an arg to an existing endpoint; plus a minor refactor.
Checklist: