From 7b9939b43e02308ec7c984bf434808a3843b1bcc Mon Sep 17 00:00:00 2001 From: neo773 <62795688+neo773@users.noreply.github.com> Date: Tue, 10 Mar 2026 21:09:21 +0530 Subject: [PATCH] fix: validate input before formatting in MultiItemFieldInput (#18334) Reorder validateInput to run before formatInput to prevent parsePhoneNumber from throwing INVALID_COUNTRY on bad input. Fixes TWENTY-FRONT-5RQ /closes https://github.com/twentyhq/twenty/issues/17670 --------- Co-authored-by: Charles Bochet --- .../input/components/MultiItemFieldInput.tsx | 97 ++++++++---------- .../computeUpdatedMultiItemFieldItems.test.ts | 99 +++++++++++++++++++ .../sanitizeAndValidateInput.test.ts | 40 ++++++++ .../computeUpdatedMultiItemFieldItems.ts | 37 +++++++ .../input/utils/sanitizeAndValidateInput.ts | 17 ++++ 5 files changed, 234 insertions(+), 56 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/__tests__/computeUpdatedMultiItemFieldItems.test.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/__tests__/sanitizeAndValidateInput.test.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/computeUpdatedMultiItemFieldItems.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/sanitizeAndValidateInput.ts diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/MultiItemFieldInput.tsx b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/MultiItemFieldInput.tsx index f22ff019a3..210a257f2f 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/MultiItemFieldInput.tsx +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/MultiItemFieldInput.tsx @@ -6,6 +6,8 @@ import { MultiItemBaseInput, type MultiItemBaseInputProps, } from '@/object-record/record-field/ui/meta-types/input/components/MultiItemBaseInput'; +import { computeUpdatedMultiItemFieldItems } from '@/object-record/record-field/ui/meta-types/input/utils/computeUpdatedMultiItemFieldItems'; +import { sanitizeAndValidateInput } from '@/object-record/record-field/ui/meta-types/input/utils/sanitizeAndValidateInput'; import { RecordFieldComponentInstanceContext } from '@/object-record/record-field/ui/states/contexts/RecordFieldComponentInstanceContext'; import { type PhoneRecord } from '@/object-record/record-field/ui/types/FieldMetadata'; import { DropdownContent } from '@/ui/layout/dropdown/components/DropdownContent'; @@ -18,6 +20,7 @@ import { useHotkeysOnFocusedElement } from '@/ui/utilities/hotkey/hooks/useHotke import { useListenClickOutside } from '@/ui/utilities/pointer-event/hooks/useListenClickOutside'; import { useAvailableComponentInstanceIdOrThrow } from '@/ui/utilities/state/component-state/hooks/useAvailableComponentInstanceIdOrThrow'; import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue'; +import { isNonEmptyString } from '@sniptt/guards'; import { CustomError, isDefined } from 'twenty-shared/utils'; import { IconCheck, IconPlus } from 'twenty-ui/display'; import { LightIconButton } from 'twenty-ui/input'; @@ -88,13 +91,13 @@ export const MultiItemFieldInput = ({ ) { return; } - const { isValid } = validateInputAndComputeUpdatedItems(); + const { isValid, updatedItems } = validateInputAndComputeUpdatedItems(); - if (!isValid && isInputDisplayed) { + if (!isValid) { return; } - handleSubmitChanges(); + onChange(updatedItems); onClickOutside(items, event); }, listenerId: instanceId, @@ -214,7 +217,7 @@ export const MultiItemFieldInput = ({ return; } - handleSubmitChanges(); + onChange(updatedItems); if (shouldAutoEnterBecauseOnlyOneItemIsAllowed) { onEnter(updatedItems); } @@ -223,62 +226,51 @@ export const MultiItemFieldInput = ({ setInputValue(''); }; - const handleSubmitChanges = () => { - const { isValid, updatedItems } = validateInputAndComputeUpdatedItems(); - if (!isValid) { - return; + const showInputIfNoItemsRemain = (remainingItems: T[]) => { + const shouldShowInput = + remainingItems.length === 0 && !isDefined(onAddClick); + setIsInputDisplayed(shouldShowInput); + setIsAddingNewItem(false); + if (shouldShowInput) { + setInputValue(''); } - - onChange(updatedItems); }; const validateInputAndComputeUpdatedItems = (): { isValid: boolean; updatedItems: T[]; } => { - const sanitizedInput = inputValue.trim(); + const { sanitizedInput, isValid, errorMessage } = sanitizeAndValidateInput( + inputValue, + validateInput, + ); - if (sanitizedInput === '' && isAddingNewItem) { - return { isValid: true, updatedItems: items }; + if (!isValid) { + onError?.(true, items); + setErrorData({ isValid: false, errorMessage }); + return { isValid: false, updatedItems: items }; } - if (sanitizedInput === '' && shouldAutoEnterBecauseOnlyOneItemIsAllowed) { - return { - isValid: true, - updatedItems: [], - }; + const editingIndex = isAddingNewItem ? null : itemToEditIndex; + + const updatedItems = computeUpdatedMultiItemFieldItems({ + sanitizedInput, + items, + editingIndex, + singleItemMode: shouldAutoEnterBecauseOnlyOneItemIsAllowed, + formatInput, + }); + + const isItemDeletion = + !isNonEmptyString(sanitizedInput) && + isDefined(editingIndex) && + !shouldAutoEnterBecauseOnlyOneItemIsAllowed; + + if (isItemDeletion) { + showInputIfNoItemsRemain(updatedItems); } - if (sanitizedInput === '' && !isAddingNewItem) { - handleDeleteItem(itemToEditIndex); - return { - isValid: true, - updatedItems: toSpliced(items, itemToEditIndex, 1), - }; - } - - const newItem = formatInput - ? formatInput( - sanitizedInput, - isAddingNewItem ? undefined : itemToEditIndex, - ) - : (sanitizedInput as unknown as T); - - if (validateInput !== undefined) { - const validationData = validateInput(sanitizedInput) ?? { isValid: true }; - if (!validationData.isValid) { - onError?.(true, items); - setErrorData(validationData); - return { isValid: false, updatedItems: items }; - } - } - - return { - isValid: true, - updatedItems: isAddingNewItem - ? [...items, newItem] - : toSpliced(items, itemToEditIndex, 1, newItem), - }; + return { isValid: true, updatedItems }; }; const handleSetPrimaryItem = (index: number) => { @@ -289,14 +281,7 @@ export const MultiItemFieldInput = ({ const handleDeleteItem = (index: number) => { const updatedItems = toSpliced(items, index, 1); onChange(updatedItems); - - const shouldShowInputAfterDeletion = - updatedItems.length === 0 && !isDefined(onAddClick); - setIsInputDisplayed(shouldShowInputAfterDeletion); - setIsAddingNewItem(false); - if (shouldShowInputAfterDeletion) { - setInputValue(''); - } + showInputIfNoItemsRemain(updatedItems); }; const handleEscape = () => { diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/__tests__/computeUpdatedMultiItemFieldItems.test.ts b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/__tests__/computeUpdatedMultiItemFieldItems.test.ts new file mode 100644 index 0000000000..1cab1a564c --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/__tests__/computeUpdatedMultiItemFieldItems.test.ts @@ -0,0 +1,99 @@ +import { computeUpdatedMultiItemFieldItems } from '@/object-record/record-field/ui/meta-types/input/utils/computeUpdatedMultiItemFieldItems'; + +const items = ['a', 'b', 'c']; + +describe('computeUpdatedMultiItemFieldItems', () => { + describe('empty input', () => { + it('should return items unchanged when adding', () => { + const result = computeUpdatedMultiItemFieldItems({ + sanitizedInput: '', + items, + editingIndex: null, + singleItemMode: false, + }); + + expect(result).toBe(items); + }); + + it('should clear all items in single-item mode', () => { + const result = computeUpdatedMultiItemFieldItems({ + sanitizedInput: '', + items, + editingIndex: 0, + singleItemMode: true, + }); + + expect(result).toEqual([]); + }); + + it('should remove the edited item', () => { + const result = computeUpdatedMultiItemFieldItems({ + sanitizedInput: '', + items, + editingIndex: 1, + singleItemMode: false, + }); + + expect(result).toEqual(['a', 'c']); + }); + }); + + describe('adding items', () => { + it('should append the new item', () => { + const result = computeUpdatedMultiItemFieldItems({ + sanitizedInput: 'd', + items, + editingIndex: null, + singleItemMode: false, + }); + + expect(result).toEqual(['a', 'b', 'c', 'd']); + }); + + it('should use formatInput to transform the value', () => { + type Link = { url: string; label: string }; + const formatInput = jest.fn((input: string) => ({ + url: input, + label: input, + })); + + const result = computeUpdatedMultiItemFieldItems({ + sanitizedInput: 'example.com', + items: [], + editingIndex: null, + singleItemMode: false, + formatInput, + }); + + expect(formatInput).toHaveBeenCalledWith('example.com', undefined); + expect(result).toEqual([{ url: 'example.com', label: 'example.com' }]); + }); + }); + + describe('editing items', () => { + it('should replace at editingIndex', () => { + const result = computeUpdatedMultiItemFieldItems({ + sanitizedInput: 'x', + items, + editingIndex: 1, + singleItemMode: false, + }); + + expect(result).toEqual(['a', 'x', 'c']); + }); + + it('should pass editingIndex to formatInput', () => { + const formatInput = jest.fn((input: string) => input); + + computeUpdatedMultiItemFieldItems({ + sanitizedInput: 'edited', + items, + editingIndex: 2, + singleItemMode: false, + formatInput, + }); + + expect(formatInput).toHaveBeenCalledWith('edited', 2); + }); + }); +}); diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/__tests__/sanitizeAndValidateInput.test.ts b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/__tests__/sanitizeAndValidateInput.test.ts new file mode 100644 index 0000000000..6448634d72 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/__tests__/sanitizeAndValidateInput.test.ts @@ -0,0 +1,40 @@ +import { sanitizeAndValidateInput } from '@/object-record/record-field/ui/meta-types/input/utils/sanitizeAndValidateInput'; + +describe('sanitizeAndValidateInput', () => { + it('should trim whitespace from input', () => { + expect(sanitizeAndValidateInput(' hello ').sanitizedInput).toBe('hello'); + expect(sanitizeAndValidateInput(' ').sanitizedInput).toBe(''); + }); + + it('should be valid when no validator is provided', () => { + const result = sanitizeAndValidateInput('anything'); + + expect(result.isValid).toBe(true); + expect(result.errorMessage).toBe(''); + }); + + it('should skip validation for empty input', () => { + const validateInput = jest.fn(() => ({ + isValid: false, + errorMessage: 'fail', + })); + + const result = sanitizeAndValidateInput('', validateInput); + + expect(validateInput).not.toHaveBeenCalled(); + expect(result.isValid).toBe(true); + }); + + it('should validate the trimmed input and propagate the result', () => { + const validateInput = jest.fn(() => ({ + isValid: false, + errorMessage: 'Invalid format', + })); + + const result = sanitizeAndValidateInput(' bad ', validateInput); + + expect(validateInput).toHaveBeenCalledWith('bad'); + expect(result.isValid).toBe(false); + expect(result.errorMessage).toBe('Invalid format'); + }); +}); diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/computeUpdatedMultiItemFieldItems.ts b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/computeUpdatedMultiItemFieldItems.ts new file mode 100644 index 0000000000..5b776aac2f --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/computeUpdatedMultiItemFieldItems.ts @@ -0,0 +1,37 @@ +import { isNonEmptyString } from '@sniptt/guards'; +import { isDefined } from 'twenty-shared/utils'; +import { toSpliced } from '~/utils/array/toSpliced'; + +export const computeUpdatedMultiItemFieldItems = ({ + sanitizedInput, + items, + editingIndex, + singleItemMode, + formatInput, +}: { + sanitizedInput: string; + items: T[]; + editingIndex: number | null; + singleItemMode: boolean; + formatInput?: (input: string, itemIndex?: number) => T; +}): T[] => { + const isAdding = !isDefined(editingIndex); + + if (!isNonEmptyString(sanitizedInput)) { + if (isAdding) { + return items; + } + if (singleItemMode) { + return []; + } + return toSpliced(items, editingIndex, 1); + } + + const newItem = isDefined(formatInput) + ? formatInput(sanitizedInput, isAdding ? undefined : editingIndex) + : (sanitizedInput as unknown as T); + + return isAdding + ? [...items, newItem] + : toSpliced(items, editingIndex, 1, newItem); +}; diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/sanitizeAndValidateInput.ts b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/sanitizeAndValidateInput.ts new file mode 100644 index 0000000000..14c90fe500 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/sanitizeAndValidateInput.ts @@ -0,0 +1,17 @@ +import { isNonEmptyString } from '@sniptt/guards'; +import { isDefined } from 'twenty-shared/utils'; + +export const sanitizeAndValidateInput = ( + rawInput: string, + validateInput?: (input: string) => { isValid: boolean; errorMessage: string }, +): { sanitizedInput: string; isValid: boolean; errorMessage: string } => { + const sanitizedInput = rawInput.trim(); + + if (!isNonEmptyString(sanitizedInput) || !isDefined(validateInput)) { + return { sanitizedInput, isValid: true, errorMessage: '' }; + } + + const { isValid, errorMessage } = validateInput(sanitizedInput); + + return { sanitizedInput, isValid, errorMessage }; +};