Ticket #6461: nested_has_many_through_with_correct_primary_keys.diff

activerecord/lib/active_record/associations.rb

@@ -1570 +1570 @@
-              @aliased_table_name = table_name #.tr('.', '_') # start with the table name, sub out any .'s
-              @parent_table_name  = parent.active_record.table_name
-
-
++
-                join_dependency.table_aliases[aliased_table_name] += 1
-              end
-              

activerecord/lib/active_record/associations/has_many_through_association.rb

@@ -4 +4 @@
-      def initialize(owner, reflection)
-        super
-        reflection.check_validity!
-        @finder_sql = construct_conditions
-        construct_sql
-      end
-
-      def find(*args)
-        options = args.extract_options!
-
-"#{@finder_sql}"
+construct_conditions
-        if sanitized_conditions = sanitize_sql(options[:conditions])
-
+= conditions.dup 
-        end
-        options[:conditions] = conditions
-
@@ -25 +23 @@
-
-        options[:select]  = construct_select(options[:select])
-        options[:from]  ||= construct_from
-(options[:joins])
+ + " #{options[:joins]}"
-        options[:include] = @reflection.source_reflection.options[:include] if options[:include].nil?
-
-        merge_options_from_reflection!(options)
@@ -161 +159 @@
-          join_attributes
-        end
-
-        # Associate attributes pointing to owner, quoted.
-        def construct_quoted_owner_attributes(reflection)
-          if as = reflection.options[:as]
-            { "#{as}_id" => @owner.quoted_id,
-              "#{as}_type" => reflection.klass.quote_value(
-                @owner.class.base_class.name.to_s,
-                reflection.klass.columns_hash["#{as}_type"]) }
-          else
-            { reflection.primary_key_name => @owner.quoted_id }
-          end
-        end
-
-        # Build SQL conditions from attributes, qualified by table name.
-        def construct_conditions
-table_name = @reflection.through_reflection.table_name
-conditions = construct_quoted_owner_attributes(@reflection.through_reflection).map do |attr, value|
-"#{table_name}.#{attr} = #{value
+if @constructed_conditions.nil?
+  @join_components ||= construct_join_components
+@constructed_conditions = "#{@join_components[:remote_key]} = #{@owner.quoted_id} #{@join_components[:conditions]
-          end
-
-
+
-        end
-
-        def construct_from
@@ -191 +176 @@
-          selected = custom_select || @reflection.options[:select] || "#{@reflection.table_name}.*"
-        end
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
-        end
-
-        def construct_scope
@@ -228 +188 @@
-                         :joins       => construct_joins,
-                         :select      => construct_select } }
-        end
+        
+        # Given any belongs_to or has_many (including has_many :through) association,
+        # return the essential components of a join corresponding to that association, namely:
+        # joins: any additional joins required to get from the association's table (reflection.table_name)
+        #    to the table that's actually joining to the active record's table
+        # remote_key: the name of the key in the join table (qualified by table name) which will join
+        #    to a field of the active record's table
+        # local_key: the name of the key in the local table (not qualified by table name) which will
+        #    take part in the join
+        # conditions: any additional conditions (e.g. filtering by type for a polymorphic association,
+        #    or a :conditions clause explicitly given in the association), including a leading AND
+        def construct_join_components(reflection = @reflection, association_class = reflection.klass, table_ids = {association_class.table_name => 1})
+        
+          if reflection.macro == :has_many and reflection.through_reflection
+            # Construct the join components of the source association, so that we have a path from
+            # the eventual target table of the association up to the table named in :through, and
+            # all tables involved are allocated table IDs.
+            source_join_components = construct_join_components(reflection.source_reflection, reflection.klass, table_ids)
+            # Determine the alias of the :through table; this will be the last table assigned
+            # when constructing the source join components above.
+            through_table_alias = through_table_name = reflection.through_reflection.table_name
+            through_table_alias += "_#{table_ids[through_table_name]}" unless table_ids[through_table_name] == 1
-
-
-
-
-
+
+
+
-
-
-
-
+
+
+
+
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
-          else
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-          end
-        end
-
-
-
-
-
-
-
+
+
-        end
-
-        alias_method :sql_conditions, :conditions
-    end
-  end
-end

activerecord/lib/active_record/reflection.rb

@@ -187 +187 @@
-            raise HasManyThroughAssociationPolymorphicError.new(active_record.name, self, source_reflection)
-          end
-          
- && source_reflection.options[:through].nil?
+
-            raise HasManyThroughSourceAssociationMacroError.new(self)
-          end
-        end

activerecord/test/associations/join_model_test.rb

@@ -368 +368 @@
-    end
-  end
-
-
-
+
+
+
+
+
-  end
-
+  def test_remote_nested_through_associations
+    author = authors(:david)
+
+    # polymorphic
+    assert_equal [tags(:general)], author.tags.uniq.sort_by { |t| t.id }
+    assert_equal [], author.invalid_tags
+
+    # non-polymorphic
+    assert_equal [author, authors(:mary)], author.similar_authors.uniq.sort_by { |t| t.id }
+  end
+
+  def test_local_and_remote_nested_through_associations
+    author = authors(:david)
+
+    # polymorphic
+    assert_equal [taggings(:welcome_general), taggings(:thinking_general), taggings(:fake), taggings(:godfather)], author.tag_taggings.uniq.sort_by { |t| t.id }
+
+    expected_posts = [
+      posts(:welcome),
+      posts(:thinking),
+      posts(:sti_comments),
+      posts(:sti_post_and_comments),
+      posts(:sti_habtm),
+      posts(:eager_other)
+    ]
+    assert_equal expected_posts, author.posts_of_similar_authors.uniq.sort_by { |t| t.id }
+  end
+
+  def test_multiple_table_references_in_nested_through_associations
+    author = authors(:david)
+
+    # polymorphic
+    assert_equal [tags(:general)], author.tag_tagging_tags.uniq.sort_by { |t| t.id }
+
+    assert_equal [categorizations(:david_welcome_general), categorizations(:mary_thinking_general)], author.categorizations_of_similar_posts.uniq.sort_by { |t| t.id }
+    assert_equal [author, authors(:mary)], author.similar_authors_2.uniq.sort_by { |t| t.id }
+
+    expected_posts = [
+      posts(:welcome),
+      posts(:thinking),
+      posts(:sti_comments),
+      posts(:sti_post_and_comments),
+      posts(:sti_habtm),
+      posts(:eager_other)
+    ]
+    assert_equal expected_posts, author.posts_of_similar_authors_2.uniq.sort_by { |t| t.id }
+  end
+  
+  def test_independence_of_repeated_has_many_through_finds
+    author = authors(:david)
+    assert_equal [taggings(:welcome_general)], author.taggings.find(:all, :conditions => ['taggings.taggable_id = ?', 1])
+    assert_equal [taggings(:welcome_general), taggings(:thinking_general)], author.taggings.find(:all).uniq.sort_by { |t| t.id }
+  end
+
-  def test_has_many_through_habtm
-    assert_raise(ActiveRecord::HasManyThroughSourceAssociationMacroError) { authors(:david).post_categories }
-  end

activerecord/test/fixtures/author.rb

@@ -56 +56 @@
-
-  has_many :tagging,  :through => :posts # through polymorphic has_one
-  has_many :taggings, :through => :posts, :source => :taggings # through polymorphic has_many
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-  has_many :post_categories, :through => :posts, :source => :categories
-
-  belongs_to :author_address