diff --git a/app/workers/question_worker.rb b/app/workers/question_worker.rb index d0c9eb98..69ee8091 100644 --- a/app/workers/question_worker.rb +++ b/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 diff --git a/app/workers/send_to_inbox_job.rb b/app/workers/send_to_inbox_job.rb new file mode 100644 index 00000000..ef1f30b5 --- /dev/null +++ b/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 diff --git a/lib/use_case/question/create_followers.rb b/lib/use_case/question/create_followers.rb index fd4a73de..52277b2b 100644 --- a/lib/use_case/question/create_followers.rb +++ b/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, diff --git a/spec/workers/question_worker_spec.rb b/spec/workers/question_worker_spec.rb index 5ced7465..0795e55b 100644 --- a/spec/workers/question_worker_spec.rb +++ b/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 diff --git a/spec/workers/send_to_inbox_job_spec.rb b/spec/workers/send_to_inbox_job_spec.rb new file mode 100644 index 00000000..237ca75d --- /dev/null +++ b/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