address comments

app/controllers/merge_requests_controller.rb

@@ -127,16 +127,20 @@ private
   def validate_response
     if page == "absorbing_organisation" && merge_request_params[:absorbing_organisation_id].blank? && merge_request_params[:new_absorbing_organisation].blank?
       @merge_request.errors.add(:absorbing_organisation_id, I18n.t("validations.merge_request.absorbing_organisation_blank"))
-      render previous_template
+      render previous_template, status: :unprocessable_entity
     end
 
     if page == "confirm_telephone_number"
       if merge_request_params[:telephone_number_correct].blank? && merge_request_params[:new_telephone_number].blank?
-        @merge_request.errors.add(:telephone_number_correct, I18n.t("validations.merge_request.telephone_number_correct_blank"))
-        render previous_template
+        if @merge_request.absorbing_organisation.phone.present?
+          @merge_request.errors.add(:telephone_number_correct, I18n.t("validations.merge_request.telephone_number_correct_blank"))
+        else
+          @merge_request.errors.add(:telephone_number_correct, I18n.t("validations.merge_request.new_telephone_number_blank"))
+        end
+        render previous_template, status: :unprocessable_entity
       elsif merge_request_params[:telephone_number_correct] == "0" && merge_request_params[:new_telephone_number].blank?
         @merge_request.errors.add(:new_telephone_number, I18n.t("validations.merge_request.new_telephone_number_blank"))
-        render previous_template
+        render previous_template, status: :unprocessable_entity
       end
     end
   end

app/views/merge_requests/absorbing_organisation.html.erb

@@ -4,14 +4,13 @@
   <%= govuk_back_link href: organisations_merge_request_path(id: @merge_request) %>
 <% end %>
 
-<h2 class="govuk-heading-l">Which organisation is absorbing the others?</h2>
+<%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %>
+  <%= f.govuk_error_summary %>
 
-<div class="govuk-grid-row">
-  <div class="govuk-grid-column-two-thirds-from-desktop">
-    <p class="govuk-body">Select the organisation that the other organisations are merging into.</p>
-
-    <%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %>
-      <%= f.govuk_error_summary %>
+  <h2 class="govuk-heading-l">Which organisation is absorbing the others?</h2>
+  <div class="govuk-grid-row">
+    <div class="govuk-grid-column-two-thirds-from-desktop">
+      <p class="govuk-body">Select the organisation that the other organisations are merging into.</p>
 
       <%= f.govuk_radio_buttons_fieldset(
         :absorbing_organisation_id,

app/views/merge_requests/confirm_telephone_number.html.erb

@@ -4,25 +4,24 @@
   <%= govuk_back_link href: absorbing_organisation_merge_request_path(id: @merge_request) %>
 <% end %>
 
-<h2 class="govuk-heading-l">What is <%= @merge_request.absorbing_organisation.name %>'s telephone number?</h2>
+<%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %>
+  <%= f.govuk_error_summary %>
 
-<div class="govuk-grid-row">
-  <div class="govuk-grid-column-two-thirds-from-desktop">
-    <% if @merge_request.absorbing_organisation.phone.present? %>
-      <p class="govuk-body">Confirm the telephone number on file, or enter a new one.</p>
-    <% end %>
-
-    <div class="govuk-inset-text">
-      <%= @merge_request.absorbing_organisation.phone %>
-    </div>
+  <h2 class="govuk-heading-l">What is <%= @merge_request.absorbing_organisation.name %>'s telephone number?</h2>
 
-    <%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %>
-      <%= f.govuk_error_summary %>
+  <div class="govuk-grid-row">
+    <div class="govuk-grid-column-two-thirds-from-desktop">
+      <% if @merge_request.absorbing_organisation.phone.present? %>
+        <p class="govuk-body">Confirm the telephone number on file, or enter a new one.</p>
+        <div class="govuk-inset-text">
+          <%= @merge_request.absorbing_organisation.phone %>
+        </div>
+      <% end %>
 
       <% if @merge_request.absorbing_organisation.phone.present? %>
         <%= f.govuk_radio_buttons_fieldset(:telephone_number_correct, legend: nil) do %>
           <%= f.govuk_radio_button :telephone_number_correct, 1, checked: @merge_request.telephone_number_correct?, label: { text: "This telephone number is correct" }, link_errors: true %>
-          <%= f.govuk_radio_button :telephone_number_correct, 0, checked: (@merge_request.telephone_number_correct? == false), label: { text: "Enter a new phone number" } do %>
+          <%= f.govuk_radio_button :telephone_number_correct, 0, checked: @merge_request.new_telephone_number.present?, label: { text: "Enter a new phone number" } do %>
             <%= f.govuk_text_field :new_telephone_number, label: { text: "Telephone number" }, width: "two-thirds" %>
           <% end %>
           <%= f.hidden_field :page, value: "confirm_telephone_number" %>

app/views/merge_requests/organisations.html.erb

@@ -4,16 +4,16 @@
   <%= govuk_back_link href: merge_request_organisation_path(id: @merge_request.requesting_organisation_id) %>
 <% end %>
 
-<h2 class="govuk-heading-l">Which organisations are merging?</h2>
+<%= form_with model: @merge_request, url: organisations_merge_request_path, method: :patch do |f| %>
+  <%= f.govuk_error_summary %>
+  <h2 class="govuk-heading-l">Which organisations are merging?</h2>
 
-<div class="govuk-grid-row">
-  <div class="govuk-grid-column-two-thirds-from-desktop">
-   <p class="govuk-body">
-    Add all organisations to be merged - we have already added your own.
-  </p>
+  <div class="govuk-grid-row">
+    <div class="govuk-grid-column-two-thirds-from-desktop">
+    <p class="govuk-body">
+      Add all organisations to be merged - we have already added your own.
+    </p>
 
-  <%= form_with model: @merge_request, url: organisations_merge_request_path, method: :patch do |f| %>
-    <%= f.govuk_error_summary %>
     <p class="govuk-body">Start typing to search</p>
     <%= render partial: "organisation_relationships/related_organisation_select_question", locals: {
       field: :merging_organisation,

spec/requests/merge_requests_controller_spec.rb

@@ -249,6 +249,7 @@ RSpec.describe MergeRequestsController, type: :request do
         it "asks to confirm or provide new number" do
           expect(page).to have_content("This telephone number is correct")
           expect(page).to have_content("Confirm the telephone number on file, or enter a new one.")
+          expect(page).to have_content(phone_number)
           expect(page).to have_content("What is #{merge_request.absorbing_organisation.name}'s telephone number?")
         end
       end
@@ -411,8 +412,8 @@ RSpec.describe MergeRequestsController, type: :request do
           end
         end
 
-        context "when not answering the question" do
-          let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) }
+        context "when not answering the question and the org has phone number" do
+          let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: create(:organisation, phone: "123")) }
           let(:params) do
             { merge_request: { page: "confirm_telephone_number" } }
           end
@@ -431,6 +432,26 @@ RSpec.describe MergeRequestsController, type: :request do
           end
         end
 
+        context "when not answering the question and the org does not have a phone number" do
+          let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) }
+          let(:params) do
+            { merge_request: { page: "confirm_telephone_number" } }
+          end
+          let(:request) do
+            patch "/merge-request/#{merge_request.id}", headers:, params:
+          end
+
+          it "renders the error" do
+            request
+
+            expect(page).to have_content("Enter a valid telephone number")
+          end
+
+          it "does not update the request" do
+            expect { request }.not_to(change { merge_request.reload.attributes })
+          end
+        end
+
         context "when not answering the phone number" do
           let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) }
           let(:params) do