From ca6e222566d6f7c1c86a2a0497600aea71c43d5f Mon Sep 17 00:00:00 2001 From: allgandalf Date: Sat, 26 Jul 2025 22:18:43 +0530 Subject: [PATCH 1/6] refactor Search.ts to remove usage of `Onyx.Connect` --- .../Search/ReportListItemHeader.tsx | 15 ++++++- .../Search/TransactionListItem.tsx | 8 +++- src/libs/actions/Search.ts | 41 +++++++----------- .../Search/handleActionButtonPressTest.ts | 43 +++++++++++++++++-- 4 files changed, 75 insertions(+), 32 deletions(-) diff --git a/src/components/SelectionList/Search/ReportListItemHeader.tsx b/src/components/SelectionList/Search/ReportListItemHeader.tsx index 82461715a592..359c2181bde3 100644 --- a/src/components/SelectionList/Search/ReportListItemHeader.tsx +++ b/src/components/SelectionList/Search/ReportListItemHeader.tsx @@ -7,6 +7,7 @@ import ReportSearchHeader from '@components/ReportSearchHeader'; import {useSearchContext} from '@components/Search/SearchContext'; import type {ListItem, TransactionReportGroupListItemType} from '@components/SelectionList/types'; import TextWithTooltip from '@components/TextWithTooltip'; +import useOnyx from '@hooks/useOnyx'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useStyleUtils from '@hooks/useStyleUtils'; import useTheme from '@hooks/useTheme'; @@ -14,6 +15,7 @@ import useThemeStyles from '@hooks/useThemeStyles'; import {convertToDisplayString} from '@libs/CurrencyUtils'; import {handleActionButtonPress} from '@userActions/Search'; import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; import type * as OnyxTypes from '@src/types/onyx'; import ActionCell from './ActionCell'; import UserInfoAndActionButtonRow from './UserInfoAndActionButtonRow'; @@ -169,12 +171,21 @@ function ReportListItemHeader({ const {isLargeScreenWidth, shouldUseNarrowLayout} = useResponsiveLayout(); const thereIsFromAndTo = !!reportItem?.from && !!reportItem?.to; const showUserInfo = (reportItem.type === CONST.REPORT.TYPE.IOU && thereIsFromAndTo) || (reportItem.type === CONST.REPORT.TYPE.EXPENSE && !!reportItem?.from); - + const [snapshot] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${currentSearchHash}`); + const [lastPaymentMethod] = useOnyx(`${ONYXKEYS.NVP_LAST_PAYMENT_METHOD}`); const avatarBorderColor = StyleUtils.getItemBackgroundColorStyle(!!reportItem.isSelected, !!isFocused, !!isDisabled, theme.activeComponentBG, theme.hoverComponentBG)?.backgroundColor ?? theme.highlightBG; const handleOnButtonPress = () => { - handleActionButtonPress(currentSearchHash, reportItem, () => onSelectRow(reportItem as unknown as TItem), shouldUseNarrowLayout && !!canSelectMultiple, currentSearchKey); + handleActionButtonPress( + currentSearchHash, + reportItem, + () => onSelectRow(reportItem as unknown as TItem), + shouldUseNarrowLayout && !!canSelectMultiple, + snapshot, + lastPaymentMethod, + currentSearchKey, + ); }; return !isLargeScreenWidth ? ( diff --git a/src/components/SelectionList/Search/TransactionListItem.tsx b/src/components/SelectionList/Search/TransactionListItem.tsx index c01de802fc42..87a5b44651f8 100644 --- a/src/components/SelectionList/Search/TransactionListItem.tsx +++ b/src/components/SelectionList/Search/TransactionListItem.tsx @@ -9,6 +9,7 @@ import type {ListItem, TransactionListItemProps, TransactionListItemType} from ' import TransactionItemRow from '@components/TransactionItemRow'; import useAnimatedHighlightStyle from '@hooks/useAnimatedHighlightStyle'; import useHover from '@hooks/useHover'; +import useOnyx from '@hooks/useOnyx'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useStyleUtils from '@hooks/useStyleUtils'; import useSyncFocus from '@hooks/useSyncFocus'; @@ -17,6 +18,7 @@ import useThemeStyles from '@hooks/useThemeStyles'; import {handleActionButtonPress as handleActionButtonPressUtil} from '@libs/actions/Search'; import variables from '@styles/variables'; import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; import UserInfoAndActionButtonRow from './UserInfoAndActionButtonRow'; function TransactionListItem({ @@ -38,6 +40,8 @@ function TransactionListItem({ const {isLargeScreenWidth, shouldUseNarrowLayout} = useResponsiveLayout(); const {currentSearchHash, currentSearchKey} = useSearchContext(); + const [snapshot] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${currentSearchHash}`); + const [lastPaymentMethod] = useOnyx(`${ONYXKEYS.NVP_LAST_PAYMENT_METHOD}`); const pressableStyle = [ styles.transactionListItemStyle, @@ -80,8 +84,8 @@ function TransactionListItem({ ); const handleActionButtonPress = useCallback(() => { - handleActionButtonPressUtil(currentSearchHash, transactionItem, () => onSelectRow(item), shouldUseNarrowLayout && !!canSelectMultiple, currentSearchKey); - }, [canSelectMultiple, currentSearchHash, currentSearchKey, item, onSelectRow, shouldUseNarrowLayout, transactionItem]); + handleActionButtonPressUtil(currentSearchHash, transactionItem, () => onSelectRow(item), shouldUseNarrowLayout && !!canSelectMultiple, snapshot, lastPaymentMethod, currentSearchKey); + }, [canSelectMultiple, currentSearchHash, currentSearchKey, item, onSelectRow, shouldUseNarrowLayout, transactionItem, snapshot, lastPaymentMethod]); const handleCheckboxPress = useCallback(() => { onCheckboxPress?.(item); diff --git a/src/libs/actions/Search.ts b/src/libs/actions/Search.ts index d2b27aff7621..455817cc8f2b 100644 --- a/src/libs/actions/Search.ts +++ b/src/libs/actions/Search.ts @@ -1,5 +1,5 @@ import Onyx from 'react-native-onyx'; -import type {OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx'; +import type {OnyxEntry, OnyxUpdate} from 'react-native-onyx'; import type {ValueOf} from 'type-fest'; import type {FormOnyxValues} from '@components/Form/types'; import type {PaymentData, SearchQueryJSON} from '@components/Search/types'; @@ -27,28 +27,13 @@ import type {ConnectionName} from '@src/types/onyx/Policy'; import type {SearchPolicy, SearchReport, SearchTransaction} from '@src/types/onyx/SearchResults'; import type Nullable from '@src/types/utils/Nullable'; -let lastPaymentMethod: OnyxEntry; -Onyx.connect({ - key: ONYXKEYS.NVP_LAST_PAYMENT_METHOD, - callback: (val) => { - lastPaymentMethod = val; - }, -}); - -let allSnapshots: OnyxCollection; -Onyx.connect({ - key: ONYXKEYS.COLLECTION.SNAPSHOT, - callback: (val) => { - allSnapshots = val; - }, - waitForCollectionCallback: true, -}); - function handleActionButtonPress( hash: number, item: TransactionListItemType | TransactionReportGroupListItemType, goToItem: () => void, isInMobileSelectionMode: boolean, + snapshot: OnyxEntry, + lastPaymentMethod: OnyxEntry, currentSearchKey?: SuggestedSearchKey, ) { // The transactionIDList is needed to handle actions taken on `status:""` where transactions on single expense reports can be approved/paid. @@ -56,7 +41,6 @@ function handleActionButtonPress( const transactionID = isTransactionListItemType(item) ? [item.transactionID] : undefined; const allReportTransactions = (isTransactionGroupListItemType(item) ? item.transactions : [item]) as SearchTransaction[]; const hasHeldExpense = hasHeldExpenses('', allReportTransactions); - if (hasHeldExpense || isInMobileSelectionMode) { goToItem(); return; @@ -64,13 +48,13 @@ function handleActionButtonPress( switch (item.action) { case CONST.SEARCH.ACTION_TYPES.PAY: - getPayActionCallback(hash, item, goToItem, currentSearchKey); + getPayActionCallback(hash, item, goToItem, snapshot, lastPaymentMethod, currentSearchKey); return; case CONST.SEARCH.ACTION_TYPES.APPROVE: approveMoneyRequestOnSearch(hash, [item.reportID], transactionID, currentSearchKey); return; case CONST.SEARCH.ACTION_TYPES.SUBMIT: { - const policy = (allSnapshots?.[`${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`]?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`] ?? {}) as SearchPolicy; + const policy = (snapshot?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`] ?? {}) as SearchPolicy; submitMoneyRequestOnSearch(hash, [item], [policy], transactionID, currentSearchKey); return; } @@ -79,7 +63,7 @@ function handleActionButtonPress( return; } - const policy = (allSnapshots?.[`${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`]?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`] ?? {}) as Policy; + const policy = (snapshot?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`] ?? {}) as Policy; const connectedIntegration = getValidConnectedIntegration(policy); if (!connectedIntegration) { @@ -108,7 +92,14 @@ function getLastPolicyPaymentMethod(policyID: string | undefined, lastPaymentMet return lastPolicyPaymentMethod; } -function getPayActionCallback(hash: number, item: TransactionListItemType | TransactionReportGroupListItemType, goToItem: () => void, currentSearchKey?: SuggestedSearchKey) { +function getPayActionCallback( + hash: number, + item: TransactionListItemType | TransactionReportGroupListItemType, + goToItem: () => void, + snapshot: OnyxEntry, + lastPaymentMethod: OnyxEntry, + currentSearchKey?: SuggestedSearchKey, +) { const lastPolicyPaymentMethod = getLastPolicyPaymentMethod(item.policyID, lastPaymentMethod); if (!lastPolicyPaymentMethod) { @@ -116,7 +107,7 @@ function getPayActionCallback(hash: number, item: TransactionListItemType | Tran return; } - const report = (allSnapshots?.[`${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`]?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`] ?? {}) as SearchReport; + const report = (snapshot?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`] ?? {}) as SearchReport; const amount = Math.abs((report?.total ?? 0) - (report?.nonReimbursableTotal ?? 0)); const transactionID = isTransactionListItemType(item) ? [item.transactionID] : undefined; @@ -125,7 +116,7 @@ function getPayActionCallback(hash: number, item: TransactionListItemType | Tran return; } - const hasVBBA = !!allSnapshots?.[`${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`]?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`]?.achAccount?.bankAccountID; + const hasVBBA = !!snapshot?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`]?.achAccount?.bankAccountID; if (hasVBBA) { payMoneyRequestOnSearch(hash, [{reportID: item.reportID, amount, paymentType: lastPolicyPaymentMethod}], transactionID, currentSearchKey); return; diff --git a/tests/unit/Search/handleActionButtonPressTest.ts b/tests/unit/Search/handleActionButtonPressTest.ts index bdd52532dd5b..cefb74327885 100644 --- a/tests/unit/Search/handleActionButtonPressTest.ts +++ b/tests/unit/Search/handleActionButtonPressTest.ts @@ -1,6 +1,10 @@ /* eslint-disable @typescript-eslint/naming-convention */ +import type {OnyxEntry} from 'react-native-onyx'; +import Onyx from 'react-native-onyx'; import type {TransactionReportGroupListItemType} from '@components/SelectionList/types'; import {handleActionButtonPress} from '@libs/actions/Search'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {LastPaymentMethod, SearchResults} from '@src/types/onyx'; jest.mock('@src/components/ConfirmedRoute.tsx'); @@ -208,23 +212,56 @@ const updatedMockReportItem = { }), }; +const mockSnapshotForItem: OnyxEntry = { + // @ts-expect-error: Allow partial record in snapshot update for testing + data: { + [`${ONYXKEYS.COLLECTION.POLICY}${mockReportItemWithHold?.policyID}`]: { + ...(mockReportItemWithHold.policyID + ? { + [String(mockReportItemWithHold.policyID)]: { + type: 'policy', + id: String(mockReportItemWithHold.policyID), + role: 'admin', + owner: 'apb@apb.com', + ...mockReportItemWithHold, + }, + } + : {}), + }, + }, +}; + +const mockLastPaymentMethod: OnyxEntry = { + expense: 'Elsewhere', + lastUsed: 'Elsewhere', +}; + describe('handleActionButtonPress', () => { const searchHash = 1; + beforeAll(() => { + Onyx.merge( + `${ONYXKEYS.COLLECTION.SNAPSHOT}${searchHash}`, + // @ts-expect-error: Allow partial record in snapshot update for testing + mockSnapshotForItem, + ); + Onyx.merge(ONYXKEYS.NVP_LAST_PAYMENT_METHOD, mockLastPaymentMethod); + }); + test('Should navigate to item when report has one transaction on hold', () => { const goToItem = jest.fn(() => {}); - handleActionButtonPress(searchHash, mockReportItemWithHold, goToItem, false); + handleActionButtonPress(searchHash, mockReportItemWithHold, goToItem, false, mockSnapshotForItem, mockLastPaymentMethod); expect(goToItem).toHaveBeenCalledTimes(1); }); test('Should not navigate to item when the hold is removed', () => { const goToItem = jest.fn(() => {}); - handleActionButtonPress(searchHash, updatedMockReportItem, goToItem, false); + handleActionButtonPress(searchHash, updatedMockReportItem, goToItem, false, mockSnapshotForItem, mockLastPaymentMethod); expect(goToItem).toHaveBeenCalledTimes(0); }); test('Should run goToItem callback when user is in mobile selection mode', () => { const goToItem = jest.fn(() => {}); - handleActionButtonPress(searchHash, updatedMockReportItem, goToItem, true); + handleActionButtonPress(searchHash, updatedMockReportItem, goToItem, true, mockSnapshotForItem, mockLastPaymentMethod); expect(goToItem).toHaveBeenCalledTimes(1); }); }); From 35587211f1096aa6e4d05395143b54f1b7f89a54 Mon Sep 17 00:00:00 2001 From: allgandalf Date: Sat, 26 Jul 2025 22:25:18 +0530 Subject: [PATCH 2/6] fix failing esLint --- src/components/SelectionList/Search/ReportListItemHeader.tsx | 4 ++-- src/components/SelectionList/Search/TransactionListItem.tsx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/SelectionList/Search/ReportListItemHeader.tsx b/src/components/SelectionList/Search/ReportListItemHeader.tsx index 359c2181bde3..05d40ab2d2ed 100644 --- a/src/components/SelectionList/Search/ReportListItemHeader.tsx +++ b/src/components/SelectionList/Search/ReportListItemHeader.tsx @@ -171,8 +171,8 @@ function ReportListItemHeader({ const {isLargeScreenWidth, shouldUseNarrowLayout} = useResponsiveLayout(); const thereIsFromAndTo = !!reportItem?.from && !!reportItem?.to; const showUserInfo = (reportItem.type === CONST.REPORT.TYPE.IOU && thereIsFromAndTo) || (reportItem.type === CONST.REPORT.TYPE.EXPENSE && !!reportItem?.from); - const [snapshot] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${currentSearchHash}`); - const [lastPaymentMethod] = useOnyx(`${ONYXKEYS.NVP_LAST_PAYMENT_METHOD}`); + const [snapshot] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${currentSearchHash}`, {canBeMissing: true}); + const [lastPaymentMethod] = useOnyx(`${ONYXKEYS.NVP_LAST_PAYMENT_METHOD}`, {canBeMissing: true}); const avatarBorderColor = StyleUtils.getItemBackgroundColorStyle(!!reportItem.isSelected, !!isFocused, !!isDisabled, theme.activeComponentBG, theme.hoverComponentBG)?.backgroundColor ?? theme.highlightBG; diff --git a/src/components/SelectionList/Search/TransactionListItem.tsx b/src/components/SelectionList/Search/TransactionListItem.tsx index 77eb7fc9d392..b213b433b654 100644 --- a/src/components/SelectionList/Search/TransactionListItem.tsx +++ b/src/components/SelectionList/Search/TransactionListItem.tsx @@ -39,8 +39,8 @@ function TransactionListItem({ const {isLargeScreenWidth, shouldUseNarrowLayout} = useResponsiveLayout(); const {currentSearchHash, currentSearchKey} = useSearchContext(); - const [snapshot] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${currentSearchHash}`); - const [lastPaymentMethod] = useOnyx(`${ONYXKEYS.NVP_LAST_PAYMENT_METHOD}`); + const [snapshot] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${currentSearchHash}`, {canBeMissing: true}); + const [lastPaymentMethod] = useOnyx(`${ONYXKEYS.NVP_LAST_PAYMENT_METHOD}`, {canBeMissing: true}); const pressableStyle = [ styles.transactionListItemStyle, From 4b9c8e634a30aa870b9ee498b7855a4da9ee8fa7 Mon Sep 17 00:00:00 2001 From: allgandalf Date: Thu, 31 Jul 2025 17:39:10 +0530 Subject: [PATCH 3/6] fix ts failures --- src/libs/actions/Search.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/Search.ts b/src/libs/actions/Search.ts index ecad99f85b53..c56263a67de1 100644 --- a/src/libs/actions/Search.ts +++ b/src/libs/actions/Search.ts @@ -34,13 +34,14 @@ function handleActionButtonPress( isInMobileSelectionMode: boolean, snapshot: OnyxEntry, lastPaymentMethod: OnyxEntry, - currentSearchKey?: SuggestedSearchKey, + currentSearchKey?: SearchKey, ) { // The transactionIDList is needed to handle actions taken on `status:""` where transactions on single expense reports can be approved/paid. // We need the transactionID to display the loading indicator for that list item's action. const transactionID = isTransactionListItemType(item) ? [item.transactionID] : undefined; const allReportTransactions = (isTransactionGroupListItemType(item) ? item.transactions : [item]) as SearchTransaction[]; const hasHeldExpense = hasHeldExpenses('', allReportTransactions); + if (hasHeldExpense || isInMobileSelectionMode) { goToItem(); return; @@ -98,7 +99,7 @@ function getPayActionCallback( goToItem: () => void, snapshot: OnyxEntry, lastPaymentMethod: OnyxEntry, - currentSearchKey?: SuggestedSearchKey, + currentSearchKey?: SearchKey, ) { const lastPolicyPaymentMethod = getLastPolicyPaymentMethod(item.policyID, lastPaymentMethod); From 03cabab0b85e9475a338c27782462c9e7028f07c Mon Sep 17 00:00:00 2001 From: allgandalf Date: Fri, 1 Aug 2025 12:24:56 +0530 Subject: [PATCH 4/6] address reviewer comments --- .../Search/ReportListItemHeader.tsx | 2 +- .../SelectionList/Search/TransactionListItem.tsx | 10 +++++++++- src/libs/actions/Search.ts | 16 ++++++++-------- tests/unit/Search/handleActionButtonPressTest.ts | 6 +++--- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/components/SelectionList/Search/ReportListItemHeader.tsx b/src/components/SelectionList/Search/ReportListItemHeader.tsx index 05d40ab2d2ed..f3477f35d53f 100644 --- a/src/components/SelectionList/Search/ReportListItemHeader.tsx +++ b/src/components/SelectionList/Search/ReportListItemHeader.tsx @@ -182,7 +182,7 @@ function ReportListItemHeader({ reportItem, () => onSelectRow(reportItem as unknown as TItem), shouldUseNarrowLayout && !!canSelectMultiple, - snapshot, + snapshot?.data, lastPaymentMethod, currentSearchKey, ); diff --git a/src/components/SelectionList/Search/TransactionListItem.tsx b/src/components/SelectionList/Search/TransactionListItem.tsx index b213b433b654..2d12dd83688d 100644 --- a/src/components/SelectionList/Search/TransactionListItem.tsx +++ b/src/components/SelectionList/Search/TransactionListItem.tsx @@ -83,7 +83,15 @@ function TransactionListItem({ ); const handleActionButtonPress = useCallback(() => { - handleActionButtonPressUtil(currentSearchHash, transactionItem, () => onSelectRow(item), shouldUseNarrowLayout && !!canSelectMultiple, snapshot, lastPaymentMethod, currentSearchKey); + handleActionButtonPressUtil( + currentSearchHash, + transactionItem, + () => onSelectRow(item), + shouldUseNarrowLayout && !!canSelectMultiple, + snapshot?.data, + lastPaymentMethod, + currentSearchKey, + ); }, [canSelectMultiple, currentSearchHash, currentSearchKey, item, onSelectRow, shouldUseNarrowLayout, transactionItem, snapshot, lastPaymentMethod]); const handleCheckboxPress = useCallback(() => { diff --git a/src/libs/actions/Search.ts b/src/libs/actions/Search.ts index c56263a67de1..fef1577c64af 100644 --- a/src/libs/actions/Search.ts +++ b/src/libs/actions/Search.ts @@ -32,7 +32,7 @@ function handleActionButtonPress( item: TransactionListItemType | TransactionReportGroupListItemType, goToItem: () => void, isInMobileSelectionMode: boolean, - snapshot: OnyxEntry, + snapshotData: SearchResults['data'] | undefined, lastPaymentMethod: OnyxEntry, currentSearchKey?: SearchKey, ) { @@ -42,20 +42,20 @@ function handleActionButtonPress( const allReportTransactions = (isTransactionGroupListItemType(item) ? item.transactions : [item]) as SearchTransaction[]; const hasHeldExpense = hasHeldExpenses('', allReportTransactions); - if (hasHeldExpense || isInMobileSelectionMode) { + if (hasHeldExpense || isInMobileSelectionMode || !snapshotData) { goToItem(); return; } switch (item.action) { case CONST.SEARCH.ACTION_TYPES.PAY: - getPayActionCallback(hash, item, goToItem, snapshot, lastPaymentMethod, currentSearchKey); + getPayActionCallback(hash, item, goToItem, snapshotData, lastPaymentMethod, currentSearchKey); return; case CONST.SEARCH.ACTION_TYPES.APPROVE: approveMoneyRequestOnSearch(hash, [item.reportID], transactionID, currentSearchKey); return; case CONST.SEARCH.ACTION_TYPES.SUBMIT: { - const policy = (snapshot?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`] ?? {}) as SearchPolicy; + const policy = snapshotData?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`] ?? {}; submitMoneyRequestOnSearch(hash, [item], [policy], transactionID, currentSearchKey); return; } @@ -64,7 +64,7 @@ function handleActionButtonPress( return; } - const policy = (snapshot?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`] ?? {}) as Policy; + const policy = (snapshotData?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`] ?? {}) as Policy; const connectedIntegration = getValidConnectedIntegration(policy); if (!connectedIntegration) { @@ -97,7 +97,7 @@ function getPayActionCallback( hash: number, item: TransactionListItemType | TransactionReportGroupListItemType, goToItem: () => void, - snapshot: OnyxEntry, + snapshotData: SearchResults['data'], lastPaymentMethod: OnyxEntry, currentSearchKey?: SearchKey, ) { @@ -108,7 +108,7 @@ function getPayActionCallback( return; } - const report = (snapshot?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`] ?? {}) as SearchReport; + const report = snapshotData?.[`${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`] ?? {}; const amount = Math.abs((report?.total ?? 0) - (report?.nonReimbursableTotal ?? 0)); const transactionID = isTransactionListItemType(item) ? [item.transactionID] : undefined; @@ -117,7 +117,7 @@ function getPayActionCallback( return; } - const hasVBBA = !!snapshot?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`]?.achAccount?.bankAccountID; + const hasVBBA = !!snapshotData?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`]?.achAccount?.bankAccountID; if (hasVBBA) { payMoneyRequestOnSearch(hash, [{reportID: item.reportID, amount, paymentType: lastPolicyPaymentMethod}], transactionID, currentSearchKey); return; diff --git a/tests/unit/Search/handleActionButtonPressTest.ts b/tests/unit/Search/handleActionButtonPressTest.ts index cefb74327885..ec48f5f883ee 100644 --- a/tests/unit/Search/handleActionButtonPressTest.ts +++ b/tests/unit/Search/handleActionButtonPressTest.ts @@ -249,19 +249,19 @@ describe('handleActionButtonPress', () => { test('Should navigate to item when report has one transaction on hold', () => { const goToItem = jest.fn(() => {}); - handleActionButtonPress(searchHash, mockReportItemWithHold, goToItem, false, mockSnapshotForItem, mockLastPaymentMethod); + handleActionButtonPress(searchHash, mockReportItemWithHold, goToItem, false, mockSnapshotForItem?.data, mockLastPaymentMethod); expect(goToItem).toHaveBeenCalledTimes(1); }); test('Should not navigate to item when the hold is removed', () => { const goToItem = jest.fn(() => {}); - handleActionButtonPress(searchHash, updatedMockReportItem, goToItem, false, mockSnapshotForItem, mockLastPaymentMethod); + handleActionButtonPress(searchHash, updatedMockReportItem, goToItem, false, mockSnapshotForItem?.data, mockLastPaymentMethod); expect(goToItem).toHaveBeenCalledTimes(0); }); test('Should run goToItem callback when user is in mobile selection mode', () => { const goToItem = jest.fn(() => {}); - handleActionButtonPress(searchHash, updatedMockReportItem, goToItem, true, mockSnapshotForItem, mockLastPaymentMethod); + handleActionButtonPress(searchHash, updatedMockReportItem, goToItem, true, mockSnapshotForItem?.data, mockLastPaymentMethod); expect(goToItem).toHaveBeenCalledTimes(1); }); }); From ab1e0f22ec7284a71a10c3f8de9e92572ddb9eb3 Mon Sep 17 00:00:00 2001 From: allgandalf Date: Fri, 1 Aug 2025 14:02:15 +0530 Subject: [PATCH 5/6] address reviewers comment --- .../Search/ReportListItemHeader.tsx | 12 ++++++++-- .../Search/TransactionListItem.tsx | 13 +++++++++-- src/libs/actions/Search.ts | 22 +++++++++---------- .../Search/handleActionButtonPressTest.ts | 12 +++++++--- 4 files changed, 41 insertions(+), 18 deletions(-) diff --git a/src/components/SelectionList/Search/ReportListItemHeader.tsx b/src/components/SelectionList/Search/ReportListItemHeader.tsx index f3477f35d53f..c008c56a6b4f 100644 --- a/src/components/SelectionList/Search/ReportListItemHeader.tsx +++ b/src/components/SelectionList/Search/ReportListItemHeader.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {useMemo} from 'react'; import {View} from 'react-native'; import type {ColorValue} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; @@ -17,6 +17,7 @@ import {handleActionButtonPress} from '@userActions/Search'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type * as OnyxTypes from '@src/types/onyx'; +import type {SearchPolicy, SearchReport} from '@src/types/onyx/SearchResults'; import ActionCell from './ActionCell'; import UserInfoAndActionButtonRow from './UserInfoAndActionButtonRow'; @@ -172,6 +173,12 @@ function ReportListItemHeader({ const thereIsFromAndTo = !!reportItem?.from && !!reportItem?.to; const showUserInfo = (reportItem.type === CONST.REPORT.TYPE.IOU && thereIsFromAndTo) || (reportItem.type === CONST.REPORT.TYPE.EXPENSE && !!reportItem?.from); const [snapshot] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${currentSearchHash}`, {canBeMissing: true}); + const snapshotReport = useMemo(() => { + return (snapshot?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${reportItem.reportID}`] ?? {}) as SearchReport; + }, [snapshot, reportItem.reportID]); + const snapshotPolicy = useMemo(() => { + return (snapshot?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${reportItem.policyID}`] ?? {}) as SearchPolicy; + }, [snapshot, reportItem.policyID]); const [lastPaymentMethod] = useOnyx(`${ONYXKEYS.NVP_LAST_PAYMENT_METHOD}`, {canBeMissing: true}); const avatarBorderColor = StyleUtils.getItemBackgroundColorStyle(!!reportItem.isSelected, !!isFocused, !!isDisabled, theme.activeComponentBG, theme.hoverComponentBG)?.backgroundColor ?? theme.highlightBG; @@ -182,7 +189,8 @@ function ReportListItemHeader({ reportItem, () => onSelectRow(reportItem as unknown as TItem), shouldUseNarrowLayout && !!canSelectMultiple, - snapshot?.data, + snapshotReport, + snapshotPolicy, lastPaymentMethod, currentSearchKey, ); diff --git a/src/components/SelectionList/Search/TransactionListItem.tsx b/src/components/SelectionList/Search/TransactionListItem.tsx index 2d12dd83688d..a6a952a6fed8 100644 --- a/src/components/SelectionList/Search/TransactionListItem.tsx +++ b/src/components/SelectionList/Search/TransactionListItem.tsx @@ -18,6 +18,7 @@ import {handleActionButtonPress as handleActionButtonPressUtil} from '@libs/acti import variables from '@styles/variables'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; +import type {SearchPolicy, SearchReport} from '@src/types/onyx/SearchResults'; import UserInfoAndActionButtonRow from './UserInfoAndActionButtonRow'; function TransactionListItem({ @@ -40,6 +41,13 @@ function TransactionListItem({ const {isLargeScreenWidth, shouldUseNarrowLayout} = useResponsiveLayout(); const {currentSearchHash, currentSearchKey} = useSearchContext(); const [snapshot] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${currentSearchHash}`, {canBeMissing: true}); + const snapshotReport = useMemo(() => { + return (snapshot?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionItem.reportID}`] ?? {}) as SearchReport; + }, [snapshot, transactionItem.reportID]); + + const snapshotPolicy = useMemo(() => { + return (snapshot?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${transactionItem.policyID}`] ?? {}) as SearchPolicy; + }, [snapshot, transactionItem.policyID]); const [lastPaymentMethod] = useOnyx(`${ONYXKEYS.NVP_LAST_PAYMENT_METHOD}`, {canBeMissing: true}); const pressableStyle = [ @@ -88,11 +96,12 @@ function TransactionListItem({ transactionItem, () => onSelectRow(item), shouldUseNarrowLayout && !!canSelectMultiple, - snapshot?.data, + snapshotReport, + snapshotPolicy, lastPaymentMethod, currentSearchKey, ); - }, [canSelectMultiple, currentSearchHash, currentSearchKey, item, onSelectRow, shouldUseNarrowLayout, transactionItem, snapshot, lastPaymentMethod]); + }, [currentSearchHash, transactionItem, shouldUseNarrowLayout, canSelectMultiple, snapshotReport, snapshotPolicy, lastPaymentMethod, currentSearchKey, onSelectRow, item]); const handleCheckboxPress = useCallback(() => { onCheckboxPress?.(item); diff --git a/src/libs/actions/Search.ts b/src/libs/actions/Search.ts index fef1577c64af..96e1465118fb 100644 --- a/src/libs/actions/Search.ts +++ b/src/libs/actions/Search.ts @@ -22,7 +22,7 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import {FILTER_KEYS} from '@src/types/form/SearchAdvancedFiltersForm'; import type {SearchAdvancedFiltersForm} from '@src/types/form/SearchAdvancedFiltersForm'; -import type {LastPaymentMethod, LastPaymentMethodType, Policy, SearchResults} from '@src/types/onyx'; +import type {LastPaymentMethod, LastPaymentMethodType, Policy} from '@src/types/onyx'; import type {ConnectionName} from '@src/types/onyx/Policy'; import type {SearchPolicy, SearchReport, SearchTransaction} from '@src/types/onyx/SearchResults'; import type Nullable from '@src/types/utils/Nullable'; @@ -32,7 +32,8 @@ function handleActionButtonPress( item: TransactionListItemType | TransactionReportGroupListItemType, goToItem: () => void, isInMobileSelectionMode: boolean, - snapshotData: SearchResults['data'] | undefined, + snapshotReport: SearchReport, + snapshotPolicy: SearchPolicy, lastPaymentMethod: OnyxEntry, currentSearchKey?: SearchKey, ) { @@ -42,21 +43,20 @@ function handleActionButtonPress( const allReportTransactions = (isTransactionGroupListItemType(item) ? item.transactions : [item]) as SearchTransaction[]; const hasHeldExpense = hasHeldExpenses('', allReportTransactions); - if (hasHeldExpense || isInMobileSelectionMode || !snapshotData) { + if (hasHeldExpense || isInMobileSelectionMode) { goToItem(); return; } switch (item.action) { case CONST.SEARCH.ACTION_TYPES.PAY: - getPayActionCallback(hash, item, goToItem, snapshotData, lastPaymentMethod, currentSearchKey); + getPayActionCallback(hash, item, goToItem, snapshotReport, snapshotPolicy, lastPaymentMethod, currentSearchKey); return; case CONST.SEARCH.ACTION_TYPES.APPROVE: approveMoneyRequestOnSearch(hash, [item.reportID], transactionID, currentSearchKey); return; case CONST.SEARCH.ACTION_TYPES.SUBMIT: { - const policy = snapshotData?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`] ?? {}; - submitMoneyRequestOnSearch(hash, [item], [policy], transactionID, currentSearchKey); + submitMoneyRequestOnSearch(hash, [item], [snapshotPolicy], transactionID, currentSearchKey); return; } case CONST.SEARCH.ACTION_TYPES.EXPORT_TO_ACCOUNTING: { @@ -64,7 +64,7 @@ function handleActionButtonPress( return; } - const policy = (snapshotData?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`] ?? {}) as Policy; + const policy = (snapshotPolicy ?? {}) as Policy; const connectedIntegration = getValidConnectedIntegration(policy); if (!connectedIntegration) { @@ -97,7 +97,8 @@ function getPayActionCallback( hash: number, item: TransactionListItemType | TransactionReportGroupListItemType, goToItem: () => void, - snapshotData: SearchResults['data'], + snapshotReport: SearchReport, + snapshotPolicy: SearchPolicy, lastPaymentMethod: OnyxEntry, currentSearchKey?: SearchKey, ) { @@ -108,8 +109,7 @@ function getPayActionCallback( return; } - const report = snapshotData?.[`${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`] ?? {}; - const amount = Math.abs((report?.total ?? 0) - (report?.nonReimbursableTotal ?? 0)); + const amount = Math.abs((snapshotReport?.total ?? 0) - (snapshotReport?.nonReimbursableTotal ?? 0)); const transactionID = isTransactionListItemType(item) ? [item.transactionID] : undefined; if (lastPolicyPaymentMethod === CONST.IOU.PAYMENT_TYPE.ELSEWHERE) { @@ -117,7 +117,7 @@ function getPayActionCallback( return; } - const hasVBBA = !!snapshotData?.[`${ONYXKEYS.COLLECTION.POLICY}${item.policyID}`]?.achAccount?.bankAccountID; + const hasVBBA = !!snapshotPolicy?.achAccount?.bankAccountID; if (hasVBBA) { payMoneyRequestOnSearch(hash, [{reportID: item.reportID, amount, paymentType: lastPolicyPaymentMethod}], transactionID, currentSearchKey); return; diff --git a/tests/unit/Search/handleActionButtonPressTest.ts b/tests/unit/Search/handleActionButtonPressTest.ts index ec48f5f883ee..fa6c8258b3ab 100644 --- a/tests/unit/Search/handleActionButtonPressTest.ts +++ b/tests/unit/Search/handleActionButtonPressTest.ts @@ -247,21 +247,27 @@ describe('handleActionButtonPress', () => { Onyx.merge(ONYXKEYS.NVP_LAST_PAYMENT_METHOD, mockLastPaymentMethod); }); + const snapshotReport = mockSnapshotForItem?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${mockReportItemWithHold.reportID}`] ?? {}; + const snapshotPolicy = mockSnapshotForItem?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${mockReportItemWithHold.policyID}`] ?? {}; + test('Should navigate to item when report has one transaction on hold', () => { const goToItem = jest.fn(() => {}); - handleActionButtonPress(searchHash, mockReportItemWithHold, goToItem, false, mockSnapshotForItem?.data, mockLastPaymentMethod); + // @ts-expect-error: Allow partial record in snapshot update for testing + handleActionButtonPress(searchHash, mockReportItemWithHold, goToItem, false, snapshotReport, snapshotPolicy, mockLastPaymentMethod); expect(goToItem).toHaveBeenCalledTimes(1); }); test('Should not navigate to item when the hold is removed', () => { const goToItem = jest.fn(() => {}); - handleActionButtonPress(searchHash, updatedMockReportItem, goToItem, false, mockSnapshotForItem?.data, mockLastPaymentMethod); + // @ts-expect-error: Allow partial record in snapshot update for testing + handleActionButtonPress(searchHash, updatedMockReportItem, goToItem, false, snapshotReport, snapshotPolicy, mockLastPaymentMethod); expect(goToItem).toHaveBeenCalledTimes(0); }); test('Should run goToItem callback when user is in mobile selection mode', () => { const goToItem = jest.fn(() => {}); - handleActionButtonPress(searchHash, updatedMockReportItem, goToItem, true, mockSnapshotForItem?.data, mockLastPaymentMethod); + // @ts-expect-error: Allow partial record in snapshot update for testing + handleActionButtonPress(searchHash, updatedMockReportItem, goToItem, true, snapshotReport, snapshotPolicy, mockLastPaymentMethod); expect(goToItem).toHaveBeenCalledTimes(1); }); }); From 76f843efa88696fa3722390736d297ce1498fb63 Mon Sep 17 00:00:00 2001 From: Gandalf Date: Fri, 1 Aug 2025 14:03:25 +0530 Subject: [PATCH 6/6] Update package.json --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 50cc3a255de3..169dc7e7404a 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "test:debug": "TZ=utc NODE_OPTIONS='--inspect-brk --experimental-vm-modules' jest --runInBand", "perf-test": "NODE_OPTIONS=--experimental-vm-modules npx reassure", "typecheck": "NODE_OPTIONS=--max_old_space_size=8192 tsc", - "lint": "NODE_OPTIONS=--max_old_space_size=8192 eslint . --max-warnings=329 --cache --cache-location=node_modules/.cache/eslint", + "lint": "NODE_OPTIONS=--max_old_space_size=8192 eslint . --max-warnings=327 --cache --cache-location=node_modules/.cache/eslint", "lint-changed": "NODE_OPTIONS=--max_old_space_size=8192 ./scripts/lintChanged.sh", "lint-watch": "npx eslint-watch --watch --changed", "shellcheck": "./scripts/shellCheck.sh",