CLDC 2094: View a merge request (#2565)

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

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("<br>").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<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" } },
+    ]
+  end
+end

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

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

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? %>
+          <div class="govuk-!-margin-left-8 govuk-!-margin-right-4">
+            <%= raw(detail[:value]) %>
+          </div>
+          <% elsif detail[:value].is_a?(Date) || detail[:value].is_a?(Time) %>
+          <div class="govuk-!-margin-left-8 govuk-!-margin-right-4">
+            <%= detail[:value].strftime("%d %B %Y") %>
+          </div>
+          <% 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 %>

app/views/merge_requests/_merge_request_list.html.erb

@@ -3,7 +3,7 @@
     <p>No merge requests</p>
   <% 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 %>
       <strong><%= @merge_requests.where.not(status: 4).count %></strong> 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(html_attributes: { scope: "row" }) do %>
-              <% row.with_cell(text: "You didn't answer this question",
+              <%= display_value_or_placeholder(merge_request.absorbing_organisation_name) %>
-                               html_attributes: {
-                                  scope: "row",
-                                  class: "app-!-colour-muted",
-                               }) %>
-            <% else %>
-              <% row.with_cell(text: merge_request.absorbing_organisation_name) %>
             <% end %>
             <% merge_date = merge_request.merge_date %>
-            <% if merge_date.nil? %>
+            <%= row.with_cell(html_attributes: { scope: "row" }) do %>
-              <% row.with_cell(text: "You didn't answer this question",
+              <%= display_value_or_placeholder(merge_date&.strftime("%d %B %Y")) %>
-                               html_attributes: {
-                                 scope: "row",
-                                 class: "app-!-colour-muted",
-                               }) %>
-            <% else %>
-              <% row.with_cell(text: 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 %>

app/views/merge_requests/_summary_card.html.erb

@@ -0,0 +1,8 @@
+<div class="govuk-summary-card govuk-!-margin-bottom-6 govuk-!-width-three-quarters">
+  <div class="govuk-summary-card__title-wrapper">
+    <h3 class="govuk-summary-card__title govuk-!-font-weight-regular"><%= title %></h3>
+  </div>
+  <div class="govuk-summary-card__content">
+    <%= render partial: "merge_requests/details_list", locals: { details: } %>
+  </div>
+</div>

app/views/merge_requests/organisations.html.erb

@@ -48,4 +48,5 @@
       <%= f.govuk_submit "Continue" %>
     <% end %>
   <% end %>
+  </div>
 </div>

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? %>
+  <div class="govuk-notification-banner" role="region" aria-labelledby="govuk-notification-banner-title" data-module="govuk-notification-banner">
+    <div class="govuk-notification-banner__header">
+      <h2 class="govuk-notification-banner__title" id="govuk-notification-banner-title">
+        Important
+      </h2>
+    </div>
+    <div class="govuk-notification-banner__content">
+      <strong>The absorbing organisation must accept the Data Sharing Agreement before merging.</strong>
+      <br>
+      <% 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 %>
+    </div>
+  </div>
+<% end %>
+<h1 class="govuk-heading-l">
+  <span class="govuk-caption-l">Merge details</span>
+  <%= display_value_or_placeholder(@merge_request.absorbing_organisation_name) %>
+</h1>
+<% unless @merge_request.status == "request_merged" %>
+<div class="govuk-button-group">
+  <button type="submit" <%= @merge_request.status == "ready_to_merge" ? "" : "disabled aria-disabled=\"true\"" %> class="govuk-button" data-module="govuk-button">
+    Begin merge
+  </button>
+  <button type="submit" class="govuk-button govuk-button--warning" data-module="govuk-button">
+    Delete merge request
+  </button>
+</div>
+<% 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 %>

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 %>
 </div>

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
 

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

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|

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

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