Fix Account::Merging concern not supporting Quotes or Collections, refactor it (#39884)

app/models/concerns/account/merging.rb

@@ -1,8 +1,30 @@
 # frozen_string_literal: true
 
+# TODO: remove some time after 4.7.0
 module Account::Merging
   extend ActiveSupport::Concern
 
+  ACCOUNT_MERGING_CLASSES = {
+    account_id: [
+      Status, StatusPin, MediaAttachment, Poll, Report, Tombstone, Favourite,
+      Follow, FollowRequest, Block, Mute,
+      AccountModerationNote, AccountPin, AccountStat, ListAccount,
+      PollVote, Mention, AccountDeletionRequest, AccountNote, FollowRecommendationSuppression,
+      Appeal, TagFollow, Quote, Collection, CollectionItem
+    ],
+    from_account_id: [
+      Notification, NotificationPermission, NotificationRequest
+    ],
+    target_account_id: [
+      Follow, FollowRequest, Block, Mute, AccountModerationNote, AccountPin, AccountNote
+    ],
+    reference_account_id: [CanonicalEmailBlock],
+    account_warning_id: [Appeal],
+    local_account_id: [SeveredRelationship],
+    remote_account_id: [SeveredRelationship],
+    quoted_account_id: [Quote],
+  }.freeze
+
   def merge_with!(other_account)
     # Since it's the same remote resource, the remote resource likely
     # already believes we are following/blocking, so it's safe to
@@ -11,65 +33,16 @@ module Account::Merging
     # account already, therefore mass update will not work and we need
     # to check for (and skip past) uniqueness errors
 
-    owned_classes = [
+    ACCOUNT_MERGING_CLASSES.each do |attribute, classes|
-      Status, StatusPin, MediaAttachment, Poll, Report, Tombstone, Favourite,
+      classes.each do |klass|
-      Follow, FollowRequest, Block, Mute,
+        klass.where({ attribute => other_account.id }).reorder(nil).find_each do |record|
-      AccountModerationNote, AccountPin, AccountStat, ListAccount,
+          record.update_attribute(attribute, id)
-      PollVote, Mention, AccountDeletionRequest, AccountNote, FollowRecommendationSuppression,
+        rescue ActiveRecord::RecordNotUnique
-      Appeal, TagFollow
+          next
-    ]
+        end
-
-    owned_classes.each do |klass|
-      klass.where(account_id: other_account.id).reorder(nil).find_each do |record|
-        record.update_attribute(:account_id, id)
-      rescue ActiveRecord::RecordNotUnique
-        next
       end
     end
 
-    [
-      Notification, NotificationPermission, NotificationRequest
-    ].each do |klass|
-      klass.where(from_account_id: other_account.id).reorder(nil).find_each do |record|
-        record.update_attribute(:from_account_id, id)
-      rescue ActiveRecord::RecordNotUnique
-        next
-      end
-    end
-
-    target_classes = [
-      Follow, FollowRequest, Block, Mute, AccountModerationNote, AccountPin,
-      AccountNote
-    ]
-
-    target_classes.each do |klass|
-      klass.where(target_account_id: other_account.id).reorder(nil).find_each do |record|
-        record.update_attribute(:target_account_id, id)
-      rescue ActiveRecord::RecordNotUnique
-        next
-      end
-    end
-
-    CanonicalEmailBlock.where(reference_account_id: other_account.id).find_each do |record|
-      record.update_attribute(:reference_account_id, id)
-    end
-
-    Appeal.where(account_warning_id: other_account.id).find_each do |record|
-      record.update_attribute(:account_warning_id, id)
-    end
-
-    SeveredRelationship.about_local_account(other_account).reorder(nil).find_each do |record|
-      record.update_attribute(:local_account_id, id)
-    rescue ActiveRecord::RecordNotUnique
-      next
-    end
-
-    SeveredRelationship.about_remote_account(other_account).reorder(nil).find_each do |record|
-      record.update_attribute(:remote_account_id, id)
-    rescue ActiveRecord::RecordNotUnique
-      next
-    end
-
     # Some follow relationships have moved, so the cache is stale
     Rails.cache.delete_matched("followers_hash:#{id}:*")
     Rails.cache.delete_matched("relationships:#{id}:*")

spec/models/concerns/account/merging_spec.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Account::Merging do
+  let!(:account) { Fabricate(:account) }
+
+  describe '#merge_with!' do
+    let(:other_account) { Fabricate(:account) }
+    let!(:status) { Fabricate(:status, account: other_account) }
+    let!(:follow) { Fabricate(:follow, account: other_account) }
+    let!(:reverse_follow) { Fabricate(:follow, target_account: other_account) }
+
+    it 'reattributes records' do
+      expect { account.merge_with!(other_account) }
+        .to change { status.reload.account_id }.to(account.id)
+        .and change { follow.reload.account_id }.to(account.id)
+        .and change { reverse_follow.reload.target_account_id }.to(account.id)
+    end
+  end
+end