From cf4bd5016090633bf4482b1db1609e3739c2a2f5 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Thu, 29 Aug 2024 11:32:31 +0100 Subject: [PATCH 1/2] Update error message copy Updates the error messages on the verify claim form, and the nested assertions forms, to match the copy on figma. --- .../provider/verify_claim_form.rb | 46 +++++- .../provider/claims/verify_claim.html.erb | 2 +- config/locales/en.yml | 73 +++++++-- .../provider/verify_claim_form_spec.rb | 145 ++++++++++++++++-- 4 files changed, 236 insertions(+), 30 deletions(-) diff --git a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb index 8e9d21a8ae..b1a0bd8461 100644 --- a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb +++ b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb @@ -29,7 +29,9 @@ class VerifyClaimForm < Form attribute :declaration, :boolean - validates :declaration, acceptance: true + validates :declaration, acceptance: { + message: i18n_error_message("declaration.acceptance") + } validate :all_assertions_answered @@ -63,7 +65,11 @@ def course_descriptions def assertions @assertions ||= ASSERTIONS.fetch(contract_type).map do |assertion_name| - AssertionForm.new(name: assertion_name) + AssertionForm.new( + name: assertion_name, + claim: claim, + type: contract_type + ) end end @@ -116,7 +122,7 @@ def all_assertions_answered assertion.errors.each do |error| errors.add( "assertions_attributes[#{i}][#{error.attribute}]", - error.full_message + error.message ) end end @@ -132,15 +138,39 @@ class AssertionForm include ActiveModel::Model include ActiveModel::Attributes + attr_reader :claim, :type + attribute :name, :string attribute :outcome, :boolean validates :name, presence: true validates :outcome, inclusion: { in: [true, false], - message: "Select an option" + message: ->(form, _) do + I18n.t( + [ + "further_education_payments_provider", + "forms", + "verify_claim", + "assertions", + form.type, + form.name, + "errors", + "inclusion" + ].join("."), + claimant: form.claimant, + provider: form.provider + ) + end } + def initialize(name:, claim:, type:) + @claim = claim + @type = type + + super(name: name) + end + def radio_options [ RadioOption.new(id: true, name: "Yes"), @@ -149,6 +179,14 @@ def radio_options end class RadioOption < Struct.new(:id, :name, keyword_init: true); end + + def claimant + claim.first_name + end + + def provider + claim.school.name + end end end end diff --git a/app/views/further_education_payments/provider/claims/verify_claim.html.erb b/app/views/further_education_payments/provider/claims/verify_claim.html.erb index 0902792c1d..d956200eb9 100644 --- a/app/views/further_education_payments/provider/claims/verify_claim.html.erb +++ b/app/views/further_education_payments/provider/claims/verify_claim.html.erb @@ -60,7 +60,7 @@ legend: { size: "s", text: f.object.t( - [:assertions, f.object.contract_type, ff.object.name], + [:assertions, f.object.contract_type, ff.object.name, :label], claimant: f.object.claimant_name, provider: f.object.claim.school.name, ), diff --git a/config/locales/en.yml b/config/locales/en.yml index a68a521fac..c9269dd615 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1116,23 +1116,68 @@ en: claimant_date_of_birth: "Claimant date of birth" claimant_trn: "Claimant teacher reference number (TRN)" claim_date: "Claim date" + errors: + declaration: + acceptance: "Tick the box to confirm that the information provided in this form is correct to the best of your knowledge" assertions: fixed_contract: - contract_type: "Does %{claimant} have a permanent contract of employment at %{provider}?" - teaching_responsibilities: "Is %{claimant} a member of staff with teaching responsibilities?" - further_education_teaching_start_year: "Is %{claimant} in the first 5 years of their further education teaching career in England?" - teaching_hours_per_week: "Is %{claimant} timetabled to teach an average of 12 hours per week during the current term?" - hours_teaching_eligible_subjects: "For at least half of their timetabled teaching hours, does %{claimant} teach 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP)?" - subjects_taught: "For at least half of their timetabled teaching hours, does %{claimant} teach:" + contract_type: + label: "Does %{claimant} have a permanent contract of employment at %{provider}?" + errors: + inclusion: "Select yes if %{claimant} has a fixed term contract of employment at %{provider}" + teaching_responsibilities: + label: "Is %{claimant} a member of staff with teaching responsibilities?" + errors: + inclusion: "Select yes if %{claimant} is a member of staff with teaching responsibilities" + further_education_teaching_start_year: + label: "Is %{claimant} in the first 5 years of their further education teaching career in England?" + errors: + inclusion: "Select yes if %{claimant} is in the first 5 years of their further education teaching career in England" + teaching_hours_per_week: + label: "Is %{claimant} timetabled to teach an average of 12 hours per week during the current term?" + errors: + inclusion: "Select yes if %{claimant} is timetabled to teach an average of 12 hours per week during the current term" + hours_teaching_eligible_subjects: + label: "For at least half of their timetabled teaching hours, does %{claimant} teach 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP)?" + errors: + inclusion: "Select yes if %{claimant} teaches 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP), for at least half of their timetabled teaching hours" + subjects_taught: + label: "For at least half of their timetabled teaching hours, does %{claimant} teach:" + errors: + inclusion: "Select yes if %{claimant} teaches this course for at least half their timetabled teaching hours" variable_contract: - contract_type: "Does %{claimant} have a variable hour contract of employment at %{provider}?" - teaching_responsibilities: "Is %{claimant} a member of staff with teaching responsibilities?" - further_education_teaching_start_year: "Is %{claimant} in the first 5 years of their further education teaching career in England?" - taught_at_least_one_term: "Has %{claimant} taught for at least one academic term at %{provider}?" - teaching_hours_per_week: "Is %{claimant} timetabled to teach an average of 12 hours per week during the current term?" - hours_teaching_eligible_subjects: "For at least half of their timetabled teaching hours, does %{claimant} teach 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP)?" - subjects_taught: "For at least half of their timetabled teaching hours, does %{claimant} teach:" - teaching_hours_per_week_next_term: "Will %{claimant} be timetabled to teach at least 2.5 hours per week next term?" + contract_type: + label: "Does %{claimant} have a variable hour contract of employment at %{provider}?" + errors: + inclusion: "Select yes if %{claimant} has a variable hours contract of employment at %{provider}" + teaching_responsibilities: + label: "Is %{claimant} a member of staff with teaching responsibilities?" + errors: + inclusion: "Select yes if %{claimant} is a member of staff with teaching responsibilities" + further_education_teaching_start_year: + label: "Is %{claimant} in the first 5 years of their further education teaching career in England?" + errors: + inclusion: "Select yes if %{claimant} is in the first 5 years of their further education teaching career in England" + taught_at_least_one_term: + label: "Has %{claimant} taught for at least one academic term at %{provider}?" + errors: + inclusion: "Select yes if %{claimant} has taught at least one academic term at %{provider}" + teaching_hours_per_week: + label: "Is %{claimant} timetabled to teach an average of 12 hours per week during the current term?" + errors: + inclusion: "Select yes if %{claimant} is timetabled to teach an average of 12 hours per week during the current term" + hours_teaching_eligible_subjects: + label: "For at least half of their timetabled teaching hours, does %{claimant} teach 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP)?" + errors: + inclusion: "Select yes if %{claimant} teaches 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP), for at least half of their timetabled teaching hours" + subjects_taught: + label: "For at least half of their timetabled teaching hours, does %{claimant} teach:" + errors: + inclusion: "Select yes if %{claimant} teaches this course for at least half their timetabled teaching hours" + teaching_hours_per_week_next_term: + label: "Will %{claimant} be timetabled to teach at least 2.5 hours per week next term?" + errors: + inclusion: "Select yes if %{claimant} will be timetabled to teach at least 2.5 hours per week next term" early_years_payment: claim_description: for an early years financial incentive payment diff --git a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb index 835c20c6c1..c4014e0b0e 100644 --- a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb +++ b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb @@ -3,9 +3,26 @@ RSpec.describe Journeys::FurtherEducationPayments::Provider::VerifyClaimForm, type: :model do let(:journey) { Journeys::FurtherEducationPayments::Provider } - let(:eligibility) { create(:further_education_payments_eligibility) } + let(:school) do + create(:school, :further_education, name: "Springfield Elementary") + end - let(:claim) { eligibility.claim } + let(:eligibility) do + create( + :further_education_payments_eligibility, + school: school + ) + end + + let(:claim) do + create( + :claim, + eligibility: eligibility, + policy: Policies::FurtherEducationPayments, + first_name: "Edna", + surname: "Krabappel" + ) + end let(:journey_session) do create( @@ -34,15 +51,11 @@ subject { form } it do - is_expected.to validate_acceptance_of(:declaration) - end - - it "validates all assertions are answered" do - form.validate - - form.assertions.each do |assertion| - expect(assertion.errors[:outcome]).to eq(["Select an option"]) - end + is_expected.to( + validate_acceptance_of(:declaration).with_message( + "Tick the box to confirm that the information provided in this form is correct to the best of your knowledge" + ) + ) end it "validates the claim hasn't already been verified" do @@ -110,6 +123,116 @@ end end + describe "AssertionForm" do + let(:contract_type) { "fixed_contract" } + + subject do + described_class::AssertionForm.new( + name: assertion_name, + type: contract_type, + claim: claim + ) + end + + context "when the assertion is `contract_type`" do + let(:assertion_name) { "contract_type" } + + context "when fixed term" do + let(:contract_type) { "fixed_contract" } + + it do + is_expected.not_to(allow_value(nil).for(:outcome).with_message( + "Select yes if Edna has a fixed term contract of employment at Springfield Elementary" + )) + end + end + + context "when variable" do + let(:contract_type) { "variable_contract" } + + it do + is_expected.not_to(allow_value(nil).for(:outcome).with_message( + "Select yes if Edna has a variable hours contract of employment at Springfield Elementary" + )) + end + end + end + + context "when the assertion is `teaching_responsibilities`" do + let(:assertion_name) { "teaching_responsibilities" } + + it do + is_expected.not_to(allow_value(nil).for(:outcome).with_message( + "Select yes if Edna is a member of staff with teaching responsibilities" + )) + end + end + + context "when the assertion is `further_education_teaching_start_year`" do + let(:assertion_name) { "further_education_teaching_start_year" } + + it do + is_expected.not_to(allow_value(nil).for(:outcome).with_message( + "Select yes if Edna is in the first 5 years of their further education teaching career in England" + )) + end + end + + context "when the assertion is `taught_at_least_one_term`" do + let(:assertion_name) { "taught_at_least_one_term" } + + let(:contract_type) { "variable_contract" } + + it do + is_expected.not_to(allow_value(nil).for(:outcome).with_message( + "Select yes if Edna has taught at least one academic term at Springfield Elementary" + )) + end + end + + context "when the assertion is `teaching_hours_per_week`" do + let(:assertion_name) { "teaching_hours_per_week" } + + it do + is_expected.not_to(allow_value(nil).for(:outcome).with_message( + "Select yes if Edna is timetabled to teach an average of 12 hours per week during the current term" + )) + end + end + + context "when the assertion is `hours_teaching_eligible_subjects`" do + let(:assertion_name) { "hours_teaching_eligible_subjects" } + + it do + is_expected.not_to(allow_value(nil).for(:outcome).with_message( + "Select yes if Edna teaches 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP), for at least half of their timetabled teaching hours" + )) + end + end + + context "when the assertion is `subjects_taught`" do + let(:assertion_name) { "subjects_taught" } + + it do + is_expected.not_to(allow_value(nil).for(:outcome).with_message( + "Select yes if Edna teaches this course for at least half their timetabled teaching hours" + )) + end + end + + context "when the assertion is `teaching_hours_per_week_next_term`" do + let(:assertion_name) { "teaching_hours_per_week_next_term" } + + let(:contract_type) { "variable_contract" } + + it do + is_expected.not_to(allow_value(nil).for(:outcome).with_message( + "Select yes if Edna will be timetabled to teach at least 2.5 hours per week next term" + )) + end + end + end + describe "#save" do let(:params) do ActionController::Parameters.new( From d51e0616b4e57c3210bdc40136366468010aa1c4 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Thu, 29 Aug 2024 15:32:36 +0100 Subject: [PATCH 2/2] Fix teaching hours label and error message The teaching hours per week label needs to be variable depending on the option selected by the claimant. This commit updates the label and error message to be variable. We also now pass the parent form into the assertion form rather than passing in the claim and contract type. --- .../provider/verify_claim_form.rb | 42 +++++++++++++------ .../provider/claims/verify_claim.html.erb | 1 + config/locales/en.yml | 8 ++-- .../provider_verifying_claims_spec.rb | 16 ++++--- .../provider/verify_claim_form_spec.rb | 41 +++++++++++------- 5 files changed, 71 insertions(+), 37 deletions(-) diff --git a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb index b1a0bd8461..367951adc4 100644 --- a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb +++ b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb @@ -63,13 +63,21 @@ def course_descriptions @course_descriptions ||= claim.eligibility.courses_taught.map(&:description) end + def teaching_hours_per_week + I18n.t( + [ + "further_education_payments", + "forms", + "teaching_hours_per_week", + "options", + claim.eligibility.teaching_hours_per_week + ].join(".") + ).downcase + end + def assertions @assertions ||= ASSERTIONS.fetch(contract_type).map do |assertion_name| - AssertionForm.new( - name: assertion_name, - claim: claim, - type: contract_type - ) + AssertionForm.new(name: assertion_name, parent_form: self) end end @@ -138,7 +146,7 @@ class AssertionForm include ActiveModel::Model include ActiveModel::Attributes - attr_reader :claim, :type + attr_reader :parent_form attribute :name, :string attribute :outcome, :boolean @@ -153,20 +161,20 @@ class AssertionForm "forms", "verify_claim", "assertions", - form.type, + form.contract_type, form.name, "errors", "inclusion" ].join("."), claimant: form.claimant, - provider: form.provider + provider: form.provider, + hours: form.hours ) end } - def initialize(name:, claim:, type:) - @claim = claim - @type = type + def initialize(name:, parent_form:) + @parent_form = parent_form super(name: name) end @@ -181,11 +189,19 @@ def radio_options class RadioOption < Struct.new(:id, :name, keyword_init: true); end def claimant - claim.first_name + parent_form.claim.first_name end def provider - claim.school.name + parent_form.claim.school.name + end + + def hours + parent_form.teaching_hours_per_week + end + + def contract_type + parent_form.contract_type end end end diff --git a/app/views/further_education_payments/provider/claims/verify_claim.html.erb b/app/views/further_education_payments/provider/claims/verify_claim.html.erb index d956200eb9..2418aa6baf 100644 --- a/app/views/further_education_payments/provider/claims/verify_claim.html.erb +++ b/app/views/further_education_payments/provider/claims/verify_claim.html.erb @@ -63,6 +63,7 @@ [:assertions, f.object.contract_type, ff.object.name, :label], claimant: f.object.claimant_name, provider: f.object.claim.school.name, + hours: f.object.teaching_hours_per_week ), }, hint: -> do diff --git a/config/locales/en.yml b/config/locales/en.yml index c9269dd615..e4f3618eaa 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1134,9 +1134,9 @@ en: errors: inclusion: "Select yes if %{claimant} is in the first 5 years of their further education teaching career in England" teaching_hours_per_week: - label: "Is %{claimant} timetabled to teach an average of 12 hours per week during the current term?" + label: "Is %{claimant} timetabled to teach an average of %{hours} during the current term?" errors: - inclusion: "Select yes if %{claimant} is timetabled to teach an average of 12 hours per week during the current term" + inclusion: "Select yes if %{claimant} is timetabled to teach an average of %{hours} during the current term" hours_teaching_eligible_subjects: label: "For at least half of their timetabled teaching hours, does %{claimant} teach 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP)?" errors: @@ -1163,9 +1163,9 @@ en: errors: inclusion: "Select yes if %{claimant} has taught at least one academic term at %{provider}" teaching_hours_per_week: - label: "Is %{claimant} timetabled to teach an average of 12 hours per week during the current term?" + label: "Is %{claimant} timetabled to teach an average of %{hours} during the current term?" errors: - inclusion: "Select yes if %{claimant} is timetabled to teach an average of 12 hours per week during the current term" + inclusion: "Select yes if %{claimant} is timetabled to teach an average of %{hours} during the current term" hours_teaching_eligible_subjects: label: "For at least half of their timetabled teaching hours, does %{claimant} teach 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP)?" errors: diff --git a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb index baef59cae5..ff6ed7b41c 100644 --- a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb +++ b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb @@ -269,7 +269,8 @@ create( :further_education_payments_eligibility, claim: claim_1, - school: fe_provider + school: fe_provider, + teaching_hours_per_week: "more_than_12" ) claim_2 = create( @@ -284,7 +285,8 @@ create( :further_education_payments_eligibility, claim: claim_2, - school: fe_provider + school: fe_provider, + teaching_hours_per_week: "more_than_12" ) mock_dfe_sign_in_auth_session( @@ -395,6 +397,7 @@ :further_education_payments_eligibility, claim: claim, school: fe_provider, + teaching_hours_per_week: "more_than_12", contract_type: "fixed_term", subjects_taught: ["engineering_manufacturing"], engineering_manufacturing_courses: [ @@ -461,8 +464,8 @@ end within_fieldset( - "Is Edna Krabappel timetabled to teach an average of 12 hours per " \ - "week during the current term?" + "Is Edna Krabappel timetabled to teach an average of more than 12 " \ + "hours per week during the current term?" ) do choose "Yes" end @@ -518,6 +521,7 @@ claim: claim, school: fe_provider, contract_type: "variable_hours", + teaching_hours_per_week: "between_2_5_and_12", subjects_taught: ["engineering_manufacturing"], engineering_manufacturing_courses: [ "approved_level_321_transportation", @@ -590,8 +594,8 @@ end within_fieldset( - "Is Edna Krabappel timetabled to teach an average of 12 hours per " \ - "week during the current term?" + "Is Edna Krabappel timetabled to teach an average of between 2.5 and " \ + "12 hours per week during the current term?" ) do choose "Yes" end diff --git a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb index c4014e0b0e..c2d7cb0ddf 100644 --- a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb +++ b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb @@ -7,10 +7,16 @@ create(:school, :further_education, name: "Springfield Elementary") end + let(:teaching_hours_per_week) { "more_than_12" } + + let(:contract_type) { "fixed_term" } + let(:eligibility) do create( :further_education_payments_eligibility, - school: school + school: school, + teaching_hours_per_week: teaching_hours_per_week, + contract_type: contract_type ) end @@ -124,13 +130,10 @@ end describe "AssertionForm" do - let(:contract_type) { "fixed_contract" } - subject do described_class::AssertionForm.new( name: assertion_name, - type: contract_type, - claim: claim + parent_form: form ) end @@ -138,8 +141,6 @@ let(:assertion_name) { "contract_type" } context "when fixed term" do - let(:contract_type) { "fixed_contract" } - it do is_expected.not_to(allow_value(nil).for(:outcome).with_message( "Select yes if Edna has a fixed term contract of employment at Springfield Elementary" @@ -148,7 +149,7 @@ end context "when variable" do - let(:contract_type) { "variable_contract" } + let(:contract_type) { "variable_hours" } it do is_expected.not_to(allow_value(nil).for(:outcome).with_message( @@ -181,7 +182,7 @@ context "when the assertion is `taught_at_least_one_term`" do let(:assertion_name) { "taught_at_least_one_term" } - let(:contract_type) { "variable_contract" } + let(:contract_type) { "variable_hours" } it do is_expected.not_to(allow_value(nil).for(:outcome).with_message( @@ -193,10 +194,22 @@ context "when the assertion is `teaching_hours_per_week`" do let(:assertion_name) { "teaching_hours_per_week" } - it do - is_expected.not_to(allow_value(nil).for(:outcome).with_message( - "Select yes if Edna is timetabled to teach an average of 12 hours per week during the current term" - )) + context "when more that 12" do + it do + is_expected.not_to(allow_value(nil).for(:outcome).with_message( + "Select yes if Edna is timetabled to teach an average of more than 12 hours per week during the current term" + )) + end + end + + context "when between 2.5 and 12" do + let(:teaching_hours_per_week) { "between_2_5_and_12" } + + it do + is_expected.not_to(allow_value(nil).for(:outcome).with_message( + "Select yes if Edna is timetabled to teach an average of between 2.5 and 12 hours per week during the current term" + )) + end end end @@ -223,7 +236,7 @@ context "when the assertion is `teaching_hours_per_week_next_term`" do let(:assertion_name) { "teaching_hours_per_week_next_term" } - let(:contract_type) { "variable_contract" } + let(:contract_type) { "variable_hours" } it do is_expected.not_to(allow_value(nil).for(:outcome).with_message(