-
Notifications
You must be signed in to change notification settings - Fork 47
Wrong value for lteq_datetime predicate #61
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
Comments
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. |
Hi @workgena, I work with @aleksey-alt, who reported this issue. Thanks so much for the fast fix! |
Current PR doesn't work correctly. It may take a few days for find working solution. |
@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. |
@workgena Thank you! We only use UTC time anyway. |
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.
The text was updated successfully, but these errors were encountered: