-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Fix performance issues with structured data and products with lots of variations #59431
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
""" WalkthroughA changelog entry was added documenting a patch release for the WooCommerce plugin, highlighting performance improvements related to structured data and variable products with many variations. The core code change refactors the logic for determining the lowest sale price among product variations, replacing a manual iteration with a more efficient built-in method. Changes
Assessment against linked issues
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@jorgeatorres this is same the as #56855 but with the corrections you requested. Could you review please? |
Testing GuidelinesHi , Apart 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:
|
Hi @MajorChump, thanks a lot for your contribution! I think there's a case where though in which the result could be slightly different than before, if there's a product with the following 3 variations 👇 the
Would be great if you could add some tests to it. Thanks a lot again! |
Hi @albarin I believe you're correct, both the existing code and my changes seem to be arbitrary in terms of the date selected for on sale to date - I think the regression here makes no difference as it was random to begin with. In a real world scenario, variations of the same product discounted to the same price most likely will come out of sale at the same time so may be a none issue. How do you want to proceed? |
Hey @MajorChump, thanks for your reply!
After some discussion, we think this is ok, as it seems the date was also arbitrarily selected before. There's a small detail that would be good to correct: the code is using the variable Thanks a lot! |
@albarin Thank you, changes made as requested. Thanks, |
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.
Tested on a product with thousands of variations and confirmed that the loading time is significantly improved and the LD-JSON is equivalent.
Submission Review Guidelines:
Changes proposed in this Pull Request:
Closes #55799 .
(For Bug Fixes) Bug introduced in PR #52105 .
Screenshots or screen recordings:
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Testing that has already taken place:Tested on our live site which has products with 10k+ variations. Load times reduced from 20-30 seconds to under a second.
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment
Summary by CodeRabbit