Skip to content
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.

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

Fix datetimepicker erroneously updating the date field (bsc#1210253, bsc#1215820) #7611

Merged
merged 1 commit into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ button.is-plain {
-webkit-appearance: none;
-moz-appearance: none;
appearance: none;
background: none;
border: none;
border-radius: 0;
box-sizing: border-box;
Expand Down
1 change: 1 addition & 0 deletions web/html/src/branding/css/uyuni/buttons.less
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ button.is-plain {
-webkit-appearance: none;
-moz-appearance: none;
appearance: none;
background: none;
border: none;
border-radius: 0;
box-sizing: border-box;
Expand Down
92 changes: 92 additions & 0 deletions web/html/src/components/datetime/DateTimePicker.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import * as React from "react";
import { useState } from "react";

import { localizedMoment } from "utils";
import { render, screen, type } from "utils/test-utils";

import { DateTimePicker } from "./DateTimePicker";

describe("DateTimePicker", () => {
const getInputs = () => {
const datePicker = screen.getByTestId<HTMLInputElement>("date-picker");
const timePicker = screen.getByTestId<HTMLInputElement>("time-picker");
return { datePicker, timePicker };
};

test("prop value is rendered in the user's timezone", () => {
const validISOString = "2020-02-01T04:00:00.000Z";
const Setup = () => {
const [value, setValue] = useState(localizedMoment(validISOString));
return <DateTimePicker value={value} onChange={setValue} />;
};
render(<Setup />);

const { datePicker, timePicker } = getInputs();
// These values need to be offset by the user's timezone
expect(datePicker.value).toEqual("2020-01-31");
expect(timePicker.value).toEqual("20:00");
});

test("picking a date and time uses the user's timezone", (done) => {
const validISOString = "2020-02-01T04:00:00.000Z";
let changeEventCount = 0;

const Setup = () => {
const [value, setValue] = useState(localizedMoment(validISOString));
return (
<DateTimePicker
value={value}
onChange={(newValue) => {
changeEventCount += 1;
setValue(newValue);

if (changeEventCount === 2) {
// Note that this value should be in a different day due to the timezone offset
expect(newValue.toISOString()).toEqual("2020-01-17T07:30:00.000Z");
done();
}
}}
/>
);
};
render(<Setup />);

const { datePicker, timePicker } = getInputs();
datePicker.click();
screen.getByText("16").click();
timePicker.click();
screen.getByText("23:30").click();

expect(datePicker.value).toEqual("2020-01-16");
expect(timePicker.value).toEqual("23:30");
});

test("clearing the time input doesn't change the date (bsc#1210253)", (done) => {
const validISOString = "2020-01-30T15:00:00.000Z";
let changeEventCount = 0;

const Setup = () => {
const [value, setValue] = useState(localizedMoment(validISOString));
return (
<DateTimePicker
value={value}
onChange={(newValue) => {
changeEventCount += 1;
setValue(newValue);

if (changeEventCount === 2) {
expect(newValue.toUserDateTimeString()).toEqual("2020-01-16 00:00");
done();
}
}}
/>
);
};
render(<Setup />);

const { datePicker, timePicker } = getInputs();
datePicker.click();
screen.getByText("16").click();
type(timePicker, "0", false);
});
});
29 changes: 20 additions & 9 deletions web/html/src/components/datetime/DateTimePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ export const DateTimePicker = (props: Props) => {
timePickerRef.current?.setOpen(true);
};

const onChange = (newDateValue: Date | null) => {
const onChange = (date: Date | null) => {
// Currently we don't support propagating null values, we might want to do this in the future
if (newDateValue === null) {
if (date === null) {
return;
}
// The date we get here is now in the browsers local timezone but with values that should be reinterpreted
Expand All @@ -76,7 +76,7 @@ export const DateTimePicker = (props: Props) => {
localizedMoment(
// We first clone the date again to not modify the original. This has the unintended side effect of converting to
// UTC and adjusting the values.
localizedMoment(newDateValue)
localizedMoment(date)
// To get back to the values we want we just convert back to the browsers local timezone as it was before.
.local()
// Then we set the timezone of the date to the users configured timezone without adjusting the values.
Expand Down Expand Up @@ -144,6 +144,7 @@ export const DateTimePicker = (props: Props) => {
className="form-control no-right-border"
// This is used by Cucumber to interact with the component
data-testid="date-picker"
maxLength={10}
/>
}
previousMonthAriaLabel={previousMonth}
Expand Down Expand Up @@ -172,7 +173,18 @@ export const DateTimePicker = (props: Props) => {
portalId="time-picker-portal"
ref={timePickerRef}
selected={browserTimezoneValue.toDate()}
onChange={onChange}
onChange={(date) => {
if (date === null) {
return;
}
/**
* NB! Only take the hours and minutes from this change event since react-datepicker updates the date
* value when it should only update the time value (bsc#1202991, bsc#1215820)
*/
const mergedDate = browserTimezoneValue.toDate();
mergedDate.setHours(date.getHours(), date.getMinutes());
onChange(mergedDate);
}}
showTimeSelect
showTimeSelectOnly
// We want the regular primary display to only show the time here, so using TIME_FORMAT is intentional
Expand All @@ -188,22 +200,21 @@ export const DateTimePicker = (props: Props) => {
className="form-control"
// This is used by Cucumber to interact with the component
data-testid="time-picker"
maxLength={5}
/>
}
/>
</>
)}
<span className="input-group-addon" key="tz">
{props.serverTimeZone ? localizedMoment.serverTimeZoneAbbr : localizedMoment.userTimeZoneAbbr}
{timeZone}
</span>
</div>
{process.env.NODE_ENV !== "production" && SHOW_DEBUG_VALUES ? (
<pre>
user:{" "}
{props.value.toUserString()}
<br />
server: {props.value.toServerString()}
<br />
{props.value.toUserDateTimeString()} ({localizedMoment.userTimeZone})<br />
server: {props.value.toServerDateTimeString()} ({localizedMoment.serverTimeZone})<br />
iso:{" "}
{props.value.toISOString()}
</pre>
Expand Down
10 changes: 0 additions & 10 deletions web/html/src/utils/test-utils/mock.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,3 @@
// Mock the datetime picker to avoid it causing issues due to missing jQuery/Bootstrap parts
jest.mock("components/datetime/DateTimePicker", () => {
return {
__esModule: true,
DateTimePicker: () => {
return <div>DateTimePicker mockup</div>;
},
};
});

// Mock the ACE Editor to avoid it causing issues due to the missing proper library import
jest.mock("components/ace-editor", () => {
return {
Expand Down
32 changes: 30 additions & 2 deletions web/html/src/utils/test-utils/screen.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getDefaultNormalizer, Screen, screen as rawScreen } from "@testing-library/react";
import { getDefaultNormalizer, queryHelpers, Screen, screen as rawScreen } from "@testing-library/react";

// Utility type, if a function TargetFunction returns a Promise, return an intersection with Promise<T>, otherwise with T
type ReturnFromWith<
Expand Down Expand Up @@ -34,14 +34,42 @@ const labelNormalizer = (input: string) => {
// Override `screen.getByLabelText`
const getByLabelText = rawScreen.getByLabelText;
type GetByLabelTextArgs = Parameters<typeof rawScreen.getByLabelText>;

// Helpers for querying by testid so we use similar selectors to Cucumber, see https://testing-library.com/docs/dom-testing-library/api-custom-queries/
const queryByTestId = queryHelpers.queryByAttribute.bind(null, "data-testid");
const queryAllByTestId = queryHelpers.queryAllByAttribute.bind(null, "data-testid");

function getByTestId(...[container, id, ...rest]: Parameters<typeof getAllByTestId>) {
const result = getAllByTestId(container, id, ...rest);
if (result.length > 1) {
throw queryHelpers.getElementError(`Found multiple elements with the [data-testid="${id}"]`, container);
}
return result[0];
}
function getAllByTestId(...[container, id, ...rest]: Parameters<typeof queryAllByTestId>) {
const els = queryAllByTestId(container, id, ...rest);
if (!els.length) {
throw queryHelpers.getElementError(`Unable to find an element by: [data-testid="${id}"]`, container);
}
return els;
}

const additionalQueries = {
queryByTestId,
queryAllByTestId,
getByTestId,
getAllByTestId,
};

Object.assign(rawScreen, {
getByLabelText: (...[text, options, waitForElementOptions]: GetByLabelTextArgs) => {
options ??= {};
options.normalizer ??= labelNormalizer;
return getByLabelText(text, options, waitForElementOptions);
},
additionalQueries,
} as Partial<Screen>);

const screen = rawScreen as GenericScreen;
const screen = rawScreen as GenericScreen & typeof additionalQueries;

export { screen };
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix datetimepicker erroneously updating the date field (bsc#1210253, bsc#1215820)
Loading