Update methods

app/helpers/merge_requests_helper.rb

@@ -104,17 +104,17 @@ module MergeRequestsHelper
   end
 
   def total_users_after_merge_text(merge_request)
-    count = merge_request.total_visible_users_after_merge || 0
+    count = merge_request.total_visible_users_after_merge
     "#{"#{count} user".pluralize(count)} after merge"
   end
 
   def total_stock_owners_after_merge_text(merge_request)
-    count = merge_request.total_stock_owners_after_merge || 0
+    count = merge_request.total_stock_owners_after_merge
     "#{"#{count} stock owner".pluralize(count)} after merge"
   end
 
   def total_managing_agents_after_merge_text(merge_request)
-    count = merge_request.total_managing_agents_after_merge || 0
+    count = merge_request.total_managing_agents_after_merge
     "#{"#{count} managing agent".pluralize(count)} after merge"
   end
 
@@ -126,50 +126,52 @@ module MergeRequestsHelper
     relationship_text(merge_request, "managing_agent", :managing_agents_organisation_path)
   end
 
-  def relationship_text(merge_request, relationship_type, relationship_path_helper)
+  def related_organisations(merge_request, relationship_type)
-    html_content = ""
+    organisations =  merge_request.absorbing_organisation.send(relationship_type.pluralize).visible + merge_request.merging_organisations.flat_map { |org| org.send(relationship_type.pluralize).visible }
-    duplicates = duplicate_relationships(merge_request, relationship_type)
+    organisations += [merge_request.absorbing_organisation] + merge_request.merging_organisations
-    organisations_without_relationships = ([merge_request.absorbing_organisation] + merge_request.merging_organisations).select { |org| org.send(relationship_type.pluralize).visible.empty? }
+    organisations.group_by { |relationship| relationship }.select { |_, occurrences| occurrences.size > 1 }.keys
+  end
 
-    if duplicates.any?
+  def relationship_text(merge_request, relationship_type, organisation_path_helper)
-      html_content += "Some of the organisations merging have common #{relationship_type.humanize(capitalize: false).pluralize}.<br><br>"
+    text = ""
-    end
+    organisations_without_relationships = organisations_without_relationships(merge_request, relationship_type)
 
-    if organisations_without_relationships.any?
+    text += related_organisations_text(merge_request, relationship_type)
-      org_names = organisations_without_relationships.map(&:name).to_sentence
+    text += organisations_without_relationships_text(organisations_without_relationships, relationship_type)
-      verb = organisations_without_relationships.count > 1 ? "have" : "has"
-      html_content += "#{org_names} #{verb} no #{relationship_type.humanize(capitalize: false).pluralize}.<br><br>"
-    end
 
     ([merge_request.absorbing_organisation] + merge_request.merging_organisations).each do |org|
-      relationship_count = org.send(relationship_type.pluralize).visible.count
+      organisation_count = org.send(relationship_type.pluralize).visible.count
-      next if relationship_count.zero?
+      next if organisation_count.zero?
-
+
-      link_text = if relationship_count == 1
+      link_text = generate_link_text(organisation_count, org, relationship_type)
-                    "View the #{relationship_count} #{org.name} #{relationship_type.humanize(capitalize: false)} (opens in a new tab)"
+      text += "#{govuk_link_to(link_text, send(organisation_path_helper, org), target: '_blank')}<br><br>"
-                  else
-                    "View all #{relationship_count} #{org.name} #{relationship_type.humanize(capitalize: false).pluralize} (opens in a new tab)"
-                  end
-      html_content += "#{govuk_link_to(link_text, send(relationship_path_helper, org), target: '_blank')}<br><br>"
     end
 
-    html_content.html_safe
+    text.html_safe
   end
 
-  def duplicate_relationships(merge_request, relationship_type)
+  def organisations_without_relationships(merge_request, relationship_type)
-    relationships = merge_request.absorbing_organisation.send(relationship_type.pluralize) + merge_request.merging_organisations.flat_map { |org| org.send(relationship_type.pluralize) }
+    ([merge_request.absorbing_organisation] + merge_request.merging_organisations).select { |org| org.send(relationship_type.pluralize).visible.empty? }
-    seen = Set.new
+  end
-    duplicates = []
 
-    relationships.each do |relationship|
+  def related_organisations_text(merge_request, relationship_type)
-      if seen.include?(relationship)
+    if related_organisations(merge_request, relationship_type).any?
-        duplicates << relationship
+      "Some of the organisations merging have common #{relationship_type.humanize(capitalize: false).pluralize}.<br><br>"
-      else
+    else
-        seen.add(relationship)
+      ""
-      end
     end
+  end
+
+  def organisations_without_relationships_text(organisations_without_relationships, relationship_type)
+    return "" unless organisations_without_relationships.any?
+
+    org_names = organisations_without_relationships.map(&:name).to_sentence
+    verb = organisations_without_relationships.count > 1 ? "have" : "has"
+    "#{org_names} #{verb} no #{relationship_type.humanize(capitalize: false).pluralize}.<br><br>"
+  end
 
-    duplicates
+  def generate_link_text(organisation_count, org, relationship_type)
+    "View #{organisation_count == 1 ? 'the' : 'all'} #{organisation_count} #{org.name} #{relationship_type.humanize(capitalize: false).pluralize(organisation_count)} (opens in a new tab)"
   end
 
   def merging_organisations_without_schemes_text(organisations)
@@ -188,7 +190,7 @@ module MergeRequestsHelper
   end
 
   def total_schemes_after_merge_text(merge_request)
-    count = merge_request.total_visible_schemes_after_merge || 0
+    count = merge_request.total_visible_schemes_after_merge
     "#{"#{count} scheme".pluralize(count)} after merge"
   end
 end

app/models/merge_request.rb

@@ -61,31 +61,10 @@ class MergeRequest < ApplicationRecord
     absorbing_organisation.users.visible.count + merging_organisations.sum { |org| org.users.visible.count }
   end
 
-  def total_stock_owners_after_merge
-    return total_stock_owners if status == STATUS[:request_merged] || status == STATUS[:processing]
-
-    stock_owners = absorbing_organisation.stock_owners.visible + merging_organisations.flat_map { |org| org.stock_owners.visible }
-    stock_owners.uniq.count
-  end
-
-  def total_managing_agents_after_merge
-    return total_managing_agents if status == STATUS[:request_merged] || status == STATUS[:processing]
-
-    managing_agents = absorbing_organisation.managing_agents.visible + merging_organisations.flat_map { |org| org.managing_agents.visible }
-    managing_agents.uniq.count
-  end
-
   def total_users_label
     "#{total_visible_users_after_merge} #{'user'.pluralize(total_visible_users_after_merge)}"
   end
 
-  def total_stock_owners_managing_agents_label
-    stock_owners_count = total_stock_owners_after_merge
-    managing_agents_count = total_managing_agents_after_merge
-
-    "#{stock_owners_count} #{'stock owner'.pluralize(stock_owners_count)}\n#{managing_agents_count} #{'managing agent'.pluralize(managing_agents_count)}"
-  end
-
   def organisations_with_users
     return [] unless absorbing_organisation.present? && merging_organisations.any?
 
@@ -119,4 +98,43 @@ class MergeRequest < ApplicationRecord
 
     ([absorbing_organisation] + merging_organisations).reject(&:has_visible_schemes?)
   end
+
+  def filter_relationships(absorbing_relationships, merging_relationships, absorbing_organisation, merging_organisations)
+    filtered_absorbing_relationships = absorbing_relationships.reject do |relationship|
+      merging_relationships.include?(relationship) || merging_organisations.include?(relationship)
+    end
+
+    filtered_merging_relationships = merging_relationships.reject do |relationship|
+      absorbing_relationships.include?(relationship) || relationship == absorbing_organisation || merging_organisations.include?(relationship)
+    end
+
+    (filtered_absorbing_relationships + filtered_merging_relationships).uniq
+  end
+
+  def total_stock_owners_after_merge
+    return total_stock_owners if status == STATUS[:request_merged] || status == STATUS[:processing]
+
+    absorbing_stock_owners = absorbing_organisation.stock_owners.visible
+    merging_stock_owners = merging_organisations.flat_map { |org| org.stock_owners.visible }
+
+    total_filtered_stock_owners = filter_relationships(absorbing_stock_owners, merging_stock_owners, absorbing_organisation, merging_organisations)
+    total_filtered_stock_owners.count
+  end
+
+  def total_managing_agents_after_merge
+    return total_managing_agents if status == STATUS[:request_merged] || status == STATUS[:processing]
+
+    absorbing_managing_agents = absorbing_organisation.managing_agents.visible
+    merging_managing_agents = merging_organisations.flat_map { |org| org.managing_agents.visible }
+
+    total_filtered_managing_agents = filter_relationships(absorbing_managing_agents, merging_managing_agents, absorbing_organisation, merging_organisations)
+    total_filtered_managing_agents.count
+  end
+
+  def total_stock_owners_managing_agents_label
+    stock_owners_count = total_stock_owners_after_merge
+    managing_agents_count = total_managing_agents_after_merge
+
+    "#{stock_owners_count} #{'stock owner'.pluralize(stock_owners_count)}\n#{managing_agents_count} #{'managing agent'.pluralize(managing_agents_count)}"
+  end
 end