Skip to content

Conversation

@kyrena
Copy link
Contributor

@kyrena kyrena commented Jun 23, 2023

Description

This PR fix Implicit conversion from float 10.1 to int loses precision with PHP 8.1/8.2, with a coupon code of 10.1% for all items of cart.

See also this discussion comment.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added the Component: SalesRule Relates to Mage_SalesRule label Jun 23, 2023
@fballiano
Copy link
Contributor

Actually I'm not sure I get the reason behind this PR, why cast to 10 if the discount is 10.1? I understand that 10.1 is an unusual number but remove the possibility for decimal discounts doesn't seem the way to go, to me.

@fballiano
Copy link
Contributor

So, I checked that part of the core, I think casting to string is much better, that variable is only used as an array_key, so as a string we risk less conflict if a cart as 2 discounts of the same value (at least for the integer part).

I think the core is wrong in this part, they shouldn't have used the discount percent as a key, but the rule id, or something unique.

@kiatng
Copy link
Contributor

kiatng commented Jul 14, 2023

Is this PR fixing a bug?

@fballiano fballiano merged commit 7f73fa5 into OpenMage:main Jul 15, 2023
@kyrena kyrena deleted the php82-implicitint branch August 4, 2023 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: SalesRule Relates to Mage_SalesRule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants