From 6af8ddd32465e7d918dff27b1d66f9efab160b74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 12 Oct 2024 15:56:47 +0200 Subject: [PATCH 01/11] Extract component to render the date of birth We reduce code duplication thanks to that, and make it easier to change this field. Note that there was one place where the "16.years" value was hardcoded. We're moving the test for this case to the component and changing it so the test doesn't use the default age. We're also removing the redundant helper method that had the same code as a method in the User class which is called everywhere else. --- .../date_of_birth_field_component.html.erb | 1 + .../shared/date_of_birth_field_component.rb | 26 +++++++++++++++++++ app/helpers/verification_helper.rb | 4 --- .../admin/local_census_records/_form.html.erb | 4 +-- .../document_verifications/index.html.erb | 4 +-- app/views/management/users/new.html.erb | 5 +--- app/views/officing/residence/new.html.erb | 4 +-- app/views/verification/residence/new.html.erb | 4 +-- .../date_of_birth_field_component_spec.rb | 26 +++++++++++++++++++ spec/system/verification/residence_spec.rb | 15 ----------- 10 files changed, 58 insertions(+), 35 deletions(-) create mode 100644 app/components/shared/date_of_birth_field_component.html.erb create mode 100644 app/components/shared/date_of_birth_field_component.rb create mode 100644 spec/components/shared/date_of_birth_field_component_spec.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 new file mode 100644 index 000000000..91fedf367 --- /dev/null +++ b/app/components/shared/date_of_birth_field_component.html.erb @@ -0,0 +1 @@ +<%= form.date_select :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 new file mode 100644 index 000000000..9289ff05f --- /dev/null +++ b/app/components/shared/date_of_birth_field_component.rb @@ -0,0 +1,26 @@ +class Shared::DateOfBirthFieldComponent < ApplicationComponent + attr_reader :form, :options + + def initialize(form, **options) + @form = form + @options = options + end + + private + + def default_options + { + prompt: true, + start_year: 1900, + end_year: minimum_required_age.years.ago.year + } + end + + def field_options + default_options.merge(options) + end + + def minimum_required_age + User.minimum_required_age + end +end diff --git a/app/helpers/verification_helper.rb b/app/helpers/verification_helper.rb index 3dd9123c9..17f57b774 100644 --- a/app/helpers/verification_helper.rb +++ b/app/helpers/verification_helper.rb @@ -5,10 +5,6 @@ module VerificationHelper [t("verification.residence.new.document_type.residence_card"), 3]] end - def minimum_required_age - (Setting["min_age_to_participate"] || 16).to_i - end - def mask_phone(number) match = number.match(/\d{3}$/) "******#{match}" diff --git a/app/views/admin/local_census_records/_form.html.erb b/app/views/admin/local_census_records/_form.html.erb index c84bb997c..11ef1fb01 100644 --- a/app/views/admin/local_census_records/_form.html.erb +++ b/app/views/admin/local_census_records/_form.html.erb @@ -15,9 +15,7 @@
- <%= f.date_select :date_of_birth, - prompt: true, - start_year: 1900, end_year: minimum_required_age.years.ago.year %> + <%= render Shared::DateOfBirthFieldComponent.new(f) %>
diff --git a/app/views/management/document_verifications/index.html.erb b/app/views/management/document_verifications/index.html.erb index 4c81f65ef..59010bf0b 100644 --- a/app/views/management/document_verifications/index.html.erb +++ b/app/views/management/document_verifications/index.html.erb @@ -17,9 +17,7 @@ <% if Setting.force_presence_date_of_birth? %>
- <%= f.date_select :date_of_birth, - prompt: true, - start_year: 1900, end_year: minimum_required_age.years.ago.year %> + <%= render Shared::DateOfBirthFieldComponent.new(f) %>
<% end %> diff --git a/app/views/management/users/new.html.erb b/app/views/management/users/new.html.erb index e7431d0f3..a5f01c432 100644 --- a/app/views/management/users/new.html.erb +++ b/app/views/management/users/new.html.erb @@ -11,10 +11,7 @@ <%= f.text_field :email, label: t("management.users.email_optional_label") %>
- <%= f.date_select :date_of_birth, - prompt: true, - start_year: 1900, end_year: 16.years.ago.year, - label: t("management.date_of_birth") %> + <%= render Shared::DateOfBirthFieldComponent.new(f, label: t("management.date_of_birth")) %>
<%= f.submit t("management.users.create_user_submit"), class: "button success" %> <% end %> diff --git a/app/views/officing/residence/new.html.erb b/app/views/officing/residence/new.html.erb index 93c510b14..49d6ce437 100644 --- a/app/views/officing/residence/new.html.erb +++ b/app/views/officing/residence/new.html.erb @@ -14,9 +14,7 @@ <% if Setting.force_presence_date_of_birth? %>
- <%= f.date_select :date_of_birth, - prompt: true, - start_year: 1900, end_year: minimum_required_age.years.ago.year %> + <%= render Shared::DateOfBirthFieldComponent.new(f) %>
<% else %>
diff --git a/app/views/verification/residence/new.html.erb b/app/views/verification/residence/new.html.erb index c361d7d4c..9909d41af 100644 --- a/app/views/verification/residence/new.html.erb +++ b/app/views/verification/residence/new.html.erb @@ -57,9 +57,7 @@
- <%= f.date_select :date_of_birth, - prompt: true, - start_year: 1900, end_year: minimum_required_age.years.ago.year %> + <%= render Shared::DateOfBirthFieldComponent.new(f) %>
diff --git a/spec/components/shared/date_of_birth_field_component_spec.rb b/spec/components/shared/date_of_birth_field_component_spec.rb new file mode 100644 index 000000000..8687b84cf --- /dev/null +++ b/spec/components/shared/date_of_birth_field_component_spec.rb @@ -0,0 +1,26 @@ +require "rails_helper" + +describe Shared::DateOfBirthFieldComponent do + before do + dummy_model = Class.new do + include ActiveModel::Model + attr_accessor :date_of_birth + end + + stub_const("DummyModel", dummy_model) + end + + let(:form) { ConsulFormBuilder.new(:dummy, DummyModel.new, ApplicationController.new.view_context, {}) } + let(:component) { Shared::DateOfBirthFieldComponent.new(form) } + + it "uses the minimum required age as the latest date by default" do + Setting["min_age_to_participate"] = 13 + + 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] + end + end +end diff --git a/spec/system/verification/residence_spec.rb b/spec/system/verification/residence_spec.rb index a44d6e3ae..a806cc378 100644 --- a/spec/system/verification/residence_spec.rb +++ b/spec/system/verification/residence_spec.rb @@ -38,21 +38,6 @@ describe "Residence" do expect(page).to have_content "Residence verified" end - scenario "Residence form use min age to participate" do - min_age = (Setting["min_age_to_participate"] = 16).to_i - underage = min_age - 1 - user = create(:user) - login_as(user) - - visit account_path - click_link "Verify my account" - - expect(page).to have_select("residence_date_of_birth_1i", - with_options: [min_age.years.ago.year]) - expect(page).not_to have_select("residence_date_of_birth_1i", - with_options: [underage.years.ago.year]) - end - scenario "When trying to verify a deregistered account old votes are reassigned" do erased_user = create(:user, document_number: "12345678Z", document_type: "1", erased_at: Time.current) vote = create(:vote, voter: erased_user) 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 02/11] 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" From b68047f2652a434a2995cb3e082949921ad0313a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 12 Oct 2024 17:49:51 +0200 Subject: [PATCH 03/11] Fix missing "for" attribute in officer assignment label Since this attribute was missing, the label wasn't correctly associated with its field. --- app/views/officing/results/new.html.erb | 2 +- spec/system/budget_polls/ballot_sheets_spec.rb | 4 ++-- spec/system/officing/results_spec.rb | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/views/officing/results/new.html.erb b/app/views/officing/results/new.html.erb index 45aa443a3..aba3e17da 100644 --- a/app/views/officing/results/new.html.erb +++ b/app/views/officing/results/new.html.erb @@ -4,7 +4,7 @@ <%= form_tag(officing_poll_results_path(@poll), { id: "officer_assignment_form" }) do %>
- + <%= label_tag :officer_assignment_id, t("officing.results.new.booth") %> <%= select_tag :officer_assignment_id, booths_for_officer_select_options(@officer_assignments), { prompt: t("officing.results.new.select_booth") } %> diff --git a/spec/system/budget_polls/ballot_sheets_spec.rb b/spec/system/budget_polls/ballot_sheets_spec.rb index 91c6aab56..931ce8d1c 100644 --- a/spec/system/budget_polls/ballot_sheets_spec.rb +++ b/spec/system/budget_polls/ballot_sheets_spec.rb @@ -103,7 +103,7 @@ describe "Poll budget ballot sheets" do scenario "Ballot sheet is saved" do visit new_officing_poll_ballot_sheet_path(poll) - select booth.name, from: "officer_assignment_id" + select booth.name, from: "Booth" fill_in "data", with: "1234;5678" click_button "Save" @@ -119,7 +119,7 @@ describe "Poll budget ballot sheets" do scenario "Ballot sheet is not saved" do visit new_officing_poll_ballot_sheet_path(poll) - select booth.name, from: "officer_assignment_id" + select booth.name, from: "Booth" click_button "Save" expect(page).to have_content("CSV data can't be blank") diff --git a/spec/system/officing/results_spec.rb b/spec/system/officing/results_spec.rb index a3147093d..59abe9497 100644 --- a/spec/system/officing/results_spec.rb +++ b/spec/system/officing/results_spec.rb @@ -57,7 +57,7 @@ describe "Officing Results", :with_frozen_time do expect(page).not_to have_content("Your results") - select booth.name, from: "officer_assignment_id" + select booth.name, from: "Booth" fill_in "questions[#{question_1.id}][0]", with: "100" fill_in "questions[#{question_1.id}][1]", with: "200" @@ -100,7 +100,7 @@ describe "Officing Results", :with_frozen_time do visit new_officing_poll_result_path(poll) booth_name = partial_result.booth_assignment.booth.name - select booth_name, from: "officer_assignment_id" + select booth_name, from: "Booth" fill_in "questions[#{question_1.id}][0]", with: "5555" fill_in "questions[#{question_1.id}][1]", with: "200" From d254fcd7ca4571a3ee82d63685c0f3dd44607ba0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 12 Oct 2024 17:49:56 +0200 Subject: [PATCH 04/11] Fix missing "for" attribute in user segments label Since the attribute was missing, the label wasn't correctly associated with its field. --- app/views/admin/emails_download/index.html.erb | 2 +- spec/system/admin/emails/emails_download_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/admin/emails_download/index.html.erb b/app/views/admin/emails_download/index.html.erb index 3fd0f2f01..49b086ef5 100644 --- a/app/views/admin/emails_download/index.html.erb +++ b/app/views/admin/emails_download/index.html.erb @@ -5,7 +5,7 @@ method: :get, id: "admin_download_emails" do %> - + <%= label_tag :users_segment, t("admin.emails_download.index.download_segment") %>

<%= t("admin.emails_download.index.download_segment_help_text") %>

diff --git a/spec/system/admin/emails/emails_download_spec.rb b/spec/system/admin/emails/emails_download_spec.rb index 7de835565..9e784cc05 100644 --- a/spec/system/admin/emails/emails_download_spec.rb +++ b/spec/system/admin/emails/emails_download_spec.rb @@ -26,7 +26,7 @@ describe "Admin download user emails" do visit admin_emails_download_index_path within("#admin_download_emails") do - select "Administrators", from: "users_segment" + select "Administrators", from: "Download email addresses" click_button "Download emails list" end From 9ab6c1597599fcf94dfac342e2a154aad07e6801 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 12 Nov 2024 15:16:23 +0100 Subject: [PATCH 05/11] Remove unused method in shifts helper We don't use this method since commit 452723628. --- app/helpers/shifts_helper.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/helpers/shifts_helper.rb b/app/helpers/shifts_helper.rb index 34a64e2a2..9dbc3d4a5 100644 --- a/app/helpers/shifts_helper.rb +++ b/app/helpers/shifts_helper.rb @@ -32,10 +32,6 @@ module ShiftsHelper polls.maximum(:ends_at).to_date end - def officer_select_options(officers) - officers.map { |officer| [officer.name, officer.id] } - end - private def officer_shifts(task_id, booth) From ee34ead4eebf9b0a10a90d56c1a948a412faaddc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 12 Oct 2024 18:23:54 +0200 Subject: [PATCH 06/11] Move poll shifts form partial to a component Thanks to it, we can move a few helper methods to the component. --- .../poll/shifts/form.js} | 2 +- app/assets/javascripts/application.js | 3 +- .../poll/shifts/form_component.html.erb} | 16 +++--- .../admin/poll/shifts/form_component.rb | 56 +++++++++++++++++++ .../admin/poll/shifts_controller.rb | 2 - app/helpers/shifts_helper.rb | 40 ------------- app/views/admin/poll/shifts/new.html.erb | 2 +- 7 files changed, 67 insertions(+), 54 deletions(-) rename app/assets/javascripts/{polls_admin.js => admin/poll/shifts/form.js} (95%) rename app/{views/admin/poll/shifts/_form.html.erb => components/admin/poll/shifts/form_component.html.erb} (74%) create mode 100644 app/components/admin/poll/shifts/form_component.rb delete mode 100644 app/helpers/shifts_helper.rb diff --git a/app/assets/javascripts/polls_admin.js b/app/assets/javascripts/admin/poll/shifts/form.js similarity index 95% rename from app/assets/javascripts/polls_admin.js rename to app/assets/javascripts/admin/poll/shifts/form.js index 87e55756b..ee37951f9 100644 --- a/app/assets/javascripts/polls_admin.js +++ b/app/assets/javascripts/admin/poll/shifts/form.js @@ -1,6 +1,6 @@ (function() { "use strict"; - App.PollsAdmin = { + App.AdminPollShiftsForm = { initialize: function() { $("select[class='js-poll-shifts']").on({ change: function() { diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 9351e8ad2..cce7d7dd9 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -101,7 +101,6 @@ //= require imageable //= require tree_navigator //= require tag_autocomplete -//= require polls_admin //= require leaflet/dist/leaflet //= require leaflet.markercluster/dist/leaflet.markercluster //= require map @@ -157,7 +156,6 @@ var initialize_modules = function() { App.Documentable.initialize(); App.Imageable.initialize(); App.TagAutocomplete.initialize(); - App.PollsAdmin.initialize(); App.Map.initialize(); App.Polls.initialize(); App.Sortable.initialize(); @@ -171,6 +169,7 @@ var initialize_modules = function() { } App.AdminBudgetsWizardCreationStep.initialize(); App.AdminMachineLearningScripts.initialize(); + App.AdminPollShiftsForm.initialize(); App.AdminTenantsForm.initialize(); App.AdminVotationTypesFields.initialize(); App.AdminMenu.initialize(); diff --git a/app/views/admin/poll/shifts/_form.html.erb b/app/components/admin/poll/shifts/form_component.html.erb similarity index 74% rename from app/views/admin/poll/shifts/_form.html.erb rename to app/components/admin/poll/shifts/form_component.html.erb index 2c0fc6249..bdb00af68 100644 --- a/app/views/admin/poll/shifts/_form.html.erb +++ b/app/components/admin/poll/shifts/form_component.html.erb @@ -1,5 +1,5 @@ -<%= form_for @shift, as: :shift, url: admin_booth_shifts_path do |f| %> - <%= render "shared/errors", resource: @shift %> +<%= form_for shift, as: :shift, url: admin_booth_shifts_path do |f| %> + <%= render "shared/errors", resource: shift %>
@@ -8,8 +8,8 @@
<%= t("admin.poll_shifts.new.officer") %> -
<%= @officer.name %> - <%= f.hidden_field :officer_id, value: @officer.id %> +
<%= officer.name %> + <%= f.hidden_field :officer_id, value: officer.id %>
@@ -22,17 +22,17 @@
<%= select "shift[date]", "vote_collection_date", - options_for_select(shift_vote_collection_dates(@booth, @voting_polls)), - { prompt: @voting_polls.present? ? t("admin.poll_shifts.new.select_date") : t("admin.poll_shifts.new.no_voting_days") }, + options_for_select(shift_vote_collection_dates(booth, voting_polls)), + { prompt: voting_polls.present? ? t("admin.poll_shifts.new.select_date") : t("admin.poll_shifts.new.no_voting_days") }, class: "js-shift-vote-collection-dates" %> <%= select "shift[date]", "recount_scrutiny_date", - options_for_select(shift_recount_scrutiny_dates(@booth, @recount_polls)), + options_for_select(shift_recount_scrutiny_dates(booth, recount_polls)), { prompt: t("admin.poll_shifts.new.select_date") }, class: "js-shift-recount-scrutiny-dates", hidden: "hidden" %>
- <%= f.hidden_field :booth_id, value: @booth.id %> + <%= f.hidden_field :booth_id, value: booth.id %>
<%= f.submit t("admin.poll_shifts.new.add_shift"), diff --git a/app/components/admin/poll/shifts/form_component.rb b/app/components/admin/poll/shifts/form_component.rb new file mode 100644 index 000000000..a2ec1d967 --- /dev/null +++ b/app/components/admin/poll/shifts/form_component.rb @@ -0,0 +1,56 @@ +class Admin::Poll::Shifts::FormComponent < ApplicationComponent + attr_reader :shift, :booth, :officer + + def initialize(shift, booth:, officer:) + @shift = shift + @booth = booth + @officer = officer + end + + private + + def voting_polls + booth.polls.current + end + + def recount_polls + booth.polls.current_or_recounting + end + + def shift_vote_collection_dates(booth, polls) + return [] if polls.blank? + + date_options((start_date(polls)..end_date(polls)), Poll::Shift.tasks[:vote_collection], booth) + end + + def shift_recount_scrutiny_dates(booth, polls) + return [] if polls.blank? + + dates = polls.map(&:ends_at).map(&:to_date).sort.reduce([]) do |total, date| + initial_date = [date, Date.current].max + total << (initial_date..date + Poll::RECOUNT_DURATION).to_a + end + date_options(dates.flatten.uniq, Poll::Shift.tasks[:recount_scrutiny], booth) + end + + def date_options(dates, task_id, booth) + valid_dates(dates, task_id, booth).map { |date| [l(date, format: :long), l(date)] } + end + + def valid_dates(dates, task_id, booth) + dates.reject { |date| officer_shifts(task_id, booth).include?(date) } + end + + def start_date(polls) + start_date = polls.minimum(:starts_at).to_date + [start_date, Date.current].max + end + + def end_date(polls) + polls.maximum(:ends_at).to_date + end + + def officer_shifts(task_id, booth) + officer.shifts.where(task: task_id, booth: booth).map(&:date) + end +end diff --git a/app/controllers/admin/poll/shifts_controller.rb b/app/controllers/admin/poll/shifts_controller.rb index 2aa60ecbb..b8ab83852 100644 --- a/app/controllers/admin/poll/shifts_controller.rb +++ b/app/controllers/admin/poll/shifts_controller.rb @@ -5,8 +5,6 @@ class Admin::Poll::ShiftsController < Admin::Poll::BaseController def new load_shifts @shift = ::Poll::Shift.new - @voting_polls = @booth.polls.current - @recount_polls = @booth.polls.current_or_recounting end def create diff --git a/app/helpers/shifts_helper.rb b/app/helpers/shifts_helper.rb deleted file mode 100644 index 9dbc3d4a5..000000000 --- a/app/helpers/shifts_helper.rb +++ /dev/null @@ -1,40 +0,0 @@ -module ShiftsHelper - def shift_vote_collection_dates(booth, polls) - return [] if polls.blank? - - date_options((start_date(polls)..end_date(polls)), Poll::Shift.tasks[:vote_collection], booth) - end - - def shift_recount_scrutiny_dates(booth, polls) - return [] if polls.blank? - - dates = polls.map(&:ends_at).map(&:to_date).sort.reduce([]) do |total, date| - initial_date = [date, Date.current].max - total << (initial_date..date + Poll::RECOUNT_DURATION).to_a - end - date_options(dates.flatten.uniq, Poll::Shift.tasks[:recount_scrutiny], booth) - end - - def date_options(dates, task_id, booth) - valid_dates(dates, task_id, booth).map { |date| [l(date, format: :long), l(date)] } - end - - def valid_dates(dates, task_id, booth) - dates.reject { |date| officer_shifts(task_id, booth).include?(date) } - end - - def start_date(polls) - start_date = polls.minimum(:starts_at).to_date - [start_date, Date.current].max - end - - def end_date(polls) - polls.maximum(:ends_at).to_date - end - - private - - def officer_shifts(task_id, booth) - @officer.shifts.where(task: task_id, booth: booth).map(&:date) - end -end diff --git a/app/views/admin/poll/shifts/new.html.erb b/app/views/admin/poll/shifts/new.html.erb index 4e0665ea2..b6b6245d8 100644 --- a/app/views/admin/poll/shifts/new.html.erb +++ b/app/views/admin/poll/shifts/new.html.erb @@ -8,7 +8,7 @@

<%= render "search_officers" %> <% else %> - <%= render "form" %> + <%= render Admin::Poll::Shifts::FormComponent.new(@shift, booth: @booth, officer: @officer) %> <% end %>
From 5915194fc4b8bbe922d6bb9162b99e8fdb4c86dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 14 Oct 2024 21:24:58 +0200 Subject: [PATCH 07/11] Simplify passing the booth in shift form component Since we're using helpers instead of components, the booth is now a method and we don't need to pass it around. --- .../admin/poll/shifts/form_component.html.erb | 4 ++-- .../admin/poll/shifts/form_component.rb | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/components/admin/poll/shifts/form_component.html.erb b/app/components/admin/poll/shifts/form_component.html.erb index bdb00af68..0d72452ab 100644 --- a/app/components/admin/poll/shifts/form_component.html.erb +++ b/app/components/admin/poll/shifts/form_component.html.erb @@ -22,11 +22,11 @@
<%= select "shift[date]", "vote_collection_date", - options_for_select(shift_vote_collection_dates(booth, voting_polls)), + options_for_select(shift_vote_collection_dates(voting_polls)), { prompt: voting_polls.present? ? t("admin.poll_shifts.new.select_date") : t("admin.poll_shifts.new.no_voting_days") }, class: "js-shift-vote-collection-dates" %> <%= select "shift[date]", "recount_scrutiny_date", - options_for_select(shift_recount_scrutiny_dates(booth, recount_polls)), + options_for_select(shift_recount_scrutiny_dates(recount_polls)), { prompt: t("admin.poll_shifts.new.select_date") }, class: "js-shift-recount-scrutiny-dates", hidden: "hidden" %> diff --git a/app/components/admin/poll/shifts/form_component.rb b/app/components/admin/poll/shifts/form_component.rb index a2ec1d967..cd7a0b2f2 100644 --- a/app/components/admin/poll/shifts/form_component.rb +++ b/app/components/admin/poll/shifts/form_component.rb @@ -17,28 +17,28 @@ class Admin::Poll::Shifts::FormComponent < ApplicationComponent booth.polls.current_or_recounting end - def shift_vote_collection_dates(booth, polls) + def shift_vote_collection_dates(polls) return [] if polls.blank? - date_options((start_date(polls)..end_date(polls)), Poll::Shift.tasks[:vote_collection], booth) + date_options((start_date(polls)..end_date(polls)), Poll::Shift.tasks[:vote_collection]) end - def shift_recount_scrutiny_dates(booth, polls) + def shift_recount_scrutiny_dates(polls) return [] if polls.blank? dates = polls.map(&:ends_at).map(&:to_date).sort.reduce([]) do |total, date| initial_date = [date, Date.current].max total << (initial_date..date + Poll::RECOUNT_DURATION).to_a end - date_options(dates.flatten.uniq, Poll::Shift.tasks[:recount_scrutiny], booth) + date_options(dates.flatten.uniq, Poll::Shift.tasks[:recount_scrutiny]) end - def date_options(dates, task_id, booth) - valid_dates(dates, task_id, booth).map { |date| [l(date, format: :long), l(date)] } + def date_options(dates, task_id) + valid_dates(dates, task_id).map { |date| [l(date, format: :long), l(date)] } end - def valid_dates(dates, task_id, booth) - dates.reject { |date| officer_shifts(task_id, booth).include?(date) } + def valid_dates(dates, task_id) + dates.reject { |date| officer_shifts(task_id).include?(date) } end def start_date(polls) @@ -50,7 +50,7 @@ class Admin::Poll::Shifts::FormComponent < ApplicationComponent polls.maximum(:ends_at).to_date end - def officer_shifts(task_id, booth) + def officer_shifts(task_id) officer.shifts.where(task: task_id, booth: booth).map(&:date) end end From 79b7ec91dd02e29d8a01f7d0c9c5ca5d87f98172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 14 Oct 2024 21:32:08 +0200 Subject: [PATCH 08/11] Simplify passing polls in shifts form component --- .../admin/poll/shifts/form_component.html.erb | 4 ++-- .../admin/poll/shifts/form_component.rb | 20 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/components/admin/poll/shifts/form_component.html.erb b/app/components/admin/poll/shifts/form_component.html.erb index 0d72452ab..8c0ad752a 100644 --- a/app/components/admin/poll/shifts/form_component.html.erb +++ b/app/components/admin/poll/shifts/form_component.html.erb @@ -22,11 +22,11 @@
<%= select "shift[date]", "vote_collection_date", - options_for_select(shift_vote_collection_dates(voting_polls)), + options_for_select(shift_vote_collection_dates), { prompt: voting_polls.present? ? t("admin.poll_shifts.new.select_date") : t("admin.poll_shifts.new.no_voting_days") }, class: "js-shift-vote-collection-dates" %> <%= select "shift[date]", "recount_scrutiny_date", - options_for_select(shift_recount_scrutiny_dates(recount_polls)), + options_for_select(shift_recount_scrutiny_dates), { prompt: t("admin.poll_shifts.new.select_date") }, class: "js-shift-recount-scrutiny-dates", hidden: "hidden" %> diff --git a/app/components/admin/poll/shifts/form_component.rb b/app/components/admin/poll/shifts/form_component.rb index cd7a0b2f2..5a0c626ba 100644 --- a/app/components/admin/poll/shifts/form_component.rb +++ b/app/components/admin/poll/shifts/form_component.rb @@ -17,16 +17,16 @@ class Admin::Poll::Shifts::FormComponent < ApplicationComponent booth.polls.current_or_recounting end - def shift_vote_collection_dates(polls) - return [] if polls.blank? + def shift_vote_collection_dates + return [] if voting_polls.blank? - date_options((start_date(polls)..end_date(polls)), Poll::Shift.tasks[:vote_collection]) + date_options((voting_start_date..voting_end_date), Poll::Shift.tasks[:vote_collection]) end - def shift_recount_scrutiny_dates(polls) - return [] if polls.blank? + def shift_recount_scrutiny_dates + return [] if recount_polls.blank? - dates = polls.map(&:ends_at).map(&:to_date).sort.reduce([]) do |total, date| + dates = recount_polls.map(&:ends_at).map(&:to_date).sort.reduce([]) do |total, date| initial_date = [date, Date.current].max total << (initial_date..date + Poll::RECOUNT_DURATION).to_a end @@ -41,13 +41,13 @@ class Admin::Poll::Shifts::FormComponent < ApplicationComponent dates.reject { |date| officer_shifts(task_id).include?(date) } end - def start_date(polls) - start_date = polls.minimum(:starts_at).to_date + def voting_start_date + start_date = voting_polls.minimum(:starts_at).to_date [start_date, Date.current].max end - def end_date(polls) - polls.maximum(:ends_at).to_date + def voting_end_date + voting_polls.maximum(:ends_at).to_date end def officer_shifts(task_id) From ddaf320d8a0cc78a3a3c596551fbec75d3d9902c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 12 Oct 2024 19:33:41 +0200 Subject: [PATCH 09/11] Add proper labels to shift date selectors We were using one label for both date selectors, but it wasn't associated with any of them. So we're now rendering one label per control and, just like we only show one of these date selectors at a time, we're only showing one label at a time. --- .../javascripts/admin/poll/shifts/form.js | 8 +++--- .../admin/poll/shifts/form_component.html.erb | 9 ++++++- spec/system/admin/poll/shifts_spec.rb | 25 ++++++++----------- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/app/assets/javascripts/admin/poll/shifts/form.js b/app/assets/javascripts/admin/poll/shifts/form.js index ee37951f9..f6a695de3 100644 --- a/app/assets/javascripts/admin/poll/shifts/form.js +++ b/app/assets/javascripts/admin/poll/shifts/form.js @@ -6,12 +6,12 @@ change: function() { switch ($(this).val()) { case "vote_collection": - $("select[class='js-shift-vote-collection-dates']").show(); - $("select[class='js-shift-recount-scrutiny-dates']").hide(); + $(".js-shift-vote-collection-dates").show(); + $(".js-shift-recount-scrutiny-dates").hide(); break; case "recount_scrutiny": - $("select[class='js-shift-recount-scrutiny-dates']").show(); - $("select[class='js-shift-vote-collection-dates']").hide(); + $(".js-shift-recount-scrutiny-dates").show(); + $(".js-shift-vote-collection-dates").hide(); } } }); diff --git a/app/components/admin/poll/shifts/form_component.html.erb b/app/components/admin/poll/shifts/form_component.html.erb index 8c0ad752a..74ac8105e 100644 --- a/app/components/admin/poll/shifts/form_component.html.erb +++ b/app/components/admin/poll/shifts/form_component.html.erb @@ -20,11 +20,18 @@
- + <%= label_tag :shift_date_vote_collection_date, + t("admin.poll_shifts.new.date"), + class: "js-shift-vote-collection-dates" %> <%= select "shift[date]", "vote_collection_date", options_for_select(shift_vote_collection_dates), { prompt: voting_polls.present? ? t("admin.poll_shifts.new.select_date") : t("admin.poll_shifts.new.no_voting_days") }, class: "js-shift-vote-collection-dates" %> + + <%= label_tag :shift_date_recount_scrutiny_date, + t("admin.poll_shifts.new.date"), + class: "js-shift-recount-scrutiny-dates", + hidden: "hidden" %> <%= select "shift[date]", "recount_scrutiny_date", options_for_select(shift_recount_scrutiny_dates), { prompt: t("admin.poll_shifts.new.select_date") }, diff --git a/spec/system/admin/poll/shifts_spec.rb b/spec/system/admin/poll/shifts_spec.rb index ad55359f8..841002c3d 100644 --- a/spec/system/admin/poll/shifts_spec.rb +++ b/spec/system/admin/poll/shifts_spec.rb @@ -53,10 +53,10 @@ describe "Admin shifts", :admin do click_button "Search" click_link "Edit shifts" - expect(page).to have_select("shift_date_vote_collection_date", - options: ["Select day", *vote_collection_dates]) - expect(page).not_to have_select("shift_date_recount_scrutiny_date") - select I18n.l(Date.current, format: :long), from: "shift_date_vote_collection_date" + expect(page).to have_select "Date", options: ["Select day", *vote_collection_dates] + expect(page).not_to have_select with_options: recount_scrutiny_dates + + select I18n.l(Date.current, format: :long), from: "Date" click_button "Add shift" expect(page).to have_content "Shift added" @@ -82,10 +82,9 @@ describe "Admin shifts", :admin do select "Recount & Scrutiny", from: "shift_task" - expect(page).to have_select("shift_date_recount_scrutiny_date", - options: ["Select day", *recount_scrutiny_dates]) - expect(page).not_to have_select("shift_date_vote_collection_date") - select I18n.l(poll.ends_at.to_date + 4.days, format: :long), from: "shift_date_recount_scrutiny_date" + expect(page).to have_select "Date", options: ["Select day", *recount_scrutiny_dates] + expect(page).not_to have_select with_options: vote_collection_dates + select I18n.l(poll.ends_at.to_date + 4.days, format: :long), from: "Date" click_button "Add shift" expect(page).to have_content "Shift added" @@ -126,11 +125,9 @@ describe "Admin shifts", :admin do click_button "Search" click_link "Edit shifts" - expect(page).to have_select("shift_date_vote_collection_date", - options: ["Select day", *vote_collection_dates]) + expect(page).to have_select "Date", options: ["Select day", *vote_collection_dates] select "Recount & Scrutiny", from: "shift_task" - expect(page).to have_select("shift_date_recount_scrutiny_date", - options: ["Select day", *recount_scrutiny_dates]) + expect(page).to have_select "Date", options: ["Select day", *recount_scrutiny_dates] end scenario "Change option from Recount & Scrutinity to Collect Votes" do @@ -144,11 +141,11 @@ describe "Admin shifts", :admin do select "Recount & Scrutiny", from: "shift_task" - expect(page).to have_select("shift_date_recount_scrutiny_date", options: ["Select day"]) + expect(page).to have_select "Date", options: ["Select day"] select "Collect Votes", from: "shift_task" - expect(page).to have_select("shift_date_vote_collection_date", options: ["Voting days ended"]) + expect(page).to have_select "Date", options: ["Voting days ended"] end scenario "Error on create" do From 39d620539c1e27e6556ba4a3476b0f0d89a36cd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 12 Oct 2024 19:13:45 +0200 Subject: [PATCH 10/11] Remove unused tabindex attribute In order for this attribute to be applied, due to the syntax of the `select` method, it should be in a separate hash. We're removing it instead of correctly applying it because we never use the `tabindex` attribute with a positive value, since it might break keyboard navigation. --- app/views/officing/booth/new.html.erb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/views/officing/booth/new.html.erb b/app/views/officing/booth/new.html.erb index 0b4cda988..afb1b090a 100644 --- a/app/views/officing/booth/new.html.erb +++ b/app/views/officing/booth/new.html.erb @@ -13,8 +13,7 @@ <%= f.select :id, @booths.map { |booth| [booth.location, booth.id] }, selected: @booths.first, - label: false, - tabindex: "1" %> + label: false %> <%= f.submit(t("devise_views.sessions.new.submit"), class: "button expanded") %>
From 8130e4fbc81482e0c2f122021b95b523868111b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 12 Oct 2024 19:34:01 +0200 Subject: [PATCH 11/11] Add an aria-label to the "choose your booth" select We're using the `aria-label` attribute instead of a label because this is a page where only one field is rendered and the text of the label is the same as the text of the

tag. We're using `aria-label` instead of `aria-labelledby` because the former is supported by Capybara. --- app/views/officing/booth/new.html.erb | 4 ++-- spec/system/officing/booth_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/officing/booth/new.html.erb b/app/views/officing/booth/new.html.erb index afb1b090a..a520a234f 100644 --- a/app/views/officing/booth/new.html.erb +++ b/app/views/officing/booth/new.html.erb @@ -12,8 +12,8 @@
<%= f.select :id, @booths.map { |booth| [booth.location, booth.id] }, - selected: @booths.first, - label: false %> + { selected: @booths.first, label: false }, + { "aria-label": t("officing.booth.new.title") } %> <%= f.submit(t("devise_views.sessions.new.submit"), class: "button expanded") %>
diff --git a/spec/system/officing/booth_spec.rb b/spec/system/officing/booth_spec.rb index 2f86cbdd0..cf5b66aaa 100644 --- a/spec/system/officing/booth_spec.rb +++ b/spec/system/officing/booth_spec.rb @@ -47,7 +47,7 @@ describe "Booth", :with_frozen_time do expect(page).to have_content "Choose your booth" - select booth2.location, from: "booth_id" + select booth2.location, from: "Choose your booth" click_button "Enter" within("#officing-booth") do @@ -72,6 +72,6 @@ describe "Booth", :with_frozen_time do expect(page).to have_content "Choose your booth" - expect(page).to have_select("booth_id", options: [booth1.location, booth2.location]) + expect(page).to have_select "Choose your booth", options: [booth1.location, booth2.location] end end