Contact custom attributes filled in the pre-chat form (e.g. a CPF field)
were silently lost when the visitor's email or phone matched an existing
contact. The widget sent the attributes in a separate request that raced
the conversation create: creating the conversation merges the widget
contact into the existing contact, so the attribute update landed on the
destroyed contact and vanished without an error. New visitors were
unaffected, which made the bug hard to spot.
The fix sends contact custom attributes inside the same request that
identifies the contact. The widget conversation create endpoint now
accepts `contact.custom_attributes` and applies them through
`ContactIdentifyAction` in the same transaction as the merge, so the
submitted values always land on the surviving contact. The campaign path
folds the attributes into the existing contact update call the same way.
<details>
<summary>Reproduction script (rails runner, concurrent
threads)</summary>
```ruby
# Concurrent reproduction of the pre-chat form race that loses contact
# custom attributes when the visitor's email matches an existing contact.
#
# The old widget fired two unawaited requests on pre-chat submit. Each
# iteration replays them as real concurrent threads:
# - create thread = POST /widget/conversations: resolves the widget contact,
# then runs ContactIdentifyAction (which merges the widget contact into the
# existing contact) inside a transaction held open while the conversation
# and message are created.
# - patch thread = PATCH /widget/contact: resolves the widget contact via its
# contact inbox and applies the custom attributes through
# ContactIdentifyAction, exactly like Widget::ContactsController#update.
#
# Run with: bundle exec rails runner confirm_prechat_race.rb
# Creates throwaway contacts on Account.first and deletes them afterwards.
ITERATIONS = 20
CPF = '123.456.789-09'.freeze
account = Account.first!
inbox = account.inboxes.first!
losses = 0
ITERATIONS.times do |i|
existing = account.contacts.create!(name: 'Existing Contact', email: "race-existing-#{SecureRandom.hex(6)}@example.com")
temp = account.contacts.create!(name: 'Widget Visitor')
contact_inbox = ContactInbox.create!(contact: temp, inbox: inbox, source_id: SecureRandom.uuid)
begin
create_request = Thread.new do
ActiveRecord::Base.connection_pool.with_connection do
widget_contact = ContactInbox.find(contact_inbox.id).contact # set_contact before_action
ActiveRecord::Base.transaction do
ContactIdentifyAction.new(
contact: widget_contact,
params: { email: existing.email, phone_number: nil, name: 'Widget Visitor' },
retain_original_contact_name: true,
discard_invalid_attrs: true
).perform
sleep(0.02) # conversation + message creation keeps the transaction open
end
end
end
patch_request = Thread.new do
ActiveRecord::Base.connection_pool.with_connection do
sleep(rand * 0.02) # network jitter between the two requests
widget_contact = ContactInbox.find(contact_inbox.id).contact # set_contact before_action
ContactIdentifyAction.new(
contact: widget_contact,
params: { custom_attributes: { cpf: CPF } },
discard_invalid_attrs: true
).perform
end
end
create_request.join
patch_request.join
survivor = existing.reload
if survivor.custom_attributes['cpf'] == CPF
puts "iteration #{i + 1}: CPF survived"
else
losses += 1
puts "iteration #{i + 1}: CPF LOST (survivor custom_attributes: #{survivor.custom_attributes.inspect})"
end
ensure
account.contacts.where(id: [existing.id, temp.id]).find_each(&:destroy!)
end
end
puts
puts "#{losses}/#{ITERATIONS} iterations lost the CPF -- race #{losses.positive? ? 'confirmed' : 'not reproduced in this run'}"
```
Result: **20/20 iterations lose the CPF** (consistent across repeated
runs). The conversation create holds its transaction open across the
contact merge, so the fast attribute update either resolves the
soon-to-be-destroyed widget contact or blocks on its row lock and then
updates 0 rows — silently, with no error. This is why the customer sees
the loss every time, not intermittently.
Note: the script replays the old two-request flow at the service layer,
so it reproduces the loss even with this fix applied — the fix works by
removing the second request from the widget, not by changing the raced
code paths. The new request spec (`saves contact custom attributes on
the surviving contact when merged into an existing contact`) covers the
fixed contract.
</details>
## Closes
- Reported via support conversation:
https://app.chatwoot.com/app/accounts/1/conversations/84465
## How to reproduce
1. Create a website inbox with a pre-chat form that has a contact custom
attribute field (e.g. CPF).
2. Create a contact with a known email address.
3. As a visitor, open the widget and fill the pre-chat form using that
same email plus a value for the custom attribute.
4. Before: the attribute never appears on the contact profile. After: it
is saved on the existing contact, overwriting any stale value.
## What changed
- `POST /api/v1/widget/conversations` now permits
`contact.custom_attributes` and passes it to `ContactIdentifyAction`,
which deep-merges it atomically with the contact merge.
- The widget pre-chat form sends contact custom attributes inside the
conversation create payload instead of a separate `setCustomAttributes`
call; the campaign path includes them in the `contacts/update` payload.
380 lines
14 KiB
Ruby
380 lines
14 KiB
Ruby
require 'rails_helper'
|
|
|
|
RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do
|
|
let(:account) { create(:account) }
|
|
let(:web_widget) { create(:channel_widget, account: account) }
|
|
let(:contact) { create(:contact, account: account, email: nil) }
|
|
let(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: web_widget.inbox) }
|
|
let(:second_session) { create(:contact_inbox, contact: contact, inbox: web_widget.inbox) }
|
|
let!(:conversation) { create(:conversation, contact: contact, account: account, inbox: web_widget.inbox, contact_inbox: contact_inbox) }
|
|
let(:payload) { { source_id: contact_inbox.source_id, inbox_id: web_widget.inbox.id } }
|
|
let(:token) { Widget::TokenService.new(payload: payload).generate_token }
|
|
let(:token_without_conversation) do
|
|
Widget::TokenService.new(payload: { source_id: second_session.source_id, inbox_id: web_widget.inbox.id }).generate_token
|
|
end
|
|
|
|
def conversation_params
|
|
{
|
|
website_token: web_widget.website_token,
|
|
contact: {
|
|
name: 'contact-name',
|
|
email: 'contact-email@chatwoot.com',
|
|
phone_number: '+919745313456'
|
|
},
|
|
message: {
|
|
content: 'This is a test message'
|
|
},
|
|
custom_attributes: { order_id: '12345' }
|
|
}
|
|
end
|
|
|
|
describe 'GET /api/v1/widget/conversations' do
|
|
context 'with a conversation' do
|
|
it 'returns the correct conversation params' do
|
|
allow(Rails.configuration.dispatcher).to receive(:dispatch)
|
|
get '/api/v1/widget/conversations',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: { website_token: web_widget.website_token },
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
json_response = response.parsed_body
|
|
|
|
expect(json_response['id']).to eq(conversation.display_id)
|
|
expect(json_response['status']).to eq(conversation.status)
|
|
end
|
|
end
|
|
|
|
context 'with a conversation but invalid source id' do
|
|
it 'returns the correct conversation params' do
|
|
allow(Rails.configuration.dispatcher).to receive(:dispatch)
|
|
|
|
payload = { source_id: 'invalid source id', inbox_id: web_widget.inbox.id }
|
|
token = Widget::TokenService.new(payload: payload).generate_token
|
|
get '/api/v1/widget/conversations',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: { website_token: web_widget.website_token },
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:not_found)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe 'POST /api/v1/widget/conversations' do
|
|
it 'creates a conversation with correct details' do
|
|
post '/api/v1/widget/conversations',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: conversation_params,
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
json_response = response.parsed_body
|
|
expect(json_response['id']).not_to be_nil
|
|
expect(json_response['contact']['email']).to eq 'contact-email@chatwoot.com'
|
|
expect(json_response['contact']['phone_number']).to eq '+919745313456'
|
|
expect(json_response['contact']['name']).to eq 'contact-name'
|
|
end
|
|
|
|
it 'creates a conversation with correct message and custom attributes' do
|
|
post '/api/v1/widget/conversations',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: conversation_params,
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
json_response = response.parsed_body
|
|
expect(json_response['custom_attributes']['order_id']).to eq '12345'
|
|
expect(json_response['messages'][0]['content']).to eq 'This is a test message'
|
|
expect(json_response['messages'][0]['message_type']).to eq 0
|
|
end
|
|
|
|
it 'create a conversation with a name and without an email' do
|
|
post '/api/v1/widget/conversations',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: {
|
|
website_token: web_widget.website_token,
|
|
contact: {
|
|
name: 'alphy'
|
|
},
|
|
message: {
|
|
content: 'This is a test message'
|
|
}
|
|
},
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
json_response = response.parsed_body
|
|
expect(json_response['id']).not_to be_nil
|
|
expect(json_response['contact']['email']).to be_nil
|
|
expect(json_response['contact']['name']).to eq 'alphy'
|
|
expect(json_response['messages'][0]['content']).to eq 'This is a test message'
|
|
end
|
|
|
|
it 'does not update the name if the contact already exist' do
|
|
existing_contact = create(:contact, account: account, email: 'contact-email@chatwoot.com')
|
|
|
|
post '/api/v1/widget/conversations',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: {
|
|
website_token: web_widget.website_token,
|
|
contact: {
|
|
name: 'contact-name',
|
|
email: existing_contact.email,
|
|
phone_number: '+919745313456'
|
|
},
|
|
message: {
|
|
content: 'This is a test message'
|
|
},
|
|
custom_attributes: { order_id: '12345' }
|
|
},
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
json_response = response.parsed_body
|
|
expect(json_response['id']).not_to be_nil
|
|
expect(json_response['contact']['email']).to eq existing_contact.email
|
|
expect(json_response['contact']['name']).not_to eq 'contact-name'
|
|
expect(json_response['contact']['phone_number']).to eq '+919745313456'
|
|
expect(json_response['custom_attributes']['order_id']).to eq '12345'
|
|
expect(json_response['messages'][0]['content']).to eq 'This is a test message'
|
|
end
|
|
|
|
it 'saves contact custom attributes on the widget contact' do
|
|
post '/api/v1/widget/conversations',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: {
|
|
website_token: web_widget.website_token,
|
|
contact: {
|
|
name: 'contact-name',
|
|
email: 'contact-email@chatwoot.com',
|
|
custom_attributes: { cpf: '123.456.789-09' }
|
|
},
|
|
message: {
|
|
content: 'This is a test message'
|
|
}
|
|
},
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
expect(contact.reload.custom_attributes['cpf']).to eq('123.456.789-09')
|
|
end
|
|
|
|
it 'saves contact custom attributes on the surviving contact when merged into an existing contact' do
|
|
existing_contact = create(:contact, account: account, email: 'contact-email@chatwoot.com', custom_attributes: { 'cpf' => 'old-value' })
|
|
|
|
post '/api/v1/widget/conversations',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: {
|
|
website_token: web_widget.website_token,
|
|
contact: {
|
|
name: 'contact-name',
|
|
email: existing_contact.email,
|
|
custom_attributes: { cpf: '123.456.789-09' }
|
|
},
|
|
message: {
|
|
content: 'This is a test message'
|
|
}
|
|
},
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
# the widget contact is merged into the existing contact; the freshly
|
|
# submitted value must land on the surviving contact and win over stale data
|
|
expect(Contact.exists?(contact.id)).to be(false)
|
|
expect(existing_contact.reload.custom_attributes['cpf']).to eq('123.456.789-09')
|
|
end
|
|
|
|
it 'doesnt not add phone number if the invalid phone number is provided' do
|
|
existing_contact = create(:contact, account: account)
|
|
|
|
post '/api/v1/widget/conversations',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: {
|
|
website_token: web_widget.website_token,
|
|
contact: {
|
|
name: 'contact-name-1',
|
|
email: existing_contact.email,
|
|
phone_number: '13456'
|
|
},
|
|
message: {
|
|
content: 'This is a test message'
|
|
}
|
|
},
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
json_response = response.parsed_body
|
|
expect(json_response['contact']['phone_number']).to be_nil
|
|
end
|
|
end
|
|
|
|
describe 'POST /api/v1/widget/conversations/toggle_typing' do
|
|
context 'with a conversation' do
|
|
it 'dispatches the correct typing status' do
|
|
allow(Rails.configuration.dispatcher).to receive(:dispatch)
|
|
post '/api/v1/widget/conversations/toggle_typing',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: { typing_status: 'on', website_token: web_widget.website_token },
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
expect(Rails.configuration.dispatcher).to have_received(:dispatch)
|
|
.with(Conversation::CONVERSATION_TYPING_ON, kind_of(Time), { conversation: conversation, user: contact })
|
|
end
|
|
end
|
|
end
|
|
|
|
describe 'POST /api/v1/widget/conversations/update_last_seen' do
|
|
context 'with a conversation' do
|
|
it 'returns the correct conversation params' do
|
|
current_time = DateTime.now.utc
|
|
allow(DateTime).to receive(:now).and_return(current_time)
|
|
|
|
allow(Rails.configuration.dispatcher).to receive(:dispatch)
|
|
expect(conversation.contact_last_seen_at).to be_nil
|
|
expect(Conversations::UpdateMessageStatusJob).to receive(:perform_later).with(conversation.id, current_time)
|
|
|
|
post '/api/v1/widget/conversations/update_last_seen',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: { website_token: web_widget.website_token },
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
|
|
expect(conversation.reload.contact_last_seen_at).not_to be_nil
|
|
end
|
|
end
|
|
end
|
|
|
|
describe 'POST /api/v1/widget/conversations/transcript' do
|
|
context 'with a conversation' do
|
|
it 'sends transcript email' do
|
|
contact.update(email: 'test@test.com')
|
|
mailer = double
|
|
allow(ConversationReplyMailer).to receive(:with).and_return(mailer)
|
|
allow(mailer).to receive(:conversation_transcript)
|
|
|
|
post '/api/v1/widget/conversations/transcript',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: { website_token: web_widget.website_token },
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
expect(mailer).to have_received(:conversation_transcript).with(conversation, 'test@test.com')
|
|
contact.update(email: nil)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe 'GET /api/v1/widget/conversations/toggle_status' do
|
|
context 'when user end conversation from widget' do
|
|
it 'resolves the conversation' do
|
|
expect(conversation.open?).to be true
|
|
|
|
get '/api/v1/widget/conversations/toggle_status',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: { website_token: web_widget.website_token },
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
expect(conversation.reload.resolved?).to be true
|
|
expect(Conversations::ActivityMessageJob).to have_been_enqueued.at_least(:once).with(
|
|
conversation,
|
|
{
|
|
account_id: conversation.account_id,
|
|
inbox_id: conversation.inbox_id,
|
|
message_type: :activity,
|
|
content: "Conversation was resolved by #{contact.name}"
|
|
}
|
|
)
|
|
end
|
|
end
|
|
|
|
context 'when end conversation is not permitted' do
|
|
before do
|
|
web_widget.end_conversation = false
|
|
web_widget.save!
|
|
end
|
|
|
|
it 'returns action not permitted status' do
|
|
expect(conversation.open?).to be true
|
|
|
|
get '/api/v1/widget/conversations/toggle_status',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: { website_token: web_widget.website_token },
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:forbidden)
|
|
expect(conversation.reload.resolved?).to be false
|
|
end
|
|
end
|
|
|
|
context 'when a token without any conversation is used' do
|
|
it 'returns not found status' do
|
|
get '/api/v1/widget/conversations/toggle_status',
|
|
headers: { 'X-Auth-Token' => token_without_conversation },
|
|
params: { website_token: web_widget.website_token },
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:not_found)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe 'POST /api/v1/widget/conversations/set_custom_attributes' do
|
|
let(:params) { { website_token: web_widget.website_token, custom_attributes: { 'product_name': 'Chatwoot' } } }
|
|
|
|
context 'with invalid website token' do
|
|
it 'returns unauthorized' do
|
|
post '/api/v1/widget/conversations/set_custom_attributes', params: { website_token: '' }
|
|
expect(response).to have_http_status(:not_found)
|
|
end
|
|
end
|
|
|
|
context 'with correct website token' do
|
|
it 'sets the values when provided' do
|
|
post '/api/v1/widget/conversations/set_custom_attributes',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: params,
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
conversation.reload
|
|
# conversation custom attributes should have "product_name" key with value "Chatwoot"
|
|
expect(conversation.custom_attributes).to include('product_name' => 'Chatwoot')
|
|
end
|
|
end
|
|
end
|
|
|
|
describe 'POST /api/v1/widget/conversations/destroy_custom_attributes' do
|
|
let(:params) { { website_token: web_widget.website_token, custom_attribute: ['product_name'] } }
|
|
|
|
context 'with invalid website token' do
|
|
it 'returns unauthorized' do
|
|
post '/api/v1/widget/conversations/destroy_custom_attributes', params: { website_token: '' }
|
|
expect(response).to have_http_status(:not_found)
|
|
end
|
|
end
|
|
|
|
context 'with correct website token' do
|
|
it 'sets the values when provided' do
|
|
# ensure conversation has the attribute
|
|
conversation.custom_attributes = { 'product_name': 'Chatwoot' }
|
|
conversation.save!
|
|
expect(conversation.custom_attributes).to include('product_name' => 'Chatwoot')
|
|
|
|
post '/api/v1/widget/conversations/destroy_custom_attributes',
|
|
headers: { 'X-Auth-Token' => token },
|
|
params: params,
|
|
as: :json
|
|
|
|
expect(response).to have_http_status(:success)
|
|
conversation.reload
|
|
# conversation custom attributes should not have "product_name" key with value "Chatwoot"
|
|
expect(conversation.custom_attributes).not_to include('product_name' => 'Chatwoot')
|
|
end
|
|
end
|
|
end
|
|
end
|