Add explicit decimal precision to Money - #534
derikthiessen-shopify wants to merge 18 commits into
Conversation
Assisted-By: devx/345c75c7-270f-479c-894f-2febe9415c92
Assisted-By: devx/345c75c7-270f-479c-894f-2febe9415c92
Assisted-By: devx/345c75c7-270f-479c-894f-2febe9415c92
Assisted-By: devx/345c75c7-270f-479c-894f-2febe9415c92
Assisted-By: devx/345c75c7-270f-479c-894f-2febe9415c92
Assisted-By: devx/345c75c7-270f-479c-894f-2febe9415c92
Assisted-By: devx/345c75c7-270f-479c-894f-2febe9415c92
Assisted-By: devx/345c75c7-270f-479c-894f-2febe9415c92
Assisted-By: devx/345c75c7-270f-479c-894f-2febe9415c92
Assisted-By: devx/345c75c7-270f-479c-894f-2febe9415c92
Assisted-By: devx/345c75c7-270f-479c-894f-2febe9415c92
Assisted-By: devx/ec9e0de4-fb50-4089-9e14-c59b7c4dacce
Assisted-By: devx/ec9e0de4-fb50-4089-9e14-c59b7c4dacce
This reverts commit aa129ba. Assisted-By: devx/ec9e0de4-fb50-4089-9e14-c59b7c4dacce
Assisted-By: devx/ec9e0de4-fb50-4089-9e14-c59b7c4dacce
Assisted-By: devx/ec9e0de4-fb50-4089-9e14-c59b7c4dacce
Assisted-By: devx/ec9e0de4-fb50-4089-9e14-c59b7c4dacce
| return if money.decimal_precision == decimal_precision | ||
|
|
||
| raise MoneyColumn::PrecisionMismatchError, | ||
| "Invalid #{column}: Money decimal precision #{money.decimal_precision} does not match money column decimal precision #{decimal_precision.inspect}." |
There was a problem hiding this comment.
so this would reject writes with a different declared precision. I'm not sure if that is the contract that the Money gem maintainers want, so calling it out here
Assisted-By: devx/ec9e0de4-fb50-4089-9e14-c59b7c4dacce
derikthiessen-shopify
left a comment
There was a problem hiding this comment.
Inline usage examples for the public precision contracts introduced by this PR.
|
|
||
| def new(value = 0, currency = nil) | ||
| return new_from_money(value, currency) if value.is_a?(Money) | ||
| def new(value = 0, currency = nil, decimal_precision: nil) |
There was a problem hiding this comment.
Construction example — explicit precision retains extra calculation digits until an output boundary:
unit_price = Money.new("0.0057", "USD", decimal_precision: 3)
unit_price.value.to_s("F") # => "0.0057"
unit_price.to_s # => "0.006"| Money.new(value + money.value, calculated_currency(money.currency)) | ||
| result_decimal_precision = calculated_decimal_precision(money) | ||
| return self if money.value.zero? && !no_currency? && result_decimal_precision == precision_argument | ||
| Money.new(value + money.value, calculated_currency(money.currency), decimal_precision: result_decimal_precision) |
There was a problem hiding this comment.
Arithmetic example — matching precision propagates through the result, while conflicting declared precision raises Money::IncompatiblePrecisionError:
price = Money.new("1.000", "USD", decimal_precision: 3)
tax = Money.new("0.057", "USD", decimal_precision: 3)
(price + tax).to_s # => "1.057"
(price * 2).to_s # => "2.000"| to_s | ||
| else | ||
| { value: to_s(:amount), currency: currency.to_s } | ||
| hash = { value: to_s(:amount), currency: currency.to_s } |
There was a problem hiding this comment.
JSON is a rendered boundary, so it records the rounded value together with the precision needed to reconstruct its representation:
money = Money.new("0.0057", "USD", decimal_precision: 3)
money.as_json
# => { value: "0.006", currency: "USD", decimal_precision: 3 }
Money.from_json(money.to_json).to_s # => "0.006"| def to_subunits(money) | ||
| raise ArgumentError, "money cannot be nil" if money.nil? | ||
| (money.value * subunit_to_unit(money.currency)).to_i | ||
| value = money.value.round(money.decimal_precision) |
There was a problem hiding this comment.
Subunit conversion is also an output boundary and rounds instead of truncating retained digits:
Money.new("0.0099", "USD", decimal_precision: 2).subunits
# => 1| allocation_currency = extract_currency(maximums + [__getobj__]) | ||
| maximums = maximums.map { |max| max.to_money(allocation_currency) } | ||
| maximums_total = maximums.reduce(Money.new(0, allocation_currency), :+) | ||
| maximums = maximums.map { |max| coerce_maximum(max, allocation_currency) } |
There was a problem hiding this comment.
All maximums are interpreted in the receiver's explicit units and must be exactly representable at that precision:
amount = Money.new("0.057", "USD", decimal_precision: 3)
amount.allocate_max_amounts(["0.029", "0.028"]).map(&:to_s)
# => ["0.029", "0.028"]| subunits = @money.subunits | ||
| low = Money.from_subunits(subunits / @num, @money.currency) | ||
| high = Money.from_subunits(low.subunits + 1, @money.currency) | ||
| units = Helpers.money_to_units(@money) |
There was a problem hiding this comment.
Splitting uses the declared precision as its indivisible unit:
Money.new("0.057", "USD", decimal_precision: 3)
.split(2)
.map(&:to_s)
# => ["0.029", "0.028"]| def serialize(money) | ||
| super("value" => money.value.to_s("F"), "currency" => money.currency.iso_code) | ||
| attributes = { "value" => money.value.to_s("F"), "currency" => money.currency.iso_code } | ||
| attributes["decimal_precision"] = money.decimal_precision if money.explicit_decimal_precision? |
There was a problem hiding this comment.
Active Job transport preserves both the raw calculation value and its declared precision:
money = Money.new("0.0574", "USD", decimal_precision: 3)
SomeJob.perform_later(money)
# The job receives value 0.0574 with decimal_precision 3.| attr_reader :money_column_options | ||
|
|
||
| def money_column(*columns, currency_column: nil, currency: nil, currency_read_only: false, coerce_null: false) | ||
| def money_column(*columns, currency_column: nil, currency: nil, currency_read_only: false, coerce_null: false, decimal_precision: nil) |
There was a problem hiding this comment.
Model configuration example — the column owns the persistence precision, and explicitly precise assignments must match it:
class Product < ActiveRecord::Base
money_column :price,
currency_column: :price_currency,
decimal_precision: 4
end
Product.create!(
price: Money.new("0.0574", "USD", decimal_precision: 4),
)
Part of #306
TL;DR
Added opt-in decimal precision to
Moneyvalues, retained additional calculation digits until output boundaries, and preserved the precision contract through arithmetic, allocation, splitting, serialization, and money columns.Why Change?
Some prices and thresholds need more decimal places than their currency supports, especially before quantities are applied. Rounding those values during construction loses information, while silently combining or persisting values with incompatible precision can change their meaning.
What Changed?
decimal_precisionoption that defaults to the currency's minor units.decimal_precisionconfiguration for money columns so stored values reconstruct without losing their declared precision.Fractional unit prices can retain calculation digits that are not individually payable:
Allocation can use the same explicit units while preventing maxima from being rounded into larger limits:
Models can declare the precision used for database round trips:
Tests
Risks
decimal_precision:API follows the approach proposed in the issue rather than scoped/global configuration. This choice needs confirmation from the Money team before merge.money_column decimal_precision:is an explicit persistence contract: writes with a different declared precision are rejected, and explicit-precision writes are rejected when the column has no fixed precision. This policy also needs Money-team confirmation.decimal_precision; code readingvalueorto_dobserves those digits, while rendered and discrete-unit output is rounded.