Skip to content
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

Fix block templates not being rendered in extension taxonomies #44850

Merged
merged 4 commits into from Mar 18, 2024

Conversation

Aljullu
Copy link
Contributor

@Aljullu Aljullu commented Feb 21, 2024

Changes proposed in this Pull Request:

Alternative approach from #44646.

Closes #44453. (Update: it turns out #44537 already fixed this issue)

In some circumstances, woocommerce_has_block_template wasn't set to true correctly, that meant that we were rendering a broken PHP template instead of the correct block template.

The issue was only reproducible in block themes with certain custom WooCommerce templates (ie: Tsubaki, which has the archive-product.html template), and were only reproducible in these cases:

This PR removes a check we had in the code that would only set woocommerce_has_block_template if the template was not in the theme.

How to test the changes in this Pull Request:

Verify correct template is rendered with certain settings (#44453)

(Update: this issue was actually fixed in #44537, but I'm adding the testing steps here just to verify there are no regressions)

  1. With a theme with custom WooCommerce templates (ie: Tsubaki).
  2. Go to WooCommerce > Settings > Products > Advanced and disable Use the product attributes lookup table for catalog filtering.
  3. With a block theme with custom WooCommerce templates (ie: Tsubaki) go to /shop/?product_cat=Clothing&filter_size=large. Note: the values of product_cat and filter_xyz might vary depending on the categories and attributes of your store. In case of doubt, you can import the products from sample-data.
  4. Verify the page is rendered correctly. Ie: verify the header is displayed, verify the page has the Shop title, verify the styles are loaded, etc.
Before After
image image

Verify taxonomies added by extensions are rendered correctly (testing for devs)

  1. With a theme with custom WooCommerce templates (ie: Tsubaki).
  2. Install WooCommerce Brands.
  3. In the admin, create a brand, edit a product and assign that product to the brand you just created.
  4. Visit the brand page in the frontend (the page should render correctly).
  5. Edit class-wc-brands-block-templates.php and comment out this line: add_filter( 'woocommerce_has_block_template', array( $this, 'has_block_template' ), 10, 2 );.
  6. Visit the brand page again. Verify the page still renders correctly.

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Comment

@Aljullu Aljullu added focus: template Issue related to WooCommerce templates. team: Kirigami & Origami labels Feb 21, 2024
@Aljullu Aljullu self-assigned this Feb 21, 2024
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Feb 21, 2024
@Aljullu Aljullu marked this pull request as ready for review February 21, 2024 14:06
Copy link
Contributor

github-actions bot commented Feb 21, 2024

Test Results Summary

Commit SHA: d275bac

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 39s
E2E Tests760027503516m 31s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@woocommercebot woocommercebot requested review from a team and imanish003 and removed request for a team February 21, 2024 14:08
Copy link
Contributor

github-actions bot commented Feb 21, 2024

Hi @WunderBart, @imanish003,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@Aljullu Aljullu force-pushed the fix/44453-fix-block-templates-not-rendering branch from 6724c96 to 434c232 Compare February 22, 2024 10:38
@Aljullu Aljullu force-pushed the fix/44453-fix-block-templates-not-rendering branch from 434c232 to 1bba52a Compare February 27, 2024 18:11
@WunderBart WunderBart self-requested a review March 1, 2024 15:28
Copy link
Contributor

@imanish003 imanish003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Aljullu,

I've tested the changes and everything is functioning as anticipated. I've left some minor comments and will await your response before giving my approval to this PR 🚀.

Thank you for your excellent work around templates so far 🙌🏻

Copy link
Contributor

@WunderBart WunderBart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can repro #44453, but this PR doesn't seem to fix it. Here's what I see for the following themes on both trunk and this branch:

2024 Tsubaki Storefront
Screenshot 2024-03-13 at 15 46 05 Screenshot 2024-03-13 at 15 45 15 Screenshot 2024-03-13 at 15 45 36

Looks like Storefront is affected as well, and only 2024 seems to render the results correctly.

EDIT

I misunderstood the issue, but it seems to have been actually fixed on trunk as well as the page does not render differently on this branch.

@Aljullu
Copy link
Contributor Author

Aljullu commented Mar 15, 2024

I misunderstood the issue, but it seems to have been actually fixed on trunk as well as the page does not render differently on this branch.

You're right, @WunderBart! It looks like the refactor PR (#44537) already fixed that issue. 😄 I updated the description of this PR and the refactor PR to account for that.

@Aljullu
Copy link
Contributor Author

Aljullu commented Mar 15, 2024

Thanks for the reviews, folks! I think all comments have been answered and I updated the PR description and testing steps to account for the fact that #44453 was already fixed in trunk (sorry for the confusion!). This PR is ready for another look.

PS: even though #44453 has been fixed in trunk, I still think there is value in merging this PR because (1) it simplifies the code and (2) solves the issue described under the Verify taxonomies added by extensions are rendered correctly testing section.

Copy link
Contributor

@imanish003 imanish003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified the changes again & everything is working as expected :shipit:

@Aljullu Aljullu merged commit d8c23c4 into trunk Mar 18, 2024
76 checks passed
@Aljullu Aljullu deleted the fix/44453-fix-block-templates-not-rendering branch March 18, 2024 12:03
@github-actions github-actions bot added this to the 8.8.0 milestone Mar 18, 2024
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Mar 18, 2024
@alvarothomas alvarothomas added needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: template Issue related to WooCommerce templates. needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris team: Kirigami & Origami
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Shop Page Filter Query Causes Inconsistent Block UI Rendering Without Product Attribute Lookup Table
5 participants