Skip to content

Commit

Permalink
fix(dashboard-timezone-selection): toggling timezone should update qu…
Browse files Browse the repository at this point in the history
…eries timerange to respect timezone selection (#19146)
  • Loading branch information
asalem1 authored Aug 5, 2020
1 parent d48dc69 commit 2397f7f
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 32 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

1. [19043](https://github.com/influxdata/influxdb/pull/19043): Enforce all influx CLI flag args are valid
1. [19188](https://github.com/influxdata/influxdb/pull/19188): Dashboard cells correctly map results when multiple queries exist
1. [19146](https://github.com/influxdata/influxdb/pull/19146): Dashboard cells and overlay use UTC as query time when toggling to UTC timezone

## v2.0.0-beta.15 [2020-07-23]

Expand Down
102 changes: 92 additions & 10 deletions ui/src/dashboards/selectors/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
// Funcs
import {mocked} from 'ts-jest/utils'
import {
getTimeRange,
getTimeRangeWithTimezone,
} from 'src/dashboards/selectors/index'
import moment from 'moment'
import {getTimezoneOffset} from 'src/dashboards/utils/getTimezoneOffset'

jest.mock('src/dashboards/utils/getTimezoneOffset')

// Types
import {RangeState} from 'src/dashboards/reducers/ranges'
Expand All @@ -30,22 +33,37 @@ const untypedGetTimeRangeWithTimeZone = getTimeRangeWithTimezone as (a: {
}) => TimeRange

describe('Dashboards.Selector', () => {
beforeEach(() => {
jest.clearAllMocks()
})
const dashboardIDs = [
'04c6f3976f4b8001',
'04c6f3976f4b8000',
'04c6f3976f4b8002',
'04c6f3976f4b8003',
'04c6f3976f4b8004',
]
const lower = `2020-05-05T10:00:00${moment().format('Z')}`
const upper = `2020-05-05T11:00:00${moment().format('Z')}`
const customTimeRange = {
lower,
upper,
const customTimeRangePST = {
lower: '2020-05-05T10:00:00-07:00',
upper: '2020-05-05T11:00:00-07:00',
type: 'custom',
} as CustomTimeRange
const customTimeRangeCET = {
lower: '2020-05-05T10:00:00+02:00',
upper: '2020-05-05T11:00:00+02:00',
type: 'custom',
} as CustomTimeRange
const customTimeRangeGMT = {
lower: '2020-05-05T10:00:00+00:00',
upper: '2020-05-05T11:00:00+00:00',
type: 'custom',
} as CustomTimeRange
const ranges: RangeState = {
[dashboardIDs[0]]: pastFifteenMinTimeRange,
[dashboardIDs[1]]: pastHourTimeRange,
[dashboardIDs[2]]: customTimeRange,
[dashboardIDs[2]]: customTimeRangePST,
[dashboardIDs[3]]: customTimeRangeCET,
[dashboardIDs[4]]: customTimeRangeGMT,
}

it('should return the correct range when a matching dashboard ID is found', () => {
Expand All @@ -72,7 +90,7 @@ describe('Dashboards.Selector', () => {
).toEqual(DEFAULT_TIME_RANGE)
})

it('should return the an unmodified version of the timeRange when the timeZone is local', () => {
it('should return an unmodified version of the timeRange when the timeZone is local', () => {
const currentDashboard = {id: dashboardIDs[2]}
const app: AppPresentationState = {
ephemeral: {
Expand All @@ -91,10 +109,10 @@ describe('Dashboards.Selector', () => {

expect(
untypedGetTimeRangeWithTimeZone({ranges, currentDashboard, app})
).toEqual(customTimeRange)
).toEqual(customTimeRangePST)
})

it('should return the timeRange for the same hour with a UTC timezone when the timeZone is UTC', () => {
it('should return the timeRange for the same hour with a UTC timezone when the timeZone is UTC and the locale is 7 timezones behind UTC', () => {
const currentDashboard = {id: dashboardIDs[2]}

const app: AppPresentationState = {
Expand All @@ -117,6 +135,70 @@ describe('Dashboards.Selector', () => {
upper: `2020-05-05T11:00:00Z`,
type: 'custom',
}
// Offset for PST
mocked(getTimezoneOffset).mockImplementation(() => 420)

expect(
untypedGetTimeRangeWithTimeZone({ranges, currentDashboard, app})
).toEqual(expected)
})

it('should return the timeRange for the same hour with a UTC timezone when the timeZone is UTC and the locale is 2 timezones ahead of UTC', () => {
const currentDashboard = {id: dashboardIDs[3]}

const app: AppPresentationState = {
ephemeral: {
inPresentationMode: false,
hasUpdatedTimeRangeInVEO: false,
},
persisted: {
autoRefresh: 0,
showTemplateControlBar: false,
navBarState: 'expanded',
notebookMiniMapState: 'expanded',
timeZone: 'UTC' as TimeZone,
theme: 'dark',
},
}

const expected = {
lower: `2020-05-05T10:00:00Z`,
upper: `2020-05-05T11:00:00Z`,
type: 'custom',
}
// Offset for CET
mocked(getTimezoneOffset).mockImplementation(() => -120)

expect(
untypedGetTimeRangeWithTimeZone({ranges, currentDashboard, app})
).toEqual(expected)
})

it('should return the timeRange when the timezone has no offset', () => {
const currentDashboard = {id: dashboardIDs[4]}

const app: AppPresentationState = {
ephemeral: {
inPresentationMode: false,
hasUpdatedTimeRangeInVEO: false,
},
persisted: {
autoRefresh: 0,
showTemplateControlBar: false,
navBarState: 'expanded',
notebookMiniMapState: 'expanded',
timeZone: 'UTC' as TimeZone,
theme: 'dark',
},
}

const expected = {
lower: `2020-05-05T10:00:00Z`,
upper: `2020-05-05T11:00:00Z`,
type: 'custom',
}

mocked(getTimezoneOffset).mockImplementation(() => 0)

expect(
untypedGetTimeRangeWithTimeZone({ranges, currentDashboard, app})
Expand Down
29 changes: 13 additions & 16 deletions ui/src/dashboards/selectors/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// Libraries
import {get} from 'lodash'
import moment from 'moment'

// Types
import {
AppState,
Check,
Expand All @@ -9,7 +12,10 @@ import {
View,
ViewType,
} from 'src/types'

// Utility
import {currentContext} from 'src/shared/selectors/currentContext'
import {getTimezoneOffset} from 'src/dashboards/utils/getTimezoneOffset'

// Constants
import {DEFAULT_TIME_RANGE} from 'src/shared/constants/timeRanges'
Expand Down Expand Up @@ -53,25 +59,16 @@ export const isCurrentPageDashboard = (state: AppState): boolean =>
// from the local time to the same time in UTC if UTC is selected from the
// timezone dropdown. This is feature was original requested here:
// https://github.com/influxdata/influxdb/issues/17877
// and finalized across the dashboards & the data explorer here:
// https://github.com/influxdata/influxdb/pull/19146
// 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)
export const setTimeToUTC = (date: string): string => {
const offset = new Date(date).getTimezoneOffset()
if (offset > 0) {
return moment
.utc(date)
.subtract(offset, 'minutes')
.format()
}
if (offset < 0) {
return moment
.utc(date)
.add(offset, 'minutes')
.format()
}
return moment.utc(date).format()
}
export const setTimeToUTC = (date: string): string =>
moment
.utc(date)
.subtract(getTimezoneOffset(), 'minutes')
.format()

export const getTimeZone = (state: AppState): TimeZone => {
return state.app.persisted.timeZone || 'Local'
Expand Down
13 changes: 13 additions & 0 deletions ui/src/dashboards/utils/getTimezoneOffset.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/**
* This files has been created as a way to effectively test
* the getTimeRangeWithTimezone function since current system (circleCI, Jenkins)
* and JS Date limitations prevent us from fully testing out its dependent functions
*
* It should be noted that the native getTimezoneOffset function returns a number
* that represents the number of minutes (not hours) the "local" timezone is offset
* where locations West of UTC are positive (+420) and locations East of UTC are negative (-120):
*
* https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/getTimezoneOffset
**/

export const getTimezoneOffset = (): number => new Date().getTimezoneOffset()
4 changes: 2 additions & 2 deletions ui/src/shared/components/RefreshingView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import CellEvent from 'src/perf/components/CellEvent'

// Utils
import {GlobalAutoRefresher} from 'src/utils/AutoRefresher'
import {getTimeRange} from 'src/dashboards/selectors'
import {getTimeRangeWithTimezone} from 'src/dashboards/selectors'
import {checkResultsLength} from 'src/shared/utils/vis'
import {getActiveTimeRange} from 'src/timeMachine/selectors/index'

Expand Down Expand Up @@ -147,7 +147,7 @@ class RefreshingView extends PureComponent<Props, State> {
}

const mstp = (state: AppState, ownProps: OwnProps) => {
const timeRange = getTimeRange(state)
const timeRange = getTimeRangeWithTimezone(state)
const ranges = getActiveTimeRange(timeRange, ownProps.properties.queries)
const {timeZone, theme} = state.app.persisted

Expand Down
4 changes: 2 additions & 2 deletions ui/src/shared/components/TimeSeries.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {getCachedResultsThunk} from 'src/shared/apis/queryCache'

// Utils
import {
getTimeRange,
getTimeRangeWithTimezone,
isCurrentPageDashboard as isCurrentPageDashboardSelector,
} from 'src/dashboards/selectors'
import {getVariables, asAssignment} from 'src/variables/selectors'
Expand Down Expand Up @@ -369,7 +369,7 @@ class TimeSeries extends Component<Props, State> {
}

const mstp = (state: AppState, props: OwnProps) => {
const timeRange = getTimeRange(state)
const timeRange = getTimeRangeWithTimezone(state)

// NOTE: cannot use getAllVariables here because the TimeSeries
// component appends it automatically. That should be fixed
Expand Down
4 changes: 2 additions & 2 deletions ui/src/timeMachine/components/Vis.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
getFillColumnsSelection,
getSymbolColumnsSelection,
} from 'src/timeMachine/selectors'
import {getTimeRange, getTimeZone} from 'src/dashboards/selectors'
import {getTimeRangeWithTimezone, getTimeZone} from 'src/dashboards/selectors'

// Types
import {RemoteDataState, AppState} from 'src/types'
Expand Down Expand Up @@ -126,7 +126,7 @@ const mstp = (state: AppState) => {
statuses,
},
} = activeTimeMachine
const timeRange = getTimeRange(state)
const timeRange = getTimeRangeWithTimezone(state)
const {
alertBuilder: {type: checkType, thresholds: checkThresholds},
} = state
Expand Down

0 comments on commit 2397f7f

Please sign in to comment.