Merge pull request rails#47928 from Shopify/pm/cpk-delete

Support deleting records from associations for CPK

Author: eileencodes

activerecord/lib/active_record/associations/has_many_association.rb

@@ -128,7 +128,9 @@ def delete_records(records, method)
             records.each(&:destroy!)
             update_counter(-records.length) unless reflection.inverse_updates_counter_cache?
           else
-            scope = self.scope.where(reflection.klass.primary_key => records)
+            query_constraints = reflection.klass.composite_query_constraints_list
+            values = records.map { |r| query_constraints.map { |col| r._read_attribute(col) } }
+            scope = self.scope.where(query_constraints => values)
             update_counter(-delete_count(method, scope))
           end
         end

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -41,6 +41,8 @@
 require "models/zine"
 require "models/interest"
 require "models/human"
+require "models/sharded"
+require "models/cpk"
 
 class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCase
   fixtures :authors, :author_addresses, :posts, :comments
@@ -116,7 +118,8 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   fixtures :accounts, :categories, :companies, :developers, :projects,
            :developers_projects, :topics, :authors, :author_addresses, :comments,
            :posts, :readers, :taggings, :cars, :tags,
-           :categorizations, :zines, :interests, :humans
+           :categorizations, :zines, :interests, :humans,
+           :sharded_blog_posts, :sharded_comments, :cpk_books, :cpk_authors
 
   def setup
     Client.destroyed_client_ids.clear
@@ -1287,6 +1290,45 @@ def test_deleting_before_save
     assert_equal 0, new_firm.clients_of_firm.size
   end
 
+  def test_deleting_models_with_composite_keys
+    great_author = cpk_authors(:cpk_great_author)
+    books = great_author.books
+
+    assert_equal 2, books.size
+
+    great_author.books.delete(books.first)
+    great_author.reload
+
+    assert_equal 1, great_author.books.size
+  end
+
+  def test_sharded_deleting_models
+    blog_post = sharded_blog_posts(:great_post_blog_one)
+    comments = blog_post.delete_comments
+
+    assert_equal 3, comments.size
+
+    comments_to_delete = [comments.first, comments.second]
+
+    sql = capture_sql do
+      blog_post.delete_comments.delete(comments_to_delete)
+    end
+
+    c = Sharded::Comment.connection
+
+    blog_id = Regexp.escape(c.quote_table_name("sharded_comments.blog_id"))
+    id = Regexp.escape(c.quote_table_name("sharded_comments.id"))
+
+    query_constraints = /#{blog_id} = .* AND #{id} = .*/
+    expectation = /DELETE.*WHERE.* \(#{query_constraints} OR #{query_constraints}\)/
+
+    assert_match(expectation, sql.first)
+
+    blog_post.reload
+
+    assert_equal 1, blog_post.comments.size
+  end
+
   def test_has_many_without_counter_cache_option
     # Ship has a conventionally named `treasures_count` column, but the counter_cache
     # option is not given on the association.

activerecord/test/fixtures/cpk_authors.yml

@@ -0,0 +1,8 @@
+_fixture:
+  model_class: Cpk::Author
+
+cpk_great_author:
+  name: "Alice"
+
+cpk_famous_author_first_book:
+  name: "Bob"

activerecord/test/fixtures/cpk_books.yml

@@ -2,14 +2,17 @@ _fixture:
   model_class: Cpk::Book
 
 cpk_great_author_first_book:
+  author: cpk_great_author
   title: "The first book"
   revision: 1
 
 cpk_great_author_second_book:
+  author: cpk_great_author
   title: "The second book"
   revision: 1
 
 cpk_famous_author_first_book:
+  author: cpk_famous_author
   title: "Ruby on Rails"
   revision: 1
 

activerecord/test/models/cpk.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require_relative "cpk/author"
 require_relative "cpk/book"
 require_relative "cpk/order"
 require_relative "cpk/order_agreement"

activerecord/test/models/cpk/author.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+module Cpk
+  class Author < ActiveRecord::Base
+    self.table_name = :cpk_authors
+
+    has_many :books, class_name: "Cpk::Book", dependent: :delete_all
+  end
+end

activerecord/test/models/cpk/book.rb

@@ -6,6 +6,7 @@ class Book < ActiveRecord::Base
     self.primary_key = [:author_id, :number]
 
     belongs_to :order
+    belongs_to :author, class_name: "Cpk::Author"
   end
 
   class BestSeller < Book

activerecord/test/models/sharded/blog_post.rb

@@ -7,6 +7,7 @@ class BlogPost < ActiveRecord::Base
 
     belongs_to :blog
     has_many :comments, query_constraints: [:blog_id, :blog_post_id]
+    has_many :delete_comments, class_name: "Sharded::Comment", query_constraints: [:blog_id, :blog_post_id], dependent: :delete_all
 
     has_many :blog_post_tags, query_constraints: [:blog_id, :blog_post_id]
     has_many :tags, through: :blog_post_tags

activerecord/test/schema/schema.rb

@@ -247,6 +247,10 @@
     t.integer :order_id
   end
 
+  create_table :cpk_authors, force: true do |t|
+    t.string :name
+  end
+
   create_table :cpk_orders, primary_key: [:shop_id, :id], force: true do |t|
     t.integer :shop_id
     t.integer :id