fix: fix several field bugs (twentyhq#5339)

After discussing with @charlesBochet, several fixes are needed on fields: - [x] Disable Boolean field `defaultValue` edition for now (On `defaultValue` update, newly created records are not taking the updated `defaultValue` into account. Setting the `defaultValue` on creation is fine.) - [x] Disable Phone field creation for now - [x] For the Person object, display the "Phone" field as a field of type Phone (right now its type is Text; later we'll migrate it to a proper Phone field). - [x] Fix RawJson field display (displaying `[object Object]` in Record Table cells). - [x] In Settings/Data Model, on Relation field creation/edition, "Object destination" select is not working properly if an object was not manually selected (displays Companies by default but creates a relation to another random object than Companies).
synapsenet-arena · May 8, 2024 · 7728c09 · 7728c09
1 parent 005045c
commit 7728c09
Show file tree

Hide file tree

Showing 23 changed files with 333 additions and 168 deletions.
diff --git a/packages/twenty-front/src/modules/object-record/cache/utils/getRecordFromRecordNode.ts b/packages/twenty-front/src/modules/object-record/cache/utils/getRecordFromRecordNode.ts
@@ -1,3 +1,5 @@
+import pick from 'lodash.pick';
+
 import { getRecordsFromRecordConnection } from '@/object-record/cache/utils/getRecordsFromRecordConnection';
 import { RecordGqlNode } from '@/object-record/graphql/types/RecordGqlNode';
 import { ObjectRecord } from '@/object-record/types/ObjectRecord';
@@ -12,15 +14,11 @@ export const getRecordFromRecordNode = <T extends ObjectRecord>({
   return {
     ...Object.fromEntries(
       Object.entries(recordNode).map(([fieldName, value]) => {
-        if (isUndefinedOrNull(value)) {
-          return [fieldName, value];
-        }
-
-        if (Array.isArray(value)) {
-          return [fieldName, value];
-        }
-
-        if (typeof value !== 'object') {
+        if (
+          isUndefinedOrNull(value) ||
+          Array.isArray(value) ||
+          typeof value !== 'object'
+        ) {
           return [fieldName, value];
         }
 
@@ -32,7 +30,10 @@ export const getRecordFromRecordNode = <T extends ObjectRecord>({
           : [fieldName, getRecordFromRecordNode<T>({ recordNode: value })];
       }),
     ),
-    id: recordNode.id,
-    __typename: recordNode.__typename,
+    // Only adds `id` and `__typename` if they exist.
+    // RawJson field value passes through this method and does not have `id` or `__typename`.
+    // This prevents adding an undefined `id` and `__typename` to the RawJson field value,
+    // which is invalid JSON.
+    ...pick(recordNode, ['id', '__typename'] as const),
   } as T;
 };
diff --git a/packages/twenty-front/src/modules/object-record/record-field/components/FieldDisplay.tsx b/packages/twenty-front/src/modules/object-record/record-field/components/FieldDisplay.tsx
@@ -1,6 +1,7 @@
 import { useContext } from 'react';
 
 import { LinksFieldDisplay } from '@/object-record/record-field/meta-types/display/components/LinksFieldDisplay';
+import { isFieldDisplayedAsPhone } from '@/object-record/record-field/types/guards/isFieldDisplayedAsPhone';
 import { isFieldLinks } from '@/object-record/record-field/types/guards/isFieldLinks';
 import { ExpandableListProps } from '@/ui/layout/expandable-list/components/ExpandableList';
 
@@ -56,7 +57,8 @@ export const FieldDisplay = ({
     <ChipFieldDisplay />
   ) : isFieldRelation(fieldDefinition) ? (
     <RelationFieldDisplay />
-  ) : isFieldPhone(fieldDefinition) ? (
+  ) : isFieldPhone(fieldDefinition) ||
+    isFieldDisplayedAsPhone(fieldDefinition) ? (
     <PhoneFieldDisplay />
   ) : isFieldText(fieldDefinition) ? (
     <TextFieldDisplay />

diff --git a/packages/twenty-front/src/modules/object-record/record-field/components/FieldInput.tsx b/packages/twenty-front/src/modules/object-record/record-field/components/FieldInput.tsx
@@ -9,6 +9,7 @@ import { RawJsonFieldInput } from '@/object-record/record-field/meta-types/input
 import { SelectFieldInput } from '@/object-record/record-field/meta-types/input/components/SelectFieldInput';
 import { RecordFieldInputScope } from '@/object-record/record-field/scopes/RecordFieldInputScope';
 import { isFieldDate } from '@/object-record/record-field/types/guards/isFieldDate';
+import { isFieldDisplayedAsPhone } from '@/object-record/record-field/types/guards/isFieldDisplayedAsPhone';
 import { isFieldFullName } from '@/object-record/record-field/types/guards/isFieldFullName';
 import { isFieldLinks } from '@/object-record/record-field/types/guards/isFieldLinks';
 import { isFieldMultiSelect } from '@/object-record/record-field/types/guards/isFieldMultiSelect';
@@ -71,7 +72,8 @@ export const FieldInput = ({
     >
       {isFieldRelation(fieldDefinition) ? (
         <RelationFieldInput onSubmit={onSubmit} onCancel={onCancel} />
-      ) : isFieldPhone(fieldDefinition) ? (
+      ) : isFieldPhone(fieldDefinition) ||
+        isFieldDisplayedAsPhone(fieldDefinition) ? (
         <PhoneFieldInput
           onEnter={onEnter}
           onEscape={onEscape}

diff --git a/packages/twenty-front/src/modules/object-record/record-field/hooks/useGetButtonIcon.ts b/packages/twenty-front/src/modules/object-record/record-field/hooks/useGetButtonIcon.ts
@@ -1,6 +1,7 @@
 import { useContext } from 'react';
 import { IconComponent, IconPencil } from 'twenty-ui';
 
+import { isFieldDisplayedAsPhone } from '@/object-record/record-field/types/guards/isFieldDisplayedAsPhone';
 import { isFieldLinks } from '@/object-record/record-field/types/guards/isFieldLinks';
 import { isFieldMultiSelect } from '@/object-record/record-field/types/guards/isFieldMultiSelect';
 import { isFieldRelation } from '@/object-record/record-field/types/guards/isFieldRelation';
@@ -20,6 +21,7 @@ export const useGetButtonIcon = (): IconComponent | undefined => {
     isFieldLink(fieldDefinition) ||
     isFieldEmail(fieldDefinition) ||
     isFieldPhone(fieldDefinition) ||
+    isFieldDisplayedAsPhone(fieldDefinition) ||
     isFieldMultiSelect(fieldDefinition) ||
     (isFieldRelation(fieldDefinition) &&
       fieldDefinition.metadata.relationObjectMetadataNameSingular !==

diff --git a/...src/modules/object-record/record-field/meta-types/display/components/JsonFieldDisplay.tsx b/...src/modules/object-record/record-field/meta-types/display/components/JsonFieldDisplay.tsx
@@ -1,12 +1,13 @@
 import { useJsonField } from '@/object-record/record-field/meta-types/hooks/useJsonField';
+import { isFieldRawJsonValue } from '@/object-record/record-field/types/guards/isFieldRawJsonValue';
 import { JsonDisplay } from '@/ui/field/display/components/JsonDisplay';
 
 export const JsonFieldDisplay = () => {
   const { fieldValue, maxWidth } = useJsonField();
 
   return (
     <JsonDisplay
-      text={fieldValue ? JSON.stringify(JSON.parse(fieldValue), null, 2) : ''}
+      text={isFieldRawJsonValue(fieldValue) ? JSON.stringify(fieldValue) : ''}
       maxWidth={maxWidth}
     />
   );

diff --git a/...ages/twenty-front/src/modules/object-record/record-field/meta-types/hooks/useJsonField.ts b/...ages/twenty-front/src/modules/object-record/record-field/meta-types/hooks/useJsonField.ts
@@ -1,6 +1,7 @@
 import { useContext } from 'react';
 import { useRecoilState, useRecoilValue } from 'recoil';
 
+import { usePersistField } from '@/object-record/record-field/hooks/usePersistField';
 import { useRecordFieldInput } from '@/object-record/record-field/hooks/useRecordFieldInput';
 import { FieldJsonValue } from '@/object-record/record-field/types/FieldMetadata';
 import { recordStoreFamilySelector } from '@/object-record/record-store/states/selectors/recordStoreFamilySelector';
@@ -9,7 +10,6 @@ import { FieldMetadataType } from '~/generated-metadata/graphql';
 import { FieldContext } from '../../contexts/FieldContext';
 import { assertFieldMetadata } from '../../types/guards/assertFieldMetadata';
 import { isFieldRawJson } from '../../types/guards/isFieldRawJson';
-import { isFieldTextValue } from '../../types/guards/isFieldTextValue';
 
 export const useJsonField = () => {
   const { entityId, fieldDefinition, hotkeyScope, maxWidth } =
@@ -29,7 +29,18 @@ export const useJsonField = () => {
       fieldName: fieldName,
     }),
   );
-  const fieldTextValue = isFieldTextValue(fieldValue) ? fieldValue : '';
+
+  const persistField = usePersistField();
+
+  const persistJsonField = (nextValue: string) => {
+    if (!nextValue) persistField(null);
+
+    try {
+      persistField(JSON.parse(nextValue));
+    } catch {
+      // Do nothing
+    }
+  };
 
   const { setDraftValue, getDraftValueSelector } =
     useRecordFieldInput<FieldJsonValue>(`${entityId}-${fieldName}`);
@@ -41,8 +52,9 @@ export const useJsonField = () => {
     setDraftValue,
     maxWidth,
     fieldDefinition,
-    fieldValue: fieldTextValue,
+    fieldValue,
     setFieldValue,
     hotkeyScope,
+    persistJsonField,
   };
 };
diff --git a/...ges/twenty-front/src/modules/object-record/record-field/meta-types/hooks/usePhoneField.ts b/...ges/twenty-front/src/modules/object-record/record-field/meta-types/hooks/usePhoneField.ts
@@ -4,6 +4,7 @@ import { useRecoilState, useRecoilValue } from 'recoil';
 
 import { useRecordFieldInput } from '@/object-record/record-field/hooks/useRecordFieldInput';
 import { FieldPhoneValue } from '@/object-record/record-field/types/FieldMetadata';
+import { isFieldDisplayedAsPhone } from '@/object-record/record-field/types/guards/isFieldDisplayedAsPhone';
 import { recordStoreFamilySelector } from '@/object-record/record-store/states/selectors/recordStoreFamilySelector';
 import { FieldMetadataType } from '~/generated-metadata/graphql';
 
@@ -15,7 +16,17 @@ import { isFieldPhone } from '../../types/guards/isFieldPhone';
 export const usePhoneField = () => {
   const { entityId, fieldDefinition, hotkeyScope } = useContext(FieldContext);
 
-  assertFieldMetadata(FieldMetadataType.Phone, isFieldPhone, fieldDefinition);
+  try {
+    // TODO: temporary - remove when 'Phone' field in 'Person' object
+    // is migrated to use FieldMetadataType.Phone as type.
+    assertFieldMetadata(
+      FieldMetadataType.Text,
+      isFieldDisplayedAsPhone,
+      fieldDefinition,
+    );
+  } catch {
+    assertFieldMetadata(FieldMetadataType.Phone, isFieldPhone, fieldDefinition);
+  }
 
   const fieldName = fieldDefinition.metadata.fieldName;
 

diff --git a/.../src/modules/object-record/record-field/meta-types/input/components/RawJsonFieldInput.tsx b/.../src/modules/object-record/record-field/meta-types/input/components/RawJsonFieldInput.tsx
@@ -1,8 +1,6 @@
-import { isValidJSON } from '@/object-record/record-field/utils/isFieldValueJson';
 import { FieldTextAreaOverlay } from '@/ui/field/input/components/FieldTextAreaOverlay';
 import { TextAreaInput } from '@/ui/field/input/components/TextAreaInput';
 
-import { usePersistField } from '../../../hooks/usePersistField';
 import { useJsonField } from '../../hooks/useJsonField';
 
 import { FieldInputEvent } from './DateFieldInput';
@@ -22,53 +20,47 @@ export const RawJsonFieldInput = ({
   onTab,
   onShiftTab,
 }: RawJsonFieldInputProps) => {
-  const { fieldDefinition, draftValue, hotkeyScope, setDraftValue } =
-    useJsonField();
-
-  const persistField = usePersistField();
-
-  const handlePersistField = (newText: string) => {
-    if (!newText || isValidJSON(newText)) persistField(newText || null);
-  };
+  const {
+    fieldDefinition,
+    draftValue,
+    hotkeyScope,
+    setDraftValue,
+    persistJsonField,
+  } = useJsonField();
 
   const handleEnter = (newText: string) => {
-    onEnter?.(() => handlePersistField(newText));
+    onEnter?.(() => persistJsonField(newText));
   };
 
   const handleEscape = (newText: string) => {
-    onEscape?.(() => handlePersistField(newText));
+    onEscape?.(() => persistJsonField(newText));
   };
 
   const handleClickOutside = (
     _event: MouseEvent | TouchEvent,
     newText: string,
   ) => {
-    onClickOutside?.(() => handlePersistField(newText));
+    onClickOutside?.(() => persistJsonField(newText));
   };
 
   const handleTab = (newText: string) => {
-    onTab?.(() => handlePersistField(newText));
+    onTab?.(() => persistJsonField(newText));
   };
 
   const handleShiftTab = (newText: string) => {
-    onShiftTab?.(() => handlePersistField(newText));
+    onShiftTab?.(() => persistJsonField(newText));
   };
 
   const handleChange = (newText: string) => {
     setDraftValue(newText);
   };
 
-  const value =
-    draftValue && isValidJSON(draftValue)
-      ? JSON.stringify(JSON.parse(draftValue), null, 2)
-      : draftValue ?? '';
-
   return (
     <FieldTextAreaOverlay>
       <TextAreaInput
         placeholder={fieldDefinition.metadata.placeHolder}
         autoFocus
-        value={value}
+        value={draftValue ?? ''}
         onClickOutside={handleClickOutside}
         onEnter={handleEnter}
         onEscape={handleEscape}

diff --git a/packages/twenty-front/src/modules/object-record/record-field/types/FieldInputDraftValue.ts b/packages/twenty-front/src/modules/object-record/record-field/types/FieldInputDraftValue.ts
@@ -6,6 +6,7 @@ import {
   FieldDateTimeValue,
   FieldEmailValue,
   FieldFullNameValue,
+  FieldJsonValue,
   FieldLinksValue,
   FieldLinkValue,
   FieldMultiSelectValue,
@@ -47,6 +48,7 @@ export type FieldAddressDraftValue = {
   addressLat: number | null;
   addressLng: number | null;
 };
+export type FieldJsonDraftValue = string;
 
 export type FieldInputDraftValue<FieldValue> = FieldValue extends FieldTextValue
   ? FieldTextDraftValue
@@ -80,4 +82,6 @@ export type FieldInputDraftValue<FieldValue> = FieldValue extends FieldTextValue
                               ? FieldRelationDraftValue
                               : FieldValue extends FieldAddressValue
                                 ? FieldAddressDraftValue
-                                : never;
+                                : FieldValue extends FieldJsonValue
+                                  ? FieldJsonDraftValue
+                                  : never;
diff --git a/packages/twenty-front/src/modules/object-record/record-field/types/FieldMetadata.ts b/packages/twenty-front/src/modules/object-record/record-field/types/FieldMetadata.ts
@@ -173,4 +173,8 @@ export type FieldSelectValue = string | null;
 export type FieldMultiSelectValue = string[] | null;
 
 export type FieldRelationValue = EntityForSelect | null;
-export type FieldJsonValue = string;
+
+// See https://zod.dev/?id=json-type
+type Literal = string | number | boolean | null;
+export type Json = Literal | { [key: string]: Json } | Json[];
+export type FieldJsonValue = Record<string, Json> | Json[] | null;
diff --git a/...enty-front/src/modules/object-record/record-field/types/guards/isFieldDisplayedAsPhone.ts b/...enty-front/src/modules/object-record/record-field/types/guards/isFieldDisplayedAsPhone.ts
@@ -0,0 +1,14 @@
+import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular';
+import { FieldMetadataType } from '~/generated-metadata/graphql';
+
+import { FieldDefinition } from '../FieldDefinition';
+import { FieldMetadata, FieldTextMetadata } from '../FieldMetadata';
+
+// TODO: temporary - remove when 'Phone' field in 'Person' object
+// is migrated to use FieldMetadataType.Phone as type.
+export const isFieldDisplayedAsPhone = (
+  field: Pick<FieldDefinition<FieldMetadata>, 'type' | 'metadata'>,
+): field is FieldDefinition<FieldTextMetadata> =>
+  field.metadata.objectMetadataNameSingular === CoreObjectNameSingular.Person &&
+  field.type === FieldMetadataType.Text &&
+  field.metadata.fieldName === 'phone';
diff --git a/...s/twenty-front/src/modules/object-record/record-field/types/guards/isFieldRawJsonValue.ts b/...s/twenty-front/src/modules/object-record/record-field/types/guards/isFieldRawJsonValue.ts
@@ -1,8 +1,20 @@
-import { isNull, isString } from '@sniptt/guards';
+import { z } from 'zod';
 
-import { FieldJsonValue } from '../FieldMetadata';
+import { FieldJsonValue, Json } from '../FieldMetadata';
+
+// See https://zod.dev/?id=json-type
+const literalSchema = z.union([z.string(), z.number(), z.boolean(), z.null()]);
+const jsonSchema: z.ZodType<Json> = z.lazy(() =>
+  z.union([literalSchema, z.array(jsonSchema), z.record(jsonSchema)]),
+);
+
+export const jsonWithoutLiteralsSchema: z.ZodType<FieldJsonValue> = z.union([
+  z.null(), // Exclude literal values other than null
+  z.array(jsonSchema),
+  z.record(jsonSchema),
+]);
 
-// TODO: add zod
 export const isFieldRawJsonValue = (
   fieldValue: unknown,
-): fieldValue is FieldJsonValue => isString(fieldValue) || isNull(fieldValue);
+): fieldValue is FieldJsonValue =>
+  jsonWithoutLiteralsSchema.safeParse(fieldValue).success;
diff --git a/...nty-front/src/modules/object-record/record-field/utils/computeDraftValueFromFieldValue.ts b/...nty-front/src/modules/object-record/record-field/utils/computeDraftValueFromFieldValue.ts
@@ -3,6 +3,8 @@ import { FieldInputDraftValue } from '@/object-record/record-field/types/FieldIn
 import { FieldMetadata } from '@/object-record/record-field/types/FieldMetadata';
 import { isFieldCurrency } from '@/object-record/record-field/types/guards/isFieldCurrency';
 import { isFieldCurrencyValue } from '@/object-record/record-field/types/guards/isFieldCurrencyValue';
+import { isFieldRawJson } from '@/object-record/record-field/types/guards/isFieldRawJson';
+import { isFieldRawJsonValue } from '@/object-record/record-field/types/guards/isFieldRawJsonValue';
 import { isFieldRelation } from '@/object-record/record-field/types/guards/isFieldRelation';
 import { computeEmptyDraftValue } from '@/object-record/record-field/utils/computeEmptyDraftValue';
 import { isFieldValueEmpty } from '@/object-record/record-field/utils/isFieldValueEmpty';
@@ -33,9 +35,20 @@ export const computeDraftValueFromFieldValue = <FieldValue>({
       currencyCode: fieldValue?.currencyCode ?? '',
     } as unknown as FieldInputDraftValue<FieldValue>;
   }
+
   if (isFieldRelation(fieldDefinition)) {
     return computeEmptyDraftValue<FieldValue>({ fieldDefinition });
   }
 
+  if (isFieldRawJson(fieldDefinition)) {
+    return isFieldRawJsonValue(fieldValue)
+      ? (JSON.stringify(
+          fieldValue,
+          null,
+          2,
+        ) as FieldInputDraftValue<FieldValue>)
+      : computeEmptyDraftValue<FieldValue>({ fieldDefinition });
+  }
+
   return fieldValue as FieldInputDraftValue<FieldValue>;
 };
diff --git a/packages/twenty-front/src/modules/object-record/record-field/utils/computeEmptyDraftValue.ts b/packages/twenty-front/src/modules/object-record/record-field/utils/computeEmptyDraftValue.ts
@@ -8,7 +8,8 @@ import { isFieldEmail } from '@/object-record/record-field/types/guards/isFieldE
 import { isFieldFullName } from '@/object-record/record-field/types/guards/isFieldFullName';
 import { isFieldLink } from '@/object-record/record-field/types/guards/isFieldLink';
 import { isFieldNumber } from '@/object-record/record-field/types/guards/isFieldNumber';
-import { isFieldRelationValue } from '@/object-record/record-field/types/guards/isFieldRelationValue';
+import { isFieldRawJson } from '@/object-record/record-field/types/guards/isFieldRawJson';
+import { isFieldRelation } from '@/object-record/record-field/types/guards/isFieldRelation';
 import { isFieldText } from '@/object-record/record-field/types/guards/isFieldText';
 import { isFieldUuid } from '@/object-record/record-field/types/guards/isFieldUuid';
 
@@ -26,7 +27,8 @@ export const computeEmptyDraftValue = <FieldValue>({
     isFieldDateTime(fieldDefinition) ||
     isFieldNumber(fieldDefinition) ||
     isFieldEmail(fieldDefinition) ||
-    isFieldRelationValue(fieldDefinition)
+    isFieldRelation(fieldDefinition) ||
+    isFieldRawJson(fieldDefinition)
   ) {
     return '' as FieldInputDraftValue<FieldValue>;
   }

diff --git a/packages/twenty-front/src/modules/object-record/record-field/utils/isFieldValueJson.ts b/packages/twenty-front/src/modules/object-record/record-field/utils/isFieldValueJson.ts