diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb
index 5d2225a6e..04e100ce6 100644
--- a/app/controllers/organisations_controller.rb
+++ b/app/controllers/organisations_controller.rb
@@ -238,6 +238,7 @@ class OrganisationsController < ApplicationController
def merge_request
@merge_request = MergeRequest.new
+ render "merge_requests/merge_request"
end
def data_sharing_agreement
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
new file mode 100644
index 000000000..9f6afe1df
--- /dev/null
+++ b/app/helpers/merge_requests_helper.rb
@@ -0,0 +1,30 @@
+module MergeRequestsHelper
+ def display_value_or_placeholder(value, placeholder = "You didn't answer this question")
+ value.presence || content_tag(:span, placeholder, class: "app-!-colour-muted")
+ end
+
+ def request_details(merge_request)
+ [
+ { label: "Requester", value: display_value_or_placeholder(merge_request.requester&.name) },
+ { label: "Helpdesk ticket", value: merge_request.helpdesk_ticket.present? ? link_to("#{merge_request.helpdesk_ticket} (opens in a new tab)", "https://dluhcdigital.atlassian.net/browse/#{merge_request.helpdesk_ticket}", target: "_blank", rel: "noopener noreferrer") : display_value_or_placeholder(nil), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: "#", visually_hidden_text: "helpdesk ticket" } },
+ { label: "Status", value: status_tag(merge_request.status) },
+ ]
+ end
+
+ def merge_details(merge_request)
+ [
+ { label: "Absorbing organisation", value: display_value_or_placeholder(merge_request.absorbing_organisation_name), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: absorbing_organisation_merge_request_path(merge_request), visually_hidden_text: "absorbing organisation" } },
+ { label: "Merging organisations", value: merge_request.merge_request_organisations.any? ? merge_request.merge_request_organisations.map(&:merging_organisation_name).join(" ").html_safe : display_value_or_placeholder(nil), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: organisations_merge_request_path(merge_request), visually_hidden_text: "merging organisations" } },
+ { label: "Merge date", value: display_value_or_placeholder(merge_request.merge_date), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: merge_date_merge_request_path(merge_request), visually_hidden_text: "merge date" } },
+ ]
+ end
+
+ 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 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 #{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" } },
+ ]
+ end
+end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 42496a8fe..ec844576b 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -3,6 +3,7 @@ class MergeRequest < ApplicationRecord
has_many :merge_request_organisations
belongs_to :absorbing_organisation, class_name: "Organisation", optional: true
has_many :merging_organisations, through: :merge_request_organisations, source: :merging_organisation
+ belongs_to :requester, class_name: "User", optional: true
validate :organisation_name_uniqueness, if: :new_organisation_name
validates :new_telephone_number, presence: true, if: -> { telephone_number_correct == false }
@@ -31,4 +32,8 @@ class MergeRequest < ApplicationRecord
def absorbing_organisation_name
absorbing_organisation&.name || ""
end
+
+ def dpo_user
+ absorbing_organisation.data_protection_officers.filter_by_active.first
+ end
end
diff --git a/app/models/merge_request_organisation.rb b/app/models/merge_request_organisation.rb
index 567b3f654..fd57b5e51 100644
--- a/app/models/merge_request_organisation.rb
+++ b/app/models/merge_request_organisation.rb
@@ -10,6 +10,10 @@ class MergeRequestOrganisation < ApplicationRecord
has_paper_trail
+ def merging_organisation_name
+ merging_organisation.name || ""
+ end
+
private
def validate_merging_organisations
diff --git a/app/views/merge_requests/_details_list.html.erb b/app/views/merge_requests/_details_list.html.erb
new file mode 100644
index 000000000..e93278b08
--- /dev/null
+++ b/app/views/merge_requests/_details_list.html.erb
@@ -0,0 +1,33 @@
+<%= govuk_summary_list do |summary_list| %>
+ <% details.each do |detail| %>
+ <% summary_list.with_row do |row| %>
+ <% row.with_key { detail[:label] } %>
+
+ <% row.with_value do %>
+ <% if detail[:value].html_safe? %>
+
+ <%= raw(detail[:value]) %>
+
+ <% elsif detail[:value].is_a?(Date) || detail[:value].is_a?(Time) %>
+
+ <%= detail[:value].strftime("%d %B %Y") %>
+
+ <% else %>
+ <%= simple_format(
+ detail[:value],
+ wrapper_tag: "span",
+ class: "govuk-!-margin-left-8 govuk-!-margin-right-4",
+ ) %>
+ <% end %>
+ <% end %>
+
+ <% if detail[:action].present? %>
+ <% row.with_action(
+ text: detail[:action][:text],
+ href: detail[:action][:href],
+ visually_hidden_text: detail[:action][:visually_hidden_text],
+ ) %>
+ <% end %>
+ <% end %>
+ <% end %>
+<% end %>
diff --git a/app/views/organisations/_merge_request_list.html.erb b/app/views/merge_requests/_merge_request_list.html.erb
similarity index 59%
rename from app/views/organisations/_merge_request_list.html.erb
rename to app/views/merge_requests/_merge_request_list.html.erb
index 6e4bab9fa..e048472a7 100644
--- a/app/views/organisations/_merge_request_list.html.erb
+++ b/app/views/merge_requests/_merge_request_list.html.erb
@@ -3,7 +3,7 @@
No merge requests
<% else %>
<%= govuk_table do |table| %>
- <%= table.with_caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %>
+ <%= table.with_caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do %>
<%= @merge_requests.where.not(status: 4).count %> unresolved merge requests
<% end %>
<%= table.with_head do |head| %>
@@ -25,30 +25,18 @@
<% @merge_requests.each do |merge_request| %>
<%= table.with_body do |body| %>
<%= body.with_row do |row| %>
- <% if merge_request.absorbing_organisation_name.blank? %>
- <% row.with_cell(text: "You didn't answer this question",
- html_attributes: {
- scope: "row",
- class: "app-!-colour-muted",
- }) %>
- <% else %>
- <% row.with_cell(text: merge_request.absorbing_organisation_name) %>
+ <%= row.with_cell(html_attributes: { scope: "row" }) do %>
+ <%= display_value_or_placeholder(merge_request.absorbing_organisation_name) %>
<% end %>
<% merge_date = merge_request.merge_date %>
- <% if merge_date.nil? %>
- <% row.with_cell(text: "You didn't answer this question",
- html_attributes: {
- scope: "row",
- class: "app-!-colour-muted",
- }) %>
- <% else %>
- <% row.with_cell(text: merge_date.strftime("%d %B %Y")) %>
+ <%= row.with_cell(html_attributes: { scope: "row" }) do %>
+ <%= display_value_or_placeholder(merge_date&.strftime("%d %B %Y")) %>
<% end %>
<% row.with_cell(text: status_tag(merge_request.status)) %>
<% row.with_cell(html_attributes: {
scope: "row",
}) do %>
- <%= govuk_link_to("View details", "/merge-request/#{merge_request.id}/organisations") %>
+ <%= govuk_link_to("View details", merge_request_path(merge_request.id)) %>
<% end %>
<% end %>
<% end %>
diff --git a/app/views/merge_requests/_summary_card.html.erb b/app/views/merge_requests/_summary_card.html.erb
new file mode 100644
index 000000000..4a372f0a0
--- /dev/null
+++ b/app/views/merge_requests/_summary_card.html.erb
@@ -0,0 +1,8 @@
+
+
+
<%= title %>
+
+
+ <%= render partial: "merge_requests/details_list", locals: { details: } %>
+
+
diff --git a/app/views/organisations/merge_request.html.erb b/app/views/merge_requests/merge_request.html.erb
similarity index 100%
rename from app/views/organisations/merge_request.html.erb
rename to app/views/merge_requests/merge_request.html.erb
diff --git a/app/views/merge_requests/organisations.html.erb b/app/views/merge_requests/organisations.html.erb
index 8a47a8641..6b6a0783c 100644
--- a/app/views/merge_requests/organisations.html.erb
+++ b/app/views/merge_requests/organisations.html.erb
@@ -48,4 +48,5 @@
<%= f.govuk_submit "Continue" %>
<% end %>
<% end %>
+
diff --git a/app/views/merge_requests/show.html.erb b/app/views/merge_requests/show.html.erb
new file mode 100644
index 000000000..b28ee3c1e
--- /dev/null
+++ b/app/views/merge_requests/show.html.erb
@@ -0,0 +1,45 @@
+<% content_for :before_content do %>
+ <% title = "Merge details: #{@merge_request.absorbing_organisation_name}" %>
+ <% content_for :title, title %>
+ <%= govuk_back_link href: "#{organisations_path}#merge-requests" %>
+<% end %>
+<% unless @merge_request.signed_dsa || @merge_request.absorbing_organisation_id.blank? %>
+
+
+
+ The absorbing organisation must accept the Data Sharing Agreement before merging.
+
+ <% if @merge_request.dpo_user %>
+ Contact the Data Protection Officer: <%= link_to @merge_request.dpo_user.name, user_path(@merge_request.dpo_user.id) %>
+ <% else %>
+ <%= @merge_request.absorbing_organisation_name %> does not have a Data Protection Officer. You can assign one on the <%= link_to "users page", "#{organisation_path(@merge_request.absorbing_organisation_id)}/users" %>.
+ <% end %>
+
+
+<% end %>
+
+ Merge details
+ <%= display_value_or_placeholder(@merge_request.absorbing_organisation_name) %>
+
+<% unless @merge_request.status == "request_merged" %>
+
+ class="govuk-button" data-module="govuk-button">
+ Begin merge
+
+
+ Delete merge request
+
+
+<% end %>
+
+<%= render partial: "merge_requests/summary_card", locals: { title: "Request details", details: request_details(@merge_request) } %>
+
+<%= render partial: "merge_requests/summary_card", locals: { title: "Merge details", details: merge_details(@merge_request) } %>
+
+<% unless @merge_request.status == "incomplete" %>
+ <%= render partial: "merge_requests/summary_card", locals: { title: "Merge outcomes", details: merge_outcomes(@merge_request) } %>
+<% end %>
diff --git a/app/views/organisations/index.html.erb b/app/views/organisations/index.html.erb
index d95516eba..64c311472 100644
--- a/app/views/organisations/index.html.erb
+++ b/app/views/organisations/index.html.erb
@@ -16,7 +16,7 @@
<% end %>
<% c.with_tab(label: "Merge requests") do %>
<%= govuk_button_to "Create new merge request", merge_requests_path, html: { method: :post } %>
- <%= render partial: "merge_request_list", locals: { merge_requests: @merge_requests, title: "Merge requests", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count } %>
+ <%= render partial: "merge_requests/merge_request_list", locals: { merge_requests: @merge_requests, title: "Merge requests", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count } %>
<% end %>
<% end %>
diff --git a/config/routes.rb b/config/routes.rb
index e0d9631e9..fbb59a7d0 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -205,6 +205,7 @@ Rails.application.routes.draw do
get "new-organisation-telephone-number"
get "new-organisation-type"
get "merge-date"
+ get "details", to: "merge_requests#details"
end
end
diff --git a/db/migrate/20240809154241_add_additional_fields_to_merge_requests.rb b/db/migrate/20240809154241_add_additional_fields_to_merge_requests.rb
new file mode 100644
index 000000000..6f2a37fd0
--- /dev/null
+++ b/db/migrate/20240809154241_add_additional_fields_to_merge_requests.rb
@@ -0,0 +1,16 @@
+class AddAdditionalFieldsToMergeRequests < ActiveRecord::Migration[7.0]
+ def change
+ change_table :merge_requests, bulk: true do |t|
+ t.integer :requester_id
+ t.string :helpdesk_ticket
+ t.integer :total_users
+ t.integer :total_schemes
+ t.integer :total_lettings_logs
+ t.integer :total_sales_logs
+ t.integer :total_stock_owners
+ t.integer :total_managing_agents
+ t.boolean :signed_dsa, default: false
+ t.datetime :discarded_at
+ end
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f9a17bd49..8296332f5 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema[7.0].define(version: 2024_08_08_134014) do
+ActiveRecord::Schema[7.0].define(version: 2024_08_09_154241) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -431,6 +431,16 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_08_134014) do
t.string "new_organisation_postcode"
t.string "new_organisation_telephone_number"
t.datetime "merge_date"
+ t.integer "requester_id"
+ t.string "helpdesk_ticket"
+ t.integer "total_users"
+ t.integer "total_schemes"
+ t.integer "total_lettings_logs"
+ t.integer "total_sales_logs"
+ t.integer "total_stock_owners"
+ t.integer "total_managing_agents"
+ t.boolean "signed_dsa", default: false
+ t.datetime "discarded_at"
end
create_table "notifications", force: :cascade do |t|
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index 888639b82..19020fce1 100644
--- a/spec/factories/merge_requests.rb
+++ b/spec/factories/merge_requests.rb
@@ -2,6 +2,7 @@ FactoryBot.define do
factory :merge_request do
status { "incomplete" }
merge_date { nil }
+ helpdesk_ticket { "MSD-99999" }
association :requesting_organisation, factory: :organisation
end
end
diff --git a/spec/views/merge_requests/show.html.erb_spec.rb b/spec/views/merge_requests/show.html.erb_spec.rb
new file mode 100644
index 000000000..0f0e42b2d
--- /dev/null
+++ b/spec/views/merge_requests/show.html.erb_spec.rb
@@ -0,0 +1,104 @@
+require "rails_helper"
+
+RSpec.describe "merge_requests/show.html.erb", type: :view do
+ let(:absorbing_organisation) { create(:organisation, name: "Absorbing Org") }
+ let(:dpo_user) { create(:user, name: "DPO User", is_dpo: true, organisation: absorbing_organisation) }
+ let(:merge_request) { create(:merge_request, absorbing_organisation_id: absorbing_organisation.id, signed_dsa: false, status: 1) }
+
+ before do
+ assign(:merge_request, merge_request)
+ render
+ end
+
+ it "displays the correct title" do
+ expect(rendered).to have_selector("h1.govuk-heading-l") do |h1|
+ expect(h1).to have_selector("span.govuk-caption-l", text: "Merge details")
+ expect(h1).to have_content("Absorbing Org")
+ end
+ end
+
+ it "displays the notification banner when DSA is not signed" do
+ expect(rendered).to have_selector(".govuk-notification-banner")
+ expect(rendered).to have_content("The absorbing organisation must accept the Data Sharing Agreement before merging.")
+ end
+
+ it "displays the requester details" do
+ expect(rendered).to have_selector("dt", text: "Requester")
+ expect(rendered).to have_selector("dd", text: merge_request.requester&.name || "You didn't answer this question")
+ end
+
+ it "displays the helpdesk ticket details" do
+ expect(rendered).to have_selector("dt", text: "Helpdesk ticket")
+ if merge_request.helpdesk_ticket.present?
+ expect(rendered).to have_link(merge_request.helpdesk_ticket, href: "https://dluhcdigital.atlassian.net/browse/#{merge_request.helpdesk_ticket}")
+ else
+ expect(rendered).to have_selector("dd", text: "You didn't answer this question")
+ end
+ end
+
+ it "displays the status details" do
+ expect(rendered).to have_selector("dt", text: "Status")
+ expect(rendered).to have_selector("dd", text: "Incomplete")
+ end
+
+ it "displays the absorbing organisation details" do
+ expect(rendered).to have_selector("dt", text: "Absorbing organisation")
+ expect(rendered).to have_selector("dd", text: merge_request.absorbing_organisation_name)
+ end
+
+ it "displays the merging organisations details" do
+ expect(rendered).to have_selector("dt", text: "Merging organisations")
+ if merge_request.other_merging_organisations.present?
+ merge_request.other_merging_organisations.split(",").each do |organisation|
+ expect(rendered).to have_selector("dd", text: organisation.strip)
+ end
+ end
+ end
+
+ it "displays the merge date details" do
+ expect(rendered).to have_selector("dt", text: "Merge date")
+ expect(rendered).to have_selector("dd", text: merge_request.merge_date || "You didn't answer this question")
+ end
+
+ context "when the merge request is complete" do
+ before do
+ merge_request.update!(status: 4, signed_dsa: true, total_users: 10, total_schemes: 5, total_lettings_logs: 20, total_sales_logs: 30, total_stock_owners: 40, total_managing_agents: 50)
+ assign(:merge_request, merge_request)
+ render
+ end
+
+ it "has status of 'Merged'" do
+ expect(rendered).to have_selector("dd", text: "Merged")
+ end
+
+ it "displays the total users after merge details" do
+ expect(rendered).to have_selector("dt", text: "Total users after merge")
+ expect(rendered).to have_selector("dd", text: merge_request.total_users)
+ end
+
+ it "displays the total schemes after merge details" do
+ expect(rendered).to have_selector("dt", text: "Total schemes after merge")
+ expect(rendered).to have_selector("dd", text: merge_request.total_schemes)
+ end
+
+ it "displays the total logs after merge details" do
+ expect(rendered).to have_selector("dt", text: "Total logs after merge")
+ if merge_request.total_lettings_logs.present? || merge_request.total_sales_logs.present?
+ combined_text = []
+ combined_text << "#{merge_request.total_lettings_logs} lettings logs" if merge_request.total_lettings_logs.present?
+ combined_text << "#{merge_request.total_sales_logs} sales logs" if merge_request.total_sales_logs.present?
+ expect(rendered).to have_selector("dd", text: combined_text.join(""))
+ end
+ end
+
+ 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
+ end
+ end
+end