From 9e00a67b25625135b159c66d86cd4ee27228bb7f Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Thu, 15 May 2025 10:27:47 +0200 Subject: [PATCH] Fix kanban loading bug (#12042) Fixes https://github.com/twentyhq/core-team-issues/issues/956 This PR fixes a bug that appeared when switching between two kanban views multiple times. Steps to reproduce : - Go to kanban view A - Go to kanban view B - Go back to kanban view A Video before : https://github.com/user-attachments/assets/4fa789ae-7187-498e-82b4-ee7896cd95d1 Video after : https://github.com/user-attachments/assets/2b323a2d-2f76-405d-9abd-38fe72ee2214 The problem was that we allowed a hook to take a nullable parameter that can be nullable between page switch, and threw when it was undefined. In order to be more cautious in the future, let's be sure that we don't throw for undefined props of a hook, when it is expected that those props can be in an undefined state for several React render loops, while fetching new data. Here I identified the parent effect : `` that loads all states for a board when we switch between views, for each column, by calling `` in a `.map()`. Each `RecordIndexBoardColumnLoaderEffect` was calling `useLoadRecordIndexBoardColumn` with a potentially undefined `boardFieldMetadataId`. So to fix this, I cut the render flow higher than the throw in `useLoadRecordIndexBoardColumn`, and by doing that I was able to ensure that `recordIndexKanbanFieldMetadataItem` in `RecordIndexBoardDataLoader` was already defined when using it inside `useLoadRecordIndexBoardColumn`. `recordIndexKanbanFieldMetadataItem` was unnecessarily fetched two times, one time in `RecordIndexBoardDataLoader` and another time in its child `useLoadRecordIndexBoardColumn` hook. By implementing this flow-cut higher up, I could then remove the `| null` in TypeScript in children components, and expect a defined value in all the children of `RecordIndexBoardDataLoader`, thus removing the need to ask ourselves if we should throw or not in `useLoadRecordIndexBoardColumn`. --- .../RecordIndexBoardColumnLoaderEffect.tsx | 7 ++-- .../components/RecordIndexBoardDataLoader.tsx | 15 +++++--- .../hooks/useLoadRecordIndexBoardColumn.ts | 34 +++++-------------- 3 files changed, 22 insertions(+), 34 deletions(-) diff --git a/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexBoardColumnLoaderEffect.tsx b/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexBoardColumnLoaderEffect.tsx index bde4108f5..88a70a61c 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexBoardColumnLoaderEffect.tsx +++ b/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexBoardColumnLoaderEffect.tsx @@ -1,6 +1,7 @@ import { useEffect } from 'react'; import { useRecoilState, useSetRecoilState } from 'recoil'; +import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem'; import { isRecordBoardFetchingRecordsByColumnFamilyState } from '@/object-record/record-board/states/isRecordBoardFetchingRecordsByColumnFamilyState'; import { recordBoardShouldFetchMoreInColumnComponentFamilyState } from '@/object-record/record-board/states/recordBoardShouldFetchMoreInColumnComponentFamilyState'; import { useLoadRecordIndexBoardColumn } from '@/object-record/record-index/hooks/useLoadRecordIndexBoardColumn'; @@ -9,13 +10,13 @@ import { useRecoilComponentFamilyStateV2 } from '@/ui/utilities/state/component- export const RecordIndexBoardColumnLoaderEffect = ({ objectNameSingular, - boardFieldMetadataId, recordBoardId, + kanbanFieldMetadataItem, columnId, }: { recordBoardId: string; objectNameSingular: string; - boardFieldMetadataId: string | null; + kanbanFieldMetadataItem: FieldMetadataItem; columnId: string; }) => { const [shouldFetchMore, setShouldFetchMore] = useRecoilComponentFamilyStateV2( @@ -31,7 +32,7 @@ export const RecordIndexBoardColumnLoaderEffect = ({ useLoadRecordIndexBoardColumn({ objectNameSingular, recordBoardId, - boardFieldMetadataId, + kanbanFieldMetadataItem, columnId, }); diff --git a/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexBoardDataLoader.tsx b/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexBoardDataLoader.tsx index 9c189ee05..099ab70bc 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexBoardDataLoader.tsx +++ b/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexBoardDataLoader.tsx @@ -5,6 +5,7 @@ import { recordGroupIdsComponentState } from '@/object-record/record-group/state import { RecordIndexBoardColumnLoaderEffect } from '@/object-record/record-index/components/RecordIndexBoardColumnLoaderEffect'; import { recordIndexKanbanFieldMetadataIdState } from '@/object-record/record-index/states/recordIndexKanbanFieldMetadataIdState'; import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; +import { isDefined } from 'twenty-shared/utils'; type RecordIndexBoardDataLoaderProps = { objectNameSingular: string; @@ -31,21 +32,25 @@ export const RecordIndexBoardDataLoader = ({ (field) => field.id === recordIndexKanbanFieldMetadataId, ); + if (!isDefined(recordIndexKanbanFieldMetadataItem)) { + return null; + } + return ( <> - {recordGroupIds.map((recordGroupId, index) => ( + {recordGroupIds.map((recordGroupId) => ( ))} - {recordIndexKanbanFieldMetadataItem?.isNullable && ( + {recordIndexKanbanFieldMetadataItem.isNullable === true && ( diff --git a/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexBoardColumn.ts b/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexBoardColumn.ts index a601f3979..e3b1ee7a8 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexBoardColumn.ts +++ b/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexBoardColumn.ts @@ -12,22 +12,22 @@ import { computeRecordGqlOperationFilter } from '@/object-record/record-filter/u import { recordGroupDefinitionFamilyState } from '@/object-record/record-group/states/recordGroupDefinitionFamilyState'; import { useRecordBoardRecordGqlFields } from '@/object-record/record-index/hooks/useRecordBoardRecordGqlFields'; +import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem'; import { currentRecordSortsComponentState } from '@/object-record/record-sort/states/currentRecordSortsComponentState'; import { useUpsertRecordsInStore } from '@/object-record/record-store/hooks/useUpsertRecordsInStore'; import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; -import { isNonEmptyString } from '@sniptt/guards'; import { isDefined } from 'twenty-shared/utils'; type UseLoadRecordIndexBoardProps = { objectNameSingular: string; - boardFieldMetadataId: string | null; + kanbanFieldMetadataItem: FieldMetadataItem; recordBoardId: string; columnId: string; }; export const useLoadRecordIndexBoardColumn = ({ objectNameSingular, - boardFieldMetadataId, + kanbanFieldMetadataItem, recordBoardId, columnId, }: UseLoadRecordIndexBoardProps) => { @@ -69,29 +69,11 @@ export const useLoadRecordIndexBoardColumn = ({ recordBoardId, }); - let recordIndexKanbanFieldMetadataFilter: { [x: string]: any } = {}; - - if (isDefined(boardFieldMetadataId)) { - const recordIndexKanbanFieldMetadataItem = objectMetadataItem.fields.find( - (field) => field.id === boardFieldMetadataId, - ); - - if (!isDefined(recordIndexKanbanFieldMetadataItem)) { - throw new Error('Record index kanban field metadata item not found'); - } - - if (!isNonEmptyString(recordIndexKanbanFieldMetadataItem?.name ?? '')) { - throw new Error('Record index kanban field metadata item name not found'); - } - - recordIndexKanbanFieldMetadataFilter = { - [recordIndexKanbanFieldMetadataItem.name]: isDefined( - recordGroupDefinition?.value, - ) - ? { in: [recordGroupDefinition.value] } - : { is: 'NULL' }, - }; - } + const recordIndexKanbanFieldMetadataFilter = { + [kanbanFieldMetadataItem.name]: isDefined(recordGroupDefinition?.value) + ? { in: [recordGroupDefinition.value] } + : { is: 'NULL' }, + }; const filter = { ...requestFilters,