Hi all, In October I worked on a patch for a long-requested feature: associations that can go :through anything, including other :through associations. The Lighthouse ticket is here: https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1152 I believe this is a robust, well-tested patch. I want to get it merged, but there has been very little response to the patch on Lighthouse. I should probably have posted on here before now - but here I am now anyway. Can anyone help verify/review this patch? Any suggestions about how best to get it accepted by the core team? Also, can somebody with the relevant permissions changes the status from "incomplete" to "open" please? I am actively maintaining the branch and merging in the latest master from time to time. I will also happily deal with any issues which arise as a result of this patch. Thanks, Jon Leighton -- http://jonathanleighton.com/
Jon, I would make sure this currently merges into master and send a pull request on github with the lighthouse ticket in the comments... Need get more eyes on it. On Nov 17, 3:40 am, Jon Leighton <j...@jonathanleighton.com> wrote:> Hi all, > > In October I worked on a patch for a long-requested feature: > associations that can go :through anything, including other :through > associations. > > The Lighthouse ticket is here:https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1152 > > I believe this is a robust, well-tested patch. I want to get it merged, > but there has been very little response to the patch on Lighthouse. I > should probably have posted on here before now - but here I am now > anyway. > > Can anyone help verify/review this patch? Any suggestions about how best > to get it accepted by the core team? > > Also, can somebody with the relevant permissions changes the status from > "incomplete" to "open" please? I am actively maintaining the branch and > merging in the latest master from time to time. I will also happily deal > with any issues which arise as a result of this patch. > > Thanks, > Jon Leighton > > --http://jonathanleighton.com/ > > signature.asc > < 1KViewDownload-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
The patch has now been "verified" on Lighthouse. I.e. 3 people have tested and +1ed it. I''ve done a pull request here: https://github.com/rails/rails/pull/121 I merged latest master over the weekend and will continue to merge as necessary to keep it up to date. Jon On Nov 29, 1:42 am, lardawge <larda...@gmail.com> wrote:> Jon, > I would make sure this currently merges into master and send a pull > request on github with the lighthouse ticket in the comments... Need > get more eyes on it. > > On Nov 17, 3:40 am, Jon Leighton <j...@jonathanleighton.com> wrote: > > > Hi all, > > > In October I worked on a patch for a long-requested feature: > > associations that can go :through anything, including other :through > > associations. > > > The Lighthouse ticket is here:https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1152 > > > I believe this is a robust, well-tested patch. I want to get it merged, > > but there has been very little response to the patch on Lighthouse. I > > should probably have posted on here before now - but here I am now > > anyway. > > > Can anyone help verify/review this patch? Any suggestions about how best > > to get it accepted by the core team? > > > Also, can somebody with the relevant permissions changes the status from > > "incomplete" to "open" please? I am actively maintaining the branch and > > merging in the latest master from time to time. I will also happily deal > > with any issues which arise as a result of this patch. > > > Thanks, > > Jon Leighton > > > --http://jonathanleighton.com/ > > > signature.asc > > < 1KViewDownload-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Hi all, Jose has responded on the pull request: ==========================================Hey mate, thanks for the pull request. But could you please send this request to Rails mailing list and get feedback from more people? At first, I am -1 on this feature. Rails has_many :through, without being nested, already have a few limitations that I really would like to see fixed before adding more complexity. For example, in the case "User has many groups through subscriptions", the following does not work: group = user.groups.build group.save As it does not save the associated object subscriptions. Calling .create() works as expected though. The lack of the ability to support a dependent option (like :destroy, :delete) for the associated record is also another limitation (currently we have dangling records). We can find others examples in LH. ========================================== I wanted to respond here to prevent the discussion being split over multiple places: I see the concern. Though we should be clear that nested through associations will only ever be read-only, as writing to them would not make any sense (not enough information to determine correct join records). What I would propose is that I do an audit of tickets on LH that are related to has many through. I suspect some of them will be solved by my patch (as it rewrites lots of HMT code). I could identify others which are still an issue. Then maybe we could identify which other tickets should be fixed before this nested patch goes in. I''m happy to look at other tickets as a stepping stone to getting this patch in. However it would be good if people could be up front and say now if there are general concerns with this patch in principle, or whether concerns are just around not introducing more bugs in HMT given the number of existing bugs. Hope that makes sense. Cheers, Jon On Mon, 2010-11-29 at 02:27 -0800, Jon Leighton wrote:> The patch has now been "verified" on Lighthouse. I.e. 3 people have > tested and +1ed it. I''ve done a pull request here: https://github.com/rails/rails/pull/121 > > I merged latest master over the weekend and will continue to merge as > necessary to keep it up to date. > > Jon > > On Nov 29, 1:42 am, lardawge <larda...@gmail.com> wrote: > > Jon, > > I would make sure this currently merges into master and send a pull > > request on github with the lighthouse ticket in the comments... Need > > get more eyes on it. > > > > On Nov 17, 3:40 am, Jon Leighton <j...@jonathanleighton.com> wrote: > > > > > Hi all, > > > > > In October I worked on a patch for a long-requested feature: > > > associations that can go :through anything, including other :through > > > associations. > > > > > The Lighthouse ticket is here:https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1152 > > > > > I believe this is a robust, well-tested patch. I want to get it merged, > > > but there has been very little response to the patch on Lighthouse. I > > > should probably have posted on here before now - but here I am now > > > anyway. > > > > > Can anyone help verify/review this patch? Any suggestions about how best > > > to get it accepted by the core team? > > > > > Also, can somebody with the relevant permissions changes the status from > > > "incomplete" to "open" please? I am actively maintaining the branch and > > > merging in the latest master from time to time. I will also happily deal > > > with any issues which arise as a result of this patch. > > > > > Thanks, > > > Jon Leighton > > > > > --http://jonathanleighton.com/ > > > > > signature.asc > > > < 1KViewDownload >-- http://jonathanleighton.com/
I''m very much in favor of hmt working to an infinite depth. From my own experience as well as responding to others, I know it''s something that people just expect to work. I''m also concerned about bugs, but I like your suggestion Jon. On Dec 1, 2010 12:18 PM, "Jon Leighton" <j@jonathanleighton.com> wrote:> Hi all, > > Jose has responded on the pull request: > > ==========================================> Hey mate, thanks for the pull request. But could you please send this > request to Rails mailing list and get feedback from more people? > > At first, I am -1 on this feature. Rails has_many :through, without > being nested, already have a few limitations that I really would like to > see fixed before adding more complexity. For example, in the case "User > has many groups through subscriptions", the following does not work: > > group = user.groups.build > group.save > > As it does not save the associated object subscriptions. > Calling .create() works as expected though. The lack of the ability to > support a dependent option (like :destroy, :delete) for the associated > record is also another limitation (currently we have dangling records). > We can find others examples in LH. > ==========================================> > I wanted to respond here to prevent the discussion being split over > multiple places: > > I see the concern. Though we should be clear that nested through > associations will only ever be read-only, as writing to them would not > make any sense (not enough information to determine correct join > records). > > What I would propose is that I do an audit of tickets on LH that are > related to has many through. I suspect some of them will be solved by my > patch (as it rewrites lots of HMT code). I could identify others which > are still an issue. Then maybe we could identify which other tickets > should be fixed before this nested patch goes in. I''m happy to look at > other tickets as a stepping stone to getting this patch in. > > However it would be good if people could be up front and say now if > there are general concerns with this patch in principle, or whether > concerns are just around not introducing more bugs in HMT given the > number of existing bugs. > > Hope that makes sense. > > Cheers, > Jon > > On Mon, 2010-11-29 at 02:27 -0800, Jon Leighton wrote: >> The patch has now been "verified" on Lighthouse. I.e. 3 people have >> tested and +1ed it. I''ve done a pull request here:https://github.com/rails/rails/pull/121>> >> I merged latest master over the weekend and will continue to merge as >> necessary to keep it up to date. >> >> Jon >> >> On Nov 29, 1:42 am, lardawge <larda...@gmail.com> wrote: >> > Jon, >> > I would make sure this currently merges into master and send a pull >> > request on github with the lighthouse ticket in the comments... Need >> > get more eyes on it. >> > >> > On Nov 17, 3:40 am, Jon Leighton <j...@jonathanleighton.com> wrote: >> > >> > > Hi all, >> > >> > > In October I worked on a patch for a long-requested feature: >> > > associations that can go :through anything, including other :through >> > > associations. >> > >> > > The Lighthouse ticket is here:https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1152>> > >> > > I believe this is a robust, well-tested patch. I want to get itmerged,>> > > but there has been very little response to the patch on Lighthouse. I >> > > should probably have posted on here before now - but here I am now >> > > anyway. >> > >> > > Can anyone help verify/review this patch? Any suggestions about howbest>> > > to get it accepted by the core team? >> > >> > > Also, can somebody with the relevant permissions changes the statusfrom>> > > "incomplete" to "open" please? I am actively maintaining the branchand>> > > merging in the latest master from time to time. I will also happilydeal>> > > with any issues which arise as a result of this patch. >> > >> > > Thanks, >> > > Jon Leighton >> > >> > > --http://jonathanleighton.com/ >> > >> > > signature.asc >> > > < 1KViewDownload >> > > -- > http://jonathanleighton.com/-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
+1 for sorting bugs out and commiting this. HMT just helps doing some associations where it would be hard to do. Even if it is not fool proof, allowing us to associate models further where needed is welcomed. I have been in a couple of projects where coding would be easier with this patch. On Wed, Dec 1, 2010 at 23:44, Jason King <jk@handle.it> wrote:> I''m very much in favor of hmt working to an infinite depth. From my own > experience as well as responding to others, I know it''s something that > people just expect to work. > > I''m also concerned about bugs, but I like your suggestion Jon. > On Dec 1, 2010 12:18 PM, "Jon Leighton" <j@jonathanleighton.com> wrote: > > Hi all, > > > > Jose has responded on the pull request: > > > > ==========================================> > Hey mate, thanks for the pull request. But could you please send this > > request to Rails mailing list and get feedback from more people? > > > > At first, I am -1 on this feature. Rails has_many :through, without > > being nested, already have a few limitations that I really would like to > > see fixed before adding more complexity. For example, in the case "User > > has many groups through subscriptions", the following does not work: > > > > group = user.groups.build > > group.save > > > > As it does not save the associated object subscriptions. > > Calling .create() works as expected though. The lack of the ability to > > support a dependent option (like :destroy, :delete) for the associated > > record is also another limitation (currently we have dangling records). > > We can find others examples in LH. > > ==========================================> > > > I wanted to respond here to prevent the discussion being split over > > multiple places: > > > > I see the concern. Though we should be clear that nested through > > associations will only ever be read-only, as writing to them would not > > make any sense (not enough information to determine correct join > > records). > > > > What I would propose is that I do an audit of tickets on LH that are > > related to has many through. I suspect some of them will be solved by my > > patch (as it rewrites lots of HMT code). I could identify others which > > are still an issue. Then maybe we could identify which other tickets > > should be fixed before this nested patch goes in. I''m happy to look at > > other tickets as a stepping stone to getting this patch in. > > > > However it would be good if people could be up front and say now if > > there are general concerns with this patch in principle, or whether > > concerns are just around not introducing more bugs in HMT given the > > number of existing bugs. > > > > Hope that makes sense. > > > > Cheers, > > Jon > > > > On Mon, 2010-11-29 at 02:27 -0800, Jon Leighton wrote: > >> The patch has now been "verified" on Lighthouse. I.e. 3 people have > >> tested and +1ed it. I''ve done a pull request here: > https://github.com/rails/rails/pull/121 > >> > >> I merged latest master over the weekend and will continue to merge as > >> necessary to keep it up to date. > >> > >> Jon > >> > >> On Nov 29, 1:42 am, lardawge <larda...@gmail.com> wrote: > >> > Jon, > >> > I would make sure this currently merges into master and send a pull > >> > request on github with the lighthouse ticket in the comments... Need > >> > get more eyes on it. > >> > > >> > On Nov 17, 3:40 am, Jon Leighton <j...@jonathanleighton.com> wrote: > >> > > >> > > Hi all, > >> > > >> > > In October I worked on a patch for a long-requested feature: > >> > > associations that can go :through anything, including other :through > >> > > associations. > >> > > >> > > The Lighthouse ticket is here: > https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1152 > >> > > >> > > I believe this is a robust, well-tested patch. I want to get it > merged, > >> > > but there has been very little response to the patch on Lighthouse. > I > >> > > should probably have posted on here before now - but here I am now > >> > > anyway. > >> > > >> > > Can anyone help verify/review this patch? Any suggestions about how > best > >> > > to get it accepted by the core team? > >> > > >> > > Also, can somebody with the relevant permissions changes the status > from > >> > > "incomplete" to "open" please? I am actively maintaining the branch > and > >> > > merging in the latest master from time to time. I will also happily > deal > >> > > with any issues which arise as a result of this patch. > >> > > >> > > Thanks, > >> > > Jon Leighton > >> > > >> > > --http://jonathanleighton.com/ > >> > > >> > > signature.asc > >> > > < 1KViewDownload > >> > > > > -- > > http://jonathanleighton.com/ > > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To post to this group, send email to rubyonrails-core@googlegroups.com. > To unsubscribe from this group, send email to > rubyonrails-core+unsubscribe@googlegroups.com<rubyonrails-core%2Bunsubscribe@googlegroups.com> > . > For more options, visit this group at > http://groups.google.com/group/rubyonrails-core?hl=en. >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
+1 for the usefulness of Jon''s patch. During a previous project I was very disappointed to find that Rails didn''t support nested :through associations. Besides my own experiences, a quick search on Google for "nested has many through associations" shows a litany of blog posts and discussions on hacks to simulate the functionality. For example: http://justinfrench.com/notebook/nested-hasmany-through-associations-in-rails http://geoff.evason.name/2010/04/23/nested-has_many-through-in-rails-or-how-to-do-a-3-table-join/ http://kconrails.com/2010/01/28/nesting-has_many-through-relationships-in-ruby-on-rails/ I agree too with José Valim''s comments that has_many :through needs to be fixed. The surprising inconsistencies between has_many :through and has_many adds cognitive load to developers and, for me at least, the lack of dependent options has pushed me into using has_many over has_many :through quite a bit. If Jon''s code and proposed audit can help iron out these issues then all the better! On Dec 2, 2:01 am, Lunks <pnascime...@gmail.com> wrote:> +1 for sorting bugs out and commiting this. HMT just helps doing some > associations where it would be hard to do. Even if it is not fool proof, > allowing us to associate models further where needed is welcomed. I have > been in a couple of projects where coding would be easier with this patch. > > On Wed, Dec 1, 2010 at 23:44, Jason King <j...@handle.it> wrote: > > I''m very much in favor of hmt working to an infinite depth. From my own > > experience as well as responding to others, I know it''s something that > > people just expect to work. > > > I''m also concerned about bugs, but I like your suggestion Jon. > > On Dec 1, 2010 12:18 PM, "Jon Leighton" <j...@jonathanleighton.com> wrote: > > > Hi all, > > > > Jose has responded on the pull request: > > > > ==========================================> > > Hey mate, thanks for the pull request. But could you please send this > > > request to Rails mailing list and get feedback from more people? > > > > At first, I am -1 on this feature. Rails has_many :through, without > > > being nested, already have a few limitations that I really would like to > > > see fixed before adding more complexity. For example, in the case "User > > > has many groups through subscriptions", the following does not work: > > > > group = user.groups.build > > > group.save > > > > As it does not save the associated object subscriptions. > > > Calling .create() works as expected though. The lack of the ability to > > > support a dependent option (like :destroy, :delete) for the associated > > > record is also another limitation (currently we have dangling records). > > > We can find others examples in LH. > > > ==========================================> > > > I wanted to respond here to prevent the discussion being split over > > > multiple places: > > > > I see the concern. Though we should be clear that nested through > > > associations will only ever be read-only, as writing to them would not > > > make any sense (not enough information to determine correct join > > > records). > > > > What I would propose is that I do an audit of tickets on LH that are > > > related to has many through. I suspect some of them will be solved by my > > > patch (as it rewrites lots of HMT code). I could identify others which > > > are still an issue. Then maybe we could identify which other tickets > > > should be fixed before this nested patch goes in. I''m happy to look at > > > other tickets as a stepping stone to getting this patch in. > > > > However it would be good if people could be up front and say now if > > > there are general concerns with this patch in principle, or whether > > > concerns are just around not introducing more bugs in HMT given the > > > number of existing bugs. > > > > Hope that makes sense. > > > > Cheers, > > > Jon > > > > On Mon, 2010-11-29 at 02:27 -0800, Jon Leighton wrote: > > >> The patch has now been "verified" on Lighthouse. I.e. 3 people have > > >> tested and +1ed it. I''ve done a pull request here: > >https://github.com/rails/rails/pull/121 > > > >> I merged latest master over the weekend and will continue to merge as > > >> necessary to keep it up to date. > > > >> Jon > > > >> On Nov 29, 1:42 am, lardawge <larda...@gmail.com> wrote: > > >> > Jon, > > >> > I would make sure this currently merges into master and send a pull > > >> > request on github with the lighthouse ticket in the comments... Need > > >> > get more eyes on it. > > > >> > On Nov 17, 3:40 am, Jon Leighton <j...@jonathanleighton.com> wrote: > > > >> > > Hi all, > > > >> > > In October I worked on a patch for a long-requested feature: > > >> > > associations that can go :through anything, including other :through > > >> > > associations. > > > >> > > The Lighthouse ticket is here: > >https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1152 > > > >> > > I believe this is a robust, well-tested patch. I want to get it > > merged, > > >> > > but there has been very little response to the patch on Lighthouse. > > I > > >> > > should probably have posted on here before now - but here I am now > > >> > > anyway. > > > >> > > Can anyone help verify/review this patch? Any suggestions about how > > best > > >> > > to get it accepted by the core team? > > > >> > > Also, can somebody with the relevant permissions changes the status > > from > > >> > > "incomplete" to "open" please? I am actively maintaining the branch > > and > > >> > > merging in the latest master from time to time. I will also happily > > deal > > >> > > with any issues which arise as a result of this patch. > > > >> > > Thanks, > > >> > > Jon Leighton > > > >> > > --http://jonathanleighton.com/ > > > >> > > signature.asc > > >> > > < 1KViewDownload > > > > -- > > >http://jonathanleighton.com/ > > > -- > > You received this message because you are subscribed to the Google Groups > > "Ruby on Rails: Core" group. > > To post to this group, send email to rubyonrails-core@googlegroups.com. > > To unsubscribe from this group, send email to > > rubyonrails-core+unsubscribe@googlegroups.com<rubyonrails-core%2Bunsubscribe@googlegroups.com> > > . > > For more options, visit this group at > >http://groups.google.com/group/rubyonrails-core?hl=en. > >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.