Cannot remove the only element in nested embeds when using `inputs_for`

[mewa]

Environment

• Elixir version (elixir -v): 1.16.3
• Phoenix version (mix deps): 1.7.11
• Phoenix LiveView version (mix deps): 0.20.11
• Operating system: Linux
• Browsers you attempted to reproduce this bug on (the more the merrier): Chromium, Brave

Actual behavior

When using inputs_for with a nested embed, the removal of the last item of a nested embed is not persisted (no changes are detected, no database updates are triggered). It's worth noting, it does work in the UI. Also, changes are persisted when there's at least 1 (nested) embedded item.

I.e., using schema from the example below, removing Subdivision from the form equivalent of

%Rule{countries: [%Country{subdivisions: [%Subdivision{}, %Subdivision{}]}

will persist, while removing from

%Rule{countries: [%Country{subdivisions: [%Subdivision{}]}

will not.

I've managed to narrow it down to the parameters handed to phx-change and phx-submit callbacks – i.e. phx-change receives proper params for Ecto to detect an item is being removed, while phx-submit does not – resulting in a no-op changeset being generated.

Example:

defmodule Rule do
  use Ecto.Schema

  @primary_key {:id, Ecto.UUID, autogenerate: true}
  schema "rules" do
    field :name, :string

    embeds_many :countries, __MODULE__.Country, on_replace: :delete

    timestamps()
  end

  defmodule Country do
    use Ecto.Schema

    @primary_key {:code, :string, autogenerate: false}
    embedded_schema do
      embeds_many :subdivisions, Subdivision, primary_key: {:code, :string, autogenerate: false}, on_replace: :delete do
      end
    end
  end

When removing the only Subdivision in a Country the following params passed to phx-change callback:

%{
  "countries" => %{
    "0" => %{
      "_persistent_id" => "0",
      "code" => "PL",
      "subdivisions" => %{
        "0" => %{"_persistent_id" => "0", "code" => "pl02"}
      },
      "subdivisions_drop" => ["0"],
      "subdivisions_sort" => ["0"]
    }
  },
  "countries_drop" => [""],
  "countries_sort" => ["0"],
  "id" => "754e3c75-1a54-4c21-9e77-884f89b03b69",
  "name" => "Rule 1"
}

The UI successfully reflects the change by removing the Subdivision.

But when submitting the form, phx-submit callback receives :point_down:, losing the change as a result:

%{
  "countries" => %{"0" => %{"_persistent_id" => "0", "code" => "PL"}},
  "countries_drop" => [""],
  "countries_sort" => ["0"],
  "id" => "754e3c75-1a54-4c21-9e77-884f89b03b69",
  "name" => "Rule 1"
}

Expected behavior

Correct params are passed to phx-submit callback, resulting in an Ecto.Changeset that causes the DB to update.

[SteffenDE]

Edit: see answer at the bottom :)

So this is indeed tricky. You can make it work like this:

1. Check if the embeds are empty

defp is_empty?([]), do: true

defp is_empty?(values) do
  Enum.all?(values, &match?(%Ecto.Changeset{action: :replace}, &1))
end

Outside the inputs_for:

        <%!--
          empty country to ensure we are able to delete the last one,
          as no country parameter would be sent otherwise
        --%>
        <input
          :if={is_empty?(@form[:countries].value)}
          type="hidden"
          name={@form[:countries].name <> "[]"}
        />

(same for subdivisions)

2. Filtering [""] before casting the changesets:

    def changeset(country, params) do
      params = filter_empty(params)

      country
      |> cast(params, [:code])
      |> cast_embed(:subdivisions,
        with: &subdivision_changeset/2,
        sort_param: :subdivisions_sort,
        drop_param: :subdivisions_drop
      )
    end

    def subdivision_changeset(subdivision, params) do
      subdivision
      |> cast(params, [:code])
    end

    defp filter_empty(%{"subdivisions" => [""]} = params) do
      Map.put(params, "subdivisions", %{})
    end

    defp filter_empty(params), do: params

Full example:

# removed because it's unnecessary

@josevalim I think we should make this easier. I needed ~1h to come up with this solution and it's really not intuitive. Maybe you've got a better solution already?

I see two main connected problems:

1. inputs_for does not render any inputs in case there are no embeds, therefore no parameters are sent at all for the embed
2. we cannot encode an empty map as form parameters, therefore we cannot simply render a hidden input in that case

[josevalim]

@SteffenDE I didn't check yet but it should work by adding a

    <input
      type="hidden"
      name={@form[:countries_drop] <> "[]"}
    />

For both of them. This makes sure that one parameter (drop) is sent and fields are correctly discarded. I believe this is mentioned in our docs too. It does not work as expected?

[josevalim]

If it works, perhaps we should be clearer in our docs about such requirement.

[SteffenDE]

Well indeed that's it! And this happens when you don't read the docs because you already implemented this multiple times and think copy paste will do 😅

I think the documentation is quite clear:

Outside the inputs_for, we render an empty mailing_list[emails_drop][] input, to ensure that all children are deleted when saving a form where the user dropped all entries. This hidden input is required whenever dropping associations.

Sorry for bothering!

@mewa The correct code looks like this and works as expected:

Application.put_env(:sample, Example.Endpoint,
  http: [ip: {127, 0, 0, 1}, port: 5001],
  server: true,
  live_view: [signing_salt: "aaaaaaaa"],
  secret_key_base: String.duplicate("a", 64)
)

Mix.install([
  {:plug_cowboy, "~> 2.5"},
  {:jason, "~> 1.0"},
  {:phoenix, "~> 1.7"},
  # please test your issue using the latest version of LV from GitHub!
  {:phoenix_live_view,
   github: "phoenixframework/phoenix_live_view", branch: "main", override: true},
  # {:phoenix_live_view, path: "/Users/steffen/oss/phoenix_live_view", override: true},
  {:phoenix_ecto, "~> 4.5"},
  {:ecto, "~> 3.11"}
])

# build the LiveView JavaScript assets (this needs mix and npm available in your path!)
path = Phoenix.LiveView.__info__(:compile)[:source] |> Path.dirname() |> Path.join("../")
System.cmd("mix", ["deps.get"], cd: path, into: IO.binstream())
System.cmd("npm", ["install"], cd: Path.join(path, "./assets"), into: IO.binstream())
System.cmd("mix", ["assets.build"], cd: path, into: IO.binstream())

defmodule Rule do
  use Ecto.Schema

  import Ecto.Changeset

  @primary_key {:id, Ecto.UUID, autogenerate: true}
  schema "rules" do
    field(:name, :string)

    embeds_many(:countries, __MODULE__.Country, on_replace: :delete)

    timestamps()
  end

  defmodule Country do
    use Ecto.Schema
    import Ecto.Changeset

    @primary_key {:code, :string, autogenerate: false}
    embedded_schema do
      embeds_many :subdivisions, Subdivision,
        primary_key: {:code, :string, autogenerate: false},
        on_replace: :delete do
      end
    end

    def changeset(country, params) do
      country
      |> cast(params, [:code])
      |> cast_embed(:subdivisions,
        with: &subdivision_changeset/2,
        sort_param: :subdivisions_sort,
        drop_param: :subdivisions_drop
      )
    end

    def subdivision_changeset(subdivision, params) do
      subdivision
      |> cast(params, [:code])
    end
  end

  def changeset(rule, params) do
    rule
    |> cast(params, [:name])
    |> cast_embed(:countries,
      with: &Country.changeset/2,
      sort_param: :countries_sort,
      drop_param: :countries_drop
    )
  end
end

defmodule Example.ErrorView do
  def render(template, _), do: Phoenix.Controller.status_message_from_template(template)
end

defmodule Example.HomeLive do
  use Phoenix.LiveView, layout: {__MODULE__, :live}

  alias Phoenix.LiveView.JS

  def mount(_params, _session, socket) do
    socket =
      assign(socket, :rule, %Rule{
        name: "My Rule",
        countries: [
          %Rule.Country{
            code: "US",
            subdivisions: [
              %Rule.Country.Subdivision{code: "FL"},
              %Rule.Country.Subdivision{code: "CA"}
            ]
          }
        ]
      })

    changeset =
      Rule.changeset(
        socket.assigns.rule,
        %{}
      )

    {:ok, assign(socket, :form, to_form(changeset))}
  end

  def render("live.html", assigns) do
    ~H"""
    <script src="/assets/phoenix/phoenix.js">
    </script>
    <script src="/assets/phoenix_live_view/phoenix_live_view.js">
    </script>
    <%!-- uncomment to use enable tailwind --%>
    <script src="https://cdn.tailwindcss.com">
    </script>
    <script>
      let liveSocket = new window.LiveView.LiveSocket("/live", window.Phoenix.Socket)
      liveSocket.connect()
    </script>
    <style>
      * { font-size: 1.1rem; }
    </style>
    <%= @inner_content %>
    """
  end

  def handle_event("validate", %{"rule" => params}, socket) do
    changeset = Rule.changeset(socket.assigns.rule, params)

    dbg(changeset)

    {:noreply, assign(socket, :form, to_form(changeset))}
  end

  def handle_event("save", %{"rule" => params}, socket) do
    changeset = Rule.changeset(socket.assigns.rule, params)

    # this is the data ecto would use when calling Repo.insert/update
    submitted = Ecto.Changeset.apply_action!(changeset, :insert)
    dbg(submitted)

    {:noreply, assign(socket, form: to_form(changeset), submitted: submitted)}
  end

  def render(assigns) do
    ~H"""
    <div class="flex flex-col gap-4 p-16">
      <.form for={@form} phx-change="validate" phx-submit="save" class="flex flex-col gap-4">
        <input
          type="text"
          id={@form[:name].id}
          name={@form[:name].name}
          value={@form[:name].value}
          placeholder="Rule Name"
        />

        <.inputs_for :let={country_form} field={@form[:countries]}>
          <div class="border border-zinc-300">
            <input
              type="hidden"
              name={@form[:countries_sort].name <> "[]"}
              value={country_form.index}
            />
            <input
              type="text"
              id={country_form[:code].id}
              name={country_form[:code].name}
              value={country_form[:code].value}
              placeholder="Country Code"
            />

            <button
              type="button"
              name={@form[:countries_drop].name <> "[]"}
              value={country_form.index}
              phx-click={JS.dispatch("change")}
            >
              Remove country
            </button>

            <.inputs_for :let={subdivision_form} field={country_form[:subdivisions]}>
              <div class="pl-8 border border-zinc-200">
                <input
                  type="hidden"
                  name={country_form[:subdivisions_sort].name <> "[]"}
                  value={subdivision_form.index}
                />
                <input
                  type="text"
                  id={subdivision_form[:code].id}
                  name={subdivision_form[:code].name}
                  value={subdivision_form[:code].value}
                  placeholder="Subdivision Code"
                />

                <button
                  type="button"
                  name={country_form[:subdivisions_drop].name <> "[]"}
                  value={subdivision_form.index}
                  phx-click={JS.dispatch("change")}
                >
                  Remove subdivision
                </button>
              </div>
            </.inputs_for>

            <input type="hidden" name={country_form[:subdivisions_drop].name <> "[]"} />

            <button
              type="button"
              name={country_form[:subdivisions_sort].name <> "[]"}
              value="new"
              phx-click={JS.dispatch("change")}
            >
              Add another subdivision
            </button>
          </div>
        </.inputs_for>

        <input type="hidden" name={@form[:countries_drop].name <> "[]"} />

        <button
          type="button"
          name={@form[:countries_sort].name <> "[]"}
          value="new"
          phx-click={JS.dispatch("change")}
        >
          Add another country
        </button>

        <button type="submit">Save</button>
      </.form>

      <%= if assigns[:submitted] do %>
        <pre class="text-xs p-4 bg-zinc-100"><code><%= inspect(@submitted, pretty: true) %></code></pre>
      <% end %>
    </div>
    """
  end
end

defmodule Example.Router do
  use Phoenix.Router
  import Phoenix.LiveView.Router

  pipeline :browser do
    plug(:accepts, ["html"])
  end

  scope "/", Example do
    pipe_through(:browser)

    live("/", HomeLive, :index)
  end
end

defmodule Example.Endpoint do
  use Phoenix.Endpoint, otp_app: :sample
  socket("/live", Phoenix.LiveView.Socket)

  plug(Plug.Static, from: {:phoenix, "priv/static"}, at: "/assets/phoenix")
  plug(Plug.Static, from: {:phoenix_live_view, "priv/static"}, at: "/assets/phoenix_live_view")

  plug(Example.Router)
end

{:ok, _} = Supervisor.start_link([Example.Endpoint], strategy: :one_for_one)
Process.sleep(:infinity)

[mewa]

Thank you guys for very quick and detailed responses!

Oh my, that's a bit embarrassing :sweat_smile:

It's part of a more complex form with several inputs_for and I added the hidden drop fields for all of them, but this nested one had it placed within its own inputs_for tag, which caused it not to render when the form was being submitted.

Looking at your example @SteffenDE and comparing it with my code with a fresh mind made me realize this rookie mistake...

Once again, thank you for your help (and sorry for wasting your time here :sweat_smile:). The Elixir community really is one of a kind!

[josevalim]

Looking at your example @SteffenDE and comparing it with my code with a fresh mind made me realize this rookie mistake...

Nothing rookie about it. It had already bit Chris and me in the past, and now we welcome @SteffenDE into the pantheon as well. :D