CLDC-2057 Telephone number confirmation (#1595)

* CLDC-2057 Merge request: ask to confirm number

* address comments

app/controllers/merge_requests_controller.rb

@@ -7,6 +7,7 @@ class MergeRequestsController < ApplicationController
     absorbing_organisation
     confirm_telephone_number
     new_org_name
+    merge_date
   ]
   before_action :authenticate_user!
   before_action :authenticate_scope!, except: [:create]
@@ -15,6 +16,7 @@ class MergeRequestsController < ApplicationController
   def absorbing_organisation; end
   def confirm_telephone_number; end
   def new_org_name; end
+  def merge_date; end
 
   def create
     ActiveRecord::Base.transaction do
@@ -70,6 +72,8 @@ private
       end
     when "organisations"
       absorbing_organisation_merge_request_path(@merge_request)
+    when "confirm_telephone_number"
+      merge_date_merge_request_path(@merge_request)
     end
   end
 
@@ -96,6 +100,8 @@ private
       :other_merging_organisations,
       :status,
       :absorbing_organisation_id,
+      :telephone_number_correct,
+      :new_telephone_number,
     )
 
     if merge_params[:requesting_organisation_id].present? && (current_user.data_coordinator? || current_user.data_provider?)
@@ -111,13 +117,31 @@ private
       end
     end
 
+    if merge_params[:telephone_number_correct] == "1"
+      merge_params[:new_telephone_number] = nil
+    end
+
     merge_params
   end
 
   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?
+        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, 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,10 +4,33 @@
   <%= 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">
-    <p class="govuk-body">Confirm the telephone number on file, or enter a new one.</p>
+  <h2 class="govuk-heading-l">What is <%= @merge_request.absorbing_organisation.name %>'s telephone number?</h2>
+
+  <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.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" %>
+        <% end %>
+      <% else %>
+        <%= f.govuk_text_field :new_telephone_number, label: nil, width: "two-thirds" %>
+        <%= f.hidden_field :page, value: "confirm_telephone_number" %>
+      <% end %>
+      <%= f.govuk_submit %>
+    <% end %>
   </div>
 </div>

app/views/merge_requests/merge_date.html.erb

@@ -0,0 +1,8 @@
+<% content_for :before_content do %>
+  <% title = "Tell us if your organisation is merging" %>
+  <% content_for :title, title %>
+  <%# TODO: Update this backlink to also work with the create org flow %>
+  <%= govuk_back_link href: confirm_telephone_number_merge_request_path(@merge_request) %>
+<% end %>
+
+<h2 class="govuk-heading-l">What is the merge date?</h2>

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,

config/locales/en.yml

@@ -513,6 +513,8 @@ en:
       organisation_part_of_another_merge: "This organisation is part of another merge - select a different one"
       organisation_not_selected: "Select an organisation from the search list"
       absorbing_organisation_blank: Select the organisation absorbing the others
+      telephone_number_correct_blank: Select to confirm or enter a new telephone number
+      new_telephone_number_blank: Enter a valid telephone number
 
   soft_validations:
     net_income:

config/routes.rb

@@ -139,6 +139,7 @@ Rails.application.routes.draw do
       get "absorbing-organisation"
       get "confirm-telephone-number"
       get "new-org-name"
+      get "merge-date"
     end
   end
 

db/migrate/20230427130334_add_phone_number_to_merge_request.rb

@@ -0,0 +1,6 @@
+class AddPhoneNumberToMergeRequest < ActiveRecord::Migration[7.0]
+  change_table :merge_requests, bulk: true do |t|
+    t.column :telephone_number_correct, :boolean
+    t.column :new_telephone_number, :string
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_04_21_124536) do
+ActiveRecord::Schema[7.0].define(version: 2023_04_27_130334) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -369,6 +369,8 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_21_124536) do
     t.integer "status"
     t.integer "absorbing_organisation_id"
     t.boolean "new_absorbing_organisation"
+    t.boolean "telephone_number_correct"
+    t.string "new_telephone_number"
   end
 
   create_table "organisation_relationships", force: :cascade do |t|

spec/requests/merge_requests_controller_spec.rb

@@ -233,6 +233,38 @@ RSpec.describe MergeRequestsController, type: :request do
       end
     end
 
+    describe "#confirm_telephone_number" do
+      let(:merge_request) do
+        MergeRequest.create!(
+          absorbing_organisation: create(:organisation, phone: phone_number),
+          requesting_organisation: organisation,
+        )
+      end
+
+      before { get "/merge-request/#{merge_request.id}/confirm-telephone-number", headers: }
+
+      context "when org has phone number" do
+        let(:phone_number) { 123 }
+
+        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
+
+      context "when org does not have a phone number set" do
+        let(:phone_number) { nil }
+
+        it "asks provide new number" do
+          expect(page).not_to have_content("This telephone number is correct")
+          expect(page).not_to have_content("Confirm the telephone number on file, or enter a new one.")
+          expect(page).to have_content("What is #{merge_request.absorbing_organisation.name}'s telephone number?")
+        end
+      end
+    end
+
     describe "#update" do
       before { sign_in user }
 
@@ -260,72 +292,184 @@ 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) }
-        let(:params) do
-          { merge_request: { page: "absorbing_organisation" } }
-        end
-        let(:request) do
-          patch "/merge-request/#{merge_request.id}", headers:, params:
+      describe "from absorbing_organisation page" do
+        context "when not answering the question" do
+          let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) }
+          let(:params) do
+            { merge_request: { page: "absorbing_organisation" } }
+          end
+          let(:request) do
+            patch "/merge-request/#{merge_request.id}", headers:, params:
+          end
+
+          it "renders the error" do
+            request
+
+            expect(page).to have_content("Select the organisation absorbing the others")
+          end
+
+          it "does not update the request" do
+            expect { request }.not_to(change { merge_request.reload.attributes })
+          end
         end
 
-        it "renders the error" do
-          request
+        context "when absorbing_organisation_id set to other" do
+          let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) }
+          let(:params) do
+            { merge_request: { absorbing_organisation_id: "other", page: "absorbing_organisation" } }
+          end
+          let(:request) do
+            patch "/merge-request/#{merge_request.id}", headers:, params:
+          end
+
+          it "redirects to new org path" do
+            request
 
-          expect(page).to have_content("Select the organisation absorbing the others")
+            expect(response).to redirect_to(new_org_name_merge_request_path(merge_request))
+          end
+
+          it "resets absorbing_organisation and sets new_absorbing_organisation to true" do
+            expect { request }.to change {
+              merge_request.reload.absorbing_organisation
+            }.from(other_organisation).to(nil).and change {
+              merge_request.reload.new_absorbing_organisation
+            }.from(nil).to(true)
+          end
         end
 
-        it "does not update the request" do
-          expect { request }.not_to(change { merge_request.reload.attributes })
+        context "when absorbing_organisation_id set to id" do
+          let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, new_absorbing_organisation: true) }
+          let(:params) do
+            { merge_request: { absorbing_organisation_id: other_organisation.id, page: "absorbing_organisation" } }
+          end
+
+          let(:request) do
+            patch "/merge-request/#{merge_request.id}", headers:, params:
+          end
+
+          it "redirects telephone number path" do
+            request
+
+            expect(response).to redirect_to(confirm_telephone_number_merge_request_path(merge_request))
+          end
+
+          it "updates absorbing_organisation_id and sets new_absorbing_organisation to false" do
+            expect { request }.to change {
+              merge_request.reload.absorbing_organisation
+            }.from(nil).to(other_organisation).and change {
+              merge_request.reload.new_absorbing_organisation
+            }.from(true).to(false)
+          end
         end
       end
 
-      context "when absorbing_organisation_id set to other" do
-        let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) }
-        let(:params) do
-          { merge_request: { absorbing_organisation_id: "other", page: "absorbing_organisation" } }
-        end
-        let(:request) do
-          patch "/merge-request/#{merge_request.id}", headers:, params:
-        end
+      describe "from confirm_telephone_number page" do
+        context "when confirming the number" do
+          let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, new_absorbing_organisation: true, new_telephone_number: "123") }
+          let(:params) do
+            { merge_request: { telephone_number_correct: "1", page: "confirm_telephone_number" } }
+          end
 
-        it "redirects to new org path" do
-          request
+          let(:request) do
+            patch "/merge-request/#{merge_request.id}", headers:, params:
+          end
 
-          expect(response).to redirect_to(new_org_name_merge_request_path(merge_request))
-        end
+          it "redirects telephone number path" do
+            request
 
-        it "resets absorbing_organisation and sets new_absorbing_organisation to true" do
-          expect { request }.to change {
-            merge_request.reload.absorbing_organisation
-          }.from(other_organisation).to(nil).and change {
-            merge_request.reload.new_absorbing_organisation
-          }.from(nil).to(true)
+            expect(response).to redirect_to(merge_date_merge_request_path(merge_request))
+          end
+
+          it "updates telephone_number_correct and sets new_telephone_number to nil" do
+            expect { request }.to change {
+              merge_request.reload.telephone_number_correct
+            }.from(nil).to(true).and change {
+              merge_request.reload.new_telephone_number
+            }.from("123").to(nil)
+          end
         end
-      end
 
-      context "when absorbing_organisation_id set to id" do
-        let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, new_absorbing_organisation: true) }
-        let(:params) do
-          { merge_request: { absorbing_organisation_id: other_organisation.id, page: "absorbing_organisation" } }
+        context "when setting new number" do
+          let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, new_absorbing_organisation: true) }
+          let(:params) do
+            { merge_request: { telephone_number_correct: "0", new_telephone_number: "123", page: "confirm_telephone_number" } }
+          end
+
+          let(:request) do
+            patch "/merge-request/#{merge_request.id}", headers:, params:
+          end
+
+          it "redirects telephone number path" do
+            request
+
+            expect(response).to redirect_to(merge_date_merge_request_path(merge_request))
+          end
+
+          it "updates telephone_number_correct and sets new_telephone_number to nil" do
+            expect { request }.to change {
+              merge_request.reload.new_telephone_number
+            }.from(nil).to("123")
+          end
         end
 
-        let(:request) do
-          patch "/merge-request/#{merge_request.id}", headers:, params:
+        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
+          let(:request) do
+            patch "/merge-request/#{merge_request.id}", headers:, params:
+          end
+
+          it "renders the error" do
+            request
+
+            expect(page).to have_content("Select to confirm or enter a new telephone number")
+          end
+
+          it "does not update the request" do
+            expect { request }.not_to(change { merge_request.reload.attributes })
+          end
         end
 
-        it "redirects telephone number path" do
-          request
+        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
 
-          expect(response).to redirect_to(confirm_telephone_number_merge_request_path(merge_request))
+          it "does not update the request" do
+            expect { request }.not_to(change { merge_request.reload.attributes })
+          end
         end
 
-        it "updates absorbing_organisation_id and sets new_absorbing_organisation to false" do
-          expect { request }.to change {
-            merge_request.reload.absorbing_organisation
-          }.from(nil).to(other_organisation).and change {
-            merge_request.reload.new_absorbing_organisation
-          }.from(true).to(false)
+        context "when not answering the phone number" do
+          let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) }
+          let(:params) do
+            { merge_request: { page: "confirm_telephone_number", telephone_number_correct: "0" } }
+          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
       end
     end