From 9715fc294bd2ab86bf19680641029a716307e16c Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 14 Aug 2024 11:20:25 +0100 Subject: [PATCH] Create delete merge request functionality (#2568) --- 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 | 19 +++++++++++++ spec/requests/merge_request_spec.rb | 28 +++++++++++++++++++ 7 files changed, 85 insertions(+), 9 deletions(-) create mode 100644 app/views/merge_requests/delete_confirmation.html.erb create mode 100644 spec/requests/merge_request_spec.rb diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 9874cfffb..746de0a5a 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -42,6 +42,11 @@ class MergeRequestsController < ApplicationController render :merging_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 2abea9d7f..7c95e199c 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -18,7 +18,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 absorbing_organisation_name @@ -28,4 +28,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..c874946b2 --- /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 %> + +
+
+

+ <%= content_for(:title) %> +

+ + <%= govuk_warning_text(text: "You cannot undo this.") %> + +
+ <%= govuk_button_to( + "Delete merge request", + delete_merge_request_path(@merge_request), + method: :delete, + ) %> + <%= govuk_button_link_to "Cancel", merge_request_path(@merge_request), secondary: true %> +
+
+
diff --git a/app/views/merge_requests/show.html.erb b/app/views/merge_requests/show.html.erb index b28ee3c1e..ce4fbb38d 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? %>
@@ -27,12 +27,8 @@ <% unless @merge_request.status == "request_merged" %>
- - + <%= 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 %>
<% end %> diff --git a/config/routes.rb b/config/routes.rb index 8920e1587..ecdb8f2c1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -201,7 +201,8 @@ Rails.application.routes.draw do get "absorbing-organisation" get "merge-date" get "helpdesk-ticket" - 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..cf38d5033 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -23,4 +23,23 @@ 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" do + merge_request.discard! + expect(merge_request.discarded_at).not_to be_nil + 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 diff --git a/spec/requests/merge_request_spec.rb b/spec/requests/merge_request_spec.rb new file mode 100644 index 000000000..3666244dd --- /dev/null +++ b/spec/requests/merge_request_spec.rb @@ -0,0 +1,28 @@ +require "rails_helper" + +RSpec.describe MergeRequest, type: :request do + let(:user) { create(:user, :data_coordinator) } + let(:organisation) { user.organisation } + let(:merge_request) { create(:merge_request) } + let(:support_user) { create(:user, :support, organisation:) } + let(:page) { Capybara::Node::Simple.new(response.body) } + + before do + allow(support_user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in support_user + end + + context "when deleting a merge request" do + it "discards the merge request" do + delete delete_merge_request_path(merge_request) + expect(merge_request.reload.discarded_at).not_to be_nil + end + + it "redirects to the merge request list" do + delete delete_merge_request_path(merge_request) + expect(response).to redirect_to(organisations_path(anchor: "merge-requests")) + follow_redirect! + expect(page).to have_content("Merge requests") + end + end +end