(core) Fix date filter for DateTime columns.

Summary:
Date filter was not taking timezone correclty into account, which was
causing to wrong-inclusion and wrong-exclusion of dates near the
bounds.

Diff fixes that, it also bring little refactoring that hopefully clarifies things a little.

Test Plan: Includes brand new test for `app/common/ColumnFilterFunc`.

Reviewers: jarek

Reviewed By: jarek

Differential Revision: https://phab.getgrist.com/D3763

app/client/models/ColumnFilter.ts

@@ -3,7 +3,7 @@ import {CellValue} from 'app/common/DocActions';
 import {
   FilterSpec, FilterState, IRelativeDateSpec, isRangeFilter, isRelativeBound, makeFilterState
 } from "app/common/FilterState";
-import {toUnixTimestamp} from "app/common/RelativeDates";
+import {relativeDateToUnixTimestamp} from "app/common/RelativeDates";
 import {nativeCompare} from 'app/common/gutil';
 import {Computed, Disposable, Observable} from 'grainjs';
 
@@ -124,9 +124,11 @@ export class ColumnFilter extends Disposable {
     return this.makeFilterJson() !== this._initialFilterJson;
   }
 
-  public getBoundsValue(minMax: 'min' | 'max'): number | undefined {
+  // Retuns min or max as a numeric value.
+  public getBoundsValue(minMax: 'min' | 'max'): number {
     const value = this[minMax].get();
-    return isRelativeBound(value) ? toUnixTimestamp(value) : value;
+    if (value === undefined) { return minMax === 'min' ? -Infinity : +Infinity; }
+    return isRelativeBound(value) ? relativeDateToUnixTimestamp(value) : value;
   }
 
 

app/client/ui/ColumnFilterCalendarView.ts

@@ -5,7 +5,6 @@ import { textButton } from "app/client/ui2018/buttons";
 import { IColumnFilterViewType } from "app/client/ui/ColumnFilterMenu";
 import getCurrentTime from "app/common/getCurrentTime";
 import { IRelativeDateSpec, isRelativeBound } from "app/common/FilterState";
-import { toUnixTimestamp } from "app/common/RelativeDates";
 import { updateRelativeDate } from "app/client/ui/RelativeDatesOptions";
 import moment from "moment-timezone";
 
@@ -79,7 +78,7 @@ export class ColumnFilterCalendarView extends Disposable {
 
     if (minMax !== null) {
       const value = this.columnFilter.getBoundsValue(minMax);
-      if (value !== undefined) {
+      if (isFinite(value)) {
         dateValue = new Date(value * 1000);
       }
     }
@@ -100,12 +99,12 @@ export class ColumnFilterCalendarView extends Disposable {
     // TODO: also perform this check when users pick relative dates from popup
     if (this.selectedBoundObs.get() === 'min') {
       min.set(this._updateBoundValue(min.get(), d));
-      if (max.get() !== undefined && toUnixTimestamp(max.get()!) < d) {
+      if (this.columnFilter.getBoundsValue('max') < d) {
         max.set(this._updateBoundValue(max.get(), d));
       }
     } else {
       max.set(this._updateBoundValue(max.get(), d));
-      if (min.get() !== undefined && d < toUnixTimestamp(min.get()!)) {
+      if (this.columnFilter.getBoundsValue('min') > d) {
         min.set(this._updateBoundValue(min.get(), d));
       }
     }
@@ -119,13 +118,13 @@ export class ColumnFilterCalendarView extends Disposable {
       const m = moment.utc(val * 1000);
       return new Date(Date.UTC(m.year(), m.month(), m.date()));
     };
-    if (min === undefined && max === undefined) {
+    if (!isFinite(min) && !isFinite(max)) {
       return [];
     }
-    if (min === undefined) {
+    if (!isFinite(min)) {
-      return [{valueOf: () => -Infinity}, toDate(max!)];
+      return [{valueOf: () => -Infinity}, toDate(max)];
     }
-    if (max === undefined) {
+    if (!isFinite(max)) {
       return [toDate(min), {valueOf: () => +Infinity}];
     }
     return [toDate(min), toDate(max)];

app/client/ui/RelativeDatesOptions.ts

@@ -1,5 +1,11 @@
 import {
-  CURRENT_DATE, diffUnit, formatRelBounds, IPeriod, IRelativeDateSpec, isEquivalentRelativeDate, toUnixTimestamp
+  CURRENT_DATE,
+  diffUnit,
+  formatRelBounds,
+  IPeriod,
+  IRelativeDateSpec,
+  isEquivalentRelativeDate,
+  relativeDateToUnixTimestamp
 } from "app/common/RelativeDates";
 import { IRangeBoundType, isRelativeBound } from "app/common/FilterState";
 import getCurrentTime from "app/common/getCurrentTime";
@@ -55,7 +61,7 @@ function relativeDateOptionsSpec(value: IRangeBoundType): Array<IRangeBoundType>
   if (value === undefined) {
     return DEFAULT_OPTION_LIST;
   } else if (isRelativeBound(value)) {
-    value = toUnixTimestamp(value);
+    value = relativeDateToUnixTimestamp(value);
   }
 
   const date = moment.utc(value * 1000);

app/common/ColumnFilterFunc.ts

@@ -1,11 +1,10 @@
 import {CellValue} from "app/common/DocActions";
-import {
+import {FilterState, IRangeBoundType, isRangeFilter, makeFilterState} from "app/common/FilterState";
-  FilterState, IRangeBoundType, isRangeFilter, isRelativeBound, makeFilterState
-} from "app/common/FilterState";
 import {decodeObject} from "app/plugin/objtypes";
-import moment from "moment-timezone";
+import moment, { Moment } from "moment-timezone";
-import {isDateLikeType, isList, isListType, isNumberType} from "./gristTypes";
+import {extractInfoFromColType, isDateLikeType, isList, isListType, isNumberType} from "app/common/gristTypes";
-import {toUnixTimestamp} from "app/common/RelativeDates";
+import {isRelativeBound, relativeDateToUnixTimestamp} from "app/common/RelativeDates";
+import {noop} from "lodash";
 
 export type ColumnFilterFunc = (value: CellValue) => boolean;
 
@@ -18,8 +17,12 @@ export function makeFilterFunc(state: FilterState,
     let {min, max} = state;
     if (isNumberType(columnType) || isDateLikeType(columnType)) {
 
-      min = getBoundsValue(state, 'min');
+      if (isDateLikeType(columnType)) {
-      max = getBoundsValue(state, 'max');
+        const info = extractInfoFromColType(columnType);
+        const timezone = (info.type === 'DateTime' && info.timezone) || 'utc';
+        min = changeTimezone(min, timezone, m => m.startOf('day'));
+        max = changeTimezone(max, timezone, m => m.endOf('day'));
+      }
 
       return (val) => {
         if (typeof val !== 'number') { return false; }
@@ -59,18 +62,14 @@ export function buildColFilter(filterJson: string | undefined,
   return filterJson ? makeFilterFunc(makeFilterState(filterJson), columnType) : null;
 }
 
-
+// Returns the unix timestamp for date in timezone. Function support relative date. Also support
-function getBoundsValue(state: {min?: IRangeBoundType, max?: IRangeBoundType}, minMax: 'min'|'max') {
+// optional mod argument that let you modify date as a moment instance.
-  const value = state[minMax];
+function changeTimezone(date: IRangeBoundType,
-  if (isRelativeBound(value)) {
+                        timezone: string,
-    const val = toUnixTimestamp(value);
+                        mod: (m: Moment) => void = noop): number|undefined {
-    const m = moment.utc(val * 1000);
+  if (date === undefined) { return undefined; }
-    if (minMax === 'min') {
+  const val = isRelativeBound(date) ? relativeDateToUnixTimestamp(date) : date;
-      m.startOf('day');
+  const m = moment.tz(val * 1000, timezone);
-    } else {
+  mod(m);
-      m.endOf('day');
+  return Math.floor(m.valueOf() / 1000);
-    }
-    return Math.floor(m.valueOf() / 1000);
-  }
-  return value;
 }

app/common/RelativeDates.ts

@@ -32,31 +32,25 @@ export function isRelativeBound(bound?: number|IRelativeDateSpec): bound is IRel
 
 // Returns the number of seconds between 1 January 1970 00:00:00 UTC and the given bound, may it be
 // a relative date.
-export function toUnixTimestamp(bound: IRelativeDateSpec|number): number {
+export function relativeDateToUnixTimestamp(bound: IRelativeDateSpec): number {
+  const localDate = getCurrentTime().startOf('day');
+  const date = moment.utc(localDate.toObject());
+  const periods = Array.isArray(bound) ? bound : [bound];
 
-  if (isRelativeBound(bound)) {
+  for (const period of periods) {
-    const localDate = getCurrentTime().startOf('day');
+    const {quantity, unit, endOf} = period;
-    const date = moment.utc(localDate.toObject());
-    const periods = Array.isArray(bound) ? bound : [bound];
 
-    for (const period of periods) {
+    date.add(quantity, unit);
-      const {quantity, unit, endOf} = period;
+    if (endOf) {
+      date.endOf(unit);
 
-      date.add(quantity, unit);
+      // date must have "hh:mm:ss" set to "00:00:00"
-      if (endOf) {
+      date.startOf('day');
-        date.endOf(unit);
+    } else {
-
+      date.startOf(unit);
-        // date must have "hh:mm:ss" set to "00:00:00"
-        date.startOf('day');
-      } else {
-        date.startOf(unit);
-      }
     }
-
-    return Math.floor(date.valueOf() / 1000);
-  } else {
-    return bound;
   }
+  return Math.floor(date.valueOf() / 1000);
 }
 
 // Format a relative date.

test/common/ColumnFilterFunc.ts

@@ -0,0 +1,44 @@
+import { makeFilterFunc } from "app/common/ColumnFilterFunc";
+import { FilterState } from "app/common/FilterState";
+import moment from "moment-timezone";
+import { assert } from 'chai';
+
+const format = "YYYY-MM-DD HH:mm:ss";
+const timezone = 'Europe/Paris';
+const parseDateTime = (dateStr: string) => moment.tz(dateStr, format, true, timezone).valueOf() / 1000;
+const columnType = `DateTime:${timezone}`;
+
+describe('ColumnFilterFunc', function() {
+
+
+  [
+    {date: '2023-01-01 23:59:59', expected: false},
+    {date: '2023-01-02 00:00:00', expected: true},
+    {date: '2023-01-02 00:00:01', expected: true},
+    {date: '2023-01-02 01:00:01', expected: true},
+  ].forEach(({date, expected}) => {
+
+    const minStr = '2023-01-02';
+    const state: FilterState = { min: moment.utc(minStr).valueOf() / 1000 };
+    const filterFunc = makeFilterFunc(state, columnType);
+
+    it(`${minStr} <= ${date} should be ${expected}`, function() {
+      assert.equal(filterFunc(parseDateTime(date)), expected);
+    });
+  });
+
+  [
+    {date: '2023-01-11 00:00:00', expected: true},
+    {date: '2023-01-11 23:59:59', expected: true},
+    {date: '2023-01-12 00:00:01', expected: false},
+  ].forEach(({date, expected}) => {
+
+    const maxStr = '2023-01-11';
+    const state: FilterState = { max: moment.utc(maxStr).valueOf() / 1000 };
+    const filterFunc = makeFilterFunc(state, columnType);
+
+    it(`${maxStr} >= ${date} should be ${expected}`, function() {
+      assert.equal(filterFunc(parseDateTime(date)), expected);
+    });
+  });
+});