From d087394baec9298def249d0e00cfde0a0dcaba6d Mon Sep 17 00:00:00 2001 From: VickyStash Date: Mon, 11 Aug 2025 17:31:55 +0200 Subject: [PATCH 01/11] Support one transaction thread report creation if it wasn't created optimistically --- src/CONST/index.ts | 1 + src/libs/ReportActionsUtils.ts | 7 +++++-- src/libs/ReportUtils.ts | 2 ++ src/pages/home/ReportScreen.tsx | 25 +++++++++++++++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/CONST/index.ts b/src/CONST/index.ts index 3d93ec30c2507..70f6ee333954d 100755 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -912,6 +912,7 @@ const CONST = { EMPTY_ARRAY, EMPTY_OBJECT, DEFAULT_NUMBER_ID, + FAKE_REPORT_ID: 'FAKE_REPORT_ID', USE_EXPENSIFY_URL, EXPENSIFY_URL, EXPENSIFY_MOBILE_URL, diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index 2ea06e8a137c1..e21bf49da7537 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -1304,7 +1304,6 @@ function getOneTransactionThreadReportAction( if ( actionType && iouRequestTypesSet.has(actionType) && - action.childReportID && // Include deleted IOU reportActions if: // - they have an associated IOU transaction ID or // - the action is pending deletion and the user is offline @@ -1336,7 +1335,11 @@ function getOneTransactionThreadReportAction( * Returns a reportID if there is exactly one transaction thread for the report, and undefined otherwise. */ function getOneTransactionThreadReportID(...args: Parameters): string | undefined { - return getOneTransactionThreadReportAction(...args)?.childReportID; + const reportAction = getOneTransactionThreadReportAction(...args); + if (reportAction) { + // Since we don't always create transaction thread optimistically, we return CONST.FAKE_REPORT_ID + return reportAction.childReportID ?? CONST.FAKE_REPORT_ID; + } } /** diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 74614d6a16a53..d18499a2b4207 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -7812,6 +7812,7 @@ function buildTransactionThread( reportAction: OnyxEntry, moneyRequestReport: OnyxEntry, existingTransactionThreadReportID?: string, + optimisticTransactionThreadReportID?: string, ): OptimisticChatReport { const participantAccountIDs = [...new Set([currentUserAccountID, Number(reportAction?.actorAccountID)])].filter(Boolean) as number[]; const existingTransactionThreadReport = getReportOrDraftReport(existingTransactionThreadReportID); @@ -7834,6 +7835,7 @@ function buildTransactionThread( notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, parentReportActionID: reportAction?.reportActionID, parentReportID: moneyRequestReport?.reportID, + optimisticReportID: optimisticTransactionThreadReportID, }); } diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index e6df82f391a66..babe146b997c4 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -42,6 +42,7 @@ import {getDisplayNameOrDefault} from '@libs/PersonalDetailsUtils'; import { getCombinedReportActions, getFilteredReportActionsForReportView, + getIOUActionForReportID, getOneTransactionThreadReportID, isCreatedAction, isDeletedParentAction, @@ -50,11 +51,14 @@ import { shouldReportActionBeVisible, } from '@libs/ReportActionsUtils'; import { + buildTransactionThread, canEditReportAction, canUserPerformWriteAction, findLastAccessedReport, + generateReportID, getParticipantsAccountIDsForDisplay, getReportOfflinePendingActionAndErrors, + getReportTransactions, isChatThread, isConciergeChatReport, isGroupChat, @@ -306,6 +310,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { const reportTransactionIDs = useMemo(() => visibleTransactions?.map((transaction) => transaction.transactionID), [visibleTransactions]); const transactionThreadReportID = getOneTransactionThreadReportID(report, chatReport, reportActions ?? [], isOffline, reportTransactionIDs); + const [transactionThreadReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`, {canBeMissing: true}); const [transactionThreadReportActions = getEmptyObject()] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThreadReportID}`, { canBeMissing: true, }); @@ -467,6 +472,15 @@ function ReportScreen({route, navigation}: ReportScreenProps) { [firstRender, shouldShowNotFoundLinkedAction, reportID, isOptimisticDelete, reportMetadata?.isLoadingInitialReportActions, userLeavingStatus, currentReportIDFormRoute], ); + const createOneTransactionThreadReport = useCallback(() => { + const optimisticTransactionThreadReportID = generateReportID(); + const currentReportTransaction = getReportTransactions(reportID).filter((transaction) => transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); + const oneTransactionID = currentReportTransaction.at(0)?.transactionID; + const iouAction = getIOUActionForReportID(reportID, oneTransactionID); + const optimisticTransactionThread = buildTransactionThread(iouAction, report, undefined, optimisticTransactionThreadReportID); + openReport(optimisticTransactionThreadReportID, undefined, currentUserEmail ? [currentUserEmail] : [], optimisticTransactionThread, iouAction?.reportActionID); + }, [currentUserEmail, report, reportID]); + const fetchReport = useCallback(() => { if (reportMetadata.isOptimisticReport && report?.type === CONST.REPORT.TYPE.CHAT && !isPolicyExpenseChat(report)) { return; @@ -485,6 +499,13 @@ function ReportScreen({route, navigation}: ReportScreenProps) { openReport(reportIDFromRoute, '', [currentUserEmail], undefined, moneyRequestReportActionID, false, [], undefined, transactionID); return; } + + // If there is one transaction thread that has not yet been created, we should create it. + if (transactionThreadReportID === CONST.FAKE_REPORT_ID && !transactionThreadReport && parentReportAction?.childMoneyRequestCount === 1) { + createOneTransactionThreadReport(); + return; + } + openReport(reportIDFromRoute, reportActionIDFromRoute); }, [ reportMetadata.isOptimisticReport, @@ -493,8 +514,12 @@ function ReportScreen({route, navigation}: ReportScreenProps) { route.params?.moneyRequestReportActionID, route.params?.transactionID, currentUserEmail, + transactionThreadReportID, + transactionThreadReport, + parentReportAction?.childMoneyRequestCount, reportIDFromRoute, reportActionIDFromRoute, + createOneTransactionThreadReport, ]); const prevTransactionThreadReportID = usePrevious(transactionThreadReportID); From c1946abf5d6679812ed6c48d8aaf572bb2c4600d Mon Sep 17 00:00:00 2001 From: VickyStash Date: Tue, 12 Aug 2025 10:22:02 +0200 Subject: [PATCH 02/11] Make Edit, Hold, Delete options be shows in the menu --- .../home/report/ContextMenu/BaseReportActionContextMenu.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx b/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx index 408c42874c3bf..e0aaade23d76f 100755 --- a/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx +++ b/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx @@ -165,6 +165,7 @@ function BaseReportActionContextMenu({ const [download] = useOnyx(`${ONYXKEYS.COLLECTION.DOWNLOAD}${sourceID}`, {canBeMissing: true}); const [childReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportAction?.childReportID}`, {canBeMissing: true}); + const [childReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportAction?.childReportID}`, {canBeMissing: true}); const [childChatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${childReport?.chatReportID}`, {canBeMissing: true}); const parentReportAction = getReportAction(childReport?.parentReportID, childReport?.parentReportActionID); const {reportActions: paginatedReportActions} = usePaginatedReportActions(childReport?.reportID); @@ -181,13 +182,16 @@ function BaseReportActionContextMenu({ const requestParentReportAction = useMemo(() => { if (isMoneyRequestReport || isInvoiceReport) { + if (transactionThreadReportID === CONST.FAKE_REPORT_ID) { + return Object.values(childReportActions ?? {}).find((action) => action.actionName === CONST.REPORT.ACTIONS.TYPE.IOU); + } if (!paginatedReportActions || !transactionThreadReport?.parentReportActionID) { return undefined; } return paginatedReportActions.find((action) => action.reportActionID === transactionThreadReport.parentReportActionID); } return parentReportAction; - }, [parentReportAction, isMoneyRequestReport, isInvoiceReport, paginatedReportActions, transactionThreadReport?.parentReportActionID]); + }, [parentReportAction, isMoneyRequestReport, isInvoiceReport, paginatedReportActions, transactionThreadReport?.parentReportActionID, transactionThreadReportID, childReportActions]); const moneyRequestAction = transactionThreadReportID ? requestParentReportAction : parentReportAction; const isChildReportArchived = useReportIsArchived(childReport?.reportID); From f86a986d944740c7797be7a3bd66ad9ce228c293 Mon Sep 17 00:00:00 2001 From: VickyStash Date: Tue, 12 Aug 2025 16:20:44 +0200 Subject: [PATCH 03/11] Support Hold functionality if there is no transaction thread report --- src/ROUTES.ts | 9 +- .../API/parameters/HoldMoneyRequestParams.ts | 2 + src/libs/ReportUtils.ts | 16 ++-- src/libs/actions/IOU.ts | 96 ++++++++++++++++--- 4 files changed, 98 insertions(+), 25 deletions(-) diff --git a/src/ROUTES.ts b/src/ROUTES.ts index 937e74c6231dd..204c81b5b8f88 100644 --- a/src/ROUTES.ts +++ b/src/ROUTES.ts @@ -586,11 +586,10 @@ const ROUTES = { }, MONEY_REQUEST_HOLD_REASON: { route: ':type/edit/reason/:transactionID?/:searchHash?', - getRoute: (type: ValueOf, transactionID: string, reportID: string, backTo: string, searchHash?: number) => { - const route = searchHash - ? (`${type as string}/edit/reason/${transactionID}/${searchHash}/?backTo=${backTo}&reportID=${reportID}` as const) - : (`${type as string}/edit/reason/${transactionID}/?backTo=${backTo}&reportID=${reportID}` as const); - return route; + getRoute: (type: ValueOf, transactionID: string, reportID: string | undefined, backTo: string, searchHash?: number) => { + const searchPart = searchHash ? `/${searchHash}` : ''; + const reportPart = reportID ? `&reportID=${reportID}` : ''; + return `${type as string}/edit/reason/${transactionID}${searchPart}/?backTo=${backTo}${reportPart}` as const; }, }, MONEY_REQUEST_CREATE: { diff --git a/src/libs/API/parameters/HoldMoneyRequestParams.ts b/src/libs/API/parameters/HoldMoneyRequestParams.ts index 357194d7ae56a..b22e421898d10 100644 --- a/src/libs/API/parameters/HoldMoneyRequestParams.ts +++ b/src/libs/API/parameters/HoldMoneyRequestParams.ts @@ -3,6 +3,8 @@ type HoldMoneyRequestParams = { comment: string; reportActionID: string; commentReportActionID: string; + transactionThreadReportID?: string; + createdReportActionIDForThread?: string; }; export default HoldMoneyRequestParams; diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index d18499a2b4207..8021fc70257d6 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -4415,8 +4415,8 @@ const changeMoneyRequestHoldStatus = (reportAction: OnyxEntry): vo const transactionID = getOriginalMessage(reportAction)?.IOUTransactionID; - if (!transactionID || !reportAction.childReportID) { - Log.warn('Missing transactionID and reportAction.childReportID during the change of the money request hold status'); + if (!transactionID) { + Log.warn('Missing transactionID during the change of the money request hold status'); return; } @@ -4425,7 +4425,11 @@ const changeMoneyRequestHoldStatus = (reportAction: OnyxEntry): vo const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${moneyRequestReport.policyID}`] ?? null; if (isOnHold) { - unholdRequest(transactionID, reportAction.childReportID); + if (reportAction.childReportID) { + unholdRequest(transactionID, reportAction.childReportID); + } else { + Log.warn('Missing reportAction.childReportID during money request unhold'); + } } else { const activeRoute = encodeURIComponent(Navigation.getActiveRoute()); Navigation.navigate(ROUTES.MONEY_REQUEST_HOLD_REASON.getRoute(policy?.type ?? CONST.POLICY.TYPE.PERSONAL, transactionID, reportAction.childReportID, activeRoute)); @@ -9685,12 +9689,12 @@ function getAllAncestorReportActionIDs(report: Report | null | undefined, includ /** * Get optimistic data of parent report action - * @param reportID The reportID of the report that is updated + * @param reportOrID The reportID of the report that is updated or the optimistic report on its own * @param lastVisibleActionCreated Last visible action created of the child report * @param type The type of action in the child report */ -function getOptimisticDataForParentReportAction(reportID: string | undefined, lastVisibleActionCreated: string, type: string): Array { - const report = getReportOrDraftReport(reportID); +function getOptimisticDataForParentReportAction(reportOrID: Report | string | undefined, lastVisibleActionCreated: string, type: string): Array { + const report = typeof reportOrID === 'string' ? getReportOrDraftReport(reportOrID) : reportOrID; if (!report || isEmptyObject(report)) { return []; diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 6f6e21fbd2a90..5f018bca70f6a 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -18,6 +18,7 @@ import type { CreateWorkspaceParams, DeleteMoneyRequestParams, DetachReceiptParams, + HoldMoneyRequestParams, MergeDuplicatesParams, PayInvoiceParams, PayMoneyRequestParams, @@ -129,9 +130,11 @@ import { buildOptimisticSubmittedReportAction, buildOptimisticUnapprovedReportAction, buildOptimisticUnHoldReportAction, + buildTransactionThread, canBeAutoReimbursed, canUserPerformWriteAction as canUserPerformWriteActionReportUtils, findSelfDMReportID, + generateReportID, getAllHeldTransactions as getAllHeldTransactionsReportUtils, getAllPolicyReports, getApprovalChain, @@ -10905,17 +10908,28 @@ function adjustRemainingSplitShares(transaction: NonNullable notifyNewAction(currentReportID, userAccountID)); @@ -11166,7 +11234,7 @@ function unholdRequest(transactionID: string, reportID: string) { ]; API.write( - 'UnHoldRequest', + WRITE_COMMANDS.UNHOLD_MONEY_REQUEST, { transactionID, reportActionID: createdReportAction.reportActionID, From 2c59f9046ab5dcedd8ddf67435c2f6c30cee45b9 Mon Sep 17 00:00:00 2001 From: VickyStash Date: Wed, 13 Aug 2025 16:53:39 +0200 Subject: [PATCH 04/11] Add failure data and improve typing --- src/libs/actions/IOU.ts | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 5f018bca70f6a..1c155d10295a9 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -10919,7 +10919,7 @@ function putOnHold(transactionID: string, comment: string, initialReportID: stri const transaction = allTransactions[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; const iouReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transaction?.reportID}`]; const iouAction = getIOUActionForReportID(transaction?.reportID, transactionID); - let report; + let report: OnyxTypes.Report | undefined; if (initialReportID) { report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${initialReportID}`]; @@ -11057,6 +11057,11 @@ function putOnHold(transactionID: string, comment: string, initialReportID: stri key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`, value: {[iouAction?.reportActionID]: {childReportID: reportID, childType: CONST.REPORT.TYPE.CHAT}}, }); + failureData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`, + value: {[iouAction?.reportActionID]: {childReportID: null, childType: null}}, + }); } successData.push( @@ -11073,6 +11078,24 @@ function putOnHold(transactionID: string, comment: string, initialReportID: stri }, }, ); + + failureData.push( + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, + value: null, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, + value: null, + }, + { + onyxMethod: Onyx.METHOD.SET, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, + value: null, + }, + ); } // If we are holding from the search page, we optimistically update the snapshot data that search uses so that it is kept in sync From c7d7fba9e303ae7b64dd68911b770ed5bfb5e8d4 Mon Sep 17 00:00:00 2001 From: VickyStash Date: Mon, 18 Aug 2025 09:40:40 +0200 Subject: [PATCH 05/11] Update getOptimisticDataForParentReportAction to pass report param --- src/libs/ReportUtils.ts | 6 ++---- src/libs/actions/Report.ts | 5 +++-- src/libs/actions/Task.ts | 4 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 3cd412ef8b302..c76251f296b5f 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -9701,13 +9701,11 @@ function getAllAncestorReportActionIDs(report: Report | null | undefined, includ /** * Get optimistic data of parent report action - * @param reportOrID The reportID of the report that is updated or the optimistic report on its own + * @param report The report that is updated * @param lastVisibleActionCreated Last visible action created of the child report * @param type The type of action in the child report */ -function getOptimisticDataForParentReportAction(reportOrID: Report | string | undefined, lastVisibleActionCreated: string, type: string): Array { - const report = typeof reportOrID === 'string' ? getReportOrDraftReport(reportOrID) : reportOrID; - +function getOptimisticDataForParentReportAction(report: Report | undefined, lastVisibleActionCreated: string, type: string): Array { if (!report || isEmptyObject(report)) { return []; } diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index f20705aa5d2ac..4f16733329efe 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -802,7 +802,7 @@ function addActions(reportID: string, text = '', file?: FileObject) { ]; // Update optimistic data for parent report action if the report is a child report - const optimisticParentReportData = getOptimisticDataForParentReportAction(reportID, currentTime, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD); + const optimisticParentReportData = getOptimisticDataForParentReportAction(report, currentTime, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD); optimisticParentReportData.forEach((parentReportData) => { if (isEmptyObject(parentReportData)) { return; @@ -1957,8 +1957,9 @@ function deleteReportComment(reportID: string | undefined, reportAction: ReportA // Update optimistic data for parent report action if the report is a child report and the reportAction has no visible child const childVisibleActionCount = reportAction.childVisibleActionCount ?? 0; if (childVisibleActionCount === 0) { + const originalReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${originalReportID}`]; const optimisticParentReportData = getOptimisticDataForParentReportAction( - originalReportID, + originalReport, optimisticReport?.lastVisibleActionCreated ?? '', CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, ); diff --git a/src/libs/actions/Task.ts b/src/libs/actions/Task.ts index 255d91532e66a..ff8f0f3fe71e1 100644 --- a/src/libs/actions/Task.ts +++ b/src/libs/actions/Task.ts @@ -307,7 +307,7 @@ function createTaskAndNavigate( }); // If needed, update optimistic data for parent report action of the parent report. - const optimisticParentReportData = ReportUtils.getOptimisticDataForParentReportAction(parentReportID, currentTime, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD); + const optimisticParentReportData = ReportUtils.getOptimisticDataForParentReportAction(parentReport, currentTime, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD); optimisticParentReportData.forEach((parentReportData) => { if (isEmptyObject(parentReportData)) { return; @@ -1145,7 +1145,7 @@ function deleteTask(report: OnyxEntry) { const childVisibleActionCount = parentReportAction?.childVisibleActionCount ?? 0; if (childVisibleActionCount === 0) { const optimisticParentReportData = ReportUtils.getOptimisticDataForParentReportAction( - parentReport?.reportID, + parentReport, parentReport?.lastVisibleActionCreated ?? '', CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, ); From 9d7337fe34b8b0ca57a99853fdd41d27bf9b2de1 Mon Sep 17 00:00:00 2001 From: VickyStash Date: Mon, 18 Aug 2025 10:35:37 +0200 Subject: [PATCH 06/11] Add comments --- src/libs/actions/IOU.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index b72ef99b3847c..5bea8ab0d2731 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -10926,6 +10926,8 @@ function putOnHold(transactionID: string, comment: string, initialReportID: stri const iouAction = getIOUActionForReportID(transaction?.reportID, transactionID); let report: OnyxTypes.Report | undefined; + // If there is no existing transaction thread report, we should create one + // This way we ensure every held request has a dedicated thread for comments if (initialReportID) { report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${initialReportID}`]; } else { @@ -11029,7 +11031,7 @@ function putOnHold(transactionID: string, comment: string, initialReportID: stri }, ]; - // if the transaction thread report wasn't created before, we do it now + // If the transaction thread report wasn't created before, we create it optimistically if (!initialReportID) { optimisticData.push( { @@ -11057,11 +11059,13 @@ function putOnHold(transactionID: string, comment: string, initialReportID: stri ); if (iouAction?.reportActionID) { + // We link the IOU action to the new transaction thread by setting childReportID optimistically optimisticData.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`, value: {[iouAction?.reportActionID]: {childReportID: reportID, childType: CONST.REPORT.TYPE.CHAT}}, }); + // We reset the childReportID if the request fails failureData.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`, From 5398bc15938b2d907d8c33d44fe45a5fd10a4425 Mon Sep 17 00:00:00 2001 From: VickyStash Date: Mon, 18 Aug 2025 10:44:21 +0200 Subject: [PATCH 07/11] Update MERGE to SET --- src/libs/actions/IOU.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 5bea8ab0d2731..3ff022a5f49fb 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -11035,7 +11035,7 @@ function putOnHold(transactionID: string, comment: string, initialReportID: stri if (!initialReportID) { optimisticData.push( { - onyxMethod: Onyx.METHOD.MERGE, + onyxMethod: Onyx.METHOD.SET, key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, value: { ...report, @@ -11090,12 +11090,12 @@ function putOnHold(transactionID: string, comment: string, initialReportID: stri failureData.push( { - onyxMethod: Onyx.METHOD.MERGE, + onyxMethod: Onyx.METHOD.SET, key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, value: null, }, { - onyxMethod: Onyx.METHOD.MERGE, + onyxMethod: Onyx.METHOD.SET, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, value: null, }, From 58c3945437b253c5b6df4208abb12e8d91fac945 Mon Sep 17 00:00:00 2001 From: VickyStash Date: Mon, 18 Aug 2025 12:15:17 +0200 Subject: [PATCH 08/11] Update putOnHold and getOneTransactionThreadReportID unit tests --- tests/actions/IOUTest.ts | 52 ++++++++++++++++++++++++++++ tests/unit/ReportActionsUtilsTest.ts | 11 ++++++ 2 files changed, 63 insertions(+) diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index a4c2a36bbcaa9..7d07b89b31d86 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -4689,6 +4689,58 @@ describe('actions/IOU', () => { }); }); }); + + test('should create transaction thread optimistically when initialReportID is undefined', () => { + const iouReport = buildOptimisticIOUReport(1, 2, 100, '1', 'USD'); + const transaction = buildOptimisticTransaction({ + transactionParams: { + amount: 100, + currency: 'USD', + reportID: iouReport.reportID, + }, + }); + const transactionCollectionDataSet: TransactionCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`]: transaction, + }; + const iouAction: ReportAction = buildOptimisticIOUReportAction({ + type: CONST.IOU.REPORT_ACTION_TYPE.CREATE, + amount: transaction.amount, + currency: transaction.currency, + comment: '', + participants: [], + transactionID: transaction.transactionID, + }); + const actions: OnyxInputValue = {[iouAction.reportActionID]: iouAction}; + const reportCollectionDataSet: ReportCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.REPORT}${iouReport.reportID}`]: iouReport, + }; + const actionCollectionDataSet: ReportActionsCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReport.reportID}`]: actions, + }; + const comment = 'hold reason for new thread'; + + return waitForBatchedUpdates() + .then(() => Onyx.multiSet({...reportCollectionDataSet, ...transactionCollectionDataSet, ...actionCollectionDataSet})) + .then(() => { + // When an expense is put on hold without existing transaction thread (undefined initialReportID) + putOnHold(transaction.transactionID, comment, undefined); + return waitForBatchedUpdates(); + }) + .then(() => { + return new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReport.reportID}`, + callback: (reportActions) => { + Onyx.disconnect(connection); + const updatedIOUAction = reportActions?.[iouAction.reportActionID]; + // Verify that IOU action now has childReportID set optimistically + expect(updatedIOUAction?.childReportID).toBeDefined(); + resolve(); + }, + }); + }); + }); + }); }); describe('unHoldRequest', () => { diff --git a/tests/unit/ReportActionsUtilsTest.ts b/tests/unit/ReportActionsUtilsTest.ts index dd5d8b5134be8..e7ab1831d99be 100644 --- a/tests/unit/ReportActionsUtilsTest.ts +++ b/tests/unit/ReportActionsUtilsTest.ts @@ -372,6 +372,12 @@ describe('ReportActionsUtils', () => { originalMessage: {...getOriginalMessage(mockIOUAction), IOUTransactionID}, }; + const linkedCreateActionWithoutChildReportID = { + ...mockIOUAction, + originalMessage: {...getOriginalMessage(mockIOUAction), IOUTransactionID}, + childReportID: undefined, + }; + const unlinkedCreateAction = { ...mockIOUAction, originalMessage: {...getOriginalMessage(mockIOUAction), IOUTransactionID: IOUExpenseTransactionID}, @@ -400,6 +406,11 @@ describe('ReportActionsUtils', () => { expect(result).toEqual(linkedCreateAction.childReportID); }); + it('should return CONST.FAKE_REPORT_ID when action exists but childReportID is undefined', () => { + const result = getOneTransactionThreadReportID(mockedReports[IOUReportID], mockedReports[mockChatReportID], [linkedCreateActionWithoutChildReportID], false, [IOUTransactionID]); + expect(result).toEqual(CONST.FAKE_REPORT_ID); + }); + it('should return undefined for action with a transaction that is not linked to it', () => { const result = getOneTransactionThreadReportID(mockedReports[IOUReportID], mockedReports[mockChatReportID], [unlinkedCreateAction], false, [IOUTransactionID]); expect(result).toBeUndefined(); From ac4ebc8020b6a95d45a1285b808e09555afd6902 Mon Sep 17 00:00:00 2001 From: VickyStash Date: Mon, 18 Aug 2025 14:10:00 +0200 Subject: [PATCH 09/11] Add getOneTransactionThreadReportAction test --- tests/unit/ReportActionsUtilsTest.ts | 76 ++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/tests/unit/ReportActionsUtilsTest.ts b/tests/unit/ReportActionsUtilsTest.ts index e7ab1831d99be..06e6d01fa421f 100644 --- a/tests/unit/ReportActionsUtilsTest.ts +++ b/tests/unit/ReportActionsUtilsTest.ts @@ -357,6 +357,82 @@ describe('ReportActionsUtils', () => { }); }); + describe('getOneTransactionThreadReportAction', () => { + const IOUReportID = `${ONYXKEYS.COLLECTION.REPORT}REPORT_IOU` as const; + const IOUTransactionID = `${ONYXKEYS.COLLECTION.TRANSACTION}TRANSACTION_IOU` as const; + const IOUExpenseTransactionID = `${ONYXKEYS.COLLECTION.TRANSACTION}TRANSACTION_EXPENSE` as const; + const mockChatReportID = `${ONYXKEYS.COLLECTION.REPORT}${mockChatReport.reportID}` as const; + const mockedReports: Record<`${typeof ONYXKEYS.COLLECTION.REPORT}${string}`, Report> = { + [IOUReportID]: {...mockIOUReport, reportID: IOUReportID}, + [mockChatReportID]: mockChatReport, + }; + + const linkedActionWithChildReportID = { + ...mockIOUAction, + originalMessage: {...getOriginalMessage(mockIOUAction), IOUTransactionID}, + childReportID: 'existingChildReportID', + }; + + const linkedActionWithoutChildReportID = { + ...mockIOUAction, + originalMessage: {...getOriginalMessage(mockIOUAction), IOUTransactionID}, + childReportID: undefined, + }; + + const unlinkedAction = { + ...mockIOUAction, + originalMessage: {...getOriginalMessage(mockIOUAction), IOUTransactionID: IOUExpenseTransactionID}, + }; + + const payAction = { + ...mockIOUAction, + originalMessage: { + ...getOriginalMessage(mockIOUAction), + IOUTransactionID, + type: CONST.IOU.REPORT_ACTION_TYPE.PAY, + }, + }; + + it('should return action when single IOU action exists', () => { + const result = ReportActionsUtils.getOneTransactionThreadReportAction(mockedReports[IOUReportID], mockedReports[mockChatReportID], [linkedActionWithChildReportID], false, [ + IOUTransactionID, + ]); + expect(result).toEqual(linkedActionWithChildReportID); + }); + + it('should return undefined when no linked actions exist', () => { + const result = ReportActionsUtils.getOneTransactionThreadReportAction(mockedReports[IOUReportID], mockedReports[mockChatReportID], [unlinkedAction], false, [IOUTransactionID]); + expect(result).toBeUndefined(); + }); + + it('should return undefined when multiple IOU actions exist', () => { + const result = ReportActionsUtils.getOneTransactionThreadReportAction( + mockedReports[IOUReportID], + mockedReports[mockChatReportID], + [linkedActionWithChildReportID, linkedActionWithoutChildReportID], + false, + [IOUTransactionID], + ); + expect(result).toBeUndefined(); + }); + + it('should skip PAY actions and return valid IOU action', () => { + const result = ReportActionsUtils.getOneTransactionThreadReportAction( + mockedReports[IOUReportID], + mockedReports[mockChatReportID], + [payAction, linkedActionWithoutChildReportID], + false, + [IOUTransactionID], + ); + expect(result).toEqual(linkedActionWithoutChildReportID); + }); + + it('should return undefined when only PAY actions exist', () => { + const result = ReportActionsUtils.getOneTransactionThreadReportAction(mockedReports[IOUReportID], mockedReports[mockChatReportID], [payAction], false, [IOUTransactionID]); + expect(result).toBeUndefined(); + }); + }); + describe('getOneTransactionThreadReportID', () => { const IOUReportID = `${ONYXKEYS.COLLECTION.REPORT}REPORT_IOU` as const; const IOUTransactionID = `${ONYXKEYS.COLLECTION.TRANSACTION}TRANSACTION_IOU` as const; From abb8ac884541a0a01bed35ba1594447b20750f0c Mon Sep 17 00:00:00 2001 From: VickyStash Date: Mon, 18 Aug 2025 19:26:09 +0200 Subject: [PATCH 10/11] Code improvements --- src/libs/actions/IOU.ts | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 3ff022a5f49fb..9ab901029f2d1 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -10924,19 +10924,19 @@ function putOnHold(transactionID: string, comment: string, initialReportID: stri const transaction = allTransactions[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; const iouReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transaction?.reportID}`]; const iouAction = getIOUActionForReportID(transaction?.reportID, transactionID); - let report: OnyxTypes.Report | undefined; + let transactionThreadReport: OnyxTypes.Report; // If there is no existing transaction thread report, we should create one // This way we ensure every held request has a dedicated thread for comments if (initialReportID) { - report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${initialReportID}`]; + transactionThreadReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${initialReportID}`] ?? ({} as OnyxTypes.Report); } else { const moneyRequestReport = getReportOrDraftReport(transaction?.reportID); - report = buildTransactionThread(iouAction, moneyRequestReport, undefined, reportID); + transactionThreadReport = buildTransactionThread(iouAction, moneyRequestReport, undefined, reportID); } const optimisticCreatedAction = buildOptimisticCreatedReportAction(currentUserEmail); - const parentReportActionOptimistic = getOptimisticDataForParentReportAction(report, createdReportActionComment.created, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD); + const parentReportActionOptimistic = getOptimisticDataForParentReportAction(transactionThreadReport, createdReportActionComment.created, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD); const optimisticData: OnyxUpdate[] = [ { @@ -11026,23 +11026,21 @@ function putOnHold(transactionID: string, comment: string, initialReportID: stri onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, value: { - lastVisibleActionCreated: report?.lastVisibleActionCreated, + lastVisibleActionCreated: transactionThreadReport.lastVisibleActionCreated, }, }, ]; // If the transaction thread report wasn't created before, we create it optimistically if (!initialReportID) { + transactionThreadReport.pendingFields = { + createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, + }; optimisticData.push( { onyxMethod: Onyx.METHOD.SET, key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, - value: { - ...report, - pendingFields: { - createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, - }, - }, + value: transactionThreadReport, }, { onyxMethod: Onyx.METHOD.MERGE, @@ -11062,13 +11060,13 @@ function putOnHold(transactionID: string, comment: string, initialReportID: stri // We link the IOU action to the new transaction thread by setting childReportID optimistically optimisticData.push({ onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThreadReport.parentReportID}`, value: {[iouAction?.reportActionID]: {childReportID: reportID, childType: CONST.REPORT.TYPE.CHAT}}, }); // We reset the childReportID if the request fails failureData.push({ onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThreadReport.parentReportID}`, value: {[iouAction?.reportActionID]: {childReportID: null, childType: null}}, }); } From 04911a35f26d50d061ccb7cd20f4d8bf41db2c6e Mon Sep 17 00:00:00 2001 From: VickyStash Date: Mon, 18 Aug 2025 19:41:17 +0200 Subject: [PATCH 11/11] TS fixes after merging main --- tests/unit/ReportActionsUtilsTest.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/unit/ReportActionsUtilsTest.ts b/tests/unit/ReportActionsUtilsTest.ts index 2f3796c3ed3f8..05db10b1a504f 100644 --- a/tests/unit/ReportActionsUtilsTest.ts +++ b/tests/unit/ReportActionsUtilsTest.ts @@ -366,28 +366,30 @@ describe('ReportActionsUtils', () => { [IOUReportID]: {...mockIOUReport, reportID: IOUReportID}, [mockChatReportID]: mockChatReport, }; + // eslint-disable-next-line @typescript-eslint/non-nullable-type-assertion-style + const originalMessage = getOriginalMessage(mockIOUAction) as OriginalMessageIOU; const linkedActionWithChildReportID = { ...mockIOUAction, - originalMessage: {...getOriginalMessage(mockIOUAction), IOUTransactionID}, + originalMessage: {...originalMessage, IOUTransactionID}, childReportID: 'existingChildReportID', }; const linkedActionWithoutChildReportID = { ...mockIOUAction, - originalMessage: {...getOriginalMessage(mockIOUAction), IOUTransactionID}, + originalMessage: {...originalMessage, IOUTransactionID}, childReportID: undefined, }; const unlinkedAction = { ...mockIOUAction, - originalMessage: {...getOriginalMessage(mockIOUAction), IOUTransactionID: IOUExpenseTransactionID}, + originalMessage: {...originalMessage, IOUTransactionID: IOUExpenseTransactionID}, }; const payAction = { ...mockIOUAction, originalMessage: { - ...getOriginalMessage(mockIOUAction), + ...originalMessage, IOUTransactionID, type: CONST.IOU.REPORT_ACTION_TYPE.PAY, }, @@ -452,7 +454,7 @@ describe('ReportActionsUtils', () => { const linkedCreateActionWithoutChildReportID = { ...mockIOUAction, - originalMessage: {...getOriginalMessage(mockIOUAction), IOUTransactionID}, + originalMessage: {...originalMessage, IOUTransactionID}, childReportID: undefined, };