kipcole9 / money_sql

Money functions for the serialization of a money data type in Elixir
Other
27 stars 18 forks source link

Unmatched clause for `Cldr.Decimal.parse/1` #10

Closed NikitaAvvakumov closed 3 years ago

NikitaAvvakumov commented 3 years ago

Hi 👋

Thanks for the great work on Money & CLDR!

I think there is a small bug in Money.Ecto.Composite.Type#cast/1: on L62 (https://github.com/kipcole9/money_sql/blob/master/lib/money/ecto/money_ecto_composite_type.ex#L62), the code calls Cldr.Decimal.parse/1 and the with clause matches on {amount, ""}, with else matching on {:error, {_, message}} or :error. However, when the amount cannot be parsed, Cldr.Decimal.parse/1 returns {:error, amount} (https://github.com/elixir-cldr/cldr_utils/blob/master/lib/cldr/decimal/decimal.ex#L34), which cannot match any of those.

iex(3)> Money.Ecto.Composite.Type.cast(%{"currency" => "USD", "amount" => "yes"})
** (WithClauseError) no with clause matching: {:error, "yes"}
    (ex_money_sql 1.3.1) lib/money/ecto/money_ecto_composite_type.ex:62: Money.Ecto.Composite.Type.cast/1

One option might be to change Money.Ecto.Composite.Type#cast/1 to match the correct return value, while the alternative could be to have Cldr.Decimal return the same :error atom that comes back from Decimal.parse/1 (https://github.com/ericmj/decimal/blob/v2.0.0/lib/decimal.ex#L1177) and which Money.Ecto.Composite.Type#cast/1 already expects to handle. I'll be happy to submit a PR in either case.

kipcole9 commented 3 years ago

My apologies for being so slow to respond. Github doesn't seem to be sending my notifications when new issues are opened on this repo.

As of commit ffec50c..3cfa502, the bug is fixed. The changeling entry is:

Bug Fixes