From 7478c751dc60e643b1c006a2a481897f6333a5ad Mon Sep 17 00:00:00 2001 From: Kat Date: Wed, 26 Jul 2023 11:24:46 +0100 Subject: [PATCH] Update merge logging --- .../merge/merge_organisations_service.rb | 28 ++++++++++++++----- .../merge/merge_organisations_service_spec.rb | 22 +++++++++++---- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/app/services/merge/merge_organisations_service.rb b/app/services/merge/merge_organisations_service.rb index 3d922e8d7..795ee4c00 100644 --- a/app/services/merge/merge_organisations_service.rb +++ b/app/services/merge/merge_organisations_service.rb @@ -6,8 +6,8 @@ class Merge::MergeOrganisationsService def call ActiveRecord::Base.transaction do - @users_success_message = "Absorbing organisation users: #{@absorbing_organisation.users.map { |user| "#{user.name} (#{user.email})" }.join(', ')}\n" - @schemes_success_message = "" + @merged_users = {} + @merged_schemes = {} merge_organisation_details @merging_organisations.each do |merging_organisation| merge_rent_periods(merging_organisation) @@ -19,8 +19,7 @@ class Merge::MergeOrganisationsService mark_organisation_as_merged(merging_organisation) end @absorbing_organisation.save! - Rails.logger.info(@users_success_message) - Rails.logger.info(@schemes_success_message) + log_success_message rescue ActiveRecord::RecordInvalid => e Rails.logger.error("Organisation merge failed with: #{e.message}") raise ActiveRecord::Rollback @@ -57,12 +56,12 @@ private end def merge_users(merging_organisation) - @users_success_message += "Merged users from #{merging_organisation.name}: #{merging_organisation.users.map { |user| "#{user.name} (#{user.email})" }.join(', ')}\n" + @merged_users[merging_organisation.name] = merging_organisation.users.map { |user| { name: user.name, email: user.email } } merging_organisation.users.update_all(organisation_id: @absorbing_organisation.id) end def merge_schemes_and_locations(merging_organisation) - @schemes_success_message += "New schemes from #{merging_organisation.name}:\n" + @merged_schemes[merging_organisation.name] = [] merging_organisation.owned_schemes.each do |scheme| next if scheme.deactivated? @@ -70,7 +69,7 @@ private scheme.locations.each do |location| new_scheme.locations << Location.new(location.attributes.except("id", "scheme_id")) unless location.deactivated? end - @schemes_success_message += "Scheme #{new_scheme.service_name} with locations: #{new_scheme.locations.map { |location| "#{location.name} (#{location.postcode})" }.join(', ')}\n" + @merged_schemes[merging_organisation.name] << { name: new_scheme.service_name, code: new_scheme.id } SchemeDeactivationPeriod.create!(scheme:, deactivation_date: Time.zone.now) end end @@ -103,6 +102,21 @@ private merging_organisation.update(merge_date: Time.zone.today) end + def log_success_message + @merged_users.each do |organisation_name, users| + Rails.logger.info("Merged users from #{organisation_name}:") + users.each do |user| + Rails.logger.info("\t#{user[:name]} (#{user[:email]})") + end + end + @merged_schemes.each do |organisation_name, schemes| + Rails.logger.info("New schemes from #{organisation_name}:") + schemes.each do |scheme| + Rails.logger.info("\t#{scheme[:name]} (S#{scheme[:code]})") + end + end + end + def merge_boolean_organisation_attribute(attribute) @absorbing_organisation[attribute] ||= @merging_organisations.any? { |merging_organisation| merging_organisation[attribute] } end diff --git a/spec/services/merge/merge_organisations_service_spec.rb b/spec/services/merge/merge_organisations_service_spec.rb index 330ea4624..d8642acbc 100644 --- a/spec/services/merge/merge_organisations_service_spec.rb +++ b/spec/services/merge/merge_organisations_service_spec.rb @@ -14,8 +14,10 @@ RSpec.describe Merge::MergeOrganisationsService do let!(:merging_organisation_user) { create(:user, organisation: merging_organisation, name: "fake name", email: "fake@email.com") } it "moves the users from merging organisation to absorbing organisation" do - expect(Rails.logger).to receive(:info).with("Absorbing organisation users: Danny Rojas (#{absorbing_organisation.data_protection_officers.first.email})\nMerged users from fake org: Danny Rojas (#{merging_organisation.data_protection_officers.first.email}), fake name (fake@email.com)\n") - expect(Rails.logger).to receive(:info).with("New schemes from fake org:\n") + expect(Rails.logger).to receive(:info).with("Merged users from fake org:") + expect(Rails.logger).to receive(:info).with("\tDanny Rojas (#{merging_organisation.data_protection_officers.first.email})") + expect(Rails.logger).to receive(:info).with("\tfake name (fake@email.com)") + expect(Rails.logger).to receive(:info).with("New schemes from fake org:") merge_organisations_service.call merging_organisation_user.reload @@ -137,8 +139,11 @@ RSpec.describe Merge::MergeOrganisationsService do end it "combines organisation schemes and locations" do - expect(Rails.logger).to receive(:info).with("Absorbing organisation users: Danny Rojas (#{absorbing_organisation.data_protection_officers.first.email})\nMerged users from fake org: Danny Rojas (#{merging_organisation.data_protection_officers.first.email}), fake name (fake@email.com)\n") - expect(Rails.logger).to receive(:info).with("New schemes from fake org:\nScheme #{scheme.service_name} with locations: #{location.name} (#{location.postcode}), fake location (A1 1AA)\n") + expect(Rails.logger).to receive(:info).with("Merged users from fake org:") + expect(Rails.logger).to receive(:info).with("\tDanny Rojas (#{merging_organisation.data_protection_officers.first.email})") + expect(Rails.logger).to receive(:info).with("\tfake name (fake@email.com)") + expect(Rails.logger).to receive(:info).with("New schemes from fake org:") + expect(Rails.logger).to receive(:info).with(/\t#{scheme.service_name} \(S/) merge_organisations_service.call absorbing_organisation.reload @@ -218,8 +223,13 @@ RSpec.describe Merge::MergeOrganisationsService do end it "moves the users from merging organisations to absorbing organisation" do - expect(Rails.logger).to receive(:info).with(/Merged users from second org:/) - expect(Rails.logger).to receive(:info).with("New schemes from fake org:\nNew schemes from second org:\n") + expect(Rails.logger).to receive(:info).with("Merged users from fake org:") + expect(Rails.logger).to receive(:info).with("\tDanny Rojas (#{merging_organisation.data_protection_officers.first.email})") + expect(Rails.logger).to receive(:info).with("\tfake name (fake@email.com)") + expect(Rails.logger).to receive(:info).with("Merged users from second org:") + expect(Rails.logger).to receive(:info).with(/\tDanny Rojas/).exactly(6).times + expect(Rails.logger).to receive(:info).with("New schemes from fake org:") + expect(Rails.logger).to receive(:info).with("New schemes from second org:") merge_organisations_service.call merging_organisation_user.reload