Move messages send_push delay calls to ActiveJob (#3314)

app/controllers/messages_controller.rb

@@ -13,7 +13,7 @@ class MessagesController < ApplicationController
     end
     if @message.save
       begin
-        @message.delay.send_push
+        @message.send_push
         success = true
       rescue Pusher::Error => e
         logger.info "PUSHER ERROR: #{e.message}"

app/jobs/messages/send_push_job.rb

@@ -0,0 +1,14 @@
+module Messages
+  class SendPushJob < ApplicationJob
+    queue_as :messages_send_push
+
+    def perform(user_id, chat_channel_id, message_html, service = Messages::SendPush)
+      user = User.find_by(id: user_id)
+      chat_channel = ChatChannel.find_by(id: chat_channel_id)
+
+      return unless user && chat_channel
+
+      service.call(user, chat_channel, message_html)
+    end
+  end
+end

app/models/message.rb

@@ -22,25 +22,7 @@ class Message < ApplicationRecord
   end
 
   def send_push
-    receiver_ids = chat_channel.chat_channel_memberships.
-      where.not(user_id: user.id).pluck(:user_id)
-
-    PushNotificationSubscription.where(user_id: receiver_ids).find_each do |sub|
-      break if no_push_necessary?(sub)
-
-      Webpush.payload_send(
-        endpoint: sub.endpoint,
-        message: ActionView::Base.full_sanitizer.sanitize(message_html),
-        p256dh: sub.p256dh_key,
-        auth: sub.auth_key,
-        ttl: 24 * 60 * 60,
-        vapid: {
-          subject: "https://dev.to",
-          public_key: ApplicationConfig["VAPID_PUBLIC_KEY"],
-          private_key: ApplicationConfig["VAPID_PRIVATE_KEY"]
-        },
-      )
-    end
+    Messages::SendPushJob.perform_later(user.id, chat_channel.id, message_html)
   end
 
   def direct_receiver
@@ -96,11 +78,6 @@ class Message < ApplicationRecord
     errors.add(:base, "You are not a participant of this chat channel.") unless channel.has_member?(user)
   end
 
-  def no_push_necessary?(sub)
-    membership = sub.user.chat_channel_memberships.order("last_opened_at DESC").first
-    membership.last_opened_at > 40.seconds.ago
-  end
-
   def rich_link_article(link)
     Article.find_by(slug: link["href"].split("/")[4].split("?")[0]) if link["href"].include?("//#{ApplicationConfig['APP_DOMAIN']}/") && link["href"].split("/")[4]
   end

app/services/messages/send_push.rb

@@ -0,0 +1,44 @@
+module Messages
+  class SendPush
+    def initialize(user, chat_channel, message_html)
+      @user = user
+      @chat_channel = chat_channel
+      @message_html = message_html
+    end
+
+    def self.call(*args)
+      new(*args).call
+    end
+
+    def call
+      receiver_ids = chat_channel.chat_channel_memberships.
+        where.not(user_id: user.id).pluck(:user_id)
+
+      PushNotificationSubscription.where(user_id: receiver_ids).find_each do |sub|
+        break if no_push_necessary?(sub)
+
+        Webpush.payload_send(
+          endpoint: sub.endpoint,
+          message: ActionView::Base.full_sanitizer.sanitize(message_html),
+          p256dh: sub.p256dh_key,
+          auth: sub.auth_key,
+          ttl: 24 * 60 * 60,
+          vapid: {
+            subject: "https://dev.to",
+            public_key: ApplicationConfig["VAPID_PUBLIC_KEY"],
+            private_key: ApplicationConfig["VAPID_PRIVATE_KEY"]
+          },
+        )
+      end
+    end
+
+    private
+
+    attr_reader :user, :chat_channel, :message_html
+
+    def no_push_necessary?(sub)
+      membership = sub.user.chat_channel_memberships.order("last_opened_at DESC").first
+      membership.last_opened_at > 40.seconds.ago
+    end
+  end
+end

spec/jobs/messages/send_push_job_spec.rb

@@ -0,0 +1,49 @@
+require "rails_helper"
+
+RSpec.describe Messages::SendPushJob, type: :job do
+  include_examples "#enqueues_job", "messages_send_push", 456, 789, "<h1>Hello</h1"
+
+  describe "#perform_now" do
+    let(:messages_send_push_service) { double }
+
+    context "when no user found" do
+      before do
+        allow(User).to receive(:find_by)
+        allow(messages_send_push_service).to receive(:call)
+      end
+
+      it "does not call the service" do
+        described_class.perform_now(456, 789, "<html>", messages_send_push_service)
+        expect(messages_send_push_service).not_to have_received(:call)
+      end
+    end
+
+    context "when no chat channel found" do
+      before do
+        allow(ChatChannel).to receive(:find_by)
+        allow(messages_send_push_service).to receive(:call)
+      end
+
+      it "does not call the service" do
+        described_class.perform_now(456, 789, "<html>", messages_send_push_service)
+        expect(messages_send_push_service).not_to have_received(:call)
+      end
+    end
+
+    context "when user + chat channel" do
+      let(:user) { double }
+      let(:chat_channel) { double }
+
+      before do
+        allow(User).to receive(:find_by).and_return(user)
+        allow(ChatChannel).to receive(:find_by).and_return(chat_channel)
+        allow(messages_send_push_service).to receive(:call).with(user, chat_channel, "<html>")
+      end
+
+      it "does call the service" do
+        described_class.perform_now(456, 789, "<html>", messages_send_push_service)
+        expect(messages_send_push_service).to have_received(:call).with(user, chat_channel, "<html>")
+      end
+    end
+  end
+end

spec/services/messages/send_push_spec.rb

@@ -0,0 +1,32 @@
+require "rails_helper"
+
+RSpec.describe Messages::SendPush do
+  let!(:user1) { create(:user) }
+  let!(:user2) { create(:user) }
+  let!(:chat_channel) { create(:chat_channel) }
+  let!(:message) { build(:message, chat_channel_id: chat_channel.id, user_id: user2.id) }
+
+  before do
+    create(:chat_channel_membership, user_id: user2.id, chat_channel_id: chat_channel.id)
+    PushNotificationSubscription.create(user_id: user2.id, endpoint: "http://nowhere.togo", p256dh_key: "BBoN_OkTfE_0uObue", auth_key: "aW1hcm    thcmF", notification_type: "browser")
+    allow(Webpush).to receive(:payload_send).and_return(true)
+  end
+
+  context "when push is needed" do
+    it "pushes notification subscription messages" do
+      described_class.call(user1, chat_channel, message.message_html)
+      expect(Webpush).to have_received(:payload_send)
+    end
+  end
+
+  context "when push is not necessary" do
+    before do
+      PushNotificationSubscription.last.user.chat_channel_memberships.order("last_opened_at DESC").first.update(last_opened_at: 3.seconds.ago)
+    end
+
+    it "does not push subscription message" do
+      described_class.call(user1, chat_channel, message.message_html)
+      expect(Webpush).not_to have_received(:payload_send)
+    end
+  end
+end