Fixed has_many :through to include :conditions set on the :through association. closes #4020 [jonathan@bluewire.net.nz]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3958 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

activerecord/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Fixed has_many :through to include :conditions set on the :through association. closes #4020 [jonathan@bluewire.net.nz]
+
 * Fix that has_many :through honors the foreign key set by the belongs_to association in the join model (closes #4259) [andylien@gmail.com / Rick]
 
 * SQL Server adapter gets some love #4298 [rtomayko@gmail.com]

activerecord/lib/active_record/associations.rb

@@ -1157,7 +1157,7 @@ module ActiveRecord
 
           class JoinBase
             attr_reader :active_record
-            delegate    :table_name, :column_names, :primary_key, :reflections, :to => :active_record
+            delegate    :table_name, :column_names, :primary_key, :reflections, :sanitize_sql, :to => :active_record
 
             def initialize(active_record)
               @active_record = active_record
@@ -1244,7 +1244,7 @@ module ActiveRecord
                   case
                     when reflection.macro == :has_many && reflection.options[:through]
                       through_reflection = parent.active_record.reflect_on_association(reflection.options[:through])
-                      through_conditions = through_reflection.options[:conditions] ? "AND #{eval("%(#{through_reflection.active_record.send :sanitize_sql, through_reflection.options[:conditions]})")}" : ''
+                      through_conditions = through_reflection.options[:conditions] ? "AND #{interpolate_sql(sanitize_sql(through_reflection.options[:conditions]))}" : ''
                       if through_reflection.options[:as] # has_many :through against a polymorphic join
                         polymorphic_foreign_key  = through_reflection.options[:as].to_s + '_id'
                         polymorphic_foreign_type = through_reflection.options[:as].to_s + '_type'
@@ -1296,7 +1296,7 @@ module ActiveRecord
                 aliased_table_name, 
                 reflection.active_record.connection.quote_column_name(reflection.active_record.inheritance_column), 
                 klass.quote(klass.name)] if sti?
-              join << "AND #{eval("%(#{reflection.active_record.send :sanitize_sql, reflection.options[:conditions]})")} " if reflection.options[:conditions]
+              join << "AND #{interpolate_sql(sanitize_sql(reflection.options[:conditions]))} " if reflection.options[:conditions]
               join
             end
             
@@ -1316,6 +1316,10 @@ module ActiveRecord
               def table_name_and_alias
                 table_alias_for table_name, @aliased_table_name
               end
+
+              def interpolate_sql(sql)
+                instance_eval("%@#{sql.gsub('@', '\@')}@")
+              end
           end
         end
     end

activerecord/lib/active_record/associations/has_many_through_association.rb

@@ -40,6 +40,21 @@ module ActiveRecord
       end
 
       protected
+        def through_reflection
+          unless @through_reflection ||= @owner.class.reflections[@reflection.options[:through]]
+            raise ActiveRecordError, "Could not find the association '#{@reflection.options[:through]}' in model #{@reflection.klass}"
+          end
+          @through_reflection
+        end
+
+        def source_reflection
+          @source_reflection_name ||= @reflection.name.to_s.singularize.to_sym
+          unless @source_reflection ||= through_reflection.klass.reflect_on_association(@source_reflection_name)
+            raise ActiveRecordError, "Could not find the source association '#{@source_reflection_name}' in model #{@through_reflection.klass}"
+          end
+          @source_reflection
+        end
+
         def method_missing(method, *args, &block)
           if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method))
             super
@@ -61,12 +76,8 @@ module ActiveRecord
         end
 
         def construct_conditions
-          unless through_reflection = @owner.class.reflections[@reflection.options[:through]]
-            raise ActiveRecordError, "Could not find the association '#{@reflection.options[:through]}' in model #{@reflection.klass}"
-          end
-          
           # Get the actual primary key of the belongs_to association that the reflection is going through
-          source_primary_key = through_reflection.klass.reflect_on_association(@reflection.name.to_s.singularize.to_sym).primary_key_name
+          source_primary_key = source_reflection.primary_key_name
           
           if through_reflection.options[:as]
             conditions = 
@@ -119,6 +130,9 @@ module ActiveRecord
           end
         end
         
+        def sql_conditions
+          @conditions ||= interpolate_sql(@reflection.active_record.send(:sanitize_sql, through_reflection.options[:conditions])) if through_reflection.options[:conditions]
+        end
     end
   end
 end

activerecord/test/associations_join_model_test.rb

@@ -216,6 +216,11 @@ class AssociationsJoinModelTest < Test::Unit::TestCase
     assert_raises (ActiveRecord::ActiveRecordError) { authors(:david).nothings }
   end
 
+  def test_has_many_through_join_model_with_conditions
+    assert_equal [], posts(:welcome).invalid_taggings
+    assert_equal [], posts(:welcome).invalid_tags
+  end
+
   private
     # create dynamic Post models to allow different dependency options
     def find_post_with_dependency(post_id, association, association_name, dependency)

activerecord/test/fixtures/author.rb

@@ -4,20 +4,21 @@ class Author < ActiveRecord::Base
   has_many :posts_with_categories, :include => :categories, :class_name => "Post"
   has_many :posts_with_comments_and_categories, :include => [ :comments, :categories ], :order => "posts.id", :class_name => "Post"
 
-  has_many :special_posts, :class_name => "Post"
-  has_many :hello_posts, :class_name => "Post", :conditions=>"\#{aliased_table_name}.body = 'hello'"
-  has_many :nonexistent_posts, :class_name => "Post", :conditions=>"\#{aliased_table_name}.body = 'nonexistent'"
+  has_many :special_posts,        :class_name => "Post"
+  has_many :hello_posts,          :class_name => "Post", :conditions=>"\#{aliased_table_name}.body = 'hello'"
+  has_many :nonexistent_posts,    :class_name => "Post", :conditions=>"\#{aliased_table_name}.body = 'nonexistent'"
   has_many :posts_with_callbacks, :class_name => "Post", :before_add => :log_before_adding,
-           :after_add => :log_after_adding, :before_remove => :log_before_removing,
-           :after_remove => :log_after_removing
+           :after_add     => :log_after_adding, 
+           :before_remove => :log_before_removing,
+           :after_remove  => :log_after_removing
   has_many :posts_with_proc_callbacks, :class_name => "Post",
-           :before_add => Proc.new {|o, r| o.post_log << "before_adding#{r.id}"},
-           :after_add => Proc.new {|o, r| o.post_log << "after_adding#{r.id}"},
+           :before_add    => Proc.new {|o, r| o.post_log << "before_adding#{r.id}"},
+           :after_add     => Proc.new {|o, r| o.post_log << "after_adding#{r.id}"},
            :before_remove => Proc.new {|o, r| o.post_log << "before_removing#{r.id}"},
-           :after_remove => Proc.new {|o, r| o.post_log << "after_removing#{r.id}"}
+           :after_remove  => Proc.new {|o, r| o.post_log << "after_removing#{r.id}"}
   has_many :posts_with_multiple_callbacks, :class_name => "Post",
            :before_add => [:log_before_adding, Proc.new {|o, r| o.post_log << "before_adding_proc#{r.id}"}],
-           :after_add => [:log_after_adding, Proc.new {|o, r| o.post_log << "after_adding_proc#{r.id}"}]
+           :after_add  => [:log_after_adding,  Proc.new {|o, r| o.post_log << "after_adding_proc#{r.id}"}]
   has_many :unchangable_posts, :class_name => "Post", :before_add => :raise_exception, :after_add => :log_after_adding
 
   has_many :categorizations

activerecord/test/fixtures/post.rb

@@ -23,9 +23,11 @@ class Post < ActiveRecord::Base
   has_many :taggings, :as => :taggable
   has_many :tags, :through => :taggings
   has_many :super_tags, :through => :taggings
-
   has_one :tagging, :as => :taggable
 
+  has_many :invalid_taggings, :as => :taggable, :class_name => "Tagging", :conditions => 'taggings.id < 0'
+  has_many :invalid_tags, :through => :invalid_taggings, :class_name => "Tag"
+
   has_many :categorizations, :foreign_key => :category_id
   has_many :authors, :through => :categorizations
 

activerecord/test/fixtures/tagging.rb

@@ -1,5 +1,6 @@
 class Tagging < ActiveRecord::Base
   belongs_to :tag
-  belongs_to :super_tag, :class_name => 'Tag', :foreign_key => 'super_tag_id'
+  belongs_to :super_tag,   :class_name => 'Tag', :foreign_key => 'super_tag_id'
+  belongs_to :invalid_tag, :class_name => 'Tag', :foreign_key => 'tag_id'
   belongs_to :taggable, :polymorphic => true, :counter_cache => true
 end