From a0281f635bda14f12801c972d5f40d23fbc2ad28 Mon Sep 17 00:00:00 2001 From: Thomas Trompette Date: Wed, 29 Jul 2026 14:17:18 +0200 Subject: [PATCH] Restore soft-deleted junction record when re-adding a junction relation (#23371) Fixes #23305. Removing a junction relation soft-deletes the intermediate junction record. Re-adding the same relation created a brand new record, which the composite unique index on the junction object (e.g. `personId` + `companyId`) rejected with "This record already exists". `useUpdateJunctionRelationFromCell` now creates the junction record through `useCreateManyRecords` with `upsert: true`. The server matches on the unique index with `withDeleted()` and clears `deletedAt` on the matched row instead of inserting. ## This revives, it does not create Worth being explicit, because it is a deliberate trade and not obvious from the diff. When a soft-deleted row exists for the pair, the user gets that row back. Same id, same `createdAt`, same `createdBy`, and anything attached to it (notes, files, and any fields the app added to the junction object). Verified on a dev instance: after re-adding a link through the picker, the row still reported a creation date from days earlier. That is fine for a pure link. It is a lie for a junction that carries data, for instance a `PersonCompanyRelationship` with a role and dates. The alternative designs are hard-deleting on detach (needs `canDestroyObjectRecords` on the junction object, which most roles do not grant) and partial unique indexes (needs `indexWhereClause` exposed to app-declared indexes, which the SDK does not support today). Both are larger changes. This one unblocks affected apps without requiring anything from them. Note that upsert conflict detection ignores `indexWhereClause`, so shipping partial indexes later would not change this behaviour on its own. ## Why the id handling changed The hook no longer sends a client-generated id. Under upsert the server decides which row you get, so the id in the input would be discarded. The optimistic store entry still uses a local id so the chip appears immediately, then adopts the persisted id once the mutation resolves. Without that, a revive left an id in the store that exists nowhere on the server, and the next removal failed with "This record does not exist or has been deleted". Supersedes #23365, which took the same approach but added `$upsert` to the shared `createOne` mutation. That capability already exists per call on `createMany`, and widening the shared document broke the `useCreateOneRecordMutation` and `useCreateOneRecord` tests. ## Testing Verified end to end on a dev instance with a junction object carrying a non-partial composite unique index, since the dev seed does not create one and the bug cannot reproduce without it: - before: re-adding after a removal fails with "This record already exists" - after: the original row is restored, `deletedAt` cleared, one row throughout, and removing again in the same session works, with no GraphQL errors Added an integration test for the path this depends on: a soft-deleted record matched on its unique fields alone, with no id in the input. The existing coverage only exercised upsert by explicit id. ## Known gaps, deliberately not addressed here **Toggling a link off while its creation is still in flight.** The store id is provisional until the mutation returns, so a removal issued inside that window deletes an id the server never received. Reproduced by stalling the create and clicking remove during it: `CombinedGraphQLErrors: Record not found`, and the link stays active despite the user removing it. The window is one mutation round trip. Left for a follow-up; the likely fix is a per-record operation queue so adds and removes on a field run in click order. **Junction objects with no unique index.** Remove then re-add still creates a duplicate row there, on this branch as on main, because conflict detection is driven by unique indexes. --------- Co-authored-by: Shinu Cherian <129690295+Shinu-Cherian@users.noreply.github.com> --- .../useUpdateJunctionRelationFromCell.ts | 103 +++++++++++++++--- .../suites/upsert/upsert.integration-spec.ts | 44 ++++++++ 2 files changed, 132 insertions(+), 15 deletions(-) diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/hooks/useUpdateJunctionRelationFromCell.ts b/packages/twenty-front/src/modules/object-record/record-field/ui/hooks/useUpdateJunctionRelationFromCell.ts index 21e5cb3ce2..ba194b6687 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/ui/hooks/useUpdateJunctionRelationFromCell.ts +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/hooks/useUpdateJunctionRelationFromCell.ts @@ -6,7 +6,8 @@ import { v4 } from 'uuid'; import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems'; import { type FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem'; import { getObjectTypename } from '@/object-record/cache/utils/getObjectTypename'; -import { useCreateOneRecord } from '@/object-record/hooks/useCreateOneRecord'; +import { getRecordFromRecordNode } from '@/object-record/cache/utils/getRecordFromRecordNode'; +import { useCreateManyRecords } from '@/object-record/hooks/useCreateManyRecords'; import { useDeleteOneRecord } from '@/object-record/hooks/useDeleteOneRecord'; import { type FieldDefinition } from '@/object-record/record-field/ui/types/FieldDefinition'; import { @@ -56,11 +57,8 @@ export const useUpdateJunctionRelationFromCell = ({ junctionObjectMetadata?.nameSingular ?? fieldDefinition.metadata.relationObjectMetadataNameSingular; - // Skip the post-optimistic effect since we handle optimistic updates manually - // Otherwise Apollo would also add the record, resulting in duplicates - const { createOneRecord: createJunctionRecord } = useCreateOneRecord({ + const { createManyRecords: createJunctionRecords } = useCreateManyRecords({ objectNameSingular: junctionObjectNameSingular, - skipPostOptimisticEffect: true, }); const { deleteOneRecord: deleteJunctionRecord } = useDeleteOneRecord({ @@ -175,11 +173,11 @@ export const useUpdateJunctionRelationFromCell = ({ } const targetRecord = searchRecord.record; - const newJunctionId = v4(); + const optimisticJunctionId = v4(); const now = new Date().toISOString(); const junctionRecordForStore = { - id: newJunctionId, + id: optimisticJunctionId, createdAt: now, updatedAt: now, __typename: getObjectTypename(junctionObjectName), @@ -188,12 +186,6 @@ export const useUpdateJunctionRelationFromCell = ({ [targetFieldName]: targetRecord, }; - const newJunctionRecordForApi = { - id: newJunctionId, - [sourceJoinColumnName]: recordId, - [targetJoinColumnName]: morphItem.recordId, - }; - store.set( recordStoreFamilyState.atomFamily(recordId), (currentRecord: Record | null | undefined) => { @@ -213,12 +205,93 @@ export const useUpdateJunctionRelationFromCell = ({ }, ); - await createJunctionRecord(newJunctionRecordForApi); + const removeOptimisticJunctionRecord = () => + store.set( + recordStoreFamilyState.atomFamily(recordId), + (currentRecord: Record | null | undefined) => { + if (!isDefined(currentRecord)) { + return currentRecord; + } + + const currentFieldValue = currentRecord[fieldName]; + + return { + ...currentRecord, + [fieldName]: Array.isArray(currentFieldValue) + ? currentFieldValue.filter( + (junctionRecord) => + junctionRecord.id !== optimisticJunctionId, + ) + : currentFieldValue, + } as ObjectRecord; + }, + ); + + const persistedJunctionRecordNode = await createJunctionRecords({ + recordsToCreate: [ + { + [sourceJoinColumnName]: recordId, + [targetJoinColumnName]: morphItem.recordId, + }, + ], + upsert: true, + }) + .then(([createdJunctionRecordNode]) => createdJunctionRecordNode) + .catch((error: Error) => { + removeOptimisticJunctionRecord(); + + throw error; + }); + + if (!isDefined(persistedJunctionRecordNode)) { + removeOptimisticJunctionRecord(); + + return; + } + + const persistedJunctionRecord = getRecordFromRecordNode({ + recordNode: persistedJunctionRecordNode, + }); + + store.set( + recordStoreFamilyState.atomFamily(recordId), + (currentRecord: Record | null | undefined) => { + if (!isDefined(currentRecord)) { + return currentRecord; + } + + const currentFieldValue = currentRecord[fieldName]; + + if (!Array.isArray(currentFieldValue)) { + return currentRecord as ObjectRecord; + } + + const junctionRecordsWithoutOptimistic = currentFieldValue.filter( + (junctionRecord) => junctionRecord.id !== optimisticJunctionId, + ); + + const isPersistedJunctionRecordAlreadyInStore = + junctionRecordsWithoutOptimistic.some( + (junctionRecord) => + junctionRecord.id === persistedJunctionRecord.id, + ); + + return { + ...currentRecord, + [fieldName]: isPersistedJunctionRecordAlreadyInStore + ? junctionRecordsWithoutOptimistic + : [ + ...junctionRecordsWithoutOptimistic, + { ...junctionRecordForStore, ...persistedJunctionRecord }, + ], + } as ObjectRecord; + }, + ); } }, [ store, - createJunctionRecord, + createJunctionRecords, deleteJunctionRecord, fieldDefinition.metadata.fieldName, junctionConfig, diff --git a/packages/twenty-server/test/integration/graphql/suites/upsert/upsert.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/upsert/upsert.integration-spec.ts index 64c10acc2e..88d91d293d 100644 --- a/packages/twenty-server/test/integration/graphql/suites/upsert/upsert.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/upsert/upsert.integration-spec.ts @@ -228,6 +228,50 @@ describe('upsert (createMany with upsert:true)', () => { ); }); + it('should restore a soft-deleted record matched on its unique fields only', async () => { + const createResponse = await makeGraphqlAPIRequest({ + query: createRecordsQuery, + variables: { + data: [ + { + firstUniqueTestField: 'softDeletedByUniqueFields', + secondUniqueTestField: 'softDeletedByUniqueFieldsSecond', + name: 'originalRecord', + }, + ], + upsert: false, + }, + }); + + const createdRecord = createResponse.body.data.createTestRecordObjects[0]; + + await makeGraphqlAPIRequest({ + query: deleteRecordsQuery, + variables: { + filter: { id: { eq: createdRecord.id } }, + }, + }); + + const upsertResponse = await makeGraphqlAPIRequest({ + query: createRecordsQuery, + variables: { + data: [ + { + firstUniqueTestField: 'softDeletedByUniqueFields', + secondUniqueTestField: 'softDeletedByUniqueFieldsSecond', + }, + ], + upsert: true, + }, + }); + + const upsertedRecord = upsertResponse.body.data.createTestRecordObjects[0]; + + expect(upsertedRecord.id).toEqual(createdRecord.id); + expect(upsertedRecord.deletedAt).toBeNull(); + expect(upsertedRecord.name).toEqual('originalRecord'); + }); + it('should update and restore updated soft-deleted record', async () => { const createResponse = await makeGraphqlAPIRequest({ query: createRecordsQuery,