diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 070d3a04e..930a79b04 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/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 diff --git a/app/views/merge_requests/absorbing_organisation.html.erb b/app/views/merge_requests/absorbing_organisation.html.erb index cd2441d2b..e63154bac 100644 --- a/app/views/merge_requests/absorbing_organisation.html.erb +++ b/app/views/merge_requests/absorbing_organisation.html.erb @@ -4,14 +4,13 @@ <%= govuk_back_link href: organisations_merge_request_path(id: @merge_request) %> <% end %> -

Which organisation is absorbing the others?

+<%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> + <%= f.govuk_error_summary %> -
-
-

Select the organisation that the other organisations are merging into.

- - <%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> - <%= f.govuk_error_summary %> +

Which organisation is absorbing the others?

+
+
+

Select the organisation that the other organisations are merging into.

<%= f.govuk_radio_buttons_fieldset( :absorbing_organisation_id, diff --git a/app/views/merge_requests/confirm_telephone_number.html.erb b/app/views/merge_requests/confirm_telephone_number.html.erb index 63bd2a9e5..9a05ee54e 100644 --- a/app/views/merge_requests/confirm_telephone_number.html.erb +++ b/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 %> -

What is <%= @merge_request.absorbing_organisation.name %>'s telephone number?

+<%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> + <%= f.govuk_error_summary %> -
-
- <% if @merge_request.absorbing_organisation.phone.present? %> -

Confirm the telephone number on file, or enter a new one.

- <% end %> - -
- <%= @merge_request.absorbing_organisation.phone %> -
+

What is <%= @merge_request.absorbing_organisation.name %>'s telephone number?

- <%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> - <%= f.govuk_error_summary %> +
+
+ <% if @merge_request.absorbing_organisation.phone.present? %> +

Confirm the telephone number on file, or enter a new one.

+
+ <%= @merge_request.absorbing_organisation.phone %> +
+ <% 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" %> diff --git a/app/views/merge_requests/organisations.html.erb b/app/views/merge_requests/organisations.html.erb index 221fbe87e..2a3c7a444 100644 --- a/app/views/merge_requests/organisations.html.erb +++ b/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 %> -

Which organisations are merging?

+<%= form_with model: @merge_request, url: organisations_merge_request_path, method: :patch do |f| %> + <%= f.govuk_error_summary %> +

Which organisations are merging?

-
-
-

- Add all organisations to be merged - we have already added your own. -

+
+
+

+ Add all organisations to be merged - we have already added your own. +

- <%= form_with model: @merge_request, url: organisations_merge_request_path, method: :patch do |f| %> - <%= f.govuk_error_summary %>

Start typing to search

<%= render partial: "organisation_relationships/related_organisation_select_question", locals: { field: :merging_organisation, diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb index f81cdfa5f..943be6374 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/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