From 171f4fccbe92433a4ffeabaab67182ea03d27264 Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 22 Oct 2026 13:38:56 +0200 Subject: [PATCH 01/11] Add parity tests against Ecto embeds ecto_parity_test.exs mirrors test/ecto/changeset/embedded_test.exs from Ecto v3.12.1 for polymorphic embeds. form_parity_test.exs compares the forms of embeds_one and embeds_many with the forms of the polymorphic helpers for the same params. The goal is output parity: for the same input, a polymorphic embed gives the same output as an Ecto embed. Tests for Ecto features that polymorphic embeds do not have are left out. Most tests fail on purpose. Each failure shows a difference from Ecto that a later commit addresses. --- test/polymorphic_embed/ecto_parity_test.exs | 535 ++++++++++++++++++++ test/polymorphic_embed/form_parity_test.exs | 335 ++++++++++++ 2 files changed, 870 insertions(+) create mode 100644 test/polymorphic_embed/ecto_parity_test.exs create mode 100644 test/polymorphic_embed/form_parity_test.exs diff --git a/test/polymorphic_embed/ecto_parity_test.exs b/test/polymorphic_embed/ecto_parity_test.exs new file mode 100644 index 0000000..409952e --- /dev/null +++ b/test/polymorphic_embed/ecto_parity_test.exs @@ -0,0 +1,535 @@ +# Mirrors test/ecto/changeset/embedded_test.exs from Ecto v3.12.1. +# +# The goal is output parity: for the same input, a polymorphic embed gives +# the same output as an Ecto embed. Each test keeps the name and the +# assertions of the Ecto test. The only changes are the ones a polymorphic +# field needs: a type key in the params, cast_polymorphic_embed/3 in place of +# cast_embed/3, `with:` as a keyword list of types, and +# PolymorphicEmbed.traverse_errors/2. Tests for Ecto features that +# polymorphic embeds do not have are left out. +defmodule PolymorphicEmbed.EctoParityTest do + use ExUnit.Case, async: true + + alias Ecto.Changeset + + alias __MODULE__.{Author, Post, Profile} + + defmodule Post do + use Ecto.Schema + import Ecto.Changeset + + # Ecto uses `schema "posts"`. Polymorphic embeds generate ids at cast time + # and crash on an autogenerated integer id, see the test for AutoIdPost. + @primary_key {:id, :id, autogenerate: false} + schema "posts" do + field :title, :string + field :position, :integer + end + + def changeset(schema, params) do + cast(schema, params, ~w(title)a) + |> validate_required(:title) + |> validate_length(:title, min: 3) + end + + def optional_changeset(schema, params) do + cast(schema, params, ~w(title)a) + end + end + + defmodule Profile do + use Ecto.Schema + import Ecto.Changeset + + embedded_schema do + field :name + end + + def changeset(schema, params) do + cast(schema, params, ~w(name id)a) + |> validate_required(:name) + |> validate_length(:name, min: 3) + end + + def optional_changeset(schema, params) do + cast(schema, params, ~w(name)a) + end + end + + defmodule AutoIdPost do + use Ecto.Schema + + schema "posts" do + field :title, :string + end + + def changeset(schema, params), do: Ecto.Changeset.cast(schema, params, [:title]) + end + + defmodule Author do + use Ecto.Schema + import PolymorphicEmbed + + # Polymorphic embeds only accept on_replace: :update for one and + # on_replace: :delete for many. + schema "authors" do + field :name, :string + polymorphic_embeds_one :profile, types: [profile: Profile], on_replace: :update + polymorphic_embeds_many :posts, types: [post: Post], on_replace: :delete + polymorphic_embeds_many :auto_id_posts, types: [post: AutoIdPost], on_replace: :delete + end + end + + defp cast(schema, params, embed, opts \\ []) do + schema + |> Changeset.cast(params, ~w()) + |> PolymorphicEmbed.cast_polymorphic_embed(embed, opts) + end + + defp profile(params), do: Map.put(params, "__type__", "profile") + defp post(params), do: Map.put(params, "__type__", "post") + + ## Cast embeds one + + test "cast embeds_one with valid params" do + changeset = cast(%Author{}, %{"profile" => profile(%{"name" => "michal"})}, :profile) + profile = changeset.changes.profile + assert profile.changes == %{name: "michal"} + assert profile.errors == [] + assert profile.action == :insert + assert profile.valid? + assert changeset.valid? + end + + test "cast embeds_one with invalid params" do + changeset = cast(%Author{}, %{"profile" => profile(%{})}, :profile) + assert changeset.changes.profile.changes == %{} + assert changeset.changes.profile.errors == [name: {"can't be blank", [validation: :required]}] + assert changeset.changes.profile.action == :insert + refute changeset.changes.profile.valid? + refute changeset.valid? + end + + test "cast embeds_one with invalid params: string" do + changeset = cast(%Author{}, %{"profile" => "value"}, :profile, required: true) + assert changeset.errors == [profile: {"is invalid", [validation: :embed, type: :map]}] + refute changeset.valid? + end + + test "cast embeds_one with invalid params: list" do + changeset = cast(%Author{}, %{"profile" => [profile(%{"name" => "michal"})]}, :profile) + assert changeset.changes == %{} + assert changeset.errors == [profile: {"is invalid", [validation: :embed, type: :map]}] + end + + test "cast embeds_one with existing struct updating" do + changeset = + cast( + %Author{profile: %Profile{name: "michal", id: "michal"}}, + %{"profile" => profile(%{"name" => "new", "id" => "michal"})}, + :profile + ) + + profile = changeset.changes.profile + assert profile.changes == %{name: "new"} + assert profile.errors == [] + assert profile.action == :update + assert profile.valid? + assert changeset.valid? + end + + test "cast embeds_one with existing struct updating from atom params" do + # Emulate atom params from nested associations + changeset = + Changeset.cast(%Author{profile: %Profile{name: "michal", id: "michal"}}, %{}, ~w()) + + changeset = + put_in(changeset.params, %{"profile" => %{name: "new", id: "michal", __type__: "profile"}}) + + changeset = PolymorphicEmbed.cast_polymorphic_embed(changeset, :profile, []) + profile = changeset.changes.profile + assert profile.changes == %{name: "new"} + assert profile.errors == [] + assert profile.action == :update + assert profile.valid? + assert changeset.valid? + end + + test "cast embeds_one without changes skips" do + changeset = + cast( + %Author{profile: %Profile{name: "michal", id: "michal"}}, + %{"profile" => profile(%{"id" => "michal"})}, + :profile + ) + + assert changeset.changes == %{} + assert changeset.errors == [] + end + + test "cast embeds_one when required" do + changeset = cast(%Author{profile: nil}, %{}, :profile, required: true) + assert changeset.required == [:profile] + assert changeset.changes == %{} + assert changeset.errors == [profile: {"can't be blank", [validation: :required]}] + end + + test "cast embeds_one when required with existing data" do + changeset = cast(%Author{profile: %Profile{}}, %{}, :profile, required: true) + assert changeset.required == [:profile] + assert changeset.changes == %{} + assert changeset.errors == [] + end + + test "cast embeds_one when required with nil params" do + changeset = cast(%Author{profile: nil}, %{"profile" => nil}, :profile, required: true) + assert changeset.required == [:profile] + assert changeset.changes == %{} + assert changeset.errors == [profile: {"can't be blank", [validation: :required]}] + end + + test "cast embeds_one when required with nil params and existing data" do + changeset = cast(%Author{profile: %Profile{}}, %{"profile" => nil}, :profile, required: true) + assert changeset.required == [:profile] + assert changeset.changes == %{profile: nil} + assert changeset.errors == [profile: {"can't be blank", [validation: :required]}] + end + + test "cast embeds_one with optional" do + changeset = cast(%Author{profile: %Profile{id: "id"}}, %{"profile" => nil}, :profile) + assert changeset.changes.profile == nil + assert changeset.valid? + end + + test "cast embeds_one with custom changeset" do + changeset = + cast(%Author{}, %{"profile" => profile(%{"name" => "michal"})}, :profile, + with: [profile: &Profile.optional_changeset/2] + ) + + profile = changeset.changes.profile + assert profile.changes == %{name: "michal"} + assert profile.errors == [] + assert profile.action == :insert + assert profile.valid? + assert changeset.valid? + end + + test "cast embeds_one with empty parameters" do + changeset = cast(%Author{profile: nil}, %{}, :profile) + assert changeset.changes == %{} + + changeset = cast(%Author{profile: %Profile{}}, %{}, :profile) + assert changeset.changes == %{} + end + + test "cast embeds_one with on_replace: :update" do + schema = %Author{name: "Enio", profile: %Profile{id: "id", name: "Enio"}} + + changeset = cast(schema, %{"profile" => profile(%{"name" => "Jose"})}, :profile) + assert changeset.changes.profile.changes == %{name: "Jose"} + assert changeset.changes.profile.action == :update + assert changeset.errors == [] + assert changeset.valid? + end + + test "raises when :update is used on embeds_many" do + assert_raise ArgumentError, fn -> + defmodule Topic do + use Ecto.Schema + import PolymorphicEmbed + + schema "topics" do + polymorphic_embeds_many :tags, types: [post: Post], on_replace: :update + end + end + end + end + + # Not in Ecto: Ecto casts this schema without an error. + test "cast embeds_many with an autogenerated integer id" do + changeset = + cast(%Author{}, %{"auto_id_posts" => [post(%{"title" => "hello"})]}, :auto_id_posts) + + assert [%Changeset{changes: %{title: "hello"}}] = changeset.changes.auto_id_posts + end + + ## cast embeds many + + test "cast embeds_many with only new schemas" do + changeset = cast(%Author{}, %{"posts" => [post(%{"title" => "hello"})]}, :posts) + [post_change] = changeset.changes.posts + assert post_change.changes == %{title: "hello"} + assert post_change.errors == [] + assert post_change.action == :insert + assert post_change.valid? + assert changeset.valid? + end + + test "cast embeds_many with map" do + changeset = cast(%Author{}, %{"posts" => %{0 => post(%{"title" => "hello"})}}, :posts) + [post_change] = changeset.changes.posts + assert post_change.changes == %{title: "hello"} + assert post_change.errors == [] + assert post_change.action == :insert + assert post_change.valid? + assert changeset.valid? + end + + describe "cast embeds_many with map and sort_param and delete_param" do + setup do + posts = %{ + 1 => post(%{"title" => "one"}), + 2 => post(%{"title" => "two"}), + 3 => post(%{"title" => "three"}) + } + + %{posts: posts, opts: [sort_param: :sort, drop_param: :drop]} + end + + test "no sort or drop", %{posts: posts, opts: opts} do + changeset = cast(%Author{}, %{"posts" => posts}, :posts, opts) + assert Enum.map(changeset.changes.posts, & &1.changes[:title]) == ~w(one two three) + end + + test "drop with string keys", %{posts: posts, opts: opts} do + changeset = cast(%Author{}, %{"posts" => posts, "drop" => [2]}, :posts, opts) + assert Enum.map(changeset.changes.posts, & &1.changes[:title]) == ["one", "three"] + end + + test "drop with atom keys", %{posts: posts, opts: opts} do + changeset = cast(%Author{}, %{posts: posts, drop: [2]}, :posts, opts) + assert Enum.map(changeset.changes.posts, & &1.changes[:title]) == ["one", "three"] + end + + test "sort with string keys", %{posts: posts, opts: opts} do + changeset = cast(%Author{}, %{"posts" => posts, "sort" => [2, 3, 1]}, :posts, opts) + assert Enum.map(changeset.changes.posts, & &1.changes[:title]) == ~w(two three one) + end + + test "sort creates a new entry", %{opts: opts} do + changeset = cast(%Author{}, %{sort: ["new"]}, :posts, opts) + assert Enum.map(changeset.changes.posts, & &1.changes[:title]) == [nil] + end + + test "drop replaces existing data", %{opts: opts} do + changeset = cast(%Author{posts: [%Post{title: "one"}]}, %{drop: [0]}, :posts, opts) + assert Enum.map(changeset.changes.posts, & &1.action) == [:replace] + end + + test "partial sort and new entries", %{posts: posts, opts: opts} do + changeset = cast(%Author{}, %{posts: posts, sort: [2, 3, 1]}, :posts, opts) + assert Enum.map(changeset.changes.posts, & &1.changes[:title]) == ~w(two three one) + + changeset = cast(%Author{}, %{posts: posts, sort: [2]}, :posts, opts) + assert Enum.map(changeset.changes.posts, & &1.changes[:title]) == ~w(two one three) + + changeset = cast(%Author{}, %{posts: posts, sort: [2, "new"]}, :posts, opts) + + assert Enum.map(changeset.changes.posts, & &1.changes[:title]) == + ["two", nil, "one", "three"] + end + + test "sort and drop together", %{posts: posts, opts: opts} do + changeset = cast(%Author{}, %{posts: posts, sort: [3, 2, 1], drop: [2]}, :posts, opts) + assert Enum.map(changeset.changes.posts, & &1.changes[:title]) == ["three", "one"] + + changeset = + cast(%Author{}, %{posts: posts, sort: [1, "new"], drop: [2, "new"]}, :posts, opts) + + assert Enum.map(changeset.changes.posts, & &1.changes[:title]) == ["one", "three"] + end + + test "sort and drop that are not lists", %{posts: posts, opts: opts} do + changeset = cast(%Author{}, %{posts: posts, sort: :x, drop: :y}, :posts, opts) + + assert Enum.map(changeset.changes.posts, & &1.changes[:title]) == + ["one", "two", "three"] + end + end + + test "cast embeds_many with custom changeset" do + changeset = + cast(%Author{}, %{"posts" => [post(%{"title" => "hello"})]}, :posts, + with: [post: &Post.optional_changeset/2] + ) + + [post_change] = changeset.changes.posts + assert post_change.changes == %{title: "hello"} + assert post_change.errors == [] + assert post_change.action == :insert + assert post_change.valid? + assert changeset.valid? + end + + # Please note the order is important in this test. + test "cast embeds_many changing schemas" do + posts = [ + %Post{title: "first", id: 1}, + %Post{title: "second", id: 2}, + %Post{title: "third", id: 3} + ] + + params = [ + post(%{"title" => "new"}), + post(%{"id" => "2", "title" => nil}), + post(%{"id" => "3", "title" => "new name"}) + ] + + changeset = cast(%Author{posts: posts}, %{"posts" => params}, :posts) + [first, new, second, third] = changeset.changes.posts + + assert first.data.id == 1 + # Check for not running changeset function + assert first.required == [] + assert first.action == :replace + assert first.valid? + + assert new.changes == %{title: "new"} + assert new.action == :insert + assert new.valid? + + assert second.data.id == 2 + assert second.errors == [title: {"can't be blank", [validation: :required]}] + assert second.action == :update + refute second.valid? + + assert third.data.id == 3 + assert third.action == :update + assert third.valid? + refute changeset.valid? + end + + test "cast embeds_many with invalid params: string" do + changeset = cast(%Author{}, %{"posts" => "value"}, :posts) + assert changeset.errors == [posts: {"is invalid", [validation: :embed, type: {:array, :map}]}] + refute changeset.valid? + end + + test "cast embeds_many with invalid params: list of strings" do + changeset = cast(%Author{}, %{"posts" => ["value"]}, :posts) + assert changeset.errors == [posts: {"is invalid", [validation: :embed, type: {:array, :map}]}] + refute changeset.valid? + end + + test "cast embeds_many with invalid params: nil" do + changeset = cast(%Author{}, %{"posts" => nil}, :posts) + assert changeset.errors == [posts: {"is invalid", [validation: :embed, type: {:array, :map}]}] + refute changeset.valid? + end + + test "cast embeds_many with invalid params: map that is not indexed" do + changeset = cast(%Author{}, %{"posts" => %{"id" => "invalid"}}, :posts) + assert changeset.errors == [posts: {"is invalid", [validation: :embed, type: {:array, :map}]}] + refute changeset.valid? + end + + test "cast embeds_many without changes skips" do + changeset = + cast( + %Author{posts: [%Post{title: "hello", id: 1}]}, + %{"posts" => [post(%{"id" => 1})]}, + :posts + ) + + refute Map.has_key?(changeset.changes, :posts) + end + + test "cast embeds_many when required" do + changeset = cast(%Author{posts: []}, %{}, :posts, required: true) + assert changeset.required == [:posts] + assert changeset.changes == %{} + assert changeset.errors == [posts: {"can't be blank", [validation: :required]}] + end + + test "cast embeds_many when required with nil params" do + changeset = cast(%Author{posts: []}, %{"posts" => nil}, :posts, required: true) + assert changeset.changes == %{} + assert changeset.errors == [posts: {"is invalid", [validation: :embed, type: {:array, :map}]}] + end + + test "cast embeds_many with empty parameters" do + changeset = cast(%Author{posts: []}, %{}, :posts) + assert changeset.changes == %{} + + changeset = cast(%Author{posts: [%Post{}]}, %{}, :posts) + assert changeset.changes == %{} + end + + ## traverse_errors + + defp upcase_errors(changeset) do + PolymorphicEmbed.traverse_errors(changeset, fn {msg, opts} -> + msg + |> String.replace("%{count}", to_string(opts[:count])) + |> String.upcase() + end) + end + + test "traverses changeset errors with embeds_one error" do + params = %{"name" => "hi", "profile" => profile(%{"name" => "hi"})} + + changeset = + %Author{} + |> Changeset.cast(params, ~w(name)a) + |> PolymorphicEmbed.cast_polymorphic_embed(:profile) + |> Changeset.add_error(:name, "is invalid") + + assert upcase_errors(changeset) == %{ + profile: %{name: ["SHOULD BE AT LEAST 3 CHARACTER(S)"]}, + name: ["IS INVALID"] + } + end + + test "traverses changeset errors with embeds_many errors" do + params = %{ + "name" => "hi", + "posts" => [post(%{"title" => "hi"}), post(%{"title" => "valid"})] + } + + changeset = + %Author{} + |> Changeset.cast(params, ~w(name)a) + |> PolymorphicEmbed.cast_polymorphic_embed(:posts) + |> Changeset.add_error(:name, "is invalid") + + assert upcase_errors(changeset) == %{ + posts: [%{title: ["SHOULD BE AT LEAST 3 CHARACTER(S)"]}, %{}], + name: ["IS INVALID"] + } + end + + test "traverses changeset errors with embeds_many when required" do + changeset = cast(%Author{posts: []}, %{}, :posts, required: true) + assert changeset.errors == [posts: {"can't be blank", [validation: :required]}] + + assert PolymorphicEmbed.traverse_errors(changeset, & &1) == %{ + posts: [{"can't be blank", [validation: :required]}] + } + + changeset = cast(%Author{}, %{"posts" => []}, :posts, required: true) + assert changeset.errors == [posts: {"can't be blank", [validation: :required]}] + + assert PolymorphicEmbed.traverse_errors(changeset, & &1) == %{ + posts: [{"can't be blank", [validation: :required]}] + } + + changeset = cast(%Author{posts: []}, %{"posts" => nil}, :posts, required: true) + assert changeset.errors == [posts: {"is invalid", [validation: :embed, type: {:array, :map}]}] + + assert PolymorphicEmbed.traverse_errors(changeset, & &1) == %{ + posts: [{"is invalid", [validation: :embed, type: {:array, :map}]}] + } + + changeset = + cast(%Author{posts: []}, %{"posts" => [%{title: nil, __type__: "post"}]}, :posts, + required: true + ) + + assert changeset.errors == [] + + assert PolymorphicEmbed.traverse_errors(changeset, & &1) == %{ + posts: [%{title: [{"can't be blank", [validation: :required]}]}] + } + end +end diff --git a/test/polymorphic_embed/form_parity_test.exs b/test/polymorphic_embed/form_parity_test.exs new file mode 100644 index 0000000..b345aac --- /dev/null +++ b/test/polymorphic_embed/form_parity_test.exs @@ -0,0 +1,335 @@ +# Compares the forms that Phoenix builds for embeds_one/embeds_many +# (Phoenix.HTML.FormData, which inputs_for uses) with the forms that +# PolymorphicEmbed.HTML.Helpers builds for polymorphic_embeds_one/many. +# +# Both parents use the same child schema and get the same params. Each test +# takes a snapshot of every form, its field values and its nested forms, and +# asserts that the polymorphic snapshot has no differences from the regular +# one. The only expected difference is the type field, which the snapshot +# drops. A failure lists each differing path with the regular and the +# polymorphic value. +defmodule PolymorphicEmbed.FormParityTest do + use ExUnit.Case, async: true + + alias Ecto.Changeset + alias PolymorphicEmbed.HTML.Helpers + + alias __MODULE__.{Item, Meta, PolyParent, RegularParent, Tag} + + defmodule Meta do + use Ecto.Schema + import Ecto.Changeset + + embedded_schema do + field :note, :string + end + + def changeset(meta, params) do + meta + |> cast(params, [:note]) + |> validate_length(:note, min: 3) + end + end + + defmodule Tag do + use Ecto.Schema + import Ecto.Changeset + + embedded_schema do + field :name, :string + end + + def changeset(tag, params) do + tag + |> cast(params, [:name]) + |> validate_required([:name]) + end + end + + defmodule Item do + use Ecto.Schema + import Ecto.Changeset + + embedded_schema do + field :title, :string + embeds_one :meta, Meta, on_replace: :update + embeds_many :tags, Tag, on_replace: :delete + end + + def changeset(item, params) do + item + |> cast(params, [:title]) + |> validate_required([:title]) + |> cast_embed(:meta) + |> cast_embed(:tags) + end + end + + defmodule RegularParent do + use Ecto.Schema + + embedded_schema do + embeds_one :item, Item, on_replace: :update + embeds_many :items, Item, on_replace: :delete + end + end + + defmodule PolyParent do + use Ecto.Schema + import PolymorphicEmbed + + embedded_schema do + polymorphic_embeds_one :item, types: [item: Item], on_replace: :update + polymorphic_embeds_many :items, types: [item: Item], on_replace: :delete + end + end + + @type_key "__type__" + @cast_opts [sort_param: :order, drop_param: :drop] + + ## Scenarios + + # {name, data, field params, parent action} + # Field params for :items use a map of rows, as an HTML form sends them. + @scenarios [ + {"new valid row", %{}, %{"title" => "Hello", "_persistent_id" => "p0"}, :validate}, + {"new invalid row", %{}, %{"title" => "", "_persistent_id" => "p0"}, :validate}, + {"new invalid row before submit", %{}, %{"title" => "", "_persistent_id" => "p0"}, nil}, + {"new row with valid nested embeds", %{}, + %{ + "title" => "Hello", + "meta" => %{"note" => "a note"}, + "tags" => %{"0" => %{"name" => "a"}, "1" => %{"name" => "b"}} + }, :validate}, + {"new row with invalid nested embeds", %{}, + %{ + "title" => "Hello", + "meta" => %{"note" => "x"}, + "tags" => %{"0" => %{"name" => ""}} + }, :validate}, + {"existing data without params", :existing, nil, nil}, + {"existing data updated by id", :existing, + %{ + "id" => "i1", + "title" => "New", + "meta" => %{"id" => "m1", "note" => "new note"}, + "tags" => %{"0" => %{"id" => "t1", "name" => "new tag"}} + }, :validate} + ] + + for {name, data, field_params, action} <- @scenarios do + test "embeds_one: #{name}" do + data = data(unquote(Macro.escape(data)), :one) + params = params(:item, unquote(Macro.escape(field_params))) + assert_parity(data, params, :item, unquote(action)) + end + + test "embeds_many: #{name}" do + data = data(unquote(Macro.escape(data)), :many) + params = params(:items, unquote(Macro.escape(field_params))) + assert_parity(data, params, :items, unquote(action)) + end + end + + test "embeds_one: no data and no params" do + assert_parity(%{}, %{}, :item, nil, polymorphic_type: :item) + end + + test "embeds_many: two rows with list params" do + params = %{ + "items" => [ + %{"title" => "One", "_persistent_id" => "p0"}, + %{"title" => "", "_persistent_id" => "p1"} + ] + } + + assert_parity(%{}, params, :items, :validate) + end + + test "embeds_many: sort and drop" do + params = %{ + "items" => %{ + "0" => %{"title" => "Zero", "_persistent_id" => "p0"}, + "1" => %{"title" => "", "_persistent_id" => "p1"}, + "2" => %{"title" => "Two", "_persistent_id" => "p2"} + }, + "order" => ["2", "1", "0", "new"], + "drop" => ["0"] + } + + assert_parity(%{}, params, :items, :validate) + end + + test "embeds_many: sort and drop on existing data" do + params = %{ + "items" => %{ + "0" => %{"id" => "i1", "title" => "Old"}, + "1" => %{"id" => "i2", "title" => "Other"} + }, + "order" => ["1", "0"], + "drop" => ["1"] + } + + assert_parity(data(:existing, :many), params, :items, :validate) + end + + ## Helpers + + defp data(:existing, :one), do: %{item: existing_item()} + defp data(:existing, :many), do: %{items: [existing_item(), existing_item2()]} + defp data(%{} = data, _), do: data + + defp existing_item do + %Item{ + id: "i1", + title: "Old", + meta: %Meta{id: "m1", note: "old note"}, + tags: [%Tag{id: "t1", name: "old tag"}] + } + end + + defp existing_item2, do: %Item{id: "i2", title: "Other", meta: nil, tags: []} + + defp params(_field, nil), do: %{} + defp params(:item, row), do: %{"item" => row} + defp params(:items, row), do: %{"items" => %{"0" => row}} + + defp assert_parity(data, params, field, action, poly_opts \\ []) do + regular = + struct(RegularParent, data) + |> Changeset.cast(params, []) + |> Changeset.cast_embed(field, @cast_opts) + |> Map.put(:action, action) + + poly = + struct(PolyParent, data) + |> Changeset.cast(add_type(params, field), []) + |> PolymorphicEmbed.cast_polymorphic_embed(field, @cast_opts) + |> Map.put(:action, action) + + regular_forms = + Phoenix.HTML.FormData.to_form( + regular, + Phoenix.Component.to_form(regular, as: "parent"), + field, + [] + ) + + poly_forms = + Helpers.to_form(poly, Phoenix.Component.to_form(poly, as: "parent"), field, poly_opts) + + assert diff(snapshot_forms(regular_forms), snapshot_forms(poly_forms), [field]) == [] + end + + defp add_type(params, :item) do + case params do + %{"item" => %{} = row} -> %{params | "item" => Map.put(row, @type_key, "item")} + params -> params + end + end + + defp add_type(params, :items) do + case params do + %{"items" => rows} when is_list(rows) -> + %{params | "items" => Enum.map(rows, &Map.put(&1, @type_key, "item"))} + + %{"items" => rows} -> + %{params | "items" => Map.new(rows, fn {k, v} -> {k, Map.put(v, @type_key, "item")} end)} + + params -> + params + end + end + + # A snapshot holds what a template can read from a form. + defp snapshot_forms(forms), do: Enum.map(forms, &snapshot_form/1) + + defp snapshot_form(%Phoenix.HTML.Form{} = form) do + module = form.source.data.__struct__ + + %{ + id: form.id, + name: form.name, + index: form.index, + action: form.action, + errors: form.errors, + params: drop_type(form.params), + hidden: Keyword.delete(form.hidden, String.to_atom(@type_key)) |> drop_type_hidden(), + data: normalize(form.data), + source_action: form.source.action, + source_valid?: form.source.valid?, + values: Map.new(module.__schema__(:fields), &{&1, normalize(form[&1].value)}), + applied_values: + Map.new(module.__schema__(:fields), &{&1, normalize(applied(form[&1].value))}), + nested: + Map.new(module.__schema__(:embeds), fn embed -> + {embed, snapshot_forms(Phoenix.HTML.FormData.to_form(form.source, form, embed, []))} + end) + } + end + + defp drop_type(params) when is_map(params), do: Map.delete(params, @type_key) + defp drop_type(params), do: params + + defp drop_type_hidden(hidden), + do: Enum.reject(hidden, fn {key, _} -> to_string(key) == @type_key end) + + defp applied(%Changeset{} = changeset), do: Changeset.apply_changes(changeset) + defp applied(list) when is_list(list), do: Enum.map(list, &applied/1) + defp applied(value), do: value + + # Shows a value by its kind, so a diff tells a changeset, a struct and raw + # params apart. Generated UUIDs are replaced, so that runs are stable. + defp normalize(%Changeset{} = changeset) do + {:changeset, + %{ + action: changeset.action, + changes: normalize(changeset.changes), + errors: changeset.errors, + valid?: changeset.valid? + }} + end + + defp normalize(%_{} = struct) do + {:struct, struct.__struct__, + struct |> Map.from_struct() |> Map.delete(:__meta__) |> normalize()} + end + + defp normalize(%{} = map), do: Map.new(map, fn {k, v} -> {k, normalize(v)} end) + defp normalize(list) when is_list(list), do: Enum.map(list, &normalize/1) + + defp normalize(value) when is_binary(value) do + if Regex.match?(~r/\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/, value), + do: "", + else: value + end + + defp normalize(value), do: value + + # Returns a list of {path, regular, polymorphic} for each leaf that differs. + defp diff(same, same, _path), do: [] + + defp diff(%{} = regular, %{} = poly, path) + when not is_struct(regular) and not is_struct(poly) do + (Map.keys(regular) ++ Map.keys(poly)) + |> Enum.uniq() + |> Enum.flat_map( + &diff(Map.get(regular, &1, :missing), Map.get(poly, &1, :missing), path ++ [&1]) + ) + end + + defp diff(regular, poly, path) + when is_list(regular) and is_list(poly) and length(regular) == length(poly) and + path != [] do + if Keyword.keyword?(regular) and regular != [] do + [{Enum.join(path, "."), regular, poly}] + else + regular + |> Enum.zip(poly) + |> Enum.with_index() + |> Enum.flat_map(fn {{r, p}, i} -> diff(r, p, path ++ [i]) end) + end + end + + defp diff(regular, poly, path), do: [{Enum.join(path, "."), regular, poly}] +end From 98acb8a7fe11496d95d937a6ef83cc4be424dd2b Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 22 Oct 2026 13:47:34 +0200 Subject: [PATCH 02/11] Return "is invalid" for params that are not maps Before, a string or a list for a single embed, or a string, nil or a list with values that are not maps for a list of embeds, crashed with a CaseClauseError or a FunctionClauseError. Now the cast adds the same error as Ecto: {"is invalid", [validation: :embed, type: :map]} or type {:array, :map}. nil for a list of embeds is now invalid, as in Ecto. Before, the cast ignored it. The test "supports lists of polymorphic embeds" sent contexts2: nil without a reason, so the line is removed. The new test "nil for a list of embeds is invalid" checks the error for the regular and the polymorphic schema. --- lib/polymorphic_embed.ex | 30 ++++++++++++++++++++++++++++++ test/polymorphic_embed_test.exs | 23 ++++++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/lib/polymorphic_embed.ex b/lib/polymorphic_embed.ex index 47e8ad9..ce7c448 100644 --- a/lib/polymorphic_embed.ex +++ b/lib/polymorphic_embed.ex @@ -166,6 +166,17 @@ defmodule PolymorphicEmbed do changeset end + {:ok, params_for_field} + when array? and not is_list(params_for_field) and not is_map(params_for_field) -> + add_invalid_error(changeset, field, array?) + + {:ok, nil} when array? -> + add_invalid_error(changeset, field, array?) + + {:ok, params_for_field} + when not array? and not is_map(params_for_field) and not is_nil(params_for_field) -> + add_invalid_error(changeset, field, array?) + {:ok, nil} when required -> Ecto.Changeset.add_error(changeset, field, "can't be blank", validation: :required) @@ -393,7 +404,26 @@ defmodule PolymorphicEmbed do end end + defp cast_polymorphic_embeds_many(changeset, field, _changeset_fun, list_params, _field_opts) + when not is_list(list_params) do + add_invalid_error(changeset, field, true) + end + defp cast_polymorphic_embeds_many(changeset, field, changeset_fun, list_params, field_opts) do + if Enum.all?(list_params, &is_map/1) do + do_cast_polymorphic_embeds_many(changeset, field, changeset_fun, list_params, field_opts) + else + add_invalid_error(changeset, field, true) + end + end + + # The same error as Ecto gives for params that are not a map (or a list of maps). + defp add_invalid_error(changeset, field, array?) do + type = if array?, do: {:array, :map}, else: :map + Ecto.Changeset.add_error(changeset, field, "is invalid", validation: :embed, type: type) + end + + defp do_cast_polymorphic_embeds_many(changeset, field, changeset_fun, list_params, field_opts) do %{ types_metadata: types_metadata, on_type_not_found: on_type_not_found, diff --git a/test/polymorphic_embed_test.exs b/test/polymorphic_embed_test.exs index ba50279..020c597 100644 --- a/test/polymorphic_embed_test.exs +++ b/test/polymorphic_embed_test.exs @@ -1777,7 +1777,6 @@ defmodule PolymorphicEmbedTest do address: "address" } ], - contexts2: nil, contexts3: [ %{ __type__: "device", @@ -2213,6 +2212,28 @@ defmodule PolymorphicEmbedTest do end end + test "nil for a list of embeds is invalid" do + for generator <- @generators do + reminder_module = get_module(Reminder, generator) + + attrs = %{ + date: ~U[2020-05-28 02:57:19Z], + text: "This is a reminder with nil contexts", + contexts: nil + } + + changeset = + struct(reminder_module) + |> reminder_module.changeset(attrs) + + refute changeset.valid? + refute Map.has_key?(changeset.changes, :contexts) + + assert changeset.errors[:contexts] == + {"is invalid", [validation: :embed, type: {:array, :map}]} + end + end + test "list of embeds defaults to []" do for generator <- @generators do reminder_module = get_module(Reminder, generator) From 80653d003938bd83aa628b431f544156e684902d Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 22 Oct 2026 13:47:55 +0200 Subject: [PATCH 03/11] Handle the required option as Ecto does With required: true, the cast now: - adds the field to changeset.required - adds "can't be blank" when the value is nil or an empty list after the cast, also for an existing empty list - puts a change to nil when the params set an existing embed to nil - adds no "can't be blank" when the field already has an error --- lib/polymorphic_embed.ex | 52 +++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/lib/polymorphic_embed.ex b/lib/polymorphic_embed.ex index ce7c448..7a19be4 100644 --- a/lib/polymorphic_embed.ex +++ b/lib/polymorphic_embed.ex @@ -125,7 +125,7 @@ defmodule PolymorphicEmbed do raise_if_invalid_options(field, field_opts) - %{array?: array?, types_metadata: types_metadata} = field_opts + %{types_metadata: types_metadata} = field_opts required = Keyword.get(cast_opts, :required, false) with = Keyword.get(cast_opts, :with, nil) @@ -136,6 +136,29 @@ defmodule PolymorphicEmbed do sort = param_value_for_cast_opt(:sort_param, cast_opts, changeset.params) drop = param_value_for_cast_opt(:drop_param, cast_opts, changeset.params) + changeset = + if required, do: update_in(changeset.required, &[field | &1]), else: changeset + + changeset + |> do_cast_polymorphic_embed(field, cast_opts, field_opts, changeset_fun, sort, drop) + |> maybe_add_required_error(field, required) + end + + def cast_polymorphic_embed(_, _, _) do + raise "cast_polymorphic_embed/3 only accepts a changeset as first argument" + end + + defp do_cast_polymorphic_embed( + changeset, + field, + cast_opts, + field_opts, + changeset_fun, + sort, + drop + ) do + %{array?: array?} = field_opts + case Map.fetch(changeset.params || %{}, to_string(field)) do # consider sort and drop params even if the assoc param was not given, as in Ecto :error when (array? and is_list(sort)) or is_list(drop) -> @@ -150,15 +173,7 @@ defmodule PolymorphicEmbed do field_opts ) - :error when required -> - if data_for_field = Map.fetch!(changeset.data, field) do - data_for_field = autogenerate_id(data_for_field, changeset.action) - Ecto.Changeset.put_change(changeset, field, data_for_field) - else - Ecto.Changeset.add_error(changeset, field, "can't be blank", validation: :required) - end - - :error when not required -> + :error -> if data_for_field = Map.fetch!(changeset.data, field) do data_for_field = autogenerate_id(data_for_field, changeset.action) Ecto.Changeset.put_change(changeset, field, data_for_field) @@ -177,10 +192,7 @@ defmodule PolymorphicEmbed do when not array? and not is_map(params_for_field) and not is_nil(params_for_field) -> add_invalid_error(changeset, field, array?) - {:ok, nil} when required -> - Ecto.Changeset.add_error(changeset, field, "can't be blank", validation: :required) - - {:ok, nil} when not required -> + {:ok, nil} -> Ecto.Changeset.put_change(changeset, field, nil) {:ok, map} when map == %{} and not array? -> @@ -209,8 +221,16 @@ defmodule PolymorphicEmbed do end end - def cast_polymorphic_embed(_, _, _) do - raise "cast_polymorphic_embed/3 only accepts a changeset as first argument" + # As in Ecto, a required embed must not be nil or an empty list after the cast. + defp maybe_add_required_error(changeset, _field, false), do: changeset + + defp maybe_add_required_error(changeset, field, true) do + if Keyword.has_key?(changeset.errors, field) or + Ecto.Changeset.get_field(changeset, field) not in [nil, []] do + changeset + else + Ecto.Changeset.add_error(changeset, field, "can't be blank", validation: :required) + end end defp sort_create(%{sort_param: _} = cast_opts, field_opts) do From e93d5d3e7d44dfc990eb3ebd806d020945033e85 Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 22 Oct 2026 13:43:14 +0200 Subject: [PATCH 04/11] Match existing embeds_many rows by primary key The cast looked for the row id with params[:id]. Params from a form have string keys, so no row matched. Each existing row became a new :insert, and the row lost all data that the params did not set. Now a row matches when it has the same type and each primary key, cast to its type, is equal to the param value. This is the rule that Ecto uses, so "1" matches the integer id 1. --- lib/polymorphic_embed.ex | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/polymorphic_embed.ex b/lib/polymorphic_embed.ex index 7a19be4..906de2e 100644 --- a/lib/polymorphic_embed.ex +++ b/lib/polymorphic_embed.ex @@ -437,6 +437,22 @@ defmodule PolymorphicEmbed do end end + # As in Ecto, the params match a row when each primary key, cast to its + # type, is equal to the primary key of the row. + defp same_pk?(%module{} = datum, params) do + pks = module.__schema__(:primary_key) + + pks != [] and + Enum.all?(pks, fn pk -> + value = Map.get(datum, pk) + + case Ecto.Type.cast(module.__schema__(:type, pk), Attrs.get(params, pk)) do + {:ok, param_value} -> not is_nil(value) and param_value == value + _ -> false + end + end) + end + # The same error as Ecto gives for params that are not a map (or a list of maps). defp add_invalid_error(changeset, field, array?) do type = if array?, do: {:array, :map}, else: :map @@ -466,13 +482,7 @@ defmodule PolymorphicEmbed do module -> data_for_field = - Enum.find(list_data_for_field, fn - %{id: id} = datum when not is_nil(id) -> - id == params[:id] and datum.__struct__ == module - - _ -> - nil - end) + Enum.find(list_data_for_field, &(&1.__struct__ == module and same_pk?(&1, params))) embed_changeset = if data_for_field do From c0d57c2d5b5a722db4fed1d71db35f6ed3be08e6 Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 22 Oct 2026 13:44:07 +0200 Subject: [PATCH 05/11] Raise ArgumentError for an invalid on_replace option Ecto raises an ArgumentError when on_replace has a value that the embed does not accept. The polymorphic embed raised a RuntimeError. The messages do not change. --- lib/polymorphic_embed.ex | 11 ++++++----- lib/polymorphic_embed/options_validator.ex | 6 ++++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/polymorphic_embed.ex b/lib/polymorphic_embed.ex index 906de2e..bc104c2 100644 --- a/lib/polymorphic_embed.ex +++ b/lib/polymorphic_embed.ex @@ -79,9 +79,8 @@ defmodule PolymorphicEmbed do OptionsValidator.validate!(opts) if Keyword.get(opts, :on_replace) not in [:update, :delete] do - raise( - "`:on_replace` option for polymorphic embed must be set to `:update` (single embed) or `:delete` (list of embeds)" - ) + raise ArgumentError, + "`:on_replace` option for polymorphic embed must be set to `:update` (single embed) or `:delete` (list of embeds)" end types_metadata = @@ -709,11 +708,13 @@ defmodule PolymorphicEmbed do end if array? and on_replace != :delete do - raise "`:on_replace` option for field #{inspect(field)} must be set to `:delete`" + raise ArgumentError, + "`:on_replace` option for field #{inspect(field)} must be set to `:delete`" end if not array? and on_replace != :update do - raise "`:on_replace` option for field #{inspect(field)} must be set to `:update`" + raise ArgumentError, + "`:on_replace` option for field #{inspect(field)} must be set to `:update`" end end diff --git a/lib/polymorphic_embed/options_validator.ex b/lib/polymorphic_embed/options_validator.ex index 8ef81db..376e52f 100644 --- a/lib/polymorphic_embed/options_validator.ex +++ b/lib/polymorphic_embed/options_validator.ex @@ -24,11 +24,13 @@ defmodule PolymorphicEmbed.OptionsValidator do end if is_nil(options[:default]) and options[:on_replace] != :update do - raise "`:on_replace` must be set to `:update` for a single polymorphic embed." + raise ArgumentError, + "`:on_replace` must be set to `:update` for a single polymorphic embed." end if is_list(options[:default]) and options[:on_replace] != :delete do - raise "`:on_replace` must be set to `:delete` for a list of polymorphic embeds." + raise ArgumentError, + "`:on_replace` must be set to `:delete` for a list of polymorphic embeds." end unless Keyword.fetch!(options, :on_type_not_found) in @valid_on_type_not_found_options do From 015c17317d11690f2e6e097752b0f676e4f4360d Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 22 Oct 2026 13:52:01 +0200 Subject: [PATCH 06/11] Keep an autogenerated integer id nil on cast The cast crashed with a CaseClauseError for an embedded schema with an autogenerated integer id, because Ecto 3.12 returns a 3-tuple from __schema__(:autogenerate_id). Now the cast keeps the id nil, as Ecto does. dump/3 raises the same ArgumentError as Ecto when the id is still nil on insert. --- lib/polymorphic_embed.ex | 13 +++++++++++-- test/polymorphic_embed/ecto_parity_test.exs | 7 +++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/polymorphic_embed.ex b/lib/polymorphic_embed.ex index bc104c2..1da88d9 100644 --- a/lib/polymorphic_embed.ex +++ b/lib/polymorphic_embed.ex @@ -592,6 +592,14 @@ defmodule PolymorphicEmbed do raise "polymorphic_embed is not able to add an autogenerated key without casting through cast_polymorphic_embed/3" end + {key, _source, :id} -> + unless Map.get(struct, key) do + raise ArgumentError, + "embedded schema `#{inspect(module)}` cannot autogenerate `:id` primary keys, " <> + "those are typically used for auto-incrementing constraints. " <> + "Maybe you meant to use `:binary_id` instead?" + end + _ -> nil end @@ -844,8 +852,9 @@ defmodule PolymorphicEmbed do schema end - {_key, :id} -> - raise("embedded schemas cannot autogenerate `:id` primary keys") + # As in Ecto, the cast keeps an integer id. Ecto raises on insert, see dump/3. + {_key, _source, :id} -> + schema nil -> schema diff --git a/test/polymorphic_embed/ecto_parity_test.exs b/test/polymorphic_embed/ecto_parity_test.exs index 409952e..d49007a 100644 --- a/test/polymorphic_embed/ecto_parity_test.exs +++ b/test/polymorphic_embed/ecto_parity_test.exs @@ -12,7 +12,7 @@ defmodule PolymorphicEmbed.EctoParityTest do alias Ecto.Changeset - alias __MODULE__.{Author, Post, Profile} + alias __MODULE__.{Author, AutoIdPost, Post, Profile} defmodule Post do use Ecto.Schema @@ -251,7 +251,10 @@ defmodule PolymorphicEmbed.EctoParityTest do changeset = cast(%Author{}, %{"auto_id_posts" => [post(%{"title" => "hello"})]}, :auto_id_posts) - assert [%Changeset{changes: %{title: "hello"}}] = changeset.changes.auto_id_posts + assert changeset.valid? + + assert [%AutoIdPost{id: nil, title: "hello"}] = + Changeset.apply_changes(changeset).auto_id_posts end ## cast embeds many From 64cd7a9474478b1db6059b44eafda608aa3c8d5f Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 24 Oct 2026 11:22:28 +0200 Subject: [PATCH 07/11] Keep child changesets in changes, as Ecto embeds do cast_polymorphic_embed/3 now gives the same changeset output as cast_embed/3: - Each child is an Ecto.Changeset in changes. Before, a valid child was a struct. - A list of embeds gets a :replace changeset for each row of the data that the params do not keep. Rows that the load options kept as nil or plain maps match no params and are dropped without a :replace changeset. - A valid child with action :update and without changes is not a change. When the params do not contain the field, the data does not go into changes. When the changes of the field hold changesets, the field gets an :embed type in changeset.types, so that apply_changes/1, get_field/2 and traverse_errors/2 handle them as for an Ecto embed. Else the field keeps its PolymorphicEmbed type: on insert, Ecto builds changesets from an embed type with one schema module only, and a struct that put_change/3 set before the cast must stay a struct. The type then keeps the changeset function as on_cast for the forms. The cast still generates the binary id of a new child, in the data of the child changeset, so that apply_changes/1 and a later insert of that struct have the id. The changes of the child stay as in Ecto. A prepare_changes/2 function turns the child changesets into structs before the repo writes the parent, and puts the schema type back. It also generates missing ids of structs that the repo puts into the changes on insert, and of data without a change, because Ecto runs the prepare functions of an ordinary embed before it puts its data into the changes. On a parent insert, a child update counts as an insert, as in Ecto. The ordinary Ecto embeds of a child get the same preparation as Ecto gives to the embeds of a schema: their prepare functions run, and their ids and timestamps are generated. PolymorphicEmbed.Embedded is a copy of the logic of Ecto.Embedded.prepare/4, because that function is not public API. As in Ecto, the prepare functions of a child must not make it invalid: then the insert or update raises an ArgumentError. On insert, the data of a child goes into its changes, as in Ecto, so that ordinary embeds and polymorphic fields in the data of a child also get their ids. A struct on insert gets the same preparation as a new child changeset. Ecto reads the prepare functions of a changeset before it runs them. A cast inside a prepare function of the parent therefore prepares the embeds at once, so that the returned struct also holds structs. PolymorphicEmbed.HTML.Helpers.to_form/4 now reads the child changesets from changes, so that the forms keep the errors of invalid children. Changes to existing tests: - Four tests read the id as a field of the struct in changes. They now assert the output of Ecto embeds. - "validates lists of polymorphic embeds" now expects the :replace changeset for the data row without id, before the new row. CHANGELOG.md lists the breaking changes of this branch. --- CHANGELOG.md | 35 +++ lib/polymorphic_embed.ex | 294 +++++++++++++++++++++----- lib/polymorphic_embed/embedded.ex | 160 ++++++++++++++ lib/polymorphic_embed/html/helpers.ex | 38 +++- test/polymorphic_embed_test.exs | 65 ++---- 5 files changed, 483 insertions(+), 109 deletions(-) create mode 100644 lib/polymorphic_embed/embedded.ex diff --git a/CHANGELOG.md b/CHANGELOG.md index bf62aa2..a448e3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,40 @@ # Changelog +## Unreleased + + `cast_polymorphic_embed/3` now gives the same output as `Ecto.Changeset.cast_embed/3` + for the same input. + + **Breaking changes**: + + * `changeset.changes` keeps each child as an `Ecto.Changeset`, as for an Ecto embed. + Before, a valid child was a struct. `get_change/2` gives changesets now. + `apply_changes/1`, `apply_action/2` and `get_field/2` still give structs. + * `Ecto.Changeset.merge/2` takes the types of the second changeset. Put the changeset of + `cast_polymorphic_embed/3` second, for example `merge(change(parent), changeset)`. + Else `get_field/2` and `apply_changes/1` give the child changesets. + * The repo runs the `prepare_changes/2` functions of the children, as for Ecto embeds. + Before, the cast lost them. If such a function makes a child invalid, the insert or + update raises `ArgumentError`, as in Ecto. + * Ordinary Ecto embeds inside a polymorphic embed get ids and timestamps when the + repo inserts or updates the parent, as Ecto embeds do. Before, their ids stayed + `nil`. + * A list of embeds gets a changeset with action `:replace` for each row of the data + that the params do not keep. Before, the row was removed from the list. + * A list of embeds matches the rows of the data by primary key. Before, no row + matched params with string keys, so each row got a new id and lost the data that + the params did not set. + * When the params do not contain the field, the cast does not put the data into + `changes`. + * A child that is valid, has action `:update` and has no changes is not a change. + * When the cast puts changesets into the changes of the field, `changeset.types` has an + `:embed` type for it. For example, `validate_required/3` on the field then shows the + same warning as for an Ecto embed. + * `nil` for a list of embeds gives the error "is invalid". Before, the cast ignored it. + * With `required: true`, an empty list in the data gives "can't be blank", and + `changeset.required` contains the field. + * An invalid `:on_replace` option raises `ArgumentError`, not `RuntimeError`. + ## 5.0.2 * Remove warning when aliases cannot be expanded (#123). diff --git a/lib/polymorphic_embed.ex b/lib/polymorphic_embed.ex index 1da88d9..3944432 100644 --- a/lib/polymorphic_embed.ex +++ b/lib/polymorphic_embed.ex @@ -140,7 +140,9 @@ defmodule PolymorphicEmbed do changeset |> do_cast_polymorphic_embed(field, cast_opts, field_opts, changeset_fun, sort, drop) + |> put_embed_type(field, field_opts, &changeset_fun.(&1, &2)) |> maybe_add_required_error(field, required) + |> register_prepare(field) end def cast_polymorphic_embed(_, _, _) do @@ -173,12 +175,7 @@ defmodule PolymorphicEmbed do ) :error -> - if data_for_field = Map.fetch!(changeset.data, field) do - data_for_field = autogenerate_id(data_for_field, changeset.action) - Ecto.Changeset.put_change(changeset, field, data_for_field) - else - changeset - end + changeset {:ok, params_for_field} when array? and not is_list(params_for_field) and not is_map(params_for_field) -> @@ -192,7 +189,7 @@ defmodule PolymorphicEmbed do add_invalid_error(changeset, field, array?) {:ok, nil} -> - Ecto.Changeset.put_change(changeset, field, nil) + put_nil(changeset, field) {:ok, map} when map == %{} and not array? -> changeset @@ -344,22 +341,16 @@ defmodule PolymorphicEmbed do Ecto.Changeset.add_error(changeset, field, "is invalid") :type_not_found when on_type_not_found == :nilify -> - Ecto.Changeset.put_change(changeset, field, nil) + put_nil(changeset, field) {action, struct} -> - embed_changeset = changeset_fun.(struct, params) - embed_changeset = %{embed_changeset | action: action} - - case embed_changeset do - %{valid?: true} = embed_changeset -> - embed_schema = Ecto.Changeset.apply_changes(embed_changeset) - embed_schema = autogenerate_id(embed_schema, embed_changeset.action) - Ecto.Changeset.put_change(changeset, field, embed_schema) - - %{valid?: false} = embed_changeset -> - changeset - |> Ecto.Changeset.put_change(field, embed_changeset) - |> Map.put(:valid?, false) + struct = if action == :insert, do: autogenerate_id(struct, :insert), else: struct + embed_changeset = %{changeset_fun.(struct, params) | action: action} + + if skip?(embed_changeset) do + delete_embed_change(changeset, field) + else + put_embed_change(changeset, field, embed_changeset) end end end @@ -467,61 +458,252 @@ defmodule PolymorphicEmbed do list_data_for_field = Map.fetch!(changeset.data, field) || [] - embeds = - Enum.map(list_params, fn params -> + # With nilify_unlisted_types_on_load or retain_unlisted_types_on_load, the + # loaded list can hold nil or plain maps. They match no params, and the + # cast drops them without a :replace changeset. + structs = Enum.filter(list_data_for_field, &is_struct/1) + + # Each matched row is removed from the remaining data, as in Ecto. + {embeds, remaining} = + Enum.map_reduce(list_params, structs, fn params, remaining -> case get_polymorphic_module_from_map(params, type_field_name, types_metadata) do nil when on_type_not_found == :raise -> raise_cannot_infer_type_from_data(params) nil when on_type_not_found == :changeset_error -> - :error + {:error, remaining} nil when on_type_not_found == :ignore -> - :ignore + {:ignore, remaining} module -> - data_for_field = - Enum.find(list_data_for_field, &(&1.__struct__ == module and same_pk?(&1, params))) - - embed_changeset = - if data_for_field do - %{changeset_fun.(data_for_field, params) | action: :update} - else - %{changeset_fun.(struct(module), params) | action: :insert} - end - - maybe_apply_changes(embed_changeset) + index = + Enum.find_index(remaining, &(&1.__struct__ == module and same_pk?(&1, params))) + + if index do + {data_for_field, remaining} = List.pop_at(remaining, index) + {%{changeset_fun.(data_for_field, params) | action: :update}, remaining} + else + new_struct = module |> struct() |> autogenerate_id(:insert) + {%{changeset_fun.(new_struct, params) | action: :insert}, remaining} + end end end) if Enum.any?(embeds, &(&1 == :error)) do Ecto.Changeset.add_error(changeset, field, "is invalid") else - embeds = Enum.filter(embeds, &(&1 != :ignore)) + embeds = Enum.reject(embeds, &(&1 == :ignore)) - any_invalid? = - Enum.any?(embeds, fn - %{valid?: false} -> true - _ -> false - end) - - changeset = Ecto.Changeset.put_change(changeset, field, embeds) + # As in Ecto, rows of the data that the params do not keep are replaced. + replaced = + remaining + |> Enum.map(&%{Ecto.Changeset.change(&1) | action: :replace}) + |> Enum.reverse() - if any_invalid? do - Map.put(changeset, :valid?, false) + if replaced == [] and Enum.all?(embeds, &skip?/1) and + Enum.map(embeds, & &1.data) == list_data_for_field do + delete_embed_change(changeset, field) else - changeset + put_embed_change(changeset, field, replaced ++ embeds) end end end - defp maybe_apply_changes(%{valid?: true} = embed_changeset) do - embed_changeset - |> Ecto.Changeset.apply_changes() - |> autogenerate_id(embed_changeset.action) + # As in Ecto, an update without changes is not a change. + defp skip?(%Changeset{valid?: true, action: :update, changes: changes}), do: changes == %{} + defp skip?(_changeset), do: false + + defp put_embed_change(changeset, field, value) do + valid? = + changeset.valid? and + value |> List.wrap() |> Enum.all?(fn embed_changeset -> embed_changeset.valid? end) + + %{changeset | changes: Map.put(changeset.changes, field, value), valid?: valid?} + end + + defp delete_embed_change(changeset, field) do + %{changeset | changes: Map.delete(changeset.changes, field)} + end + + defp put_nil(changeset, field) do + if Map.fetch!(changeset.data, field) == nil do + delete_embed_change(changeset, field) + else + %{changeset | changes: Map.put(changeset.changes, field, nil)} + end + end + + # The field keeps the PolymorphicEmbed type in the schema. When the field has + # a change, it gets an embed type in the changeset, so that apply_changes/1, + # get_field/2 and traverse_errors/2 handle the child changesets as for an Ecto + # embed. Without a change, the field keeps its type, because on insert Ecto + # builds changesets for the data of an embed type with one module only. The + # type then keeps the on_cast function for the forms. + defp put_embed_type(changeset, field, field_opts, on_cast) do + if changesets_in_changes?(changeset, field) do + put_embed_relation(changeset, field, field_opts, on_cast) + else + type = changeset.data.__struct__.__schema__(:type, field) + %{changeset | types: Map.put(changeset.types, field, put_on_cast(type, on_cast))} + end + end + + # A struct that put_change/3 set before the cast stays a struct, so the field + # keeps its type, as before. Ecto applies the embed type only to changesets. + defp changesets_in_changes?(changeset, field) do + case Map.fetch(changeset.changes, field) do + {:ok, value} -> value |> List.wrap() |> Enum.all?(&match?(%Changeset{}, &1)) + :error -> false + end + end + + defp put_on_cast({:array, type}, on_cast), do: {:array, put_on_cast(type, on_cast)} + + defp put_on_cast({:parameterized, {PolymorphicEmbed, opts}}, on_cast), + do: {:parameterized, {PolymorphicEmbed, Map.put(opts, :on_cast, on_cast)}} + + defp put_embed_relation(changeset, field, field_opts, on_cast) do + %{array?: array?, on_replace: on_replace, types_metadata: [first_type | _]} = field_opts + + related = + case List.wrap(Map.get(changeset.changes, field)) do + [%Changeset{data: %module{}} | _] -> module + _ -> first_type.module + end + + embed = %Ecto.Embedded{ + cardinality: if(array?, do: :many, else: :one), + field: field, + owner: changeset.data.__struct__, + related: related, + on_cast: on_cast, + on_replace: on_replace + } + + %{changeset | types: Map.put(changeset.types, field, {:embed, embed})} + end + + # Ecto reads the prepare functions of a changeset before it runs them. A + # cast or a put inside a prepare function of the parent therefore prepares + # the embeds at once. Ecto sets the repo on the changeset before it runs + # the prepare functions. A changeset that the repo returns as an error is + # invalid, so it keeps its child changesets. + defp register_prepare(%Changeset{repo: repo, valid?: true} = changeset, field) + when not is_nil(repo), + do: prepare_embeds(changeset, field) + + defp register_prepare(changeset, field), + do: Ecto.Changeset.prepare_changes(changeset, &prepare_embeds(&1, field)) + + # Runs before the repo inserts or updates the parent. As Ecto does for + # embeds, it turns the child changesets into structs and generates the ids. + defp prepare_embeds(changeset, field) do + case Map.fetch(changeset.changes, field) do + {:ok, value} -> + value = apply_embed(value, changeset.repo, changeset.action) + # The value is a struct now, so the field gets its schema type back. + type = changeset.data.__struct__.__schema__(:type, field) + + %{ + changeset + | changes: Map.put(changeset.changes, field, value), + types: Map.put(changeset.types, field, type) + } + + # Ecto runs the prepare functions of an ordinary embed before it puts its + # data into the changes. So data without a change can still need ids. + :error -> + data = Map.fetch!(changeset.data, field) + + case autogenerate_id(data, :insert) do + ^data -> changeset + data -> put_in(changeset.changes[field], data) + end + end end - defp maybe_apply_changes(%Changeset{valid?: false} = changeset), do: changeset + defp apply_embed(list, repo, repo_action) when is_list(list) do + list |> Enum.map(&apply_embed(&1, repo, repo_action)) |> Enum.reject(&is_nil/1) + end + + defp apply_embed(%Changeset{action: action}, _repo, _repo_action) + when action in [:replace, :delete], + do: nil + + defp apply_embed(%Changeset{data: %module{}} = embed_changeset, repo, repo_action) do + # As in Ecto, a child update of a parent insert is an insert, so that the + # ordinary embeds of the child also get their ids. + action = if repo_action == :insert, do: :insert, else: embed_changeset.action + + embed_changeset = + embed_changeset.prepare + |> Enum.reverse() + |> Enum.reduce(%{embed_changeset | repo: repo}, fn prepare, acc -> prepare.(acc) end) + + # As in Ecto, a prepare function cannot make a child invalid. + unless embed_changeset.valid? do + raise ArgumentError, + "changeset for embedded #{inspect(module)} is invalid, " <> + "but the parent changeset was not marked as invalid" + end + + # As in Ecto, on insert the data of the child goes into its changes, so + # that embeds in the data are prepared too. + embed_changeset = + PolymorphicEmbed.Embedded.maybe_surface_changes( + embed_changeset, + embed_changeset.data, + module, + action + ) + + # The ordinary Ecto embeds of the child get the same preparation as the + # embeds of the parent: their prepare functions run and their ids are + # generated. This also prepares polymorphic fields inside them. + embeds = + PolymorphicEmbed.Embedded.prepare( + embed_changeset, + module.__schema__(:embeds), + repo.__adapter__(), + action + ) + + changes = + embed_changeset.changes + |> Map.merge(embeds) + |> prepare_polymorphic_changes(module, repo, action) + + embed_changeset.data + |> struct(changes) + |> autogenerate_id(action) + end + + # On insert, a struct gets the same preparation as a new child changeset, + # so that the embeds in its data get ids too. + defp apply_embed(%_{} = struct, repo, :insert), + do: apply_embed(%{Ecto.Changeset.change(struct) | action: :insert}, repo, :insert) + + defp apply_embed(%_{} = struct, _repo, _repo_action), do: autogenerate_id(struct, :insert) + + defp apply_embed(value, _repo, _repo_action), do: value + + # The polymorphic fields of a child can hold structs or changesets that no + # cast prepared, for example from put_polymorphic_embed/3. + defp prepare_polymorphic_changes(changes, module, repo, action) do + Map.new(changes, fn {field, value} -> + case module.__schema__(:type, field) do + {:parameterized, {PolymorphicEmbed, _}} -> + {field, apply_embed(value, repo, action)} + + {:array, {:parameterized, {PolymorphicEmbed, _}}} -> + {field, apply_embed(value, repo, action)} + + _ -> + {field, value} + end + end) + end @impl true def cast(_data, _params), @@ -579,7 +761,10 @@ defmodule PolymorphicEmbed do end def dump(%Ecto.Changeset{valid?: true} = changeset, dumper, params) do - dump(Ecto.Changeset.apply_changes(changeset), dumper, params) + changeset + |> Ecto.Changeset.apply_changes() + |> autogenerate_id(changeset.action) + |> dump(dumper, params) end def dump(%module{} = struct, dumper, %{ @@ -834,6 +1019,9 @@ defmodule PolymorphicEmbed do [autogenerate_id(schema, action) | autogenerate_id(rest, action)] end + # nil, and plain maps that the load options keep, have no id to generate. + defp autogenerate_id(value, _action) when not is_struct(value), do: value + defp autogenerate_id(schema, :update) do # in case there is no primary key, Ecto.primary_key/1 returns an empty keyword list [] for {_, nil} <- Ecto.primary_key(schema) do diff --git a/lib/polymorphic_embed/embedded.ex b/lib/polymorphic_embed/embedded.ex new file mode 100644 index 0000000..519baee --- /dev/null +++ b/lib/polymorphic_embed/embedded.ex @@ -0,0 +1,160 @@ +defmodule PolymorphicEmbed.Embedded do + @moduledoc false + + # A copy of the logic of Ecto.Embedded.prepare/4 from Ecto 3.12. That + # function is not public API, so polymorphic_embed keeps its own copy. + # + # The repo calls Ecto.Embedded.prepare/4 only for the embeds of the schema + # that it writes. A polymorphic child is not such an embed, so + # polymorphic_embed calls this copy for the ordinary embeds of each child. + # It replaces their changesets with structs, runs their prepare functions + # and generates their ids and timestamps, as Ecto does. + + alias Ecto.Changeset + + def prepare(changeset, embeds, adapter, repo_action) do + %{changes: changes, types: types, repo: repo} = changeset + + changes + |> Map.take(embeds) + |> Map.new(fn {name, changeset_or_changesets} -> + {:embed, embed} = Map.fetch!(types, name) + {name, prepare_each(embed, changeset_or_changesets, adapter, repo, repo_action)} + end) + end + + defp prepare_each(%{cardinality: :one}, nil, _adapter, _repo, _repo_action), do: nil + + defp prepare_each(%{cardinality: :one} = embed, changeset, adapter, repo, repo_action) do + action = normalize_action(changeset.action, repo_action, embed) + changeset = run_prepare(changeset, repo) + to_struct(changeset, action, embed, adapter) + end + + defp prepare_each(%{cardinality: :many} = embed, changesets, adapter, repo, repo_action) do + for changeset <- changesets, + action = normalize_action(changeset.action, repo_action, embed), + changeset = run_prepare(changeset, repo), + prepared = to_struct(changeset, action, embed, adapter), + do: prepared + end + + defp to_struct(%Changeset{valid?: false}, _action, %{related: schema}, _adapter) do + raise ArgumentError, + "changeset for embedded #{inspect(schema)} is invalid, " <> + "but the parent changeset was not marked as invalid" + end + + defp to_struct(%Changeset{data: %{__struct__: actual}}, _action, %{related: expected}, _adapter) + when actual != expected do + raise ArgumentError, + "expected changeset for embedded schema `#{inspect(expected)}`, " <> + "got: #{inspect(actual)}" + end + + defp to_struct(%Changeset{changes: changes, data: schema}, :update, _embed, _adapter) + when changes == %{} do + schema + end + + defp to_struct(%Changeset{}, :delete, _embed, _adapter), do: nil + + defp to_struct(%Changeset{data: data} = changeset, action, %{related: schema}, adapter) do + %{data: struct, changes: changes} = + changeset = + maybe_surface_changes(changeset, data, schema, action) + + embeds = prepare(changeset, schema.__schema__(:embeds), adapter, action) + + changes + |> Map.merge(embeds) + |> autogenerate_id(struct, action, schema, adapter) + |> autogenerate(action, schema) + |> then(&struct(struct, &1)) + end + + # On insert, Ecto puts the fields of the data that are not nil into the + # changes, and it turns embeds of the data into changesets with the action + # :insert. This copy does the same without the internal Ecto relation code. + def maybe_surface_changes(changeset, data, schema, :insert) do + embeds = schema.__schema__(:embeds) + + changes = + Enum.reduce(schema.__schema__(:fields), changeset.changes, fn field, changes -> + case {changes, Map.get(data, field)} do + {%{^field => _}, _} -> changes + {_, nil} -> changes + {_, value} -> Map.put(changes, field, surface_value(field in embeds, value)) + end + end) + + %{changeset | changes: changes} + end + + def maybe_surface_changes(changeset, _data, _schema, _action), do: changeset + + defp surface_value(false, value), do: value + defp surface_value(true, list) when is_list(list), do: Enum.map(list, &surface_value(true, &1)) + defp surface_value(true, struct), do: %{Changeset.change(struct) | action: :insert} + + defp run_prepare(changeset, repo) do + changeset = %{changeset | repo: repo} + + Enum.reduce(Enum.reverse(changeset.prepare), changeset, fn fun, acc -> + case fun.(acc) do + %Changeset{} = acc -> + acc + + other -> + raise "expected function #{inspect(fun)} given to Ecto.Changeset.prepare_changes/2 " <> + "to return an Ecto.Changeset, got: `#{inspect(other)}`" + end + end) + end + + defp autogenerate_id(changes, _struct, :insert, schema, adapter) do + case schema.__schema__(:autogenerate_id) do + {key, _source, :binary_id} -> + Map.put_new_lazy(changes, key, fn -> adapter.autogenerate(:embed_id) end) + + {_key, _source, :id} -> + raise ArgumentError, + "embedded schema `#{inspect(schema)}` cannot autogenerate `:id` primary keys, " <> + "those are typically used for auto-incrementing constraints. " <> + "Maybe you meant to use `:binary_id` instead?" + + nil -> + changes + end + end + + defp autogenerate_id(changes, struct, :update, _schema, _adapter) do + for {_, nil} <- Ecto.primary_key(struct) do + raise Ecto.NoPrimaryKeyValueError, struct: struct + end + + changes + end + + defp autogenerate(changes, action, schema) do + autogen_fields = action |> action_to_auto() |> schema.__schema__() + + Enum.reduce(autogen_fields, changes, fn {fields, {mod, fun, args}}, acc -> + case Enum.reject(fields, &Map.has_key?(changes, &1)) do + [] -> + acc + + fields -> + generated = apply(mod, fun, args) + Enum.reduce(fields, acc, &Map.put(&2, &1, generated)) + end + end) + end + + defp action_to_auto(:insert), do: :autogenerate + defp action_to_auto(:update), do: :autoupdate + + defp normalize_action(:replace, _, %{on_replace: :delete}), do: :delete + defp normalize_action(:update, :insert, _), do: :insert + defp normalize_action(action, _, _), do: action +end diff --git a/lib/polymorphic_embed/html/helpers.ex b/lib/polymorphic_embed/html/helpers.ex index bcd4b76..c8b9e97 100644 --- a/lib/polymorphic_embed/html/helpers.ex +++ b/lib/polymorphic_embed/html/helpers.ex @@ -57,15 +57,19 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d struct = Ecto.Changeset.apply_changes(source_changeset) + # The cast keeps the child changesets in the changes. Rows that the cast + # replaces or deletes get no form. list_data = - case Map.get(struct, field) do - nil -> - type = Keyword.get(options, :polymorphic_type, get_polymorphic_type(form, field)) - module = PolymorphicEmbed.get_polymorphic_module(struct.__struct__, field, type) - if module, do: [struct(module)], else: [] - - data -> - List.wrap(data) + case Map.fetch(source_changeset.changes, field) do + {:ok, value} when value not in [nil, []] -> + value + |> List.wrap() + |> Enum.reject( + &match?(%Ecto.Changeset{action: action} when action in [:replace, :delete], &1) + ) + + _ -> + list_data_from_struct(struct, form, field, options) end list_data @@ -76,10 +80,11 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d %Ecto.Changeset{} = changeset = data - |> Ecto.Changeset.change() + |> to_changeset() |> apply_action(parent_action) errors = get_errors(changeset) + data = changeset.data changeset = %Ecto.Changeset{ changeset @@ -116,6 +121,21 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d end) end + defp list_data_from_struct(struct, form, field, options) do + case Map.get(struct, field) do + nil -> + type = Keyword.get(options, :polymorphic_type, get_polymorphic_type(form, field)) + module = PolymorphicEmbed.get_polymorphic_module(struct.__struct__, field, type) + if module, do: [struct(module)], else: [] + + data -> + List.wrap(data) + end + end + + defp to_changeset(%Ecto.Changeset{} = changeset), do: changeset + defp to_changeset(data), do: Ecto.Changeset.change(data) + # If the parent changeset had no action, we need to remove the action # from children changeset so we ignore all errors accordingly. defp apply_action(changeset, nil), do: %{changeset | action: nil} diff --git a/test/polymorphic_embed_test.exs b/test/polymorphic_embed_test.exs index 020c597..116c706 100644 --- a/test/polymorphic_embed_test.exs +++ b/test/polymorphic_embed_test.exs @@ -1934,19 +1934,11 @@ defmodule PolymorphicEmbedTest do changeset = reminder_module.changeset(struct, %{}) - if polymorphic?(generator) do - assert changeset.changes.channel.id - else - assert map_size(changeset.changes) == 0 - end + assert map_size(changeset.changes) == 0 struct = Repo.insert!(changeset) - if polymorphic?(generator) do - assert changeset.changes.channel.id == struct.channel.id - else - assert struct.channel.id - end + assert struct.channel.id end end @@ -1976,19 +1968,11 @@ defmodule PolymorphicEmbedTest do } ) - if polymorphic?(generator) do - assert changeset.changes.channel.id - else - refute Map.has_key?(changeset.changes.channel, :id) - end + refute Map.has_key?(changeset.changes.channel, :id) struct = Repo.insert!(changeset) - if polymorphic?(generator) do - assert changeset.changes.channel.id == struct.channel.id - else - assert struct.channel.id - end + assert struct.channel.id end end @@ -2009,22 +1993,12 @@ defmodule PolymorphicEmbedTest do changeset = reminder_module.changeset(struct, %{}) - if polymorphic?(generator) do - assert Enum.at(changeset.changes.contexts, 0).id - assert Enum.at(changeset.changes.contexts, 1).id - else - assert map_size(changeset.changes) == 0 - end + assert map_size(changeset.changes) == 0 struct = Repo.insert!(changeset) - if polymorphic?(generator) do - assert Enum.at(changeset.changes.contexts, 0).id == Enum.at(struct.contexts, 0).id - assert Enum.at(changeset.changes.contexts, 1).id == Enum.at(struct.contexts, 1).id - else - assert Enum.at(struct.contexts, 0).id - assert Enum.at(struct.contexts, 1).id - end + assert Enum.at(struct.contexts, 0).id + assert Enum.at(struct.contexts, 1).id end end @@ -2049,22 +2023,12 @@ defmodule PolymorphicEmbedTest do } ) - if polymorphic?(generator) do - assert Enum.at(changeset.changes.contexts, 0).id - assert Enum.at(changeset.changes.contexts, 1).id - else - refute Map.has_key?(Enum.at(changeset.changes.contexts, 0), :id) - end + refute Map.has_key?(Enum.at(changeset.changes.contexts, 0), :id) struct = Repo.insert!(changeset) - if polymorphic?(generator) do - assert Enum.at(changeset.changes.contexts, 0).id == Enum.at(struct.contexts, 0).id - assert Enum.at(changeset.changes.contexts, 1).id == Enum.at(struct.contexts, 1).id - else - assert Enum.at(struct.contexts, 0).id - assert Enum.at(struct.contexts, 1).id - end + assert Enum.at(struct.contexts, 0).id + assert Enum.at(struct.contexts, 1).id end end @@ -2198,12 +2162,19 @@ defmodule PolymorphicEmbedTest do |> reminder_module.changeset(attrs) |> Repo.insert() + # The device in the data has no id, so the params cannot match it. As in + # Ecto, the cast replaces it and inserts a new device. assert {:error, %Ecto.Changeset{ valid?: false, action: :insert, errors: errors, - changes: %{contexts: [%{errors: device_errors, action: :insert} | _]} + changes: %{ + contexts: [ + %{action: :replace}, + %{errors: device_errors, action: :insert} | _ + ] + } }} = insert_result assert [] = errors From ac3ba70169bc21e39ab8933d489aaa8a80775eb4 Mon Sep 17 00:00:00 2001 From: Nick Date: Fri, 23 Oct 2026 14:31:49 +0200 Subject: [PATCH 08/11] Build polymorphic embed forms as phoenix_ecto does PolymorphicEmbed.HTML.Helpers.to_form/4 now follows the rules that phoenix_ecto uses for embeds_one and embeds_many: - A child form uses the child changeset from changes, with its own params and its own action. Before, row 0 of a list got the params of all rows, and each changeset got the parent action. Structs in the changes, for example after the repo prepared them or after put_change/3, become changesets in the same way as the data. - The form errors follow the parent action, and valid? does not change. - form.hidden contains the primary key of the data, as in phoenix_ecto, and the type field. - Without a change, a form is built with the changeset function that the cast stores as on_cast in the field type. - A new Ecto embed has no id before the insert, so the form of a new child hides the id that the cast generated: in form.data, in the data of form.source and in form.hidden. Only a cast child has a params map, so a child that the caller gives as a struct keeps its id. Two existing tests asserted the old output. The list test now expects the value of the first row, as for the regular embed. The nil embed test now expects the errors of the new SMS changeset, as for an Ecto embed. All form parity tests pass. --- CHANGELOG.md | 10 ++ lib/polymorphic_embed/html/helpers.ex | 131 +++++++++++++++----------- test/polymorphic_embed_test.exs | 13 ++- 3 files changed, 96 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a448e3e..ea1b8f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,16 @@ * With `required: true`, an empty list in the data gives "can't be blank", and `changeset.required` contains the field. * An invalid `:on_replace` option raises `ArgumentError`, not `RuntimeError`. + * The forms of `polymorphic_embed_inputs_for` give the same output as `inputs_for` + for an Ecto embed: + * Each form gets the params of its own row. Before, the first row of a list got + the params of all rows, and the other rows got no params. + * `form.source` is the child changeset with its action. Before, it was a new + changeset with the parent action. + * `form.hidden` contains the primary key of the data, so the form sends the id + back. + * A form for an embed that is `nil` uses the changeset function. With a parent + action, it shows the errors of that changeset. ## 5.0.2 diff --git a/lib/polymorphic_embed/html/helpers.ex b/lib/polymorphic_embed/html/helpers.ex index c8b9e97..8bd8784 100644 --- a/lib/polymorphic_embed/html/helpers.ex +++ b/lib/polymorphic_embed/html/helpers.ex @@ -49,60 +49,24 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d form.source.data.__struct__ end + # Builds the child forms as phoenix_ecto does for embeds_one and embeds_many. def to_form(%{action: parent_action} = source_changeset, form, field, options) do id = to_string(form.id <> "_#{field}") name = to_string(form.name <> "[#{field}]") - params = Map.get(source_changeset.params || %{}, to_string(field), %{}) |> List.wrap() + %schema{} = source_changeset.data + field_opts = PolymorphicEmbed.get_field_opts(schema, field) + type_field_name = Map.fetch!(field_opts, :type_field_name) + # correctly set id and name for embeds_many inputs + array? = Map.get(field_opts, :array?, false) - struct = Ecto.Changeset.apply_changes(source_changeset) - - # The cast keeps the child changesets in the changes. Rows that the cast - # replaces or deletes get no form. - list_data = - case Map.fetch(source_changeset.changes, field) do - {:ok, value} when value not in [nil, []] -> - value - |> List.wrap() - |> Enum.reject( - &match?(%Ecto.Changeset{action: action} when action in [:replace, :delete], &1) - ) - - _ -> - list_data_from_struct(struct, form, field, options) - end - - list_data + source_changeset + |> child_changesets(form, field, options) |> Enum.with_index() - |> Enum.map(fn {data, i} -> - params = Enum.at(params, i) || %{} - - %Ecto.Changeset{} = - changeset = - data - |> to_changeset() - |> apply_action(parent_action) - - errors = get_errors(changeset) - data = changeset.data - - changeset = %Ecto.Changeset{ - changeset - | action: parent_action, - params: params, - errors: errors, - valid?: errors == [] - } - - %schema{} = source_changeset.data - - field_opts = PolymorphicEmbed.get_field_opts(schema, field) - type_field_name = Map.fetch!(field_opts, :type_field_name) - # correctly set id and name for embeds_many inputs - array? = Map.get(field_opts, :array?, false) - + |> Enum.map(fn {changeset, i} -> + changeset = changeset |> hide_generated_id() |> apply_action(parent_action) + errors = get_errors(changeset, parent_action) index_string = Integer.to_string(i) - type = PolymorphicEmbed.get_polymorphic_type(schema, field, changeset.data) %Phoenix.HTML.Form{ @@ -112,15 +76,39 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d name: if(array?, do: name <> "[" <> index_string <> "]", else: name), index: if(array?, do: i), errors: errors, - data: data, + data: changeset.data, action: parent_action, - params: params, - hidden: [{type_field_name, to_string(type)}], + params: changeset.params || %{}, + hidden: primary_key_hidden(changeset.data) ++ [{type_field_name, to_string(type)}], options: options } end) end + # The cast keeps the child changesets in the changes. Rows that the cast + # replaces get no form. Without a change, the forms use the data. The + # changes can also hold structs, for example after the repo prepared them + # or after put_change/3, so they become changesets as the data does. + defp child_changesets(source_changeset, form, field, options) do + case Map.fetch(source_changeset.changes, field) do + {:ok, value} -> + on_cast = on_cast(source_changeset, field) + + value + |> List.wrap() + |> Enum.reject(&match?(%Ecto.Changeset{action: :replace}, &1)) + |> Enum.map(&to_changeset(&1, on_cast)) + + :error -> + struct = Ecto.Changeset.apply_changes(source_changeset) + on_cast = on_cast(source_changeset, field) + + struct + |> list_data_from_struct(form, field, options) + |> Enum.map(&to_changeset(&1, on_cast)) + end + end + defp list_data_from_struct(struct, form, field, options) do case Map.get(struct, field) do nil -> @@ -133,16 +121,49 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d end end - defp to_changeset(%Ecto.Changeset{} = changeset), do: changeset - defp to_changeset(data), do: Ecto.Changeset.change(data) + # The cast stores its changeset function in the embed type of a changed + # field, or in the options of the PolymorphicEmbed type. + defp on_cast(changeset, field) do + case Map.get(changeset.types, field) do + {:embed, %{on_cast: on_cast}} when is_function(on_cast, 2) -> on_cast + {:parameterized, {PolymorphicEmbed, %{on_cast: on_cast}}} -> on_cast + {:array, {:parameterized, {PolymorphicEmbed, %{on_cast: on_cast}}}} -> on_cast + _ -> nil + end + end + + defp to_changeset(%Ecto.Changeset{} = changeset, _on_cast), do: changeset + defp to_changeset(data, nil), do: Ecto.Changeset.change(data) + defp to_changeset(data, on_cast), do: on_cast.(data, %{}) + + # The cast generates the id of a new child into its data, so that + # apply_changes/1 and the repo give the same id. A new Ecto embed has no id + # before the insert, so the form of a new child does not show that id. + # Only a cast child has a params map. A struct that the caller gives to + # put_polymorphic_embed/3 keeps its id in the form. + defp hide_generated_id( + %Ecto.Changeset{action: :insert, params: %{}, data: %module{} = data} = changeset + ) do + case module.__schema__(:autogenerate_id) do + {key, _source, :binary_id} -> %{changeset | data: Map.put(data, key, nil)} + _ -> changeset + end + end + + defp hide_generated_id(changeset), do: changeset + + # As in phoenix_ecto, the hidden inputs hold the primary key of the data. + defp primary_key_hidden(%module{} = data) do + for key <- module.__schema__(:primary_key), value = Map.fetch!(data, key), do: {key, value} + end # If the parent changeset had no action, we need to remove the action # from children changeset so we ignore all errors accordingly. defp apply_action(changeset, nil), do: %{changeset | action: nil} defp apply_action(changeset, _action), do: changeset - defp get_errors(%{action: nil}), do: [] - defp get_errors(%{action: :ignore}), do: [] - defp get_errors(%{errors: errors}), do: errors + defp get_errors(_changeset, nil), do: [] + defp get_errors(_changeset, :ignore), do: [] + defp get_errors(%{errors: errors}, _action), do: errors end end diff --git a/test/polymorphic_embed_test.exs b/test/polymorphic_embed_test.exs index 116c706..75a339d 100644 --- a/test/polymorphic_embed_test.exs +++ b/test/polymorphic_embed_test.exs @@ -2859,7 +2859,7 @@ defmodule PolymorphicEmbedTest do if(polymorphic?(generator), do: ~s""" - + """, @@ -2886,7 +2886,7 @@ defmodule PolymorphicEmbedTest do if(polymorphic?(generator), do: ~s""" - + """, @@ -3252,7 +3252,14 @@ defmodule PolymorphicEmbedTest do assert contents == expected_contents assert f.impl == Phoenix.HTML.FormData.Ecto.Changeset - assert f.errors == [] + + # As phoenix_ecto does for an Ecto embed, the form casts the new SMS + # with the changeset function, so the parent action shows its errors. + assert f.errors == [ + number: {"can't be blank", [validation: :required]}, + country_code: {"can't be blank", [validation: :required]}, + provider: {"can't be blank", [validation: :required]} + ] "from safe_inputs_for" end) From 7f999f2372f110de637231abcbff4e37f478de30 Mon Sep 17 00:00:00 2001 From: Nick Date: Fri, 23 Oct 2026 12:58:27 +0200 Subject: [PATCH 09/11] Take the parent action of child forms from the form phoenix_ecto takes the parent action of the child forms from the form. The polymorphic form helper took it from the changeset argument. The two differ when a form is built with to_form(changeset, action: :validate) and the helper gets the changeset without an action: the child forms then had no action and showed no errors. The new form parity tests build the form with action: :validate, once with form.source and once with the changeset without action. --- CHANGELOG.md | 2 + lib/polymorphic_embed/html/helpers.ex | 4 +- test/polymorphic_embed/form_parity_test.exs | 48 +++++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea1b8f1..a53bbd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ back. * A form for an embed that is `nil` uses the changeset function. With a parent action, it shows the errors of that changeset. + * The child forms take the parent action from the form, as in phoenix_ecto. Before, + `PolymorphicEmbed.HTML.Helpers.to_form/4` took it from the changeset argument. ## 5.0.2 diff --git a/lib/polymorphic_embed/html/helpers.ex b/lib/polymorphic_embed/html/helpers.ex index 8bd8784..716916d 100644 --- a/lib/polymorphic_embed/html/helpers.ex +++ b/lib/polymorphic_embed/html/helpers.ex @@ -50,7 +50,9 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d end # Builds the child forms as phoenix_ecto does for embeds_one and embeds_many. - def to_form(%{action: parent_action} = source_changeset, form, field, options) do + # As in phoenix_ecto, the parent action comes from the form, because + # to_form(changeset, action: :validate) sets it on the form. + def to_form(source_changeset, %{action: parent_action} = form, field, options) do id = to_string(form.id <> "_#{field}") name = to_string(form.name <> "[#{field}]") diff --git a/test/polymorphic_embed/form_parity_test.exs b/test/polymorphic_embed/form_parity_test.exs index b345aac..f691104 100644 --- a/test/polymorphic_embed/form_parity_test.exs +++ b/test/polymorphic_embed/form_parity_test.exs @@ -173,6 +173,54 @@ defmodule PolymorphicEmbed.FormParityTest do assert_parity(data(:existing, :many), params, :items, :validate) end + # In LiveView, the action is often set on the form with + # to_form(changeset, action: :validate), and the changeset has no action. + # The child forms then take the action of the form. + @validate_params %{ + item: %{"item" => %{"title" => "", "meta" => %{"note" => "x"}}}, + items: %{ + "items" => %{ + "0" => %{"title" => "", "tags" => %{"0" => %{"name" => ""}}}, + "1" => %{"title" => "Hello"} + } + } + } + + for {field, cardinality} <- [item: "embeds_one", items: "embeds_many"] do + test "#{cardinality}: action :validate on the form" do + assert_form_action_parity(unquote(field), :form_source) + end + + test "#{cardinality}: action :validate on the form, with the changeset without action" do + assert_form_action_parity(unquote(field), :changeset) + end + end + + defp assert_form_action_parity(field, source) do + params = Map.fetch!(@validate_params, field) + + regular = %RegularParent{} |> Changeset.cast(params, []) |> Changeset.cast_embed(field) + + poly = + %PolyParent{} + |> Changeset.cast(add_type(params, field), []) + |> PolymorphicEmbed.cast_polymorphic_embed(field) + + regular_form = Phoenix.Component.to_form(regular, as: "parent", action: :validate) + poly_form = Phoenix.Component.to_form(poly, as: "parent", action: :validate) + + {regular_source, poly_source} = + case source do + :form_source -> {regular_form.source, poly_form.source} + :changeset -> {regular, poly} + end + + regular_forms = Phoenix.HTML.FormData.to_form(regular_source, regular_form, field, []) + poly_forms = Helpers.to_form(poly_source, poly_form, field, []) + + assert diff(snapshot_forms(regular_forms), snapshot_forms(poly_forms), [field]) == [] + end + ## Helpers defp data(:existing, :one), do: %{item: existing_item()} From de4175aa7551f4494a0491c1e30325866d52c8f4 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 24 Oct 2026 11:22:49 +0200 Subject: [PATCH 10/11] Add regression tests for inserts, forms and load options Reviews of the change that keeps the child changesets in the changes found these flows. The tests check that: - an insert keeps embeds of more than one type that are only in the data, and generates their ids - the form helper builds forms when the changes hold structs, also after the repo prepared them - polymorphic embeds inside an ordinary embed of a child, and that ordinary embed, get ids on insert - a polymorphic embed that is only in the data gets its id on insert, inside an ordinary embed and inside a polymorphic child - an ordinary embed in a polymorphic child gets its id when a parent insert updates the child - a struct that put_change/3 set before the cast stays usable for apply_changes/1, get_field/2 and required: true - lists with entries that the load kept as maps or set to nil can be replaced - the struct that apply_changes/1 returns can be inserted, with ids - a child that its prepare function makes invalid stops the insert - merge/2 keeps the structs when the cast changeset is the second argument - a cast inside a prepare function of the parent gives structs with ids --- .../review_regression_test.exs | 326 ++++++++++++++++++ 1 file changed, 326 insertions(+) create mode 100644 test/polymorphic_embed/review_regression_test.exs diff --git a/test/polymorphic_embed/review_regression_test.exs b/test/polymorphic_embed/review_regression_test.exs new file mode 100644 index 0000000..4215e2b --- /dev/null +++ b/test/polymorphic_embed/review_regression_test.exs @@ -0,0 +1,326 @@ +# Regression tests for inserts, forms and load options. A review of the +# change that keeps the child changesets in the changes found these flows. +defmodule PolymorphicEmbed.ReviewRegressionTest do + use ExUnit.Case + + alias Ecto.Changeset + alias PolymorphicEmbed.HTML.Helpers + alias PolymorphicEmbed.Repo + + alias __MODULE__.{A, B, Holder, Inner, Parent, Wrapper} + + defmodule A do + use Ecto.Schema + + embedded_schema do + field :name, :string + end + + def changeset(struct, params), do: Changeset.cast(struct, params, [:name]) + + def failing_prepare_changeset(struct, params) do + struct + |> changeset(params) + |> Changeset.prepare_changes(&Changeset.add_error(&1, :name, "is rejected")) + end + end + + defmodule B do + use Ecto.Schema + + embedded_schema do + field :name, :string + end + + def changeset(struct, params), do: Changeset.cast(struct, params, [:name]) + end + + # An ordinary Ecto embed with a polymorphic field. + defmodule Inner do + use Ecto.Schema + import PolymorphicEmbed + + embedded_schema do + polymorphic_embeds_one :leaf, types: [a: A], on_replace: :update + end + + def changeset(struct, params) do + struct + |> Changeset.cast(params, []) + |> cast_polymorphic_embed(:leaf) + end + end + + # An embedded schema with a field and a polymorphic field. The tests use it + # as an ordinary Ecto embed and as a polymorphic type. + defmodule Holder do + use Ecto.Schema + import PolymorphicEmbed + + embedded_schema do + field :name, :string + polymorphic_embeds_one :leaf, types: [a: A], on_replace: :update + end + + def changeset(struct, params) do + struct + |> Changeset.cast(params, [:name]) + |> cast_polymorphic_embed(:leaf) + end + end + + # A polymorphic child with ordinary Ecto embeds. + defmodule Wrapper do + use Ecto.Schema + + embedded_schema do + embeds_one :inner, Inner + embeds_one :holder, Holder + end + + def changeset(struct, params) do + struct + |> Changeset.cast(params, []) + |> Changeset.cast_embed(:inner) + |> Changeset.cast_embed(:holder) + end + end + + defmodule Parent do + use Ecto.Schema + import PolymorphicEmbed + + schema "reminders" do + field :date, :utc_datetime + field :text, :string + + polymorphic_embeds_one :channel, + types: [a: A, b: B, wrapper: Wrapper, holder: Holder], + on_replace: :update + + polymorphic_embeds_many :contexts, types: [a: A, b: B], on_replace: :delete + + polymorphic_embeds_many :contexts2, + types: [a: A], + retain_unlisted_types_on_load: [:old], + on_replace: :delete + + polymorphic_embeds_many :contexts3, + types: [a: A], + nilify_unlisted_types_on_load: [:gone], + on_replace: :delete + + embeds_one :holder, Holder, source: :channel2 + + timestamps() + end + end + + setup do + :ok = Ecto.Adapters.SQL.Sandbox.checkout(Repo) + end + + @parent_params %{"date" => ~U[2020-05-28 02:57:19Z], "text" => "text"} + + defp cast(parent, params, fields) do + changeset = Changeset.cast(parent, Map.merge(@parent_params, params), [:date, :text]) + Enum.reduce(fields, changeset, &PolymorphicEmbed.cast_polymorphic_embed(&2, &1)) + end + + # Runs the prepare functions as the repo does before it writes the parent. + defp run_prepare(changeset) do + changeset.prepare + |> Enum.reverse() + |> Enum.reduce(%{changeset | repo: Repo, action: :insert}, & &1.(&2)) + end + + test "inserts embeds of more than one type that are only in the data" do + parent = %Parent{channel: %B{name: "b"}, contexts: [%A{name: "a"}, %B{name: "b"}]} + + inserted = parent |> cast(%{}, [:channel, :contexts]) |> Repo.insert!() + + assert %B{name: "b", id: id} = inserted.channel + assert id + assert [%A{name: "a"}, %B{name: "b"}] = inserted.contexts + assert Enum.all?(inserted.contexts, & &1.id) + end + + test "builds forms when the changes hold structs" do + changeset = + %Parent{} + |> Changeset.change() + |> Changeset.put_change(:contexts, [%A{name: "a"}]) + + form = Phoenix.Component.to_form(changeset) + assert [%{data: %A{name: "a"}}] = Helpers.to_form(changeset, form, :contexts, []) + end + + test "builds forms after the repo prepared the changes" do + changeset = + %Parent{} + |> cast(%{"contexts" => [%{"__type__" => "a", "name" => "a"}]}, [:contexts]) + |> run_prepare() + + form = Phoenix.Component.to_form(changeset) + assert [%{data: %A{name: "a"}}] = Helpers.to_form(changeset, form, :contexts, []) + end + + test "generates ids of polymorphic embeds inside an ordinary embed of a child" do + params = %{ + "channel" => %{ + "__type__" => "wrapper", + "inner" => %{"leaf" => %{"__type__" => "a", "name" => "leaf"}} + } + } + + inserted = %Parent{} |> cast(params, [:channel]) |> Repo.insert!() + + assert %Wrapper{inner: %Inner{id: inner_id, leaf: %A{name: "leaf", id: id}}} = + inserted.channel + + assert id + assert inner_id + end + + test "replaces a list with entries that the load kept as maps" do + parent = %Parent{contexts2: [%{"__type__" => "old", "name" => "old"}, %A{id: "1"}]} + + changeset = + cast(parent, %{"contexts2" => [%{"__type__" => "a", "name" => "new"}]}, [:contexts2]) + + assert changeset.valid? + assert [%A{name: "new"}] = Changeset.apply_changes(changeset).contexts2 + + changeset = cast(parent, %{"contexts2" => []}, [:contexts2]) + assert changeset.valid? + assert [] = Changeset.apply_changes(changeset).contexts2 + end + + test "replaces a list with entries that the load set to nil" do + parent = %Parent{contexts3: [nil, %A{id: "1"}]} + + changeset = + cast(parent, %{"contexts3" => [%{"__type__" => "a", "name" => "new"}]}, [:contexts3]) + + assert changeset.valid? + assert [%A{name: "new"}] = Changeset.apply_changes(changeset).contexts3 + + changeset = cast(parent, %{"contexts3" => []}, [:contexts3]) + assert changeset.valid? + assert [] = Changeset.apply_changes(changeset).contexts3 + end + + test "generates the id of a polymorphic embed that is only in the data of an ordinary embed" do + parent = %Parent{holder: %Holder{name: "old", leaf: %A{name: "leaf"}}} + + inserted = + parent + |> Changeset.cast(Map.merge(@parent_params, %{"holder" => %{"name" => "new"}}), [ + :date, + :text + ]) + |> Changeset.cast_embed(:holder) + |> Repo.insert!() + + assert %Holder{name: "new", leaf: %A{name: "leaf", id: id}} = inserted.holder + assert id + end + + test "generates the id of a polymorphic embed that is only in the data of a polymorphic child" do + parent = %Parent{channel: %Holder{id: "h1", name: "old", leaf: %A{name: "leaf"}}} + params = %{"channel" => %{"__type__" => "holder", "name" => "new"}} + + inserted = parent |> cast(params, [:channel]) |> Repo.insert!() + + assert %Holder{name: "new", leaf: %A{name: "leaf", id: id}} = inserted.channel + assert id + end + + test "applies a struct that put_change/3 set before the cast" do + changeset = + %Parent{} + |> Changeset.change() + |> Changeset.put_change(:channel, %A{name: "a"}) + |> PolymorphicEmbed.cast_polymorphic_embed(:channel) + + assert %A{name: "a"} = Changeset.apply_changes(changeset).channel + assert %A{name: "a"} = Changeset.get_field(changeset, :channel) + + changeset = + %Parent{} + |> Changeset.change() + |> Changeset.put_change(:channel, %A{name: "a"}) + |> PolymorphicEmbed.cast_polymorphic_embed(:channel, required: true) + + assert changeset.valid? + end + + test "generates the id of an ordinary embed in a polymorphic child that the insert updates" do + parent = %Parent{channel: %Wrapper{id: "w1", holder: %Holder{name: "old"}}} + params = %{"channel" => %{"__type__" => "wrapper", "holder" => %{"name" => "new"}}} + + inserted = parent |> cast(params, [:channel]) |> Repo.insert!() + + assert %Wrapper{id: "w1", holder: %Holder{name: "new", id: id}} = inserted.channel + assert id + end + + test "raises when a prepare function of a child adds an error" do + changeset = + %Parent{} + |> Changeset.cast( + Map.merge(@parent_params, %{"channel" => %{"__type__" => "a", "name" => "a"}}), + [:date, :text] + ) + |> PolymorphicEmbed.cast_polymorphic_embed(:channel, + with: [a: &A.failing_prepare_changeset/2] + ) + + assert changeset.valid? + + assert_raise ArgumentError, + ~r/is invalid, but the parent changeset was not marked as invalid/, + fn -> + Repo.insert(changeset) + end + end + + # merge/2 takes the types of the second changeset, so the cast changeset + # must be the second argument. + test "keeps structs after merge with the cast changeset as second argument" do + changeset = cast(%Parent{}, %{"channel" => %{"__type__" => "a", "name" => "a"}}, [:channel]) + merged = Changeset.merge(Changeset.change(%Parent{}), changeset) + + assert %A{name: "a"} = Changeset.get_field(merged, :channel) + assert %A{name: "a"} = Changeset.apply_changes(merged).channel + end + + test "prepares a cast that a prepare function of the parent runs" do + changeset = + %Parent{} + |> Changeset.cast(@parent_params, [:date, :text]) + |> Changeset.prepare_changes(fn changeset -> + changeset + |> Changeset.cast(%{"channel" => %{"__type__" => "a", "name" => "a"}}, []) + |> PolymorphicEmbed.cast_polymorphic_embed(:channel) + end) + + inserted = Repo.insert!(changeset) + assert %A{name: "a", id: id} = inserted.channel + assert id + assert %A{name: "a", id: ^id} = Repo.get!(Parent, inserted.id).channel + end + + test "inserts the struct that apply_changes returns" do + params = %{"channel" => %{"__type__" => "a", "name" => "a"}} + + inserted = + %Parent{} + |> cast(params, [:channel]) + |> Changeset.apply_changes() + |> Repo.insert!() + + assert %A{name: "a", id: id} = inserted.channel + assert id + end +end From 44b1449e6d2c12d65d28681206a374d0a3cb0479 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 24 Oct 2026 11:23:14 +0200 Subject: [PATCH 11/11] Add put_polymorphic_embed/3 When the cast puts changesets into the changes of a polymorphic field, the field has an Ecto embed type with one schema module. Ecto's put_change/3, update_change/3 and change/2 then check each child against that module, so they raise for children of another type. Ecto has no place where polymorphic_embed can change that check. put_polymorphic_embed/3 works as Ecto.Changeset.put_embed/4, but it finds the matching data row by module and primary key, per element. So a list can hold more than one type, and a single embed can change its type. It accepts structs, changesets, maps and keyword lists. The type of a map comes from its type field, from the data row with the same primary key, or from the only type of the field. A new child gets its id at once, as in the cast. The changelog lists the new function, and the limit of Ecto's functions as a breaking change. The function keeps the changeset function that an earlier cast stored for the forms, for example a with: function. An empty list replaces data that holds only entries from the load options. The tests also check that ordinary embeds in the data of a put child get their ids on insert, and that the form keeps the id of a struct that the caller gives. As Ecto.Changeset.put_embed/4 does, the function gives the error "is invalid" for a value that the field does not accept, for example nil for a list of embeds, and keeps the data and the changes. A put inside a prepare function of the parent prepares the embeds at once, as the cast does. --- CHANGELOG.md | 10 + lib/polymorphic_embed.ex | 194 ++++++++++++++++++ .../review_regression_test.exs | 176 ++++++++++++++++ 3 files changed, 380 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a53bbd7..8a71286 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ `cast_polymorphic_embed/3` now gives the same output as `Ecto.Changeset.cast_embed/3` for the same input. + **Features**: + + * Add `PolymorphicEmbed.put_polymorphic_embed/3`. It works as + `Ecto.Changeset.put_embed/4`, and it accepts children of all the types of the + field, also in one list. + **Breaking changes**: * `changeset.changes` keeps each child as an `Ecto.Changeset`, as for an Ecto embed. @@ -30,6 +36,10 @@ * When the cast puts changesets into the changes of the field, `changeset.types` has an `:embed` type for it. For example, `validate_required/3` on the field then shows the same warning as for an Ecto embed. + * After the cast puts changesets into the changes of the field, + `Ecto.Changeset.put_change/3`, `update_change/3` and `change/2` accept only one + schema module for the field, as for an Ecto embed. Before, they accepted all types. + Use `PolymorphicEmbed.put_polymorphic_embed/3` for children of other types. * `nil` for a list of embeds gives the error "is invalid". Before, the cast ignored it. * With `required: true`, an empty list in the data gives "can't be blank", and `changeset.required` contains the field. diff --git a/lib/polymorphic_embed.ex b/lib/polymorphic_embed.ex index 3944432..82faa50 100644 --- a/lib/polymorphic_embed.ex +++ b/lib/polymorphic_embed.ex @@ -149,6 +149,200 @@ defmodule PolymorphicEmbed do raise "cast_polymorphic_embed/3 only accepts a changeset as first argument" end + @doc """ + Puts an embed or a list of embeds into a polymorphic field, as + `Ecto.Changeset.put_embed/4` does for an Ecto embed. + + After `cast_polymorphic_embed/3` puts a change, the field has an Ecto embed + type with one schema module. `Ecto.Changeset.put_change/3`, + `Ecto.Changeset.update_change/3` and `Ecto.Changeset.change/2` then accept + only children of that module. This function accepts children of all the + types of the field. + + The value can be a struct, a changeset, a map or a keyword list, or a list of + them for a list of embeds. The type of a map or a keyword list comes from its + type field. Without a type field, the type is the type of the data with the + same primary key, or the only type of the field. + """ + def put_polymorphic_embed(%Ecto.Changeset{} = changeset, field, value) do + field_opts = get_field_opts(changeset.data.__struct__, field) + + if valid_put_value?(value, field_opts.array?) do + do_put_polymorphic_embed(changeset, field, value, field_opts) + else + # The same error as Ecto.Changeset.put_embed/4 gives. The data and the + # changes stay as they are. + type = if field_opts.array?, do: {:array, :map}, else: :map + error = {field, {"is invalid", [type: type]}} + %{changeset | errors: [error | changeset.errors], valid?: false} + end + end + + defp valid_put_value?(list, true = _array?) when is_list(list), + do: Enum.all?(list, &valid_put_value?(&1, false)) + + defp valid_put_value?(_value, true = _array?), do: false + defp valid_put_value?(nil, false = _array?), do: true + defp valid_put_value?(map, false = _array?) when is_map(map), do: true + defp valid_put_value?(list, false = _array?) when is_list(list), do: Keyword.keyword?(list) + defp valid_put_value?(_value, false = _array?), do: false + + defp do_put_polymorphic_embed(changeset, field, value, field_opts) do + original = Map.fetch!(changeset.data, field) + + result = + if field_opts.array?, + do: put_embeds_many(value, original || [], field_opts), + else: put_embeds_one(value, original, field_opts) + + changeset = + case result do + :ignore -> delete_embed_change(changeset, field) + {:ok, change} -> put_embed_change(changeset, field, change) + end + + # The forms keep the changeset function of an earlier cast, for example a + # with: function. + on_cast = + stored_on_cast(changeset, field) || + (&changeset_fun(&1, &2, nil, field_opts.types_metadata)) + + changeset + |> put_embed_type(field, field_opts, on_cast) + |> register_prepare(field) + end + + defp stored_on_cast(changeset, field) do + case Map.get(changeset.types, field) do + {:embed, %{on_cast: on_cast}} when is_function(on_cast, 2) -> on_cast + {:parameterized, {PolymorphicEmbed, %{on_cast: on_cast}}} -> on_cast + {:array, {:parameterized, {PolymorphicEmbed, %{on_cast: on_cast}}}} -> on_cast + _ -> nil + end + end + + defp put_embeds_one(nil, nil, _field_opts), do: :ignore + defp put_embeds_one(nil, _original, _field_opts), do: {:ok, nil} + + # As in Ecto with on_replace: :update, a map or a keyword list updates the + # data of the same type. A struct or a changeset updates the data only when + # the primary keys match. Else the value replaces the data. + defp put_embeds_one(value, original, field_opts) do + module = put_module(value, List.wrap(original), field_opts) + attrs? = not is_struct(value) + + current = + if is_struct(original) and original.__struct__ == module and + (attrs? or put_pk_match?(value, original)), + do: original + + embed_changeset = put_changeset(value, current, module, field_opts) + + cond do + embed_changeset.action == :ignore -> :ignore + current && skip?(embed_changeset) -> :ignore + true -> {:ok, embed_changeset} + end + end + + # As in Ecto, each value updates the data row with the same module and + # primary key. Rows of the data that the value does not keep are replaced. + defp put_embeds_many(values, original, field_opts) do + structs = Enum.filter(original, &is_struct/1) + + {embeds, remaining} = + values + |> List.wrap() + |> Enum.map_reduce(structs, fn value, remaining -> + module = put_module(value, remaining, field_opts) + + index = + Enum.find_index(remaining, &(&1.__struct__ == module and put_pk_match?(value, &1))) + + {current, remaining} = + if index, do: List.pop_at(remaining, index), else: {nil, remaining} + + {{put_changeset(value, current, module, field_opts), current}, remaining} + end) + + embeds = Enum.reject(embeds, fn {embed_changeset, _} -> embed_changeset.action == :ignore end) + + # As in Ecto, the list is not a change when each value updates its data row + # without changes, in the order of the data. + if remaining == [] and Enum.map(embeds, &elem(&1, 1)) == original and + Enum.all?(embeds, fn {embed_changeset, _} -> skip?(embed_changeset) end) do + :ignore + else + replaced = + remaining + |> Enum.map(&%{Ecto.Changeset.change(&1) | action: :replace}) + |> Enum.reverse() + + {:ok, replaced ++ Enum.map(embeds, &elem(&1, 0))} + end + end + + defp put_changeset(%Changeset{} = embed_changeset, current, _module, _field_opts), + do: put_new_action(embed_changeset, current) + + defp put_changeset(%_{} = struct, current, _module, _field_opts), + do: struct |> Ecto.Changeset.change() |> put_new_action(current) + + defp put_changeset(attrs, current, module, field_opts) do + attrs = + attrs + |> Map.new() + |> Map.drop([field_opts.type_field_name, to_string(field_opts.type_field_name)]) + + (current || struct(module)) + |> Ecto.Changeset.change(attrs) + |> put_new_action(current) + end + + # A new child gets its id at once, as in the cast. + defp put_new_action(%Changeset{action: nil} = embed_changeset, nil), + do: %{embed_changeset | action: :insert, data: autogenerate_id(embed_changeset.data, :insert)} + + defp put_new_action(%Changeset{action: nil} = embed_changeset, _current), + do: %{embed_changeset | action: :update} + + defp put_new_action(embed_changeset, _current), do: embed_changeset + + defp put_module(%Changeset{data: %module{}}, _data, _field_opts), do: module + defp put_module(%module{}, _data, _field_opts), do: module + + defp put_module(attrs, data, field_opts) do + %{types_metadata: types_metadata, type_field_name: type_field_name} = field_opts + attrs = Map.new(attrs) + + cond do + module = get_polymorphic_module_from_map(attrs, type_field_name, types_metadata) -> + module + + current = Enum.find(data, &put_pk_match?(attrs, &1)) -> + current.__struct__ + + match?([_], types_metadata) -> + hd(types_metadata).module + + true -> + raise_cannot_infer_type_from_data(attrs) + end + end + + # As in Ecto, the primary key values of the value are compared as they are. + # The data row must have a primary key. + defp put_pk_match?(value, %module{} = current) do + pks = module.__schema__(:primary_key) + + pks != [] and + Enum.map(pks, &put_pk_value(value, &1)) == Enum.map(pks, &Map.get(current, &1)) + end + + defp put_pk_value(%Changeset{data: data}, pk), do: Map.get(data, pk) + defp put_pk_value(%_{} = struct, pk), do: Map.get(struct, pk) + defp put_pk_value(attrs, pk), do: Attrs.get(Map.new(attrs), pk) + defp do_cast_polymorphic_embed( changeset, field, diff --git a/test/polymorphic_embed/review_regression_test.exs b/test/polymorphic_embed/review_regression_test.exs index 4215e2b..929ee6d 100644 --- a/test/polymorphic_embed/review_regression_test.exs +++ b/test/polymorphic_embed/review_regression_test.exs @@ -18,6 +18,12 @@ defmodule PolymorphicEmbed.ReviewRegressionTest do def changeset(struct, params), do: Changeset.cast(struct, params, [:name]) + def required_changeset(struct, params) do + struct + |> Changeset.cast(params, [:name]) + |> Changeset.validate_required([:name]) + end + def failing_prepare_changeset(struct, params) do struct |> changeset(params) @@ -255,6 +261,176 @@ defmodule PolymorphicEmbed.ReviewRegressionTest do assert changeset.valid? end + describe "put_polymorphic_embed/3" do + # After the cast, Ecto.Changeset.put_change/3 and update_change/3 accept + # only one schema module for the field, as for an Ecto embed. + test "edits a list of more than one type" do + params = %{ + "contexts" => [%{"__type__" => "a", "name" => "a"}, %{"__type__" => "b", "name" => "b"}] + } + + changeset = cast(%Parent{}, params, [:contexts]) + + assert_raise ArgumentError, ~r/expected changeset data to be a/, fn -> + Changeset.update_change(changeset, :contexts, & &1) + end + + same = + PolymorphicEmbed.put_polymorphic_embed( + changeset, + :contexts, + Changeset.get_change(changeset, :contexts) + ) + + assert [%A{name: "a"}, %B{name: "b"}] = Changeset.apply_changes(same).contexts + + structs = + PolymorphicEmbed.put_polymorphic_embed(changeset, :contexts, [ + %A{name: "x"}, + %B{name: "y"} + ]) + + assert [%A{name: "x"}, %B{name: "y"}] = Changeset.apply_changes(structs).contexts + + maps = + PolymorphicEmbed.put_polymorphic_embed(changeset, :contexts, [ + %{__type__: "b", name: "m"}, + [__type__: "a", name: "n"] + ]) + + assert [%B{name: "m"}, %A{name: "n"}] = Changeset.apply_changes(maps).contexts + + inserted = Repo.insert!(maps) + assert [%B{name: "m", id: b_id}, %A{name: "n", id: a_id}] = inserted.contexts + assert b_id && a_id + end + + test "updates, keeps and replaces rows of the data by module and primary key" do + parent = %Parent{contexts: [%A{id: "a1", name: "a"}, %B{id: "b1", name: "b"}]} + changeset = Changeset.change(parent) + + assert PolymorphicEmbed.put_polymorphic_embed(changeset, :contexts, parent.contexts).changes == + %{} + + changeset = + PolymorphicEmbed.put_polymorphic_embed(changeset, :contexts, [ + %{id: "b1", name: "new b"}, + %A{name: "new a"} + ]) + + assert [ + %{action: :replace, data: %A{id: "a1"}}, + %{action: :update, data: %B{id: "b1"}, changes: %{name: "new b"}}, + %{action: :insert, data: %A{id: new_id}} + ] = changeset.changes.contexts + + assert new_id + + assert [%B{id: "b1", name: "new b"}, %A{name: "new a"}] = + Changeset.apply_changes(changeset).contexts + end + + test "generates ids of ordinary embeds in the data of a child" do + changeset = + %Parent{} + |> cast(%{}, []) + |> PolymorphicEmbed.put_polymorphic_embed(:channel, %Wrapper{holder: %Holder{name: "h"}}) + + inserted = Repo.insert!(changeset) + + assert %Wrapper{id: wrapper_id, holder: %Holder{name: "h", id: holder_id}} = + inserted.channel + + assert wrapper_id + assert holder_id + end + + test "replaces data that holds only entries from the load options with an empty list" do + parent = %Parent{contexts2: [%{"__type__" => "old", "name" => "old"}]} + + changeset = PolymorphicEmbed.put_polymorphic_embed(Changeset.change(parent), :contexts2, []) + + assert Map.has_key?(changeset.changes, :contexts2) + assert [] = Changeset.apply_changes(changeset).contexts2 + end + + test "keeps the with: function of the cast for the forms" do + parent = %Parent{channel: %A{id: "a1", name: nil}} + + changeset = + parent + |> Changeset.cast(@parent_params, [:date, :text]) + |> PolymorphicEmbed.cast_polymorphic_embed(:channel, + with: [a: &A.required_changeset/2] + ) + |> PolymorphicEmbed.put_polymorphic_embed(:channel, parent.channel) + + refute Map.has_key?(changeset.changes, :channel) + + form = Phoenix.Component.to_form(changeset, action: :validate) + [child_form] = Helpers.to_form(form.source, form, :channel, []) + assert child_form.errors == [name: {"can't be blank", [validation: :required]}] + end + + test "keeps the id of a new child that the caller gives in the form" do + id = Ecto.UUID.generate() + + changeset = + %Parent{} + |> cast(%{}, []) + |> PolymorphicEmbed.put_polymorphic_embed(:channel, %A{id: id, name: "a"}) + + form = Phoenix.Component.to_form(changeset) + [child_form] = Helpers.to_form(changeset, form, :channel, []) + + assert child_form.data.id == id + assert {:id, id} in child_form.hidden + end + + test "prepares a put that a prepare function of the parent runs" do + changeset = + %Parent{} + |> Changeset.cast(@parent_params, [:date, :text]) + |> Changeset.prepare_changes( + &PolymorphicEmbed.put_polymorphic_embed(&1, :channel, %A{name: "a"}) + ) + + inserted = Repo.insert!(changeset) + assert %A{name: "a", id: id} = inserted.channel + assert id + end + + test "gives an error and keeps the data for nil as a list of embeds" do + parent = %Parent{contexts: [%A{id: "a1", name: "a"}]} + changeset = PolymorphicEmbed.put_polymorphic_embed(Changeset.change(parent), :contexts, nil) + + refute changeset.valid? + refute Map.has_key?(changeset.changes, :contexts) + assert changeset.errors == [contexts: {"is invalid", [type: {:array, :map}]}] + end + + test "gives an error for a single embed that is not a map" do + changeset = + PolymorphicEmbed.put_polymorphic_embed(Changeset.change(%Parent{}), :channel, "a") + + refute changeset.valid? + assert changeset.errors == [channel: {"is invalid", [type: :map]}] + end + + test "changes the type of a single embed" do + changeset = + %Parent{} + |> cast(%{"channel" => %{"__type__" => "a", "name" => "a"}}, [:channel]) + |> PolymorphicEmbed.put_polymorphic_embed(:channel, %B{name: "b"}) + + assert %{action: :insert, data: %B{}} = changeset.changes.channel + assert %B{name: "b"} = Changeset.apply_changes(changeset).channel + + changeset = PolymorphicEmbed.put_polymorphic_embed(changeset, :channel, nil) + refute Map.has_key?(changeset.changes, :channel) + end + end + test "generates the id of an ordinary embed in a polymorphic child that the insert updates" do parent = %Parent{channel: %Wrapper{id: "w1", holder: %Holder{name: "old"}}} params = %{"channel" => %{"__type__" => "wrapper", "holder" => %{"name" => "new"}}}