From 35c2f217b0ea476512dcfed1031eda55671bd9c2 Mon Sep 17 00:00:00 2001 From: Julien Mougenot Date: Thu, 23 Jun 2022 06:50:04 +0000 Subject: [PATCH] [FIX] web: parseDate(Time): can handle short notations In the wowl codebase, we use luxon instead of moment to manipulate dates. Moment was able to parse dates with short notations (e.g. 08/15/22 for the format MM/DD/YYYY). Luxon is more strict and does not support those cases (the format must be perfectly respected). As a consequence, moving from moment to luxon would introduce a regression in our date and datetime fields. This commit implements the same kind of logic as what is done in moment, in our date parsers, such that those cases are handled. Part-of: odoo/odoo#92475 --- .../static/src/core/datepicker/datepicker.js | 149 +++++++++--------- .../static/src/core/datepicker/datepicker.xml | 9 +- addons/web/static/src/core/l10n/dates.js | 113 ++++++++----- .../web/static/tests/core/datepicker_tests.js | 41 ++++- .../web/static/tests/core/l10n/dates_tests.js | 34 ++++ 5 files changed, 227 insertions(+), 119 deletions(-) diff --git a/addons/web/static/src/core/datepicker/datepicker.js b/addons/web/static/src/core/datepicker/datepicker.js index e24b2e5f44a..48cc42b07f9 100644 --- a/addons/web/static/src/core/datepicker/datepicker.js +++ b/addons/web/static/src/core/datepicker/datepicker.js @@ -1,7 +1,14 @@ /** @odoo-module **/ +import { + areDateEquals, + formatDate, + formatDateTime, + parseDate, + parseDateTime, +} from "@web/core/l10n/dates"; +import { getActiveHotkey } from "@web/core/hotkeys/hotkey_service"; import { localization } from "@web/core/l10n/localization"; -import { registry } from "@web/core/registry"; import { useAutofocus } from "@web/core/utils/hooks"; const { @@ -15,38 +22,47 @@ const { } = owl; const { DateTime } = luxon; -const formatters = registry.category("formatters"); -const parsers = registry.category("parsers"); - let datePickerId = 0; /** * @param {DateTime} date * @returns {moment} */ -const luxonDateToMomentDate = (date) => { +function luxonDateToMomentDate(date) { return window.moment(String(date)); -}; +} /** * @param {string} format * @returns {string} */ -const luxonFormatToMomentFormat = (format) => { +function luxonFormatToMomentFormat(format) { return format.replace(/[dy]/g, (x) => x.toUpperCase()); -}; +} /** * @param {string} format * @returns {boolean} */ -const isValidStaticFormat = (format) => { - try { - return /^[\d\s/:-]+$/.test(DateTime.local().toFormat(format)); - } catch (_err) { - return false; - } -}; +function isValidStaticFormat(format) { + return /^[\d\s/:-]+$/.test(DateTime.local().toFormat(format)); +} + +/** + * @param {Function} fn + * @returns {[any, null] | [null, Error]} + */ +function wrapError(fn) { + return (...args) => { + const result = [null, null]; + try { + result[0] = fn(...args); + } catch (_err) { + result[1] = _err; + } + return result; + }; +} /** * Date picker @@ -63,7 +79,7 @@ const isValidStaticFormat = (format) => { */ export class DatePicker extends Component { setup() { - this.root = useRef("root"); + this.rootRef = useRef("root"); this.inputRef = useRef("input"); this.state = useState({ warning: false }); @@ -73,8 +89,7 @@ export class DatePicker extends Component { this.datePickerShown = false; this.initFormat(); - this.setDate(this.props); - this.setFormat(this.props); + this.setDateAndFormat(this.props); useAutofocus(); useExternalListener(window, "scroll", this.onWindowScroll); @@ -88,43 +103,33 @@ export class DatePicker extends Component { this.bootstrapDateTimePicker(this.props); this.updateInput(); - window.$(this.root.el).on("show.datetimepicker", () => { + window.$(this.rootRef.el).on("show.datetimepicker", () => { this.datePickerShown = true; this.inputRef.el.select(); }); - window.$(this.root.el).on("hide.datetimepicker", () => { + window.$(this.rootRef.el).on("hide.datetimepicker", () => { this.datePickerShown = false; this.onDateChange({ useStatic: true }); }); - window.$(this.root.el).on("error.datetimepicker", () => false); + window.$(this.rootRef.el).on("error.datetimepicker", () => false); } onWillUpdateProps(nextProps) { const pickerParams = {}; - let shouldUpdateInput = false; for (const prop in nextProps) { - const prev = this.props[prop]; - const next = nextProps[prop]; - if ((prev instanceof DateTime && !prev.equals(next)) || prev !== next) { + if (!areDateEquals(this.props[prop], nextProps[prop])) { pickerParams[prop] = nextProps[prop]; - if (prop === "date") { - this.setDate(nextProps); - shouldUpdateInput = true; - } else if (prop === "format") { - this.setFormat(nextProps); - shouldUpdateInput = true; - } } } - if (shouldUpdateInput) { + this.setDateAndFormat(nextProps); + if ("date" in pickerParams || "format" in pickerParams) { this.updateInput(); } this.bootstrapDateTimePicker(pickerParams); } onWillUnmount() { - window.$(this.root.el).off(); // Removes all jQuery events - + window.$(this.rootRef.el).off(); // Removes all jQuery events this.bootstrapDateTimePicker("destroy"); } @@ -136,7 +141,7 @@ export class DatePicker extends Component { return { format: !useStatic || isValidStaticFormat(this.format) ? this.format : this.staticFormat, - locale: this.date.locale, + locale: this.props.locale || (this.date && this.date.locale), timezone: this.isLocal, }; } @@ -147,8 +152,8 @@ export class DatePicker extends Component { initFormat() { this.defaultFormat = localization.dateFormat; this.staticFormat = "yyyy/MM/dd"; - this.formatValue = formatters.get("date"); - this.parseValue = parsers.get("date"); + this.formatValue = wrapError(formatDate); + this.parseValue = wrapError(parseDate); this.isLocal = false; } @@ -158,17 +163,10 @@ export class DatePicker extends Component { * @param {Object} params * @param {DateTime} params.date * @param {string} [params.locale] - */ - setDate({ date, locale }) { - this.date = locale ? date.setLocale(locale) : date; - } - - /** - * Sets the current format. - * @param {Object} params * @param {string} [params.format] */ - setFormat({ format }) { + setDateAndFormat({ date, locale, format }) { + this.date = date && locale ? date.setLocale(locale) : date; // Fallback to default localization format in `@web/core/l10n/dates.js`. this.format = format || this.defaultFormat; } @@ -179,10 +177,9 @@ export class DatePicker extends Component { * @param {boolean} [params.useStatic] */ updateInput({ useStatic } = {}) { - try { - this.inputRef.el.value = this.formatValue(this.date, this.getOptions(useStatic)); - } catch (_err) { - // Do nothing + const [formattedDate] = this.formatValue(this.date, this.getOptions(useStatic)); + if (formattedDate) { + this.inputRef.el.value = formattedDate; } } @@ -197,20 +194,20 @@ export class DatePicker extends Component { bootstrapDateTimePicker(commandOrParams) { if (typeof commandOrParams === "object") { const format = isValidStaticFormat(this.format) ? this.format : this.staticFormat; - const params = { ...commandOrParams, format: luxonFormatToMomentFormat(format) }; - if (!params.locale && commandOrParams.date) { - params.locale = commandOrParams.date.locale; - } + const params = { + ...commandOrParams, + date: this.date || null, + format: luxonFormatToMomentFormat(format), + locale: commandOrParams.locale || (this.date && this.date.locale), + }; for (const prop in params) { if (params[prop] instanceof DateTime) { - const luxonDate = params[prop]; - const momentDate = luxonDateToMomentDate(luxonDate); - params[prop] = momentDate; + params[prop] = luxonDateToMomentDate(params[prop]); } } commandOrParams = params; } - return window.$(this.root.el).datetimepicker(commandOrParams); + return window.$(this.rootRef.el).datetimepicker(commandOrParams); } //--------------------------------------------------------------------- @@ -232,17 +229,26 @@ export class DatePicker extends Component { * @param {boolean} [params.useStatic] */ onDateChange({ useStatic } = {}) { - let date; - try { - date = this.parseValue(this.inputRef.el.value, this.getOptions(useStatic)); - } catch (_err) { + const { value } = this.inputRef.el; + const options = this.getOptions(useStatic); + const parsedDate = value && this.parseValue(value, options)[0]; + this.state.warning = parsedDate && parsedDate > DateTime.local(); + if (value && !parsedDate) { // Reset to default (= given) date. - } - if (!date || date.equals(this.date)) { this.updateInput(); - } else { - this.state.warning = date > DateTime.local(); - this.props.onDateTimeChanged(date); + } + if (!areDateEquals(this.date, parsedDate)) { + this.props.onDateTimeChanged(parsedDate); + } + } + + onInputKeydown(ev) { + const hotkey = getActiveHotkey(ev); + if (this.datePickerShown && hotkey === "escape") { + this.bootstrapDateTimePicker("hide"); + this.inputRef.el.select(); + ev.preventDefault(); + ev.stopPropagation(); } } @@ -277,7 +283,7 @@ DatePicker.defaultProps = { DatePicker.props = { // Components props onDateTimeChanged: Function, - date: DateTime, + date: { type: [DateTime, { value: false }], optional: true }, warn_future: { type: Boolean, optional: true }, // Bootstrap datepicker options buttons: { @@ -313,6 +319,7 @@ DatePicker.props = { readonly: { type: Boolean, optional: true }, useCurrent: { type: Boolean, optional: true }, widgetParent: { type: String, optional: true }, + daysOfWeekDisabled: { type: Array, optional: true }, }; DatePicker.template = "web.DatePicker"; @@ -333,8 +340,8 @@ export class DateTimePicker extends DatePicker { initFormat() { this.defaultFormat = localization.dateTimeFormat; this.staticFormat = "yyyy/MM/dd HH:mm:ss"; - this.formatValue = formatters.get("datetime"); - this.parseValue = parsers.get("datetime"); + this.formatValue = wrapError(formatDateTime); + this.parseValue = wrapError(parseDateTime); this.isLocal = true; } } diff --git a/addons/web/static/src/core/datepicker/datepicker.xml b/addons/web/static/src/core/datepicker/datepicker.xml index 3278d0c3705..66665ca7c52 100644 --- a/addons/web/static/src/core/datepicker/datepicker.xml +++ b/addons/web/static/src/core/datepicker/datepicker.xml @@ -13,10 +13,13 @@ t-ref="input" t-on-change="() => this.onDateChange()" t-on-click="onInputClick" + t-on-keydown="onInputKeydown" + /> + - - This date is on the future. Make sure it is what you expected. - diff --git a/addons/web/static/src/core/l10n/dates.js b/addons/web/static/src/core/l10n/dates.js index a6ace94719a..8a737a2e4d8 100644 --- a/addons/web/static/src/core/l10n/dates.js +++ b/addons/web/static/src/core/l10n/dates.js @@ -51,6 +51,15 @@ const smartDateUnits = { }; const smartDateRegex = new RegExp(`^([+-])(\\d+)([${Object.keys(smartDateUnits).join("")}]?)$`); +/** + * @param {any} d1 + * @param {any} d2 + * @returns {boolean} + */ +export function areDateEquals(d1, d2) { + return d1 instanceof DateTime && d2 instanceof DateTime ? d1.equals(d2) : d1 === d2; +} + /** * Smart date inputs are shortcuts to write dates quicker. * These shortcuts should respect the format ^[+-]\d+[dmwy]?$ @@ -86,14 +95,10 @@ function parseSmartDateInput(value) { * Returns false otherwise. * * @param {DateTime | false} date - * @returns {DateTime | false} + * @returns {boolean} */ -function constrain(dt) { - let valid = dt !== false; - valid = valid && dt.isValid; - valid = valid && dt.year >= 1000; - valid = valid && dt.year < 10000; - return valid ? dt : false; +function isValidDateTime(dt) { + return dt && dt.isValid && dt.year >= 1000 && dt.year < 10000; } /** @@ -187,7 +192,7 @@ export function formatDateTime(value, options = {}) { return ""; } const format = options.format || localization.dateTimeFormat; - const numberingSystem = options.numberingSystem || Settings.defaultNumberingSystem; + const numberingSystem = options.numberingSystem || Settings.defaultNumberingSystem || "latn"; const zone = options.timezone ? "system" : "utc"; value = value.setZone(zone, { keepLocaltime: options.timezone }); return value.toFormat(format, { numberingSystem }); @@ -208,6 +213,9 @@ export function formatDateTime(value, options = {}) { * @returns {DateTime | false} Luxon DateTime object */ export function parseDate(value, options = {}) { + if (!value) { + return false; + } return parseDateTime(value, { timezone: false, ...options }).startOf("day"); } @@ -256,46 +264,75 @@ export function parseDateTime(value, options = {}) { return false; } - const valueDigitsOnly = value.replace(nonDigitsRegex, ""); + const fmt = options.format || localization.dateTimeFormat; const parseOpts = { setZone: true, zone: options.timezone ? "system" : "utc", locale: options.locale, - numberingSystem: options.numberingSystem || Settings.defaultNumberingSystem, + numberingSystem: options.numberingSystem || Settings.defaultNumberingSystem || "latn", }; - let result = constrain(parseSmartDateInput(value)); + // Base case: try parsing with the given format and options + let result = DateTime.fromFormat(value, fmt, parseOpts); - if (!result) { - const fmt = options.format || localization.dateTimeFormat; + // Try parsing as a smart date + if (!isValidDateTime(result)) { + result = parseSmartDateInput(value); + } + + // Try parsing with partial date parts + if (!isValidDateTime(result)) { const fmtWoZero = stripAlphaDupes(fmt); + result = DateTime.fromFormat(value, fmtWoZero, parseOpts); + } + // Try parsing with custom shorthand date parts + if (!isValidDateTime(result)) { // Luxon is not permissive regarding delimiting characters in the format. // So if the value to parse has less characters than the format, we would // try to parse without the delimiting characters. - const woSeps = { - val: valueDigitsOnly, - fmt: fmt.replace(nonAlphaRegex, "").slice(0, valueDigitsOnly.length), - }; + const digitList = value.split(nonDigitsRegex).filter(Boolean); + const fmtList = fmt.split(nonAlphaRegex).filter(Boolean); + const valWoSeps = digitList.join(""); - result = - constrain(DateTime.fromFormat(value, fmt, parseOpts)) || - constrain(DateTime.fromFormat(value, fmtWoZero, parseOpts)) || - constrain(DateTime.fromFormat(woSeps.val, woSeps.fmt, parseOpts)); + // This is the weird part: we try to adapt the given format to comply with + // the amount of digits in the given value. To do this we split the format + // and the value on non-letter and non-digit characters respectively. This + // should create the same amount of grouping parameters, and the format + // groups are trimmed according to the length of their corresponding + // digit group. The 'carry' variable allows for the length of a digit + // group to overflow to the next format group. This is typically the case + // when the given value doesn't have non-digit separators and generates + // one big digit group instead. + let carry = 0; + const fmtWoSeps = fmtList + .map((part, i) => { + const digitLength = (digitList[i] || "").length; + const actualPart = part.slice(0, digitLength + carry); + carry += digitLength - actualPart.length; + return actualPart; + }) + .join(""); + + result = DateTime.fromFormat(valWoSeps, fmtWoSeps, parseOpts); } - if (!result) { - if (valueDigitsOnly.length > 4) { - // Also try some fallback formats, but only if value counts more than - // four digit characters as this could get misinterpreted as the time of - // the actual date. - result = - constrain(DateTime.fromISO(value, parseOpts)) || // ISO8601 - constrain(DateTime.fromSQL(value, parseOpts)); // last try: SQL + // Try with defaul ISO or SQL formats + if (!isValidDateTime(result)) { + // Also try some fallback formats, but only if value counts more than + // four digit characters as this could get misinterpreted as the time of + // the actual date. + const valueDigits = value.replace(nonDigitsRegex, ""); + if (valueDigits.length > 4) { + result = DateTime.fromISO(value, parseOpts); // ISO8601 + if (!isValidDateTime(result)) { + result = DateTime.fromSQL(value, parseOpts); // last try: SQL + } } } - if (!result) { + // No working parsing methods: throw an error + if (!isValidDateTime(result)) { throw new Error(sprintf(_t("'%s' is not a correct date or datetime"), value)); } @@ -307,45 +344,45 @@ export function parseDateTime(value, options = {}) { * @param {string} value * @returns {DateTime | false} */ -export const deserializeDate = (value) => { +export function deserializeDate(value) { return parseDate(value, { format: SERVER_DATE_FORMAT, numberingSystem: "latn", }); -}; +} /** * Returns a datetime object parsed from the given serialized string. * @param {string} value * @returns {DateTime | false} */ -export const deserializeDateTime = (value) => { +export function deserializeDateTime(value) { return parseDateTime(value, { format: `${SERVER_DATE_FORMAT} ${SERVER_TIME_FORMAT}`, numberingSystem: "latn", }); -}; +} /** * Returns a serialized string representing the given date. * @param {DateTime} value * @returns {string} */ -export const serializeDate = (value) => { +export function serializeDate(value) { return formatDate(value, { format: SERVER_DATE_FORMAT, numberingSystem: "latn", }); -}; +} /** * Returns a serialized string representing the given datetime. * @param {DateTime} value * @returns {string} */ -export const serializeDateTime = (value) => { +export function serializeDateTime(value) { return formatDateTime(value, { format: `${SERVER_DATE_FORMAT} ${SERVER_TIME_FORMAT}`, numberingSystem: "latn", }); -}; +} diff --git a/addons/web/static/tests/core/datepicker_tests.js b/addons/web/static/tests/core/datepicker_tests.js index ce3d76cb1b5..ce72f2a32a6 100644 --- a/addons/web/static/tests/core/datepicker_tests.js +++ b/addons/web/static/tests/core/datepicker_tests.js @@ -4,6 +4,7 @@ import { applyFilter, toggleMenu } from "@web/../tests/search/helpers"; import { DatePicker, DateTimePicker } from "@web/core/datepicker/datepicker"; import { registry } from "@web/core/registry"; import { uiService } from "@web/core/ui/ui_service"; +import { hotkeyService } from "@web/core/hotkeys/hotkey_service"; import ActionModel from "web.ActionModel"; import CustomFilterItem from "web.CustomFilterItem"; import { createComponent } from "web.test_utils"; @@ -25,7 +26,7 @@ let target; * @param {Object} props * @returns {Promise} */ -const mountPicker = async (Picker, props) => { +async function mountPicker(Picker, props) { serviceRegistry .add( "localization", @@ -34,7 +35,8 @@ const mountPicker = async (Picker, props) => { dateTimeFormat: "dd/MM/yyyy HH:mm:ss", }) ) - .add("ui", uiService); + .add("ui", uiService) + .add("hotkey", hotkeyService); class Parent extends Component { setup() { @@ -54,9 +56,9 @@ const mountPicker = async (Picker, props) => { const env = await makeTestEnv(); await mount(Parent, target, { env, props: { Picker } }); -}; +} -const useFRLocale = () => { +function useFRLocale() { if (!window.moment.locales().includes("fr")) { // Mocks the FR locale if not loaded const originalLocale = window.moment.locale(); @@ -76,9 +78,9 @@ const useFRLocale = () => { registerCleanup(() => window.moment.updateLocale("fr", null)); } return "fr"; -}; +} -const useNOLocale = () => { +function useNOLocale() { if (!window.moment.locales().includes("nb")) { const originalLocale = window.moment.locale(); window.moment.defineLocale("nb", { @@ -97,7 +99,7 @@ const useNOLocale = () => { registerCleanup(() => window.moment.updateLocale("nb", null)); } return "nb"; -}; +} QUnit.module("Components", ({ beforeEach }) => { beforeEach(() => { @@ -527,4 +529,29 @@ QUnit.module("Components", ({ beforeEach }) => { await editSelect(valueInput, "05/05/2005"); await applyFilter(target); }); + + QUnit.test("start with no value", async function (assert) { + assert.expect(6); + + await mountPicker(DateTimePicker, { + onDateTimeChanged(date) { + assert.step("datetime-changed"); + assert.strictEqual( + date.toFormat("dd/MM/yyyy HH:mm:ss"), + "08/02/1997 15:45:05", + "Event should transmit the correct date" + ); + }, + }); + + const input = target.querySelector(".o_datepicker_input"); + assert.strictEqual(input.value, ""); + + assert.verifySteps([]); + input.value = "08/02/1997 15:45:05"; + await triggerEvent(target, ".o_datepicker_input", "change"); + + assert.verifySteps(["datetime-changed"]); + assert.strictEqual(input.value, "08/02/1997 15:45:05"); + }); }); diff --git a/addons/web/static/tests/core/l10n/dates_tests.js b/addons/web/static/tests/core/l10n/dates_tests.js index 0b40a6de5fb..866f60fff96 100644 --- a/addons/web/static/tests/core/l10n/dates_tests.js +++ b/addons/web/static/tests/core/l10n/dates_tests.js @@ -664,6 +664,33 @@ QUnit.module( }); }); + QUnit.test("parseDate with short notations", async (assert) => { + assert.strictEqual( + parseDate("20-10-20", { format: "yyyy-MM-dd" }).toISO(), + "2020-10-20T00:00:00.000Z" + ); + assert.strictEqual( + parseDate("20/10/20", { format: "yyyy/MM/dd" }).toISO(), + "2020-10-20T00:00:00.000Z" + ); + assert.strictEqual( + parseDate("10-20-20", { format: "MM-dd-yyyy" }).toISO(), + "2020-10-20T00:00:00.000Z" + ); + assert.strictEqual( + parseDate("10-20-20", { format: "MM-yyyy-dd" }).toISO(), + "2020-10-20T00:00:00.000Z" + ); + assert.strictEqual( + parseDate("1-20-2", { format: "MM-yyyy-dd" }).toISO(), + "2020-01-02T00:00:00.000Z" + ); + assert.strictEqual( + parseDate("20/1/2", { format: "yyyy/MM/dd" }).toISO(), + "2020-01-02T00:00:00.000Z" + ); + }); + QUnit.test("parseDateTime", async (assert) => { /** * Type of testSet key: string @@ -756,6 +783,13 @@ QUnit.module( }); }); + QUnit.test("parseDateTime with short notations", async (assert) => { + assert.strictEqual( + parseDateTime("20-10-20 8:5:3", { format: "yyyy-MM-dd hh:mm:ss" }).toISO(), + "2020-10-20T08:05:03.000Z" + ); + }); + QUnit.test("parseDateTime (with legacy options.isUTC = true)", async (assert) => { /** * Type of testSet key: string