Implement line_discount feature for orders #60156
Conversation
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:
|
📝 WalkthroughWalkthroughAdds per-line-item Changes
Sequence Diagram(s)sequenceDiagram
participant Cart
participant WC_Cart_Totals
participant Checkout
participant OrderItem
participant DataStore
participant REST_API
participant Client
Cart->>WC_Cart_Totals: calculate_item_totals()
WC_Cart_Totals->>Cart: set cart item fields (line_total, line_tax, line_discount)
Checkout->>Cart: get cart contents
Checkout->>OrderItem: create order line item with props (including line_discount)
OrderItem->>DataStore: save_item_data() (persist _line_discount)
DataStore->>OrderItem: read() (populate line_discount)
REST_API->>OrderItem: prepare_object_for_response_core() (reads line_discount)
REST_API->>Client: return order JSON with line_items[].line_discount
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ 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: 3
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
plugins/woocommerce/includes/class-wc-cart-totals.php(2 hunks)plugins/woocommerce/includes/class-wc-checkout.php(1 hunks)plugins/woocommerce/includes/class-wc-order-item-product.php(5 hunks)plugins/woocommerce/includes/data-stores/class-wc-order-item-product-data-store.php(3 hunks)plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-orders-controller.php(2 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/includes/rest-api/Controllers/Version3/class-wc-rest-orders-controller.phpplugins/woocommerce/includes/data-stores/class-wc-order-item-product-data-store.phpplugins/woocommerce/includes/class-wc-checkout.phpplugins/woocommerce/includes/class-wc-order-item-product.phpplugins/woocommerce/includes/class-wc-cart-totals.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/includes/rest-api/Controllers/Version3/class-wc-rest-orders-controller.phpplugins/woocommerce/includes/data-stores/class-wc-order-item-product-data-store.phpplugins/woocommerce/includes/class-wc-checkout.phpplugins/woocommerce/includes/class-wc-order-item-product.phpplugins/woocommerce/includes/class-wc-cart-totals.php
🧠 Learnings (6)
📓 Common learnings
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.
📚 Learning: in woocommerce's cartschema::get_item_response() method, when called in the context of blocksshareds...
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().
Applied to files:
plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-orders-controller.php
📚 Learning: in woocommerce blocks, when porting existing code patterns that have known issues (like parseint tru...
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.
Applied to files:
plugins/woocommerce/includes/class-wc-checkout.phpplugins/woocommerce/includes/class-wc-cart-totals.php
📚 Learning: in woocommerce's session handler, the cart merging behavior was revised to always merge guest and us...
Learnt from: mikejolley
PR: woocommerce/woocommerce#57961
File: plugins/woocommerce/includes/class-wc-session-handler.php:302-333
Timestamp: 2025-06-19T11:58:57.484Z
Learning: In WooCommerce's session handler, the cart merging behavior was revised to always merge guest and user carts when both contain items, rather than having the guest cart take precedence. The migrate_cart_data() method in class-wc-session-handler.php implements this by using array_merge() to combine both carts when neither is empty.
Applied to files:
plugins/woocommerce/includes/class-wc-checkout.phpplugins/woocommerce/includes/class-wc-cart-totals.php
📚 Learning: in `plugins/woocommerce/src/blocks/blocktypes/productbutton.php`, the team intentionally relies on t...
Learnt from: Aljullu
PR: woocommerce/woocommerce#58809
File: plugins/woocommerce/src/Blocks/BlockTypes/ProductButton.php:222-225
Timestamp: 2025-06-13T17:11:13.732Z
Learning: In `plugins/woocommerce/src/Blocks/BlockTypes/ProductButton.php`, the team intentionally relies on toggling the `disabled` CSS class (via `data-wp-class--disabled`) instead of binding the `disabled` attribute, to mirror the behavior of the classic WooCommerce template.
Applied to files:
plugins/woocommerce/includes/class-wc-order-item-product.php
📚 Learning: in woocommerce mini cart implementation, the cart state is server-populated before javascript initia...
Learnt from: samueljseay
PR: woocommerce/woocommerce#58716
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/iapi-frontend.ts:78-96
Timestamp: 2025-06-18T06:05:25.472Z
Learning: In WooCommerce mini cart implementation, the cart state is server-populated before JavaScript initialization, so wooStoreState.cart and wooStoreState.cart.totals will be available on first render without requiring null-safe guards.
Applied to files:
plugins/woocommerce/includes/class-wc-cart-totals.php
🔇 Additional comments (8)
plugins/woocommerce/includes/class-wc-order-item-product.php (3)
53-53: LGTM - Property definition follows WooCommerce patterns.The addition of
'line_discount' => 0to the$extra_dataarray follows the established pattern for order item properties and uses an appropriate default value.
164-186: LGTM - Well-implemented getter and setter methods.The
get_line_discount()andset_line_discount()methods are properly implemented:
- Getter includes proper null/empty value handling and formatting
- Setter uses
wc_format_decimal()for consistent decimal formatting- Both follow WooCommerce conventions for property access methods
528-572: LGTM - Array access methods properly updated.The array access methods (
offsetGet,offsetSet,offsetExists) are correctly updated to handle the'line_discount'offset, maintaining backwards compatibility for legacy array-style access.plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-orders-controller.php (2)
438-443: LGTM - Proper REST API schema definition.The
line_discountproperty is correctly added to the schema with:
- Appropriate description and type definition
- Proper readonly status (since this is calculated data)
- Correct context specification
502-507: LGTM - Defensive programming approach for adding line_discount to API response.The code properly:
- Verifies the line item has an ID before proceeding
- Safely retrieves the order item object
- Checks method existence before calling
get_line_discount()- This defensive approach prevents potential errors if the method doesn't exist
plugins/woocommerce/includes/data-stores/class-wc-order-item-product-data-store.php (3)
25-25: LGTM! Proper addition of line_discount to internal meta keys.The addition of
_line_discountto the$internal_meta_keysarray follows WooCommerce conventions for internal meta field handling and ensures proper cleanup and management of the meta data.
36-50: LGTM! Consistent implementation of line_discount property mapping.The addition of the
line_discountproperty mapping in theread()method correctly retrieves the_line_discountmeta value and sets it on the order item object. The implementation is consistent with other property mappings in the same method.
63-74: LGTM! Complete persistence implementation for line_discount.The addition of
'_line_discount' => 'line_discount'to the$meta_key_to_propsmapping ensures that the line_discount property is properly saved when the order item is updated. This completes the read/write cycle for the new property.
| $this->cart->cart_contents[ $item_key ]['line_tax_data']['total'] = wc_remove_number_precision_deep( $item->taxes ); | ||
| $this->cart->cart_contents[ $item_key ]['line_total'] = wc_remove_number_precision( $item->total ); | ||
| $this->cart->cart_contents[ $item_key ]['line_tax'] = wc_remove_number_precision( $item->total_tax ); | ||
|
|
||
| if ( isset( $item->product ) ) { | ||
| $regular_price = $item->product->get_regular_price(); | ||
| $quantity = $item->quantity; | ||
| $regular_total = $regular_price * $quantity; | ||
| $actual_total = wc_remove_number_precision( $item->total ); | ||
|
|
||
| $line_discount = $regular_total - $actual_total; | ||
| $this->cart->cart_contents[ $item_key ]['line_discount'] = max( 0, $line_discount ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good implementation of line_discount calculation with room for improvement.
The line_discount calculation logic is sound, but there are several areas that need attention:
- Guard against null/empty regular price: The code doesn't validate if
get_regular_price()returns a valid numeric value - Consider variable products: For variable products, the regular price might not reflect the correct variant price
- Precision handling: The calculation mixes precision-handled and non-precision values
Apply this diff to improve robustness:
if ( isset( $item->product ) ) {
- $regular_price = $item->product->get_regular_price();
+ $regular_price = (float) $item->product->get_regular_price();
+
+ // Skip if no valid regular price is available
+ if ( $regular_price <= 0 ) {
+ $this->cart->cart_contents[ $item_key ]['line_discount'] = 0;
+ continue;
+ }
+
$quantity = $item->quantity;
- $regular_total = $regular_price * $quantity;
+ $regular_total = wc_add_number_precision( $regular_price * $quantity );
$actual_total = wc_remove_number_precision( $item->total );
- $line_discount = $regular_total - $actual_total;
+ $line_discount = wc_remove_number_precision( $regular_total ) - $actual_total;
$this->cart->cart_contents[ $item_key ]['line_discount'] = max( 0, $line_discount );
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In plugins/woocommerce/includes/class-wc-cart-totals.php around lines 688 to
700, improve the line_discount calculation by first checking if
get_regular_price() returns a valid numeric value before using it. For variable
products, ensure you retrieve the correct variant's regular price rather than
the parent product's price. Also, handle all price values consistently with
precision functions to avoid mixing raw and precision-handled numbers. Update
the code to validate and sanitize prices, fetch variant prices correctly, and
apply wc_remove_number_precision or similar uniformly in calculations.
|
@Kirov-B Hi there, thank you for your contribution. There are some comments added by Code Rabbit. Did you get the chance to look at those? |
|
@ralucaStan sorry have been bit busy, now its implemented. |
Submission Review Guidelines:
Changes proposed in this Pull Request:
Save the price difference between the sales and list price in new meta
_line_discountCloses #30308
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Add a 'line_discount' line item for all orders which saves the difference between the regular price and the sales price for products that are discounted and ordered. Visible in the Order Object and Order API.Comment