From 2b24167fba0981238c8addcc0058fc33a5680bec Mon Sep 17 00:00:00 2001
From: Joshua Wehner <joshua@forem.com>
Date: Mon, 12 Sep 2022 10:12:46 +0200
Subject: [PATCH] Try fixing comment like count bugs (#18438)

* Try fixing comment like count bugs

* Tests for the CommentCreator
---
 .../utilities/buildCommentHTML.js.erb         | 10 +--
 app/controllers/comments_controller.rb        | 17 +----
 app/models/comment.rb                         |  9 ++-
 app/services/comment_creator.rb               | 60 +++++++++++++++++
 app/views/comments/_comment.json.jbuilder     |  1 +
 .../comments/create_first_reaction_worker.rb  | 18 -----
 spec/models/comment_spec.rb                   |  6 --
 spec/services/comment_creator_spec.rb         | 67 +++++++++++++++++++
 .../create_first_reaction_worker_spec.rb      | 32 ---------
 9 files changed, 140 insertions(+), 80 deletions(-)
 create mode 100644 app/services/comment_creator.rb
 delete mode 100644 app/workers/comments/create_first_reaction_worker.rb
 create mode 100644 spec/services/comment_creator_spec.rb
 delete mode 100644 spec/workers/comments/create_first_reaction_worker_spec.rb

diff --git a/app/assets/javascripts/utilities/buildCommentHTML.js.erb b/app/assets/javascripts/utilities/buildCommentHTML.js.erb
index 0b677caee..63d49c493 100644
--- a/app/assets/javascripts/utilities/buildCommentHTML.js.erb
+++ b/app/assets/javascripts/utilities/buildCommentHTML.js.erb
@@ -125,12 +125,14 @@ function react(comment) {
   var iconSmallHeart = `<svg width="24" height="24" xmlns="http://www.w3.org/2000/svg" class="crayons-icon reaction-icon not-reacted"><path d="M18.884 12.595l.01.011L12 19.5l-6.894-6.894.01-.01A4.875 4.875 0 0112 5.73a4.875 4.875 0 016.884 6.865zM6.431 7.037a3.375 3.375 0 000 4.773L12 17.38l5.569-5.569a3.375 3.375 0 10-4.773-4.773L9.613 10.22l-1.06-1.062 2.371-2.372a3.375 3.375 0 00-4.492.25v.001z"/></svg>`;
   var iconSmallHeartFilled = `<svg width="24" height="24" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg" class="crayons-icon reaction-icon--like reaction-icon reacted"><path d="M5.116 12.595a4.875 4.875 0 015.56-7.68h-.002L7.493 8.098l1.06 1.061 3.181-3.182a4.875 4.875 0 016.895 6.894L12 19.5l-6.894-6.894.01-.01z"/></svg>`;
 
-  if (!comment.newly_created && comment.heart_ids.indexOf(userData().id) > -1) {
-    reactedClass = "reacted"
+  if (comment.newly_created) {
+    num = 1;
+  } else {
+    num = comment.public_reactions_count;
   }
 
-  if (!comment.newly_created) {
-    num = comment.public_reactions_count;
+  if (comment.newly_created || comment.heart_ids.indexOf(userData().id) > -1) {
+    reactedClass = "reacted"
   }
 
   var reactButton = `<button class="crayons-tooltip__activator crayons-btn crayons-btn--ghost crayons-btn--icon${num === 0 ? '' : '--left'} crayons-btn--s mr-1 reaction-like inline-flex reaction-button" id="button-for-comment-${ comment.id }" data-comment-id="${ comment.id }" data-tracking-name="comment_heart_button">
diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb
index 3fa54cd5b..ad1e88b01 100644
--- a/app/controllers/comments_controller.rb
+++ b/app/controllers/comments_controller.rb
@@ -45,31 +45,18 @@ class CommentsController < ApplicationController
   # POST /comments.json
   def create
     rate_limit!(rate_limit_to_use)
+    @comment = CommentCreator.build_comment(permitted_attributes(Comment), current_user: current_user)
 
-    @comment = Comment.includes(user: :profile).new(permitted_attributes(Comment))
-    @comment.user_id = current_user.id
-
+    # authorize & permit depend on @comment
     authorize @comment
     permit_commenter
 
     if @comment.save
-      checked_code_of_conduct = params[:checked_code_of_conduct].present? && !current_user.checked_code_of_conduct
-      current_user.update(checked_code_of_conduct: true) if checked_code_of_conduct
-
-      NotificationSubscription.create(
-        user: current_user, notifiable_id: @comment.id, notifiable_type: "Comment", config: "all_comments",
-      )
-      Notification.send_new_comment_notifications_without_delay(@comment)
-      Mention.create_all(@comment)
-
       if @comment.invalid?
-        @comment.destroy
         render json: { error: I18n.t("comments_controller.create.failure") }, status: :unprocessable_entity
         return
       end
-
       render partial: "comments/comment", formats: :json
-
     elsif (comment = Comment.where(
       body_markdown: @comment.body_markdown,
       commentable_id: @comment.commentable_id,
diff --git a/app/models/comment.rb b/app/models/comment.rb
index 73abfdb34..2d5829a1e 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -63,7 +63,6 @@ class Comment < ApplicationRecord
 
   after_create_commit :record_field_test_event
   after_create_commit :send_email_notification, if: :should_send_email_notification?
-  after_create_commit :create_first_reaction
   after_create_commit :send_to_moderator
 
   after_commit :calculate_score, on: %i[create update]
@@ -103,6 +102,10 @@ class Comment < ApplicationRecord
     I18n.t("models.comment.hidden")
   end
 
+  def self.build_comment(params, &blk)
+    includes(user: :profile).new(params, &blk)
+  end
+
   def search_id
     "comment_#{id}"
   end
@@ -256,10 +259,6 @@ class Comment < ApplicationRecord
     end
   end
 
-  def create_first_reaction
-    Comments::CreateFirstReactionWorker.perform_async(id, user_id)
-  end
-
   def after_destroy_actions
     Users::BustCacheWorker.perform_async(user_id)
     user.touch(:last_comment_at)
diff --git a/app/services/comment_creator.rb b/app/services/comment_creator.rb
new file mode 100644
index 000000000..01601bbfc
--- /dev/null
+++ b/app/services/comment_creator.rb
@@ -0,0 +1,60 @@
+require "delegate"
+
+class CommentCreator < Delegator
+  attr_reader :record
+  alias __getobj__ record
+
+  def self.build_comment(params, current_user:)
+    new(params, current_user: current_user)
+  end
+
+  # rubocop:disable Lint/MissingSuper
+  def initialize(params, current_user:)
+    @current_user = current_user
+    @params = params
+    @record = comment
+  end
+  # rubocop:enable Lint/MissingSuper
+
+  def save
+    return unless record.save
+
+    check_code_of_conduct
+    create_first_reaction
+    notify_subscribers
+
+    # copying this from the controller even though it seems impossible?
+    if record.invalid?
+      record.destroy
+      return self
+    end
+
+    self
+  end
+
+  private
+
+  attr_reader :current_user, :params
+
+  def check_code_of_conduct
+    checked_code_of_conduct = params[:checked_code_of_conduct].present? && !current_user.checked_code_of_conduct
+    current_user.update(checked_code_of_conduct: true) if checked_code_of_conduct
+  end
+
+  def comment
+    @comment ||= Comment.build_comment params.merge(user: current_user)
+  end
+
+  def create_first_reaction
+    Reaction.create user: current_user, category: "like", reactable: @comment
+  end
+
+  def notify_subscribers
+    NotificationSubscription.create config: "all_comments",
+                                    notifiable_id: @comment.id,
+                                    notifiable_type: "Comment",
+                                    user: current_user
+    Notification.send_new_comment_notifications_without_delay(@comment)
+    Mention.create_all(@comment)
+  end
+end
diff --git a/app/views/comments/_comment.json.jbuilder b/app/views/comments/_comment.json.jbuilder
index b870b3dc2..214a15032 100644
--- a/app/views/comments/_comment.json.jbuilder
+++ b/app/views/comments/_comment.json.jbuilder
@@ -4,6 +4,7 @@ json.depth @comment.depth
 json.url @comment.path
 json.readable_publish_date @comment.readable_publish_date
 json.published_timestamp @comment.decorate.published_timestamp
+json.public_reactions_count @comment.public_reactions_count.to_i
 json.body_html @comment.processed_html
 json.id @comment.id
 json.id_code @comment.id_code_generated
diff --git a/app/workers/comments/create_first_reaction_worker.rb b/app/workers/comments/create_first_reaction_worker.rb
deleted file mode 100644
index acc3a4784..000000000
--- a/app/workers/comments/create_first_reaction_worker.rb
+++ /dev/null
@@ -1,18 +0,0 @@
-module Comments
-  class CreateFirstReactionWorker
-    include Sidekiq::Job
-
-    sidekiq_options queue: :high_priority, retry: 10
-
-    def perform(comment_id, user_id)
-      return unless Comment.exists?(id: comment_id)
-
-      Reaction.create(
-        user_id: user_id,
-        reactable_id: comment_id,
-        reactable_type: "Comment",
-        category: "like",
-      )
-    end
-  end
-end
diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb
index a1ab87002..d6b8a2188 100644
--- a/spec/models/comment_spec.rb
+++ b/spec/models/comment_spec.rb
@@ -376,12 +376,6 @@ RSpec.describe Comment, type: :model do
       expect(comment.reload.id_code).to eq(comment.id.to_s(26))
     end
 
-    it "enqueue a worker to create the first reaction" do
-      expect do
-        comment.save
-      end.to change(Comments::CreateFirstReactionWorker.jobs, :size).by(1)
-    end
-
     it "enqueues a worker to calculate comment score" do
       expect do
         comment.save
diff --git a/spec/services/comment_creator_spec.rb b/spec/services/comment_creator_spec.rb
new file mode 100644
index 000000000..d7eb975c3
--- /dev/null
+++ b/spec/services/comment_creator_spec.rb
@@ -0,0 +1,67 @@
+require "rails_helper"
+
+RSpec.describe CommentCreator, type: :service do
+  subject(:creator) { described_class.build_comment params, current_user: user }
+
+  let(:user) { create :user }
+  let(:commentable) { create :article }
+  let(:record) { Comment.new user: user }
+  let(:params) do
+    {
+      commentable_id: commentable.id,
+      commentable_type: commentable.class_name.to_s,
+      body_markdown: "Hi, there!"
+    }
+  end
+
+  before do
+    allow(Comment).to receive(:build_comment).and_return(record)
+    allow(NotificationSubscription).to receive(:create)
+    allow(Notification).to receive(:send_new_comment_notifications_without_delay)
+    allow(Mention).to receive(:create_all)
+    allow(Reaction).to receive(:create)
+
+    allow(user).to receive(:comments)
+  end
+
+  it "responds as if Comment" do
+    methods = Comment.instance_methods - [:save]
+    methods.each do |method_name|
+      expect(creator).to respond_to(method_name)
+    end
+  end
+
+  context "when save is successful" do
+    before do
+      allow(record).to receive(:save).and_return(true)
+      creator.save
+    end
+
+    it "notifies subscribers" do
+      expect(NotificationSubscription).to have_received(:create)
+      expect(Notification).to have_received(:send_new_comment_notifications_without_delay)
+      expect(Mention).to have_received(:create_all)
+    end
+
+    it "creates a new reaction" do
+      expect(Reaction).to have_received(:create)
+    end
+  end
+
+  context "when save is unsuccessful" do
+    before do
+      allow(record).to receive(:save).and_return(false)
+      creator.save
+    end
+
+    it "does not notify subscribers" do
+      expect(NotificationSubscription).not_to have_received(:create)
+      expect(Notification).not_to have_received(:send_new_comment_notifications_without_delay)
+      expect(Mention).not_to have_received(:create_all)
+    end
+
+    it "does not create a new reaction" do
+      expect(Reaction).not_to have_received(:create)
+    end
+  end
+end
diff --git a/spec/workers/comments/create_first_reaction_worker_spec.rb b/spec/workers/comments/create_first_reaction_worker_spec.rb
deleted file mode 100644
index 3f62e7c26..000000000
--- a/spec/workers/comments/create_first_reaction_worker_spec.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-require "rails_helper"
-
-RSpec.describe Comments::CreateFirstReactionWorker, type: :worker do
-  include_examples "#enqueues_on_correct_queue", "high_priority", 1
-
-  describe "#perform_now" do
-    let(:worker) { subject }
-
-    context "with comment" do
-      let(:article) { create(:article) }
-      let(:comment) { create(:comment, commentable: article) }
-
-      it "creates a first reaction" do
-        expect do
-          worker.perform(comment.id, comment.user_id)
-        end.to change(comment.reactions, :count).by(1)
-      end
-
-      it "creates a like reaction" do
-        worker.perform(comment.id, comment.user_id)
-
-        expect(comment.reactions.last.category).to eq("like")
-      end
-    end
-
-    context "without comment" do
-      it "does not break" do
-        expect { worker.perform(nil, nil) }.not_to raise_error
-      end
-    end
-  end
-end