Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
NikitaAvvakumov opened this issue Dec 13, 2020 · 1 comment
Closed

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

NikitaAvvakumov opened this issue Dec 13, 2020 · 1 comment

Comments

@NikitaAvvakumov
Copy link

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants