From 4c67ee35e632b6c288b326a3097e8b824793735b Mon Sep 17 00:00:00 2001 From: Fraser Speirs Date: Thu, 4 Jun 2026 15:35:33 +0100 Subject: [PATCH 1/4] Guard Salesforce::RoleSyncJob against missing parent records Defer the Contact_Editor_Affiliation__c write until both the parent Editor__c (school) and Contact (user) have been pushed to Salesforce, using the existing SalesforceRecordNotFound retry path. Co-authored-by: Cursor --- app/jobs/salesforce/role_sync_job.rb | 16 +++++++ spec/jobs/salesforce/role_sync_job_spec.rb | 52 ++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/app/jobs/salesforce/role_sync_job.rb b/app/jobs/salesforce/role_sync_job.rb index 2465ad9db..9663b4ac4 100644 --- a/app/jobs/salesforce/role_sync_job.rb +++ b/app/jobs/salesforce/role_sync_job.rb @@ -18,6 +18,15 @@ def perform(role_id:) return if role.student? + # The Contact_Editor_Affiliation__c row uses Salesforce external-ID lookups to resolve + # its parent Editor__c (school) and Contact (user). If either parent has not yet been + # pushed to Salesforce, Heroku Connect rejects the INSERT permanently with a + # "Foreign key external ID ... not found" error and the row is stuck FAILED in the + # mirror. Raising SalesforceRecordNotFound here defers the affiliation write via the + # SalesforceSyncJob retry_on, giving the parent records time to land in Salesforce. + ensure_parent_synced!(Salesforce::School, :editoruuid__c, role.school_id, 'Editor__c') + ensure_parent_synced!(Salesforce::Contact, :pi_accounts_unique_id__c, role.user_id, 'Contact') + sf_role = Salesforce::Role.find_or_initialize_by(affiliation_id__c: role_id) sf_role.attributes = sf_role_attributes(role:) @@ -32,6 +41,13 @@ def perform(role_id:) private + def ensure_parent_synced!(model, external_id_field, external_id, label) + return if model.where(external_id_field => external_id).where.not(sfid: nil).exists? + + raise SalesforceRecordNotFound, + "#{label} not yet synced for #{external_id_field}: #{external_id}" + end + def sf_role_attributes(role:) mapped_attributes(role:).to_h do |sf_field, value| value = truncate_value(sf_field:, value:) if value.is_a?(String) diff --git a/spec/jobs/salesforce/role_sync_job_spec.rb b/spec/jobs/salesforce/role_sync_job_spec.rb index 2f77bd98e..24be54bf5 100644 --- a/spec/jobs/salesforce/role_sync_job_spec.rb +++ b/spec/jobs/salesforce/role_sync_job_spec.rb @@ -6,6 +6,12 @@ subject(:perform_job) { described_class.perform_now(role_id: role.id) } let(:role) { create(:role) } + let!(:sf_school) do + create(:salesforce_school, editoruuid__c: role.school_id, sfid: SecureRandom.alphanumeric(18)) + end + let!(:sf_contact) do + create(:salesforce_contact, pi_accounts_unique_id__c: role.user_id, sfid: SecureRandom.alphanumeric(18)) + end around do |example| ClimateControl.modify(SALESFORCE_ENABLED: 'true') { example.run } @@ -53,6 +59,52 @@ end end + context 'when the parent Editor__c is not yet synced to Salesforce' do + before { sf_school.update!(sfid: nil) } + + it 'raises SalesforceRecordNotFound to defer the affiliation write' do + expect { perform_job } + .to raise_error(Salesforce::SalesforceSyncJob::SalesforceRecordNotFound, /Editor__c not yet synced/) + end + + it 'does not write the affiliation to the mirror' do + expect { perform_job }.to raise_error(Salesforce::SalesforceSyncJob::SalesforceRecordNotFound) + expect(Salesforce::Role.find_by(affiliation_id__c: role.id)).to be_nil + end + end + + context 'when there is no Salesforce::School row for the role school' do + before { sf_school.destroy } + + it 'raises SalesforceRecordNotFound' do + expect { perform_job } + .to raise_error(Salesforce::SalesforceSyncJob::SalesforceRecordNotFound, /Editor__c not yet synced/) + end + end + + context 'when the parent Contact is not yet synced to Salesforce' do + before { sf_contact.update!(sfid: nil) } + + it 'raises SalesforceRecordNotFound to defer the affiliation write' do + expect { perform_job } + .to raise_error(Salesforce::SalesforceSyncJob::SalesforceRecordNotFound, /Contact not yet synced/) + end + + it 'does not write the affiliation to the mirror' do + expect { perform_job }.to raise_error(Salesforce::SalesforceSyncJob::SalesforceRecordNotFound) + expect(Salesforce::Role.find_by(affiliation_id__c: role.id)).to be_nil + end + end + + context 'when there is no Salesforce::Contact row for the role user' do + before { sf_contact.destroy } + + it 'raises SalesforceRecordNotFound' do + expect { perform_job } + .to raise_error(Salesforce::SalesforceSyncJob::SalesforceRecordNotFound, /Contact not yet synced/) + end + end + context 'when SALESFORCE_ENABLED is false' do around do |example| ClimateControl.modify(SALESFORCE_ENABLED: 'false') do From ff9a2ddc4212e7093ae20058ae9c41d8eda46297 Mon Sep 17 00:00:00 2001 From: Fraser Speirs Date: Fri, 5 Jun 2026 09:30:00 +0100 Subject: [PATCH 2/4] Assert retry enqueue instead of raise in parent-guard specs `SalesforceSyncJob` retries on `SalesforceRecordNotFound`, so when `perform_now` raises it ActiveJob catches the error and re-enqueues the job rather than propagating. The new contexts asserted `raise_error`, which never matched. Mirror the existing `ContactSyncJob` spec pattern and assert `have_enqueued_job` (plus, for the side-effect tests, that no `Salesforce::Role` row is written). Co-authored-by: Cursor --- spec/jobs/salesforce/role_sync_job_spec.rb | 24 +++++++++------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/spec/jobs/salesforce/role_sync_job_spec.rb b/spec/jobs/salesforce/role_sync_job_spec.rb index 24be54bf5..6ddef0182 100644 --- a/spec/jobs/salesforce/role_sync_job_spec.rb +++ b/spec/jobs/salesforce/role_sync_job_spec.rb @@ -62,13 +62,12 @@ context 'when the parent Editor__c is not yet synced to Salesforce' do before { sf_school.update!(sfid: nil) } - it 'raises SalesforceRecordNotFound to defer the affiliation write' do - expect { perform_job } - .to raise_error(Salesforce::SalesforceSyncJob::SalesforceRecordNotFound, /Editor__c not yet synced/) + it 'retries the job to defer the affiliation write' do + expect { perform_job }.to have_enqueued_job(described_class).with(role_id: role.id) end it 'does not write the affiliation to the mirror' do - expect { perform_job }.to raise_error(Salesforce::SalesforceSyncJob::SalesforceRecordNotFound) + perform_job expect(Salesforce::Role.find_by(affiliation_id__c: role.id)).to be_nil end end @@ -76,22 +75,20 @@ context 'when there is no Salesforce::School row for the role school' do before { sf_school.destroy } - it 'raises SalesforceRecordNotFound' do - expect { perform_job } - .to raise_error(Salesforce::SalesforceSyncJob::SalesforceRecordNotFound, /Editor__c not yet synced/) + it 'retries the job' do + expect { perform_job }.to have_enqueued_job(described_class).with(role_id: role.id) end end context 'when the parent Contact is not yet synced to Salesforce' do before { sf_contact.update!(sfid: nil) } - it 'raises SalesforceRecordNotFound to defer the affiliation write' do - expect { perform_job } - .to raise_error(Salesforce::SalesforceSyncJob::SalesforceRecordNotFound, /Contact not yet synced/) + it 'retries the job to defer the affiliation write' do + expect { perform_job }.to have_enqueued_job(described_class).with(role_id: role.id) end it 'does not write the affiliation to the mirror' do - expect { perform_job }.to raise_error(Salesforce::SalesforceSyncJob::SalesforceRecordNotFound) + perform_job expect(Salesforce::Role.find_by(affiliation_id__c: role.id)).to be_nil end end @@ -99,9 +96,8 @@ context 'when there is no Salesforce::Contact row for the role user' do before { sf_contact.destroy } - it 'raises SalesforceRecordNotFound' do - expect { perform_job } - .to raise_error(Salesforce::SalesforceSyncJob::SalesforceRecordNotFound, /Contact not yet synced/) + it 'retries the job' do + expect { perform_job }.to have_enqueued_job(described_class).with(role_id: role.id) end end From 59d9af013fc1f6c3f1d335c70d28b3f257bea63c Mon Sep 17 00:00:00 2001 From: Fraser Speirs Date: Fri, 5 Jun 2026 09:44:08 +0100 Subject: [PATCH 3/4] Assert that rows are not written on retry Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- spec/jobs/salesforce/role_sync_job_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/jobs/salesforce/role_sync_job_spec.rb b/spec/jobs/salesforce/role_sync_job_spec.rb index 6ddef0182..b873f3856 100644 --- a/spec/jobs/salesforce/role_sync_job_spec.rb +++ b/spec/jobs/salesforce/role_sync_job_spec.rb @@ -78,6 +78,11 @@ it 'retries the job' do expect { perform_job }.to have_enqueued_job(described_class).with(role_id: role.id) end + + it 'does not write the affiliation to the mirror' do + perform_job + expect(Salesforce::Role.find_by(affiliation_id__c: role.id)).to be_nil + end end context 'when the parent Contact is not yet synced to Salesforce' do From 77f42ebf215e67487a9c11dd4963fabf220f6351 Mon Sep 17 00:00:00 2001 From: Fraser Speirs Date: Fri, 5 Jun 2026 09:44:17 +0100 Subject: [PATCH 4/4] Assert that rows are not written on retry Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- spec/jobs/salesforce/role_sync_job_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/jobs/salesforce/role_sync_job_spec.rb b/spec/jobs/salesforce/role_sync_job_spec.rb index b873f3856..2df328a3b 100644 --- a/spec/jobs/salesforce/role_sync_job_spec.rb +++ b/spec/jobs/salesforce/role_sync_job_spec.rb @@ -104,6 +104,11 @@ it 'retries the job' do expect { perform_job }.to have_enqueued_job(described_class).with(role_id: role.id) end + + it 'does not write the affiliation to the mirror' do + perform_job + expect(Salesforce::Role.find_by(affiliation_id__c: role.id)).to be_nil + end end context 'when SALESFORCE_ENABLED is false' do