fix: align email validation between front and server and roll back optimistic value on failed save (#22490)
## Summary
Inline edits of EMAILS fields could leave the UI in a misleading state:
the frontend validated with Zod's default `z.email()` while the server
used the stricter `z.regexes.unicodeEmail` pattern (which caps the local
part at 64 characters). A very long email passed client validation and
was optimistically written to the UI; the server then rejected the
mutation. An error snackbar was shown, but the field kept displaying the
unsaved value until a page reload.
## Changes
- **Single source of truth for email validation**: added a shared
`emailSchema` (`z.email({ pattern: z.regexes.unicodeEmail })`) in
`twenty-shared/utils`, now used by:
- the server-side EMAILS field validator
(`validate-emails-primary-email-subfield-or-throw.util.ts`)
- the `EmailsFieldInput` inline editor
- spreadsheet import validation
- **Rollback on failed save**: `useUpdateOneRecord` now restores the
optimistically updated fields in the record store when the mutation
fails, mirroring the store upsert already done in the success path.
Previously the catch block only rolled back the Apollo cache — which
stopped reverting the UI after table virtualization, since the record
store (the render source of truth) is no longer synced reactively from
the cache. The error is still rethrown, so the existing global
promise-rejection handler keeps showing the error snackbar. This fixes
the stale-value-until-reload behavior for all field types and all
callers, not just EMAILS fields.
- **Regression tests**: added unit tests for the shared schema,
including the >64-character local part case.
Fixes [sonarly issue
#54034](https://sonarly.com/issue/54034?share=eyJ0aWQiOjMzMCwidHlwIjoiYnVnIiwicmlkIjo1NDAzNCwiZXhwIjoxNzgzNTI1OTQzfQ.9e7639034a677301512fceeafab764b1)
<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/22490?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
This commit is contained in:
@@ -20,8 +20,8 @@ import { computeOptimisticRecordFromInput } from '@/object-record/utils/computeO
|
||||
import { getUpdatedFieldsFromRecordInput } from '@/object-record/utils/getUpdatedFieldsFromRecordInput';
|
||||
import { getUpdateOneRecordMutationResponseField } from '@/object-record/utils/getUpdateOneRecordMutationResponseField';
|
||||
import { sanitizeRecordInput } from '@/object-record/utils/sanitizeRecordInput';
|
||||
import { isNull } from '@sniptt/guards';
|
||||
import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue';
|
||||
import { isNull } from '@sniptt/guards';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { buildRecordFromKeysWithSameValue } from '~/utils/array/buildRecordFromKeysWithSameValue';
|
||||
|
||||
@@ -239,6 +239,28 @@ export const useUpdateOneRecord = () => {
|
||||
upsertRecordsInStore,
|
||||
});
|
||||
|
||||
const optimisticallyUpdatedFieldsToRestore = Object.keys(
|
||||
optimisticRecordInput,
|
||||
).reduce<Partial<ObjectRecord>>(
|
||||
(restoredFields, fieldName) => ({
|
||||
...restoredFields,
|
||||
[fieldName]: cachedRecordKeys.has(fieldName)
|
||||
? cachedRecord?.[fieldName]
|
||||
: null,
|
||||
}),
|
||||
{},
|
||||
);
|
||||
|
||||
upsertRecordsInStore({
|
||||
partialRecords: [
|
||||
{
|
||||
id: idToUpdate,
|
||||
__typename: getObjectTypename(objectMetadataItem.nameSingular),
|
||||
...optimisticallyUpdatedFieldsToRestore,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
throw error;
|
||||
});
|
||||
|
||||
|
||||
+1
-2
@@ -5,12 +5,11 @@ import { MULTI_ITEM_FIELD_INPUT_DROPDOWN_ID_PREFIX } from '@/object-record/recor
|
||||
import { recordFieldInputIsFieldInErrorComponentState } from '@/object-record/record-field/ui/states/recordFieldInputIsFieldInErrorComponentState';
|
||||
import { type FieldEmailsValue } from '@/object-record/record-field/ui/types/FieldMetadata';
|
||||
import { emailsFieldValueSchema } from '@/object-record/record-field/ui/validation-schemas/emailsFieldValueSchema';
|
||||
import { emailSchema } from '@/object-record/record-field/ui/validation-schemas/emailSchema';
|
||||
import { useSetAtomComponentState } from '@/ui/utilities/state/jotai/hooks/useSetAtomComponentState';
|
||||
import { useLingui } from '@lingui/react/macro';
|
||||
import { useCallback, useContext, useMemo } from 'react';
|
||||
import { MULTI_ITEM_FIELD_DEFAULT_MAX_VALUES } from 'twenty-shared/constants';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { emailSchema, isDefined } from 'twenty-shared/utils';
|
||||
import { FieldMetadataType } from '~/generated-metadata/graphql';
|
||||
import { useCopyToClipboard } from '~/hooks/useCopyToClipboard';
|
||||
import { MultiItemFieldInput } from './MultiItemFieldInput';
|
||||
|
||||
-3
@@ -1,3 +0,0 @@
|
||||
import { z } from 'zod';
|
||||
|
||||
export const emailSchema = z.email();
|
||||
+1
-1
@@ -1,5 +1,4 @@
|
||||
import { isFieldRatingValue } from '@/object-record/record-field/ui/types/guards/isFieldRatingValue';
|
||||
import { emailSchema } from '@/object-record/record-field/ui/validation-schemas/emailSchema';
|
||||
import { type SpreadsheetImportFieldValidationDefinition } from '@/spreadsheet-import/types';
|
||||
import { t } from '@lingui/core/macro';
|
||||
import { isDate, isString } from '@sniptt/guards';
|
||||
@@ -7,6 +6,7 @@ import { parsePhoneNumberWithError } from 'libphonenumber-js';
|
||||
import { RATING_VALUES } from 'twenty-shared/constants';
|
||||
import {
|
||||
absoluteUrlSchema,
|
||||
emailSchema,
|
||||
getCountryCodesForCallingCode,
|
||||
isDefined,
|
||||
isValidCountryCode,
|
||||
|
||||
+2
-5
@@ -2,7 +2,7 @@ import { inspect } from 'util';
|
||||
|
||||
import { msg } from '@lingui/core/macro';
|
||||
import { isNonEmptyString, isNull } from '@sniptt/guards';
|
||||
import { z } from 'zod';
|
||||
import { emailSchema } from 'twenty-shared/utils';
|
||||
|
||||
import {
|
||||
CommonQueryRunnerException,
|
||||
@@ -25,10 +25,7 @@ export const validateEmailsPrimaryEmailSubfieldOrThrow = (
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
!z.email({ pattern: z.regexes.unicodeEmail }).safeParse(value).success &&
|
||||
isNonEmptyString(value)
|
||||
) {
|
||||
if (!emailSchema.safeParse(value).success && isNonEmptyString(value)) {
|
||||
const inspectedValue = inspect(value);
|
||||
|
||||
throw new CommonQueryRunnerException(
|
||||
|
||||
@@ -226,6 +226,7 @@ export { normalizeUrlOrigin } from './url/normalizeUrlOrigin';
|
||||
export { safeDecodeURIComponent } from './url/safeDecodeURIComponent';
|
||||
export { uuidToBase36 } from './uuidToBase36';
|
||||
export { assertIsDefinedOrThrow } from './validation/assertIsDefinedOrThrow';
|
||||
export { emailSchema } from './validation/emailSchema';
|
||||
export { isDefined } from './validation/isDefined';
|
||||
export { isEmptyObject } from './validation/isEmptyObject';
|
||||
export { isLabelIdentifierFieldMetadataTypes } from './validation/isLabelIdentifierFieldMetadataTypes';
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
import { emailSchema } from '@/utils/validation/emailSchema';
|
||||
|
||||
describe('emailSchema', () => {
|
||||
it('should accept valid emails', () => {
|
||||
expect(emailSchema.safeParse('john.doe@example.com').success).toBe(true);
|
||||
expect(emailSchema.safeParse('jöhn@example.com').success).toBe(true);
|
||||
expect(emailSchema.safeParse('john+tag@sub.example.co').success).toBe(true);
|
||||
});
|
||||
|
||||
it('should reject invalid emails', () => {
|
||||
expect(emailSchema.safeParse('not-an-email').success).toBe(false);
|
||||
expect(emailSchema.safeParse('john@').success).toBe(false);
|
||||
expect(emailSchema.safeParse('@example.com').success).toBe(false);
|
||||
expect(emailSchema.safeParse('john doe@example.com').success).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject emails with a local part longer than 64 characters', () => {
|
||||
const longLocalPart = 'a'.repeat(65);
|
||||
|
||||
expect(emailSchema.safeParse(`${longLocalPart}@example.com`).success).toBe(
|
||||
false,
|
||||
);
|
||||
expect(emailSchema.safeParse(`${'a'.repeat(64)}@example.com`).success).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,3 @@
|
||||
import { z } from 'zod';
|
||||
|
||||
export const emailSchema = z.email({ pattern: z.regexes.unicodeEmail });
|
||||
Reference in New Issue
Block a user