Fixes #13557 - Rubocop enforce specifying a timezone #3156
Conversation
Basically making sure the content of theforeman/foreman#3156 remains a guideline and new developers or plugin developers know how to store time.
| @@ -27,7 +27,7 @@ def trend_title(trend) | |||
| end | |||
| end | |||
|
|
|||
| def chart_data(trend, from = Setting.max_trend, to = Time.zone.now) | |||
| def chart_data(trend, from = Setting.max_trend, to = Time.now.utc) | |||
There was a problem hiding this comment.
Did you have to make this change? I ask because Time.zone should have a timezone and according to the Rails documentation, it may not be UTC.
There was a problem hiding this comment.
Actually, I think this is fine since it's a default value.
There was a problem hiding this comment.
It's not actually used in the trend counter retrieval, but if it was, then UTC would be correct.
dLobatog
force-pushed
the
timezone
branch
2 times, most recently
from
e51e69d
to
191e062
Compare
|
This seems to ignore user's timezone (which defaults to utc if not set). I'd think we may want to use |
|
@shlomizadok Where exactly do you mean? The goal of the PR is precisely to save everything under UTC for consistency, then only change the time zone in views using the |
|
👍 got it. |
| @@ -39,7 +39,7 @@ def flavor_with_object | |||
| end | |||
|
|
|||
| def created_at | |||
| Time.parse attributes['created'] | |||
| Time.parse(attributes['created']).utc | |||
There was a problem hiding this comment.
This might also need a change to use the user's timezone in the VM show view.
There was a problem hiding this comment.
set_timezone changes the time zone that's then used in prop, which is what shows the time in OpenStack.
https://github.com/theforeman/foreman/blob/develop/app/helpers/compute_resources_vms_helper.rb#L37
Rubocop can enforce what timezone to store in the database , so we can ensure everything is stored using UTC and we don't miss these things in code reviews. When objects are displayed, they must use the time provided by set_timezone in the controller. This is particularly relevant for Trends, Puppet graphs, etc... to ensure they are stored always properly
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
Rubocop can enforce what timezone to store in the database ,
so we can ensure everything is stored using UTC and we don't
miss these things in code reviews. When objects are displayed,
they must use the time provided by set_timezone in the
controller.
This is particularly relevant for Trends, Puppet graphs, etc... to
ensure they are stored always properly
theforeman/theforeman.org#535