Cht-core: In date filter for Reports tab, the selected dates are being offset by 1 day

Created on 12 Apr 2016  路  11Comments  路  Source: medic/cht-core

The start date is decreased and end date is increased by by 1 day.

Issue was originally filed in medic_projects. More details and discussions there:
https://github.com/medic/medic-projects/issues/78

Bug

Most helpful comment

Pretty sure we always want to store dates in GMT.

All 11 comments

I can recreate the bug, but only with reports dated one day MORE RECENT than the expected boundary:

screen shot 2016-05-31 at 18 40 15

Reports which are one day LESS RECENT do not appear in the filtered list.

At some point in the filtering we add a day to the boundary. I think we might do this for timezone reasons.

@garethbowen so do we actually want to "fix" this? In many cases it would be safe to assume that reports are being submitted from the same timezone that webapp is running in, but I can think of scenarios where this might not be the case.

@alxndrsn Yeah I think we should fix this. I think the correct approach is the reported_date should be UTC not the local time and then the query converted into UTC before being executed. We keep getting stung by timezone bugs in different forms so it'd be worth exploration to make sure we're consistent. It's just so hard to test when it works in your tz...

From https://github.com/medic/medic-projects/issues/78:

Reports are stored and filtered consistently irregardless of the timezone of the webapp user. It's not immediately obvious what timezone this filtering is done in, but I suspect GMT. A clear decision should be taken whether server, project or user timezones should be used for each of the following:

  • storing "date reported"
  • displaying report date to a webapp user
  • filtering reports

Pretty sure we always want to store dates in GMT.

N.B. need to consider migrations if changes made in this ticket also require changes to data in existing projects

From the other channel, it looks like we want to store local reported time, and one of either GMT reported time, or the timezone that the initial report was made in.

Storing local+GMT would probably allow for more flexibility in the future than storing local+localTimezone.

Checking dates are always stored as UTC using one of: moment.valueOf(), moment.toISOString(), Date.now(), Date.toISOString()

  • [x] reported_date
  • [x] scheduled task due date
  • [x] enketo dates

As far as I can see all dates are stored in UTC, either as a number of milliseconds between 1 January 1970 00:00:00 UTC or ISO 8601.

Ok. Found the bug in 2.x. Fixed it. Wrote an e2e test to prove it.

The root problem was we were adding a day to ensure the end date was inclusive, but our datetime UI widget was already setting the time to 23:59:59.999 to achieve the same result. Removing our addition fixed the issue.

This is timezone sensitive so if you ask for reports that were submitted on the 24th of January you will get those that were submitted between 00:00 and 23:59 in your timezone. This is correct because it's consistent with what will be shown as the submitted date in the reports list in the UI.

In 2.x we query a view with a full datetime so we can do clever timezone stuff. In 0.4 we query lucene with the date only so we can't easily fix this. I propose we upgrade the affected instances to 2.x once this fix is released.

Tested this a while ago - moving to ready.

Was this page helpful?
0 / 5 - 0 ratings