+ <%= 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/config/routes.rb b/config/routes.rb index 96d738062..63bafbd51 100644 --- a/config/routes.rb +++ b/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" 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/jobs/process_merge_request_job_spec.rb b/spec/jobs/process_merge_request_job_spec.rb index 071285c55..a8373ca5e 100644 --- a/spec/jobs/process_merge_request_job_spec.rb +++ b/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 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 0c84b3c95..cad862688 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/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