Skip to content

Commit

Permalink
Allow selecting the time when a poll starts/ends
Browse files Browse the repository at this point in the history
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
  • Loading branch information
javierm committed Sep 14, 2022
1 parent f5a524d commit 5a0fde4
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 56 deletions.
27 changes: 23 additions & 4 deletions app/assets/javascripts/datepicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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];
}
};

Expand Down
10 changes: 5 additions & 5 deletions app/models/poll.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -67,11 +67,11 @@ def title
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

Expand Down
4 changes: 2 additions & 2 deletions app/views/admin/poll/polls/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
<div class="row">
<div class="clear">
<div class="small-12 medium-6 column">
<%= f.date_field :starts_at %>
<%= f.datetime_field :starts_at %>
</div>

<div class="small-12 medium-6 column">
<%= f.date_field :ends_at %>
<%= f.datetime_field :ends_at %>
</div>
</div>
</div>
Expand Down
4 changes: 2 additions & 2 deletions app/views/admin/poll/polls/_poll.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<tr id="<%= dom_id(poll) %>" class="poll">
<td><strong><%= poll.name %></strong></td>
<td class="text-center"><%= l poll.starts_at.beginning_of_day, format: :short_datetime %></td>
<td class="text-center"><%= l poll.ends_at.end_of_day, format: :short_datetime %></td>
<td class="text-center"><%= l poll.starts_at, format: :short_datetime %></td>
<td class="text-center"><%= l poll.ends_at, format: :short_datetime %></td>
<% if feature?(:sdg) %>
<td class="text-center"><%= poll.sdg_goal_list %></td>
<td class="text-center"><%= poll.sdg_target_list %></td>
Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/poll/polls/_poll_header.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<div class="inline-block">
<strong><%= t("admin.polls.index.dates") %></strong>
<br>
<%= render Admin::DateRangeComponent.new(@poll.starts_at.beginning_of_day, @poll.ends_at.end_of_day) %>
<%= render Admin::DurationComponent.new(@poll) %>
</div>

<% if @poll.geozone_restricted %>
Expand Down
3 changes: 2 additions & 1 deletion lib/tasks/consul.rake
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 10 additions & 0 deletions lib/tasks/polls.rake
Original file line number Diff line number Diff line change
@@ -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
23 changes: 23 additions & 0 deletions spec/lib/tasks/polls_spec.rb
Original file line number Diff line number Diff line change
@@ -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
56 changes: 28 additions & 28 deletions spec/models/poll/poll_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,12 @@
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
Expand All @@ -100,11 +100,11 @@
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
Expand Down Expand Up @@ -325,12 +325,12 @@
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

Expand All @@ -340,11 +340,11 @@
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
Expand All @@ -355,11 +355,11 @@
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
Expand All @@ -371,12 +371,12 @@
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
Expand Down Expand Up @@ -474,7 +474,7 @@
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
Expand Down
22 changes: 10 additions & 12 deletions spec/system/admin/poll/polls_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,14 @@
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..."

Expand All @@ -67,33 +66,32 @@

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")

expect(page).to have_content "Upcoming poll"
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
Expand Down
2 changes: 1 addition & 1 deletion spec/system/budget_polls/budgets_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
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
Expand Down

0 comments on commit 5a0fde4

Please sign in to comment.