search

activeadmin-plugins / active_admin_datetimepicker

:calendar: active_admin_datetimepicker gem
MIT License
72 stars 48 forks source link

Wrong value for lteq_datetime predicate #61

Closed aleksey-alt closed 4 years ago

aleksey-alt commented 4 years ago

DateTimeRangeInput inherits from ActiveAdmin's DateRangeInput. So it uses lteq_datetime predicate. But definition of this predicate is correct if we use date part of the datetime/timestamp value rather than whole value https://github.com/activeadmin/activeadmin/blob/v2.3.1/lib/ransack_ext.rb#L17

So now if end range field of the date_time_range filter is non-empty we always get incorrect request to DB.

For the whole value when lteq predicate is being converted to lt predicate then microseconds have to be increased by 1 (at least for Postgresql that stores timestamps with microsecond resolution) instead of days.

workgena commented 4 years ago

Oh, thank you for this finding. It is a shame we haven't noticed it by our-self's 🦀

And I'm afraid we have this bug for a long time. I'll create PR with new failing specs pointing this issue. We'll try to fix this ASP 👷 🛠

doridayan commented 4 years ago

Hi @workgena, I work with @aleksey-alt, who reported this issue. Thanks so much for the fast fix! Do you know when it's expected to be merged?

workgena commented 4 years ago

Current PR doesn't work correctly. It may take a few days for find working solution.

workgena commented 4 years ago

@aleksey-alt @doridayan fix version 0.7.3 has been published.

But during test we found another issue(which won't be fixed soon). When submitting the form, the value(for example "2019-10-16 10:00") will be directly passed to SQL query. It may lead to User-Experience misunderstanding in case the server and user are located in different time-zones. Often database timestamp is UTC. Thus, user must understand that time hi selected is an UTC, not his timezone.

doridayan commented 4 years ago

@workgena Thank you! We only use UTC time anyway.