Fix batch update optimistic and prevent accidental mass-update (#17213)
## Problem - ⚠️ Multi-edit could silently update ALL records of an object with no undo, no ctrl-z - After a batch update the table showed stale data — `useIncrementalUpdateManyRecords` had no explicit `findMany` refetch and `skipOptimisticEffect` was missing - Clicking inside the side panel deselected all kanban cards — `RecordBoardClickOutsideEffect` uses `refs:[]` and the side panel had no `data-click-outside-id` - Clicking a currency/select dropdown inside the panel also triggered deselection — `FloatingPortal` renders outside the side panel DOM, bypassing the click-outside-id exclusion - An empty selection silently matched every record — `computeContextStoreFilters` returned `undefined` filter when `selectedRecordIds` was `[]` ## Fix - Table refreshes correctly after batch update — `useRefetchFindManyRecords` explicitly refetches `FindMany<Object>` queries; `useIncrementalUpdateManyRecords` adds `skipOptimisticEffect: true` and calls it in `finally` - Clicking the side panel no longer deselects kanban cards — `SidePanelForDesktop` carries `data-click-outside-id`; `RecordBoardClickOutsideEffect` + `RecordTableBodyFocusClickOutsideEffect` exclude it - Clicking dropdowns inside the panel no longer deselects either — `ParentClickOutsideIdContext` propagates the side panel ID into `FloatingPortal` content via `DropdownInternalContainer` - Empty selection can no longer match all records — `computeContextStoreFilters` returns `{ id: { in: [] } }` instead of `undefined` - Apply is disabled with no selection; a confirmation modal shows the count + no-undo warning before executing — `UpdateMultipleRecordsContainer` ## Not included - Undo / snapshot restore — requires backend changes, out of scope ## Blast radius - `ParentClickOutsideIdContext` touches `DropdownInternalContainer` (207 `<Dropdown>` usages). `parentClickOutsideId` is `undefined` everywhere outside the side panel → attribute not rendered → zero behavioral change for existing consumers. --------- Co-authored-by: Samuel Arbibe <samuelarbibe@Samuels-MacBook-Pro.local> Co-authored-by: Lucas Bordeau <bordeau.lucas@gmail.com> Co-authored-by: Félix Malfait <felix.malfait@gmail.com>
This commit is contained in:
+15
@@ -1,6 +1,7 @@
|
||||
import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem';
|
||||
import { useIncrementalFetchAndMutateRecords } from '@/object-record/hooks/useIncrementalFetchAndMutateRecords';
|
||||
import { useIncrementalUpdateManyRecords } from '@/object-record/hooks/useIncrementalUpdateManyRecords';
|
||||
import { useRefetchFindManyRecords } from '@/object-record/hooks/useRefetchFindManyRecords';
|
||||
import { useUpdateManyRecords } from '@/object-record/hooks/useUpdateManyRecords';
|
||||
import { dispatchObjectRecordOperationBrowserEvent } from '@/browser-event/utils/dispatchObjectRecordOperationBrowserEvent';
|
||||
import { renderHook } from '@testing-library/react';
|
||||
@@ -15,6 +16,9 @@ jest.mock('@/object-record/hooks/useRefetchAggregateQueries', () => ({
|
||||
refetchAggregateQueries: jest.fn(),
|
||||
}),
|
||||
}));
|
||||
jest.mock('@/object-record/hooks/useRefetchFindManyRecords', () => ({
|
||||
useRefetchFindManyRecords: jest.fn(),
|
||||
}));
|
||||
jest.mock('@/object-record/hooks/useIncrementalFetchAndMutateRecords');
|
||||
|
||||
const mockUseObjectMetadataItem = jest.mocked(useObjectMetadataItem);
|
||||
@@ -25,6 +29,7 @@ const mockUseUpdateManyRecords = jest.mocked(useUpdateManyRecords);
|
||||
const mockUseIncrementalFetchAndMutateRecords = jest.mocked(
|
||||
useIncrementalFetchAndMutateRecords,
|
||||
);
|
||||
const mockUseRefetchFindManyRecords = jest.mocked(useRefetchFindManyRecords);
|
||||
|
||||
describe('useIncrementalUpdateManyRecords', () => {
|
||||
const mockUpdateManyRecords = jest.fn();
|
||||
@@ -53,6 +58,11 @@ describe('useIncrementalUpdateManyRecords', () => {
|
||||
updateProgress: mockUpdateProgress,
|
||||
cancel: jest.fn(),
|
||||
});
|
||||
|
||||
const mockRefetchFindManyRecords = jest.fn();
|
||||
mockUseRefetchFindManyRecords.mockReturnValue({
|
||||
refetchFindManyRecords: mockRefetchFindManyRecords,
|
||||
});
|
||||
});
|
||||
|
||||
it('should call incrementalFetchAndMutate and execute mutations via useUpdateManyRecords', async () => {
|
||||
@@ -81,6 +91,7 @@ describe('useIncrementalUpdateManyRecords', () => {
|
||||
delayInMsBetweenRequests: 50,
|
||||
skipRegisterObjectOperation: true,
|
||||
skipRefetchAggregateQueries: true,
|
||||
skipOptimisticEffect: true,
|
||||
abortSignal: expect.any(AbortSignal),
|
||||
});
|
||||
expect(mockUpdateProgress).toHaveBeenCalledWith(2, 2);
|
||||
@@ -102,6 +113,10 @@ describe('useIncrementalUpdateManyRecords', () => {
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const { refetchFindManyRecords } =
|
||||
mockUseRefetchFindManyRecords.mock.results[0].value;
|
||||
expect(refetchFindManyRecords).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should pass abortSignal to updateManyRecords', async () => {
|
||||
|
||||
+8
-1
@@ -1,11 +1,12 @@
|
||||
import { dispatchObjectRecordOperationBrowserEvent } from '@/browser-event/utils/dispatchObjectRecordOperationBrowserEvent';
|
||||
import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem';
|
||||
import { DEFAULT_QUERY_PAGE_SIZE } from '@/object-record/constants/DefaultQueryPageSize';
|
||||
import { type UseFindManyRecordsParams } from '@/object-record/hooks/useFetchMoreRecordsWithPagination';
|
||||
import { useIncrementalFetchAndMutateRecords } from '@/object-record/hooks/useIncrementalFetchAndMutateRecords';
|
||||
import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries';
|
||||
import { useRefetchFindManyRecords } from '@/object-record/hooks/useRefetchFindManyRecords';
|
||||
import { useUpdateManyRecords } from '@/object-record/hooks/useUpdateManyRecords';
|
||||
import { type ObjectRecord } from '@/object-record/types/ObjectRecord';
|
||||
import { dispatchObjectRecordOperationBrowserEvent } from '@/browser-event/utils/dispatchObjectRecordOperationBrowserEvent';
|
||||
import { getUpdatedFieldsFromRecordInput } from '@/object-record/utils/getUpdatedFieldsFromRecordInput';
|
||||
|
||||
const DEFAULT_DELAY_BETWEEN_MUTATIONS_MS = 50;
|
||||
@@ -39,6 +40,10 @@ export const useIncrementalUpdateManyRecords = <
|
||||
|
||||
const { refetchAggregateQueries } = useRefetchAggregateQueries();
|
||||
|
||||
const { refetchFindManyRecords } = useRefetchFindManyRecords({
|
||||
objectMetadataNamePlural: objectMetadataItem.namePlural,
|
||||
});
|
||||
|
||||
const {
|
||||
incrementalFetchAndMutate,
|
||||
progress,
|
||||
@@ -68,6 +73,7 @@ export const useIncrementalUpdateManyRecords = <
|
||||
delayInMsBetweenRequests: delayInMsBetweenMutations,
|
||||
skipRegisterObjectOperation: true,
|
||||
skipRefetchAggregateQueries: true,
|
||||
skipOptimisticEffect: true,
|
||||
abortSignal,
|
||||
});
|
||||
|
||||
@@ -78,6 +84,7 @@ export const useIncrementalUpdateManyRecords = <
|
||||
},
|
||||
);
|
||||
} finally {
|
||||
await refetchFindManyRecords();
|
||||
await refetchAggregateQueries({
|
||||
objectMetadataNamePlural: objectMetadataItem.namePlural,
|
||||
});
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
import { useApolloCoreClient } from '@/object-metadata/hooks/useApolloCoreClient';
|
||||
import { capitalize } from 'twenty-shared/utils';
|
||||
|
||||
export const useRefetchFindManyRecords = ({
|
||||
objectMetadataNamePlural,
|
||||
}: {
|
||||
objectMetadataNamePlural: string;
|
||||
}) => {
|
||||
const apolloCoreClient = useApolloCoreClient();
|
||||
|
||||
const refetchFindManyRecords = async () => {
|
||||
const findManyRecordsQueryName = `FindMany${capitalize(
|
||||
objectMetadataNamePlural,
|
||||
)}`;
|
||||
|
||||
await apolloCoreClient.refetchQueries({
|
||||
include: [findManyRecordsQueryName],
|
||||
});
|
||||
};
|
||||
|
||||
return {
|
||||
refetchFindManyRecords,
|
||||
};
|
||||
};
|
||||
+2
@@ -1,6 +1,7 @@
|
||||
import { COMMAND_MENU_DROPDOWN_CLICK_OUTSIDE_ID } from '@/command-menu-item/constants/CommandMenuDropdownClickOutsideId';
|
||||
import { COMMAND_MENU_CLICK_OUTSIDE_ID } from '@/command-menu/constants/CommandMenuClickOutsideId';
|
||||
import { RECORD_BOARD_CLICK_OUTSIDE_LISTENER_ID } from '@/object-record/record-board/constants/RecordBoardClickOutsideListenerId';
|
||||
import { SIDE_PANEL_CLICK_OUTSIDE_ID } from '@/side-panel/constants/SidePanelClickOutsideId';
|
||||
import { RecordBoardContext } from '@/object-record/record-board/contexts/RecordBoardContext';
|
||||
import { useActiveRecordBoardCard } from '@/object-record/record-board/hooks/useActiveRecordBoardCard';
|
||||
import { useFocusedRecordBoardCard } from '@/object-record/record-board/hooks/useFocusedRecordBoardCard';
|
||||
@@ -37,6 +38,7 @@ export const RecordBoardClickOutsideEffect = () => {
|
||||
PAGE_ACTION_CONTAINER_CLICK_OUTSIDE_ID,
|
||||
RECORD_BOARD_CARD_CLICK_OUTSIDE_ID,
|
||||
LINK_CHIP_CLICK_OUTSIDE_ID,
|
||||
SIDE_PANEL_CLICK_OUTSIDE_ID,
|
||||
],
|
||||
listenerId: RECORD_BOARD_CLICK_OUTSIDE_LISTENER_ID,
|
||||
refs: [],
|
||||
|
||||
+2
@@ -1,6 +1,7 @@
|
||||
import { COMMAND_MENU_DROPDOWN_CLICK_OUTSIDE_ID } from '@/command-menu-item/constants/CommandMenuDropdownClickOutsideId';
|
||||
import { COMMAND_MENU_CLICK_OUTSIDE_ID } from '@/command-menu/constants/CommandMenuClickOutsideId';
|
||||
import { RECORD_TABLE_CLICK_OUTSIDE_LISTENER_ID } from '@/object-record/record-table/constants/RecordTableClickOutsideListenerId';
|
||||
import { SIDE_PANEL_CLICK_OUTSIDE_ID } from '@/side-panel/constants/SidePanelClickOutsideId';
|
||||
import { useRecordTableContextOrThrow } from '@/object-record/record-table/contexts/RecordTableContext';
|
||||
import { useLeaveTableFocus } from '@/object-record/record-table/hooks/internal/useLeaveTableFocus';
|
||||
import { MODAL_BACKDROP_CLICK_OUTSIDE_ID } from '@/ui/layout/modal/constants/ModalBackdropClickOutsideId';
|
||||
@@ -30,6 +31,7 @@ export const RecordTableBodyFocusClickOutsideEffect = ({
|
||||
COMMAND_MENU_CLICK_OUTSIDE_ID,
|
||||
PAGE_ACTION_CONTAINER_CLICK_OUTSIDE_ID,
|
||||
MODAL_BACKDROP_CLICK_OUTSIDE_ID,
|
||||
SIDE_PANEL_CLICK_OUTSIDE_ID,
|
||||
],
|
||||
listenerId: RECORD_TABLE_CLICK_OUTSIDE_LISTENER_ID,
|
||||
refs: [tableBodyRef],
|
||||
|
||||
+33
-11
@@ -1,15 +1,22 @@
|
||||
import { contextStoreNumberOfSelectedRecordsComponentState } from '@/context-store/states/contextStoreNumberOfSelectedRecordsComponentState';
|
||||
import { useSidePanelMenu } from '@/side-panel/hooks/useSidePanelMenu';
|
||||
import { UpdateMultipleRecordsFooter } from '@/object-record/record-update-multiple/components/UpdateMultipleRecordsFooter';
|
||||
import { UpdateMultipleRecordsForm } from '@/object-record/record-update-multiple/components/UpdateMultipleRecordsForm';
|
||||
import { useUpdateMultipleRecordsActions } from '@/object-record/record-update-multiple/hooks/useUpdateMultipleRecordsActions';
|
||||
import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar';
|
||||
import { ConfirmationModal } from '@/ui/layout/modal/components/ConfirmationModal';
|
||||
import { useModal } from '@/ui/layout/modal/hooks/useModal';
|
||||
import { ShowPageContainer } from '@/ui/layout/page/components/ShowPageContainer';
|
||||
import { SidePanelProvider } from '@/ui/layout/side-panel/contexts/SidePanelContext';
|
||||
import { useAtomComponentStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateValue';
|
||||
import { styled } from '@linaria/react';
|
||||
import { useLingui } from '@lingui/react/macro';
|
||||
import { useState } from 'react';
|
||||
import { themeCssVariables } from 'twenty-ui/theme-constants';
|
||||
|
||||
const UPDATE_MULTIPLE_RECORDS_CONFIRMATION_MODAL_ID =
|
||||
'update-multiple-records-confirmation';
|
||||
|
||||
const StyledShowPageRightContainer = styled.div`
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
@@ -41,23 +48,30 @@ export const UpdateMultipleRecordsContainer = ({
|
||||
contextStoreInstanceId,
|
||||
});
|
||||
|
||||
const contextStoreNumberOfSelectedRecords = useAtomComponentStateValue(
|
||||
contextStoreNumberOfSelectedRecordsComponentState,
|
||||
contextStoreInstanceId,
|
||||
);
|
||||
|
||||
const hasSelectedRecords = contextStoreNumberOfSelectedRecords > 0;
|
||||
|
||||
const { t } = useLingui();
|
||||
const { enqueueSuccessSnackBar, enqueueErrorSnackBar } = useSnackBar();
|
||||
const { enqueueErrorSnackBar } = useSnackBar();
|
||||
const { openModal } = useModal();
|
||||
const { closeSidePanelMenu } = useSidePanelMenu();
|
||||
|
||||
const [fieldUpdates, setFieldUpdates] = useState<UpdateMultipleRecordsState>(
|
||||
{},
|
||||
);
|
||||
|
||||
const handleUpdate = async () => {
|
||||
const handleUpdateClick = () => {
|
||||
openModal(UPDATE_MULTIPLE_RECORDS_CONFIRMATION_MODAL_ID);
|
||||
};
|
||||
|
||||
const handleConfirmedUpdate = async () => {
|
||||
try {
|
||||
const count = await updateRecords(fieldUpdates);
|
||||
if (count !== undefined) {
|
||||
enqueueSuccessSnackBar({
|
||||
message: t`Successfully updated ${count} records`,
|
||||
});
|
||||
closeSidePanelMenu();
|
||||
}
|
||||
await updateRecords(fieldUpdates);
|
||||
closeSidePanelMenu();
|
||||
} catch (error) {
|
||||
enqueueErrorSnackBar({
|
||||
message:
|
||||
@@ -99,12 +113,20 @@ export const UpdateMultipleRecordsContainer = ({
|
||||
<UpdateMultipleRecordsFooter
|
||||
isUpdating={isUpdating}
|
||||
progress={progress}
|
||||
onUpdate={handleUpdate}
|
||||
onUpdate={handleUpdateClick}
|
||||
onCancel={handleCancel}
|
||||
isUpdateDisabled={!hasChanges}
|
||||
isUpdateDisabled={!hasChanges || !hasSelectedRecords}
|
||||
/>
|
||||
</StyledShowPageRightContainer>
|
||||
</ShowPageContainer>
|
||||
<ConfirmationModal
|
||||
modalInstanceId={UPDATE_MULTIPLE_RECORDS_CONFIRMATION_MODAL_ID}
|
||||
title={t`Update ${contextStoreNumberOfSelectedRecords} records`}
|
||||
subtitle={t`This will modify ${contextStoreNumberOfSelectedRecords} records. This action cannot be undone.`}
|
||||
onConfirmClick={handleConfirmedUpdate}
|
||||
confirmButtonText={t`Update records`}
|
||||
confirmButtonAccent="blue"
|
||||
/>
|
||||
</SidePanelProvider>
|
||||
);
|
||||
};
|
||||
|
||||
+22
-1
@@ -1,10 +1,17 @@
|
||||
import { CommandMenuContext } from '@/command-menu-item/contexts/CommandMenuContext';
|
||||
import { MAIN_CONTEXT_STORE_INSTANCE_ID } from '@/context-store/constants/MainContextStoreInstanceId';
|
||||
import { contextStoreNumberOfSelectedRecordsComponentState } from '@/context-store/states/contextStoreNumberOfSelectedRecordsComponentState';
|
||||
import { ApolloCoreClientContext } from '@/object-metadata/contexts/ApolloCoreClientContext';
|
||||
import { UpdateMultipleRecordsContainer } from '@/object-record/record-update-multiple/components/UpdateMultipleRecordsContainer';
|
||||
import { useSetAtomComponentState } from '@/ui/utilities/state/jotai/hooks/useSetAtomComponentState';
|
||||
import { useEffect } from 'react';
|
||||
import { ApolloClient, InMemoryCache } from '@apollo/client';
|
||||
import { MockLink } from '@apollo/client/testing';
|
||||
import { type Meta, type StoryObj } from '@storybook/react-vite';
|
||||
import {
|
||||
type Decorator,
|
||||
type Meta,
|
||||
type StoryObj,
|
||||
} from '@storybook/react-vite';
|
||||
import gql from 'graphql-tag';
|
||||
import { expect, userEvent, within } from 'storybook/test';
|
||||
import { ContextStoreDecorator } from '~/testing/decorators/ContextStoreDecorator';
|
||||
@@ -46,6 +53,19 @@ const mockApolloCoreClient = new ApolloClient({
|
||||
cache: new InMemoryCache(),
|
||||
});
|
||||
|
||||
const SelectedRecordsSeedDecorator: Decorator = (Story) => {
|
||||
const setNumberOfSelectedRecords = useSetAtomComponentState(
|
||||
contextStoreNumberOfSelectedRecordsComponentState,
|
||||
MAIN_CONTEXT_STORE_INSTANCE_ID,
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
setNumberOfSelectedRecords(3);
|
||||
}, [setNumberOfSelectedRecords]);
|
||||
|
||||
return <Story />;
|
||||
};
|
||||
|
||||
const meta: Meta<typeof UpdateMultipleRecordsContainer> = {
|
||||
title:
|
||||
'Modules/ObjectRecord/RecordUpdateMultiple/Components/UpdateMultipleRecordsContainer',
|
||||
@@ -65,6 +85,7 @@ const meta: Meta<typeof UpdateMultipleRecordsContainer> = {
|
||||
</CommandMenuContext.Provider>
|
||||
</ApolloCoreClientContext.Provider>
|
||||
),
|
||||
SelectedRecordsSeedDecorator,
|
||||
ContextStoreDecorator,
|
||||
ObjectMetadataItemsDecorator,
|
||||
SnackBarDecorator,
|
||||
|
||||
+1
-6
@@ -62,13 +62,8 @@ export const useUpdateMultipleRecordsActions = ({
|
||||
filter: graphqlFilter,
|
||||
});
|
||||
|
||||
const updateRecords = async (fieldsToUpdate: Record<string, any>) => {
|
||||
const count = await incrementalUpdateManyRecords(fieldsToUpdate);
|
||||
return count;
|
||||
};
|
||||
|
||||
return {
|
||||
updateRecords,
|
||||
updateRecords: incrementalUpdateManyRecords,
|
||||
isUpdating,
|
||||
progress,
|
||||
cancel,
|
||||
|
||||
Reference in New Issue
Block a user