-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Add min and max price methods for grouped products #59586
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
Add min and max price methods for grouped products #59586
Conversation
0f28dac
to
426ef9b
Compare
@@ -417,7 +471,7 @@ public function test_collection_param_include_meta() { | |||
$this->assertEquals( 200, $response->get_status() ); | |||
|
|||
$response_data = $response->get_data(); | |||
$this->assertCount( 4, $response_data ); |
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.
At the setup of the test, three new products have been created: a grouped product with two simple products associated.
426ef9b
to
b7cd257
Compare
Testing GuidelinesHi @mikejolley @dinhtungdu , 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 update introduces an experimental WooCommerce REST API feature that exposes minimum and maximum prices for grouped products. It adds new methods to retrieve these prices, extends the REST API schema and response, introduces a feature flag to control exposure, and updates related tests and configuration files to support and validate the new functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant REST_API_Controller
participant WC_Product_Grouped
Client->>REST_API_Controller: GET /wp-json/wc/v3/products
REST_API_Controller->>REST_API_Controller: Check feature flag (experimental-wc-rest-api)
alt Feature flag enabled and grouped product requested
REST_API_Controller->>WC_Product_Grouped: get_min_price()
WC_Product_Grouped-->>REST_API_Controller: min price
REST_API_Controller->>WC_Product_Grouped: get_max_price()
WC_Product_Grouped-->>REST_API_Controller: max price
REST_API_Controller->>Client: Respond with product data including __experimental_min_price and __experimental_max_price
else Feature flag disabled or not grouped product
REST_API_Controller->>Client: Respond with standard product data (no experimental fields)
end
📜 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). (26)
✨ 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
Documentation and Community
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
plugins/woocommerce/includes/class-wc-product-grouped.php (1)
163-195
: LGTM - Implementation is solid with minor optimization opportunity.The new methods correctly implement min/max price calculation for grouped products using appropriate WooCommerce functions and patterns.
However, consider these improvements:
Minor optimization opportunity:
Both methods perform identical operations to get visible children. Consider extracting this logic to avoid duplication:+ /** + * Get prices from visible child products. + * + * @return array Array of prices from visible children + */ + private function get_visible_children_prices() { + $children = array_filter( array_map( 'wc_get_product', $this->get_children() ), 'wc_products_array_filter_visible_grouped' ); + return array_map( 'wc_get_price_to_display', $children ); + } + public function get_min_price() { - $children = array_filter( array_map( 'wc_get_product', $this->get_children() ), 'wc_products_array_filter_visible_grouped' ); - $min_price = array_map( 'wc_get_price_to_display', $children ); + $prices = $this->get_visible_children_prices(); - if ( empty( $min_price ) ) { + if ( empty( $prices ) ) { return ''; } - return (string) min( $min_price ); + return (string) min( $prices ); }Return type documentation:
The docblock indicatesstring|float
but the implementation always returnsstring
. Consider updating the docblock to reflect the actual return type:- * @return string|float Minimum price or empty string if no children + * @return string Minimum price or empty string if no childrenplugins/woocommerce/tests/legacy/framework/helpers/class-wc-helper-product.php (1)
145-152
: Fix indentation inconsistency.The indentation is inconsistent between the variable assignment and the method calls. Apply consistent indentation:
- $simple_product_2 = self::create_simple_product(); - $product = new WC_Product_Grouped(); - $product->set_props( - array( - 'name' => 'Dummy Grouped Product', - 'sku' => 'DUMMY GROUPED SKU', - ) - ); + $simple_product_2 = self::create_simple_product(); + $product = new WC_Product_Grouped(); + $product->set_props( + array( + 'name' => 'Dummy Grouped Product', + 'sku' => 'DUMMY GROUPED SKU', + ) + );plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller-tests.php (1)
88-96
: Remove redundant product retrieval.The grouped product is retrieved again on line 96 without apparent need:
} self::$products[] = $grouped_product; - $grouped_product = wc_get_product( $grouped_product->get_id() );
The product is already saved and available in the
$grouped_product
variable.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
plugins/woocommerce/changelog/59586-wooplug-4867-the-endpoint-wp-jsonwcv3productsorderbyprice-returns
(1 hunks)plugins/woocommerce/client/admin/config/core.json
(1 hunks)plugins/woocommerce/client/admin/config/development.json
(1 hunks)plugins/woocommerce/includes/class-wc-product-grouped.php
(2 hunks)plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php
(3 hunks)plugins/woocommerce/tests/legacy/framework/helpers/class-wc-helper-product.php
(2 hunks)plugins/woocommerce/tests/legacy/unit-tests/rest-api/Tests/Version3/products.php
(1 hunks)plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller-tests.php
(12 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
plugins/woocommerce/tests/**/*.php
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/woo-phpunit.mdc
**/*.{php,js,ts,jsx,tsx}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (8)
📓 Common learnings
Learnt from: samueljseay
PR: woocommerce/woocommerce#59051
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/mini-cart-contents/inner-blocks/mini-cart-footer-block/index.tsx:66-70
Timestamp: 2025-06-23T05:47:52.696Z
Learning: For WooCommerce mini-cart blocks in plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/, the standardized conditional pattern for experimental features should be `if ( isExperimentalMiniCartEnabled() ) { blockSettings.save = () => <InnerBlocks.Content />; }` - defaulting to the traditional Save component and only overriding when the experimental feature is enabled.
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.
plugins/woocommerce/changelog/59586-wooplug-4867-the-endpoint-wp-jsonwcv3productsorderbyprice-returns (5)
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: 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.
Learnt from: samueljseay
PR: woocommerce/woocommerce#59051
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/mini-cart-contents/inner-blocks/mini-cart-footer-block/index.tsx:66-70
Timestamp: 2025-06-23T05:47:52.696Z
Learning: For WooCommerce mini-cart blocks in plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/, the standardized conditional pattern for experimental features should be `if ( isExperimentalMiniCartEnabled() ) { blockSettings.save = () => <InnerBlocks.Content />; }` - defaulting to the traditional Save component and only overriding when the experimental feature is enabled.
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: vladolaru
PR: woocommerce/woocommerce#59486
File: plugins/woocommerce/src/Internal/Admin/Settings/PaymentsProviders/WooPayments/WooPaymentsService.php:1544-1544
Timestamp: 2025-07-08T11:18:07.871Z
Learning: WooCommerce has polyfills for newer PHP functions (like str_starts_with() from PHP 8.0+), so these functions can be safely used even though WooCommerce supports PHP 7.4+. No need to suggest PHP 7.4 compatible alternatives when polyfills are available.
plugins/woocommerce/client/admin/config/core.json (6)
Learnt from: samueljseay
PR: woocommerce/woocommerce#59051
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/mini-cart-contents/inner-blocks/mini-cart-footer-block/index.tsx:66-70
Timestamp: 2025-06-23T05:47:52.696Z
Learning: For WooCommerce mini-cart blocks in plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/, the standardized conditional pattern for experimental features should be `if ( isExperimentalMiniCartEnabled() ) { blockSettings.save = () => <InnerBlocks.Content />; }` - defaulting to the traditional Save component and only overriding when the experimental feature is enabled.
Learnt from: vladolaru
PR: woocommerce/woocommerce#58784
File: plugins/woocommerce/src/Internal/Admin/Settings/Payments.php:484-488
Timestamp: 2025-06-17T11:30:23.806Z
Learning: In the WooCommerce Payments settings provider state tracking system (plugins/woocommerce/src/Internal/Admin/Settings/Payments.php), when an extension is deactivated and its snapshot key disappears, only the 'extension_active' flag should be set to false while keeping other state flags like 'account_connected', 'needs_setup', 'test_mode', etc. unchanged. This is intentional behavior to preserve historical state information.
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.
Learnt from: gigitux
PR: woocommerce/woocommerce#58785
File: plugins/woocommerce/client/blocks/package.json:0-0
Timestamp: 2025-06-17T10:25:36.686Z
Learning: Do not suggest using `cross-env` in the WooCommerce repository as it's deprecated/archived and the team is working to remove it from blocks commands to reduce the dependency tree. Instead, inline environment variables like `WP_EXPERIMENTAL_MODULES=true knip` should work fine in supported environments.
Learnt from: triple0t
PR: woocommerce/woocommerce#59186
File: packages/js/email-editor/src/store/initial-state.ts:9-10
Timestamp: 2025-06-26T12:13:32.062Z
Learning: In WooCommerce email editor store initialization (packages/js/email-editor/src/store/initial-state.ts), the current_post_id and current_post_type from window.WooCommerceEmailEditor are required parameters that should cause explicit errors if missing, rather than using fallback values or optional chaining. The design preference is to fail fast when critical initialization data is unavailable.
Learnt from: vladolaru
PR: woocommerce/woocommerce#58784
File: plugins/woocommerce/client/admin/client/settings-payments/components/other-payment-gateways/other-payment-gateways.tsx:43-50
Timestamp: 2025-06-18T07:56:06.961Z
Learning: In plugins/woocommerce/client/admin/client/settings-payments/components/other-payment-gateways/other-payment-gateways.tsx, the user vladolaru prefers to keep the current setUpPlugin function signature with optional positional context parameter rather than refactoring to an options object or making context required.
plugins/woocommerce/client/admin/config/development.json (5)
Learnt from: samueljseay
PR: woocommerce/woocommerce#59051
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/mini-cart-contents/inner-blocks/mini-cart-footer-block/index.tsx:66-70
Timestamp: 2025-06-23T05:47:52.696Z
Learning: For WooCommerce mini-cart blocks in plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/, the standardized conditional pattern for experimental features should be `if ( isExperimentalMiniCartEnabled() ) { blockSettings.save = () => <InnerBlocks.Content />; }` - defaulting to the traditional Save component and only overriding when the experimental feature is enabled.
Learnt from: gigitux
PR: woocommerce/woocommerce#58785
File: plugins/woocommerce/client/blocks/package.json:0-0
Timestamp: 2025-06-17T10:25:36.686Z
Learning: Do not suggest using `cross-env` in the WooCommerce repository as it's deprecated/archived and the team is working to remove it from blocks commands to reduce the dependency tree. Instead, inline environment variables like `WP_EXPERIMENTAL_MODULES=true knip` should work fine in supported environments.
Learnt from: vladolaru
PR: woocommerce/woocommerce#58784
File: plugins/woocommerce/src/Internal/Admin/Settings/Payments.php:484-488
Timestamp: 2025-06-17T11:30:23.806Z
Learning: In the WooCommerce Payments settings provider state tracking system (plugins/woocommerce/src/Internal/Admin/Settings/Payments.php), when an extension is deactivated and its snapshot key disappears, only the 'extension_active' flag should be set to false while keeping other state flags like 'account_connected', 'needs_setup', 'test_mode', etc. unchanged. This is intentional behavior to preserve historical state information.
Learnt from: triple0t
PR: woocommerce/woocommerce#59186
File: packages/js/email-editor/src/store/initial-state.ts:9-10
Timestamp: 2025-06-26T12:13:32.062Z
Learning: In WooCommerce email editor store initialization (packages/js/email-editor/src/store/initial-state.ts), the current_post_id and current_post_type from window.WooCommerceEmailEditor are required parameters that should cause explicit errors if missing, rather than using fallback values or optional chaining. The design preference is to fail fast when critical initialization data is unavailable.
Learnt from: vladolaru
PR: woocommerce/woocommerce#58784
File: plugins/woocommerce/client/admin/client/settings-payments/components/other-payment-gateways/other-payment-gateways.tsx:43-50
Timestamp: 2025-06-18T07:56:06.961Z
Learning: In plugins/woocommerce/client/admin/client/settings-payments/components/other-payment-gateways/other-payment-gateways.tsx, the user vladolaru prefers to keep the current setUpPlugin function signature with optional positional context parameter rather than refactoring to an options object or making context required.
plugins/woocommerce/tests/legacy/unit-tests/rest-api/Tests/Version3/products.php (1)
Learnt from: gigitux
PR: woocommerce/woocommerce#58846
File: plugins/woocommerce/client/blocks/tests/e2e/tests/all-products/all-products.block_theme.spec.ts:41-52
Timestamp: 2025-06-16T09:20:22.981Z
Learning: In WooCommerce E2E tests, the database is reset to the initial state for each test, so there's no need to manually restore global template changes (like clearing the header template) as the test infrastructure handles cleanup automatically.
plugins/woocommerce/tests/legacy/framework/helpers/class-wc-helper-product.php (1)
Learnt from: gigitux
PR: woocommerce/woocommerce#58846
File: plugins/woocommerce/client/blocks/tests/e2e/tests/all-products/all-products.block_theme.spec.ts:41-52
Timestamp: 2025-06-16T09:20:22.981Z
Learning: In WooCommerce E2E tests, the database is reset to the initial state for each test, so there's no need to manually restore global template changes (like clearing the header template) as the test infrastructure handles cleanup automatically.
plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php (5)
Learnt from: samueljseay
PR: woocommerce/woocommerce#59051
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/mini-cart-contents/inner-blocks/mini-cart-footer-block/index.tsx:66-70
Timestamp: 2025-06-23T05:47:52.696Z
Learning: For WooCommerce mini-cart blocks in plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/, the standardized conditional pattern for experimental features should be `if ( isExperimentalMiniCartEnabled() ) { blockSettings.save = () => <InnerBlocks.Content />; }` - defaulting to the traditional Save component and only overriding when the experimental feature is enabled.
Learnt from: Aljullu
PR: woocommerce/woocommerce#58845
File: plugins/woocommerce/src/Blocks/BlockTypes/AddToCartWithOptions/Utils.php:128-140
Timestamp: 2025-06-16T11:06:45.637Z
Learning: In WooCommerce, the `$product->is_in_stock()` method returns `true` when backorders are allowed, even if the actual stock quantity is 0 or negative. This means the method already considers backorder status when determining stock availability.
Learnt from: mreishus
PR: woocommerce/woocommerce#58817
File: plugins/woocommerce/includes/wc-product-functions.php:140-140
Timestamp: 2025-06-13T23:52:46.221Z
Learning: In WooCommerce's wc_delete_product_transients function, the context check includes both $is_admin_page (for regular admin screens) and $is_privileged_ajax (for AJAX requests from users with edit_products capability), ensuring that legitimate admin AJAX operations like product imports/exports can still delete transients.
Learnt from: dinhtungdu
PR: woocommerce/woocommerce#59499
File: plugins/woocommerce/src/Internal/ProductFilters/QueryClauses.php:327-332
Timestamp: 2025-07-10T04:22:27.648Z
Learning: In the WooCommerce ProductFilters QueryClauses class, the $chosen_taxonomies parameter in add_taxonomy_clauses() already contains only validated public product taxonomies. The validation occurs upstream in the Params class during parameter registration, so additional taxonomy existence validation in the processing methods is redundant.
Learnt from: vladolaru
PR: woocommerce/woocommerce#58784
File: plugins/woocommerce/src/Internal/Admin/Settings/PaymentsProviders/Paytrail.php:29-35
Timestamp: 2025-06-18T09:58:10.616Z
Learning: In WooCommerce codebase, prefer using `wc_string_to_bool()` over `filter_var($value, FILTER_VALIDATE_BOOLEAN)` when converting option values (especially 'yes'/'no' style flags) to boolean. The WooCommerce helper function is more idiomatic and handles the conversion consistently with core WooCommerce patterns.
plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller-tests.php (4)
Learnt from: samueljseay
PR: woocommerce/woocommerce#59051
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/mini-cart-contents/inner-blocks/mini-cart-footer-block/index.tsx:66-70
Timestamp: 2025-06-23T05:47:52.696Z
Learning: For WooCommerce mini-cart blocks in plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/, the standardized conditional pattern for experimental features should be `if ( isExperimentalMiniCartEnabled() ) { blockSettings.save = () => <InnerBlocks.Content />; }` - defaulting to the traditional Save component and only overriding when the experimental feature is enabled.
Learnt from: gigitux
PR: woocommerce/woocommerce#58846
File: plugins/woocommerce/client/blocks/tests/e2e/tests/all-products/all-products.block_theme.spec.ts:41-52
Timestamp: 2025-06-16T09:20:22.981Z
Learning: In WooCommerce E2E tests, the database is reset to the initial state for each test, so there's no need to manually restore global template changes (like clearing the header template) as the test infrastructure handles cleanup automatically.
Learnt from: mreishus
PR: woocommerce/woocommerce#58817
File: plugins/woocommerce/includes/wc-product-functions.php:140-140
Timestamp: 2025-06-13T23:52:46.221Z
Learning: In WooCommerce's wc_delete_product_transients function, the context check includes both $is_admin_page (for regular admin screens) and $is_privileged_ajax (for AJAX requests from users with edit_products capability), ensuring that legitimate admin AJAX operations like product imports/exports can still delete transients.
Learnt from: mreishus
PR: woocommerce/woocommerce#58891
File: plugins/woocommerce/src/Blocks/Utils/BlocksSharedState.php:0-0
Timestamp: 2025-06-16T21:59:26.255Z
Learning: In WooCommerce's CartSchema::get_item_response() method, when called in the context of BlocksSharedState::register_cart_interactivity(), the method returns a plain array rather than a WP_REST_Response object, making it directly suitable for wp_interactivity_state() without needing to call ->get_data().
⏰ 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). (9)
- GitHub Check: build
- GitHub Check: Blocks e2e tests 2/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 6/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: PHP: 7.4 WP: prerelease [WP 6.8.2-RC1] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: Core e2e tests 1/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: PHP: 8.4 WP: latest [WP latest] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: Core e2e tests 4/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: PHP: 7.4 WP: latest - 1 [WP 6.7.2] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: PHP: 8.4 WP: latest [WP latest] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
🔇 Additional comments (10)
plugins/woocommerce/client/admin/config/core.json (1)
45-45
: LGTM - Feature flag follows established patterns.The new experimental feature flag is correctly disabled by default in the core configuration, which is appropriate for experimental features.
plugins/woocommerce/client/admin/config/development.json (1)
45-45
: LGTM - Correctly enables experimental feature in development.The feature flag is appropriately enabled in the development configuration, allowing for proper testing of the experimental functionality.
plugins/woocommerce/changelog/59586-wooplug-4867-the-endpoint-wp-jsonwcv3productsorderbyprice-returns (1)
1-4
: LGTM - Changelog entry is properly formatted.The changelog entry correctly documents the addition of experimental REST API support for grouped product min/max prices with appropriate significance and type classifications.
plugins/woocommerce/tests/legacy/unit-tests/rest-api/Tests/Version3/products.php (1)
654-654
: LGTM - Test update correctly reflects new schema properties.The property count increase from 72 to 74 accurately reflects the addition of the two new experimental price fields (
__experimental_min_price
and__experimental_max_price
) to the product schema.plugins/woocommerce/tests/legacy/framework/helpers/class-wc-helper-product.php (1)
253-253
: Good formatting improvement!Adding a space after the
function
keyword follows PHP coding standards.plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php (3)
11-11
: Required import for feature flag check.The Features import is necessary for the experimental feature flag check.
1768-1781
: Well-implemented experimental schema fields.The experimental fields are properly gated behind the feature flag and use the appropriate
__experimental_
prefix to indicate their experimental status. The string type is consistent with other price fields in the schema.
1976-1983
: Confirmed existence ofget_min_price()
andget_max_price()
methodsI’ve verified that both methods are implemented in
plugins/woocommerce/includes/class-wc-product-grouped.php
(lines 169 and 186), so there’s no risk of calling undefined methods at runtime. No further changes are needed here.plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller-tests.php (2)
14-39
: Well-structured feature flag control methods.The methods correctly use WordPress filters to enable/disable the experimental feature for testing. The implementation is clean and follows testing best practices.
1652-1683
: Comprehensive test coverage for grouped product pricing.The test properly validates:
- Presence of experimental fields when the feature is enabled
- Absence of fields when disabled
- Correct min/max price values based on child products
This provides good coverage for the new experimental functionality.
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. |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller-tests.php (1)
1651-1681
: New test method provides comprehensive coverage for experimental feature.The test correctly verifies:
- Experimental fields are present when feature is enabled
- Experimental fields are absent when feature is disabled
- Expected min/max price values match the grouped product setup
However, consider adding a test cleanup step to re-enable the experimental feature if needed for subsequent tests.
Consider adding feature flag cleanup in the test:
$this->disable_experimental_rest_api_feature(); $response = $this->server->dispatch( new WP_REST_Request( 'GET', '/wc/v3/products/' . $grouped_product->get_id() ) ); $this->assertEquals( 200, $response->get_status() ); $data = $response->get_data(); $this->assertArrayNotHasKey( '__experimental_min_price', $data ); $this->assertArrayNotHasKey( '__experimental_max_price', $data ); + + // Re-enable for any subsequent tests that might need it + $this->enable_experimental_rest_api_feature();
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/woocommerce/includes/class-wc-product-grouped.php
(2 hunks)plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller-tests.php
(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/woocommerce/includes/class-wc-product-grouped.php
🧰 Additional context used
📓 Path-based instructions (2)
plugins/woocommerce/tests/**/*.php
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/woo-phpunit.mdc
**/*.{php,js,ts,jsx,tsx}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (2)
📓 Common learnings
Learnt from: samueljseay
PR: woocommerce/woocommerce#59051
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/mini-cart-contents/inner-blocks/mini-cart-footer-block/index.tsx:66-70
Timestamp: 2025-06-23T05:47:52.696Z
Learning: For WooCommerce mini-cart blocks in plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/, the standardized conditional pattern for experimental features should be `if ( isExperimentalMiniCartEnabled() ) { blockSettings.save = () => <InnerBlocks.Content />; }` - defaulting to the traditional Save component and only overriding when the experimental feature is enabled.
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.
plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller-tests.php (4)
Learnt from: samueljseay
PR: woocommerce/woocommerce#59051
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/mini-cart-contents/inner-blocks/mini-cart-footer-block/index.tsx:66-70
Timestamp: 2025-06-23T05:47:52.696Z
Learning: For WooCommerce mini-cart blocks in plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/, the standardized conditional pattern for experimental features should be `if ( isExperimentalMiniCartEnabled() ) { blockSettings.save = () => <InnerBlocks.Content />; }` - defaulting to the traditional Save component and only overriding when the experimental feature is enabled.
Learnt from: gigitux
PR: woocommerce/woocommerce#58846
File: plugins/woocommerce/client/blocks/tests/e2e/tests/all-products/all-products.block_theme.spec.ts:41-52
Timestamp: 2025-06-16T09:20:22.981Z
Learning: In WooCommerce E2E tests, the database is reset to the initial state for each test, so there's no need to manually restore global template changes (like clearing the header template) as the test infrastructure handles cleanup automatically.
Learnt from: mreishus
PR: woocommerce/woocommerce#58817
File: plugins/woocommerce/includes/wc-product-functions.php:140-140
Timestamp: 2025-06-13T23:52:46.221Z
Learning: In WooCommerce's wc_delete_product_transients function, the context check includes both $is_admin_page (for regular admin screens) and $is_privileged_ajax (for AJAX requests from users with edit_products capability), ensuring that legitimate admin AJAX operations like product imports/exports can still delete transients.
Learnt from: mreishus
PR: woocommerce/woocommerce#58891
File: plugins/woocommerce/src/Blocks/Utils/BlocksSharedState.php:0-0
Timestamp: 2025-06-16T21:59:26.255Z
Learning: In WooCommerce's CartSchema::get_item_response() method, when called in the context of BlocksSharedState::register_cart_interactivity(), the method returns a plain array rather than a WP_REST_Response object, making it directly suitable for wp_interactivity_state() without needing to call ->get_data().
⏰ 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). (26)
- GitHub Check: Blocks e2e tests 9/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 6/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 5/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 4/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 8/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 7/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 10/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 3/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 3/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 5/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 1/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 6/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 2/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 4/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Metrics - @woocommerce/plugin-woocommerce [performance]
- GitHub Check: PHP: 8.4 WP: latest [WP latest] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: Core API tests - @woocommerce/plugin-woocommerce [api]
- GitHub Check: Core e2e tests 1/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: PHP: 7.4 WP: prerelease [WP 6.8.2-RC1] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: PHP: 7.4 WP: latest - 1 [WP 6.7.2] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: Core e2e tests 2/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.2] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: PHP: 7.4 WP: prerelease [WP 6.8.2-RC1] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: Lint - @woocommerce/plugin-woocommerce
- GitHub Check: build
🔇 Additional comments (8)
plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller-tests.php (8)
14-38
: Helper methods for experimental feature flag management look good.The implementation correctly uses WordPress filters to enable/disable the experimental REST API feature. The disable method properly removes the feature from the array using
array_diff
.
88-95
: Test setup for grouped products is well-implemented.The setup correctly creates a grouped product, extracts its children, and adds them all to the test products array. This provides the necessary test data for the experimental min/max price functionality.
132-134
: Method signature update is consistent and well-documented.The addition of the
$with_experimental_rest_api
parameter maintains backward compatibility through the method's usage pattern and includes proper documentation.
212-216
: Conditional experimental fields addition is correctly implemented.The logic properly adds the experimental min/max price fields only when the feature flag is enabled, maintaining clean separation between experimental and stable features.
224-255
: Test method updates handle experimental features correctly.The method now accepts the experimental feature parameter and properly enables/disables the feature flag during testing. The test data setup correctly handles both scenarios.
275-302
: Field-by-field testing properly handles experimental features.The test correctly enables the experimental feature when needed and iterates through all expected fields including the experimental ones.
472-472
: Assertion count correctly updated for expanded test data.The count of 7 reflects the addition of grouped products and their children to the test dataset (4 original + 1 grouped + 2 children = 7 total).
1025-1025
: Grouped product count is correctThe test class’s
wpSetUpBeforeClass
creates one grouped product (along with its children), and this test method creates a second grouped product. Filtering byinclude_types => [GROUPED]
therefore returns exactly two grouped products, soassertCount( 2, $response_products )
is valid. No change required.
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.
Thanks for updating tests @gigitux I left some feedback. Ping when when I can look again for final approval 👍🏻
Aside, I see we have the beta tester plugin and the features page (wp-admin/admin.php?page=wc-settings&tab=advanced§ion=features). Which is best?
return ''; | ||
} | ||
|
||
return (string) min( $min_price ); |
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.
Consider using wc_format_decimal
on these values. Internally thats how product classes format prices, so for 'price', 'regular_price', 'sale_price' they have all been ran through wc_format_decimal
.
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.
Fixed with 5cbd67e
@@ -148,6 +148,7 @@ public function get_children( $context = 'view' ) { | |||
return $this->get_prop( 'children', $context ); | |||
} | |||
|
|||
|
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.
Extra white space seems to be mistake.
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.
Fixed with 5cbd67e
return ''; | ||
} | ||
|
||
return (string) max( $max_price ); |
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.
Same feedback as above on wc_format_decimal
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.
Fixed with 5cbd67e
@@ -1958,6 +1973,14 @@ protected function get_product_data( $product, $context = 'view' ) { | |||
$data['global_unique_id'] = $product->get_global_unique_id( $context ); | |||
} | |||
|
|||
if ( in_array( '__experimental_min_price', $fields, true ) ) { | |||
$data['__experimental_min_price'] = $product->get_type() === ProductType::GROUPED ? $product->get_min_price() : ''; |
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.
Rather than basing this on product type, how about checking if get_min_price is callable? That way it will work with other product types that choose to define it.
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.
Good point! I addressed it with 3570943
Thanks for the feedback! Usually, for features that are under development, we use Also, I addressed your feedback! |
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.
Thanks for addressing those issues. Approving. You may need to re-run CI to get it green.
Hi @gigitux! Your PR contains REST API changes. Please consider updating the REST API documentation if your changes affect the public API. Changed API files:
|
No changes are needed because the new fields are under a feature flag. |
Submission Review Guidelines:
Changes proposed in this Pull Request:
Fixes #59354 (WOOPLUG-4867)
This PR introduces experimental REST API functionality for grouped products to expose minimum and maximum price information. The changes include:
experimental-wc-rest-api
(disabled in core, enabled in development)WC_Product_Grouped
class withget_min_price()
andget_max_price()
methodsScreenshots or screen recordings:
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Enable the experimental feature:
/wp-admin/tools.php?page=woocommerce-admin-test-helper&tab=features
.experimental-wc-rest-api
feature flag.Test grouped product creation:
Test REST API response:
/wp-json/wc/v3/products/{grouped_product_id}
__experimental_min_price
and__experimental_max_price
fields are present/wp-json/wc/v3/products/
__experimental_min_price
and__experimental_max_price
fields are present__experimental_min_price
and__experimental_max_price
fields aren't empty only for grouped products.Test edge cases:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Add experimental REST API support for grouped product min/max prices
<detail