From 252027ad9c0309716a795ca1cd46a344e27bbb0d Mon Sep 17 00:00:00 2001 From: serena Date: Tue, 7 Jan 2020 13:16:17 +0000 Subject: [PATCH] use new_badge_achievement_worker instead of active job (#5358) [deploy] --- app/models/badge_achievement.rb | 2 +- app/models/notification.rb | 6 +---- .../new_badge_achievement_worker.rb | 11 ++++++++++ spec/models/badge_achievement_spec.rb | 6 +++++ spec/models/notification_spec.rb | 10 +-------- spec/requests/notifications_spec.rb | 2 +- .../internal/admin_awards_badges_spec.rb | 9 ++++---- .../new_badge_achievement_worker_spec.rb | 22 +++++++++++++++++++ 8 files changed, 48 insertions(+), 20 deletions(-) create mode 100644 app/workers/notifications/new_badge_achievement_worker.rb create mode 100644 spec/workers/notifications/new_badge_achievement_worker_spec.rb diff --git a/app/models/badge_achievement.rb b/app/models/badge_achievement.rb index 5ac7cb403..674a001f1 100644 --- a/app/models/badge_achievement.rb +++ b/app/models/badge_achievement.rb @@ -7,7 +7,7 @@ class BadgeAchievement < ApplicationRecord validates :badge_id, uniqueness: { scope: :user_id } - after_create :notify_recipient + after_create_commit :notify_recipient after_create :send_email_notification after_create :award_credits before_validation :render_rewarding_context_message_html diff --git a/app/models/notification.rb b/app/models/notification.rb index 9aa3bd4b2..d4a7cb2bf 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -62,12 +62,8 @@ class Notification < ApplicationRecord end def send_new_badge_achievement_notification(badge_achievement) - Notifications::NewBadgeAchievementJob.perform_later(badge_achievement.id) + Notifications::NewBadgeAchievementWorker.perform_async(badge_achievement.id) end - # NOTE: this alias is temporary until the transition to ActiveJob is completed - # and all old DelayedJob jobs are processed by the queue workers. - # It can be removed after pre-existing jobs are done - alias send_new_badge_notification send_new_badge_achievement_notification def send_reaction_notification(reaction, receiver) return if reaction.skip_notification_for?(receiver) diff --git a/app/workers/notifications/new_badge_achievement_worker.rb b/app/workers/notifications/new_badge_achievement_worker.rb new file mode 100644 index 000000000..a78d2e499 --- /dev/null +++ b/app/workers/notifications/new_badge_achievement_worker.rb @@ -0,0 +1,11 @@ +module Notifications + class NewBadgeAchievementWorker + include Sidekiq::Worker + sidekiq_options queue: :low_priority, retry: 10 + + def perform(badge_achievement_id) + badge_achievement = BadgeAchievement.find_by(id: badge_achievement_id) + Notifications::NewBadgeAchievement::Send.call(badge_achievement) if badge_achievement + end + end +end diff --git a/spec/models/badge_achievement_spec.rb b/spec/models/badge_achievement_spec.rb index 6e3c76db8..4bf325899 100644 --- a/spec/models/badge_achievement_spec.rb +++ b/spec/models/badge_achievement_spec.rb @@ -17,4 +17,10 @@ RSpec.describe BadgeAchievement, type: :model do it "awards credits after create" do expect(achievement.user.credits.size).to eq(5) end + + it "notifies recipients after commit" do + allow(Notification).to receive(:send_new_badge_achievement_notification) + achievement.run_callbacks(:commit) + expect(Notification).to have_received(:send_new_badge_achievement_notification).with(achievement) + end end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index f6041c649..46b77f2c6 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -491,20 +491,12 @@ RSpec.describe Notification, type: :model do describe "#send_new_badge_achievement_notification" do it "enqueues a new badge achievement job" do - assert_enqueued_with(job: Notifications::NewBadgeAchievementJob, args: [badge_achievement.id]) do + sidekiq_assert_enqueued_with(job: Notifications::NewBadgeAchievementWorker, args: [badge_achievement.id]) do described_class.send_new_badge_achievement_notification(badge_achievement) end end end - describe "#send_new_badge_notification (deprecated)" do - it "enqueues a new badge achievement job" do - assert_enqueued_with(job: Notifications::NewBadgeAchievementJob, args: [badge_achievement.id]) do - described_class.send_new_badge_notification(badge_achievement) - end - end - end - describe "#remove_all" do it "removes all mention related notifications" do mention = create(:mention, user: user, mentionable: comment) diff --git a/spec/requests/notifications_spec.rb b/spec/requests/notifications_spec.rb index a9624c3dc..3c1c41183 100644 --- a/spec/requests/notifications_spec.rb +++ b/spec/requests/notifications_spec.rb @@ -309,7 +309,7 @@ RSpec.describe "NotificationsIndex", type: :request do sign_in user badge = create(:badge) badge_achievement = create(:badge_achievement, user: user, badge: badge) - perform_enqueued_jobs do + sidekiq_perform_enqueued_jobs do Notification.send_new_badge_achievement_notification(badge_achievement) end get "/notifications" diff --git a/spec/system/internal/admin_awards_badges_spec.rb b/spec/system/internal/admin_awards_badges_spec.rb index 4e3c01498..87162a94e 100644 --- a/spec/system/internal/admin_awards_badges_spec.rb +++ b/spec/system/internal/admin_awards_badges_spec.rb @@ -40,9 +40,10 @@ RSpec.describe "Admin awards badges", type: :system do end it "notifies users of new badges" do - expect { award_two_badges }.to enqueue_job(Notifications::NewBadgeAchievementJob). - exactly(2).times. - and enqueue_job(BadgeAchievements::SendEmailNotificationJob). - exactly(2).times + assert_enqueued_jobs(2, only: BadgeAchievements::SendEmailNotificationJob) do + sidekiq_assert_enqueued_jobs(2) do + award_two_badges + end + end end end diff --git a/spec/workers/notifications/new_badge_achievement_worker_spec.rb b/spec/workers/notifications/new_badge_achievement_worker_spec.rb new file mode 100644 index 000000000..4eee26b63 --- /dev/null +++ b/spec/workers/notifications/new_badge_achievement_worker_spec.rb @@ -0,0 +1,22 @@ +require "rails_helper" +RSpec.describe Notifications::NewBadgeAchievementWorker, type: :worker do + describe "#perform" do + let(:badge_achievement) { create(:badge_achievement) } + let(:service) { Notifications::NewBadgeAchievement::Send } + let(:worker) { subject } + + before do + allow(service).to receive(:call) + end + + it "calls a service" do + worker.perform(badge_achievement.id) + expect(service).to have_received(:call).with(badge_achievement).once + end + + it "does nothing for non-existent badge achievement" do + worker.perform(nil) + expect(service).not_to have_received(:call) + end + end +end