From 8db5d9fac3702a847b07cbf536bdbc172f412973 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Thu, 29 Aug 2024 14:48:39 +0100 Subject: [PATCH] CLDC-3605 View relationships page (#2606) --- Gemfile.lock | 2 +- app/controllers/merge_requests_controller.rb | 10 ++- app/helpers/merge_requests_helper.rb | 68 ++++++++++++++++++- app/models/merge_request.rb | 43 +++++++++++- .../relationship_outcomes.html.erb | 23 +++++++ config/routes.rb | 1 + spec/helpers/merge_requests_helper_spec.rb | 48 +++++++++++++ spec/models/merge_request_spec.rb | 39 +++++++++++ .../merge_requests_controller_spec.rb | 30 +++++--- .../merge_requests/show.html.erb_spec.rb | 10 ++- 10 files changed, 255 insertions(+), 19 deletions(-) create mode 100644 app/views/merge_requests/relationship_outcomes.html.erb diff --git a/Gemfile.lock b/Gemfile.lock index 2f51d6a29..086491321 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -379,7 +379,7 @@ GEM responders (3.1.1) actionpack (>= 5.2) railties (>= 5.2) - rexml (3.3.3) + rexml (3.3.6) strscan roo (2.10.1) nokogiri (~> 1) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index c34e00581..7b5ed1ba9 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -9,6 +9,7 @@ class MergeRequestsController < ApplicationController def helpdesk_ticket; end def merge_start_confirmation; end def user_outcomes; end + def relationship_outcomes; end def scheme_outcomes; end def create @@ -64,7 +65,14 @@ class MergeRequestsController < ApplicationController def start_merge if @merge_request.status == "ready_to_merge" - @merge_request.update!(processing: true, last_failed_attempt: nil, total_users: @merge_request.total_visible_users_after_merge, total_schemes: @merge_request.total_visible_schemes_after_merge) + @merge_request.update!( + processing: true, + last_failed_attempt: nil, + total_users: @merge_request.total_visible_users_after_merge, + total_schemes: @merge_request.total_visible_schemes_after_merge, + total_stock_owners: @merge_request.total_stock_owners_after_merge, + total_managing_agents: @merge_request.total_managing_agents_after_merge, + ) ProcessMergeRequestJob.perform_later(merge_request: @merge_request) end diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 23623b0a6..a5d2ea14e 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -27,7 +27,7 @@ module MergeRequestsHelper { label: "Total users after merge", value: display_value_or_placeholder(merge_request.total_users_label), action: merge_outcome_action(merge_request, "user_outcomes") }, { label: "Total schemes after merge", value: display_value_or_placeholder(merge_request.total_schemes_label), action: merge_outcome_action(merge_request, "scheme_outcomes") }, { label: "Total logs after merge", value: merge_request.total_lettings_logs.present? || merge_request.total_sales_logs.present? ? "#{merge_request.total_lettings_logs} lettings logs
#{merge_request.total_sales_logs} sales logs".html_safe : display_value_or_placeholder(nil), action: { text: "View", href: "#", visually_hidden_text: "total logs after merge" } }, - { label: "Total stock owners & managing agents after merge", value: merge_request.total_stock_owners.present? || merge_request.total_managing_agents.present? ? "#{merge_request.total_stock_owners} stock owners
#{merge_request.total_managing_agents} managing agents".html_safe : display_value_or_placeholder(nil), action: { text: "View", href: "#", visually_hidden_text: "total stock owners & managing agents after merge" } }, + { label: "Total stock owners & managing agents after merge", value: display_value_or_placeholder(merge_request.total_stock_owners_managing_agents_label), action: merge_outcome_action(merge_request, "relationship_outcomes") }, ] end @@ -108,6 +108,72 @@ module MergeRequestsHelper "#{"#{count} user".pluralize(count)} after merge" end + def total_stock_owners_after_merge_text(merge_request) + 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 + "#{"#{count} managing agent".pluralize(count)} after merge" + end + + def related_organisations(merge_request, relationship_type) + organisations = merge_request.absorbing_organisation.send(relationship_type.pluralize).visible + merge_request.merging_organisations.flat_map { |org| org.send(relationship_type.pluralize).visible } + organisations += [merge_request.absorbing_organisation] + merge_request.merging_organisations + organisations.group_by { |relationship| relationship }.select { |_, occurrences| occurrences.size > 1 }.keys + end + + def related_organisations_text(merge_request, relationship_type) + if related_organisations(merge_request, relationship_type).any? + "Some of the organisations merging have common #{relationship_type.humanize(capitalize: false).pluralize}.

" + else + "" + end + end + + def organisations_without_relationships(merge_request, relationship_type) + ([merge_request.absorbing_organisation] + merge_request.merging_organisations).select { |org| org.send(relationship_type.pluralize).visible.empty? } + 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}.

" + end + + def generate_organisation_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 relationship_text(merge_request, relationship_type, organisation_path_helper) + text = "" + organisations_without_relationships = organisations_without_relationships(merge_request, relationship_type) + + text += related_organisations_text(merge_request, relationship_type) + text += organisations_without_relationships_text(organisations_without_relationships, relationship_type) + + ([merge_request.absorbing_organisation] + merge_request.merging_organisations).each do |org| + organisation_count = org.send(relationship_type.pluralize).visible.count + next if organisation_count.zero? + + link_text = generate_organisation_link_text(organisation_count, org, relationship_type) + text += "#{govuk_link_to(link_text, send(organisation_path_helper, org), target: '_blank')}

" + end + + text.html_safe + end + + def stock_owners_text(merge_request) + relationship_text(merge_request, "stock_owner", :stock_owners_organisation_path) + end + + def managing_agent_text(merge_request) + relationship_text(merge_request, "managing_agent", :managing_agents_organisation_path) + end + def merging_organisations_without_schemes_text(organisations) return "" unless organisations.count.positive? diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index fe45bd18e..3560756e4 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -62,7 +62,7 @@ class MergeRequest < ApplicationRecord end def total_users_label - "#{total_visible_users_after_merge} #{'User'.pluralize(total_visible_users_after_merge)}" + "#{total_visible_users_after_merge} #{'user'.pluralize(total_visible_users_after_merge)}" end def organisations_with_users @@ -84,7 +84,7 @@ class MergeRequest < ApplicationRecord end def total_schemes_label - "#{total_visible_schemes_after_merge} #{'Scheme'.pluralize(total_visible_schemes_after_merge)}" + "#{total_visible_schemes_after_merge} #{'scheme'.pluralize(total_visible_schemes_after_merge)}" end def organisations_with_schemes @@ -98,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 diff --git a/app/views/merge_requests/relationship_outcomes.html.erb b/app/views/merge_requests/relationship_outcomes.html.erb new file mode 100644 index 000000000..9a322efb6 --- /dev/null +++ b/app/views/merge_requests/relationship_outcomes.html.erb @@ -0,0 +1,23 @@ +<% content_for :before_content do %> + <% title = "Stock owners & managing agents".html_safe %> + <% content_for :title, title %> + <%= govuk_back_link href: merge_request_path(@merge_request) %> +<% end %> + +

+ <%= @merge_request.absorbing_organisation_name %> + Stock owners & managing agents +

+ +<% unless @merge_request.status == "request_merged" || @merge_request.status == "processing" %> +

<%= total_stock_owners_after_merge_text(@merge_request) %>

+

+ <%= stock_owners_text(@merge_request) %> +

+
+
+

<%= total_managing_agents_after_merge_text(@merge_request) %>

+

+ <%= managing_agent_text(@merge_request) %> +

+<% end %> diff --git a/config/routes.rb b/config/routes.rb index d8e61c04b..a6f2ae8c7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -211,6 +211,7 @@ Rails.application.routes.draw do get "helpdesk-ticket" get "merge-start-confirmation" get "user-outcomes" + get "relationship-outcomes" get "scheme-outcomes" get "delete-confirmation", to: "merge_requests#delete_confirmation" delete "delete", to: "merge_requests#delete" diff --git a/spec/helpers/merge_requests_helper_spec.rb b/spec/helpers/merge_requests_helper_spec.rb index 1f4bf5733..8907ce70d 100644 --- a/spec/helpers/merge_requests_helper_spec.rb +++ b/spec/helpers/merge_requests_helper_spec.rb @@ -110,4 +110,52 @@ RSpec.describe MergeRequestsHelper do end end end + + describe "when creating relationship outcomes content" do + let(:stock_owner1) { create(:organisation, name: "Stock owner 1") } + let(:stock_owner2) { create(:organisation, name: "Stock owner 2") } + let(:managing_agent1) { create(:organisation, name: "Managing agent 1") } + let(:managing_agent2) { create(:organisation, name: "Managing agent 2") } + let(:absorbing_organisation) { create(:organisation, name: "Absorbing Org") } + let(:merging_organisations) { create_list(:organisation, 2) { |org, i| org.name = "Dummy Org #{i + 1}" } } + let(:merge_request) { create(:merge_request, absorbing_organisation:, merging_organisations:) } + + context "when there are no relationships" do + it "returns text stating there are no stock owners" do + expect(stock_owners_text(merge_request)).to eq("Absorbing Org, Dummy Org 1, and Dummy Org 2 have no stock owners.

") + end + + it "returns text stating there are no managing agents" do + expect(managing_agent_text(merge_request)).to eq("Absorbing Org, Dummy Org 1, and Dummy Org 2 have no managing agents.

") + end + end + + context "when there are stock owners" do + before do + create(:organisation_relationship, child_organisation: absorbing_organisation, parent_organisation: stock_owner1) + create(:organisation_relationship, child_organisation: merging_organisations.first, parent_organisation: stock_owner2) + create(:organisation_relationship, child_organisation: merging_organisations.first, parent_organisation: stock_owner1) + end + + it "returns text stating the relationships" do + expect(stock_owners_text(merge_request)).to include("Some of the organisations merging have common stock owners.") + expect(stock_owners_text(merge_request)).to include("Dummy Org 2 has no stock owners.") + expect(stock_owners_text(merge_request)).to include("View all 2 Dummy Org 1 stock owners (opens in a new tab)") + end + end + + context "when there are managing agents" do + before do + create(:organisation_relationship, parent_organisation: absorbing_organisation, child_organisation: managing_agent1) + create(:organisation_relationship, parent_organisation: absorbing_organisation, child_organisation: managing_agent2) + create(:organisation_relationship, parent_organisation: merging_organisations.first, child_organisation: managing_agent2) + end + + it "returns text stating the relationships" do + expect(managing_agent_text(merge_request)).to include("Some of the organisations merging have common managing agents.") + expect(managing_agent_text(merge_request)).to include("Dummy Org 2 has no managing agents.") + expect(managing_agent_text(merge_request)).to include("View the 1 Dummy Org 1 managing agent (opens in a new tab)") + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 554ab7a02..855fe03da 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -397,4 +397,43 @@ RSpec.describe MergeRequest, type: :model do end end end + + describe "relationship outcomes" do + let(:stock_owner1) { create(:organisation, name: "Stock owner 1") } + let(:stock_owner2) { create(:organisation, name: "Stock owner 2") } + let(:stock_owner3) { create(:organisation, name: "Stock owner 3") } + let(:managing_agent1) { create(:organisation, name: "Managing agent 1") } + let(:managing_agent2) { create(:organisation, name: "Managing agent 2") } + let(:absorbing_organisation) { create(:organisation, name: "Absorbing Org") } + let(:merging_organisations) { create_list(:organisation, 2) { |org, i| org.name = "Dummy Org #{i + 1}" } } + let(:merge_request) { create(:merge_request, absorbing_organisation:, merging_organisations:) } + + before do + create(:organisation_relationship, child_organisation: absorbing_organisation, parent_organisation: stock_owner1) + create(:organisation_relationship, child_organisation: merging_organisations.first, parent_organisation: stock_owner2) + create(:organisation_relationship, child_organisation: merging_organisations.first, parent_organisation: stock_owner1) + create(:organisation_relationship, child_organisation: merging_organisations.first, parent_organisation: stock_owner3) + create(:organisation_relationship, parent_organisation: absorbing_organisation, child_organisation: managing_agent1) + create(:organisation_relationship, parent_organisation: absorbing_organisation, child_organisation: managing_agent2) + create(:organisation_relationship, parent_organisation: merging_organisations.first, child_organisation: managing_agent2) + end + + describe "#total_stock_owners_after_merge" do + it "returns the correct count of stock owners after merge" do + expect(merge_request.total_stock_owners_after_merge).to eq(2) + end + end + + describe "#total_managing_agents_after_merge" do + it "returns the correct count of managing agents after merge" do + expect(merge_request.total_managing_agents_after_merge).to eq(1) + end + end + + describe "#total_stock_owners_managing_agents_label" do + it "returns the correct label" do + expect(merge_request.total_stock_owners_managing_agents_label).to eq("2 stock owners\n1 managing agent") + end + end + end end diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb index 30941360e..530656858 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/spec/requests/merge_requests_controller_spec.rb @@ -485,40 +485,54 @@ RSpec.describe MergeRequestsController, type: :request do it "shows users and schemes count and has links to view merge outcomes" do expect(page).to have_link("View", href: scheme_outcomes_merge_request_path(merge_request)) - expect(page).to have_content("4 Users") - expect(page).to have_content("4 Schemes") + expect(page).to have_content("4 users") + expect(page).to have_content("4 schemes") end end context "with a merged request" do - let(:merge_request) { create(:merge_request, request_merged: true, total_users: 34, total_schemes: 12) } + let(:merge_request) { create(:merge_request, request_merged: true, total_users: 34, total_schemes: 12, total_stock_owners: 8, total_managing_agents: 5) } it "shows saved users count and doesn't have links to view merge outcomes" do expect(merge_request.status).to eq("request_merged") expect(page).not_to have_link("View", href: user_outcomes_merge_request_path(merge_request)) - expect(page).to have_content("34 Users") + expect(page).to have_content("34 users") end it "shows saved schemes count and doesn't have links to view merge outcomes" do expect(merge_request.status).to eq("request_merged") expect(page).not_to have_link("View", href: scheme_outcomes_merge_request_path(merge_request)) - expect(page).to have_content("12 Schemes") + expect(page).to have_content("12 schemes") + end + + it "shows stock owners and managing agents count and doesn't have links to view merge outcomes" do + expect(merge_request.status).to eq("request_merged") + expect(page).not_to have_link("View", href: relationship_outcomes_merge_request_path(merge_request)) + expect(page).to have_content("8 stock owners") + expect(page).to have_content("5 managing agents") end end context "with a processing request" do - let(:merge_request) { create(:merge_request, processing: true, total_users: 51, total_schemes: 33) } + let(:merge_request) { create(:merge_request, processing: true, total_users: 51, total_schemes: 33, total_stock_owners: 15, total_managing_agents: 20) } it "shows saved users count and doesn't have links to view merge outcomes" do expect(merge_request.status).to eq("processing") expect(page).not_to have_link("View", href: user_outcomes_merge_request_path(merge_request)) - expect(page).to have_content("51 Users") + expect(page).to have_content("51 users") end it "shows saved schemes count and doesn't have links to view merge outcomes" do expect(merge_request.status).to eq("processing") expect(page).not_to have_link("View", href: scheme_outcomes_merge_request_path(merge_request)) - expect(page).to have_content("33 Schemes") + expect(page).to have_content("33 schemes") + end + + it "shows stock owners and managing agents count and doesn't have links to view merge outcomes" do + expect(merge_request.status).to eq("processing") + expect(page).not_to have_link("View", href: relationship_outcomes_merge_request_path(merge_request)) + expect(page).to have_content("15 stock owners") + expect(page).to have_content("20 managing agents") end end end diff --git a/spec/views/merge_requests/show.html.erb_spec.rb b/spec/views/merge_requests/show.html.erb_spec.rb index 75fa4197d..d03239d2a 100644 --- a/spec/views/merge_requests/show.html.erb_spec.rb +++ b/spec/views/merge_requests/show.html.erb_spec.rb @@ -84,12 +84,10 @@ RSpec.describe "merge_requests/show.html.erb", type: :view do it "displays the total stock owners & managing agents after merge details" do expect(rendered).to have_selector("dt", text: "Total stock owners & managing agents after merge") - if merge_request.total_stock_owners.present? || merge_request.total_managing_agents.present? - combined_text = [] - combined_text << "#{merge_request.total_stock_owners} stock owners" if merge_request.total_stock_owners.present? - combined_text << "#{merge_request.total_managing_agents} managing agents" if merge_request.total_managing_agents.present? - expect(rendered).to have_selector("dd", text: combined_text.join("")) - end + combined_text = [] + combined_text << "#{merge_request.total_stock_owners} stock owners" if merge_request.total_stock_owners.present? + combined_text << "#{merge_request.total_managing_agents} managing agents" if merge_request.total_managing_agents.present? + expect(rendered).to have_selector("dd", text: combined_text.join("\n")) end end end