From faad0b83e62f0db0ea4b88b7dde89ce7fc21ff40 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 7 Feb 2020 20:10:52 +0100 Subject: [PATCH] Handle Reactions Without Users When Sending Notifications (#5820) * Add reaction's owner existence in the database * Add test --- app/services/notifications/reactions/send.rb | 4 ++-- spec/models/notification_spec.rb | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/services/notifications/reactions/send.rb b/app/services/notifications/reactions/send.rb index 3187c1e3b..84645ece0 100644 --- a/app/services/notifications/reactions/send.rb +++ b/app/services/notifications/reactions/send.rb @@ -24,8 +24,8 @@ module Notifications reaction_siblings = Reaction.where(reactable_id: reaction.reactable_id, reactable_type: reaction.reactable_type). where.not(reactions: { user_id: reaction.reactable_user_id }). - includes(:reactable, :user). - order("created_at DESC") + preload(:reactable).includes(:user).where.not(users: { id: nil }). + order("reactions.created_at DESC") aggregated_reaction_siblings = reaction_siblings.map { |reaction| { category: reaction.category, created_at: reaction.created_at, user: user_data(reaction.user) } } diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index b6fb04c44..e14ab3965 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -292,6 +292,18 @@ RSpec.describe Notification, type: :model do end end.to change(article.user.notifications, :count).by(1) end + + it "does not send a notification to the author of an article if the reaction owner is deleted" do + user4 = create(:user) + reaction = create(:reaction, reactable: article, user: user4) + user4.delete + + expect do + sidekiq_perform_enqueued_jobs do + described_class.send_reaction_notification(reaction, reaction.reactable.user) + end + end.not_to change(article.user.notifications, :count) + end end context "when a reaction is destroyed" do