diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 33acc2e1f..d59eb9164 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -1,4 +1,7 @@ module MergeRequestsHelper + include GovukLinkHelper + include GovukVisuallyHiddenHelper + def display_value_or_placeholder(value, placeholder = "You didn't answer this question") value.presence || content_tag(:span, placeholder, class: "app-!-colour-muted") end @@ -84,4 +87,19 @@ module MergeRequestsHelper def submit_merge_request_url(referrer) referrer == "check_answers" ? merge_request_path(referrer: "check_answers") : merge_request_path end + + def merging_organisations_without_users_text(organisations) + return "" unless organisations.count.positive? + + if organisations.count == 1 + "#{organisations.first.name} has no users." + else + "#{organisations.map(&:name).to_sentence} have no users." + end + end + + def link_to_merging_organisation_users(organisation) + count_text = organisation.users.count == 1 ? "1 #{organisation.name} user" : "all #{organisation.users.count} #{organisation.name} users" + govuk_link_to "View #{count_text} (opens in a new tab)", users_organisation_path(organisation), target: "_blank" + end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 9df255516..aae36195d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -55,13 +55,25 @@ class MergeRequest < ApplicationRecord absorbing_organisation&.data_protection_confirmed? end - def total_users_after_merge + def total_visible_users_after_merge return total_users if status == STATUS[:request_merged] || status == STATUS[:processing] - absorbing_organisation.users.count + merging_organisations.sum { |org| org.users.count } + absorbing_organisation.users.visible.count + merging_organisations.sum { |org| org.users.visible.count } end def total_users_label - "#{total_users_after_merge} Users" + "#{total_visible_users_after_merge} #{'User'.pluralize(total_visible_users_after_merge)}" + end + + def organisations_with_users + return [] unless absorbing_organisation.present? && merging_organisations.any? + + ([absorbing_organisation] + merging_organisations).select(&:has_visible_users?) + end + + def organisations_without_users + return [] unless absorbing_organisation.present? && merging_organisations.any? + + ([absorbing_organisation] + merging_organisations).reject(&:has_visible_users?) end end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 65b35c24e..6e75b79e5 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -191,4 +191,8 @@ class Organisation < ApplicationRecord def label status == :deleted ? "#{name} (deleted)" : name end + + def has_visible_users? + users.visible.count.positive? + end end diff --git a/app/views/merge_requests/user_outcomes.html.erb b/app/views/merge_requests/user_outcomes.html.erb new file mode 100644 index 000000000..b0946a444 --- /dev/null +++ b/app/views/merge_requests/user_outcomes.html.erb @@ -0,0 +1,23 @@ +<% content_for :before_content do %> + <% title = "Users" %> + <% content_for :title, title %> + <%= govuk_back_link href: organisations_path(tab: "merge-requests") %> +<% end %> + +
+ <%= merging_organisations_without_users_text(@merge_request.organisations_without_users) %> +
+ + <% @merge_request.organisations_with_users.map do |org| %> ++ <%= link_to_merging_organisation_users(org) %> +
+ <% end %> +<% end %> diff --git a/spec/helpers/merge_requests_helper_spec.rb b/spec/helpers/merge_requests_helper_spec.rb new file mode 100644 index 000000000..db7ae1b9d --- /dev/null +++ b/spec/helpers/merge_requests_helper_spec.rb @@ -0,0 +1,56 @@ +require "rails_helper" + +RSpec.describe MergeRequestsHelper do + describe "#merging_organisations_without_users_text" do + context "with 1 organisation" do + let(:organisation) { build(:organisation, name: "Org 1") } + + it "returns the correct text" do + expect(merging_organisations_without_users_text([organisation])).to eq("Org 1 has no users.") + end + end + + context "with 2 organisations" do + let(:organisation) { build(:organisation, name: "Org 1") } + let(:organisation_2) { build(:organisation, name: "Org 2") } + + it "returns the correct text" do + expect(merging_organisations_without_users_text([organisation, organisation_2])).to eq("Org 1 and Org 2 have no users.") + end + end + + context "with 3 organisations" do + let(:organisation) { build(:organisation, name: "Org 1") } + let(:organisation_2) { build(:organisation, name: "Org 2") } + let(:organisation_3) { build(:organisation, name: "Org 3") } + + it "returns the correct text" do + expect(merging_organisations_without_users_text([organisation, organisation_2, organisation_3])).to eq("Org 1, Org 2, and Org 3 have no users.") + end + end + end + + describe "#link_to_merging_organisation_users" do + context "with 1 organisation user" do + let(:organisation) { create(:organisation, name: "Org 1") } + + it "returns the correct link" do + expect(link_to_merging_organisation_users(organisation)).to include("View 1 Org 1 user (opens in a new tab)") + expect(link_to_merging_organisation_users(organisation)).to include(users_organisation_path(organisation)) + end + end + + context "with multiple organisation users" do + let(:organisation) { create(:organisation, name: "Org 1") } + + before do + create(:user, organisation:) + end + + it "returns the correct link" do + expect(link_to_merging_organisation_users(organisation)).to include("View all 2 Org 1 users (opens in a new tab)") + expect(link_to_merging_organisation_users(organisation)).to include(users_organisation_path(organisation)) + end + end + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 1f52dd640..9af0671f1 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -89,4 +89,164 @@ RSpec.describe MergeRequest, type: :model do expect(merge_request.status).to eq MergeRequest::STATUS[:processing] end end + + describe "#organisations_with_users" do + context "when absorbing organisation has users" do + let(:merge_request) { create(:merge_request, absorbing_organisation:) } + let(:absorbing_organisation) { create(:organisation) } + + before do + create(:merge_request_organisation, merge_request:, merging_organisation: merging_organisation_1) + create(:merge_request_organisation, merge_request:, merging_organisation: merging_organisation_2) + end + + context "and some merging organisations have users" do + let(:merging_organisation_1) { create(:organisation) } + let(:merging_organisation_2) { create(:organisation, with_dsa: false) } + + it "returns correct organisations with users" do + expect(absorbing_organisation.users.count).to eq(1) + expect(merging_organisation_1.users.count).to eq(1) + expect(merging_organisation_2.users.count).to eq(0) + + expect(merge_request.organisations_with_users.count).to eq(2) + expect(merge_request.organisations_with_users).to include(merging_organisation_1) + expect(merge_request.organisations_with_users).to include(absorbing_organisation) + end + end + + context "and no merging organisations have users" do + let(:merging_organisation_1) { create(:organisation, with_dsa: false) } + let(:merging_organisation_2) { create(:organisation, with_dsa: false) } + + it "returns correct organisations with users" do + expect(absorbing_organisation.users.count).to eq(1) + expect(merging_organisation_1.users.count).to eq(0) + expect(merging_organisation_2.users.count).to eq(0) + + expect(merge_request.organisations_with_users.count).to eq(1) + expect(merge_request.organisations_with_users).to include(absorbing_organisation) + end + end + end + + context "when absorbing organisation has no users" do + let(:merge_request) { create(:merge_request, absorbing_organisation:) } + let(:absorbing_organisation) { create(:organisation, with_dsa: false) } + + before do + create(:merge_request_organisation, merge_request:, merging_organisation: merging_organisation_1) + create(:merge_request_organisation, merge_request:, merging_organisation: merging_organisation_2) + end + + context "and some merging organisations have users" do + let(:merging_organisation_1) { create(:organisation) } + let(:merging_organisation_2) { create(:organisation, with_dsa: false) } + + it "returns correct organisations with users" do + expect(merging_organisation_1.users.count).to eq(1) + expect(absorbing_organisation.users.count).to eq(0) + expect(merging_organisation_2.users.count).to eq(0) + + expect(merge_request.organisations_with_users.count).to eq(1) + expect(merge_request.organisations_with_users).to include(merging_organisation_1) + end + end + + context "and no merging organisations have users" do + let(:merging_organisation_1) { create(:organisation, with_dsa: false) } + let(:merging_organisation_2) { create(:organisation, with_dsa: false) } + + it "returns correct organisations with users" do + expect(absorbing_organisation.users.count).to eq(0) + expect(merging_organisation_1.users.count).to eq(0) + expect(merging_organisation_2.users.count).to eq(0) + + expect(merge_request.organisations_with_users.count).to eq(0) + end + end + end + end + + describe "#organisations_without_users" do + context "when absorbing organisation has users" do + let(:merge_request) { create(:merge_request, absorbing_organisation:) } + let(:absorbing_organisation) { create(:organisation) } + + before do + create(:merge_request_organisation, merge_request:, merging_organisation: merging_organisation_1) + create(:merge_request_organisation, merge_request:, merging_organisation: merging_organisation_2) + end + + context "and some merging organisations have users" do + let(:merging_organisation_1) { create(:organisation) } + let(:merging_organisation_2) { create(:organisation, with_dsa: false) } + + it "returns correct organisations with users" do + expect(absorbing_organisation.users.count).to eq(1) + expect(merging_organisation_1.users.count).to eq(1) + expect(merging_organisation_2.users.count).to eq(0) + + expect(merge_request.organisations_without_users.count).to eq(1) + expect(merge_request.organisations_without_users).to include(merging_organisation_2) + end + end + + context "and no merging organisations have users" do + let(:merging_organisation_1) { create(:organisation, with_dsa: false) } + let(:merging_organisation_2) { create(:organisation, with_dsa: false) } + + it "returns correct organisations with users" do + expect(absorbing_organisation.users.count).to eq(1) + expect(merging_organisation_1.users.count).to eq(0) + expect(merging_organisation_2.users.count).to eq(0) + + expect(merge_request.organisations_without_users.count).to eq(2) + expect(merge_request.organisations_without_users).to include(merging_organisation_1) + expect(merge_request.organisations_without_users).to include(merging_organisation_2) + end + end + end + + context "when absorbing organisation has no users" do + let(:merge_request) { create(:merge_request, absorbing_organisation:) } + let(:absorbing_organisation) { create(:organisation, with_dsa: false) } + + before do + create(:merge_request_organisation, merge_request:, merging_organisation: merging_organisation_1) + create(:merge_request_organisation, merge_request:, merging_organisation: merging_organisation_2) + end + + context "and some merging organisations have users" do + let(:merging_organisation_1) { create(:organisation) } + let(:merging_organisation_2) { create(:organisation, with_dsa: false) } + + it "returns correct organisations with users" do + expect(merging_organisation_1.users.count).to eq(1) + expect(absorbing_organisation.users.count).to eq(0) + expect(merging_organisation_2.users.count).to eq(0) + + expect(merge_request.organisations_without_users.count).to eq(2) + expect(merge_request.organisations_without_users).to include(absorbing_organisation) + expect(merge_request.organisations_without_users).to include(merging_organisation_2) + end + end + + context "and no merging organisations have users" do + let(:merging_organisation_1) { create(:organisation, with_dsa: false) } + let(:merging_organisation_2) { create(:organisation, with_dsa: false) } + + it "returns correct organisations with users" do + expect(absorbing_organisation.users.count).to eq(0) + expect(merging_organisation_1.users.count).to eq(0) + expect(merging_organisation_2.users.count).to eq(0) + + expect(merge_request.organisations_without_users.count).to eq(3) + expect(merge_request.organisations_without_users).to include(absorbing_organisation) + expect(merge_request.organisations_without_users).to include(merging_organisation_1) + expect(merge_request.organisations_without_users).to include(merging_organisation_2) + end + end + end + end end diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb index 4cc4c0376..a0d87a0da 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/spec/requests/merge_requests_controller_spec.rb @@ -504,6 +504,32 @@ RSpec.describe MergeRequestsController, type: :request do end end end + + describe "#user_outcomes" do + let(:merge_request) { create(:merge_request, absorbing_organisation: organisation) } + let(:organisation_with_no_users) { create(:organisation, name: "Organisation with no users", with_dsa: false) } + let(:organisation_with_no_users_too) { create(:organisation, name: "Organisation with no users too", with_dsa: false) } + let(:organisation_with_some_users) { create(:organisation, name: "Organisation with some users", with_dsa: false) } + let(:organisation_with_some_more_users) { create(:organisation, name: "Organisation with many users", with_dsa: false) } + + before do + create_list(:user, 4, organisation: organisation_with_some_users) + create_list(:user, 12, organisation: organisation_with_some_more_users) + create(:merge_request_organisation, merge_request:, merging_organisation: organisation_with_no_users) + create(:merge_request_organisation, merge_request:, merging_organisation: organisation_with_no_users_too) + create(:merge_request_organisation, merge_request:, merging_organisation: organisation_with_some_users) + create(:merge_request_organisation, merge_request:, merging_organisation: organisation_with_some_more_users) + get "/merge-request/#{merge_request.id}/user-outcomes", headers: + end + + it "shows user outcomes after merge" do + expect(page).to have_link("View all 4 Organisation with some users users (opens in a new tab)", href: users_organisation_path(organisation_with_some_users)) + expect(page).to have_link("View all 12 Organisation with many users users (opens in a new tab)", href: users_organisation_path(organisation_with_some_more_users)) + expect(page).to have_link("View all 3 MHCLG users (opens in a new tab)", href: users_organisation_path(organisation)) + expect(page).to have_content("Organisation with no users and Organisation with no users too have no users.") + expect(page).to have_content("19 users after merge") + end + end end context "when user is signed in with a data coordinator user" do