[BUG] Handle optimistic cache deletion operation (#9914)

# Introduction
This PR is highly related to previous optimistic cache refactor:
https://github.com/twentyhq/twenty/pull/9881
Here we've added some logic within the
`triggerUpdateRelationsOptimisticEffect` which will now run if given
recordInput `deletedAt` field is defined.

If deletion, we will iterate over all the fields searching for
`RELATION` for which deletion might implies necessity to detach the
relation

## Known troubleshooting ( also on main )

![image](https://github.com/user-attachments/assets/10ad59cd-e87b-4f26-89df-0a028fcfa518)
We might have to refactor the `prefillRecord` to spread and
overrides`inputValue` over defaultOne as inputValue could be a partial
one for more info please a look to

# Conclusion
Any suggestions are welcomed !
fixes https://github.com/twentyhq/twenty/issues/9580

---------

Co-authored-by: Charles Bochet <charles@twenty.com>
This commit is contained in:
Paul Rastoin
2025-01-30 11:12:37 +01:00
committed by GitHub
parent e7da9b6b87
commit 9635fe9222
15 changed files with 177 additions and 140 deletions

View File

@ -1,3 +1,4 @@
import { DeleteManyRecordsProps } from '@/object-record/hooks/useDeleteManyRecords';
import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
import { renderHook, waitFor } from '@testing-library/react';
import { act } from 'react';
@ -75,13 +76,12 @@ describe('useDeleteMultipleRecordsAction', () => {
result.current.ConfirmationModal?.props?.onConfirmClick();
});
const expectedParams: DeleteManyRecordsProps = {
recordIdsToDelete: [peopleMock[0].id, peopleMock[1].id],
};
await waitFor(() => {
expect(resetTableRowSelectionMock).toHaveBeenCalled();
expect(deleteManyRecordsMock).toHaveBeenCalledWith([
peopleMock[0].id,
peopleMock[1].id,
]);
expect(deleteManyRecordsMock).toHaveBeenCalledWith(expectedParams);
});
});
});

View File

@ -73,7 +73,9 @@ export const useDeleteMultipleRecordsAction: ActionHookWithObjectMetadataItem =
resetTableRowSelection();
await deleteManyRecords(recordIdsToDelete);
await deleteManyRecords({
recordIdsToDelete,
});
}, [deleteManyRecords, fetchAllRecordIds, resetTableRowSelection]);
const isRemoteObject = objectMetadataItem.isRemote;

View File

@ -4,11 +4,13 @@ import { ActionMenuContext } from '@/action-menu/contexts/ActionMenuContext';
import { useDestroyOneRecord } from '@/object-record/hooks/useDestroyOneRecord';
import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
import { useRecordTable } from '@/object-record/record-table/hooks/useRecordTable';
import { AppPath } from '@/types/AppPath';
import { ConfirmationModal } from '@/ui/layout/modal/components/ConfirmationModal';
import { useRightDrawer } from '@/ui/layout/right-drawer/hooks/useRightDrawer';
import { useCallback, useContext, useState } from 'react';
import { useRecoilValue } from 'recoil';
import { isDefined } from 'twenty-ui';
import { useNavigateApp } from '~/hooks/useNavigateApp';
export const useDestroySingleRecordAction: ActionHookWithObjectMetadataItem = ({
objectMetadataItem,
@ -18,6 +20,8 @@ export const useDestroySingleRecordAction: ActionHookWithObjectMetadataItem = ({
const [isDestroyRecordsModalOpen, setIsDestroyRecordsModalOpen] =
useState(false);
const navigateApp = useNavigateApp();
const { resetTableRowSelection } = useRecordTable({
recordTableId: objectMetadataItem.namePlural,
});
@ -34,7 +38,16 @@ export const useDestroySingleRecordAction: ActionHookWithObjectMetadataItem = ({
resetTableRowSelection();
await destroyOneRecord(recordId);
}, [resetTableRowSelection, destroyOneRecord, recordId]);
navigateApp(AppPath.RecordIndexPage, {
objectNamePlural: objectMetadataItem.namePlural,
});
}, [
resetTableRowSelection,
destroyOneRecord,
recordId,
navigateApp,
objectMetadataItem.namePlural,
]);
const isRemoteObject = objectMetadataItem.isRemote;

View File

@ -92,28 +92,26 @@ export const useOpenCreateActivityDrawer = ({
const targetableObjectRelationIdName = `${targetableObjects[0].targetObjectNameSingular}Id`;
await createOneActivityTarget({
taskId:
activityObjectNameSingular === CoreObjectNameSingular.Task
? activity.id
: undefined,
noteId:
activityObjectNameSingular === CoreObjectNameSingular.Note
? activity.id
: undefined,
...(activityObjectNameSingular === CoreObjectNameSingular.Task
? {
taskId: activity.id,
}
: {
noteId: activity.id,
}),
[targetableObjectRelationIdName]: targetableObjects[0].id,
});
setActivityTargetableEntityArray(targetableObjects);
} else {
await createOneActivityTarget({
taskId:
activityObjectNameSingular === CoreObjectNameSingular.Task
? activity.id
: undefined,
noteId:
activityObjectNameSingular === CoreObjectNameSingular.Note
? activity.id
: undefined,
...(activityObjectNameSingular === CoreObjectNameSingular.Task
? {
taskId: activity.id,
}
: {
noteId: activity.id,
}),
});
setActivityTargetableEntityArray([]);

View File

@ -15,8 +15,8 @@ import { getJoinObjectNameSingular } from '@/activities/utils/getJoinObjectNameS
import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem';
import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular';
import { useCreateManyRecordsInCache } from '@/object-record/cache/hooks/useCreateManyRecordsInCache';
import { useCreateManyRecords } from '@/object-record/hooks/useCreateManyRecords';
import { useDeleteManyRecords } from '@/object-record/hooks/useDeleteManyRecords';
import { useCreateOneRecord } from '@/object-record/hooks/useCreateOneRecord';
import { useDeleteOneRecord } from '@/object-record/hooks/useDeleteOneRecord';
import { activityTargetObjectRecordFamilyState } from '@/object-record/record-field/states/activityTargetObjectRecordFamilyState';
import { objectRecordMultiSelectCheckedRecordsIdsComponentState } from '@/object-record/record-field/states/objectRecordMultiSelectCheckedRecordsIdsComponentState';
import {
@ -54,17 +54,15 @@ export const ActivityTargetInlineCellEditMode = ({
}),
);
const { createManyRecords: createManyActivityTargets } = useCreateManyRecords<
const { createOneRecord: createOneActivityTarget } = useCreateOneRecord<
NoteTarget | TaskTarget
>({
objectNameSingular: getJoinObjectNameSingular(activityObjectNameSingular),
});
const { deleteManyRecords: deleteManyActivityTargets } = useDeleteManyRecords(
{
objectNameSingular: getJoinObjectNameSingular(activityObjectNameSingular),
},
);
const { deleteOneRecord: deleteOneActivityTarget } = useDeleteOneRecord({
objectNameSingular: getJoinObjectNameSingular(activityObjectNameSingular),
});
const { closeInlineCell: closeEditableField } = useInlineCell();
@ -168,36 +166,21 @@ export const ActivityTargetInlineCellEditMode = ({
);
const newActivityTargetId = v4();
const fieldName = record.objectMetadataItem.nameSingular;
const fieldNameWithIdSuffix = getActivityTargetObjectFieldIdName({
nameSingular: record.objectMetadataItem.nameSingular,
});
const newActivityTargetInput = {
id: newActivityTargetId,
...(activityObjectNameSingular === CoreObjectNameSingular.Task
? { taskId: activity.id }
: { noteId: activity.id }),
[fieldNameWithIdSuffix]: recordId,
};
const newActivityTarget = prefillRecord<NoteTarget | TaskTarget>({
objectMetadataItem: objectMetadataItemActivityTarget,
input: {
id: newActivityTargetId,
taskId:
activityObjectNameSingular === CoreObjectNameSingular.Task
? activity.id
: null,
task:
activityObjectNameSingular === CoreObjectNameSingular.Task
? activity
: null,
noteId:
activityObjectNameSingular === CoreObjectNameSingular.Note
? activity.id
: null,
note:
activityObjectNameSingular === CoreObjectNameSingular.Note
? activity
: null,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
[fieldName]: record.record,
[fieldNameWithIdSuffix]: recordId,
},
input: newActivityTargetInput,
});
activityTargetsAfterUpdate.push(newActivityTarget);
@ -215,7 +198,7 @@ export const ActivityTargetInlineCellEditMode = ({
},
});
} else {
await createManyActivityTargets([newActivityTarget]);
await createOneActivityTarget(newActivityTargetInput);
}
set(activityTargetObjectRecordFamilyState(recordId), {
@ -252,7 +235,7 @@ export const ActivityTargetInlineCellEditMode = ({
},
});
} else {
await deleteManyActivityTargets([activityTargetToDeleteId]);
await deleteOneActivityTarget(activityTargetToDeleteId);
}
set(activityTargetObjectRecordFamilyState(recordId), {
@ -263,9 +246,9 @@ export const ActivityTargetInlineCellEditMode = ({
[
activity,
activityTargetWithTargetRecords,
createManyActivityTargets,
createOneActivityTarget,
createManyActivityTargetsInCache,
deleteManyActivityTargets,
deleteOneActivityTarget,
isActivityInCreateMode,
objectMetadataItemActivityTarget,
recordPickerInstanceId,

View File

@ -27,6 +27,10 @@ export const triggerUpdateRelationsOptimisticEffect = ({
updatedSourceRecord,
objectMetadataItems,
}: triggerUpdateRelationsOptimisticEffectArgs) => {
const isDeletion =
isDefined(updatedSourceRecord) &&
isDefined(updatedSourceRecord['deletedAt']);
return sourceObjectMetadataItem.fields.forEach(
(fieldMetadataItemOnSourceRecord) => {
const notARelationField =
@ -72,15 +76,15 @@ export const triggerUpdateRelationsOptimisticEffect = ({
| RecordGqlNode
| null = updatedSourceRecord?.[fieldMetadataItemOnSourceRecord.name];
if (
isDeeplyEqual(
currentFieldValueOnSourceRecord,
updatedFieldValueOnSourceRecord,
{ strict: true },
)
) {
const noDiff = isDeeplyEqual(
currentFieldValueOnSourceRecord,
updatedFieldValueOnSourceRecord,
{ strict: true },
);
if (noDiff && !isDeletion) {
return;
}
const extractTargetRecordsFromRelation = (
value: RecordGqlConnection | RecordGqlNode | null,
): RecordGqlNode[] => {
@ -96,11 +100,12 @@ export const triggerUpdateRelationsOptimisticEffect = ({
return [value];
};
const recordToExtractDetachFrom = isDeletion
? updatedFieldValueOnSourceRecord
: currentFieldValueOnSourceRecord;
const targetRecordsToDetachFrom = extractTargetRecordsFromRelation(
currentFieldValueOnSourceRecord,
);
const targetRecordsToAttachTo = extractTargetRecordsFromRelation(
updatedFieldValueOnSourceRecord,
recordToExtractDetachFrom,
);
// TODO: see if we can de-hardcode this, put cascade delete in relation metadata item
@ -129,7 +134,11 @@ export const triggerUpdateRelationsOptimisticEffect = ({
});
}
if (isDefined(updatedSourceRecord)) {
if (!isDeletion && isDefined(updatedSourceRecord)) {
const targetRecordsToAttachTo = extractTargetRecordsFromRelation(
updatedFieldValueOnSourceRecord,
);
targetRecordsToAttachTo.forEach((targetRecordToAttachTo) =>
triggerAttachRelationOptimisticEffect({
cache,

View File

@ -22,7 +22,6 @@ export const useCreateFavoriteFolder = () => {
);
await createFavoriteFolder({
workspaceMemberId: currentWorkspaceMemberId,
name,
position: maxPosition + 1,
});

View File

@ -52,7 +52,9 @@ describe('useDeleteManyRecords', () => {
);
await act(async () => {
const res = await result.current.deleteManyRecords(personIds);
const res = await result.current.deleteManyRecords({
recordIdsToDelete: personIds,
});
expect(res).toBeDefined();
expect(res[0]).toHaveProperty('id');
});

View File

@ -135,7 +135,7 @@ export const useCreateManyRecords = <
update: (cache, { data }) => {
const records = data?.[mutationResponseField];
if (!records?.length || skipPostOptmisticEffect) return;
if (!isDefined(records?.length) || skipPostOptmisticEffect) return;
triggerCreateRecordsOptimisticEffect({
cache,

View File

@ -24,7 +24,8 @@ type useDeleteManyRecordProps = {
refetchFindManyQuery?: boolean;
};
type DeleteManyRecordsOptions = {
export type DeleteManyRecordsProps = {
recordIdsToDelete: string[];
skipOptimisticEffect?: boolean;
delayInMsBetweenRequests?: number;
};
@ -61,16 +62,18 @@ export const useDeleteManyRecords = ({
objectMetadataItem.namePlural,
);
const deleteManyRecords = async (
idsToDelete: string[],
options?: DeleteManyRecordsOptions,
) => {
const numberOfBatches = Math.ceil(idsToDelete.length / mutationPageSize);
const deleteManyRecords = async ({
recordIdsToDelete,
delayInMsBetweenRequests,
skipOptimisticEffect = false,
}: DeleteManyRecordsProps) => {
const numberOfBatches = Math.ceil(
recordIdsToDelete.length / mutationPageSize,
);
const deletedRecords = [];
for (let batchIndex = 0; batchIndex < numberOfBatches; batchIndex++) {
const batchedIdsToDelete = idsToDelete.slice(
const batchedIdsToDelete = recordIdsToDelete.slice(
batchIndex * mutationPageSize,
(batchIndex + 1) * mutationPageSize,
);
@ -81,22 +84,21 @@ export const useDeleteManyRecords = ({
.map((idToDelete) => getRecordFromCache(idToDelete, apolloClient.cache))
.filter(isDefined);
const cachedRecordsWithConnection: RecordGqlNode[] = [];
const optimisticRecordsWithConnection: RecordGqlNode[] = [];
if (!skipOptimisticEffect) {
const cachedRecordsNode: RecordGqlNode[] = [];
const computedOptimisticRecordsNode: RecordGqlNode[] = [];
if (!options?.skipOptimisticEffect) {
cachedRecords.forEach((cachedRecord) => {
if (!cachedRecord || !cachedRecord.id) {
if (!isDefined(cachedRecord) || !isDefined(cachedRecord.id)) {
return;
}
const cachedRecordWithConnection =
getRecordNodeFromRecord<ObjectRecord>({
record: cachedRecord,
objectMetadataItem,
objectMetadataItems,
computeReferences: true,
});
const cachedRecordNode = getRecordNodeFromRecord<ObjectRecord>({
record: cachedRecord,
objectMetadataItem,
objectMetadataItems,
computeReferences: false,
});
const computedOptimisticRecord = {
...cachedRecord,
@ -104,34 +106,36 @@ export const useDeleteManyRecords = ({
...{ __typename: capitalize(objectMetadataItem.nameSingular) },
};
const optimisticRecordWithConnection =
getRecordNodeFromRecord<ObjectRecord>({
record: computedOptimisticRecord,
objectMetadataItem,
objectMetadataItems,
computeReferences: true,
});
const optimisticRecordNode = getRecordNodeFromRecord<ObjectRecord>({
record: computedOptimisticRecord,
objectMetadataItem,
objectMetadataItems,
computeReferences: false,
});
if (!optimisticRecordWithConnection || !cachedRecordWithConnection) {
return null;
if (
!isDefined(optimisticRecordNode) ||
!isDefined(cachedRecordNode)
) {
return;
}
cachedRecordsWithConnection.push(cachedRecordWithConnection);
optimisticRecordsWithConnection.push(optimisticRecordWithConnection);
updateRecordFromCache({
objectMetadataItems,
objectMetadataItem,
cache: apolloClient.cache,
record: computedOptimisticRecord,
});
computedOptimisticRecordsNode.push(optimisticRecordNode);
cachedRecordsNode.push(cachedRecordNode);
});
triggerUpdateRecordOptimisticEffectByBatch({
cache: apolloClient.cache,
objectMetadataItem,
currentRecords: cachedRecordsWithConnection,
updatedRecords: optimisticRecordsWithConnection,
currentRecords: cachedRecordsNode,
updatedRecords: computedOptimisticRecordsNode,
objectMetadataItems,
});
}
@ -144,8 +148,8 @@ export const useDeleteManyRecords = ({
},
})
.catch((error: Error) => {
const cachedRecordsWithConnection: RecordGqlNode[] = [];
const optimisticRecordsWithConnection: RecordGqlNode[] = [];
const cachedRecordsNode: RecordGqlNode[] = [];
const computedOptimisticRecordsNode: RecordGqlNode[] = [];
cachedRecords.forEach((cachedRecord) => {
if (isUndefinedOrNull(cachedRecord?.id)) {
@ -164,7 +168,7 @@ export const useDeleteManyRecords = ({
record: cachedRecord,
objectMetadataItem,
objectMetadataItems,
computeReferences: true,
computeReferences: false,
});
const computedOptimisticRecord = {
@ -178,27 +182,25 @@ export const useDeleteManyRecords = ({
record: computedOptimisticRecord,
objectMetadataItem,
objectMetadataItems,
computeReferences: true,
computeReferences: false,
});
if (
!optimisticRecordWithConnection ||
!cachedRecordWithConnection
!isDefined(optimisticRecordWithConnection) ||
!isDefined(cachedRecordWithConnection)
) {
return;
}
cachedRecordsWithConnection.push(cachedRecordWithConnection);
optimisticRecordsWithConnection.push(
optimisticRecordWithConnection,
);
cachedRecordsNode.push(cachedRecordWithConnection);
computedOptimisticRecordsNode.push(optimisticRecordWithConnection);
});
triggerUpdateRecordOptimisticEffectByBatch({
cache: apolloClient.cache,
objectMetadataItem,
currentRecords: optimisticRecordsWithConnection,
updatedRecords: cachedRecordsWithConnection,
currentRecords: computedOptimisticRecordsNode,
updatedRecords: cachedRecordsNode,
objectMetadataItems,
});
@ -210,8 +212,8 @@ export const useDeleteManyRecords = ({
deletedRecords.push(...deletedRecordsForThisBatch);
if (isDefined(options?.delayInMsBetweenRequests)) {
await sleep(options.delayInMsBetweenRequests);
if (isDefined(delayInMsBetweenRequests)) {
await sleep(delayInMsBetweenRequests);
}
}
await refetchAggregateQueries();

View File

@ -12,6 +12,7 @@ import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggr
import { ObjectRecord } from '@/object-record/types/ObjectRecord';
import { getDeleteOneRecordMutationResponseField } from '@/object-record/utils/getDeleteOneRecordMutationResponseField';
import { capitalize } from 'twenty-shared';
import { isDefined } from 'twenty-ui';
type useDeleteOneRecordProps = {
objectNameSingular: string;
@ -49,11 +50,11 @@ export const useDeleteOneRecord = ({
const cachedRecord = getRecordFromCache(idToDelete, apolloClient.cache);
const cachedRecordWithConnection = getRecordNodeFromRecord<ObjectRecord>({
const cachedRecordNode = getRecordNodeFromRecord<ObjectRecord>({
record: cachedRecord,
objectMetadataItem,
objectMetadataItems,
computeReferences: true,
computeReferences: false,
});
const computedOptimisticRecord = {
@ -62,15 +63,14 @@ export const useDeleteOneRecord = ({
...{ __typename: capitalize(objectMetadataItem.nameSingular) },
};
const optimisticRecordWithConnection =
getRecordNodeFromRecord<ObjectRecord>({
record: computedOptimisticRecord,
objectMetadataItem,
objectMetadataItems,
computeReferences: true,
});
const optimisticRecordNode = getRecordNodeFromRecord<ObjectRecord>({
record: computedOptimisticRecord,
objectMetadataItem,
objectMetadataItems,
computeReferences: false,
});
if (!optimisticRecordWithConnection || !cachedRecordWithConnection) {
if (!isDefined(optimisticRecordNode) || !isDefined(cachedRecordNode)) {
return null;
}
@ -84,8 +84,8 @@ export const useDeleteOneRecord = ({
triggerUpdateRecordOptimisticEffect({
cache: apolloClient.cache,
objectMetadataItem,
currentRecord: cachedRecordWithConnection,
updatedRecord: optimisticRecordWithConnection,
currentRecord: cachedRecordNode,
updatedRecord: optimisticRecordNode,
objectMetadataItems,
});
@ -98,12 +98,13 @@ export const useDeleteOneRecord = ({
update: (cache, { data }) => {
const record = data?.[mutationResponseField];
if (!record || !cachedRecord) return;
if (!isDefined(record) || !isDefined(computedOptimisticRecord))
return;
triggerUpdateRecordOptimisticEffect({
cache,
objectMetadataItem,
currentRecord: cachedRecord,
currentRecord: computedOptimisticRecord,
updatedRecord: record,
objectMetadataItems,
});
@ -123,8 +124,8 @@ export const useDeleteOneRecord = ({
triggerUpdateRecordOptimisticEffect({
cache: apolloClient.cache,
objectMetadataItem,
currentRecord: optimisticRecordWithConnection,
updatedRecord: cachedRecordWithConnection,
currentRecord: optimisticRecordNode,
updatedRecord: cachedRecordNode,
objectMetadataItems,
});

View File

@ -14,6 +14,7 @@ import { computeOptimisticRecordFromInput } from '@/object-record/utils/computeO
import { getUpdateOneRecordMutationResponseField } from '@/object-record/utils/getUpdateOneRecordMutationResponseField';
import { sanitizeRecordInput } from '@/object-record/utils/sanitizeRecordInput';
import { capitalize } from 'twenty-shared';
import { isDefined } from 'twenty-ui';
import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull';
type useUpdateOneRecordProps = {
@ -130,7 +131,8 @@ export const useUpdateOneRecord = <
update: (cache, { data }) => {
const record = data?.[mutationResponseField];
if (!record || !computedOptimisticRecord) return;
if (!isDefined(record) || !isDefined(computedOptimisticRecord))
return;
triggerUpdateRecordOptimisticEffect({
cache,

View File

@ -179,4 +179,28 @@ describe('computeOptimisticRecordFromInput', () => {
`"Should never provide relation mutation through anything else than the fieldId e.g companyId"`,
);
});
it('should throw an error if recordInput contains both the relationFieldId and relationField even if null', () => {
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: null,
},
cache,
}),
).toThrowErrorMatchingInlineSnapshot(
`"Should never provide relation mutation through anything else than the fieldId e.g companyId"`,
);
});
});

View File

@ -91,7 +91,7 @@ export const computeOptimisticRecordFromInput = ({
continue;
}
if (isDefined(recordInputFieldValue)) {
if (!isUndefined(recordInputFieldValue)) {
throw new Error(
'Should never provide relation mutation through anything else than the fieldId e.g companyId',
);

View File

@ -34,8 +34,10 @@ export const sanitizeRecordInput = ({
(field) => field.name === relationIdFieldName,
);
const relationIdFieldValue = recordInput[relationIdFieldName];
return relationIdFieldMetadataItem
? [relationIdFieldName, fieldValue?.id ?? null]
? [relationIdFieldName, relationIdFieldValue ?? null]
: undefined;
}