Added descriptive error messages for invalid has_many :through associations: going through :has_one or :has_and_belongs_to_many [Rick]

Added support for going through a polymorphic has_many association: (closes #4401) [Rick]


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

3 files changed, 29 insertions, 10 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 3ad0eeecc7..5319fa39e8 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -44,6 +44,18 @@ module ActiveRecord
     end
   end
 
+  class HasManyThroughSourceAssociationMacroError < ActiveRecordError #:nodoc
+    def initialize(reflection)
+      @reflection         = reflection
+      @through_reflection = reflection.through_reflection
+      @source_reflection  = reflection.source_reflection
+    end
+    
+    def message
+      "Invalid source reflection macro :#{@source_reflection.macro}#{" :through" if @source_reflection.options[:through]} for has_many #{@reflection.name.inspect}, :through => #{@through_reflection.name.inspect}.  Use :source to specify the source reflection."
+    end
+  end
+
   class EagerLoadPolymorphicError < ActiveRecordError #:nodoc:
     def initialize(reflection)
       @reflection = reflection
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index dafae89861..44054b42e1 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -63,17 +63,12 @@ module ActiveRecord
           )
         end
 
-        def construct_conditions          
+        def construct_conditions
           conditions = if @reflection.through_reflection.options[:as]
               "#{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.options[:as]}_id = #{@owner.quoted_id} " + 
               "AND #{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.options[:as]}_type = #{@owner.class.quote @owner.class.base_class.name.to_s}"
           else
-            case @reflection.source_reflection.macro
-              when :belongs_to, :has_many
-                "#{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.primary_key_name} = #{@owner.quoted_id}"
-              else
-                raise ActiveRecordError, "Invalid source reflection macro :#{@reflection.source_reflection.macro} for has_many #{@reflection.name}, :through => #{@reflection.through_reflection.name}.  Use :source to specify the source reflection."
-            end
+            "#{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.primary_key_name} = #{@owner.quoted_id}"
           end
           conditions << " AND (#{sql_conditions})" if sql_conditions
           
@@ -88,19 +83,27 @@ module ActiveRecord
           selected = custom_select || @reflection.options[:select] || "#{@reflection.table_name}.*"          
         end
         
-        def construct_joins(custom_joins = nil)
+        def construct_joins(custom_joins = nil)          
+          polymorphic_join = nil
           if @reflection.through_reflection.options[:as] || @reflection.source_reflection.macro == :belongs_to
             reflection_primary_key = @reflection.klass.primary_key
             source_primary_key     = @reflection.source_reflection.primary_key_name
           else
             reflection_primary_key = @reflection.source_reflection.primary_key_name
             source_primary_key     = @reflection.klass.primary_key
+            if @reflection.source_reflection.options[:as]
+              polymorphic_join = "AND %s.%s = %s" % [
+                @reflection.table_name, "#{@reflection.source_reflection.options[:as]}_type",
+                @owner.class.quote(@reflection.through_reflection.klass.name)
+              ]
+            end
           end
 
-          "INNER JOIN %s ON %s.%s = %s.%s #{@reflection.options[:joins]} #{custom_joins}" % [
+          "INNER JOIN %s ON %s.%s = %s.%s %s #{@reflection.options[:joins]} #{custom_joins}" % [
             @reflection.through_reflection.table_name,
             @reflection.table_name, reflection_primary_key,
-            @reflection.through_reflection.table_name, source_primary_key
+            @reflection.through_reflection.table_name, source_primary_key,
+            polymorphic_join
           ]
         end
         
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 40a5e5f2d1..d2c5393b59 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -176,6 +176,10 @@ module ActiveRecord
           if source_reflection.options[:polymorphic]
             raise HasManyThroughAssociationPolymorphicError.new(class_name, self, source_reflection)
           end
+          
+          unless [:belongs_to, :has_many].include?(source_reflection.macro) && source_reflection.options[:through].nil?
+            raise HasManyThroughSourceAssociationMacroError.new(self)
+          end
         end
       end