feat(custom-timerange-tz): timeZone dropdown is now integrated in setting custom time ranges for queries #17992
Conversation
…ing a query with a custom timeRange Need to integrate this functionality for the queryBuilder
…ueryBuilder to correct set keys and values
…ng custom timeRanges to UTC by creating a selector to handle that logic wrote tests to test the new selector
…imeZone in AppState mock
| expect( | ||
| untypedGetTimeRangeWithTimeZone({ranges, currentDashboard, app}) | ||
| ).toEqual(expected) | ||
| }) |
There was a problem hiding this comment.
these tests look fine, and are wise in that they don't reference like today or now but instead solid dates in the past.
but having said that, a career of having tests around checking date/times that fail randomly has made me extremely wary and gunshy of them. just keep an eye on tests that involve dates and time. they can fail and blow up in weird and unexpected and painful ways.
| @@ -164,7 +164,7 @@ const mstp = (state: AppState): StateProps => { | |||
| const fillColumns = getFillColumnsSelection(state) | |||
| const symbolColumns = getSymbolColumnsSelection(state) | |||
|
|
|||
| const timeZone = state.app.persisted.timeZone | |||
| const timeZone = getTimeZone(state) | |||
There was a problem hiding this comment.
so the line this is replacing wouldn't set the timeZone to local if it state.app.persisted.timeZone didn't exist. this function will. is that okay for this component? i imagine it is, but just want to be explicit
There was a problem hiding this comment.
I also noticed that. It's weird because we set local as the default in only one place, but I think it's good to have a default for this value, especially if it's a required property. I'm very wary of assigning defaultProps, but in this case I think it's safe to assign a default even if none were specified
Update the .circleci config file to set the timezone
…dated mockState to see whether the test failures happen differently
| // https://github.com/influxdata/influxdb/issues/17877 | ||
| // Example: user selected 10-11:00am and sets the dropdown to UTC | ||
| // Query should run against 10-11:00am UTC rather than querying | ||
| // 10-11:00am local time (offset depending on timezone) |
There was a problem hiding this comment.
nice comment!
nit: put it above the function definition
| // 10-11:00am local time (offset depending on timezone) | ||
| const offset = new Date(date).getTimezoneOffset() | ||
| if (offset > 0) { | ||
| // subtract tz minute difference |
There was a problem hiding this comment.
OH! is that what the subtract function does?
Closes #17877
Purpose
The primary purpose of this PR is to ensure that whenever a user sets UTC as their timeZone and is querying a custom timeRange, their query will query data that was set at that set time in UTC (not the adjusted locale -> UTC time).
Problem
The UI for the timeZone (local or UTC) was often confusing and seemed to indicate functionality beyond displaying graphs in UTC.
Solution
Created a selector to update the timeRange whenever a query is executed with a custom timeRange and the timeZone set to UTC.