#4329 has_many through association does not link models on association save - 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.

#4329 ✓resolved

has_many through association does not link models on association save

Reported by Johannes Schmidt | April 5th, 2010 @ 07:21 PM

Having a :through association like

class Post < ActiveRecord::Base
  has_many :readers
  has_many :people, :through => :readers
end

class Reader < ActiveRecord::Base
  belongs_to :person
  belongs_to :post
end

class Person < ActiveRecord::Base
  has_many :readers
  has_many :posts, :through => :readers
end

When you build a new Person through the association via build or new like

person1 = post.people.build(:first_name => "Bob")
person2 = post.people.build(:first_name => "Ted")

and save the person directly (instead of the post):

person1.save
person2.save

than the association does not work as expected:

post.reload.people(true).collect(&:first_name).include?("Bob") #=> false
post.reload.people(true).collect(&:first_name).include?("Ted") #=> false

You can reproduce this by adding a test to active_record:

in test/cases/associations/has_many_through_associations_test.rb add the following lines (or apply the patch):

  def test_associate_by_saving_association
    person1 = posts(:thinking).people.build(:first_name=>"Bob")
    person2 = posts(:thinking).people.new(:first_name=>"Ted")
    
    person1.save
    person2.save
    
    assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Bob")
    assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Ted")
  end

and it will fail in master.

Saving the associated object is very useful in scoped controllers where you have an association based eg. on the current user.

Greetings
Johannes

test_associate_by_saving_association.diff 1021 Bytes

Comments and changes to this ticket

You flagged this item as spam.
Johannes Schmidt April 5th, 2010 @ 07:26 PM
- Tag changed from “has_many_through” to “has_many_through, rails3”
You flagged this item as spam.
Johannes Schmidt April 5th, 2010 @ 07:28 PM
btw. using
```
post.people.create(:first_name => "Bob")
```
works as expected.
You flagged this item as spam.
Delineate April 22nd, 2010 @ 04:15 AM
Just to clarify a little, I'm experiencing this in 2.3.5 on non-polymorphic has_many_through associations.
(The opening line suggests this is for polymorphic associations, though the example is not)

Imho, the dropping of the intermediate model/association when saving the "built" child was very unexpected.
You flagged this item as spam.
Johannes Schmidt April 22nd, 2010 @ 06:25 AM
@Delinieate: Polymorphic was a typo. Just changed that.
You flagged this item as spam.
Sigurd April 22nd, 2010 @ 07:41 AM
From rails doc:
:autosave

If true, always save the associated object or destroy it if marked for destruction, when saving the parent object. **Off by default**.

You can manipulate objects and associations before they are saved to the database, but there is some special behavior you should be aware of, mostly involving the saving of associated objects.

Unless you set the :autosave option on a has_one, belongs_to, has_many, or has_and_belongs_to_many association. Setting it to true will always save the members, whereas setting it to false will never save the members.
You flagged this item as spam.
Johannes Schmidt April 22nd, 2010 @ 08:13 AM
The same behavior when using the :autosave option.
You flagged this item as spam.
Delineate April 22nd, 2010 @ 10:35 PM
I've tried w/ various combinations of :autosave on different associations on the child=>intermediary=>parent relationships.
None seem to work.
Of course, :autosave on the parent => child associations aren't even needed if we were talking about saving the whole set by saving the parent.

I have a feeling it's more a matter of the reverse relationship not being created in the first place.
```
child = parent.children.build()
# Parent has the child associations
parent.intermediaries => [<Intermediary id: 1....>]
parent.children => [<Child id: 1 ...]

# However, the child doesn't have the parent or intermediary.
child.intermediaries => []
child.parent => []

# Thus, it's unlikely saving the child (w/ or w/out :autosave) will change this
child.save
child.intermediaries => []
child.parent => []  # Still empty.
```
On the one hand, I suppose it shouldn't be assumed that the child => parent associations have been defined just because the parent=>child associations have been (which are necessarily exist if we're using build() in the first place ),

On the other, it seems very counter-intuitive that the only way to save the child correctly is to save the parent.

My guess is it's something that never made it off the TODO list.
You flagged this item as spam.
joost baaij April 28th, 2010 @ 08:36 PM
I can confirm that this bug exists in 2.3.5 as well as master, and that adding autosave => true does not help.

IMO it's truly a bug since the intermediary model is built in memory automatically, but never saved when the child is saved.
You flagged this item as spam.
Joe None May 4th, 2010 @ 09:37 PM
Also confirming bug exists in Rails beta3
Neeraj Singh May 12th, 2010 @ 04:01 AM
I looked into issue and this is what I found.

Let's say that relationship is
```
class Person < AR
 has_many :readers
 has_many :posts, :through =>  :readers
end

post = Person.first.posts.build(:quality => 'good')
puts post.person_id #=> 8
```
As you can see in the above case the relationship already exists. It's just that when post.save is called then no after_save callback is made to persist a new reader record.

In the after_save callback following two lines are needed. These two lines create the association record when you create a new post using Person.last.posts.create(..)
```
    through_association = @owner.send(@reflection.through_reflection.name)
    through_record = through_association.create!(construct_join_attributes(record))
```
However I am stuck. I am stuck because I do not know how to set an after_save callback on a given instance of post record. In this case there is no such after_save callback on the Post class. It is just that if a post instance is crated using build then those instances need an after_save callback to persist the association record.

Long writeup. However I wanted to explain everything I did. Someone who has more intimate knowledge of ActivRecord and a better ruby user should be able to fix it.
Ryan Bigg May 13th, 2010 @ 11:48 PM
- Tag changed from “has_many_through, rails3” to “bugmash, has_many_through, rails3”
- State changed from “new” to “incomplete”
Please provide a failing test case and patch for this issue.
You flagged this item as spam.
joost baaij May 14th, 2010 @ 01:24 AM
The failing test case is right there in the ticket.

Also here:
http://github.com/tilsammans/rails/commit/e613561734afb08cccae048d2...
You flagged this item as spam.
Lake May 14th, 2010 @ 08:41 PM
After applying "test_associate_by_saving_association.diff", included in the OP, I can confirm it's failing on master.
You flagged this item as spam.
Mark Mulder October 19th, 2010 @ 12:42 PM
- Importance changed from “” to “Low”
Just got bitten by this bug on 2.3.10.
Ryan Bigg January 4th, 2011 @ 09:16 AM
Also bitten by this bug on 3.0.3. See http://github.com/radar/account_users_bug.
Repository February 18th, 2011 @ 05:15 PM
- State changed from “incomplete” to “resolved”
(from [91fd6510563f84ee473bb217bc63ed598abe3f24]) Allow building and then later saving has_many :through records, such that the join record is automatically saved too. This requires the :inverse_of option to be set on the source association in the join model. See the CHANGELOG for details. [#4329 state:resolved] https://github.com/rails/rails/commit/91fd6510563f84ee473bb217bc63e...