Problem with nested .inputs_for

Hello,
I have a problem (and potential bug) with nested form.
I have a form for a “product” witch has many “languages” witch has many “attributes”.


I want to be able to add some “attributes”.

If I have a create a new product with this changeset :

      Product.changeset(
        %Product{
          languages: [
            %Language{name: "FR", attributes: []},
            %Language{name: "EN", attributes: []}
          ]
        },
        %{}
      )

Everything seems to work fine.

But if I want to edit a product from the following seed

Repo.insert!(%Product{
  name: "a product",
  languages: [
      %Language{name: "French",
      attributes: [
        %Attribute{label: "Couleur",
        value: "Noir"}
      ]
    },
    %Language{name: "English",
    attributes: [
      %Attribute{label: "Color",
      value: "Black"}
    ]
  }]
})

the button add attribute doesn’t work any more.
The line Ecto.Changeset.put_assoc(changeset, :languages, udpated_languages)
returns

#Ecto.Changeset<action: nil, changes: %{}, errors: [],
 data: #NestedInputsFor.Product.Product<>, valid?: true>

Here is my live view component :

defmodule NestedInputsForWeb.ProductForm do
  use Phoenix.LiveView
  alias NestedInputsFor.Product.{Attribute, Language, Product}
  alias NestedInputsFor.Repo
  import NestedInputsForWeb.CoreComponents

  def render(assigns) do
    ~H"""
    <h1>Product form</h1>
    <.form for={@form} as={:product} phx-submit={:save} phx-change={:validate}>
      <label>Product name</label>
      <.input field={@form[:name]} type="text" />
      <div class="flex">
        <.inputs_for :let={language_form} field={@form[:languages]}>
          <.input field={language_form[:name]} type="hidden" />
          <div class="flex-initial">
            <%= Ecto.Changeset.get_field(language_form.source, :name) %>
            <br/>
            <.inputs_for :let={attribute_form} field={language_form[:attributes]}>
              <label>Label</label>
              <.input field={attribute_form[:label]} type="text" />
              <label>Value</label>
              <.input field={attribute_form[:value]} type="text" />
            </.inputs_for>
          </div>
        </.inputs_for>
      </div>
      <div phx-click="add_attribute">Add attribute</div>
      <button>Save</button>
    </.form>
    """
  end

  def mount(_params, _, socket) do
    changeset = Repo.all(Product)
    |> Repo.preload(languages: :attributes)
    |> hd()
    |> Product.changeset(%{})

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

  def handle_event("add_attribute", _params, socket) do
    changeset = socket.assigns.changeset
    udpated_languages =
      Ecto.Changeset.get_field(changeset, :languages)
      |> Enum.map(fn language ->
        Map.put(language, :attributes,  language.attributes ++ [%Attribute{}])
      end)

    changeset = Ecto.Changeset.put_assoc(changeset, :languages, udpated_languages)
    {:noreply, assign(socket, %{form: to_form(changeset), changeset: changeset})}
  end

  def handle_event("validate", %{"product" => params}, socket) do
    changeset = Product.changeset(socket.assigns.changeset, params)
    {:noreply, assign(socket, %{form: to_form(changeset), changeset: changeset})}
  end

  def handle_event("save", params, socket) do
    IO.inspect(params)
    {:noreply, socket}
  end
end

This demo project is available here https://github.com/ChristopheBelpaire/nested_inputs_for

Did I do something wrong or is it a bug :slight_smile: ?

Thanks in advance!

I think where you’re putting the attribute into language is the problem.

EctoNestedChangeset module simplifies that.

I had several forms like this in an app i am working on and I ended up creating a small module to provide the nested form:

1 Like

I suspect this is part of the problem: manually updating associations on Language structs won’t be detected by the changeset machinery.

Yes probably, I suppose put_assoc don’t support properly nested structure.
It would be nice to have it, maybe in future version of liveview/ecto.
It is weird it is working with new record.

Thanks a lot!
I will have a look :slight_smile:

It does. The issue you’re running into is that you’re supplying structs to put_assoc though instead of changesets.

Different to passing changesets, structs are not change tracked in any fashion. In other words, if you change a comment struct and give it to put_assoc/4, the updates in the struct won’t be persisted.

You need the following:

  def handle_event("add_attribute", _params, socket) do
    changeset = socket.assigns.changeset

    udpated_languages =
      Ecto.Changeset.get_field(changeset, :languages)
      |> Enum.map(fn language ->
        Ecto.Changeset.change(language)
        |> Ecto.Changeset.put_assoc(:attributes, language.attributes ++ [%Attribute{}])
      end)

    changeset = Ecto.Changeset.put_assoc(changeset, :languages, udpated_languages)
    {:noreply, assign(socket, %{form: to_form(changeset), changeset: changeset})}
  end

In the upcoming ecto release (3.10) there will be Ecto.Changeset.get_assoc/3, which will return you changesets by default (including existing changes), which will work better then get_field/2.

5 Likes

It seems to work!
Thanks a lot :grin:
Indeed it make sense.
Waw this is quite powerful, I could edit the whole database with one n-nested form :sunglasses:

Hello,
I went further with my little exemple and had another problem
I want to create and edit a structure like this one :

Product.changeset(
        %Product{
          languages: [
            %Language{name: "FR", attributes: [
              %Attribute{label: "Label FR 1", value: "Value FR 1"},
              %Attribute{label: "Label FR 2", value: "Value FR 2"}
            ]},
            %Language{name: "EN", attributes: [
              %Attribute{label: "Label EN 1", value: "Value EN 1"},
              %Attribute{label: "Label EN 2", value: "Value EN 2"}
            ]}
          ]
        },
        %{}
      )

I added buttons add and delete attributes for each languages :

It fail to delete the attributes with the following warning :

[warning] found duplicate primary keys for association/embed `:attributes` in `NestedInputsFor.Product.Language`. In case of duplicate IDs, only the last entry with the same ID will be kept. Make sure that all entries in `:attributes` have an ID and the IDs are unique between them

I understand my attributes needs an id, it works if I replace my changeset with :

      Product.changeset(
        %Product{
          languages: [
            %Language{name: "FR", attributes: [
              %Attribute{id: 1, label: "Label FR 1", value: "Value FR 1"},
              %Attribute{id: 2, label: "Label FR 2", value: "Value FR 2"}
            ]},
            %Language{name: "EN", attributes: [
              %Attribute{id: 3, label: "Label EN 1", value: "Value EN 1"},
              %Attribute{id: 4, label: "Label EN 2", value: "Value EN 2"}
            ]}
          ]
        },
        %{}
      )

But of course, I don’t have the id before the records are inserted.
Is there a way to fix this in a clean way ?

Here is my updated component :

defmodule NestedInputsForWeb.ProductForm do
  use Phoenix.LiveView
  alias NestedInputsFor.Product.{Attribute, Language, Product}
  alias NestedInputsFor.Repo
  import NestedInputsForWeb.CoreComponents

  @spec render(any) :: Phoenix.LiveView.Rendered.t()
  def render(assigns) do
    ~H"""
    <h1>Product form</h1>
    <.form for={@form} as={:product} phx-submit={:save} phx-change={:validate}>
      <label>Product name</label>
      <.input field={@form[:name]} type="text" />
      <div class="flex">
        <.inputs_for :let={language_form} field={@form[:languages]}>
          <.input field={language_form[:name]} type="hidden" />
          <div class="flex-initial">
            <%= Ecto.Changeset.get_field(language_form.source, :name) %>
            <br/>
            <.inputs_for :let={attribute_form} field={language_form[:attributes]}>
              <div>
                <label  class="inline-block">Label</label>
                <.input field={attribute_form[:label]} type="text" class="inline-block"/>
                <label  class="inline-block">Value</label>
                <.input field={attribute_form[:value]} type="text"  class="inline-block"/>
                <button type="button" class="btn-blue" phx-click="delete_attribute" phx-value-index={attribute_form.index} phx-value-language={Ecto.Changeset.get_field(language_form.source, :name)}>X</button>
              </div>
            </.inputs_for>
            <br/>
            <button type="button" class="btn-blue" phx-click="add_attribute" phx-value-language={Ecto.Changeset.get_field(language_form.source, :name)}>Add attribute</button>

          </div>
        </.inputs_for>
      </div>
      <br/>
      <button class="btn-blue">Save</button>
    </.form>
    """
  end

  def mount(_params, _, socket) do
    changeset =
    # Repo.all(Product)
    # |> Repo.preload(languages: :attributes)
    # |> hd()
    # |> Product.changeset(%{})

      Product.changeset(
        %Product{
          languages: [
            %Language{name: "FR", attributes: [
              %Attribute{id: 1, label: "Label FR 1", value: "Value FR 1"},
              %Attribute{id: 2, label: "Label FR 2", value: "Value FR 2"}
            ]},
            %Language{name: "EN", attributes: [
              %Attribute{id: 3, label: "Label EN 1", value: "Value EN 1"},
              %Attribute{id: 4, label: "Label EN 2", value: "Value EN 2"}
            ]}
          ]
        },
        %{}
      )

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

  def handle_event("add_attribute", %{"language" => clicked_language}, socket) do
    changeset = socket.assigns.changeset
    udpated_languages =
      Ecto.Changeset.get_field(changeset, :languages)
      |> Enum.map(fn language ->
        if(language.name == clicked_language) do
          Ecto.Changeset.change(language)
          |> Ecto.Changeset.put_assoc(:attributes, language.attributes ++ [%Attribute{}])
        else
          Ecto.Changeset.change(language)
        end
      end)
    changeset = Ecto.Changeset.put_assoc(changeset, :languages, udpated_languages)
    {:noreply, assign(socket, %{form: to_form(changeset), changeset: changeset})}
  end


  def handle_event("delete_attribute", %{"index" => index, "language" => clicked_language}, socket) do
    changeset = socket.assigns.changeset
    index = String.to_integer(index)
    udpated_languages =
      Ecto.Changeset.get_field(changeset, :languages)
      |> Enum.map(fn language ->
        if(language.name == clicked_language) do
          Ecto.Changeset.change(language)
          |> Ecto.Changeset.put_assoc(:attributes, List.delete_at(language.attributes, index))
        else
          Ecto.Changeset.change(language)
        end
      end)
    changeset = Ecto.Changeset.put_assoc(changeset, :languages, udpated_languages)
    {:noreply, assign(socket, %{form: to_form(changeset), changeset: changeset})}
  end


  def handle_event("validate", %{"product" => params}, socket) do
    changeset = Product.changeset(socket.assigns.changeset, params)
    {:noreply, assign(socket, %{form: to_form(changeset), changeset: changeset})}
  end

  def handle_event("save", params, socket) do
    IO.inspect(params)
    {:noreply, socket}
  end
end

The code is also here : https://github.com/ChristopheBelpaire/nested_inputs_for

Thanks in advance!

I’d suggest using indexes instead of ids. I’ve written this down in this one:

1 Like

I’ve run into this before - the issue is that the unsaved, no-id Languages that were passed into Product.changeset are considered “pre-existing data” by put_assoc. That triggers the code path that checks for duplicate IDs.

You should get better results by starting with languages empty and using put_assoc to build the initial state:

%Product{languages: []}
|> Product.changeset(%{})
|> Ecto.Changeset.put_assoc(:languages, [
  %Language{name: "FR", attributes: [
    %Attribute{label: "Label FR 1", value: "Value FR 1"},
    %Attribute{label: "Label FR 2", value: "Value FR 2"}
  ]},
  %Language{name: "EN", attributes: [
    %Attribute{label: "Label EN 1", value: "Value EN 1"},
    %Attribute{label: "Label EN 2", value: "Value EN 2"}
  ]}
 ])

Thanks for the link!
I think I have this problem only with nested inputs_for.

Ok, still digging :slight_smile:

Now I have a single script showing the problem :

Mix.install([:ecto])

defmodule Product do
  use Ecto.Schema
  import Ecto.Changeset

  schema "products" do
    field :name, :string
    has_many :languages, Language
    timestamps()
  end

  def changeset(product, attrs) do
    product
    |> cast(attrs, [:name])
    |> cast_assoc(:languages)
  end
end

defmodule Language do
  use Ecto.Schema
  import Ecto.Changeset

  schema "languages" do
    field :name, :string
    belongs_to :product, Product
    has_many :attributes, Attribute, on_replace: :delete
    timestamps()
  end

  def changeset(language, attrs) do
    language
    |> cast(attrs, [:name])
    |> cast_assoc(:attributes)
  end
end


defmodule Attribute do
  use Ecto.Schema
  import Ecto.Changeset

  schema "attributes" do
    field :label, :string
    field :value, :string
    belongs_to :language, Language
    timestamps()
  end

  def changeset(attribute, attrs) do
    attribute
    |> cast(attrs, [:label, :value])
  end
end


defmodule Main do
  def error() do
    changeset = Product.changeset(%Product{},%{
      "languages" => %{
        "0" => %{
          "attributes" => %{
            "0" => %{"label" => "", "value" => ""},
            "1" => %{"label" => "", "value" => "k"}
          },
          "name" => "FR"
        },
        "1" => %{"name" => "EN"}
      },
      "name" => ""
    })

    index = 1
    selected_language = "FR"
    udpated_languages = Ecto.Changeset.get_field(changeset, :languages) |> Enum.map(fn language ->
      if(language.name == selected_language) do
        Ecto.Changeset.change(language)
        |> Ecto.Changeset.put_assoc(:attributes,
           List.delete_at(language.attributes, index)
        )
      else
        Ecto.Changeset.change(language)
      end
    end)

    updated_changeset = Ecto.Changeset.put_assoc(changeset, :languages, udpated_languages)
    Ecto.Changeset.get_field(updated_changeset, :languages)
    |> IO.inspect()
  end

  def ok() do
    changeset = Product.changeset(%Product{},%{
      "languages" => %{
        "0" => %{"name" => "FR"},
        "1" => %{"name" => "EN"}
      },
      "name" => ""
    })
    index = 1
    languages = Ecto.Changeset.get_field(changeset, :languages)
    updated_changeset = Ecto.Changeset.put_assoc(changeset, :languages, List.delete_at(languages, index))
    Ecto.Changeset.get_field(updated_changeset, :languages)
    |> IO.inspect()
  end
end

IO.puts("Delete in normal input for, working : ")
Main.ok()
IO.puts("Delete in nested input for, not working and warning: ")
Main.error()

It is still about the warning :

12:45:35.661 [warning] found duplicate primary keys for association/embed `:attributes` in `Language`. In case of duplicate IDs, only the last entry with the same ID will be kept. Make sure that all entries in `:attributes` have an ID and the IDs are unique between them

It only happen with nested forms on two (or more) levels.
Maybe it is linked to : Warning about duplicated primary keys for new records · Issue #3514 · elixir-ecto/ecto · GitHub

If someone has an idea how to make this work, I’m super interresed :slight_smile:

Thanks!

If you have previous changes don’t use Ecto.Changeset.get_field, but Ecto.Changeset.get_change. Or use ecto master till 3.10 is released and Ecto.Changeset.get_assoc, so you don’t need to care for the difference.

Thanks for the suggestion!
I tried with get_assoc and it seems to work, for instance :

    changeset =
      Product.changeset(%Product{}, %{
        "languages" => %{
          "0" => %{
            "attributes" => %{
              "0" => %{"label" => "", "value" => ""},
              "1" => %{"label" => "", "value" => "k"}
            },
            "name" => "FR"
          },
          "1" => %{"name" => "EN"}
        },
        "name" => ""
      })

    index = 1
    selected_language = "FR"

    udpated_languages =
      Ecto.Changeset.get_assoc(changeset, :languages)
      |> Enum.map(fn language ->
        if(Ecto.Changeset.get_field(language, :name) == selected_language) do
          attributes = Ecto.Changeset.get_field(language, :attributes)

          language
          |> Ecto.Changeset.put_assoc(
            :attributes,
            List.delete_at(attributes, index)
          )
        else
          Ecto.Changeset.change(language)
        end
      end)

    updated_changeset = Ecto.Changeset.put_assoc(changeset, :languages, udpated_languages)

The full example is here : nested_inputs_for/error.exs at main · ChristopheBelpaire/nested_inputs_for · GitHub

Thanks a lot for the suggestion!

I think you want to use it consistently for all assoc/embes – so :languages as well as their :attributes.

Indeed, the changeset seems to loose track of some changes if I use get_field.
If you have time, you should try to make a multiple nested inputs_for (and maybe update your article ;)) it is quite challenging :slight_smile:

Thanks again !

I don’t think that’s really necessary. The things you’re running into are exactly the same issues, just on more than one level.

Probably, but I ran on much less issues on one level.
Thanks again for the article, it really helped!

Does repeatedly deleting items work for you with this one? I tried pulling the repo and running it since I’ve also been hitting some issues with change tracking and input_for components.

Adding and editing items is working great, as well as deleting a single item, but if I make a list of 5 items, delete the 4th and then delete the new 4th (now the last item left), that one doesn’t delete and I see duplicate primary key errors:

[debug] HANDLE EVENT
  View: OneToManyWeb.ListLive
  Event: "delete-line"
  Parameters: %{"index" => "4", "value" => ""}
[warning] found duplicate primary keys for association/embed `:lines` in `OneToMany.GroceriesList`. In case of duplicate IDs, only the last entry with the same ID will be kept. Make sure that all entries in `:lines` have an ID and the IDs are unique between them

[debug] Replied in 338µs
[debug] HANDLE EVENT
  View: OneToManyWeb.ListLive
  Event: "delete-line"
  Parameters: %{"index" => "4", "value" => ""}
[warning] found duplicate primary keys for association/embed `:lines` in `OneToMany.GroceriesList`. In case of duplicate IDs, only the last entry with the same ID will be kept. Make sure that all entries in `:lines` have an ID and the IDs are unique between them

I tried extracting Lines into its own schema in one_to_many/grocery_list/lines.ex and setting @primary_key false. This eliminated that error but there are still issues with deleting (even after removing the explicit check for the id in the “delete-line” handler.

I wonder if it could be a deps issue. The mix file specifies Ecto 3.6, but I know it made a change in how it handles empty list fields in v. 3.8.

Edit: I ruled out the Ecto version issue. Even with 3.10, multiple consecutive deletions fail (at least on my machine)

Hello,
According to the last talk of Chris McCord at ElixirConf, there is now a much easier way to do this.
I need to update my example.

2 Likes