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

@@ -21,12 +21,9 @@ class MergeRequestsController < ApplicationController
     validate_response
 
     if @merge_request.errors.blank? && @merge_request.update(merge_request_params)
-      if page == "merging_organisations"
+      add_merging_organsations if page == "merging_organisations"
-        new_merging_org_ids = params["merge_request"]["new_merging_org_ids"].split(" ")
+      remove_absorbing_org_from_merging_organisations if page == "absorbing_organisation" && @merge_request.absorbing_organisation_id.present?
-        new_merging_org_ids.each do |org_id|
+
-          MergeRequestOrganisation.create!(merge_request: @merge_request, merging_organisation_id: org_id)
-        end
-      end
       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

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

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 %>
 
   <h1 class="govuk-heading-l">Which organisation is absorbing the others?</h1>

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 %>
 
   <h1 class="govuk-heading-l">Which helpdesk ticket reported this merge?</h1>

app/views/merge_requests/merge_date.html.erb

@@ -5,7 +5,7 @@
 <% end %>
  <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds-from-desktop">
-    <%= 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 %>
 
       <h2 class="govuk-heading-l">What is the merge date?</h2>

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