mathieuprog / polymorphic_embed

Polymorphic embeds in Ecto
Apache License 2.0
341 stars 63 forks source link

polymorphic embed doesnt run embed`s changeset #28

Closed BlueHotDog closed 3 years ago

BlueHotDog commented 3 years ago

given the following embed:

defmodule Embedded do
  @moduledoc false
  use Ecto.Schema
  import Ecto.Changeset

  @primary_key false
  embedded_schema do
    field :x, :string
  end

  def changeset(embedded, params) do
    embedded
    |> cast(params, [:x])
    |> validate_required([:x])
  end
end

And the following usage:

defmodule Parent do
  @moduledoc false
  use Ecto.Schema
  import PolymorphicEmbed, only: [cast_polymorphic_embed: 2]

  schema "patches" do
    field :data, PolymorphicEmbed, types: [
      embedded: Embedded
    ]
    timestamps()
  end

  @doc false
  def create_changeset(%Patch{} = patch) do
    patch
    |> cast(%{}, [])
    |> cast_polymorphic_embed(:data)
    |> validate_required([:data])
  end
end

I was expecting the changeset to run, but they're not. (checked via tests, by removing x attribute expecting things to fail, also the coveralls says it's never run)

mathieuprog commented 3 years ago

Will look into it 🎅

mathieuprog commented 3 years ago

Can you set the :on_type_not_found option to :raise and check if you have an error? I think the only way the changeset wouldn't run is that it cannot find the type (to pick the right embed).

Note also that I made a new release (1.0.0), and there is a breaking change, you have to set a new mandatory option :on_replace to :update.

BlueHotDog commented 3 years ago

Just tried, updated to v1, added the on_replace + on_type_not_found. still same issue

mathieuprog commented 3 years ago

We have tests that for example check for validation error messages. Without running the embed's changeset function that wouldn't be possible. Also since v1 there is no more way to cast an embed without having a changeset.

I would recommend you to inspect the changeset. As a last resort add some IO.inspect into the libary code and delete the _build folder and run mix deps.compile to see where the error lies.

The embed's changeset is called here: https://github.com/mathieuprog/polymorphic_embed/blob/v1.0.0/lib/polymorphic_embed.ex#L71 I would add inspects to check in which branch the code goes, delete the _build folder and run mix deps.compile, and we'll have an answer.

Let me know

BlueHotDog commented 3 years ago

Ok, i think i found it. I was expecting i could just pass the %Embedded{} module as a :data to Parent. But seems like i need to specifically specify a __type__ and take the struct and convert it to a map.

I think this part is actually just missing in the documentation

mathieuprog commented 3 years ago

Actually that should work. We know for example Embedded is of which type.

But you mentioned that you expect the changeset function to run. That means that you send in params for that field? In that case you need indeed the __type__.

It's a bit confusing sorry. But you can check this test: https://github.com/mathieuprog/polymorphic_embed/blob/v1.0.0/test/polymorphic_embed_test.exs#L95

Structs can be stored without converting to a map with __type__ field. I would not recommend going that way and find the issue instead.

BlueHotDog commented 3 years ago

Sorry for using this issue to debug and troubleshoot, but i think what i'm trying is something other people will tackle too. Ok, managed to proceed, but now stuck on another weird error:

  ** (FunctionClauseError) no function clause matching in PolymorphicEmbed.dump/3

     The following arguments were given to PolymorphicEmbed.dump/3:

         # 1
         %{__type__: :embedded, x: "3"}

         # 2
         #Function<31.132136930/2 in Ecto.Type.process_dumpers/3>

         # 3
         %{metadata: [%{identify_by_fields: [], module: Embedded, type: "embedded"}], on_type_not_found: :raise}

     Attempted function clauses (showing 3 out of 3):

         def dump(%Ecto.Changeset{valid?: false}, _dumper, _params)
         def dump(%_module{} = struct, dumper, %{metadata: metadata})
         def dump(nil, dumper, _params)
mathieuprog commented 3 years ago

That's interesting. I would like to add this as a test case in the project. As soon as I can reproduce it, it will be fixed. Is your code shareable somehow?

If not, I need more info as to how that error got raised.

mathieuprog commented 3 years ago

Troubleshoot'ed through Discord.

Harrisonl commented 1 year ago

I'm running into this issue as well - (the last one RE dump/3) is there anywhere I can we where this was resolved?

Harrisonl commented 1 year ago

Ok figured it out - this was because the schema with the polymorphic defined, was being called by a parent schema's changeset using put_assoc instead of cast_assoc