Merge remote-tracking branch 'origin/feature/cw-7495-api' into cook/pr-15017-review

# Conflicts:
#	enterprise/app/builders/captain/assistant_stats_builder.rb
#	enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb
#	spec/enterprise/builders/captain/assistant_stats_builder_spec.rb

spec/builders/agent_builder_spec.rb

@@ -23,6 +23,12 @@ RSpec.describe AgentBuilder, type: :model do
   end
 
   describe '#perform' do
+    it 'locks the account while checking and creating the agent' do
+      expect(account).to receive(:with_lock).and_call_original
+
+      agent_builder.perform
+    end
+
     context 'when user does not exist' do
       it 'creates a new user' do
         expect { agent_builder.perform }.to change(User, :count).by(1)
@@ -67,5 +73,17 @@ RSpec.describe AgentBuilder, type: :model do
         expect(user.encrypted_password).not_to be_empty
       end
     end
+
+    context 'when the account has reached its agent limit' do
+      before do
+        allow(account).to receive(:usage_limits).and_return({ agents: account.account_users.count })
+      end
+
+      it 'raises a limit exceeded error without creating a user' do
+        expect { agent_builder.perform }.to raise_error(described_class::LimitExceededError, described_class::LIMIT_EXCEEDED_MESSAGE)
+
+        expect(User.from_email(email)).to be_nil
+      end
+    end
   end
 end

spec/controllers/api/v1/accounts/integrations/linear_controller_spec.rb

@@ -13,7 +13,9 @@ RSpec.describe 'Linear Integration API', type: :request do
   end
 
   describe 'DELETE /api/v1/accounts/:account_id/integrations/linear' do
-    it 'deletes the linear integration' do
+    let(:admin) { create(:user, account: account, role: :administrator) }
+
+    it 'deletes the linear integration when the user is an administrator' do
       # Stub the HTTP call to Linear's revoke endpoint
       allow(HTTParty).to receive(:post).with(
         'https://api.linear.app/oauth/revoke',
@@ -21,11 +23,19 @@ RSpec.describe 'Linear Integration API', type: :request do
       ).and_return(instance_double(HTTParty::Response, success?: true))
 
       delete "/api/v1/accounts/#{account.id}/integrations/linear",
-             headers: agent.create_new_auth_token,
+             headers: admin.create_new_auth_token,
              as: :json
       expect(response).to have_http_status(:ok)
       expect(account.hooks.count).to eq(0)
     end
+
+    it 'returns unauthorized for an agent and keeps the integration' do
+      delete "/api/v1/accounts/#{account.id}/integrations/linear",
+             headers: agent.create_new_auth_token,
+             as: :json
+      expect(response).to have_http_status(:unauthorized)
+      expect(account.hooks.count).to eq(1)
+    end
   end
 
   describe 'GET /api/v1/accounts/:account_id/integrations/linear/teams' do

spec/controllers/api/v1/accounts/integrations/shopify_controller_spec.rb

@@ -159,15 +159,17 @@ RSpec.describe 'Shopify Integration API', type: :request do
   end
 
   describe 'DELETE /api/v1/accounts/:account_id/integrations/shopify' do
+    let(:admin) { create(:user, account: account, role: :administrator) }
+
     before do
       create(:integrations_hook, :shopify, account: account)
     end
 
-    context 'when it is an authenticated user' do
+    context 'when it is an administrator' do
       it 'deletes the shopify integration' do
         expect do
           delete "/api/v1/accounts/#{account.id}/integrations/shopify",
-                 headers: agent.create_new_auth_token,
+                 headers: admin.create_new_auth_token,
                  as: :json
         end.to change { account.hooks.count }.by(-1)
 
@@ -175,6 +177,18 @@ RSpec.describe 'Shopify Integration API', type: :request do
       end
     end
 
+    context 'when it is an agent' do
+      it 'returns unauthorized and keeps the integration' do
+        expect do
+          delete "/api/v1/accounts/#{account.id}/integrations/shopify",
+                 headers: agent.create_new_auth_token,
+                 as: :json
+        end.not_to(change { account.hooks.count })
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
     context 'when it is an unauthenticated user' do
       it 'returns unauthorized' do
         delete "/api/v1/accounts/#{account.id}/integrations/shopify",

spec/enterprise/builders/captain/assistant_stats_builder_spec.rb

@@ -27,7 +27,7 @@ RSpec.describe Captain::AssistantStatsBuilder do
 
       expect(metrics.keys).to contain_exactly(
         :conversations_handled, :auto_resolution_rate, :handoff_rate,
-        :hours_saved, :reopen_rate, :conversation_depth, :knowledge
+        :hours_saved, :reopen_rate, :conversation_depth
       )
       expect(metrics[:conversations_handled]).to include(:current, :previous, :trend)
     end
@@ -229,6 +229,32 @@ RSpec.describe Captain::AssistantStatsBuilder do
     end
   end
 
+  describe '#faq_stats' do
+    before do
+      create_list(:captain_assistant_response, 3, assistant: assistant, account: account, status: :approved)
+      assistant.faq_suggestions.create!(
+        question: 'How do I enable the feature?',
+        answer: 'Turn it on in settings.'
+      )
+      create_list(:captain_document, 2, assistant: assistant, account: account)
+    end
+
+    it 'returns approved FAQ, open suggestion, document counts and coverage' do
+      stats = described_class.new(assistant).faq_stats
+
+      expect(stats).to eq(approved: 3, suggestions: 1, documents: 2, coverage: 75)
+    end
+
+    it 'reports zero coverage when there are no FAQs or suggestions' do
+      Captain::AssistantResponse.where(assistant: assistant).delete_all
+      Captain::FaqSuggestion.where(assistant: assistant).delete_all
+
+      stats = described_class.new(assistant).faq_stats
+
+      expect(stats).to eq(approved: 0, suggestions: 0, documents: 2, coverage: 0)
+    end
+  end
+
   describe '#period' do
     it 'labels a day range and exposes its bounds' do
       period = described_class.new(assistant, '30').period

spec/enterprise/controllers/api/v1/accounts/agents_controller_spec.rb

@@ -21,6 +21,27 @@ RSpec.describe 'Agents API', type: :request do
         expect(response).to have_http_status(:payment_required)
         expect(response.body).to include('Account limit exceeded. Please purchase more licenses')
       end
+
+      it 'prevents adding an agent if the last seat is consumed before creation' do
+        account.update!(limits: { agents: account.account_users.count + 1 })
+        competing_agent_created = false
+
+        allow(AgentBuilder).to receive(:new).and_wrap_original do |method, *args|
+          unless competing_agent_created
+            create(:user, account: account, role: :agent)
+            competing_agent_created = true
+          end
+
+          method.call(*args)
+        end
+
+        post "/api/v1/accounts/#{account.id}/agents", params: params, headers: admin.create_new_auth_token, as: :json
+
+        expect(response).to have_http_status(:payment_required)
+        expect(response.body).to include('Account limit exceeded. Please purchase more licenses')
+        expect(User.from_email(params[:email])).to be_nil
+        expect(account.account_users.count).to eq(account.usage_limits[:agents])
+      end
     end
   end
 

spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb

@@ -252,7 +252,7 @@ RSpec.describe 'Api::V1::Accounts::Captain::Assistants', type: :request do
     end
   end
 
-  describe 'GET /api/v1/accounts/{account.id}/captain/assistants/{id}/stats' do
+  describe 'GET /api/v1/accounts/{account.id}/captain/assistants/{id}/faq_stats' do
     let(:assistant) { create(:captain_assistant, account: account) }
 
     it 'returns approved FAQ, open suggestion, document counts and coverage' do
@@ -260,23 +260,21 @@ RSpec.describe 'Api::V1::Accounts::Captain::Assistants', type: :request do
       assistant.faq_suggestions.create!(question: 'How do I enable the feature?', answer: 'Turn it on in settings.')
       create_list(:captain_document, 2, assistant: assistant, account: account)
 
-      get "/api/v1/accounts/#{account.id}/captain/assistants/#{assistant.id}/stats",
-          params: { range: '30' },
+      get "/api/v1/accounts/#{account.id}/captain/assistants/#{assistant.id}/faq_stats",
           headers: admin.create_new_auth_token,
           as: :json
 
       expect(response).to have_http_status(:success)
-      expect(json_response[:knowledge]).to eq(approved: 3, suggestions: 1, documents: 2, coverage: 75)
+      expect(json_response).to eq(approved: 3, suggestions: 1, documents: 2, coverage: 75)
     end
 
     it 'returns zero coverage when there are no FAQs or suggestions' do
-      get "/api/v1/accounts/#{account.id}/captain/assistants/#{assistant.id}/stats",
-          params: { range: '30' },
+      get "/api/v1/accounts/#{account.id}/captain/assistants/#{assistant.id}/faq_stats",
           headers: admin.create_new_auth_token,
           as: :json
 
       expect(response).to have_http_status(:success)
-      expect(json_response[:knowledge]).to include(approved: 0, suggestions: 0, coverage: 0)
+      expect(json_response).to include(approved: 0, suggestions: 0, coverage: 0)
     end
 
     it 'counts only suggestions backed by conversations the agent can access' do
@@ -300,13 +298,12 @@ RSpec.describe 'Api::V1::Accounts::Captain::Assistants', type: :request do
         language: hidden_suggestion.language
       )
 
-      get "/api/v1/accounts/#{account.id}/captain/assistants/#{assistant.id}/stats",
-          params: { range: '30' },
+      get "/api/v1/accounts/#{account.id}/captain/assistants/#{assistant.id}/faq_stats",
           headers: agent.create_new_auth_token,
           as: :json
 
       expect(response).to have_http_status(:success)
-      expect(json_response[:knowledge]).to include(approved: 1, suggestions: 1, coverage: 50)
+      expect(json_response).to include(approved: 1, suggestions: 1, coverage: 50)
     end
   end
 

spec/enterprise/policies/captain/assistant_policy_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe Captain::AssistantPolicy, type: :policy do
   let(:administrator_context) { { user: administrator, account: account, account_user: account.account_users.first } }
   let(:agent_context) { { user: agent, account: account, account_user: account.account_users.first } }
 
-  permissions :index?, :show?, :playground? do
+  permissions :index?, :show?, :playground?, :metrics?, :faq_stats? do
     context 'when administrator' do
       it { expect(assistant_policy).to permit(administrator_context, assistant) }
     end

spec/jobs/webhooks/whatsapp_events_job_spec.rb

@@ -345,6 +345,94 @@ RSpec.describe Webhooks::WhatsappEventsJob do
       end.not_to change(Conversation, :count)
     end
 
+    it 'finds channel using normalized Brazil phone number when display_phone_number is missing the 9 digit' do
+      brazil_channel = create(:channel_whatsapp, phone_number: '+5541999887766', provider: 'whatsapp_cloud',
+                                                 sync_templates: false, validate_provider_config: false)
+      wb_params = {
+        object: 'whatsapp_business_account',
+        entry: [{
+          changes: [{
+            value: {
+              metadata: {
+                phone_number_id: brazil_channel.provider_config['phone_number_id'],
+                display_phone_number: '554199887766'
+              }
+            }
+          }]
+        }]
+      }
+      allow(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new).and_return(process_service)
+      expect(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new).with(inbox: brazil_channel.inbox, params: wb_params)
+      job.perform_now(wb_params)
+    end
+
+    it 'finds channel using normalized Argentina phone number when display_phone_number has extra 9 digit' do
+      argentina_channel = create(:channel_whatsapp, phone_number: '+541112345678', provider: 'whatsapp_cloud',
+                                                    sync_templates: false, validate_provider_config: false)
+      wb_params = {
+        object: 'whatsapp_business_account',
+        entry: [{
+          changes: [{
+            value: {
+              metadata: {
+                phone_number_id: argentina_channel.provider_config['phone_number_id'],
+                display_phone_number: '5491112345678'
+              }
+            }
+          }]
+        }]
+      }
+      allow(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new).and_return(process_service)
+      expect(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new).with(inbox: argentina_channel.inbox, params: wb_params)
+      job.perform_now(wb_params)
+    end
+
+    it 'finds channel when display_phone_number contains formatting characters' do
+      formatted_channel = create(:channel_whatsapp, phone_number: '+14155552671', provider: 'whatsapp_cloud',
+                                                    sync_templates: false, validate_provider_config: false)
+      wb_params = {
+        object: 'whatsapp_business_account',
+        entry: [{
+          changes: [{
+            value: {
+              metadata: {
+                phone_number_id: formatted_channel.provider_config['phone_number_id'],
+                display_phone_number: '+1 415-555-2671'
+              }
+            }
+          }]
+        }]
+      }
+      allow(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new).and_return(process_service)
+      expect(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new).with(inbox: formatted_channel.inbox, params: wb_params)
+      job.perform_now(wb_params)
+    end
+
+    it 'prefers the phone_number_id match when a raw display_phone_number collision exists' do
+      normalized_channel = create(:channel_whatsapp, phone_number: '+5541999887766', provider: 'whatsapp_cloud',
+                                                     sync_templates: false, validate_provider_config: false)
+      create(:channel_whatsapp, phone_number: '+554199887766', provider: 'whatsapp_cloud',
+                                sync_templates: false, validate_provider_config: false).tap do |raw_channel|
+        raw_channel.update!(provider_config: raw_channel.provider_config.merge('phone_number_id' => 'other-id'))
+      end
+      wb_params = {
+        object: 'whatsapp_business_account',
+        entry: [{
+          changes: [{
+            value: {
+              metadata: {
+                phone_number_id: normalized_channel.provider_config['phone_number_id'],
+                display_phone_number: '554199887766'
+              }
+            }
+          }]
+        }]
+      }
+      allow(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new).and_return(process_service)
+      expect(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new).with(inbox: normalized_channel.inbox, params: wb_params)
+      job.perform_now(wb_params)
+    end
+
     it 'will not enque Whatsapp::IncomingMessageWhatsappCloudService when invalid phone number id' do
       other_channel = create(:channel_whatsapp, phone_number: '+1987654', provider: 'whatsapp_cloud', sync_templates: false,
                                                 validate_provider_config: false)

spec/services/whatsapp/send_on_whatsapp_service_spec.rb

@@ -72,6 +72,21 @@ describe Whatsapp::SendOnWhatsappService do
         expect(message.reload.source_id).to eq('123456789')
       end
 
+      it 'fails a free-form message without contacting the provider when outside the 24 hour limit' do
+        create(:message, message_type: :incoming, content: 'test', created_at: 25.hours.ago,
+                         conversation: conversation, account: conversation.account)
+        message = create(:message, message_type: :outgoing, content: 'test',
+                                   conversation: conversation, account: conversation.account)
+
+        expect(Whatsapp::TemplateProcessorService).not_to receive(:new)
+
+        described_class.new(message: message).perform
+
+        expect(message.reload.status).to eq('failed')
+        expect(message.external_error).to eq(I18n.t('errors.whatsapp.message_outside_messaging_window'))
+        expect(a_request(:post, 'https://waba.360dialog.io/v1/messages')).not_to have_been_made
+      end
+
       it 'marks message as failed when template name is blank' do
         processor = instance_double(Whatsapp::TemplateProcessorService)
         allow(Whatsapp::TemplateProcessorService).to receive(:new).and_return(processor)