From 3b87ab3bcd741174580d2af86620048c5bfa061b Mon Sep 17 00:00:00 2001 From: Peter David Hamilton Date: Tue, 4 Feb 2025 14:48:11 +0000 Subject: [PATCH] CYA registering a mentor - add change links - make conditional back links - populated fields - reset corrected name --- .../register_mentor_wizard_controller.rb | 6 +++- app/services/schools/register_mentor.rb | 2 ++ .../check_answers.html.erb | 2 ++ .../email_address.html.erb | 10 ++++-- .../review_mentor_details.html.erb | 14 +++++--- .../schools/register_mentor_wizard/mentor.rb | 10 +++++- .../review_mentor_details_step.rb | 8 ++++- .../register_mentor/happy_path_spec.rb | 36 +++++++++++++++++++ .../check_answers.html.erb_spec.rb | 19 +++++++--- .../email_address.html.erb_spec.rb | 31 +++++++++++++--- .../review_mentor_details.html.erb_spec.rb | 23 +++++++++--- .../email_address_step_spec.rb | 20 +++++++++++ 12 files changed, 159 insertions(+), 22 deletions(-) create mode 100644 spec/wizards/schools/register_mentor_wizard/email_address_step_spec.rb diff --git a/app/controllers/schools/register_mentor_wizard_controller.rb b/app/controllers/schools/register_mentor_wizard_controller.rb index d675fd9e..52038497 100644 --- a/app/controllers/schools/register_mentor_wizard_controller.rb +++ b/app/controllers/schools/register_mentor_wizard_controller.rb @@ -18,7 +18,7 @@ def new def create if @wizard.save! - redirect_to @wizard.next_step_path + redirect_to cya? ? schools_register_mentor_wizard_check_answers_path : @wizard.next_step_path else render current_step end @@ -52,5 +52,9 @@ def store store.update(school_urn: @school.urn) end end + + def cya? + %i[review_mentor_details email_address].include?(current_step) && @mentor.email.present? + end end end diff --git a/app/services/schools/register_mentor.rb b/app/services/schools/register_mentor.rb index f89920fa..50d54912 100644 --- a/app/services/schools/register_mentor.rb +++ b/app/services/schools/register_mentor.rb @@ -28,6 +28,8 @@ def already_registered_as_a_mentor? def create_teacher! raise ActiveRecord::RecordInvalid if already_registered_as_a_mentor? + # FIXME: UX needs graceful redirect at this point + @teacher = ::Teacher.create_with(trs_first_name:, trs_last_name:, corrected_name:) .find_or_create_by!(trn:) end diff --git a/app/views/schools/register_mentor_wizard/check_answers.html.erb b/app/views/schools/register_mentor_wizard/check_answers.html.erb index 70c656bb..1ad47fb5 100644 --- a/app/views/schools/register_mentor_wizard/check_answers.html.erb +++ b/app/views/schools/register_mentor_wizard/check_answers.html.erb @@ -9,11 +9,13 @@ summary_list.with_row do |row| row.with_key(text: 'Name') row.with_value(text: @mentor.full_name) + row.with_action(text: 'Change', href: schools_register_mentor_wizard_review_mentor_details_path) end summary_list.with_row do |row| row.with_key(text: 'Email address') row.with_value(text: @mentor.email) + row.with_action(text: 'Change', href: schools_register_mentor_wizard_email_address_path) end end %> diff --git a/app/views/schools/register_mentor_wizard/email_address.html.erb b/app/views/schools/register_mentor_wizard/email_address.html.erb index 7a20682c..2c6c63f2 100644 --- a/app/views/schools/register_mentor_wizard/email_address.html.erb +++ b/app/views/schools/register_mentor_wizard/email_address.html.erb @@ -1,4 +1,10 @@ -<% page_data(title: "What is #{@mentor.full_name}'s email address?", error: @wizard.current_step.errors.present?, backlink_href: schools_register_mentor_wizard_review_mentor_details_path) %> +<% + page_data( + title: "What is #{@mentor.full_name}'s email address?", + error: @wizard.current_step.errors.present?, + backlink_href: @mentor.email.present? ? schools_register_mentor_wizard_check_answers_path : schools_register_mentor_wizard_review_mentor_details_path + ) +%>

We need an email so we can ask for their consent to share their details if needed for training.

@@ -6,7 +12,7 @@ <%= form_with(model: @wizard.current_step, url: @wizard.current_step_path) do |f| %> <%= content_for(:error_summary) { f.govuk_error_summary } %> - <%= f.govuk_email_field(:email, label: { text: "Email address", hidden: true, class: 'govuk-!-margin-bottom-4' }) %> + <%= f.govuk_email_field(:email, value: @mentor.email, label: { text: "Email address", hidden: true, class: 'govuk-!-margin-bottom-4' }) %> <%= f.govuk_submit "Continue" %> <% end %> diff --git a/app/views/schools/register_mentor_wizard/review_mentor_details.html.erb b/app/views/schools/register_mentor_wizard/review_mentor_details.html.erb index 71fcb06c..e08e69f9 100644 --- a/app/views/schools/register_mentor_wizard/review_mentor_details.html.erb +++ b/app/views/schools/register_mentor_wizard/review_mentor_details.html.erb @@ -1,11 +1,16 @@ -<% page_data(title: "Check mentor details", backlink_href: schools_register_mentor_wizard_find_mentor_path) %> +<% + page_data( + title: "Check mentor details", + backlink_href: @mentor.email.present? ? schools_register_mentor_wizard_check_answers_path : schools_register_mentor_wizard_find_mentor_path + ) +%>

There is a teacher record that matches the details you have provided.

<%= govuk_summary_list do |summary_list| summary_list.with_row do |row| row.with_key(text: 'Name') - row.with_value(text: @mentor.full_name) + row.with_value(text: @mentor.trs_full_name) end summary_list.with_row do |row| @@ -30,9 +35,10 @@ end %> <%= content_for(:error_summary) { f.govuk_error_summary } %> <%= f.govuk_radio_buttons_fieldset :change_name, legend: { text: "Are these details correct for the mentor?" } do %> - <%= f.govuk_radio_button :change_name, :no, label: { text: "Yes" }, link_errors: true %> - <%= f.govuk_radio_button :change_name, :yes, label: { text: "No, they changed their name or it's spelt wrong" } do %> + <%= f.govuk_radio_button :change_name, :no, checked: !@mentor.corrected_name?, label: { text: "Yes" }, link_errors: true %> + <%= f.govuk_radio_button :change_name, :yes, checked: @mentor.corrected_name?, label: { text: "No, they changed their name or it's spelt wrong" } do %> <%= f.govuk_text_field :corrected_name, + value: @mentor.corrected_name, label: { text: "Enter the correct full name" }, diff --git a/app/wizards/schools/register_mentor_wizard/mentor.rb b/app/wizards/schools/register_mentor_wizard/mentor.rb index 9ee5224b..4c03fe74 100644 --- a/app/wizards/schools/register_mentor_wizard/mentor.rb +++ b/app/wizards/schools/register_mentor_wizard/mentor.rb @@ -16,7 +16,11 @@ def active_record_at_school end def full_name - @full_name ||= (corrected_name || [trs_first_name, trs_last_name].join(" ")).strip + @full_name ||= (corrected_name || trs_full_name).strip + end + + def trs_full_name + @trs_full_name ||= [trs_first_name, trs_last_name].join(" ") end def govuk_date_of_birth @@ -27,6 +31,10 @@ def in_trs? trs_first_name.present? end + def corrected_name? + corrected_name.present? + end + def matches_trs_dob? return false if [date_of_birth, trs_date_of_birth].any?(&:blank?) diff --git a/app/wizards/schools/register_mentor_wizard/review_mentor_details_step.rb b/app/wizards/schools/register_mentor_wizard/review_mentor_details_step.rb index 17610ecc..90e2e9bc 100644 --- a/app/wizards/schools/register_mentor_wizard/review_mentor_details_step.rb +++ b/app/wizards/schools/register_mentor_wizard/review_mentor_details_step.rb @@ -17,7 +17,13 @@ def next_step private def persist - mentor.update(corrected_name: Schools::Validation::CorrectedName.new(corrected_name).formatted_name) + mentor.update(corrected_name: formatted_name) + end + + def formatted_name + return nil if change_name == "no" + + Schools::Validation::CorrectedName.new(corrected_name).formatted_name end end end diff --git a/spec/features/schools/register_mentor/happy_path_spec.rb b/spec/features/schools/register_mentor/happy_path_spec.rb index a8747cd8..c0813262 100644 --- a/spec/features/schools/register_mentor/happy_path_spec.rb +++ b/spec/features/schools/register_mentor/happy_path_spec.rb @@ -36,6 +36,42 @@ and_the_ect_is_shown_linked_to_the_mentor_just_registered end + scenario 'check your answers' do + given_there_is_a_school_in_the_service + and_there_is_an_ect_with_no_mentor_registered_at_the_school + and_i_sign_in_as_that_school_user + and_i_am_on_the_schools_landing_page + when_i_click_to_assign_a_mentor_to_the_ect + then_i_am_in_the_requirements_page + + when_i_click_continue + then_i_should_be_taken_to_the_find_mentor_page + + when_i_submit_the_find_mentor_form + then_i_should_be_taken_to_the_review_mentor_details_page + and_i_should_see_the_mentor_details_in_the_review_page + + when_i_select_that_my_mentor_name_is_incorrect + and_i_enter_the_corrected_name + and_i_click_confirm_and_continue + then_i_should_be_taken_to_the_email_address_page + + when_i_enter_the_mentor_email_address + and_i_click_continue + then_i_should_be_taken_to_the_check_answers_page + and_i_should_see_all_the_mentor_data_on_the_page + + page.get_by_role('link', name: 'Change').first.click + then_i_should_be_taken_to_the_review_mentor_details_page + and_i_click_confirm_and_continue + then_i_should_be_taken_to_the_check_answers_page + + page.get_by_role('link', name: 'Change').last.click + then_i_should_be_taken_to_the_email_address_page + and_i_click_continue + then_i_should_be_taken_to_the_check_answers_page + end + def given_there_is_a_school_in_the_service @school = FactoryBot.create(:school, urn: "1234567") end diff --git a/spec/views/schools/register_mentor_wizard/check_answers.html.erb_spec.rb b/spec/views/schools/register_mentor_wizard/check_answers.html.erb_spec.rb index 50893193..9bc98206 100644 --- a/spec/views/schools/register_mentor_wizard/check_answers.html.erb_spec.rb +++ b/spec/views/schools/register_mentor_wizard/check_answers.html.erb_spec.rb @@ -4,12 +4,14 @@ let(:ect_name) { "Michael Dixon" } let(:title) { "Check your answers and confirm mentor details" } let(:store) do - double(trn: "1234567", - trs_first_name: "John", - trs_last_name: "Wayne", - corrected_name: "Jim Wayne", - email: "john.wayne@example.com") + FactoryBot.build(:session_repository, + trn: "1234567", + trs_first_name: "John", + trs_last_name: "Wayne", + corrected_name: "Jim Wayne", + email: "john.wayne@example.com") end + let(:wizard) { FactoryBot.build(:register_mentor_wizard, current_step: :check_answers, store:) } let(:mentor) { wizard.mentor } @@ -55,4 +57,11 @@ expect(rendered).to have_button('Confirm details') expect(rendered).to have_selector("form[action='#{confirm_details_path}']") end + + it "has change links" do + render + + expect(rendered).to have_link("Change", href: schools_register_mentor_wizard_review_mentor_details_path) + expect(rendered).to have_link("Change", href: schools_register_mentor_wizard_email_address_path) + end end diff --git a/spec/views/schools/register_mentor_wizard/email_address.html.erb_spec.rb b/spec/views/schools/register_mentor_wizard/email_address.html.erb_spec.rb index 5e53c2bc..b221b160 100644 --- a/spec/views/schools/register_mentor_wizard/email_address.html.erb_spec.rb +++ b/spec/views/schools/register_mentor_wizard/email_address.html.erb_spec.rb @@ -3,8 +3,17 @@ let(:continue_path) { schools_register_mentor_wizard_email_address_path } let(:mentor) { wizard.mentor } let(:title) { "What is Jim Waters's email address?" } - let(:store) { double(trs_first_name: "John", trs_last_name: "Waters", corrected_name: "Jim Waters") } + let(:email) { nil } let(:wizard) { FactoryBot.build(:register_mentor_wizard, current_step: :email_address, store:) } + let(:store) do + FactoryBot.build(:session_repository, + trn: "1234567", + trs_first_name: "John", + trs_last_name: "Waters", + trs_date_of_birth: "1950-01-01", + corrected_name: "Jim Waters", + email:) + end before do assign(:wizard, wizard) @@ -22,6 +31,7 @@ wizard.valid_step? render end + it "prefixes the page with 'Error:' when the email is invalid" do expect(view.content_for(:page_title)).to start_with('Error:') end @@ -31,10 +41,23 @@ end end - it 'includes a back button that links to check mentor details of the journey' do - render + describe "back link" do + before { render } - expect(view.content_for(:backlink_or_breadcrumb)).to have_link('Back', href: back_path) + context "when not checking answers" do + it 'targets review mentor details page' do + expect(view.content_for(:backlink_or_breadcrumb)).to have_link('Back', href: back_path) + end + end + + context "when checking answers" do + let(:email) { 'foo@example.com' } + let(:back_path) { schools_register_mentor_wizard_check_answers_path } + + it 'targets check your answers page' do + expect(view.content_for(:backlink_or_breadcrumb)).to have_link('Back', href: back_path) + end + end end it 'includes a continue button that posts to the email address page' do diff --git a/spec/views/schools/register_mentor_wizard/review_mentor_details.html.erb_spec.rb b/spec/views/schools/register_mentor_wizard/review_mentor_details.html.erb_spec.rb index 00d602b6..938e1dd5 100644 --- a/spec/views/schools/register_mentor_wizard/review_mentor_details.html.erb_spec.rb +++ b/spec/views/schools/register_mentor_wizard/review_mentor_details.html.erb_spec.rb @@ -6,6 +6,7 @@ let(:confirm_and_continue_path) { schools_register_mentor_wizard_review_mentor_details_path } let(:national_insurance_number) { "AD12345ED" } let(:title) { "Check mentor details" } + let(:email) { nil } let(:store) do FactoryBot.build(:session_repository, trn: "1234567", @@ -14,7 +15,8 @@ trs_date_of_birth: "1950-01-01", corrected_name: nil, date_of_birth:, - national_insurance_number:) + national_insurance_number:, + email:) end let(:wizard) { FactoryBot.build(:register_mentor_wizard, current_step: :review_mentor_details, store:) } let(:mentor) { wizard.mentor } @@ -30,10 +32,23 @@ expect(sanitize(view.content_for(:page_title))).to eql(sanitize(title)) end - it 'includes a back button that links to trn and dob page of the journey' do - render + describe "back link" do + before { render } + + context "when not checking answers" do + it 'targets find mentor page' do + expect(view.content_for(:backlink_or_breadcrumb)).to have_link('Back', href: back_path) + end + end - expect(view.content_for(:backlink_or_breadcrumb)).to have_link('Back', href: back_path) + context "when checking answers" do + let(:email) { 'foo@example.com' } + let(:back_path) { schools_register_mentor_wizard_check_answers_path } + + it 'targets check your answers page' do + expect(view.content_for(:backlink_or_breadcrumb)).to have_link('Back', href: back_path) + end + end end it 'displays Name and TRN' do diff --git a/spec/wizards/schools/register_mentor_wizard/email_address_step_spec.rb b/spec/wizards/schools/register_mentor_wizard/email_address_step_spec.rb new file mode 100644 index 00000000..51c3a174 --- /dev/null +++ b/spec/wizards/schools/register_mentor_wizard/email_address_step_spec.rb @@ -0,0 +1,20 @@ +describe Schools::RegisterMentorWizard::EmailAddressStep, type: :model do + subject { described_class.new } + + describe 'validations' do + it { is_expected.to allow_value('valid@email.com').for(:email) } + it { is_expected.not_to allow_value('invalid email').for(:email) } + end + + describe '#next_step' do + let(:wizard) do + FactoryBot.build(:register_mentor_wizard, current_step: :email_address, step_params:) + end + + let(:step_params) do + ActionController::Parameters.new("email_address" => { "email" => 'valid@email.com' }) + end + + it { expect(wizard.next_step).to eq(:check_answers) } + end +end