From 5ec98d3d84cf02b662e70b2f2502115b3a6a64fd Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Wed, 24 Jun 2026 08:53:08 +0200 Subject: [PATCH] fix(filter): guard isMatchingDateFilter against empty date values (#22029) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. --- .../__tests__/isMatchingDateFilter.test.ts | 40 +++++++++++++++++++ .../filter/utils/isMatchingDateFilter.ts | 7 +++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/packages/twenty-shared/src/utils/filter/utils/__tests__/isMatchingDateFilter.test.ts b/packages/twenty-shared/src/utils/filter/utils/__tests__/isMatchingDateFilter.test.ts index d359bc34a6..9b6f7c44ef 100644 --- a/packages/twenty-shared/src/utils/filter/utils/__tests__/isMatchingDateFilter.test.ts +++ b/packages/twenty-shared/src/utils/filter/utils/__tests__/isMatchingDateFilter.test.ts @@ -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( diff --git a/packages/twenty-shared/src/utils/filter/utils/isMatchingDateFilter.ts b/packages/twenty-shared/src/utils/filter/utils/isMatchingDateFilter.ts index 78492810a8..823e4405f7 100644 --- a/packages/twenty-shared/src/utils/filter/utils/isMatchingDateFilter.ts +++ b/packages/twenty-shared/src/utils/filter/utils/isMatchingDateFilter.ts @@ -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));