From d870095803445f4bf8886865f5fc47763f8d9d62 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 24 Oct 2026 12:00:45 +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. A polymorphic embed keeps a valid child as a struct in the changes, and an Ecto embed keeps a changeset. So the changeset tests leave out the tests that read the changes, the action or the validity of a valid child, and the tests that expect :replace changesets. The form tests compare the form contract: ids, names, errors, row params, hidden inputs, input values, and the data that Ecto.Changeset.get_field/2 gives for each field. Some tests fail on purpose. Each failure shows a difference from Ecto that a later commit fixes. --- test/polymorphic_embed/ecto_parity_test.exs | 334 +++++++++++++++++++ test/polymorphic_embed/form_parity_test.exs | 340 ++++++++++++++++++++ 2 files changed, 674 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..7773cd4 --- /dev/null +++ b/test/polymorphic_embed/ecto_parity_test.exs @@ -0,0 +1,334 @@ +# 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. +# +# A polymorphic embed keeps a valid child as a struct in the changes, and an +# Ecto embed keeps a changeset. Tests that read the changes, the action or +# the validity of a valid child, and tests that expect :replace changesets, +# are therefore left out too. +defmodule PolymorphicEmbed.EctoParityTest do + use ExUnit.Case, async: true + + alias Ecto.Changeset + + alias __MODULE__.{Author, AutoIdPost, 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 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 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 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 "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.valid? + + assert [%AutoIdPost{id: nil, title: "hello"}] = + Changeset.apply_changes(changeset).auto_id_posts + end + + ## cast embeds many + + 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 "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 + 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..47fb832 --- /dev/null +++ b/test/polymorphic_embed/form_parity_test.exs @@ -0,0 +1,340 @@ +# 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 and its nested forms, and asserts that the +# polymorphic snapshot has no differences from the regular one. A failure +# lists each differing path with the regular and the polymorphic value. +# +# The snapshot holds the form contract: what a template reads from a form. +# A valid polymorphic child is a struct, not a changeset. So the action and +# the data of its changeset, and the params of its nested forms, differ from +# Ecto by design, and the snapshot leaves them out. Templates read the data +# of an embed with Ecto.Changeset.get_field/2, so the snapshot compares it +# for each field. The type field is the only other expected difference. +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 the form contract. Nested forms have no params in it. + defp snapshot_forms(forms, nested? \\ false), do: Enum.map(forms, &snapshot_form(&1, nested?)) + + defp snapshot_form(%Phoenix.HTML.Form{} = form, nested?) do + module = form.source.data.__struct__ + embeds = module.__schema__(:embeds) + fields = module.__schema__(:fields) + + snapshot = %{ + id: form.id, + name: form.name, + index: form.index, + action: form.action, + errors: form.errors, + hidden: Keyword.delete(form.hidden, String.to_atom(@type_key)) |> drop_type_hidden(), + values: Map.new(fields -- embeds, &{&1, normalize(form[&1].value)}), + fields: Map.new(fields, &{&1, normalize(Changeset.get_field(form.source, &1))}), + nested: + Map.new(embeds, fn embed -> + nested_forms = Phoenix.HTML.FormData.to_form(form.source, form, embed, []) + {embed, snapshot_forms(nested_forms, true)} + end) + } + + if nested?, do: snapshot, else: Map.put(snapshot, :params, drop_type(form.params)) + 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) + + # 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: [] + + # The cast generates the id of a new child at once. Ecto generates it on + # insert, so its form shows nil. + defp diff(nil, "", _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 87b3adfaa2a25255ff1d8257190a2d749c6d9887 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 24 Oct 2026 12:00:54 +0200 Subject: [PATCH 02/11] Preserve child form params, data and actions The changes of PR #132, in one commit: - After the cast, changeset.params holds the params of each child for the field. For a list of embeds, it is a list in the order of the cast, without dropped and ignored rows. The README describes this. - Each child form of a list gets the params of its own row. Before, the first form got the params of all rows, so _persistent_id and other row values were lost. - form.data of a child form is the data of the child, not a changeset. - A child form takes the action of the parent form. A parent without an action hides the errors of the children, and a parent with an action shows them, also for a child changeset without an action. test/polymorphic_embed/html/helpers_test.exs holds regression tests for these cases. --- README.md | 2 + lib/polymorphic_embed.ex | 15 +- lib/polymorphic_embed/html/helpers.ex | 96 ++++++---- test/polymorphic_embed/html/helpers_test.exs | 190 +++++++++++++++++++ test/polymorphic_embed_test.exs | 179 +++++++++++++++-- 5 files changed, 429 insertions(+), 53 deletions(-) create mode 100644 test/polymorphic_embed/html/helpers_test.exs diff --git a/README.md b/README.md index 28c7bfc..7cf493d 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,8 @@ add(:channel, :map) `cast_polymorphic_embed/3` must be called to cast the polymorphic embed's parameters. +After casting a polymorphic embed, its entry in `changeset.params` contains the child changeset's parameters. For `polymorphic_embeds_many`, this is a list in the final cast order, with dropped and ignored rows removed. Form helpers use these parameters to preserve submitted values and row identifiers. + #### Options * `:required` – if the embed is a required field. diff --git a/lib/polymorphic_embed.ex b/lib/polymorphic_embed.ex index 47e8ad9..388a80b 100644 --- a/lib/polymorphic_embed.ex +++ b/lib/polymorphic_embed.ex @@ -319,6 +319,7 @@ defmodule PolymorphicEmbed do {action, struct} -> embed_changeset = changeset_fun.(struct, params) embed_changeset = %{embed_changeset | action: action} + changeset = put_embed_params(changeset, field, embed_changeset.params || params) case embed_changeset do %{valid?: true} = embed_changeset -> @@ -431,14 +432,17 @@ defmodule PolymorphicEmbed do %{changeset_fun.(struct(module), params) | action: :insert} end - maybe_apply_changes(embed_changeset) + {maybe_apply_changes(embed_changeset), embed_changeset.params || params} 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, params} = + embeds + |> Enum.reject(&(&1 == :ignore)) + |> Enum.unzip() any_invalid? = Enum.any?(embeds, fn @@ -446,6 +450,9 @@ defmodule PolymorphicEmbed do _ -> false end) + # Keep each row's parameters in cast order, including children converted to structs. + changeset = put_embed_params(changeset, field, params) + changeset = Ecto.Changeset.put_change(changeset, field, embeds) if any_invalid? do @@ -464,6 +471,10 @@ defmodule PolymorphicEmbed do defp maybe_apply_changes(%Changeset{valid?: false} = changeset), do: changeset + defp put_embed_params(changeset, field, params) do + %{changeset | params: Map.put(changeset.params || %{}, to_string(field), params)} + end + @impl true def cast(_data, _params), do: diff --git a/lib/polymorphic_embed/html/helpers.ex b/lib/polymorphic_embed/html/helpers.ex index bcd4b76..4cb577f 100644 --- a/lib/polymorphic_embed/html/helpers.ex +++ b/lib/polymorphic_embed/html/helpers.ex @@ -49,7 +49,7 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d form.source.data.__struct__ end - def to_form(%{action: parent_action} = source_changeset, form, field, options) do + def to_form(source_changeset, %{action: parent_action} = form, field, options) do id = to_string(form.id <> "_#{field}") name = to_string(form.name <> "[#{field}]") @@ -57,37 +57,16 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d struct = Ecto.Changeset.apply_changes(source_changeset) - 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) - end - - list_data + resolve_field_data(source_changeset, struct, form, field, options) |> Enum.with_index() - |> Enum.map(fn {data, i} -> - params = Enum.at(params, i) || %{} - - %Ecto.Changeset{} = - changeset = - data - |> Ecto.Changeset.change() - |> apply_action(parent_action) - - errors = get_errors(changeset) - - changeset = %Ecto.Changeset{ - changeset - | action: parent_action, - params: params, - errors: errors, - valid?: errors == [] - } + |> Enum.map(&prepare_changeset(&1, params, parent_action)) + |> Enum.map(fn prepared_data -> + %{ + changeset: changeset, + params: params, + errors: errors, + index: i + } = prepared_data %schema{} = source_changeset.data @@ -107,7 +86,7 @@ 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)}], @@ -116,9 +95,56 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d end) 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 resolve_field_data(source_changeset, struct, form, field, options) do + case Map.get(source_changeset.changes, field) do + nil -> + 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 + + data -> + List.wrap(data) + end + end + + defp prepare_changeset({%Ecto.Changeset{} = changeset, i}, _params, parent_action) do + params = changeset.params || %{} + changeset = apply_action(changeset, parent_action) + errors = get_errors(changeset) + + %{ + changeset: changeset, + params: params, + errors: errors, + index: i + } + end + + defp prepare_changeset({data, i}, params, parent_action) do + params = Enum.at(params, i) || %{} + + changeset = + data + |> Ecto.Changeset.change() + |> apply_action(parent_action) + + errors = get_errors(changeset) + + changeset = %{changeset | params: params} + + %{changeset: changeset, params: params, errors: errors, index: i} + end + + defp apply_action(changeset, action) when action in [nil, :ignore], + do: %{changeset | action: action} + + defp apply_action(%{action: nil} = changeset, action), do: %{changeset | action: action} defp apply_action(changeset, _action), do: changeset defp get_errors(%{action: nil}), do: [] diff --git a/test/polymorphic_embed/html/helpers_test.exs b/test/polymorphic_embed/html/helpers_test.exs new file mode 100644 index 0000000..a9b4e2d --- /dev/null +++ b/test/polymorphic_embed/html/helpers_test.exs @@ -0,0 +1,190 @@ +defmodule PolymorphicEmbed.HTML.HelpersTest do + use ExUnit.Case, async: true + + alias Ecto.Changeset + alias PolymorphicEmbed.Channel.Email + alias PolymorphicEmbed.HTML.Helpers + alias PolymorphicEmbed.Reminder + alias PolymorphicEmbed.Reminder.Context.{Age, Location} + + defmodule Group do + use Ecto.Schema + import PolymorphicEmbed + + embedded_schema do + polymorphic_embeds_many(:contexts, types: [age: Age], on_replace: :delete) + end + + def changeset(data, params) do + data + |> Changeset.cast(params, []) + |> cast_polymorphic_embed(:contexts, sort_param: :order) + end + end + + defmodule Container do + use Ecto.Schema + import PolymorphicEmbed + + embedded_schema do + polymorphic_embeds_one(:group, types: [group: Group], on_replace: :update) + end + end + + test "preserves sorted row parameters inside a valid single embed" do + first = %{"__type__" => "age", "age" => "same", "_persistent_id" => "first"} + second = %{"__type__" => "age", "age" => "same", "_persistent_id" => "second"} + + parent = + %Container{} + |> Changeset.cast( + %{ + "group" => %{ + "__type__" => "group", + "contexts" => %{"0" => first, "1" => second}, + "order" => ["1", "0"] + } + }, + [] + ) + |> PolymorphicEmbed.cast_polymorphic_embed(:group) + + [group] = forms(parent, :group) + rows = Helpers.to_form(group.source, group, :contexts, []) + assert Enum.map(rows, & &1.params) == [second, first] + end + + test "keeps row parameters through sorting, dropping and ignored types" do + first = %{"__type__" => "age", "age" => "same", "_persistent_id" => "first"} + second = %{"__type__" => "age", "age" => "same", "_persistent_id" => "second"} + invalid = %{"__type__" => "location", "address" => "", "_persistent_id" => "invalid"} + + params = %{ + "contexts2" => %{ + "0" => first, + "1" => second, + "2" => invalid, + "3" => %{"__type__" => "unknown"}, + "4" => %{"__type__" => "age", "age" => "dropped"} + }, + "order" => ["1", "2", "3", "4", "0"], + "remove" => ["4"] + } + + options = [sort_param: :order, drop_param: :remove] + + changeset = + %Reminder{} + |> Changeset.cast(params, []) + |> PolymorphicEmbed.cast_polymorphic_embed(:contexts2, options) + + forms = forms(changeset, :contexts2) + assert Enum.map(forms, & &1.params) == [second, invalid, first] + assert Enum.map(forms, & &1.data.__struct__) == [Age, Location, Age] + assert Enum.map(forms, & &1.source.valid?) == [true, false, true] + + recast = PolymorphicEmbed.cast_polymorphic_embed(changeset, :contexts2, options) + assert Enum.map(forms(recast, :contexts2), & &1.params) == [second, invalid, first] + end + + test "uses numeric row order without an explicit sort parameter" do + earlier = %{"__type__" => "age", "age" => "earlier", "_persistent_id" => "two"} + later = %{"__type__" => "age", "age" => "later", "_persistent_id" => "ten"} + + changeset = + %Reminder{} + |> Changeset.cast(%{"contexts" => %{"10" => later, "2" => earlier}}, []) + |> PolymorphicEmbed.cast_polymorphic_embed(:contexts) + + assert Enum.map(forms(changeset, :contexts), & &1.params) == [earlier, later] + end + + test "keeps parameters for a valid row created by sort without field parameters" do + options = [sort_param: :order, default_type_on_sort_create: :age] + + changeset = + %Reminder{} + |> Changeset.cast(%{"order" => ["new"]}, []) + |> PolymorphicEmbed.cast_polymorphic_embed(:contexts, options) + + assert [%{data: %Age{}, params: %{"__type__" => :age}}] = forms(changeset, :contexts) + + recast = PolymorphicEmbed.cast_polymorphic_embed(changeset, :contexts, options) + assert [%{data: %Age{}, params: %{"__type__" => :age}}] = forms(recast, :contexts) + end + + test "keeps list parameters aligned after an unknown type is ignored" do + row = %{"__type__" => "age", "age" => "kept", "_persistent_id" => "kept"} + + changeset = + %Reminder{} + |> Changeset.cast(%{"contexts2" => [%{"__type__" => "unknown"}, row]}, []) + |> PolymorphicEmbed.cast_polymorphic_embed(:contexts2) + + assert [%{params: ^row}] = forms(changeset, :contexts2) + end + + for {parent_action, child_action, effective_action, show_errors?} <- [ + {nil, :insert, nil, false}, + {:ignore, :insert, :ignore, false}, + {:insert, nil, :insert, true}, + {:validate, nil, :validate, true}, + {:validate, :update, :update, true}, + {:insert, :ignore, :ignore, false} + ] do + test "prepares child action #{inspect(child_action)} with parent action #{inspect(parent_action)}" do + child = + %Email{} + |> Email.changeset(%{"address" => "a"}) + |> Map.put(:action, unquote(child_action)) + + parent = + %Reminder{} + |> Changeset.change() + |> Changeset.put_change(:channel, child) + |> Map.put(:action, unquote(parent_action)) + + [form] = forms(parent, :channel) + assert form.action == unquote(parent_action) + assert form.source.action == unquote(effective_action) + assert form.source.errors == child.errors + refute form.source.valid? + assert form.errors == if(unquote(show_errors?), do: child.errors, else: []) + assert Phoenix.Component.to_form(form.source).errors == form.errors + end + end + + test "assigns the parent action when preparing a struct" do + for action <- [:insert, :validate] do + parent = + %Reminder{channel: %Email{address: "valid"}} + |> Changeset.change() + |> Map.put(:action, action) + + [form] = forms(parent, :channel) + assert form.source.action == action + assert form.errors == [] + end + end + + test "retains nested errors when another form uses the prepared source" do + child = Location.changeset(%Location{}, %{address: "valid", country: %{name: "A"}}) + + parent = + %Reminder{} + |> Changeset.change() + |> Changeset.put_change(:contexts, [child]) + |> Map.put(:action, :insert) + + [form] = forms(parent, :contexts) + rebuilt = Phoenix.Component.to_form(form.source) + + [country] = Phoenix.HTML.FormData.to_form(form.source, rebuilt, :country, []) + assert country.errors == child.changes.country.errors + assert country.errors != [] + end + + defp forms(changeset, field) do + Helpers.to_form(changeset, Phoenix.Component.to_form(changeset), field, []) + end +end diff --git a/test/polymorphic_embed_test.exs b/test/polymorphic_embed_test.exs index ba50279..7f24242 100644 --- a/test/polymorphic_embed_test.exs +++ b/test/polymorphic_embed_test.exs @@ -2770,25 +2770,58 @@ defmodule PolymorphicEmbedTest do for generator <- @generators do reminder_module = get_module(Reminder, generator) - attrs = %{ - date: ~U[2020-05-28 02:57:19Z], - text: "This is an Email reminder", - channel: %{ - address: "a", - valid: true, - confirmed: true - } - } + attrs = + if polymorphic?(generator) do + %{ + date: ~U[2020-05-28 02:57:19Z], + text: "This is an Email reminder", + channel: %{ + address: "a", + valid: true, + confirmed: true + } + } + else + %{ + number: ~U[2020-05-28 02:57:19Z], + text: "This is a non polymorphic reminder", + channel: %{ + number: "a" + } + } + end changeset = struct(reminder_module) |> reminder_module.changeset(attrs) + # Without parent action, errors should be empty in both cases + # (mirroring standard Ecto embed behavior) + safe_inputs_for(changeset, :channel, generator, fn f -> + assert f.impl == Phoenix.HTML.FormData.Ecto.Changeset + assert is_struct(f.data), "f.data should be a struct, not a changeset" + refute match?(%Ecto.Changeset{}, f.data), "f.data should not be a changeset" + assert f.errors == [] + assert f.action == nil + + if polymorphic?(generator), do: text_input(f, :address), else: text_input(f, :number) + end) + + changeset = Map.put(changeset, :action, :insert) + + # With parent action, validation errors should surface in both cases contents = safe_inputs_for(changeset, :channel, generator, fn f -> assert f.impl == Phoenix.HTML.FormData.Ecto.Changeset - assert f.errors == [] - text_input(f, :address) + assert is_struct(f.data), "f.data should be a struct, not a changeset" + refute match?(%Ecto.Changeset{}, f.data), "f.data should not be a changeset" + + assert f.errors != [], + "errors should be present when parent has action and data is invalid" + + assert f.action == :insert + + if polymorphic?(generator), do: text_input(f, :address), else: text_input(f, :number) end) expected_contents = @@ -2798,7 +2831,7 @@ defmodule PolymorphicEmbedTest do """, else: ~s""" - + """ ) @@ -2811,7 +2844,12 @@ defmodule PolymorphicEmbedTest do generator, fn f -> assert f.impl == Phoenix.HTML.FormData.Ecto.Changeset - text_input(f, :address) + + if polymorphic?(generator) do + text_input(f, :address) + else + text_input(f, :number) + end end ) @@ -2822,7 +2860,7 @@ defmodule PolymorphicEmbedTest do """, else: ~s""" - + """ ) @@ -2830,6 +2868,57 @@ defmodule PolymorphicEmbedTest do end end + test "polymorphic_embed_inputs_for ignores child errors when parent action is nil" do + reminder_module = get_module(Reminder, :polymorphic) + + attrs = %{ + date: ~U[2020-05-28 02:57:19Z], + text: "This is an Email reminder", + channel: %{ + address: "a", + valid: true, + confirmed: true + } + } + + changeset = + struct(reminder_module) + |> reminder_module.changeset(attrs) + + safe_inputs_for(changeset, :channel, :polymorphic, fn f -> + assert f.impl == Phoenix.HTML.FormData.Ecto.Changeset + assert f.errors == [] + text_input(f, :address) + end) + end + + test "polymorphic_embed_inputs_for/4 applies parent action to a prebuilt child changeset" do + child_changeset = + PolymorphicEmbed.Channel.Email.changeset( + %PolymorphicEmbed.Channel.Email{}, + %{address: "a", valid: true, confirmed: true} + ) + + changeset = + Ecto.Changeset.change(%PolymorphicEmbed.Reminder{}) + |> Ecto.Changeset.put_change(:channel, child_changeset) + |> Map.put(:action, :insert) + + safe_inputs_for(changeset, :channel, :polymorphic, fn f -> + assert f.impl == Phoenix.HTML.FormData.Ecto.Changeset + assert f.action == :insert + assert f.source.action == :insert + + assert f.errors == [ + {:address, + {"should be at least %{count} character(s)", + [count: 3, validation: :length, kind: :min, type: :string]}} + ] + + text_input(f, :address) + end) + end + test "polymorphic_embed_inputs_for/4 for list of embeds" do for generator <- @generators do reminder_module = get_module(Reminder, generator) @@ -2867,7 +2956,7 @@ defmodule PolymorphicEmbedTest do if(polymorphic?(generator), do: ~s""" - + """, @@ -2894,7 +2983,7 @@ defmodule PolymorphicEmbedTest do if(polymorphic?(generator), do: ~s""" - + """, @@ -2908,6 +2997,64 @@ defmodule PolymorphicEmbedTest do end end + test "to_form/4 keeps sorted embeds_many params aligned with each row" do + reminder_module = get_module(Reminder, :polymorphic) + + attrs = %{ + "date" => ~U[2020-05-28 02:57:19Z], + "text" => "This is a reminder with sorted contexts", + "channel" => %{ + "my_type_field" => "sms", + "number" => "02/807.05.53", + "country_code" => 1, + "provider" => %{ + "__type__" => "twilio", + "api_key" => "foo" + } + }, + "contexts" => %{ + "0" => %{ + "__type__" => "device", + "_persistent_id" => "device-row", + "ref" => "12345", + "type" => "cellphone" + }, + "1" => %{ + "__type__" => "age", + "_persistent_id" => "age-row", + "age" => "aquarius" + } + }, + "contexts_sort" => ["1", "0"] + } + + changeset = + struct(reminder_module) + |> reminder_module.changeset(attrs) + + form = Phoenix.Component.to_form(changeset) + forms = PolymorphicEmbed.HTML.Helpers.to_form(changeset, form, :contexts, []) + + assert Enum.map(forms, & &1.data.__struct__) == [ + PolymorphicEmbed.Reminder.Context.Age, + PolymorphicEmbed.Reminder.Context.Device + ] + + assert Enum.map(forms, & &1.params) == [ + %{ + "__type__" => "age", + "_persistent_id" => "age-row", + "age" => "aquarius" + }, + %{ + "__type__" => "device", + "_persistent_id" => "device-row", + "ref" => "12345", + "type" => "cellphone" + } + ] + end + test "polymorphic_embed_inputs_for/4 after invalid insert" do for generator <- @generators do reminder_module = get_module(Reminder, generator) From faed296ed558ab16c991a35f2885a62adc76463a Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 22 Oct 2026 13:47:34 +0200 Subject: [PATCH 03/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 | 22 ++++++++++++++++++++++ test/polymorphic_embed_test.exs | 23 ++++++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/lib/polymorphic_embed.ex b/lib/polymorphic_embed.ex index 388a80b..317bce9 100644 --- a/lib/polymorphic_embed.ex +++ b/lib/polymorphic_embed.ex @@ -166,6 +166,14 @@ 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, 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) @@ -395,6 +403,20 @@ defmodule PolymorphicEmbed do 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 7f24242..00efc95 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 e2273f8330dbd864d12303d105adc9e710154a79 Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 22 Oct 2026 13:47:55 +0200 Subject: [PATCH 04/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 317bce9..5baf2aa 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) @@ -174,10 +189,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? -> @@ -206,8 +218,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 9caecc3554161368bcec4dd831c152a121d64304 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 24 Oct 2026 12:01:34 +0200 Subject: [PATCH 05/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. With nilify_unlisted_types_on_load or retain_unlisted_types_on_load, a loaded list can hold nil or plain maps. These entries match no params. A new test checks such a list. --- lib/polymorphic_embed.ex | 29 ++++++++++++++++++++++------- test/polymorphic_embed_test.exs | 20 ++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/lib/polymorphic_embed.ex b/lib/polymorphic_embed.ex index 5baf2aa..2474e95 100644 --- a/lib/polymorphic_embed.ex +++ b/lib/polymorphic_embed.ex @@ -430,6 +430,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 @@ -458,14 +474,13 @@ defmodule PolymorphicEmbed do :ignore module -> + # 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. 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, + &(is_struct(&1, module) and same_pk?(&1, params)) + ) embed_changeset = if data_for_field do diff --git a/test/polymorphic_embed_test.exs b/test/polymorphic_embed_test.exs index 00efc95..8109d29 100644 --- a/test/polymorphic_embed_test.exs +++ b/test/polymorphic_embed_test.exs @@ -2234,6 +2234,26 @@ defmodule PolymorphicEmbedTest do end end + test "matches rows of a list that holds entries from the load options" do + reminder = %PolymorphicEmbed.Reminder{ + contexts: [ + %{"__type__" => "old"}, + nil, + %PolymorphicEmbed.Reminder.Context.Age{id: "a1", age: "old"} + ] + } + + params = %{"contexts" => [%{"__type__" => "age", "id" => "a1", "age" => "new"}]} + + changeset = + reminder + |> Ecto.Changeset.cast(params, []) + |> PolymorphicEmbed.cast_polymorphic_embed(:contexts) + + assert [%PolymorphicEmbed.Reminder.Context.Age{id: "a1", age: "new"}] = + Ecto.Changeset.get_field(changeset, :contexts) + end + test "list of embeds defaults to []" do for generator <- @generators do reminder_module = get_module(Reminder, generator) From 7f589907e3b9317cd1e4640e1d776d6e6c5b0a40 Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 22 Oct 2026 13:44:07 +0200 Subject: [PATCH 06/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 2474e95..50b20f7 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 = @@ -717,11 +716,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 f4582f6dcd28d8a5d2b79b0d50d0240fa4627595 Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 22 Oct 2026 13:52:01 +0200 Subject: [PATCH 07/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 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/polymorphic_embed.ex b/lib/polymorphic_embed.ex index 50b20f7..f730ca2 100644 --- a/lib/polymorphic_embed.ex +++ b/lib/polymorphic_embed.ex @@ -600,6 +600,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 @@ -852,8 +860,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 From e5ad17c4ec219bd26c476bd582c8a59477a3410f Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 24 Oct 2026 11:54:44 +0200 Subject: [PATCH 08/11] Put the primary key of existing rows into the hidden inputs phoenix_ecto puts the primary key of the data into form.hidden, so that the form sends the id back. The polymorphic form helper put only the type field there. Without the id, the cast could not match the row of the data and built a new row. Now a child form gets the primary key when the parent data holds the row. A new row gets no id in an Ecto form, so its form gets none, also when the cast generated one. All form parity tests pass. --- lib/polymorphic_embed/html/helpers.ex | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/polymorphic_embed/html/helpers.ex b/lib/polymorphic_embed/html/helpers.ex index 4cb577f..a3a4047 100644 --- a/lib/polymorphic_embed/html/helpers.ex +++ b/lib/polymorphic_embed/html/helpers.ex @@ -89,12 +89,34 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d data: changeset.data, action: parent_action, params: params, - hidden: [{type_field_name, to_string(type)}], + hidden: + primary_key_hidden(changeset.data, Map.get(source_changeset.data, field)) ++ + [{type_field_name, to_string(type)}], options: options } end) end + # As in phoenix_ecto, the hidden inputs hold the primary key of a row that + # the parent data holds, so that the form sends the id back and the cast + # can match the row. A new row has no id in an Ecto form, so its form + # gets none, also when the cast generated one. + defp primary_key_hidden(%module{} = data, parent_data) do + pks = Enum.map(module.__schema__(:primary_key), &{&1, Map.get(data, &1)}) + + existing? = + parent_data + |> List.wrap() + |> Enum.any?(fn row -> + is_struct(row, module) and + Enum.all?(pks, fn {key, value} -> Map.get(row, key) == value end) + end) + + if existing?, do: Enum.reject(pks, &is_nil(elem(&1, 1))), else: [] + end + + defp primary_key_hidden(_data, _parent_data), do: [] + defp resolve_field_data(source_changeset, struct, form, field, options) do case Map.get(source_changeset.changes, field) do nil -> From a8048a801e7dc340cd102f400ae768f769694eca Mon Sep 17 00:00:00 2001 From: Nick Date: Sun, 25 Oct 2026 10:55:38 +0100 Subject: [PATCH 09/11] Test the type of a nested embed with a parent type field With use_parent_field_for_type, the params of the nested embed have no type key. get_polymorphic_type/2 reads form[field].value, which is the raw param map, so it returns nil. The test checks a single embed, a list of rows and an index map of rows, as an HTML form sends them. The test fails. The next commit fixes it. --- test/polymorphic_embed/html/helpers_test.exs | 47 ++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/polymorphic_embed/html/helpers_test.exs b/test/polymorphic_embed/html/helpers_test.exs index a9b4e2d..4425026 100644 --- a/test/polymorphic_embed/html/helpers_test.exs +++ b/test/polymorphic_embed/html/helpers_test.exs @@ -31,6 +31,37 @@ defmodule PolymorphicEmbed.HTML.HelpersTest do end end + defmodule Notice do + use Ecto.Schema + import PolymorphicEmbed + + embedded_schema do + field(:type, :string) + + polymorphic_embeds_one(:channel, + types: [sms: PolymorphicEmbed.Channel.SMS, email: Email], + on_replace: :update, + use_parent_field_for_type: :type + ) + end + + def changeset(data, params) do + data + |> Changeset.cast(params, [:type]) + |> cast_polymorphic_embed(:channel) + end + end + + defmodule Board do + use Ecto.Schema + import PolymorphicEmbed + + embedded_schema do + polymorphic_embeds_one(:notice, types: [notice: Notice], on_replace: :update) + polymorphic_embeds_many(:notices, types: [notice: Notice], on_replace: :delete) + end + end + test "preserves sorted row parameters inside a valid single embed" do first = %{"__type__" => "age", "age" => "same", "_persistent_id" => "first"} second = %{"__type__" => "age", "age" => "same", "_persistent_id" => "second"} @@ -184,6 +215,22 @@ defmodule PolymorphicEmbed.HTML.HelpersTest do assert country.errors != [] end + # The parent field gives the type, so the params of the nested embed have no type key. + test "infers the type of a nested embed from the cast data" do + row = %{"__type__" => "notice", "type" => "email", "channel" => %{"address" => "valid"}} + + for {field, field_params} <- [notice: row, notices: [row], notices: %{"0" => row}] do + parent = + %Board{} + |> Changeset.cast(%{to_string(field) => field_params}, []) + |> PolymorphicEmbed.cast_polymorphic_embed(field) + |> Map.put(:action, :validate) + + [form] = forms(parent, field) + assert Helpers.get_polymorphic_type(form, :channel) == :email, inspect(field_params) + end + end + defp forms(changeset, field) do Helpers.to_form(changeset, Phoenix.Component.to_form(changeset), field, []) end From 12d065d6e09a42ab8bfd80cccd1e01a0cc5f904f Mon Sep 17 00:00:00 2001 From: Nick Date: Sun, 25 Oct 2026 10:55:55 +0100 Subject: [PATCH 10/11] Read the polymorphic type from the cast data first get_polymorphic_type/2 read form[field].value: the change, else the param, else the data. Each child form now holds the params of its own row. So for a nested embed the value is the raw param map. With use_parent_field_for_type, that map has no type key, and the function returned nil. Now the function reads Ecto.Changeset.get_field/2 first, and uses the input value only when the field has no data. After a cast, the cast data gives the type also when the params have no type key. --- lib/polymorphic_embed/html/helpers.ex | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/polymorphic_embed/html/helpers.ex b/lib/polymorphic_embed/html/helpers.ex index a3a4047..38cb624 100644 --- a/lib/polymorphic_embed/html/helpers.ex +++ b/lib/polymorphic_embed/html/helpers.ex @@ -6,7 +6,12 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d def get_polymorphic_type(%Phoenix.HTML.Form{} = form, field) do %schema{} = form.source.data - case form[field] && form[field].value do + # The cast data gives the type also when the params have no type key, + # as with use_parent_field_for_type. The input value gives the type + # when the field has no data yet. + value = Ecto.Changeset.get_field(form.source, field) || (form[field] && form[field].value) + + case value do %Ecto.Changeset{data: value} -> PolymorphicEmbed.get_polymorphic_type(schema, field, value) From 383465edca88e3d964ce03dc34f4965f6aefa9b2 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 24 Oct 2026 11:55:20 +0200 Subject: [PATCH 11/11] Add the changes of this branch to the changelog The changelog lists the breaking changes and the fixes of the cast and of the form helpers. It also says how to read the data of a nested embed in a child form: with Ecto.Changeset.get_field/2, because form[:field].value gives the value for an input. --- CHANGELOG.md | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf62aa2..47b5993 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,44 @@ # Changelog +## Unreleased + + **Breaking changes**: + + * `nil` for a list of embeds gives the error "is invalid", as in Ecto. Before, the cast + ignored it. + * With `required: true`, `changeset.required` contains the field, and an empty list in + the data gives "can't be blank", as in Ecto. + * An invalid `:on_replace` option raises `ArgumentError`, not `RuntimeError`. + * After the cast, `changeset.params` holds the params of each child for the field. For a + list of embeds, it is a list in the order of the cast, without dropped and ignored rows. + * The form of an invalid child takes its params from the child changeset, as in + phoenix_ecto. Before, it took the params of the parent by index. When a `with:` function + builds the changeset without `cast/4`, the changeset has no params, and the form gets + `%{}`. Use `cast/4` in a `with:` function, so that the form keeps the params of the row. + * The hidden inputs of a child form hold the primary key of a row that the data holds, + as in phoenix_ecto, so the form sends the id back. + + **Fixes**: + + * Params that are not maps give the error "is invalid", as in Ecto. Before, the cast + crashed. + * A list of embeds matches the rows of the data by primary key, as in Ecto. Before, params + with string keys matched no row, so each row got a new id and lost the data that the + params did not set. + * An embedded schema with an autogenerated integer id no longer crashes the cast. + `dump/3` raises the same `ArgumentError` as Ecto when the id is still `nil` on insert. + * Each child form of a list gets the params of its own row. Before, the first form got + the params of all rows. + * A child form takes the action of the parent form, so it shows the errors of an invalid + child, and `form.data` is the data of the child, not a changeset. + * `get_polymorphic_type/2` of the HTML helpers reads the type from the cast data first, + then from the input value. Before, it returned `nil` for a nested embed when the params + had no type key, for example with `:use_parent_field_for_type`. + + `form[:field].value` gives the value for an input: the change, else the param, else the + data. For a nested embed of a child form, read the data with + `Ecto.Changeset.get_field(form.source, field)`. + ## 5.0.2 * Remove warning when aliases cannot be expanded (#123).