-
Notifications
You must be signed in to change notification settings - Fork 0
Fixed bug where price was being multiplied by 100 twice #96
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe change updates price-to-cents conversion in helloextend-protection/includes/class-helloextend-protection-orders.php. Previously, listPrice and purchasePrice were multiplied by 100 before being passed to get_price_in_cents, which also multiplies by 100, causing double scaling. The patch now passes the raw price values directly to get_price_in_cents. Only the price fields within the product block of line item data are affected. No control flow, structure, or other fields were modified. Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Comment |
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 (2)
helloextend-protection/includes/class-helloextend-protection-orders.php (2)
157-160
: Make minor-units conversion currency-aware and avoid float truncation.Current cast-to-int can truncate (e.g., 19.995 → 1999) and always assumes 2 decimals. If the store (or order) currency has 0 or 3 decimals, values will be off. Prefer rounding with a decimals-derived factor.
Apply:
- return (int) floatval($item_price * 100); + $decimals = (int) wc_get_price_decimals(); // honors store/currency settings + $multiplier = (int) pow( 10, max( 0, $decimals ) ); + return (int) round( (float) $item_price * $multiplier );If you support multi-currency per order, consider passing the order currency into this helper and deriving decimals from it.
248-251
: Consider using the order line’s actual unit price for purchasePrice.Using $product->get_price() may ignore discounts/coupons applied on the order. Computing from the line total keeps Extend in sync with what the shopper paid.
- 'purchasePrice' => $this->get_price_in_cents($product->get_price()), + // Unit price actually paid (exclude/including tax per your API contract) + // To include tax instead, use: ($item->get_total() + $item->get_total_tax()) + 'purchasePrice' => $this->get_price_in_cents( + $item->get_quantity() ? ($item->get_total() / $item->get_quantity()) : 0 + ),Confirm whether Extend expects tax-inclusive or tax-exclusive unit price and adjust the formula accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
helloextend-protection/includes/class-helloextend-protection-orders.php
(1 hunks)
⏰ 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). (1)
- GitHub Check: deploy-and-test
🔇 Additional comments (1)
helloextend-protection/includes/class-helloextend-protection-orders.php (1)
248-249
: Approve changes. Double scaling removed by passing raw prices into get_price_in_cents; verification found no other pre-multiplication before this call.
Summary by CodeRabbit