Browse Source

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
pull/3085/head
Samuel Young 2 months ago committed by GitHub
parent
commit
dae1e9830b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 2
      app/services/merge/merge_organisations_service.rb
  2. 40
      spec/services/merge/merge_organisations_service_spec.rb

2
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

40
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

Loading…
Cancel
Save