From 11014452a9b85564a70b84029c3390c4c0424a03 Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Tue, 13 Aug 2024 11:24:25 +0100
Subject: [PATCH 1/5] Create delete merge request functionality

---
 app/controllers/merge_requests_controller.rb  |  5 ++++
 app/models/merge_request.rb                   |  6 ++++-
 .../delete_confirmation.html.erb              | 23 +++++++++++++++++++
 app/views/merge_requests/show.html.erb        | 10 +++-----
 config/routes.rb                              |  3 ++-
 spec/models/merge_request_spec.rb             | 21 +++++++++++++++++
 6 files changed, 59 insertions(+), 9 deletions(-)
 create mode 100644 app/views/merge_requests/delete_confirmation.html.erb

diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb
index 6d7738204..6b3197e72 100644
--- a/app/controllers/merge_requests_controller.rb
+++ b/app/controllers/merge_requests_controller.rb
@@ -46,6 +46,11 @@ class MergeRequestsController < ApplicationController
     render :organisations
   end
 
+  def delete
+    @merge_request.discard!
+    redirect_to organisations_path(anchor: "merge-requests")
+  end
+
 private
 
   def page
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index ec844576b..3222bad33 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -20,7 +20,7 @@ class MergeRequest < ApplicationRecord
   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)
+    merged.where("merge_requests.merge_date >= ?", open_collection_period_start_date).or(not_merged).where(discarded_at: nil)
   }
 
   def organisation_name_uniqueness
@@ -36,4 +36,8 @@ class MergeRequest < ApplicationRecord
   def dpo_user
     absorbing_organisation.data_protection_officers.filter_by_active.first
   end
+
+  def discard!
+    update!(discarded_at: Time.zone.now)
+  end
 end
diff --git a/app/views/merge_requests/delete_confirmation.html.erb b/app/views/merge_requests/delete_confirmation.html.erb
new file mode 100644
index 000000000..8b8103d42
--- /dev/null
+++ b/app/views/merge_requests/delete_confirmation.html.erb
@@ -0,0 +1,23 @@
+<% content_for :before_content do %>
+  <% content_for :title, "Are you sure you want to delete this merge request?" %>
+  <%= govuk_back_link(href: :back) %>
+<% end %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds-from-desktop">
+    <h1 class="govuk-heading-xl">
+      <%= content_for(:title) %>
+    </h1>
+
+    <%= govuk_warning_text(text: "You cannot undo this.") %>
+
+    <div class="govuk-button-group">
+      <%= govuk_button_to(
+        "Delete merge request",
+        delete_merge_request_path(@merge_request),
+        method: :delete,
+      ) %>
+      <%= govuk_button_link_to "Cancel", merge_request_path(@merge_request), html: { method: :get }, secondary: true %>
+    </div>
+  </div>
+</div>
diff --git a/app/views/merge_requests/show.html.erb b/app/views/merge_requests/show.html.erb
index b28ee3c1e..cf85e287b 100644
--- a/app/views/merge_requests/show.html.erb
+++ b/app/views/merge_requests/show.html.erb
@@ -1,7 +1,7 @@
 <% 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" %>
+  <%= govuk_back_link href: organisations_path(anchor: "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">
@@ -27,12 +27,8 @@
 </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>
+  <%= govuk_button_link_to "Begin merge", nil, html: { method: :get }, disabled: @merge_request.status == "ready_to_merge" ? false : true %>
+  <%= govuk_button_link_to "Delete merge request", delete_confirmation_merge_request_path(@merge_request), warning: true%>
 </div>
 <% end %>
 
diff --git a/config/routes.rb b/config/routes.rb
index fbb59a7d0..9b3b13b53 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -205,7 +205,8 @@ Rails.application.routes.draw do
       get "new-organisation-telephone-number"
       get "new-organisation-type"
       get "merge-date"
-      get "details", to: "merge_requests#details"
+      get "delete-confirmation", to: "merge_requests#delete_confirmation"
+      delete "delete", to: "merge_requests#delete"
     end
   end
 
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index e3bb24b46..10901623f 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -23,4 +23,25 @@ RSpec.describe MergeRequest, type: :model do
       expect(described_class.visible).to include(not_merged)
     end
   end
+
+  describe "#discard!" do
+    let(:merge_request) { create(:merge_request) }
+
+    it "sets the discarded_at field to the current time" do
+      Timecop.freeze(Time.zone.now) do
+        merge_request.discard!
+        expect(merge_request.discarded_at).to eq(Time.zone.now)
+      end
+    end
+
+    it "does not delete the record" do
+      merge_request.discard!
+      expect(merge_request).to be_persisted
+    end
+
+    it "is not visible in the visible scope" do
+      merge_request.discard!
+      expect(described_class.visible).not_to include(merge_request)
+    end
+  end
 end

From 5dd72d86215c43cd3d5ee1adf85c2b4773e4eacb Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Tue, 13 Aug 2024 12:03:44 +0100
Subject: [PATCH 2/5] Fix lint offences

---
 app/views/merge_requests/show.html.erb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/views/merge_requests/show.html.erb b/app/views/merge_requests/show.html.erb
index cf85e287b..b0a8d0736 100644
--- a/app/views/merge_requests/show.html.erb
+++ b/app/views/merge_requests/show.html.erb
@@ -27,8 +27,8 @@
 </h1>
 <% unless @merge_request.status == "request_merged" %>
 <div class="govuk-button-group">
-  <%= govuk_button_link_to "Begin merge", nil, html: { method: :get }, disabled: @merge_request.status == "ready_to_merge" ? false : true %>
-  <%= govuk_button_link_to "Delete merge request", delete_confirmation_merge_request_path(@merge_request), warning: true%>
+  <%= govuk_button_link_to "Begin merge", nil, html: { method: :get }, disabled: !(@merge_request.status == "ready_to_merge") %>
+  <%= govuk_button_link_to "Delete merge request", delete_confirmation_merge_request_path(@merge_request), warning: true %>
 </div>
 <% end %>
 

From 7c13c29c38366bec8f94e57d7a3989e429ef0e80 Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Tue, 13 Aug 2024 12:08:40 +0100
Subject: [PATCH 3/5] Fix tests

---
 app/views/merge_requests/show.html.erb | 2 +-
 spec/models/merge_request_spec.rb      | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/views/merge_requests/show.html.erb b/app/views/merge_requests/show.html.erb
index b0a8d0736..dc35f40ca 100644
--- a/app/views/merge_requests/show.html.erb
+++ b/app/views/merge_requests/show.html.erb
@@ -27,7 +27,7 @@
 </h1>
 <% unless @merge_request.status == "request_merged" %>
 <div class="govuk-button-group">
-  <%= govuk_button_link_to "Begin merge", nil, html: { method: :get }, disabled: !(@merge_request.status == "ready_to_merge") %>
+  <%= govuk_button_link_to "Begin merge", "#", disabled: !(@merge_request.status == "ready_to_merge") %>
   <%= govuk_button_link_to "Delete merge request", delete_confirmation_merge_request_path(@merge_request), warning: true %>
 </div>
 <% end %>
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 10901623f..41031c840 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -28,9 +28,10 @@ RSpec.describe MergeRequest, type: :model do
     let(:merge_request) { create(:merge_request) }
 
     it "sets the discarded_at field to the current time" do
-      Timecop.freeze(Time.zone.now) do
+      let(:time) { Time.zone.now }
+      Timecop.freeze(time) do
         merge_request.discard!
-        expect(merge_request.discarded_at).to eq(Time.zone.now)
+        expect(merge_request.discarded_at).to eq(time)
       end
     end
 

From 1fbed4353b3c776c7908f65d320d0a6c213a1ccb Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Tue, 13 Aug 2024 12:13:23 +0100
Subject: [PATCH 4/5] Fix lint offences

---
 app/views/merge_requests/show.html.erb | 2 +-
 spec/models/merge_request_spec.rb      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/views/merge_requests/show.html.erb b/app/views/merge_requests/show.html.erb
index dc35f40ca..ce4fbb38d 100644
--- a/app/views/merge_requests/show.html.erb
+++ b/app/views/merge_requests/show.html.erb
@@ -27,7 +27,7 @@
 </h1>
 <% unless @merge_request.status == "request_merged" %>
 <div class="govuk-button-group">
-  <%= govuk_button_link_to "Begin merge", "#", disabled: !(@merge_request.status == "ready_to_merge") %>
+  <%= govuk_button_link_to "Begin merge", "#", disabled: @merge_request.status != "ready_to_merge" %>
   <%= govuk_button_link_to "Delete merge request", delete_confirmation_merge_request_path(@merge_request), warning: true %>
 </div>
 <% end %>
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 41031c840..46c508483 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -26,9 +26,9 @@ RSpec.describe MergeRequest, type: :model do
 
   describe "#discard!" do
     let(:merge_request) { create(:merge_request) }
+    let(:time) { Time.zone.now }
 
     it "sets the discarded_at field to the current time" do
-      let(:time) { Time.zone.now }
       Timecop.freeze(time) do
         merge_request.discard!
         expect(merge_request.discarded_at).to eq(time)

From 94b28f9d58d1561aeae5d4193f86e85c915f17f1 Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Tue, 13 Aug 2024 12:42:54 +0100
Subject: [PATCH 5/5] Change test

---
 spec/models/merge_request_spec.rb | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 46c508483..cf38d5033 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -26,13 +26,10 @@ RSpec.describe MergeRequest, type: :model do
 
   describe "#discard!" do
     let(:merge_request) { create(:merge_request) }
-    let(:time) { Time.zone.now }
 
-    it "sets the discarded_at field to the current time" do
-      Timecop.freeze(time) do
-        merge_request.discard!
-        expect(merge_request.discarded_at).to eq(time)
-      end
+    it "sets the discarded_at field" do
+      merge_request.discard!
+      expect(merge_request.discarded_at).not_to be_nil
     end
 
     it "does not delete the record" do