From f4e42717d9a4fbd604670ff882609433cc0036db Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Thu, 29 May 2025 12:00:22 +0200 Subject: [PATCH 1/4] Revert "Revert "[Better Expense Report Views] Display transactions and IOU actions only if linked to the report"" This reverts commit 7d29429f62c05c0ae0afac38cb46c990e9a50771. --- .../MoneyRequestReportActionsList.tsx | 9 ++++-- src/libs/ReportActionsUtils.ts | 30 ++++++++++++++++++- src/pages/home/ReportScreen.tsx | 3 +- src/pages/home/report/ReportActionsView.tsx | 14 +++++++-- 4 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index fac4ad177c63..52e44dc7e4ff 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -39,6 +39,7 @@ import { isConsecutiveChronosAutomaticTimerAction, isCurrentActionUnread, isDeletedParentAction, + isIOUActionMatchingTransactionList, shouldReportActionBeVisible, wasMessageReceivedWhileOffline, } from '@libs/ReportActionsUtils'; @@ -122,6 +123,7 @@ function MoneyRequestReportActionsList({ const [isVisible, setIsVisible] = useState(Visibility.isVisible); const isFocused = useIsFocused(); const route = useRoute>(); + const reportTransactionIDs = transactions.map((transaction) => transaction.transactionID); const reportID = report?.reportID; const linkedReportActionID = route?.params?.reportActionID; @@ -133,7 +135,7 @@ function MoneyRequestReportActionsList({ }); const mostRecentIOUReportActionID = useMemo(() => getMostRecentIOURequestActionID(reportActions), [reportActions]); - const transactionThreadReportID = getOneTransactionThreadReportID(reportID, reportActions ?? [], false); + const transactionThreadReportID = getOneTransactionThreadReportID(reportID, reportActions ?? [], false, reportTransactionIDs); const firstVisibleReportActionID = useMemo(() => getFirstVisibleReportActionID(reportActions, isOffline), [reportActions, isOffline]); const [transactionThreadReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`, {canBeMissing: true}); const [currentUserAccountID] = useOnyx(ONYXKEYS.SESSION, {canBeMissing: false, selector: (session) => session?.accountID}); @@ -163,12 +165,13 @@ function MoneyRequestReportActionsList({ return ( isActionVisibleOnMoneyReport && (isOffline || isDeletedParentAction(reportAction) || reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || reportAction.errors) && - shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canPerformWriteAction) + shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canPerformWriteAction) && + isIOUActionMatchingTransactionList(reportAction, reportTransactionIDs) ); }); return filteredActions.toReversed(); - }, [reportActions, isOffline, canPerformWriteAction]); + }, [reportActions, isOffline, canPerformWriteAction, reportTransactionIDs]); const reportActionSize = useRef(visibleReportActions.length); const lastAction = visibleReportActions.at(-1); diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index 7b046ae3ea84..786a2b84349e 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -1191,6 +1191,32 @@ function isTagModificationAction(actionName: string): boolean { ); } +/** + * Determines whether the given action is an IOU and, if a list of report transaction IDs is provided, + * whether it corresponds to one of those transactions. + * + * For compatibility and to avoid using isMoneyRequest next to this function as it is checked here already: + * - If the action is not a money request and `defaultToFalseForNonIOU` is false (default), the result is true. + * - If no `reportTransactionIDs` are provided, the function returns true if the action is an IOU. + * - If `reportTransactionIDs` are provided, the function checks if the IOU transaction ID from the action matches any of them. + */ +const isIOUActionMatchingTransactionList = ( + action: ReportAction, + reportTransactionIDs?: string[], + defaultToFalseForNonIOU = false, +): action is ReportAction => { + if (!isMoneyRequestAction(action)) { + return !defaultToFalseForNonIOU; + } + + if (reportTransactionIDs === undefined) { + return true; + } + + const {IOUTransactionID} = getOriginalMessage(action) ?? {}; + return !!IOUTransactionID && reportTransactionIDs.includes(IOUTransactionID); +}; + /** * Gets the reportID for the transaction thread associated with a report by iterating over the reportActions and identifying the IOU report actions. * Returns a reportID if there is exactly one transaction thread for the report, and null otherwise. @@ -1199,6 +1225,7 @@ function getOneTransactionThreadReportID( reportID: string | undefined, reportActions: OnyxEntry | ReportAction[], isOffline: boolean | undefined = undefined, + reportTransactionIDs?: string[], ): string | undefined { // If the report is not an IOU, Expense report, or Invoice, it shouldn't be treated as one-transaction report. const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; @@ -1213,7 +1240,7 @@ function getOneTransactionThreadReportID( const iouRequestActions = []; for (const action of reportActionsArray) { - if (!isMoneyRequestAction(action)) { + if (!isIOUActionMatchingTransactionList(action, reportTransactionIDs, true)) { // eslint-disable-next-line no-continue continue; } @@ -2531,6 +2558,7 @@ export { isForwardedAction, isWhisperActionTargetedToOthers, isTagModificationAction, + isIOUActionMatchingTransactionList, isResolvedActionableWhisper, shouldHideNewMarker, shouldReportActionBeVisible, diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 1029581065ac..20aca664b3a6 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -297,7 +297,8 @@ function ReportScreen({route, navigation}: ReportScreenProps) { selector: (allTransactions): OnyxTypes.Transaction[] => selectAllTransactionsForReport(allTransactions, reportIDFromRoute, reportActions), canBeMissing: false, }); - const transactionThreadReportID = getOneTransactionThreadReportID(reportID, reportActions ?? [], isOffline); + const reportTransactionIDs = reportTransactions?.map((transaction) => transaction.transactionID); + const transactionThreadReportID = getOneTransactionThreadReportID(reportID, reportActions ?? [], isOffline, reportTransactionIDs); const [transactionThreadReportActions = {}] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThreadReportID}`, {canBeMissing: true}); const combinedReportActions = getCombinedReportActions(reportActions, transactionThreadReportID ?? null, Object.values(transactionThreadReportActions)); const lastReportAction = [...combinedReportActions, parentReportAction].find((action) => canEditReportAction(action) && !isMoneyRequestAction(action)); diff --git a/src/pages/home/report/ReportActionsView.tsx b/src/pages/home/report/ReportActionsView.tsx index 8ce56cb58c19..c0679a28f2b1 100755 --- a/src/pages/home/report/ReportActionsView.tsx +++ b/src/pages/home/report/ReportActionsView.tsx @@ -1,7 +1,7 @@ import {useIsFocused, useRoute} from '@react-navigation/native'; import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; import {InteractionManager} from 'react-native'; -import type {OnyxEntry} from 'react-native-onyx'; +import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import {useOnyx} from 'react-native-onyx'; import ReportActionsSkeletonView from '@components/ReportActionsSkeletonView'; import useCopySelectionHelper from '@hooks/useCopySelectionHelper'; @@ -13,6 +13,7 @@ import {updateLoadingInitialReportAction} from '@libs/actions/Report'; import Timing from '@libs/actions/Timing'; import DateUtils from '@libs/DateUtils'; import getIsReportFullyVisible from '@libs/getIsReportFullyVisible'; +import {selectAllTransactionsForReport} from '@libs/MoneyRequestReportUtils'; import type {PlatformStackRouteProp} from '@libs/Navigation/PlatformStackNavigation/types'; import type {ReportsSplitNavigatorParamList} from '@libs/Navigation/types'; import {generateNewRandomInt, rand64} from '@libs/NumberUtils'; @@ -25,6 +26,7 @@ import { getSortedReportActionsForDisplay, isCreatedAction, isDeletedParentAction, + isIOUActionMatchingTransactionList, isMoneyRequestAction, shouldReportActionBeVisible, } from '@libs/ReportActionsUtils'; @@ -93,6 +95,11 @@ function ReportActionsView({ const prevShouldUseNarrowLayoutRef = useRef(shouldUseNarrowLayout); const reportID = report.reportID; const isReportFullyVisible = useMemo((): boolean => getIsReportFullyVisible(isFocused), [isFocused]); + const [reportTransactionIDs] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION, { + selector: (allTransactions: OnyxCollection) => + selectAllTransactionsForReport(allTransactions, reportID, allReportActions ?? []).map((transaction) => transaction.transactionID), + canBeMissing: true, + }); useEffect(() => { // When we linked to message - we do not need to wait for initial actions - they already exists @@ -192,9 +199,10 @@ function ReportActionsView({ reportActions.filter( (reportAction) => (isOffline || isDeletedParentAction(reportAction) || reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || reportAction.errors) && - shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canPerformWriteAction), + shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canPerformWriteAction) && + isIOUActionMatchingTransactionList(reportAction, reportTransactionIDs), ), - [reportActions, isOffline, canPerformWriteAction], + [reportActions, isOffline, canPerformWriteAction, reportTransactionIDs], ); const newestReportAction = useMemo(() => reportActions?.at(0), [reportActions]); From 8374e9f9e3662daed1d6a99b97b71ddb55f79b1c Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Fri, 30 May 2025 15:31:52 +0200 Subject: [PATCH 2/4] Exclude selfDMs & splits from filtering --- src/libs/ReportActionsUtils.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index 546dc928989b..77683ba4abf4 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -10,7 +10,7 @@ import CONST from '@src/CONST'; import type {TranslationPaths} from '@src/languages/types'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; -import type {Card, Locale, OnyxInputOrEntry, PrivatePersonalDetails} from '@src/types/onyx'; +import type {Card, Locale, OnyxInputOrEntry, OriginalMessageIOU, PrivatePersonalDetails} from '@src/types/onyx'; import type {JoinWorkspaceResolution, OriginalMessageChangeLog, OriginalMessageExportIntegration} from '@src/types/onyx/OriginalMessage'; import type {PolicyReportFieldType} from '@src/types/onyx/Policy'; import type Report from '@src/types/onyx/Report'; @@ -1195,9 +1195,14 @@ function isTagModificationAction(actionName: string): boolean { ); } +/** Whether action has no linked report by design */ +const isIOUActionTypeExcludedFromFiltering = (type: OriginalMessageIOU['type'] | undefined) => + [CONST.IOU.REPORT_ACTION_TYPE.SPLIT, CONST.IOU.REPORT_ACTION_TYPE.TRACK].some((actionType) => actionType === type); + /** * Determines whether the given action is an IOU and, if a list of report transaction IDs is provided, - * whether it corresponds to one of those transactions. + * whether it corresponds to one of those transactions. This covers a rare case where IOU report actions was + * not deleted or moved after the expense was removed from the report. * * For compatibility and to avoid using isMoneyRequest next to this function as it is checked here already: * - If the action is not a money request and `defaultToFalseForNonIOU` is false (default), the result is true. @@ -1213,7 +1218,7 @@ const isIOUActionMatchingTransactionList = ( return !defaultToFalseForNonIOU; } - if (reportTransactionIDs === undefined) { + if (isIOUActionTypeExcludedFromFiltering(getOriginalMessage(action)?.type) || reportTransactionIDs === undefined) { return true; } From d2ee0d9b3395664340efd9e913072f5997fcfc93 Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Tue, 3 Jun 2025 09:25:30 +0200 Subject: [PATCH 3/4] Exclude PAY from filter & add test --- __mocks__/reportData/actions.ts | 8 ++--- src/libs/ReportActionsUtils.ts | 2 +- tests/unit/ReportActionsUtilsTest.ts | 52 ++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 5 deletions(-) diff --git a/__mocks__/reportData/actions.ts b/__mocks__/reportData/actions.ts index 795dbc161793..573bbc9dea16 100644 --- a/__mocks__/reportData/actions.ts +++ b/__mocks__/reportData/actions.ts @@ -19,7 +19,7 @@ const REPORT_R14932 = { childReportID: 'CHILD_REPORT_ID_R14932', }; -const originalMessageR14932: OriginalMessageIOU = { +const originalMessageR14932 = { currency, amount, IOUReportID: REPORT_R14932.IOUReportID, @@ -28,7 +28,7 @@ const originalMessageR14932: OriginalMessageIOU = { type: CONST.IOU.TYPE.CREATE, lastModified: '2025-02-14 08:12:05.165', comment: '', -}; +} satisfies OriginalMessageIOU; const message = [ { @@ -50,7 +50,7 @@ const person = [ }, ]; -const actionR14932: ReportAction = { +const actionR14932: ReportAction = { person, message, reportActionID: REPORT_R14932.reportActionID, @@ -86,4 +86,4 @@ const actionR98765: ReportAction = { created: '2025-02-14 08:12:05.165', }; -export {actionR14932, actionR98765}; +export {actionR14932, actionR98765, originalMessageR14932}; diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index 6aac86080c34..f491057e5bdf 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -1187,7 +1187,7 @@ function isTagModificationAction(actionName: string): boolean { /** Whether action has no linked report by design */ const isIOUActionTypeExcludedFromFiltering = (type: OriginalMessageIOU['type'] | undefined) => - [CONST.IOU.REPORT_ACTION_TYPE.SPLIT, CONST.IOU.REPORT_ACTION_TYPE.TRACK].some((actionType) => actionType === type); + [CONST.IOU.REPORT_ACTION_TYPE.SPLIT, CONST.IOU.REPORT_ACTION_TYPE.TRACK, CONST.IOU.REPORT_ACTION_TYPE.PAY].some((actionType) => actionType === type); /** * Determines whether the given action is an IOU and, if a list of report transaction IDs is provided, diff --git a/tests/unit/ReportActionsUtilsTest.ts b/tests/unit/ReportActionsUtilsTest.ts index bcbdcc8da684..a3f05912c014 100644 --- a/tests/unit/ReportActionsUtilsTest.ts +++ b/tests/unit/ReportActionsUtilsTest.ts @@ -1,8 +1,10 @@ import type {KeyValueMapping} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import {isExpenseReport} from '@libs/ReportUtils'; +import {actionR14932 as mockIOUAction, originalMessageR14932 as mockOriginalMessage} from '../../__mocks__/reportData/actions'; import CONST from '../../src/CONST'; import * as ReportActionsUtils from '../../src/libs/ReportActionsUtils'; +import {isIOUActionMatchingTransactionList} from '../../src/libs/ReportActionsUtils'; import ONYXKEYS from '../../src/ONYXKEYS'; import type {Report, ReportAction} from '../../src/types/onyx'; import createRandomReport from '../utils/collections/reports'; @@ -301,6 +303,56 @@ describe('ReportActionsUtils', () => { }); }); + describe('isIOUActionMatchingTransactionList', () => { + const nonIOUAction = { + created: '2022-11-13 22:27:01.825', + reportActionID: '8401445780099176', + actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT, + originalMessage: { + html: 'Hello world', + whisperedTo: [], + }, + message: [ + { + html: 'Hello world', + type: 'Action type', + text: 'Action text', + }, + ], + }; + + it('returns false for non-money request actions when defaultToFalseForNonIOU is true', () => { + expect(isIOUActionMatchingTransactionList(nonIOUAction, undefined, true)).toBeFalsy(); + }); + + it('returns true for non-money request actions when defaultToFalseForNonIOU is false', () => { + expect(isIOUActionMatchingTransactionList(nonIOUAction, undefined, false)).toBeTruthy(); + }); + + it('returns true if no reportTransactionIDs are provided', () => { + expect(isIOUActionMatchingTransactionList(mockIOUAction)).toBeTruthy(); + }); + + it('returns true if action is of excluded type', () => { + const action = { + ...mockIOUAction, + originalMessage: { + ...mockOriginalMessage, + type: CONST.IOU.REPORT_ACTION_TYPE.TRACK, + }, + }; + expect(isIOUActionMatchingTransactionList(action, ['124', '125', '126'])).toBeTruthy(); + }); + + it('returns true if IOUTransactionID matches any provided reportTransactionIDs', () => { + expect(isIOUActionMatchingTransactionList(mockIOUAction, ['123', '124', mockOriginalMessage.IOUTransactionID])).toBeTruthy(); + }); + + it('returns false if IOUTransactionID does not match any provided reportTransactionIDs', () => { + expect(isIOUActionMatchingTransactionList(mockIOUAction, ['123', '124'])).toBeFalsy(); + }); + }); + describe('getSortedReportActionsForDisplay', () => { it('should filter out non-whitelisted actions', () => { const input: ReportAction[] = [ From 00c189f1e03925f342d062fe5cf342a747312cf5 Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Tue, 3 Jun 2025 10:24:30 +0200 Subject: [PATCH 4/4] Fix ESLint check --- tests/unit/ReportSecondaryActionUtilsTest.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index 1c6bbe5806f0..d3fb7c1e84ac 100644 --- a/tests/unit/ReportSecondaryActionUtilsTest.ts +++ b/tests/unit/ReportSecondaryActionUtilsTest.ts @@ -2,11 +2,10 @@ import Onyx from 'react-native-onyx'; import {getSecondaryReportActions, getSecondaryTransactionThreadActions} from '@libs/ReportSecondaryActionUtils'; import CONST from '@src/CONST'; import * as ReportActionsUtils from '@src/libs/ReportActionsUtils'; -import {getOriginalMessage} from '@src/libs/ReportActionsUtils'; import * as ReportUtils from '@src/libs/ReportUtils'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {OriginalMessageIOU, Policy, Report, ReportAction, Transaction, TransactionViolation} from '@src/types/onyx'; -import {actionR14932} from '../../__mocks__/reportData/actions'; +import type {Policy, Report, ReportAction, Transaction, TransactionViolation} from '@src/types/onyx'; +import {actionR14932, originalMessageR14932} from '../../__mocks__/reportData/actions'; const EMPLOYEE_ACCOUNT_ID = 1; const EMPLOYEE_EMAIL = 'employee@mail.com'; @@ -400,7 +399,7 @@ describe('getSecondaryAction', () => { const policy = {} as unknown as Policy; jest.spyOn(ReportUtils, 'canHoldUnholdReportAction').mockReturnValueOnce({canHoldRequest: true, canUnholdRequest: true}); - jest.spyOn(ReportActionsUtils, 'getOneTransactionThreadReportID').mockReturnValueOnce((getOriginalMessage(actionR14932) as OriginalMessageIOU).IOUTransactionID); + jest.spyOn(ReportActionsUtils, 'getOneTransactionThreadReportID').mockReturnValueOnce(originalMessageR14932.IOUTransactionID); const result = getSecondaryReportActions(report, [transaction], {}, policy, undefined, [actionR14932]); expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.HOLD)).toBe(true); });