Support for nested :through associations

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?

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.

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.

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

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,

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

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.

Is someone working to turn the plugin into a patch?

+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....

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.

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

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?

Marcos A Piccinini,

Unfortunately, it is not working on Rails 3 too.

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

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??)

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

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

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!

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

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

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 :)

+1, obviously.

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

I would really like to it in core rails.

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

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.

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

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.

Done. +1

Lets get some eyes on this!

+1

Solves something that should "just work"

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

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

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 :)

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.

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.]

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

Thanks, Jon! Looking forward to this.