From ecd6dc6a37cd31ec71d8242c83ea85bf6c465b9e Mon Sep 17 00:00:00 2001 From: Felix Clack Date: Mon, 6 Nov 2023 11:44:13 +0000 Subject: [PATCH] Format date errors according to the style guide The GOVUK design system describes specific error styling for date fields that the current helpers don't support. Due to the way we build the errors and Rails conventions, there is no support for highlighting errors with specific parts of a date input. Adding support for this requires a few changes in the form builder. I considered adding these changes upstream in the form builder gem but opted instead to monkey-patch them initially so that we can explore them in a real environment first. Once we have seen these changes work, I will propose the changes to the form library and remove the monkey-patch if they are accepted. Mostly, the changes are around how we determine if an error message is present for one of the date part input fields. --- app/models/search.rb | 138 +++++++++++++++--- config/locales/en.yml | 3 +- spec/models/search_spec.rb | 12 +- ...ches_with_an_invalid_date_of_birth_spec.rb | 31 ++++ .../user_searches_with_invalid_values_spec.rb | 2 +- 5 files changed, 154 insertions(+), 32 deletions(-) create mode 100644 spec/system/check_records/user_searches_with_an_invalid_date_of_birth_spec.rb diff --git a/app/models/search.rb b/app/models/search.rb index 34db3f2f..169da72d 100644 --- a/app/models/search.rb +++ b/app/models/search.rb @@ -1,3 +1,102 @@ +module GOVUKDesignSystemFormBuilder + module Elements + class Date + using PrefixableArray + + def field_id(segment:, link_errors: false) + if link_errors && has_errors?(segment) + build_id('field-error', attribute_name: segment, include_value: false) + else + build_id('field') + end + end + + private + + def classes(width, segment = nil) + build_classes( + %(input), + %(date-input__input), + %(input--width-#{width}), + %(input--error) => has_errors?(segment), + ).prefix(brand) + end + + def has_errors?(segment = nil) + @builder.object.respond_to?(:errors) && @builder.object.errors.any? && + (@builder.object.errors.messages[@attribute_name].present? || + @builder.object.errors.messages[segment].present?) + end + + def id(segment, link_errors) + if has_errors?(segment) && link_errors + field_id(link_errors:, segment:) + else + [@object_name, @attribute_name, SEGMENTS.fetch(segment)].join("_") + end + end + + def input(segment, link_errors, width, value) + tag.input( + id: id(segment, link_errors), + class: classes(width, segment), + name: name(segment), + type: 'text', + inputmode: 'numeric', + value:, + autocomplete: date_of_birth_autocomplete_value(segment), + maxlength: (width if maxlength_enabled?), + ) + end + + def day + date_part(:day, width: 2, link_errors: true) + end + + def month + date_part(:month, width: 2, link_errors: true) + end + + def year + date_part(:year, width: 4, link_errors: true) + end + end + + class ErrorMessage < Base + def has_errors? + @builder.object.respond_to?(:errors) && @builder.object.errors.any? && + (@builder.object.errors[@attribute_name].present? || + (@attribute_name == :date_of_birth && + (@builder.object.errors.messages.keys & %i[date_of_birth day month year]).any? + ) + ) + end + + private + + def message + error = @builder.object.errors.messages[@attribute_name]&.first + error ||= @builder.object.errors.messages[:day]&.first if has_errors? + set_message_safety(error) + end + end + end + + module Containers + class FormGroup < Base + def has_errors? + @builder.object.respond_to?(:errors) && + @builder.object.errors.any? && + (@builder.object.errors[@attribute_name].present? || + (@attribute_name == :date_of_birth && + (@builder.object.errors.messages.keys & %i[date_of_birth day month year]).any? + ) + ) + end + end + end +end + class Search DateOfBirth = Struct.new(:year, :month, :day) do @@ -12,7 +111,8 @@ def to_s attr_reader :date_of_birth validates :last_name, presence: true - validate :date_of_birth_is_valid + validates :date_of_birth, presence: true + validate :date_of_birth_is_valid, if: -> { date_of_birth.present? } def date_of_birth=(date_fields) return if date_fields.nil? @@ -28,28 +128,20 @@ def date_of_birth=(date_fields) private def date_of_birth_is_valid - if @date_of_birth.nil? - errors.add(:date_of_birth, t(:blank)) - return - end - - year = @date_of_birth.year.to_i - month = @date_of_birth.month.to_i - day = @date_of_birth.day.to_i - - if day.zero? && month.zero? && year.zero? - errors.add(:date_of_birth, t(:blank)) - return - end + year = date_of_birth.year.to_i + month = date_of_birth.month.to_i + day = date_of_birth.day.to_i if day.zero? - errors.add(:date_of_birth, t(:missing_day)) - return + errors.add(:day, t(:missing_day)) end if month.zero? - errors.add(:date_of_birth, t(:missing_month)) - nil + errors.add(:month, t(:missing_month)) + end + + if year.zero? + errors.add(:year, t(:missing_year)) end begin @@ -60,22 +152,20 @@ def date_of_birth_is_valid end if date.after?(16.years.ago) - errors.add(:date_of_birth, t(:inclusion)) + errors.add(:year, t(:inclusion)) return end if date.year < 1000 - errors.add(:date_of_birth, t(:missing_year)) + errors.add(:year, t(:missing_year)) return end if date.year < 1900 - errors.add(:date_of_birth, t(:born_after_1900)) - nil + errors.add(:year, t(:born_after_1900)) end rescue Date::Error - errors.add(:date_of_birth, t(:blank)) - nil + errors.add(:date_of_birth, t(:invalid)) if (errors.messages.keys & %i[day month year]).empty? end end diff --git a/config/locales/en.yml b/config/locales/en.yml index ecd374de..38e2a2bf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -18,9 +18,10 @@ en: search: attributes: date_of_birth: - blank: Enter a valid date of birth + blank: Enter a your date of birth born_after_1900: Enter a year of birth later than 1900 inclusion: People must be 16 or over to use this service + invalid: Date of birth must be a real date missing_day: Enter a day for the date of birth, formatted as a number missing_month: Enter a month for the date of birth, formatted as a number missing_year: Enter a year with 4 digits diff --git a/spec/models/search_spec.rb b/spec/models/search_spec.rb index b7057296..720a731a 100644 --- a/spec/models/search_spec.rb +++ b/spec/models/search_spec.rb @@ -22,7 +22,7 @@ it "is invalid" do expect(search).to be_invalid - expect(search.errors[:date_of_birth]).to include( + expect(search.errors[:day]).to include( "Enter a day for the date of birth, formatted as a number" ) end @@ -33,7 +33,7 @@ it "is invalid" do expect(search).to be_invalid - expect(search.errors[:date_of_birth]).to include( + expect(search.errors[:month]).to include( "Enter a month for the date of birth, formatted as a number" ) end @@ -62,7 +62,7 @@ it "is invalid" do expect(search).to be_invalid - expect(search.errors[:date_of_birth]).to include("Enter a valid date of birth") + expect(search.errors[:date_of_birth]).to include("Date of birth must be a real date") end end @@ -80,7 +80,7 @@ it "is invalid" do expect(search).to be_invalid - expect(search.errors[:date_of_birth]).to include( + expect(search.errors[:year]).to include( "People must be 16 or over to use this service" ) end @@ -91,7 +91,7 @@ it "is invalid" do expect(search).to be_invalid - expect(search.errors[:date_of_birth]).to include("Enter a year of birth later than 1900") + expect(search.errors[:year]).to include("Enter a year of birth later than 1900") end end @@ -100,7 +100,7 @@ it "is invalid" do expect(search).to be_invalid - expect(search.errors[:date_of_birth]).to include("Enter a year with 4 digits") + expect(search.errors[:year]).to include("Enter a year with 4 digits") end end end diff --git a/spec/system/check_records/user_searches_with_an_invalid_date_of_birth_spec.rb b/spec/system/check_records/user_searches_with_an_invalid_date_of_birth_spec.rb new file mode 100644 index 00000000..b0863ede --- /dev/null +++ b/spec/system/check_records/user_searches_with_an_invalid_date_of_birth_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe "Teacher search", host: :check_records, type: :system do + include ActivateFeaturesSteps + include CheckRecords::AuthenticationSteps + + scenario "User searches with an invalid date of birth", + test: %i[with_stubbed_auth with_fake_quals_api] do + given_the_service_is_open + when_i_sign_in_via_dsi + and_search_with_a_missing_dob + then_i_see_an_error_message + end + + private + + def and_search_with_a_missing_dob + fill_in "Last name", with: "Walsh" + fill_in "Day", with: "5" + fill_in "Month", with: "April" + click_button "Find record" + end + + def then_i_see_an_error_message + expect(page).to have_text("Enter a year with 4 digits") + page.click_link("Enter a year with 4 digits") + expect(page).to have_css("#search-year-field-error") + end +end \ No newline at end of file diff --git a/spec/system/check_records/user_searches_with_invalid_values_spec.rb b/spec/system/check_records/user_searches_with_invalid_values_spec.rb index e85e124d..2e616982 100644 --- a/spec/system/check_records/user_searches_with_invalid_values_spec.rb +++ b/spec/system/check_records/user_searches_with_invalid_values_spec.rb @@ -47,7 +47,7 @@ def then_i_see_the_error_summary def then_i_see_the_missing_dob_error within "#search-date-of-birth-error" do expect(page).to have_content "Error:" - expect(page).to have_content "Enter a valid date of birth" + expect(page).to have_content "Enter a day for the date of birth, formatted as a number" end end