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). 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..f730ca2 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 = @@ -125,7 +124,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 +135,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 +172,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) @@ -166,10 +180,15 @@ defmodule PolymorphicEmbed do changeset end - {:ok, nil} when required -> - Ecto.Changeset.add_error(changeset, field, "can't be blank", validation: :required) + {: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 not required -> + {: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} -> Ecto.Changeset.put_change(changeset, field, nil) {:ok, map} when map == %{} and not array? -> @@ -198,8 +217,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 @@ -319,6 +346,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 -> @@ -394,6 +422,36 @@ 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 + + # 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 + 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, @@ -415,14 +473,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 @@ -431,14 +488,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 +506,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 +527,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: @@ -533,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 @@ -649,11 +724,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 @@ -783,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 diff --git a/lib/polymorphic_embed/html/helpers.ex b/lib/polymorphic_embed/html/helpers.ex index bcd4b76..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) @@ -49,7 +54,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 +62,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,18 +91,87 @@ 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)}], + hidden: + primary_key_hidden(changeset.data, Map.get(source_changeset.data, field)) ++ + [{type_field_name, to_string(type)}], options: options } 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} + # 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 -> + 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/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 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 diff --git a/test/polymorphic_embed/html/helpers_test.exs b/test/polymorphic_embed/html/helpers_test.exs new file mode 100644 index 0000000..4425026 --- /dev/null +++ b/test/polymorphic_embed/html/helpers_test.exs @@ -0,0 +1,237 @@ +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 + + 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"} + + 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 + + # 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 +end diff --git a/test/polymorphic_embed_test.exs b/test/polymorphic_embed_test.exs index ba50279..8109d29 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,48 @@ 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 "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) @@ -2770,25 +2811,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 +2872,7 @@ defmodule PolymorphicEmbedTest do """, else: ~s""" - + """ ) @@ -2811,7 +2885,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 +2901,7 @@ defmodule PolymorphicEmbedTest do """, else: ~s""" - + """ ) @@ -2830,6 +2909,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 +2997,7 @@ defmodule PolymorphicEmbedTest do if(polymorphic?(generator), do: ~s""" - + """, @@ -2894,7 +3024,7 @@ defmodule PolymorphicEmbedTest do if(polymorphic?(generator), do: ~s""" - + """, @@ -2908,6 +3038,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)