Restore QuestionWorker for compatibility during upgrade

Co-authored-by: Georg Gadinger <nilsding@nilsding.org>

app/workers/question_worker.rb

@@ -1,38 +1,40 @@
 # frozen_string_literal: true
 
+# @deprecated This is to be replaced by SendToInboxJob. Remaining here so that remaining QuestionWorker jobs can finish.
 class QuestionWorker
   include Sidekiq::Worker
 
   sidekiq_options queue: :question, retry: false
 
-  # @param follower_id [Integer] user id passed from Devise
+  # @param user_id [Integer] user id passed from Devise
   # @param question_id [Integer] newly created question id
-  def perform(follower_id, question_id)
-    follower = User.includes(:web_push_subscriptions, :mute_rules, :muted_users).find(follower_id)
-    question = Question.includes(:user).find(question_id)
+  def perform(user_id, question_id)
+    user = User.find(user_id)
+    question = Question.find(question_id)
     webpush_app = Rpush::App.find_by(name: "webpush")
 
-    return if skip_inbox?(follower, question)
+    user.followers.each do |f|
+      next if skip_inbox?(f, question, user)
 
-    inbox = Inbox.create(user_id: follower.id, question_id:, new: true)
-    follower.push_notification(webpush_app, inbox) if webpush_app
+      inbox = Inbox.create(user_id: f.id, question_id:, new: true)
+      f.push_notification(webpush_app, inbox) if webpush_app
+    end
+  rescue StandardError => e
+    logger.info "failed to ask question: #{e.message}"
+    Sentry.capture_exception(e)
   end
 
   private
 
-  def skip_inbox?(follower, question)
+  def skip_inbox?(follower, question, user)
     return true if follower.inbox_locked?
     return true if follower.banned?
     return true if muted?(follower, question)
-    return true if follower.muting?(question.user)
+    return true if user.muting?(question.user)
     return true if question.long? && !follower.profile.allow_long_questions
 
     false
   end
 
-  # @param [User] user
-  # @param [Question] question
-  def muted?(user, question)
-    user.mute_rules.any? { |rule| rule.applies_to? question }
-  end
+  def muted?(user, question) = MuteRule.where(user:).any? { |rule| rule.applies_to? question }
 end

app/workers/send_to_inbox_job.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+class SendToInboxJob
+  include Sidekiq::Worker
+
+  sidekiq_options queue: :question, retry: false
+
+  # @param follower_id [Integer] user id passed from Devise
+  # @param question_id [Integer] newly created question id
+  def perform(follower_id, question_id)
+    follower = User.includes(:web_push_subscriptions, :mute_rules, :muted_users).find(follower_id)
+    question = Question.includes(:user).find(question_id)
+    webpush_app = Rpush::App.find_by(name: "webpush")
+
+    return if skip_inbox?(follower, question)
+
+    inbox = Inbox.create(user_id: follower.id, question_id:, new: true)
+    follower.push_notification(webpush_app, inbox) if webpush_app
+  end
+
+  private
+
+  def skip_inbox?(follower, question)
+    return true if follower.inbox_locked?
+    return true if follower.banned?
+    return true if muted?(follower, question)
+    return true if follower.muting?(question.user)
+    return true if question.long? && !follower.profile.allow_long_questions
+
+    false
+  end
+
+  # @param [User] user
+  # @param [Question] question
+  def muted?(user, question) = user.mute_rules.any? { |rule| rule.applies_to? question }
+end

lib/use_case/question/create_followers.rb

@@ -20,7 +20,7 @@ module UseCase
         increment_metric
 
         args = source_user.followers.map { |f| [f.id, question.id] }
-        QuestionWorker.perform_bulk(args)
+        SendToInboxJob.perform_bulk(args)
 
         {
           status:   201,

spec/workers/question_worker_spec.rb

@@ -9,21 +9,22 @@ describe QuestionWorker do
     let(:content) { Faker::Lorem.sentence }
     let(:question) { FactoryBot.create(:question, content:, user:) }
     let(:question_id) { question.id }
-    let(:follower) { FactoryBot.create(:user) }
-    let(:follower_id) { follower.id }
 
     before do
-      follower.follow(user)
+      5.times do
+        other_user = FactoryBot.create(:user)
+        other_user.follow(user)
+      end
     end
 
-    subject { described_class.new.perform(follower_id, question_id) }
+    subject { described_class.new.perform(user_id, question_id) }
 
     it "places the question in the inbox of the user's followers" do
       expect { subject }
         .to(
-          change { Inbox.where(user_id: follower_id, question_id:, new: true).count }
+          change { Inbox.where(user_id: user.followers.ids, question_id:, new: true).count }
             .from(0)
-            .to(1),
+            .to(5)
         )
     end
 
@@ -31,41 +32,56 @@ describe QuestionWorker do
       question.content = "Some spicy question text"
       question.save
 
-      MuteRule.create(user_id: follower_id, muted_phrase: "spicy")
+      MuteRule.create(user_id: user.followers.first.id, muted_phrase: "spicy")
 
-      subject
-      expect(Inbox.where(user_id: follower_id, question_id:, new: true).count).to eq(0)
+      expect { subject }
+        .to(
+          change { Inbox.where(user_id: user.followers.ids, question_id:, new: true).count }
+            .from(0)
+            .to(4)
+        )
     end
 
     it "respects inbox locks" do
-      follower.update(privacy_lock_inbox: true)
+      user.followers.first.update(privacy_lock_inbox: true)
 
-      subject
-      expect(Inbox.where(user_id: follower_id, question_id:, new: true).count).to eq(0)
+      expect { subject }
+        .to(
+          change { Inbox.where(user_id: user.followers.ids, question_id:, new: true).count }
+            .from(0)
+            .to(4)
+        )
     end
 
     it "does not send questions to banned users" do
-      follower.ban
+      user.followers.first.ban
 
-      subject
-      expect(Inbox.where(user_id: follower_id, question_id:, new: true).count).to eq(0)
+      expect { subject }
+        .to(
+          change { Inbox.where(user_id: user.followers.ids, question_id:, new: true).count }
+            .from(0)
+            .to(4)
+        )
     end
 
     context "receiver has push notifications enabled" do
+      let(:receiver) { FactoryBot.create(:user) }
+
       before do
         Rpush::Webpush::App.create(
           name:        "webpush",
           certificate: { public_key: "AAAA", private_key: "AAAA", subject: "" }.to_json,
-          connections: 1,
+          connections: 1
         )
 
         WebPushSubscription.create!(
-          user:         follower,
+          user:         receiver,
           subscription: {
             endpoint: "This will not be used",
             keys:     {},
-          },
+          }
         )
+        receiver.follow(user)
       end
 
       it "sends notifications" do
@@ -73,7 +89,7 @@ describe QuestionWorker do
           .to(
             change { Rpush::Webpush::Notification.count }
               .from(0)
-              .to(1),
+              .to(1)
           )
       end
     end
@@ -82,20 +98,15 @@ describe QuestionWorker do
       let(:content) { "x" * 1000 }
 
       it "sends to recipients who allow long questions" do
-        follower.profile.update(allow_long_questions: true)
+        user.followers.first.profile.update(allow_long_questions: true)
 
         expect { subject }
           .to(
-            change { Inbox.where(user_id: follower_id, question_id:, new: true).count }
+            change { Inbox.where(user_id: user.followers.ids, question_id:, new: true).count }
               .from(0)
-              .to(1),
+              .to(1)
           )
       end
-
-      it "does not send to recipients who do not allow long questions" do
-        subject
-        expect(Inbox.where(user_id: follower_id, question_id:, new: true).count).to eq(0)
-      end
     end
   end
 end

spec/workers/send_to_inbox_job_spec.rb

@@ -0,0 +1,101 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+describe SendToInboxJob do
+  describe "#perform" do
+    let(:user) { FactoryBot.create(:user) }
+    let(:user_id) { user.id }
+    let(:content) { Faker::Lorem.sentence }
+    let(:question) { FactoryBot.create(:question, content:, user:) }
+    let(:question_id) { question.id }
+    let(:follower) { FactoryBot.create(:user) }
+    let(:follower_id) { follower.id }
+
+    before do
+      follower.follow(user)
+    end
+
+    subject { described_class.new.perform(follower_id, question_id) }
+
+    it "places the question in the inbox of the user's followers" do
+      expect { subject }
+        .to(
+          change { Inbox.where(user_id: follower_id, question_id:, new: true).count }
+            .from(0)
+            .to(1),
+        )
+    end
+
+    it "respects mute rules" do
+      question.content = "Some spicy question text"
+      question.save
+
+      MuteRule.create(user_id: follower_id, muted_phrase: "spicy")
+
+      subject
+      expect(Inbox.where(user_id: follower_id, question_id:, new: true).count).to eq(0)
+    end
+
+    it "respects inbox locks" do
+      follower.update(privacy_lock_inbox: true)
+
+      subject
+      expect(Inbox.where(user_id: follower_id, question_id:, new: true).count).to eq(0)
+    end
+
+    it "does not send questions to banned users" do
+      follower.ban
+
+      subject
+      expect(Inbox.where(user_id: follower_id, question_id:, new: true).count).to eq(0)
+    end
+
+    context "receiver has push notifications enabled" do
+      before do
+        Rpush::Webpush::App.create(
+          name:        "webpush",
+          certificate: { public_key: "AAAA", private_key: "AAAA", subject: "" }.to_json,
+          connections: 1,
+        )
+
+        WebPushSubscription.create!(
+          user:         follower,
+          subscription: {
+            endpoint: "This will not be used",
+            keys:     {},
+          },
+        )
+      end
+
+      it "sends notifications" do
+        expect { subject }
+          .to(
+            change { Rpush::Webpush::Notification.count }
+              .from(0)
+              .to(1),
+          )
+      end
+    end
+
+    context "long question" do
+      let(:content) { "x" * 1000 }
+
+      it "sends to recipients who allow long questions" do
+        follower.profile.update(allow_long_questions: true)
+
+        expect { subject }
+          .to(
+            change { Inbox.where(user_id: follower_id, question_id:, new: true).count }
+              .from(0)
+              .to(1),
+          )
+      end
+
+      it "does not send to recipients who do not allow long questions" do
+        subject
+        expect(Inbox.where(user_id: follower_id, question_id:, new: true).count).to eq(0)
+      end
+    end
+  end
+end