-
Notifications
You must be signed in to change notification settings - Fork 0
PTHMINT-81: Change the tax_rate_percentage to integer #30
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
PTHMINT-81: Change the tax_rate_percentage to integer #30
Conversation
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.
Pull Request Overview
This pull request changes the tax_rate_percentage parameter type from float to int in the CartItem class to improve type consistency, while adding comprehensive test coverage for edge cases including zero values.
- Updated
add_tax_rate_percentagemethod signature and documentation to expect integer input - Added intermediate variable for calculation clarity
- Enhanced test coverage with edge cases for zero tax rates
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/multisafepay/api/shared/cart/cart_item.py | Changed parameter type from float to int and updated calculation logic |
| tests/multisafepay/unit/api/shared/cart/test_unit_cart_item.py | Updated existing test and added new tests for zero tax rate scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| """ | ||
| item = CartItem() | ||
| item.add_tax_rate_percentage(0.0) | ||
| print(item) |
Copilot
AI
Aug 28, 2025
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.
Debug print statement should be removed from test code. This appears to be leftover debugging code that should not be committed.
| print(item) |
| def add_tax_rate_percentage( | ||
| self: "CartItem", | ||
| tax_rate_percentage: float, | ||
| tax_rate_percentage: int, |
Copilot
AI
Aug 28, 2025
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.
The test shows that 0.0 (float) is still being passed to this method, but the type annotation now requires int. This creates an inconsistency where the API accepts floats in practice but the type annotation suggests only integers are valid.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #30 +/- ##
=======================================
Coverage 90.48% 90.49%
=======================================
Files 107 107
Lines 2323 2324 +1
=======================================
+ Hits 2102 2103 +1
Misses 221 221 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2b03e10 to
0482a4a
Compare
This pull request updates the handling of tax rate percentages in the
CartItemclass to use integers instead of floats, and adds new tests to verify correct behavior for both integer and float inputs, including edge cases like zero. These changes improve type consistency and ensure robust handling of tax rate values.Type consistency improvements:
tax_rate_percentageparameter type fromfloattointin theadd_tax_rate_percentagemethod and its docstring incart_item.py, ensuring that only integer values are expected for tax rates. [1] [2]Behavioral adjustments:
add_tax_rate_percentageto use an intermediate variablerating, clarifying the conversion logic before settingtax_table_selector.Test coverage enhancements:
0and0.0tax rate percentages are correctly handled, ensuring that edge cases and type flexibility are covered.