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

<!--
*Note*: if you have more than one GitHub issue that this PR closes, be
sure to preface
each issue link with a [closing
keyword](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests#linking-a-pull-request-to-an-issue).
This ensures that the issue(s) are auto-closed once the PR has been
merged.
-->

## 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.
This commit is contained in:
Dominik Broj 2024-03-25 17:20:10 +01:00 committed by GitHub
parent 3eca7a6a89
commit b64366231d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 137 additions and 42 deletions

View file

@ -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));
});

View file

@ -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}`)
);

View file

@ -41,7 +41,7 @@ interface RotationProps {
export const Rotation: FC<RotationProps> = observer((props) => {
const {
timezoneStore: { calendarStartDate },
timezoneStore: { calendarStartDate, getDateInSelectedTimezone },
} = useStore();
const {
events,
@ -133,11 +133,14 @@ export const Rotation: FC<RotationProps> = 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 (
<div className={cx('root')} onClick={onClick && handleRotationClick}>

View file

@ -187,3 +187,24 @@ export const getDateForDatePicker = (dayJsDate: Dayjs) => {
date.setSeconds(dayJsDate.second());
return date;
};
export const dayJSAddWithDSTFixed = ({
baseDate,
addParams,
}: {
baseDate: Dayjs;
addParams: Parameters<Dayjs['add']>;
}) => {
// 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;
};

View file

@ -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]],
})
);
}
}
},

View file

@ -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 (
<VerticalGroup>
<div style={{ display: 'flex', flexWrap: 'nowrap', gap: '8px' }}>
<div className={styles.wrapper}>
<div
onFocus={onFocus}
onBlur={onBlur}
@ -90,3 +92,12 @@ export const DateTimePicker = observer(
);
}
);
const getStyles = () => ({
wrapper: css`
display: flex;
flex-wrap: nowrap;
gap: 8px;
z-index: 2;
`,
});

View file

@ -76,7 +76,7 @@
}
.details {
width: 250px;
width: 300px;
padding: 5px 0;
}
@ -121,7 +121,7 @@
}
.second-column {
width: 102px;
width: 120px;
}
.icon {

View file

@ -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<ScheduleSlotProps> = observer((props) => {
const {
timezoneStore: { getDateInSelectedTimezone },
} = useStore();
const {
event,
color,
@ -46,14 +50,12 @@ export const ScheduleSlot: FC<ScheduleSlotProps> = 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<ScheduleSlotProps> = 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
<br />
{currentMoment.tz(user?.timezone).format('DD MMM, HH:mm')}
<br />({getTzOffsetString(currentMoment.tz(user?.timezone))})
<br />({user?.timezone})
</Text>
<Text type="secondary" data-testid="schedule-slot-current-timezone">
Current timezone
@ -427,9 +429,9 @@ const ScheduleSlotDetails = observer((props: ScheduleSlotDetailsProps) => {
<Text type="primary" className={cx('second-column')}>
This shift
<br />
{dayjs(event.start).utcOffset(getOffsetOfCurrentUser()).format('DD MMM, HH:mm')}
{dayjs(event.start).tz(user?.timezone).format('DD MMM, HH:mm')}
<br />
{dayjs(event.end).utcOffset(getOffsetOfCurrentUser()).format('DD MMM, HH:mm')}
{dayjs(event.end).tz(user?.timezone).format('DD MMM, HH:mm')}
</Text>
<Text type="secondary">
&nbsp; <br />

View file

@ -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<UserTimezoneSelectProps> = observer(({ sched
);
}, [users, extraOptions]);
const selectedOption = options.find(({ value }) => value === store.timezoneStore.selectedTimezoneOffset);
const filterOption = useCallback((item: SelectableValue<number>, searchQuery: string) => {
const { data } = item;
@ -77,6 +79,12 @@ export const UserTimezoneSelect: FC<UserTimezoneSelectProps> = 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<UserTimezoneSelectProps> = observer(({ sched
return (
<div className={cx('root')} data-testid="timezone-select">
<Select
value={options.find(({ value }) => value === store.timezoneStore.selectedTimezoneOffset)}
value={selectedOption}
onChange={(option) => store.timezoneStore.setSelectedTimezoneOffset(option.value)}
width={30}
options={options}

View file

@ -35,8 +35,6 @@ export const ScheduleUserDetails: FC<ScheduleUserDetailsProps> = observer((props
timezoneStore: { calendarStartDate },
} = useStore();
const { user, currentMoment, isOncall, scheduleId } = props;
const userMoment = currentMoment.tz(user.timezone);
const userOffsetHoursStr = getTzOffsetString(userMoment);
const isInWH = isInWorkingHours(currentMoment, user.working_hours, user.timezone);
const store = useStore();
@ -89,7 +87,7 @@ export const ScheduleUserDetails: FC<ScheduleUserDetailsProps> = observer((props
<VerticalGroup className={cx('timezone-info')} spacing="none">
<Text>User's local time</Text>
<Text>{`${getCurrentDateInTimezone(user.timezone).format('DD MMM, HH:mm')}`}</Text>
<Text>({userOffsetHoursStr})</Text>
<Text>({user.timezone})</Text>
</VerticalGroup>
</div>
</div>

View file

@ -2,5 +2,5 @@ import { Dayjs } from 'dayjs';
export const calculateTimePassedInDayPercentage = (date: Dayjs) => {
const midnight = date.startOf('day');
return (date.diff(midnight, 'minutes') / 1_440) * 100;
return (date.diff(midnight, 'minutes') / (60 * 24)) * 100;
};

View file

@ -23,7 +23,7 @@ export class TimezoneStore {
calendarStartDate = getStartOfWeekBasedOnCurrentDate(this.currentDateInSelectedTimezone);
@action.bound
async setSelectedTimezoneOffset(offset: number) {
setSelectedTimezoneOffset(offset: number) {
this.selectedTimezoneOffset = offset;
this.calendarStartDate = getStartOfWeekBasedOnCurrentDate(this.currentDateInSelectedTimezone);
}