From ef90b7a3d88530710c184b0a3726fe551fc34e3c Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Wed, 6 Mar 2024 20:51:56 +0530 Subject: [PATCH] feat: Revamp the notification title and content (#8988) --- app/models/notification.rb | 45 ++++++-- .../notification/push_notification_service.rb | 4 +- config/locales/en.yml | 10 +- spec/models/notification_spec.rb | 105 +++++++++--------- 4 files changed, 95 insertions(+), 69 deletions(-) diff --git a/app/models/notification.rb b/app/models/notification.rb index 113c6cf3e..c7f8d6a2d 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -70,7 +70,8 @@ class Notification < ApplicationRecord } if primary_actor.present? payload[:primary_actor] = primary_actor&.push_event_data - payload[:push_message_title] = push_message_title + # TODO: Rename push_message_title to push_message_body + payload[:push_message_title] = push_message_body end payload end @@ -93,13 +94,20 @@ class Notification < ApplicationRecord when 'conversation_assignment' I18n.t('notifications.notification_title.conversation_assignment', display_id: conversation.display_id) when 'assigned_conversation_new_message', 'participating_conversation_new_message' - I18n.t( - 'notifications.notification_title.assigned_conversation_new_message', - display_id: conversation.display_id, - content: content - ) + I18n.t('notifications.notification_title.assigned_conversation_new_message', display_id: conversation.display_id) when 'conversation_mention' - "[##{conversation&.display_id}] #{transform_user_mention_content content}" + I18n.t('notifications.notification_title.conversation_mention', display_id: conversation.display_id) + else + '' + end + end + + def push_message_body + case notification_type + when 'conversation_creation' + message_body(conversation.messages.first) + when 'assigned_conversation_new_message', 'participating_conversation_new_message', 'conversation_assignment', 'conversation_mention' + message_body(secondary_actor) else '' end @@ -109,11 +117,28 @@ class Notification < ApplicationRecord primary_actor end - def content - transform_user_mention_content(secondary_actor&.content&.truncate_words(10) || '') + private + + def message_body(actor) + sender_name = sender_name(actor) + content = message_content(actor) + "#{sender_name}: #{content}" end - private + def sender_name(actor) + actor.try(:sender)&.name || '' + end + + def message_content(actor) + content = actor.try(:content) + attachments = actor.try(:attachments) + + if content.present? + transform_user_mention_content(content.truncate_words(10)) + else + attachments.present? ? I18n.t('notifications.attachment') : I18n.t('notifications.no_content') + end + end def process_notification_delivery Notification::PushNotificationJob.perform_later(self) if user_subscribed_to_notification?('push') diff --git a/app/services/notification/push_notification_service.rb b/app/services/notification/push_notification_service.rb index 765483f5e..b7fc231fa 100644 --- a/app/services/notification/push_notification_service.rb +++ b/app/services/notification/push_notification_service.rb @@ -93,8 +93,8 @@ class Notification::PushNotificationService def fcm_options { notification: { - title: notification.notification_type.titleize, - body: notification.push_message_title, + title: notification.push_message_title, + body: notification.push_message_body, sound: 'default' }, android: { priority: 'high' }, diff --git a/config/locales/en.yml b/config/locales/en.yml index 78f1f9490..075d27d1d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -118,10 +118,12 @@ en: notifications: notification_title: - conversation_creation: "[New conversation] - #%{display_id} has been created in %{inbox_name}" - conversation_assignment: "[Assigned to you] - #%{display_id} has been assigned to you" - assigned_conversation_new_message: "[New message] - #%{display_id} %{content}" - conversation_mention: "You have been mentioned in conversation [ID - %{display_id}] by %{name}" + conversation_creation: "A conversation (#%{display_id}) has been created in %{inbox_name}" + conversation_assignment: "A conversation (#%{display_id}) has been assigned to you" + assigned_conversation_new_message: "A new message is created in conversation (#%{display_id})" + conversation_mention: "You have been mentioned in conversation (#%{display_id})" + attachment: "Attachment" + no_content: "No content" conversations: messages: instagram_story_content: "%{story_sender} mentioned you in the story: " diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index cb3679290..087e245f0 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -24,14 +24,14 @@ RSpec.describe Notification do context 'when push_title is called' do it 'returns appropriate title suited for the notification type conversation_creation' do notification = create(:notification, notification_type: 'conversation_creation') - expect(notification.push_message_title).to eq "[New conversation] - ##{notification.primary_actor.display_id} has\ - been created in #{notification.primary_actor.inbox.name}" + expect(notification.push_message_title).to eq "A conversation (##{notification.primary_actor.display_id}) \ +has been created in #{notification.primary_actor.inbox.name}" end it 'returns appropriate title suited for the notification type conversation_assignment' do notification = create(:notification, notification_type: 'conversation_assignment') - - expect(notification.push_message_title).to eq "[Assigned to you] - ##{notification.primary_actor.display_id} has been assigned to you" + expect(notification.push_message_title).to eq "A conversation (##{notification.primary_actor.display_id}) \ +has been assigned to you" end it 'returns appropriate title suited for the notification type assigned_conversation_new_message' do @@ -39,8 +39,7 @@ RSpec.describe Notification do notification = create(:notification, notification_type: 'assigned_conversation_new_message', primary_actor: message.conversation, secondary_actor: message) - expect(notification.push_message_title).to eq "[New message] - ##{notification.conversation.display_id} \ -#{message.content.truncate_words(10)}" + expect(notification.push_message_title).to eq "A new message is created in conversation (##{notification.primary_actor.display_id})" end it 'returns appropriate title suited for the notification type assigned_conversation_new_message when attachment message' do @@ -49,7 +48,7 @@ RSpec.describe Notification do notification = create(:notification, notification_type: 'assigned_conversation_new_message', primary_actor: message.conversation, secondary_actor: message) - expect(notification.push_message_title).to eq "[New message] - ##{notification.conversation.display_id} " + expect(notification.push_message_title).to eq "A new message is created in conversation (##{notification.primary_actor.display_id})" end it 'returns appropriate title suited for the notification type participating_conversation_new_message' do @@ -57,61 +56,61 @@ RSpec.describe Notification do notification = create(:notification, notification_type: 'participating_conversation_new_message', primary_actor: message.conversation, secondary_actor: message) - expect(notification.push_message_title).to eq "[New message] - ##{notification.conversation.display_id} \ -#{message.content.truncate_words(10)}" - end - - it 'returns appropriate title suited for the notification type participating_conversation_new_message having mention' do - message = create(:message, sender: create(:user), content: 'Hey [@John](mention://user/1/john), can you check this ticket?') - notification = create(:notification, notification_type: 'participating_conversation_new_message', primary_actor: message.conversation, - secondary_actor: message) - expect(notification.push_message_title).to eq "[New message] - ##{notification.conversation.display_id} Hey @John, can you check this ticket?" - end - - it 'returns appropriate title suited for the notification type participating_conversation_new_message having multple mention' do - message = create(:message, sender: create(:user), - content: 'Hey [@John](mention://user/1/john), [@Alisha Peter](mention://user/2/alisha) can you check this ticket?') - notification = create(:notification, notification_type: 'participating_conversation_new_message', primary_actor: message.conversation, - secondary_actor: message) - - expect(notification.push_message_title).to eq "[New message] - ##{notification.conversation.display_id} \ -Hey @John, @Alisha Peter can you check this ticket?" - end - - it 'returns appropriate title suited for the notification type participating_conversation_new_message if username contains white space' do - message = create(:message, sender: create(:user), content: 'Hey [@John Peter](mention://user/1/john%20K) please check this?') - notification = create(:notification, notification_type: 'participating_conversation_new_message', primary_actor: message.conversation, - secondary_actor: message) - - expect(notification.push_message_title).to eq "[New message] - ##{notification.conversation.display_id} Hey @John Peter please check this?" + expect(notification.push_message_title).to eq "A new message is created in conversation (##{notification.primary_actor.display_id})" end it 'returns appropriate title suited for the notification type conversation_mention' do message = create(:message, sender: create(:user), content: 'Hey [@John](mention://user/1/john), can you check this ticket?') - notification = create(:notification, notification_type: 'conversation_mention', primary_actor: message.conversation, secondary_actor: message) + 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, can you check this ticket?" + expect(notification.push_message_title).to eq "You have been mentioned in conversation (##{notification.primary_actor.display_id})" + end + end + + context 'when push_message_body is called' do + it 'returns appropriate body suited for the notification type conversation_creation' do + conversation = create(:conversation) + message = create(:message, sender: create(:user), content: Faker::Lorem.paragraphs(number: 2), conversation: conversation) + notification = create(:notification, notification_type: 'conversation_creation', primary_actor: conversation, secondary_actor: message) + expect(notification.push_message_body).to eq "#{message.sender.name}: #{message.content.truncate_words(10)}" end - it 'returns appropriate title suited for the notification type conversation_mention having multiple mentions' do - message = create( - :message, sender: - create(:user), - content: 'Hey [@John](mention://user/1/john), [@Alisha Peter](mention://user/2/alisha) 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, @Alisha Peter can you check this ticket?" + it 'returns appropriate body suited for the notification type assigned_conversation_new_message' do + conversation = create(:conversation) + message = create(:message, sender: create(:user), content: Faker::Lorem.paragraphs(number: 2), conversation: conversation) + notification = create(:notification, notification_type: 'assigned_conversation_new_message', primary_actor: conversation, + secondary_actor: message) + expect(notification.push_message_body).to eq "#{message.sender.name}: #{message.content.truncate_words(10)}" end - it 'returns appropriate title suited for the notification type conversation_mention if username contains white space' do - message = create( - :message, sender: - create(:user), - content: 'Hey [@John Peter](mention://user/1/john%20K) please check this?' - ) - 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?" + it 'returns appropriate body suited for the notification type assigned_conversation_new_message when attachment message' do + conversation = create(:conversation) + message = create(:message, sender: create(:user), content: nil, conversation: conversation) + attachment = message.attachments.new(file_type: :image, account_id: message.account_id) + attachment.file.attach(io: Rails.root.join('spec/assets/avatar.png').open, filename: 'avatar.png', content_type: 'image/png') + message.save! + notification = create(:notification, notification_type: 'assigned_conversation_new_message', primary_actor: conversation, + secondary_actor: message) + expect(notification.push_message_body).to eq "#{message.sender.name}: Attachment" + end + + it 'returns appropriate body suited for the notification type participating_conversation_new_message having multple mention' do + conversation = create(:conversation) + message = create(:message, sender: create(:user), + content: 'Hey [@John](mention://user/1/john), [@Alisha Peter](mention://user/2/alisha) can you check this ticket?', + conversation: conversation) + notification = create(:notification, notification_type: 'participating_conversation_new_message', primary_actor: conversation, + secondary_actor: message) + expect(notification.push_message_body).to eq "#{message.sender.name}: Hey @John, @Alisha Peter can you check this ticket?" + end + + it 'returns appropriate body suited for the notification type conversation_mention if username contains white space' do + conversation = create(:conversation) + message = create(:message, sender: create(:user), content: 'Hey [@John Peter](mention://user/1/john%20K) please check this?', + conversation: conversation) + notification = create(:notification, notification_type: 'conversation_mention', primary_actor: conversation, secondary_actor: message) + expect(notification.push_message_body).to eq "#{message.sender.name}: Hey @John Peter please check this?" end it 'calls remove duplicate notification job' do