fix: Added date time parsing for conditional formatting (#1120)

Added datetime string parsing / validation. fixes #1108
deephaven · Mar 2, 2023 · 4c7710e · 4c7710e
1 parent c4a3795
commit 4c7710e
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 29 deletions.
diff --git 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
@@ -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
@@ -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;
+    }
   }
 }