This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Fix reverting customisations by refactoring pre_get_block_file_template #5746
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
tjcafferkey
added
focus: FSE
rubikuserbot
requested review from
a team and
gigitux
and removed request for
a team
tjcafferkey
requested review from
tomasztunik,
opr and
Aljullu
and removed request for
gigitux
|
Size Change: +364 B (0%) Total Size: 813 kB
ℹ️ View Unchanged
|
tjcafferkey
changed the title
|
Noticed one little glitch, but not related to the changes here. If we revert multiple templates from the templates list we'll get multiple notifications/flash messages without any context when entering one of the pages creating more noise than providing information - esp. with toast notifications in bottom left.
|
|
There is another glitch where the "Clear Customizations" is not present in the dropdown. But also not related to this change - It can happen when:
Going to templates lists and back fixes it and button is available again. Clear customisations on the templates list from the |
tomasztunik
approved these changes
Feb 10, 2022
There was a problem hiding this comment.
Looks and works great.
The refactor was a great call and the BlockTemplatesController is much easier to follow now, highlights edge cases much better and is better aligned with how Gutenberg handles the get_block_file_template flow.
With handling for deprecated slugs and template fallbacks don't think there is anything else there to add 🎉
Confirmed the "Clear customisations" to be on our templates only apparently as per updated comment above - so there is something extra there. Could you evaluate the glitches we've seen and create follow up issues where it makes sense? 🙇
|
Thank you for the thorough testing and feedback on this PR @tomasztunik and @frontdevde !
Yep, having looked at the issues you've raised I believe these should be logged in the Gutenberg repo as issues, I'll add it to be to do list to do this. Thanks for the detailed descriptions and highlighting these. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #5704
Description
pre_get_block_file_templatein BlockTemplateController.gutenberg_prefix fromgutenberg_create_new_block_template_objectas it's confusing and unncessary.get_single_block_templateas it's no longer being used.woocommerceinto a const value alongside the correct one.trunkand the latest version of WooCommerce (6.2 at the time of writing)Context
There are two methods used to get block templates in Gutenberg, they are
get_block_templateandget_block_file_template.get_block_template- Will query the database for a customised post, and if one does not exist it will then callget_block_file_templateget_block_file_template- Will load the template from the themes file system and return aWP_Templateobject.When we revert a template, we pass specific query params with the request which lands us here calling the
get_block_file_template. This will ultimately fail because of this check as$themefor our template iswoocommerce/woocommercewhich does not equalwp_get_theme()->get_stylesheet()which would be the active themes id (e.g.twentytwentytwo). This was happening because we were using cores method recursively on itself (but removing hooks so we didn't end up with an infinite loop), and core does not currently account for templates being loaded from plugins like WooCommerce, only themes. So we needed to refactor this.With all of the above in mind we can conclude that
pre_get_block_file_templatefilter only runs when its retrieving the original block template fileWP_Templateobject from the filesystem (withinget_block_file_template). Knowing this I have refactored the callback to only do this for our block templates, but to also consider the edge case of a theme having aproduct-archive.htmltemplate file only, which would also be used for Product Category and Tag templates.Testing
Automated Tests
Manual Testing
How to test the changes in this Pull Request:
single-product.html) into your theme or download and install this version of TwentyTwentyTwo and test steps 1-10 again.Please also do some regression testing of your own around the feature with and without WooCommerce templates within the theme as this is a significant refactor to fix this bug and we don't have the luxury of a test suite as of yet.
Video of testing steps
revert-customizations-fix.mp4
User Facing Testing
These are steps for user testing (where "user" is someone interacting with this change that is not editing any code).
Changelog