chore: Enhance contact merge action for identified users (#4886)

- Discard conflicting keys 
- Do not merge if there is already an identified contact

Co-authored-by: Pranav Raj S <pranav@chatwoot.com>

app/actions/contact_identify_action.rb

@@ -1,7 +1,16 @@
+# retain_original_contact_name: false / true
+# In case of setUser we want to update the name of the identified contact,
+# which is the default behaviour
+#
+# But, In case of contact merge during prechat form contact update.
+# We don't want to update the name of the identified original contact.
+
 class ContactIdentifyAction
-  pattr_initialize [:contact!, :params!]
+  pattr_initialize [:contact!, :params!, { retain_original_contact_name: false }]
 
   def perform
+    @attributes_to_update = [:identifier, :name, :email, :phone_number]
+
     ActiveRecord::Base.transaction do
       merge_if_existing_identified_contact
       merge_if_existing_email_contact
@@ -18,49 +27,88 @@ def account
   end
 
   def merge_if_existing_identified_contact
-    @contact = merge_contact(existing_identified_contact, @contact) if merge_contacts?(existing_identified_contact, @contact)
+    return unless merge_contacts?(existing_identified_contact, :identifier)
+
+    process_contact_merge(existing_identified_contact)
   end
 
   def merge_if_existing_email_contact
-    @contact = merge_contact(existing_email_contact, @contact) if merge_contacts?(existing_email_contact, @contact)
+    return unless merge_contacts?(existing_email_contact, :email)
+
+    process_contact_merge(existing_email_contact)
   end
 
   def merge_if_existing_phone_number_contact
-    @contact = merge_contact(existing_phone_number_contact, @contact) if merge_contacts?(existing_phone_number_contact, @contact)
+    return unless merge_contacts?(existing_phone_number_contact, :phone_number)
+    return unless mergable_phone_contact?
+
+    process_contact_merge(existing_phone_number_contact)
+  end
+
+  def process_contact_merge(mergee_contact)
+    @contact = merge_contact(mergee_contact, @contact)
+    @attributes_to_update.delete(:name) if retain_original_contact_name
   end
 
   def existing_identified_contact
     return if params[:identifier].blank?
 
-    @existing_identified_contact ||= Contact.where(account_id: account.id).find_by(identifier: params[:identifier])
+    @existing_identified_contact ||= account.contacts.find_by(identifier: params[:identifier])
   end
 
   def existing_email_contact
     return if params[:email].blank?
 
-    @existing_email_contact ||= Contact.where(account_id: account.id).find_by(email: params[:email])
+    @existing_email_contact ||= account.contacts.find_by(email: params[:email])
   end
 
   def existing_phone_number_contact
     return if params[:phone_number].blank?
 
-    @existing_phone_number_contact ||= Contact.where(account_id: account.id).find_by(phone_number: params[:phone_number])
+    @existing_phone_number_contact ||= account.contacts.find_by(phone_number: params[:phone_number])
   end
 
-  def merge_contacts?(existing_contact, _contact)
-    existing_contact && existing_contact.id != @contact.id
+  def merge_contacts?(existing_contact, key)
+    return if existing_contact.blank?
+
+    return true if params[:identifier].blank?
+
+    # we want to prevent merging contacts with different identifiers
+    if existing_contact.identifier.present? && existing_contact.identifier != params[:identifier]
+      # we will remove attribute from update list
+      @attributes_to_update.delete(key)
+      return false
+    end
+
+    true
+  end
+
+  # case: contact 1: email: 1@test.com, phone: 123456789
+  # params: email: 2@test.com, phone: 123456789
+  # we don't want to overwrite 1@test.com since email parameter takes higer priority
+  def mergable_phone_contact?
+    return true if params[:email].blank?
+
+    if existing_phone_number_contact.email.present? && existing_phone_number_contact.email != params[:email]
+      @attributes_to_update.delete(:phone_number)
+      return false
+    end
+    true
   end
 
   def update_contact
+    params_to_update = params.slice(*@attributes_to_update).reject do |_k, v|
+      v.blank?
+    end.merge({ custom_attributes: custom_attributes, additional_attributes: additional_attributes })
     # blank identifier or email will throw unique index error
     # TODO: replace reject { |_k, v| v.blank? } with compact_blank when rails is upgraded
-    @contact.update!(params.slice(:name, :email, :identifier, :phone_number).reject do |_k, v|
-                       v.blank?
-                     end.merge({ custom_attributes: custom_attributes, additional_attributes: additional_attributes }))
+    @contact.update!(params_to_update)
     ContactAvatarJob.perform_later(@contact, params[:avatar_url]) if params[:avatar_url].present?
   end
 
   def merge_contact(base_contact, merge_contact)
+    return base_contact if base_contact.id == merge_contact.id
+
     ContactMergeAction.new(
       account: account,
       base_contact: base_contact,
@@ -69,14 +117,14 @@ def merge_contact(base_contact, merge_contact)
   end
 
   def custom_attributes
-    params[:custom_attributes] ? @contact.custom_attributes.deep_merge(params[:custom_attributes].stringify_keys) : @contact.custom_attributes
+    return @contact.custom_attributes if params[:custom_attributes].blank?
+
+    (@contact.custom_attributes || {}).deep_merge(params[:custom_attributes].stringify_keys)
   end
 
   def additional_attributes
-    if params[:additional_attributes]
-      @contact.additional_attributes.deep_merge(params[:additional_attributes].stringify_keys)
-    else
-      @contact.additional_attributes
-    end
+    return @contact.additional_attributes if params[:additional_attributes].blank?
+
+    (@contact.additional_attributes || {}).deep_merge(params[:additional_attributes].stringify_keys)
   end
 end

app/controllers/api/v1/widget/base_controller.rb

@@ -44,38 +44,6 @@ def conversation_params
     }
   end
 
-  def update_contact(email)
-    contact_with_email = @current_account.contacts.find_by(email: email)
-    if contact_with_email
-      @contact = ::ContactMergeAction.new(
-        account: @current_account,
-        base_contact: contact_with_email,
-        mergee_contact: @contact
-      ).perform
-    else
-      @contact.update!(email: email)
-      update_contact_name
-    end
-  end
-
-  def update_contact_phone_number(phone_number)
-    contact_with_phone_number = @current_account.contacts.find_by(phone_number: phone_number)
-    if contact_with_phone_number
-      @contact = ::ContactMergeAction.new(
-        account: @current_account,
-        base_contact: contact_with_phone_number,
-        mergee_contact: @contact
-      ).perform
-    else
-      @contact.update!(phone_number: phone_number)
-      update_contact_name
-    end
-  end
-
-  def update_contact_name
-    @contact.update!(name: contact_name) if contact_name.present?
-  end
-
   def contact_email
     permitted_params.dig(:contact, :email)&.downcase
   end

app/controllers/api/v1/widget/contacts_controller.rb

@@ -1,14 +1,27 @@
 class Api::V1::Widget::ContactsController < Api::V1::Widget::BaseController
-  before_action :process_hmac, only: [:update]
+  include WidgetHelper
+
+  before_action :validate_hmac, only: [:set_user]
 
   def show; end
 
   def update
-    contact_identify_action = ContactIdentifyAction.new(
-      contact: @contact,
-      params: permitted_params.to_h.deep_symbolize_keys
-    )
-    @contact = contact_identify_action.perform
+    identify_contact(@contact)
+  end
+
+  def set_user
+    contact = nil
+
+    if a_different_contact?
+      @contact_inbox, @widget_auth_token = build_contact_inbox_with_token(@web_widget)
+      contact = @contact_inbox.contact
+    else
+      contact = @contact
+    end
+
+    @contact_inbox.update(hmac_verified: true) if should_verify_hmac? && valid_hmac?
+
+    identify_contact(contact)
   end
 
   # TODO : clean up this with proper routes delete contacts/custom_attributes
@@ -20,12 +33,22 @@ def destroy_custom_attributes
 
   private
 
-  def process_hmac
+  def identify_contact(contact)
+    contact_identify_action = ContactIdentifyAction.new(
+      contact: contact,
+      params: permitted_params.to_h.deep_symbolize_keys
+    )
+    @contact = contact_identify_action.perform
+  end
+
+  def a_different_contact?
+    @contact.identifier.present? && @contact.identifier != permitted_params[:identifier]
+  end
+
+  def validate_hmac
     return unless should_verify_hmac?
 
     render json: { error: 'HMAC failed: Invalid Identifier Hash Provided' }, status: :unauthorized unless valid_hmac?
-
-    @contact_inbox.update(hmac_verified: true)
   end
 
   def should_verify_hmac?

app/controllers/api/v1/widget/conversations_controller.rb

@@ -14,8 +14,11 @@ def create
   end
 
   def process_update_contact
-    update_contact(contact_email) if @contact.email.blank? && contact_email.present?
-    update_contact_phone_number(contact_phone_number) if @contact.phone_number.blank? && contact_phone_number.present?
+    @contact = ContactIdentifyAction.new(
+      contact: @contact,
+      params: { email: contact_email, phone_number: contact_phone_number, name: contact_name },
+      retain_original_contact_name: true
+    ).perform
   end
 
   def update_last_seen

app/controllers/api/v1/widget/messages_controller.rb

@@ -15,7 +15,10 @@ def create
   def update
     if @message.content_type == 'input_email'
       @message.update!(submitted_email: contact_email)
-      update_contact(contact_email)
+      ContactIdentifyAction.new(
+        contact: @contact,
+        params: { email: contact_email }
+      ).perform
     else
       @message.update!(message_update_params[:message])
     end

app/controllers/widgets_controller.rb

@@ -1,5 +1,7 @@
 # TODO : Delete this and associated spec once 'api/widget/config' end point is merged
 class WidgetsController < ActionController::Base
+  include WidgetHelper
+
   before_action :set_global_config
   before_action :set_web_widget
   before_action :set_token
@@ -40,11 +42,8 @@ def set_contact
   def build_contact
     return if @contact.present?
 
-    @contact_inbox = @web_widget.create_contact_inbox(additional_attributes)
+    @contact_inbox, @token = build_contact_inbox_with_token(@web_widget, additional_attributes)
     @contact = @contact_inbox.contact
-
-    payload = { source_id: @contact_inbox.source_id, inbox_id: @web_widget.inbox.id }
-    @token = ::Widget::TokenService.new(payload: payload).generate_token
   end
 
   def additional_attributes

app/helpers/widget_helper.rb

@@ -0,0 +1,9 @@
+module WidgetHelper
+  def build_contact_inbox_with_token(web_widget, additional_attributes = {})
+    contact_inbox = web_widget.create_contact_inbox(additional_attributes)
+    payload = { source_id: contact_inbox.source_id, inbox_id: web_widget.inbox.id }
+    token = ::Widget::TokenService.new(payload: payload).generate_token
+
+    [contact_inbox, token]
+  end
+end

app/javascript/sdk/IFrameHelper.js

@@ -33,6 +33,12 @@ import {
 import { isFlatWidgetStyle } from './settingsHelper';
 import { popoutChatWindow } from '../widget/helpers/popoutHelper';
 
+const updateAuthCookie = cookieContent =>
+  Cookies.set('cw_conversation', cookieContent, {
+    expires: 365,
+    sameSite: 'Lax',
+  });
+
 export const IFrameHelper = {
   getUrl({ baseUrl, websiteToken }) {
     return `${baseUrl}/widget?website_token=${websiteToken}`;
@@ -134,10 +140,7 @@ export const IFrameHelper = {
 
   events: {
     loaded: message => {
-      Cookies.set('cw_conversation', message.config.authToken, {
-        expires: 365,
-        sameSite: 'Lax',
-      });
+      updateAuthCookie(message.config.authToken);
       window.$chatwoot.hasLoaded = true;
       IFrameHelper.sendMessage('config-set', {
         locale: window.$chatwoot.locale,
@@ -178,6 +181,10 @@ export const IFrameHelper = {
       setBubbleText(window.$chatwoot.launcherTitle || message.label);
     },
 
+    setAuthCookie({ data: { widgetAuthToken } }) {
+      updateAuthCookie(widgetAuthToken);
+    },
+
     toggleBubble: state => {
       let bubbleState = {};
       if (state === 'open') {

app/javascript/widget/App.vue

@@ -83,12 +83,11 @@ export default {
     const { websiteToken, locale, widgetColor } = window.chatwootWebChannel;
     this.setLocale(locale);
     this.setWidgetColor(widgetColor);
+    setHeader(window.authToken);
     if (this.isIFrame) {
       this.registerListeners();
       this.sendLoadedEvent();
-      setHeader('X-Auth-Token', window.authToken);
     } else {
-      setHeader('X-Auth-Token', window.authToken);
       this.fetchOldConversations();
       this.fetchAvailableAgents(websiteToken);
       this.setLocale(getLocale(window.location.search));
@@ -253,7 +252,7 @@ export default {
         } else if (message.event === 'remove-label') {
           this.$store.dispatch('conversationLabels/destroy', message.label);
         } else if (message.event === 'set-user') {
-          this.$store.dispatch('contacts/update', message);
+          this.$store.dispatch('contacts/setUser', message);
         } else if (message.event === 'set-custom-attributes') {
           this.$store.dispatch(
             'contacts/setCustomAttributes',

app/javascript/widget/api/contacts.js

@@ -6,8 +6,11 @@ export default {
   get() {
     return API.get(buildUrl('widget/contact'));
   },
-  update(identifier, userObject) {
-    return API.patch(buildUrl('widget/contact'), {
+  update(userObject) {
+    return API.patch(buildUrl('widget/contact'), userObject);
+  },
+  setUser(identifier, userObject) {
+    return API.patch(buildUrl('widget/contact/set_user'), {
       identifier,
       ...userObject,
     });

app/javascript/widget/helpers/axios.js

@@ -6,7 +6,7 @@ export const API = axios.create({
   withCredentials: false,
 });
 
-export const setHeader = (key, value) => {
+export const setHeader = (value, key = 'X-Auth-Token') => {
   API.defaults.headers.common[key] = value;
 };
 

app/javascript/widget/helpers/utils.js

@@ -10,14 +10,16 @@ export const arrayToHashById = array =>
     return newMap;
   }, {});
 
+export const sendMessage = msg => {
+  window.parent.postMessage(
+    `chatwoot-widget:${JSON.stringify({ ...msg })}`,
+    '*'
+  );
+};
+
 export const IFrameHelper = {
   isIFrame: () => window.self !== window.top,
-  sendMessage: msg => {
-    window.parent.postMessage(
-      `chatwoot-widget:${JSON.stringify({ ...msg })}`,
-      '*'
-    );
-  },
+  sendMessage,
   isAValidEvent: e => {
     const isDataAString = typeof e.data === 'string';
     const isAValidWootEvent =