diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb
index 51317bcfd..c34e00581 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 scheme_outcomes; end
def create
ActiveRecord::Base.transaction do
@@ -63,7 +64,7 @@ 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)
+ @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)
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 6fd078553..23623b0a6 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -25,7 +25,7 @@ module MergeRequestsHelper
def merge_outcomes(merge_request)
[
{ 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 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" } },
]
@@ -107,4 +107,24 @@ module MergeRequestsHelper
count = merge_request.total_visible_users_after_merge
"#{"#{count} user".pluralize(count)} after merge"
end
+
+ def merging_organisations_without_schemes_text(organisations)
+ return "" unless organisations.count.positive?
+
+ if organisations.count == 1
+ "#{organisations.first.name} has no schemes."
+ else
+ "#{organisations.map(&:name).to_sentence} have no schemes."
+ end
+ end
+
+ def link_to_merging_organisation_schemes(organisation)
+ count_text = organisation.owned_schemes.count == 1 ? "1 #{organisation.name} scheme" : "all #{organisation.owned_schemes.count} #{organisation.name} schemes"
+ govuk_link_to "View #{count_text} (opens in a new tab)", schemes_organisation_path(organisation), target: "_blank"
+ end
+
+ def total_schemes_after_merge_text(merge_request)
+ count = merge_request.total_visible_schemes_after_merge
+ "#{"#{count} scheme".pluralize(count)} after merge"
+ end
end
diff --git a/app/jobs/process_merge_request_job.rb b/app/jobs/process_merge_request_job.rb
index 62c5109cc..be506e1e4 100644
--- a/app/jobs/process_merge_request_job.rb
+++ b/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, total_users: nil)
+ merge_request.update!(last_failed_attempt: Time.zone.now, processing: false, total_users: nil, total_schemes: nil)
end
end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index aae36195d..fe45bd18e 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -76,4 +76,26 @@ class MergeRequest < ApplicationRecord
([absorbing_organisation] + merging_organisations).reject(&:has_visible_users?)
end
+
+ def total_visible_schemes_after_merge
+ return total_schemes if status == STATUS[:request_merged] || status == STATUS[:processing]
+
+ absorbing_organisation.owned_schemes.visible.count + merging_organisations.sum { |org| org.owned_schemes.visible.count }
+ end
+
+ def total_schemes_label
+ "#{total_visible_schemes_after_merge} #{'Scheme'.pluralize(total_visible_schemes_after_merge)}"
+ end
+
+ def organisations_with_schemes
+ return [] unless absorbing_organisation.present? && merging_organisations.any?
+
+ ([absorbing_organisation] + merging_organisations).select(&:has_visible_schemes?)
+ end
+
+ def organisations_without_schemes
+ return [] unless absorbing_organisation.present? && merging_organisations.any?
+
+ ([absorbing_organisation] + merging_organisations).reject(&:has_visible_schemes?)
+ end
end
diff --git a/app/models/organisation.rb b/app/models/organisation.rb
index 6e75b79e5..e769f7b3d 100644
--- a/app/models/organisation.rb
+++ b/app/models/organisation.rb
@@ -195,4 +195,8 @@ class Organisation < ApplicationRecord
def has_visible_users?
users.visible.count.positive?
end
+
+ def has_visible_schemes?
+ owned_schemes.visible.count.positive?
+ end
end
diff --git a/app/views/merge_requests/scheme_outcomes.html.erb b/app/views/merge_requests/scheme_outcomes.html.erb
new file mode 100644
index 000000000..9669f5dc8
--- /dev/null
+++ b/app/views/merge_requests/scheme_outcomes.html.erb
@@ -0,0 +1,23 @@
+<% content_for :before_content do %>
+ <% title = "Schemes" %>
+ <% content_for :title, title %>
+ <%= govuk_back_link href: organisations_path(tab: "merge-requests") %>
+<% end %>
+
+
+ <%= merging_organisations_without_schemes_text(@merge_request.organisations_without_schemes) %> +
+ + <% @merge_request.organisations_with_schemes.map do |org| %> ++ <%= link_to_merging_organisation_schemes(org) %> +
+ <% end %> +<% end %> diff --git a/config/routes.rb b/config/routes.rb index 63bafbd51..d8e61c04b 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 "scheme-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 index db7ae1b9d..1f4bf5733 100644 --- a/spec/helpers/merge_requests_helper_spec.rb +++ b/spec/helpers/merge_requests_helper_spec.rb @@ -53,4 +53,61 @@ RSpec.describe MergeRequestsHelper do end end end + + describe "#merging_organisations_without_schemes_text" do + context "with 1 organisation" do + let(:organisation) { build(:organisation, name: "Org 1") } + + it "returns the correct text" do + expect(merging_organisations_without_schemes_text([organisation])).to eq("Org 1 has no schemes.") + 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_schemes_text([organisation, organisation_2])).to eq("Org 1 and Org 2 have no schemes.") + 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_schemes_text([organisation, organisation_2, organisation_3])).to eq("Org 1, Org 2, and Org 3 have no schemes.") + end + end + end + + describe "#link_to_merging_organisation_schemes" do + context "with 1 organisation scheme" do + let(:organisation) { create(:organisation, name: "Org 1") } + + before do + create(:scheme, owning_organisation: organisation) + end + + it "returns the correct link" do + expect(link_to_merging_organisation_schemes(organisation)).to include("View 1 Org 1 scheme (opens in a new tab)") + expect(link_to_merging_organisation_schemes(organisation)).to include(schemes_organisation_path(organisation)) + end + end + + context "with multiple organisation schemes" do + let(:organisation) { create(:organisation, name: "Org 1") } + + before do + create_list(:scheme, 2, owning_organisation: organisation) + end + + it "returns the correct link" do + expect(link_to_merging_organisation_schemes(organisation)).to include("View all 2 Org 1 schemes (opens in a new tab)") + expect(link_to_merging_organisation_schemes(organisation)).to include(schemes_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 a8373ca5e..c026db6f1 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), total_users: 5) } + let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: organisation, merge_date: Time.zone.local(2022, 3, 3), total_users: 5, total_schemes: 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, sets processing to false and clears total_users if there's an error" do + it "sets last_failed_attempt value, sets processing to false and clears total_schemes and 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 @@ -37,6 +37,7 @@ describe ProcessMergeRequestJob do 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 + expect(merge_request.total_schemes).to be_nil end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 9af0671f1..554ab7a02 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -168,6 +168,78 @@ RSpec.describe MergeRequest, type: :model do end end + describe "#organisations_with_schemes" do + let(:merge_request) { create(:merge_request, absorbing_organisation:) } + let(:absorbing_organisation) { create(:organisation) } + let(:merging_organisation_1) { create(:organisation) } + let(:merging_organisation_2) { 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 "when absorbing organisation has schemes" do + before do + create(:scheme, owning_organisation: absorbing_organisation) + end + + context "and some merging organisations have schemes" do + before do + create(:scheme, owning_organisation: merging_organisation_1) + end + + it "returns correct organisations with schemes" do + expect(absorbing_organisation.owned_schemes.count).to eq(1) + expect(merging_organisation_1.owned_schemes.count).to eq(1) + expect(merging_organisation_2.owned_schemes.count).to eq(0) + + expect(merge_request.organisations_with_schemes.count).to eq(2) + expect(merge_request.organisations_with_schemes).to include(merging_organisation_1) + expect(merge_request.organisations_with_schemes).to include(absorbing_organisation) + end + end + + context "and no merging organisations have schemes" do + it "returns correct organisations with schemes" do + expect(absorbing_organisation.owned_schemes.count).to eq(1) + expect(merging_organisation_1.owned_schemes.count).to eq(0) + expect(merging_organisation_2.owned_schemes.count).to eq(0) + + expect(merge_request.organisations_with_schemes.count).to eq(1) + expect(merge_request.organisations_with_schemes).to include(absorbing_organisation) + end + end + end + + context "when absorbing organisation has no schemes" do + context "and some merging organisations have schemes" do + before do + create(:scheme, owning_organisation: merging_organisation_1) + end + + it "returns correct organisations with schemes" do + expect(merging_organisation_1.owned_schemes.count).to eq(1) + expect(absorbing_organisation.owned_schemes.count).to eq(0) + expect(merging_organisation_2.owned_schemes.count).to eq(0) + + expect(merge_request.organisations_with_schemes.count).to eq(1) + expect(merge_request.organisations_with_schemes).to include(merging_organisation_1) + end + end + + context "and no merging organisations have schemes" do + it "returns correct organisations with schemes" do + expect(absorbing_organisation.owned_schemes.count).to eq(0) + expect(merging_organisation_1.owned_schemes.count).to eq(0) + expect(merging_organisation_2.owned_schemes.count).to eq(0) + + expect(merge_request.organisations_with_schemes.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:) } @@ -249,4 +321,80 @@ RSpec.describe MergeRequest, type: :model do end end end + + describe "#organisations_without_schemes" do + let(:merge_request) { create(:merge_request, absorbing_organisation:) } + let(:absorbing_organisation) { create(:organisation) } + let(:merging_organisation_1) { create(:organisation) } + let(:merging_organisation_2) { 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 "when absorbing organisation has schemes" do + before do + create(:scheme, owning_organisation: absorbing_organisation) + end + + context "and some merging organisations have schemes" do + before do + create(:scheme, owning_organisation: merging_organisation_1) + end + + it "returns correct organisations with schemes" do + expect(absorbing_organisation.owned_schemes.count).to eq(1) + expect(merging_organisation_1.owned_schemes.count).to eq(1) + expect(merging_organisation_2.owned_schemes.count).to eq(0) + + expect(merge_request.organisations_without_schemes.count).to eq(1) + expect(merge_request.organisations_without_schemes).to include(merging_organisation_2) + end + end + + context "and no merging organisations have schemes" do + it "returns correct organisations with schemes" do + expect(absorbing_organisation.owned_schemes.count).to eq(1) + expect(merging_organisation_1.owned_schemes.count).to eq(0) + expect(merging_organisation_2.owned_schemes.count).to eq(0) + + expect(merge_request.organisations_without_schemes.count).to eq(2) + expect(merge_request.organisations_without_schemes).to include(merging_organisation_1) + expect(merge_request.organisations_without_schemes).to include(merging_organisation_2) + end + end + end + + context "when absorbing organisation has no schemes" do + context "and some merging organisations have schemes" do + before do + create(:scheme, owning_organisation: merging_organisation_1) + end + + it "returns correct organisations with schemes" do + expect(merging_organisation_1.owned_schemes.count).to eq(1) + expect(absorbing_organisation.owned_schemes.count).to eq(0) + expect(merging_organisation_2.owned_schemes.count).to eq(0) + + expect(merge_request.organisations_without_schemes.count).to eq(2) + expect(merge_request.organisations_without_schemes).to include(absorbing_organisation) + expect(merge_request.organisations_without_schemes).to include(merging_organisation_2) + end + end + + context "and no merging organisations have schemes" do + it "returns correct organisations with schemes" do + expect(absorbing_organisation.owned_schemes.count).to eq(0) + expect(merging_organisation_1.owned_schemes.count).to eq(0) + expect(merging_organisation_2.owned_schemes.count).to eq(0) + + expect(merge_request.organisations_without_schemes.count).to eq(3) + expect(merge_request.organisations_without_schemes).to include(absorbing_organisation) + expect(merge_request.organisations_without_schemes).to include(merging_organisation_1) + expect(merge_request.organisations_without_schemes).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 cad862688..30941360e 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/spec/requests/merge_requests_controller_spec.rb @@ -427,6 +427,7 @@ RSpec.describe MergeRequestsController, type: :request do before do create(:merge_request_organisation, merge_request:, merging_organisation: other_organisation) create(:merge_request_organisation, merge_request:, merging_organisation:) + create_list(:scheme, 2, owning_organisation: organisation) end it "runs the job with correct merge request" do @@ -435,6 +436,7 @@ RSpec.describe MergeRequestsController, type: :request do patch "/merge-request/#{merge_request.id}/start-merge" expect(merge_request.reload.status).to eq("processing") expect(merge_request.total_users).to eq(5) + expect(merge_request.total_schemes).to eq(2) end end @@ -451,6 +453,8 @@ RSpec.describe MergeRequestsController, type: :request do describe "#show" do before do create(:merge_request_organisation, merge_request:, merging_organisation: other_organisation) + create_list(:scheme, 2, owning_organisation: organisation) + create_list(:scheme, 2, owning_organisation: other_organisation) get "/merge-request/#{merge_request.id}", headers: end @@ -479,30 +483,43 @@ RSpec.describe MergeRequestsController, type: :request do 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)) + 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") end end context "with a merged request" do - let(:merge_request) { create(:merge_request, request_merged: true, total_users: 34) } + let(:merge_request) { create(:merge_request, request_merged: true, total_users: 34, total_schemes: 12) } 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 + + 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") + end end context "with a processing request" do - let(:merge_request) { create(:merge_request, processing: true, total_users: 51) } + let(:merge_request) { create(:merge_request, processing: true, total_users: 51, total_schemes: 33) } 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 + + 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") + end end end @@ -531,6 +548,33 @@ RSpec.describe MergeRequestsController, type: :request do expect(page).to have_content("19 users after merge") end end + + describe "#scheme_outcomes" do + let(:merge_request) { create(:merge_request, absorbing_organisation: organisation) } + let(:organisation_with_no_schemes) { create(:organisation, name: "Organisation with no schemes") } + let(:organisation_with_no_schemes_too) { create(:organisation, name: "Organisation with no schemes too") } + let(:organisation_with_some_schemes) { create(:organisation, name: "Organisation with some schemes") } + let(:organisation_with_some_more_schemes) { create(:organisation, name: "Organisation with many schemes") } + + before do + create_list(:scheme, 4, owning_organisation: organisation_with_some_schemes) + create_list(:scheme, 6, owning_organisation: organisation_with_some_more_schemes) + create_list(:scheme, 3, owning_organisation: organisation) + create(:merge_request_organisation, merge_request:, merging_organisation: organisation_with_no_schemes) + create(:merge_request_organisation, merge_request:, merging_organisation: organisation_with_no_schemes_too) + create(:merge_request_organisation, merge_request:, merging_organisation: organisation_with_some_schemes) + create(:merge_request_organisation, merge_request:, merging_organisation: organisation_with_some_more_schemes) + get "/merge-request/#{merge_request.id}/scheme-outcomes", headers: + end + + it "shows scheme outcomes after merge" do + expect(page).to have_link("View all 4 Organisation with some schemes schemes (opens in a new tab)", href: schemes_organisation_path(organisation_with_some_schemes)) + expect(page).to have_link("View all 6 Organisation with many schemes schemes (opens in a new tab)", href: schemes_organisation_path(organisation_with_some_more_schemes)) + expect(page).to have_link("View all 3 MHCLG schemes (opens in a new tab)", href: schemes_organisation_path(organisation)) + expect(page).to have_content("Organisation with no schemes and Organisation with no schemes too have no schemes.") + expect(page).to have_content("13 schemes after merge") + end + end end context "when user is signed in with a data coordinator user" do