Skip to content

ERA-11347: [P1] Desktop time field malfunctioning (Offset by 1 hour) #1283

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 23, 2025
Merged

ERA-11347: [P1] Desktop time field malfunctioning (Offset by 1 hour) #1283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/DatePicker/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { memo, useEffect, useImperativeHandle, useRef, useState } from 'react';
import { parseISO } from 'date-fns';
import { lastDayOfMonth, parseISO } from 'date-fns';
import { useTranslation } from 'react-i18next';

import { ReactComponent as CalendarIcon } from '../common/images/icons/calendar.svg';
Expand All @@ -13,6 +13,7 @@ import {
isMonthInputComplete,
isSecondDayDigitPossible,
isSecondMonthDigitPossible,
isValidDate,
isValidDayInput,
isValidMonthInput,
isValidYearInput,
Expand Down Expand Up @@ -231,14 +232,20 @@ const DatePicker = ({
};

const onDayInputKeyDown = (event) => {
// If the year and month inputs are complete, we can calculate the last valid day of the current month to set when
// the user decreases or increases the day with the arrows.
const lastValidDayOfMonth = isYearInputComplete(year) && isMonthInputComplete(month)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not part of the bug, just an extra addition to the PR to improve our date picker. If the user decreased the day input with the down arrow or increase with the up arrow, we were setting always 31 as the upper limit of the month. But not all months have 31 days, some of them vary with the leap year. So this new lastValidDayOfMonth calculation sets the last valid day instead of a fixed 31 if we have the year and month available (otherwise, it's impossible to calculate the last valid day of the month).

? lastDayOfMonth(new Date(year, month - 1)).getDate().toString()
: '31';

switch (event.key) {
case 'ArrowDown':
if (!readOnly) {
event.preventDefault();

// Decrease the day when the user presses the down arrow.
if (day === '' || parseInt(day) === 1) {
onDayChange('31');
onDayChange(lastValidDayOfMonth);
} else if (isValidDayInput(day)) {
const dayMinusOne = (parseInt(day) - 1).toString().padStart(2, '0');
onDayChange(dayMinusOne);
Expand All @@ -263,7 +270,7 @@ const DatePicker = ({
event.preventDefault();

// Increase the day when the user presses the up arrow.
if (day === '' || day === '31') {
if (day === '' || day === lastValidDayOfMonth) {
onDayChange('01');
} else if (isValidDayInput(day)) {
const dayPlusOne = (parseInt(day) + 1).toString().padStart(2, '0');
Expand Down Expand Up @@ -380,6 +387,6 @@ const DatePicker = ({
</div>;
};

export { EMPTY_DATE_VALUE };
export { EMPTY_DATE_VALUE, isValidDate };

export default memo(DatePicker);
30 changes: 28 additions & 2 deletions src/DatePicker/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ describe('DatePicker', () => {
expect(onChange).toHaveBeenCalledWith('--01');
});

test('sets the day to 01 if the input has the value 31 and is focused and the user presses the up arrow', async () => {
test('sets the day to 01 if the year and month are not valid and the day input has the value 31 and is focused and the user presses the up arrow', async () => {
renderDatePicker({ value: '--31' });

await userEvent.click(screen.getByLabelText('Day'));
Expand All @@ -811,6 +811,19 @@ describe('DatePicker', () => {
expect(onChange).toHaveBeenCalledWith('--01');
});

test('sets the day to 01 if the year and month are valid and the day input has the last valid day of the month and is focused and the user presses the up arrow', async () => {
renderDatePicker({ value: '2020-02-29' });

await userEvent.click(screen.getByLabelText('Day'));

expect(onChange).not.toHaveBeenCalled();

await userEvent.keyboard('[ArrowUp]');

expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledWith('2020-02-01');
});

test('increments the day if the input has a valid value and is focused and the user presses the up arrow', async () => {
renderDatePicker({ value: '--18' });

Expand All @@ -837,7 +850,7 @@ describe('DatePicker', () => {
expect(onChange).toHaveBeenCalledWith('--31');
});

test('sets the day to 31 if the input has the value 01 and is focused and the user presses the down arrow', async () => {
test('sets the day to 31 if the year and month are not valid and the day input has the value 01 and is focused and the user presses the down arrow', async () => {
renderDatePicker({ value: '--01' });

await userEvent.click(screen.getByLabelText('Day'));
Expand All @@ -850,6 +863,19 @@ describe('DatePicker', () => {
expect(onChange).toHaveBeenCalledWith('--31');
});

test('sets the day to the last valid day of the month if the year and month are valid and the day input has the value 01 and is focused and the user presses the down arrow', async () => {
renderDatePicker({ value: '2020-02-01' });

await userEvent.click(screen.getByLabelText('Day'));

expect(onChange).not.toHaveBeenCalled();

await userEvent.keyboard('[ArrowDown]');

expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledWith('2020-02-29');
});

test('decrements the day if the input has a valid value and is focused and the user presses the down arrow', async () => {
renderDatePicker({ value: '--18' });

Expand Down
2 changes: 1 addition & 1 deletion src/DatePicker/utils/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,4 @@ export const shouldCompleteFirstDayDigitWithZero = (day) => /^[1-3]$/.test(day);

export const isDayInputComplete = (day) => /^(0[1-9]|[12][0-9]|3[01])$/.test(day);

export const isValidDate = (time) => /^[0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])$/.test(time);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small word fix.

export const isValidDate = (date) => /^[0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])$/.test(date);
20 changes: 10 additions & 10 deletions src/DateRangeSelector/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,21 +70,21 @@ const DateRangeSelector = ({
onFilterSettingsToggle && onFilterSettingsToggle(!filterSettingsOpen);
}, [filterSettingsOpen, onFilterSettingsToggle]);

const onStartDateTimePickerChange = (dateTime) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes here, I just updated the variable name to match other places where we implemented the date time picker.

setStartDateTime(dateTime);
const onStartDateTimePickerChange = (newStartDateTime) => {
setStartDateTime(newStartDateTime);

const parsedDateTime = parseISO(dateTime);
if (isValid(parsedDateTime)) {
onStartDateChange(parsedDateTime);
const parsedNewStartDateTime = parseISO(newStartDateTime);
if (isValid(parsedNewStartDateTime)) {
onStartDateChange(parsedNewStartDateTime);
}
};

const onEndDateTimePickerChange = (dateTime) => {
setEndDateTime(dateTime);
const onEndDateTimePickerChange = (newEndDateTime) => {
setEndDateTime(newEndDateTime);

const parsedDateTime = parseISO(dateTime);
if (isValid(parsedDateTime)) {
onEndDateChange(parsedDateTime);
const parsedNewEndDateTime = parseISO(newEndDateTime);
if (isValid(parsedNewEndDateTime)) {
onEndDateChange(parsedNewEndDateTime);
}
};

Expand Down
8 changes: 4 additions & 4 deletions src/DateTimePicker/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import React, { memo, useImperativeHandle, useRef } from 'react';

import { getMaxDateAndTime, getMinDateAndTime } from './utils';
import { EMPTY_DATE_TIME_VALUE, getMaxDateAndTime, getMinDateAndTime } from './utils';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the EMPTY_DATE_TIME_VALUE constant to the utils file to match the other pickers.


import DatePicker, { EMPTY_DATE_VALUE } from '../DatePicker';
import DatePicker from '../DatePicker';
import TimePicker, { EMPTY_TIME_VALUE } from '../TimePicker';

import * as styles from './styles.module.scss';

export const EMPTY_DATE_TIME_VALUE = `${EMPTY_DATE_VALUE}T${EMPTY_TIME_VALUE}`;

const DateTimePicker = ({
className = '',
datePickerProps = {},
Expand Down Expand Up @@ -77,4 +75,6 @@ const DateTimePicker = ({
</div>;
};

export { EMPTY_DATE_TIME_VALUE };

export default memo(DateTimePicker);
5 changes: 5 additions & 0 deletions src/DateTimePicker/utils/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import { EMPTY_DATE_VALUE } from '../../DatePicker';
import { EMPTY_TIME_VALUE } from '../../TimePicker';

export const EMPTY_DATE_TIME_VALUE = `${EMPTY_DATE_VALUE}T${EMPTY_TIME_VALUE}`;

export const getMaxDateAndTime = (max, value) => {
const [dateValue] = value.split('T');
const [maxDate, maxTime] = max.split('T');
Expand Down
64 changes: 29 additions & 35 deletions src/PatrolDetailView/PlanSection/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
displayStartTimeForPatrol
} from '../../utils/patrols';
import { fetchTrackedBySchema } from '../../ducks/trackedby';
import { getHoursAndMinutesString, getTimezoneOffsetString } from '../../utils/datetime';
import { getHoursAndMinutesString } from '../../utils/datetime';
import { updateUserPreferences } from '../../ducks/user-preferences';
import { setMapLocationSelectionPatrol } from '../../ducks/map-ui';
import { useMatchMedia } from '../../hooks';
Expand Down Expand Up @@ -58,55 +58,49 @@ const PlanSection = ({
const [startDate, setStartDate] = useState(format(displayStartDate ?? new Date(), 'yyyy-MM-dd'));
const [startTime, setStartTime] = useState(getHoursAndMinutesString(displayStartDate));

const handleEndDateChange = useCallback((date) => {
setEndDate(date);
const handleEndDateChange = useCallback((newEndDate) => {
setEndDate(newEndDate);

let dateISO = `${date}T`;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file changes weren't really necessary, these pickers were working fine, but using the new approach is clearer, more concise, and I would even think stable since now we delegate the responsibility of calculating the offsets to date-fns.

dateISO += isValidTime(endTime) ? endTime : '00:00';
dateISO += `:00${getTimezoneOffsetString()}`;

const parsedDate = parseISO(dateISO);
if (isValid(parsedDate)) {
onPatrolEndDateChange(parsedDate, shouldScheduleDate(parsedDate, isAutoEnd));
const parsedNewEndDate = parseISO(`${newEndDate}T${isValidTime(endTime) ? endTime : '00:00'}`);
if (isValid(parsedNewEndDate)) {
onPatrolEndDateChange(parsedNewEndDate, shouldScheduleDate(parsedNewEndDate, isAutoEnd));
} else {
onPatrolEndDateChange(undefined);
}
}, [endTime, isAutoEnd, onPatrolEndDateChange]);

const handleStartDateChange = useCallback((date) => {
setStartDate(date);

let dateISO = `${date}T`;
dateISO += isValidTime(startTime) ? startTime : '00:00';
dateISO += `:00${getTimezoneOffsetString()}`;
const handleStartDateChange = useCallback((newStartDate) => {
setStartDate(newStartDate);

const parsedDate = parseISO(dateISO);
if (isValid(parsedDate)) {
onPatrolStartDateChange(parsedDate, shouldScheduleDate(parsedDate, isAutoStart));
const parsedNewStartDate = parseISO(`${newStartDate}T${isValidTime(startTime) ? startTime : '00:00'}`);
if (isValid(parsedNewStartDate)) {
onPatrolStartDateChange(parsedNewStartDate, shouldScheduleDate(parsedNewStartDate, isAutoStart));
} else {
onPatrolStartDateChange(undefined);
}
}, [isAutoStart, onPatrolStartDateChange, startTime]);

const handleEndTimeChange = useCallback((endTime) => {
setEndTime(endTime);

const newEndTimeParts = endTime.split(':');
const updatedEndDateTime = displayEndDate ? new Date(displayEndDate) : new Date();
updatedEndDateTime.setHours(newEndTimeParts[0], newEndTimeParts[1], '00');
const handleEndTimeChange = useCallback((newEndTime) => {
setEndTime(newEndTime);

onPatrolEndDateChange(updatedEndDateTime, shouldScheduleDate(updatedEndDateTime, isAutoEnd));
}, [displayEndDate, isAutoEnd, onPatrolEndDateChange]);

const handleStartTimeChange = useCallback((startTime) => {
setStartTime(startTime);
const parsedNewEndDate = parseISO(`${endDate}T${newEndTime}`);
if (isValid(parsedNewEndDate)) {
onPatrolEndDateChange(parsedNewEndDate, shouldScheduleDate(parsedNewEndDate, isAutoEnd));
} else {
onPatrolEndDateChange(undefined);
}
}, [endDate, isAutoEnd, onPatrolEndDateChange]);

const newStartTimeParts = startTime.split(':');
const updatedStartDateTime = displayStartDate ? new Date(displayStartDate) : new Date();
updatedStartDateTime.setHours(newStartTimeParts[0], newStartTimeParts[1], '00');
const handleStartTimeChange = useCallback((newStartTime) => {
setStartTime(newStartTime);

onPatrolStartDateChange(updatedStartDateTime, shouldScheduleDate(updatedStartDateTime, isAutoStart));
}, [displayStartDate, isAutoStart, onPatrolStartDateChange]);
const parsedNewStartDate = parseISO(`${startDate}T${newStartTime}`);
if (isValid(parsedNewStartDate)) {
onPatrolStartDateChange(parsedNewStartDate, shouldScheduleDate(parsedNewStartDate, isAutoStart));
} else {
onPatrolStartDateChange(undefined);
}
}, [isAutoStart, onPatrolStartDateChange, startDate]);

const handleAutoEndChange = useCallback(() => {
const newIsAutoEnd = !isAutoEnd;
Expand Down
19 changes: 10 additions & 9 deletions src/ReportManager/DetailsSection/SchemaForm/fields/DateTime/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ import React, { memo, useEffect, useState } from 'react';
import { format, isValid, parseISO } from 'date-fns';

import { DATE_TIME_ELEMENT_INPUT_TYPES } from '../../constants';
import { getTimezoneOffsetString } from '../../../../../utils/datetime';

import DatePicker, { EMPTY_DATE_VALUE } from '../../../../../DatePicker';
import DatePicker, { isValidDate, EMPTY_DATE_VALUE } from '../../../../../DatePicker';
import DateTimePicker, { EMPTY_DATE_TIME_VALUE } from '../../../../../DateTimePicker';
import TimePicker, { EMPTY_TIME_VALUE } from '../../../../../TimePicker';

Expand All @@ -26,8 +25,11 @@ const DateTimeInput = ({ onChange, value, ...otherProps }) => {
if (newDateTime === EMPTY_DATE_TIME_VALUE) {
onChange(undefined);
} else {
// JSON schema time format expects the time with the timezone offset.
const dateTimeWithSecondsAndOffset = `${newDateTime}:00${getTimezoneOffsetString()}`;
// JSON schema date time format expects the date time with seconds and timezone offset. If the new date is valid,
// consider the offset of that date (to match the daylight saving), otherwise consider the current date offset.
const [newDateValue] = newDateTime.split('T');
const newOffset = format(isValidDate(newDateValue) ? newDateValue : new Date(), 'xxx');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the actual fix for the new schemas component (EFB), this was not the reported issue since this is behind a feature flag, but the bug was here too. Instead of setting the user's current offset, we now calculate the offset corresponding to the selected date (considering daylight saving and avoiding that unwanted incorrect fixing).

const dateTimeWithSecondsAndOffset = `${newDateTime}:00${newOffset}`;
onChange(dateTimeWithSecondsAndOffset);
}
};
Expand All @@ -49,8 +51,8 @@ const TimeInput = ({ onChange, value, ...otherProps }) => {
if (newTime === EMPTY_TIME_VALUE) {
onChange(undefined);
} else {
// JSON schema time format expects the time with the timezone offset.
const timeWithSecondsAndOffset = `${newTime}:00${getTimezoneOffsetString()}`;
// JSON schema time format expects the time with seconds and timezone offset.
const timeWithSecondsAndOffset = `${newTime}:00${format(new Date(), 'xxx')}`;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSON schemas time formats need offsets too so the fix is also applied to time fields.

onChange(timeWithSecondsAndOffset);
}
};
Expand All @@ -76,7 +78,7 @@ const DateTime = ({ autofillDefaultInput: _autofillDefaultInput, details, error,
// Date-time and time input types have a timezone offset, so we correct the input value to the current user timezone
// before rendering it.
useEffect(() => {
if (!hasTimezoneBeenCorrected && value) {
if (value && !hasTimezoneBeenCorrected) {
if (details.inputType === DATE_TIME_ELEMENT_INPUT_TYPES.DATE_TIME) {
const parsedDateTimeValue = parseISO(value);
if (isValid(parsedDateTimeValue)) {
Expand All @@ -85,8 +87,7 @@ const DateTime = ({ autofillDefaultInput: _autofillDefaultInput, details, error,
}

if (details.inputType === DATE_TIME_ELEMENT_INPUT_TYPES.TIME) {
// We add a dummy date just to make it a valid ISO date
const parsedTimeValue = parseISO(`2000-01-01T${value}`);
const parsedTimeValue = parseISO(`${format(new Date(), 'yyyy-MM-dd')}T${value}`);
Copy link
Contributor Author

@luixlive luixlive Apr 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We used to fix the time value with a dummy date. I thought it wasn't really important, but it is, due to cities with daylight saving time. So now we use the current date to calculate the offset (in this case we do want the current date because this is the place where we fix the stored value into the current time zone offset).

if (isValid(parsedTimeValue)) {
onFieldChange(id, format(parsedTimeValue, 'HH:mm:ssXXX'));
}
Expand Down
6 changes: 3 additions & 3 deletions src/ReportManager/DetailsSection/SchemaForm/fields/DateTime/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { render, screen } from '../../../../../test-utils';
import { DATE_TIME_ELEMENT_INPUT_TYPES } from '../../constants';

import DateTime from './';
import { getTimezoneOffsetString } from '../../../../../utils/datetime';

const transformISOToCurrentTimezone = (dateValue) => format(parseISO(dateValue), 'yyyy-MM-dd\'T\'HH:mm:ssXXX');

Expand Down Expand Up @@ -138,12 +137,13 @@ describe('ReportManager - DetailsSection - SchemaForm - fields - DateTime', () =
await userEvent.click(screen.getByLabelText('Choose Monday, January 13th, 2020'));

expect(onFieldChange).toHaveBeenCalledTimes(2);
expect(onFieldChange).toHaveBeenCalledWith('date-time-1', `2020-01-13T06:30:00${getTimezoneOffsetString()}`);
expect(onFieldChange).toHaveBeenCalledWith('date-time-1', transformISOToCurrentTimezone('2020-01-13T06:30'));

await userEvent.click(screen.getByLabelText('Open time options'));
await userEvent.click(screen.getByText('08:00 AM'));

expect(onFieldChange).toHaveBeenCalledTimes(3);
expect(onFieldChange).toHaveBeenCalledWith('date-time-1', `2020-01-01T08:00:00${getTimezoneOffsetString()}`);
expect(onFieldChange.mock.calls[2][0]).toBe('date-time-1');
expect(onFieldChange).toHaveBeenCalledWith('date-time-1', transformISOToCurrentTimezone('2020-01-01T08:00'));
});
});
24 changes: 9 additions & 15 deletions src/ReportManager/DetailsSection/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { memo, useCallback, useContext, useEffect, useState } from 'react';
import Dropdown from 'react-bootstrap/Dropdown';
import Form from '@rjsf/bootstrap-4';
import { format, isToday, isValid as isValidDate, parseISO } from 'date-fns';
import { format, isToday, isValid, parseISO } from 'date-fns';
import MoonLoader from 'react-spinners/MoonLoader';
import { useDispatch, useSelector } from 'react-redux';
import { useTranslation } from 'react-i18next';
Expand Down Expand Up @@ -106,13 +106,9 @@ const DetailsSection = ({
const onDatePickerChange = (newDate) => {
setDate(newDate);

const parsedDate = parseISO(newDate);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another place were the changes weren't really needed but the new pattern improves the code and it makes it more resilient.

if (isValidDate(parsedDate)) {
if (isValidTime(time)) {
const [hour, minute] = time.split(':');
parsedDate.setHours(hour, minute, '00');
}
onReportDateChange(parsedDate);
const parsedNewDate = parseISO(`${newDate}T${isValidTime(time) ? time : '00:00'}`);
if (isValid(parsedNewDate)) {
onReportDateChange(parsedNewDate);
} else {
onReportDateChange(undefined);
}
Expand All @@ -123,13 +119,11 @@ const DetailsSection = ({
const onTimePickerChange = (newTime) => {
setTime(newTime);

const parsedDate = parseISO(date);
if (isValidDate(parsedDate)) {
if (isValidTime(newTime)) {
const [newHour, newMinute] = newTime.split(':');
parsedDate.setHours(newHour, newMinute, '00');
}
onReportDateChange(parsedDate);
const parsedNewDate = parseISO(`${date}T${newTime}`);
if (isValid(parsedNewDate)) {
onReportDateChange(parsedNewDate);
} else {
onReportDateChange(undefined);
}

reportTracker.track('Change Report Time');
Expand Down
Loading