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 <charles@twenty.com>
2026-03-10 21:09:21 +05:30
parent ec1f08bc3b
commit 7b9939b43e
5 changed files with 234 additions and 56 deletions
@@ -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 = <T,>({
      ) {
        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 = <T,>({
      return;
    }

-    handleSubmitChanges();
+    onChange(updatedItems);
    if (shouldAutoEnterBecauseOnlyOneItemIsAllowed) {
      onEnter(updatedItems);
    }
@@ -223,62 +226,51 @@ export const MultiItemFieldInput = <T,>({
    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 = <T,>({
  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 = () => {
@@ -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<Link>({
+        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);
+    });
+  });
+});
@@ -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');
+  });
+});
@@ -0,0 +1,37 @@
+import { isNonEmptyString } from '@sniptt/guards';
+import { isDefined } from 'twenty-shared/utils';
+import { toSpliced } from '~/utils/array/toSpliced';
+
+export const computeUpdatedMultiItemFieldItems = <T>({
+  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);
+};
@@ -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 };
+};