daytoday reports are always in the profile's time zone #7836
Conversation
Midnight for the profile should always be at the start of the day (midnight) for the chart.
This more closely mirrors logic in loopalyzer, as well as the intent from the surrounding code in reportclient.js.
|
Been a little busy with Loop 3 docs. I appreciate this and I will try it soon. |
|
Thanks for the updates! I expected there to be a kind of "off by one" error, so the right column second row actually makes sense to me. This is progress. As for the date being wrong, can you be a bit more explicit. There seems to be a date rollover effect with respect to midnight. Did you pull up these reports at a time when the -2, +3 would have resulting in rolling over midnight? Is the date correct in the other timezone for one of those cases? We're close. If possible, I'd like to peek at your NS instance to poke at it, we can discuss that in a private channel. |
To illustrate the difference, I used chrome "sensors" feature to change my timezone to one that spans the dateline.
> moment.tz('2023-01-19', 'America/Los_Angeles').endOf('day').format( )
'2023-01-19T23:59:59-08:00'
> moment.tz(moment('2023-01-19'), 'America/Los_Angeles').endOf('day').format( )
'2023-01-18T23:59:59-08:00'
The old code uses a string replacement, which is equivalent to the first
test. This causes the dates on the reports to be off by one, as well as
risks the data wrapping around the dateline so it can't be seen. For example,
replacing "23:59:59" with "00:00:00" in the first example doesn't correctly
wrap around the dateline.
The patch introduces a way to parse the dates requested in the browser's
time zone, and then translates them to the profile's timezone. The
difference is shown in the second example above. With this change, the
correct date label should be rendered, and the data should start at
midnight without wrapping around the dateline.
This fixes the label on the days in the daytoday charts. Passing a moment object that is already zoned prevents toLocaleDateString from reinterpreting the zone information when the date is already relative to the profile. This also ensures that the datefilter is adjusted to the profile's zone rather than truncating the time to the end or beginning of the day. This should prevent incorrectly wrapping arounnd the dateline.
|
Tests do not pass, but I suspect it's something with the tests themselves. This is worth trying, as it should handle both remaining issues:
|
|
Yea if you've changed the date logic, it's likely the report outputs different values. Check reports.test.js starting line 273 |
Try latest from nightscout#7836 up through commit 8764b84
patch to match nightscout#7836
Additional change to match nightscout#7836
|
I tried the changes through 8764b84 alone and with 7833 patched into master. Did not fix the problem. |
|
Strange tests... on my machine, many tests fail (careportal, hashauth) because loading the html into This is puzzling as these changes don't interact with any of that code. Notably, this is different from the failures seen in github actions, which fails the report test, which does not occur on my machine. In theory just widening the date range for the query should in theory should allow the test to pass, but given the other oddities here, it's a little odd. |
|
closing in favor of #7857 |
Midnight for the profile should always be at the start of the day (midnight) for the chart. There's logic for this in
lib/report/reportclient.js, but believe it needs to be here as well.