-
Notifications
You must be signed in to change notification settings - Fork 10
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
Need to fix time handling for filter by date-range #73
Comments
@JGreenlee maybe you can tackle this |
The UX fixes for the datepicker itself were quick and I implemented those on #96 Swapping out Is there anything that is currently broken with the current implementation using |
@JGreenlee I think it is a bit of both. using arrow is consistent with e-mission-server, and, as indicated above, it also handles timezones better. If I select Jan 1st to Jan 10th, should I expect to see a trip back home after an NYE party? I would think so, but to ensure that it happens, I think we need to handle timezones correctly |
In the NYE scenario, suppose I'm a program administrator in California and a user in the UK has a trip on Jan 1 from 1am-2am. When that trip occurred, it was still Dec 31st in California time. So if there's a different user in California who took a trip at the instant, that trip won't show up. In other words, the timezone of the program administrator does not matter; all that matters is what date + time it was for the user when their trip occurred. Is that correct? |
The problem is that we're currently matching by timestamps (which are Unix time and always relative to 0 seconds GMT). To accomplish the above NYE scenario, we'd need to support a different kind of matching on |
Right, I created the Maybe we just have to use the timezone of the administrator, but make it more clear that is what it is doing (maybe by supporting a date/time popup but then representing the date as a timestamp once it is selected) |
Can we just reformat to YYYYMMDD while doing the gt / lt comparison ? |
Either way, as a first step I'm currently working on adding Then a second step can handle timezones once we decide how to proceed (maybe rework UI, maybe server changes) |
I think so (would have to think through and test to make sure that it works for all cases). BUT the database does not actually store data in YYYYMMDD - the fmt_time does store the data as a string, but does so as YYYYMMDDThh:mm:ss Having said that:
|
We weighed 3 approaches yesterday for how to handle these queries
The first two options would use a timestamp-based query method, while the third option would need a different query method using We determined that no particular approach is universally better and the expected behavior depends on use case. For example, if the admin user is inspecting weekday / weekend travel patterns, this only makes sense in the context of the local timezone where the trip occurred – people behave according to what day/time it is where they are regardless of what time it was where the program admin is based. From this, we will plan to support switching between these different query methods. For now, we will support GMT timezone and admin user's timezone. |
From e-mission#73 (comment) Implements a dropdown which displays below the datepicker and allows choosing between UTC vs local time as the basis for the date selection queries. So 'timezone' is passed now as an additional argument to the querying functions.
Since datetime does not handle timezones very well. We should use arrow like the rest of the e-mission codebase, and also consider date formatting etc.
The text was updated successfully, but these errors were encountered: