chore: Refactor the notification service for participants (#6461)

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

app/listeners/notification_listener.rb

@@ -28,50 +28,9 @@ class NotificationListener < BaseListener
   end
 
   def message_created(event)
-    message, account = extract_message_and_account(event)
+    message = extract_message_and_account(event)[0]
-    conversation = message.conversation
 
-    generate_notifications_for_mentions(message, account)
+    Messages::MentionService.new(message: message).perform
-
+    Messages::NewMessageNotificationService.new(message: message).perform
-    # 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
   end
 end

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

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

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

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

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

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