#2852 accepts_nested_attributes_for creating new associated record when one already exists - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#2852 ✓invalid

accepts_nested_attributes_for creating new associated record when one already exists

Reported by Josh Sharpe | June 29th, 2009 @ 02:20 PM | in 3.0.2

I'm seeing that updating nested attributes is creating additional rows for the nested object:

class Profile < ActiveRecord::Base
  has_one :address
  accepts_nested_attributes_for :address
end







class Address < ActiveRecord::Base
  belongs_to :profile
end

instantiate a profile and save it w/ associated address attributes:

>> p = Profile.new
=> #<Profile id: nil, name: nil, created_at: nil, updated_at: nil>
>> p.update_attributes(:name => "josh", :address_attributes => {:line1 => "my street"})
=> true
>> Profile.count
=> 1
>> Address.count
=> 1

So far this is fine. Now reload the profile and update its attributes again:

>> p = Profile.first
=> #<Profile id: 3, name: "josh", created_at: "2009-06-29 13:23:18", updated_at: "2009-06-29 13:23:18">
>> p.update_attributes(:name => "josh", :address_attributes => {:line1 => "my other street"})
=> true
>> Profile.count
=> 1
>> Address.count
=> 2

Why are there two addresses at this point?

>> Address.first
=> #<Address id: 6, profile_id: nil, line1: "my street", created_at: "2009-06-29 13:23:18", updated_at: "2009-06-29 13:23:50">
>> Address.find(7)
=> #<Address id: 7, profile_id: 3, line1: "my other street", created_at: "2009-06-29 13:23:50", updated_at: "2009-06-29 13:23:50">
>> Address.find(6).profile_id
=> nil

The old address record's foreign key now points to nothing. This would be invalid if Address validated its association.

I'm willing to dig into this if someone can confirm that it is a bug.

Thanks!

Comments and changes to this ticket

You flagged this item as spam.
Josh Sharpe June 29th, 2009 @ 02:40 PM
- Tag changed from “2-3, accepts_nested_attributes_for, has_one, stable” to “2-3, accepts_nested_attributes_for, has_one”
Yehuda Katz (wycats) July 2nd, 2009 @ 01:10 AM
- State changed from “new” to “verified”
- Milestone cleared.
This does seem to be a bug. Dig away!
Pratik July 2nd, 2009 @ 01:30 AM
- State changed from “verified” to “open”

Josh Sharpe July 2nd, 2009 @ 04:31 PM

There are tests that explicitly test for this behavior making me think this is not a bug.

See nested_attributes_test.rb:

def test_should_not_replace_an_existing_record_if_there_is_no_id_and_delete_is_truthy
  @pirate.reload.ship_attributes = { :name => 'Davy Jones Gold Dagger', :_delete => '1' }

  assert_equal @ship, @pirate.ship
  assert_equal 'Nights Dirty Lightning', @pirate.ship.name
end

def test_should_modify_an_existing_record_if_there_is_a_matching_id
  @pirate.reload.ship_attributes = { :id => @ship.id, :name => 'Davy Jones Gold Dagger' }

  assert_equal @ship, @pirate.ship
  assert_equal 'Davy Jones Gold Dagger', @pirate.ship.name
end

This still seems a bit counterintuitive to me. I would think that the existing record would be used if it exists regardless if an ID is passed. However, maybe there's a really good reason for this behavior that I'm missing.

Carl Lerche July 2nd, 2009 @ 06:24 PM
- State changed from “open” to “invalid”
I am not intimately familiar with the behavior of nested attribute updating, but I can take a shot at this.

The question specifically would be: When are nested attributes updates vs. created. In the original update_attributes call, in :address_attributes, you do not specify the ID of the record to update, so my guess is that AR decides that it should create a new record.
```
p.update_attributes(:name => "josh", :address_attributes => {:line1 => "my street"})
```
You also do not specify a :dependent option on your has_one association, so the default is to nullify the foreign key. So, I guess that if you want to update the record, you should specify the ID in the nested Hash. The tests that specify updating nested records all contain the ID of the records to update.
You flagged this item as spam.
Josh Sharpe July 2nd, 2009 @ 08:35 PM
I just want to quickly reiterate, to be really clear -- this issue only applies to has_one relations.

That said, passing :id as the option to decide whether or not to update or create a new record doesn't make much sense. IMO wouldn't it make more sense that the nested attribute ALWAYS reference the existing one. This wouldn't break the ability to pass _delete => true, but it would prevent dangling records being created inadvertently and transparently.
Carl Lerche July 2nd, 2009 @ 09:40 PM
- State changed from “invalid” to “open”
Alright, I checked up on this some more and it seems that you are correct with regards to has_one should work without an ID. I'll mark this back as open.
José Valim July 9th, 2009 @ 08:57 AM
@Josh, @Carl, For me it makes sense creating a new record when an ID is not specified, even for has_one. It's common that I want to replace the associated object by another one.

I think the problem here is the dangling record. In such cases, when a record is being replaced, it should check for the :dependent behavior given to the association and invoke it properly.
You flagged this item as spam.
Josh Sharpe July 9th, 2009 @ 07:42 PM
@Jose, Would you be strongly opposed if has_one returned the current association and you had to pass _delete => true in order to delete this record?

I still feel that (per the original example)

p.update_attributes(:address_attributes => {:id => p.address.id, :line1 => "my other street"})

is redundant.
José Valim July 9th, 2009 @ 07:59 PM
@Josh, that would be a way to go, but I'm not sure if it works right now. Besides, there would be no way to nullify the record anymore.

Although the question of when nullify, destroy or delete is another problem yet to be handled by nested attributes. I would prefer if _delete just trigger whatever is configured on the association, instead of destroying it.

Currently we also have the same behavior to has_one and has_many, giving different behaviors can lead to further confusion. :/

Have you posted on the mailing list? Maybe we can bring more people to the discussion. :)
Michael Koziarski July 9th, 2009 @ 11:59 PM
- Assigned user set to “Eloy Duran”
Eloy Duran July 10th, 2009 @ 08:10 AM
- State changed from “open” to “invalid”
This is not a bug, as José already noted. If you want to update, specify the ID, otherwise a new one is created.

I also agree that _delete should do what's configured on the association and we might apply the same behaviour on one-to-one associations in the case where a record would be orphaned.

I just haven't had the need for it yet and thus haven't implemented it yet. If you need it on your app and are willing to write a patch, please do so and re-open the ticket.

Thanks!
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to “3.0.2”
[bulk edit]
You flagged this item as spam.
Andrea Campi October 16th, 2010 @ 11:56 PM
- Tag changed from “2-3, accepts_nested_attributes_for, has_one” to “2-3-stable, accepts_nested_attributes_for, has_one”
You flagged this item as spam.
ssupreme11 May 10th, 2011 @ 10:38 PM
Its my first time to visit this site and as I was exploring I cant believe that this site was made up of a very informative articles that you should try to have compliment with so as what I am doing now I really love to look forward with more interesting information on this site.. Dissertation Uk