Add :joins to an array and let AR handle making them unique

lib/searchlogic/named_scopes/associations.rb

@@ -88,7 +88,7 @@ def association_condition_options(association_name, association_condition, args)
             # The underlying condition doesn't require any parameters, so let's just create a simple
             # named scope that is based on a hash.
             options = scope.proxy_options
-            add_left_outer_join(options, association)
+            add_left_outer_joins(options, association)
             options
           else
             # The underlying condition requires parameters, let's match the parameters it requires
@@ -110,7 +110,7 @@ def association_condition_options(association_name, association_condition, args)
             eval <<-"end_eval"
               searchlogic_lambda(:#{scope_options.searchlogic_arg_type}) { |#{proc_args.join(",")}|
                 options = association.klass.named_scope_options(association_condition).call(#{proc_args.join(",")})
-                add_left_outer_join(options, association)
+                add_left_outer_joins(options, association)
                 options
               }
             end_eval
@@ -130,11 +130,10 @@ def association_condition_options(association_name, association_condition, args)
         # JoinDependency which creates a LEFT OUTER JOIN, which is what we want.
         #
         # The code below was extracted out of AR's add_joins! method and then modified.
-        def add_left_outer_join(options, association)
-          join = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, association.name, nil).join_associations.collect { |assoc| assoc.association_join }.join
-          options[:joins] ||= ""
-          next if options[:joins].include?(join)
-          options[:joins] = join.strip + (options[:joins].blank? ? "" : " #{options[:joins]}")
+        def add_left_outer_joins(options, association)
+          join = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, association.name, nil).join_associations.collect { |assoc| assoc.association_join }.join.strip
+          options[:joins] ||= []
+          options[:joins].unshift(join)
         end
     end
   end

lib/searchlogic/search.rb

@@ -89,6 +89,8 @@ def type_cast(value, type)
         when Array
           value.collect { |v| type_cast(v, type) }
         else
+          # Let's leverage ActiveRecord's type casting, so that casting is consistent
+          # with the other models.
           column_for_type_cast = ActiveRecord::ConnectionAdapters::Column.new("", nil)
           column_for_type_cast.instance_variable_set(:@type, type)
           column_for_type_cast.type_cast(value)

spec/named_scopes/associations_spec.rb

@@ -2,8 +2,8 @@
 
 describe "Associations" do
   before(:each) do
-    @users_join_sql = "LEFT OUTER JOIN \"users\" ON users.company_id = companies.id"
-    @orders_join_sql = "LEFT OUTER JOIN \"users\" ON users.company_id = companies.id LEFT OUTER JOIN \"orders\" ON orders.user_id = users.id"
+    @users_join_sql = ["LEFT OUTER JOIN \"users\" ON users.company_id = companies.id"]
+    @orders_join_sql = ["LEFT OUTER JOIN \"users\" ON users.company_id = companies.id", "LEFT OUTER JOIN \"orders\" ON orders.user_id = users.id"]
   end
 
   it "should create a named scope" do
@@ -86,6 +86,14 @@
     Company.users_orders_total_gt(10).users_orders_taxes_lt(5).ascend_by_users_orders_total.all.should == Company.all
   end
 
+  it "should not create the same join twice when traveling through the duplicate join" do
+    Company.users_username_like("bjohnson").users_orders_total_gt(100).all.should == Company.all
+  end
+
+  it "should not create the same join twice when traveling through the duplicate join 2" do
+    Company.users_orders_total_gt(100).users_orders_line_items_price_gt(20).all.should == Company.all
+  end
+
   it "should allow the use of :include when a join was created" do
     company = Company.create
     user = company.users.create
@@ -99,4 +107,18 @@
     order = user.orders.create(:total => 20, :taxes => 3)
     Company.users_orders_total_gt(10).users_orders_taxes_lt(5).ascend_by_users_orders_total.all(:include => {:users => :orders}).should == Company.all
   end
+
+  it "should allow the use of :include when traveling through the duplicate join" do
+    company = Company.create
+    user = company.users.create(:username => "bjohnson")
+    order = user.orders.create(:total => 20, :taxes => 3)
+    Company.users_username_like("bjohnson").users_orders_taxes_lt(5).ascend_by_users_orders_total.all(:include => :users).should == Company.all
+  end
+
+  it "should allow the use of deep :include when traveling through the duplicate join" do
+    company = Company.create
+    user = company.users.create(:username => "bjohnson")
+    order = user.orders.create(:total => 20, :taxes => 3)
+    Company.ascend_by_users_orders_total.users_orders_taxes_lt(50).all(:include => {:users => :orders}).should == Company.all
+  end
 end

spec/spec_helper.rb

@@ -29,6 +29,13 @@
     t.float :taxes
     t.float :total
   end
+
+  create_table :line_items do |t|
+    t.datetime :created_at
+    t.datetime :updated_at
+    t.integer :order_id
+    t.float :price
+  end
 end
 
 $LOAD_PATH.unshift(File.dirname(__FILE__))
@@ -48,16 +55,23 @@ class User < ActiveRecord::Base
 
     class Order < ActiveRecord::Base
       belongs_to :user
+      has_many :line_items, :dependent => :destroy
+    end
+
+    class LineItem < ActiveRecord::Base
+      belongs_to :order
     end
 
     Company.destroy_all
     User.destroy_all
     Order.destroy_all
+    LineItem.destroy_all
   end
 
   config.after(:each) do
     Object.send(:remove_const, :Company)
     Object.send(:remove_const, :User)
     Object.send(:remove_const, :Order)
+    Object.send(:remove_const, :LineItem)
   end
 end