From a132a0e88bf7a016908a4ba691f9fe2ccac3dfaa Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Fri, 29 Aug 2025 20:01:12 +0200 Subject: [PATCH] Core view migration fixes (#14166) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugs found with feature flag IS_CORE_VIEW_ENABLED: - Switching a view type does switches but does not update the option dropdown selected menu item, we have to refresh ==> **fixed** - Reordering a field in the option dropdown causes a bug because it tries to update with a decimal position like 1.5 but the position field on core views is integer ==> **fixed** Hidding a field triggers the update, but making it visible again right after that does not trigger the DB update, we have to refresh the app ==> **not fixed, medium bug** Reordering core view groups in option dropdown fails ==> **fixed** Showing / hidding core view groups fails ==> **fixed** Creating a table group view works but if we select kanban right after it fails ==> **fixed** Move right / move left on kanban core view group doesn’t do anything ==> **fixed** Deleting a view from the view dropdown fails ==> **fixed** Creating a view from another view does not copy the view groups ==> **not fixed, medium bug** Move left / right has a strange behavior, not working consistently, it sends always the same position ==> **not fixed, medium bug** View re-order optimistic update is broken, but the DB update works ==> **now it's dancing not fixed, medium bug** Next steps: - we should re-work the optimistic behaviors of view updates but let's clean the code first --- .../useSetViewTypeFromLayoutOptionsMenu.ts | 23 +--------- .../hooks/useRecordGroupActions.ts | 4 +- .../useRecordGroupReorderConfirmationModal.ts | 2 +- .../hooks/useReorderRecordGroups.ts | 16 ++++--- .../internal/usePersistViewGroupRecords.ts | 2 +- .../src/modules/views/hooks/useDeleteView.ts | 46 +++++++++++++++++-- .../views/hooks/useUpdateCurrentView.ts | 17 +++++++ 7 files changed, 76 insertions(+), 34 deletions(-) diff --git a/packages/twenty-front/src/modules/object-record/object-options-dropdown/hooks/useSetViewTypeFromLayoutOptionsMenu.ts b/packages/twenty-front/src/modules/object-record/object-options-dropdown/hooks/useSetViewTypeFromLayoutOptionsMenu.ts index 90fb60a6c81..7269552b0ea 100644 --- a/packages/twenty-front/src/modules/object-record/object-options-dropdown/hooks/useSetViewTypeFromLayoutOptionsMenu.ts +++ b/packages/twenty-front/src/modules/object-record/object-options-dropdown/hooks/useSetViewTypeFromLayoutOptionsMenu.ts @@ -93,29 +93,10 @@ export const useSetViewTypeFromLayoutOptionsMenu = () => { if (availableFieldsForKanban.length === 0) { throw new Error('No fields for kanban - should not happen'); } - const previouslySelectedKanbanField = availableFieldsForKanban.find( - (fieldsForKanban) => - fieldsForKanban.id === - currentView.viewGroups[0].fieldMetadataId, - ); - const kanbanField = isDefined(previouslySelectedKanbanField) - ? previouslySelectedKanbanField - : availableFieldsForKanban[0]; - - if (!isDefined(previouslySelectedKanbanField)) { - updateCurrentViewParams.kanbanFieldMetadataId = - currentView.viewGroups[0].fieldMetadataId; - } - - const hasViewGroups = currentView.viewGroups.some( - (viewGroup: ViewGroup) => - viewGroup.fieldMetadataId === kanbanField.id, - ); - - if (!hasViewGroups) { + if (currentView.viewGroups.length === 0) { const viewGroups = await createViewGroupAssociatedWithKanbanField( - kanbanField.id, + availableFieldsForKanban[0].id, currentView.id, ); loadRecordIndexStates( diff --git a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupActions.ts b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupActions.ts index 8bc6b1cd27e..7d4c5613e41 100644 --- a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupActions.ts +++ b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupActions.ts @@ -6,6 +6,7 @@ import { recordGroupFieldMetadataComponentState } from '@/object-record/record-g import { visibleRecordGroupIdsComponentFamilySelector } from '@/object-record/record-group/states/selectors/visibleRecordGroupIdsComponentFamilySelector'; import { type RecordGroupAction } from '@/object-record/record-group/types/RecordGroupActions'; import { useRecordIndexContextOrThrow } from '@/object-record/record-index/contexts/RecordIndexContext'; +import { useRecordIndexIdFromCurrentContextStore } from '@/object-record/record-index/hooks/useRecordIndexIdFromCurrentContextStore'; import { useHasPermissionFlag } from '@/settings/roles/hooks/useHasPermissionFlag'; import { SettingsPath } from '@/types/SettingsPath'; import { navigationMemorizedUrlState } from '@/ui/navigation/states/navigationMemorizedUrlState'; @@ -34,6 +35,7 @@ type UseRecordGroupActionsParams = { export const useRecordGroupActions = ({ viewType, }: UseRecordGroupActionsParams) => { + const { recordIndexId } = useRecordIndexIdFromCurrentContextStore(); const navigate = useNavigateSettings(); const location = useLocation(); @@ -66,7 +68,7 @@ export const useRecordGroupActions = ({ ); const { reorderRecordGroups } = useReorderRecordGroups({ - viewBarId: objectMetadataItem.id, + recordIndexId, viewType, }); diff --git a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorderConfirmationModal.ts b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorderConfirmationModal.ts index ec4a0318329..91b885960cf 100644 --- a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorderConfirmationModal.ts +++ b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorderConfirmationModal.ts @@ -33,7 +33,7 @@ export const useRecordGroupReorderConfirmationModal = ({ useState | null>(null); const { reorderRecordGroups } = useReorderRecordGroups({ - viewBarId: recordIndexId, + recordIndexId, viewType, }); diff --git a/packages/twenty-front/src/modules/object-record/record-group/hooks/useReorderRecordGroups.ts b/packages/twenty-front/src/modules/object-record/record-group/hooks/useReorderRecordGroups.ts index 4ba9540b76d..3d0f6a09cd0 100644 --- a/packages/twenty-front/src/modules/object-record/record-group/hooks/useReorderRecordGroups.ts +++ b/packages/twenty-front/src/modules/object-record/record-group/hooks/useReorderRecordGroups.ts @@ -14,7 +14,7 @@ import { moveArrayItem } from '~/utils/array/moveArrayItem'; import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; type UseReorderRecordGroupsParams = { - viewBarId: string; + recordIndexId: string; viewType: ViewType; }; @@ -24,7 +24,7 @@ type ReorderRecordGroupsParams = { }; export const useReorderRecordGroups = ({ - viewBarId, + recordIndexId, viewType, }: UseReorderRecordGroupsParams) => { const { setRecordGroups } = useSetRecordGroups(); @@ -32,7 +32,7 @@ export const useReorderRecordGroups = ({ const visibleRecordGroupIdsFamilySelector = useRecoilComponentCallbackState( visibleRecordGroupIdsComponentFamilySelector, - viewBarId, + recordIndexId, ); const { saveViewGroups } = useSaveCurrentViewGroups(); @@ -80,16 +80,20 @@ export const useReorderRecordGroups = ({ ]; }, []); - setRecordGroups(updatedRecordGroups, viewBarId, objectMetadataItem.id); + setRecordGroups( + updatedRecordGroups, + recordIndexId, + objectMetadataItem.id, + ); saveViewGroups( mapRecordGroupDefinitionsToViewGroups(updatedRecordGroups), ); }, [ - objectMetadataItem, + objectMetadataItem.id, + recordIndexId, saveViewGroups, setRecordGroups, - viewBarId, viewType, visibleRecordGroupIdsFamilySelector, ], diff --git a/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewGroupRecords.ts b/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewGroupRecords.ts index 68478b9fc9c..ae11a758469 100644 --- a/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewGroupRecords.ts +++ b/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewGroupRecords.ts @@ -142,7 +142,7 @@ export const usePersistViewGroupRecords = () => { apolloClient.mutate<{ updateCoreViewGroup: ViewGroup }>({ mutation: UPDATE_CORE_VIEW_GROUP, variables: { - idToUpdate: viewGroup.id, + id: viewGroup.id, input: { isVisible: viewGroup.isVisible, position: viewGroup.position, diff --git a/packages/twenty-front/src/modules/views/hooks/useDeleteView.ts b/packages/twenty-front/src/modules/views/hooks/useDeleteView.ts index 1cc88ceffd9..efd9ed29850 100644 --- a/packages/twenty-front/src/modules/views/hooks/useDeleteView.ts +++ b/packages/twenty-front/src/modules/views/hooks/useDeleteView.ts @@ -1,17 +1,55 @@ import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; import { useDeleteOneRecord } from '@/object-record/hooks/useDeleteOneRecord'; +import { prefetchViewFromViewIdFamilySelector } from '@/prefetch/states/selector/prefetchViewFromViewIdFamilySelector'; +import { useRefreshCoreViews } from '@/views/hooks/useRefreshCoreViews'; +import { useFeatureFlagsMap } from '@/workspace/hooks/useFeatureFlagsMap'; import { useRecoilCallback } from 'recoil'; +import { isDefined } from 'twenty-shared/utils'; +import { FeatureFlagKey, useDeleteCoreViewMutation } from '~/generated/graphql'; export const useDeleteView = () => { + const featureFlags = useFeatureFlagsMap(); + const isCoreViewEnabled = featureFlags[FeatureFlagKey.IS_CORE_VIEW_ENABLED]; + + const [deleteCoreViewMutation] = useDeleteCoreViewMutation(); + const { refreshCoreViews } = useRefreshCoreViews(); const { deleteOneRecord } = useDeleteOneRecord({ objectNameSingular: CoreObjectNameSingular.View, }); const deleteView = useRecoilCallback( - () => async (viewId: string) => { - await deleteOneRecord(viewId); - }, - [deleteOneRecord], + ({ snapshot }) => + async (viewId: string) => { + const currentView = snapshot + .getLoadable( + prefetchViewFromViewIdFamilySelector({ + viewId, + }), + ) + .getValue(); + + if (!isDefined(currentView)) { + return; + } + + if (isCoreViewEnabled) { + await deleteCoreViewMutation({ + variables: { + id: viewId, + }, + }); + + await refreshCoreViews(currentView.objectMetadataId); + } else { + await deleteOneRecord(viewId); + } + }, + [ + deleteCoreViewMutation, + deleteOneRecord, + isCoreViewEnabled, + refreshCoreViews, + ], ); return { deleteView }; diff --git a/packages/twenty-front/src/modules/views/hooks/useUpdateCurrentView.ts b/packages/twenty-front/src/modules/views/hooks/useUpdateCurrentView.ts index c63875a6b80..0c066006659 100644 --- a/packages/twenty-front/src/modules/views/hooks/useUpdateCurrentView.ts +++ b/packages/twenty-front/src/modules/views/hooks/useUpdateCurrentView.ts @@ -3,7 +3,9 @@ import { useRecoilCallback } from 'recoil'; import { contextStoreCurrentViewIdComponentState } from '@/context-store/states/contextStoreCurrentViewIdComponentState'; import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; import { useUpdateOneRecord } from '@/object-record/hooks/useUpdateOneRecord'; +import { prefetchViewFromViewIdFamilySelector } from '@/prefetch/states/selector/prefetchViewFromViewIdFamilySelector'; import { useRecoilComponentCallbackState } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentCallbackState'; +import { useRefreshCoreViews } from '@/views/hooks/useRefreshCoreViews'; import { type GraphQLView } from '@/views/types/GraphQLView'; import { convertUpdateViewInputToCore } from '@/views/utils/convertUpdateViewInputToCore'; import { useFeatureFlagsMap } from '@/workspace/hooks/useFeatureFlagsMap'; @@ -22,6 +24,7 @@ export const useUpdateCurrentView = () => { const { updateOneRecord } = useUpdateOneRecord({ objectNameSingular: CoreObjectNameSingular.View, }); + const { refreshCoreViews } = useRefreshCoreViews(); const [updateOneCoreView] = useUpdateCoreViewMutation(); @@ -32,6 +35,18 @@ export const useUpdateCurrentView = () => { .getLoadable(currentViewIdCallbackState) .getValue(); + const currentView = snapshot + .getLoadable( + prefetchViewFromViewIdFamilySelector({ + viewId: currentViewId ?? '', + }), + ) + .getValue(); + + if (!isDefined(currentView)) { + return; + } + if (isDefined(currentViewId)) { if (isCoreViewEnabled) { const input = convertUpdateViewInputToCore(view); @@ -42,6 +57,7 @@ export const useUpdateCurrentView = () => { input, }, }); + await refreshCoreViews(currentView.objectMetadataId); } else { await updateOneRecord({ idToUpdate: currentViewId, @@ -53,6 +69,7 @@ export const useUpdateCurrentView = () => { [ currentViewIdCallbackState, isCoreViewEnabled, + refreshCoreViews, updateOneCoreView, updateOneRecord, ],