fix: recover from stale LeadSquared lead ids on activity sync (#14818)
LeadSquared sync now recovers automatically when a contact's cached lead has been deleted or merged on the LeadSquared side. Previously the stale lead id was never cleared, so every new conversation or contact update for that contact failed with "Lead not found" (`MXInvalidEntityReferenceException`) indefinitely. ## What changed - Activity sync: on a "Lead not found" error while posting a conversation/transcript activity, clear the cached `leadsquared_id`, re-resolve the contact to a fresh lead, and retry the activity once (guarded against loops and duplicate leads). - Contact sync: on the same error while updating an existing lead, clear the cached id and create a fresh lead instead. - Fix `get_lead_id` to actually return early for unidentifiable contacts (the guard previously fell through). ## How to reproduce 1. For a LeadSquared-enabled account, point a contact's cached lead id at a lead that no longer exists in LeadSquared. 2. Update the contact, or create/resolve a conversation for it. 3. Before: the sync fails repeatedly with "Lead not found" and never self-corrects. After: the stale id is cleared, a fresh lead is resolved/created, and subsequent syncs reuse the healed id. --------- Co-authored-by: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com>
This commit is contained in:
co-authored by
Tanmay Deep Sharma
parent
7522457740
commit
299bc6c0a4
@@ -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?
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user