-
-
Notifications
You must be signed in to change notification settings - Fork 178
fix: floating point error #1211
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
Conversation
MrWeez
left a 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.
Why do you divide and multiply by 1000? If we store prices in cents, then we need to use 100 or am I wrong?
Oh, my bad, I didn't know about the existence of currencies with three 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.
Copilot reviewed 45 out of 65 changed files in this pull request and generated no comments.
Files not reviewed (20)
- app/Casts/Settings/CurrencyCast.php: Language not supported
- app/Classes/PaymentExtension.php: Language not supported
- app/Extensions/PaymentGateways/MercadoPago/MercadoPagoExtension.php: Language not supported
- app/Extensions/PaymentGateways/Mollie/MollieExtension.php: Language not supported
- app/Extensions/PaymentGateways/PayPal/PayPalExtension.php: Language not supported
- app/Extensions/PaymentGateways/Stripe/StripeExtension.php: Language not supported
- app/Facades/Currency.php: Language not supported
- app/Helpers/CurrencyHelper.php: Language not supported
- app/Http/Controllers/Admin/CouponController.php: Language not supported
- app/Http/Controllers/Admin/OverViewController.php: Language not supported
- app/Http/Controllers/Admin/PaymentController.php: Language not supported
- app/Http/Controllers/Admin/ProductController.php: Language not supported
- app/Http/Controllers/Admin/SettingsController.php: Language not supported
- app/Http/Controllers/Admin/ShopProductController.php: Language not supported
- app/Http/Controllers/Admin/UserController.php: Language not supported
- app/Http/Controllers/Admin/VoucherController.php: Language not supported
- app/Http/Controllers/Auth/RegisterController.php: Language not supported
- app/Http/Controllers/HomeController.php: Language not supported
- app/Http/Controllers/ServerController.php: Language not supported
- app/Http/Controllers/StoreController.php: Language not supported
|
code LGTM, will spin up a test env |
S0ly
left a 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.
seems good to me
|
This PR seems to include breaking changes for Payments gateway we may want to release as 1.1.0 so show that |
This PR fixes the problem reported here: #1095
It also already has the fix for: #1210.
PR implements a
1000division and multiplication factor to support currencies with 3 decimal places.Edit:
In the commit 7d2c102 the way
minimum_creditsworks has been changed.Now the old
-1that was used to use the configuration value, becomesnull, to avoid users putting-1, -10, -100thinking it would work.Todo: