From 99f42ddc8a53b1067b3455a70d0a6656f3218659 Mon Sep 17 00:00:00 2001 From: Kat Date: Tue, 25 Jul 2023 11:21:52 +0100 Subject: [PATCH] Rollback transaction if there's an error merging orgs --- .../merge/merge_organisations_service.rb | 25 ++++--- .../merge/merge_organisations_service_spec.rb | 69 ++++++++++++++++++- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/app/services/merge/merge_organisations_service.rb b/app/services/merge/merge_organisations_service.rb index 85988cf3c..3f7b4dce6 100644 --- a/app/services/merge/merge_organisations_service.rb +++ b/app/services/merge/merge_organisations_service.rb @@ -5,17 +5,22 @@ class Merge::MergeOrganisationsService end def call - merge_organisation_details - @merging_organisations.each do |merging_organisation| - merge_rent_periods(merging_organisation) - merge_organisation_relationships(merging_organisation) - merge_users(merging_organisation) - merge_schemes_and_locations(merging_organisation) - merge_lettings_logs(merging_organisation) - merge_sales_logs(merging_organisation) + ActiveRecord::Base.transaction do + merge_organisation_details + @merging_organisations.each do |merging_organisation| + merge_rent_periods(merging_organisation) + merge_organisation_relationships(merging_organisation) + merge_users(merging_organisation) + merge_schemes_and_locations(merging_organisation) + merge_lettings_logs(merging_organisation) + merge_sales_logs(merging_organisation) + end + @absorbing_organisation.save! + mark_organisations_as_merged + rescue ActiveRecord::RecordInvalid => e + Rails.logger.error("Organisation merge failed with: #{e.message}") + raise ActiveRecord::Rollback end - @absorbing_organisation.save! - mark_organisations_as_merged end private diff --git a/spec/services/merge/merge_organisations_service_spec.rb b/spec/services/merge/merge_organisations_service_spec.rb index 5e5929495..5fe8b2336 100644 --- a/spec/services/merge/merge_organisations_service_spec.rb +++ b/spec/services/merge/merge_organisations_service_spec.rb @@ -33,22 +33,49 @@ RSpec.describe Merge::MergeOrganisationsService do expect(absorbing_organisation.holds_own_stock).to eq(true) end + it "rolls back if there's an error" do + allow(Organisation).to receive(:find).with([merging_organisation_ids]).and_return(Organisation.find(merging_organisation_ids)) + 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 + + absorbing_organisation.reload + expect(absorbing_organisation.holds_own_stock).to eq(false) + # expect(merging_organisation.merge_date).to eq(nil) + expect(merging_organisation_user.organisation).to eq(merging_organisation) + end + context "and merging organisation rent periods" do before do OrganisationRentPeriod.create!(organisation: absorbing_organisation, rent_period: 1) OrganisationRentPeriod.create!(organisation: absorbing_organisation, rent_period: 3) OrganisationRentPeriod.create!(organisation: merging_organisation, rent_period: 1) OrganisationRentPeriod.create!(organisation: merging_organisation, rent_period: 2) - merge_organisations_service.call end it "combines organisation rent periods" do + expect(absorbing_organisation.rent_periods.count).to eq(2) + merge_organisations_service.call + absorbing_organisation.reload expect(absorbing_organisation.rent_periods.count).to eq(3) expect(absorbing_organisation.rent_periods).to include(1) expect(absorbing_organisation.rent_periods).to include(2) expect(absorbing_organisation.rent_periods).to include(3) end + + it "rolls back if there's an error" do + allow(Organisation).to receive(:find).with([merging_organisation_ids]).and_return(Organisation.find(merging_organisation_ids)) + 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 + + absorbing_organisation.reload + expect(absorbing_organisation.rent_periods.count).to eq(2) + expect(merging_organisation.rent_periods.count).to eq(2) + end end context "and merging organisation relationships" do @@ -73,6 +100,20 @@ RSpec.describe Merge::MergeOrganisationsService do expect(absorbing_organisation.parent_organisations.count).to eq(0) expect(absorbing_organisation.child_organisations.count).to eq(3) end + + it "rolls back if there's an error" do + allow(Organisation).to receive(:find).with([merging_organisation_ids]).and_return(Organisation.find(merging_organisation_ids)) + 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 + + absorbing_organisation.reload + expect(absorbing_organisation.child_organisations.count).to eq(3) + expect(absorbing_organisation.child_organisations).to include(other_organisation) + expect(absorbing_organisation.child_organisations).to include(merging_organisation) + expect(absorbing_organisation.child_organisations).to include(absorbing_organisation_relationship.child_organisation) + end end context "and merging organisation schemes and locations" do @@ -115,6 +156,20 @@ RSpec.describe Merge::MergeOrganisationsService do expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).scheme).to eq(absorbing_organisation.owned_schemes.first) expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(nil) end + + it "rolls back if there's an error" do + allow(Organisation).to receive(:find).with([merging_organisation_ids]).and_return(Organisation.find(merging_organisation_ids)) + 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 + + absorbing_organisation.reload + expect(absorbing_organisation.owned_schemes.count).to eq(0) + expect(scheme.scheme_deactivation_periods.count).to eq(0) + expect(owned_lettings_log.owning_organisation).to eq(merging_organisation) + expect(owned_lettings_log_no_location.owning_organisation).to eq(merging_organisation) + end end context "and merging sales logs" do @@ -131,6 +186,18 @@ RSpec.describe Merge::MergeOrganisationsService do expect(absorbing_organisation.owned_sales_logs.count).to eq(1) expect(absorbing_organisation.owned_sales_logs.first).to eq(sales_log) end + + it "rolls back if there's an error" do + allow(Organisation).to receive(:find).with([merging_organisation_ids]).and_return(Organisation.find(merging_organisation_ids)) + 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 + + absorbing_organisation.reload + expect(absorbing_organisation.owned_sales_logs.count).to eq(0) + expect(sales_log.owning_organisation).to eq(merging_organisation) + end end end end