Skip to content
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

Wip/bewest/daytoday redo 02 #7849

Closed
wants to merge 6 commits into from
Closed

Wip/bewest/daytoday redo 02 #7849

wants to merge 6 commits into from

Conversation

bewest
Copy link
Member

@bewest bewest commented Jan 23, 2023

Here's a happier version of 01750cf

Revert "Bump jsonwebtoken from 8.5.1 to 9.0.0 (#7787)"

This rolls back changes to node_modules via package-lock.json as a
result of new dependencies.  More work is needed before involving these
changes.  Without this rollback, the tests do consistently complete within
the allowed time in a reproducible way.
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.
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.
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.
Ensure that the test fixtures will return; the previous changes
correctly forward the time to end of day across zones and
datelines.  One side effect  is that the date formatted for the end
of the day uses all the microseconds for the day as well.  This
changes the query from the form of 23:59:59.000Z to 23:59:59.999Z.
This also ensures that anything that happens during that one second
will be included rather than excluded.
@bewest
Copy link
Member Author

bewest commented Jan 24, 2023

closing in favor of #7857

@bewest bewest closed this Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant