-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Template Activation: Add more checks for active_templates #73519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| public function get_template_fallback( $request ) { | ||
| // Check active_templates experiment status. | ||
| if ( ! gutenberg_is_experiment_enabled( 'active_templates' ) ) { | ||
| return rest_ensure_response( new stdClass() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't we have to call the parent get_template_fallback method here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, we probably would. You mean like this: parent::get_template_fallback( $request )?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed this in 409c107.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| */ | ||
| function gutenberg_resolve_block_template( $template_type, $template_hierarchy, $fallback_template ) { | ||
| // Check active_templates experiment status. | ||
| if ( ! gutenberg_is_experiment_enabled( 'active_templates' ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this one is not needed, it's not a filter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, the fix still works without the check in gutenberg_resolve_block_template. Removed in 5d22b8b.
|
When the experiment is toggled off it's working great, but when it's on, there's too many "created templates" and it's impossible to delete them, they just come back. Looking into it now. |
|
The issue seems to happen in trunk too. I'll investigate separately and perhaps we also need that to be fixed in a point release 😬 |
|
Flaky tests detected in 5d22b8b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19637963501
|
Ah OK, if it's happening in trunk I guess we can handle it separately to this PR. I'm also struggling to reproduce this locally. With the experiment on, I only see one copy of the edited template under "Created templates". |
|
Actually, I had the collab experiment on by accident, which is breaking things, so all good in normal trunk 😃 |
|
I just cherry-picked this PR to the release/22.1 branch to get it included in the next release: ded4f61 |
* Add more checks for active_templates * Use get_template_fallback() * Remove check for gutenberg_resolve_block_template Co-authored-by: mikachan <mikachan@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
What?
Related to #73252
Adds more checks for the
active_templatesexperiment.Why? / How?
These runtime checks ensure that none of this code is run when the experiment is disabled.
Testing Instructions