Update merge logging

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"
+      @merged_users = {}
-      @schemes_success_message = ""
+      @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)
+      log_success_message
-      Rails.logger.info(@schemes_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

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("Merged users from fake org:")
-        expect(Rails.logger).to receive(:info).with("New schemes from fake org:\n")
+        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("Merged users from fake org:")
-          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("\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("Merged users from fake 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("\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