Fix timezone issues with datetime type

[caiohsramos]

Fixes #105

Bug description at linked issue

[haffla]

Hello @cainwatson thank you so much for your contribution. I can see that your PR fixes the issue and I am very inclined to just hit "Merge" and be done with it. The thing is that I was hoping for a PR that fixes the issue from the ground up.

Rails by default saves timestamps in UTC. It also converts all datetime objects in where to UTC. This of course only unless config.active_record.time_zone_aware_attributes = false because by default all :datetime and :time attributes are time zone aware. So while your PR fixes the problem for most people it doesn't fix it for people who have disabled this feature.

To illustrate this. It is now around 17:01 here in Berlin (UTC +2). So UTC time is 15:01.

# time_zone_aware_attributes ENABLED (default)
Book.where('created_at < ', DateTime.current)
#=> SELECT "books".* FROM "books" WHERE (created_at < '2021-05-25 15:01:52.021088')

# time_zone_aware_attributes DISABLED
Book.where('created_at < ', DateTime.current)
#=> SELECT "books".* FROM "books" WHERE (created_at < '2021-05-25 17:01:52.021088')

Can you see that your fix will fail when this setting is disabled?

So I think the real underlying issue here is that this gem does not respect this Rails setting and simply saves datetime objects as is. We need to make this gem read this Rails setting and save the value accordingly or somehow tell Rails: "Hey this jsonb field needs to be converted, too". I am afraid though that this is not possible.

[caiohsramos]

Thanks for the response @haffla, and for explaining this behaviour. I will try to work later on this today to make sure the gem saves jsonb datetime fields in UTC. {"sold_at"=>"2021-05-24T20:32:04.890-03:00"} should be {"sold_at"=>"2021-05-24T23:32:04.890"}, right?

PS: I think you mentioned the wrong username in your comment, mine is @caiohsramos :smile:

[haffla]

Ah sorry yes @caiohsramos. Sorry about that.

You are absolutely right. It should be {"sold_at"=>"2021-05-24T23:32:04.890"} in your case.

[caiohsramos]

Hello @haffla. So, I updated my code.

Doing some research I concluded that:

• Rails saves time-like objects in the database based on the config config.active_record.default_timezone. By default is UTC, but may be in the local timezone
• time_zone_aware_attributes is probably not the problem here. It works just fine, converting (or not) whatever the dabase returned

Does it makes sense to you?

[haffla]

mmh no, Rails never saves something like 2021-05-27T12:29:11+02:00 to the database.

For example:

book.touch
# or
book.update(updated_at: DateTime.yesterday)

Check the generated SQL query and compare it to your local time and repeat with different settings for config.active_record.default_timezone. In any case what ends up in the database will be something like 2021-05-27 12:26:05.

I am afraid this is a very complicated issue to solve.

[caiohsramos]

Rails never saves something like 2021-05-27T12:29:11+02:00

Agreed, it would be:

• When active_record.default_timezone is UTC, saves something like 2021-05-27 10:29:11
• When active_record.default_timezone is local, saves something like 2021-05-27 12:29:11

To keep this behaviour I purposed using a formatter that does not consider TZ as strftime("%FT%R:%S.%LZ") to save time as string (for JSON compatibility)

[haffla]

But strftime("%FT%R:%S.%LZ") always leaves the Z at the end which stands for UTC.

It's now 15:07 in Berlin. When I do DateTime.current.strftime("%FT%R:%S.%LZ") I get "2021-05-27T15:07:51.044Z" which says 15:07 in UTC which is not true. I think what you want is .strftime("%F %R:%S").

This is just guessing. You need to write some extensive tests to make sure this is working. Anyway thanks for your work. Appreciated a lot.

[caiohsramos]

Saving with .strftime("%F %R:%S") would loose milliseconds information, right? 😕 I'll try to work on more tests to this feature, changing active_record.default_timezone and time_zone_aware_attributes. Then, we can focus on the implementation

[haffla]

Yes correct. .strftime("%F %R:%S:%L") preserves milliseconds. Though .to_s(:db) doesn't have milliseconds either.

P.S.: Don't use .to_s(:db). It converts to UTC.

[haffla]

@caiohsramos can you merge the current master into your branch and push? I am gonna attend to this PR. Sorry. Didn't have time.

[caiohsramos]

Hey @haffla. I did a commit and some tests to remember what was left to do here, the results were:

• Datetime fields are saved as strings with the format %FT%R:%S.%L (no timezone or UTC information)
• The implementation respects active_record.default_timezone, both on querying and on persisting
• The implementation does not interfere with the active_record.time_zone_aware_attributes configuration. It only matters when the attributes are instantiated, and it is working

Some tests changing the default_timezone config would be great

[haffla]

@caiohsramos sorry for being so picky, last change requests now before merging this. Thank you so much!

[caiohsramos]

@haffla thanks for the review :) Fixed the points you listed