From 4c7710ece0d5cdfb3b196b06a396f2e760460ef9 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 2 Mar 2023 11:45:32 -0600 Subject: [PATCH] fix: Added date time parsing for conditional formatting (#1120) Added datetime string parsing / validation. fixes #1108 --- __mocks__/dh-core.js | 5 ++ .../ConditionalFormattingUtils.test.ts | 65 ++++++++++++------- .../ConditionalFormattingUtils.ts | 27 ++++++-- 3 files changed, 68 insertions(+), 29 deletions(-) diff --git a/__mocks__/dh-core.js b/__mocks__/dh-core.js index 45cd0279fd..2f748b908e 100644 --- a/__mocks__/dh-core.js +++ b/__mocks__/dh-core.js @@ -1750,6 +1750,11 @@ class DateWrapper extends LongWrapper { class TimeZone { static getTimeZone(id) { + if (id == null || id === '' || id.includes(' ')) { + // Usually there would be a java.lang.IllegalArgumentException for any invalid id. + // We at least know that '' and undefined, so throw an error. + throw new Error('Unsupported time zone'); + } return { id }; } } diff --git a/packages/iris-grid/src/sidebar/conditional-formatting/ConditionalFormattingUtils.test.ts b/packages/iris-grid/src/sidebar/conditional-formatting/ConditionalFormattingUtils.test.ts index f472467ad7..787be57c20 100644 --- a/packages/iris-grid/src/sidebar/conditional-formatting/ConditionalFormattingUtils.test.ts +++ b/packages/iris-grid/src/sidebar/conditional-formatting/ConditionalFormattingUtils.test.ts @@ -220,43 +220,58 @@ describe('getFormatColumns', () => { describe('isDateConditionValid', () => { const values = { - valid: '2023-02-23T11:46:31.000000000 NY', - invalid: 'blah', + valid: [ + '2023-02-23T11:46:31.000000000 NY', + '2023-02-23T00:00:00 NY', + '2023-02-23 NY', + ], + invalid: ['blah', '2023-02-23', '2023-02-23T00:00:00 NY blah'], empty: '', undefined, }; - it.each([DateCondition.IS_NULL, DateCondition.IS_NOT_NULL])( - 'should return true for null check conditions: %s', + const conditions = { + valueNotRequired: [DateCondition.IS_NULL, DateCondition.IS_NOT_NULL], + valueRequired: [ + DateCondition.IS_AFTER, + DateCondition.IS_AFTER_OR_EQUAL, + DateCondition.IS_BEFORE_OR_EQUAL, + DateCondition.IS_BEFORE, + DateCondition.IS_EXACTLY, + DateCondition.IS_NOT_EXACTLY, + ], + }; + + describe.each(conditions.valueNotRequired)( + 'Not-Required condition: %s', condition => { - const testValues = [ - values.valid, - values.invalid, + it.each([ + ...values.valid, + ...values.invalid, values.empty, values.undefined, - ]; - - testValues.forEach(value => { - expect(isDateConditionValid(condition, value)).toBeTruthy(); + ])('should ignore value when not required: %s', testValue => { + expect(isDateConditionValid(condition, testValue)).toBeTruthy(); }); } ); - it.each([ - DateCondition.IS_AFTER, - DateCondition.IS_AFTER_OR_EQUAL, - DateCondition.IS_BEFORE_OR_EQUAL, - DateCondition.IS_BEFORE, - DateCondition.IS_EXACTLY, - DateCondition.IS_NOT_EXACTLY, - ])( - 'should return false for empty value when condition requires it: %s', + describe.each(conditions.valueRequired)( + 'Required condition: %s', condition => { - const testValues = [values.empty, values.undefined]; - - testValues.forEach(value => { - expect(isDateConditionValid(condition, value)).toBeFalsy(); - }); + it.each([ + [values.empty, false], + [values.undefined, false], + [values.invalid, false], + [values.valid, true], + ] as const)( + 'should return true only if value is valid date format: %s, %s', + (testValues, expected) => { + [testValues].flat().forEach(value => { + expect(isDateConditionValid(condition, value)).toEqual(expected); + }); + } + ); } ); }); diff --git a/packages/iris-grid/src/sidebar/conditional-formatting/ConditionalFormattingUtils.ts b/packages/iris-grid/src/sidebar/conditional-formatting/ConditionalFormattingUtils.ts index d352ed0423..16376a1cfe 100644 --- a/packages/iris-grid/src/sidebar/conditional-formatting/ConditionalFormattingUtils.ts +++ b/packages/iris-grid/src/sidebar/conditional-formatting/ConditionalFormattingUtils.ts @@ -1,6 +1,6 @@ import Log from '@deephaven/log'; import { Column, CustomColumn } from '@deephaven/jsapi-shim'; -import { TableUtils } from '@deephaven/jsapi-utils'; +import { DateUtils, TableUtils } from '@deephaven/jsapi-utils'; import { makeColumnFormatColumn, makeRowFormatColumn, @@ -717,9 +717,28 @@ export function isDateConditionValid(condition: DateCondition, value?: string) { case DateCondition.IS_NOT_NULL: return true; - default: - // Proper date validation will be addressed by Issue #1108 - return value != null && value !== ''; + default: { + const [dateTimeString, ...rest] = (value ?? '').split(' '); + // Reconstitute all tokens after the first ' ' in case the user included garbage data at the end + // e.g. '2020-01-01 NY blah' + const tzCode = rest.join(' '); + + try { + DateUtils.parseDateTimeString(dateTimeString); + } catch (e) { + log.debug('Invalid datetime string', dateTimeString); + return false; + } + + try { + dh.i18n.TimeZone.getTimeZone(tzCode); + } catch (e) { + log.debug('Invalid timezone string', tzCode); + return false; + } + + return true; + } } }