From 15d0652b095e9f1a5eeb92737fa1243c7b5abdee Mon Sep 17 00:00:00 2001 From: Nick Date: Wed, 13 Aug 2025 11:55:19 +0200 Subject: [PATCH 1/3] fix param handling and pass action from form not changeset --- lib/polymorphic_embed/html/helpers.ex | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/polymorphic_embed/html/helpers.ex b/lib/polymorphic_embed/html/helpers.ex index 7822d46..59ebc03 100644 --- a/lib/polymorphic_embed/html/helpers.ex +++ b/lib/polymorphic_embed/html/helpers.ex @@ -49,11 +49,11 @@ 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}]") - params = Map.get(source_changeset.params || %{}, to_string(field), %{}) |> List.wrap() + params = Map.get(source_changeset.params || %{}, to_string(field), %{}) struct = Ecto.Changeset.apply_changes(source_changeset) @@ -71,7 +71,17 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d list_data |> Enum.with_index() |> Enum.map(fn {data, i} -> - params = Enum.at(params, i) || %{} + params = + case params do + nil -> + %{} + + params when is_list(params) -> + Enum.at(params, i) || %{} + + params when is_map(params) -> + Map.get(params, to_string(i), %{}) + end changeset = data From 5db99e31998accbe6089f3c4c21ec13fc33de2b6 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 16 Aug 2025 09:57:01 +0200 Subject: [PATCH 2/3] add tests --- test/polymorphic_embed_test.exs | 91 +++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/test/polymorphic_embed_test.exs b/test/polymorphic_embed_test.exs index 7dced00..6199203 100644 --- a/test/polymorphic_embed_test.exs +++ b/test/polymorphic_embed_test.exs @@ -3204,6 +3204,97 @@ defmodule PolymorphicEmbedTest do end) end + test "form with improved param handling for different param types" do + reminder_module = get_module(Reminder, :polymorphic) + + # Test with nil params - when contexts is empty, safe_inputs_for returns empty string + changeset_nil_params = + struct(reminder_module) + |> reminder_module.changeset(%{text: "Test reminder", contexts: []}) + |> Map.put(:params, %{"contexts" => nil}) + |> Map.put(:action, :insert) + + safe_form_for(changeset_nil_params, fn _f -> + safe_inputs_for(changeset_nil_params, :contexts, :polymorphic, fn f -> + assert f.impl == Phoenix.HTML.FormData.Ecto.Changeset + assert f.errors == [] + assert f.params == %{} + + 1 + end) + + 1 + end) + + # Test with list params - need to add some contexts to the data + changeset_list_params = + struct(reminder_module) + |> reminder_module.changeset(%{ + text: "Test reminder", + contexts: [ + %{__type__: "device", ref: "123", type: "cellphone"}, + %{__type__: "location", address: "456 Main St"} + ] + }) + |> Map.put(:params, %{"contexts" => [%{"ref" => "123"}, %{"address" => "456 Main St"}]}) + |> Map.put(:action, :insert) + + safe_form_for(changeset_list_params, fn _f -> + safe_inputs_for(changeset_list_params, :contexts, :polymorphic, fn f -> + assert f.impl == Phoenix.HTML.FormData.Ecto.Changeset + assert f.errors == [] + # First context should have params from index 0 + if f.index == 0 do + assert f.params == %{"ref" => "123"} + end + + # Second context should have params from index 1 + if f.index == 1 do + assert f.params == %{"address" => "456 Main St"} + end + + 1 + end) + + 1 + end) + + # Test with map params - need to add some contexts to the data + changeset_map_params = + struct(reminder_module) + |> reminder_module.changeset(%{ + text: "Test reminder", + contexts: [ + %{__type__: "device", ref: "789", type: "cellphone"}, + %{__type__: "location", address: "012 Oak Ave"} + ] + }) + |> Map.put(:params, %{ + "contexts" => %{"0" => %{"ref" => "789"}, "1" => %{"address" => "012 Oak Ave"}} + }) + |> Map.put(:action, :insert) + + safe_form_for(changeset_map_params, fn _f -> + safe_inputs_for(changeset_map_params, :contexts, :polymorphic, fn f -> + assert f.impl == Phoenix.HTML.FormData.Ecto.Changeset + assert f.errors == [] + # First context should have params from key "0" + if f.index == 0 do + assert f.params == %{"ref" => "789"} + end + + # Second context should have params from key "1" + if f.index == 1 do + assert f.params == %{"address" => "012 Oak Ave"} + end + + 1 + end) + + 1 + end) + end + describe "get_polymorphic_type/3" do test "returns the type for a module" do assert PolymorphicEmbed.get_polymorphic_type( From ed3e4fa59619435daee4eaea7556e5b90acc7b18 Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 21 Aug 2025 10:38:57 +0200 Subject: [PATCH 3/3] go for source_changeset.changes first, fallback to struct field --- lib/polymorphic_embed/html/helpers.ex | 83 ++++++++++++++++----------- test/polymorphic_embed_test.exs | 55 +++++++++++++----- 2 files changed, 90 insertions(+), 48 deletions(-) diff --git a/lib/polymorphic_embed/html/helpers.ex b/lib/polymorphic_embed/html/helpers.ex index 59ebc03..285f59f 100644 --- a/lib/polymorphic_embed/html/helpers.ex +++ b/lib/polymorphic_embed/html/helpers.ex @@ -53,50 +53,64 @@ if Code.ensure_loaded?(Phoenix.HTML) && Code.ensure_loaded?(Phoenix.HTML.Form) d id = to_string(form.id <> "_#{field}") name = to_string(form.name <> "[#{field}]") - params = Map.get(source_changeset.params || %{}, to_string(field), %{}) + params = Map.get(source_changeset.params || %{}, to_string(field), %{}) |> List.wrap() 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: [] + Map.get(source_changeset.changes, field) + |> case 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 - list_data + data when is_list(data) -> + data + + data -> + List.wrap(data) + end |> Enum.with_index() - |> Enum.map(fn {data, i} -> - params = - case params do - nil -> - %{} + |> Enum.map(fn + {%Ecto.Changeset{} = changeset, i} -> + params = changeset.params || %{} + errors = get_errors(changeset) - params when is_list(params) -> - Enum.at(params, i) || %{} + %{changeset: changeset, params: params, errors: errors, index: i} - params when is_map(params) -> - Map.get(params, to_string(i), %{}) - end + {data, i} -> + params = Enum.at(params, i) || %{} - changeset = - data - |> Ecto.Changeset.change() - |> apply_action(parent_action) + changeset = + data + |> Ecto.Changeset.change() + |> apply_action(parent_action) - errors = get_errors(changeset) + errors = get_errors(changeset) - changeset = %Ecto.Changeset{ - changeset - | action: parent_action, - params: params, - errors: errors, - valid?: errors == [] - } + changeset = %{ + changeset + | action: parent_action, + params: params, + errors: errors, + valid?: errors == [] + } + + %{changeset: changeset, params: params, errors: errors, index: i} + end) + |> Enum.map(fn prepared_data -> + %{ + changeset: changeset, + params: params, + errors: errors, + index: i + } = prepared_data %schema{} = source_changeset.data @@ -116,7 +130,8 @@ 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)}], options: options diff --git a/test/polymorphic_embed_test.exs b/test/polymorphic_embed_test.exs index 6199203..01d3d44 100644 --- a/test/polymorphic_embed_test.exs +++ b/test/polymorphic_embed_test.exs @@ -2704,15 +2704,26 @@ 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) @@ -2721,8 +2732,19 @@ defmodule PolymorphicEmbedTest do contents = safe_inputs_for(changeset, :channel, generator, fn f -> assert f.impl == Phoenix.HTML.FormData.Ecto.Changeset - assert f.errors == [] - text_input(f, :address) + + if polymorphic?(generator) do + assert f.errors == [ + {:address, + {"should be at least %{count} character(s)", + [count: 3, validation: :length, kind: :min, type: :string]}} + ] + + text_input(f, :address) + else + assert f.errors == [] + text_input(f, :number) + end end) expected_contents = @@ -2732,7 +2754,7 @@ defmodule PolymorphicEmbedTest do """, else: ~s""" - + """ ) @@ -2745,7 +2767,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 ) @@ -2756,7 +2783,7 @@ defmodule PolymorphicEmbedTest do """, else: ~s""" - + """ )