diff --git a/app/jobs/notification/remove_duplicate_notification_job.rb b/app/jobs/notification/remove_duplicate_notification_job.rb new file mode 100644 index 000000000..6baa87460 --- /dev/null +++ b/app/jobs/notification/remove_duplicate_notification_job.rb @@ -0,0 +1,16 @@ +class Notification::RemoveDuplicateNotificationJob < ApplicationJob + queue_as :default + + def perform(notification) + return unless notification.is_a?(Notification) + + user_id = notification.user_id + primary_actor_id = notification.primary_actor_id + + # Find older notifications with the same user and primary_actor_id, excluding the new one + duplicate_notifications = Notification.where(user_id: user_id, primary_actor_id: primary_actor_id) + .where.not(id: notification.id) + + duplicate_notifications.each(&:destroy) + end +end diff --git a/app/listeners/action_cable_listener.rb b/app/listeners/action_cable_listener.rb index 01e6b2735..b74c46909 100644 --- a/app/listeners/action_cable_listener.rb +++ b/app/listeners/action_cable_listener.rb @@ -10,7 +10,7 @@ class ActionCableListener < BaseListener def notification_deleted(event) notification, account, unread_count, count = extract_notification_and_account(event) tokens = [event.data[:notification].user.pubsub_token] - broadcast(account, tokens, NOTIFICATION_DELETED, notification: notification, unread_count: unread_count, count: count) + broadcast(account, tokens, NOTIFICATION_DELETED, { notification: notification.push_event_data, unread_count: unread_count, count: count }) end def account_cache_invalidated(event) diff --git a/app/models/notification.rb b/app/models/notification.rb index 1c5c42780..5f5969a97 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -52,19 +52,23 @@ class Notification < ApplicationRecord def push_event_data # Secondary actor could be nil for cases like system assigning conversation - { + payload = { id: id, notification_type: notification_type, primary_actor_type: primary_actor_type, primary_actor_id: primary_actor_id, - primary_actor: primary_actor_data, read_at: read_at, secondary_actor: secondary_actor&.push_event_data, user: user&.push_event_data, created_at: created_at.to_i, - account_id: account_id, - push_message_title: push_message_title + account_id: account_id + } + if primary_actor.present? + payload[:primary_actor] = primary_actor_data + payload[:push_message_title] = push_message_title + end + payload end def primary_actor_data @@ -121,6 +125,9 @@ class Notification < ApplicationRecord # In future, we could probably add condition here to enqueue the job for 30 seconds later # when push enabled and then check in email job whether notification has been read already. Notification::EmailNotificationJob.perform_later(self) + + # Remove duplicate notifications + Notification::RemoveDuplicateNotificationJob.perform_later(self) end def dispatch_create_event diff --git a/spec/jobs/notification/remove_duplicate_notification_job_spec.rb b/spec/jobs/notification/remove_duplicate_notification_job_spec.rb new file mode 100644 index 000000000..c48524e1a --- /dev/null +++ b/spec/jobs/notification/remove_duplicate_notification_job_spec.rb @@ -0,0 +1,22 @@ +require 'rails_helper' + +RSpec.describe Notification::RemoveDuplicateNotificationJob do + let(:user) { create(:user) } + let(:conversation) { create(:conversation) } + + it 'enqueues the job' do + duplicate_notification = create(:notification, user: user, notification_type: 'conversation_creation', primary_actor: conversation) + expect do + described_class.perform_later(duplicate_notification) + end.to have_enqueued_job(described_class) + .on_queue('default') + end + + it 'removes duplicate notifications' do + create(:notification, user: user, notification_type: 'conversation_creation', primary_actor: conversation) + duplicate_notification = create(:notification, user: user, notification_type: 'conversation_creation', primary_actor: conversation) + + described_class.perform_now(duplicate_notification) + expect(Notification.count).to eq(1) + end +end diff --git a/spec/listeners/action_cable_listener_spec.rb b/spec/listeners/action_cable_listener_spec.rb index 35983ab91..6275c8dc3 100644 --- a/spec/listeners/action_cable_listener_spec.rb +++ b/spec/listeners/action_cable_listener_spec.rb @@ -142,7 +142,7 @@ describe ActionCableListener do 'notification.deleted', { account_id: notification.account_id, - notification: notification, + notification: notification.push_event_data, unread_count: 1, count: 1 } diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 5c81be123..31d127844 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -113,6 +113,12 @@ Hey @John, @Alisha Peter can you check this ticket?" notification = create(:notification, notification_type: 'conversation_mention', primary_actor: message.conversation, secondary_actor: message) expect(notification.push_message_title).to eq "[##{message.conversation.display_id}] Hey @John Peter please check this?" end + + it 'calls remove duplicate notification job' do + allow(Notification::RemoveDuplicateNotificationJob).to receive(:perform_later) + notification = create(:notification, notification_type: 'conversation_mention') + expect(Notification::RemoveDuplicateNotificationJob).to have_received(:perform_later).with(notification) + end end context 'when fcm push data' do