From d69e4d7008068475b8d6e0730b8063312c0f44c8 Mon Sep 17 00:00:00 2001 From: oniani1 Date: Sat, 21 Mar 2026 21:27:01 +0400 Subject: [PATCH] fix: prevent FIND_RECORDS from silently dropping unresolved filter variables (#18814) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes #18744 — The workflow FIND_RECORDS action silently drops filter conditions when a variable resolves to null/empty, causing the query to return **all records** instead of erroring. **Root cause (three compounding layers):** 1. **`variable-resolver.ts`** — `resolveString` returns `undefined` when a variable lookup fails (e.g., `{{steps.trigger.output.userId}}` where `userId` doesn't exist in context). The return type says `string` but `evalFromContext` actually returns `undefined` at runtime. 2. **`checkIfShouldSkipFiltering.ts`** — Treats `undefined`/`null`/`""` values as "skip this filter." This is correct for the **UI filter builder** (user hasn't finished typing), but wrong for **workflow execution** (variable resolution failed = misconfigured workflow). 3. **`find-records.workflow-action.ts`** — When all filters are silently skipped, `computeRecordGqlOperationFilter` returns `{}` (match everything). The query runs with no filter, returning all records — silently succeeding with wrong results. ## Fix Added validation in `find-records.workflow-action.ts` **after** `resolveInput` but **before** `computeRecordGqlOperationFilter`. For each filter with a value-requiring operand (i.e., not IS_EMPTY, IS_NOT_EMPTY, IS_IN_PAST, IS_IN_FUTURE, IS_TODAY), if the resolved value is `undefined`, `null`, or `""`, throw `INVALID_STEP_INPUT` with a descriptive error message. **Why this approach:** - Scoped to the workflow executor — does **not** break the UI filter builder's intentional skip-on-empty behavior - Does not change shared utilities (`checkIfShouldSkipFiltering`, `resolveInput`) used across the app - Fails fast with a clear error instead of silently returning wrong data - 1 file changed, 23 lines added ## Test plan - [x] Backend typecheck passes - [x] oxlint passes (0 warnings, 0 errors) - [x] Prettier passes - [ ] Manual: Create a workflow with FIND_RECORDS using a variable that doesn't exist → should error with "Filter condition has an empty value after variable resolution" instead of returning all records - [ ] Manual: Create a workflow with FIND_RECORDS using IS_EMPTY operand (no value needed) → should still work correctly --------- Co-authored-by: Claude Opus 4.6 (1M context) Co-authored-by: Charles Bochet --- .../isFilterOperandExpectingValue.test.ts | 6 +- .../utils/isFilterOperandExpectingValue.ts | 24 ---- .../utils/isRecordFilterConsideredEmpty.ts | 20 +-- .../mapRLSOperandToRecordFilterOperand.ts | 47 +++++++ ...gsRolePermissionsObjectLevelObjectForm.tsx | 25 ++-- .../find-records.workflow-action.ts | 12 ++ .../filterOutInvalidRecordFilters.test.ts | 72 +++++++++++ ...sRecordFilterOperandExpectingValue.test.ts | 48 +++++++ .../isRecordFilterValueValid.test.ts | 120 ++++++++++++++++++ .../filter/checkIfShouldSkipFiltering.ts | 27 ---- .../filter/filterOutInvalidRecordFilters.ts | 11 ++ .../twenty-shared/src/utils/filter/index.ts | 4 +- .../isRecordFilterOperandExpectingValue.ts | 17 +++ .../utils/filter/isRecordFilterValueValid.ts | 19 +++ .../turnRecordFilterIntoGqlOperationFilter.ts | 6 +- packages/twenty-shared/src/utils/index.ts | 4 +- 16 files changed, 370 insertions(+), 92 deletions(-) delete mode 100644 packages/twenty-front/src/modules/object-record/object-filter-dropdown/utils/isFilterOperandExpectingValue.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-filter/utils/mapRLSOperandToRecordFilterOperand.ts create mode 100644 packages/twenty-shared/src/utils/filter/__tests__/filterOutInvalidRecordFilters.test.ts create mode 100644 packages/twenty-shared/src/utils/filter/__tests__/isRecordFilterOperandExpectingValue.test.ts create mode 100644 packages/twenty-shared/src/utils/filter/__tests__/isRecordFilterValueValid.test.ts delete mode 100644 packages/twenty-shared/src/utils/filter/checkIfShouldSkipFiltering.ts create mode 100644 packages/twenty-shared/src/utils/filter/filterOutInvalidRecordFilters.ts create mode 100644 packages/twenty-shared/src/utils/filter/isRecordFilterOperandExpectingValue.ts create mode 100644 packages/twenty-shared/src/utils/filter/isRecordFilterValueValid.ts diff --git a/packages/twenty-front/src/modules/object-record/object-filter-dropdown/utils/__tests__/isFilterOperandExpectingValue.test.ts b/packages/twenty-front/src/modules/object-record/object-filter-dropdown/utils/__tests__/isFilterOperandExpectingValue.test.ts index be1f034ea2..9e9f20eac1 100644 --- a/packages/twenty-front/src/modules/object-record/object-filter-dropdown/utils/__tests__/isFilterOperandExpectingValue.test.ts +++ b/packages/twenty-front/src/modules/object-record/object-filter-dropdown/utils/__tests__/isFilterOperandExpectingValue.test.ts @@ -1,8 +1,8 @@ import { ViewFilterOperand } from 'twenty-shared/types'; -import { isFilterOperandExpectingValue } from '@/object-record/object-filter-dropdown/utils/isFilterOperandExpectingValue'; +import { isRecordFilterOperandExpectingValue } from 'twenty-shared/utils'; -describe('isFilterOperandExpectingValue', () => { +describe('isRecordFilterOperandExpectingValue', () => { const testCases = [ { operand: ViewFilterOperand.CONTAINS, expectedResult: true }, { operand: ViewFilterOperand.DOES_NOT_CONTAIN, expectedResult: true }, @@ -24,7 +24,7 @@ describe('isFilterOperandExpectingValue', () => { testCases.forEach(({ operand, expectedResult }) => { it(`should return ${expectedResult} for ViewFilterOperand.${operand}`, () => { - expect(isFilterOperandExpectingValue(operand)).toBe(expectedResult); + expect(isRecordFilterOperandExpectingValue(operand)).toBe(expectedResult); }); }); }); diff --git a/packages/twenty-front/src/modules/object-record/object-filter-dropdown/utils/isFilterOperandExpectingValue.ts b/packages/twenty-front/src/modules/object-record/object-filter-dropdown/utils/isFilterOperandExpectingValue.ts deleted file mode 100644 index 86778d9939..0000000000 --- a/packages/twenty-front/src/modules/object-record/object-filter-dropdown/utils/isFilterOperandExpectingValue.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { ViewFilterOperand } from 'twenty-shared/types'; - -export const isFilterOperandExpectingValue = (operand: ViewFilterOperand) => { - switch (operand) { - case ViewFilterOperand.IS_NOT_NULL: - case ViewFilterOperand.IS_EMPTY: - case ViewFilterOperand.IS_NOT_EMPTY: - case ViewFilterOperand.IS_IN_PAST: - case ViewFilterOperand.IS_IN_FUTURE: - case ViewFilterOperand.IS_TODAY: - return false; - case ViewFilterOperand.IS_NOT: - case ViewFilterOperand.CONTAINS: - case ViewFilterOperand.DOES_NOT_CONTAIN: - case ViewFilterOperand.GREATER_THAN_OR_EQUAL: - case ViewFilterOperand.LESS_THAN_OR_EQUAL: - case ViewFilterOperand.IS_BEFORE: - case ViewFilterOperand.IS_AFTER: - case ViewFilterOperand.IS: - case ViewFilterOperand.IS_RELATIVE: - default: - return true; - } -}; diff --git a/packages/twenty-front/src/modules/object-record/record-filter/utils/isRecordFilterConsideredEmpty.ts b/packages/twenty-front/src/modules/object-record/record-filter/utils/isRecordFilterConsideredEmpty.ts index b431b04fa2..30790d7ee4 100644 --- a/packages/twenty-front/src/modules/object-record/record-filter/utils/isRecordFilterConsideredEmpty.ts +++ b/packages/twenty-front/src/modules/object-record/record-filter/utils/isRecordFilterConsideredEmpty.ts @@ -1,24 +1,8 @@ import { type RecordFilter } from '@/object-record/record-filter/types/RecordFilter'; -import { RecordFilterOperand } from '@/object-record/record-filter/types/RecordFilterOperand'; -import { isDefined } from 'twenty-shared/utils'; +import { isRecordFilterValueValid } from 'twenty-shared/utils'; export const isRecordFilterConsideredEmpty = ( recordFilter: RecordFilter, ): boolean => { - const { value, operand } = recordFilter; - - if ( - (!isDefined(value) || value === '' || value === '[]') && - ![ - RecordFilterOperand.IS_EMPTY, - RecordFilterOperand.IS_NOT_EMPTY, - RecordFilterOperand.IS_IN_PAST, - RecordFilterOperand.IS_IN_FUTURE, - RecordFilterOperand.IS_TODAY, - ].includes(operand) - ) { - return true; - } - - return false; + return !isRecordFilterValueValid(recordFilter); }; diff --git a/packages/twenty-front/src/modules/object-record/record-filter/utils/mapRLSOperandToRecordFilterOperand.ts b/packages/twenty-front/src/modules/object-record/record-filter/utils/mapRLSOperandToRecordFilterOperand.ts new file mode 100644 index 0000000000..e76d73f456 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-filter/utils/mapRLSOperandToRecordFilterOperand.ts @@ -0,0 +1,47 @@ +import type { RecordFilterOperand } from '@/object-record/record-filter/types/RecordFilterOperand'; +import { + RowLevelPermissionPredicateOperand, + ViewFilterOperand, +} from 'twenty-shared/types'; +import { assertUnreachable } from 'twenty-shared/utils'; + +export const mapRLSOperandToRecordFilterOperand = ( + operand: RowLevelPermissionPredicateOperand, +): RecordFilterOperand => { + switch (operand) { + case RowLevelPermissionPredicateOperand.IS: + return ViewFilterOperand.IS; + case RowLevelPermissionPredicateOperand.IS_NOT_NULL: + return ViewFilterOperand.IS_NOT_NULL; + case RowLevelPermissionPredicateOperand.IS_NOT: + return ViewFilterOperand.IS_NOT; + case RowLevelPermissionPredicateOperand.LESS_THAN_OR_EQUAL: + return ViewFilterOperand.LESS_THAN_OR_EQUAL; + case RowLevelPermissionPredicateOperand.GREATER_THAN_OR_EQUAL: + return ViewFilterOperand.GREATER_THAN_OR_EQUAL; + case RowLevelPermissionPredicateOperand.IS_BEFORE: + return ViewFilterOperand.IS_BEFORE; + case RowLevelPermissionPredicateOperand.IS_AFTER: + return ViewFilterOperand.IS_AFTER; + case RowLevelPermissionPredicateOperand.CONTAINS: + return ViewFilterOperand.CONTAINS; + case RowLevelPermissionPredicateOperand.DOES_NOT_CONTAIN: + return ViewFilterOperand.DOES_NOT_CONTAIN; + case RowLevelPermissionPredicateOperand.IS_EMPTY: + return ViewFilterOperand.IS_EMPTY; + case RowLevelPermissionPredicateOperand.IS_NOT_EMPTY: + return ViewFilterOperand.IS_NOT_EMPTY; + case RowLevelPermissionPredicateOperand.IS_RELATIVE: + return ViewFilterOperand.IS_RELATIVE; + case RowLevelPermissionPredicateOperand.IS_IN_PAST: + return ViewFilterOperand.IS_IN_PAST; + case RowLevelPermissionPredicateOperand.IS_IN_FUTURE: + return ViewFilterOperand.IS_IN_FUTURE; + case RowLevelPermissionPredicateOperand.IS_TODAY: + return ViewFilterOperand.IS_TODAY; + case RowLevelPermissionPredicateOperand.VECTOR_SEARCH: + return ViewFilterOperand.VECTOR_SEARCH; + default: + assertUnreachable(operand); + } +}; diff --git a/packages/twenty-front/src/modules/settings/roles/role-permissions/object-level-permissions/object-form/components/SettingsRolePermissionsObjectLevelObjectForm.tsx b/packages/twenty-front/src/modules/settings/roles/role-permissions/object-level-permissions/object-form/components/SettingsRolePermissionsObjectLevelObjectForm.tsx index 4259604e03..c2bd1a708f 100644 --- a/packages/twenty-front/src/modules/settings/roles/role-permissions/object-level-permissions/object-form/components/SettingsRolePermissionsObjectLevelObjectForm.tsx +++ b/packages/twenty-front/src/modules/settings/roles/role-permissions/object-level-permissions/object-form/components/SettingsRolePermissionsObjectLevelObjectForm.tsx @@ -1,6 +1,6 @@ import { currentWorkspaceState } from '@/auth/states/currentWorkspaceState'; import { useObjectMetadataItemById } from '@/object-metadata/hooks/useObjectMetadataItemById'; -import { isFilterOperandExpectingValue } from '@/object-record/object-filter-dropdown/utils/isFilterOperandExpectingValue'; +import { mapRLSOperandToRecordFilterOperand } from '@/object-record/record-filter/utils/mapRLSOperandToRecordFilterOperand'; import { SettingsPageContainer } from '@/settings/components/SettingsPageContainer'; import { SettingsRolePermissionsObjectLevelObjectFieldPermissionTable } from '@/settings/roles/role-permissions/object-level-permissions/field-permissions/components/SettingsRolePermissionsObjectLevelObjectFieldPermissionTable'; import { SettingsRolePermissionsObjectLevelObjectFormObjectLevel } from '@/settings/roles/role-permissions/object-level-permissions/object-form/components/SettingsRolePermissionsObjectLevelObjectFormObjectLevel'; @@ -12,8 +12,12 @@ import { useFeatureFlagsMap } from '@/workspace/hooks/useFeatureFlagsMap'; import { t } from '@lingui/core/macro'; import { useSearchParams } from 'react-router-dom'; import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue'; -import { SettingsPath, type ViewFilterOperand } from 'twenty-shared/types'; -import { getSettingsPath, isDefined } from 'twenty-shared/utils'; +import { SettingsPath } from 'twenty-shared/types'; +import { + getSettingsPath, + isDefined, + isRecordFilterValueValid, +} from 'twenty-shared/utils'; import { Button } from 'twenty-ui/input'; import { useQuery } from '@apollo/client/react'; import { @@ -128,17 +132,10 @@ export const SettingsRolePermissionsObjectLevelObjectForm = ({ return false; } - const operand = predicate.operand as unknown as ViewFilterOperand; - - if (!isFilterOperandExpectingValue(operand)) { - return false; - } - - return ( - !isDefined(predicate.value) || - predicate.value === '' || - predicate.value === '[]' - ); + return !isRecordFilterValueValid({ + operand: mapRLSOperandToRecordFilterOperand(predicate.operand), + value: predicate.value ?? '', + }); }); const isFinishDisabled = hasInvalidPredicate; diff --git a/packages/twenty-server/src/modules/workflow/workflow-executor/workflow-actions/record-crud/find-records.workflow-action.ts b/packages/twenty-server/src/modules/workflow/workflow-executor/workflow-actions/record-crud/find-records.workflow-action.ts index 96a12d05b4..0b5fbbc148 100644 --- a/packages/twenty-server/src/modules/workflow/workflow-executor/workflow-actions/record-crud/find-records.workflow-action.ts +++ b/packages/twenty-server/src/modules/workflow/workflow-executor/workflow-actions/record-crud/find-records.workflow-action.ts @@ -7,6 +7,7 @@ import { import { computeRecordGqlOperationFilter, isDefined, + isRecordFilterValueValid, resolveInput, } from 'twenty-shared/utils'; @@ -94,6 +95,17 @@ export class FindRecordsWorkflowAction implements WorkflowAction { }) .filter(isDefined); + if (workflowActionInput.filter?.recordFilters) { + for (const filter of workflowActionInput.filter.recordFilters) { + if (!isRecordFilterValueValid(filter)) { + throw new WorkflowStepExecutorException( + `Filter condition has an empty value after variable resolution. This likely means a workflow variable could not be resolved. Filter field: ${filter.fieldMetadataId}, operand: ${filter.operand}`, + WorkflowStepExecutorExceptionCode.INVALID_STEP_INPUT, + ); + } + } + } + const gqlOperationFilter = workflowActionInput.filter?.recordFilters && workflowActionInput.filter?.recordFilterGroups diff --git a/packages/twenty-shared/src/utils/filter/__tests__/filterOutInvalidRecordFilters.test.ts b/packages/twenty-shared/src/utils/filter/__tests__/filterOutInvalidRecordFilters.test.ts new file mode 100644 index 0000000000..3bede36c8d --- /dev/null +++ b/packages/twenty-shared/src/utils/filter/__tests__/filterOutInvalidRecordFilters.test.ts @@ -0,0 +1,72 @@ +import { ViewFilterOperand } from '@/types/ViewFilterOperand'; + +import { filterOutInvalidRecordFilters } from '../filterOutInvalidRecordFilters'; + +describe('filterOutInvalidRecordFilters', () => { + it('should keep filters with valid values', () => { + const filters = [ + { operand: ViewFilterOperand.IS, value: 'some-value' }, + { operand: ViewFilterOperand.CONTAINS, value: 'search-term' }, + ]; + + expect(filterOutInvalidRecordFilters(filters)).toEqual(filters); + }); + + it('should remove filters with empty values for value-requiring operands', () => { + const filters = [ + { operand: ViewFilterOperand.IS, value: '' }, + { operand: ViewFilterOperand.CONTAINS, value: 'keep-me' }, + { operand: ViewFilterOperand.IS_NOT, value: '[]' }, + ]; + + expect(filterOutInvalidRecordFilters(filters)).toEqual([ + { operand: ViewFilterOperand.CONTAINS, value: 'keep-me' }, + ]); + }); + + it('should keep filters with operands that do not require a value', () => { + const filters = [ + { operand: ViewFilterOperand.IS_EMPTY, value: '' }, + { operand: ViewFilterOperand.IS_NOT_EMPTY, value: '' }, + { operand: ViewFilterOperand.IS_NOT_NULL, value: '' }, + { operand: ViewFilterOperand.IS_IN_PAST, value: '' }, + { operand: ViewFilterOperand.IS_IN_FUTURE, value: '' }, + { operand: ViewFilterOperand.IS_TODAY, value: '' }, + ]; + + expect(filterOutInvalidRecordFilters(filters)).toEqual(filters); + }); + + it('should return an empty array when all filters are invalid', () => { + const filters = [ + { operand: ViewFilterOperand.IS, value: '' }, + { + operand: ViewFilterOperand.IS_NOT, + value: undefined as unknown as string, + }, + ]; + + expect(filterOutInvalidRecordFilters(filters)).toEqual([]); + }); + + it('should return an empty array for empty input', () => { + expect(filterOutInvalidRecordFilters([])).toEqual([]); + }); + + it('should preserve extra properties on the filter objects', () => { + const filters = [ + { + id: 'filter-1', + operand: ViewFilterOperand.IS, + value: 'valid', + fieldMetadataId: 'field-1', + }, + ]; + + const result = filterOutInvalidRecordFilters(filters); + + expect(result).toEqual(filters); + expect(result[0].id).toBe('filter-1'); + expect(result[0].fieldMetadataId).toBe('field-1'); + }); +}); diff --git a/packages/twenty-shared/src/utils/filter/__tests__/isRecordFilterOperandExpectingValue.test.ts b/packages/twenty-shared/src/utils/filter/__tests__/isRecordFilterOperandExpectingValue.test.ts new file mode 100644 index 0000000000..dd7c2d1188 --- /dev/null +++ b/packages/twenty-shared/src/utils/filter/__tests__/isRecordFilterOperandExpectingValue.test.ts @@ -0,0 +1,48 @@ +import { ViewFilterOperand } from '@/types/ViewFilterOperand'; + +import { isRecordFilterOperandExpectingValue } from '../isRecordFilterOperandExpectingValue'; + +describe('isRecordFilterOperandExpectingValue', () => { + const operandsNotExpectingValue: ViewFilterOperand[] = [ + ViewFilterOperand.IS_NOT_NULL, + ViewFilterOperand.IS_EMPTY, + ViewFilterOperand.IS_NOT_EMPTY, + ViewFilterOperand.IS_IN_PAST, + ViewFilterOperand.IS_IN_FUTURE, + ViewFilterOperand.IS_TODAY, + ]; + + const operandsExpectingValue: ViewFilterOperand[] = [ + ViewFilterOperand.IS, + ViewFilterOperand.IS_NOT, + ViewFilterOperand.CONTAINS, + ViewFilterOperand.DOES_NOT_CONTAIN, + ViewFilterOperand.GREATER_THAN_OR_EQUAL, + ViewFilterOperand.LESS_THAN_OR_EQUAL, + ViewFilterOperand.IS_BEFORE, + ViewFilterOperand.IS_AFTER, + ViewFilterOperand.IS_RELATIVE, + ViewFilterOperand.VECTOR_SEARCH, + ]; + + it.each(operandsNotExpectingValue)( + 'should return false for %s', + (operand) => { + expect(isRecordFilterOperandExpectingValue(operand)).toBe(false); + }, + ); + + it.each(operandsExpectingValue)('should return true for %s', (operand) => { + expect(isRecordFilterOperandExpectingValue(operand)).toBe(true); + }); + + it('should cover all ViewFilterOperand values', () => { + const allOperands = Object.values(ViewFilterOperand); + const coveredOperands = [ + ...operandsNotExpectingValue, + ...operandsExpectingValue, + ]; + + expect(coveredOperands.sort()).toEqual(allOperands.sort()); + }); +}); diff --git a/packages/twenty-shared/src/utils/filter/__tests__/isRecordFilterValueValid.test.ts b/packages/twenty-shared/src/utils/filter/__tests__/isRecordFilterValueValid.test.ts new file mode 100644 index 0000000000..175c8c2f20 --- /dev/null +++ b/packages/twenty-shared/src/utils/filter/__tests__/isRecordFilterValueValid.test.ts @@ -0,0 +1,120 @@ +import { ViewFilterOperand } from '@/types/ViewFilterOperand'; + +import { isRecordFilterValueValid } from '../isRecordFilterValueValid'; + +describe('isRecordFilterValueValid', () => { + describe('operands not expecting a value', () => { + const operandsNotExpectingValue = [ + ViewFilterOperand.IS_NOT_NULL, + ViewFilterOperand.IS_EMPTY, + ViewFilterOperand.IS_NOT_EMPTY, + ViewFilterOperand.IS_IN_PAST, + ViewFilterOperand.IS_IN_FUTURE, + ViewFilterOperand.IS_TODAY, + ]; + + it.each(operandsNotExpectingValue)( + 'should return true for %s regardless of value', + (operand) => { + expect(isRecordFilterValueValid({ operand, value: '' })).toBe(true); + expect( + isRecordFilterValueValid({ + operand, + value: undefined as unknown as string, + }), + ).toBe(true); + expect(isRecordFilterValueValid({ operand, value: '[]' })).toBe(true); + expect(isRecordFilterValueValid({ operand, value: 'some-value' })).toBe( + true, + ); + }, + ); + }); + + describe('operands expecting a value', () => { + const operandsExpectingValue = [ + ViewFilterOperand.IS, + ViewFilterOperand.IS_NOT, + ViewFilterOperand.CONTAINS, + ViewFilterOperand.DOES_NOT_CONTAIN, + ViewFilterOperand.GREATER_THAN_OR_EQUAL, + ViewFilterOperand.LESS_THAN_OR_EQUAL, + ViewFilterOperand.IS_BEFORE, + ViewFilterOperand.IS_AFTER, + ViewFilterOperand.IS_RELATIVE, + ]; + + it.each(operandsExpectingValue)( + 'should return true for %s with a valid value', + (operand) => { + expect(isRecordFilterValueValid({ operand, value: 'some-value' })).toBe( + true, + ); + }, + ); + + it.each(operandsExpectingValue)( + 'should return false for %s with empty string', + (operand) => { + expect(isRecordFilterValueValid({ operand, value: '' })).toBe(false); + }, + ); + + it.each(operandsExpectingValue)( + 'should return false for %s with undefined', + (operand) => { + expect( + isRecordFilterValueValid({ + operand, + value: undefined as unknown as string, + }), + ).toBe(false); + }, + ); + + it.each(operandsExpectingValue)( + 'should return false for %s with empty array string "[]"', + (operand) => { + expect(isRecordFilterValueValid({ operand, value: '[]' })).toBe(false); + }, + ); + }); + + describe('edge cases', () => { + it('should return true for "0" as a valid value', () => { + expect( + isRecordFilterValueValid({ + operand: ViewFilterOperand.IS, + value: '0', + }), + ).toBe(true); + }); + + it('should return true for "false" as a valid value', () => { + expect( + isRecordFilterValueValid({ + operand: ViewFilterOperand.IS, + value: 'false', + }), + ).toBe(true); + }); + + it('should return true for whitespace-only string', () => { + expect( + isRecordFilterValueValid({ + operand: ViewFilterOperand.IS, + value: ' ', + }), + ).toBe(true); + }); + + it('should return true for a non-empty array string', () => { + expect( + isRecordFilterValueValid({ + operand: ViewFilterOperand.IS, + value: '["value1","value2"]', + }), + ).toBe(true); + }); + }); +}); diff --git a/packages/twenty-shared/src/utils/filter/checkIfShouldSkipFiltering.ts b/packages/twenty-shared/src/utils/filter/checkIfShouldSkipFiltering.ts deleted file mode 100644 index 4fe7e4e77c..0000000000 --- a/packages/twenty-shared/src/utils/filter/checkIfShouldSkipFiltering.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { ViewFilterOperand as RecordFilterOperand } from '@/types'; -import { isDefined } from '@/utils'; -import { isEmptinessOperand, type RecordFilter } from '@/utils/filter'; - -type CheckIfShouldSkipFilteringParams = { - recordFilter: Pick; -}; - -export const checkIfShouldSkipFiltering = ({ - recordFilter, -}: CheckIfShouldSkipFilteringParams) => { - const isAnEmptinessOperand = isEmptinessOperand(recordFilter.operand); - - const isDateOperandWithoutValue = [ - RecordFilterOperand.IS_IN_PAST, - RecordFilterOperand.IS_IN_FUTURE, - RecordFilterOperand.IS_TODAY, - ].includes(recordFilter.operand); - - const isFilterValueEmpty = - !isDefined(recordFilter.value) || recordFilter.value === ''; - - const shouldSkipFiltering = - !isAnEmptinessOperand && !isDateOperandWithoutValue && isFilterValueEmpty; - - return shouldSkipFiltering; -}; diff --git a/packages/twenty-shared/src/utils/filter/filterOutInvalidRecordFilters.ts b/packages/twenty-shared/src/utils/filter/filterOutInvalidRecordFilters.ts new file mode 100644 index 0000000000..b496afc167 --- /dev/null +++ b/packages/twenty-shared/src/utils/filter/filterOutInvalidRecordFilters.ts @@ -0,0 +1,11 @@ +import { type ViewFilterOperand } from '@/types'; + +import { isRecordFilterValueValid } from './isRecordFilterValueValid'; + +export const filterOutInvalidRecordFilters = < + T extends { operand: ViewFilterOperand; value: string }, +>( + recordFilters: T[], +): T[] => { + return recordFilters.filter(isRecordFilterValueValid); +}; diff --git a/packages/twenty-shared/src/utils/filter/index.ts b/packages/twenty-shared/src/utils/filter/index.ts index ab2874e647..05ce5692b1 100644 --- a/packages/twenty-shared/src/utils/filter/index.ts +++ b/packages/twenty-shared/src/utils/filter/index.ts @@ -1,11 +1,13 @@ export * from './checkIfShouldComputeEmptinessFilter'; -export * from './checkIfShouldSkipFiltering'; +export * from './filterOutInvalidRecordFilters'; export * from './compute-record-gql-operation-filter/for-composite-field/computeGqlOperationFilterForEmails'; export * from './compute-record-gql-operation-filter/for-composite-field/computeGqlOperationFilterForLinks'; export * from './computeEmptyGqlOperationFilterForEmails'; export * from './computeEmptyGqlOperationFilterForLinks'; export * from './computeRecordGqlOperationFilter'; export * from './isEmptinessOperand'; +export * from './isRecordFilterOperandExpectingValue'; +export * from './isRecordFilterValueValid'; export * from './turnAnyFieldFilterIntoRecordGqlFilter'; export * from './turnRecordFilterGroupIntoGqlOperationFilter'; export * from './turnRecordFilterIntoGqlOperationFilter'; diff --git a/packages/twenty-shared/src/utils/filter/isRecordFilterOperandExpectingValue.ts b/packages/twenty-shared/src/utils/filter/isRecordFilterOperandExpectingValue.ts new file mode 100644 index 0000000000..295f6188ce --- /dev/null +++ b/packages/twenty-shared/src/utils/filter/isRecordFilterOperandExpectingValue.ts @@ -0,0 +1,17 @@ +import { ViewFilterOperand } from '@/types'; + +export const isRecordFilterOperandExpectingValue = ( + operand: ViewFilterOperand, +): boolean => { + switch (operand) { + case ViewFilterOperand.IS_NOT_NULL: + case ViewFilterOperand.IS_EMPTY: + case ViewFilterOperand.IS_NOT_EMPTY: + case ViewFilterOperand.IS_IN_PAST: + case ViewFilterOperand.IS_IN_FUTURE: + case ViewFilterOperand.IS_TODAY: + return false; + default: + return true; + } +}; diff --git a/packages/twenty-shared/src/utils/filter/isRecordFilterValueValid.ts b/packages/twenty-shared/src/utils/filter/isRecordFilterValueValid.ts new file mode 100644 index 0000000000..ad0d3f3a03 --- /dev/null +++ b/packages/twenty-shared/src/utils/filter/isRecordFilterValueValid.ts @@ -0,0 +1,19 @@ +import { type ViewFilterOperand } from '@/types'; +import { isDefined } from '@/utils'; + +import { isRecordFilterOperandExpectingValue } from './isRecordFilterOperandExpectingValue'; + +export const isRecordFilterValueValid = (recordFilter: { + operand: ViewFilterOperand; + value: string; +}): boolean => { + if (!isRecordFilterOperandExpectingValue(recordFilter.operand)) { + return true; + } + + return ( + isDefined(recordFilter.value) && + recordFilter.value !== '' && + recordFilter.value !== '[]' + ); +}; diff --git a/packages/twenty-shared/src/utils/filter/turnRecordFilterIntoGqlOperationFilter.ts b/packages/twenty-shared/src/utils/filter/turnRecordFilterIntoGqlOperationFilter.ts index 39fb0d7884..2a1a27c9b5 100644 --- a/packages/twenty-shared/src/utils/filter/turnRecordFilterIntoGqlOperationFilter.ts +++ b/packages/twenty-shared/src/utils/filter/turnRecordFilterIntoGqlOperationFilter.ts @@ -40,12 +40,12 @@ import { import { type DateTimeFilter } from '@/types/RecordGqlOperationFilter'; import { checkIfShouldComputeEmptinessFilter, - checkIfShouldSkipFiltering, CustomError, getFilterTypeFromFieldType, getNextPeriodStart, getPeriodStart, isDefined, + isRecordFilterValueValid, resolveDateFilter, resolveDateTimeFilter, resolveRelativeDateFilterStringified, @@ -83,9 +83,7 @@ export const turnRecordFilterIntoRecordGqlOperationFilter = ({ return; } - const shouldSkipFiltering = checkIfShouldSkipFiltering({ recordFilter }); - - if (shouldSkipFiltering) { + if (!isRecordFilterValueValid(recordFilter)) { return; } diff --git a/packages/twenty-shared/src/utils/index.ts b/packages/twenty-shared/src/utils/index.ts index 1bedcb0288..ebdf8072ed 100644 --- a/packages/twenty-shared/src/utils/index.ts +++ b/packages/twenty-shared/src/utils/index.ts @@ -51,7 +51,6 @@ export { isFieldMetadataSelectKind } from './fieldMetadata/isFieldMetadataSelect export { isFieldMetadataTextKind } from './fieldMetadata/isFieldMetadataTextKind'; export { extractFolderPathFilenameAndTypeOrThrow } from './files/extractFolderPathFilenameAndTypeOrThrow.util'; export { checkIfShouldComputeEmptinessFilter } from './filter/checkIfShouldComputeEmptinessFilter'; -export { checkIfShouldSkipFiltering } from './filter/checkIfShouldSkipFiltering'; export { computeGqlOperationFilterForEmails } from './filter/compute-record-gql-operation-filter/for-composite-field/computeGqlOperationFilterForEmails'; export { computeGqlOperationFilterForLinks } from './filter/compute-record-gql-operation-filter/for-composite-field/computeGqlOperationFilterForLinks'; export { computeEmptyGqlOperationFilterForEmails } from './filter/computeEmptyGqlOperationFilterForEmails'; @@ -89,7 +88,10 @@ export { resolveRelativeDateTimeFilter } from './filter/dates/utils/resolveRelat export { resolveRelativeDateTimeFilterStringified } from './filter/dates/utils/resolveRelativeDateTimeFilterStringified'; export { subUnitFromDateTime } from './filter/dates/utils/subUnitFromDateTime'; export { subUnitFromZonedDateTime } from './filter/dates/utils/subUnitFromZonedDateTime'; +export { filterOutInvalidRecordFilters } from './filter/filterOutInvalidRecordFilters'; export { isEmptinessOperand } from './filter/isEmptinessOperand'; +export { isRecordFilterOperandExpectingValue } from './filter/isRecordFilterOperandExpectingValue'; +export { isRecordFilterValueValid } from './filter/isRecordFilterValueValid'; export { turnAnyFieldFilterIntoRecordGqlFilter } from './filter/turnAnyFieldFilterIntoRecordGqlFilter'; export type { RecordFilter,