diff --git a/app/controllers/ajax/question_controller.rb b/app/controllers/ajax/question_controller.rb index f3bef810..e8b516d8 100644 --- a/app/controllers/ajax/question_controller.rb +++ b/app/controllers/ajax/question_controller.rb @@ -1,19 +1,6 @@ # frozen_string_literal: true class Ajax::QuestionController < AjaxController - def destroy - params.require :question - - UseCase::Question::Destroy.call( - question_id: params[:question], - current_user: current_user - ) - - @response[:status] = :okay - @response[:message] = t(".success") - @response[:success] = true - end - def create params.require :question params.require :anonymousQuestion @@ -24,14 +11,14 @@ class Ajax::QuestionController < AjaxController @response = { success: true, message: t(".success"), - status: :okay + status: :okay, } if user_signed_in? && params[:rcpt] == "followers" UseCase::Question::CreateFollowers.call( source_user_id: current_user.id, content: params[:question], - author_identifier: AnonymousBlock.get_identifier(request.remote_ip) + author_identifier: AnonymousBlock.get_identifier(request.remote_ip), ) return end @@ -41,7 +28,20 @@ class Ajax::QuestionController < AjaxController target_user_id: params[:rcpt], content: params[:question], anonymous: params[:anonymousQuestion], - author_identifier: AnonymousBlock.get_identifier(request.remote_ip) + author_identifier: AnonymousBlock.get_identifier(request.remote_ip), ) end + + def destroy + params.require :question + + UseCase::Question::Destroy.call( + question_id: params[:question], + current_user:, + ) + + @response[:status] = :okay + @response[:message] = t(".success") + @response[:success] = true + end end diff --git a/app/workers/question_worker.rb b/app/workers/question_worker.rb index 969b725f..69ee8091 100644 --- a/app/workers/question_worker.rb +++ b/app/workers/question_worker.rb @@ -1,5 +1,6 @@ # 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 @@ -35,7 +36,5 @@ class QuestionWorker false end - def muted?(user, question) - MuteRule.where(user:).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..6b8eec8a --- /dev/null +++ b/app/workers/send_to_inbox_job.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class SendToInboxJob + include Sidekiq::Job + + 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 04fe3844..52277b2b 100644 --- a/lib/use_case/question/create_followers.rb +++ b/lib/use_case/question/create_followers.rb @@ -13,13 +13,14 @@ module UseCase author_is_anonymous: false, author_identifier:, user: source_user, - direct: false + direct: false, ) increment_asked_count increment_metric - QuestionWorker.perform_async(source_user_id, question.id) + args = source_user.followers.map { |f| [f.id, question.id] } + SendToInboxJob.perform_bulk(args) { status: 201, @@ -40,12 +41,12 @@ module UseCase anonymous: false, followers: true, generated: false, - } + }, ) end def source_user - @source_user ||= ::User.find(source_user_id) + @source_user ||= ::User.includes(:followers).find(source_user_id) end end end diff --git a/spec/controllers/ajax/question_controller_spec.rb b/spec/controllers/ajax/question_controller_spec.rb index 021a4172..eaa37774 100644 --- a/spec/controllers/ajax/question_controller_spec.rb +++ b/spec/controllers/ajax/question_controller_spec.rb @@ -48,21 +48,25 @@ describe Ajax::QuestionController, :ajax_controller, type: :controller do include_examples "returns the expected response" end - shared_examples "enqueues a QuestionWorker job" do |_expected_rcpt| - it "enqueues a QuestionWorker job" do - allow(QuestionWorker).to receive(:perform_async) + shared_examples "enqueues SendToInboxJob jobs" do + it "enqueues a SendToInboxJob job" do + allow(SendToInboxJob).to receive(:perform_bulk) subject - expect(QuestionWorker).to have_received(:perform_async).with(user.id, Question.last.id) + question_id = Question.last.id + bulk_args = followers.map { |f| [f.id, question_id] } + expect(SendToInboxJob).to have_received(:perform_bulk).with(bulk_args) end include_examples "returns the expected response" end - shared_examples "does not enqueue a QuestionWorker job" do - it "does not enqueue a QuestionWorker job" do - allow(QuestionWorker).to receive(:perform_async) + shared_examples "does not enqueue a SendToInboxJob job" do + it "does not enqueue a SendToInboxJob job" do + allow(SendToInboxJob).to receive(:perform_async) + allow(SendToInboxJob).to receive(:perform_bulk) subject - expect(QuestionWorker).not_to have_received(:perform_async) + expect(SendToInboxJob).not_to have_received(:perform_async) + expect(SendToInboxJob).not_to have_received(:perform_bulk) end include_examples "returns the expected response" @@ -194,13 +198,18 @@ describe Ajax::QuestionController, :ajax_controller, type: :controller do context "when rcpt is followers" do let(:rcpt) { "followers" } + let(:followers) { FactoryBot.create_list(:user, 3) } + + before do + followers.each { |follower| follower.follow(user) } + end context "when anonymousQuestion is true" do let(:anonymous_question) { "true" } let(:expected_question_anonymous) { false } include_examples "creates the question", false - include_examples "enqueues a QuestionWorker job", "followers" + include_examples "enqueues SendToInboxJob jobs" end context "when anonymousQuestion is false" do @@ -208,7 +217,7 @@ describe Ajax::QuestionController, :ajax_controller, type: :controller do let(:expected_question_anonymous) { false } include_examples "creates the question", false - include_examples "enqueues a QuestionWorker job", "followers" + include_examples "enqueues SendToInboxJob jobs" end end @@ -375,7 +384,7 @@ describe Ajax::QuestionController, :ajax_controller, type: :controller do } end - include_examples "does not enqueue a QuestionWorker job" + include_examples "does not enqueue a SendToInboxJob job" end context "when rcpt is an invalid value" do diff --git a/spec/lib/use_case/question/create_followers_spec.rb b/spec/lib/use_case/question/create_followers_spec.rb index 7b3e2d75..b5511459 100644 --- a/spec/lib/use_case/question/create_followers_spec.rb +++ b/spec/lib/use_case/question/create_followers_spec.rb @@ -6,13 +6,20 @@ describe UseCase::Question::CreateFollowers do subject do UseCase::Question::CreateFollowers.call( source_user_id: source_user.id, - content: content, - author_identifier: author_identifier + content:, + author_identifier:, ) end context "user is logged in" do + before do + followers.each do |target_user| + target_user.follow source_user + end + end + let(:source_user) { create(:user) } + let(:followers) { create_list(:user, 5) } let(:content) { "content" } let(:author_identifier) { nil } @@ -21,7 +28,9 @@ describe UseCase::Question::CreateFollowers do end it "enqueues a QuestionWorker job" do - expect(QuestionWorker).to have_enqueued_sidekiq_job(source_user.id, subject[:resource].id) + followers.each do |target_user| + expect(SendToInboxJob).to have_enqueued_sidekiq_job(target_user.id, subject[:resource].id) + end end it "increments the asked count" do 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