From ee0b0d97c5796cafca518f4525ea690052b73a5d Mon Sep 17 00:00:00 2001 From: Felix Clack Date: Fri, 31 May 2024 08:59:18 +0100 Subject: [PATCH 1/3] Remove unnecessary assignment of param The base Form class handles assigning params to defined attributes and so this line in the initialize method is redundant. --- app/forms/current_school_form.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/forms/current_school_form.rb b/app/forms/current_school_form.rb index f8d8b7297a..e12d576330 100644 --- a/app/forms/current_school_form.rb +++ b/app/forms/current_school_form.rb @@ -10,7 +10,6 @@ def initialize(claim:, journey_session:, journey:, params:) super load_schools - self.current_school_id = permitted_params[:current_school_id] end def save From 81bf14ceafa8ec5678dba4a473d0aea6fe8be5ee Mon Sep 17 00:00:00 2001 From: Felix Clack Date: Fri, 31 May 2024 09:01:01 +0100 Subject: [PATCH 2/3] Save the current_school_id on the journey session We're moving all the answers to the journey session, so this change ensures we save the current_school_id here too. --- app/forms/current_school_form.rb | 2 ++ spec/forms/current_school_form_spec.rb | 7 +++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/forms/current_school_form.rb b/app/forms/current_school_form.rb index e12d576330..d65c38ab55 100644 --- a/app/forms/current_school_form.rb +++ b/app/forms/current_school_form.rb @@ -15,6 +15,8 @@ def initialize(claim:, journey_session:, journey:, params:) def save return false unless valid? + journey_session.answers.assign_attributes(current_school_id:) + journey_session.save update!({"eligibility_attributes" => {"current_school_id" => current_school_id}}) end diff --git a/spec/forms/current_school_form_spec.rb b/spec/forms/current_school_form_spec.rb index fab796f101..ef76546000 100644 --- a/spec/forms/current_school_form_spec.rb +++ b/spec/forms/current_school_form_spec.rb @@ -12,7 +12,7 @@ CurrentClaim.new(claims: claims) end - let(:journey_session) { build(:"#{journey::I18N_NAMESPACE}_session") } + let(:journey_session) { create(:"#{journey::I18N_NAMESPACE}_session") } let(:slug) { "current-school" } @@ -82,9 +82,12 @@ describe "#save" do context "current_school_id submitted" do let(:params) { ActionController::Parameters.new({slug: slug, claim: {current_school_id: school.id}}) } - let(:school) { create(:school, :eligible_for_journey, journey: journey) } + it 'updates the journey_session' do + expect { form.save }.to change { journey_session.reload.answers.current_school_id }.to(school.id) + end + context "claim eligibility didn't have current_school" do let(:current_claim) do claims = journey::POLICIES.map { |policy| create(:claim, policy: policy) } From 62c2510ea9014c5419dffc955094dec14ee8d77a Mon Sep 17 00:00:00 2001 From: Felix Clack Date: Wed, 5 Jun 2024 11:52:56 +0100 Subject: [PATCH 3/3] Reference the journey session for the current school value The journey session holds a reference to the current school and this can be used when calculating the slug and page sequence. --- app/forms/current_school_form.rb | 16 +++++++++------- .../slug_sequence.rb | 5 +++-- .../admin_tasks_presenter.rb | 4 ++-- spec/forms/current_school_form_spec.rb | 2 +- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/app/forms/current_school_form.rb b/app/forms/current_school_form.rb index d65c38ab55..a292fea904 100644 --- a/app/forms/current_school_form.rb +++ b/app/forms/current_school_form.rb @@ -16,13 +16,11 @@ def save return false unless valid? journey_session.answers.assign_attributes(current_school_id:) - journey_session.save - update!({"eligibility_attributes" => {"current_school_id" => current_school_id}}) + journey_session.save! + update!("eligibility_attributes" => {"current_school_id" => current_school_id}) end - def current_school_name - claim.eligibility.current_school_name - end + delegate :name, to: :current_school, prefix: true, allow_nil: true def no_search_results? params[:school_search].present? && errors.empty? @@ -30,6 +28,10 @@ def no_search_results? private + def current_school + @current_school ||= School.find_by(id: current_school_id) + end + def load_schools return unless params[:school_search] @@ -41,8 +43,8 @@ def load_schools end def current_school_must_be_open - if (school = School.find_by(id: current_school_id)) - errors.add(:current_school_id, i18n_errors_path("the_selected_school_is_closed")) unless school.open? + if current_school + errors.add(:current_school_id, i18n_errors_path("the_selected_school_is_closed")) unless current_school.open? else errors.add(:current_school_id, i18n_errors_path("school_not_found")) end diff --git a/app/models/journeys/additional_payments_for_teaching/slug_sequence.rb b/app/models/journeys/additional_payments_for_teaching/slug_sequence.rb index bb3efa6844..40bac53910 100644 --- a/app/models/journeys/additional_payments_for_teaching/slug_sequence.rb +++ b/app/models/journeys/additional_payments_for_teaching/slug_sequence.rb @@ -236,9 +236,10 @@ def induction_question_required? end def ecp_school_selected? - return false unless claim.eligibility.current_school + school = answers&.current_school || claim.eligibility.current_school + return false unless school - Policies::EarlyCareerPayments::SchoolEligibility.new(claim.eligibility.current_school).eligible? + Policies::EarlyCareerPayments::SchoolEligibility.new(school).eligible? end # We only retrieve dqt teacher status when the user is signed in with DfE diff --git a/app/models/policies/early_career_payments/admin_tasks_presenter.rb b/app/models/policies/early_career_payments/admin_tasks_presenter.rb index ace7f0fe1f..2f06200673 100644 --- a/app/models/policies/early_career_payments/admin_tasks_presenter.rb +++ b/app/models/policies/early_career_payments/admin_tasks_presenter.rb @@ -19,8 +19,8 @@ def employment def identity_confirmation [ - ["Current school", eligibility.current_school.name], - ["Contact number", eligibility.current_school.phone_number] + ["Current school", eligibility.current_school&.name], + ["Contact number", eligibility.current_school&.phone_number] ] end diff --git a/spec/forms/current_school_form_spec.rb b/spec/forms/current_school_form_spec.rb index ef76546000..a8677ce930 100644 --- a/spec/forms/current_school_form_spec.rb +++ b/spec/forms/current_school_form_spec.rb @@ -84,7 +84,7 @@ let(:params) { ActionController::Parameters.new({slug: slug, claim: {current_school_id: school.id}}) } let(:school) { create(:school, :eligible_for_journey, journey: journey) } - it 'updates the journey_session' do + it "updates the journey_session" do expect { form.save }.to change { journey_session.reload.answers.current_school_id }.to(school.id) end