Skip to content

Commit

Permalink
Merge pull request #1370 from concord-consortium/187799270-IValueType…
Browse files Browse the repository at this point in the history
…-Date

Make Date a valid IValueType, refactor how dates are handled in formulas and case table
  • Loading branch information
pjanik authored Jul 24, 2024
2 parents 83b49e7 + a855991 commit 86868a3
Show file tree
Hide file tree
Showing 10 changed files with 108 additions and 51 deletions.
15 changes: 10 additions & 5 deletions v3/src/components/case-table/use-columns.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { kDefaultColumnWidth, symDom, TColumn, TRenderCellProps } from "./case-t
import CellTextEditor from "./cell-text-editor"
import ColorCellTextEditor from "./color-cell-text-editor"
import { ColumnHeader } from "./column-header"
import { parseDate } from "../../utilities/date-parser"
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
Expand Down Expand Up @@ -54,10 +54,15 @@ export function renderValue(str = "", num = NaN, attr?: IAttribute, key?: number
if (formatter) str = formatter(num)
}

// dates
// Note that CODAP v2 formats dates in the case table ONLY if the user explicitly specified type as "date".
// Dates are not interpreted as dates and formatted by default.
if (userType === "date" && str !== "") {
// Dates
// Note that CODAP v2 formats dates in the case table ONLY if the user explicitly specifies the type as "date".
// Dates are not interpreted as dates and formatted by default. However, V3 adds one exception to this rule:
// if the date string is strictly an ISO string produced by the browser's Date.toISOString(), it will be treated as
// a Date object that should be formatted. The main reason for this is to format the results of date formulas.
// 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 !== "") {
const date = parseDate(str, true)
if (date) {
// TODO: add precision support for date formatting
Expand Down
2 changes: 1 addition & 1 deletion v3/src/data-interactive/data-interactive-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { ITileModel } from "../models/tiles/tile-model"
import { ICollectionLabels, ICollectionModel } from "../models/data/collection"
import { V2SpecificComponent } from "./data-interactive-component-types"

export type DICaseValue = string | number | boolean | undefined
export type DICaseValue = string | number | boolean | Date | undefined
export type DICaseValues = Record<string, DICaseValue>
export interface DIFullCase {
children?: number[]
Expand Down
2 changes: 1 addition & 1 deletion v3/src/data-interactive/handlers/item-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const diItemHandler: DIHandler = {
_items.forEach(item => {
let newItem: DIItem
if (typeof item.values === "object") {
newItem = item.values
newItem = item.values as DIItem
} else {
newItem = item as DIItem
}
Expand Down
1 change: 1 addition & 0 deletions v3/src/models/data/attribute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +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")

const attr = Attribute.create({ name: "a" })
expect(attr.toNumeric(null as any)).toBeNaN()
Expand Down
18 changes: 15 additions & 3 deletions v3/src/models/data/attribute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,26 @@ export const kDefaultFormatStr = ".3~f"
const isDevelopment = () => process.env.NODE_ENV !== "production"
const isProduction = () => process.env.NODE_ENV === "production"

export type IValueType = string | number | boolean | undefined
export type IValueType = string | number | boolean | Date | undefined

export interface ISetValueOptions {
noInvalidate?: boolean
}

export function importValueToString(value: IValueType) {
return value == null ? "" : typeof value === "string" ? value : value.toString()
export function importValueToString(value: IValueType): string {
if (value == null) {
return ""
}
if (typeof value === "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 value.toString()
}

export const attributeTypes = [
Expand Down
2 changes: 1 addition & 1 deletion v3/src/models/formula/formula-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export interface ILookupDependency {

export type IFormulaDependency = ILocalAttributeDependency | IGlobalValueDependency | ILookupDependency

export type FValue = string | number | boolean
export type FValue = string | number | boolean | Date

export type FValueOrArray = FValue | FValue[]

Expand Down
66 changes: 41 additions & 25 deletions v3/src/models/formula/functions/date-functions.test.ts
Original file line number Diff line number Diff line change
@@ -1,59 +1,58 @@
import { formatDate } from "../../../utilities/date-utils"
import { UNDEF_RESULT } from "./function-utils"
import { math } from "./math"

describe("date", () => {
it("returns current date if no arguments are provided", () => {
const fn = math.compile("date()")
expect(fn.evaluate()).toEqual(formatDate(new Date()))
expect(fn.evaluate().valueOf()).toBeCloseTo(Date.now(), -3)
})

it("interprets [0, 50) as 20xx year", () => {
const fn = math.compile("date(x)")
expect(fn.evaluate({ x: 1 })).toEqual(formatDate(new Date(2001, 0, 1)))
expect(fn.evaluate({ x: 10 })).toEqual(formatDate(new Date(2010, 0, 1)))
expect(fn.evaluate({ x: 20 })).toEqual(formatDate(new Date(2020, 0, 1)))
expect(fn.evaluate({ x: 30 })).toEqual(formatDate(new Date(2030, 0, 1)))
expect(fn.evaluate({ x: 49 })).toEqual(formatDate(new Date(2049, 0, 1)))
expect(fn.evaluate({ x: 1 })).toEqual(new Date(2001, 0, 1))
expect(fn.evaluate({ x: 10 })).toEqual(new Date(2010, 0, 1))
expect(fn.evaluate({ x: 20 })).toEqual(new Date(2020, 0, 1))
expect(fn.evaluate({ x: 30 })).toEqual(new Date(2030, 0, 1))
expect(fn.evaluate({ x: 49 })).toEqual(new Date(2049, 0, 1))
})

it("interprets [50, 99] as 19xx year", () => {
const fn = math.compile("date(x)")
expect(fn.evaluate({ x: 50 })).toEqual(formatDate(new Date(1950, 0, 1)))
expect(fn.evaluate({ x: 60 })).toEqual(formatDate(new Date(1960, 0, 1)))
expect(fn.evaluate({ x: 70 })).toEqual(formatDate(new Date(1970, 0, 1)))
expect(fn.evaluate({ x: 99 })).toEqual(formatDate(new Date(1999, 0, 1)))
expect(fn.evaluate({ x: 50 })).toEqual(new Date(1950, 0, 1))
expect(fn.evaluate({ x: 60 })).toEqual(new Date(1960, 0, 1))
expect(fn.evaluate({ x: 70 })).toEqual(new Date(1970, 0, 1))
expect(fn.evaluate({ x: 99 })).toEqual(new Date(1999, 0, 1))
})

it("interprets number as year if it's bigger than 100 but smaller than 5000", () => {
const fn = math.compile("date(x)")
expect(fn.evaluate({ x: 100 })).toEqual(formatDate(new Date(100, 0, 1)))
expect(fn.evaluate({ x: 1000 })).toEqual(formatDate(new Date(1000, 0, 1)))
expect(fn.evaluate({ x: 2000 })).toEqual(formatDate(new Date(2000, 0, 1)))
expect(fn.evaluate({ x: 4999 })).toEqual(formatDate(new Date(4999, 0, 1)))
expect(fn.evaluate({ x: 100 })).toEqual(new Date(100, 0, 1))
expect(fn.evaluate({ x: 1000 })).toEqual(new Date(1000, 0, 1))
expect(fn.evaluate({ x: 2000 })).toEqual(new Date(2000, 0, 1))
expect(fn.evaluate({ x: 4999 })).toEqual(new Date(4999, 0, 1))
})

it("interprets number as epoch seconds if it's bigger than 5000", () => {
const fn = math.compile("date(x)")
expect(fn.evaluate({ x: 5000 })).toEqual(formatDate(new Date(5000 * 1000)))
expect(fn.evaluate({ x: 10000 })).toEqual(formatDate(new Date(10000 * 1000)))
expect(fn.evaluate({ x: 12345 })).toEqual(formatDate(new Date(12345 * 1000)))
expect(fn.evaluate({ x: 5000 })).toEqual(new Date(5000 * 1000))
expect(fn.evaluate({ x: 10000 })).toEqual(new Date(10000 * 1000))
expect(fn.evaluate({ x: 12345 })).toEqual(new Date(12345 * 1000))
})

it("supports month, day, hours, minutes, seconds, and milliseconds arguments", () => {
const fn = math.compile("date(2020, 2, 3, 4, 5, 6, 7)")
expect(fn.evaluate()).toEqual(formatDate(new Date(2020, 1, 3, 4, 5, 6, 7)))
expect(fn.evaluate()).toEqual(new Date(2020, 1, 3, 4, 5, 6, 7))
const fn2 = math.compile("date(2020, 2, 3)")
expect(fn2.evaluate()).toEqual(formatDate(new Date(2020, 1, 3)))
expect(fn2.evaluate()).toEqual(new Date(2020, 1, 3))
const fn3 = math.compile("date(2020, 2)")
expect(fn3.evaluate()).toEqual(formatDate(new Date(2020, 1, 1)))
expect(fn3.evaluate()).toEqual(new Date(2020, 1, 1))
})

it("assumes month is in range 1-12, but 0 is interpreted as January", () => {
const fn = math.compile("date(2020, 0, 1)")
expect(fn.evaluate()).toEqual(formatDate(new Date(2020, 0, 1)))
expect(fn.evaluate()).toEqual(new Date(2020, 0, 1))
const fn2 = math.compile("date(2020, 5, 1)")
expect(fn2.evaluate()).toEqual(formatDate(new Date(2020, 4, 1)))
expect(fn2.evaluate()).toEqual(new Date(2020, 4, 1))
})
})

Expand Down Expand Up @@ -193,17 +192,34 @@ describe("minutes", () => {
})
})

describe("seconds", () => {
it("returns the seconds of the provided date object", () => {
const fn = math.compile("seconds(date(2020, 2, 3, 4, 5, 6, 7))")
expect(fn.evaluate()).toEqual(6)
})

it("returns the seconds of the provided date string", () => {
const fn = math.compile("seconds('2020-02-03T04:05:06.007Z')")
expect(fn.evaluate()).toEqual(6)
})

it("returns undefined if the date is incorrect", () => {
const fn = math.compile("seconds(null)")
expect(fn.evaluate()).toEqual(UNDEF_RESULT)
})
})

describe("now", () => {
it("returns the current date", () => {
const fn = math.compile("now()")
expect(fn.evaluate()).toEqual(formatDate(new Date()))
expect(fn.evaluate().valueOf()).toBeCloseTo(Date.now(), -3)
})
})

describe("today", () => {
it("returns the current date without time", () => {
const fn = math.compile("today()")
const now = new Date()
expect(fn.evaluate()).toEqual(formatDate(new Date(now.getFullYear(), now.getMonth(), now.getDate())))
expect(fn.evaluate()).toEqual(new Date(now.getFullYear(), now.getMonth(), now.getDate()))
})
})
24 changes: 10 additions & 14 deletions v3/src/models/formula/functions/date-functions.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import { FValue } from "../formula-types"
import { UNDEF_RESULT } from "./function-utils"
import { convertToDate, createDate, formatDate } from "../../../utilities/date-utils"
import { convertToDate, createDate } from "../../../utilities/date-utils"
import { t } from "../../../utilities/translation/translate"

function formatDateWithUndefFallback(date: Date | null) {
return formatDate(date) ?? UNDEF_RESULT
}

export const dateFunctions = {
date: {
numOfRequiredArguments: 1,
evaluate: (...args: FValue[]) => formatDateWithUndefFallback(createDate(...args as (string | number)[]))
evaluate: (...args: FValue[]) => createDate(...args as (string | number)[]) ?? UNDEF_RESULT
},

year: {
Expand Down Expand Up @@ -93,24 +89,24 @@ export const dateFunctions = {
evaluate: (date: FValue) => convertToDate(date)?.getMinutes() ?? UNDEF_RESULT
},

// TODO: this revealed an issue in the new implementation (date is converted to string too early), fix it
// when the formatting / storage of dates is refactored.
// seconds: {
// numOfRequiredArguments: 1,
// evaluate: (date: FValue) => convertToDate(date)?.getSeconds() ?? UNDEF_RESULT
// },
seconds: {
numOfRequiredArguments: 1,
evaluate: (date: FValue) => {
return convertToDate(date)?.getSeconds() ?? UNDEF_RESULT
}
},

today: {
numOfRequiredArguments: 0,
evaluate: () => {
const now = new Date()
// eliminate time within the day
return formatDateWithUndefFallback(new Date(now.getFullYear(), now.getMonth(), now.getDate()))
return new Date(now.getFullYear(), now.getMonth(), now.getDate())
}
},

now: {
numOfRequiredArguments: 0,
evaluate: () => formatDateWithUndefFallback(createDate())
evaluate: () => new Date()
}
}
20 changes: 19 additions & 1 deletion v3/src/utilities/date-parser.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fixYear, isDateString, isValidDateSpec, parseDate } from './date-parser'
import { fixYear, isBrowserISOString, isDateString, isValidDateSpec, parseDate } from './date-parser'

describe('Date Parser tests - V2 compatibility', () => {
// These tests are ported from V2 and should always pass unchanged as long as we want to maintain compatibility.
Expand Down Expand Up @@ -223,3 +223,21 @@ describe('fixYear', () => {
expect(fixYear(99)).toEqual(1999)
})
})

describe('isBrowserISOString', () => {
test('returns true for strings that were produced by native Date.toISOString() method', () => {
expect(isBrowserISOString(new Date().toISOString())).toBe(true)
expect(isBrowserISOString(new Date(2023, 7, 17, 15, 30, 45, 123).toISOString())).toBe(true)
expect(isBrowserISOString(new Date(-2023, 7, 17, 15, 30, 45, 123).toISOString())).toBe(true)
expect(isBrowserISOString('2023-08-17T15:30:45.123Z')).toBe(true)
expect(isBrowserISOString('-002023-08-17T15:30:45.123Z')).toBe(true)
})
test('returns false for strings that were not produced by native Date.toISOString() method', () => {
// Still valid ISO date strings, but not produced by native Date.toISOString() method
expect(isBrowserISOString('2023-08-17T15:30:45.123')).toBe(false)
expect(isBrowserISOString('2023-08-17T15:30:45.123Z+07:00')).toBe(false)
expect(isBrowserISOString('2023-08-17T15:30:45.123+07:00')).toBe(false)
expect(isBrowserISOString('2023-08-17T15:30:45.123-07:00')).toBe(false)
expect(isBrowserISOString('002023-08-17T15:30:45.123Z')).toBe(false)
})
})
9 changes: 9 additions & 0 deletions v3/src/utilities/date-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,3 +302,12 @@ export function isDateString(iValue: any, iLoose?: boolean) {
return spec.regex.test(iValue)
}) || (!!iLoose && parseDateV3(iValue) != null)
}

// Regular expression to match ISO 8601 date strings as produced by Date.toISOString.
// Note that this regular expression is more strict than the one used in parseDate (isoDateTimeRE) which supports
// additional formats.
const browserIsoDatePattern = /^([+-]\d{6}|\d{4})-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{3}Z$/

export function isBrowserISOString(value: string): boolean {
return browserIsoDatePattern.test(value)
}

0 comments on commit 86868a3

Please sign in to comment.