From 00b6d651f467f1e0f62eff5f8d82cb9fb29b53ea Mon Sep 17 00:00:00 2001 From: Thomas Trompette Date: Fri, 7 Aug 2026 10:48:32 +0200 Subject: [PATCH] fix(front): preserve currency decimals when opening the field editor (#23874) Fixes #23828 ## Problem Opening and closing a currency field editor corrupts the stored amount when the value has more decimals than the field's `decimals` setting (which defaults to 0). No keystroke is needed. - With the `1.234,56` number format, `458.64` is persisted back as `45864`. - With the `1,234.56` number format, `458.64` is persisted back as `458`. ## Root cause The draft amount is serialized with a dot decimal separator (`amountMicros / 1000000).toString()`), and `CurrencyInput` hands that string to the IMask `Number` mask with `scale={decimals}` alongside the workspace `thousandsSeparator` and `radix`. With `scale` 0 and a dot thousands separator, imask reads `458.64` as `45864`; with a dot radix it drops the fraction and yields `458`. That misreading is only half of it. `react-imask` re-emits `accept` while it formats the value it was given, so **merely mounting the editor** pushed the mask's own reading of the amount into `internalText` and into the draft value. From there every exit path persisted it, escape included. That is why the corruption needs no keystroke. ## Fix Two layers, smallest first: 1. `CurrencyInput` ignores `accept` events that carry no originating input event. In imask, `_inputEvent` is set only inside `_onInput` and deleted right after, so a user keystroke (including the reformat emitted within the same turn) always carries it, while mount and programmatic updates never do. The draft can now only change because someone typed. 2. The exit handlers pass `skipPersist` when the resulting value already matches the stored one, so opening and closing a field writes nothing at all - no redundant update, no timeline entry. `getSafeScaleForCurrencyInput` is kept as well: the mask scale is what makes the editor *display and edit* the right number. Without it, a `458.64` amount still opens as `45.864`, and a genuine edit would then build on the wrong base and persist legitimately. ## Tests Unit tests on both utils, plus an imask round-trip of `458.64` with `decimals` 0 across all four number formats, which fails on `main` for both dot-separator formats. Verified in a local instance against the reported case (`458.64`, `decimals` 0): | scenario | result | |---|---| | open + close, `1.234,56` | value unchanged, `updatedAt` untouched (no write at all) | | open + close, `1,234.56` | value unchanged | | type a new amount | persists normally | | type `12.5` on a `decimals` 0 field | yields `125`, separator still rejected | --- .../input/components/CurrencyFieldInput.tsx | 51 ++-- .../CurrencyFieldInput.stories.tsx | 251 ++++++++++++++++++ .../__tests__/hasCurrencyValueChanged.test.ts | 34 +++ .../input/utils/hasCurrencyValueChanged.ts | 21 ++ .../field/input/components/CurrencyInput.tsx | 29 +- .../getSafeScaleForCurrencyInput.test.ts | 66 +++++ .../utils/getSafeScaleForCurrencyInput.ts | 17 ++ 7 files changed, 434 insertions(+), 35 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/__stories__/CurrencyFieldInput.stories.tsx create mode 100644 packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/__tests__/hasCurrencyValueChanged.test.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/hasCurrencyValueChanged.ts create mode 100644 packages/twenty-front/src/modules/ui/field/input/utils/__tests__/getSafeScaleForCurrencyInput.test.ts create mode 100644 packages/twenty-front/src/modules/ui/field/input/utils/getSafeScaleForCurrencyInput.ts diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/CurrencyFieldInput.tsx b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/CurrencyFieldInput.tsx index 039ee6d5cd..f59d26d205 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/CurrencyFieldInput.tsx +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/CurrencyFieldInput.tsx @@ -10,6 +10,7 @@ import { useCurrencyField } from '@/object-record/record-field/ui/meta-types/hoo import { FieldInputEventContext } from '@/object-record/record-field/ui/contexts/FieldInputEventContext'; import { RecordFieldComponentInstanceContext } from '@/object-record/record-field/ui/states/contexts/RecordFieldComponentInstanceContext'; +import { hasCurrencyValueChanged } from '@/object-record/record-field/ui/meta-types/input/utils/hasCurrencyValueChanged'; import { isFieldCurrencyValue } from '@/object-record/record-field/ui/types/guards/isFieldCurrencyValue'; import { useAvailableComponentInstanceIdOrThrow } from '@/ui/utilities/state/component-state/hooks/useAvailableComponentInstanceIdOrThrow'; import { useContext } from 'react'; @@ -17,7 +18,7 @@ import { convertCurrencyAmountToCurrencyMicros } from '~/utils/convertCurrencyTo import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull'; export const CurrencyFieldInput = () => { - const { draftValue, setDraftValue, defaultValue, decimals } = + const { fieldValue, draftValue, setDraftValue, defaultValue, decimals } = useCurrencyField(); const { onClickOutside, onEnter, onEscape, onShiftTab, onTab } = useContext( @@ -71,53 +72,39 @@ export const CurrencyFieldInput = () => { return newCurrencyValue; }; - const handleEnter = (newValue: string) => { - onEnter?.({ - newValue: getNewCurrencyValue({ - amountText: newValue, - currencyCode, + const getExitArgs = (amountText: string) => { + const newValue = getNewCurrencyValue({ amountText, currencyCode }); + + return { + newValue, + skipPersist: !hasCurrencyValueChanged({ + newValue, + currentValue: fieldValue, }), - }); + }; + }; + + const handleEnter = (newValue: string) => { + onEnter?.(getExitArgs(newValue)); }; const handleEscape = (newValue: string) => { - onEscape?.({ - newValue: getNewCurrencyValue({ - amountText: newValue, - currencyCode, - }), - }); + onEscape?.(getExitArgs(newValue)); }; const handleClickOutside = ( event: MouseEvent | TouchEvent, newValue: string, ) => { - onClickOutside?.({ - newValue: getNewCurrencyValue({ - amountText: newValue, - currencyCode, - }), - event, - }); + onClickOutside?.({ ...getExitArgs(newValue), event }); }; const handleTab = (newValue: string) => { - onTab?.({ - newValue: getNewCurrencyValue({ - amountText: newValue, - currencyCode, - }), - }); + onTab?.(getExitArgs(newValue)); }; const handleShiftTab = (newValue: string) => { - onShiftTab?.({ - newValue: getNewCurrencyValue({ - amountText: newValue, - currencyCode, - }), - }); + onShiftTab?.(getExitArgs(newValue)); }; const handleChange = (newValue: string) => { diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/__stories__/CurrencyFieldInput.stories.tsx b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/__stories__/CurrencyFieldInput.stories.tsx new file mode 100644 index 0000000000..46cee908fa --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/__stories__/CurrencyFieldInput.stories.tsx @@ -0,0 +1,251 @@ +import { + type Decorator, + type Meta, + type StoryObj, +} from '@storybook/react-vite'; +import { useEffect, useState } from 'react'; +import { expect, userEvent, waitFor, within } from 'storybook/test'; + +import { usePushFocusItemToFocusStack } from '@/ui/utilities/focus/hooks/usePushFocusItemToFocusStack'; +import { FieldMetadataType } from '~/generated-metadata/graphql'; +import { SnackBarDecorator } from '~/testing/decorators/SnackBarDecorator'; + +import { NumberFormat } from '@/localization/constants/NumberFormat'; +import { workspaceMemberFormatPreferencesState } from '@/localization/states/workspaceMemberFormatPreferencesState'; +import { RecordFieldsScopeContextProvider } from '@/object-record/record-field-list/contexts/RecordFieldsScopeContext'; +import { FieldContext } from '@/object-record/record-field/ui/contexts/FieldContext'; +import { useCurrencyField } from '@/object-record/record-field/ui/meta-types/hooks/useCurrencyField'; +import { CurrencyFieldInput } from '@/object-record/record-field/ui/meta-types/input/components/CurrencyFieldInput'; +import { getFieldInputEventContextProviderWithJestMocks } from '@/object-record/record-field/ui/meta-types/input/components/__stories__/utils/getFieldInputEventContextProviderWithJestMocks'; +import { RecordFieldComponentInstanceContext } from '@/object-record/record-field/ui/states/contexts/RecordFieldComponentInstanceContext'; +import { RECORD_TABLE_CELL_INPUT_ID_PREFIX } from '@/object-record/record-table/constants/RecordTableCellInputIdPrefix'; +import { getRecordFieldInputInstanceId } from '@/object-record/utils/getRecordFieldInputId'; +import { useSetAtomState } from '@/ui/utilities/state/jotai/hooks/useSetAtomState'; +import { FocusComponentType } from '@/ui/utilities/focus/types/FocusComponentType'; +import { CurrencyCode } from 'twenty-shared/constants'; +import { StorybookFieldInputDropdownFocusIdSetterEffect } from '~/testing/components/StorybookFieldInputDropdownFocusIdSetterEffect'; + +const { + FieldInputEventContextProviderWithJestMocks, + handleEnterMocked, + handleEscapeMocked, + handleClickoutsideMocked, + handleTabMocked, + handleShiftTabMocked, +} = getFieldInputEventContextProviderWithJestMocks(); + +const AMOUNT_MICROS_WITH_CENTS = 458640000; + +const CurrencyFieldValueSetterEffect = ({ + amountMicros, + numberFormat, +}: { + amountMicros: number; + numberFormat: NumberFormat; +}) => { + const { setFieldValue, setDraftValue } = useCurrencyField(); + const setFormatPreferences = useSetAtomState( + workspaceMemberFormatPreferencesState, + ); + + useEffect(() => { + setFormatPreferences((previous) => ({ ...previous, numberFormat })); + setFieldValue({ amountMicros, currencyCode: CurrencyCode.USD }); + setDraftValue({ + amount: (amountMicros / 1000000).toString(), + currencyCode: CurrencyCode.USD, + }); + }, [ + setFieldValue, + setDraftValue, + amountMicros, + setFormatPreferences, + numberFormat, + ]); + + return <>; +}; + +type CurrencyFieldInputWithContextProps = { + amountMicros: number; + numberFormat: NumberFormat; + recordId: string; +}; + +const CurrencyFieldInputWithContext = ({ + recordId, + amountMicros, + numberFormat, +}: CurrencyFieldInputWithContextProps) => { + const { pushFocusItemToFocusStack } = usePushFocusItemToFocusStack(); + + const [isReady, setIsReady] = useState(false); + + const instanceId = getRecordFieldInputInstanceId({ + recordId, + fieldName: 'Amount', + prefix: RECORD_TABLE_CELL_INPUT_ID_PREFIX, + }); + + useEffect(() => { + if (!isReady) { + pushFocusItemToFocusStack({ + focusId: instanceId, + component: { + type: FocusComponentType.OPENED_FIELD_INPUT, + instanceId: instanceId, + }, + }); + setIsReady(true); + } + }, [isReady, pushFocusItemToFocusStack, instanceId]); + + return ( + + + + + {isReady && } + + + + + + {isReady &&
} +
+ + ); +}; + +const clearMocksDecorator: Decorator = (Story, context) => { + if (context.parameters.clearMocks === true) { + handleEnterMocked.mockClear(); + handleEscapeMocked.mockClear(); + handleClickoutsideMocked.mockClear(); + handleTabMocked.mockClear(); + handleShiftTabMocked.mockClear(); + } + return ; +}; + +const meta: Meta = { + title: 'UI/Data/Field/Input/CurrencyFieldInput', + component: CurrencyFieldInputWithContext, + args: { + recordId: '123', + amountMicros: AMOUNT_MICROS_WITH_CENTS, + numberFormat: NumberFormat.DOTS_AND_COMMA, + }, + decorators: [clearMocksDecorator, SnackBarDecorator], + parameters: { + clearMocks: true, + }, +}; + +export default meta; + +type Story = StoryObj; + +export const Default: Story = {}; + +export const ClickOutsideKeepsCentsWithDotsAndComma: Story = { + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + expect(handleClickoutsideMocked).toHaveBeenCalledTimes(0); + + await canvas.findByTestId('is-ready-marker'); + await userEvent.click( + canvas.getByTestId('data-field-input-click-outside-div'), + ); + + await waitFor(() => { + expect(handleClickoutsideMocked).toHaveBeenCalledTimes(1); + }); + + expect(handleClickoutsideMocked).toHaveBeenCalledWith( + expect.objectContaining({ + newValue: { + amountMicros: AMOUNT_MICROS_WITH_CENTS, + currencyCode: CurrencyCode.USD, + }, + skipPersist: true, + }), + ); + }, +}; + +export const ClickOutsideKeepsCentsWithCommasAndDot: Story = { + args: { numberFormat: NumberFormat.COMMAS_AND_DOT }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + expect(handleClickoutsideMocked).toHaveBeenCalledTimes(0); + + await canvas.findByTestId('is-ready-marker'); + await userEvent.click( + canvas.getByTestId('data-field-input-click-outside-div'), + ); + + await waitFor(() => { + expect(handleClickoutsideMocked).toHaveBeenCalledTimes(1); + }); + + expect(handleClickoutsideMocked).toHaveBeenCalledWith( + expect.objectContaining({ + newValue: { + amountMicros: AMOUNT_MICROS_WITH_CENTS, + currencyCode: CurrencyCode.USD, + }, + skipPersist: true, + }), + ); + }, +}; + +export const EscapeKeepsCents: Story = { + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + expect(handleEscapeMocked).toHaveBeenCalledTimes(0); + + await canvas.findByTestId('is-ready-marker'); + await userEvent.keyboard('{esc}'); + + await waitFor(() => { + expect(handleEscapeMocked).toHaveBeenCalledTimes(1); + }); + + expect(handleEscapeMocked).toHaveBeenCalledWith( + expect.objectContaining({ + newValue: { + amountMicros: AMOUNT_MICROS_WITH_CENTS, + currencyCode: CurrencyCode.USD, + }, + skipPersist: true, + }), + ); + }, +}; diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/__tests__/hasCurrencyValueChanged.test.ts b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/__tests__/hasCurrencyValueChanged.test.ts new file mode 100644 index 0000000000..dc8b5d7004 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/__tests__/hasCurrencyValueChanged.test.ts @@ -0,0 +1,34 @@ +import { hasCurrencyValueChanged } from '@/object-record/record-field/ui/meta-types/input/utils/hasCurrencyValueChanged'; +import { CurrencyCode } from 'twenty-shared/constants'; + +describe('hasCurrencyValueChanged', () => { + it('should not report a change when reopening a field leaves the value untouched', () => { + expect( + hasCurrencyValueChanged({ + newValue: { amountMicros: 458640000, currencyCode: CurrencyCode.USD }, + currentValue: { + amountMicros: 458640000, + currencyCode: CurrencyCode.USD, + }, + }), + ).toBe(false); + }); + + it('should report a change when the amount or currency differs', () => { + expect( + hasCurrencyValueChanged({ + newValue: { amountMicros: 45864000000, currencyCode: CurrencyCode.EUR }, + currentValue: { + amountMicros: 458640000, + currencyCode: CurrencyCode.USD, + }, + }), + ).toBe(true); + }); + + it('should report a change when either side is not a currency value', () => { + expect( + hasCurrencyValueChanged({ newValue: undefined, currentValue: null }), + ).toBe(true); + }); +}); diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/hasCurrencyValueChanged.ts b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/hasCurrencyValueChanged.ts new file mode 100644 index 0000000000..3a73624272 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/utils/hasCurrencyValueChanged.ts @@ -0,0 +1,21 @@ +import { type FieldCurrencyValue } from '@/object-record/record-field/ui/types/FieldMetadata'; +import { isFieldCurrencyValue } from '@/object-record/record-field/ui/types/guards/isFieldCurrencyValue'; + +type HasCurrencyValueChangedParams = { + newValue: FieldCurrencyValue | undefined; + currentValue: unknown; +}; + +export const hasCurrencyValueChanged = ({ + newValue, + currentValue, +}: HasCurrencyValueChangedParams): boolean => { + if (!isFieldCurrencyValue(newValue) || !isFieldCurrencyValue(currentValue)) { + return true; + } + + return ( + newValue.amountMicros !== currentValue.amountMicros || + newValue.currencyCode !== currentValue.currencyCode + ); +}; diff --git a/packages/twenty-front/src/modules/ui/field/input/components/CurrencyInput.tsx b/packages/twenty-front/src/modules/ui/field/input/components/CurrencyInput.tsx index 4c2db1732c..804039d437 100644 --- a/packages/twenty-front/src/modules/ui/field/input/components/CurrencyInput.tsx +++ b/packages/twenty-front/src/modules/ui/field/input/components/CurrencyInput.tsx @@ -6,6 +6,7 @@ import { useRegisterInputEvents } from '@/object-record/record-field/ui/meta-typ import { useNumberFormat } from '@/localization/hooks/useNumberFormat'; import { CURRENCIES } from '@/settings/data-model/constants/Currencies'; import { CurrencyPickerDropdownButton } from '@/ui/input/components/internal/currency/components/CurrencyPickerDropdownButton'; +import { getSafeScaleForCurrencyInput } from '@/ui/field/input/utils/getSafeScaleForCurrencyInput'; import { type Currency } from '@/ui/input/components/internal/types/Currency'; import { IMaskInput } from 'react-imask'; import { type IconComponent } from 'twenty-ui/icon'; @@ -89,6 +90,9 @@ export const CurrencyInput = ({ }: CurrencyInputProps) => { const { theme } = useContext(ThemeContext); const [internalText, setInternalText] = useState(value); + const [scale, setScale] = useState(() => + getSafeScaleForCurrencyInput({ value, decimals }), + ); const { numberFormat } = useNumberFormat(); const wrapperRef = useRef(null); @@ -96,7 +100,13 @@ export const CurrencyInput = ({ const { thousandsSeparator, radix } = getSeparatorsForNumberFormat(numberFormat); - const handleChange = (value: string) => { + // imask re-emits accept while formatting the incoming value, with no + // originating input event; only a user keystroke may change the draft + const handleAccept = (value: string, event?: InputEvent) => { + if (!isDefined(event)) { + return; + } + setInternalText(value); onChange?.(value); }; @@ -118,6 +128,17 @@ export const CurrencyInput = ({ const currency = CURRENCIES.find(({ value }) => value === currencyCode); + const scaleForCurrentValue = getSafeScaleForCurrencyInput({ + value, + decimals, + }); + + // deleting a decimal must not narrow the mask for the rest of the edit, + // it would make the digit impossible to type back + if (scale < scaleForCurrentValue) { + setScale(scaleForCurrentValue); + } + useEffect(() => { setInternalText(value); }, [value]); @@ -140,8 +161,10 @@ export const CurrencyInput = ({ mask={Number} thousandsSeparator={thousandsSeparator} radix={radix} - scale={decimals} - onAccept={(value: string) => handleChange(value)} + scale={scale} + onAccept={(value: string, _maskRef: unknown, event?: InputEvent) => + handleAccept(value, event) + } inputRef={wrapperRef} autoComplete="off" placeholder={placeholder} diff --git a/packages/twenty-front/src/modules/ui/field/input/utils/__tests__/getSafeScaleForCurrencyInput.test.ts b/packages/twenty-front/src/modules/ui/field/input/utils/__tests__/getSafeScaleForCurrencyInput.test.ts new file mode 100644 index 0000000000..14e63a7c0a --- /dev/null +++ b/packages/twenty-front/src/modules/ui/field/input/utils/__tests__/getSafeScaleForCurrencyInput.test.ts @@ -0,0 +1,66 @@ +import { IMask } from 'react-imask'; + +import { NumberFormat } from '@/localization/constants/NumberFormat'; +import { getSafeScaleForCurrencyInput } from '@/ui/field/input/utils/getSafeScaleForCurrencyInput'; +import { getSeparatorsForNumberFormat } from '~/utils/format/getSeparatorsForNumberFormat'; + +describe('getSafeScaleForCurrencyInput', () => { + it('should keep the field decimals when the value has no decimal part', () => { + expect(getSafeScaleForCurrencyInput({ value: '458', decimals: 2 })).toBe(2); + }); + + it('should keep the field decimals when the value fits in it', () => { + expect(getSafeScaleForCurrencyInput({ value: '458.6', decimals: 2 })).toBe( + 2, + ); + }); + + it('should widen the scale to the decimals present in the value', () => { + expect(getSafeScaleForCurrencyInput({ value: '458.64', decimals: 0 })).toBe( + 2, + ); + }); + + it('should handle negative values', () => { + expect( + getSafeScaleForCurrencyInput({ value: '-458.64', decimals: 0 }), + ).toBe(2); + }); + + it('should default to no decimals when neither decimals nor value provide any', () => { + expect(getSafeScaleForCurrencyInput({ value: '' })).toBe(0); + }); + + it('should ignore a value that is not a plain unmasked number', () => { + expect( + getSafeScaleForCurrencyInput({ value: '1.234,56', decimals: 1 }), + ).toBe(1); + }); +}); + +describe('currency mask round trip with the safe scale', () => { + it.each([ + [NumberFormat.DOTS_AND_COMMA, '458,64'], + [NumberFormat.COMMAS_AND_DOT, '458.64'], + [NumberFormat.SPACES_AND_COMMA, '458,64'], + [NumberFormat.APOSTROPHE_AND_DOT, '458.64'], + ])( + 'should not alter 458.64 with decimals 0 under the %s format', + (numberFormat, expectedMaskedValue) => { + const { thousandsSeparator, radix } = + getSeparatorsForNumberFormat(numberFormat); + + const mask = IMask.createMask({ + mask: Number, + thousandsSeparator, + radix, + scale: getSafeScaleForCurrencyInput({ value: '458.64', decimals: 0 }), + }); + + mask.unmaskedValue = '458.64'; + + expect(mask.value).toBe(expectedMaskedValue); + expect(mask.unmaskedValue).toBe('458.64'); + }, + ); +}); diff --git a/packages/twenty-front/src/modules/ui/field/input/utils/getSafeScaleForCurrencyInput.ts b/packages/twenty-front/src/modules/ui/field/input/utils/getSafeScaleForCurrencyInput.ts new file mode 100644 index 0000000000..1ac77a0607 --- /dev/null +++ b/packages/twenty-front/src/modules/ui/field/input/utils/getSafeScaleForCurrencyInput.ts @@ -0,0 +1,17 @@ +import { DEFAULT_DECIMAL_VALUE } from '~/utils/format/formatNumber'; + +type GetSafeScaleForCurrencyInputParams = { + value: string; + decimals?: number; +}; + +const UNMASKED_VALUE_PATTERN = /^-?\d*\.(\d+)$/; + +export const getSafeScaleForCurrencyInput = ({ + value, + decimals = DEFAULT_DECIMAL_VALUE, +}: GetSafeScaleForCurrencyInputParams): number => { + const decimalPart = UNMASKED_VALUE_PATTERN.exec(value)?.[1]; + + return Math.max(decimals, decimalPart?.length ?? 0); +};