Looks good. Patch is missing tests though. You should also open a lighthouse ticket and upload the tested patch there using git format-patch - http://guides.rubyonrails.org/contributing_to_rails.html#create-a-patch
On Tue, Dec 22, 2009 at 1:50 PM, donV <[email protected]> wrote: > From the RoR docs at > > http://api.rubyonrails.org/classes/ActiveRecord/Base.html > > :joins - Either an SQL fragment for additional joins like "LEFT JOIN > comments ON comments.post_id = id" (rarely needed), named associations > in the same form used for the :include option, which will perform > anINNER JOIN on the associated table(s), or an array containing a > mixture of both strings and named associations. > > When using the "array containing a mixture of both strings and named > associations", for example > > :joins => ['LEFT JOIN foos ON foo_id = foos.id', :bar] > > the following error is raised: > > Association named 'LEFT JOIN foos ON foo_id = foos.id' was not found; > perhaps you misspelled it? > > The following patch allows arrays containing a mix of strings and > symbols. Please include this in ActiveRecord 2.3.6. > > Index: base.rb > =================================================================== > --- base.rb (revision 13824) > +++ base.rb (working copy) > @@ -1782,15 +1782,20 @@ > scope = scope(:find) if :auto == scope > merged_joins = scope && scope[:joins] && joins ? merge_joins > (scope[:joins], joins) : (joins || scope && scope[:joins]) > case merged_joins > - when Symbol, Hash, Array > - if array_of_strings?(merged_joins) > - sql << merged_joins.join(' ') + " " > - else > - join_dependency = > ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new > (self, merged_joins, nil) > - sql << " #{join_dependency.join_associations.collect { | > assoc| assoc.association_join }.join} " > - end > + when Array > + strings = merged_joins.select{|j| j.is_a? String} > + sql << strings.join(' ') + " " > + symbols = merged_joins - strings > + join_dependency = > ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new > (self, symbols, nil) > + sql << " #{join_dependency.join_associations.collect { | > assoc| assoc.association_join }.join} " > + when Symbol, Hash > + join_dependency = > ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new > (self, merged_joins, nil) > + sql << " #{join_dependency.join_associations.collect { | > assoc| assoc.association_join }.join} " > when String > sql << " #{merged_joins} " > + when NilClass > + else > + raise "Illegal joins value: #{merged_joins.class.name}" > end > end > > -- > > 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 [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/rubyonrails-core?hl=en. > > > -- Cheers! - Pratik http://m.onkey.org | http://twitter.com/lifo -- 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 [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
