e-mission / op-admin-dashboard

An admin/deployer dashboard for the NREL OpenPATH platform
0 stars 8 forks source link

Need to fix time handling for filter by date-range #73

Open achasmita opened 10 months ago

achasmita commented 10 months ago

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.

shankari commented 7 months ago

@jgreenlee maybe you can tackle this

JGreenlee commented 7 months ago

The UX fixes for the datepicker itself were quick and I implemented those on https://github.com/e-mission/op-admin-dashboard/pull/96

Swapping out datetime for arrow seems slightly more involved (I can see it was attempted before) and I'd like a bit more information.

Is there anything that is currently broken with the current implementation using datetime? Or is it just because we want to be consistent with e-mission-server?

shankari commented 7 months ago

@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

JGreenlee commented 7 months ago

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. I select the date range of Jan 1 - Jan 10 and that user's trip shows up.

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?

JGreenlee commented 7 months ago

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 e-mission-server (ie matching by calendar date in local time).

shankari commented 7 months ago

Right, I created the local_date fields to help with this. EDITED: I am not sure if that will work the best because the lt and gt can be problematic when using the split values So if we want to get everything from Jan 5th to Jan 20th, it is not as challenging But if we want to get everything from Jan 20th to Feb 3rd, the setting up the <> checks will be challenging.

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)

JGreenlee commented 7 months ago

Can we just reformat to YYYYMMDD while doing the gt / lt comparison ?

JGreenlee commented 7 months ago

Either way, as a first step I'm currently working on adding arrow as a drop-in replacement for datetime.

Then a second step can handle timezones once we decide how to proceed (maybe rework UI, maybe server changes)

shankari commented 7 months ago

Can we just reformat to YYYYMMDD while doing the gt / lt comparison ?

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 We want to be able to retrieve only the matching subset of data (or matching subset + delta) from the database - that is the whole point of the scalability improvement.

Having said that:

JGreenlee commented 7 months ago

We weighed 3 approaches yesterday for how to handle these queries

  1. use GMT timezone
  2. use local timezone of admin dashboard user
  3. use local timezone of trip

The first two options would use a timestamp-based query method, while the third option would need a different query method using fmt_time.

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. However, if the use case is to pull a large amount of data, as a "dump" (likely for further analysis) - it should guarantee a continuous record of activity sorted by timestamp.


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. Later, we will implement a way to query by fmt_time.