Ensure records are updated properly after a user merge (#7416)

app/models/reaction.rb

@@ -20,6 +20,7 @@ class Reaction < ApplicationRecord
 
   scope :positive, -> { where("points > ?", 0) }
   scope :readinglist, -> { where(category: "readinglist") }
+  scope :for_articles, ->(ids) { where(reactable_type: "Article", reactable_id: ids) }
   scope :eager_load_serialized_data, -> { includes(:reactable, :user) }
 
   validates :category, inclusion: { in: CATEGORIES }

app/services/moderator/merge_user.rb

@@ -23,6 +23,7 @@ module Moderator
       update_social
       Users::DeleteWorker.new.perform(@delete_user.id, true)
       @keep_user.touch(:profile_updated_at)
+      Users::MergeSyncWorker.perform_async(@keep_user.id)
 
       CacheBuster.bust("/#{@keep_user.username}")
     end

app/workers/users/merge_sync_worker.rb

@@ -0,0 +1,28 @@
+module Users
+  class MergeSyncWorker
+    include Sidekiq::Worker
+
+    sidekiq_options queue: :high_priority, retry: 10
+
+    def perform(user_id)
+      user = User.find_by(id: user_id)
+      return unless user
+
+      # [@practicaldev/sre]: These saves will do a lot of duplicate work. Since we are not merging users
+      # a lot I think this is fine for now and can be optimized in the future when we
+      # decrease our dependency on callbacks.
+      resave_content(user.articles)
+      resave_content(user.comments)
+      resave_content(user.reactions.readinglist)
+      resave_content(Reaction.for_articles(user.articles.pluck(:id)).readinglist)
+    end
+
+    private
+
+    def resave_content(content_ar_relation)
+      # Triggers callbacks to ensure caches are busted, scores updated, and docs
+      # are reindexed with the correct new user
+      content_ar_relation.find_each(&:save)
+    end
+  end
+end

spec/services/moderator/merge_user_spec.rb

@@ -7,6 +7,12 @@ RSpec.describe Moderator::MergeUser, type: :service do
   let(:admin) { create(:user, :super_admin) }
 
   describe "#merge" do
+    let(:article) { create(:article, user: delete_user) }
+    let(:comment) { create(:comment, user: delete_user) }
+    let(:reaction) { create(:reaction, user: delete_user, category: "readinglist") }
+    let(:article_reaction) { create(:reaction, reactable: article, category: "readinglist") }
+    let(:related_records) { [article, comment, reaction, article_reaction] }
+
     before { sidekiq_perform_enqueued_jobs }
 
     it "deletes delete_user_id and keeps keep_user" do
@@ -16,5 +22,23 @@ RSpec.describe Moderator::MergeUser, type: :service do
       expect(User.find_by(id: delete_user_id)).to be_nil
       expect(User.find_by(id: keep_user.id)).not_to be_nil
     end
+
+    it "updates documents in Elasticsearch" do
+      related_records
+      drain_all_sidekiq_jobs
+      expect(article.elasticsearch_doc.dig("_source", "user", "id")).to eq(delete_user_id)
+      expect(comment.elasticsearch_doc.dig("_source", "user", "id")).to eq(delete_user_id)
+      expect(reaction.elasticsearch_doc.dig("_source", "user_id")).to eq(delete_user_id)
+      expect(article_reaction.elasticsearch_doc.dig("_source", "reactable", "user", "id")).to eq(delete_user_id)
+
+      sidekiq_perform_enqueued_jobs do
+        described_class.call_merge(admin: admin, keep_user: keep_user, delete_user_id: delete_user.id)
+      end
+      drain_all_sidekiq_jobs
+      expect(article.reload.elasticsearch_doc.dig("_source", "user", "id")).to eq(keep_user.id)
+      expect(comment.reload.elasticsearch_doc.dig("_source", "user", "id")).to eq(keep_user.id)
+      expect(reaction.reload.elasticsearch_doc.dig("_source", "user_id")).to eq(keep_user.id)
+      expect(article_reaction.reload.elasticsearch_doc.dig("_source", "reactable", "user", "id")).to eq(keep_user.id)
+    end
   end
 end

spec/support/sidekiq_test_helpers.rb

@@ -73,6 +73,11 @@ module SidekiqTestHelpers
     end
   end
 
+  # Perform all Sidekiq jobs until there are no longer any in the queues
+  def drain_all_sidekiq_jobs
+    sidekiq_perform_enqueued_jobs while Sidekiq::Worker.jobs.any?
+  end
+
   class Utils
     class << self
       def drain_jobs(jobs, only: nil, except: nil)

spec/workers/users/merge_sync_worker_spec.rb

@@ -0,0 +1,14 @@
+require "rails_helper"
+
+RSpec.describe Users::MergeSyncWorker, type: :worker do
+  describe "#perform" do
+    let(:user) { create(:user) }
+    let(:worker) { subject }
+
+    it "resaves all user content" do
+      allow(worker).to receive(:resave_content)
+      worker.perform(user.id)
+      expect(worker).to have_received(:resave_content).exactly(4).times
+    end
+  end
+end