fix: unsupported operand types: string in calculation #2534
Conversation
mralaminahamed
added
Needs: Testing
WalkthroughThe pull request focuses on modifying the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
includes/Commission.php (1)
Line range hint
267-269: Consider extracting tax calculations to improve readability.While the
floatval()additions correctly fix the type safety issues, the tax calculation logic is complex and hard to follow. Consider extracting this into a separate method for better maintainability.-$earning_or_commission += ( ( floatval( $order->get_total_tax() ) - floatval( $order->get_total_tax_refunded() ) ) - ( floatval( $order->get_shipping_tax() ) - floatval( dokan()->fees->get_total_shipping_tax_refunded( $order ) ) ) ); +$earning_or_commission += $this->calculate_tax_earnings( $order ); + +private function calculate_tax_earnings( $order ) { + $total_tax = floatval( $order->get_total_tax() ); + $total_tax_refunded = floatval( $order->get_total_tax_refunded() ); + $shipping_tax = floatval( $order->get_shipping_tax() ); + $shipping_tax_refunded = floatval( dokan()->fees->get_total_shipping_tax_refunded( $order ) ); + + return ( $total_tax - $total_tax_refunded ) - ( $shipping_tax - $shipping_tax_refunded ); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/Commission.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (1, 3)
🔇 Additional comments (3)
includes/Commission.php (3)
263-263: LGTM! Good fix for type safety.The addition of
floatval()ensures type safety when calculating shipping earnings by explicitly converting the shipping total to float before arithmetic operations.
Line range hint
272-272: LGTM! Good fix for type safety.The addition of
floatval()ensures type safety when calculating shipping tax earnings by explicitly converting values to float before arithmetic operations.
Line range hint
263-272: Verify the impact on existing orders.The type conversion changes might affect the precision of calculations for existing orders. Please ensure:
- The changes don't affect historical commission calculations
- The precision of calculations remains consistent
- No rounding issues are introduced
✅ Verification successful
Type conversion changes maintain calculation precision
The floatval() changes are safe and won't affect historical commission calculations based on:
- Extensive test coverage verifying exact commission values
- Strict equality assertions in tests without rounding tolerance
- Proper handling of edge cases including zero amounts and null values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any existing test cases or code that might be affected by the precision changes rg -A 5 "get_earning_by_order.*test" .Length of output: 40
Script:
#!/bin/bash # Search for Commission test files fd -e php test.*Commission # Search for commission calculation related tests rg -l "class.*Commission.*Test" . # Look for precision/rounding specifications in commission calculations rg -A 3 "round|precision|decimal" includes/Commission.php # Check for other commission calculation methods ast-grep --pattern 'function calculate_commission($$$)'Length of output: 338
Script:
#!/bin/bash # Check commission test implementations cat tests/php/src/Commission/CommissionTest.php cat tests/php/src/Commission/CommissionModelTest.php # Look for floating-point assertions or precision checks in tests rg -A 3 "assertEquals|assertEqualsWithDelta|assertSame" tests/php/src/Commission/Length of output: 66910
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit