diff --git a/app/services/crm/base_processor_service.rb b/app/services/crm/base_processor_service.rb index 305a09014..f7e4aece1 100644 --- a/app/services/crm/base_processor_service.rb +++ b/app/services/crm/base_processor_service.rb @@ -78,6 +78,14 @@ class Crm::BaseProcessorService contact.save! end + def clear_external_id(contact) + return if contact.additional_attributes.blank? + return if contact.additional_attributes['external'].blank? + + contact.additional_attributes['external'].delete("#{crm_name}_id") + contact.save! + end + def store_conversation_metadata(conversation, metadata) # Initialize additional_attributes if it's nil conversation.additional_attributes = {} if conversation.additional_attributes.nil? diff --git a/app/services/crm/leadsquared/processor_service.rb b/app/services/crm/leadsquared/processor_service.rb index 9ffa3d12c..e8e30cdd4 100644 --- a/app/services/crm/leadsquared/processor_service.rb +++ b/app/services/crm/leadsquared/processor_service.rb @@ -64,7 +64,7 @@ class Crm::Leadsquared::ProcessorService < Crm::BaseProcessorService # may not be marked as unique, same with the phone number field # So we just use the update API if we already have a lead ID if lead_id.present? - @lead_client.update_lead(lead_data, lead_id) + with_stale_lead_recovery(contact, lead_id) { |id| @lead_client.update_lead(lead_data, id) } else new_lead_id = @lead_client.create_or_update_lead(lead_data) store_external_id(contact, new_lead_id) @@ -82,7 +82,9 @@ class Crm::Leadsquared::ProcessorService < Crm::BaseProcessorService return if lead_id.blank? activity_code = get_activity_code(activity_code_key) - activity_id = @activity_client.post_activity(lead_id, activity_code, activity_note) + activity_id = with_stale_lead_recovery(conversation.contact, lead_id) do |id| + @activity_client.post_activity(id, activity_code, activity_note) + end return if activity_id.blank? metadata = {} @@ -94,6 +96,31 @@ class Crm::Leadsquared::ProcessorService < Crm::BaseProcessorService log_activity_error(e, activity_type, conversation) end + # The cached lead id can become stale when the lead is deleted/merged in LeadSquared, + # making LeadSquared reject the call with "Lead not found". When that happens, clear the + # stored id, re-resolve the contact to a fresh lead, and run the operation again once. + def with_stale_lead_recovery(contact, lead_id) + yield(lead_id) + rescue Crm::Leadsquared::Api::BaseClient::ApiError => e + raise unless lead_not_found_error?(e) + + Rails.logger.warn("LeadSquared stale lead #{lead_id} for contact ##{contact.id}, clearing and retrying") + clear_external_id(contact) + fresh_lead_id = get_lead_id(contact) + raise if fresh_lead_id.blank? || fresh_lead_id == lead_id + + yield(fresh_lead_id) + end + + def lead_not_found_error?(error) + return false if error.response.blank? + + parsed = error.response.parsed_response + parsed.is_a?(Hash) && parsed['ExceptionType'] == 'MXInvalidEntityReferenceException' + rescue StandardError + false + end + def log_activity_error(error, activity_type, conversation, payload: nil) ChatwootExceptionTracker.new(error, account: @account).capture_exception context = "account_id=#{conversation.account_id}, conversation_display_id=#{conversation.display_id}" @@ -116,7 +143,7 @@ class Crm::Leadsquared::ProcessorService < Crm::BaseProcessorService unless identifiable_contact?(contact) Rails.logger.info("Contact not identifiable. Skipping activity for ##{contact.id}") - nil + return nil end lead_id = @lead_finder.find_or_create(contact) diff --git a/spec/services/crm/leadsquared/processor_service_spec.rb b/spec/services/crm/leadsquared/processor_service_spec.rb index 7b99721c5..ea1a3661f 100644 --- a/spec/services/crm/leadsquared/processor_service_spec.rb +++ b/spec/services/crm/leadsquared/processor_service_spec.rb @@ -82,6 +82,36 @@ RSpec.describe Crm::Leadsquared::ProcessorService do end end + context 'when the existing lead no longer exists' do + let(:error_response) do + instance_double(HTTParty::Response, blank?: false, parsed_response: { 'ExceptionType' => 'MXInvalidEntityReferenceException' }) + end + let(:lead_not_found_error) do + Crm::Leadsquared::Api::BaseClient::ApiError.new('Lead not found', 500, error_response) + end + + before do + contact.update!(additional_attributes: { 'external' => { 'leadsquared_id' => 'stale_lead_id' } }) + + allow(lead_client).to receive(:update_lead) + .with(any_args, 'stale_lead_id') + .and_raise(lead_not_found_error) + allow(lead_client).to receive(:update_lead) + .with(any_args, 'fresh_lead_id') + .and_return(nil) + allow(lead_finder).to receive(:find_or_create) + .with(contact) + .and_return('fresh_lead_id') + end + + it 'clears the stale id and re-resolves the lead' do + service.handle_contact(contact) + + expect(lead_finder).to have_received(:find_or_create).with(contact) + expect(contact.reload.additional_attributes['external']['leadsquared_id']).to eq('fresh_lead_id') + end + end + context 'when API call raises an error' do before do allow(lead_client).to receive(:create_or_update_lead) @@ -160,6 +190,63 @@ RSpec.describe Crm::Leadsquared::ProcessorService do expect(Rails.logger).to have_received(:error).with(/LeadSquared conversation activity failed/) end end + + context 'when post_activity fails because the lead no longer exists' do + let(:error_response) do + instance_double(HTTParty::Response, blank?: false, parsed_response: { 'ExceptionType' => 'MXInvalidEntityReferenceException' }) + end + let(:lead_not_found_error) do + Crm::Leadsquared::Api::BaseClient::ApiError.new('Lead not found', 500, error_response) + end + + before do + contact.update!(additional_attributes: { 'external' => { 'leadsquared_id' => 'stale_lead_id' } }) + + allow(lead_finder).to receive(:find_or_create) + .with(contact) + .and_return('stale_lead_id', 'fresh_lead_id') + + allow(activity_client).to receive(:post_activity) + .with('stale_lead_id', 1001, activity_note) + .and_raise(lead_not_found_error) + allow(activity_client).to receive(:post_activity) + .with('fresh_lead_id', 1001, activity_note) + .and_return('healed_activity_id') + end + + it 'clears the stale id, re-resolves the lead, and retries the activity once' do + service.handle_conversation_created(conversation) + + expect(activity_client).to have_received(:post_activity).with('fresh_lead_id', 1001, activity_note) + expect(contact.reload.additional_attributes['external']['leadsquared_id']).to eq('fresh_lead_id') + expect(conversation.reload.additional_attributes['leadsquared']['created_activity_id']).to eq('healed_activity_id') + end + end + + context 'when post_activity fails with a non-recoverable error' do + let(:error_response) do + instance_double(HTTParty::Response, blank?: false, parsed_response: { 'ExceptionType' => 'MXSomeOtherException' }) + end + let(:other_error) do + Crm::Leadsquared::Api::BaseClient::ApiError.new('boom', 500, error_response) + end + + before do + allow(lead_finder).to receive(:find_or_create) + .with(contact) + .and_return('test_lead_id') + + allow(activity_client).to receive(:post_activity).and_raise(other_error) + allow(Rails.logger).to receive(:error) + end + + it 'logs once and does not retry' do + service.handle_conversation_created(conversation) + + expect(activity_client).to have_received(:post_activity).once + expect(Rails.logger).to have_received(:error).with(/LeadSquared conversation activity failed/) + end + end end context 'when conversation activities are disabled' do