Add integer conversion to Strata::Money

[MichaelCrawfordNava]

Changes

• Added an integer conversion for the Strata::Money type

Context

This will assist in the completion of TSS-414.

Helps solve a situation where I would want to use earned income as an integer value, such as:

Testing

CI

[MichaelCrawfordNava]

@lorenyu Would you mind taking a look at this?

[lorenyu]

Suggestion on an alternative approach

[lorenyu]

I worry about adding this coercion method because it can lead to bugs if the developer didn't realize that money casts to the cents amount. I can see a very reasonable developer assuming that money_amount + 1 would add one dollar to the amount

[MichaelCrawfordNava]

My only counter is that a dev is going to need to know about cents anyways, just because of how Strata::Money is setup.

In the doc comment example for making a new Money object, you have to pass cents into the initializer. Also, the only settable attribute is cents.

money = Strata::Money.new(1250)  # $12.50 in cents
money.cents = 625 # $6.25 in cents

Basically, since everything revolves around cents for money anyways, my mind immediately thinks that to_i would give me cents.

[lorenyu]

That's only true if the dev explicitly constructs Money objects. But with the SDK they don't have to. They can use the generator to generate the column which if they don't look closely they won't know if it's storing a decimal dollar amount or an integer cents amount, and they can use form builder's f.money (which doesn't exists yet but it should eventually) to collect money as dollars since that's more intuitive from a user perspective. To implement f.money we'd do something like render <input name="attribute_name[dollar_amount]" type="text"> which would send over something like { foo: { dollar_amount: 12.75 } }, which is converted to cents here. Then at the end of the day they'll have money objects to work with and never need to construct them. Even in tests we'd probably offer built-in SDK money generators.

Also, even if one dev on a project understands how money works under the hood, it's possible another dev that just works with the models that already have money objects on them and is implementing new business logic and/or new views may not know about the internals of how money works.

All is to say I'm somewhat nervous about this, especially given the fact that money is so important to get right (calculating incorrect payment amonuts is a pretty serious bug in the real world)

[lorenyu]

As an aside, if we want to do type coercion, the way to do that I believe is to define a coerce method rather than to_i which may allow the Money object to be used in places that accept integers that don't make sense (e.g. array indices)

[MichaelCrawfordNava]

Good to know (the coerce method). Another reason we might want a coerce method is for things like validations. I just ran into a problem with:

class EarnedIncomeActivity < Activity
  include Strata::Attributes

  strata_attribute :earned_income, :money
  validates :earned_income, presence: true, numericality: { greater_than: 0 } # <-- numericality becomes a problem
end

The numericality built-in validator won't work because it can't coerce the income to be a number @lorenyu

[lorenyu]

That's an interesting use case. Curious, does it work to do

class EarnedIncomeActivity < Activity
  include Strata::Attributes

  strata_attribute :earned_income, :money
  validates :earned_income, presence: true, numericality: { greater_than: Strata::Money.ZERO }
end

where Money.ZERO is defined as

class Money
  ZERO = new(cents: 0)
end

[lorenyu]

And arguably we'd want to use the comparison validator like

validates :earned_income, comparison: { greater_than: Money.ZERO }

if we don't think of Money as a number. (Tangentially, we're only ever dealing with USD but conceptually the Money type depends on currency so we can't convert it to a number without knowing the currency conversion)

[lorenyu]

Instead of adding to_i, prefer to add a zero money constant and use that for summing e.g.

ZERO = new(0).freeze

then you can do [money1, money2, money3].sum(Strata::Money.ZERO)

(may want to add to the docs that this is how you sum an array of moneys)