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 : `<RecordIndexBoardDataLoader />` that loads all states for a board when we switch between views, for each column, by calling `<RecordIndexBoardColumnLoaderEffect />` 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`.
This commit is contained in:
Lucas Bordeau
@ -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,
|
||||
});
|
||||
|
||||
|
||||
@ -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) => (
|
||||
<RecordIndexBoardColumnLoaderEffect
|
||||
objectNameSingular={objectNameSingular}
|
||||
boardFieldMetadataId={recordIndexKanbanFieldMetadataId}
|
||||
kanbanFieldMetadataItem={recordIndexKanbanFieldMetadataItem}
|
||||
recordBoardId={recordBoardId}
|
||||
columnId={recordGroupId}
|
||||
key={index}
|
||||
key={recordGroupId}
|
||||
/>
|
||||
))}
|
||||
{recordIndexKanbanFieldMetadataItem?.isNullable && (
|
||||
{recordIndexKanbanFieldMetadataItem.isNullable === true && (
|
||||
<RecordIndexBoardColumnLoaderEffect
|
||||
objectNameSingular={objectNameSingular}
|
||||
boardFieldMetadataId={recordIndexKanbanFieldMetadataId}
|
||||
kanbanFieldMetadataItem={recordIndexKanbanFieldMetadataItem}
|
||||
recordBoardId={recordBoardId}
|
||||
columnId={'no-value'}
|
||||
/>
|
||||
|
||||
@ -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,
|
||||
|
||||
Reference in New Issue
Block a user