Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ecto.Changeset.put_assoc() fails when trying to change belongs_to association (sometimes) #1432

Closed
carusso opened this issue May 14, 2016 · 5 comments

Comments

@carusso
Copy link
Contributor

carusso commented May 14, 2016

Elixir: 1.2.4
Postgres: 9.5.2
ecto 2.0.0-rc.5 (Hex package) (mix)
phoenix_ecto 3.0.0-rc.0 (Hex package) (mix)
postgrex 0.11.1 (Hex package) (mix)
OSX 10.11.3

I created a new phoenix project and performed the upgrade to Ecto 2.0.0rc5

Following simple models were created:

defmodule Demo.Club do
  use Demo.Web, :model

  schema "clubs" do
    field :name, :string
    has_many :members, Demo.Member

    timestamps
  end
end

defmodule Demo.Member do
  use Demo.Web, :model

  schema "members" do
    field :name, :string
    belongs_to :club, Demo.Club

    timestamps
  end
end

So a Member can only belong to one Club and a Club can have many Members

The following test to put_assoc succeeds

  test "can associate existing member with existing club" do
    member = Repo.insert!(%Member{name: "Dave"}) |> Repo.preload(:club)
    club = Repo.insert!(%Club{name: "Wentworth"})

    changeset = member
                |> Changeset.change
                |> Changeset.put_assoc(:club, club)
    assert changeset.valid?
    member = Repo.update! changeset
    assert member
  end

The following test fails trying to change the association using actual db model references

  test "can change association for existing member with existing club" do
    member = Repo.insert!(%Member{name: "Dave"}) |> Repo.preload(:club)
    club = Repo.insert!(%Club{name: "Wentworth Club"})
    club2 = Repo.insert!(%Club{name: "Palm Club"})

    changeset = member
                |> Changeset.change
                |> Changeset.put_assoc(:club, club)
    assert changeset.valid?
    member = Repo.update! changeset
    assert member

    changeset = member
                |> Changeset.change
                |> Changeset.put_assoc(:club, club2)
    assert changeset.valid?
    member = Repo.update! changeset
    assert member
  end

Curiously, the following test succeeds by using a struct Member instead

of the actual Member from the insert!().

  test "can change association for existing member with existing club using struct reference for member" do
    member = Repo.insert!(%Member{name: "Dave"}) |> Repo.preload(:club)
    club = Repo.insert!(%Club{name: "Wentworth Club"})
    club2 = Repo.insert!(%Club{name: "Palm Club"})

    changeset = member
                |> Changeset.change
                |> Changeset.put_assoc(:club, club)
    assert changeset.valid?
    member = Repo.update! changeset
    assert member

    changeset = %Member{id: member.id}
                |> Changeset.change
                |> Changeset.put_assoc(:club, club2)
    assert changeset.valid?
    member = Repo.update! changeset
    assert member
  end

The error that is returned for that second test case is:

 ** (RuntimeError) you are attempting to change relation :club of
 Demo.Member, but there is missing data.

 If you are attempting to update an existing entry, please make sure
 you include the entry primary key (ID) alongside the data.

 If you have a relationship with many children, at least the same N
 children must be given on update. By default it is not possible to
 orphan embed nor associated records, attempting to do so results in
 this error message.

 It is possible to change this behaviour by setting `:on_replace` when
 defining the relation. See `Ecto.Changeset`'s section on related data
 for more info.
@josevalim
Copy link
Member

Thank you @carusso!

It is all correct. The reason the third does not fail is because you don't have any ID in your structure, so Ecto does not know data is being replaced. The solution is to set :on_replace when you declare the belongs_to and specify what you want to happen. :nilify makes the most sense if you simply want to replace one club by the other.

@carusso
Copy link
Contributor Author

carusso commented May 15, 2016

Not sure what you mean by "don't have any ID in your structure", since I did have an id on Member:
%Member{id: member.id}

Maybe my confusion was on the naming of that :on_replace option.

What exactly is being nilified? Effectively, all I want is for member.club_id to be "replaced".

@josevalim
Copy link
Member

I meant FK instead ID. What is being nullified is the FK as well, which is
then immediately set to the ID of the new club.

On Sunday, May 15, 2016, Chris R notifications@github.com wrote:

Not sure what you mean by "don't have any ID in your structure", since I
did have an id on Member:
%Member{id: member.id}

Maybe my confusion was on the naming of that :on_replace option.

What exactly is being nilified? Effectively, all I want is for
member.club_id to be "replaced".


You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub
#1432 (comment)

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

@carusso
Copy link
Contributor Author

carusso commented May 15, 2016

Okay, thank you. I would recommend renaming the :nilify option to something like :force, :accept, or :allow. :nilify implies an end state of nil but doesn't really tell me that the :on_replace is allowing the fk relation to simply be replaced. When I read the documentation looking for replacing the fk, I saw ":nilify" and thought, "that can't be it."

In general, the documentation could use a lot more explanation around what is occurring in operations involving associations. When I used similar concepts in Rails, I found it much easier to comprehend what was happening at an abstract level - even though I know that ActiveRecord was doing a bunch of magic behind the scenes that you want to avoid in Phoenix.

The flip side of not having the framework do all the magic is that the developer needs a better understanding of the internals.

I'll think about it and experiment so I can understand it better and possibly contribute something to the docs.

Thanks again.

@josevalim
Copy link
Member

Awesome, PRs for docs are always welcome!

On Sunday, May 15, 2016, Chris R notifications@github.com wrote:

Okay, thank you. I would recommend renaming the :nilify option to
something like :force, :accept, or :allow. :nilify implies an end state of
nil but doesn't really tell me that the :on_replace is allowing the fk
relation to simply be replaced. When I read the documentation looking for
replacing the fk, I saw ":nilify" and thought, "that can't be it."

In general, the documentation could use a lot more explanation around what
is occurring in operations involving associations. When I used similar
concepts in Rails, I found it much easier to comprehend what was happening
at an abstract level - even though I know that ActiveRecord was doing a
bunch of magic behind the scenes that you want to avoid in Phoenix.

The flip side of not having the framework do all the magic is that the
user needs a better understanding of the internals.

I'll think about it and experiment so I can understand it better and
possibly contribute something to the docs.

Thanks again.


You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub
#1432 (comment)

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

axelson pushed a commit to axelson/ecto that referenced this issue Dec 15, 2017
Even though I've previously read issue elixir-ecto#1432 the knowledge did not stick
and I didn't correctly set `on_replace` to `:nilify`. Hopefully this doc
update will help.
josevalim pushed a commit that referenced this issue Dec 16, 2017
Even though I've previously read issue #1432 the knowledge did not stick
and I didn't correctly set `on_replace` to `:nilify`. Hopefully this doc
update will help.
bartekupartek pushed a commit to bartekupartek/ecto that referenced this issue Mar 19, 2019
…cto#2350)

Even though I've previously read issue elixir-ecto#1432 the knowledge did not stick
and I didn't correctly set `on_replace` to `:nilify`. Hopefully this doc
update will help.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants