CLDC-2093 Merge organisations page - view all merge requests (#2561)

app/controllers/merge_requests_controller.rb

@@ -25,7 +25,7 @@ class MergeRequestsController < ApplicationController
 
   def create
     ActiveRecord::Base.transaction do
-      @merge_request = MergeRequest.create!(merge_request_params.merge(status: :unsubmitted))
+      @merge_request = MergeRequest.create!(merge_request_params.merge(status: :incomplete))
       MergeRequestOrganisation.create!({ merge_request: @merge_request, merging_organisation: @merge_request.requesting_organisation })
     end
     redirect_to organisations_merge_request_path(@merge_request)

app/controllers/organisations_controller.rb

@@ -16,6 +16,9 @@ class OrganisationsController < ApplicationController
 
     all_organisations = Organisation.order(:name)
     @pagy, @organisations = pagy(filtered_collection(all_organisations.visible, search_term))
+    @merge_requests = MergeRequest.visible
+                                  .joins("LEFT JOIN organisations ON organisations.id = merge_requests.absorbing_organisation_id")
+                                  .order("organisations.name")
     @searched = search_term.presence
     @total_count = all_organisations.visible.size
   end

app/helpers/tag_helper.rb

@@ -15,6 +15,10 @@ module TagHelper
     deleted: "Deleted",
     merged: "Merged",
     unconfirmed: "Unconfirmed",
+    merge_issues: "Merge issues",
+    request_merged: "Merged",
+    ready_to_merge: "Ready to merge",
+    processing: "Processing",
   }.freeze
 
   COLOUR = {
@@ -31,6 +35,10 @@ module TagHelper
     deleted: "red",
     merged: "orange",
     unconfirmed: "blue",
+    merge_issues: "orange",
+    request_merged: "green",
+    ready_to_merge: "blue",
+    processing: "yellow",
   }.freeze
 
   def status_tag(status, classes = [])

app/models/merge_request.rb

@@ -7,14 +7,28 @@ class MergeRequest < ApplicationRecord
   validates :new_telephone_number, presence: true, if: -> { telephone_number_correct == false }
 
   STATUS = {
-    "unsubmitted" => 0,
+    "merge_issues" => 0,
-    "submitted" => 1,
+    "incomplete" => 1,
+    "ready_to_merge" => 2,
+    "processing" => 3,
+    "request_merged" => 4,
   }.freeze
   enum status: STATUS
 
+  scope :not_merged, -> { where.not(status: "request_merged") }
+  scope :merged, -> { where(status: "request_merged") }
+  scope :visible, lambda {
+    open_collection_period_start_date = FormHandler.instance.start_date_of_earliest_open_collection_period
+    merged.where("merge_requests.merge_date >= ?", open_collection_period_start_date).or(not_merged)
+  }
+
   def organisation_name_uniqueness
     if Organisation.where("lower(name) = ?", new_organisation_name&.downcase).exists?
       errors.add(:new_organisation_name, :invalid)
     end
   end
+
+  def absorbing_organisation_name
+    absorbing_organisation&.name || ""
+  end
 end

app/models/merge_request_organisation.rb

@@ -5,7 +5,7 @@ class MergeRequestOrganisation < ApplicationRecord
   validates :merging_organisation, presence: { message: I18n.t("validations.merge_request.merging_organisation_id.blank") }
   validate :validate_merging_organisations
 
-  scope :not_unsubmitted, -> { joins(:merge_request).where.not(merge_requests: { status: "unsubmitted" }) }
+  scope :merged, -> { joins(:merge_request).where(merge_requests: { status: "request_merged" }) }
   scope :with_merging_organisation, ->(merging_organisation) { where(merging_organisation:) }
 
   has_paper_trail
@@ -17,12 +17,12 @@ private
       errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
     end
 
-    if MergeRequestOrganisation.not_unsubmitted.with_merging_organisation(merging_organisation).count.positive?
+    if MergeRequestOrganisation.merged.with_merging_organisation(merging_organisation).count.positive?
       errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
       merge_request.errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
     end
 
-    if MergeRequest.not_unsubmitted.where.not(id: merge_request_id).where(requesting_organisation: merging_organisation).count.positive?
+    if MergeRequest.merged.where.not(id: merge_request_id).where(requesting_organisation: merging_organisation).count.positive?
       errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
       merge_request.errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
     end

app/views/organisations/_merge_request_list.html.erb

@@ -0,0 +1,58 @@
+<section class="app-table-group" tabindex="0" aria-labelledby="<%= title.dasherize %>">
+  <% if @merge_requests.empty? %>
+    <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| %>
+      <strong><%= @merge_requests.where.not(status: 4).count %></strong> unresolved merge requests
+    <% end %>
+    <%= table.with_head do |head| %>
+      <%= head.with_row do |row| %>
+        <% row.with_cell(header: true, text: "Absorbing organisation", html_attributes: {
+          scope: "col",
+        }) %>
+        <% row.with_cell(header: true, text: "Merge date", html_attributes: {
+          scope: "col",
+        }) %>
+        <% row.with_cell(header: true, text: "Status", html_attributes: {
+          scope: "col",
+        }) %>
+        <% row.with_cell(header: true, text: "", html_attributes: {
+          scope: "col",
+        }) %>
+      <% end %>
+    <% end %>
+      <% @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) %>
+            <% 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")) %>
+            <% 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") %>
+            <% end %>
+          <% end %>
+        <% end %>
+      <% end %>
+    <% end %>
+  <% end %>
+</section>

app/views/organisations/index.html.erb

@@ -5,13 +5,18 @@
 
 <%= render partial: "organisations/headings", locals: request.path == organisations_path ? { main: "Organisations", sub: nil } : { main: @organisation.name, sub: "Organisations" } %>
 
-<% if current_user.support? %>
+<div class="app-tab__list-view">
-  <%= govuk_button_link_to "Create a new organisation", new_organisation_path, html: { method: :get } %>
+  <%= govuk_tabs(title: "Collection resources", classes: %w[app-tab__large-headers]) do |c| %>
-<% end %>
+    <% c.with_tab(label: "All organisations") do %>
-
+      <%= govuk_button_link_to "Create a new organisation", new_organisation_path, html: { method: :get } %>
-<%= render SearchComponent.new(current_user:, search_label: "Search by organisation name", value: @searched) %>
+      <%= render SearchComponent.new(current_user:, search_label: "Search by organisation name", value: @searched) %>
-
+      <%= govuk_section_break(visible: true, size: "m") %>
-<%= govuk_section_break(visible: true, size: "m") %>
+      <%= render partial: "organisation_list", locals: { organisations: @organisations, title: "Organisations", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count } %>
-
+      <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "organisations" } %>
-<%= render partial: "organisation_list", locals: { organisations: @organisations, title: "Organisations", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count } %>
+    <% end %>
-<%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "organisations" } %>
+    <% c.with_tab(label: "Merge requests") do %>
+      <%= govuk_button_link_to "Create new merge request", "/organisations/#{current_user.organisation_id}/merge-request", html: { method: :get } %>
+      <%= render partial: "merge_request_list", locals: { merge_requests: @merge_requests, title: "Merge requests", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count } %>
+    <% end %>
+  <% end %>
+</div>

db/migrate/20240808134014_add_merge_date_to_merge_requests.rb

@@ -0,0 +1,5 @@
+class AddMergeDateToMergeRequests < ActiveRecord::Migration[7.0]
+  def change
+    add_column :merge_requests, :merge_date, :datetime
+  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_07_15_082338) do
+ActiveRecord::Schema[7.0].define(version: 2024_08_08_134014) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -430,6 +430,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_07_15_082338) do
     t.string "new_organisation_address_line2"
     t.string "new_organisation_postcode"
     t.string "new_organisation_telephone_number"
+    t.datetime "merge_date"
   end
 
   create_table "notifications", force: :cascade do |t|

spec/factories/merge_requests.rb

@@ -0,0 +1,7 @@
+FactoryBot.define do
+  factory :merge_request do
+    status { "incomplete" }
+    merge_date { nil }
+    association :requesting_organisation, factory: :organisation
+  end
+end

spec/features/schemes_spec.rb

@@ -766,8 +766,10 @@ RSpec.describe "Schemes scheme Features" do
 
           before do
             Timecop.freeze(Time.zone.local(2023, 10, 10))
+            Singleton.__init__(FormHandler)
             FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 4), location: deactivated_location)
             Timecop.unfreeze
+            Singleton.__init__(FormHandler)
             click_link(scheme.service_name)
           end
 

spec/models/merge_request_spec.rb

@@ -0,0 +1,26 @@
+require "rails_helper"
+
+RSpec.describe MergeRequest, type: :model do
+  describe ".visible" do
+    let(:open_collection_period_start_date) { 1.year.ago }
+    let!(:merged_recent) { create(:merge_request, status: "request_merged", merge_date: 3.months.ago) }
+    let!(:merged_old) { create(:merge_request, status: "request_merged", merge_date: 18.months.ago) }
+    let!(:not_merged) { create(:merge_request, status: "incomplete") }
+
+    before do
+      allow(FormHandler.instance).to receive(:start_date_of_earliest_open_collection_period).and_return(open_collection_period_start_date)
+    end
+
+    it "includes merged requests with merge dates after the open collection period start date" do
+      expect(described_class.visible).to include(merged_recent)
+    end
+
+    it "excludes merged requests with merge dates before the open collection period start date" do
+      expect(described_class.visible).not_to include(merged_old)
+    end
+
+    it "includes not_merged requests" do
+      expect(described_class.visible).to include(not_merged)
+    end
+  end
+end

spec/requests/merge_requests_controller_spec.rb

@@ -14,7 +14,7 @@ RSpec.describe MergeRequestsController, type: :request do
     before { sign_in user }
 
     describe "#organisations" do
-      let(:params) { { merge_request: { requesting_organisation_id: "9", status: "unsubmitted" } } }
+      let(:params) { { merge_request: { requesting_organisation_id: "9", status: "incomplete" } } }
 
       context "when creating a new merge request" do
         before do
@@ -29,7 +29,7 @@ RSpec.describe MergeRequestsController, type: :request do
         end
 
         context "when passing a different requesting organisation id" do
-          let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "unsubmitted" } } }
+          let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "incomplete" } } }
 
           it "creates merge request with current user organisation" do
             follow_redirect!
@@ -84,7 +84,7 @@ RSpec.describe MergeRequestsController, type: :request do
         let(:params) { { merge_request: { merging_organisation: other_organisation.id } } }
 
         before do
-          MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "submitted")
+          MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "request_merged")
           patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
         end
 
@@ -100,7 +100,7 @@ RSpec.describe MergeRequestsController, type: :request do
         let(:params) { { merge_request: { merging_organisation: other_organisation.id } } }
 
         before do
-          MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "unsubmitted")
+          MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "incomplete")
           patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
         end
 
@@ -116,7 +116,7 @@ RSpec.describe MergeRequestsController, type: :request do
         let(:params) { { merge_request: { merging_organisation: another_organisation.id } } }
 
         before do
-          existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "submitted")
+          existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "request_merged")
           MergeRequestOrganisation.create!(merge_request_id: existing_merge_request.id, merging_organisation_id: another_organisation.id)
           patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
         end
@@ -134,7 +134,7 @@ RSpec.describe MergeRequestsController, type: :request do
         let(:params) { { merge_request: { merging_organisation: another_organisation.id } } }
 
         before do
-          existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "unsubmitted")
+          existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "incomplete")
           MergeRequestOrganisation.create!(merge_request_id: existing_merge_request.id, merging_organisation_id: another_organisation.id)
           patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
         end
@@ -701,7 +701,7 @@ RSpec.describe MergeRequestsController, type: :request do
     end
 
     describe "#organisations" do
-      let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "unsubmitted" } } }
+      let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "incomplete" } } }
 
       before do
         post "/merge-request", headers:, params:

spec/requests/organisations_controller_spec.rb

@@ -1017,6 +1017,19 @@ RSpec.describe OrganisationsController, type: :request do
           expect(page).to have_field("search", type: "search")
         end
 
+        it "shows the merge request list" do
+          expect(page).to have_content("Merge requests")
+        end
+
+        it "has a create new merge request button" do
+          expect(page).to have_link("Create new merge request")
+        end
+
+        it "displays 'No merge requests' when @merge_requests is empty" do
+          allow(MergeRequest).to receive(:visible).and_return(nil)
+          expect(page).to have_content("No merge requests")
+        end
+
         context "when viewing a specific organisation's lettings logs" do
           let(:parent_organisation) { create(:organisation) }
           let(:child_organisation) { create(:organisation) }