New comment notifications to Active Jobs #1996 (#2287)

* Send new comment notification service + specs

* Call new comment notification service from the model

* Return to non-bang notifications create

* An ActiveJob for the new comment notifications

* Call comments notification ActiveJob from Notification

app/jobs/notifications/new_comment_job.rb

@@ -0,0 +1,9 @@
+module Notifications
+  class NewCommentJob < ApplicationJob
+    queue_as :send_new_comment_notification
+    def perform(comment_id, service = NewComment::Send)
+      comment = Comment.find_by(id: comment_id)
+      service.call(comment) if comment
+    end
+  end
+end

app/models/notification.rb

@@ -51,35 +51,14 @@ class Notification < ApplicationRecord
     def send_new_comment_notifications(comment)
       return if comment.commentable_type == "PodcastEpisode"
 
-      user_ids = comment.ancestors.select(:user_id).where(receive_notifications: true).pluck(:user_id).to_set
-      user_ids.add(comment.commentable.user_id) if comment.commentable.receive_notifications
-      json_data = {
-        user: user_data(comment.user),
-        comment: comment_data(comment)
-      }
-      user_ids.delete(comment.user_id).each do |user_id|
-        Notification.create(
-          user_id: user_id,
-          notifiable_id: comment.id,
-          notifiable_type: comment.class.name,
-          action: nil,
-          json_data: json_data,
-        )
-        # Be careful with this basic first implementation of push notification. Has dependency of Pusher/iPhone sort of tough to test reliably.
-        send_push_notifications(user_id, "@#{comment.user.username} replied to you:", comment.title, "/notifications/comments") if User.find_by(id: user_id)&.mobile_comment_notifications
-      end
-      return unless comment.commentable.organization_id
-
-      Notification.create(
-        organization_id: comment.commentable.organization_id,
-        notifiable_id: comment.id,
-        notifiable_type: comment.class.name,
-        action: nil,
-        json_data: json_data,
-      )
-      # no push notifications for organizations yet
+      Notifications::NewCommentJob.perform_later(comment.id)
+    end
+
+    def send_new_comment_notifications_without_delay(comment)
+      return if comment.commentable_type == "PodcastEpisode"
+
+      Notifications::NewCommentJob.perform_now(comment.id)
     end
-    handle_asynchronously :send_new_comment_notifications
 
     def send_new_badge_notification(badge_achievement)
       json_data = {
@@ -262,6 +241,10 @@ class Notification < ApplicationRecord
       Notifications.user_data(user)
     end
 
+    def comment_data(comment)
+      Notifications.comment_data(comment)
+    end
+
     def reaction_notification_attributes(reaction, receiver)
       reactable_data = {
         reactable_id: reaction.reactable_id,
@@ -283,24 +266,6 @@ class Notification < ApplicationRecord
       }
     end
 
-    def comment_data(comment)
-      {
-        id: comment.id,
-        class: { name: "Comment" },
-        path: comment.path,
-        processed_html: comment.processed_html,
-        updated_at: comment.updated_at,
-        commentable: {
-          id: comment.commentable.id,
-          title: comment.commentable.title,
-          path: comment.commentable.path,
-          class: {
-            name: comment.commentable.class.name
-          }
-        }
-      }
-    end
-
     def article_data(article)
       {
         id: article.id,
@@ -346,25 +311,6 @@ class Notification < ApplicationRecord
         milestones[milestones.index(closest_number) - 1]
       end
     end
-
-    def send_push_notifications(user_id, title, body, path)
-      return unless ApplicationConfig["PUSHER_BEAMS_KEY"] && ApplicationConfig["PUSHER_BEAMS_KEY"].size == 64
-
-      payload = {
-        apns: {
-          aps: {
-            alert: {
-              title: title,
-              body: CGI.unescapeHTML(body.strip!)
-            }
-          },
-          data: {
-            url: "https://dev.to" + path
-          }
-        }
-      }
-      Pusher::PushNotifications.publish(interests: ["user-notifications-#{user_id}"], payload: payload)
-    end
   end
 
   # instance methods

app/services/notifications.rb

@@ -11,4 +11,22 @@ module Notifications
       created_at: user.created_at
     }
   end
+
+  def self.comment_data(comment)
+    {
+      id: comment.id,
+      class: { name: "Comment" },
+      path: comment.path,
+      processed_html: comment.processed_html,
+      updated_at: comment.updated_at,
+      commentable: {
+        id: comment.commentable.id,
+        title: comment.commentable.title,
+        path: comment.commentable.path,
+        class: {
+          name: comment.commentable.class.name
+        }
+      }
+    }
+  end
 end

app/services/notifications/new_comment/send.rb

@@ -0,0 +1,69 @@
+# send notifications about the new comment
+module Notifications
+  module NewComment
+    class Send
+      def initialize(comment)
+        @comment = comment
+      end
+
+      delegate :user_data, :comment_data, to: Notifications
+
+      def self.call(*args)
+        new(*args).call
+      end
+
+      def call
+        user_ids = comment.ancestors.select(:user_id).where(receive_notifications: true).pluck(:user_id).to_set
+        user_ids.add(comment.commentable.user_id) if comment.commentable.receive_notifications
+        json_data = {
+          user: user_data(comment.user),
+          comment: comment_data(comment)
+        }
+        user_ids.delete(comment.user_id).each do |user_id|
+          Notification.create(
+            user_id: user_id,
+            notifiable_id: comment.id,
+            notifiable_type: comment.class.name,
+            action: nil,
+            json_data: json_data,
+          )
+          # Be careful with this basic first implementation of push notification. Has dependency of Pusher/iPhone sort of tough to test reliably.
+          send_push_notifications(user_id, "@#{comment.user.username} replied to you:", comment.title, "/notifications/comments") if User.find_by(id: user_id)&.mobile_comment_notifications
+        end
+        return unless comment.commentable.organization_id
+
+        Notification.create(
+          organization_id: comment.commentable.organization_id,
+          notifiable_id: comment.id,
+          notifiable_type: comment.class.name,
+          action: nil,
+          json_data: json_data,
+        )
+        # no push notifications for organizations yet
+      end
+
+      private
+
+      attr_reader :comment
+
+      def send_push_notifications(user_id, title, body, path)
+        return unless ApplicationConfig["PUSHER_BEAMS_KEY"] && ApplicationConfig["PUSHER_BEAMS_KEY"].size == 64
+
+        payload = {
+          apns: {
+            aps: {
+              alert: {
+                title: title,
+                body: CGI.unescapeHTML(body.strip!)
+              }
+            },
+            data: {
+              url: "https://dev.to" + path
+            }
+          }
+        }
+        Pusher::PushNotifications.publish(interests: ["user-notifications-#{user_id}"], payload: payload)
+      end
+    end
+  end
+end

spec/jobs/notifications/new_comment_job_spec.rb

@@ -0,0 +1,25 @@
+require "rails_helper"
+
+RSpec.describe Notifications::NewCommentJob, type: :job do
+  include_examples "#enqueues_job", "send_new_comment_notification", 5
+
+  describe "#perform_now" do
+    let(:new_comment_service) { double }
+
+    before do
+      allow(new_comment_service).to receive(:call)
+    end
+
+    it "calls the service" do
+      comment = create(:comment, commentable: create(:article))
+
+      described_class.perform_now(comment.id, new_comment_service)
+      expect(new_comment_service).to have_received(:call).with(comment).once
+    end
+
+    it "doesn't call a service if a nonexistent comment passed" do
+      described_class.perform_now(Comment.maximum(:id).to_i + 1, new_comment_service)
+      expect(new_comment_service).not_to have_received(:call)
+    end
+  end
+end

spec/services/notifications/new_comment/send_spec.rb

@@ -0,0 +1,37 @@
+require "rails_helper"
+
+RSpec.describe Notifications::NewComment::Send, type: :service do
+  let(:user)            { create(:user) }
+  let(:user2)           { create(:user) }
+  let(:user3)           { create(:user) }
+  let(:organization)    { create(:organization) }
+  let(:article)         { create(:article, user_id: user.id) }
+  let(:comment)         { create(:comment, commentable: article, user: user2) }
+  let!(:child_comment) { create(:comment, commentable: article, parent: comment, user: user3) }
+
+  it "creates users notifications" do
+    expect do
+      described_class.call(child_comment)
+    end.to change(Notification, :count).by(2)
+  end
+
+  it "creates notifications for the article author and the parent comment author" do
+    described_class.call(child_comment)
+    child_comment.reload
+    notified_user_ids = Notification.where(notifiable_type: "Comment", notifiable_id: child_comment.id).map(&:user_id)
+    expect(notified_user_ids.sort).to eq([user.id, user2.id].sort)
+  end
+
+  it "doesn't create notification if the receive notification is false" do
+    comment.update_column(:receive_notifications, false)
+    described_class.call(child_comment)
+    notified_user_ids = Notification.where(notifiable_type: "Comment", notifiable_id: child_comment.id).map(&:user_id)
+    expect(notified_user_ids).to eq([user.id].sort)
+  end
+
+  it "creates an organization notification" do
+    article.update_column(:organization_id, organization.id)
+    described_class.call(child_comment)
+    expect(Notification.where(notifiable_type: "Comment", notifiable_id: child_comment.id, organization_id: organization.id)).to be_any
+  end
+end