From cbd3f759b515590f62ce47d6289f4f0bd2312ab8 Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma Date: Thu, 30 Apr 2026 17:46:24 +0700 Subject: [PATCH] fix(voice): order accept guards correctly and validate contact phone upstream --- config/locales/en.yml | 1 + .../api/v1/accounts/whatsapp_calls_controller.rb | 6 +++--- enterprise/app/services/whatsapp/call_service.rb | 4 +++- .../api/v1/accounts/whatsapp_calls_controller_spec.rb | 11 +++++++++++ .../enterprise/services/whatsapp/call_service_spec.rb | 9 ++++++++- 5 files changed, 26 insertions(+), 5 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index c8876754a..ca77e45a7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -119,6 +119,7 @@ en: no_recording: 'No recording file provided' no_message: 'Call has no associated message' sdp_offer_required: 'sdp_offer is required' + contact_phone_required: 'Contact phone number is required' permission_request_failed: 'Failed to send call permission request' inboxes: imap: diff --git a/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb b/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb index bf1fa9ca9..61f3f9cea 100644 --- a/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb @@ -44,6 +44,7 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseContro def initiate return render_could_not_create_error(I18n.t('errors.whatsapp.calls.not_enabled')) unless calling_enabled?(@conversation) return render_could_not_create_error(I18n.t('errors.whatsapp.calls.sdp_offer_required')) if params[:sdp_offer].blank? + return render_could_not_create_error(I18n.t('errors.whatsapp.calls.contact_phone_required')) if @conversation.contact&.phone_number.blank? @call = create_outbound_call(@conversation, params[:sdp_offer]) @message = Voice::CallMessageBuilder.new(@call).perform! @@ -74,10 +75,9 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseContro end # Browser-built SDP offer is forwarded to Meta; the connect webhook later delivers Meta's answer. + # Caller must validate `conversation.contact.phone_number` is present (initiate's guard does). def create_outbound_call(conversation, sdp_offer) - contact_phone = conversation.contact&.phone_number - raise ArgumentError, 'Contact phone number not available' if contact_phone.blank? - + contact_phone = conversation.contact.phone_number result = conversation.inbox.channel.provider_service.initiate_call(contact_phone.delete('+'), sdp_offer) provider_call_id = result.dig('calls', 0, 'id') || result['call_id'] diff --git a/enterprise/app/services/whatsapp/call_service.rb b/enterprise/app/services/whatsapp/call_service.rb index e7e1d9b7c..fee97bfa6 100644 --- a/enterprise/app/services/whatsapp/call_service.rb +++ b/enterprise/app/services/whatsapp/call_service.rb @@ -35,8 +35,10 @@ class Whatsapp::CallService private def transition_to_in_progress! - raise Voice::CallErrors::NotRinging, 'Call is not in ringing state' unless call.ringing? + # Order matters: in_progress and terminal both make ringing? false, so we have to + # branch on in_progress? first to surface the distinct AlreadyAccepted state. raise Voice::CallErrors::AlreadyAccepted, 'Call already accepted by another agent' if call.in_progress? + raise Voice::CallErrors::NotRinging, 'Call is not in ringing state' unless call.ringing? forward_answer_to_meta! call.update!(status: 'in_progress', accepted_by_agent_id: agent.id, started_at: Time.current, diff --git a/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb index 31aac16fe..f86d5745c 100644 --- a/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb @@ -137,6 +137,17 @@ RSpec.describe 'WhatsApp Calls API', type: :request do expect(response.parsed_body['error']).to eq('Meta error') end + it 'returns 422 when the conversation contact has no phone number' do + contact.update!(phone_number: nil) + + post "/api/v1/accounts/#{account.id}/whatsapp_calls/initiate", + params: { conversation_id: initiate_conversation.display_id, sdp_offer: 'sdp_offer' }, + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.parsed_body['error']).to eq(I18n.t('errors.whatsapp.calls.contact_phone_required')) + end + it 'returns 422 when the conversation belongs to a non-WhatsApp inbox' do twilio_channel = create(:channel_twilio_sms, :with_voice, account: account, phone_number: '+15551239998') create(:inbox_member, user: agent, inbox: twilio_channel.inbox) diff --git a/spec/enterprise/services/whatsapp/call_service_spec.rb b/spec/enterprise/services/whatsapp/call_service_spec.rb index eb2fdbee8..fdcf154eb 100644 --- a/spec/enterprise/services/whatsapp/call_service_spec.rb +++ b/spec/enterprise/services/whatsapp/call_service_spec.rb @@ -50,9 +50,16 @@ describe Whatsapp::CallService do expect(conversation.reload.assignee_id).to eq(agent.id) end - it 'raises NotRinging when the call is no longer ringing' do + it 'raises AlreadyAccepted when another agent has already accepted the call' do call.update!(status: 'in_progress') + expect { described_class.new(call: call, agent: agent, sdp_answer: sdp_answer).accept } + .to raise_error(Voice::CallErrors::AlreadyAccepted) + end + + it 'raises NotRinging when the call has reached a terminal state' do + call.update!(status: 'completed') + expect { described_class.new(call: call, agent: agent, sdp_answer: sdp_answer).accept } .to raise_error(Voice::CallErrors::NotRinging) end