From 6b45b97d9c8b5ebd8ccb3eea946637aebd02baea Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma Date: Thu, 7 May 2026 17:06:46 +0700 Subject: [PATCH] chore(voice): drop non-UI changes from feat/whatsapp-call-ui Reset BE/migration/spec files to base so this PR diffs as a pure UI change. The deleted migration and the BE-side comment/edits belong on the BE PR chain (feat/whatsapp-call-incoming-pipeline) rather than the UI PR. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../contacts/conversations_controller.rb | 4 -- app/services/base/send_on_channel_service.rb | 3 +- ...00_cleanup_legacy_channel_voice_inboxes.rb | 53 ------------------- db/schema.rb | 2 +- .../v1/accounts/whatsapp_calls_controller.rb | 2 - .../controllers/twilio/voice_controller.rb | 18 ------- .../provider/twilio/conference_service.rb | 15 +++--- .../whatsapp/incoming_call_service.rb | 4 -- .../twilio/conference_service_spec.rb | 15 +----- 9 files changed, 12 insertions(+), 104 deletions(-) delete mode 100644 db/migrate/20260502090000_cleanup_legacy_channel_voice_inboxes.rb diff --git a/app/controllers/api/v1/accounts/contacts/conversations_controller.rb b/app/controllers/api/v1/accounts/contacts/conversations_controller.rb index e9ef0224b..20d66fb4d 100644 --- a/app/controllers/api/v1/accounts/contacts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/contacts/conversations_controller.rb @@ -4,10 +4,6 @@ class Api::V1::Accounts::Contacts::ConversationsController < Api::V1::Accounts:: conversations = Current.account.conversations.includes( :assignee, :contact, :inbox, :taggings ).where(contact_id: @contact.id) - # Optional inbox scoping so callers (e.g. the WhatsApp call button) can - # find the latest open conversation in a specific inbox even when the - # 20-row cap below would otherwise cut it off. - conversations = conversations.where(inbox_id: params[:inbox_id]) if params[:inbox_id].present? # Apply permission-based filtering using the existing service conversations = Conversations::PermissionFilterService.new( diff --git a/app/services/base/send_on_channel_service.rb b/app/services/base/send_on_channel_service.rb index 1f48b607c..44380a179 100644 --- a/app/services/base/send_on_channel_service.rb +++ b/app/services/base/send_on_channel_service.rb @@ -46,8 +46,7 @@ class Base::SendOnChannelService def invalid_message? # private notes aren't send to the channels # we should also avoid the case of message loops, when outgoing messages are created from channel - # voice_call bubbles are call status indicators, not deliverable messages — skip the send pipeline - # otherwise it falls through to "Template not found" / 24-hour-window errors + # voice_call bubbles are call status indicators, not deliverable messages message.private? || outgoing_message_originated_from_channel? || message.content_type == 'voice_call' end diff --git a/db/migrate/20260502090000_cleanup_legacy_channel_voice_inboxes.rb b/db/migrate/20260502090000_cleanup_legacy_channel_voice_inboxes.rb deleted file mode 100644 index f8957a550..000000000 --- a/db/migrate/20260502090000_cleanup_legacy_channel_voice_inboxes.rb +++ /dev/null @@ -1,53 +0,0 @@ -class CleanupLegacyChannelVoiceInboxes < ActiveRecord::Migration[7.1] - # Inboxes whose channel_type column still says 'Channel::Voice' became - # orphans after 20260326120001_drop_channel_voice.rb dropped both the - # channel_voice table and the model class. The polymorphic - # `belongs_to :channel` lookup on those rows fails to constantize - # `Channel::Voice`, crashing the inbox serializer with - # `uninitialized constant Channel::Voice`. - # - # Delete the orphan inboxes and their dependents via raw SQL so we - # bypass the polymorphic load that would crash inside Rails callbacks. - def up - legacy_ids = ActiveRecord::Base.connection - .exec_query("SELECT id FROM inboxes WHERE channel_type = 'Channel::Voice'") - .rows.flatten - - return if legacy_ids.empty? - - say_with_time "Cleaning up #{legacy_ids.size} legacy Channel::Voice inbox(es): #{legacy_ids.inspect}" do - delete_dependents(legacy_ids) - execute("DELETE FROM inboxes WHERE id IN (#{legacy_ids.join(',')})") - end - end - - def down - raise ActiveRecord::IrreversibleMigration - end - - private - - # Tables with an inbox_id FK that need clearing before the inbox row is removed. - # Order matters where one table FKs another (messages → conversations). - DEPENDENT_TABLES = %w[ - messages - conversations - contact_inboxes - inbox_members - agent_bot_inboxes - campaigns - webhooks - integrations_hooks - inbox_assignment_policies - ].freeze - - def delete_dependents(inbox_ids) - in_clause = inbox_ids.join(',') - DEPENDENT_TABLES.each do |table| - next unless ActiveRecord::Base.connection.table_exists?(table) - next unless ActiveRecord::Base.connection.column_exists?(table, :inbox_id) - - execute("DELETE FROM #{table} WHERE inbox_id IN (#{in_clause})") - end - end -end diff --git a/db/schema.rb b/db/schema.rb index 9470861ae..f7c3cce6d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2026_05_02_090000) do +ActiveRecord::Schema[7.1].define(version: 2026_04_30_114500) do # These extensions should be enabled to support this database enable_extension "pg_stat_statements" enable_extension "pg_trgm" 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 42631fb8c..b54940586 100644 --- a/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb @@ -59,7 +59,6 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseContro authorize @conversation, :show? end - # Twilio voice also exposes voice_enabled? but uses a different initiation path. def ensure_calling_enabled channel = @conversation.inbox.channel return if channel.is_a?(Channel::Whatsapp) && channel.voice_enabled? @@ -98,7 +97,6 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseContro 'uploaded' end - # Browser-built SDP offer is forwarded to Meta; the connect webhook later delivers Meta's answer. def create_outbound_call contact_phone = @conversation.contact.phone_number.delete('+') result = provider_service.initiate_call(contact_phone, params[:sdp_offer]) diff --git a/enterprise/app/controllers/twilio/voice_controller.rb b/enterprise/app/controllers/twilio/voice_controller.rb index 40da0b323..eb29c00bd 100644 --- a/enterprise/app/controllers/twilio/voice_controller.rb +++ b/enterprise/app/controllers/twilio/voice_controller.rb @@ -7,12 +7,6 @@ class Twilio::VoiceController < ApplicationController }.freeze before_action :set_inbox! - # Twilio's recording webhook fetches the audio file with the channel's - # auth credentials, so accepting an unsigned POST would let an attacker - # who guesses (or is leaked) a ConferenceSid coerce credential-bearing - # requests to an arbitrary host. Verify the signature before doing - # anything with the payload. - before_action :verify_twilio_signature!, only: :recording_status def status Voice::StatusUpdateService.new( @@ -192,18 +186,6 @@ class Twilio::VoiceController < ApplicationController call.update!(twilio_conference_sid: sid) end - def verify_twilio_signature! - signature = request.headers['X-Twilio-Signature'].to_s - auth_token = inbox_channel.auth_token.to_s - return head :forbidden if signature.blank? || auth_token.blank? - - validator = Twilio::Security::RequestValidator.new(auth_token) - payload = request.request_method.to_s.upcase == 'POST' ? request.request_parameters : request.query_parameters - return if validator.validate(request.original_url, payload, signature) - - head :forbidden - end - def set_inbox! digits = params[:phone].to_s.gsub(/\D/, '') phone_number = "+#{digits}" diff --git a/enterprise/app/services/voice/provider/twilio/conference_service.rb b/enterprise/app/services/voice/provider/twilio/conference_service.rb index 45b15d0c6..0ee368d00 100644 --- a/enterprise/app/services/voice/provider/twilio/conference_service.rb +++ b/enterprise/app/services/voice/provider/twilio/conference_service.rb @@ -8,14 +8,8 @@ class Voice::Provider::Twilio::ConferenceService call.conference_sid end - # Surface the 409 collision to a second agent who clicks accept, but DON'T - # claim accepted_by_agent here — the actual claim happens when Twilio's - # participant-join webhook fires for this agent's leg (Voice::Conference::Manager). - # If we claimed up-front and the browser's joinClientCall failed (device init - # error, tab close, network drop), the call would stay ringing-but-claimed - # and every other agent would 409 with no recovery path. def mark_agent_joined(user:) - raise_already_accepted!(call.accepted_by_agent) if claimed_by_other_agent?(user) + claim_call!(user) assign_conversation!(user) end @@ -31,6 +25,13 @@ class Voice::Provider::Twilio::ConferenceService private + def claim_call!(user) + call.with_lock do + raise_already_accepted!(call.accepted_by_agent) if claimed_by_other_agent?(user) + call.update!(accepted_by_agent: user) if call.accepted_by_agent_id != user.id + end + end + def claimed_by_other_agent?(user) call.accepted_by_agent_id.present? && call.accepted_by_agent_id != user.id end diff --git a/enterprise/app/services/whatsapp/incoming_call_service.rb b/enterprise/app/services/whatsapp/incoming_call_service.rb index 998c063bf..c5621950f 100644 --- a/enterprise/app/services/whatsapp/incoming_call_service.rb +++ b/enterprise/app/services/whatsapp/incoming_call_service.rb @@ -71,10 +71,6 @@ class Whatsapp::IncomingCallService payload.dig(:session, :sdp_type).to_s.downcase == 'offer' end - def inbound_offer?(payload) - payload.dig(:session, :sdp_type).to_s.downcase == 'offer' - end - def create_inbound_call(payload) sdp_offer = payload.dig(:session, :sdp) call = Voice::InboundCallBuilder.perform!( diff --git a/spec/enterprise/services/voice/provider/twilio/conference_service_spec.rb b/spec/enterprise/services/voice/provider/twilio/conference_service_spec.rb index 71902f146..519519eed 100644 --- a/spec/enterprise/services/voice/provider/twilio/conference_service_spec.rb +++ b/spec/enterprise/services/voice/provider/twilio/conference_service_spec.rb @@ -36,23 +36,12 @@ describe Voice::Provider::Twilio::ConferenceService do end describe '#mark_agent_joined' do - it 'assigns the conversation to the agent without claiming the call (claim defers to participant-join webhook)' do + it 'sets accepted_by_agent on the Call' do agent = create(:user, account: account) - create(:inbox_member, inbox: channel.inbox, user: agent) service.mark_agent_joined(user: agent) - expect(call.reload.accepted_by_agent_id).to be_nil - expect(conversation.reload.assignee_id).to eq(agent.id) - end - - it 'raises CallAlreadyAccepted when another agent has already claimed the call' do - first_agent = create(:user, account: account) - second_agent = create(:user, account: account) - call.update!(accepted_by_agent: first_agent) - - expect { service.mark_agent_joined(user: second_agent) } - .to raise_error(CustomExceptions::CallAlreadyAccepted) + expect(call.reload.accepted_by_agent_id).to eq(agent.id) end end