From da733435408da164af96414ac4c285e2c38c9121 Mon Sep 17 00:00:00 2001 From: PJ Date: Tue, 19 Sep 2023 15:03:20 +0100 Subject: [PATCH] Skip round robin mod notifications for limited users (#20128) * return early if user is limited * fix worker specs --- app/services/notifications/moderation/send.rb | 5 +- ...d_robin_moderation_notifications_worker.rb | 4 +- .../notifications/moderation/send_spec.rb | 16 +++ ...in_moderation_notifications_worker_spec.rb | 101 ++++++++++-------- 4 files changed, 80 insertions(+), 46 deletions(-) diff --git a/app/services/notifications/moderation/send.rb b/app/services/notifications/moderation/send.rb index 685e3e8ba..2af4e8c6b 100644 --- a/app/services/notifications/moderation/send.rb +++ b/app/services/notifications/moderation/send.rb @@ -19,8 +19,9 @@ module Notifications def call return unless notifiable_supported?(notifiable) - # do not create the notification if the comment/article was created by the moderator - return if moderator == notifiable.user + # do not create the notification if the comment/article was created by + # the moderator of the user has the `limited` role + return if notifiable.user.limited? || moderator == notifiable.user json_data = { user: user_data(User.staff_account) } notifiable_name = notifiable.class.name.downcase diff --git a/app/workers/notifications/create_round_robin_moderation_notifications_worker.rb b/app/workers/notifications/create_round_robin_moderation_notifications_worker.rb index 77979b700..31b41d75e 100644 --- a/app/workers/notifications/create_round_robin_moderation_notifications_worker.rb +++ b/app/workers/notifications/create_round_robin_moderation_notifications_worker.rb @@ -14,13 +14,13 @@ module Notifications if notifiable_type == "Comment" notifiable = Comment.find_by(id: notifiable_id) # return if it's a comment whose commentable has been deleted - return unless notifiable.commentable + return unless notifiable&.commentable elsif notifiable_type == "Article" notifiable = Article.find_by(id: notifiable_id) end - return unless notifiable + return unless notifiable && !notifiable.user.limited? random_moderators.each do |mod| next if mod == notifiable.user diff --git a/spec/services/notifications/moderation/send_spec.rb b/spec/services/notifications/moderation/send_spec.rb index 9781633ea..8a03b6f47 100644 --- a/spec/services/notifications/moderation/send_spec.rb +++ b/spec/services/notifications/moderation/send_spec.rb @@ -63,6 +63,14 @@ RSpec.describe Notifications::Moderation::Send, type: :service do expect(notification.json_data["user"]["id"]).to eq(staff_account.id) expect(notification.json_data["comment_user"]["id"]).to eq(comment.user.id) end + + context "when the comment's author is limited" do + let(:user) { create(:user, :limited) } + + it "does not create a notification" do + expect { described_class.call(moderator, comment) }.not_to change(Notification, :count) + end + end end context "when notifying on articles" do @@ -113,5 +121,13 @@ RSpec.describe Notifications::Moderation::Send, type: :service do expect(notification.json_data["user"]["id"]).to eq(staff_account.id) expect(notification.json_data["article_user"]["id"]).to eq(article.user.id) end + + context "when the article's author is limited" do + let(:user) { create(:user, :limited) } + + it "does not create a notification" do + expect { described_class.call(moderator, article) }.not_to change(Notification, :count) + end + end end end diff --git a/spec/workers/notifications/create_round_robin_moderation_notifications_worker_spec.rb b/spec/workers/notifications/create_round_robin_moderation_notifications_worker_spec.rb index c6431e596..a178f8dea 100644 --- a/spec/workers/notifications/create_round_robin_moderation_notifications_worker_spec.rb +++ b/spec/workers/notifications/create_round_robin_moderation_notifications_worker_spec.rb @@ -1,28 +1,17 @@ require "rails_helper" RSpec.describe Notifications::CreateRoundRobinModerationNotificationsWorker do - let(:id) { rand(1000) } - let(:comment) do - comment = double - allow(Comment).to receive(:find_by).and_return(comment) - allow(comment).to receive(:user) - allow(comment).to receive(:commentable).and_return(true) - comment - end - let(:article) do - article = double - allow(Article).to receive(:find_by).and_return(article) - allow(article).to receive(:user) - article - end - let(:mod) do - last_moderation_time = Time.current - Notifications::Moderation::MODERATORS_AVAILABILITY_DELAY - 1.week - u = create(:user, :trusted, last_moderation_notification: last_moderation_time, last_reacted_at: 2.days.ago) - u.notification_setting.update(mod_roundrobin_notifications: true) - u - end + let(:comment) { create(:comment) } + let(:article) { create(:article) } + let!(:mod) { create(:user, :trusted) } let(:worker) { subject } + def prepare_for_round_robin(user) + last_moderation_time = Time.current - Notifications::Moderation::MODERATORS_AVAILABILITY_DELAY - 1.week + user.update(last_moderation_notification: last_moderation_time, last_reacted_at: 2.days.ago) + user.notification_setting.update(mod_roundrobin_notifications: true) + end + def check_received_call(notifiable_type = nil) worker.perform(id, notifiable_type) expect(Notifications::Moderation::Send).to have_received(:call) @@ -40,57 +29,87 @@ RSpec.describe Notifications::CreateRoundRobinModerationNotificationsWorker do describe "When available moderator(s) + comment" do it "calls the service" do - mod - comment - check_received_call("Comment") + prepare_for_round_robin(mod) + worker.perform(comment.id, "Comment") + expect(Notifications::Moderation::Send).to have_received(:call) end end describe "when available moderator(s) + article" do it "calls the service" do - mod - article - check_received_call("Article") + prepare_for_round_robin(mod) + worker.perform(article.id, "Article") + expect(Notifications::Moderation::Send).to have_received(:call) end end describe "when no available moderator for comment" do it "does not call the service" do - comment - check_non_received_call("Comment") + worker.perform(comment.id, "Comment") + expect(Notifications::Moderation::Send).not_to have_received(:call) end end describe "when no available moderator for article" do it "does not call the service" do - article - check_non_received_call("Article") + worker.perform(article.id, "Article") + expect(Notifications::Moderation::Send).not_to have_received(:call) end end describe "when no valid comment or article" do it "does not call the service" do - mod - check_non_received_call("Article") + prepare_for_round_robin(mod) + + worker.perform(Article.maximum(:id).to_i + 1, "Article") + expect(Notifications::Moderation::Send).not_to have_received(:call) + + worker.perform(Comment.maximum(:id).to_i + 1, "Comment") + expect(Notifications::Moderation::Send).not_to have_received(:call) end end describe "when no valid comment/article + no moderator" do it "does not call the service" do - check_non_received_call("Comment") - check_non_received_call("Article") + worker.perform(Article.maximum(:id).to_i + 1, "Article") + expect(Notifications::Moderation::Send).not_to have_received(:call) + + worker.perform(Comment.maximum(:id).to_i + 1, "Comment") + expect(Notifications::Moderation::Send).not_to have_received(:call) + end + end + + describe "when the notifiable user is limited" do + let(:user) { create(:user, :limited) } + let(:article) { create(:article, user: user) } + let(:comment) { create(:comment, user: user) } + + it "does not call the send service" do + worker.perform(article.id, "Comment") + expect(Notifications::Moderation::Send).not_to have_received(:call) + + worker.perform(comment.id, "Comment") + expect(Notifications::Moderation::Send).not_to have_received(:call) end end describe "when moderator is the comment author" do + before do + prepare_for_round_robin(mod) + end + it "does not call the service" do - comment = create(:comment, user: mod, commentable: create(:article)) + comment = create(:comment, user: mod, commentable: article) worker.perform(comment.id, "Comment") expect(Notifications::Moderation::Send).not_to have_received(:call) end end describe "when moderator is the article author" do + before do + prepare_for_round_robin(mod) + end + it "does not call the service" do article = create(:article, user: mod) worker.perform(article.id, "Article") @@ -99,14 +118,12 @@ RSpec.describe Notifications::CreateRoundRobinModerationNotificationsWorker do end describe "when the comment's commentable does not exist" do + before do + prepare_for_round_robin(mod) + comment.commentable.destroy! + end + it "does not call the service" do - mod # prepare a moderator - - article = create(:article) - comment = create(:comment, commentable: article) - - article.destroy! - worker.perform(comment.id, "Comment") expect(Notifications::Moderation::Send).not_to have_received(:call) end