From 6b3800e25bd5acbd5527b149762ad2da1081606f Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Mon, 13 Jul 2026 13:51:58 +0530 Subject: [PATCH] fix: pre-chat form contact custom attributes lost for existing contacts (#14956) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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.
Reproduction script (rails runner, concurrent threads) ```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.
## 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. --- .../api/v1/widget/base_controller.rb | 4 + .../api/v1/widget/conversations_controller.rb | 4 +- app/javascript/widget/api/endPoints.js | 1 + .../widget/api/specs/endPoints.spec.js | 44 +++++++++ app/javascript/widget/views/PreChatForm.vue | 13 ++- .../widget/views/specs/PreChatForm.spec.js | 89 +++++++++++++++++++ .../widget/conversations_controller_spec.rb | 45 ++++++++++ 7 files changed, 191 insertions(+), 9 deletions(-) create mode 100644 app/javascript/widget/views/specs/PreChatForm.spec.js diff --git a/app/controllers/api/v1/widget/base_controller.rb b/app/controllers/api/v1/widget/base_controller.rb index 5b87e2d1a..3912e5b6e 100644 --- a/app/controllers/api/v1/widget/base_controller.rb +++ b/app/controllers/api/v1/widget/base_controller.rb @@ -59,6 +59,10 @@ class Api::V1::Widget::BaseController < ApplicationController permitted_params.dig(:contact, :phone_number) end + def contact_custom_attributes + permitted_params.dig(:contact, :custom_attributes)&.to_h + end + def browser_params { browser_name: browser.name, diff --git a/app/controllers/api/v1/widget/conversations_controller.rb b/app/controllers/api/v1/widget/conversations_controller.rb index 00e718614..8f5977d54 100644 --- a/app/controllers/api/v1/widget/conversations_controller.rb +++ b/app/controllers/api/v1/widget/conversations_controller.rb @@ -19,7 +19,7 @@ class Api::V1::Widget::ConversationsController < Api::V1::Widget::BaseController def process_update_contact @contact = ContactIdentifyAction.new( contact: @contact, - params: { email: contact_email, phone_number: contact_phone_number, name: contact_name }, + params: { email: contact_email, phone_number: contact_phone_number, name: contact_name, custom_attributes: contact_custom_attributes }, retain_original_contact_name: true, discard_invalid_attrs: true ).perform @@ -95,7 +95,7 @@ class Api::V1::Widget::ConversationsController < Api::V1::Widget::BaseController end def permitted_params - params.permit(:id, :typing_status, :website_token, :email, contact: [:name, :email, :phone_number], + params.permit(:id, :typing_status, :website_token, :email, contact: [:name, :email, :phone_number, { custom_attributes: {} }], message: [:content, :referer_url, :timestamp, :echo_id], custom_attributes: {}) end diff --git a/app/javascript/widget/api/endPoints.js b/app/javascript/widget/api/endPoints.js index 713de56f1..b1c76e94f 100755 --- a/app/javascript/widget/api/endPoints.js +++ b/app/javascript/widget/api/endPoints.js @@ -11,6 +11,7 @@ const createConversation = params => { name: params.fullName, email: params.emailAddress, phone_number: params.phoneNumber, + custom_attributes: params.contactCustomAttributes, }, message: { content: params.message, diff --git a/app/javascript/widget/api/specs/endPoints.spec.js b/app/javascript/widget/api/specs/endPoints.spec.js index b95b2f659..cf7f2486b 100644 --- a/app/javascript/widget/api/specs/endPoints.spec.js +++ b/app/javascript/widget/api/specs/endPoints.spec.js @@ -32,6 +32,50 @@ describe('#sendMessage', () => { }); }); +describe('#createConversation', () => { + it('includes contact custom attributes in the payload', () => { + const spy = vi.spyOn(global, 'Date').mockImplementation(() => ({ + toString: () => 'mock date', + })); + vi.spyOn(window, 'location', 'get').mockReturnValue({ + ...window.location, + search: '?param=1', + }); + + window.WOOT_WIDGET = { + $root: { $i18n: { locale: 'ar' } }, + }; + + const result = endPoints.createConversation({ + fullName: 'John', + emailAddress: 'john@example.com', + phoneNumber: '+919745313456', + message: 'hey', + customAttributes: { order_id: '12345' }, + contactCustomAttributes: { cpf: '123.456.789-09' }, + }); + + expect(result).toEqual({ + url: `/api/v1/widget/conversations?param=1&locale=ar`, + params: { + contact: { + name: 'John', + email: 'john@example.com', + phone_number: '+919745313456', + custom_attributes: { cpf: '123.456.789-09' }, + }, + message: { + content: 'hey', + timestamp: 'mock date', + referer_url: '', + }, + custom_attributes: { order_id: '12345' }, + }, + }); + spy.mockRestore(); + }); +}); + describe('#sendMessage with pending metadata', () => { it('includes custom_attributes and labels in payload', () => { const spy = vi.spyOn(global, 'Date').mockImplementation(() => ({ diff --git a/app/javascript/widget/views/PreChatForm.vue b/app/javascript/widget/views/PreChatForm.vue index 4872edbcd..5bbac0596 100644 --- a/app/javascript/widget/views/PreChatForm.vue +++ b/app/javascript/widget/views/PreChatForm.vue @@ -3,7 +3,6 @@ import { mapActions } from 'vuex'; import { useRouter } from 'vue-router'; import PreChatForm from '../components/PreChat/Form.vue'; import configMixin from '../mixins/configMixin'; -import { isEmptyObject } from 'widget/helpers/utils'; import { ON_CONVERSATION_CREATED } from '../constants/widgetBusEvents'; import { emitter } from 'shared/helpers/mitt'; @@ -42,6 +41,10 @@ export default { contactCustomAttributes, conversationCustomAttributes, }) { + // Contact custom attributes are sent within the same request that + // identifies the contact. A separate update call would race the contact + // merge on the server (matching email/phone) and write the values to + // the destroyed contact, silently losing them. if (activeCampaignId) { emitter.emit('execute-campaign', { campaignId: activeCampaignId, @@ -52,6 +55,7 @@ export default { email: emailAddress, name: fullName, phone_number: phoneNumber, + custom_attributes: contactCustomAttributes, }, }); } else { @@ -63,14 +67,9 @@ export default { message: message, phoneNumber: phoneNumber, customAttributes: conversationCustomAttributes, + contactCustomAttributes: contactCustomAttributes, }); } - if (!isEmptyObject(contactCustomAttributes)) { - this.$store.dispatch( - 'contacts/setCustomAttributes', - contactCustomAttributes - ); - } }, }, }; diff --git a/app/javascript/widget/views/specs/PreChatForm.spec.js b/app/javascript/widget/views/specs/PreChatForm.spec.js new file mode 100644 index 000000000..25bb16b53 --- /dev/null +++ b/app/javascript/widget/views/specs/PreChatForm.spec.js @@ -0,0 +1,89 @@ +import { shallowMount, flushPromises } from '@vue/test-utils'; +import { createStore } from 'vuex'; +import PreChatFormView from '../PreChatForm.vue'; + +global.chatwootWebChannel = { + preChatFormEnabled: true, + preChatFormOptions: { pre_chat_fields: [], pre_chat_message: '' }, +}; + +describe('PreChatForm view', () => { + let createConversation; + let setCustomAttributes; + let updateContact; + let store; + + beforeEach(() => { + createConversation = vi.fn(); + setCustomAttributes = vi.fn(); + updateContact = vi.fn(); + store = createStore({ + modules: { + conversation: { + namespaced: true, + actions: { createConversation, clearConversations: vi.fn() }, + }, + conversationAttributes: { + namespaced: true, + actions: { clearConversationAttributes: vi.fn() }, + }, + contacts: { + namespaced: true, + actions: { setCustomAttributes, update: updateContact }, + }, + }, + }); + }); + + const mountView = () => + shallowMount(PreChatFormView, { global: { plugins: [store] } }); + + it('sends contact custom attributes with the conversation create request', async () => { + const wrapper = mountView(); + wrapper.vm.onSubmit({ + fullName: 'John', + emailAddress: 'john@example.com', + message: 'hey', + contactCustomAttributes: { cpf: '123.456.789-09' }, + conversationCustomAttributes: { order_id: '12345' }, + }); + await flushPromises(); + + expect(createConversation).toHaveBeenCalledWith(expect.anything(), { + fullName: 'John', + emailAddress: 'john@example.com', + message: 'hey', + phoneNumber: undefined, + customAttributes: { order_id: '12345' }, + contactCustomAttributes: { cpf: '123.456.789-09' }, + }); + // attributes ride along in the create request itself; a separate call + // would race the contact merge on the server and write to a destroyed + // contact + expect(setCustomAttributes).not.toHaveBeenCalled(); + }); + + it('sends contact custom attributes along with the contact update for campaigns', async () => { + const wrapper = mountView(); + wrapper.vm.onSubmit({ + fullName: 'John', + emailAddress: 'john@example.com', + phoneNumber: null, + activeCampaignId: 42, + contactCustomAttributes: { cpf: '123.456.789-09' }, + conversationCustomAttributes: {}, + }); + await flushPromises(); + + expect(updateContact).toHaveBeenCalledWith(expect.anything(), { + user: { + email: 'john@example.com', + name: 'John', + phone_number: null, + custom_attributes: { cpf: '123.456.789-09' }, + }, + }); + expect(createConversation).not.toHaveBeenCalled(); + expect(setCustomAttributes).not.toHaveBeenCalled(); + }); +}); diff --git a/spec/controllers/api/v1/widget/conversations_controller_spec.rb b/spec/controllers/api/v1/widget/conversations_controller_spec.rb index 6966c87ea..56bb01282 100644 --- a/spec/controllers/api/v1/widget/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/widget/conversations_controller_spec.rb @@ -140,6 +140,51 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do 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)