Merge pull request rails#47721 from Shopify/fix-nullifying-has-many-through-association

Fix nullification of has_many :through association with `query_constraints

Author: eileencodes

activerecord/lib/active_record/associations/through_association.rb

@@ -59,7 +59,7 @@ def construct_join_attributes(*records)
 
           association_primary_key = source_reflection.association_primary_key(reflection.klass)
 
-          if association_primary_key == reflection.klass.primary_key && !options[:source_type]
+          if Array(association_primary_key) == reflection.klass.composite_query_constraints_list && !options[:source_type]
             join_attributes = { source_reflection.name => records }
           else
             join_attributes = {

activerecord/lib/active_record/persistence.rb

@@ -499,6 +499,13 @@ def query_constraints_list # :nodoc:
         end
       end
 
+      # Returns an array of column names to be used in queries. The source of column
+      # names is derived from +query_constraints_list+ or +primary_key+. This method
+      # is for internal use when the primary key is to be treated as an array.
+      def composite_query_constraints_list # :nodoc:
+        @composite_query_constraints_list ||= query_constraints_list || Array(primary_key)
+      end
+
       # Destroy an object (or multiple objects) that has the given id. The object is instantiated first,
       # therefore all callbacks and filters are fired off before the object is deleted. This method is
       # less efficient than #delete but allows cleanup methods and other actions to be run.

activerecord/test/cases/associations_test.rb

@@ -42,7 +42,7 @@
 class AssociationsTest < ActiveRecord::TestCase
   fixtures :accounts, :companies, :developers, :projects, :developers_projects,
            :computers, :people, :readers, :authors, :author_addresses, :author_favorites,
-           :comments, :posts, :sharded_blogs, :sharded_blog_posts, :sharded_comments
+           :comments, :posts, :sharded_blogs, :sharded_blog_posts, :sharded_comments, :sharded_tags, :sharded_blog_posts_tags
 
   def test_eager_loading_should_not_change_count_of_children
     liquid = Liquid.create(name: "salty")
@@ -301,6 +301,17 @@ def test_append_composite_has_many_through_association_with_autosave
     assert_includes(blog_post.reload.tags, tag)
     assert_predicate Sharded::BlogPostTag.where(blog_post_id: blog_post.id, blog_id: blog_post.blog_id, tag_id: tag.id), :exists?
   end
+
+  def test_nullify_composite_has_many_through_association
+    blog_post = sharded_blog_posts(:great_post_blog_one)
+    assert_not_empty(blog_post.tags)
+
+    blog_post.tags = []
+
+    assert_empty(blog_post.tags)
+    assert_empty(blog_post.reload.tags)
+    assert_not_predicate Sharded::BlogPostTag.where(blog_post_id: blog_post.id, blog_id: blog_post.blog_id), :exists?
+  end
 end
 
 class AssociationProxyTest < ActiveRecord::TestCase