From 587d1040dc8df8c58a706601a96c531f22b0d465 Mon Sep 17 00:00:00 2001 From: Lucas Hiago Date: Tue, 28 Jan 2020 10:42:26 -0300 Subject: [PATCH] Change ModerationNotificationJob to ModerationNotificationWorker and move to sidekiq (#5683) [deploy] --- app/models/comment.rb | 2 +- app/models/notification.rb | 2 +- .../moderation_notification_worker.rb | 20 +++++++ spec/requests/notifications_spec.rb | 4 +- .../notifications/moderation/send_spec.rb | 4 +- .../moderation_notification_worker_spec.rb | 58 +++++++++++++++++++ 6 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 app/workers/notifications/moderation_notification_worker.rb create mode 100644 spec/workers/notifications/moderation_notification_worker_spec.rb diff --git a/app/models/comment.rb b/app/models/comment.rb index 92f257f3d..3b195560d 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -28,7 +28,7 @@ class Comment < ApplicationRecord before_destroy :before_destroy_actions after_create_commit :send_email_notification, if: :should_send_email_notification? after_create_commit :create_first_reaction - after_create :send_to_moderator + after_create_commit :send_to_moderator before_save :set_markdown_character_count, if: :body_markdown before_create :adjust_comment_parent_based_on_depth after_update :update_notifications, if: proc { |comment| comment.saved_changes.include? "body_markdown" } diff --git a/app/models/notification.rb b/app/models/notification.rb index 2b722a1cb..44f06aad8 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -82,7 +82,7 @@ class Notification < ApplicationRecord # TODO: make this work for articles in the future. only works for comments right now return if UserBlock.blocking?(notifiable.commentable.user_id, notifiable.user_id) - Notifications::ModerationNotificationJob.perform_later(notifiable.id) + Notifications::ModerationNotificationWorker.perform_async(notifiable.id) end def send_tag_adjustment_notification(tag_adjustment) diff --git a/app/workers/notifications/moderation_notification_worker.rb b/app/workers/notifications/moderation_notification_worker.rb new file mode 100644 index 000000000..a1e5f7a2c --- /dev/null +++ b/app/workers/notifications/moderation_notification_worker.rb @@ -0,0 +1,20 @@ +module Notifications + class ModerationNotificationWorker + include Sidekiq::Worker + + sidekiq_options queue: :medium_priority, retry: 10 + + def perform(notifiable_id) + random_moderators = Notifications::Moderation.available_moderators.order(Arel.sql("RANDOM()")).first(2) + return unless random_moderators.any? + + # notifiable is currently only comment + notifiable = Comment.find_by(id: notifiable_id) + return unless notifiable + + random_moderators.each do |mod| + Notifications::Moderation::Send.call(mod, notifiable) + end + end + end +end diff --git a/spec/requests/notifications_spec.rb b/spec/requests/notifications_spec.rb index a25f9721b..63f567c40 100644 --- a/spec/requests/notifications_spec.rb +++ b/spec/requests/notifications_spec.rb @@ -216,7 +216,7 @@ RSpec.describe "NotificationsIndex", type: :request do before do user.add_role :trusted sign_in user - perform_enqueued_jobs do + sidekiq_perform_enqueued_jobs do Notification.send_moderation_notification(comment) end get "/notifications" @@ -242,7 +242,7 @@ RSpec.describe "NotificationsIndex", type: :request do before do sign_in user - perform_enqueued_jobs do + sidekiq_perform_enqueued_jobs do Notification.send_moderation_notification(comment) end get "/notifications" diff --git a/spec/services/notifications/moderation/send_spec.rb b/spec/services/notifications/moderation/send_spec.rb index 21aebda74..f53d1e028 100644 --- a/spec/services/notifications/moderation/send_spec.rb +++ b/spec/services/notifications/moderation/send_spec.rb @@ -13,11 +13,11 @@ RSpec.describe Notifications::Moderation::Send, type: :service do create(:user, :trusted, last_moderation_notification: last_moderation_time) allow(User).to receive(:dev_account).and_return(dev_account) # Creating a comment calls moderation job which itself call moderation service - Comment.skip_callback(:create, :after, :send_to_moderator) + Comment.skip_callback(:commit, :after, :send_to_moderator) end after do - Comment.set_callback(:create, :after, :send_to_moderator) + Comment.set_callback(:commit, :after, :send_to_moderator) end it "calls comment_data since parameter is a comment" do diff --git a/spec/workers/notifications/moderation_notification_worker_spec.rb b/spec/workers/notifications/moderation_notification_worker_spec.rb new file mode 100644 index 000000000..81308a443 --- /dev/null +++ b/spec/workers/notifications/moderation_notification_worker_spec.rb @@ -0,0 +1,58 @@ +require "rails_helper" + +RSpec.describe Notifications::ModerationNotificationWorker do + describe "#perform" do + let(:id) { rand(1000) } + let(:comment) do + comment = double + allow(Comment).to receive(:find_by).and_return(comment) + end + let(:mod) do + last_moderation_time = Time.zone.now - Notifications::Moderation::MODERATORS_AVAILABILITY_DELAY - 2.hours + create(:user, :trusted, last_moderation_notification: last_moderation_time) + end + let(:worker) { subject } + + before do + allow(Notifications::Moderation::Send).to receive(:call) + end + + describe "When available moderator(s) + comment" do + it "calls the service" do + mod + comment + check_received_call + end + end + + describe "When no available moderator" do + it "does not call the service" do + comment + check_non_received_call + end + end + + describe "When no valid comment" do + it "does not call the service" do + mod + check_non_received_call + end + end + + describe "When no valid comment + no moderator" do + it "does not call the service" do + check_non_received_call + end + end + + def check_received_call + worker.perform(id) + expect(Notifications::Moderation::Send).to have_received(:call) + end + + def check_non_received_call + worker.perform(id) + expect(Notifications::Moderation::Send).not_to have_received(:call) + end + end +end