From cd4800eb86ddd60d4361f8b0c984c44f644e5679 Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Sat, 4 Oct 2025 20:19:26 +0200 Subject: [PATCH] Fix exception handling on messaging + optimistic on object delete (#14894) ## What In this PR, we are fixing two issues: - while deleting an object, the views are not properly refreshed leading to FE bug. This is due to the fact that we were refetching **views associated to the object** which has been deleted. As views are properly deleted in the BE, the FE gets no views from this call and cannot optimistically react. In this case, I'm triggering a complete view refetch - messaging error handling had a hole --- .../useDeleteOneObjectMetadataItem.test.tsx | 15 ++++++++++++++- .../hooks/useDeleteOneObjectMetadataItem.ts | 7 +++---- .../views/hooks/useRefreshAllCoreViews.ts | 8 ++++---- ...messaging-import-exception-handler.service.ts | 16 +++++++++++----- .../messaging-message-list-fetch.service.ts | 2 +- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/packages/twenty-front/src/modules/object-metadata/hooks/__tests__/useDeleteOneObjectMetadataItem.test.tsx b/packages/twenty-front/src/modules/object-metadata/hooks/__tests__/useDeleteOneObjectMetadataItem.test.tsx index f3144b58dd..5665d95c22 100644 --- a/packages/twenty-front/src/modules/object-metadata/hooks/__tests__/useDeleteOneObjectMetadataItem.test.tsx +++ b/packages/twenty-front/src/modules/object-metadata/hooks/__tests__/useDeleteOneObjectMetadataItem.test.tsx @@ -12,11 +12,13 @@ import { } from '../__mocks__/useDeleteOneObjectMetadataItem'; import { GET_CURRENT_USER } from '@/users/graphql/queries/getCurrentUser'; +import { FIND_ALL_CORE_VIEWS } from '@/views/graphql/queries/findAllCoreViews'; +import { mockedUserData } from '~/testing/mock-data/users'; +import { mockedCoreViewsData } from '~/testing/mock-data/views'; import { query as findManyObjectMetadataItemsQuery, responseData as findManyObjectMetadataItemsResponseData, } from '../__mocks__/useFindManyObjectMetadataItems'; -import { mockedUserData } from '~/testing/mock-data/users'; const mocks = [ { @@ -41,6 +43,17 @@ const mocks = [ }, })), }, + { + request: { + query: FIND_ALL_CORE_VIEWS, + variables: {}, + }, + result: jest.fn(() => ({ + data: { + getCoreViews: mockedCoreViewsData, + }, + })), + }, { request: { query: findManyObjectMetadataItemsQuery, diff --git a/packages/twenty-front/src/modules/object-metadata/hooks/useDeleteOneObjectMetadataItem.ts b/packages/twenty-front/src/modules/object-metadata/hooks/useDeleteOneObjectMetadataItem.ts index 3fbc174ac6..770b71fdef 100644 --- a/packages/twenty-front/src/modules/object-metadata/hooks/useDeleteOneObjectMetadataItem.ts +++ b/packages/twenty-front/src/modules/object-metadata/hooks/useDeleteOneObjectMetadataItem.ts @@ -8,7 +8,7 @@ import { import { DELETE_ONE_OBJECT_METADATA_ITEM } from '../graphql/mutations'; import { useRefreshObjectMetadataItems } from '@/object-metadata/hooks/useRefreshObjectMetadataItems'; -import { useRefreshCoreViewsByObjectMetadataId } from '@/views/hooks/useRefreshCoreViewsByObjectMetadataId'; +import { useRefreshAllCoreViews } from '@/views/hooks/useRefreshAllCoreViews'; export const useDeleteOneObjectMetadataItem = () => { const [mutate] = useMutation< @@ -19,8 +19,7 @@ export const useDeleteOneObjectMetadataItem = () => { const { refreshObjectMetadataItems } = useRefreshObjectMetadataItems('network-only'); - const { refreshCoreViewsByObjectMetadataId } = - useRefreshCoreViewsByObjectMetadataId(); + const { refreshAllCoreViews } = useRefreshAllCoreViews(); const deleteOneObjectMetadataItem = async ( idToDelete: DeleteOneObjectMetadataItemMutationVariables['idToDelete'], @@ -32,7 +31,7 @@ export const useDeleteOneObjectMetadataItem = () => { }); await refreshObjectMetadataItems(); - await refreshCoreViewsByObjectMetadataId(idToDelete); + await refreshAllCoreViews(); return result; }; diff --git a/packages/twenty-front/src/modules/views/hooks/useRefreshAllCoreViews.ts b/packages/twenty-front/src/modules/views/hooks/useRefreshAllCoreViews.ts index 2cbc5715ec..1a690360b8 100644 --- a/packages/twenty-front/src/modules/views/hooks/useRefreshAllCoreViews.ts +++ b/packages/twenty-front/src/modules/views/hooks/useRefreshAllCoreViews.ts @@ -1,10 +1,10 @@ +import { FIND_ALL_CORE_VIEWS } from '@/views/graphql/queries/findAllCoreViews'; import { coreViewsState } from '@/views/states/coreViewState'; import { type FetchPolicy, useApolloClient } from '@apollo/client'; import { useRecoilCallback } from 'recoil'; import { isDefined } from 'twenty-shared/utils'; -import { type FindManyCoreViewsQuery } from '~/generated/graphql'; +import { type FindAllCoreViewsQuery } from '~/generated/graphql'; import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; -import { FIND_MANY_CORE_VIEWS } from '../graphql/queries/findManyCoreViews'; export const useRefreshAllCoreViews = ( fetchPolicy: FetchPolicy = 'network-only', @@ -14,8 +14,8 @@ export const useRefreshAllCoreViews = ( const refreshAllCoreViews = useRecoilCallback( ({ snapshot, set }) => async () => { - const result = await client.query({ - query: FIND_MANY_CORE_VIEWS, + const result = await client.query({ + query: FIND_ALL_CORE_VIEWS, variables: {}, fetchPolicy, }); diff --git a/packages/twenty-server/src/modules/messaging/message-import-manager/services/messaging-import-exception-handler.service.ts b/packages/twenty-server/src/modules/messaging/message-import-manager/services/messaging-import-exception-handler.service.ts index 2ef3c9180d..736ed317ef 100644 --- a/packages/twenty-server/src/modules/messaging/message-import-manager/services/messaging-import-exception-handler.service.ts +++ b/packages/twenty-server/src/modules/messaging/message-import-manager/services/messaging-import-exception-handler.service.ts @@ -15,7 +15,7 @@ import { } from 'src/modules/messaging/common/standard-objects/message-channel.workspace-entity'; import { MESSAGING_THROTTLE_MAX_ATTEMPTS } from 'src/modules/messaging/message-import-manager/constants/messaging-throttle-max-attempts'; import { - type MessageImportDriverException, + MessageImportDriverException, MessageImportDriverExceptionCode, } from 'src/modules/messaging/message-import-manager/drivers/exceptions/message-import-driver.exception'; import { MessageNetworkExceptionCode } from 'src/modules/messaging/message-import-manager/drivers/exceptions/message-network.exception'; @@ -25,7 +25,6 @@ import { } from 'src/modules/messaging/message-import-manager/exceptions/message-import.exception'; export enum MessageImportSyncStep { - FULL_MESSAGE_LIST_FETCH = 'FULL_MESSAGE_LIST_FETCH', // TODO: deprecate to only use MESSAGE_LIST_FETCH MESSAGE_LIST_FETCH = 'MESSAGE_LIST_FETCH', MESSAGES_IMPORT_PENDING = 'MESSAGES_IMPORT_PENDING', MESSAGES_IMPORT_ONGOING = 'MESSAGES_IMPORT_ONGOING', @@ -150,7 +149,7 @@ export class MessageImportExceptionHandlerService { ); switch (syncStep) { - case MessageImportSyncStep.FULL_MESSAGE_LIST_FETCH: + case MessageImportSyncStep.MESSAGE_LIST_FETCH: await this.messageChannelSyncStatusService.scheduleMessageListFetch([ messageChannel.id, ]); @@ -230,8 +229,15 @@ export class MessageImportExceptionHandlerService { messageChannel: Pick, workspaceId: string, ): Promise { - if (syncStep === MessageImportSyncStep.FULL_MESSAGE_LIST_FETCH) { - return; + if (syncStep === MessageImportSyncStep.MESSAGE_LIST_FETCH) { + await this.handleUnknownException( + new MessageImportDriverException( + 'Not Found exception occurred while fetching message list, which should never happen', + MessageImportDriverExceptionCode.UNKNOWN, + ), + messageChannel, + workspaceId, + ); } await this.messageChannelSyncStatusService.resetAndScheduleMessageListFetch( diff --git a/packages/twenty-server/src/modules/messaging/message-import-manager/services/messaging-message-list-fetch.service.ts b/packages/twenty-server/src/modules/messaging/message-import-manager/services/messaging-message-list-fetch.service.ts index 75958a4a25..8909124b1a 100644 --- a/packages/twenty-server/src/modules/messaging/message-import-manager/services/messaging-message-list-fetch.service.ts +++ b/packages/twenty-server/src/modules/messaging/message-import-manager/services/messaging-message-list-fetch.service.ts @@ -257,7 +257,7 @@ export class MessagingMessageListFetchService { } catch (error) { await this.messageImportErrorHandlerService.handleDriverException( error, - MessageImportSyncStep.FULL_MESSAGE_LIST_FETCH, + MessageImportSyncStep.MESSAGE_LIST_FETCH, messageChannel, workspaceId, );