From dae1e9830baf1ab58c44b8a56769139fd5688b51 Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Thu, 19 Jun 2025 13:40:16 +0100 Subject: [PATCH] CLDC-4043: Ensure that if an Organisation merge fails due to a validation error it will be marked as incomplete (#3081) * no longer raise a rollback on merge fail this caused the error to not be bubbled up correctly, meaning the catch in process_merge_request_job was not being triggered, and so not setting the merge back to ready raising any exception will trigger a rollback * update tests --- .../merge/merge_organisations_service.rb | 2 +- .../merge/merge_organisations_service_spec.rb | 40 +++++++++---------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/app/services/merge/merge_organisations_service.rb b/app/services/merge/merge_organisations_service.rb index 40749b54c..c831c875f 100644 --- a/app/services/merge/merge_organisations_service.rb +++ b/app/services/merge/merge_organisations_service.rb @@ -26,7 +26,7 @@ class Merge::MergeOrganisationsService log_success_message rescue ActiveRecord::RecordInvalid => e Rails.logger.error("Organisation merge failed with: #{e.message}") - raise ActiveRecord::Rollback + raise end end diff --git a/spec/services/merge/merge_organisations_service_spec.rb b/spec/services/merge/merge_organisations_service_spec.rb index 9355a757a..84016a9b4 100644 --- a/spec/services/merge/merge_organisations_service_spec.rb +++ b/spec/services/merge/merge_organisations_service_spec.rb @@ -59,7 +59,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation) allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) absorbing_organisation.reload merging_organisation.reload @@ -103,7 +103,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation) allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) absorbing_organisation.reload merging_organisation.reload @@ -140,7 +140,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation) allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) absorbing_organisation.reload expect(absorbing_organisation.child_organisations.count).to eq(3) @@ -557,7 +557,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation) allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) absorbing_organisation.reload merging_organisation.reload @@ -590,7 +590,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation) allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) absorbing_organisation.reload expect(absorbing_organisation.sales_logs.count).to eq(0) @@ -652,7 +652,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation) allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) absorbing_organisation.reload expect(absorbing_organisation.sales_logs.count).to eq(0) @@ -688,7 +688,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation) allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) absorbing_organisation.reload expect(absorbing_organisation.lettings_logs.count).to eq(0) @@ -831,7 +831,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation) allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) absorbing_organisation.reload merging_organisation.reload @@ -950,7 +950,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation) allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) absorbing_organisation.reload merging_organisation.reload @@ -1075,7 +1075,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation) allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) absorbing_organisation.reload merging_organisation.reload @@ -1145,7 +1145,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation) allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) new_absorbing_organisation.reload merging_organisation.reload @@ -1189,7 +1189,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation) allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) new_absorbing_organisation.reload merging_organisation.reload @@ -1226,7 +1226,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation) allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) new_absorbing_organisation.reload expect(new_absorbing_organisation.child_organisations.count).to eq(3) @@ -1331,7 +1331,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation) allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) new_absorbing_organisation.reload merging_organisation.reload @@ -1363,7 +1363,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation) allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) new_absorbing_organisation.reload expect(new_absorbing_organisation.sales_logs.count).to eq(0) @@ -1402,7 +1402,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation) allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) new_absorbing_organisation.reload expect(new_absorbing_organisation.sales_logs.count).to eq(0) @@ -1438,7 +1438,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation) allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) new_absorbing_organisation.reload expect(new_absorbing_organisation.lettings_logs.count).to eq(0) @@ -1544,7 +1544,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation) allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) new_absorbing_organisation.reload merging_organisation.reload @@ -1634,7 +1634,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation) allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) new_absorbing_organisation.reload merging_organisation.reload @@ -1676,7 +1676,7 @@ RSpec.describe Merge::MergeOrganisationsService do allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation) allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid") - merge_organisations_service.call + expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid) new_absorbing_organisation.reload merging_organisation.reload