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 %> +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 2a585bc1f..9a05ee54e 100644 --- a/app/views/merge_requests/confirm_telephone_number.html.erb +++ b/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 %> -Confirm the telephone number on file, or enter a new one.
+Confirm the telephone number on file, or enter a new one.
+- 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/config/locales/en.yml b/config/locales/en.yml index ff409f2e0..a798a258f 100644 --- a/config/locales/en.yml +++ b/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: diff --git a/config/routes.rb b/config/routes.rb index cdbfc4616..bec7aed6b 100644 --- a/config/routes.rb +++ b/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 diff --git a/db/migrate/20230427130334_add_phone_number_to_merge_request.rb b/db/migrate/20230427130334_add_phone_number_to_merge_request.rb new file mode 100644 index 000000000..dec3d596f --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index f891d1024..73956bd24 100644 --- a/db/schema.rb +++ b/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| diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb index 5e2479189..943be6374 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/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