From 5a0fde40480fa59ad611d448c317d4aff3185159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 7 Sep 2022 20:10:23 +0200 Subject: [PATCH] Allow selecting the time when a poll starts/ends We were already saving it as a time, but we didn't offer an interface to select the time due to lack of decent browser support for this field back when this feature was added. However, nowadays all major browsers support this field type and, at the time of writing, at least 86.5% of the browsers support it [1]. This percentage could be much higher, since support in 11.25% of the browsers is unknown. Note we still need to support the case where this field isn't supported, and so we offer a fallback and on the server side we don't assume we're always getting a time. We're doing a strange hack so we set the field type to text before changing its value; otherwise old Firefox browsers crashed. Also note that, until now, we were storing end dates in the database as a date with 00:00 as its time, but we were considering the poll to be open until 23:59 that day. So, in order to keep backwards compatibility, we're adding a task to update the dates of existing polls so we get the same behavior we had until now. This also means budget polls are now created so they end at the beginning of the day when the balloting phase ends. This is consistent with the dates we display in the budget phases table. Finally, there's one test where we're using `beginning_of_minute` when creating a poll. That's because Chrome provides an interface to enter a time in a `%H:%M` format when the "seconds" value of the provided time is zero. However, when the "seconds" value isn't zero, Chrome provides an interface to enter a time in a `%H:%M:%S` format. Since Capybara doesn't enter the seconds when using `fill_in` with a time, the test failed when Capybara tried to enter a time in the `%H:%M` format when Chrome expected a time in the `%H:%M:%S` format. To solve this last point, an alternative would be to manually provide the format when using `fill_in` so it includes the seconds. [1] https://caniuse.com/mdn-html_elements_input_type_datetime-local --- app/assets/javascripts/datepicker.js | 27 +++++++-- app/models/poll.rb | 10 ++-- app/views/admin/poll/polls/_form.html.erb | 4 +- app/views/admin/poll/polls/_poll.html.erb | 4 +- .../admin/poll/polls/_poll_header.html.erb | 2 +- lib/tasks/consul.rake | 3 +- lib/tasks/polls.rake | 10 ++++ spec/lib/tasks/polls_spec.rb | 23 ++++++++ spec/models/poll/poll_spec.rb | 56 +++++++++---------- spec/system/admin/poll/polls_spec.rb | 22 ++++---- spec/system/budget_polls/budgets_spec.rb | 2 +- 11 files changed, 107 insertions(+), 56 deletions(-) create mode 100644 lib/tasks/polls.rake create mode 100644 spec/lib/tasks/polls_spec.rb diff --git a/app/assets/javascripts/datepicker.js b/app/assets/javascripts/datepicker.js index 1e483ea6a..eab188097 100644 --- a/app/assets/javascripts/datepicker.js +++ b/app/assets/javascripts/datepicker.js @@ -23,6 +23,16 @@ }); $(".js-calendar-full").datepicker(); + if (!App.Datepicker.browser_supports_datetime_local_field()) { + if (App.Datepicker.browser_supports_date_field()) { + $("input[type='datetime-local']").prop("type", "text") + .val(App.Datepicker.datetime_to_date) + .prop("type", "date"); + } else { + $("input[type='datetime-local']").val(App.Datepicker.datetime_to_date).datepicker(); + } + } + if (!App.Datepicker.browser_supports_date_field()) { $("input[type='date']").datepicker(); } @@ -39,11 +49,20 @@ }); }, browser_supports_date_field: function() { - var datefield; + return App.Datepicker.browser_supports_field_with_type("date"); + }, + browser_supports_datetime_local_field: function() { + return App.Datepicker.browser_supports_field_with_type("datetime-local"); + }, + browser_supports_field_with_type: function(field_type) { + var field; - datefield = document.createElement("input"); - datefield.setAttribute("type", "date"); - return datefield.type === "date"; + field = document.createElement("input"); + field.setAttribute("type", field_type); + return field.type === field_type; + }, + datetime_to_date: function(index, value) { + return value.split("T")[0]; } }; diff --git a/app/models/poll.rb b/app/models/poll.rb index dcc081168..2acd9cfb0 100644 --- a/app/models/poll.rb +++ b/app/models/poll.rb @@ -42,9 +42,9 @@ class Poll < ApplicationRecord accepts_nested_attributes_for :questions, reject_if: :all_blank, allow_destroy: true scope :for, ->(element) { where(related: element) } - scope :current, -> { where("starts_at <= :time and ends_at >= :time", time: Date.current.beginning_of_day) } - scope :expired, -> { where("ends_at < ?", Date.current.beginning_of_day) } - scope :recounting, -> { where(ends_at: (Date.current.beginning_of_day - RECOUNT_DURATION)...Date.current.beginning_of_day) } + scope :current, -> { where("starts_at <= :time and ends_at >= :time", time: Time.current) } + scope :expired, -> { where("ends_at < ?", Time.current) } + scope :recounting, -> { where(ends_at: (RECOUNT_DURATION.ago)...Time.current) } scope :published, -> { where(published: true) } scope :by_geozone_id, ->(geozone_id) { where(geozones: { id: geozone_id }.joins(:geozones)) } scope :public_for_api, -> { all } @@ -67,11 +67,11 @@ class Poll < ApplicationRecord name end - def current?(timestamp = Date.current.beginning_of_day) + def current?(timestamp = Time.current) starts_at <= timestamp && timestamp <= ends_at end - def expired?(timestamp = Date.current.beginning_of_day) + def expired?(timestamp = Time.current) ends_at < timestamp end diff --git a/app/views/admin/poll/polls/_form.html.erb b/app/views/admin/poll/polls/_form.html.erb index 926a98ce4..dd0b59292 100644 --- a/app/views/admin/poll/polls/_form.html.erb +++ b/app/views/admin/poll/polls/_form.html.erb @@ -6,11 +6,11 @@
- <%= f.date_field :starts_at %> + <%= f.datetime_field :starts_at %>
- <%= f.date_field :ends_at %> + <%= f.datetime_field :ends_at %>
diff --git a/app/views/admin/poll/polls/_poll.html.erb b/app/views/admin/poll/polls/_poll.html.erb index 8f477c586..80b64d43c 100644 --- a/app/views/admin/poll/polls/_poll.html.erb +++ b/app/views/admin/poll/polls/_poll.html.erb @@ -1,7 +1,7 @@ <%= poll.name %> - <%= l poll.starts_at.beginning_of_day, format: :short_datetime %> - <%= l poll.ends_at.end_of_day, format: :short_datetime %> + <%= l poll.starts_at, format: :short_datetime %> + <%= l poll.ends_at, format: :short_datetime %> <% if feature?(:sdg) %> <%= poll.sdg_goal_list %> <%= poll.sdg_target_list %> diff --git a/app/views/admin/poll/polls/_poll_header.html.erb b/app/views/admin/poll/polls/_poll_header.html.erb index ef24e0e0c..c6dcd0e7c 100644 --- a/app/views/admin/poll/polls/_poll_header.html.erb +++ b/app/views/admin/poll/polls/_poll_header.html.erb @@ -12,7 +12,7 @@
<%= t("admin.polls.index.dates") %>
- <%= render Admin::DateRangeComponent.new(@poll.starts_at.beginning_of_day, @poll.ends_at.end_of_day) %> + <%= render Admin::DurationComponent.new(@poll) %>
<% if @poll.geozone_restricted %> diff --git a/lib/tasks/consul.rake b/lib/tasks/consul.rake index f1074d23e..97bfb0365 100644 --- a/lib/tasks/consul.rake +++ b/lib/tasks/consul.rake @@ -11,6 +11,7 @@ namespace :consul do desc "Runs tasks needed to upgrade from 1.5.0 to 1.6.0" task "execute_release_1.6.0_tasks": [ - "db:calculate_tsv" + "db:calculate_tsv", + "polls:set_ends_at_to_end_of_day" ] end diff --git a/lib/tasks/polls.rake b/lib/tasks/polls.rake new file mode 100644 index 000000000..d909650f1 --- /dev/null +++ b/lib/tasks/polls.rake @@ -0,0 +1,10 @@ +namespace :polls do + desc "Changes the polls ending date to the end of the day" + task set_ends_at_to_end_of_day: :environment do + ApplicationLogger.new.info "Adding time to the date where a poll ends" + + Poll.find_each do |poll| + poll.update_column :ends_at, poll.ends_at.end_of_day.beginning_of_minute + end + end +end diff --git a/spec/lib/tasks/polls_spec.rb b/spec/lib/tasks/polls_spec.rb new file mode 100644 index 000000000..e30e91a71 --- /dev/null +++ b/spec/lib/tasks/polls_spec.rb @@ -0,0 +1,23 @@ +require "rails_helper" + +describe "rake polls:set_ends_at_to_end_of_day" do + before { Rake::Task["polls:set_ends_at_to_end_of_day"].reenable } + + let :run_rake_task do + Rake.application.invoke_task("polls:set_ends_at_to_end_of_day") + end + + it "updates existing polls" do + travel_to(Time.zone.local(2015, 7, 15, 13, 32, 13)) + poll = create(:poll, ends_at: 2.years.from_now) + date_poll = create(:poll, ends_at: 3.years.from_now.to_date) + + expect(I18n.l(poll.reload.ends_at, format: :datetime)).to eq "2017-07-15 13:32:13" + expect(I18n.l(date_poll.reload.ends_at, format: :datetime)).to eq "2018-07-15 00:00:00" + + run_rake_task + + expect(I18n.l(poll.reload.ends_at, format: :datetime)).to eq "2017-07-15 23:59:00" + expect(I18n.l(date_poll.reload.ends_at, format: :datetime)).to eq "2018-07-15 23:59:00" + end +end diff --git a/spec/models/poll/poll_spec.rb b/spec/models/poll/poll_spec.rb index a2b94d729..751f4a85a 100644 --- a/spec/models/poll/poll_spec.rb +++ b/spec/models/poll/poll_spec.rb @@ -86,12 +86,12 @@ describe Poll do end end - describe "#current?" do + describe "#current?", :with_frozen_time do it "returns true only when it isn't too late" do - about_to_start = create(:poll, starts_at: Date.tomorrow) - just_started = create(:poll, starts_at: Date.current) - about_to_end = create(:poll, ends_at: Date.current) - just_ended = create(:poll, ends_at: Date.yesterday) + about_to_start = create(:poll, starts_at: 1.second.from_now) + just_started = create(:poll, starts_at: Time.current) + about_to_end = create(:poll, ends_at: Time.current) + just_ended = create(:poll, ends_at: 1.second.ago) expect(just_started).to be_current expect(about_to_end).to be_current @@ -100,11 +100,11 @@ describe Poll do end end - describe "#expired?" do + describe "#expired?", :with_frozen_time do it "returns true only when it is too late" do - about_to_start = create(:poll, starts_at: Date.tomorrow) - about_to_end = create(:poll, ends_at: Date.current) - just_ended = create(:poll, ends_at: Date.yesterday) + about_to_start = create(:poll, starts_at: 1.second.from_now) + about_to_end = create(:poll, ends_at: Time.current) + just_ended = create(:poll, ends_at: 1.second.ago) recounting_ended = create(:poll, starts_at: 3.years.ago, ends_at: 2.years.ago) expect(just_ended).to be_expired @@ -325,12 +325,12 @@ describe Poll do end describe "scopes" do - describe ".current" do + describe ".current", :with_frozen_time do it "returns polls which have started but not ended" do - about_to_start = create(:poll, starts_at: Date.tomorrow) - just_started = create(:poll, starts_at: Date.current) - about_to_end = create(:poll, ends_at: Date.current) - just_ended = create(:poll, ends_at: Date.yesterday) + about_to_start = create(:poll, starts_at: 1.second.from_now) + just_started = create(:poll, starts_at: Time.current) + about_to_end = create(:poll, ends_at: Time.current) + just_ended = create(:poll, ends_at: 1.second.ago) current_polls = Poll.current @@ -340,11 +340,11 @@ describe Poll do end end - describe ".expired" do + describe ".expired", :with_frozen_time do it "returns polls which have already ended" do - about_to_start = create(:poll, starts_at: Date.tomorrow) - about_to_end = create(:poll, ends_at: Date.current) - just_ended = create(:poll, ends_at: Date.yesterday) + about_to_start = create(:poll, starts_at: 1.second.from_now) + about_to_end = create(:poll, ends_at: Time.current) + just_ended = create(:poll, ends_at: 1.second.ago) recounting_ended = create(:poll, starts_at: 3.years.ago, ends_at: 2.years.ago) expired_polls = Poll.expired @@ -355,11 +355,11 @@ describe Poll do end end - describe ".recounting" do + describe ".recounting", :with_frozen_time do it "returns polls in recount & scrutiny phase" do - about_to_start = create(:poll, starts_at: Date.tomorrow) - about_to_end = create(:poll, ends_at: Date.current) - just_ended = create(:poll, ends_at: Date.yesterday) + about_to_start = create(:poll, starts_at: 1.second.from_now) + about_to_end = create(:poll, ends_at: Time.current) + just_ended = create(:poll, ends_at: 1.second.ago) recounting_ended = create(:poll, starts_at: 3.years.ago, ends_at: 2.years.ago) recounting_polls = Poll.recounting @@ -371,12 +371,12 @@ describe Poll do end end - describe ".current_or_recounting" do + describe ".current_or_recounting", :with_frozen_time do it "returns current or recounting polls" do - about_to_start = create(:poll, starts_at: Date.tomorrow) - just_started = create(:poll, starts_at: Date.current) - about_to_end = create(:poll, ends_at: Date.current) - just_ended = create(:poll, ends_at: Date.yesterday) + about_to_start = create(:poll, starts_at: 1.second.from_now) + just_started = create(:poll, starts_at: Time.current) + about_to_end = create(:poll, ends_at: Time.current) + just_ended = create(:poll, ends_at: 1.second.ago) recounting_ended = create(:poll, starts_at: 3.years.ago, ends_at: 2.years.ago) current_or_recounting = Poll.current_or_recounting @@ -474,7 +474,7 @@ describe Poll do end it "is false for recounting polls" do - poll = create(:poll, ends_at: Date.yesterday) + poll = create(:poll, ends_at: 1.second.ago) expect(poll.recounts_confirmed?).to be false end diff --git a/spec/system/admin/poll/polls_spec.rb b/spec/system/admin/poll/polls_spec.rb index 7bd857ecc..23f023e1f 100644 --- a/spec/system/admin/poll/polls_spec.rb +++ b/spec/system/admin/poll/polls_spec.rb @@ -47,15 +47,14 @@ describe "Admin polls", :admin do end scenario "Create" do + travel_to(Time.zone.local(2015, 7, 15, 13, 32, 13)) + visit admin_polls_path click_link "Create poll" - start_date = 1.week.from_now.to_date - end_date = 2.weeks.from_now.to_date - fill_in "Name", with: "Upcoming poll" - fill_in "poll_starts_at", with: start_date - fill_in "poll_ends_at", with: end_date + fill_in "Start Date", with: 1.week.from_now + fill_in "Closing Date", with: 2.weeks.from_now fill_in "Summary", with: "Upcoming poll's summary. This poll..." fill_in "Description", with: "Upcomming poll's description. This poll..." @@ -67,8 +66,8 @@ describe "Admin polls", :admin do expect(page).to have_content "Poll created successfully" expect(page).to have_content "Upcoming poll" - expect(page).to have_content "#{I18n.l(start_date)} 00:00" - expect(page).to have_content "#{I18n.l(end_date)} 23:59" + expect(page).to have_content "2015-07-22 13:32" + expect(page).to have_content "2015-07-29 13:32" visit poll_path(id: "upcoming-poll") @@ -76,24 +75,23 @@ describe "Admin polls", :admin do end scenario "Edit" do - poll = create(:poll, :with_image) + travel_to(Time.zone.local(2015, 7, 15, 13, 32, 13)) + poll = create(:poll, :with_image, ends_at: 1.month.from_now.beginning_of_minute) visit admin_poll_path(poll) click_link "Edit poll" - end_date = 1.year.from_now.to_date - expect(page).to have_css("img[alt='#{poll.image.title}']") expect(page).to have_link "Go back", href: admin_polls_path fill_in "Name", with: "Next Poll" - fill_in "poll_ends_at", with: end_date + fill_in "Closing Date", with: 1.year.from_now click_button "Update poll" expect(page).to have_content "Poll updated successfully" expect(page).to have_content "Next Poll" - expect(page).to have_content "#{I18n.l(end_date)} 23:59" + expect(page).to have_content "2016-07-15 13:32" end scenario "Edit from index" do diff --git a/spec/system/budget_polls/budgets_spec.rb b/spec/system/budget_polls/budgets_spec.rb index 6a4f1c56b..bbd8ea26a 100644 --- a/spec/system/budget_polls/budgets_spec.rb +++ b/spec/system/budget_polls/budgets_spec.rb @@ -13,7 +13,7 @@ describe "Admin Budgets", :admin do expect(page).to have_current_path(/admin\/polls\/\d+/) expect(page).to have_content(budget.name) expect(page).to have_content("#{balloting_phase.starts_at.to_date} 00:00") - expect(page).to have_content("#{balloting_phase.ends_at.to_date} 23:59") + expect(page).to have_content("#{balloting_phase.ends_at.to_date - 1.day} 23:59") end scenario "Create poll in current locale if the budget does not have a poll associated" do