[PATCH] fix self-referential has_many :through relationships

Please note that this patch does not include the db definitions for all the adapters which will break unit tests for all but mysql.

Hi, can you try this again? I just made a commit that fixes has_many :through. It should now respect the foreign key in the join model.

I'm closing this unless you have an issue on the latest beta gems.

This still doesn't seem to work for self-referential has_many :through relationships. It still produces

SELECT posts.* FROM posts INNER JOIN related_posts
ON posts.id = related_posts.post_id
WHERE (related_posts.post_id = 4)

when it should be

SELECT posts.* FROM posts INNER JOIN related_posts
ON posts.id = related_posts.related_post_id
WHERE (related_posts.post_id = 4)

Unless I'm missing an option to set on my relationship now.

Does it work if you leave off :class_name?

# looks for RelatedPost#related_post
has_many :related_post, :through => :related_post_infos
# looks for RelatedPost#post because of the class_name setting
has_many :related_post, :class_name => 'Post', :through => :related_post_infos

I don't see a need to specify the foreign key twice.

I removed the class_name, but my tests fail now with the following error:

ActiveRecord::HasManyThroughSourceAssociationNotFoundError: Could not find the source associations manually_related_product and manually_related_products in model RelatedProduct

This is what's in my model:

  has_many :related_product_metadata, :class_name => 'RelatedProduct', :order => 'position ASC'
  has_many :manually_related_products, :through => :related_product_metadata, :order => 'position ASC'

By the way, even if this works, how would you get the reverse of the relationship: products_related_to_me? That doesn't seem possible without specifying which foreign keys to use.

That error is for the source association. It's going through :related_product_metadata and trying to access :manually_related_products or :manually_related_product (you can now go through a belongs_to OR a has_many).

I don't know how to answer your second question. That's a pretty odd schema you're rocking right there, pal. However, has_many :through associations typically use the source association for most everything. If you can introspect the info from the source, I see no reason to duplicate it in the has_many :through association.

I believe it is working now. What I was missing was the second relationship to the join table in Product. Now my model looks like this:

class Product < ActiveRecord::Base
  # the main relationship to the join table
  has_many :related_product_metadata, :class_name => 'RelatedProduct'
  # the other side of the relationship, allows the has_many :through to find its way back
  has_many :related_to_product_metadata, :class_name => 'RelatedProduct',
           :foreign_key => 'related_product_id'

  # the through relationship
  has_many :related_products, :through => :related_product_metadata
  # the inverse through relationship, just for kicks
  has_many :related_products, :through => :related_to_product_metadata
end

Thanks

I spoke too soon. I forgot to change some lines in my tests to use the through relationship before testing. It is still using the wrong join column in the query. I will attach a diff with just the tests in it for mysql, both added test methods fail.

failing tests for self-referential has_many :through relationships

failing tests for self-referential has_many :through relationships

Attachment self_referential_has_many_through_tests.2.diff has the new related_post.rb and related_posts.yml file in the diff, too.

I'm sorry, I can't quite grasp what you're trying to do. I added a couple self referential test cases, and everything worked out fine:

class AuthorFavorite < ActiveRecord::Base
  belongs_to :author
  belongs_to :favorite_author, :class_name => "Author", :foreign_key => 'favorite_author_id'
end

class Author < ActiveRecord::Base
  has_many :author_favorites
  has_many :favorite_authors, :through => :author_favorites, :order => 'name'
end

Ok, I think I figured out the problem. My names for the relationships weren't matching up. I didn't know that the singular or plural name of the has_many :through relationship had to match the relationship in the join table. It makes sense now that I know what it's doing, but it would be nice if this could be overridden like how a habtm can have the :association_foreign_key specified. This would allow you to rename your has_many :through without having to rename the relationship in the join model in lockstep.

For instance, if you want to see who the fans of an author are (the inverse of the favorite_authors relationship), you could add the following to Author:

  has_many :favorite_of, :class_name => 'AuthorFavorite', :foreign_key => 'favorite_author_id'
  has_many :fans, :through => :favorite_of

You now also need to rename the author belongs_to relationship in the join model for it to work.

So in AuthorFavorite

  belongs_to :author

needs to become

  belongs_to :fan, :foreign_key => 'author_id', :class_name => 'Author'

The test for this is simply the opposite of test_self_referential_has_many_through

  def test_self_referential_has_many_through_in_reverse
    assert_equal [authors(:david)], authors(:mary).fans
    assert_equal [],               authors(:david).fans
  end

I guess this is just convention over configuration, which I'm fine with. I suppose this ticket's status is now just invalid or an enhancement request for an :association_relationship option in has_many :through to define which relationship to use in the join model.

Thanks for putting in those new test cases, it really helped to figure out what I was doing wrong.

Oy, this is making my head spin :)

You change that association to :fan, and then the other relationship breaks. In this particular case, I think #fans would just be a method on author with a custom find. You wouldn't add fans to an author. You'd add that author as a favorite of another author.

Let me think about this. I think you'd be able to do this though:

class Author < AR::Base
  has_many :author_favorites
  # looks for :author in AuthorFavorite
  has_many :fans, :through => :favorite_of, :class_name => 'Author'
end

You'll probably have to add some conditions somewhere.

Anyways, thanks for sticking with me on this. I just want to make absolute sure I need this option.

I agree this is a somewhat contrived example. The reason my names weren't matching was I wanted to augment the has_many :through with some additional records pulled from a find_by_sql, so I wrote a method related_products that adds on to the manually_related_products relationship. I kept my join model still called RelatedProduct and it had a belongs_to :related_product instead of belongs_to :manually_related_product which matches the name of the has_many :through relationship. This probably isn't a very common use case, it just happened to be my first try at a self-referential has_many :through.

By the way, all my tests still work after making the changes to the relationship names. Here is what I have now:

class Author < ActiveRecord::Base
  has_many :author_favorites
  has_many :favorite_authors, :through => :author_favorites

  has_many :favorite_of, :class_name => 'AuthorFavorite', :foreign_key => 'favorite_author_id'
  has_many :fans, :through => :favorite_of
end
class AuthorFavorite < ActiveRecord::Base
  belongs_to :fan, :foreign_key => 'author_id', :class_name => 'Author'
  belongs_to :favorite_author, :class_name => "Author", :foreign_key => 'favorite_author_id'
end

And tested with this:

class AuthorFavoriteTest < Test::Unit::TestCase
  fixtures :authors, :author_favorites

  def test_self_referential_has_many_through
    assert_equal [authors(:mary)], authors(:david).favorite_authors
    assert_equal [],               authors(:mary).favorite_authors
  end

  def test_self_referential_has_many_through_in_reverse
    assert_equal [authors(:david)], authors(:mary).fans
    assert_equal [],               authors(:david).fans
  end

  def test_add_to_self_referential_has_many_through
    new_author = Author.create(:name => "Bob")
    authors(:david).author_favorites.create :favorite_author => new_author
    assert_equal [authors(:mary), new_author], authors(:david).reload.favorite_authors
  end
end

I think using :class_name in your has_many :through causes problem in the source_reflection_names of active_record/reflection.rb. It will more often than not cause it to join back on the wrong column in self-referential cases, as in my earlier examples.

Check out [4022]. I added a :source option for specifying the source reflection name exactly.

Looks great. It worked the way I would expect and the error messages are more helpful now.

Thanks

Rock on. Thanks for your patience and ticket.