Merge branch 'develop' into team-permissions-v2

app/controllers/api/v1/accounts/conversations/base_controller.rb

@@ -5,6 +5,6 @@ class Api::V1::Accounts::Conversations::BaseController < Api::V1::Accounts::Base
 
   def conversation
     @conversation ||= Current.account.conversations.find_by!(display_id: params[:conversation_id])
-    authorize @conversation.inbox, :show?
+    authorize @conversation, :show?
   end
 end

app/controllers/api/v1/accounts/conversations_controller.rb

@@ -160,7 +160,7 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro
 
   def conversation
     @conversation ||= Current.account.conversations.find_by!(display_id: params[:id])
-    authorize @conversation.inbox, :show?
+    authorize @conversation, :show?
   end
 
   def inbox

app/finders/conversation_finder.rb

@@ -72,7 +72,7 @@ class ConversationFinder
 
   def set_inboxes
     @inbox_ids = if params[:inbox_id]
-                   @current_user.assigned_inboxes.where(id: params[:inbox_id])
+                   @current_user.assigned_inboxes.where(id: params[:inbox_id]).pluck(:id)
                  else
                    @current_user.assigned_inboxes.pluck(:id)
                  end

app/policies/conversation_policy.rb

@@ -3,7 +3,27 @@ class ConversationPolicy < ApplicationPolicy
     true
   end
 
+  def show?
+    return true if @account_user&.administrator?
+
+    # Check if user has access to the conversation through inbox membership
+    return true if user_assigned_inboxes.include?(record.inbox)
+
+    # Check if user has access to the conversation through team membership
+    user_assigned_teams.include?(record.team) if record.team.present?
+  end
+
   def destroy?
     @account_user&.administrator?
   end
+
+  private
+
+  def user_assigned_inboxes
+    @user.inboxes.where(account_id: @account.id)
+  end
+
+  def user_assigned_teams
+    @user.teams.where(account_id: @account.id)
+  end
 end

app/services/conversations/permission_filter_service.rb

@@ -17,6 +17,7 @@ class Conversations::PermissionFilterService
 
   def accessible_conversations
     conversations.where(inbox: user.inboxes.where(account_id: account.id))
+                 .or(conversations.where(team: user.teams.where(account_id: account.id)))
   end
 
   def account_user

spec/policies/conversation_policy_spec.rb

@@ -31,4 +31,77 @@ RSpec.describe ConversationPolicy, type: :policy do
       end
     end
   end
+
+  permissions :show? do
+    let(:inbox) { create(:inbox, account: account) }
+    let(:team) { create(:team, account: account) }
+    let(:conversation_with_inbox) { create(:conversation, account: account, inbox: inbox) }
+    let(:conversation_with_team) { create(:conversation, account: account, inbox: other_inbox, team: team) }
+    let(:other_inbox) { create(:inbox, account: account) }
+    let(:inaccessible_conversation) { create(:conversation, account: account, inbox: other_inbox) }
+
+    context 'when user is an administrator' do
+      it 'allows show for any conversation' do
+        expect(subject).to permit(administrator_context, conversation_with_inbox)
+        expect(subject).to permit(administrator_context, conversation_with_team)
+        expect(subject).to permit(administrator_context, inaccessible_conversation)
+      end
+    end
+
+    context 'when user is an agent' do
+      context 'with inbox access' do
+        before do
+          create(:inbox_member, user: agent, inbox: inbox)
+        end
+
+        it 'allows show for conversations in assigned inbox' do
+          expect(subject).to permit(agent_context, conversation_with_inbox)
+        end
+
+        it 'denies show for conversations in non-assigned inbox' do
+          expect(subject).not_to permit(agent_context, conversation_with_team)
+          expect(subject).not_to permit(agent_context, inaccessible_conversation)
+        end
+      end
+
+      context 'with team access' do
+        before do
+          create(:team_member, user: agent, team: team)
+        end
+
+        it 'allows show for conversations assigned to user team' do
+          expect(subject).to permit(agent_context, conversation_with_team)
+        end
+
+        it 'denies show for conversations not assigned to user team' do
+          expect(subject).not_to permit(agent_context, conversation_with_inbox)
+          expect(subject).not_to permit(agent_context, inaccessible_conversation)
+        end
+      end
+
+      context 'with both inbox and team access' do
+        before do
+          create(:inbox_member, user: agent, inbox: inbox)
+          create(:team_member, user: agent, team: team)
+        end
+
+        it 'allows show for conversations from both inbox and team' do
+          expect(subject).to permit(agent_context, conversation_with_inbox)
+          expect(subject).to permit(agent_context, conversation_with_team)
+        end
+
+        it 'denies show for inaccessible conversations' do
+          expect(subject).not_to permit(agent_context, inaccessible_conversation)
+        end
+      end
+
+      context 'with no access' do
+        it 'denies show for all conversations' do
+          expect(subject).not_to permit(agent_context, conversation_with_inbox)
+          expect(subject).not_to permit(agent_context, conversation_with_team)
+          expect(subject).not_to permit(agent_context, inaccessible_conversation)
+        end
+      end
+    end
+  end
 end

spec/services/conversations/permission_filter_service_spec.rb

@@ -4,12 +4,20 @@ RSpec.describe Conversations::PermissionFilterService do
   let(:account) { create(:account) }
   let!(:conversation) { create(:conversation, account: account, inbox: inbox) }
   let!(:another_conversation) { create(:conversation, account: account, inbox: inbox) }
+  let!(:team_conversation) { create(:conversation, account: account, inbox: other_inbox, team: team) }
+  let!(:inaccessible_conversation) { create(:conversation, account: account, inbox: other_inbox) }
   let(:admin) { create(:user, account: account, role: :administrator) }
   let(:agent) { create(:user, account: account, role: :agent) }
+  let(:team_agent) { create(:user, account: account, role: :agent) }
   let!(:inbox) { create(:inbox, account: account) }
+  let!(:other_inbox) { create(:inbox, account: account) }
+  let!(:team) { create(:team, account: account) }
 
   # This inbox_member is used to establish the agent's access to the inbox
-  before { create(:inbox_member, user: agent, inbox: inbox) }
+  before do
+    create(:inbox_member, user: agent, inbox: inbox)
+    create(:team_member, user: team_agent, team: team)
+  end
 
   describe '#perform' do
     context 'when user is an administrator' do
@@ -22,25 +30,81 @@ RSpec.describe Conversations::PermissionFilterService do
 
         expect(result).to include(conversation)
         expect(result).to include(another_conversation)
-        expect(result.count).to eq(2)
+        expect(result).to include(team_conversation)
+        expect(result).to include(inaccessible_conversation)
+        expect(result.count).to eq(4)
       end
     end
 
     context 'when user is an agent' do
-      it 'returns all conversations with no further filtering' do
-        inbox_ids = agent.inboxes.where(account_id: account.id).pluck(:id)
+      context 'with inbox access only' do
+        it 'returns conversations from assigned inboxes' do
+          result = described_class.new(
+            account.conversations,
+            agent,
+            account
+          ).perform
 
-        # The base implementation returns all conversations
-        # expecting the caller to filter by assigned inboxes
-        result = described_class.new(
-          account.conversations.where(inbox_id: inbox_ids),
-          agent,
-          account
-        ).perform
+          expect(result).to include(conversation)
+          expect(result).to include(another_conversation)
+          expect(result).not_to include(team_conversation)
+          expect(result).not_to include(inaccessible_conversation)
+          expect(result.count).to eq(2)
+        end
+      end
 
-        expect(result).to include(conversation)
-        expect(result).to include(another_conversation)
-        expect(result.count).to eq(2)
+      context 'with team access only' do
+        it 'returns conversations from assigned teams' do
+          result = described_class.new(
+            account.conversations,
+            team_agent,
+            account
+          ).perform
+
+          expect(result).to include(team_conversation)
+          expect(result).not_to include(conversation)
+          expect(result).not_to include(another_conversation)
+          expect(result).not_to include(inaccessible_conversation)
+          expect(result.count).to eq(1)
+        end
+      end
+
+      context 'with both inbox and team access' do
+        let(:agent_with_both) { create(:user, account: account, role: :agent) }
+
+        before do
+          create(:inbox_member, user: agent_with_both, inbox: inbox)
+          create(:team_member, user: agent_with_both, team: team)
+        end
+
+        it 'returns conversations from both assigned inboxes and teams' do
+          result = described_class.new(
+            account.conversations,
+            agent_with_both,
+            account
+          ).perform
+
+          expect(result).to include(conversation)
+          expect(result).to include(another_conversation)
+          expect(result).to include(team_conversation)
+          expect(result).not_to include(inaccessible_conversation)
+          expect(result.count).to eq(3)
+        end
+      end
+
+      context 'with no access' do
+        let(:agent_no_access) { create(:user, account: account, role: :agent) }
+
+        it 'returns no conversations' do
+          result = described_class.new(
+            account.conversations,
+            agent_no_access,
+            account
+          ).perform
+
+          expect(result).to be_empty
+          expect(result.count).to eq(0)
+        end
       end
     end
   end