From 83ac1563829b36ac54907464b703d81e989589b5 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Wed, 1 Nov 2023 08:45:56 +0100 Subject: [PATCH 1/8] Refactor `QuestionWorker` to send to individual users rather than all followers at once --- app/workers/question_worker.rb | 23 ++++++++++------------- lib/use_case/question/create_followers.rb | 9 +++++---- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/app/workers/question_worker.rb b/app/workers/question_worker.rb index 969b725f..6cffd5af 100644 --- a/app/workers/question_worker.rb +++ b/app/workers/question_worker.rb @@ -5,22 +5,17 @@ class QuestionWorker sidekiq_options queue: :question, retry: false - # @param user_id [Integer] user id passed from Devise + # @param follower_id [Integer] user id passed from Devise # @param question_id [Integer] newly created question id - def perform(user_id, question_id) - user = User.find(user_id) - question = Question.find(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") - user.followers.each do |f| - next if skip_inbox?(f, question, user) + return if skip_inbox?(follower, question, user) - 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) + inbox = Inbox.create(user_id: follower.id, question_id:, new: true) + follower.push_notification(webpush_app, inbox) if webpush_app end private @@ -35,7 +30,9 @@ class QuestionWorker false end + # @param [User] user + # @param [Question] question def muted?(user, question) - MuteRule.where(user:).any? { |rule| rule.applies_to? question } + user.mute_rules.any? { |rule| rule.applies_to? question } end end diff --git a/lib/use_case/question/create_followers.rb b/lib/use_case/question/create_followers.rb index 04fe3844..fd4a73de 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] } + QuestionWorker.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 From 1dd1c828ebe8237297f6c57f4cbd6a96c41bb9f0 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Wed, 1 Nov 2023 22:40:02 +0100 Subject: [PATCH 2/8] Fix broken mute check in question worker --- app/workers/question_worker.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/workers/question_worker.rb b/app/workers/question_worker.rb index 6cffd5af..d0c9eb98 100644 --- a/app/workers/question_worker.rb +++ b/app/workers/question_worker.rb @@ -12,7 +12,7 @@ class QuestionWorker question = Question.includes(:user).find(question_id) webpush_app = Rpush::App.find_by(name: "webpush") - return if skip_inbox?(follower, question, user) + 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 @@ -20,11 +20,11 @@ class QuestionWorker private - def skip_inbox?(follower, question, user) + 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 user.muting?(question.user) + return true if follower.muting?(question.user) return true if question.long? && !follower.profile.allow_long_questions false From 2327b2ce521b54f99fae553cecb4d89d05ac3a0c Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Wed, 1 Nov 2023 22:40:41 +0100 Subject: [PATCH 3/8] Update tests for single-recipient QuestionWorker behaviour --- .../ajax/question_controller_spec.rb | 19 ++++-- .../question/create_followers_spec.rb | 15 ++++- spec/workers/question_worker_spec.rb | 59 ++++++++----------- 3 files changed, 50 insertions(+), 43 deletions(-) diff --git a/spec/controllers/ajax/question_controller_spec.rb b/spec/controllers/ajax/question_controller_spec.rb index 021a4172..0fc8922b 100644 --- a/spec/controllers/ajax/question_controller_spec.rb +++ b/spec/controllers/ajax/question_controller_spec.rb @@ -48,11 +48,13 @@ describe Ajax::QuestionController, :ajax_controller, type: :controller do include_examples "returns the expected response" end - shared_examples "enqueues a QuestionWorker job" do |_expected_rcpt| + shared_examples "enqueues QuestionWorker jobs" do it "enqueues a QuestionWorker job" do - allow(QuestionWorker).to receive(:perform_async) + allow(QuestionWorker).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(QuestionWorker).to have_received(:perform_bulk).with(bulk_args) end include_examples "returns the expected response" @@ -61,8 +63,10 @@ describe Ajax::QuestionController, :ajax_controller, type: :controller do shared_examples "does not enqueue a QuestionWorker job" do it "does not enqueue a QuestionWorker job" do allow(QuestionWorker).to receive(:perform_async) + allow(QuestionWorker).to receive(:perform_bulk) subject expect(QuestionWorker).not_to have_received(:perform_async) + expect(QuestionWorker).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 QuestionWorker 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 QuestionWorker jobs" end end diff --git a/spec/lib/use_case/question/create_followers_spec.rb b/spec/lib/use_case/question/create_followers_spec.rb index 7b3e2d75..3f048370 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(QuestionWorker).to have_enqueued_sidekiq_job(target_user.id, subject[:resource].id) + end end it "increments the asked count" do diff --git a/spec/workers/question_worker_spec.rb b/spec/workers/question_worker_spec.rb index 0795e55b..3d03b7e8 100644 --- a/spec/workers/question_worker_spec.rb +++ b/spec/workers/question_worker_spec.rb @@ -9,22 +9,21 @@ 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 - 5.times do - other_user = FactoryBot.create(:user) - other_user.follow(user) - end + follower.follow(user) end - subject { described_class.new.perform(user_id, question_id) } + 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: user.followers.ids, question_id:, new: true).count } + change { Inbox.where(user_id: follower_id, question_id:, new: true).count } .from(0) - .to(5) + .to(1) ) end @@ -32,56 +31,41 @@ describe QuestionWorker do question.content = "Some spicy question text" question.save - MuteRule.create(user_id: user.followers.first.id, muted_phrase: "spicy") + MuteRule.create(user_id: follower_id, muted_phrase: "spicy") - expect { subject } - .to( - change { Inbox.where(user_id: user.followers.ids, question_id:, new: true).count } - .from(0) - .to(4) - ) + subject + expect(Inbox.where(user_id: follower_id, question_id:, new: true).count).to eq(0) end it "respects inbox locks" do - user.followers.first.update(privacy_lock_inbox: true) + follower.update(privacy_lock_inbox: true) - expect { subject } - .to( - change { Inbox.where(user_id: user.followers.ids, question_id:, new: true).count } - .from(0) - .to(4) - ) + 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 - user.followers.first.ban + follower.ban - expect { subject } - .to( - change { Inbox.where(user_id: user.followers.ids, question_id:, new: true).count } - .from(0) - .to(4) - ) + subject + expect(Inbox.where(user_id: follower_id, question_id:, new: true).count).to eq(0) 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: receiver, + user: follower, subscription: { endpoint: "This will not be used", keys: {}, } ) - receiver.follow(user) end it "sends notifications" do @@ -98,15 +82,20 @@ describe QuestionWorker do let(:content) { "x" * 1000 } it "sends to recipients who allow long questions" do - user.followers.first.profile.update(allow_long_questions: true) + follower.profile.update(allow_long_questions: true) expect { subject } .to( - change { Inbox.where(user_id: user.followers.ids, question_id:, new: true).count } + 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 From b193ad229f84c4ea462a2ad2ed14be05e6051f45 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Wed, 1 Nov 2023 22:44:52 +0100 Subject: [PATCH 4/8] Fix lint error --- spec/workers/question_worker_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/workers/question_worker_spec.rb b/spec/workers/question_worker_spec.rb index 3d03b7e8..5ced7465 100644 --- a/spec/workers/question_worker_spec.rb +++ b/spec/workers/question_worker_spec.rb @@ -23,7 +23,7 @@ describe QuestionWorker do .to( change { Inbox.where(user_id: follower_id, question_id:, new: true).count } .from(0) - .to(1) + .to(1), ) end @@ -64,7 +64,7 @@ describe QuestionWorker do subscription: { endpoint: "This will not be used", keys: {}, - } + }, ) end @@ -73,7 +73,7 @@ describe QuestionWorker do .to( change { Rpush::Webpush::Notification.count } .from(0) - .to(1) + .to(1), ) end end @@ -88,7 +88,7 @@ describe QuestionWorker do .to( change { Inbox.where(user_id: follower_id, question_id:, new: true).count } .from(0) - .to(1) + .to(1), ) end From aaee04b5ed9553a52e05de10470f7f2d6c5996f9 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Mon, 11 Dec 2023 19:54:56 +0100 Subject: [PATCH 5/8] Restore `QuestionWorker` for compatibility during upgrade Co-authored-by: Georg Gadinger --- app/workers/question_worker.rb | 30 ++++--- app/workers/send_to_inbox_job.rb | 36 ++++++++ lib/use_case/question/create_followers.rb | 2 +- spec/workers/question_worker_spec.rb | 65 ++++++++------ spec/workers/send_to_inbox_job_spec.rb | 101 ++++++++++++++++++++++ 5 files changed, 192 insertions(+), 42 deletions(-) create mode 100644 app/workers/send_to_inbox_job.rb create mode 100644 spec/workers/send_to_inbox_job_spec.rb 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 From 64ac18843e8eed2ba49443a1a9b3150ee965ee6a Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Mon, 11 Dec 2023 19:56:50 +0100 Subject: [PATCH 6/8] Replace usages of `QuestionWorker` --- app/controllers/ajax/question_controller.rb | 33 ++++++++++--------- .../ajax/question_controller_spec.rb | 26 +++++++-------- .../question/create_followers_spec.rb | 2 +- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/app/controllers/ajax/question_controller.rb b/app/controllers/ajax/question_controller.rb index f3bef810..4cf425b5 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,21 @@ 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: current_user, + ) + + @response[:status] = :okay + @response[:message] = t(".success") + @response[:success] = true + end + end diff --git a/spec/controllers/ajax/question_controller_spec.rb b/spec/controllers/ajax/question_controller_spec.rb index 0fc8922b..eaa37774 100644 --- a/spec/controllers/ajax/question_controller_spec.rb +++ b/spec/controllers/ajax/question_controller_spec.rb @@ -48,25 +48,25 @@ describe Ajax::QuestionController, :ajax_controller, type: :controller do include_examples "returns the expected response" end - shared_examples "enqueues QuestionWorker jobs" do - it "enqueues a QuestionWorker job" do - allow(QuestionWorker).to receive(:perform_bulk) + shared_examples "enqueues SendToInboxJob jobs" do + it "enqueues a SendToInboxJob job" do + allow(SendToInboxJob).to receive(:perform_bulk) subject question_id = Question.last.id bulk_args = followers.map { |f| [f.id, question_id] } - expect(QuestionWorker).to have_received(:perform_bulk).with(bulk_args) + 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) - allow(QuestionWorker).to receive(:perform_bulk) + 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(QuestionWorker).not_to have_received(:perform_bulk) + expect(SendToInboxJob).not_to have_received(:perform_async) + expect(SendToInboxJob).not_to have_received(:perform_bulk) end include_examples "returns the expected response" @@ -209,7 +209,7 @@ describe Ajax::QuestionController, :ajax_controller, type: :controller do let(:expected_question_anonymous) { false } include_examples "creates the question", false - include_examples "enqueues QuestionWorker jobs" + include_examples "enqueues SendToInboxJob jobs" end context "when anonymousQuestion is false" do @@ -217,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 QuestionWorker jobs" + include_examples "enqueues SendToInboxJob jobs" end end @@ -384,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 3f048370..b5511459 100644 --- a/spec/lib/use_case/question/create_followers_spec.rb +++ b/spec/lib/use_case/question/create_followers_spec.rb @@ -29,7 +29,7 @@ describe UseCase::Question::CreateFollowers do it "enqueues a QuestionWorker job" do followers.each do |target_user| - expect(QuestionWorker).to have_enqueued_sidekiq_job(target_user.id, subject[:resource].id) + expect(SendToInboxJob).to have_enqueued_sidekiq_job(target_user.id, subject[:resource].id) end end From b2b98260712ec211b6dca47878af88c86c155655 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Mon, 11 Dec 2023 19:59:07 +0100 Subject: [PATCH 7/8] Fix lint errors --- app/controllers/ajax/question_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/ajax/question_controller.rb b/app/controllers/ajax/question_controller.rb index 4cf425b5..e8b516d8 100644 --- a/app/controllers/ajax/question_controller.rb +++ b/app/controllers/ajax/question_controller.rb @@ -37,12 +37,11 @@ class Ajax::QuestionController < AjaxController UseCase::Question::Destroy.call( question_id: params[:question], - current_user: current_user, + current_user:, ) @response[:status] = :okay @response[:message] = t(".success") @response[:success] = true end - end From e481721ea927c9c9859ad9133a71287c397c3307 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Mon, 11 Dec 2023 20:02:30 +0100 Subject: [PATCH 8/8] Use `Sidekiq::Job` --- app/workers/send_to_inbox_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/workers/send_to_inbox_job.rb b/app/workers/send_to_inbox_job.rb index ef1f30b5..6b8eec8a 100644 --- a/app/workers/send_to_inbox_job.rb +++ b/app/workers/send_to_inbox_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class SendToInboxJob - include Sidekiq::Worker + include Sidekiq::Job sidekiq_options queue: :question, retry: false