diff --git a/app/services/moderator/sink_articles.rb b/app/services/moderator/sink_articles.rb index 9b3722afd..feeee2d89 100644 --- a/app/services/moderator/sink_articles.rb +++ b/app/services/moderator/sink_articles.rb @@ -1,14 +1,7 @@ module Moderator class SinkArticles def self.call(user_id) - # rubocop:disable Lint/UnreachableCode - # HOTFIX - # @mstruve: This worker is broken and needs to be fixed. It can inadvertantly - # bump up scores for a user's articles if they have a lot of good reactions - # this leads to clumps of articles from the same user in feeds - return Moderator::SinkArticlesWorker.perform_async(user_id) - # rubocop:enable Lint/UnreachableCode end end end diff --git a/app/workers/moderator/sink_articles_worker.rb b/app/workers/moderator/sink_articles_worker.rb index 7ac48d9ce..9b2442335 100644 --- a/app/workers/moderator/sink_articles_worker.rb +++ b/app/workers/moderator/sink_articles_worker.rb @@ -8,10 +8,7 @@ module Moderator user = User.find_by(id: user_id) return unless user - articles = Article.where(user: user) - reactions = Reaction.where(reactable: articles) - new_score = reactions.sum(:points) + Reaction.where(reactable: user).sum(:points) - articles.update_all(score: new_score) + Article.where(user: user).each(&:update_score) rescue StandardError => e ForemStatsClient.count("moderators.sink", 1, tags: ["action:failed", "user_id:#{user.id}"]) Honeybadger.notify(e) diff --git a/spec/services/moderator/sink_articles_spec.rb b/spec/services/moderator/sink_articles_spec.rb index 9c739a3f6..6efd99c8c 100644 --- a/spec/services/moderator/sink_articles_spec.rb +++ b/spec/services/moderator/sink_articles_spec.rb @@ -7,16 +7,17 @@ RSpec.describe Moderator::SinkArticles, type: :service do create_list(:article, 3, user: user) user end + let(:scores) { -> { spam_user.articles.reload.pluck(:score) } } let(:vomit_reaction) { create(:reaction, reactable: spam_user, user: moderator, category: "vomit") } - xdescribe "#call" do + describe "#call" do it "lowers all of a user's articles' scores by 25 each if not confirmed" do vomit_reaction expect do sidekiq_perform_enqueued_jobs do described_class.call(spam_user.id) end - end.to change { spam_user.articles.sum(:score) }.from(0).to(-75) + end.to change(scores, :call).from([0, 0, 0]).to([-25, -25, -25]) end it "lowers all of the user's articles' scores by 50 each if confirmed" do @@ -25,7 +26,22 @@ RSpec.describe Moderator::SinkArticles, type: :service do sidekiq_perform_enqueued_jobs do described_class.call(spam_user.id) end - end.to change { spam_user.articles.sum(:score) }.from(0).to(-150) + end.to change(scores, :call).from([0, 0, 0]).to([-50, -50, -50]) + end + + context "when removing a user vomit reaction" do + before do + # pretend we had a confirmed vomit on the user + spam_user.articles.update(score: -50) + end + + it "raises all of the user's articles but the moderation score" do + expect do + sidekiq_perform_enqueued_jobs do + described_class.call(spam_user.id) + end + end.to change(scores, :call).from([-50, -50, -50]).to([0, 0, 0]) + end end end end