Add cache priming to product grid blocks, product Button, and Product Gallery#63750
Add cache priming to product grid blocks, product Button, and Product Gallery#63750
Conversation
github-actions
bot
added
the
plugin: woocommerce
albarin
changed the title
woocommercebot
requested review from
a team and
kraftbj
and removed request for
a team
Testing GuidelinesApart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds conditional calls to _prime_post_caches() in three block-related areas (AbstractProductGrid, ProductButton, ProductGalleryUtils) to batch-load post data before iterating product image attachments, grouped children, or variations; also adds a changelog entry and a non-functional comment in GroupedProductItem. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
plugins/woocommerce/src/Blocks/BlockTypes/ProductButton.php (1)
341-345: Normalize grouped child IDs before cache priming.At Line 341, run
get_children()throughwp_parse_id_list()before calling_prime_post_caches()and iterating child products for stricter input handling.Suggested refactor
- $grouped_product_ids = $product->get_children(); + $grouped_product_ids = wp_parse_id_list( $product->get_children() ); if ( ! empty( $grouped_product_ids ) ) { _prime_post_caches( $grouped_product_ids ); }As per coding guidelines:
Guard all code against unexpected inputsandSanitize and validate any potentially dangerous inputs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/src/Blocks/BlockTypes/ProductButton.php` around lines 341 - 345, The grouped product children IDs returned by $product->get_children() should be normalized before use: call wp_parse_id_list() on the result and assign it back to $grouped_product_ids, then only call _prime_post_caches($grouped_product_ids) and iterate the sanitized $grouped_product_ids in the foreach; update references in the ProductButton class/method where $grouped_product_ids, get_children(), _prime_post_caches(), and the foreach loop are used to ensure inputs are validated.plugins/woocommerce/src/Blocks/Utils/ProductGalleryUtils.php (1)
120-124: Normalize variation IDs before priming and iteration.At Line 120, normalize
get_children()withwp_parse_id_list()so unexpected extension-provided values don’t propagate into_prime_post_caches()andwc_get_product().Suggested refactor
- $variations = $product->get_children(); + $variations = wp_parse_id_list( $product->get_children() ); if ( ! empty( $variations ) ) { _prime_post_caches( $variations ); }As per coding guidelines:
Guard all code against unexpected inputsandSanitize and validate any potentially dangerous inputs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/src/Blocks/Utils/ProductGalleryUtils.php` around lines 120 - 124, Normalize and validate variation IDs returned by $product->get_children() before using them: replace the direct call to $product->get_children() with a sanitized list via wp_parse_id_list and ensure $variations is an array (e.g., $variations = wp_parse_id_list( $product->get_children() )); then use that $variations when calling _prime_post_caches and in the foreach that calls wc_get_product() so unexpected non-numeric or extension-provided values cannot propagate into _prime_post_caches() or wc_get_product().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plugins/woocommerce/src/Blocks/BlockTypes/ProductButton.php`:
- Around line 341-345: The grouped product children IDs returned by
$product->get_children() should be normalized before use: call
wp_parse_id_list() on the result and assign it back to $grouped_product_ids,
then only call _prime_post_caches($grouped_product_ids) and iterate the
sanitized $grouped_product_ids in the foreach; update references in the
ProductButton class/method where $grouped_product_ids, get_children(),
_prime_post_caches(), and the foreach loop are used to ensure inputs are
validated.
In `@plugins/woocommerce/src/Blocks/Utils/ProductGalleryUtils.php`:
- Around line 120-124: Normalize and validate variation IDs returned by
$product->get_children() before using them: replace the direct call to
$product->get_children() with a sanitized list via wp_parse_id_list and ensure
$variations is an array (e.g., $variations = wp_parse_id_list(
$product->get_children() )); then use that $variations when calling
_prime_post_caches and in the foreach that calls wc_get_product() so unexpected
non-numeric or extension-provided values cannot propagate into
_prime_post_caches() or wc_get_product().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 41377dd5-ed4f-4ae8-8153-948d945ae612
📒 Files selected for processing (4)
plugins/woocommerce/changelog/63750-add-cache-priming-blocksplugins/woocommerce/src/Blocks/BlockTypes/AbstractProductGrid.phpplugins/woocommerce/src/Blocks/BlockTypes/ProductButton.phpplugins/woocommerce/src/Blocks/Utils/ProductGalleryUtils.php
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
Can we add clarifying comments for those cases, so future people don't attempt to add caches there? |
Good suggestions @kalessil, added a clarifying comment in |
kalessil
left a comment
kalessil
left a comment
There was a problem hiding this comment.
Looks good to me!
- The code changes look good.
- I followed the testing instructions and can confirm there is a reduction in the number of SQL queries.
3 participants
Submission Review Guidelines:
Changes proposed in this Pull Request:
Adds missing
_prime_post_caches()calls inAbstractProductGrid.php,ProductGalleryUtils.php, and `ProductButton.php and reduces individual SQL queries.Closes #63717
Closes https://linear.app/a8c/issue/WOOPLUG-6430/performance-blocks-missing-cache-priming-cases
Measured query reduction (Query Monitor):
Query savings scale linearly with the number of products/variations/children.
Not included:
GroupedProductItem.php— Grouped product children are already cached by the time this block renders, so adding priming there would have no benefit.How to test the changes in this Pull Request:
Prerequisites:
trunkfirst (before), note the total query count from QM, then test on this branch (after)Legacy product grid blocks (
AbstractProductGrid.php)Product Gallery block on variable product (
ProductGalleryUtils.php)Product Button for grouped product (
ProductButton.php)is_on_sale()/get_price_html()which pre-cache grouped children)Milestone
Changelog entry
Changelog Entry Details
Significance
Type
Message
Reduce SQL queries on legacy product grid blocks, Product Gallery, and Product Button by adding missing "_prime_post_caches()" calls to batch-load post data.