Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Date.current and Time.current #3618

Merged
merged 5 commits into from
Sep 4, 2019
Merged

Use Date.current and Time.current #3618

merged 5 commits into from
Sep 4, 2019

Conversation

javierm
Copy link
Member

@javierm javierm commented Jun 17, 2019

Background

Using Date.today and Time.now might lead to inconsistencies if the time zone the application uses is not the same as the system time zone.

Objectives

Fix all possible date errors we might have when the computer the application is running uses a different time zone than the one we define in config.time_zone.

@javierm javierm self-assigned this Jun 17, 2019
spec/models/direct_message_spec.rb Outdated Show resolved Hide resolved
spec/models/direct_message_spec.rb Outdated Show resolved Hide resolved
spec/models/direct_message_spec.rb Outdated Show resolved Hide resolved
@javierm javierm changed the title [WIP] Use Date.current and Time.current Use Date.current and Time.current Jun 18, 2019
@javierm javierm force-pushed the fix_date_today branch 6 times, most recently from e5e8aee to 6b16994 Compare June 21, 2019 01:56
@javierm javierm force-pushed the fix_date_today branch 2 times, most recently from addd37b to 3dcf25b Compare June 30, 2019 22:57
@javierm
Copy link
Member Author

javierm commented Jul 24, 2019

Travis failure is not related to this pull request, but to issue #3642.

@javierm javierm force-pushed the fix_date_today branch 3 times, most recently from 38d5f81 to b970c4c Compare August 9, 2019 12:01
Using Date.today and Time.now might lead to inconsistencies if the time
zone the application uses is not the same as the system time zone.
So now, application zone is the one defined in `config.time_zone`, and
system zone is the one in the computer where the application is running.
The dates are saved on UTC times on the database. So, for example,
if living in West Australia, `Date.current.beginning_of_day` will be
stored as UTC's yesterday at 15:15:00, while `Date.current.end_of_day`
will be stored as UTC's today at 15:14:59.

When we use the `DATE` database function, PostgreSQL will select the
records with the same UTC date as the current UTC date. However, we need
the records with the same application date (as defined in
`config.time_zone`) as the current application date. The test passed
(for us) because we were using `beginning_of_day + 3.hours` to make sure
we were creating records when the date in Madrid was the same as the UTC
date.

Using a ruby interval for the time condition solves the problem.
Using Date.today and Time.now is a common mistake which might lead to
obscure bugs. Now we're making sure we'll receive a warning when a pull
request uses these methods.
While the correct usage would require an `in_time_zone` call, the code
can be simplified and the relationship between the current date and the
last `proposal.created_at + index.days` is now a bit easier to notice.

We use the beginning of the month as the date of the first action
because we expect all 8 actions to be created on the same month.
@javierm
Copy link
Member Author

javierm commented Sep 4, 2019

Travis failures are not related to this pull request.

@javierm javierm merged commit 9b7ce9e into master Sep 4, 2019
@javierm javierm deleted the fix_date_today branch September 4, 2019 13:20
@javierm javierm added this to Release 1.1.0 in Roadmap Sep 10, 2019
@javierm javierm added the Bug label Sep 11, 2019
smarques pushed a commit to venetochevogliamo/consul that referenced this pull request Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Roadmap
  
Release 1.1.0
Development

Successfully merging this pull request may close these issues.

None yet

2 participants