From 1003f71c8fa9faa9d5e676b6b2fd87b525bf2790 Mon Sep 17 00:00:00 2001
From: Asha Balasubramaniam <asha.ab@gmail.com>
Date: Wed, 8 Jan 2020 14:11:31 -0500
Subject: [PATCH] Migrate delayed job that calculates comment score to sidekiq
 (#5382)

---
 app/models/comment.rb                         |  4 +-
 .../comments/calculate_score_worker.rb        | 18 +++++
 spec/models/comment_spec.rb                   |  8 +++
 .../comments/calculate_score_worker_spec.rb   | 66 +++++++++++++++++++
 4 files changed, 94 insertions(+), 2 deletions(-)
 create mode 100644 app/workers/comments/calculate_score_worker.rb
 create mode 100644 spec/workers/comments/calculate_score_worker_spec.rb

diff --git a/app/models/comment.rb b/app/models/comment.rb
index 0ddf45027..c3d838033 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -21,7 +21,7 @@ class Comment < ApplicationRecord
   validates :user_id, presence: true
 
   after_create   :after_create_checks
-  after_save     :calculate_score
+  after_commit   :calculate_score
   after_save     :bust_cache
   after_save     :synchronous_bust
   after_destroy  :after_destroy_actions
@@ -238,7 +238,7 @@ class Comment < ApplicationRecord
   end
 
   def calculate_score
-    Comments::CalculateScoreJob.perform_later(id)
+    Comments::CalculateScoreWorker.perform_async(id)
   end
 
   def after_create_checks
diff --git a/app/workers/comments/calculate_score_worker.rb b/app/workers/comments/calculate_score_worker.rb
new file mode 100644
index 000000000..597281fd3
--- /dev/null
+++ b/app/workers/comments/calculate_score_worker.rb
@@ -0,0 +1,18 @@
+module Comments
+  class CalculateScoreWorker
+    include Sidekiq::Worker
+
+    sidekiq_options queue: :medium_priority
+
+    def perform(comment_id)
+      comment = Comment.find_by(id: comment_id)
+      return unless comment
+
+      score = BlackBox.comment_quality_score(comment)
+      spaminess_rating = BlackBox.calculate_spaminess(comment)
+
+      comment.update_columns(score: score, spaminess_rating: spaminess_rating)
+      comment.root.save! unless comment.is_root?
+    end
+  end
+end
diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb
index 8571382fd..fa38ef644 100644
--- a/spec/models/comment_spec.rb
+++ b/spec/models/comment_spec.rb
@@ -256,6 +256,14 @@ RSpec.describe Comment, type: :model do
       end.to change(Comments::CreateFirstReactionWorker.jobs, :size).by(1)
     end
 
+    it "enqueues a worker to calculate comment score" do
+      comment = build(:comment, user: user, commentable: article)
+
+      expect do
+        comment.save
+      end.to change(Comments::CalculateScoreWorker.jobs, :size).by(1)
+    end
+
     it "touches user updated_at" do
       user.updated_at = 1.month.ago
       user.save
diff --git a/spec/workers/comments/calculate_score_worker_spec.rb b/spec/workers/comments/calculate_score_worker_spec.rb
new file mode 100644
index 000000000..c7999d537
--- /dev/null
+++ b/spec/workers/comments/calculate_score_worker_spec.rb
@@ -0,0 +1,66 @@
+require "rails_helper"
+
+RSpec.describe Comments::CalculateScoreWorker, type: :worker do
+  include_examples "#enqueues_on_correct_queue", "medium_priority", 1
+
+  describe "#perform" do
+    let(:worker) { subject }
+
+    context "with comment" do
+      let_it_be(:article) { create(:article) }
+      let_it_be(:comment) { create(:comment, commentable: article) }
+
+      before do
+        allow(BlackBox).to receive(:comment_quality_score).and_return(7)
+        allow(BlackBox).to receive(:calculate_spaminess).and_return(99)
+      end
+
+      it "updates score and spaminess_rating", :aggregate_failures do
+        worker.perform(comment.id)
+
+        comment.reload
+        expect(comment.score).to be(7)
+        expect(comment.spaminess_rating).to be(99)
+      end
+
+      it "calls save on the root comment when given a descendant comment" do
+        child_comment = double
+        root_comment = double
+
+        allow(root_comment).to receive(:save!)
+        allow(child_comment).to receive(:update_columns)
+        allow(child_comment).to receive(:is_root?).and_return(false)
+        allow(child_comment).to receive(:root).and_return(root_comment)
+        allow(Comment).to receive(:find_by).with(id: 1).and_return(child_comment)
+
+        worker.perform(1)
+
+        expect(child_comment).to have_received(:is_root?)
+        expect(child_comment).to have_received(:root)
+        expect(root_comment).to have_received(:save!)
+      end
+
+      it "does not call save on the root comment" do
+        root_comment = double
+
+        allow(root_comment).to receive(:save)
+        allow(root_comment).to receive(:update_columns)
+        allow(root_comment).to receive(:is_root?).and_return(true)
+        allow(root_comment).to receive(:root).and_return(root_comment)
+        allow(Comment).to receive(:find_by).with(id: 1).and_return(root_comment)
+
+        worker.perform(1)
+
+        expect(root_comment).to have_received(:is_root?)
+        expect(root_comment).not_to have_received(:root)
+        expect(root_comment).not_to have_received(:save)
+      end
+    end
+
+    context "without comment" do
+      it "does not break" do
+        expect { worker.perform(nil) }.not_to raise_error
+      end
+    end
+  end
+end