From 941c9e7586f70208d09535d3c1bdc84494996469 Mon Sep 17 00:00:00 2001 From: Joseph Chiang Date: Thu, 11 Jun 2026 15:29:10 +1000 Subject: [PATCH] fix: match relation field filters in optimistic & RLS record matchers (#21301) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #21345. ## What It should be caused by the GraphQL optimistic query. `isRecordMatchingFilter` (front, Apollo optimistic cache) and `isRecordMatchingRLSRowLevelPermissionPredicate` (server, RLS) now handle a view filter that targets a **relation field object** (e.g. an "is (not) empty" filter on a relation) by matching against the related record id, instead of throwing. CleanShot 2026-06-08 at 06 44 01@2x CleanShot 2026-06-08 at 06 45 34@2x ## Why Both matchers only implemented the relation **join column** branch (`fooId`) and threw `Not implemented yet, use UUID filter instead on the corresponding "fooId" field` for the relation field itself (`foo`). In practice the UI still stores relation filters keyed on the relation object, so any view with such a filter made every create/update/delete on that object throw: the optimistic effect re-evaluates all active view filters against the changed record and hits the unimplemented branch. Repro: add a self-relation field on People (e.g. "Referred By"), put it in a view filter as "is not empty", then edit any Person. The optimistic update throws. ## Behaviour change | Scenario | Before | After | |---|---|---| | View filter on relation object (`referredBy is not empty`), then edit a record | Throws `Not implemented yet...` | Record matched by related id; update succeeds | | Filter on relation join column (`referredById`) | Worked | Unchanged | ## Test plan ```bash cd packages/twenty-front && npx jest isRecordMatchingFilter cd packages/twenty-server && npx jest is-record-matching-rls-row-level-permission-predicate ``` - [x] Front: relation `is empty` / `is not empty` / `in` match by related id; join-column path still passes (20/20) - [x] Server: relation `is empty` / `is not empty` match by related id (9/9) - [x] `lint:diff-with-main` clean on both packages Co-authored-by: Claude Opus 4.8 Co-authored-by: Félix Malfait --- .../__tests__/isRecordMatchingFilter.test.ts | 92 +++++++++++++++++++ .../utils/isRecordMatchingFilter.ts | 7 +- ...ow-level-permission-predicate.util.spec.ts | 40 ++++++++ ...rls-row-level-permission-predicate.util.ts | 7 +- 4 files changed, 140 insertions(+), 6 deletions(-) diff --git a/packages/twenty-front/src/modules/object-record/record-filter/utils/__tests__/isRecordMatchingFilter.test.ts b/packages/twenty-front/src/modules/object-record/record-filter/utils/__tests__/isRecordMatchingFilter.test.ts index 14f342b251..6b1eb722bc 100644 --- a/packages/twenty-front/src/modules/object-record/record-filter/utils/__tests__/isRecordMatchingFilter.test.ts +++ b/packages/twenty-front/src/modules/object-record/record-filter/utils/__tests__/isRecordMatchingFilter.test.ts @@ -535,4 +535,96 @@ describe('isRecordMatchingFilter', () => { ).toBe(false); }); }); + + describe('Relation Filters', () => { + const accountOwnerId = '20202020-0687-4c41-b707-ed1bfca972a7'; + + const companyWithAccountOwner = { + ...companiesMock[0], + accountOwner: { id: accountOwnerId }, + accountOwnerId, + }; + + const companyWithoutAccountOwner = { + ...companyWithAccountOwner, + accountOwner: null, + accountOwnerId: null, + }; + + it('matches "is not empty" on a relation field by its related record id', () => { + const filter: RecordGqlOperationFilter = { + accountOwner: { is: 'NOT_NULL' }, + }; + + expect( + isRecordMatchingFilter({ + record: companyWithAccountOwner, + filter, + objectMetadataItem: companyMockObjectMetadataItem, + }), + ).toBe(true); + + expect( + isRecordMatchingFilter({ + record: companyWithoutAccountOwner, + filter, + objectMetadataItem: companyMockObjectMetadataItem, + }), + ).toBe(false); + }); + + it('matches "is empty" on a relation field by its related record id', () => { + const filter: RecordGqlOperationFilter = { + accountOwner: { is: 'NULL' }, + }; + + expect( + isRecordMatchingFilter({ + record: companyWithoutAccountOwner, + filter, + objectMetadataItem: companyMockObjectMetadataItem, + }), + ).toBe(true); + + expect( + isRecordMatchingFilter({ + record: companyWithAccountOwner, + filter, + objectMetadataItem: companyMockObjectMetadataItem, + }), + ).toBe(false); + }); + + it('matches an "in" filter on a relation field by its related record id', () => { + expect( + isRecordMatchingFilter({ + record: companyWithAccountOwner, + filter: { accountOwner: { in: [accountOwnerId] } }, + objectMetadataItem: companyMockObjectMetadataItem, + }), + ).toBe(true); + + expect( + isRecordMatchingFilter({ + record: companyWithAccountOwner, + filter: { accountOwner: { in: ['unknown-id'] } }, + objectMetadataItem: companyMockObjectMetadataItem, + }), + ).toBe(false); + }); + + it('still matches the relation join column field', () => { + const filter: RecordGqlOperationFilter = { + accountOwnerId: { is: 'NOT_NULL' }, + }; + + expect( + isRecordMatchingFilter({ + record: companyWithAccountOwner, + filter, + objectMetadataItem: companyMockObjectMetadataItem, + }), + ).toBe(true); + }); + }); }); diff --git a/packages/twenty-front/src/modules/object-record/record-filter/utils/isRecordMatchingFilter.ts b/packages/twenty-front/src/modules/object-record/record-filter/utils/isRecordMatchingFilter.ts index fae0faa2df..66d5712050 100644 --- a/packages/twenty-front/src/modules/object-record/record-filter/utils/isRecordMatchingFilter.ts +++ b/packages/twenty-front/src/modules/object-record/record-filter/utils/isRecordMatchingFilter.ts @@ -434,9 +434,10 @@ export const isRecordMatchingFilter = ({ }); } - throw new Error( - `Not implemented yet, use UUID filter instead on the corresponding "${filterKey}Id" field`, - ); + return isMatchingUUIDFilter({ + uuidFilter: filterValue as UUIDFilter, + value: record[filterKey]?.id ?? null, + }); } case FieldMetadataType.TS_VECTOR: { return isMatchingTSVectorFilter({ diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/is-record-matching-rls-row-level-permission-predicate.util.spec.ts b/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/is-record-matching-rls-row-level-permission-predicate.util.spec.ts index cbeff80d97..36ef6e3cbf 100644 --- a/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/is-record-matching-rls-row-level-permission-predicate.util.spec.ts +++ b/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/is-record-matching-rls-row-level-permission-predicate.util.spec.ts @@ -286,4 +286,44 @@ describe('isRecordMatchingRLSRowLevelPermissionPredicate', () => { expect(result).toBe(true); }); + + it('matches "is not empty" on a relation field by its related record id', () => { + expect( + isRecordMatchingRLSRowLevelPermissionPredicate({ + record: { ...baseRecord, company: { id: 'company-1' } } as ObjectRecord, + filter: { company: { is: 'NOT_NULL' } }, + flatObjectMetadata, + flatFieldMetadataMaps, + }), + ).toBe(true); + + expect( + isRecordMatchingRLSRowLevelPermissionPredicate({ + record: { ...baseRecord, company: null } as ObjectRecord, + filter: { company: { is: 'NOT_NULL' } }, + flatObjectMetadata, + flatFieldMetadataMaps, + }), + ).toBe(false); + }); + + it('matches "is empty" on a relation field by its related record id', () => { + expect( + isRecordMatchingRLSRowLevelPermissionPredicate({ + record: { ...baseRecord, company: null } as ObjectRecord, + filter: { company: { is: 'NULL' } }, + flatObjectMetadata, + flatFieldMetadataMaps, + }), + ).toBe(true); + + expect( + isRecordMatchingRLSRowLevelPermissionPredicate({ + record: { ...baseRecord, company: { id: 'company-1' } } as ObjectRecord, + filter: { company: { is: 'NULL' } }, + flatObjectMetadata, + flatFieldMetadataMaps, + }), + ).toBe(false); + }); }); diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/is-record-matching-rls-row-level-permission-predicate.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/is-record-matching-rls-row-level-permission-predicate.util.ts index 6b0cffc50c..39070baae9 100644 --- a/packages/twenty-server/src/engine/twenty-orm/utils/is-record-matching-rls-row-level-permission-predicate.util.ts +++ b/packages/twenty-server/src/engine/twenty-orm/utils/is-record-matching-rls-row-level-permission-predicate.util.ts @@ -426,9 +426,10 @@ export const isRecordMatchingRLSRowLevelPermissionPredicate = ({ }); } - throw new Error( - `Not implemented yet, use UUID filter instead on the corresponding "${filterKey}Id" field`, - ); + return isMatchingUUIDFilter({ + uuidFilter: filterValue as UUIDFilter, + value: recordFieldValue?.id ?? null, + }); } case FieldMetadataType.TS_VECTOR: { return isMatchingTSVectorFilter({