Spend-Hybrid-Tap on checkbox does not select the group

src/components/Search/SearchList/ListItem/TransactionGroupListItem.tsx

@@ -131,6 +131,8 @@ function TransactionGroupListItemImpl({
 
     const selectedTransactionIDs = Object.keys(selectedTransactions);
     const selectedTransactionIDsSet = new Set(selectedTransactionIDs);
+    // A group selected before its children were fetched is stored under the group key, since no transaction IDs were known yet
+    const isGroupSelected = !!(item?.keyForList && selectedTransactions[item.keyForList]?.isSelected);
     const [transactionsSnapshot] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${groupItem.transactionsQueryJSON?.hash}`);
 
     const isExpenseReportType = searchType === CONST.SEARCH.DATA_TYPES.EXPENSE_REPORT;
@@ -175,17 +177,19 @@ function TransactionGroupListItemImpl({
         }) as [TransactionListItemType[], number, boolean];
         transactions = sectionData.map((transactionItem) => ({
             ...transactionItem,
-            isSelected: selectedTransactionIDsSet.has(transactionItem.transactionID),
+            // The whole group being selected implies every child is, even though only the group key is stored
+            isSelected: isGroupSelected || selectedTransactionIDsSet.has(transactionItem.transactionID),
         }));
     }
 
     const selectedItemsLength = transactions.reduce((acc, transaction) => (transaction.isSelected ? acc + 1 : acc), 0);
 
     const transactionsWithoutPendingDelete = transactions.filter((transaction) => transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE);
 
-    const isEmpty = transactions.length === 0;
+    // A group whose children are lazily loaded (it has a transactionsQueryJSON) is not empty, it just hasn't been fetched yet
+    const isEmpty = transactions.length === 0 && groupItem.transactions.length === 0 && !groupItem.transactionsQueryJSON;
 
-    const isEmptyReportSelected = isEmpty && item?.keyForList && selectedTransactions[item.keyForList]?.isSelected;
+    const isEmptyReportSelected = transactions.length === 0 && isGroupSelected;
 
     const isSelectAllChecked = isEmptyReportSelected || (selectedItemsLength === transactionsWithoutPendingDelete.length && transactionsWithoutPendingDelete.length > 0);
     const isIndeterminate = selectedItemsLength > 0 && selectedItemsLength !== transactionsWithoutPendingDelete.length;

src/components/Search/SearchWriteActionsProvider.tsx

@@ -418,8 +418,16 @@ function SearchWriteActionsProvider({
                     return {...selectedTransactions, [reportKey]: emptyReportSelection};
                 }
 
-                if (currentTransactions.some((transaction) => selectedTransactions[transaction.keyForList]?.isSelected)) {
+                // A group selected before its children were fetched is stored under the group key. Once the children load,
+                // deselecting has to clear that entry too, otherwise the group stays selected with no way to deselect it.
+                const groupKey = item.keyForList;
+                const isGroupKeySelected = !!(groupKey && selectedTransactions[groupKey]?.isSelected);
+
+                if (isGroupKeySelected || currentTransactions.some((transaction) => selectedTransactions[transaction.keyForList]?.isSelected)) {
                     const reducedSelectedTransactions: SelectedTransactions = {...selectedTransactions};
+                    if (groupKey) {
+                        delete reducedSelectedTransactions[groupKey];
+                    }
                     for (const transaction of currentTransactions) {
                         delete reducedSelectedTransactions[transaction.keyForList];
                     }

tests/unit/Search/LazyGroupSelectionTest.tsx

@@ -0,0 +1,145 @@
+import {act, renderHook} from '@testing-library/react-native';
+
+import {useSearchRowSelectionActions, useSearchSelectionContext} from '@components/Search/SearchContext';
+import {SearchContextProvider} from '@components/Search/SearchContextProvider';
+import type {TransactionCategoryGroupListItemType, TransactionListItemType} from '@components/Search/SearchList/ListItem/types';
+import SearchWriteActionsProvider from '@components/Search/SearchWriteActionsProvider';
+
+import {buildSearchQueryJSON} from '@libs/SearchQueryUtils';
+
+import CONST from '@src/CONST';
+import ONYXKEYS from '@src/ONYXKEYS';
+
+import type * as ReactNavigation from '@react-navigation/native';
+
+import React from 'react';
+import Onyx from 'react-native-onyx';
+
+import waitForBatchedUpdatesWithAct from '../../utils/waitForBatchedUpdatesWithAct';
+
+jest.mock('@react-navigation/native', () => ({
+    ...jest.requireActual<typeof ReactNavigation>('@react-navigation/native'),
+    useIsFocused: () => true,
+    useRoute: jest.fn(() => ({key: 'search-test-route'})),
+    useRootNavigationState: jest.fn(() => undefined),
+    useNavigation: jest.fn(() => ({
+        getState: jest.fn(() => undefined),
+        addListener: jest.fn(() => jest.fn()),
+        navigate: jest.fn(),
+    })),
+}));
+
+const GROUP_KEY = 'Advertising';
+
+/**
+ * A `group-by:category` group. Its children are fetched into a separate snapshot only once the row is expanded,
+ * so `transactions` stays empty on the group itself for the whole lifetime of the list.
+ */
+// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- minimal fixture: only the fields the selection logic reads are needed
+const categoryGroup = {
+    groupedBy: CONST.SEARCH.GROUP_BY.CATEGORY,
+    category: 'Advertising',
+    formattedCategory: 'Advertising',
+    count: 2,
+    total: -1284,
+    currency: 'USD',
+    transactions: [],
+    transactionsQueryJSON: buildSearchQueryJSON('type:expense category:Advertising'),
+    keyForList: GROUP_KEY,
+} as unknown as TransactionCategoryGroupListItemType;
+
+/** The children as they look once the group has been expanded and its snapshot has loaded. */
+// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- minimal fixture: only the fields the selection logic reads are needed
+const loadedChildren = [
+    {transactionID: '1', keyForList: '1', currency: 'USD', amount: -642, report: {reportID: '11'}},
+    {transactionID: '2', keyForList: '2', currency: 'USD', amount: -642, report: {reportID: '11'}},
+] as unknown as TransactionListItemType[];
+
+function Wrapper({children}: {children: React.ReactNode}) {
+    return (
+        <SearchContextProvider>
+            <SearchWriteActionsProvider
+                filteredData={[categoryGroup]}
+                totalSelectableItemsCount={2}
+                searchResults={undefined}
+                transactions={undefined}
+                isMobileSelectionModeEnabled={false}
+                type={CONST.SEARCH.DATA_TYPES.EXPENSE}
+                areItemsGrouped
+                isExpenseReportType={false}
+                isSearchResultsEmpty={false}
+            >
+                {children}
+            </SearchWriteActionsProvider>
+        </SearchContextProvider>
+    );
+}
+
+const renderSelection = () =>
+    renderHook(
+        () => ({
+            ...useSearchSelectionContext(),
+            ...useSearchRowSelectionActions(),
+        }),
+        {wrapper: Wrapper},
+    );
+
+describe('Lazily loaded group selection', () => {
+    beforeAll(() => Onyx.init({keys: ONYXKEYS}));
+
+    beforeEach(async () => {
+        await act(async () => {
+            await Onyx.clear();
+            await waitForBatchedUpdatesWithAct();
+        });
+    });
+
+    it('stores the selection under the group key while the children are still unknown', async () => {
+        const {result} = renderSelection();
+
+        // When the checkbox is pressed before the group has been expanded, so no children are loaded yet
+        await act(async () => {
+            result.current.toggle(categoryGroup, []);
+            await waitForBatchedUpdatesWithAct();
+        });
+
+        // Then the group itself is selected, since there are no transaction IDs to select yet
+        expect(result.current.selectedTransactions[GROUP_KEY]?.isSelected).toBe(true);
+    });
+
+    it('deselects a group that was selected before its children loaded', async () => {
+        const {result} = renderSelection();
+
+        // Given a group selected while it was still collapsed
+        await act(async () => {
+            result.current.toggle(categoryGroup, []);
+            await waitForBatchedUpdatesWithAct();
+        });
+        expect(result.current.selectedTransactions[GROUP_KEY]?.isSelected).toBe(true);
+
+        // When the group is expanded, its children load, and the checkbox is pressed again
+        await act(async () => {
+            result.current.toggle(categoryGroup, loadedChildren);
+            await waitForBatchedUpdatesWithAct();
+        });
+
+        // Then the group-level selection is cleared rather than left behind, so nothing stays selected
+        expect(result.current.selectedTransactions[GROUP_KEY]).toBeUndefined();
+        expect(Object.keys(result.current.selectedTransactions)).toHaveLength(0);
+    });
+
+    it('selects every child of a group that was not already selected once its children loaded', async () => {
+        const {result} = renderSelection();
+
+        // When the checkbox is pressed on an expanded, unselected group whose children have loaded
+        await act(async () => {
+            result.current.toggle(categoryGroup, loadedChildren);
+            await waitForBatchedUpdatesWithAct();
+        });
+
+        // Then each child is selected individually, and the group key is not used
+        expect(result.current.selectedTransactions[GROUP_KEY]).toBeUndefined();
+        expect(result.current.selectedTransactions['1']?.isSelected).toBe(true);
+        expect(result.current.selectedTransactions['2']?.isSelected).toBe(true);
+    });
+});

tests/unit/TransactionGroupListItemTest.tsx

@@ -5,7 +5,14 @@ import {LocaleContextProvider} from '@components/LocaleContextProvider';
 import OnyxListItemProvider from '@components/OnyxListItemProvider';
 import ScreenWrapper from '@components/ScreenWrapper';
 import {SearchContextProvider} from '@components/Search/SearchContextProvider';
-import type {TransactionGroupListItemProps, TransactionListItemType, TransactionReportGroupListItemType} from '@components/Search/SearchList/ListItem/types';
+import type {
+    TransactionCategoryGroupListItemType,
+    TransactionGroupListItemProps,
+    TransactionListItemType,
+    TransactionReportGroupListItemType,
+} from '@components/Search/SearchList/ListItem/types';
+
+import {buildSearchQueryJSON} from '@libs/SearchQueryUtils';
 
 import TransactionGroupListItem from '@src/components/Search/SearchList/ListItem/TransactionGroupListItem';
 import CONST from '@src/CONST';
@@ -634,3 +641,91 @@ describe('Empty Report Selection', () => {
         expect(collapsibleContent).toBeNull();
     });
 });
+
+describe('Lazily loaded group selection', () => {
+    const mockOnSelectRow = jest.fn();
+    const mockOnCheckboxPress = jest.fn();
+
+    // A `group-by:category` group whose child transactions are only fetched once the group is expanded,
+    // so `transactions` is still empty on first render even though the group is not actually empty.
+    const mockCategoryGroup: TransactionCategoryGroupListItemType = {
+        groupedBy: CONST.SEARCH.GROUP_BY.CATEGORY,
+        category: 'Advertising',
+        formattedCategory: 'Advertising',
+        count: 3,
+        total: -1284,
+        currency: 'USD',
+        transactions: [],
+        transactionsQueryJSON: buildSearchQueryJSON('type:expense category:Advertising'),
+        keyForList: 'Advertising',
+    };
+
+    beforeAll(() => {
+        Onyx.init({
+            keys: ONYXKEYS,
+            evictableKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS],
+        });
+        jest.spyOn(NativeNavigation, 'useRoute').mockReturnValue({key: '', name: ''});
+    });
+
+    beforeEach(() => {
+        jest.clearAllMocks();
+        return act(async () => {
+            await Onyx.clear();
+            await waitForBatchedUpdatesWithAct();
+        });
+    });
+
+    const defaultProps: TransactionGroupListItemProps<TransactionCategoryGroupListItemType> = {
+        item: mockCategoryGroup,
+        showTooltip: false,
+        onSelectRow: mockOnSelectRow,
+        onSelectionButtonPress: mockOnCheckboxPress,
+        searchType: CONST.SEARCH.DATA_TYPES.EXPENSE,
+        groupBy: CONST.SEARCH.GROUP_BY.CATEGORY,
+        canSelectMultiple: true,
+        keyForList: 'Advertising',
+    };
+
+    function TestWrapper({children}: {children: React.ReactNode}) {
+        return (
+            <ComposeProviders components={[OnyxListItemProvider, LocaleContextProvider]}>
+                <ScreenWrapper testID="test">
+                    <SearchContextProvider>{children}</SearchContextProvider>
+                </ScreenWrapper>
+            </ComposeProviders>
+        );
+    }
+
+    const renderCategoryGroup = () => render(<TransactionGroupListItem {...defaultProps} />, {wrapper: TestWrapper});
+
+    it('should select the group when tapping the checkbox before the group has been expanded', async () => {
+        renderCategoryGroup();
+        await waitForBatchedUpdatesWithAct();
+
+        // The checkbox of a group whose transactions have not been fetched yet should still be enabled
+        const checkbox = screen.getByRole(CONST.ROLE.CHECKBOX);
+        expect(checkbox).not.toBeDisabled();
+
+        // When pressing it
+        fireEvent.press(checkbox);
+        await waitForBatchedUpdatesWithAct();
+
+        // Then the group should be selected
+        expect(mockOnCheckboxPress).toHaveBeenCalledTimes(1);
+        expect(mockOnCheckboxPress).toHaveBeenCalledWith(mockCategoryGroup, []);
+    });
+
+    it('should expand the group instead of selecting it when tapping the expand arrow', async () => {
+        renderCategoryGroup();
+        await waitForBatchedUpdatesWithAct();
+
+        // When pressing the expand arrow of a group whose transactions have not been fetched yet
+        fireEvent.press(screen.getByLabelText('Expand'));
+        await waitForBatchedUpdatesWithAct();
+
+        // Then the group should expand, and not be selected
+        expect(screen.getByLabelText('Collapse')).toBeTruthy();
+        expect(mockOnSelectRow).not.toHaveBeenCalled();
+    });
+});