From 3b7948a139cae99190debe2024aa47d29fa1f666 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 12 Oct 2024 16:50:40 +0200 Subject: [PATCH] Use a date field to select the date of birth The default `date_select` used in fields presents an accessibility issue, because in generates three select controls but only one label. That means that there are two controls without a label. So we're using a date field instead. This type is field is supported by about 99% of the browsers, and we've already got JavaScript code converting this field to a jQuery UI datepicker in case the browser doesn't support date fields. Note that, since we no longer need to parse the three date fields into one, we can simplify the code in both the models and the tests. Another slight improvement is that, previously, we couldn't restrict the month and day controls in order to set the minimum date, so the maximum selectable date was always the 31st of December of the year set by the minimum age setting. As seen in the component test, now that we use only one field, we can set a specific date as the maximum one. --- .../date_of_birth_field_component.html.erb | 2 +- .../shared/date_of_birth_field_component.rb | 5 ++--- app/lib/active_model/dates.rb | 13 ------------- app/models/officing/residence.rb | 8 +++----- app/models/verification/management/document.rb | 11 +++-------- app/models/verification/residence.rb | 7 +++---- .../date_of_birth_field_component_spec.rb | 4 ++-- spec/models/officing/residence_spec.rb | 8 ++------ .../verification/management/document_spec.rb | 8 ++------ spec/models/verification/residence_spec.rb | 12 +++--------- spec/support/common_actions/verifications.rb | 11 +---------- spec/system/admin/local_census_records_spec.rb | 8 ++------ .../management/document_verifications_spec.rb | 4 ++-- spec/system/management/users_spec.rb | 4 ++-- spec/system/officing/residence_spec.rb | 2 +- spec/system/verification/residence_spec.rb | 18 ++++++------------ 16 files changed, 35 insertions(+), 90 deletions(-) delete mode 100644 app/lib/active_model/dates.rb diff --git a/app/components/shared/date_of_birth_field_component.html.erb b/app/components/shared/date_of_birth_field_component.html.erb index 91fedf367..7330e2c41 100644 --- a/app/components/shared/date_of_birth_field_component.html.erb +++ b/app/components/shared/date_of_birth_field_component.html.erb @@ -1 +1 @@ -<%= form.date_select :date_of_birth, **field_options %> +<%= form.date_field :date_of_birth, **field_options %> diff --git a/app/components/shared/date_of_birth_field_component.rb b/app/components/shared/date_of_birth_field_component.rb index 9289ff05f..4fa87f9de 100644 --- a/app/components/shared/date_of_birth_field_component.rb +++ b/app/components/shared/date_of_birth_field_component.rb @@ -10,9 +10,8 @@ class Shared::DateOfBirthFieldComponent < ApplicationComponent def default_options { - prompt: true, - start_year: 1900, - end_year: minimum_required_age.years.ago.year + min: "1900-01-01", + max: minimum_required_age.years.ago } end diff --git a/app/lib/active_model/dates.rb b/app/lib/active_model/dates.rb deleted file mode 100644 index b10dceafa..000000000 --- a/app/lib/active_model/dates.rb +++ /dev/null @@ -1,13 +0,0 @@ -module ActiveModel::Dates - def parse_date(field, attrs) - year, month, day = attrs["#{field}(1i)"], attrs["#{field}(2i)"], attrs["#{field}(3i)"] - - return nil if day.blank? || month.blank? || year.blank? - - Date.new(year.to_i, month.to_i, day.to_i) - end - - def remove_date(field, attrs) - attrs.except("#{field}(1i)", "#{field}(2i)", "#{field}(3i)") - end -end diff --git a/app/models/officing/residence.rb b/app/models/officing/residence.rb index 711acb018..b4c673ceb 100644 --- a/app/models/officing/residence.rb +++ b/app/models/officing/residence.rb @@ -1,10 +1,10 @@ class Officing::Residence include ActiveModel::Model - include ActiveModel::Dates + include ActiveModel::Attributes include ActiveModel::Validations::Callbacks - attr_accessor :user, :officer, :document_number, :document_type, :year_of_birth, - :date_of_birth, :postal_code + attribute :date_of_birth, :date + attr_accessor :user, :officer, :document_number, :document_type, :year_of_birth, :postal_code before_validation :retrieve_census_data @@ -18,8 +18,6 @@ class Officing::Residence validate :local_residence def initialize(attrs = {}) - self.date_of_birth = parse_date("date_of_birth", attrs) - attrs = remove_date("date_of_birth", attrs) super clean_document_number end diff --git a/app/models/verification/management/document.rb b/app/models/verification/management/document.rb index 9ffc9ce6b..7fd19e46d 100644 --- a/app/models/verification/management/document.rb +++ b/app/models/verification/management/document.rb @@ -1,8 +1,9 @@ class Verification::Management::Document include ActiveModel::Model - include ActiveModel::Dates + include ActiveModel::Attributes - attr_accessor :document_type, :document_number, :date_of_birth, :postal_code + attribute :date_of_birth, :date + attr_accessor :document_type, :document_number, :postal_code validates :document_type, :document_number, presence: true validates :date_of_birth, presence: true, if: -> { Setting.force_presence_date_of_birth? } @@ -10,12 +11,6 @@ class Verification::Management::Document delegate :username, :email, to: :user, allow_nil: true - def initialize(attrs = {}) - self.date_of_birth = parse_date("date_of_birth", attrs) - attrs = remove_date("date_of_birth", attrs) - super - end - def user @user = User.active.by_document(document_type, document_number).first end diff --git a/app/models/verification/residence.rb b/app/models/verification/residence.rb index d52e56476..5ccf8d19a 100644 --- a/app/models/verification/residence.rb +++ b/app/models/verification/residence.rb @@ -1,9 +1,10 @@ class Verification::Residence include ActiveModel::Model - include ActiveModel::Dates + include ActiveModel::Attributes include ActiveModel::Validations::Callbacks - attr_accessor :user, :document_number, :document_type, :date_of_birth, :postal_code, :terms_of_service + attribute :date_of_birth, :date + attr_accessor :user, :document_number, :document_type, :postal_code, :terms_of_service validates :document_number, presence: true validates :document_type, presence: true @@ -18,8 +19,6 @@ class Verification::Residence validate :local_residence def initialize(attrs = {}) - self.date_of_birth = parse_date("date_of_birth", attrs) - attrs = remove_date("date_of_birth", attrs) super clean_document_number end diff --git a/spec/components/shared/date_of_birth_field_component_spec.rb b/spec/components/shared/date_of_birth_field_component_spec.rb index 8687b84cf..403df8693 100644 --- a/spec/components/shared/date_of_birth_field_component_spec.rb +++ b/spec/components/shared/date_of_birth_field_component_spec.rb @@ -19,8 +19,8 @@ describe Shared::DateOfBirthFieldComponent do travel_to "2015-07-15" do render_inline component - expect(page).to have_select with_options: [2002, 2001, 2000, 1999] - expect(page).not_to have_select with_options: [2003] + expect(page).to have_field "Date of birth" + expect(page).to have_css "input[type=date][min='1900-01-01'][max='2002-07-15']" end end end diff --git a/spec/models/officing/residence_spec.rb b/spec/models/officing/residence_spec.rb index d30113111..c5049535a 100644 --- a/spec/models/officing/residence_spec.rb +++ b/spec/models/officing/residence_spec.rb @@ -84,18 +84,14 @@ describe Officing::Residence do describe "dates" do it "is not valid but not because date of birth" do - custom_residence = Officing::Residence.new("date_of_birth(3i)" => "1", - "date_of_birth(2i)" => "1", - "date_of_birth(1i)" => "1980") + custom_residence = Officing::Residence.new(date_of_birth: "1980-01-01") expect(custom_residence).not_to be_valid expect(custom_residence.errors[:date_of_birth]).to be_empty end it "is not valid without a date of birth" do - custom_residence = Officing::Residence.new("date_of_birth(3i)" => "", - "date_of_birth(2i)" => "", - "date_of_birth(1i)" => "") + custom_residence = Officing::Residence.new(date_of_birth: "") expect(custom_residence).not_to be_valid expect(custom_residence.errors[:date_of_birth]).to include("can't be blank") end diff --git a/spec/models/verification/management/document_spec.rb b/spec/models/verification/management/document_spec.rb index 78d41c486..c089c5c3a 100644 --- a/spec/models/verification/management/document_spec.rb +++ b/spec/models/verification/management/document_spec.rb @@ -57,17 +57,13 @@ describe Verification::Management::Document do describe "dates" do it "is valid with a valid date of birth" do - verification_document = Verification::Management::Document.new("date_of_birth(3i)" => "1", - "date_of_birth(2i)" => "1", - "date_of_birth(1i)" => "1980") + verification_document = Verification::Management::Document.new(date_of_birth: "1980-01-01") expect(verification_document.errors[:date_of_birth]).to be_empty end it "is not valid without a date of birth" do - verification_document = Verification::Management::Document.new("date_of_birth(3i)" => "", - "date_of_birth(2i)" => "", - "date_of_birth(1i)" => "") + verification_document = Verification::Management::Document.new(date_of_birth: "") expect(verification_document).not_to be_valid expect(verification_document.errors[:date_of_birth]).to include("can't be blank") end diff --git a/spec/models/verification/residence_spec.rb b/spec/models/verification/residence_spec.rb index 988b2a805..a9df7bb8a 100644 --- a/spec/models/verification/residence_spec.rb +++ b/spec/models/verification/residence_spec.rb @@ -11,26 +11,20 @@ describe Verification::Residence do describe "dates" do it "is valid with a valid date of birth" do - residence = Verification::Residence.new("date_of_birth(3i)" => "1", - "date_of_birth(2i)" => "1", - "date_of_birth(1i)" => "1980") + residence = Verification::Residence.new(date_of_birth: "1980-01-01") expect(residence.errors[:date_of_birth]).to be_empty end it "is not valid without a date of birth" do - residence = Verification::Residence.new("date_of_birth(3i)" => "", - "date_of_birth(2i)" => "", - "date_of_birth(1i)" => "") + residence = Verification::Residence.new(date_of_birth: "") expect(residence).not_to be_valid expect(residence.errors[:date_of_birth]).to include("can't be blank") end end it "validates user has allowed age" do - residence = Verification::Residence.new("date_of_birth(3i)" => "1", - "date_of_birth(2i)" => "1", - "date_of_birth(1i)" => 5.years.ago.year.to_s) + residence = Verification::Residence.new(date_of_birth: 5.years.ago) expect(residence).not_to be_valid expect(residence.errors[:date_of_birth]).to include("You don't have the required age to participate") end diff --git a/spec/support/common_actions/verifications.rb b/spec/support/common_actions/verifications.rb index b6e65fae4..6dad2584c 100644 --- a/spec/support/common_actions/verifications.rb +++ b/spec/support/common_actions/verifications.rb @@ -1,17 +1,8 @@ module Verifications - def select_date(values, selector) - selector = selector[:from] - day, month, year = values.split("-") - select day, from: "#{selector}_3i" - select month, from: "#{selector}_2i" - select year, from: "#{selector}_1i" - end - def verify_residence select "DNI", from: "residence_document_type" fill_in "residence_document_number", with: "12345678Z" - select_date "31-#{I18n.l(Date.current.at_end_of_year, format: "%B")}-1980", - from: "residence_date_of_birth" + fill_in "residence_date_of_birth", with: Date.new(1980, 12, 31) fill_in "residence_postal_code", with: "28013" check "residence_terms_of_service" diff --git a/spec/system/admin/local_census_records_spec.rb b/spec/system/admin/local_census_records_spec.rb index 93bb19096..c40d926c7 100644 --- a/spec/system/admin/local_census_records_spec.rb +++ b/spec/system/admin/local_census_records_spec.rb @@ -84,9 +84,7 @@ describe "Admin local census records", :admin do select "DNI", from: :local_census_record_document_type fill_in :local_census_record_document_number, with: "#DOCUMENT" - select "1982", from: :local_census_record_date_of_birth_1i - select "July", from: :local_census_record_date_of_birth_2i - select "7", from: :local_census_record_date_of_birth_3i + fill_in "Date of birth", with: Date.new(1982, 7, 7) fill_in :local_census_record_postal_code, with: "07003" click_button "Save" @@ -116,9 +114,7 @@ describe "Admin local census records", :admin do select "Passport", from: :local_census_record_document_type fill_in :local_census_record_document_number, with: "#NIE_NUMBER" - select "1982", from: :local_census_record_date_of_birth_1i - select "August", from: :local_census_record_date_of_birth_2i - select "8", from: :local_census_record_date_of_birth_3i + fill_in "Date of birth", with: Date.new(1982, 8, 8) fill_in :local_census_record_postal_code, with: "07007" click_button "Save" diff --git a/spec/system/management/document_verifications_spec.rb b/spec/system/management/document_verifications_spec.rb index 2403bcd8a..328bc647f 100644 --- a/spec/system/management/document_verifications_spec.rb +++ b/spec/system/management/document_verifications_spec.rb @@ -63,7 +63,7 @@ describe "DocumentVerifications" do login_as_manager visit management_document_verifications_path fill_in "document_verification_document_number", with: "12345678Z" - select_date "31-December-1980", from: "document_verification_date_of_birth" + fill_in "Date of birth", with: Date.new(1980, 12, 31) fill_in "document_verification_postal_code", with: "inexisting" click_button "Check document" @@ -77,7 +77,7 @@ describe "DocumentVerifications" do login_as_manager visit management_document_verifications_path fill_in "document_verification_document_number", with: "12345678Z" - select_date "31-December-1980", from: "document_verification_date_of_birth" + fill_in "Date of birth", with: Date.new(1980, 12, 31) fill_in "document_verification_postal_code", with: "28013" click_button "Check document" diff --git a/spec/system/management/users_spec.rb b/spec/system/management/users_spec.rb index fccf14b29..ce8f0647b 100644 --- a/spec/system/management/users_spec.rb +++ b/spec/system/management/users_spec.rb @@ -13,7 +13,7 @@ describe "Users" do fill_in "user_username", with: "pepe" fill_in "user_email", with: "pepe@gmail.com" - select_date "31-December-1980", from: "user_date_of_birth" + fill_in "Date of birth", with: Date.new(1980, 12, 31) click_button "Create user" @@ -54,7 +54,7 @@ describe "Users" do fill_in "user_username", with: "Kelly Sue" fill_in "user_email", with: "" - select_date "31-December-1980", from: "user_date_of_birth" + fill_in "Date of birth", with: Date.new(1980, 12, 31) click_button "Create user" diff --git a/spec/system/officing/residence_spec.rb b/spec/system/officing/residence_spec.rb index 9ccee04ed..f1ee43f8c 100644 --- a/spec/system/officing/residence_spec.rb +++ b/spec/system/officing/residence_spec.rb @@ -162,7 +162,7 @@ describe "Residence", :with_frozen_time do select "DNI", from: "residence_document_type" fill_in "residence_document_number", with: "12345678Z" - select_date "31-December-1980", from: "residence_date_of_birth" + fill_in "Date of birth", with: Date.new(1980, 12, 31) fill_in "residence_postal_code", with: "28013" click_button "Validate document" diff --git a/spec/system/verification/residence_spec.rb b/spec/system/verification/residence_spec.rb index a806cc378..fa8850730 100644 --- a/spec/system/verification/residence_spec.rb +++ b/spec/system/verification/residence_spec.rb @@ -12,7 +12,7 @@ describe "Residence" do fill_in "Document number", with: "12345678Z" select "DNI", from: "residence_document_type" - select_date "31-December-1980", from: "residence_date_of_birth" + fill_in "Date of birth", with: Date.new(1980, 12, 31) fill_in "residence_postal_code", with: "28013" check "residence_terms_of_service" click_button "Verify residence" @@ -30,7 +30,7 @@ describe "Residence" do fill_in "Document number", with: "12345678Z" select "DNI", from: "residence_document_type" - select_date "31-December-1980", from: "residence_date_of_birth" + fill_in "Date of birth", with: Date.new(1980, 12, 31) fill_in "residence_postal_code", with: "28013" check "residence_terms_of_service" click_button "Verify residence" @@ -50,7 +50,7 @@ describe "Residence" do fill_in "Document number", with: "12345678Z" select "DNI", from: "residence_document_type" - select_date "31-December-1980", from: "residence_date_of_birth" + fill_in "Date of birth", with: Date.new(1980, 12, 31) fill_in "residence_postal_code", with: "28013" check "residence_terms_of_service" @@ -85,9 +85,7 @@ describe "Residence" do fill_in "Document number", with: "12345678Z" select "DNI", from: "residence_document_type" - select "1997", from: "residence_date_of_birth_1i" - select "January", from: "residence_date_of_birth_2i" - select "1", from: "residence_date_of_birth_3i" + fill_in "Date of birth", with: Date.new(1997, 1, 1) fill_in "residence_postal_code", with: "00000" check "residence_terms_of_service" @@ -105,9 +103,7 @@ describe "Residence" do fill_in "Document number", with: "12345678Z" select "DNI", from: "residence_document_type" - select "1997", from: "residence_date_of_birth_1i" - select "January", from: "residence_date_of_birth_2i" - select "1", from: "residence_date_of_birth_3i" + fill_in "Date of birth", with: Date.new(1997, 1, 1) fill_in "residence_postal_code", with: "28013" check "residence_terms_of_service" @@ -126,9 +122,7 @@ describe "Residence" do 5.times do fill_in "Document number", with: "12345678Z" select "DNI", from: "residence_document_type" - select "1997", from: "residence_date_of_birth_1i" - select "January", from: "residence_date_of_birth_2i" - select "1", from: "residence_date_of_birth_3i" + fill_in "Date of birth", with: Date.new(1997, 1, 1) fill_in "residence_postal_code", with: "28013" check "residence_terms_of_service"