From b51343574629f5f127bac978ddf551dc1b12dfd0 Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Fri, 9 May 2025 14:20:57 +0200 Subject: [PATCH 1/5] Filter out deleted expenses from IOU actions --- .../MoneyRequestReportView/MoneyRequestReportView.tsx | 5 +++-- src/libs/actions/IOU.ts | 9 ++++++--- src/pages/home/ReportScreen.tsx | 9 ++++++++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx index 23b0117801c39..8c4e1d452ea8d 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx @@ -18,7 +18,7 @@ import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import Log from '@libs/Log'; import {selectAllTransactionsForReport, shouldDisplayReportTableView} from '@libs/MoneyRequestReportUtils'; import navigationRef from '@libs/Navigation/navigationRef'; -import {getOneTransactionThreadReportID, isMoneyRequestAction} from '@libs/ReportActionsUtils'; +import {getOneTransactionThreadReportID, isDeletedParentAction, isMoneyRequestAction} from '@libs/ReportActionsUtils'; import {canEditReportAction, getReportOfflinePendingActionAndErrors, isReportTransactionThread} from '@libs/ReportUtils'; import {buildCannedSearchQuery} from '@libs/SearchQueryUtils'; import Navigation from '@navigation/Navigation'; @@ -95,7 +95,8 @@ function MoneyRequestReportView({report, policy, reportMetadata, shouldDisplayRe const [isComposerFullSize] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_IS_COMPOSER_FULL_SIZE}${reportID}`, {initialValue: false, canBeMissing: true}); const {reportPendingAction, reportErrors} = getReportOfflinePendingActionAndErrors(report); - const {reportActions, hasNewerActions, hasOlderActions} = usePaginatedReportActions(reportID); + const {reportActions: reportActionsWithDeletedExpenses, hasNewerActions, hasOlderActions} = usePaginatedReportActions(reportID); + const reportActions = reportActionsWithDeletedExpenses.filter((value) => !isDeletedParentAction(value)); const transactionThreadReportID = getOneTransactionThreadReportID(reportID, reportActions ?? [], isOffline); const [transactions = []] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION, { diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 92f6f05c2e629..c9495bcc00b1a 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -7111,8 +7111,10 @@ function prepareToCleanUpMoneyRequest(transactionID: string, reportAction: OnyxT // STEP 2: Decide if we need to: // 1. Delete the transactionThread - delete if there are no visible comments in the thread // 2. Update the moneyRequestPreview to show [Deleted expense] - update if the transactionThread exists AND it isn't being deleted - const shouldDeleteTransactionThread = transactionThreadID ? (reportAction?.childVisibleActionCount ?? 0) === 0 : false; - const shouldShowDeletedRequestMessage = !!transactionThreadID && !shouldDeleteTransactionThread; + // The current state is that we want to get rid of the [Deleted expense] breadcrumb, + // so we never want to display it if transactionThreadID is present. + const shouldDeleteTransactionThread = !!transactionThreadID; + const shouldShowDeletedRequestMessage = !shouldDeleteTransactionThread; // STEP 3: Update the IOU reportAction and decide if the iouReport should be deleted. We delete the iouReport if there are no visible comments left in the report. const updatedReportAction = { @@ -7125,7 +7127,8 @@ function prepareToCleanUpMoneyRequest(transactionID: string, reportAction: OnyxT html: '', text: '', isEdited: true, - isDeletedParentAction: shouldShowDeletedRequestMessage, + // We need this here to filter out deleted actions + isDeletedParentAction: true, }, ], originalMessage: { diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index ce6b26989a9d6..e7c7ec6efca2c 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -266,7 +266,14 @@ function ReportScreen({route, navigation}: ReportScreenProps) { const [currentUserAccountID = -1] = useOnyx(ONYXKEYS.SESSION, {selector: (value) => value?.accountID, canBeMissing: false}); const [currentUserEmail] = useOnyx(ONYXKEYS.SESSION, {selector: (value) => value?.email, canBeMissing: false}); const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP, {canBeMissing: true}); - const {reportActions, linkedAction, sortedAllReportActions, hasNewerActions, hasOlderActions} = usePaginatedReportActions(reportID, reportActionIDFromRoute); + const { + reportActions: reportActionsWithDeletedExpenses, + linkedAction, + sortedAllReportActions, + hasNewerActions, + hasOlderActions, + } = usePaginatedReportActions(reportID, reportActionIDFromRoute); + const reportActions = reportActionsWithDeletedExpenses.filter((value) => !isDeletedParentAction(value)); const [isBannerVisible, setIsBannerVisible] = useState(true); const [scrollPosition, setScrollPosition] = useState({}); From 58df08ac77beee398391ba9122ac977e83013d62 Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Mon, 12 May 2025 13:06:44 +0200 Subject: [PATCH 2/5] Add fix suggested by Vit --- .../TransactionPreviewContent.tsx | 6 +++-- src/libs/TransactionPreviewUtils.ts | 14 +++++------ tests/unit/TransactionPreviewUtils.test.ts | 23 +++++-------------- 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/src/components/ReportActionItem/TransactionPreview/TransactionPreviewContent.tsx b/src/components/ReportActionItem/TransactionPreview/TransactionPreviewContent.tsx index 814f10d507dbb..01954d85191c4 100644 --- a/src/components/ReportActionItem/TransactionPreview/TransactionPreviewContent.tsx +++ b/src/components/ReportActionItem/TransactionPreview/TransactionPreviewContent.tsx @@ -19,7 +19,7 @@ import {getAvatarsForAccountIDs} from '@libs/OptionsListUtils'; import Parser from '@libs/Parser'; import {getCleanedTagName} from '@libs/PolicyUtils'; import {getThumbnailAndImageURIs} from '@libs/ReceiptUtils'; -import {getOriginalMessage, isMoneyRequestAction} from '@libs/ReportActionsUtils'; +import {getOriginalMessage, getReportActions, isMoneyRequestAction} from '@libs/ReportActionsUtils'; import type {TransactionDetails} from '@libs/ReportUtils'; import {canEditMoneyRequest, getTransactionDetails, getWorkspaceIcon, isIOUReport, isPolicyExpenseChat, isReportApproved, isSettled} from '@libs/ReportUtils'; import StringUtils from '@libs/StringUtils'; @@ -60,6 +60,7 @@ function TransactionPreviewContent({ const ownerAccountID = iouReport?.ownerAccountID ?? reportPreviewAction?.childOwnerAccountID ?? CONST.DEFAULT_NUMBER_ID; const isReportAPolicyExpenseChat = isPolicyExpenseChat(chatReport); const {amount: requestAmount, comment: requestComment, merchant, tag, category, currency: requestCurrency} = transactionDetails; + const reportActions = useMemo(() => (iouReport ? getReportActions(iouReport) ?? {} : {}), [iouReport]); const transactionPreviewCommonArguments = useMemo( () => ({ @@ -96,8 +97,9 @@ function TransactionPreviewContent({ ...transactionPreviewCommonArguments, shouldShowRBR, violationMessage, + reportActions, }), - [transactionPreviewCommonArguments, shouldShowRBR, violationMessage], + [transactionPreviewCommonArguments, shouldShowRBR, violationMessage, reportActions], ); const getTranslatedText = (item: TranslationPathOrText) => (item.translationPath ? translate(item.translationPath) : item.text ?? ''); diff --git a/src/libs/TransactionPreviewUtils.ts b/src/libs/TransactionPreviewUtils.ts index c74f24478f930..2562dc0e953eb 100644 --- a/src/libs/TransactionPreviewUtils.ts +++ b/src/libs/TransactionPreviewUtils.ts @@ -10,7 +10,7 @@ import {convertToDisplayString} from './CurrencyUtils'; import DateUtils from './DateUtils'; import type {PlatformStackRouteProp} from './Navigation/PlatformStackNavigation/types'; import type {TransactionDuplicateNavigatorParamList} from './Navigation/types'; -import {getOriginalMessage, getReportAction, getReportActions, isMessageDeleted, isMoneyRequestAction} from './ReportActionsUtils'; +import {getOriginalMessage, getReportAction, isMessageDeleted, isMoneyRequestAction} from './ReportActionsUtils'; import {hasActionsWithErrors, hasReportViolations, isPaidGroupPolicy, isPaidGroupPolicyExpenseReport, isReportApproved, isReportOwner, isSettled} from './ReportUtils'; import type {TransactionDetails} from './ReportUtils'; import StringUtils from './StringUtils'; @@ -140,10 +140,8 @@ function getViolationTranslatePath(violations: OnyxTypes.TransactionViolations, * it returns an empty array. It identifies the latest error in each action and filters out duplicates to * ensure only unique error messages are returned. */ -function getUniqueActionErrors(report: OnyxEntry) { - const reportActions = Object.values(report ? getReportActions(report) ?? {} : {}); - - const reportErrors = reportActions.map((reportAction) => { +function getUniqueActionErrors(reportActions: OnyxTypes.ReportActions) { + const reportErrors = Object.values(reportActions).map((reportAction) => { const errors = reportAction.errors ?? {}; const key = Object.keys(errors).sort().reverse().at(0) ?? ''; const error = errors[key]; @@ -162,6 +160,7 @@ function getTransactionPreviewTextAndTranslationPaths({ isBillSplit, shouldShowRBR, violationMessage, + reportActions, }: { iouReport: OnyxEntry; transaction: OnyxEntry; @@ -171,6 +170,7 @@ function getTransactionPreviewTextAndTranslationPaths({ isBillSplit: boolean; shouldShowRBR: boolean; violationMessage?: string; + reportActions?: OnyxTypes.ReportActions; }) { const isFetchingWaypoints = isFetchingWaypointsFromServer(transaction); const isTransactionOnHold = isOnHold(transaction); @@ -216,8 +216,8 @@ function getTransactionPreviewTextAndTranslationPaths({ } } - if (RBRMessage === undefined && hasActionWithErrors) { - const actionsWithErrors = getUniqueActionErrors(iouReport); + if (RBRMessage === undefined && hasActionWithErrors && !!reportActions) { + const actionsWithErrors = getUniqueActionErrors(reportActions); RBRMessage = actionsWithErrors.length > 1 ? {translationPath: 'violations.reviewRequired'} : {text: actionsWithErrors.at(0)}; } diff --git a/tests/unit/TransactionPreviewUtils.test.ts b/tests/unit/TransactionPreviewUtils.test.ts index 7806cc4a2b258..978351c73dadd 100644 --- a/tests/unit/TransactionPreviewUtils.test.ts +++ b/tests/unit/TransactionPreviewUtils.test.ts @@ -2,10 +2,8 @@ import {buildOptimisticIOUReport, buildOptimisticIOUReportAction} from '@libs/Re import {createTransactionPreviewConditionals, getTransactionPreviewTextAndTranslationPaths, getUniqueActionErrors, getViolationTranslatePath} from '@libs/TransactionPreviewUtils'; import {buildOptimisticTransaction} from '@libs/TransactionUtils'; import CONST from '@src/CONST'; -import * as ReportActionUtils from '@src/libs/ReportActionsUtils'; import * as ReportUtils from '@src/libs/ReportUtils'; -import type {Report, ReportActions} from '@src/types/onyx'; -import {iouReportR14932 as mockedReport} from '../../__mocks__/reportData/reports'; +import type {ReportActions} from '@src/types/onyx'; const basicProps = { iouReport: buildOptimisticIOUReport(123, 234, 1000, '1', 'USD'), @@ -258,14 +256,8 @@ describe('TransactionPreviewUtils', () => { }); describe('getUniqueActionErrors', () => { - test('returns an empty array if there is no report or it is empty', () => { - expect(getUniqueActionErrors(undefined)).toEqual([]); - expect(getUniqueActionErrors({} as Report)).toEqual([]); - }); - - test('returns an empty array if there are no actions in the report', () => { - jest.spyOn(ReportActionUtils, 'getReportActions').mockReturnValue({}); - expect(getUniqueActionErrors(mockedReport)).toEqual([]); + test('returns an empty array if there are no actions', () => { + expect(getUniqueActionErrors({})).toEqual([]); }); test('returns unique error messages from report actions', () => { @@ -276,10 +268,9 @@ describe('TransactionPreviewUtils', () => { 3: {errors: {a: 'Error A', d: 'Error D'}}, /* eslint-enable @typescript-eslint/naming-convention */ } as unknown as ReportActions; - jest.spyOn(ReportActionUtils, 'getReportActions').mockReturnValue(actions); const expectedErrors = ['Error B', 'Error C', 'Error D']; - expect(getUniqueActionErrors(mockedReport).sort()).toEqual(expectedErrors.sort()); + expect(getUniqueActionErrors(actions).sort()).toEqual(expectedErrors.sort()); }); test('returns the latest error message if multiple errors exist under a single action', () => { @@ -288,9 +279,8 @@ describe('TransactionPreviewUtils', () => { 1: {errors: {z: 'Error Z2', a: 'Error A', f: 'Error Z'}}, /* eslint-enable @typescript-eslint/naming-convention */ } as unknown as ReportActions; - jest.spyOn(ReportActionUtils, 'getReportActions').mockReturnValue(actions); - expect(getUniqueActionErrors(mockedReport)).toEqual(['Error Z2']); + expect(getUniqueActionErrors(actions)).toEqual(['Error Z2']); }); test('filters out non-string error messages', () => { @@ -300,9 +290,8 @@ describe('TransactionPreviewUtils', () => { 2: {errors: {c: null, d: 'Error D'}}, /* eslint-enable @typescript-eslint/naming-convention */ } as unknown as ReportActions; - jest.spyOn(ReportActionUtils, 'getReportActions').mockReturnValue(actions); - expect(getUniqueActionErrors(mockedReport)).toEqual(['Error B', 'Error D']); + expect(getUniqueActionErrors(actions)).toEqual(['Error B', 'Error D']); }); }); }); From 0d84b7ea99f486ecc2132983cb89dfafea42c403 Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Wed, 14 May 2025 14:06:03 +0200 Subject: [PATCH 3/5] Change isDeletedParentAction condition --- src/libs/actions/IOU.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index beefd779194b6..4a25c9e73f650 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -7128,7 +7128,7 @@ function prepareToCleanUpMoneyRequest(transactionID: string, reportAction: OnyxT text: '', isEdited: true, // We need this here to filter out deleted actions - isDeletedParentAction: true, + isDeletedParentAction: shouldDeleteTransactionThread, }, ], originalMessage: { From d9aec3dfd021874c4d637a45aa436af369b9034d Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Wed, 14 May 2025 14:31:29 +0200 Subject: [PATCH 4/5] Remove shouldShowDeletedRequestMessage --- src/libs/actions/IOU.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 4a25c9e73f650..82603f701ecd9 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -7114,12 +7114,11 @@ function prepareToCleanUpMoneyRequest(transactionID: string, reportAction: OnyxT // The current state is that we want to get rid of the [Deleted expense] breadcrumb, // so we never want to display it if transactionThreadID is present. const shouldDeleteTransactionThread = !!transactionThreadID; - const shouldShowDeletedRequestMessage = !shouldDeleteTransactionThread; // STEP 3: Update the IOU reportAction and decide if the iouReport should be deleted. We delete the iouReport if there are no visible comments left in the report. const updatedReportAction = { [reportAction.reportActionID]: { - pendingAction: shouldShowDeletedRequestMessage ? CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE : CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, + pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, previousMessage: reportAction.message, message: [ { From f29bf4a443af82cf9422c821c52651828c4289e2 Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Wed, 14 May 2025 19:24:09 +0200 Subject: [PATCH 5/5] Remove comment from actions/IOU.ts --- src/libs/actions/IOU.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index aff4f5c1ba557..e7aebfacc092a 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -7131,7 +7131,6 @@ function prepareToCleanUpMoneyRequest(transactionID: string, reportAction: OnyxT html: '', text: '', isEdited: true, - // We need this here to filter out deleted actions isDeletedParentAction: shouldDeleteTransactionThread, }, ],