fix(filter): guard isMatchingDateFilter against empty date values (#22029)
## Symptom
On the Opportunities **board (kanban)** view, creating or updating *any*
opportunity randomly crashed with:
```
Uncaught (in promise) Cannot read properties of null (reading 'split')
...
at isMatchingDateFilter
at isRecordMatchingFilter
at opportunitiesGroupBy (group-by optimistic effect)
at createOneRecord
```
Reported in quality-feedbacks as *"Can't update the Opportunity"* —
"happens randomly, no specific path." The randomness is the tell: it
depends on the **view's filter configuration**, not on which record you
edit.
## What runs on create/update
Create/update trigger an **optimistic cache update**. On a board view
that means recomputing which group each record belongs to (the
`opportunitiesGroupBy` field in the trace). To do that, the group-by
optimistic effect re-evaluates **every record in the affected groups
against the view's filters** via `isRecordMatchingFilter`, which walks
the AND/OR filter tree and dispatches each leaf to a per-field-type
matcher (`isMatchingStringFilter`, `isMatchingSelectFilter`,
`isMatchingDateFilter`, …).
## Root cause
`isMatchingDateFilter` passed the record value straight to date-fns
`parseISO` for the `eq`/`neq`/`gt`/`gte`/`lt`/`lte` operators:
```ts
case dateFilter.gte !== undefined: {
const valueDate = parseISO(value); // value = record[fieldName], declared `string` but actually nullable
...
}
```
`parseISO` parses an ISO string by first calling `argument.split(...)`
internally, so `parseISO(null)` runs `null.split(...)` → **`Cannot read
properties of null (reading 'split')`**. That's the three-deep
`utils`-chunk frame in the minified trace: `isMatchingDateFilter` →
`parseISO` → date-fns `splitDateString`.
`value` is `null` whenever a record has an **empty date field** (e.g. an
opportunity with no Close date). So the crash fires only when **both**
hold:
1. the current view has a **date filter**
(`gt`/`gte`/`lt`/`lte`/`eq`/`neq`) on some date field, **and**
2. at least one opportunity in view has that date field **empty**.
That's the "randomness" — purely a function of the view config and which
records have blank dates. The `is: NULL` operator never crashed (it
checks `value === null` before `parseISO`); only the value-parsing
operators were exposed. Sibling matchers (`isMatchingTSVectorFilter`,
`isMatchingRatingFilter`, `isMatchingSelectFilter`) already tolerate
`null` — the date matcher was the odd one out, and its `value: string`
type masked the real nullability.
## Fix
Widen the param type to the truth (`string | null | undefined`) and
guard the empty case up front:
```ts
if (!isDefined(value)) {
return dateFilter.is === 'NULL';
}
```
Semantics:
- empty value + `is: NULL` → `true` (it *is* null)
- empty value + every other operator (incl. `is: NOT_NULL`) → `false`
The `false` is the *correct* answer, not just crash avoidance: it
mirrors SQL three-valued logic where `NULL > '2024-01-01'` is `UNKNOWN`
and the row is excluded. So the optimistic match now agrees with what
the backend query returns, and a blank-date record groups the same way
before and after the server round-trip.
## Tests
Added regression cases to `isMatchingDateFilter.test.ts` running `null`
and `undefined` through every operator (assert no throw + correct
boolean). These throw without the guard.
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
import { type DateFilter } from '@/types';
|
||||
import { isMatchingDateFilter } from '@/utils/filter/utils/isMatchingDateFilter';
|
||||
|
||||
describe('isMatchingDateFilter', () => {
|
||||
@@ -82,6 +83,45 @@ describe('isMatchingDateFilter', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('null or undefined value', () => {
|
||||
it.each([null, undefined])(
|
||||
'does not throw and returns false for comparison operators (value: %s)',
|
||||
(value) => {
|
||||
const comparisonFilters: DateFilter[] = [
|
||||
{ eq: testDate },
|
||||
{ neq: testDate },
|
||||
{ gt: testDate },
|
||||
{ gte: testDate },
|
||||
{ lt: testDate },
|
||||
{ lte: testDate },
|
||||
{ in: [testDate] },
|
||||
];
|
||||
|
||||
for (const dateFilter of comparisonFilters) {
|
||||
expect(isMatchingDateFilter({ dateFilter, value })).toBe(false);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.each([null, undefined])(
|
||||
'matches "is: NULL" for an empty value (value: %s)',
|
||||
(value) => {
|
||||
expect(
|
||||
isMatchingDateFilter({ dateFilter: { is: 'NULL' }, value }),
|
||||
).toBe(true);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([null, undefined])(
|
||||
'does not match "is: NOT_NULL" for an empty value (value: %s)',
|
||||
(value) => {
|
||||
expect(
|
||||
isMatchingDateFilter({ dateFilter: { is: 'NOT_NULL' }, value }),
|
||||
).toBe(false);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe('gt', () => {
|
||||
it('value is greater than gt filter', () => {
|
||||
expect(
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { type DateFilter } from '@/types';
|
||||
import { isDefined } from '@/utils';
|
||||
import { isAfter, isBefore, isEqual, parseISO } from 'date-fns';
|
||||
|
||||
export const isMatchingDateFilter = ({
|
||||
@@ -6,8 +7,12 @@ export const isMatchingDateFilter = ({
|
||||
value,
|
||||
}: {
|
||||
dateFilter: DateFilter;
|
||||
value: string;
|
||||
value: string | null | undefined;
|
||||
}) => {
|
||||
if (!isDefined(value)) {
|
||||
return dateFilter.is === 'NULL';
|
||||
}
|
||||
|
||||
switch (true) {
|
||||
case dateFilter.eq !== undefined: {
|
||||
return isEqual(parseISO(value), parseISO(dateFilter.eq));
|
||||
|
||||
Reference in New Issue
Block a user