fix(notifications): Respect conversation access when notifying agents (#14412)

Agents with limited custom roles were receiving notifications (creation,
assignment, mentions, new messages, SLA) for conversations they couldn't
actually open. For example, an agent whose custom role only grants
`conversation_unassigned_manage` was getting notified about
conversations assigned to other agents.

Notifications now go through the same `ConversationPolicy#show?` check
that gates the conversation view itself, so an agent only gets notified
for conversations they're permitted to see. Administrators and agents
without custom roles are unaffected.

---------

Co-authored-by: Sojan Jose <sojan@pepalo.com>
Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>

app/builders/notification_builder.rb

@@ -27,6 +27,8 @@ class NotificationBuilder
     return if notification_type == 'conversation_creation' && !user_subscribed_to_notification?
     # skip notifications for blocked conversations except for user mentions
     return if primary_actor.contact.blocked? && notification_type != 'conversation_mention'
+    # respect conversation access (inbox/team membership and custom-role permissions)
+    return unless user_can_access_conversation?
 
     user.notifications.create!(
       notification_type: notification_type,
@@ -36,4 +38,17 @@ class NotificationBuilder
       secondary_actor: secondary_actor || current_user
     )
   end
+
+  def user_can_access_conversation?
+    conversation = primary_actor.is_a?(Conversation) ? primary_actor : primary_actor.try(:conversation)
+    return true if conversation.blank?
+
+    account_user = AccountUser.find_by(account_id: account.id, user_id: user.id)
+    return false if account_user.blank?
+
+    ConversationPolicy.new(
+      { user: user, account: account, account_user: account_user },
+      conversation
+    ).show?
+  end
 end

app/services/messages/mention_service.rb

@@ -8,8 +8,8 @@ class Messages::MentionService
     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)
     add_mentioned_users_as_participants(validated_mentioned_ids)
+    generate_notifications_for_mentions(validated_mentioned_ids)
   end
 
   private

spec/builders/notification_builder_spec.rb

@@ -6,9 +6,11 @@ describe NotificationBuilder do
   describe '#perform' do
     let!(:account) { create(:account) }
     let!(:user) { create(:user, account: account) }
-    let!(:primary_actor) { create(:conversation, account: account) }
+    let!(:inbox) { create(:inbox, account: account) }
+    let!(:primary_actor) { create(:conversation, account: account, inbox: inbox) }
 
     before do
+      create(:inbox_member, user: user, inbox: inbox)
       notification_setting = user.notification_settings.find_by(account_id: account.id)
       notification_setting.selected_email_flags = [:email_conversation_creation]
       notification_setting.selected_push_flags = [:push_conversation_creation]
@@ -97,5 +99,66 @@ describe NotificationBuilder do
         ).perform
       end.to change { user.notifications.count }.by(1)
     end
+
+    context 'when the user does not have access to the conversation' do
+      let!(:outsider) { create(:user, account: account) }
+
+      it 'does not create a notification for an agent without inbox or team access' do
+        expect do
+          described_class.new(
+            notification_type: 'conversation_creation',
+            user: outsider,
+            account: account,
+            primary_actor: primary_actor
+          ).perform
+        end.not_to(change { outsider.notifications.count })
+      end
+
+      it 'still creates a notification for administrators regardless of inbox membership' do
+        admin = create(:user, account: account, role: :administrator)
+        admin_setting = admin.notification_settings.find_by(account_id: account.id)
+        admin_setting.selected_email_flags = [:email_conversation_creation]
+        admin_setting.selected_push_flags = [:push_conversation_creation]
+        admin_setting.save!
+
+        expect do
+          described_class.new(
+            notification_type: 'conversation_creation',
+            user: admin,
+            account: account,
+            primary_actor: primary_actor
+          ).perform
+        end.to change { admin.notifications.count }.by(1)
+      end
+
+      it 'does not create a notification when the user is not part of the account' do
+        unrelated_user = create(:user)
+
+        expect do
+          described_class.new(
+            notification_type: 'conversation_creation',
+            user: unrelated_user,
+            account: account,
+            primary_actor: primary_actor
+          ).perform
+        end.not_to(change { unrelated_user.notifications.count })
+      end
+
+      it 'derives the conversation from a message primary_actor' do
+        outsider_inbox = create(:inbox, account: account)
+        message = create(:message, account: account, inbox: outsider_inbox,
+                                   conversation: create(:conversation, account: account, inbox: outsider_inbox))
+
+        expect do
+          described_class.new(
+            notification_type: 'conversation_mention',
+            user: outsider,
+            account: account,
+            primary_actor: message.conversation,
+            secondary_actor: message
+          ).perform
+        end.not_to(change { outsider.notifications.count })
+      end
+    end
   end
 end

spec/enterprise/builders/notification_builder_spec.rb

@@ -0,0 +1,106 @@
+require 'rails_helper'
+
+describe NotificationBuilder do
+  describe '#perform with custom role permissions' do
+    let!(:account) { create(:account) }
+    let!(:agent) { create(:user, account: account, role: :agent) }
+    let!(:inbox) { create(:inbox, account: account) }
+    let!(:account_user) { agent.account_users.find_by(account: account) }
+
+    before do
+      create(:inbox_member, user: agent, inbox: inbox)
+      notification_setting = agent.notification_settings.find_by(account_id: account.id)
+      notification_setting.selected_email_flags = [:email_conversation_creation]
+      notification_setting.selected_push_flags = [:push_conversation_creation]
+      notification_setting.save!
+    end
+
+    def build_notification(conversation, type: 'conversation_creation')
+      described_class.new(
+        notification_type: type,
+        user: agent,
+        account: account,
+        primary_actor: conversation
+      ).perform
+    end
+
+    context 'when the agent has conversation_manage permission' do
+      before do
+        custom_role = create(:custom_role, account: account, permissions: ['conversation_manage'])
+        account_user.update!(custom_role: custom_role)
+      end
+
+      it 'creates a notification for any inbox conversation' do
+        conversation = create(:conversation, account: account, inbox: inbox)
+
+        expect { build_notification(conversation) }.to change { agent.notifications.count }.by(1)
+      end
+    end
+
+    context 'when the agent has conversation_unassigned_manage permission' do
+      before do
+        custom_role = create(:custom_role, account: account, permissions: ['conversation_unassigned_manage'])
+        account_user.update!(custom_role: custom_role)
+      end
+
+      it 'creates a notification for unassigned conversations' do
+        conversation = create(:conversation, account: account, inbox: inbox, assignee: nil)
+
+        expect { build_notification(conversation) }.to change { agent.notifications.count }.by(1)
+      end
+
+      it 'creates a notification for conversations assigned to the agent' do
+        conversation = create(:conversation, account: account, inbox: inbox, assignee: agent)
+
+        expect { build_notification(conversation) }.to change { agent.notifications.count }.by(1)
+      end
+
+      it 'does not create a notification for conversations assigned to someone else' do
+        other_agent = create(:user, account: account, role: :agent)
+        create(:inbox_member, user: other_agent, inbox: inbox)
+        conversation = create(:conversation, account: account, inbox: inbox, assignee: other_agent)
+
+        expect { build_notification(conversation) }.not_to(change { agent.notifications.count })
+      end
+    end
+
+    context 'when the agent has conversation_participating_manage permission' do
+      before do
+        custom_role = create(:custom_role, account: account, permissions: ['conversation_participating_manage'])
+        account_user.update!(custom_role: custom_role)
+      end
+
+      it 'creates a notification for conversations assigned to the agent' do
+        conversation = create(:conversation, account: account, inbox: inbox, assignee: agent)
+
+        expect { build_notification(conversation) }.to change { agent.notifications.count }.by(1)
+      end
+
+      it 'creates a notification for conversations the agent participates in' do
+        conversation = create(:conversation, account: account, inbox: inbox, assignee: nil)
+        create(:conversation_participant, conversation: conversation, account: account, user: agent)
+
+        expect { build_notification(conversation) }.to change { agent.notifications.count }.by(1)
+      end
+
+      it 'does not create a notification for unassigned conversations the agent does not participate in' do
+        conversation = create(:conversation, account: account, inbox: inbox, assignee: nil)
+
+        expect { build_notification(conversation) }.not_to(change { agent.notifications.count })
+      end
+    end
+
+    context 'when the custom role grants no conversation permissions' do
+      before do
+        custom_role = create(:custom_role, account: account, permissions: ['contact_manage'])
+        account_user.update!(custom_role: custom_role)
+      end
+
+      it 'does not create a notification' do
+        conversation = create(:conversation, account: account, inbox: inbox, assignee: agent)
+
+        expect { build_notification(conversation) }.not_to(change { agent.notifications.count })
+      end
+    end
+  end
+end

spec/enterprise/models/sla_event_spec.rb

@@ -55,6 +55,7 @@ RSpec.describe SlaEvent, type: :model do
     before do
       # to ensure notifications are not sent to other users
       create(:user, account: account)
+      create(:inbox_member, inbox: inbox, user: assignee)
       create(:inbox_member, inbox: inbox, user: participant)
       create(:conversation_participant, conversation: conversation, user: participant)
     end

spec/services/messages/mention_service_spec.rb

@@ -117,6 +117,26 @@ describe Messages::MentionService do
 
         expect(conversation.conversation_participants.map(&:user_id)).to include(first_agent.id)
       end
+
+      it 'adds the mentioned user as a participant before generating the notification' do
+        message = build(
+          :message,
+          conversation: conversation,
+          account: account,
+          content: "hi (mention://user/#{first_agent.id}/#{first_agent.name})",
+          private: true
+        )
+
+        participant_user_ids_when_notified = nil
+        allow(NotificationBuilder).to receive(:new) do |**_kwargs|
+          participant_user_ids_when_notified = conversation.conversation_participants.reload.map(&:user_id)
+          builder
+        end
+
+        described_class.new(message: message).perform
+
+        expect(participant_user_ids_when_notified).to include(first_agent.id)
+      end
     end
 
     context 'when message contains multiple user mentions' do