fix: phone number handling in leadsquared (#11527)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>

app/models/message.rb

@@ -343,6 +343,7 @@ class Message < ApplicationRecord
   end
 
   def execute_message_template_hooks
+    Rails.logger.info("[Message][#{id}] Executing message template hooks")
     ::MessageTemplates::HookExecutionService.new(message: self).perform
   end
 

app/services/crm/leadsquared/lead_finder_service.rb

@@ -31,7 +31,11 @@ class Crm::Leadsquared::LeadFinderService
   def find_by_phone_number(contact)
     return if contact.phone_number.blank?
 
-    search_by_field(contact.phone_number)
+    lead_data = Crm::Leadsquared::Mappers::ContactMapper.map(contact)
+
+    return if lead_data.blank? || lead_data['Mobile'].nil?
+
+    search_by_field(lead_data['Mobile'])
   end
 
   def search_by_field(value)

app/services/crm/leadsquared/mappers/contact_mapper.rb

@@ -20,11 +20,29 @@ class Crm::Leadsquared::Mappers::ContactMapper
       'FirstName' => contact.name.presence,
       'LastName' => contact.last_name.presence,
       'EmailAddress' => contact.email.presence,
-      'Mobile' => contact.phone_number.presence,
+      'Mobile' => formatted_phone_number,
       'Source' => brand_name
     }.compact
   end
 
+  def formatted_phone_number
+    # it seems like leadsquared needs a different phone number format
+    # it's not documented anywhere, so don't bother trying to look up online
+    # After some trial and error, I figured out the format, its +<country_code>-<national_number>
+    return nil if contact.phone_number.blank?
+
+    parsed = TelephoneNumber.parse(contact.phone_number)
+    return contact.phone_number unless parsed.valid?
+
+    country_code = parsed.country.country_code
+    e164 = parsed.e164_number
+    e164 = e164.sub(/^\+/, '')
+
+    national_number = e164.sub(/^#{Regexp.escape(country_code)}/, '')
+
+    "+#{country_code}-#{national_number}"
+  end
+
   def brand_name
     ::GlobalConfig.get('BRAND_NAME')['BRAND_NAME'] || 'Chatwoot'
   end

app/services/crm/leadsquared/mappers/conversation_mapper.rb

@@ -6,17 +6,18 @@ class Crm::Leadsquared::Mappers::ConversationMapper
   # so this limits it
   ACTIVITY_NOTE_MAX_SIZE = 1800
 
-  def self.map_conversation_activity(conversation)
-    new(conversation).conversation_activity
+  def self.map_conversation_activity(hook, conversation)
+    new(hook, conversation).conversation_activity
   end
 
-  def self.map_transcript_activity(conversation, messages = nil)
-    new(conversation, messages).transcript_activity
+  def self.map_transcript_activity(hook, conversation)
+    new(hook, conversation).transcript_activity
   end
 
-  def initialize(conversation, messages = nil)
+  def initialize(hook, conversation)
+    @hook = hook
+    @timezone = Time.find_zone(hook.settings['timezone']) || Time.zone
     @conversation = conversation
-    @messages = messages
   end
 
   def conversation_activity
@@ -41,14 +42,14 @@ class Crm::Leadsquared::Mappers::ConversationMapper
 
   private
 
-  attr_reader :conversation, :messages
+  attr_reader :conversation
 
   def formatted_creation_time
-    conversation.created_at.strftime('%Y-%m-%d %H:%M:%S')
+    conversation.created_at.in_time_zone(@timezone).strftime('%Y-%m-%d %H:%M:%S')
   end
 
   def transcript_messages
-    @transcript_messages ||= messages || conversation.messages.chat.select(&:conversation_transcriptable?)
+    @transcript_messages ||= conversation.messages.chat.select(&:conversation_transcriptable?)
   end
 
   def format_messages
@@ -77,8 +78,7 @@ class Crm::Leadsquared::Mappers::ConversationMapper
   end
 
   def message_time(message)
-    # TODO: Figure out what timezone to send the time in
-    message.created_at.strftime('%Y-%m-%d %H:%M')
+    message.created_at.in_time_zone(@timezone).strftime('%Y-%m-%d %H:%M')
   end
 
   def sender_name(message)

app/services/crm/leadsquared/processor_service.rb

@@ -37,7 +37,7 @@ class Crm::Leadsquared::ProcessorService < Crm::BaseProcessorService
       activity_type: 'conversation',
       activity_code_key: 'conversation_activity_code',
       metadata_key: 'created_activity_id',
-      activity_note: Crm::Leadsquared::Mappers::ConversationMapper.map_conversation_activity(conversation)
+      activity_note: Crm::Leadsquared::Mappers::ConversationMapper.map_conversation_activity(@hook, conversation)
     )
   end
 
@@ -50,7 +50,7 @@ class Crm::Leadsquared::ProcessorService < Crm::BaseProcessorService
       activity_type: 'transcript',
       activity_code_key: 'transcript_activity_code',
       metadata_key: 'transcript_activity_id',
-      activity_note: Crm::Leadsquared::Mappers::ConversationMapper.map_transcript_activity(conversation)
+      activity_note: Crm::Leadsquared::Mappers::ConversationMapper.map_transcript_activity(@hook, conversation)
     )
   end
 

app/services/crm/leadsquared/setup_service.rb

@@ -25,11 +25,12 @@ class Crm::Leadsquared::SetupService
     response = @client.get('Authentication.svc/UserByAccessKey.Get')
     endpoint_host = response['LSQCommonServiceURLs']['api']
     app_host = response['LSQCommonServiceURLs']['app']
+    timezone = response['TimeZone']
 
     endpoint_url = "https://#{endpoint_host}/v2/"
     app_url = "https://#{app_host}/"
 
-    update_hook_settings({ :endpoint_url => endpoint_url, :app_url => app_url })
+    update_hook_settings({ :endpoint_url => endpoint_url, :app_url => app_url, :timezone => timezone })
 
     # replace the clients
     @client = Crm::Leadsquared::Api::BaseClient.new(@access_key, @secret_key, endpoint_url)

app/services/message_templates/hook_execution_service.rb

@@ -2,9 +2,17 @@ class MessageTemplates::HookExecutionService
   pattr_initialize [:message!]
 
   def perform
-    return if conversation.campaign.present?
-    return if conversation.last_incoming_message.blank?
+    if conversation.campaign.present?
+      Rails.logger.debug { "[OutOfOffice][#{conversation.id}] Not triggering templates because conversation has a campaign" }
+      return
+    end
 
+    if conversation.last_incoming_message.blank?
+      Rails.logger.debug { "[OutOfOffice][#{conversation.id}] Not triggering templates because there is no incoming message" }
+      return
+    end
+
+    Rails.logger.info "[OutOfOffice][#{conversation.id}] Triggering templates for conversation ##{conversation.id}"
     trigger_templates
   end
 
@@ -22,14 +30,41 @@ class MessageTemplates::HookExecutionService
 
   def should_send_out_of_office_message?
     # should not send if its a tweet message
-    return false if conversation.tweet?
+    if conversation.tweet?
+      Rails.logger.debug { "[OutOfOffice][#{conversation.id}] Not sending out-of-office message because it's a tweet conversation" }
+      return false
+    end
+
     # should not send for outbound messages
-    return false unless message.incoming?
+    unless message.incoming?
+      Rails.logger.debug { "[OutOfOffice][#{conversation.id}] Not sending out-of-office message because the message is outgoing" }
+      return false
+    end
+
     # prevents sending out-of-office message if an agent has sent a message in last 5 minutes
     # ensures better UX by not interrupting active conversations at the end of business hours
-    return false if conversation.messages.outgoing.exists?(['created_at > ?', 5.minutes.ago])
+    if conversation.messages.outgoing.where(private: false).exists?(['created_at > ?', 5.minutes.ago])
+      Rails.logger.debug { "[OutOfOffice][#{conversation.id}] Not sending out-of-office message because an agent responded in the last 5 minutes" }
+      return false
+    end
 
-    inbox.out_of_office? && conversation.messages.today.template.empty? && inbox.out_of_office_message.present?
+    can_send = inbox.out_of_office? && conversation.messages.today.template.empty? && inbox.out_of_office_message.present?
+
+    if can_send
+      Rails.logger.info "[OutOfOffice][#{conversation.id}] Sending out-of-office message for conversation ##{conversation.id}"
+    else
+      reasons = []
+      reasons << 'inbox not in out-of-office mode' unless inbox.out_of_office?
+      reasons << 'conversation already has a template message today' unless conversation.messages.today.template.empty?
+      reasons << 'inbox has no out-of-office message configured' unless inbox.out_of_office_message.present?
+      Rails.logger.debug { "[OutOfOffice][#{conversation.id}] Not sending out-of-office message because: #{reasons.join(', ')}" }
+    end
+
+    can_send
+  rescue StandardError => e
+    Rails.logger.error("[OutOfOffice][#{conversation.id}] Error triggering out of office message: #{e.message}")
+    ChatwootExceptionTracker.new(e, account: conversation.account).capture_exception
+    false
   end
 
   def first_message_from_contact?

app/services/message_templates/template/out_of_office.rb

@@ -2,10 +2,12 @@ class MessageTemplates::Template::OutOfOffice
   pattr_initialize [:conversation!]
 
   def perform
+    Rails.logger.info("[OutOfOffice][#{conversation.id}] Triggering out of office message")
     ActiveRecord::Base.transaction do
       conversation.messages.create!(out_of_office_message_params)
     end
   rescue StandardError => e
+    Rails.logger.error("[OutOfOffice][#{conversation.id}] Error triggering out of office message: #{e.message}")
     ChatwootExceptionTracker.new(e, account: conversation.account).capture_exception
     true
   end

config/integration/apps.yml

@@ -205,6 +205,7 @@ leadsquared:
           'secret_key': { 'type': 'string' },
           'endpoint_url': { 'type': 'string' },
           'app_url': { 'type': 'string' },
+          'timezone': { 'type': 'string' },
           'enable_conversation_activity': { 'type': 'boolean' },
           'enable_transcript_activity': { 'type': 'boolean' },
           'conversation_activity_score': { 'type': 'string' },

spec/services/crm/leadsquared/mappers/contact_mapper_spec.rb

@@ -16,6 +16,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ContactMapper do
           name: 'John',
           last_name: 'Doe',
           email: 'john@example.com',
+          # the phone number is intentionally wrong
           phone_number: '+1234567890'
         )
 
@@ -29,6 +30,19 @@ RSpec.describe Crm::Leadsquared::Mappers::ContactMapper do
           'Source' => 'Test Brand'
         )
       end
+
+      it 'represents the phone number correctly' do
+        contact.update!(
+          name: 'John',
+          last_name: 'Doe',
+          email: 'john@example.com',
+          phone_number: '+917507684392'
+        )
+
+        mapped_data = described_class.map(contact)
+
+        expect(mapped_data).to include('Mobile' => '+91-7507684392')
+      end
     end
   end
 end

spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb

@@ -6,15 +6,39 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do
   let(:conversation) { create(:conversation, account: account, inbox: inbox) }
   let(:user) { create(:user, name: 'John Doe') }
   let(:contact) { create(:contact, name: 'Jane Smith') }
+  let(:hook) do
+    create(:integrations_hook, :leadsquared, account: account, settings: {
+             'access_key' => 'test_access_key',
+             'secret_key' => 'test_secret_key',
+             'endpoint_url' => 'https://api.leadsquared.com/v2',
+             'timezone' => 'UTC'
+           })
+  end
+  let(:hook_with_pst) do
+    create(:integrations_hook, :leadsquared, account: account, settings: {
+             'access_key' => 'test_access_key',
+             'secret_key' => 'test_secret_key',
+             'endpoint_url' => 'https://api.leadsquared.com/v2',
+             'timezone' => 'America/Los_Angeles'
+           })
+  end
+  let(:hook_without_timezone) do
+    create(:integrations_hook, :leadsquared, account: account, settings: {
+             'access_key' => 'test_access_key',
+             'secret_key' => 'test_secret_key',
+             'endpoint_url' => 'https://api.leadsquared.com/v2'
+           })
+  end
 
   before do
+    account.enable_features('crm_integration')
     allow(GlobalConfig).to receive(:get).with('BRAND_NAME').and_return({ 'BRAND_NAME' => 'TestBrand' })
   end
 
   describe '.map_conversation_activity' do
-    it 'generates conversation activity note' do
-      travel_to(Time.zone.parse('2024-01-01 10:00:00')) do
-        result = described_class.map_conversation_activity(conversation)
+    it 'generates conversation activity note with UTC timezone' do
+      travel_to(Time.zone.parse('2024-01-01 10:00:00 UTC')) do
+        result = described_class.map_conversation_activity(hook, conversation)
 
         expect(result).to include('New conversation started on TestBrand')
         expect(result).to include('Channel: Test Inbox')
@@ -23,12 +47,29 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do
         expect(result).to include('View in TestBrand: http://')
       end
     end
+
+    it 'formats time according to hook timezone setting' do
+      travel_to(Time.zone.parse('2024-01-01 18:00:00 UTC')) do
+        result = described_class.map_conversation_activity(hook_with_pst, conversation)
+
+        # PST is UTC-8, so 18:00 UTC becomes 10:00:00 PST
+        expect(result).to include('Created: 2024-01-01 10:00:00')
+      end
+    end
+
+    it 'falls back to system timezone when hook has no timezone setting' do
+      travel_to(Time.zone.parse('2024-01-01 10:00:00')) do
+        result = described_class.map_conversation_activity(hook_without_timezone, conversation)
+
+        expect(result).to include('Created: 2024-01-01 10:00:00')
+      end
+    end
   end
 
   describe '.map_transcript_activity' do
     context 'when conversation has no messages' do
       it 'returns no messages message' do
-        result = described_class.map_transcript_activity(conversation)
+        result = described_class.map_transcript_activity(hook, conversation)
         expect(result).to eq('No messages in conversation')
       end
     end
@@ -68,7 +109,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do
       end
 
       it 'generates transcript with messages in reverse chronological order' do
-        result = described_class.map_transcript_activity(conversation)
+        result = described_class.map_transcript_activity(hook, conversation)
 
         expect(result).to include('Conversation Transcript from TestBrand')
         expect(result).to include('Channel: Test Inbox')
@@ -83,6 +124,22 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do
         expect(message_positions['[2024-01-01 10:01] Jane Smith: Hi there']).to be < message_positions['[2024-01-01 10:00] John Doe: Hello']
       end
 
+      it 'formats message times according to hook timezone setting' do
+        travel_to(Time.zone.parse('2024-01-01 18:00:00 UTC')) do
+          create(:message,
+                 conversation: conversation,
+                 sender: user,
+                 content: 'Test message',
+                 message_type: :outgoing,
+                 created_at: Time.zone.parse('2024-01-01 18:00:00 UTC'))
+
+          result = described_class.map_transcript_activity(hook_with_pst, conversation)
+
+          # PST is UTC-8, so 18:00 UTC becomes 10:00 PST
+          expect(result).to include('[2024-01-01 10:00] John Doe: Test message')
+        end
+      end
+
       context 'when message has attachments' do
         let(:message_with_attachment) do
           create(:message, :with_attachment,
@@ -96,7 +153,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do
         before { message_with_attachment }
 
         it 'includes attachment information' do
-          result = described_class.map_transcript_activity(conversation)
+          result = described_class.map_transcript_activity(hook, conversation)
 
           expect(result).to include('See attachment')
           expect(result).to include('[Attachment: image]')
@@ -116,7 +173,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do
         before { empty_message }
 
         it 'shows no content placeholder' do
-          result = described_class.map_transcript_activity(conversation)
+          result = described_class.map_transcript_activity(hook, conversation)
           expect(result).to include('[No content]')
         end
       end
@@ -134,25 +191,12 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do
         before { unnamed_sender_message }
 
         it 'uses sender type and id' do
-          result = described_class.map_transcript_activity(conversation)
+          result = described_class.map_transcript_activity(hook, conversation)
           expect(result).to include("User #{unnamed_sender_message.sender_id}")
         end
       end
     end
 
-    context 'when specific messages are provided' do
-      let(:message1) { create(:message, conversation: conversation, content: 'Message 1', message_type: :outgoing) }
-      let(:message2) { create(:message, conversation: conversation, content: 'Message 2', message_type: :outgoing) }
-      let(:specific_messages) { [message1] }
-
-      it 'only includes provided messages' do
-        result = described_class.map_transcript_activity(conversation, specific_messages)
-
-        expect(result).to include('Message 1')
-        expect(result).not_to include('Message 2')
-      end
-    end
-
     context 'when messages exceed the ACTIVITY_NOTE_MAX_SIZE' do
       it 'truncates messages to stay within the character limit' do
         # Create a large number of messages with reasonably sized content
@@ -169,7 +213,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do
                              created_at: Time.zone.parse("2024-01-01 #{10 + i}:00:00"))
         end
 
-        result = described_class.map_transcript_activity(conversation, messages)
+        result = described_class.map_transcript_activity(hook, conversation)
 
         # Verify latest message is included (message 14)
         expect(result).to include("[2024-01-02 00:00] John Doe: #{long_message_content} 14")
@@ -189,13 +233,13 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do
       it 'respects the ACTIVITY_NOTE_MAX_SIZE constant' do
         # Create a single message that would exceed the limit by itself
         giant_content = 'A' * 2000
-        message = create(:message,
-                         conversation: conversation,
-                         sender: user,
-                         content: giant_content,
-                         message_type: :outgoing)
+        create(:message,
+               conversation: conversation,
+               sender: user,
+               content: giant_content,
+               message_type: :outgoing)
 
-        result = described_class.map_transcript_activity(conversation, [message])
+        result = described_class.map_transcript_activity(hook, conversation)
 
         # Extract just the formatted messages part
         id = conversation.display_id

spec/services/crm/leadsquared/processor_service_spec.rb

@@ -116,7 +116,7 @@ RSpec.describe Crm::Leadsquared::ProcessorService do
 
     before do
       allow(Crm::Leadsquared::Mappers::ConversationMapper).to receive(:map_conversation_activity)
-        .with(conversation)
+        .with(hook, conversation)
         .and_return(activity_note)
     end
 
@@ -180,7 +180,7 @@ RSpec.describe Crm::Leadsquared::ProcessorService do
 
     before do
       allow(Crm::Leadsquared::Mappers::ConversationMapper).to receive(:map_transcript_activity)
-        .with(conversation)
+        .with(hook, conversation)
         .and_return(activity_note)
     end
 

spec/services/crm/leadsquared/setup_service_spec.rb

@@ -8,6 +8,7 @@ RSpec.describe Crm::Leadsquared::SetupService do
   let(:activity_client) { instance_double(Crm::Leadsquared::Api::ActivityClient) }
   let(:endpoint_response) do
     {
+      'TimeZone' => 'Asia/Kolkata',
       'LSQCommonServiceURLs' => {
         'api' => 'api-in.leadsquared.com',
         'app' => 'app.leadsquared.com'
@@ -45,6 +46,7 @@ RSpec.describe Crm::Leadsquared::SetupService do
           updated_settings = hook.reload.settings
           expect(updated_settings['endpoint_url']).to eq('https://api-in.leadsquared.com/v2/')
           expect(updated_settings['app_url']).to eq('https://app.leadsquared.com/')
+          expect(updated_settings['timezone']).to eq('Asia/Kolkata')
           expect(updated_settings['conversation_activity_code']).to eq(1001)
           expect(updated_settings['transcript_activity_code']).to eq(1002)
         end
@@ -71,6 +73,7 @@ RSpec.describe Crm::Leadsquared::SetupService do
           updated_settings = hook.reload.settings
           expect(updated_settings['endpoint_url']).to eq('https://api-in.leadsquared.com/v2/')
           expect(updated_settings['app_url']).to eq('https://app.leadsquared.com/')
+          expect(updated_settings['timezone']).to eq('Asia/Kolkata')
           expect(updated_settings['conversation_activity_code']).to eq(1001)
           expect(updated_settings['transcript_activity_code']).to eq(1002)
         end