kipcole9 / money_sql

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

Protocol.UndefinedError exception for Jason.Encoder when embedded schema is configured incorrectly with Money.Ecto.Composite.Type #33

Closed coladarci closed 1 year ago

coladarci commented 1 year ago

Heyya -

When running the following update:

  ex_money 5.12.3 => 5.13.0
  ex_money_sql 1.8.0 => 1.9.2

I suddenly get the following:

** (Protocol.UndefinedError) protocol Jason.Encoder not implemented for {"USD", Decimal.new("50.00")} of type Tuple

There isn't a good stack trace to point to anything specific, but I'm hoping you have some ideas?

Appreciate any ideas!

kipcole9 commented 1 year ago

Apologies, that's extremely inconvenient.

The type {"USD", Decimal.new("50.00")} is the format that comes back from Postgrex for the money_with_currency composite type in Postgres. Is that correct in your case?

If so, the likely culprit is ex_money_sql and as a first measure I'd recommend first moving back to ex_money_sql 1.9.0.

There were some changes to the type definition in 1.9.2 that related to embedded schemas. By any chance would you be using embedded schemas with a money type in Postgres? That would be a second line of enquiry. Because money_sql itself doesn't use JSON encoding directly - it's only (as I understand it) when Ecto is encoding a map() type field or an embedded schema.

Hope that gives you a more specific place to look and one thing to try. If you are able to isolate the Ecto schema that is raising the issue and you are able to share it that would be extremely helpful.

coladarci commented 1 year ago

Thanks! Yes we definitely embed Money Objects - we can try rolling back, but what can I do to help sort out the potential bug around embedded moneys?

kipcole9 commented 1 year ago

My guess (and it's a guess) is that in your embedded schemas the type is Money.Ecto.Composite.Type when it needs to be Money.Ecto.Map.Type. It seems to have worked a bit by accident in earlier releases and I didn't have good enough test coverage.

I merged a PR that fixed some issues with embedded schemas but it may have introduced the issue you are seeing. If money_sql version 1.9.1 works but 1.9.2 doesn't then that helps narrow down where to look. I'm a little surprised though because the tuple form like {"USD", Decimal.new("50.00")} can only come from the composite type.

Summary

  1. An embedded money type needs to be Money.Ecto.Map.Type like this:

    schema "organizations" do
    embeds_many :customers, Customer do
      field :name, :string
      field :revenue, Money.Ecto.Map.Type, default: Money.new(:USD, 0)
    end
    end
  2. If 1.9.1 works and 1.9.2 doesn't then its isolated to the referenced PR and I have at least somewhere to start looking.

coladarci commented 1 year ago

I can confirm that flipping to Money.Ecto.Map.Type solves my problems!

kipcole9 commented 1 year ago

Phew, extremely happy to hear that. I will make a much clearer note in the readme and the changelog. The previous behaviour was erroneous and I'm surprised it actually worked - but clearly not a great upgrade experience.

coladarci commented 1 year ago

What made this wonderful is that I had zero doubt you'd have an answer within 24 hours (and more likely within a few) and you did not disappoint. I appreciate you and this library more than you can know!

kipcole9 commented 1 year ago

Thanks for the kind words Greg, it's much appreciated and definitely keeps my. motivation high.

kipcole9 commented 1 year ago

I've updated the readme to be as super clear and obvious as possible. And I've updated the changelog too.

Hopefully that means other people won't have the same issue I put you through.