This project is archived and is in readonly mode.

#1152 ✓committed
Patrick Klingemann

Support for nested :through associations

Reported by Patrick Klingemann | October 2nd, 2008 @ 05:13 AM | in 3.1

Has this issue ever been resolved?

From the Trac ticket located here: Ticket #6461

There is a plugin that provides this functionality located

Comments and changes to this ticket

  • Patrick Klingemann

    Patrick Klingemann October 2nd, 2008 @ 05:16 AM

    Oops, there were some formatting errors above. The plugin is located here: http://github.com/ianwhite/neste...

    This seems like fundamental ActiveRecord functionality to me, shouldn't it be included in Rails?

  • Claudio Poli

    Claudio Poli October 7th, 2008 @ 05:45 PM

    I don't disagree, an official way of doing nested hm:t would be useful, especially now that hm:t reflection got an overhaul and the plugin occasionally don't works anymore.

  • George Deglin

    George Deglin October 7th, 2008 @ 11:47 PM

    I've been using the plugin on production for a couple months now. I don't know how I ever got by without it. Definitely something I'd like to see be included.

  • Claudio Poli

    Claudio Poli October 7th, 2008 @ 11:49 PM

    FWIW some recent changeset broke some functionality in this plugin, I replaced last line in init.rb with:

    ActiveRecord::Reflection::ThroughReflection.send :include, NestedHasManyThrough::Reflection
    to work on edge, probably there will be some checks for the current rails version, I've notified Ian.
  • Pratik

    Pratik November 18th, 2008 @ 02:00 PM

    • Assigned user set to “Pratik”
  • RurouniJones

    RurouniJones November 18th, 2008 @ 02:19 PM

    I would very much like to see this integrated into the rails core for a variety of reasons which I shall now laboriously list for completeness. This is from the perspective of an average programmer.

    1) Demand / Frequency of the issue.

    In almost every app I have written I have needed more than 3 levels of model nesting and every time I have ended up cursing as I have to write .collect statements for that extra level.

    Everyone I have ever talked to (expect the poor shmos who would have to develop it ;) are in favour of the idea, the original traq ticket seemed to get a lot of support.

    2) Maintenance.

    The journeyman programmer in my recognizes that this is a pretty big change to the ActiveRecord code and as such can be broken quite frequently by changes in ActiveRecord. Integrating it into the core will eliminate the time lag between new rails versions and the updated plugin and also reduce uncertainty to the end user (dveloper?) as to whether his particular version of the plugin will be compatible.

    3) It is logically implied...to me at least.

    The first time I used has_many :through I actually started nesting them automatically on the assumption that if you could to it for one extra level, why not two or three? I now realize the implications of these extra levels for edge cases and the like but that thought still stuck with me.

    4) There is already a plugin

    As there is already a plugin the amount of work needed to integrate this into the core would be less than for a brand new never-pondered-before feature.

    So there you have it, I hope you are all thoroughly convinced now and will hop of and integrate it posthaste.

  • Pratik

    Pratik December 7th, 2008 @ 03:24 AM

    • State changed from “new” to “incomplete”

    I'll reopen once we have a complete patch for this.

    Thanks.

  • Patrick Klingemann

    Patrick Klingemann December 9th, 2008 @ 08:34 PM

    Pratik,

    Does this mean that you're waiting on a patch to be written and submitted or does it mean that one is under development and you're waiting for it to be completed and submitted?

    Thanks, Pat

  • Pratik

    Pratik December 9th, 2008 @ 08:45 PM

    Pat,

    I'm waiting for a patch to be written/submitted. I don't know if anyone is actively working on the patch or not.

    Thanks.

  • Eric

    Eric February 21st, 2009 @ 07:16 AM

    Is someone working to turn the plugin into a patch?

  • nofxx

    nofxx April 17th, 2009 @ 04:46 PM

    +1 this feature in core. Rails 2.3 brokes the eager load (on the plugin) when you include some model in the nested chain.. trying to fix....

  • Arj

    Arj July 4th, 2009 @ 10:48 AM

    As far as I know no one is working on this actively. I messaged the plugin owner on github but received no reply.

    I still cannot believe that this isn't part of core rails when you can nest practically everything else.

    I am willing to pay a $50 bounty to anyone who can work this patch to the point that it is accepted by the core devs. Maybe if other people are willing to do the same we can actually get someone with the skills to do so to actually integrate it into core where it belongs.

  • Falk Pauser

    Falk Pauser August 21st, 2009 @ 12:53 PM

    I also assumed this functionality is standard and it took me some hours to figure out it is'nt.
    I think at minimum rails should complain about nested has_many :through associations and throw an error instead of wasting the users time... (productivity?)

    +1 for implementing this feature in rails core +1 for changing rails-behavior and throwing a "Nested has_many :through error" as long as the expacted behavior is'nt available

  • Marcos A Piccinini

    Marcos A Piccinini March 16th, 2010 @ 05:03 PM

    Cheating it to work:

    http://code.alexreisner.com/articles/has-many-through-habtm.html

    Oh, just occurred to me to check Rails 3 for this. Does arel did it?

  • Rodrigo Navarro

    Rodrigo Navarro April 16th, 2010 @ 01:42 AM

    Marcos A Piccinini,

    Unfortunately, it is not working on Rails 3 too.

  • ronin-23439 (at lighthouseapp)

    ronin-23439 (at lighthouseapp) May 3rd, 2010 @ 03:40 PM

    • Assigned user cleared.

    +1 Was shocked rails didn't already have this.

  • ronin-23439 (at lighthouseapp)

    ronin-23439 (at lighthouseapp) May 3rd, 2010 @ 03:41 PM

    • Assigned user set to “Pratik”

    Didn't mean to change the assigned user.

  • Jeremy Kemper

    Jeremy Kemper May 4th, 2010 @ 06:48 PM

    • Milestone changed from “2.x” to “3.x”
  • Alex MacCaw

    Alex MacCaw September 12th, 2010 @ 10:19 AM

    • Importance changed from “” to “”

    +1 For Rails 3.x

  • Bodaniel Jeanes

    Bodaniel Jeanes September 18th, 2010 @ 12:14 AM

    Rails 3 has come now. Is this now a goal for the 3.1 milestone or not at all?

  • Jeremy Kemper

    Jeremy Kemper September 18th, 2010 @ 12:46 AM

    • Milestone changed from “3.x” to “3.1”

    It'll be included as soon as someone implements it. I think we have a volunteer :)

  • Bodaniel Jeanes

    Bodaniel Jeanes September 23rd, 2010 @ 02:29 PM

    I've been spending all night trying to work on a patch but my god the AR source is a mission to get your head around. I've started writing some tests for it, at least.

    An update though is that releod has forked the old 2.3 plugin and made it (mostly) work in Rails 3. I've tested his plugin and I can definitely use nested has many throughs, but there are a few other problems (I can't use the association in a joins call, for instance). His code is at http://github.com/releod/nested_has_many_through

    I might continue trying to patch his version into Rails over the next few days, but I am definitely way out of my depths on this one. If any one who has patched AR code before wants to pair up with me or have a chat about how it should be structured, I am sure we can finally put this one in the bag (after something like 4 years??)

  • Bodaniel Jeanes

    Bodaniel Jeanes September 26th, 2010 @ 01:32 PM

    • Tag changed from “activerecord, association, has_many, nested, through” to “activerecord, association, has_many, nested, patch, through”

    I've taken the code from releod's fork and patched it into the latest Rails master.

    I've added a few rudimentary tests to cover the most basic behaviour and no extra tests have been broken by my implementation. I don't know enough about the innards of ActiveRecord so I think that the way I've done it (almost the plugin code verbatim) could rely on existing Rails functionality a bit more (for things like building the conditions, etc).

    Nonetheless it's a step in the right direction and hopefully it's enough to get someone else to look at it and either refactor the implementation a bit, or add some test cases and try to break it.

    I initially tried to implement it by doing a subclass of HasManyThroughAssociation but there is no way to detect if it's a nested association at the time the association is instantiated (because accessing the source_reflection causes a load error because the model hasn't finished loading, it seems), so I reverted to using a module that overrides some condition/join building when it detects a nested association.

    The code is at http://github.com/bjeanes/rails/tree/nested_has_many_through. Let's try to get this thing finally built into Rails (after 4 years, I think?)!

  • Bodaniel Jeanes

    Bodaniel Jeanes September 26th, 2010 @ 02:10 PM

  • Jon Leighton

    Jon Leighton September 30th, 2010 @ 05:59 PM

    It's Bo! Greetings from the past! :)

    I have actually been intending to write a patch for this for a while now. Great to see you have moved it forward some. I will look over your changes and see if I have anything to add. This message is my way of trying to ensure that I actually do that :)

    See ya

  • Bodaniel Jeanes

    Bodaniel Jeanes September 30th, 2010 @ 11:17 PM

    Hey Jon! Long time...

    I've added you to my fork to help encourage you :) I've been meaning to come back and do some more work on this but have been hammered with client duties, but hopefully this I can do so this weekend.

    As I mentioned, I took mostly plugin code verbatim. I know the plugin code tries to handle nested and non-nested has_many :throughs and is old, so probably doesn't benefit from Arel and other Rails improvements, so I adjusted it to only override the normal methods when it detects it's in a nested scenario.

    Ideally, we find the parts that are nested-specific and merge it with the existing code or strip the overridden methods down to their core and try to call back into existing rails code as much as possible.

    It also needs a lot more test cases so we should try our hardest to break the thing!

  • Jon Leighton

    Jon Leighton October 6th, 2010 @ 09:39 PM

    Hi all,

    Bo and I have been working on this in his branch.

    As part of it I needed to do some refactoring. I've submitted the actual refactoring as an independent patch over at #5763. The nested stuff will then be layered on top.

    Would appreciate some reviewers for the refactoring patch - seemed better to submit it independently than try to roll it into the nested patch, but obviously I want to rely on it in the nested patch so would like to hear if people have any problems with it.

    Cheers

  • Jon Leighton

    Jon Leighton October 19th, 2010 @ 11:15 AM

    Hi all,

    I am getting quite close to finishing this now.

    I have submitted another independent refactoring patch at #5838. It would really help if some people could review it - getting these refactorings accepted into core separately will help reduce the size of the actual nested through associations patch, which will obviously make it easier to review and get into core itself.

    Cheers

  • Jon Leighton

    Jon Leighton October 19th, 2010 @ 09:46 PM

    Hi all,

    TL;DR: Please help review this patch!

    I've got this patch to the point where I am happy with it now. There are plenty of tests and I have considered lots of different edge cases. The patch implements a lot more than the existing nested_has_many_through plugin:

    • You can go :through anything, even a has_and_belongs_to_many
    • Joining works - i.e. Foo.joins(:my_nested_association)
    • Eager loading works - i.e. Foo.includes(:my_nested_association)
    • has_one associations are supported, as well as has_many
    • Lots of edge cases dealt with, like the use of :foreign_key, :primary_key, conditions on nested source associations, etc.

    It's been a hell of a lot of work. I estimate it's taken me at least 10 full days of work, though I haven't been counting.

    In any case, what I need now is for a few of the 43 other people watching this ticket to spend a bit of time reviewing the changes so that we can get it into core. I can't do this on my own so please help!

    The first step is to get #5838 accepted. That will reduce the size of the diff and make it more obvious what it actually going on to add nested through support. So please go to #5838 and review that one.

    I have pushed a branch of Rails with #5838 applied to http://github.com/bjeanes/rails/tree/associations_refactor. I wanted to show the difference between "Rails with #5838 applied" and "Rails with nested through associations", but for some reason http://github.com/bjeanes/rails/compare/associations_refactor...nes... still shows the #5838 changes, even though they are also present in the nested_has_many_through branch. Sorry about that.

    Anyway, the second step is obviously to get the actual main patch accepted. You can compare the patch to rails master here: http://github.com/bjeanes/rails/compare/rails:master...bjeanes:nest...

    I have run the tests against sqlite3, mysql and postgres using Ruby 1.8.7 and 1.9.2. It would be helpful if you could also run the tests and verify whether they work or not (make sure to do a bundle update first).

    It would also be good if people could give the actual code a look over. Does anything stick out as potentially wrong? I will summarise the main changes (ignoring #5838):

    • New methods for dealing with nested through associations added to ThroughReflection. In particular through_reflection_chain and through_conditions. They are both pretty well commented so please check there.
    • Big changes to ActiveRecord::Associations::ThroughAssociationScope; this is where the bulk of the work to build the query for a through association happens. I should note that the code does not differentiate between a "nested" association and a "non-nested" one; the code is written in a general way which deals with both correctly.
    • Also big changes in ActiveRecord::Associations::ClassMethods::JoinAssociation (in associations.rb). This is where the work happens to generate the joins if you do Foo.joins(:bar). The main item of interest is the join_to method. This method is now written in a way which is sufficiently general to generate the correct joins for any association.
    • ThroughAssociationScope and JoinAssociation both need to handle generating table aliases, so I extracted that out into a class called ActiveRecord::Associations::AliasTracker. At the moment that lives in lib/active_record/associations/alias_tracker.rb, but I would like feedback about whether that is the "right place" for it, given that AliasTracker is not an AssociationProxy like all the other classes in that directory.
    • There are also some changes to ActiveRecord::AssociationPreload to get it working properly with nested through associations.
    • Finally, a lot of tests have been added to check all this stuff. They are mainly in test/cases/associations/nested_through_associations_test.rb, but there are some in other files. Also quite a lot of existing tests have had to be updated because I needed to change fixtures in order to test all the necessary things.

    I think that's it. Thanks for reading this far, and thanks for taking the time to review the patch, as I know you will ;) If we all pull together now we can get this long overdue feature into Rails finally.

    Cheers,
    Jon

    PS. If any core members want to chip in with any thoughts about how best to get this into core then that would be much appreciated too :)

  • Bodaniel Jeanes

    Bodaniel Jeanes October 22nd, 2010 @ 09:31 PM

    +1, obviously.

    I just pulled and tested Jon's latest changes and it's looking good and exciting!

  • Jon Leighton

    Jon Leighton October 31st, 2010 @ 11:33 AM

    • Title changed from “Support for nested has_many :through associations” to “Support for nested :through associations”

    Hi all,

    #5838 has been committed to Rails master, so the diff is now a lot easier to follow:

    http://github.com/bjeanes/rails/compare/rails:master...bjeanes:nest...

    I've also updated the branch with the latest master merged in and various breakages from that fixed.

    Is anyone able to help review this patch and get it in now?

    Cheers,
    Jon

  • Ryan Bigg

    Ryan Bigg November 17th, 2010 @ 11:44 AM

    • State changed from “incomplete” to “open”
  • Robert Pankowecki

    Robert Pankowecki November 17th, 2010 @ 11:49 AM

    I would really like to it in core rails.

  • Antonio Tapiador

    Antonio Tapiador November 18th, 2010 @ 11:08 AM

    +1 this feature in the core. I have missed it.

  • Bodaniel Jeanes

    Bodaniel Jeanes November 18th, 2010 @ 11:20 AM

    Have you guys who +1 it actually tested it? If so, Roberts and Jon's comments would be taken as +1s and we can make this a verified patch.

  • Larry Sprock

    Larry Sprock November 28th, 2010 @ 06:53 PM

    What are the blockers for adding this to core? Any official word? I would love to see this included...

  • Bodaniel Jeanes

    Bodaniel Jeanes November 29th, 2010 @ 12:12 AM

    Larry, the only thing stopping it is people not testing the plugin. We need people to pull down the rails code and run the tests and +1 to say that it's all working as it should.

    Jon just updated the patch to apply cleanly to the latest master on the weekend (since it wasn't applying cleanly anymore). If you can test it and +1 then that will go a long way to seeing this make it in officially.

  • Larry Sprock

    Larry Sprock November 29th, 2010 @ 01:00 AM

    Done. +1

    Lets get some eyes on this!

  • Keith Pitt

    Keith Pitt November 29th, 2010 @ 01:16 AM

    +1

    Solves something that should "just work"

  • Bodaniel Jeanes

    Bodaniel Jeanes November 29th, 2010 @ 01:18 AM

    • Tag changed from “activerecord, association, has_many, nested, patch, through” to “activerecord, association, has_many, nested, patch, through, verified”
  • Larry Sprock

    Larry Sprock November 29th, 2010 @ 01:20 AM

    • Tag changed from “activerecord, association, has_many, nested, patch, through, verified” to “activerecord, association, has_many, nested, patch, through”

    PS... Ran specs in a large app, currently using nested_has_many_through plugin, against this branch and all tests are passing. Nice job guys!

  • Bodaniel Jeanes

    Bodaniel Jeanes November 29th, 2010 @ 01:23 AM

    • Tag changed from “activerecord, association, has_many, nested, patch, through” to “activerecord, association, has_many, nested, patch, through, verified”

    Thanks. I don't think you meant to remove the verified tag did you?

  • Larry Sprock

    Larry Sprock November 29th, 2010 @ 01:23 AM

    Sorry about the tag change... didn't refresh before posting.

  • Paul Rosania

    Paul Rosania January 11th, 2011 @ 06:31 PM

    Has there been any movement on this? I just ran into this issue again in a new project.

  • Michel Pigassou

    Michel Pigassou January 14th, 2011 @ 06:50 PM

    Is it possible (= quick and easy) to merge it into the 3.0.3 branch of Rails? If so I'd be happy to test it :)

  • chadoh

    chadoh March 6th, 2011 @ 06:18 PM

    What is left to make this a reality? I'll help out. Seems like it's been done for a while now but not yet folded in.

  • Jon Leighton

    Jon Leighton March 6th, 2011 @ 06:34 PM

    chadoh, between the writing the original version of this patch and now I have been doing a lot of refactoring to the existing associations code in Active Record, with the eventual aim of making it more straightforward to merge this patch, such that the code will be maintainable going forward.

    I've basically completed that work, and as of Friday I got the nested :through patch back up-and-running against master.

    Returning to it now there are some refactorings that I want to do, and am in the process of doing. Once that's done Aaron Patterson says he'll review it, and it will probably be merged.

    I hope this will all be wrapped up in maybe 2-3 weeks, but we'll see. I'm slowly but steadily getting there though, and I definitely hope this will go into Rails 3.1.

    [It will definitely not go into any 3-0-stable releases.]

  • chadoh

    chadoh March 6th, 2011 @ 06:38 PM

    Fantastic. Thanks for the update and all your hard work!

  • Jon Atack

    Jon Atack March 6th, 2011 @ 07:09 PM

    Thanks, Jon! Looking forward to this.

  • Aaron Patterson

    Aaron Patterson March 22nd, 2011 @ 05:17 PM

    • State changed from “open” to “committed”
    • Assigned user changed from “Pratik” to “Jon Leighton”

    Merged to master. Closing.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Tags

Referenced by

Pages