From 62a58816e326f72647faa85a79d092021ac28348 Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Wed, 19 Mar 2025 11:13:14 +0100 Subject: [PATCH] Fix view groups update after field metadata update (#10995) This PR fixes a difficult to reproduce bug, where a race condition appears if we go back to a table with view groups before the update field metadata logic finishes its work. In order to reproduce this bug on localhost, you'll have to simulate a slow network in your browser. The problem was that the view groups are initialized only once by useLoadRecordIndexStates, in an effect component : RecordIndexLoadBaseOnContextStoreEffect. And that this component as an internal state loadedViewId, which prevents subsequent updates of view groups by useLoadRecordIndexStates, because it considers that loading already happened, even if it's actually stale data. So instead of creating other states to burden the effect component with complex state management, the solution was to add the only needed update in a synchronous way directly in updateOneFieldMetadataItem logic. This way we are sure that our boards and tables with view groups get updated eventually, for each field metadata update, even if the requests take time. Fixes https://github.com/twentyhq/twenty/issues/10947 Fixes https://github.com/twentyhq/twenty/issues/10944 --- .../hooks/useUpdateOneFieldMetadataItem.ts | 32 +++++++++++++++-- .../hooks/useRecordGroupReorder.ts | 5 ++- .../record-group/hooks/useSetRecordGroups.ts | 34 +++++++++---------- .../hooks/useHandleRecordGroupField.ts | 10 +++++- .../hooks/useLoadRecordIndexStates.ts | 7 +++- 5 files changed, 66 insertions(+), 22 deletions(-) diff --git a/packages/twenty-front/src/modules/object-metadata/hooks/useUpdateOneFieldMetadataItem.ts b/packages/twenty-front/src/modules/object-metadata/hooks/useUpdateOneFieldMetadataItem.ts index 8d61920df..cc4ff32b1 100644 --- a/packages/twenty-front/src/modules/object-metadata/hooks/useUpdateOneFieldMetadataItem.ts +++ b/packages/twenty-front/src/modules/object-metadata/hooks/useUpdateOneFieldMetadataItem.ts @@ -13,6 +13,11 @@ import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSi import { useFindManyRecordsQuery } from '@/object-record/hooks/useFindManyRecordsQuery'; import { GET_CURRENT_USER } from '@/users/graphql/queries/getCurrentUser'; import { useSetRecoilState } from 'recoil'; + +import { getRecordsFromRecordConnection } from '@/object-record/cache/utils/getRecordsFromRecordConnection'; +import { RecordGqlConnection } from '@/object-record/graphql/types/RecordGqlConnection'; +import { useSetRecordGroups } from '@/object-record/record-group/hooks/useSetRecordGroups'; +import { isDefined } from 'twenty-shared'; import { useApolloMetadataClient } from './useApolloMetadataClient'; export const useUpdateOneFieldMetadataItem = () => { @@ -21,6 +26,8 @@ export const useUpdateOneFieldMetadataItem = () => { const { refreshObjectMetadataItems } = useRefreshObjectMetadataItems('network-only'); + const { setRecordGroupsFromViewGroups } = useSetRecordGroups(); + const setCurrentWorkspace = useSetRecoilState(currentWorkspaceState); const { findManyRecordsQuery: findManyViewsQuery } = useFindManyRecordsQuery({ @@ -70,12 +77,14 @@ export const useUpdateOneFieldMetadataItem = () => { }, }); - await refreshObjectMetadataItems(); + const objectMetadataItemsRefreshed = await refreshObjectMetadataItems(); const { data } = await apolloClient.query({ query: GET_CURRENT_USER }); setCurrentWorkspace(data?.currentUser?.currentWorkspace); - await apolloClient.query({ + const { data: viewConnection } = await apolloClient.query<{ + views: RecordGqlConnection; + }>({ query: findManyViewsQuery, variables: { filter: { @@ -87,6 +96,25 @@ export const useUpdateOneFieldMetadataItem = () => { fetchPolicy: 'network-only', }); + const viewRecords = getRecordsFromRecordConnection({ + recordConnection: viewConnection?.views, + }); + + for (const view of viewRecords) { + const correspondingObjectMetadataItemRefreshed = + objectMetadataItemsRefreshed?.find( + (item) => item.id === objectMetadataId, + ); + + if (isDefined(correspondingObjectMetadataItemRefreshed)) { + setRecordGroupsFromViewGroups( + view.id, + view.viewGroups, + correspondingObjectMetadataItemRefreshed, + ); + } + } + return result; }; diff --git a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorder.ts b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorder.ts index 20092c5b6..ba98c1b2b 100644 --- a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorder.ts +++ b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorder.ts @@ -1,5 +1,6 @@ import { OnDragEndResponder } from '@hello-pangea/dnd'; +import { useContextStoreObjectMetadataItemOrThrow } from '@/context-store/hooks/useContextStoreObjectMetadataItemOrThrow'; import { useSetRecordGroups } from '@/object-record/record-group/hooks/useSetRecordGroups'; import { recordGroupDefinitionFamilyState } from '@/object-record/record-group/states/recordGroupDefinitionFamilyState'; import { visibleRecordGroupIdsComponentFamilySelector } from '@/object-record/record-group/states/selectors/visibleRecordGroupIdsComponentFamilySelector'; @@ -24,6 +25,7 @@ export const useRecordGroupReorder = ({ viewType, }: UseRecordGroupHandlersParams) => { const { setRecordGroups } = useSetRecordGroups(); + const { objectMetadataItem } = useContextStoreObjectMetadataItemOrThrow(); const visibleRecordGroupIdsFamilySelector = useRecoilComponentCallbackStateV2( visibleRecordGroupIdsComponentFamilySelector, @@ -78,12 +80,13 @@ export const useRecordGroupReorder = ({ ]; }, []); - setRecordGroups(updatedRecordGroups, viewBarId); + setRecordGroups(updatedRecordGroups, viewBarId, objectMetadataItem.id); saveViewGroups( mapRecordGroupDefinitionsToViewGroups(updatedRecordGroups), ); }, [ + objectMetadataItem, saveViewGroups, setRecordGroups, viewBarId, diff --git a/packages/twenty-front/src/modules/object-record/record-group/hooks/useSetRecordGroups.ts b/packages/twenty-front/src/modules/object-record/record-group/hooks/useSetRecordGroups.ts index 1d7e87970..cc2a74300 100644 --- a/packages/twenty-front/src/modules/object-record/record-group/hooks/useSetRecordGroups.ts +++ b/packages/twenty-front/src/modules/object-record/record-group/hooks/useSetRecordGroups.ts @@ -1,7 +1,5 @@ -import { MAIN_CONTEXT_STORE_INSTANCE_ID } from '@/context-store/constants/MainContextStoreInstanceId'; -import { useContextStoreObjectMetadataItemOrThrow } from '@/context-store/hooks/useContextStoreObjectMetadataItemOrThrow'; -import { contextStoreCurrentObjectMetadataItemIdComponentState } from '@/context-store/states/contextStoreCurrentObjectMetadataItemIdComponentState'; import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; +import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; import { recordGroupDefinitionFamilyState } from '@/object-record/record-group/states/recordGroupDefinitionFamilyState'; import { recordGroupFieldMetadataComponentState } from '@/object-record/record-group/states/recordGroupFieldMetadataComponentState'; import { recordGroupIdsComponentState } from '@/object-record/record-group/states/recordGroupIdsComponentState'; @@ -16,19 +14,13 @@ import { isDefined } from 'twenty-shared'; import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; export const useSetRecordGroups = () => { - const { objectMetadataItem } = useContextStoreObjectMetadataItemOrThrow(); - const setRecordGroups = useRecoilCallback( ({ snapshot, set }) => - (recordGroups: RecordGroupDefinition[], recordIndexId: string) => { - const objectMetadataItemId = snapshot - .getLoadable( - contextStoreCurrentObjectMetadataItemIdComponentState.atomFamily({ - instanceId: MAIN_CONTEXT_STORE_INSTANCE_ID, - }), - ) - .getValue(); - + ( + recordGroups: RecordGroupDefinition[], + recordIndexId: string, + objectMetadataItemId: string, + ) => { const objectMetadataItems = snapshot .getLoadable(objectMetadataItemsState) .getValue(); @@ -113,7 +105,11 @@ export const useSetRecordGroups = () => { ); const setRecordGroupsFromViewGroups = useCallback( - (viewId: string, viewGroups: ViewGroup[]) => { + ( + viewId: string, + viewGroups: ViewGroup[], + objectMetadataItem: ObjectMetadataItem, + ) => { const recordIndexId = getRecordIndexIdFromObjectNamePluralAndViewId( objectMetadataItem.namePlural, viewId, @@ -124,9 +120,13 @@ export const useSetRecordGroups = () => { viewGroups, }); - setRecordGroups(newGroupDefinitions, recordIndexId); + setRecordGroups( + newGroupDefinitions, + recordIndexId, + objectMetadataItem.id, + ); }, - [objectMetadataItem, setRecordGroups], + [setRecordGroups], ); return { diff --git a/packages/twenty-front/src/modules/object-record/record-index/hooks/useHandleRecordGroupField.ts b/packages/twenty-front/src/modules/object-record/record-index/hooks/useHandleRecordGroupField.ts index fedc1b13c..990be0a6f 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/hooks/useHandleRecordGroupField.ts +++ b/packages/twenty-front/src/modules/object-record/record-index/hooks/useHandleRecordGroupField.ts @@ -1,3 +1,4 @@ +import { useContextStoreObjectMetadataItemOrThrow } from '@/context-store/hooks/useContextStoreObjectMetadataItemOrThrow'; import { contextStoreCurrentViewIdComponentState } from '@/context-store/states/contextStoreCurrentViewIdComponentState'; import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem'; import { useSetRecordGroups } from '@/object-record/record-group/hooks/useSetRecordGroups'; @@ -17,6 +18,8 @@ export const useHandleRecordGroupField = () => { contextStoreCurrentViewIdComponentState, ); + const { objectMetadataItem } = useContextStoreObjectMetadataItemOrThrow(); + const { getViewFromPrefetchState } = useGetViewFromPrefetchState(); const { setRecordGroupsFromViewGroups } = useSetRecordGroups(); @@ -96,7 +99,11 @@ export const useHandleRecordGroupField = () => { ...viewGroupsToCreate, ]; - setRecordGroupsFromViewGroups(view.id, newViewGroupsList); + setRecordGroupsFromViewGroups( + view.id, + newViewGroupsList, + objectMetadataItem, + ); if (viewGroupsToCreate.length > 0) { await createViewGroupRecords({ viewGroupsToCreate, viewId: view.id }); @@ -107,6 +114,7 @@ export const useHandleRecordGroupField = () => { } }, [ + objectMetadataItem, currentViewIdCallbackState, getViewFromPrefetchState, setRecordGroupsFromViewGroups, diff --git a/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexStates.ts b/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexStates.ts index d270bb78e..df8b3118f 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexStates.ts +++ b/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexStates.ts @@ -176,7 +176,12 @@ export const useLoadRecordIndexStates = () => { .getValue(); onViewFieldsChange(view.viewFields, objectMetadataItem, recordIndexId); - setRecordGroupsFromViewGroups(view.id, view.viewGroups); + + setRecordGroupsFromViewGroups( + view.id, + view.viewGroups, + objectMetadataItem, + ); setContextStoreTargetedRecordsRuleComponentState((prev) => ({ ...prev,