From 47f262c97057ae3849e2b6c7ebeba5fededa97e8 Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Tue, 11 Feb 2025 16:21:44 +0100 Subject: [PATCH] [REFACTOR][BUG] Dynamically compute field to write in cache `UPDATE` & `DELETE` (#10079) # Introduction At the moment when updating any record cache occurence, we will build a fragment that will expect all of the object metadata item fields to be provided. Which result in the following traces: ( in the video companies aren't fetch with companyId and other missing fields ) https://github.com/user-attachments/assets/56eab7c1-8f01-45ff-8f5d-78737b788b92 By definition as we're using graphql we might not request every record's fields each time we wanna consume them. In this way we will now dynamically compute or expect depending on the CRUD operation specific fields to be written in the cache, and not all of them Tested all optimistic and failure management use cases ## Covering cache Added coverage only for the `deleteOne` and `deleteMany` hooks, it cover only the record record cache and not its relations hydratation ( for the moment ) ## Why not closing #9927 Unless I'm mistaken everything done here have fixed the same logs/traces issue for updates and deletion but not creation. Which means we still need to investigate the mass upload from import and prefillRecord behavior In a nutshell: went over each `updateRecordFromCache` calls, still need to do all `createRecordInCache` calls related to #9927 ## Conlusion Sorry for the big PR should have ejected into a specific one for the `MinimalRecord` refactor Will also continue covering others hooks later in my week as for the `deleteOne` As always any suggestions are welcomed ! --- .../useDestroyMultipleRecordsAction.test.tsx | 2 +- .../InformationBannerDeletedRecord.tsx | 2 +- .../objectMetadataItemSchema.test.ts | 2 +- .../cache/utils/getRecordFromCache.ts | 3 +- .../cache/utils/updateRecordFromCache.ts | 4 +- .../utils/generateDepthOneRecordGqlFields.ts | 17 +- .../hooks/__mocks__/useDeleteManyRecords.ts | 21 +- .../__tests__/useDeleteManyRecords.test.tsx | 203 ++++++++++-- .../__tests__/useDeleteOneRecord.test.tsx | 291 ++++++++++++++++-- .../hooks/useDeleteManyRecords.ts | 79 +++-- .../object-record/hooks/useDeleteOneRecord.ts | 73 +++-- .../hooks/useDestroyManyRecords.ts | 44 +-- .../hooks/useDestroyOneRecord.ts | 20 +- .../hooks/useRestoreManyRecords.ts | 113 +++---- .../object-record/hooks/useUpdateOneRecord.ts | 103 ++++--- .../computeOptimisticRecordFromInput.test.ts | 80 +++-- .../utils/generateEmptyFieldValue.ts | 9 +- .../internal/usePersistViewFieldRecords.ts | 11 +- .../usePersistViewFilterGroupRecords.ts | 15 +- .../internal/usePersistViewFilterRecords.ts | 25 +- .../internal/usePersistViewSortRecords.ts | 23 +- .../hooks/useDeleteOneWorkflowVersion.ts | 7 +- .../hooks/useDeleteWorkflowVersionStep.ts | 6 +- .../hooks/useCreateWorkflowVersionStep.ts | 6 +- .../hooks/useUpdateWorkflowVersionStep.ts | 6 +- .../getJestMetadataAndApolloMocksWrapper.tsx | 5 +- .../src/testing/mock-data/companies.ts | 13 + .../src/testing/mock-data/people.ts | 27 +- .../buildRecordFromKeysWithSameValue.test.ts | 23 ++ .../array/buildRecordFromKeysWithSameValue.ts | 10 + 30 files changed, 876 insertions(+), 367 deletions(-) create mode 100644 packages/twenty-front/src/utils/array/__tests__/buildRecordFromKeysWithSameValue.test.ts create mode 100644 packages/twenty-front/src/utils/array/buildRecordFromKeysWithSameValue.ts diff --git a/packages/twenty-front/src/modules/action-menu/actions/record-actions/multiple-records/hooks/__tests__/useDestroyMultipleRecordsAction.test.tsx b/packages/twenty-front/src/modules/action-menu/actions/record-actions/multiple-records/hooks/__tests__/useDestroyMultipleRecordsAction.test.tsx index 21117eac1..c197680c7 100644 --- a/packages/twenty-front/src/modules/action-menu/actions/record-actions/multiple-records/hooks/__tests__/useDestroyMultipleRecordsAction.test.tsx +++ b/packages/twenty-front/src/modules/action-menu/actions/record-actions/multiple-records/hooks/__tests__/useDestroyMultipleRecordsAction.test.tsx @@ -18,7 +18,7 @@ const personMockObjectMetadataItem = generatedMockObjectMetadataItems.find( const personMockObjectMetadataItemDeletedAtField = personMockObjectMetadataItem.fields.find((el) => el.name === 'deletedAt'); if (personMockObjectMetadataItemDeletedAtField === undefined) - throw new Error('Should never occurs'); + throw new Error('Should never occur'); const [firstPeopleMock, secondPeopleMock] = getPeopleMock().map((record) => ({ ...record, diff --git a/packages/twenty-front/src/modules/information-banner/components/deleted-record/InformationBannerDeletedRecord.tsx b/packages/twenty-front/src/modules/information-banner/components/deleted-record/InformationBannerDeletedRecord.tsx index 1b3eb1270..6c7e16ef7 100644 --- a/packages/twenty-front/src/modules/information-banner/components/deleted-record/InformationBannerDeletedRecord.tsx +++ b/packages/twenty-front/src/modules/information-banner/components/deleted-record/InformationBannerDeletedRecord.tsx @@ -30,7 +30,7 @@ export const InformationBannerDeletedRecord = ({ message={`This record has been deleted`} buttonTitle="Restore" buttonIcon={IconRefresh} - buttonOnClick={() => restoreManyRecords([recordId])} + buttonOnClick={() => restoreManyRecords({ idsToRestore: [recordId] })} /> ); diff --git a/packages/twenty-front/src/modules/object-metadata/validation-schemas/__tests__/objectMetadataItemSchema.test.ts b/packages/twenty-front/src/modules/object-metadata/validation-schemas/__tests__/objectMetadataItemSchema.test.ts index 8697d279a..c60ef60df 100644 --- a/packages/twenty-front/src/modules/object-metadata/validation-schemas/__tests__/objectMetadataItemSchema.test.ts +++ b/packages/twenty-front/src/modules/object-metadata/validation-schemas/__tests__/objectMetadataItemSchema.test.ts @@ -23,7 +23,7 @@ describe('objectMetadataItemSchema', () => { ); expect(validObjectMetadataItem).not.toBeUndefined(); if (validObjectMetadataItem === undefined) - throw new Error('Should never occurs'); + throw new Error('Should never occur'); // When const result = objectMetadataItemSchema.safeParse({ 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 9a2fcb63d..b86ec5f81 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 @@ -7,6 +7,7 @@ import { RecordGqlFields } from '@/object-record/graphql/types/RecordGqlFields'; import { generateDepthOneRecordGqlFields } from '@/object-record/graphql/utils/generateDepthOneRecordGqlFields'; import { ObjectRecord } from '@/object-record/types/ObjectRecord'; import { capitalize } from 'twenty-shared'; +import { isEmptyObject } from '~/utils/isEmptyObject'; import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull'; export type GetRecordFromCacheArgs = { @@ -53,7 +54,7 @@ export const getRecordFromCache = ({ returnPartialData: true, }); - if (isUndefinedOrNull(record)) { + if (isUndefinedOrNull(record) || isEmptyObject(record)) { return null; } diff --git a/packages/twenty-front/src/modules/object-record/cache/utils/updateRecordFromCache.ts b/packages/twenty-front/src/modules/object-record/cache/utils/updateRecordFromCache.ts index 4a9274227..3af8d8278 100644 --- a/packages/twenty-front/src/modules/object-record/cache/utils/updateRecordFromCache.ts +++ b/packages/twenty-front/src/modules/object-record/cache/utils/updateRecordFromCache.ts @@ -13,13 +13,13 @@ export const updateRecordFromCache = ({ objectMetadataItems, objectMetadataItem, cache, - recordGqlFields = undefined, + recordGqlFields, record, }: { objectMetadataItems: ObjectMetadataItem[]; objectMetadataItem: ObjectMetadataItem; cache: ApolloCache; - recordGqlFields?: Record; + recordGqlFields: Record; record: T; }) => { if (isUndefinedOrNull(objectMetadataItem)) { diff --git a/packages/twenty-front/src/modules/object-record/graphql/utils/generateDepthOneRecordGqlFields.ts b/packages/twenty-front/src/modules/object-record/graphql/utils/generateDepthOneRecordGqlFields.ts index fcc768267..ae321bb40 100644 --- a/packages/twenty-front/src/modules/object-record/graphql/utils/generateDepthOneRecordGqlFields.ts +++ b/packages/twenty-front/src/modules/object-record/graphql/utils/generateDepthOneRecordGqlFields.ts @@ -8,15 +8,14 @@ export const generateDepthOneRecordGqlFields = ({ objectMetadataItem: ObjectMetadataItem; record?: Record; }) => { - const gqlFieldsFromObjectMetadataItem = objectMetadataItem.fields.reduce( - (acc, field) => { - return { - ...acc, - [field.name]: true, - }; - }, - {}, - ); + const gqlFieldsFromObjectMetadataItem = objectMetadataItem.fields.reduce< + Record + >((acc, field) => { + return { + ...acc, + [field.name]: true, + }; + }, {}); if (isDefined(record)) { return Object.keys(gqlFieldsFromObjectMetadataItem).reduce((acc, key) => { diff --git a/packages/twenty-front/src/modules/object-record/hooks/__mocks__/useDeleteManyRecords.ts b/packages/twenty-front/src/modules/object-record/hooks/__mocks__/useDeleteManyRecords.ts index dbe68fda0..7228945ac 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/__mocks__/useDeleteManyRecords.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/__mocks__/useDeleteManyRecords.ts @@ -1,24 +1,31 @@ +import { ObjectRecord } from '@/object-record/types/ObjectRecord'; import { gql } from '@apollo/client'; +import { getPersonRecord } from '~/testing/mock-data/people'; export const query = gql` mutation DeleteManyPeople($filter: PersonFilterInput!) { deletePeople(filter: $filter) { id + __typename } } `; +export const personIds = [ + 'a7286b9a-c039-4a89-9567-2dfa7953cda9', + '37faabcd-cb39-4a0a-8618-7e3fda9afca0', +]; + +export const personRecords = personIds.map((personId, index) => + getPersonRecord({ id: personId, deletedAt: null }, index), +); + export const variables = { filter: { id: { - in: [ - 'a7286b9a-c039-4a89-9567-2dfa7953cda9', - '37faabcd-cb39-4a0a-8618-7e3fda9afca0', - ], + in: personIds, }, }, }; -export const responseData = { - id: '', -}; +export const responseData = personIds.map((personId) => ({ id: personId })); diff --git a/packages/twenty-front/src/modules/object-record/hooks/__tests__/useDeleteManyRecords.test.tsx b/packages/twenty-front/src/modules/object-record/hooks/__tests__/useDeleteManyRecords.test.tsx index 6d0bf4250..b46bf4ba2 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/__tests__/useDeleteManyRecords.test.tsx +++ b/packages/twenty-front/src/modules/object-record/hooks/__tests__/useDeleteManyRecords.test.tsx @@ -1,21 +1,26 @@ -import { renderHook } from '@testing-library/react'; +import { renderHook, waitFor } from '@testing-library/react'; +import { getRecordFromCache } from '@/object-record/cache/utils/getRecordFromCache'; +import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache'; +import { generateDepthOneRecordGqlFields } from '@/object-record/graphql/utils/generateDepthOneRecordGqlFields'; import { + personIds, + personRecords, query, responseData, variables, } from '@/object-record/hooks/__mocks__/useDeleteManyRecords'; import { useDeleteManyRecords } from '@/object-record/hooks/useDeleteManyRecords'; import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries'; +import { ObjectRecord } from '@/object-record/types/ObjectRecord'; +import { InMemoryCache } from '@apollo/client'; +import { MockedResponse } from '@apollo/client/testing'; import { act } from 'react'; import { getJestMetadataAndApolloMocksWrapper } from '~/testing/jest/getJestMetadataAndApolloMocksWrapper'; - -const personIds = [ - 'a7286b9a-c039-4a89-9567-2dfa7953cda9', - '37faabcd-cb39-4a0a-8618-7e3fda9afca0', -]; - -const mocks = [ +import { getPersonObjectMetadataItem } from '~/testing/mock-data/people'; +const getDefaultMocks = ( + overrides?: Partial, +): MockedResponse[] => [ { request: { query, @@ -23,9 +28,10 @@ const mocks = [ }, result: jest.fn(() => ({ data: { - deletePeople: [responseData], + deletePeople: responseData, }, })), + ...overrides, }, ]; @@ -34,32 +40,169 @@ const mockRefetchAggregateQueries = jest.fn(); (useRefetchAggregateQueries as jest.Mock).mockReturnValue({ refetchAggregateQueries: mockRefetchAggregateQueries, }); - -const Wrapper = getJestMetadataAndApolloMocksWrapper({ - apolloMocks: mocks, -}); - +const objectMetadataItem = getPersonObjectMetadataItem(); +const objectMetadataItems = [objectMetadataItem]; +const expectedCachedRecordsWithDeletedAt = personRecords.map( + (personRecord) => ({ + ...personRecord, + deletedAt: expect.any(String), + }), +); describe('useDeleteManyRecords', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - it('works as expected', async () => { - const { result } = renderHook( - () => useDeleteManyRecords({ objectNameSingular: 'person' }), - { - wrapper: Wrapper, - }, + let cache: InMemoryCache; + const assertCachedRecordsMatch = (expectedRecords: ObjectRecord[]) => { + expectedRecords.forEach((expectedRecord) => { + const cachedRecord = getRecordFromCache({ + cache, + objectMetadataItem, + objectMetadataItems, + recordId: expectedRecord.id, + }); + expect(cachedRecord).not.toBeNull(); + if (cachedRecord === null) throw new Error('Should never occur'); + // TODO find a way to reverse assertion or be more strict + expect(expectedRecord).toMatchObject(cachedRecord); + }); + }; + const assertCachedRecordsIsNull = (recordIds: string[]) => + recordIds.forEach((recordId) => + expect( + getRecordFromCache({ + cache, + objectMetadataItem, + objectMetadataItems, + recordId, + }), + ).toBeNull(), ); - await act(async () => { - const res = await result.current.deleteManyRecords({ - recordIdsToDelete: personIds, + beforeEach(() => { + jest.clearAllMocks(); + cache = new InMemoryCache(); + }); + + describe('A. Starting from empty cache ', () => { + it('1. Should handle optimistic behavior after many records deletion', async () => { + const apolloMocks = getDefaultMocks(); + const { result } = renderHook( + () => useDeleteManyRecords({ objectNameSingular: 'person' }), + { + wrapper: getJestMetadataAndApolloMocksWrapper({ + apolloMocks, + cache, + }), + }, + ); + + await act(async () => { + const res = await result.current.deleteManyRecords({ + recordIdsToDelete: personIds, + }); + expect(res).toEqual(responseData); + assertCachedRecordsIsNull(personIds); }); - expect(res).toBeDefined(); - expect(res[0]).toHaveProperty('id'); + + expect(apolloMocks[0].result).toHaveBeenCalled(); + expect(mockRefetchAggregateQueries).toHaveBeenCalledTimes(1); + }); + }); + + describe('B. Starting from filled cache', () => { + beforeEach(() => { + personRecords.forEach((record) => + updateRecordFromCache({ + cache, + objectMetadataItem, + objectMetadataItems, + record, + recordGqlFields: generateDepthOneRecordGqlFields({ + objectMetadataItem, + record, + }), + }), + ); + }); + it('1. Should handle optimistic behavior after many successful records deletion', async () => { + const apolloMocks = getDefaultMocks(); + const { result } = renderHook( + () => useDeleteManyRecords({ objectNameSingular: 'person' }), + { + wrapper: getJestMetadataAndApolloMocksWrapper({ + apolloMocks, + cache, + }), + }, + ); + + await act(async () => { + const res = await result.current.deleteManyRecords({ + recordIdsToDelete: personIds, + }); + expect(res).toEqual(responseData); + assertCachedRecordsMatch(expectedCachedRecordsWithDeletedAt); + }); + + expect(apolloMocks[0].result).toHaveBeenCalled(); + expect(mockRefetchAggregateQueries).toHaveBeenCalledTimes(1); }); - expect(mocks[0].result).toHaveBeenCalled(); - expect(mockRefetchAggregateQueries).toHaveBeenCalledTimes(1); + it('2. Should handle optimistic behavior before send many record deletion', async () => { + const apolloMocks = getDefaultMocks(); + const { result } = renderHook( + () => useDeleteManyRecords({ objectNameSingular: 'person' }), + { + wrapper: getJestMetadataAndApolloMocksWrapper({ + apolloMocks: getDefaultMocks({ + delay: Number.POSITIVE_INFINITY, + }), + cache, + }), + }, + ); + + await act(async () => { + result.current.deleteManyRecords({ + recordIdsToDelete: personIds, + }); + await waitFor(() => + assertCachedRecordsMatch(expectedCachedRecordsWithDeletedAt), + ); + }); + + expect(apolloMocks[0].result).not.toHaveBeenCalled(); + expect(mockRefetchAggregateQueries).not.toHaveBeenCalled(); + }); + + it('3. Should rollback optimistic behavior after failing to delete many records', async () => { + const apolloMocks = getDefaultMocks(); + const { result } = renderHook( + () => useDeleteManyRecords({ objectNameSingular: 'person' }), + { + wrapper: getJestMetadataAndApolloMocksWrapper({ + apolloMocks: getDefaultMocks({ + error: new Error('Internal server error'), + }), + cache, + }), + }, + ); + + await act(async () => { + try { + await result.current.deleteManyRecords({ + recordIdsToDelete: personIds, + }); + fail('Should have thrown an error'); + } catch (e) { + expect(e).toMatchInlineSnapshot( + `[ApolloError: Internal server error]`, + ); + assertCachedRecordsMatch(personRecords); + } + }); + + expect(apolloMocks[0].result).not.toHaveBeenCalled(); + expect(mockRefetchAggregateQueries).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/twenty-front/src/modules/object-record/hooks/__tests__/useDeleteOneRecord.test.tsx b/packages/twenty-front/src/modules/object-record/hooks/__tests__/useDeleteOneRecord.test.tsx index 1a5b65bc9..ba80fd6f6 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/__tests__/useDeleteOneRecord.test.tsx +++ b/packages/twenty-front/src/modules/object-record/hooks/__tests__/useDeleteOneRecord.test.tsx @@ -1,6 +1,9 @@ -import { renderHook } from '@testing-library/react'; +import { renderHook, waitFor } from '@testing-library/react'; import { act } from 'react'; +import { getRecordFromCache } from '@/object-record/cache/utils/getRecordFromCache'; +import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache'; +import { generateDepthOneRecordGqlFields } from '@/object-record/graphql/utils/generateDepthOneRecordGqlFields'; import { query, responseData, @@ -8,23 +11,15 @@ import { } from '@/object-record/hooks/__mocks__/useDeleteOneRecord'; import { useDeleteOneRecord } from '@/object-record/hooks/useDeleteOneRecord'; import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries'; +import { ObjectRecord } from '@/object-record/types/ObjectRecord'; +import { InMemoryCache } from '@apollo/client'; +import { MockedResponse } from '@apollo/client/testing'; +import { expect } from '@storybook/jest'; import { getJestMetadataAndApolloMocksWrapper } from '~/testing/jest/getJestMetadataAndApolloMocksWrapper'; - -const personId = 'a7286b9a-c039-4a89-9567-2dfa7953cda9'; - -const mocks = [ - { - request: { - query, - variables, - }, - result: jest.fn(() => ({ - data: { - deletePerson: responseData, - }, - })), - }, -]; +import { + getPersonObjectMetadataItem, + getPersonRecord, +} from '~/testing/mock-data/people'; jest.mock('@/object-record/hooks/useRefetchAggregateQueries'); const mockRefetchAggregateQueries = jest.fn(); @@ -32,29 +27,257 @@ const mockRefetchAggregateQueries = jest.fn(); refetchAggregateQueries: mockRefetchAggregateQueries, }); -const Wrapper = getJestMetadataAndApolloMocksWrapper({ - apolloMocks: mocks, -}); - +// TODO Should test relation deletion cache hydratation describe('useDeleteOneRecord', () => { + let cache: InMemoryCache; + const getDefaultMocks = ( + overrides?: Partial, + ): MockedResponse[] => [ + { + request: { + query, + variables, + }, + result: jest.fn(() => ({ + data: { + deletePerson: responseData, + }, + })), + ...overrides, + }, + ]; + const defaultMocks = getDefaultMocks(); + const personRecord = getPersonRecord({ + id: 'a7286b9a-c039-4a89-9567-2dfa7953cda9', + deletedAt: null, + }); + const objectMetadataItem = getPersonObjectMetadataItem(); + const objectMetadataItems = [objectMetadataItem]; + const assertCachedRecordMatch = (expectedRecord: ObjectRecord) => { + const cachedRecord = getRecordFromCache({ + cache, + objectMetadataItem, + objectMetadataItems, + recordId: personRecord.id, + }); + expect(cachedRecord).not.toBeNull(); + if (cachedRecord === null) throw new Error('Should never occur'); + // Find a way to reverse assertion + expect(expectedRecord).toMatchObject(cachedRecord); + }; + const assertCachedRecordIsNull = () => + expect( + getRecordFromCache({ + cache, + objectMetadataItem, + objectMetadataItems, + recordId: personRecord.id, + }), + ).toBeNull(); beforeEach(() => { jest.clearAllMocks(); + cache = new InMemoryCache(); }); - it('works as expected', async () => { - const { result } = renderHook( - () => useDeleteOneRecord({ objectNameSingular: 'person' }), - { - wrapper: Wrapper, - }, - ); - await act(async () => { - const res = await result.current.deleteOneRecord(personId); - expect(res).toBeDefined(); - expect(res).toHaveProperty('id', personId); + describe('A. Starting from empty cache', () => { + it('1. Should successfully delete record and update record cache entry', async () => { + const { result } = renderHook( + () => + useDeleteOneRecord({ + objectNameSingular: objectMetadataItem.nameSingular, + }), + { + wrapper: getJestMetadataAndApolloMocksWrapper({ + apolloMocks: defaultMocks, + cache, + }), + }, + ); + + await act(async () => { + const deleteOneResult = await result.current.deleteOneRecord( + personRecord.id, + ); + const expectedResult: ObjectRecord = { + __typename: personRecord.__typename, + deletedAt: expect.any(String), + id: personRecord.id, + }; + expect(deleteOneResult).toStrictEqual(expectedResult); + assertCachedRecordMatch(expectedResult); + }); + + expect(defaultMocks[0].result).toHaveBeenCalled(); + expect(mockRefetchAggregateQueries).toHaveBeenCalledTimes(1); }); - expect(mocks[0].result).toHaveBeenCalled(); - expect(mockRefetchAggregateQueries).toHaveBeenCalledTimes(1); + it('2. Should not handle optimistic cache update on record deletion', async () => { + const apolloMocks: MockedResponse[] = getDefaultMocks({ + delay: Number.POSITIVE_INFINITY, + }); + const { result } = renderHook( + () => + useDeleteOneRecord({ + objectNameSingular: objectMetadataItem.nameSingular, + }), + { + wrapper: getJestMetadataAndApolloMocksWrapper({ + cache, + apolloMocks, + }), + }, + ); + + await act(async () => { + result.current.deleteOneRecord(personRecord.id); + await waitFor(() => { + assertCachedRecordIsNull(); + }); + }); + + expect(defaultMocks[0].result).not.toHaveBeenCalled(); + expect(mockRefetchAggregateQueries).not.toHaveBeenCalled(); + }); + + it('3. Should not handle optimistic cache update rollback on record deletion failure', async () => { + const apolloMocks: MockedResponse[] = getDefaultMocks({ + error: new Error('Internal server error'), + }); + const { result } = renderHook( + () => + useDeleteOneRecord({ + objectNameSingular: objectMetadataItem.nameSingular, + }), + { + wrapper: getJestMetadataAndApolloMocksWrapper({ + cache, + apolloMocks, + }), + }, + ); + + await act(async () => { + try { + await result.current.deleteOneRecord(personRecord.id); + fail('Should have thrown an error'); + } catch (e) { + assertCachedRecordIsNull(); + } + }); + }); + }); + + describe('B. Starting from filled cache', () => { + beforeEach(() => { + const recordGqlFields = generateDepthOneRecordGqlFields({ + objectMetadataItem, + record: personRecord, + }); + updateRecordFromCache({ + cache, + objectMetadataItem, + objectMetadataItems, + record: personRecord, + recordGqlFields, + }); + }); + + it('1. Should handle successfull record deletion', async () => { + const { result } = renderHook( + () => + useDeleteOneRecord({ + objectNameSingular: objectMetadataItem.nameSingular, + }), + { + wrapper: getJestMetadataAndApolloMocksWrapper({ + apolloMocks: defaultMocks, + cache, + }), + }, + ); + + await act(async () => { + const res = await result.current.deleteOneRecord(personRecord.id); + expect(res).toBeDefined(); + expect(res.deletedAt).toBeDefined(); + expect(res).toHaveProperty('id', personRecord.id); + + const personRecordWithDeletedAt = { + ...personRecord, + deletedAt: expect.any(String), + }; + assertCachedRecordMatch(personRecordWithDeletedAt); + }); + + expect(defaultMocks[0].result).toHaveBeenCalled(); + expect(mockRefetchAggregateQueries).toHaveBeenCalledTimes(1); + }); + + it('2. Should handle optimistic cache on record deletion', async () => { + const apolloMocks = getDefaultMocks({ + // Used to assert loading state + delay: Number.POSITIVE_INFINITY, + }); + const { result } = renderHook( + () => + useDeleteOneRecord({ + objectNameSingular: objectMetadataItem.nameSingular, + }), + { + wrapper: getJestMetadataAndApolloMocksWrapper({ + apolloMocks, + cache, + }), + }, + ); + + await act(async () => { + result.current.deleteOneRecord(personRecord.id); + await waitFor(() => { + const personRecordWithDeletedAt = { + ...personRecord, + deletedAt: expect.any(String), + }; + assertCachedRecordMatch(personRecordWithDeletedAt); + }); + }); + + expect(apolloMocks[0].result).not.toHaveBeenCalled(); + expect(mockRefetchAggregateQueries).not.toHaveBeenCalled(); + }); + + it('3. Should handle optimistic cache rollback on record deletion failure', async () => { + const apolloMocks = getDefaultMocks({ + error: new Error('Internal server error'), + }); + const { result } = renderHook( + () => + useDeleteOneRecord({ + objectNameSingular: objectMetadataItem.nameSingular, + }), + { + wrapper: getJestMetadataAndApolloMocksWrapper({ + apolloMocks, + cache, + }), + }, + ); + + await act(async () => { + try { + await result.current.deleteOneRecord(personRecord.id); + fail('Should have thrown an error'); + } catch (e) { + const personRecordWithDeletedAt = { + ...personRecord, + deletedAt: null, + }; + assertCachedRecordMatch(personRecordWithDeletedAt); + } + }); + + expect(apolloMocks[0].result).not.toHaveBeenCalled(); + expect(mockRefetchAggregateQueries).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/twenty-front/src/modules/object-record/hooks/useDeleteManyRecords.ts b/packages/twenty-front/src/modules/object-record/hooks/useDeleteManyRecords.ts index ca03bd89a..838456718 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useDeleteManyRecords.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useDeleteManyRecords.ts @@ -5,6 +5,7 @@ import { apiConfigState } from '@/client-config/states/apiConfigState'; import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems'; import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordFromCache'; +import { getObjectTypename } from '@/object-record/cache/utils/getObjectTypename'; import { getRecordNodeFromRecord } from '@/object-record/cache/utils/getRecordNodeFromRecord'; import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache'; import { DEFAULT_MUTATION_BATCH_SIZE } from '@/object-record/constants/DefaultMutationBatchSize'; @@ -14,8 +15,7 @@ import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggr import { ObjectRecord } from '@/object-record/types/ObjectRecord'; import { getDeleteManyRecordsMutationResponseField } from '@/object-record/utils/getDeleteManyRecordsMutationResponseField'; import { useRecoilValue } from 'recoil'; -import { capitalize, isDefined } from 'twenty-shared'; -import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull'; +import { isDefined } from 'twenty-shared'; import { sleep } from '~/utils/sleep'; type useDeleteManyRecordProps = { @@ -77,21 +77,18 @@ export const useDeleteManyRecords = ({ (batchIndex + 1) * mutationPageSize, ); - const currentTimestamp = new Date().toISOString(); - const cachedRecords = batchedIdsToDelete .map((idToDelete) => getRecordFromCache(idToDelete, apolloClient.cache)) .filter(isDefined); - + const currentTimestamp = new Date().toISOString(); if (!skipOptimisticEffect) { const cachedRecordsNode: RecordGqlNode[] = []; const computedOptimisticRecordsNode: RecordGqlNode[] = []; + const recordGqlFields = { + deletedAt: true, + }; cachedRecords.forEach((cachedRecord) => { - if (!isDefined(cachedRecord) || !isDefined(cachedRecord.id)) { - return; - } - const cachedRecordNode = getRecordNodeFromRecord({ record: cachedRecord, objectMetadataItem, @@ -101,10 +98,9 @@ export const useDeleteManyRecords = ({ const computedOptimisticRecord = { ...cachedRecord, - ...{ id: cachedRecord.id, deletedAt: currentTimestamp }, - ...{ __typename: capitalize(objectMetadataItem.nameSingular) }, + deletedAt: currentTimestamp, + __typename: getObjectTypename(objectMetadataItem.nameSingular), }; - const optimisticRecordNode = getRecordNodeFromRecord({ record: computedOptimisticRecord, objectMetadataItem, @@ -112,22 +108,18 @@ export const useDeleteManyRecords = ({ computeReferences: false, }); - if ( - !isDefined(optimisticRecordNode) || - !isDefined(cachedRecordNode) - ) { - return; + if (isDefined(optimisticRecordNode) && isDefined(cachedRecordNode)) { + updateRecordFromCache({ + objectMetadataItems, + objectMetadataItem, + cache: apolloClient.cache, + record: computedOptimisticRecord, + recordGqlFields, + }); + + computedOptimisticRecordsNode.push(optimisticRecordNode); + cachedRecordsNode.push(cachedRecordNode); } - - updateRecordFromCache({ - objectMetadataItems, - objectMetadataItem, - cache: apolloClient.cache, - record: computedOptimisticRecord, - }); - - computedOptimisticRecordsNode.push(optimisticRecordNode); - cachedRecordsNode.push(cachedRecordNode); }); triggerUpdateRecordOptimisticEffectByBatch({ @@ -140,26 +132,30 @@ export const useDeleteManyRecords = ({ } const deletedRecordsResponse = await apolloClient - .mutate({ + .mutate>({ mutation: deleteManyRecordsMutation, variables: { filter: { id: { in: batchedIdsToDelete } }, }, }) .catch((error: Error) => { + if (skipOptimisticEffect) { + throw error; + } + const cachedRecordsNode: RecordGqlNode[] = []; const computedOptimisticRecordsNode: RecordGqlNode[] = []; + const recordGqlFields = { + deletedAt: true, + }; cachedRecords.forEach((cachedRecord) => { - if (isUndefinedOrNull(cachedRecord?.id)) { - return; - } - updateRecordFromCache({ objectMetadataItems, objectMetadataItem, cache: apolloClient.cache, - record: cachedRecord, + record: { ...cachedRecord, deletedAt: null }, + recordGqlFields, }); const cachedRecordWithConnection = @@ -172,8 +168,8 @@ export const useDeleteManyRecords = ({ const computedOptimisticRecord = { ...cachedRecord, - ...{ id: cachedRecord.id, deletedAt: currentTimestamp }, - ...{ __typename: capitalize(objectMetadataItem.nameSingular) }, + deletedAt: currentTimestamp, + __typename: getObjectTypename(objectMetadataItem.nameSingular), }; const optimisticRecordWithConnection = @@ -185,14 +181,14 @@ export const useDeleteManyRecords = ({ }); if ( - !isDefined(optimisticRecordWithConnection) || - !isDefined(cachedRecordWithConnection) + isDefined(optimisticRecordWithConnection) && + isDefined(cachedRecordWithConnection) ) { - return; + cachedRecordsNode.push(cachedRecordWithConnection); + computedOptimisticRecordsNode.push( + optimisticRecordWithConnection, + ); } - - cachedRecordsNode.push(cachedRecordWithConnection); - computedOptimisticRecordsNode.push(optimisticRecordWithConnection); }); triggerUpdateRecordOptimisticEffectByBatch({ @@ -208,7 +204,6 @@ export const useDeleteManyRecords = ({ const deletedRecordsForThisBatch = deletedRecordsResponse.data?.[mutationResponseField] ?? []; - deletedRecords.push(...deletedRecordsForThisBatch); if (isDefined(delayInMsBetweenRequests)) { diff --git a/packages/twenty-front/src/modules/object-record/hooks/useDeleteOneRecord.ts b/packages/twenty-front/src/modules/object-record/hooks/useDeleteOneRecord.ts index 2d3846bd3..8b977f83e 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useDeleteOneRecord.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useDeleteOneRecord.ts @@ -5,13 +5,15 @@ import { triggerUpdateRecordOptimisticEffect } from '@/apollo/optimistic-effect/ import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems'; import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordFromCache'; +import { getObjectTypename } from '@/object-record/cache/utils/getObjectTypename'; import { getRecordNodeFromRecord } from '@/object-record/cache/utils/getRecordNodeFromRecord'; import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache'; import { useDeleteOneRecordMutation } from '@/object-record/hooks/useDeleteOneRecordMutation'; import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries'; import { ObjectRecord } from '@/object-record/types/ObjectRecord'; import { getDeleteOneRecordMutationResponseField } from '@/object-record/utils/getDeleteOneRecordMutationResponseField'; -import { capitalize, isDefined } from 'twenty-shared'; +import { isNull } from '@sniptt/guards'; +import { isDefined } from 'twenty-shared'; type useDeleteOneRecordProps = { objectNameSingular: string; @@ -45,10 +47,7 @@ export const useDeleteOneRecord = ({ const deleteOneRecord = useCallback( async (idToDelete: string) => { - const currentTimestamp = new Date().toISOString(); - const cachedRecord = getRecordFromCache(idToDelete, apolloClient.cache); - const cachedRecordNode = getRecordNodeFromRecord({ record: cachedRecord, objectMetadataItem, @@ -56,12 +55,13 @@ export const useDeleteOneRecord = ({ computeReferences: false, }); + const currentTimestamp = new Date().toISOString(); const computedOptimisticRecord = { ...cachedRecord, - ...{ id: idToDelete, deletedAt: currentTimestamp }, - ...{ __typename: capitalize(objectMetadataItem.nameSingular) }, + id: idToDelete, + deletedAt: currentTimestamp, + __typename: getObjectTypename(objectMetadataItem.nameSingular), }; - const optimisticRecordNode = getRecordNodeFromRecord({ record: computedOptimisticRecord, objectMetadataItem, @@ -69,25 +69,32 @@ export const useDeleteOneRecord = ({ computeReferences: false, }); - if (!isDefined(optimisticRecordNode) || !isDefined(cachedRecordNode)) { - return null; + const shouldHandleOptimisticCache = + !isNull(cachedRecord) && + isDefined(optimisticRecordNode) && + isDefined(cachedRecordNode); + + if (shouldHandleOptimisticCache) { + const recordGqlFields = { + deletedAt: true, + }; + updateRecordFromCache({ + objectMetadataItems, + objectMetadataItem, + cache: apolloClient.cache, + record: computedOptimisticRecord, + recordGqlFields, + }); + + triggerUpdateRecordOptimisticEffect({ + cache: apolloClient.cache, + objectMetadataItem, + currentRecord: cachedRecordNode, + updatedRecord: optimisticRecordNode, + objectMetadataItems, + }); } - updateRecordFromCache({ - objectMetadataItems, - objectMetadataItem, - cache: apolloClient.cache, - record: computedOptimisticRecord, - }); - - triggerUpdateRecordOptimisticEffect({ - cache: apolloClient.cache, - objectMetadataItem, - currentRecord: cachedRecordNode, - updatedRecord: optimisticRecordNode, - objectMetadataItems, - }); - const deletedRecord = await apolloClient .mutate({ mutation: deleteOneRecordMutation, @@ -96,28 +103,36 @@ export const useDeleteOneRecord = ({ }, update: (cache, { data }) => { const record = data?.[mutationResponseField]; - - if (!isDefined(record) || !isDefined(computedOptimisticRecord)) + if (!isDefined(record) || !shouldHandleOptimisticCache) { return; + } triggerUpdateRecordOptimisticEffect({ cache, objectMetadataItem, - currentRecord: computedOptimisticRecord, + currentRecord: optimisticRecordNode, updatedRecord: record, objectMetadataItems, }); }, }) .catch((error: Error) => { - if (!cachedRecord) { + if (!shouldHandleOptimisticCache) { throw error; } + + const recordGqlFields = { + deletedAt: true, + }; updateRecordFromCache({ objectMetadataItems, objectMetadataItem, cache: apolloClient.cache, - record: cachedRecord, + record: { + ...cachedRecord, + deletedAt: null, + }, + recordGqlFields, }); triggerUpdateRecordOptimisticEffect({ diff --git a/packages/twenty-front/src/modules/object-record/hooks/useDestroyManyRecords.ts b/packages/twenty-front/src/modules/object-record/hooks/useDestroyManyRecords.ts index 3cc39f2b3..d254a4c5d 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useDestroyManyRecords.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useDestroyManyRecords.ts @@ -9,6 +9,7 @@ import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordF import { DEFAULT_MUTATION_BATCH_SIZE } from '@/object-record/constants/DefaultMutationBatchSize'; import { useDestroyManyRecordsMutation } from '@/object-record/hooks/useDestroyManyRecordsMutation'; import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries'; +import { ObjectRecord } from '@/object-record/types/ObjectRecord'; import { getDestroyManyRecordsMutationResponseField } from '@/object-record/utils/getDestroyManyRecordsMutationResponseField'; import { useRecoilValue } from 'recoil'; import { capitalize, isDefined } from 'twenty-shared'; @@ -39,9 +40,7 @@ export const useDestroyManyRecords = ({ objectNameSingular, }); - const getRecordFromCache = useGetRecordFromCache({ - objectNameSingular, - }); + const getRecordFromCache = useGetRecordFromCache({ objectNameSingular }); const { destroyManyRecordsMutation } = useDestroyManyRecordsMutation({ objectNameSingular, @@ -74,12 +73,12 @@ export const useDestroyManyRecords = ({ (batchIndex + 1) * mutationPageSize, ); - const originalRecords = batchedIdToDestroy + const cachedRecords = batchedIdToDestroy .map((recordId) => getRecordFromCache(recordId, apolloClient.cache)) .filter(isDefined); const destroyedRecordsResponse = await apolloClient - .mutate({ + .mutate>({ mutation: destroyManyRecordsMutation, variables: { filter: { id: { in: batchedIdToDestroy } }, @@ -94,31 +93,32 @@ export const useDestroyManyRecords = ({ }), ), }, - update: skipOptimisticEffect - ? undefined - : (cache, { data }) => { - const records = data?.[mutationResponseField]; + update: (cache, { data }) => { + if (skipOptimisticEffect) { + return; + } + const records = data?.[mutationResponseField]; - if (!records?.length) return; + if (!isDefined(records) || records.length === 0) return; - const cachedRecords = records - .map((record) => getRecordFromCache(record.id, cache)) - .filter(isDefined); + const cachedRecords = records + .map((record) => getRecordFromCache(record.id, cache)) + .filter(isDefined); - triggerDestroyRecordsOptimisticEffect({ - cache, - objectMetadataItem, - recordsToDestroy: cachedRecords, - objectMetadataItems, - }); - }, + triggerDestroyRecordsOptimisticEffect({ + cache, + objectMetadataItem, + recordsToDestroy: cachedRecords, + objectMetadataItems, + }); + }, }) .catch((error: Error) => { - if (originalRecords.length > 0) { + if (cachedRecords.length > 0 && !skipOptimisticEffect) { triggerCreateRecordsOptimisticEffect({ cache: apolloClient.cache, objectMetadataItem, - recordsToCreate: originalRecords, + recordsToCreate: cachedRecords, objectMetadataItems, }); } diff --git a/packages/twenty-front/src/modules/object-record/hooks/useDestroyOneRecord.ts b/packages/twenty-front/src/modules/object-record/hooks/useDestroyOneRecord.ts index 851be9d00..4f5ce94a2 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useDestroyOneRecord.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useDestroyOneRecord.ts @@ -7,10 +7,8 @@ import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadata import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems'; import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordFromCache'; import { useDestroyOneRecordMutation } from '@/object-record/hooks/useDestroyOneRecordMutation'; -import { ObjectRecord } from '@/object-record/types/ObjectRecord'; import { getDestroyOneRecordMutationResponseField } from '@/object-record/utils/getDestroyOneRecordMutationResponseField'; -import { capitalize } from 'twenty-shared'; -import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull'; +import { capitalize, isDefined } from 'twenty-shared'; type useDestroyOneRecordProps = { objectNameSingular: string; @@ -26,9 +24,7 @@ export const useDestroyOneRecord = ({ objectNameSingular, }); - const getRecordFromCache = useGetRecordFromCache({ - objectNameSingular, - }); + const getRecordFromCache = useGetRecordFromCache({ objectNameSingular }); const { destroyOneRecordMutation } = useDestroyOneRecordMutation({ objectNameSingular, @@ -41,7 +37,7 @@ export const useDestroyOneRecord = ({ const destroyOneRecord = useCallback( async (idToDestroy: string) => { - const originalRecord: ObjectRecord | null = getRecordFromCache( + const originalRecord = getRecordFromCache( idToDestroy, apolloClient.cache, ); @@ -58,13 +54,10 @@ export const useDestroyOneRecord = ({ }, update: (cache, { data }) => { const record = data?.[mutationResponseField]; - - if (!record) return; + if (!isDefined(record)) return; const cachedRecord = getRecordFromCache(record.id, cache); - - if (!cachedRecord) return; - + if (!isDefined(cachedRecord)) return; triggerDestroyRecordsOptimisticEffect({ cache, objectMetadataItem, @@ -74,7 +67,7 @@ export const useDestroyOneRecord = ({ }, }) .catch((error: Error) => { - if (!isUndefinedOrNull(originalRecord)) { + if (isDefined(originalRecord)) { triggerCreateRecordsOptimisticEffect({ cache: apolloClient.cache, objectMetadataItem, @@ -82,6 +75,7 @@ export const useDestroyOneRecord = ({ objectMetadataItems, }); } + throw error; }); diff --git a/packages/twenty-front/src/modules/object-record/hooks/useRestoreManyRecords.ts b/packages/twenty-front/src/modules/object-record/hooks/useRestoreManyRecords.ts index acee58733..75d71a1c5 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useRestoreManyRecords.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useRestoreManyRecords.ts @@ -5,6 +5,7 @@ import { apiConfigState } from '@/client-config/states/apiConfigState'; import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems'; import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordFromCache'; +import { getObjectTypename } from '@/object-record/cache/utils/getObjectTypename'; import { getRecordNodeFromRecord } from '@/object-record/cache/utils/getRecordNodeFromRecord'; import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache'; import { DEFAULT_MUTATION_BATCH_SIZE } from '@/object-record/constants/DefaultMutationBatchSize'; @@ -20,7 +21,8 @@ type useRestoreManyRecordProps = { refetchFindManyQuery?: boolean; }; -type RestoreManyRecordsOptions = { +type RestoreManyRecordsProps = { + idsToRestore: string[]; skipOptimisticEffect?: boolean; delayInMsBetweenRequests?: number; }; @@ -53,10 +55,11 @@ export const useRestoreManyRecords = ({ objectMetadataItem.namePlural, ); - const restoreManyRecords = async ( - idsToRestore: string[], - options?: RestoreManyRecordsOptions, - ) => { + const restoreManyRecords = async ({ + idsToRestore, + delayInMsBetweenRequests, + skipOptimisticEffect = false, + }: RestoreManyRecordsProps) => { const numberOfBatches = Math.ceil(idsToRestore.length / mutationPageSize); const restoredRecords = []; @@ -73,12 +76,8 @@ export const useRestoreManyRecords = ({ ) .filter(isDefined); - if (!options?.skipOptimisticEffect) { + if (!skipOptimisticEffect) { cachedRecords.forEach((cachedRecord) => { - if (!cachedRecord || !cachedRecord.id) { - return; - } - const cachedRecordWithConnection = getRecordNodeFromRecord({ record: cachedRecord, @@ -86,13 +85,11 @@ export const useRestoreManyRecords = ({ objectMetadataItems, computeReferences: true, }); - const computedOptimisticRecord = { ...cachedRecord, - ...{ id: cachedRecord.id, deletedAt: null }, - ...{ __typename: capitalize(objectMetadataItem.nameSingular) }, + deletedAt: null, + __typename: getObjectTypename(objectMetadataItem.nameSingular), }; - const optimisticRecordWithConnection = getRecordNodeFromRecord({ record: computedOptimisticRecord, @@ -101,24 +98,28 @@ export const useRestoreManyRecords = ({ computeReferences: true, }); - if (!optimisticRecordWithConnection || !cachedRecordWithConnection) { - return null; + if ( + isDefined(optimisticRecordWithConnection) && + isDefined(cachedRecordWithConnection) + ) { + const recordGqlFields = { + deletedAt: true, + }; + updateRecordFromCache({ + objectMetadataItems, + objectMetadataItem, + cache: apolloClient.cache, + record: computedOptimisticRecord, + recordGqlFields, + }); + triggerUpdateRecordOptimisticEffect({ + cache: apolloClient.cache, + objectMetadataItem, + currentRecord: cachedRecordWithConnection, + updatedRecord: optimisticRecordWithConnection, + objectMetadataItems, + }); } - - updateRecordFromCache({ - objectMetadataItems, - objectMetadataItem, - cache: apolloClient.cache, - record: computedOptimisticRecord, - }); - - triggerUpdateRecordOptimisticEffect({ - cache: apolloClient.cache, - objectMetadataItem, - currentRecord: cachedRecordWithConnection, - updatedRecord: optimisticRecordWithConnection, - objectMetadataItems, - }); }); } @@ -130,18 +131,10 @@ export const useRestoreManyRecords = ({ }, }) .catch((error: Error) => { + if (skipOptimisticEffect) { + throw error; + } cachedRecords.forEach((cachedRecord) => { - if (!cachedRecord) { - return; - } - - updateRecordFromCache({ - objectMetadataItems, - objectMetadataItem, - cache: apolloClient.cache, - record: cachedRecord, - }); - const cachedRecordWithConnection = getRecordNodeFromRecord({ record: cachedRecord, @@ -155,7 +148,6 @@ export const useRestoreManyRecords = ({ ...{ id: cachedRecord.id, deletedAt: null }, ...{ __typename: capitalize(objectMetadataItem.nameSingular) }, }; - const optimisticRecordWithConnection = getRecordNodeFromRecord({ record: computedOptimisticRecord, @@ -165,19 +157,28 @@ export const useRestoreManyRecords = ({ }); if ( - !optimisticRecordWithConnection || - !cachedRecordWithConnection + isDefined(optimisticRecordWithConnection) && + isDefined(cachedRecordWithConnection) ) { - return null; - } + const recordGqlFields = { + deletedAt: true, + }; + updateRecordFromCache({ + objectMetadataItems, + objectMetadataItem, + cache: apolloClient.cache, + record: cachedRecord, + recordGqlFields, + }); - triggerUpdateRecordOptimisticEffect({ - cache: apolloClient.cache, - objectMetadataItem, - currentRecord: optimisticRecordWithConnection, - updatedRecord: cachedRecordWithConnection, - objectMetadataItems, - }); + triggerUpdateRecordOptimisticEffect({ + cache: apolloClient.cache, + objectMetadataItem, + currentRecord: optimisticRecordWithConnection, + updatedRecord: cachedRecordWithConnection, + objectMetadataItems, + }); + } }); throw error; @@ -188,8 +189,8 @@ export const useRestoreManyRecords = ({ restoredRecords.push(...restoredRecordsForThisBatch); - if (isDefined(options?.delayInMsBetweenRequests)) { - await sleep(options.delayInMsBetweenRequests); + if (isDefined(delayInMsBetweenRequests)) { + await sleep(delayInMsBetweenRequests); } } 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 e1859244d..bc767622d 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useUpdateOneRecord.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useUpdateOneRecord.ts @@ -4,6 +4,7 @@ import { triggerUpdateRecordOptimisticEffect } from '@/apollo/optimistic-effect/ import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems'; import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordFromCache'; +import { getObjectTypename } from '@/object-record/cache/utils/getObjectTypename'; import { getRecordNodeFromRecord } from '@/object-record/cache/utils/getRecordNodeFromRecord'; import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache'; import { generateDepthOneRecordGqlFields } from '@/object-record/graphql/utils/generateDepthOneRecordGqlFields'; @@ -13,8 +14,9 @@ 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, isDefined } from 'twenty-shared'; -import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull'; +import { isNull } from '@sniptt/guards'; +import { isDefined } from 'twenty-shared'; +import { buildRecordFromKeysWithSameValue } from '~/utils/array/buildRecordFromKeysWithSameValue'; type useUpdateOneRecordProps = { objectNameSingular: string; @@ -60,15 +62,15 @@ export const useUpdateOneRecord = < updateOneRecordInput: Partial>; optimisticRecord?: Partial; }) => { - const optimisticRecordInput = computeOptimisticRecordFromInput({ - objectMetadataItem, - recordInput: updateOneRecordInput, - cache: apolloClient.cache, - objectMetadataItems, - }); - + const optimisticRecordInput = + optimisticRecord ?? + computeOptimisticRecordFromInput({ + objectMetadataItem, + recordInput: updateOneRecordInput, + cache: apolloClient.cache, + objectMetadataItems, + }); const cachedRecord = getRecordFromCache(idToUpdate); - const cachedRecordWithConnection = getRecordNodeFromRecord({ record: cachedRecord, objectMetadataItem, @@ -79,11 +81,10 @@ export const useUpdateOneRecord = < const computedOptimisticRecord = { ...cachedRecord, - ...(optimisticRecord ?? optimisticRecordInput), - ...{ id: idToUpdate }, - ...{ __typename: capitalize(objectMetadataItem.nameSingular) }, + ...optimisticRecordInput, + id: idToUpdate, + __typename: getObjectTypename(objectMetadataItem.nameSingular), }; - const optimisticRecordWithConnection = getRecordNodeFromRecord({ record: computedOptimisticRecord, @@ -92,25 +93,34 @@ export const useUpdateOneRecord = < recordGqlFields: computedRecordGqlFields, computeReferences: false, }); - if (!optimisticRecordWithConnection || !cachedRecordWithConnection) { - return null; + + const shouldHandleOptimisticCache = + !isNull(cachedRecord) && + isDefined(optimisticRecordWithConnection) && + isDefined(cachedRecordWithConnection); + + if (shouldHandleOptimisticCache) { + const recordGqlFields = generateDepthOneRecordGqlFields({ + objectMetadataItem, + record: optimisticRecordInput, + }); + updateRecordFromCache({ + objectMetadataItems, + objectMetadataItem, + cache: apolloClient.cache, + record: computedOptimisticRecord, + recordGqlFields, + }); + + triggerUpdateRecordOptimisticEffect({ + cache: apolloClient.cache, + objectMetadataItem, + currentRecord: cachedRecordWithConnection, + updatedRecord: optimisticRecordWithConnection, + objectMetadataItems, + }); } - updateRecordFromCache({ - objectMetadataItems, - objectMetadataItem, - cache: apolloClient.cache, - record: computedOptimisticRecord, - }); - - triggerUpdateRecordOptimisticEffect({ - cache: apolloClient.cache, - objectMetadataItem, - currentRecord: cachedRecordWithConnection, - updatedRecord: optimisticRecordWithConnection, - objectMetadataItems, - }); - const mutationResponseField = getUpdateOneRecordMutationResponseField(objectNameSingular); @@ -129,9 +139,7 @@ export const useUpdateOneRecord = < }, update: (cache, { data }) => { const record = data?.[mutationResponseField]; - - if (!isDefined(record) || !isDefined(computedOptimisticRecord)) - return; + if (!isDefined(record)) return; triggerUpdateRecordOptimisticEffect({ cache, @@ -143,14 +151,37 @@ export const useUpdateOneRecord = < }, }) .catch((error: Error) => { - if (isUndefinedOrNull(cachedRecord?.id)) { + if (!shouldHandleOptimisticCache) { throw error; } + const cachedRecordKeys = new Set(Object.keys(cachedRecord)); + const recordKeysAddedByOptimisticCache = Object.keys( + optimisticRecordInput, + ).filter((diffKey) => !cachedRecordKeys.has(diffKey)); + + const recordGqlFields = { + ...generateDepthOneRecordGqlFields({ + objectMetadataItem, + record: cachedRecord, + }), + ...buildRecordFromKeysWithSameValue( + recordKeysAddedByOptimisticCache, + true, + ), + }; + updateRecordFromCache({ objectMetadataItems, objectMetadataItem, cache: apolloClient.cache, - record: cachedRecord, + record: { + ...cachedRecord, + ...buildRecordFromKeysWithSameValue( + recordKeysAddedByOptimisticCache, + null, + ), + }, + recordGqlFields, }); triggerUpdateRecordOptimisticEffect({ 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 index afc1479ba..8e2b6f27a 100644 --- 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 @@ -1,36 +1,16 @@ +import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache'; +import { generateDepthOneRecordGqlFields } from '@/object-record/graphql/utils/generateDepthOneRecordGqlFields'; import { computeOptimisticRecordFromInput } from '@/object-record/utils/computeOptimisticRecordFromInput'; import { InMemoryCache } from '@apollo/client'; +import { getCompanyObjectMetadataItem } from '~/testing/mock-data/companies'; import { generatedMockObjectMetadataItems } from '~/testing/mock-data/generatedMockObjectMetadataItems'; - -const getPersonObjectMetadaItem = () => { - const personObjectMetadataItem = generatedMockObjectMetadataItems.find( - (item) => item.nameSingular === 'person', - ); - - if (!personObjectMetadataItem) { - throw new Error('Person object metadata item not found'); - } - - return personObjectMetadataItem; -}; - -const getCompanyObjectMetadataItem = () => { - const companyObjectMetadataItem = generatedMockObjectMetadataItems.find( - (item) => item.nameSingular === 'company', - ); - - if (!companyObjectMetadataItem) { - throw new Error('Company object metadata item not found'); - } - - return companyObjectMetadataItem; -}; +import { getPersonObjectMetadataItem } from '~/testing/mock-data/people'; describe('computeOptimisticRecordFromInput', () => { it('should generate correct optimistic record if no relation field is present', () => { const cache = new InMemoryCache(); - const personObjectMetadataItem = getPersonObjectMetadaItem(); + const personObjectMetadataItem = getPersonObjectMetadataItem(); const result = computeOptimisticRecordFromInput({ objectMetadataItems: generatedMockObjectMetadataItems, @@ -48,7 +28,7 @@ describe('computeOptimisticRecordFromInput', () => { it('should generate correct optimistic record if relation field is present but cache is empty', () => { const cache = new InMemoryCache(); - const personObjectMetadataItem = getPersonObjectMetadaItem(); + const personObjectMetadataItem = getPersonObjectMetadataItem(); const result = computeOptimisticRecordFromInput({ objectMetadataItems: generatedMockObjectMetadataItems, @@ -66,7 +46,7 @@ describe('computeOptimisticRecordFromInput', () => { it('should generate correct optimistic record even if recordInput contains field __typename', () => { const cache = new InMemoryCache(); - const personObjectMetadataItem = getPersonObjectMetadaItem(); + const personObjectMetadataItem = getPersonObjectMetadataItem(); const companyObjectMetadataItem = getCompanyObjectMetadataItem(); const companyRecord = { @@ -74,16 +54,22 @@ describe('computeOptimisticRecordFromInput', () => { __typename: 'Company', }; + const objectMetadataItem: ObjectMetadataItem = { + ...companyObjectMetadataItem, + fields: companyObjectMetadataItem.fields.filter( + (field) => field.name === 'id', + ), + }; + const recordGqlFields = generateDepthOneRecordGqlFields({ + objectMetadataItem, + record: companyRecord, + }); updateRecordFromCache({ objectMetadataItems: generatedMockObjectMetadataItems, - objectMetadataItem: { - ...companyObjectMetadataItem, - fields: companyObjectMetadataItem.fields.filter( - (field) => field.name === 'id', - ), - }, + objectMetadataItem, cache, record: companyRecord, + recordGqlFields, }); const result = computeOptimisticRecordFromInput({ @@ -104,7 +90,7 @@ describe('computeOptimisticRecordFromInput', () => { it('should generate correct optimistic record if relation field is present and cache is not empty', () => { const cache = new InMemoryCache(); - const personObjectMetadataItem = getPersonObjectMetadaItem(); + const personObjectMetadataItem = getPersonObjectMetadataItem(); const companyObjectMetadataItem = getCompanyObjectMetadataItem(); const companyRecord = { @@ -112,16 +98,22 @@ describe('computeOptimisticRecordFromInput', () => { __typename: 'Company', }; + const objectMetadataItem: ObjectMetadataItem = { + ...companyObjectMetadataItem, + fields: companyObjectMetadataItem.fields.filter( + (field) => field.name === 'id', + ), + }; + const recordGqlFields = generateDepthOneRecordGqlFields({ + objectMetadataItem, + record: companyRecord, + }); updateRecordFromCache({ objectMetadataItems: generatedMockObjectMetadataItems, - objectMetadataItem: { - ...companyObjectMetadataItem, - fields: companyObjectMetadataItem.fields.filter( - (field) => field.name === 'id', - ), - }, + objectMetadataItem, cache, record: companyRecord, + recordGqlFields, }); const result = computeOptimisticRecordFromInput({ @@ -141,7 +133,7 @@ describe('computeOptimisticRecordFromInput', () => { it('should generate correct optimistic record if relation field is null and cache is empty', () => { const cache = new InMemoryCache(); - const personObjectMetadataItem = getPersonObjectMetadaItem(); + const personObjectMetadataItem = getPersonObjectMetadataItem(); const result = computeOptimisticRecordFromInput({ objectMetadataItems: generatedMockObjectMetadataItems, @@ -160,7 +152,7 @@ describe('computeOptimisticRecordFromInput', () => { it('should throw an error if recordInput contains fields unrelated to the current objectMetadata', () => { const cache = new InMemoryCache(); - const personObjectMetadataItem = getPersonObjectMetadaItem(); + const personObjectMetadataItem = getPersonObjectMetadataItem(); expect(() => computeOptimisticRecordFromInput({ @@ -181,7 +173,7 @@ describe('computeOptimisticRecordFromInput', () => { it('should throw an error if recordInput contains both the relationFieldId and relationField', () => { const cache = new InMemoryCache(); - const personObjectMetadataItem = getPersonObjectMetadaItem(); + const personObjectMetadataItem = getPersonObjectMetadataItem(); expect(() => computeOptimisticRecordFromInput({ @@ -200,7 +192,7 @@ describe('computeOptimisticRecordFromInput', () => { it('should throw an error if recordInput contains both the relationFieldId and relationField even if null', () => { const cache = new InMemoryCache(); - const personObjectMetadataItem = getPersonObjectMetadaItem(); + const personObjectMetadataItem = getPersonObjectMetadataItem(); expect(() => computeOptimisticRecordFromInput({ diff --git a/packages/twenty-front/src/modules/object-record/utils/generateEmptyFieldValue.ts b/packages/twenty-front/src/modules/object-record/utils/generateEmptyFieldValue.ts index b2829d3f1..57b01306f 100644 --- a/packages/twenty-front/src/modules/object-record/utils/generateEmptyFieldValue.ts +++ b/packages/twenty-front/src/modules/object-record/utils/generateEmptyFieldValue.ts @@ -1,4 +1,5 @@ import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem'; +import { assertUnreachable } from '@/workflow/utils/assertUnreachable'; import { FieldMetadataType, RelationDefinitionType, @@ -106,8 +107,14 @@ export const generateEmptyFieldValue = ( additionalPhones: null, }; } + case FieldMetadataType.TS_VECTOR: { + throw new Error('TS_VECTOR not implemented yet'); + } default: { - throw new Error('Unhandled FieldMetadataType'); + return assertUnreachable( + fieldMetadataItem.type, + 'Unhandled FieldMetadataType', + ); } } }; diff --git a/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFieldRecords.ts b/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFieldRecords.ts index 289911241..f170bb621 100644 --- a/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFieldRecords.ts +++ b/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFieldRecords.ts @@ -12,6 +12,8 @@ import { useCreateOneRecordMutation } from '@/object-record/hooks/useCreateOneRe import { useUpdateOneRecordMutation } from '@/object-record/hooks/useUpdateOneRecordMutation'; import { GraphQLView } from '@/views/types/GraphQLView'; import { ViewField } from '@/views/types/ViewField'; +import { isNull } from '@sniptt/guards'; +import { isDefined } from 'twenty-shared'; export const usePersistViewFieldRecords = () => { const { objectMetadataItem } = useObjectMetadataItem({ @@ -93,10 +95,13 @@ export const usePersistViewFieldRecords = () => { }, update: (cache, { data }) => { const record = data?.['updateViewField']; - if (!record) return; - const cachedRecord = getRecordFromCache(record.id); + if (!isDefined(record)) return; - if (!cachedRecord) return; + const cachedRecord = getRecordFromCache( + record.id, + cache, + ); + if (isNull(cachedRecord)) return; triggerUpdateRecordOptimisticEffect({ cache, diff --git a/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFilterGroupRecords.ts b/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFilterGroupRecords.ts index edad40cff..bebb1ee53 100644 --- a/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFilterGroupRecords.ts +++ b/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFilterGroupRecords.ts @@ -58,7 +58,7 @@ export const usePersistViewFilterGroupRecords = () => { }, update: (cache, { data }) => { const record = data?.createViewFilterGroup; - if (!record) return; + if (!isDefined(record)) return; triggerCreateRecordsOptimisticEffect({ cache, @@ -140,12 +140,13 @@ export const usePersistViewFilterGroupRecords = () => { }, update: (cache, { data }) => { const record = data?.updateViewFilterGroup; - if (!record) return; + if (!isDefined(record)) return; + const cachedRecord = getRecordFromCache( record.id, + cache, ); - - if (!cachedRecord) return; + if (!isDefined(cachedRecord)) return; triggerUpdateRecordOptimisticEffect({ cache, @@ -180,12 +181,10 @@ export const usePersistViewFilterGroupRecords = () => { }, update: (cache, { data }) => { const record = data?.destroyViewFilterGroup; - - if (!record) return; + if (!isDefined(record)) return; const cachedRecord = getRecordFromCache(record.id, cache); - - if (!cachedRecord) return; + if (!isDefined(cachedRecord)) return; triggerDestroyRecordsOptimisticEffect({ cache, diff --git a/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFilterRecords.ts b/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFilterRecords.ts index cfd829785..03c915204 100644 --- a/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFilterRecords.ts +++ b/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFilterRecords.ts @@ -13,6 +13,7 @@ import { useDestroyOneRecordMutation } from '@/object-record/hooks/useDestroyOne import { useUpdateOneRecordMutation } from '@/object-record/hooks/useUpdateOneRecordMutation'; import { GraphQLView } from '@/views/types/GraphQLView'; import { ViewFilter } from '@/views/types/ViewFilter'; +import { isDefined } from 'twenty-shared'; import { v4 } from 'uuid'; export const usePersistViewFilterRecords = () => { @@ -42,7 +43,7 @@ export const usePersistViewFilterRecords = () => { const createViewFilterRecords = useCallback( (viewFiltersToCreate: ViewFilter[], view: GraphQLView) => { - if (!viewFiltersToCreate.length) return; + if (viewFiltersToCreate.length === 0) return; return Promise.all( viewFiltersToCreate.map((viewFilter) => @@ -61,7 +62,7 @@ export const usePersistViewFilterRecords = () => { }, update: (cache, { data }) => { const record = data?.['createViewFilter']; - if (!record) return; + if (!isDefined(record)) return; triggerCreateRecordsOptimisticEffect({ cache, @@ -99,10 +100,13 @@ export const usePersistViewFilterRecords = () => { }, update: (cache, { data }) => { const record = data?.['updateViewFilter']; - if (!record) return; - const cachedRecord = getRecordFromCache(record.id); + if (!isDefined(record)) return; - if (!cachedRecord) return; + const cachedRecord = getRecordFromCache( + record.id, + cache, + ); + if (!isDefined(cachedRecord)) return; triggerUpdateRecordOptimisticEffect({ cache, @@ -137,12 +141,13 @@ export const usePersistViewFilterRecords = () => { }, update: (cache, { data }) => { const record = data?.['destroyViewFilter']; + if (!isDefined(record)) return; - if (!record) return; - - const cachedRecord = getRecordFromCache(record.id, cache); - - if (!cachedRecord) return; + const cachedRecord = getRecordFromCache( + record.id, + cache, + ); + if (!isDefined(cachedRecord)) return; triggerDestroyRecordsOptimisticEffect({ cache, diff --git a/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewSortRecords.ts b/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewSortRecords.ts index 26a8987f2..7ef3948a8 100644 --- a/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewSortRecords.ts +++ b/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewSortRecords.ts @@ -13,6 +13,7 @@ import { useDestroyOneRecordMutation } from '@/object-record/hooks/useDestroyOne import { useUpdateOneRecordMutation } from '@/object-record/hooks/useUpdateOneRecordMutation'; import { GraphQLView } from '@/views/types/GraphQLView'; import { ViewSort } from '@/views/types/ViewSort'; +import { isDefined } from 'twenty-shared'; export const usePersistViewSortRecords = () => { const { objectMetadataItem } = useObjectMetadataItem({ @@ -55,7 +56,7 @@ export const usePersistViewSortRecords = () => { }, update: (cache, { data }) => { const record = data?.['createViewSort']; - if (!record) return; + if (!isDefined(record)) return; triggerCreateRecordsOptimisticEffect({ cache, @@ -91,10 +92,13 @@ export const usePersistViewSortRecords = () => { }, update: (cache, { data }) => { const record = data?.['updateViewSort']; - if (!record) return; - const cachedRecord = getRecordFromCache(record.id); + if (!isDefined(record)) return; - if (!cachedRecord) return; + const cachedRecord = getRecordFromCache( + record.id, + cache, + ); + if (!isDefined(cachedRecord)) return; triggerUpdateRecordOptimisticEffect({ cache, @@ -129,12 +133,13 @@ export const usePersistViewSortRecords = () => { }, update: (cache, { data }) => { const record = data?.['destroyViewSort']; + if (!isDefined(record)) return; - if (!record) return; - - const cachedRecord = getRecordFromCache(record.id, cache); - - if (!cachedRecord) return; + const cachedRecord = getRecordFromCache( + record.id, + cache, + ); + if (!isDefined(cachedRecord)) return; triggerDestroyRecordsOptimisticEffect({ cache, diff --git a/packages/twenty-front/src/modules/workflow/hooks/useDeleteOneWorkflowVersion.ts b/packages/twenty-front/src/modules/workflow/hooks/useDeleteOneWorkflowVersion.ts index 3a539bff9..48dbf7bf8 100644 --- a/packages/twenty-front/src/modules/workflow/hooks/useDeleteOneWorkflowVersion.ts +++ b/packages/twenty-front/src/modules/workflow/hooks/useDeleteOneWorkflowVersion.ts @@ -1,8 +1,9 @@ import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; -import { useDeleteOneRecord } from '@/object-record/hooks/useDeleteOneRecord'; import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordFromCache'; +import { useDeleteOneRecord } from '@/object-record/hooks/useDeleteOneRecord'; import { Workflow, WorkflowVersion } from '@/workflow/types/Workflow'; import { useApolloClient } from '@apollo/client'; +import { isDefined } from 'twenty-shared'; export const useDeleteOneWorkflowVersion = () => { const apolloClient = useApolloClient(); @@ -26,7 +27,7 @@ export const useDeleteOneWorkflowVersion = () => { const cachedWorkflowVersion = getWorkflowVersionFromCache(workflowVersionId); - if (!cachedWorkflowVersion) { + if (!isDefined(cachedWorkflowVersion)) { return; } @@ -34,7 +35,7 @@ export const useDeleteOneWorkflowVersion = () => { cachedWorkflowVersion.workflowId, ); - if (!cachedWorkflow) { + if (!isDefined(cachedWorkflow)) { return; } diff --git a/packages/twenty-front/src/modules/workflow/hooks/useDeleteWorkflowVersionStep.ts b/packages/twenty-front/src/modules/workflow/hooks/useDeleteWorkflowVersionStep.ts index 8ec72a7f6..be051607b 100644 --- a/packages/twenty-front/src/modules/workflow/hooks/useDeleteWorkflowVersionStep.ts +++ b/packages/twenty-front/src/modules/workflow/hooks/useDeleteWorkflowVersionStep.ts @@ -41,7 +41,7 @@ export const useDeleteWorkflowVersionStep = () => { const cachedRecord = getRecordFromCache( input.workflowVersionId, ); - if (!cachedRecord) { + if (!isDefined(cachedRecord)) { return; } @@ -52,11 +52,15 @@ export const useDeleteWorkflowVersionStep = () => { ), }; + const recordGqlFields = { + steps: true, + }; updateRecordFromCache({ objectMetadataItems, objectMetadataItem, cache: apolloClient.cache, record: newCachedRecord, + recordGqlFields, }); }; diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/hooks/useCreateWorkflowVersionStep.ts b/packages/twenty-front/src/modules/workflow/workflow-steps/hooks/useCreateWorkflowVersionStep.ts index ffa71ff8c..c921b018a 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/hooks/useCreateWorkflowVersionStep.ts +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/hooks/useCreateWorkflowVersionStep.ts @@ -42,7 +42,7 @@ export const useCreateWorkflowVersionStep = () => { const cachedRecord = getRecordFromCache( input.workflowVersionId, ); - if (!cachedRecord) { + if (!isDefined(cachedRecord)) { return; } @@ -51,11 +51,15 @@ export const useCreateWorkflowVersionStep = () => { steps: [...(cachedRecord.steps || []), createdStep], }; + const recordGqlFields = { + steps: true, + }; updateRecordFromCache({ objectMetadataItems, objectMetadataItem, cache: apolloClient.cache, record: newCachedRecord, + recordGqlFields, }); return result; }; diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/hooks/useUpdateWorkflowVersionStep.ts b/packages/twenty-front/src/modules/workflow/workflow-steps/hooks/useUpdateWorkflowVersionStep.ts index 4a2e48dc9..f93cb59d9 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/hooks/useUpdateWorkflowVersionStep.ts +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/hooks/useUpdateWorkflowVersionStep.ts @@ -42,7 +42,7 @@ export const useUpdateWorkflowVersionStep = () => { const cachedRecord = getRecordFromCache( input.workflowVersionId, ); - if (!cachedRecord) { + if (!isDefined(cachedRecord)) { return; } @@ -56,11 +56,15 @@ export const useUpdateWorkflowVersionStep = () => { }), }; + const recordGqlFields = { + steps: true, + }; updateRecordFromCache({ objectMetadataItems, objectMetadataItem, cache: apolloClient.cache, record: newCachedRecord, + recordGqlFields, }); return result; }; diff --git a/packages/twenty-front/src/testing/jest/getJestMetadataAndApolloMocksWrapper.tsx b/packages/twenty-front/src/testing/jest/getJestMetadataAndApolloMocksWrapper.tsx index c38c486b8..4a838c24b 100644 --- a/packages/twenty-front/src/testing/jest/getJestMetadataAndApolloMocksWrapper.tsx +++ b/packages/twenty-front/src/testing/jest/getJestMetadataAndApolloMocksWrapper.tsx @@ -6,13 +6,16 @@ import { RecordFiltersComponentInstanceContext } from '@/object-record/record-fi import { RecordIndexContextProvider } from '@/object-record/record-index/contexts/RecordIndexContext'; import { SnackBarProviderScope } from '@/ui/feedback/snack-bar-manager/scopes/SnackBarProviderScope'; import { ViewComponentInstanceContext } from '@/views/states/contexts/ViewComponentInstanceContext'; +import { InMemoryCache } from '@apollo/client'; import { JestObjectMetadataItemSetter } from '~/testing/jest/JestObjectMetadataItemSetter'; import { generatedMockObjectMetadataItems } from '~/testing/mock-data/generatedMockObjectMetadataItems'; export const getJestMetadataAndApolloMocksWrapper = ({ apolloMocks, + cache, onInitializeRecoilSnapshot, }: { + cache?: InMemoryCache; apolloMocks?: | readonly MockedResponse, Record>[] | undefined; @@ -21,7 +24,7 @@ export const getJestMetadataAndApolloMocksWrapper = ({ return ({ children }: { children: ReactNode }) => ( - + 'indexIdentifierUrl', diff --git a/packages/twenty-front/src/testing/mock-data/companies.ts b/packages/twenty-front/src/testing/mock-data/companies.ts index e4dafe802..bb56a3ff6 100644 --- a/packages/twenty-front/src/testing/mock-data/companies.ts +++ b/packages/twenty-front/src/testing/mock-data/companies.ts @@ -1,7 +1,20 @@ +import { generatedMockObjectMetadataItems } from '~/testing/mock-data/generatedMockObjectMetadataItems'; + export const getCompaniesMock = () => { return companiesQueryResult.companies.edges.map((edge) => edge.node); }; +export const getCompanyObjectMetadataItem = () => { + const companyObjectMetadataItem = generatedMockObjectMetadataItems.find( + (item) => item.nameSingular === 'company', + ); + + if (!companyObjectMetadataItem) { + throw new Error('Company object metadata item not found'); + } + + return companyObjectMetadataItem; +}; export const getCompanyDuplicateMock = () => { return { ...companiesQueryResult.companies.edges[0].node, diff --git a/packages/twenty-front/src/testing/mock-data/people.ts b/packages/twenty-front/src/testing/mock-data/people.ts index 1dc939206..dd81c3603 100644 --- a/packages/twenty-front/src/testing/mock-data/people.ts +++ b/packages/twenty-front/src/testing/mock-data/people.ts @@ -1,11 +1,36 @@ import { RecordGqlConnection } from '@/object-record/graphql/types/RecordGqlConnection'; +import { ObjectRecord } from '@/object-record/types/ObjectRecord'; +import { generatedMockObjectMetadataItems } from '~/testing/mock-data/generatedMockObjectMetadataItems'; -export const getPeopleMock = () => { +export const getPeopleMock = (): ObjectRecord[] => { const peopleMock = peopleQueryResult.people.edges.map((edge) => edge.node); return peopleMock; }; +export const getPersonObjectMetadataItem = () => { + const personObjectMetadataItem = generatedMockObjectMetadataItems.find( + (item) => item.nameSingular === 'person', + ); + + if (!personObjectMetadataItem) { + throw new Error('Person object metadata item not found'); + } + + return personObjectMetadataItem; +}; + +export const getPersonRecord = ( + overrides?: Partial, + index = 0, +) => { + const personRecords = getPeopleMock(); + return { + ...personRecords[index], + ...overrides, + }; +}; + export const mockedEmptyPersonData = { id: 'ce7f0a37-88d7-4cd8-8b41-6721c57195b5', firstName: '', diff --git a/packages/twenty-front/src/utils/array/__tests__/buildRecordFromKeysWithSameValue.test.ts b/packages/twenty-front/src/utils/array/__tests__/buildRecordFromKeysWithSameValue.test.ts new file mode 100644 index 000000000..fa682dd6d --- /dev/null +++ b/packages/twenty-front/src/utils/array/__tests__/buildRecordFromKeysWithSameValue.test.ts @@ -0,0 +1,23 @@ +import { buildRecordFromKeysWithSameValue } from '~/utils/array/buildRecordFromKeysWithSameValue'; + +describe('buildRecordFromKeysWithSameValue', () => { + test.each([ + { array: [], expected: {}, arg: undefined }, + { + array: ['foo', 'bar'], + expected: { foo: 'oui', bar: 'oui' }, + arg: 'oui', + }, + { + array: ['foo', 'bar'] as const, + expected: { foo: true, bar: true }, + arg: true, + }, + ])( + '.buildRecordFromKeysWithSameValue($array, $arg)', + ({ array, arg, expected }) => { + const result = buildRecordFromKeysWithSameValue(array, arg); + expect(result).toEqual(expected); + }, + ); +}); diff --git a/packages/twenty-front/src/utils/array/buildRecordFromKeysWithSameValue.ts b/packages/twenty-front/src/utils/array/buildRecordFromKeysWithSameValue.ts new file mode 100644 index 000000000..4bf6d05f4 --- /dev/null +++ b/packages/twenty-front/src/utils/array/buildRecordFromKeysWithSameValue.ts @@ -0,0 +1,10 @@ +// const tmp = ['foo', 'bar'] as const; +// const result = recordFromArrayWithValue(tmp, true); +// returns { foo: true, bar: true } +// result has strictly typed keys foo and bar + +export const buildRecordFromKeysWithSameValue = ( + array: string[] | readonly U[], + value: T, +): Record => + Object.fromEntries(array.map((key) => [key, value])) as Record;