[MEDIUM][$250] Start creating optimistic transactionThreadReportID and createdReportActionIDForThread and passing it in HoldRequest

[iwiznia]

We are about to change some of our backend code to not create the transaction thread reports when we create a new money request.

In order to do that, we need to ensure that on the action IOU.putOnHold, we start creating the data for the transaction thread report and its created action optimistically (if the data does not exist yet), the same way we do it for the requestMoney action and pass the transactionThreadReportID and createdReportActionIDForThread to the API request.

I also see that the API.write call for putOnHold is not using the HOLD_MONEY_REQUEST constant, so let's fix that too.

Issue Owner

Current Issue Owner: @allroundexperts

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021869115090392228823
• Upwork Job ID: 1869115090392228823
• Last Price Increase: 2024-12-24
• Automatic offers:
• FitseTLT | Contributor | 105584957

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (External)

[melvin-bot]

@iwiznia, @parasharrajat Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

Triggered auto assignment to @muttmuure (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-11-28 23:18:57 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Start creating optimistic transactionThreadReportID and createdReportActionIDForThread and passing it in HoldRequest

What is the root cause of that problem?

New Feature - start creating optimistic transactionThread report and created action for the thread when the transaction thread doesn't exist

What changes do you think we should make in order to solve the problem?

First now that we are not guaranteed to always have a transaction thread we will base our navigation on parentReportID and parentReportActionID params

App/src/ROUTES.ts

Lines 427 to 430 in 64eaf2f

	getRoute: (type: ValueOf<typeof CONST.POLICY.TYPE>, transactionID: string, reportID: string, backTo: string, searchHash?: number) => {
	const route = searchHash
	? (`${type}/edit/reason/${transactionID}/${searchHash}/?backTo=${backTo}&reportID=${reportID}` as const)
	: (`${type}/edit/reason/${transactionID}/?backTo=${backTo}&reportID=${reportID}` as const);

getRoute: (type: ValueOf<typeof CONST.POLICY.TYPE>, transactionID: string, parentReportID: string, parentReportActionID: string, backTo: string, searchHash?: number) => {
            const route = searchHash
                ? (`${type}/edit/reason/${transactionID}/${searchHash}/?backTo=${backTo}&parentReportID=${parentReportID}&parentReportActionID=${parentReportActionID}` as const)
                : (`${type}/edit/reason/${transactionID}/?backTo=${backTo}&parentReportID=${parentReportID}&parentReportActionID=${parentReportActionID}` as const);
            

We should navigate accordingly here

 Navigation.navigate(
            // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
            ROUTES.MONEY_REQUEST_HOLD_REASON.getRoute(
                policy?.type ?? CONST.POLICY.TYPE.PERSONAL,
                transactionID,
                moneyRequestReportID ?? '',
                reportAction.reportActionID,
                backTo || activeRoute,
                searchHash,
            ),

We will update HoldReasonPage here according to the new params

App/src/pages/iou/HoldReasonPage.tsx

Lines 39 to 47 in 64eaf2f

	const {transactionID, reportID, backTo, searchHash} = route.params;
	const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID || -1}`);
	// We first check if the report is part of a policy - if not, then it's a personal request (1:1 request)
	// For personal requests, we need to allow both users to put the request on hold
	const isWorkspaceRequest = ReportUtils.isReportInGroupPolicy(report);
	const parentReportAction = ReportActionsUtils.getReportAction(report?.parentReportID ?? '-1', report?.parentReportActionID ?? '-1');

    const {transactionID, parentReportID, parentReportActionID, backTo, searchHash} = route.params;

    const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${parentReportID || -1}`);

    // We first check if the report is part of a policy - if not, then it's a personal request (1:1 request)
    // For personal requests, we need to allow both users to put the request on hold
    const isWorkspaceRequest = ReportUtils.isReportInGroupPolicy(parentReport);
    const parentReportAction = ReportActionsUtils.getReportAction(parentReportID ?? '-1', parentReportActionID ?? '-1');

and then we will pass only the iou parent action from here

App/src/pages/iou/HoldReasonPage.tsx

Line 56 in 64eaf2f

IOU.putOnHold(transactionID, values.comment, reportID, searchHash);

        IOU.putOnHold(transactionID, values.comment, parentReportAction, searchHash);

Then inside putOnHold we will create the transaction thread and created action if the transaction thread report doesn't exist

function putOnHold(transactionID: string, comment: string, iouAction?: OnyxEntry<ReportAction>, searchHash?: number) {
    let reportID = iouAction?.childReportID;
    const optimisticData: OnyxUpdate[] = [];
    const successData: OnyxUpdate[] = [];
    const failureData: OnyxUpdate[] = [];
    if (!reportID) {
        const iouReportID = ReportActionsUtils.getOriginalMessage(iouAction)?.IOUReportID ?? 0;
        const iouReport = ReportUtils.getReport(iouReportID);
        const transactionThread = ReportUtils.buildTransactionThread(iouAction, iouReport);
        reportID = transactionThread.reportID;
        const createdActionForTransactionThread = ReportUtils.buildOptimisticCreatedReportAction(currentUserEmail);
        optimisticData.push(
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
                value: transactionThread,
            },
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThread.reportID}`,
                value: {
                    [createdActionForTransactionThread.reportActionID]: createdActionForTransactionThread as ReportAction,
                },
            },
        );
        successData.push(
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.REPORT}${transactionThread.reportID}`,
                value: {
                    participants: {},
                    pendingFields: null,
                    errorFields: null,
                    isOptimisticReport: false,
                },
            },
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThread.reportID}`,
                value: {
                    [createdActionForTransactionThread?.reportActionID ?? '-1']: {
                        pendingAction: null,
                        errors: null,
                    },
                },
            },
        );
        failureData.push(
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.REPORT}${transactionThread.reportID}`,
                value: {
                    errorFields: {
                        createChat: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('report.genericCreateReportFailureMessage'),
                    },
                },
            },
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThread.reportID}`,
                value: {
                    [createdActionForTransactionThread?.reportActionID ?? '-1']: {
                        errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericHoldExpenseFailureMessage', errorKey),
                    },
                },
            },
        );
    }

and also pass the transaction thread report id and created action id as a param here

App/src/libs/actions/IOU.ts

Lines 7959 to 7964 in 64eaf2f

	'HoldRequest',
	{
	transactionID,
	comment,
	reportActionID: createdReportAction.reportActionID,
	commentReportActionID: createdReportActionComment.reportActionID,

And on more thing is our areHoldRequirementsMet will be false here for our case

App/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx

Lines 187 to 188 in 64eaf2f

	const areHoldRequirementsMet =
	!isInvoiceReport && isMoneyRequestOrReport && !ReportUtils.isArchivedRoom(transactionThreadReportID ? childReport : parentReport, parentReportNameValuePairs);

as the calculation depends on the child report of the report action where the context menu is displayed for but in our case there is no transaction thread so no childReport so we need to determine the hold requirements only from the data we get from the report action

App/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx

Line 185 in 64eaf2f

const isMoneyRequestOrReport = isMoneyRequestReport || isSingleTransactionView;

    const isParentActionOfRequest = !isEmptyObject(parentReportAction) && ReportActionsUtils.isTransactionThread(parentReportAction);
    const isMoneyRequestOrReport = isMoneyRequestReport || isSingleTransactionView || isParentActionOfRequest;

What alternative solutions did you explore? (Optional)

[melvin-bot]

@iwiznia, @parasharrajat, @muttmuure Eep! 4 days overdue now. Issues have feelings too...

[iwiznia]

@parasharrajat can you review the proposal please?

[parasharrajat]

Sure..

[melvin-bot]

@iwiznia @parasharrajat @muttmuure this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

@iwiznia, @parasharrajat, @muttmuure Whoops! This issue is 2 days overdue. Let's get this updated quick!

[muttmuure]

@parasharrajat have you had a chance to look at the proposal yet? Could you give an ETA for when you can take a look?

[parasharrajat]

Yes, I reviewed it. Checking a few things, I will update asap.

[iwiznia]

Bump @parasharrajat

[parasharrajat]

Thanks @iwiznia. I will have the next update by tomorrow morning. Fever knocked me down last week so couldn't focus.

[melvin-bot]

@iwiznia, @JmillsExpensify, @allroundexperts, @FitseTLT Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[iwiznia]

I think this should happen automatically already once we deploy the PR to not create the threads proactively.

This will get deployed today/tomorrow. @allroundexperts you will need to stop sending the optimistic transactionThreadID in the RequestMoney api request in order for the report to not be created proactively. LMK if it works or you need some help.

[JmillsExpensify]

Thanks for the update. Not sure if I'm the one that's supposed to comment to remove the overdue?

[allroundexperts]

@FitseTLT Can you please handle #52935 (comment)?

[FitseTLT]

@iwiznia I did try to omit transactionThreadReportID from the parameter and the result I saw is the IOU action's childReportID is set to a number instead of string.

      "childReportID": 5990080141407485,

We will need to create transaction thread optimistically and pass the transactionThreadReportID and createdReportActionIDForThread as param in both open report and hold API. But we will need to know whether a transaction thread linked to the iou action exists or not because the inexistence of the transaction thread report sometimes can be due to the inavailability of it in onyx but we shouldn't create the transaction thread in that case.

So can we add some prop in the iou parent action of the transaction thread to indicate that it is not created yet, sth like transactionThreadReportCreated?

[iwiznia]

Oh wait, we had to revert the PR I mentioned because it caused issues and I haven't sent the new PR again.

[iwiznia]

The new PR is up for review

[JmillsExpensify]

PR up for review

[mallenexpensify]

Are any of these held on another?

• [MEDIUM][$250] Start creating optimistic transactionThreadReportID and createdReportActionIDForThread and passing it in HoldRequest #52935
• [Due for payment 2025-05-22] [$125] Do not create optimistic transaction thread report on RequestMoney #58828
• https://github.com/Expensify/Expensify/issues/433103

For this issue, what's the PR we're waiting to merge? Thx

[iwiznia]

We are waiting for this to get deployed https://github.com/Expensify/Web-Expensify/pull/45944
Which should be done today/tomorrow to staging, which is good enough for @FitseTLT to start testing

[JmillsExpensify]

@FitseTLT have you been able to test yet?

[FitseTLT]

@iwiznia Is it ready for test? If yes how should I test? And also, I was wondering if what we are doing here has any relation with beta TableReportView?

I also see another issue here linked to this one.

[iwiznia]

@iwiznia Is it ready for test? If

yes, it should be

If yes how should I test?

Make sure you are in the latest code (which should not create an optimistic thread when calling RequestMoney) and then put the transaction on hold.

[iwiznia]

And also, I was wondering if what we are doing here has any relation with beta TableReportView?

I think not?

[melvin-bot]

This issue has not been updated in over 15 days. @iwiznia, @JmillsExpensify, @allroundexperts, @FitseTLT eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[allroundexperts]

@FitseTLT Do you have an update here?

[JmillsExpensify]

Good question. I agree that it'd be good to pick this back up.

[FitseTLT]

We need to wait for #58828