diff --git a/app/models/reaction.rb b/app/models/reaction.rb index ff1d0daa1..a92ea8d28 100644 --- a/app/models/reaction.rb +++ b/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 } diff --git a/app/services/moderator/merge_user.rb b/app/services/moderator/merge_user.rb index 692f20af6..d2cb9bd5d 100644 --- a/app/services/moderator/merge_user.rb +++ b/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 diff --git a/app/workers/users/merge_sync_worker.rb b/app/workers/users/merge_sync_worker.rb new file mode 100644 index 000000000..65d5af969 --- /dev/null +++ b/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 diff --git a/spec/services/moderator/merge_user_spec.rb b/spec/services/moderator/merge_user_spec.rb index 0e2dca610..0b0f85eb9 100644 --- a/spec/services/moderator/merge_user_spec.rb +++ b/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 diff --git a/spec/support/sidekiq_test_helpers.rb b/spec/support/sidekiq_test_helpers.rb index 1ce3f39f8..c73d88ab1 100644 --- a/spec/support/sidekiq_test_helpers.rb +++ b/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) diff --git a/spec/workers/users/merge_sync_worker_spec.rb b/spec/workers/users/merge_sync_worker_spec.rb new file mode 100644 index 000000000..72aad8b1b --- /dev/null +++ b/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