From b64366231dca90faec58ea12ba0179d4afe6f6c8 Mon Sep 17 00:00:00 2001 From: Dominik Broj Date: Mon, 25 Mar 2024 17:20:10 +0100 Subject: [PATCH] Fix schedule gaps when DST happens between rotation start and end date (#4103) # What this PR does 1) IF in rotation form user selects start date before DST change AND according to recurrence period / selected end date it's after DST change AND there is a difference in hours identified because of DST change THEN set hour of end date to the same as start date so that there are no gaps in the schedule 2) Fix showing current system time info in schedule slot ## Which issue(s) this PR closes https://github.com/grafana/oncall/issues/3261 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- .../schedules/scheduleDetails.test.ts | 5 +- .../e2e-tests/schedules/timezones.test.ts | 10 +-- .../src/containers/Rotation/Rotation.tsx | 9 ++- .../RotationForm/RotationForm.helpers.ts | 21 ++++++ .../containers/RotationForm/RotationForm.tsx | 71 ++++++++++++++++--- .../RotationForm/parts/DateTimePicker.tsx | 15 +++- .../ScheduleSlot/ScheduleSlot.module.css | 4 +- .../containers/ScheduleSlot/ScheduleSlot.tsx | 24 ++++--- .../UserTimezoneSelect/UserTimezoneSelect.tsx | 12 +++- .../ScheduleUserDetails.tsx | 4 +- .../UsersTimezones/UsersTimezones.helpers.ts | 2 +- .../src/models/timezone/timezone.ts | 2 +- 12 files changed, 137 insertions(+), 42 deletions(-) diff --git a/grafana-plugin/e2e-tests/schedules/scheduleDetails.test.ts b/grafana-plugin/e2e-tests/schedules/scheduleDetails.test.ts index 5afabeea..e1bd7f32 100644 --- a/grafana-plugin/e2e-tests/schedules/scheduleDetails.test.ts +++ b/grafana-plugin/e2e-tests/schedules/scheduleDetails.test.ts @@ -10,9 +10,8 @@ test(`user can see the other user's details`, async ({ adminRolePage, editorRole await createOnCallScheduleWithRotation(page, onCallScheduleName, adminUserName); await createRotation(page, editorUserName, false); + await page.waitForTimeout(1_000); + await page.getByTestId('user-avatar-in-schedule').first().hover(); await expect(page.getByTestId('schedule-user-details')).toHaveText(new RegExp(editorUserName)); - - await page.getByTestId('user-avatar-in-schedule').nth(1).hover(); - await expect(page.getByTestId('schedule-user-details')).toHaveText(new RegExp(adminUserName)); }); diff --git a/grafana-plugin/e2e-tests/schedules/timezones.test.ts b/grafana-plugin/e2e-tests/schedules/timezones.test.ts index f9d098bd..32f5689d 100644 --- a/grafana-plugin/e2e-tests/schedules/timezones.test.ts +++ b/grafana-plugin/e2e-tests/schedules/timezones.test.ts @@ -11,7 +11,9 @@ import { createOnCallScheduleWithRotation } from '../utils/schedule'; dayjs.extend(utc); dayjs.extend(isoWeek); -test.use({ timezoneId: 'Europe/Moscow' }); // GMT+3 the whole year +const MOSCOW_TIMEZONE = 'Europe/Moscow'; + +test.use({ timezoneId: MOSCOW_TIMEZONE }); // GMT+3 the whole year const currentUtcTimeHour = dayjs().utc().format('HH'); const currentUtcDate = dayjs().utc().format('DD MMM'); const currentMoscowTimeHour = dayjs().utcOffset(180).format('HH'); @@ -20,7 +22,7 @@ const currentMoscowDate = dayjs().utcOffset(180).format('DD MMM'); test('dates in schedule are correct according to selected current timezone', async ({ adminRolePage }) => { const { page, userName } = adminRolePage; - await setTimezoneInProfile(page, 'Europe/Moscow'); + await setTimezoneInProfile(page, MOSCOW_TIMEZONE); const onCallScheduleName = generateRandomValue(); await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); @@ -41,7 +43,7 @@ test('dates in schedule are correct according to selected current timezone', asy await expect(page.getByTestId('schedule-user-details_your-current-time')).toHaveText( new RegExp(currentMoscowTimeHour) ); - await expect(page.getByTestId('schedule-user-details_user-local-time')).toHaveText(/GMT\+3/); + await expect(page.getByTestId('schedule-user-details_user-local-time')).toHaveText(new RegExp(MOSCOW_TIMEZONE)); await expect(page.getByTestId('schedule-user-details_user-local-time')).toHaveText(new RegExp(currentMoscowTimeHour)); // Schedule slot shows correct times and timezones @@ -50,7 +52,7 @@ test('dates in schedule are correct according to selected current timezone', asy await expect(page.getByTestId('schedule-slot-user-local-time')).toHaveText( new RegExp(`${currentMoscowDate}, ${currentMoscowTimeHour}`) ); - await expect(page.getByTestId('schedule-slot-user-local-time')).toHaveText(/\(GMT\+3\)/); + await expect(page.getByTestId('schedule-slot-user-local-time')).toHaveText(new RegExp(MOSCOW_TIMEZONE)); await expect(page.getByTestId('schedule-slot-current-timezone')).toHaveText( new RegExp(`${currentUtcDate}, ${currentUtcTimeHour}`) ); diff --git a/grafana-plugin/src/containers/Rotation/Rotation.tsx b/grafana-plugin/src/containers/Rotation/Rotation.tsx index 51ed0b7b..f75e738a 100644 --- a/grafana-plugin/src/containers/Rotation/Rotation.tsx +++ b/grafana-plugin/src/containers/Rotation/Rotation.tsx @@ -41,7 +41,7 @@ interface RotationProps { export const Rotation: FC = observer((props) => { const { - timezoneStore: { calendarStartDate }, + timezoneStore: { calendarStartDate, getDateInSelectedTimezone }, } = useStore(); const { events, @@ -133,11 +133,14 @@ export const Rotation: FC = observer((props) => { } const firstShift = events[0]; - const firstShiftOffset = dayjs(firstShift.start).diff(calendarStartDate, 'seconds'); + const firstShiftOffset = getDateInSelectedTimezone(firstShift.start).diff( + getDateInSelectedTimezone(calendarStartDate), + 'seconds' + ); const base = 60 * 60 * 24 * days; return firstShiftOffset / base; - }, [events]); + }, [events, calendarStartDate]); return (
diff --git a/grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts b/grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts index 4876f8ad..c8aee870 100644 --- a/grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts +++ b/grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts @@ -187,3 +187,24 @@ export const getDateForDatePicker = (dayJsDate: Dayjs) => { date.setSeconds(dayJsDate.second()); return date; }; + +export const dayJSAddWithDSTFixed = ({ + baseDate, + addParams, +}: { + baseDate: Dayjs; + addParams: Parameters; +}) => { + // At first we add time as usual + let newDateCandidate = baseDate.add(...addParams); + + const differenceInHoursInLocalTimezone = newDateCandidate.hour() - baseDate.hour(); + const differenceInHoursInUTC = newDateCandidate.utc().hour() - baseDate.utc().hour(); + + // But if we identify that there was a DST change before base date and the result candidate + if (differenceInHoursInLocalTimezone !== differenceInHoursInUTC) { + // then we make the resulting date to ignore DST change + newDateCandidate = newDateCandidate.subtract(differenceInHoursInUTC, 'hours'); + } + return newDateCandidate; +}; diff --git a/grafana-plugin/src/containers/RotationForm/RotationForm.tsx b/grafana-plugin/src/containers/RotationForm/RotationForm.tsx index d7e5450f..c7f42b68 100644 --- a/grafana-plugin/src/containers/RotationForm/RotationForm.tsx +++ b/grafana-plugin/src/containers/RotationForm/RotationForm.tsx @@ -24,6 +24,7 @@ import { Text } from 'components/Text/Text'; import { UserGroups } from 'components/UserGroups/UserGroups'; import { RemoteSelect } from 'containers/RemoteSelect/RemoteSelect'; import { + dayJSAddWithDSTFixed, getRepeatShiftsEveryOptions, putDownMaxValues, reduceTheLastUnitValue, @@ -279,9 +280,19 @@ export const RotationForm = observer((props: RotationFormProps) => { if (!showActiveOnSelectedPartOfDay) { if (showActiveOnSelectedDays) { - setShiftEnd(shiftStart.add(24, 'hours')); + setShiftEnd( + dayJSAddWithDSTFixed({ + baseDate: shiftStart, + addParams: [24, 'hours'], + }) + ); } else { - setShiftEnd(shiftStart.add(repeatEveryValue, repeatEveryPeriodToUnitName[value])); + setShiftEnd( + dayJSAddWithDSTFixed({ + baseDate: shiftStart, + addParams: [repeatEveryValue, repeatEveryPeriodToUnitName[value]], + }) + ); } } }, @@ -298,7 +309,12 @@ export const RotationForm = observer((props: RotationFormProps) => { setRepeatEveryValue(value); if (!showActiveOnSelectedPartOfDay) { - setShiftEnd(rotationStart.add(value, repeatEveryPeriodToUnitName[repeatEveryPeriod])); + setShiftEnd( + dayJSAddWithDSTFixed({ + baseDate: rotationStart, + addParams: [value, repeatEveryPeriodToUnitName[repeatEveryPeriod]], + }) + ); } }; @@ -307,9 +323,19 @@ export const RotationForm = observer((props: RotationFormProps) => { setRotationStart(value); setShiftStart(value); if (showActiveOnSelectedPartOfDay) { - setShiftEnd(value.add(activePeriod, 'seconds')); + setShiftEnd( + dayJSAddWithDSTFixed({ + baseDate: value, + addParams: [activePeriod, 'seconds'], + }) + ); } else { - setShiftEnd(value.add(repeatEveryValue, repeatEveryPeriodToUnitName[repeatEveryPeriod])); + setShiftEnd( + dayJSAddWithDSTFixed({ + baseDate: value, + addParams: [repeatEveryValue, repeatEveryPeriodToUnitName[repeatEveryPeriod]], + }) + ); } }, [showActiveOnSelectedPartOfDay, activePeriod, repeatEveryPeriod, repeatEveryValue] @@ -318,7 +344,12 @@ export const RotationForm = observer((props: RotationFormProps) => { const handleActivePeriodChange = useCallback( (value) => { setActivePeriod(value); - setShiftEnd(shiftStart.add(value, 'seconds')); + setShiftEnd( + dayJSAddWithDSTFixed({ + baseDate: shiftStart, + addParams: [value, 'seconds'], + }) + ); }, [shiftStart] ); @@ -337,10 +368,20 @@ export const RotationForm = observer((props: RotationFormProps) => { setShowActiveOnSelectedDays(value); if (value) { - setShiftEnd(shiftStart.add(24, 'hours')); + setShiftEnd( + dayJSAddWithDSTFixed({ + baseDate: shiftStart, + addParams: [24, 'hours'], + }) + ); } else { if (!showActiveOnSelectedPartOfDay) { - setShiftEnd(shiftStart.add(repeatEveryValue, repeatEveryPeriodToUnitName[repeatEveryPeriod])); + setShiftEnd( + dayJSAddWithDSTFixed({ + baseDate: shiftStart, + addParams: [repeatEveryValue, repeatEveryPeriodToUnitName[repeatEveryPeriod]], + }) + ); } } }, @@ -354,9 +395,19 @@ export const RotationForm = observer((props: RotationFormProps) => { if (!value) { if (showActiveOnSelectedPartOfDay) { - setShiftEnd(shiftStart.add(24, 'hours')); + setShiftEnd( + dayJSAddWithDSTFixed({ + baseDate: shiftStart, + addParams: [24, 'hours'], + }) + ); } else { - setShiftEnd(shiftStart.add(repeatEveryValue, repeatEveryPeriodToUnitName[repeatEveryPeriod])); + setShiftEnd( + dayJSAddWithDSTFixed({ + baseDate: shiftStart, + addParams: [repeatEveryValue, repeatEveryPeriodToUnitName[repeatEveryPeriod]], + }) + ); } } }, diff --git a/grafana-plugin/src/containers/RotationForm/parts/DateTimePicker.tsx b/grafana-plugin/src/containers/RotationForm/parts/DateTimePicker.tsx index 1f69c3d1..6dc88a26 100644 --- a/grafana-plugin/src/containers/RotationForm/parts/DateTimePicker.tsx +++ b/grafana-plugin/src/containers/RotationForm/parts/DateTimePicker.tsx @@ -1,7 +1,8 @@ import React from 'react'; +import { css } from '@emotion/css'; import { DateTime, dateTime } from '@grafana/data'; -import { DatePickerWithInput, TimeOfDayPicker, VerticalGroup } from '@grafana/ui'; +import { DatePickerWithInput, TimeOfDayPicker, useStyles2, VerticalGroup } from '@grafana/ui'; import cn from 'classnames/bind'; import dayjs from 'dayjs'; import { observer } from 'mobx-react'; @@ -25,6 +26,7 @@ interface DateTimePickerProps { export const DateTimePicker = observer( ({ value: propValue, onChange, disabled, onFocus, onBlur, error }: DateTimePickerProps) => { + const styles = useStyles2(getStyles); const { timezoneStore: { getDateInSelectedTimezone }, } = useStore(); @@ -61,7 +63,7 @@ export const DateTimePicker = observer( return ( -
+
({ + wrapper: css` + display: flex; + flex-wrap: nowrap; + gap: 8px; + z-index: 2; + `, +}); diff --git a/grafana-plugin/src/containers/ScheduleSlot/ScheduleSlot.module.css b/grafana-plugin/src/containers/ScheduleSlot/ScheduleSlot.module.css index 52184f56..7369f56c 100644 --- a/grafana-plugin/src/containers/ScheduleSlot/ScheduleSlot.module.css +++ b/grafana-plugin/src/containers/ScheduleSlot/ScheduleSlot.module.css @@ -76,7 +76,7 @@ } .details { - width: 250px; + width: 300px; padding: 5px 0; } @@ -121,7 +121,7 @@ } .second-column { - width: 102px; + width: 120px; } .icon { diff --git a/grafana-plugin/src/containers/ScheduleSlot/ScheduleSlot.tsx b/grafana-plugin/src/containers/ScheduleSlot/ScheduleSlot.tsx index 72313d18..43cffa8a 100644 --- a/grafana-plugin/src/containers/ScheduleSlot/ScheduleSlot.tsx +++ b/grafana-plugin/src/containers/ScheduleSlot/ScheduleSlot.tsx @@ -11,7 +11,7 @@ import { Text } from 'components/Text/Text'; import { WorkingHours } from 'components/WorkingHours/WorkingHours'; import { getShiftName, SHIFT_SWAP_COLOR } from 'models/schedule/schedule.helpers'; import { Event, ShiftSwap } from 'models/schedule/schedule.types'; -import { getOffsetOfCurrentUser, getTzOffsetString } from 'models/timezone/timezone.helpers'; +import { getTzOffsetString } from 'models/timezone/timezone.helpers'; import { ApiSchemas } from 'network/oncall-api/api.types'; import { useStore } from 'state/useStore'; @@ -32,8 +32,12 @@ interface ScheduleSlotProps { } const cx = cn.bind(styles); +const ONE_WEEK_IN_SECONDS = 7 * 24 * 60 * 60; export const ScheduleSlot: FC = observer((props) => { + const { + timezoneStore: { getDateInSelectedTimezone }, + } = useStore(); const { event, color, @@ -46,14 +50,12 @@ export const ScheduleSlot: FC = observer((props) => { showScheduleNameAsSlotTitle, } = props; - const start = dayjs(event.start); - const end = dayjs(event.end); + const start = getDateInSelectedTimezone(event.start); + const end = getDateInSelectedTimezone(event.end); - const duration = end.diff(start, 'seconds'); + const durationInSeconds = end.diff(start, 'seconds'); - const base = 60 * 60 * 24 * 7; - - const width = Math.max(duration / base, 0); + const width = Math.max(durationInSeconds / ONE_WEEK_IN_SECONDS, 0); const currentMoment = useMemo(() => dayjs(), []); @@ -90,7 +92,7 @@ export const ScheduleSlot: FC = observer((props) => { onShiftSwapClick={onShiftSwapClick} filters={filters} start={start} - duration={duration} + duration={durationInSeconds} color={color} currentMoment={currentMoment} showScheduleNameAsSlotTitle={showScheduleNameAsSlotTitle} @@ -411,7 +413,7 @@ const ScheduleSlotDetails = observer((props: ScheduleSlotDetailsProps) => { User's local time
{currentMoment.tz(user?.timezone).format('DD MMM, HH:mm')} -
({getTzOffsetString(currentMoment.tz(user?.timezone))}) +
({user?.timezone}) Current timezone @@ -427,9 +429,9 @@ const ScheduleSlotDetails = observer((props: ScheduleSlotDetailsProps) => { This shift
- {dayjs(event.start).utcOffset(getOffsetOfCurrentUser()).format('DD MMM, HH:mm')} + {dayjs(event.start).tz(user?.timezone).format('DD MMM, HH:mm')}
- {dayjs(event.end).utcOffset(getOffsetOfCurrentUser()).format('DD MMM, HH:mm')} + {dayjs(event.end).tz(user?.timezone).format('DD MMM, HH:mm')}  
diff --git a/grafana-plugin/src/containers/UserTimezoneSelect/UserTimezoneSelect.tsx b/grafana-plugin/src/containers/UserTimezoneSelect/UserTimezoneSelect.tsx index be30eff6..b4aef832 100644 --- a/grafana-plugin/src/containers/UserTimezoneSelect/UserTimezoneSelect.tsx +++ b/grafana-plugin/src/containers/UserTimezoneSelect/UserTimezoneSelect.tsx @@ -1,4 +1,4 @@ -import React, { FC, useCallback, useMemo, useState } from 'react'; +import React, { FC, useCallback, useEffect, useMemo, useState } from 'react'; import { SelectableValue } from '@grafana/data'; import { Select } from '@grafana/ui'; @@ -66,6 +66,8 @@ export const UserTimezoneSelect: FC = observer(({ sched ); }, [users, extraOptions]); + const selectedOption = options.find(({ value }) => value === store.timezoneStore.selectedTimezoneOffset); + const filterOption = useCallback((item: SelectableValue, searchQuery: string) => { const { data } = item; @@ -77,6 +79,12 @@ export const UserTimezoneSelect: FC = observer(({ sched }); }, []); + useEffect(() => { + if (selectedOption?.value) { + store.timezoneStore.setSelectedTimezoneOffset(selectedOption.value); + } + }, [options]); + const handleCreateOption = useCallback( (value: string) => { const matched = allTimezones.find((tz) => tz.toLowerCase().includes(value.toLowerCase())); @@ -110,7 +118,7 @@ export const UserTimezoneSelect: FC = observer(({ sched return (