From 2cf0ca7937ef9c175acd396e97ddfc8467ec0a64 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 19 Aug 2024 15:40:16 +0100 Subject: [PATCH] Keep check answers referrer when validation is hit (#2594) * Keep check answers referrer when validation is hit * Remove absorbing org from merging organisations list --- app/controllers/merge_requests_controller.rb | 22 ++++++++--- app/helpers/merge_requests_helper.rb | 4 ++ .../absorbing_organisation.html.erb | 2 +- .../merge_requests/helpdesk_ticket.html.erb | 2 +- app/views/merge_requests/merge_date.html.erb | 2 +- .../merge_requests_controller_spec.rb | 37 +++++++++++++++++++ 6 files changed, 60 insertions(+), 9 deletions(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 77628492b..5860c34a4 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -21,12 +21,9 @@ class MergeRequestsController < ApplicationController validate_response if @merge_request.errors.blank? && @merge_request.update(merge_request_params) - if page == "merging_organisations" - new_merging_org_ids = params["merge_request"]["new_merging_org_ids"].split(" ") - new_merging_org_ids.each do |org_id| - MergeRequestOrganisation.create!(merge_request: @merge_request, merging_organisation_id: org_id) - end - end + add_merging_organsations if page == "merging_organisations" + remove_absorbing_org_from_merging_organisations if page == "absorbing_organisation" && @merge_request.absorbing_organisation_id.present? + redirect_to next_page_path else render previous_template, status: :unprocessable_entity @@ -176,4 +173,17 @@ private parsed_params = CGI.parse(query_params) parsed_params[query_param]&.first end + + def add_merging_organsations + new_merging_org_ids = params["merge_request"]["new_merging_org_ids"].split(" ") + new_merging_org_ids.each do |org_id| + MergeRequestOrganisation.create!(merge_request: @merge_request, merging_organisation_id: org_id) + end + end + + def remove_absorbing_org_from_merging_organisations + if @merge_request.merge_request_organisations.where(merging_organisation_id: @merge_request.absorbing_organisation_id).exists? + MergeRequestOrganisation.find_by(merge_request: @merge_request, merging_organisation_id: @merge_request.absorbing_organisation_id).destroy! + end + end end diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index c018a99df..f8d757aaa 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -74,4 +74,8 @@ module MergeRequestsHelper { text: "Change", href: send("#{page}_merge_request_path", merge_request, referrer: "check_answers"), visually_hidden_text: page.humanize } end end + + def submit_merge_request_url(referrer) + referrer == "check_answers" ? merge_request_path(referrer: "check_answers") : merge_request_path + end end diff --git a/app/views/merge_requests/absorbing_organisation.html.erb b/app/views/merge_requests/absorbing_organisation.html.erb index f50bfe943..804e9f03b 100644 --- a/app/views/merge_requests/absorbing_organisation.html.erb +++ b/app/views/merge_requests/absorbing_organisation.html.erb @@ -4,7 +4,7 @@ <%= govuk_back_link href: merge_request_back_link(@merge_request, "absorbing_organisation", request.query_parameters["referrer"]) %> <% end %> -<%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> +<%= form_with model: @merge_request, url: submit_merge_request_url(request.query_parameters["referrer"]), method: :patch do |f| %> <%= f.govuk_error_summary %>

Which organisation is absorbing the others?

diff --git a/app/views/merge_requests/helpdesk_ticket.html.erb b/app/views/merge_requests/helpdesk_ticket.html.erb index 0477a76ea..4ebd11395 100644 --- a/app/views/merge_requests/helpdesk_ticket.html.erb +++ b/app/views/merge_requests/helpdesk_ticket.html.erb @@ -4,7 +4,7 @@ <%= govuk_back_link href: merge_request_back_link(@merge_request, "helpdesk_ticket", request.query_parameters["referrer"]) %> <% end %> -<%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> +<%= form_with model: @merge_request, url: submit_merge_request_url(request.query_parameters["referrer"]), method: :patch do |f| %> <%= f.govuk_error_summary %>

Which helpdesk ticket reported this merge?

diff --git a/app/views/merge_requests/merge_date.html.erb b/app/views/merge_requests/merge_date.html.erb index 09b3411ea..debc8eb2e 100644 --- a/app/views/merge_requests/merge_date.html.erb +++ b/app/views/merge_requests/merge_date.html.erb @@ -5,7 +5,7 @@ <% end %>
- <%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> + <%= form_with model: @merge_request, url: submit_merge_request_url(request.query_parameters["referrer"]), method: :patch do |f| %> <%= f.govuk_error_summary %>

What is the merge date?

diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb index 8e406ea5f..75d09fb76 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/spec/requests/merge_requests_controller_spec.rb @@ -276,6 +276,43 @@ RSpec.describe MergeRequestsController, type: :request do }.from(nil).to(other_organisation) end end + + context "when updating from check_answers page" do + let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation) } + let(:params) do + { merge_request: { absorbing_organisation_id: "", page: "absorbing_organisation" } } + end + + let(:request) do + patch "/merge-request/#{merge_request.id}?referrer=check_answers", headers:, params: + end + + it "keeps corrent links if validation fails" do + request + + expect(page).to have_link("Cancel", href: merge_request_path(merge_request)) + expect(page).to have_button("Save changes") + end + end + + context "when absorbing_organisation_id set to one of the merging organisations" do + let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation) } + let(:params) do + { merge_request: { absorbing_organisation_id: other_organisation.id, page: "absorbing_organisation" } } + end + + let(:request) do + MergeRequestOrganisation.create!(merge_request_id: merge_request.id, merging_organisation_id: other_organisation.id) + patch "/merge-request/#{merge_request.id}", headers:, params: + end + + it "removes organisation from merge request organisations" do + request + + merge_request.reload + expect(merge_request.merging_organisations.count).to eq(0) + end + end end describe "from merge_date page" do