Avoid using auto-drafts for theme templates and template parts #27910
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -870 B (0%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
youknowriad
requested review from
ajitbohra,
nerrad,
spacedmonkey,
talldan and
TimothyBJacobs
as code owners
TimothyBJacobs
previously requested changes
Dec 29, 2020
There was a problem hiding this comment.
Left feedback mainly on the REST API controllers. A number of issues apply to both, I only commented on the template parts controller since that was first.
I also wonder if we should instead be using the correct edit_post etc... caps here and setting up the CPT to use edit_theme_options when registering. That way developers can give more fine-grained capabilities if needed. Cc @peterwilsoncc, @swissspidy.
Can templates / template parts not be deleted?
lib/full-site-editing/class-wp-rest-template-parts-controller.php
Outdated
Show resolved
Hide resolved
lib/full-site-editing/class-wp-rest-template-parts-controller.php
Outdated
Show resolved
Hide resolved
lib/full-site-editing/class-wp-rest-template-parts-controller.php
Outdated
Show resolved
Hide resolved
lib/full-site-editing/class-wp-rest-template-parts-controller.php
Outdated
Show resolved
Hide resolved
lib/full-site-editing/class-wp-rest-template-parts-controller.php
Outdated
Show resolved
Hide resolved
lib/full-site-editing/class-wp-rest-template-parts-controller.php
Outdated
Show resolved
Hide resolved
lib/full-site-editing/class-wp-rest-template-parts-controller.php
Outdated
Show resolved
Hide resolved
For me, |
I'll add this. I just didn't use it initially. |
I agree that's what Core should use under the hood. But if the controller itself uses |
mtias
reviewed
Dec 30, 2020
mtias
reviewed
Dec 30, 2020
mtias
reviewed
Dec 30, 2020
packages/block-library/src/template-part/edit/placeholder/index.js
Outdated
Show resolved
Hide resolved
mtias
reviewed
Dec 30, 2020
packages/block-library/src/template-part/edit/placeholder/index.js
Outdated
Show resolved
Hide resolved
mtias
reviewed
Dec 31, 2020
|
The REST API bits are looking good to me! Can we get some tests on this please? |
youknowriad
force-pushed
the
avoid/template-template-parts-ssync
branch
from
5d664da to
34b0c07
Compare
|
About the tests, this is actually already well covered with the e2e tests which is what allowed me to have confidence in such a big refactor. That said, some additional PHP specific tests won't hurt. I'll see what I can do. |
Yes, you will also need to:
|
|
Can I get a ✅ here? I'd like to land this before the next RC so it can be tested properly. There are still a few things to consider in follow-ups and specifically the removal of the "theme" attribute from the template and template parts files. |
ntsekouras
approved these changes
Jan 11, 2021
|
I hope I didn't miss anything, that's a big PR with a lot of reviews :P |
This was referenced Jan 11, 2021
6 tasks
This was referenced Jan 12, 2021
25 tasks
This was referenced Jan 15, 2021
6 tasks
7 tasks
7 tasks
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #27321
borrows ideas from #27624
This PR borrows some ideas from #27624 and tries to avoid using auto-drafts for the template and template-parts that come from the theme (unmodified). It is based on the following principle:
Based on this premise, it provides two abstraction layers: the
full-site-editing/block-templates.phpprovides simple functions to retrieve templates (or template parts) given a simple query, or an id. This abstract away the fact that template can come from a file or from a CPT depending on whether it's customized or not.On top of this "PHP" layer, it overrides the REST Controllers to provide endpoints for the templates and template parts. The client uses these endpoints similarly to how it's done today, the difference is that instead of using regular IDs, the actual ids are "theme+slug".
This proved to have a lot of advantages: