refactor(voice): stop duplicating call state in message.content_attributes
Now that the frontend reads from message.call (previous commit), the
message row is just a display anchor — all call state lives on the Call
record and is surfaced via the serializer / push_event_data.
- CallMessageBuilder shrinks to creating the bare voice_call message.
content_attributes.data keeps only { call_sid } as a breadcrumb and a
finder fallback for the edge case where message_id isn't linked yet.
- CallStatus::Manager stops re-writing the message on status transitions.
call.message.touch triggers the standard message.updated dispatcher so
sockets re-broadcast with the fresh Call embedded.
- Spec assertions switch from content_attributes.data.* to call.* +
message.call object identity; status update spec asserts message is
touched.
This commit is contained in:
@@ -8,8 +8,7 @@ class Voice::CallMessageBuilder
|
||||
end
|
||||
|
||||
def perform!
|
||||
message = find_message
|
||||
message ? update_message!(message) : create_message!
|
||||
find_message || create_message!
|
||||
end
|
||||
|
||||
private
|
||||
@@ -23,55 +22,16 @@ class Voice::CallMessageBuilder
|
||||
.find_by("content_attributes -> 'data' ->> 'call_sid' = ?", call.provider_call_id)
|
||||
end
|
||||
|
||||
def update_message!(message)
|
||||
existing = (message.content_attributes || {}).fetch('data', {})
|
||||
message.update!(content_attributes: { 'data' => data_payload(existing) })
|
||||
message
|
||||
end
|
||||
|
||||
def create_message!
|
||||
params = {
|
||||
content: 'Voice Call',
|
||||
message_type: message_type,
|
||||
message_type: call.outgoing? ? 'outgoing' : 'incoming',
|
||||
content_type: 'voice_call',
|
||||
content_attributes: { 'data' => data_payload({}) }
|
||||
content_attributes: { 'data' => { 'call_sid' => call.provider_call_id } }
|
||||
}
|
||||
Messages::MessageBuilder.new(sender, call.conversation, params).perform
|
||||
end
|
||||
|
||||
def data_payload(existing)
|
||||
now = Time.zone.now.to_i
|
||||
meta = existing.fetch('meta', {})
|
||||
|
||||
{
|
||||
'call_sid' => call.provider_call_id,
|
||||
'status' => call.status.tr('_', '-'),
|
||||
'call_direction' => call.display_direction,
|
||||
'from_number' => from_number,
|
||||
'to_number' => to_number,
|
||||
'duration' => call.duration_seconds,
|
||||
'recording_url' => existing['recording_url'],
|
||||
'transcript' => call.transcript,
|
||||
'conference_sid' => call.conference_sid,
|
||||
'meta' => {
|
||||
'created_at' => meta['created_at'] || now,
|
||||
'ringing_at' => meta['ringing_at'] || now
|
||||
}
|
||||
}
|
||||
end
|
||||
|
||||
def from_number
|
||||
call.incoming? ? call.contact.phone_number : call.inbox.channel&.phone_number
|
||||
end
|
||||
|
||||
def to_number
|
||||
call.incoming? ? call.inbox.channel&.phone_number : call.contact.phone_number
|
||||
end
|
||||
|
||||
def message_type
|
||||
call.outgoing? ? 'outgoing' : 'incoming'
|
||||
end
|
||||
|
||||
def sender
|
||||
call.outgoing? ? call.accepted_by_agent : call.contact
|
||||
end
|
||||
|
||||
@@ -7,7 +7,9 @@ class Voice::CallStatus::Manager
|
||||
|
||||
apply_call_updates!(status, duration: duration, timestamp: timestamp)
|
||||
call.conversation.update!(last_activity_at: Time.zone.now)
|
||||
Voice::CallMessageBuilder.perform!(call: call)
|
||||
# Touch the linked message so the normal message.updated dispatcher
|
||||
# re-broadcasts it with the fresh Call embedded via push_event_data.
|
||||
call.message&.touch
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@@ -44,19 +44,13 @@ RSpec.describe Voice::InboundCallBuilder do
|
||||
call = perform_builder
|
||||
voice_message = call.conversation.messages.voice_calls.last
|
||||
|
||||
expect(voice_message).to be_present
|
||||
expect(voice_message.message_type).to eq('incoming')
|
||||
expect(call.message_id).to eq(voice_message.id)
|
||||
|
||||
data = voice_message.content_attributes['data']
|
||||
expect(data).to include(
|
||||
'call_sid' => call_sid,
|
||||
'status' => 'ringing',
|
||||
'call_direction' => 'inbound',
|
||||
'conference_sid' => call.conference_sid,
|
||||
'from_number' => from_number,
|
||||
'to_number' => inbox.channel.phone_number
|
||||
)
|
||||
aggregate_failures do
|
||||
expect(voice_message).to be_present
|
||||
expect(voice_message.message_type).to eq('incoming')
|
||||
expect(call.message_id).to eq(voice_message.id)
|
||||
expect(voice_message.content_attributes['data']['call_sid']).to eq(call_sid)
|
||||
expect(voice_message.call).to eq(call)
|
||||
end
|
||||
end
|
||||
|
||||
it 'sets the contact name to the phone number for new callers' do
|
||||
|
||||
@@ -40,14 +40,8 @@ RSpec.describe Voice::OutboundCallBuilder do
|
||||
voice_message = call.conversation.messages.voice_calls.last
|
||||
expect(call.message_id).to eq(voice_message.id)
|
||||
expect(voice_message.message_type).to eq('outgoing')
|
||||
|
||||
expect(voice_message.content_attributes['data']).to include(
|
||||
'call_sid' => call_sid,
|
||||
'call_direction' => 'outbound',
|
||||
'conference_sid' => call.conference_sid,
|
||||
'from_number' => channel.phone_number,
|
||||
'to_number' => contact.phone_number
|
||||
)
|
||||
expect(voice_message.content_attributes['data']['call_sid']).to eq(call_sid)
|
||||
expect(voice_message.call).to eq(call)
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -42,7 +42,10 @@ RSpec.describe Voice::StatusUpdateService do
|
||||
.and_return(instance_double(Twilio::VoiceWebhookSetupService, perform: "AP#{SecureRandom.hex(16)}"))
|
||||
end
|
||||
|
||||
it 'updates the Call and the matching voice_call message with the normalized status' do
|
||||
it 'updates the Call and touches the linked message on status transition' do
|
||||
previous_updated_at = message.updated_at
|
||||
travel 1.second
|
||||
|
||||
described_class.new(
|
||||
account: account,
|
||||
call_sid: call_sid,
|
||||
@@ -53,21 +56,17 @@ RSpec.describe Voice::StatusUpdateService do
|
||||
message.reload
|
||||
|
||||
expect(call.status).to eq('completed')
|
||||
expect(message.content_attributes.dig('data', 'status')).to eq('completed')
|
||||
expect(message.updated_at).to be > previous_updated_at
|
||||
end
|
||||
|
||||
it 'normalizes busy to no_answer on the Call and no-answer on the message payload' do
|
||||
it 'normalizes busy to no_answer on the Call' do
|
||||
described_class.new(
|
||||
account: account,
|
||||
call_sid: call_sid,
|
||||
call_status: 'busy'
|
||||
).perform
|
||||
|
||||
call.reload
|
||||
message.reload
|
||||
|
||||
expect(call.status).to eq('no_answer')
|
||||
expect(message.content_attributes.dig('data', 'status')).to eq('no-answer')
|
||||
expect(call.reload.status).to eq('no_answer')
|
||||
end
|
||||
|
||||
it 'no-ops when no Call matches the provided call_sid' do
|
||||
|
||||
Reference in New Issue
Block a user