Refuse a price that is a fraction of a minor unit - #2690
Open
kha333n wants to merge 2 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2689.
1.xonly —2.xhas replaced this cast, and I have not checked its behaviour.A price written as
12.99is stored as13, and the column counts minor units,so the product sells for AED 0.13.
The fix
set()returned the value unchanged, so a decimal reached abigintcolumn andwas rounded. It now refuses a value that is not a whole number of minor units,
and says what was probably meant:
Refusing rather than converting is deliberate: the cast cannot tell whether a
bare
12is twelve minor units or twelve of the major unit — both are in use —and guessing would multiply every correctly-written price by the currency factor.
The documented contract is already an integer count of minor units, so this makes
the code agree with the docs rather than changing the contract.
get()is untouched.The existing tests
PriceTestpasses a decimal in four places. Correcting those inputs to theminor-unit equivalent leaves every assertion in the file unchanged —
->value,->decimaland->formatted()all still hold:12.99129912.995(3dp currency)1299512.99129913.99(compare_price)1399They only appeared to work because the test asserts on the in-memory model and
never reloads it —
get()strips non-digits, so"12.99"reads back as1299in the request that wrote it, and
13in every request after.Tests
tests/core/Unit/Base/Casts/PriceTest.php:priceandcompare_price.1299, so the fix is actionable rather than just a refusal.float and a numeric string all still store
1299. Passes before and after, andis the guard that this rejects fractions rather than non-integers.