From ec788716be14a18a8836d912c48cddadceec444b Mon Sep 17 00:00:00 2001 From: Shujat Khalid Date: Mon, 7 Oct 2024 15:38:35 +0100 Subject: [PATCH 01/10] Change to copy on age range subjects page --- app/views/shared/_age_range_subjects_form_fields.html.erb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/shared/_age_range_subjects_form_fields.html.erb b/app/views/shared/_age_range_subjects_form_fields.html.erb index a92d81198f..764e7dd303 100644 --- a/app/views/shared/_age_range_subjects_form_fields.html.erb +++ b/app/views/shared/_age_range_subjects_form_fields.html.erb @@ -1,5 +1,7 @@ <%= f.govuk_fieldset legend: { text: "What age range is the applicant qualified to teach?" } do %> -

Based on the evidence the applicant has provided, you can either copy the age range they entered if you agree, or enter a new range.

+

You can edit the age range below if:

+

- based on evidence, you do not agree with the age range the applicant has submitted

+

- the 'To' age is greater than 19 and will not be accepted by DQT

<%= f.govuk_number_field :age_range_min, width: 3 %> <%= f.govuk_number_field :age_range_max, width: 3 %> From 5e52a3c8ec67e2ffd4102e1c898e4edeacc1fda4 Mon Sep 17 00:00:00 2001 From: Hassan Mir Date: Mon, 7 Oct 2024 16:00:55 +0100 Subject: [PATCH 02/10] Validation for age range max to be max 19 --- .../age_range_subjects_form.rb | 1 + config/locales/assessor_interface.en.yml | 5 +++ .../check_age_range_subjects_form_spec.rb | 8 ---- .../confirm_age_range_subjects_form_spec.rb | 8 ---- .../age_range_subjects_form.rb | 45 +++++++++++-------- 5 files changed, 33 insertions(+), 34 deletions(-) diff --git a/app/forms/concerns/assessor_interface/age_range_subjects_form.rb b/app/forms/concerns/assessor_interface/age_range_subjects_form.rb index 263a7f878a..03e6821559 100644 --- a/app/forms/concerns/assessor_interface/age_range_subjects_form.rb +++ b/app/forms/concerns/assessor_interface/age_range_subjects_form.rb @@ -19,6 +19,7 @@ module AssessorInterface::AgeRangeSubjectsForm presence: true, numericality: { only_integer: true, + less_than_or_equal_to: 19, greater_than_or_equal_to: :age_range_min, allow_nil: true, } diff --git a/config/locales/assessor_interface.en.yml b/config/locales/assessor_interface.en.yml index f971e968e1..a9bbcf443e 100644 --- a/config/locales/assessor_interface.en.yml +++ b/config/locales/assessor_interface.en.yml @@ -348,6 +348,11 @@ en: blank: Enter a note to the applicant written_statement_recent_notes: blank: Enter a note to the applicant + assessor_interface/check_age_range_subjects_form: + attributes: + age_range_max: + less_than_or_equal_to: We cannot accept an age greater than 19 + greater_than_or_equal_to: ‘To’ must be greater than ‘From’ assessor_interface/confirm_age_range_subjects_form: attributes: age_range_min: diff --git a/spec/forms/assessor_interface/check_age_range_subjects_form_spec.rb b/spec/forms/assessor_interface/check_age_range_subjects_form_spec.rb index ee56c606b0..75b2e688b9 100644 --- a/spec/forms/assessor_interface/check_age_range_subjects_form_spec.rb +++ b/spec/forms/assessor_interface/check_age_range_subjects_form_spec.rb @@ -27,12 +27,4 @@ it { is_expected.to validate_presence_of(:user) } it { is_expected.to allow_values(true, false).for(:passed) } end - - describe "#save" do - subject(:save) { form.save } - - describe "when invalid attributes" do - it { is_expected.to be false } - end - end end diff --git a/spec/forms/assessor_interface/confirm_age_range_subjects_form_spec.rb b/spec/forms/assessor_interface/confirm_age_range_subjects_form_spec.rb index bfc593b3aa..ac8d40ac19 100644 --- a/spec/forms/assessor_interface/confirm_age_range_subjects_form_spec.rb +++ b/spec/forms/assessor_interface/confirm_age_range_subjects_form_spec.rb @@ -17,12 +17,4 @@ it { is_expected.to validate_presence_of(:assessment) } it { is_expected.to validate_presence_of(:user) } end - - describe "#save" do - subject(:save) { form.save } - - describe "when invalid attributes" do - it { is_expected.to be false } - end - end end diff --git a/spec/support/shared_examples/age_range_subjects_form.rb b/spec/support/shared_examples/age_range_subjects_form.rb index 8b2ef72a8a..8cbc213348 100644 --- a/spec/support/shared_examples/age_range_subjects_form.rb +++ b/spec/support/shared_examples/age_range_subjects_form.rb @@ -1,38 +1,47 @@ # frozen_string_literal: true RSpec.shared_examples_for "an age range subjects form" do + it { is_expected.to validate_presence_of(:age_range_min) } + it { is_expected.to validate_presence_of(:age_range_max) } + describe "validations" do - it { is_expected.to validate_presence_of(:age_range_min) } - it { is_expected.to validate_presence_of(:age_range_max) } + let(:age_range_subjects_attributes) do + { + age_range_min: "5", + age_range_max: "16", + subject_1: "Subject", + subject_1_raw: "Subject", + } + end context "with a minimum too low" do - let(:age_range_subjects_attributes) { { age_range_min: "1" } } + before { age_range_subjects_attributes[:age_range_min] = "-1" } it { is_expected.to be_invalid } end - context "with a minimum too high" do - let(:age_range_subjects_attributes) { { age_range_min: "20" } } + context "with a minimum between 0 and 19" do + before { age_range_subjects_attributes[:age_range_min] = "8" } + + it { is_expected.to be_valid } + end + + context "with a maximum lower than the minimum" do + before { age_range_subjects_attributes[:age_range_max] = "4" } it { is_expected.to be_invalid } end - context "when minimum is set" do - context "with a maximum too low" do - let(:age_range_subjects_attributes) do - { age_range_min: "7", age_range_max: "1" } - end + context "with a maximum greater than min but below 20" do + before { age_range_subjects_attributes[:age_range_max] = "19" } - it { is_expected.to be_invalid } - end + it { is_expected.to be_valid } + end - context "with a maximum too high" do - let(:age_range_subjects_attributes) do - { age_range_min: "7", age_range_max: "20" } - end + context "with a maximum too high" do + before { age_range_subjects_attributes[:age_range_max] = "20" } - it { is_expected.to be_invalid } - end + it { is_expected.to be_invalid } end it { is_expected.not_to validate_presence_of(:age_range_note) } From c4c6f9a9a7e2017cf2a102f1510ee43ab80fcfc8 Mon Sep 17 00:00:00 2001 From: Shujat Khalid Date: Mon, 7 Oct 2024 18:19:52 +0100 Subject: [PATCH 03/10] Amendment to content styling --- app/views/shared/_age_range_subjects_form_fields.html.erb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/views/shared/_age_range_subjects_form_fields.html.erb b/app/views/shared/_age_range_subjects_form_fields.html.erb index 764e7dd303..e1335ef1cb 100644 --- a/app/views/shared/_age_range_subjects_form_fields.html.erb +++ b/app/views/shared/_age_range_subjects_form_fields.html.erb @@ -1,7 +1,10 @@ <%= f.govuk_fieldset legend: { text: "What age range is the applicant qualified to teach?" } do %>

You can edit the age range below if:

-

- based on evidence, you do not agree with the age range the applicant has submitted

-

- the 'To' age is greater than 19 and will not be accepted by DQT

+ +
    +
  • based on evidence, you do not agree with the age range the applicant has submitted
    • +
  • the 'To' age is greater than 19 and will not be accepted by DQT
    • +
<%= f.govuk_number_field :age_range_min, width: 3 %> <%= f.govuk_number_field :age_range_max, width: 3 %> From 8d9134b97258060b12b50af498ead416e7d954d6 Mon Sep 17 00:00:00 2001 From: Hassan Mir Date: Tue, 8 Oct 2024 13:40:57 +0100 Subject: [PATCH 04/10] Validate age range note if age range has been changed --- .../age_range_subjects_form.rb | 12 +++++++ config/locales/assessor_interface.en.yml | 9 +++++ .../age_range_subjects_form.rb | 35 ++++++++++++++++--- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/app/forms/concerns/assessor_interface/age_range_subjects_form.rb b/app/forms/concerns/assessor_interface/age_range_subjects_form.rb index 03e6821559..0797f7956a 100644 --- a/app/forms/concerns/assessor_interface/age_range_subjects_form.rb +++ b/app/forms/concerns/assessor_interface/age_range_subjects_form.rb @@ -8,6 +8,7 @@ module AssessorInterface::AgeRangeSubjectsForm attribute :age_range_max, :integer attribute :age_range_note, :string + validates :age_range_note, presence: true, if: :age_range_changed_from_application_form? validates :age_range_min, presence: true, numericality: { @@ -61,4 +62,15 @@ def save true end + + private + + delegate :application_form, to: :assessment, allow_nil: true + + def age_range_changed_from_application_form? + return unless assessment + + application_form.age_range_min != age_range_min || + application_form.age_range_max != age_range_max + end end diff --git a/config/locales/assessor_interface.en.yml b/config/locales/assessor_interface.en.yml index a9bbcf443e..df8e2d0d98 100644 --- a/config/locales/assessor_interface.en.yml +++ b/config/locales/assessor_interface.en.yml @@ -350,15 +350,24 @@ en: blank: Enter a note to the applicant assessor_interface/check_age_range_subjects_form: attributes: + age_range_min: + blank: Enter the minimum age age_range_max: + blank: Enter the maximum age less_than_or_equal_to: We cannot accept an age greater than 19 greater_than_or_equal_to: ‘To’ must be greater than ‘From’ + age_range_note: + blank: Explain why you have entered a new age range assessor_interface/confirm_age_range_subjects_form: attributes: age_range_min: blank: Enter the minimum age age_range_max: blank: Enter the maximum age + less_than_or_equal_to: We cannot accept an age greater than 19 + greater_than_or_equal_to: ‘To’ must be greater than ‘From’ + age_range_note: + blank: Explain why you have entered a new age range subject_1: blank: Enter the subject assessor_interface/consent_method_form: diff --git a/spec/support/shared_examples/age_range_subjects_form.rb b/spec/support/shared_examples/age_range_subjects_form.rb index 8cbc213348..f58952f570 100644 --- a/spec/support/shared_examples/age_range_subjects_form.rb +++ b/spec/support/shared_examples/age_range_subjects_form.rb @@ -9,11 +9,25 @@ { age_range_min: "5", age_range_max: "16", + age_range_note: "Note", subject_1: "Subject", subject_1_raw: "Subject", } end + before do + assessment.application_form.update(age_range_min: 5, age_range_max: 16) + end + + context "when the minimum has changed but a note has not been added" do + before do + age_range_subjects_attributes[:age_range_min] = "6" + age_range_subjects_attributes[:age_range_note] = "" + end + + it { is_expected.to be_invalid } + end + context "with a minimum too low" do before { age_range_subjects_attributes[:age_range_min] = "-1" } @@ -26,6 +40,15 @@ it { is_expected.to be_valid } end + context "when the maximum has changed but a note has not been added" do + before do + age_range_subjects_attributes[:age_range_max] = "18" + age_range_subjects_attributes[:age_range_note] = "" + end + + it { is_expected.to be_invalid } + end + context "with a maximum lower than the minimum" do before { age_range_subjects_attributes[:age_range_max] = "4" } @@ -56,6 +79,10 @@ describe "#save" do subject(:save) { form.save } + before do + assessment.application_form.update(age_range_min: 5, age_range_max: 16) + end + describe "when invalid attributes" do it { is_expected.to be false } end @@ -63,8 +90,8 @@ describe "with valid attributes and no note" do let(:age_range_subjects_attributes) do { - age_range_min: "7", - age_range_max: "11", + age_range_min: "5", + age_range_max: "16", subject_1: "Subject", subject_1_raw: "Subject", } @@ -75,8 +102,8 @@ it "sets the attributes" do save # rubocop:disable Rails/SaveBang - expect(assessment.age_range_min).to eq(7) - expect(assessment.age_range_max).to eq(11) + expect(assessment.age_range_min).to eq(5) + expect(assessment.age_range_max).to eq(16) expect(assessment.age_range_note).to be_blank expect(assessment.subjects).to eq(%w[Subject]) From 5b399b47dcd5362839657c2ddf1c6eeba99aa6a4 Mon Sep 17 00:00:00 2001 From: Hassan Mir Date: Tue, 8 Oct 2024 13:41:39 +0100 Subject: [PATCH 05/10] Fix bug on assessment review/confirm where the age range note and subjects note were not appearing --- .../assessment_recommendation_award_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/assessor_interface/assessment_recommendation_award_controller.rb b/app/controllers/assessor_interface/assessment_recommendation_award_controller.rb index 3fef073947..26a153af3a 100644 --- a/app/controllers/assessor_interface/assessment_recommendation_award_controller.rb +++ b/app/controllers/assessor_interface/assessment_recommendation_award_controller.rb @@ -59,9 +59,11 @@ def edit_age_range_subjects assessment:, age_range_min: assessment.age_range_min, age_range_max: assessment.age_range_max, + age_range_note: assessment.age_range_note, subject_1: assessment.subjects.first, subject_2: assessment.subjects.second, subject_3: assessment.subjects.third, + subjects_note: assessment.subjects_note, ) end From b2b10a7b416ef05317c08ea33017c808ddc894c4 Mon Sep 17 00:00:00 2001 From: Hassan Mir Date: Tue, 8 Oct 2024 14:10:57 +0100 Subject: [PATCH 06/10] Lint fixes --- .../concerns/assessor_interface/age_range_subjects_form.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/forms/concerns/assessor_interface/age_range_subjects_form.rb b/app/forms/concerns/assessor_interface/age_range_subjects_form.rb index 0797f7956a..4722c1e302 100644 --- a/app/forms/concerns/assessor_interface/age_range_subjects_form.rb +++ b/app/forms/concerns/assessor_interface/age_range_subjects_form.rb @@ -8,7 +8,9 @@ module AssessorInterface::AgeRangeSubjectsForm attribute :age_range_max, :integer attribute :age_range_note, :string - validates :age_range_note, presence: true, if: :age_range_changed_from_application_form? + validates :age_range_note, + presence: true, + if: :age_range_changed_from_application_form? validates :age_range_min, presence: true, numericality: { From 50b03c1a4f9adaf993e6ff24587d186dac60b6e3 Mon Sep 17 00:00:00 2001 From: Hassan Mir Date: Tue, 8 Oct 2024 14:39:36 +0100 Subject: [PATCH 07/10] Remove tests that are duplicated. These tests are already covered in the AssessorInterface::AssessmentSectionForm tests --- .../check_age_range_subjects_form_spec.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/spec/forms/assessor_interface/check_age_range_subjects_form_spec.rb b/spec/forms/assessor_interface/check_age_range_subjects_form_spec.rb index 75b2e688b9..9c166d94dd 100644 --- a/spec/forms/assessor_interface/check_age_range_subjects_form_spec.rb +++ b/spec/forms/assessor_interface/check_age_range_subjects_form_spec.rb @@ -21,10 +21,4 @@ let(:assessment) { assessment_section.assessment } let(:attributes) { { passed: true } } end - - describe "validations" do - it { is_expected.to validate_presence_of(:assessment_section) } - it { is_expected.to validate_presence_of(:user) } - it { is_expected.to allow_values(true, false).for(:passed) } - end end From 16e24d28cd2222fcb6d6cdec8c996e38209c300e Mon Sep 17 00:00:00 2001 From: Hassan Mir Date: Tue, 8 Oct 2024 14:46:03 +0100 Subject: [PATCH 08/10] Fix system spec following the updates --- spec/system/assessor_interface/completing_assessment_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/system/assessor_interface/completing_assessment_spec.rb b/spec/system/assessor_interface/completing_assessment_spec.rb index 53b161be50..b413a0650a 100644 --- a/spec/system/assessor_interface/completing_assessment_spec.rb +++ b/spec/system/assessor_interface/completing_assessment_spec.rb @@ -217,6 +217,7 @@ def given_there_is_a_verifiable_application_form :assessment, age_range_max: 11, age_range_min: 8, + age_range_note: "Explanation", application_form:, subjects: %w[mathematics], ) From bb017a9842e16c7d053ec2e27a6dc774deb2df98 Mon Sep 17 00:00:00 2001 From: Hassan Mir Date: Tue, 8 Oct 2024 14:49:56 +0100 Subject: [PATCH 09/10] Remove unnecessary allow nil which was added to fix a test --- .../concerns/assessor_interface/age_range_subjects_form.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/forms/concerns/assessor_interface/age_range_subjects_form.rb b/app/forms/concerns/assessor_interface/age_range_subjects_form.rb index 4722c1e302..86412e9f1c 100644 --- a/app/forms/concerns/assessor_interface/age_range_subjects_form.rb +++ b/app/forms/concerns/assessor_interface/age_range_subjects_form.rb @@ -67,7 +67,7 @@ def save private - delegate :application_form, to: :assessment, allow_nil: true + delegate :application_form, to: :assessment def age_range_changed_from_application_form? return unless assessment From 529217d4f81b69af1de93b82df06719bd6681cc9 Mon Sep 17 00:00:00 2001 From: Hassan Mir Date: Wed, 9 Oct 2024 15:27:46 +0100 Subject: [PATCH 10/10] Remove optional wording from the age range note text box --- config/locales/helpers.en.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/locales/helpers.en.yml b/config/locales/helpers.en.yml index 6052a9e38f..8014e65559 100644 --- a/config/locales/helpers.en.yml +++ b/config/locales/helpers.en.yml @@ -92,7 +92,7 @@ en: assessor_interface_assessment_section_form: age_range_max: To age_range_min: From - age_range_note: 'Internal note: If you''ve entered a new range please explain why (optional)' + age_range_note: 'Internal note: If you''ve entered a new range please explain why' failure_reason_notes: decline: Note to the applicant (optional) document: Note to the applicant @@ -112,7 +112,7 @@ en: assessor_interface_confirm_age_range_subjects_form: age_range_min: From age_range_max: To - age_range_note: 'Internal note: If you''ve entered a new range please explain why (optional)' + age_range_note: 'Internal note: If you''ve entered a new range please explain why' subject_1: Subject 1 subject_2: Subject 2 (optional) subject_3: Subject 3 (optional)