Fix issue where analytics reports generate duplicate rows in variations #60258
Conversation
This file contains hidden or 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
Testing GuidelinesHi @woocommerce/ventures, 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:
|
📝 WalkthroughWalkthroughThis change addresses duplicate row issues in WooCommerce Analytics reports for products and variations. It updates SQL ordering logic in both Products and Variations report data stores to ensure stable, secondary sorting by product and variation IDs. Additionally, it improves the export formatting of variation product names. A changelog entry documents the fix. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WooCommerce Analytics API
participant Products DataStore
participant Variations DataStore
participant Database
User->>WooCommerce Analytics API: Request Product/Variation Report (with orderby param)
WooCommerce Analytics API->>Products DataStore: get_noncached_data(orderby)
alt Product Report
Products DataStore->>Database: Query with order_by (may append ", product_id")
Database-->>Products DataStore: Results
end
WooCommerce Analytics API->>Variations DataStore: get_noncached_data(orderby)
alt Variation Report
Variations DataStore->>Database: Query with order_by (may append ", product_id, variation_id")
Database-->>Variations DataStore: Results
end
WooCommerce Analytics API->>User: Return Report Data
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugins/woocommerce/changelog/wooplug-5243-woocommerce-analytics-generates-duplicate-rows-in-variations (1)
1-5: Minor grammatical improvement needed.The changelog entry correctly documents the fix, but Line 4 has a grammatical issue.
Apply this diff to improve the grammar:
-Fix issue where both variation and product reports where showing duplicate entries across pages. +Fix issue where both variation and product reports were showing duplicate entries across pages.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
plugins/woocommerce/changelog/wooplug-5243-woocommerce-analytics-generates-duplicate-rows-in-variations(1 hunks)plugins/woocommerce/src/Admin/API/Reports/Products/DataStore.php(2 hunks)plugins/woocommerce/src/Admin/API/Reports/Variations/Controller.php(1 hunks)plugins/woocommerce/src/Admin/API/Reports/Variations/DataStore.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{php,js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/code-quality.mdc)
**/*.{php,js,jsx,ts,tsx}: Guard against unexpected inputs
Sanitize and validate any potentially dangerous inputs
Ensure code is backwards compatible
Write code that is readable and intuitive
Ensure code has unit or E2E tests where applicable
Files:
plugins/woocommerce/src/Admin/API/Reports/Products/DataStore.phpplugins/woocommerce/src/Admin/API/Reports/Variations/DataStore.phpplugins/woocommerce/src/Admin/API/Reports/Variations/Controller.php
**/*.{php,js,ts,jsx,tsx}
⚙️ CodeRabbit Configuration File
**/*.{php,js,ts,jsx,tsx}: Don't trust that extension developers will follow the best practices, make sure the code:
- Guards against unexpected inputs.
- Sanitizes and validates any potentially dangerous inputs.
- Is backwards compatible.
- Is readable and intuitive.
- Has unit or E2E tests where applicable.
Files:
plugins/woocommerce/src/Admin/API/Reports/Products/DataStore.phpplugins/woocommerce/src/Admin/API/Reports/Variations/DataStore.phpplugins/woocommerce/src/Admin/API/Reports/Variations/Controller.php
🧠 Learnings (5)
📓 Common learnings
Learnt from: dinhtungdu
PR: woocommerce/woocommerce#59878
File: plugins/woocommerce/src/Internal/ProductFilters/FilterData.php:364-383
Timestamp: 2025-07-23T01:06:46.155Z
Learning: In WooCommerce ProductFilters FilterData class, the $product_ids parameter in methods like get_hierarchical_taxonomy_counts() is internally generated from WP_Query results using implode(',', array_column($results, 'ID')). This creates a safe comma-separated string of integers from database IDs, not user input, so SQL injection concerns don't apply to this parameter.
Learnt from: nerrad
PR: woocommerce/woocommerce#60176
File: plugins/woocommerce/client/legacy/css/admin.scss:9088-9105
Timestamp: 2025-08-04T00:21:51.440Z
Learning: WooCommerce is currently in the midst of an admin redesign project, so temporary/short-term CSS solutions for admin pages are preferred over long-term refactoring when addressing immediate needs.
Learnt from: prettyboymp
PR: woocommerce/woocommerce#59048
File: .github/workflows/cherry-pick-milestoned-prs.yml:60-83
Timestamp: 2025-06-26T12:45:40.709Z
Learning: WooCommerce uses WordPress versioning conventions where minor versions in X.Y.Z format are constrained to 0-9 (Y cannot exceed 9). This means version increment logic should reset minor to 0 and increment major when minor reaches 9, rather than allowing two-digit minor versions like 9.10 or 9.11.
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.856Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in `plugins/woocommerce/changelog/` directory and are processed during releases, not at the repository root level.
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#60250
File: .github/workflows/release-compile-changelog.yml:165-166
Timestamp: 2025-08-07T10:34:27.702Z
Learning: In WooCommerce release workflows, the VERSION variable used in changelog generation comes from the `validate-selected-branch-version` step, which extracts the version from `plugins/woocommerce/woocommerce.php` using `grep -oP '(?<=Version: )(.+)'` and validates it matches the input version. This ensures the version is always in the correct format by the time it reaches the changelog generation step, making additional error handling for version format validation unnecessary.
Learnt from: samueljseay
PR: woocommerce/woocommerce#58716
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/iapi-frontend.ts:83-101
Timestamp: 2025-06-17T07:07:53.443Z
Learning: In WooCommerce blocks, when porting existing code patterns that have known issues (like parseInt truncating decimal money values), maintain consistency with existing implementation rather than making isolated fixes. The preference is for systematic refactoring approaches (like broader Dinero adoption) over piecemeal changes.
Learnt from: NeosinneR
PR: woocommerce/woocommerce#60129
File: plugins/woocommerce/includes/wc-coupon-functions.php:145-154
Timestamp: 2025-07-31T11:18:02.478Z
Learning: In WooCommerce codebase, when table names are hardcoded (like `$wpdb->prefix . 'wc_order_coupon_lookup'`), the preference is to maintain consistency with existing patterns throughout the codebase rather than making isolated security improvements when the risk is minimal (since $wpdb->prefix is controlled by WordPress and table names are hardcoded constants).
Learnt from: dinhtungdu
PR: woocommerce/woocommerce#59900
File: plugins/woocommerce/client/blocks/assets/js/blocks/product-filters/inner-blocks/attribute-filter/inspector.tsx:0-0
Timestamp: 2025-07-24T05:37:00.907Z
Learning: The DisplayStyleSwitcher component in plugins/woocommerce/client/blocks/assets/js/blocks/product-filters/components/display-style-switcher/index.tsx has been updated so that its onChange prop accepts only a string type (not string | number | undefined), eliminating the need for type assertions when using this component.
Learnt from: NeosinneR
PR: woocommerce/woocommerce#0
File: :0-0
Timestamp: 2025-06-26T14:25:08.421Z
Learning: In WooCommerce transactional emails, product images have historically had display issues due to lazy loading attributes being applied, which email clients cannot process since they don't execute JavaScript. This issue existed in both old and new email templates, but became more visible with the new email template system. The fix involves preventing lazy loading on attachment images specifically during email generation by adding skip classes and data attributes.
📚 Learning: in woocommerce core repository, changelog entries for all prs live in `plugins/woocommerce/changelog...
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.856Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in `plugins/woocommerce/changelog/` directory and are processed during releases, not at the repository root level.
Applied to files:
plugins/woocommerce/changelog/wooplug-5243-woocommerce-analytics-generates-duplicate-rows-in-variations
📚 Learning: in woocommerce release workflows, the version variable used in changelog generation comes from the `...
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#60250
File: .github/workflows/release-compile-changelog.yml:165-166
Timestamp: 2025-08-07T10:34:27.702Z
Learning: In WooCommerce release workflows, the VERSION variable used in changelog generation comes from the `validate-selected-branch-version` step, which extracts the version from `plugins/woocommerce/woocommerce.php` using `grep -oP '(?<=Version: )(.+)'` and validates it matches the input version. This ensures the version is always in the correct format by the time it reaches the changelog generation step, making additional error handling for version format validation unnecessary.
Applied to files:
plugins/woocommerce/changelog/wooplug-5243-woocommerce-analytics-generates-duplicate-rows-in-variations
📚 Learning: woocommerce legacy javascript files in plugins/woocommerce/client/legacy/js/ must use older javascri...
Learnt from: opr
PR: woocommerce/woocommerce#0
File: :0-0
Timestamp: 2025-06-20T17:38:16.565Z
Learning: WooCommerce legacy JavaScript files in plugins/woocommerce/client/legacy/js/ must use older JavaScript syntax and cannot use modern features like optional chaining (?.) due to browser compatibility requirements. Explicit null checking with && operators should be used instead.
Applied to files:
plugins/woocommerce/changelog/wooplug-5243-woocommerce-analytics-generates-duplicate-rows-in-variations
📚 Learning: in woocommerce productfilters filterdata class, the $product_ids parameter in methods like get_hiera...
Learnt from: dinhtungdu
PR: woocommerce/woocommerce#59878
File: plugins/woocommerce/src/Internal/ProductFilters/FilterData.php:364-383
Timestamp: 2025-07-23T01:06:46.155Z
Learning: In WooCommerce ProductFilters FilterData class, the $product_ids parameter in methods like get_hierarchical_taxonomy_counts() is internally generated from WP_Query results using implode(',', array_column($results, 'ID')). This creates a safe comma-separated string of integers from database IDs, not user input, so SQL injection concerns don't apply to this parameter.
Applied to files:
plugins/woocommerce/src/Admin/API/Reports/Products/DataStore.phpplugins/woocommerce/src/Admin/API/Reports/Variations/DataStore.phpplugins/woocommerce/src/Admin/API/Reports/Variations/Controller.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Core API tests - @woocommerce/plugin-woocommerce [api]
- GitHub Check: Metrics - @woocommerce/plugin-woocommerce [performance]
- GitHub Check: Core e2e tests 1/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 5/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 2/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: PHP: 8.4 WP: latest [WP latest] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: Core e2e tests 6/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: PHP: 7.4 WP: latest - 1 [WP 6.7.3] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: Core e2e tests 4/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: PHP: 8.4 WP: latest [WP latest] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: PHP: 7.4 WP: latest - 1 [WP 6.7.3] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: Core e2e tests 3/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Lint - @woocommerce/plugin-woocommerce
- GitHub Check: build
🔇 Additional comments (4)
plugins/woocommerce/src/Admin/API/Reports/Products/DataStore.php (2)
204-204: Cosmetic formatting improvement.The trailing newline improves code formatting consistency.
383-387: LGTM! Critical fix for pagination duplicates.The conditional addition of secondary sorting by
product_idensures stable, deterministic ordering when primary sort values are identical. This directly addresses the duplicate rows issue by preventing inconsistent pagination results.plugins/woocommerce/src/Admin/API/Reports/Variations/DataStore.php (1)
489-493: LGTM! Comprehensive fix for variation pagination duplicates.The conditional addition of secondary sorting by both
product_id, variation_idensures stable, unique ordering for variations. This is the correct approach since variations require both identifiers for uniqueness, directly addressing duplicate rows in paginated variation reports.plugins/woocommerce/src/Admin/API/Reports/Variations/Controller.php (1)
394-409: LGTM! Enhanced export formatting for variation names.The enhancement properly formats variation names in exports by:
- Using the filterable separator (defaulting to ' - ')
- Checking if separator already exists to avoid duplication
- Properly handling empty attribute options with localized "Any AttributeName" format
- Maintaining backward compatibility through existing filters
This improves export data quality and aligns with the PR's objective to enhance variation report exports.
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Submission Review Guidelines:
Changes proposed in this Pull Request:
This makes two changes:
order bySQL params like product and variation id when ordering by either 'items_sold', 'net_revenue', or 'orders_count' ( this fixes an issue where pagination causes duplicate results ).Closes #60163 .
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Also notice that variations with local attributes do not have the selected option appended to the variation name.
Duplicate python script
Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment