fix: date arithmetic; inequality comparisons involving dates, numbers…

…, and strings (#1413)

* fix: date arithmetic
* fix: comparison operators support dates, strings, etc.
* fix: eliminate time zone from serialized date strings
* feat: don't show time by default for dates without times
* chore: code review tweak

v3/cypress/e2e/attribute-types.spec.ts

@@ -19,14 +19,14 @@ context("attribute types", () => {
     })
     it("verify date", () => {
       table.getCell("4", "2").should("contain", "8/7/2017, 12:01 PM")
-      table.getCell("4", "3").should("contain", "6/15/1966, 12:00 AM")
-      table.getCell("4", "4").should("contain", "12/7/1787, 12:00 AM")
-      table.getCell("4", "5").should("contain", "1/2/2003, 12:00 AM")
-      table.getCell("4", "6").should("contain", "12/3/1818, 12:00 AM")
-      table.getCell("4", "7").should("contain", "6/1/1992, 12:00 AM")
-      table.getCell("4", "8").should("contain", "11/2/1989, 12:00 AM")
-      table.getCell("4", "9").should("contain", "11/16/2007, 12:00 AM")
-      table.getCell("4", "10").should("contain", "11/30/2000, 12:00 AM")
+      table.getCell("4", "3").should("contain", "6/15/1966")
+      table.getCell("4", "4").should("contain", "12/7/1787")
+      table.getCell("4", "5").should("contain", "1/2/2003")
+      table.getCell("4", "6").should("contain", "12/3/1818")
+      table.getCell("4", "7").should("contain", "6/1/1992")
+      table.getCell("4", "8").should("contain", "11/2/1989")
+      table.getCell("4", "9").should("contain", "11/16/2007")
+      table.getCell("4", "10").should("contain", "11/30/2000")
     })
     it.skip("verify boolean", () => {
       table.getCell("5", "2").should("contain", "false")

v3/src/components/case-table/use-columns.tsx

@@ -9,14 +9,15 @@ import { IDataSet } from "../../models/data/data-set"
 import { symParent } from "../../models/data/data-set-types"
 import { getCollectionAttrs } from "../../models/data/data-set-utils"
 import { parseColor } from "../../utilities/color-utils"
+import { isStdISODateString } from "../../utilities/date-iso-utils"
+import { parseDate } from "../../utilities/date-parser"
+import { DatePrecision, formatDate } from "../../utilities/date-utils"
 import { mstReaction } from "../../utilities/mst-reaction"
 import { isCaseEditable } from "../../utilities/plugin-utils"
 import { kDefaultColumnWidth, symDom, TColumn, TRenderCellProps } from "./case-table-types"
 import CellTextEditor from "./cell-text-editor"
 import ColorCellTextEditor from "./color-cell-text-editor"
 import { ColumnHeader } from "./column-header"
-import { isBrowserISOString, parseDate } from "../../utilities/date-parser"
-import { DatePrecision, formatDate } from "../../utilities/date-utils"
 
 // cache d3 number formatters so we don't have to generate them on every render
 type TNumberFormatter = (n: number) => string
@@ -62,7 +63,7 @@ export function renderValue(str = "", num = NaN, attr?: IAttribute, key?: number
   // This is because CODAP v3 stores all the case values as strings natively, and we cannot simply check if the value
   // is an instance of the `Date` class (as it will never be). Date.toISOString() is the native way of serializing dates
   // in CODAP v3 (check `importValueToString` from attribute.ts).
-  if (isBrowserISOString(str) || userType === "date" && str !== "") {
+  if (isStdISODateString(str) || userType === "date" && str !== "") {
     const date = parseDate(str, true)
     if (date) {
       // TODO: add precision support for date formatting

v3/src/models/data/attribute.test.ts

@@ -42,7 +42,7 @@ describe("Attribute", () => {
     expect(importValueToString(1e-6)).toBe("0.000001")
     expect(importValueToString(true)).toBe("true")
     expect(importValueToString(false)).toBe("false")
-    expect(importValueToString(new Date("2020-06-14T10:13:34.123Z"))).toBe("2020-06-14T10:13:34.123Z")
+    expect(importValueToString(new Date(2020, 5, 14, 10, 13, 34, 123))).toBe("2020-06-14T10:13:34.123Z")
 
     const attr = Attribute.create({ name: "a" })
     expect(attr.toNumeric(null as any)).toBeNaN()

v3/src/models/data/attribute.ts

@@ -26,14 +26,15 @@
  */
 
 import { Instance, SnapshotIn, types } from "mobx-state-tree"
-import { parseColor } from "../../utilities/color-utils"
 import { kAttrIdPrefix, typeV3Id } from "../../utilities/codap-utils"
+import { parseColor } from "../../utilities/color-utils"
+import { formatStdISODateString } from "../../utilities/date-iso-utils"
+import { isDateString } from "../../utilities/date-parser"
 import { cachedFnFactory } from "../../utilities/mst-utils"
 import { Formula, IFormula } from "../formula/formula"
 import { applyModelChange } from "../history/apply-model-change"
 import { withoutUndo } from "../history/without-undo"
 import { V2Model } from "./v2-model"
-import { isDateString } from "../../utilities/date-parser"
 
 export const kDefaultFormatStr = ".3~f"
 
@@ -54,10 +55,7 @@ export function importValueToString(value: IValueType): string {
     return value
   }
   if (value instanceof Date) {
-    // Convert Date to ISO string format. It's a consistent format that can be parsed back into a Date object
-    // without losing any information. Also, it's relatively compact and it can be easily recognized as a date string,
-    // in contrast to storing the date as a number (e.g. milliseconds since epoch).
-    return value.toISOString()
+    return formatStdISODateString(value)
   }
   return value.toString()
 }

v3/src/models/formula/functions/function-utils.test.ts

@@ -25,6 +25,8 @@ describe("equal", () => {
     expect(equal("1", "1")).toBe(true)
     expect(equal(true, "true")).toBe(true)
     expect(equal("true", true)).toBe(true)
+    const now = new Date()
+    expect(equal(now, new Date(now))).toBe(true)
   })
 
   it("should return false for unequal values", () => {
@@ -35,6 +37,8 @@ describe("equal", () => {
     expect(equal(true, "false")).toBe(false)
     expect(equal("true", false)).toBe(false)
     expect(equal(true, 1)).toBe(false)
+    const now = new Date()
+    expect(equal(now, new Date(Date.now() + 3600))).toBe(false)
   })
 })
 

v3/src/models/formula/functions/function-utils.ts

@@ -1,6 +1,7 @@
 import { MathNode } from "mathjs"
-import { FormulaMathJsScope } from "../formula-mathjs-scope"
+import { checkDate } from "../../../utilities/date-utils"
 import { isValueNonEmpty } from "../../../utilities/math-utils"
+import { FormulaMathJsScope } from "../formula-mathjs-scope"
 import { CurrentScope, MathJSPartitionedMap } from "../formula-types"
 export { isNumber, isValueNonEmpty } from "../../../utilities/math-utils"
 
@@ -12,6 +13,11 @@ export const isValueTruthy = (value: any) => isValueNonEmpty(value) && value !==
 
 export const
 equal = (a: any, b: any): boolean => {
+  // Date objects are compared numerically as seconds
+    const [isADate, aDate] = checkDate(a)
+    const [isBDate, bDate] = checkDate(b)
+    if (isADate) a = aDate.valueOf() / 1000
+    if (isBDate) b = bDate.valueOf() / 1000
   // Checks below might seem redundant once the data set cases start using typed values, but they are not.
   // Note that user might still compare a string with a number unintentionally, and it makes sense to try to cast
   // values when possible, so that the comparison can be performed without forcing users to think about types.

v3/src/models/formula/functions/operators.test.ts

@@ -1,6 +1,188 @@
-import { formatDate } from "../../../utilities/date-utils"
 import { math } from "./math"
 
+describe("= operator", () => {
+  it("compares dates for equality", () => {
+    const fn1 = math.compile(`today() == today()`)
+    expect(fn1.evaluate()).toBe(true)
+    const fn2 = math.compile(`today() == today() + 24 * 3600`)
+    expect(fn2.evaluate()).toBe(false)
+  })
+  it("compares dates for inequality", () => {
+    const fn1 = math.compile(`today() != today()`)
+    expect(fn1.evaluate()).toBe(false)
+    const fn2 = math.compile(`today() != today() + 24 * 3600`)
+    expect(fn2.evaluate()).toBe(true)
+  })
+})
+
+describe("< operator", () => {
+  it("compares numbers", () => {
+    const f1 = math.compile("1 < 2")
+    expect(f1.evaluate()).toBe(true)
+    const f2 = math.compile("1 < 1")
+    expect(f2.evaluate()).toBe(false)
+    const f3 = math.compile("2 < 1")
+    expect(f3.evaluate()).toBe(false)
+    // NaN comparisons are always false
+    const f4 = math.compile("0/0 < 0")
+    expect(f4.evaluate()).toBe(false)
+    const f5 = math.compile("0 < 0/0")
+    expect(f5.evaluate()).toBe(false)
+  })
+  it("compares dates numerically", () => {
+    const f1 = math.compile("today() < today()")
+    expect(f1.evaluate()).toBe(false)
+    const f2 = math.compile("today() + 24 * 3600 < today()")
+    expect(f2.evaluate()).toBe(false)
+    const f3 = math.compile("today() < today() + 24 * 3600")
+    expect(f3.evaluate()).toBe(true)
+  })
+  it("compares numeric strings", () => {
+    const f1 = math.compile("'1' < '2'")
+    expect(f1.evaluate()).toBe(true)
+    const f2 = math.compile("'1' < '1'")
+    expect(f2.evaluate()).toBe(false)
+    const f3 = math.compile("'2' < '1'")
+    expect(f3.evaluate()).toBe(false)
+    const f4 = math.compile("'1' < '12'")
+    expect(f4.evaluate()).toBe(true)
+  })
+  it("compares strings", () => {
+    const f1 = math.compile("'abc' < 'def'")
+    expect(f1.evaluate()).toBe(true)
+    const f2 = math.compile("'abc' < 'abc'")
+    expect(f2.evaluate()).toBe(false)
+    const f3 = math.compile("'def' < 'abc'")
+    expect(f3.evaluate()).toBe(false)
+  })
+})
+
+describe("<= operator", () => {
+  it("compares numbers", () => {
+    const f1 = math.compile("1 <= 2")
+    expect(f1.evaluate()).toBe(true)
+    const f2 = math.compile("1 <= 1")
+    expect(f2.evaluate()).toBe(true)
+    const f3 = math.compile("2 <= 1")
+    expect(f3.evaluate()).toBe(false)
+    // NaN comparisons are always false
+    const f4 = math.compile("0/0 <= 0")
+    expect(f4.evaluate()).toBe(false)
+    const f5 = math.compile("0 <= 0/0")
+    expect(f5.evaluate()).toBe(false)
+  })
+  it("compares dates numerically", () => {
+    const f1 = math.compile("today() <= today()")
+    expect(f1.evaluate()).toBe(true)
+    const f2 = math.compile("today() + 24 * 3600 <= today()")
+    expect(f2.evaluate()).toBe(false)
+    const f3 = math.compile("today() <= today() + 24 * 3600")
+    expect(f3.evaluate()).toBe(true)
+  })
+  it("compares numeric strings", () => {
+    const f1 = math.compile("'1' <= '2'")
+    expect(f1.evaluate()).toBe(true)
+    const f2 = math.compile("'1' <= '1'")
+    expect(f2.evaluate()).toBe(true)
+    const f3 = math.compile("'2' <= '1'")
+    expect(f3.evaluate()).toBe(false)
+    const f4 = math.compile("'1' <= '12'")
+    expect(f4.evaluate()).toBe(true)
+  })
+  it("compares strings", () => {
+    const f1 = math.compile("'abc' <= 'def'")
+    expect(f1.evaluate()).toBe(true)
+    const f2 = math.compile("'abc' <= 'abc'")
+    expect(f2.evaluate()).toBe(true)
+    const f3 = math.compile("'def' < 'abc'")
+    expect(f3.evaluate()).toBe(false)
+  })
+})
+
+describe("> operator", () => {
+  it("compares numbers", () => {
+    const f1 = math.compile("1 > 2")
+    expect(f1.evaluate()).toBe(false)
+    const f2 = math.compile("1 > 1")
+    expect(f2.evaluate()).toBe(false)
+    const f3 = math.compile("2 > 1")
+    expect(f3.evaluate()).toBe(true)
+    // NaN comparisons are always false
+    const f4 = math.compile("0/0 > 0")
+    expect(f4.evaluate()).toBe(false)
+    const f5 = math.compile("0 > 0/0")
+    expect(f5.evaluate()).toBe(false)
+  })
+  it("compares dates numerically", () => {
+    const f1 = math.compile("today() > today()")
+    expect(f1.evaluate()).toBe(false)
+    const f2 = math.compile("today() + 24 * 3600 > today()")
+    expect(f2.evaluate()).toBe(true)
+    const f3 = math.compile("today() > today() + 24 * 3600")
+    expect(f3.evaluate()).toBe(false)
+  })
+  it("compares numeric strings", () => {
+    const f1 = math.compile("'1' > '2'")
+    expect(f1.evaluate()).toBe(false)
+    const f2 = math.compile("'1' > '1'")
+    expect(f2.evaluate()).toBe(false)
+    const f3 = math.compile("'2' > '1'")
+    expect(f3.evaluate()).toBe(true)
+    const f4 = math.compile("'1' > '12'")
+    expect(f4.evaluate()).toBe(false)
+  })
+  it("compares strings", () => {
+    const f1 = math.compile("'abc' > 'def'")
+    expect(f1.evaluate()).toBe(false)
+    const f2 = math.compile("'abc' > 'abc'")
+    expect(f2.evaluate()).toBe(false)
+    const f3 = math.compile("'def' > 'abc'")
+    expect(f3.evaluate()).toBe(true)
+  })
+})
+
+describe(">= operator", () => {
+  it("compares numbers", () => {
+    const f1 = math.compile("1 >= 2")
+    expect(f1.evaluate()).toBe(false)
+    const f2 = math.compile("1 >= 1")
+    expect(f2.evaluate()).toBe(true)
+    const f3 = math.compile("2 >= 1")
+    expect(f3.evaluate()).toBe(true)
+    // NaN comparisons are always false
+    const f4 = math.compile("0/0 >= 0")
+    expect(f4.evaluate()).toBe(false)
+    const f5 = math.compile("0 >= 0/0")
+    expect(f5.evaluate()).toBe(false)
+  })
+  it("compares dates numerically", () => {
+    const f1 = math.compile("today() >= today()")
+    expect(f1.evaluate()).toBe(true)
+    const f2 = math.compile("today() + 24 * 3600 >= today()")
+    expect(f2.evaluate()).toBe(true)
+    const f3 = math.compile("today() >= today() + 24 * 3600")
+    expect(f3.evaluate()).toBe(false)
+  })
+  it("compares numeric strings", () => {
+    const f1 = math.compile("'1' >= '2'")
+    expect(f1.evaluate()).toBe(false)
+    const f2 = math.compile("'1' >= '1'")
+    expect(f2.evaluate()).toBe(true)
+    const f3 = math.compile("'2' >= '1'")
+    expect(f3.evaluate()).toBe(true)
+    const f4 = math.compile("'1' >= '12'")
+    expect(f4.evaluate()).toBe(false)
+  })
+  it("compares strings", () => {
+    const f1 = math.compile("'abc' >= 'def'")
+    expect(f1.evaluate()).toBe(false)
+    const f2 = math.compile("'abc' >= 'abc'")
+    expect(f2.evaluate()).toBe(true)
+    const f3 = math.compile("'def' >= 'abc'")
+    expect(f3.evaluate()).toBe(true)
+  })
+})
+
 describe("+ operator", () => {
   it("adds two numbers", () => {
     const fn = math.compile("1 + 2")
@@ -17,23 +199,26 @@ describe("+ operator", () => {
     expect(fn.evaluate()).toEqual(3)
   })
 
-  it("adds dates", () => {
+  it("concatenates a number and a non-numeric string", () => {
+    const fn = math.compile("1 + 'a'")
+    expect(fn.evaluate()).toBe("1a")
+  })
+
+  it("throws an exception when adding dates", () => {
     const fn = math.compile("'1/1/2020' + '1/1/2020'")
-    const val1 = new Date(2020, 0, 1).valueOf()
-    const val2 = new Date(2020, 0, 1).valueOf()
-    expect(fn.evaluate()).toEqual(formatDate(new Date(val1 + val2)))
+    expect(() => fn.evaluate()).toThrow()
   })
 
   it("adds seconds to dates", () => {
     const fn = math.compile("'1/1/2020' + 60 * 60") // + 1 hour
-    expect(fn.evaluate()).toEqual(formatDate(new Date(2020, 0, 1, 1)))
+    expect(fn.evaluate()).toEqual(new Date(2020, 0, 1, 1))
     const fn2 = math.compile("'1/1/2020' + 60 * 60 * 24") // + 1 day
-    expect(fn2.evaluate()).toEqual(formatDate(new Date(2020, 0, 2)))
+    expect(fn2.evaluate()).toEqual(new Date(2020, 0, 2))
 
     const fn3 = math.compile("60 * 60 + '1/1/2020'") // + 1 hour
-    expect(fn3.evaluate()).toEqual(formatDate(new Date(2020, 0, 1, 1)))
+    expect(fn3.evaluate()).toEqual(new Date(2020, 0, 1, 1))
     const fn4 = math.compile("60 * 60 * 24 + '1/1/2020'") // + 1 day
-    expect(fn4.evaluate()).toEqual(formatDate(new Date(2020, 0, 2)))
+    expect(fn4.evaluate()).toEqual(new Date(2020, 0, 2))
   })
 
   it("concatenates strings", () => {
@@ -62,7 +247,17 @@ describe("- operator", () => {
     const fn = math.compile("'1/2/2020' - '1/1/2020'") // 1 day
     const val1 = new Date(2020, 0, 2).valueOf()
     const val2 = new Date(2020, 0, 1).valueOf()
-    expect(fn.evaluate()).toEqual(formatDate(new Date(val1 - val2)))
+    expect(fn.evaluate()).toEqual(new Date(val1 - val2))
+  })
+
+  it("subtracts a number from a date", () => {
+    const fn = math.compile("'1/2/2020' - 86400") // 1 day
+    expect(fn.evaluate()).toEqual(new Date(2020, 0, 1))
+  })
+
+  it("throws an error when subtracting a date from a number", () => {
+    const fn = math.compile("86400 - '1/2/2020'") // 1 day
+    expect(() => fn.evaluate()).toThrow()
   })
 
   it("throws an error when subtracting strings", () => {