From 555894793e696fdbb216d244e3355383d5743e82 Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma Date: Thu, 30 Apr 2026 17:10:34 +0700 Subject: [PATCH] fix(voice): match permission reply to originating conversation via context.id --- .../whatsapp/call_permission_reply_service.rb | 33 +++++------- .../call_permission_reply_service_spec.rb | 51 +++++++++++++------ 2 files changed, 50 insertions(+), 34 deletions(-) diff --git a/enterprise/app/services/whatsapp/call_permission_reply_service.rb b/enterprise/app/services/whatsapp/call_permission_reply_service.rb index b1dfc38cd..42f4621f5 100644 --- a/enterprise/app/services/whatsapp/call_permission_reply_service.rb +++ b/enterprise/app/services/whatsapp/call_permission_reply_service.rb @@ -7,14 +7,11 @@ class Whatsapp::CallPermissionReplyService reply_data = extract_reply_data return unless reply_data&.dig(:accepted) - contact = find_contact(reply_data[:from_number]) - return unless contact - - conversation = find_active_conversation(contact) + conversation = find_requesting_conversation(reply_data[:context_id]) return unless conversation clear_permission_flag(conversation) - broadcast_permission_granted(contact, conversation) + broadcast_permission_granted(conversation.contact, conversation) end private @@ -26,28 +23,26 @@ class Whatsapp::CallPermissionReplyService accepted = reply[:response] == 'accept' Rails.logger.info "[WHATSAPP CALL] call_permission_reply from=#{message[:from]} accepted=#{accepted} permanent=#{reply[:is_permanent]}" - { from_number: message[:from], accepted: accepted } + { from_number: message[:from], accepted: accepted, context_id: message.dig(:context, :id) } end - # WhatsApp routing is anchored on contact_inboxes.source_id (the wa_id), not on - # contacts.phone_number — phone numbers can drift via normalization or edits. - def find_contact(from_number) - inbox.contact_inboxes.find_by(source_id: from_number)&.contact - end + # Match the reply to the conversation whose request message it actually points + # at (interactive replies carry context.id = our outbound wamid). Recency-based + # lookup would broadcast to the wrong thread when a contact has multiple + # parallel pending requests. + def find_requesting_conversation(context_id) + return if context_id.blank? - # Filter to threads that actually requested permission; multiple open threads otherwise hit the wrong one. - def find_active_conversation(contact) inbox.conversations - .where(contact: contact) .where.not(status: :resolved) - .where("additional_attributes ->> 'call_permission_requested_at' IS NOT NULL") - .order(:created_at) - .last + .where("additional_attributes ->> 'call_permission_request_message_id' = ?", context_id) + .first end def clear_permission_flag(conversation) - attrs = conversation.additional_attributes || {} - attrs.delete('call_permission_requested_at') + attrs = (conversation.additional_attributes || {}).except( + 'call_permission_requested_at', 'call_permission_request_message_id' + ) conversation.update!(additional_attributes: attrs) end diff --git a/spec/enterprise/services/whatsapp/call_permission_reply_service_spec.rb b/spec/enterprise/services/whatsapp/call_permission_reply_service_spec.rb index d52c1bb60..4504c7d9e 100644 --- a/spec/enterprise/services/whatsapp/call_permission_reply_service_spec.rb +++ b/spec/enterprise/services/whatsapp/call_permission_reply_service_spec.rb @@ -9,9 +9,13 @@ describe Whatsapp::CallPermissionReplyService do let(:inbox) { channel.inbox } let(:contact) { create(:contact, account: account, phone_number: '+15550001111') } let!(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: inbox, source_id: '15550001111') } + let(:request_wamid) { 'wamid.permission_request_abc' } let!(:conversation) do create(:conversation, account: account, inbox: inbox, contact: contact, contact_inbox: contact_inbox, status: :open, - additional_attributes: { 'call_permission_requested_at' => Time.zone.now.to_i }) + additional_attributes: { + 'call_permission_requested_at' => Time.current.iso8601, + 'call_permission_request_message_id' => request_wamid + }) end before do @@ -19,21 +23,22 @@ describe Whatsapp::CallPermissionReplyService do channel.save! end - def reply_params(response:) - { - entry: [{ changes: [{ value: { messages: [{ from: '15550001111', type: 'interactive', - interactive: { type: 'call_permission_reply', - call_permission_reply: { response: response, - is_permanent: false } } }] } }] }] - } + def reply_params(response:, context_id: request_wamid) + interactive = { type: 'call_permission_reply', + call_permission_reply: { response: response, is_permanent: false } } + message = { from: '15550001111', type: 'interactive', interactive: interactive } + message[:context] = { id: context_id } if context_id + { entry: [{ changes: [{ value: { messages: [message] } }] }] } end - it 'clears the requested-at flag and broadcasts voice_call.permission_granted on accept' do + it 'clears both permission flags and broadcasts voice_call.permission_granted on accept' do allow(ActionCable.server).to receive(:broadcast) described_class.new(inbox: inbox, params: reply_params(response: 'accept')).perform - expect(conversation.reload.additional_attributes).not_to include('call_permission_requested_at') + attrs = conversation.reload.additional_attributes + expect(attrs).not_to include('call_permission_requested_at') + expect(attrs).not_to include('call_permission_request_message_id') expect(ActionCable.server).to have_received(:broadcast).with( "account_#{account.id}", hash_including(event: 'voice_call.permission_granted', @@ -60,17 +65,33 @@ describe Whatsapp::CallPermissionReplyService do expect(ActionCable.server).not_to have_received(:broadcast) end - it 'targets the conversation that requested permission, not just any open one' do + it 'matches the originating conversation by context.id when the contact has multiple pending requests' do + other_request_wamid = 'wamid.permission_request_xyz' other_open = create(:conversation, account: account, inbox: inbox, contact: contact, contact_inbox: contact_inbox, - status: :open, additional_attributes: {}) + status: :open, + additional_attributes: { + 'call_permission_requested_at' => Time.current.iso8601, + 'call_permission_request_message_id' => other_request_wamid + }) allow(ActionCable.server).to receive(:broadcast) - described_class.new(inbox: inbox, params: reply_params(response: 'accept')).perform + described_class.new(inbox: inbox, params: reply_params(response: 'accept', context_id: other_request_wamid)).perform - expect(other_open.reload.additional_attributes).to eq({}) + # The reply pointed at other_open's request — it should be the cleared one, not `conversation` + expect(other_open.reload.additional_attributes).not_to include('call_permission_request_message_id') + expect(conversation.reload.additional_attributes).to include('call_permission_request_message_id') expect(ActionCable.server).to have_received(:broadcast).with( "account_#{account.id}", - hash_including(data: hash_including(conversation_id: conversation.id)) + hash_including(data: hash_including(conversation_id: other_open.id)) ) end + + it 'is a no-op when the reply has no context.id' do + allow(ActionCable.server).to receive(:broadcast) + + described_class.new(inbox: inbox, params: reply_params(response: 'accept', context_id: nil)).perform + + expect(conversation.reload.additional_attributes).to include('call_permission_request_message_id') + expect(ActionCable.server).not_to have_received(:broadcast) + end end