CLDC-3603 View merged users outcomes (#2602)

* Update user outcomes view link

* Add user outcomes page

* Set user outcomes before merge and on merge fail

* Update hardcoded total user count

* Account for singular user numbers

app/controllers/merge_requests_controller.rb

@@ -8,6 +8,7 @@ class MergeRequestsController < ApplicationController
   def merge_date; end
   def helpdesk_ticket; end
   def merge_start_confirmation; end
+  def user_outcomes; end
 
   def create
     ActiveRecord::Base.transaction do
@@ -62,7 +63,7 @@ class MergeRequestsController < ApplicationController
 
   def start_merge
     if @merge_request.status == "ready_to_merge"
-      @merge_request.update!(processing: true, last_failed_attempt: nil)
+      @merge_request.update!(processing: true, last_failed_attempt: nil, total_users: @merge_request.total_visible_users_after_merge)
       ProcessMergeRequestJob.perform_later(merge_request: @merge_request)
     end
 

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
@@ -21,7 +24,7 @@ module MergeRequestsHelper
 
   def merge_outcomes(merge_request)
     [
-      { label: "Total users after merge", value: display_value_or_placeholder(merge_request.total_users), action: { text: "View", href: "#", visually_hidden_text: "total users after merge" } },
+      { 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), action: { text: "View", href: "#", visually_hidden_text: "total schemes after merge" } },
       { 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<br>#{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<br>#{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" } },
@@ -75,7 +78,33 @@ module MergeRequestsHelper
     end
   end
 
+  def merge_outcome_action(merge_request, page)
+    unless merge_request.status == "request_merged" || merge_request.status == "processing"
+      { text: "View", href: send("#{page}_merge_request_path", merge_request), visually_hidden_text: page.humanize }
+    end
+  end
+
   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
+
+  def total_users_after_merge_text(merge_request)
+    count = merge_request.total_visible_users_after_merge
+    "#{"#{count} user".pluralize(count)} after merge"
+  end
 end

app/jobs/process_merge_request_job.rb

@@ -9,6 +9,6 @@ class ProcessMergeRequestJob < ApplicationJob
     Merge::MergeOrganisationsService.new(absorbing_organisation_id:, merging_organisation_ids:, merge_date:).call
     merge_request.update!(request_merged: true)
   rescue StandardError
-    merge_request.update!(last_failed_attempt: Time.zone.now, processing: false)
+    merge_request.update!(last_failed_attempt: Time.zone.now, processing: false, total_users: nil)
   end
 end

app/models/merge_request.rb

@@ -54,4 +54,26 @@ class MergeRequest < ApplicationRecord
   def absorbing_organisation_signed_dsa?
     absorbing_organisation&.data_protection_confirmed?
   end
+
+  def total_visible_users_after_merge
+    return total_users if status == STATUS[:request_merged] || status == STATUS[:processing]
+
+    absorbing_organisation.users.visible.count + merging_organisations.sum { |org| org.users.visible.count }
+  end
+
+  def total_users_label
+    "#{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

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

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 %>
+
+<h1 class="govuk-heading-l">
+  <span class="govuk-caption-l"><%= @merge_request.absorbing_organisation_name %></span>
+  Users
+</h1>
+
+<% unless @merge_request.status == "request_merged" || @merge_request.status == "processing" %>
+  <h2 class="govuk-heading-m"><%= total_users_after_merge_text(@merge_request) %></h2>
+   <p class="govuk-body">
+    <%= merging_organisations_without_users_text(@merge_request.organisations_without_users) %>
+  </p>
+
+  <% @merge_request.organisations_with_users.map do |org| %>
+    <p class="govuk-body">
+      <%= link_to_merging_organisation_users(org) %>
+    </p>
+  <% end %>
+<% end %>

config/routes.rb

@@ -210,6 +210,7 @@ Rails.application.routes.draw do
       get "merge-date"
       get "helpdesk-ticket"
       get "merge-start-confirmation"
+      get "user-outcomes"
       get "delete-confirmation", to: "merge_requests#delete_confirmation"
       delete "delete", to: "merge_requests#delete"
       patch "start-merge", to: "merge_requests#start_merge"

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

spec/jobs/process_merge_request_job_spec.rb

@@ -13,7 +13,7 @@ describe ProcessMergeRequestJob do
     let(:organisation) { create(:organisation) }
     let(:merging_organisation) { create(:organisation) }
     let(:other_merging_organisation) { create(:organisation) }
-    let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: organisation, merge_date: Time.zone.local(2022, 3, 3)) }
+    let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: organisation, merge_date: Time.zone.local(2022, 3, 3), total_users: 5) }
 
     before do
       create(:merge_request_organisation, merge_request:, merging_organisation:)
@@ -27,7 +27,7 @@ describe ProcessMergeRequestJob do
       expect(merge_request.reload.status).to eq("request_merged")
     end
 
-    it "sets last_failed_attempt value if there's an error and sets processing to false" do
+    it "sets last_failed_attempt value, sets processing to false and clears total_users if there's an error" do
       allow(merge_organisations_service).to receive(:call).and_raise(ActiveRecord::Rollback)
 
       expect(merge_request.last_failed_attempt).to be_nil
@@ -36,6 +36,7 @@ describe ProcessMergeRequestJob do
       merge_request.reload
       expect(merge_request.last_failed_attempt).to be_within(10.seconds).of(Time.zone.now)
       expect(merge_request.processing).to eq(false)
+      expect(merge_request.total_users).to be_nil
     end
   end
 end

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

spec/requests/merge_requests_controller_spec.rb

@@ -434,6 +434,7 @@ RSpec.describe MergeRequestsController, type: :request do
           expect(ProcessMergeRequestJob).to receive(:perform_later).with(merge_request:).once
           patch "/merge-request/#{merge_request.id}/start-merge"
           expect(merge_request.reload.status).to eq("processing")
+          expect(merge_request.total_users).to eq(5)
         end
       end
 
@@ -449,6 +450,7 @@ RSpec.describe MergeRequestsController, type: :request do
 
     describe "#show" do
       before do
+        create(:merge_request_organisation, merge_request:, merging_organisation: other_organisation)
         get "/merge-request/#{merge_request.id}", headers:
       end
 
@@ -473,6 +475,61 @@ RSpec.describe MergeRequestsController, type: :request do
       it "has begin merge button" do
         expect(page).to have_link("Begin merge", href: merge_start_confirmation_merge_request_path(merge_request))
       end
+
+      context "with unmerged request" do
+        let(:merge_request) { create(:merge_request, absorbing_organisation_id: organisation.id, merge_date: Time.zone.today) }
+
+        it "shows users count and has links to view merge outcomes" do
+          expect(page).to have_link("View", href: user_outcomes_merge_request_path(merge_request))
+          expect(page).to have_content("4 Users")
+        end
+      end
+
+      context "with a merged request" do
+        let(:merge_request) { create(:merge_request, request_merged: true, total_users: 34) }
+
+        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")
+        end
+      end
+
+      context "with a processing request" do
+        let(:merge_request) { create(:merge_request, processing: true, total_users: 51) }
+
+        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")
+        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