From 949ddf68ba83259c90af5c0f78ec039493638823 Mon Sep 17 00:00:00 2001 From: Pranav Raj S Date: Wed, 15 Feb 2023 14:14:56 -0800 Subject: [PATCH] chore: Refactor the notification service for participants (#6461) --- .../api/v1/accounts/inboxes_controller.rb | 2 +- app/listeners/notification_listener.rb | 47 +----------- app/models/concerns/message_filter_helpers.rb | 4 ++ app/models/inbox.rb | 4 ++ app/services/messages/mention_service.rb | 41 +++++++++++ .../new_message_notification_service.rb | 25 +++++++ .../services/messages/mention_service_spec.rb | 65 +++++++++++++++++ .../new_message_notification_service_spec.rb | 72 +++++++++++++++++++ 8 files changed, 215 insertions(+), 45 deletions(-) create mode 100644 app/services/messages/mention_service.rb create mode 100644 app/services/messages/new_message_notification_service.rb create mode 100644 spec/services/messages/mention_service_spec.rb create mode 100644 spec/services/messages/new_message_notification_service_spec.rb diff --git a/app/controllers/api/v1/accounts/inboxes_controller.rb b/app/controllers/api/v1/accounts/inboxes_controller.rb index 24507977e..24f797025 100644 --- a/app/controllers/api/v1/accounts/inboxes_controller.rb +++ b/app/controllers/api/v1/accounts/inboxes_controller.rb @@ -14,7 +14,7 @@ class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController # Deprecated: This API will be removed in 2.7.0 def assignable_agents - @assignable_agents = (Current.account.users.where(id: @inbox.members.select(:user_id)) + Current.account.administrators).uniq + @assignable_agents = @inbox.assignable_agents end def campaigns diff --git a/app/listeners/notification_listener.rb b/app/listeners/notification_listener.rb index 7f712e896..39303450d 100644 --- a/app/listeners/notification_listener.rb +++ b/app/listeners/notification_listener.rb @@ -28,50 +28,9 @@ class NotificationListener < BaseListener end def message_created(event) - message, account = extract_message_and_account(event) - conversation = message.conversation + message = extract_message_and_account(event)[0] - generate_notifications_for_mentions(message, account) - - # only want to notify agents about customer messages - return unless message.incoming? - return unless conversation.assignee - - NotificationBuilder.new( - notification_type: 'assigned_conversation_new_message', - user: conversation.assignee, - account: account, - primary_actor: message - ).perform - end - - private - - def get_valid_mentioned_ids(mentioned_ids, inbox) - valid_mentionable_ids = inbox.account.administrators.map(&:id) + inbox.members.map(&:id) - # Intersection of ids - mentioned_ids & valid_mentionable_ids.uniq.map(&:to_s) - end - - def generate_notifications_for_mentions(message, account) - return unless message.private? - - return if message.content.blank? - - mentioned_ids = message.content.scan(%r{\(mention://(user|team)/(\d+)/(.+?)\)}).map(&:second).uniq - - return if mentioned_ids.blank? - - valid_mentioned_ids = get_valid_mentioned_ids(mentioned_ids, message.inbox) - Conversations::UserMentionJob.perform_later(valid_mentioned_ids, message.conversation.id, account.id) - - valid_mentioned_ids.each do |user_id| - NotificationBuilder.new( - notification_type: 'conversation_mention', - user: User.find(user_id), - account: account, - primary_actor: message - ).perform - end + Messages::MentionService.new(message: message).perform + Messages::NewMessageNotificationService.new(message: message).perform end end diff --git a/app/models/concerns/message_filter_helpers.rb b/app/models/concerns/message_filter_helpers.rb index 55d04a5dc..135147276 100644 --- a/app/models/concerns/message_filter_helpers.rb +++ b/app/models/concerns/message_filter_helpers.rb @@ -9,6 +9,10 @@ module MessageFilterHelpers incoming? || outgoing? end + def notifiable? + incoming? || outgoing? + end + def conversation_transcriptable? incoming? || outgoing? end diff --git a/app/models/inbox.rb b/app/models/inbox.rb index f13fcf5f6..c015b7835 100644 --- a/app/models/inbox.rb +++ b/app/models/inbox.rb @@ -106,6 +106,10 @@ class Inbox < ApplicationRecord channel_type == 'Channel::Whatsapp' end + def assignable_agents + (account.users.where(id: members.select(:user_id)) + account.administrators).uniq + end + def active_bot? agent_bot_inbox&.active? || hooks.pluck(:app_id).include?('dialogflow') end diff --git a/app/services/messages/mention_service.rb b/app/services/messages/mention_service.rb new file mode 100644 index 000000000..c7b81213b --- /dev/null +++ b/app/services/messages/mention_service.rb @@ -0,0 +1,41 @@ +class Messages::MentionService + pattr_initialize [:message!] + + def perform + return unless valid_mention_message?(message) + + validated_mentioned_ids = filter_mentioned_ids_by_inbox + return if validated_mentioned_ids.blank? + + Conversations::UserMentionJob.perform_later(validated_mentioned_ids, message.conversation.id, message.account.id) + generate_notifications_for_mentions(validated_mentioned_ids) + end + + private + + def valid_mention_message?(message) + message.private? && message.content.present? && mentioned_ids.present? + end + + def mentioned_ids + @mentioned_ids ||= message.content.scan(%r{\(mention://(user|team)/(\d+)/(.+?)\)}).map(&:second).uniq + end + + def filter_mentioned_ids_by_inbox + inbox = message.inbox + valid_mentionable_ids = inbox.account.administrators.map(&:id) + inbox.members.map(&:id) + # Intersection of ids + mentioned_ids & valid_mentionable_ids.uniq.map(&:to_s) + end + + def generate_notifications_for_mentions(validated_mentioned_ids) + validated_mentioned_ids.each do |user_id| + NotificationBuilder.new( + notification_type: 'conversation_mention', + user: User.find(user_id), + account: message.account, + primary_actor: message + ).perform + end + end +end diff --git a/app/services/messages/new_message_notification_service.rb b/app/services/messages/new_message_notification_service.rb new file mode 100644 index 000000000..2a500fbac --- /dev/null +++ b/app/services/messages/new_message_notification_service.rb @@ -0,0 +1,25 @@ +class Messages::NewMessageNotificationService + pattr_initialize [:message!] + + def perform + return unless message.notifiable? + + notify_conversation_assignee + end + + private + + delegate :conversation, :sender, :account, to: :message + + def notify_conversation_assignee + return if conversation.assignee.blank? + return if conversation.assignee == sender + + NotificationBuilder.new( + notification_type: 'assigned_conversation_new_message', + user: conversation.assignee, + account: account, + primary_actor: message + ).perform + end +end diff --git a/spec/services/messages/mention_service_spec.rb b/spec/services/messages/mention_service_spec.rb new file mode 100644 index 000000000..e381883bf --- /dev/null +++ b/spec/services/messages/mention_service_spec.rb @@ -0,0 +1,65 @@ +require 'rails_helper' + +describe Messages::MentionService do + let!(:account) { create(:account) } + let!(:user) { create(:user, account: account) } + let!(:first_agent) { create(:user, account: account) } + let!(:second_agent) { create(:user, account: account) } + let!(:inbox) { create(:inbox, account: account) } + let!(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user) } + let(:builder) { double } + + before do + create(:inbox_member, user: first_agent, inbox: inbox) + create(:inbox_member, user: second_agent, inbox: inbox) + conversation.reload + allow(NotificationBuilder).to receive(:new).and_return(builder) + allow(builder).to receive(:perform) + end + + context 'when message contains mention' do + it 'creates notifications for inbox member who was mentioned' do + message = build( + :message, + conversation: conversation, + account: account, + content: "hi [#{first_agent.name}](mention://user/#{first_agent.id}/#{first_agent.name})", + private: true + ) + + described_class.new(message: message).perform + + expect(NotificationBuilder).to have_received(:new).with(notification_type: 'conversation_mention', + user: first_agent, + account: account, + primary_actor: message) + end + end + + context 'when message contains multiple mentions' do + let(:message) do + build( + :message, + conversation: conversation, + account: account, + content: "hey [#{second_agent.name}](mention://user/#{second_agent.id}/#{second_agent.name})/ + [#{first_agent.name}](mention://user/#{first_agent.id}/#{first_agent.name}), + please look in to this?", + private: true + ) + end + + it 'creates notifications for inbox member who was mentioned' do + described_class.new(message: message).perform + + expect(NotificationBuilder).to have_received(:new).with(notification_type: 'conversation_mention', + user: second_agent, + account: account, + primary_actor: message) + expect(NotificationBuilder).to have_received(:new).with(notification_type: 'conversation_mention', + user: first_agent, + account: account, + primary_actor: message) + end + end +end diff --git a/spec/services/messages/new_message_notification_service_spec.rb b/spec/services/messages/new_message_notification_service_spec.rb new file mode 100644 index 000000000..72a71c853 --- /dev/null +++ b/spec/services/messages/new_message_notification_service_spec.rb @@ -0,0 +1,72 @@ +require 'rails_helper' + +describe Messages::NewMessageNotificationService do + context 'when message is not notifiable' do + it 'will not create any notifications' do + message = build(:message, message_type: :activity) + expect(NotificationBuilder).not_to receive(:new) + described_class.new(message: message).perform + end + end + + context 'when message is notifiable' do + let(:account) { create(:account) } + let(:assignee) { create(:user, account: account) } + let(:participating_agent_1) { create(:user, account: account) } + let(:inbox) { create(:inbox, account: account) } + let(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: assignee) } + let(:builder) { double } + + before do + create(:inbox_member, inbox: inbox, user: participating_agent_1) + create(:inbox_member, inbox: inbox, user: assignee) + allow(NotificationBuilder).to receive(:new).and_return(builder) + allow(builder).to receive(:perform) + end + + context 'when message is created by a participant' do + let(:message) { create(:message, conversation: conversation, account: account, sender: participating_agent_1) } + + before do + described_class.new(message: message).perform + end + + it 'creates notifications for assignee' do + expect(NotificationBuilder).to have_received(:new).with(notification_type: 'assigned_conversation_new_message', + user: assignee, + account: account, + primary_actor: message) + end + end + + context 'when message is created by a contact' do + let(:message) { create(:message, conversation: conversation, account: account) } + + before do + described_class.new(message: message).perform + end + + it 'creates notifications for assignee' do + expect(NotificationBuilder).to have_received(:new).with(notification_type: 'assigned_conversation_new_message', + user: assignee, + account: account, + primary_actor: message) + end + end + + context 'when message is created by assignee' do + let(:message) { create(:message, conversation: conversation, account: account, sender: assignee) } + + before do + described_class.new(message: message).perform + end + + it 'will not create notifications for the user who created the message' do + expect(NotificationBuilder).not_to have_received(:new).with(notification_type: 'assigned_conversation_new_message', + user: assignee, + account: account, + primary_actor: message) + end + end + end +end