From 29745c675689809c79276d18feac7c9d5e219031 Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Wed, 29 Jan 2025 16:00:59 +0100 Subject: [PATCH] [BUG] Fix record relation optimistic mutation (#9881) # Introduction It seems like optimistic caching isn't working as expected for any record relation mutation, CREATE UPDATE DELETE. It should not have an impact on the destroy We included a new `computeOptimisticRecordInput` that will calculate if a relation is added or detach. Updated the `triggerCreateRecordsOptimisticEffect` signature we should have a look to each of its call to determine if it should be checking cache or not Related to #9580 --------- Co-authored-by: Charles Bochet --- packages/twenty-front/jest.config.ts | 3 + .../hooks/useOpenCreateActivityDrawer.ts | 6 +- .../triggerCreateRecordsOptimisticEffect.ts | 45 +++-- .../triggerUpdateRelationsOptimisticEffect.ts | 5 +- .../favorites/hooks/useCreateFavorite.ts | 1 - .../cache/utils/getRecordFromCache.ts | 15 +- .../hooks/__mocks__/useUpdateOneRecord.ts | 1 - .../__tests__/useUpdateOneRecord.test.tsx | 8 +- .../hooks/useCreateManyRecords.ts | 79 +++++--- .../object-record/hooks/useCreateOneRecord.ts | 53 +++-- .../object-record/hooks/useUpdateOneRecord.ts | 26 ++- .../record-field/hooks/usePersistField.ts | 5 +- .../RecordDetailRelationRecordsListItem.tsx | 5 +- .../computeOptimisticRecordFromInput.test.ts | 182 ++++++++++++++++++ .../utils/computeOptimisticRecordFromInput.ts | 153 +++++++++++++++ .../getForeignKeyNameFromRelationFieldName.ts | 2 + .../utils/sanitizeRecordInput.ts | 15 +- 17 files changed, 502 insertions(+), 102 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-record/utils/__tests__/computeOptimisticRecordFromInput.test.ts create mode 100644 packages/twenty-front/src/modules/object-record/utils/computeOptimisticRecordFromInput.ts create mode 100644 packages/twenty-front/src/modules/object-record/utils/getForeignKeyNameFromRelationFieldName.ts diff --git a/packages/twenty-front/jest.config.ts b/packages/twenty-front/jest.config.ts index 7c3777ae4..0047e60f3 100644 --- a/packages/twenty-front/jest.config.ts +++ b/packages/twenty-front/jest.config.ts @@ -6,6 +6,9 @@ process.env.TZ = 'GMT'; const jestConfig: JestConfigWithTsJest = { // to enable logs, comment out the following line silent: true, + // For more information please have a look to official docs https://jestjs.io/docs/configuration/#prettierpath-string + // Prettier v3 will should be supported in jest v30 https://github.com/jestjs/jest/releases/tag/v30.0.0-alpha.1 + prettierPath: null, displayName: 'twenty-front', preset: '../../jest.preset.js', setupFilesAfterEnv: ['./setupTests.ts'], diff --git a/packages/twenty-front/src/modules/activities/hooks/useOpenCreateActivityDrawer.ts b/packages/twenty-front/src/modules/activities/hooks/useOpenCreateActivityDrawer.ts index bbaf0c183..6513ac7d2 100644 --- a/packages/twenty-front/src/modules/activities/hooks/useOpenCreateActivityDrawer.ts +++ b/packages/twenty-front/src/modules/activities/hooks/useOpenCreateActivityDrawer.ts @@ -80,7 +80,11 @@ export const useOpenCreateActivityDrawer = ({ setViewableRecordNameSingular(activityObjectNameSingular); const activity = await createOneActivity({ - assigneeId: customAssignee?.id, + ...(activityObjectNameSingular === CoreObjectNameSingular.Task + ? { + assigneeId: customAssignee?.id, + } + : {}), position: 'last', }); diff --git a/packages/twenty-front/src/modules/apollo/optimistic-effect/utils/triggerCreateRecordsOptimisticEffect.ts b/packages/twenty-front/src/modules/apollo/optimistic-effect/utils/triggerCreateRecordsOptimisticEffect.ts index 5e8a3abf0..54af02ea6 100644 --- a/packages/twenty-front/src/modules/apollo/optimistic-effect/utils/triggerCreateRecordsOptimisticEffect.ts +++ b/packages/twenty-front/src/modules/apollo/optimistic-effect/utils/triggerCreateRecordsOptimisticEffect.ts @@ -11,6 +11,8 @@ import { isRecordMatchingFilter } from '@/object-record/record-filter/utils/isRe import { CachedObjectRecordQueryVariables } from '@/apollo/types/CachedObjectRecordQueryVariables'; import { encodeCursor } from '@/apollo/utils/encodeCursor'; +import { getRecordFromCache } from '@/object-record/cache/utils/getRecordFromCache'; +import { getRecordNodeFromRecord } from '@/object-record/cache/utils/getRecordNodeFromRecord'; import { isDefined } from '~/utils/isDefined'; import { parseApolloStoreFieldName } from '~/utils/parseApolloStoreFieldName'; @@ -19,28 +21,49 @@ import { parseApolloStoreFieldName } from '~/utils/parseApolloStoreFieldName'; We need to refactor how the record creation works in the RecordTable so the created record row is temporarily displayed with a local state, then we'll be able to uncomment the code below so the cached lists are updated coherently with the variables. */ +type TriggerCreateRecordsOptimisticEffectArgs = { + cache: ApolloCache; + objectMetadataItem: ObjectMetadataItem; + recordsToCreate: RecordGqlNode[]; + objectMetadataItems: ObjectMetadataItem[]; + shouldMatchRootQueryFilter?: boolean; + checkForRecordInCache?: boolean; +}; export const triggerCreateRecordsOptimisticEffect = ({ cache, objectMetadataItem, recordsToCreate, objectMetadataItems, shouldMatchRootQueryFilter, -}: { - cache: ApolloCache; - objectMetadataItem: ObjectMetadataItem; - recordsToCreate: RecordGqlNode[]; - objectMetadataItems: ObjectMetadataItem[]; - shouldMatchRootQueryFilter?: boolean; -}) => { - recordsToCreate.forEach((record) => + checkForRecordInCache = false, +}: TriggerCreateRecordsOptimisticEffectArgs) => { + const getRecordNodeFromCache = (recordId: string): RecordGqlNode | null => { + const cachedRecord = getRecordFromCache({ + cache, + objectMetadataItem, + objectMetadataItems, + recordId, + }); + return getRecordNodeFromRecord({ + objectMetadataItem, + objectMetadataItems, + record: cachedRecord, + computeReferences: false, + }); + }; + + recordsToCreate.forEach((record) => { + const currentSourceRecord = checkForRecordInCache + ? getRecordNodeFromCache(record.id) + : null; triggerUpdateRelationsOptimisticEffect({ cache, sourceObjectMetadataItem: objectMetadataItem, - currentSourceRecord: null, + currentSourceRecord, updatedSourceRecord: record, objectMetadataItems, - }), - ); + }); + }); cache.modify({ fields: { diff --git a/packages/twenty-front/src/modules/apollo/optimistic-effect/utils/triggerUpdateRelationsOptimisticEffect.ts b/packages/twenty-front/src/modules/apollo/optimistic-effect/utils/triggerUpdateRelationsOptimisticEffect.ts index 078f011d1..f62650f68 100644 --- a/packages/twenty-front/src/modules/apollo/optimistic-effect/utils/triggerUpdateRelationsOptimisticEffect.ts +++ b/packages/twenty-front/src/modules/apollo/optimistic-effect/utils/triggerUpdateRelationsOptimisticEffect.ts @@ -7,7 +7,6 @@ import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; import { isObjectRecordConnection } from '@/object-record/cache/utils/isObjectRecordConnection'; import { RecordGqlConnection } from '@/object-record/graphql/types/RecordGqlConnection'; import { RecordGqlNode } from '@/object-record/graphql/types/RecordGqlNode'; -import { ObjectRecord } from '@/object-record/types/ObjectRecord'; import { ApolloCache } from '@apollo/client'; import { isArray } from '@sniptt/guards'; import { FieldMetadataType } from '~/generated-metadata/graphql'; @@ -17,8 +16,8 @@ import { isDefined } from '~/utils/isDefined'; type triggerUpdateRelationsOptimisticEffectArgs = { cache: ApolloCache; sourceObjectMetadataItem: ObjectMetadataItem; - currentSourceRecord: ObjectRecord | null; - updatedSourceRecord: ObjectRecord | null; + currentSourceRecord: RecordGqlNode | null; + updatedSourceRecord: RecordGqlNode | null; objectMetadataItems: ObjectMetadataItem[]; }; export const triggerUpdateRelationsOptimisticEffect = ({ diff --git a/packages/twenty-front/src/modules/favorites/hooks/useCreateFavorite.ts b/packages/twenty-front/src/modules/favorites/hooks/useCreateFavorite.ts index e4906401d..edb7311bf 100644 --- a/packages/twenty-front/src/modules/favorites/hooks/useCreateFavorite.ts +++ b/packages/twenty-front/src/modules/favorites/hooks/useCreateFavorite.ts @@ -26,7 +26,6 @@ export const useCreateFavorite = () => { ); createOneFavorite({ - [targetObjectNameSingular]: targetRecord, [`${targetObjectNameSingular}Id`]: targetRecord.id, position: maxPosition + 1, workspaceMemberId: currentWorkspaceMemberId, diff --git a/packages/twenty-front/src/modules/object-record/cache/utils/getRecordFromCache.ts b/packages/twenty-front/src/modules/object-record/cache/utils/getRecordFromCache.ts index 1d9451f2a..9a2fcb63d 100644 --- a/packages/twenty-front/src/modules/object-record/cache/utils/getRecordFromCache.ts +++ b/packages/twenty-front/src/modules/object-record/cache/utils/getRecordFromCache.ts @@ -9,19 +9,20 @@ import { ObjectRecord } from '@/object-record/types/ObjectRecord'; import { capitalize } from 'twenty-shared'; import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull'; +export type GetRecordFromCacheArgs = { + cache: ApolloCache; + recordId: string; + objectMetadataItems: ObjectMetadataItem[]; + objectMetadataItem: ObjectMetadataItem; + recordGqlFields?: RecordGqlFields; +}; export const getRecordFromCache = ({ objectMetadataItem, objectMetadataItems, cache, recordId, recordGqlFields, -}: { - cache: ApolloCache; - recordId: string; - objectMetadataItems: ObjectMetadataItem[]; - objectMetadataItem: ObjectMetadataItem; - recordGqlFields?: RecordGqlFields; -}) => { +}: GetRecordFromCacheArgs) => { if (isUndefinedOrNull(objectMetadataItem)) { return null; } diff --git a/packages/twenty-front/src/modules/object-record/hooks/__mocks__/useUpdateOneRecord.ts b/packages/twenty-front/src/modules/object-record/hooks/__mocks__/useUpdateOneRecord.ts index b367f09cf..3f1747cdf 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/__mocks__/useUpdateOneRecord.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/__mocks__/useUpdateOneRecord.ts @@ -57,7 +57,6 @@ const connectedObjects = { export const variables = { idToUpdate: '36abbb63-34ed-4a16-89f5-f549ac55d0f9', input: { - ...basePerson, name: { firstName: 'John', lastName: 'Doe' }, }, }; diff --git a/packages/twenty-front/src/modules/object-record/hooks/__tests__/useUpdateOneRecord.test.tsx b/packages/twenty-front/src/modules/object-record/hooks/__tests__/useUpdateOneRecord.test.tsx index 557892a1f..6ab1e4894 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/__tests__/useUpdateOneRecord.test.tsx +++ b/packages/twenty-front/src/modules/object-record/hooks/__tests__/useUpdateOneRecord.test.tsx @@ -11,7 +11,7 @@ import { expect } from '@storybook/test'; import { getJestMetadataAndApolloMocksWrapper } from '~/testing/jest/getJestMetadataAndApolloMocksWrapper'; const person = { id: '36abbb63-34ed-4a16-89f5-f549ac55d0f9' }; -const update = { +const updateInput = { name: { firstName: 'John', lastName: 'Doe', @@ -20,7 +20,7 @@ const update = { const updatePerson = { ...person, ...responseData, - ...update, + ...updateInput, }; const mocks = [ @@ -64,11 +64,11 @@ describe('useUpdateOneRecord', () => { await act(async () => { const res = await result.current.updateOneRecord({ idToUpdate, - updateOneRecordInput: updatePerson, + updateOneRecordInput: updateInput, }); expect(res).toBeDefined(); expect(res).toHaveProperty('id', person.id); - expect(res).toHaveProperty('name', update.name); + expect(res).toHaveProperty('name', updateInput.name); }); expect(mocks[0].result).toHaveBeenCalled(); diff --git a/packages/twenty-front/src/modules/object-record/hooks/useCreateManyRecords.ts b/packages/twenty-front/src/modules/object-record/hooks/useCreateManyRecords.ts index d2cb0c69e..da2b76662 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useCreateManyRecords.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useCreateManyRecords.ts @@ -8,15 +8,21 @@ import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadat import { useCreateOneRecordInCache } from '@/object-record/cache/hooks/useCreateOneRecordInCache'; import { deleteRecordFromCache } from '@/object-record/cache/utils/deleteRecordFromCache'; import { getObjectTypename } from '@/object-record/cache/utils/getObjectTypename'; +import { getRecordNodeFromRecord } from '@/object-record/cache/utils/getRecordNodeFromRecord'; import { RecordGqlOperationGqlRecordFields } from '@/object-record/graphql/types/RecordGqlOperationGqlRecordFields'; import { generateDepthOneRecordGqlFields } from '@/object-record/graphql/utils/generateDepthOneRecordGqlFields'; import { useCreateManyRecordsMutation } from '@/object-record/hooks/useCreateManyRecordsMutation'; import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries'; import { ObjectRecord } from '@/object-record/types/ObjectRecord'; +import { computeOptimisticRecordFromInput } from '@/object-record/utils/computeOptimisticRecordFromInput'; import { getCreateManyRecordsMutationResponseField } from '@/object-record/utils/getCreateManyRecordsMutationResponseField'; import { sanitizeRecordInput } from '@/object-record/utils/sanitizeRecordInput'; import { isDefined } from '~/utils/isDefined'; +type PartialObjectRecordWithId = Partial & { + id: string; +}; + type useCreateManyRecordsProps = { objectNameSingular: string; recordGqlFields?: RecordGqlOperationGqlRecordFields; @@ -60,42 +66,56 @@ export const useCreateManyRecords = < recordsToCreate: Partial[], upsert?: boolean, ) => { - const sanitizedCreateManyRecordsInput = recordsToCreate.map( - (recordToCreate) => { - const idForCreation = recordToCreate?.id ?? (upsert ? undefined : v4()); + const sanitizedCreateManyRecordsInput: PartialObjectRecordWithId[] = []; + const recordOptimisticRecordsInput: PartialObjectRecordWithId[] = []; + recordsToCreate.forEach((recordToCreate) => { + const idForCreation = recordToCreate?.id ?? v4(); + const sanitizedRecord = { + ...sanitizeRecordInput({ + objectMetadataItem, + recordInput: recordToCreate, + }), + id: idForCreation, + }; + const optimisticRecordInput = { + ...computeOptimisticRecordFromInput({ + cache: apolloClient.cache, + objectMetadataItem, + objectMetadataItems, + recordInput: recordToCreate, + }), + id: idForCreation, + }; - return { - ...sanitizeRecordInput({ - objectMetadataItem, - recordInput: recordToCreate, - }), - id: idForCreation, - }; - }, - ); + sanitizedCreateManyRecordsInput.push(sanitizedRecord); + recordOptimisticRecordsInput.push(optimisticRecordInput); + }); - const recordsCreatedInCache: ObjectRecord[] = []; - - for (const recordToCreate of sanitizedCreateManyRecordsInput) { - if (recordToCreate.id === null) { - continue; - } - - const recordCreatedInCache = createOneRecordInCache({ - ...(recordToCreate as { id: string }), - __typename: getObjectTypename(objectMetadataItem.nameSingular), - }); - - if (isDefined(recordCreatedInCache)) { - recordsCreatedInCache.push(recordCreatedInCache); - } - } + const recordsCreatedInCache = recordOptimisticRecordsInput + .map((recordToCreate) => + createOneRecordInCache({ + ...recordToCreate, + __typename: getObjectTypename(objectMetadataItem.nameSingular), + }), + ) + .filter(isDefined); if (recordsCreatedInCache.length > 0) { + const recordNodeCreatedInCache = recordsCreatedInCache + .map((record) => + getRecordNodeFromRecord({ + objectMetadataItem, + objectMetadataItems, + record: record, + computeReferences: false, + }), + ) + .filter(isDefined); + triggerCreateRecordsOptimisticEffect({ cache: apolloClient.cache, objectMetadataItem, - recordsToCreate: recordsCreatedInCache, + recordsToCreate: recordNodeCreatedInCache, objectMetadataItems, shouldMatchRootQueryFilter, }); @@ -123,6 +143,7 @@ export const useCreateManyRecords = < recordsToCreate: records, objectMetadataItems, shouldMatchRootQueryFilter, + checkForRecordInCache: true, }); }, }) diff --git a/packages/twenty-front/src/modules/object-record/hooks/useCreateOneRecord.ts b/packages/twenty-front/src/modules/object-record/hooks/useCreateOneRecord.ts index 3660dbd7e..cb3acf657 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useCreateOneRecord.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useCreateOneRecord.ts @@ -9,11 +9,13 @@ import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadat import { useCreateOneRecordInCache } from '@/object-record/cache/hooks/useCreateOneRecordInCache'; import { deleteRecordFromCache } from '@/object-record/cache/utils/deleteRecordFromCache'; import { getObjectTypename } from '@/object-record/cache/utils/getObjectTypename'; +import { getRecordNodeFromRecord } from '@/object-record/cache/utils/getRecordNodeFromRecord'; import { RecordGqlOperationGqlRecordFields } from '@/object-record/graphql/types/RecordGqlOperationGqlRecordFields'; import { generateDepthOneRecordGqlFields } from '@/object-record/graphql/utils/generateDepthOneRecordGqlFields'; import { useCreateOneRecordMutation } from '@/object-record/hooks/useCreateOneRecordMutation'; import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries'; import { ObjectRecord } from '@/object-record/types/ObjectRecord'; +import { computeOptimisticRecordFromInput } from '@/object-record/utils/computeOptimisticRecordFromInput'; import { getCreateOneRecordMutationResponseField } from '@/object-record/utils/getCreateOneRecordMutationResponseField'; import { sanitizeRecordInput } from '@/object-record/utils/sanitizeRecordInput'; import { isDefined } from '~/utils/isDefined'; @@ -60,33 +62,48 @@ export const useCreateOneRecord = < objectMetadataNamePlural: objectMetadataItem.namePlural, }); - const createOneRecord = async (input: Partial) => { + const createOneRecord = async (recordInput: Partial) => { setLoading(true); - const idForCreation = input.id ?? v4(); + const idForCreation = recordInput.id ?? v4(); const sanitizedInput = { ...sanitizeRecordInput({ objectMetadataItem, - recordInput: input, + recordInput, }), id: idForCreation, }; + const optimisticRecordInput = computeOptimisticRecordFromInput({ + cache: apolloClient.cache, + objectMetadataItem, + objectMetadataItems, + recordInput: { ...recordInput, id: idForCreation }, + }); const recordCreatedInCache = createOneRecordInCache({ - ...input, + ...optimisticRecordInput, id: idForCreation, __typename: getObjectTypename(objectMetadataItem.nameSingular), }); if (isDefined(recordCreatedInCache)) { - triggerCreateRecordsOptimisticEffect({ - cache: apolloClient.cache, + const optimisticRecordNode = getRecordNodeFromRecord({ objectMetadataItem, - recordsToCreate: [recordCreatedInCache], objectMetadataItems, - shouldMatchRootQueryFilter, + record: recordCreatedInCache, + computeReferences: false, }); + + if (optimisticRecordNode !== null) { + triggerCreateRecordsOptimisticEffect({ + cache: apolloClient.cache, + objectMetadataItem, + recordsToCreate: [optimisticRecordNode], + objectMetadataItems, + shouldMatchRootQueryFilter, + }); + } } const mutationResponseField = @@ -100,16 +117,16 @@ export const useCreateOneRecord = < }, update: (cache, { data }) => { const record = data?.[mutationResponseField]; - - if (!record || skipPostOptmisticEffect) return; - - triggerCreateRecordsOptimisticEffect({ - cache, - objectMetadataItem, - recordsToCreate: [record], - objectMetadataItems, - shouldMatchRootQueryFilter, - }); + if (skipPostOptmisticEffect === false && isDefined(record)) { + triggerCreateRecordsOptimisticEffect({ + cache, + objectMetadataItem, + recordsToCreate: [record], + objectMetadataItems, + shouldMatchRootQueryFilter, + checkForRecordInCache: true, + }); + } setLoading(false); }, diff --git a/packages/twenty-front/src/modules/object-record/hooks/useUpdateOneRecord.ts b/packages/twenty-front/src/modules/object-record/hooks/useUpdateOneRecord.ts index be030f554..023b853e0 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useUpdateOneRecord.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useUpdateOneRecord.ts @@ -10,6 +10,7 @@ import { generateDepthOneRecordGqlFields } from '@/object-record/graphql/utils/g import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries'; import { useUpdateOneRecordMutation } from '@/object-record/hooks/useUpdateOneRecordMutation'; import { ObjectRecord } from '@/object-record/types/ObjectRecord'; +import { computeOptimisticRecordFromInput } from '@/object-record/utils/computeOptimisticRecordFromInput'; import { getUpdateOneRecordMutationResponseField } from '@/object-record/utils/getUpdateOneRecordMutationResponseField'; import { sanitizeRecordInput } from '@/object-record/utils/sanitizeRecordInput'; import { capitalize } from 'twenty-shared'; @@ -59,12 +60,12 @@ export const useUpdateOneRecord = < updateOneRecordInput: Partial>; optimisticRecord?: Partial; }) => { - const sanitizedInput = { - ...sanitizeRecordInput({ - objectMetadataItem, - recordInput: updateOneRecordInput, - }), - }; + const optimisticRecordInput = computeOptimisticRecordFromInput({ + objectMetadataItem, + recordInput: updateOneRecordInput, + cache: apolloClient.cache, + objectMetadataItems, + }); const cachedRecord = getRecordFromCache(idToUpdate); @@ -73,12 +74,12 @@ export const useUpdateOneRecord = < objectMetadataItem, objectMetadataItems, recordGqlFields: computedRecordGqlFields, - computeReferences: true, + computeReferences: false, }); const computedOptimisticRecord = { ...cachedRecord, - ...(optimisticRecord ?? sanitizedInput), + ...(optimisticRecord ?? optimisticRecordInput), ...{ id: idToUpdate }, ...{ __typename: capitalize(objectMetadataItem.nameSingular) }, }; @@ -89,9 +90,8 @@ export const useUpdateOneRecord = < objectMetadataItem, objectMetadataItems, recordGqlFields: computedRecordGqlFields, - computeReferences: true, + computeReferences: false, }); - if (!optimisticRecordWithConnection || !cachedRecordWithConnection) { return null; } @@ -114,6 +114,12 @@ export const useUpdateOneRecord = < const mutationResponseField = getUpdateOneRecordMutationResponseField(objectNameSingular); + const sanitizedInput = { + ...sanitizeRecordInput({ + objectMetadataItem, + recordInput: updateOneRecordInput, + }), + }; const updatedRecord = await apolloClient .mutate({ mutation: updateOneRecordMutation, diff --git a/packages/twenty-front/src/modules/object-record/record-field/hooks/usePersistField.ts b/packages/twenty-front/src/modules/object-record/record-field/hooks/usePersistField.ts index ecb9711af..40a9ce6ae 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/hooks/usePersistField.ts +++ b/packages/twenty-front/src/modules/object-record/record-field/hooks/usePersistField.ts @@ -30,6 +30,7 @@ import { isFieldArray } from '@/object-record/record-field/types/guards/isFieldA import { isFieldArrayValue } from '@/object-record/record-field/types/guards/isFieldArrayValue'; import { isFieldRichText } from '@/object-record/record-field/types/guards/isFieldRichText'; import { isFieldRichTextValue } from '@/object-record/record-field/types/guards/isFieldRichTextValue'; +import { getForeignKeyNameFromRelationFieldName } from '@/object-record/utils/getForeignKeyNameFromRelationFieldName'; import { FieldContext } from '../contexts/FieldContext'; import { isFieldBoolean } from '../types/guards/isFieldBoolean'; import { isFieldBooleanValue } from '../types/guards/isFieldBooleanValue'; @@ -153,8 +154,8 @@ export const usePersistField = () => { variables: { where: { id: recordId }, updateOneRecordInput: { - [fieldName]: value, - [`${fieldName}Id`]: value?.id ?? null, + [getForeignKeyNameFromRelationFieldName(fieldName)]: + value?.id ?? null, }, }, }); diff --git a/packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx b/packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx index 69edb617e..3c03d53ee 100644 --- a/packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx +++ b/packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx @@ -35,6 +35,7 @@ import { InlineCellHotkeyScope } from '@/object-record/record-inline-cell/types/ import { RecordDetailRecordsListItem } from '@/object-record/record-show/record-detail-section/components/RecordDetailRecordsListItem'; import { RecordValueSetterEffect } from '@/object-record/record-store/components/RecordValueSetterEffect'; import { ObjectRecord } from '@/object-record/types/ObjectRecord'; +import { getForeignKeyNameFromRelationFieldName } from '@/object-record/utils/getForeignKeyNameFromRelationFieldName'; import { isFieldCellSupported } from '@/object-record/utils/isFieldCellSupported'; import { Dropdown } from '@/ui/layout/dropdown/components/Dropdown'; import { DropdownMenuItemsContainer } from '@/ui/layout/dropdown/components/DropdownMenuItemsContainer'; @@ -162,7 +163,9 @@ export const RecordDetailRelationRecordsListItem = ({ updateOneRelationRecord({ idToUpdate: relationRecord.id, updateOneRecordInput: { - [relationFieldMetadataItem.name]: null, + [getForeignKeyNameFromRelationFieldName( + relationFieldMetadataItem.name, + )]: null, }, }); }; diff --git a/packages/twenty-front/src/modules/object-record/utils/__tests__/computeOptimisticRecordFromInput.test.ts b/packages/twenty-front/src/modules/object-record/utils/__tests__/computeOptimisticRecordFromInput.test.ts new file mode 100644 index 000000000..825e71d34 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/utils/__tests__/computeOptimisticRecordFromInput.test.ts @@ -0,0 +1,182 @@ +import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache'; +import { computeOptimisticRecordFromInput } from '@/object-record/utils/computeOptimisticRecordFromInput'; +import { InMemoryCache } from '@apollo/client'; +import { generatedMockObjectMetadataItems } from '~/testing/mock-data/generatedMockObjectMetadataItems'; + +describe('computeOptimisticRecordFromInput', () => { + it('should generate correct optimistic record if no relation field is present', () => { + const cache = new InMemoryCache(); + + const personObjectMetadataItem = generatedMockObjectMetadataItems.find( + (item) => item.nameSingular === 'person', + ); + + if (!personObjectMetadataItem) { + throw new Error('Person object metadata item not found'); + } + + const result = computeOptimisticRecordFromInput({ + objectMetadataItems: generatedMockObjectMetadataItems, + objectMetadataItem: personObjectMetadataItem, + recordInput: { + city: 'Paris', + }, + cache, + }); + + expect(result).toEqual({ + city: 'Paris', + }); + }); + + it('should generate correct optimistic record if relation field is present but cache is empty', () => { + const cache = new InMemoryCache(); + + const personObjectMetadataItem = generatedMockObjectMetadataItems.find( + (item) => item.nameSingular === 'person', + ); + + if (!personObjectMetadataItem) { + throw new Error('Person object metadata item not found'); + } + + const result = computeOptimisticRecordFromInput({ + objectMetadataItems: generatedMockObjectMetadataItems, + objectMetadataItem: personObjectMetadataItem, + recordInput: { + companyId: '123', + }, + cache, + }); + + expect(result).toEqual({ + companyId: '123', + }); + }); + + it('should generate correct optimistic record if relation field is present and cache is not empty', () => { + const cache = new InMemoryCache(); + const personObjectMetadataItem = generatedMockObjectMetadataItems.find( + (item) => item.nameSingular === 'person', + ); + + if (!personObjectMetadataItem) { + throw new Error('Person object metadata item not found'); + } + + const companyObjectMetadataItem = generatedMockObjectMetadataItems.find( + (item) => item.nameSingular === 'company', + ); + + if (!companyObjectMetadataItem) { + throw new Error('Company object metadata item not found'); + } + + const companyRecord = { + id: '123', + __typename: 'Company', + }; + + updateRecordFromCache({ + objectMetadataItems: generatedMockObjectMetadataItems, + objectMetadataItem: { + ...companyObjectMetadataItem, + fields: companyObjectMetadataItem.fields.filter( + (field) => field.name === 'id', + ), + }, + cache, + record: companyRecord, + }); + + const result = computeOptimisticRecordFromInput({ + objectMetadataItems: generatedMockObjectMetadataItems, + objectMetadataItem: personObjectMetadataItem, + recordInput: { + companyId: '123', + }, + cache, + }); + + expect(result).toEqual({ + companyId: '123', + company: companyRecord, + }); + }); + + it('should generate correct optimistic record if relation field is null and cache is empty', () => { + const cache = new InMemoryCache(); + + const personObjectMetadataItem = generatedMockObjectMetadataItems.find( + (item) => item.nameSingular === 'person', + ); + + if (!personObjectMetadataItem) { + throw new Error('Person object metadata item not found'); + } + + const result = computeOptimisticRecordFromInput({ + objectMetadataItems: generatedMockObjectMetadataItems, + objectMetadataItem: personObjectMetadataItem, + recordInput: { + companyId: null, + }, + cache, + }); + + expect(result).toEqual({ + companyId: null, + company: null, + }); + }); + + it('should throw an error if recordInput contains fiels unrelated to the current objectMetadata', () => { + const cache = new InMemoryCache(); + const personObjectMetadataItem = generatedMockObjectMetadataItems.find( + (item) => item.nameSingular === 'person', + ); + if (!personObjectMetadataItem) { + throw new Error('Person object metadata item not found'); + } + + expect(() => + computeOptimisticRecordFromInput({ + objectMetadataItems: generatedMockObjectMetadataItems, + objectMetadataItem: personObjectMetadataItem, + recordInput: { + unknwon: 'unknown', + foo: 'foo', + bar: 'bar', + city: 'Paris', + }, + cache, + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Should never occur, encountered unknown fields unknwon, foo, bar in objectMetadaItem person"`, + ); + }); + + it('should throw an error if recordInput contains both the relationFieldId and relationField', () => { + const cache = new InMemoryCache(); + const personObjectMetadataItem = generatedMockObjectMetadataItems.find( + (item) => item.nameSingular === 'person', + ); + if (!personObjectMetadataItem) { + throw new Error('Person object metadata item not found'); + } + + expect(() => + computeOptimisticRecordFromInput({ + objectMetadataItems: generatedMockObjectMetadataItems, + objectMetadataItem: personObjectMetadataItem, + recordInput: { + companyId: '123', + company: {}, + }, + cache, + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Should never provide relation mutation through anything else than the fieldId e.g companyId"`, + ); + }); +}); diff --git a/packages/twenty-front/src/modules/object-record/utils/computeOptimisticRecordFromInput.ts b/packages/twenty-front/src/modules/object-record/utils/computeOptimisticRecordFromInput.ts new file mode 100644 index 000000000..932f89166 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/utils/computeOptimisticRecordFromInput.ts @@ -0,0 +1,153 @@ +import { isNull, isUndefined } from '@sniptt/guards'; + +import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; +import { + getRecordFromCache, + GetRecordFromCacheArgs, +} from '@/object-record/cache/utils/getRecordFromCache'; +import { isFieldRelation } from '@/object-record/record-field/types/guards/isFieldRelation'; +import { isFieldUuid } from '@/object-record/record-field/types/guards/isFieldUuid'; +import { ObjectRecord } from '@/object-record/types/ObjectRecord'; +import { getForeignKeyNameFromRelationFieldName } from '@/object-record/utils/getForeignKeyNameFromRelationFieldName'; +import { RelationDefinitionType } from '~/generated-metadata/graphql'; +import { FieldMetadataType } from '~/generated/graphql'; +import { isDefined } from '~/utils/isDefined'; + +type ComputeOptimisticCacheRecordInputArgs = { + objectMetadataItem: ObjectMetadataItem; + recordInput: Partial; +} & Pick; +export const computeOptimisticRecordFromInput = ({ + objectMetadataItem, + recordInput, + cache, + objectMetadataItems, +}: ComputeOptimisticCacheRecordInputArgs) => { + const unknownRecordInputFields = Object.keys(recordInput).filter( + (fieldName) => + objectMetadataItem.fields.find(({ name }) => name === fieldName) === + undefined, + ); + if (unknownRecordInputFields.length > 0) { + throw new Error( + `Should never occur, encountered unknown fields ${unknownRecordInputFields.join(', ')} in objectMetadaItem ${objectMetadataItem.nameSingular}`, + ); + } + + const optimisticRecord: Partial = {}; + for (const fieldMetadataItem of objectMetadataItem.fields) { + if (isFieldUuid(fieldMetadataItem)) { + const isRelationFieldId = objectMetadataItem.fields.some( + ({ type, relationDefinition }) => { + if (type !== FieldMetadataType.RELATION) { + return false; + } + + if (!isDefined(relationDefinition)) { + return false; + } + + const sourceFieldName = relationDefinition.sourceFieldMetadata.name; + return ( + getForeignKeyNameFromRelationFieldName(sourceFieldName) === + fieldMetadataItem.name + ); + }, + ); + + if (isRelationFieldId) { + continue; + } + } + + const isRelationField = isFieldRelation(fieldMetadataItem); + + const recordInputFieldValue: unknown = recordInput[fieldMetadataItem.name]; + + if (!isRelationField) { + if (!isDefined(recordInputFieldValue)) { + continue; + } + + if (!fieldMetadataItem.isNullable && recordInputFieldValue == null) { + continue; + } + + optimisticRecord[fieldMetadataItem.name] = recordInputFieldValue; + continue; + } + + if ( + fieldMetadataItem.relationDefinition?.direction === + RelationDefinitionType.ONE_TO_MANY + ) { + continue; + } + + const isManyToOneRelation = + fieldMetadataItem.relationDefinition?.direction === + RelationDefinitionType.MANY_TO_ONE; + if (!isManyToOneRelation) { + continue; + } + + if (isDefined(recordInputFieldValue)) { + throw new Error( + 'Should never provide relation mutation through anything else than the fieldId e.g companyId', + ); + } + + const relationFieldIdName = getForeignKeyNameFromRelationFieldName( + fieldMetadataItem.name, + ); + const recordInputFieldIdValue: string | null | undefined = + recordInput[relationFieldIdName]; + + if (isUndefined(recordInputFieldIdValue)) { + continue; + } + + const relationIdFieldMetadataItem = objectMetadataItem.fields.find( + (field) => field.name === relationFieldIdName, + ); + if (!isDefined(relationIdFieldMetadataItem)) { + throw new Error( + 'Should never occur, encountered unknown relationId within relations definitions', + ); + } + + if (isNull(recordInputFieldIdValue)) { + optimisticRecord[relationFieldIdName] = null; + optimisticRecord[fieldMetadataItem.name] = null; + continue; + } + + const targetNameSingular = + fieldMetadataItem.relationDefinition?.targetObjectMetadata.nameSingular; + const targetObjectMetataDataItem = objectMetadataItems.find( + ({ nameSingular }) => nameSingular === targetNameSingular, + ); + if (!isDefined(targetObjectMetataDataItem)) { + throw new Error( + 'Should never occur, encountered invalid relation definition', + ); + } + + const cachedRecord = getRecordFromCache({ + cache, + objectMetadataItem: targetObjectMetataDataItem, + objectMetadataItems, + recordId: recordInputFieldIdValue as string, + }); + + optimisticRecord[relationFieldIdName] = recordInputFieldIdValue; + + if (!isDefined(cachedRecord) || Object.keys(cachedRecord).length <= 0) { + continue; + } + + optimisticRecord[fieldMetadataItem.name] = cachedRecord; + } + + return optimisticRecord; +}; diff --git a/packages/twenty-front/src/modules/object-record/utils/getForeignKeyNameFromRelationFieldName.ts b/packages/twenty-front/src/modules/object-record/utils/getForeignKeyNameFromRelationFieldName.ts new file mode 100644 index 000000000..033975cf1 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/utils/getForeignKeyNameFromRelationFieldName.ts @@ -0,0 +1,2 @@ +export const getForeignKeyNameFromRelationFieldName = (nameSingular: string) => + `${nameSingular}Id`; diff --git a/packages/twenty-front/src/modules/object-record/utils/sanitizeRecordInput.ts b/packages/twenty-front/src/modules/object-record/utils/sanitizeRecordInput.ts index 387657386..c1043c0b4 100644 --- a/packages/twenty-front/src/modules/object-record/utils/sanitizeRecordInput.ts +++ b/packages/twenty-front/src/modules/object-record/utils/sanitizeRecordInput.ts @@ -1,11 +1,8 @@ -import { isString } from '@sniptt/guards'; - import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; import { ObjectRecord } from '@/object-record/types/ObjectRecord'; import { RelationDefinitionType } from '~/generated-metadata/graphql'; import { FieldMetadataType } from '~/generated/graphql'; import { isDefined } from '~/utils/isDefined'; -import { getUrlHostName } from '~/utils/url/getUrlHostName'; export const sanitizeRecordInput = ({ objectMetadataItem, @@ -56,15 +53,5 @@ export const sanitizeRecordInput = ({ }) .filter(isDefined), ); - if ( - !( - isDefined(filteredResultRecord.domainName) && - isString(filteredResultRecord.domainName) - ) - ) - return filteredResultRecord; - return { - ...filteredResultRecord, - domainName: getUrlHostName(filteredResultRecord.domainName as string), - }; + return filteredResultRecord; };