Send optimistic transactionThreadReportID and createdReportActionIDForThread with API commands RequestMoney, CreateDistanceRequest, CompleteSplitBill, SendMoney

[cead22]

• Design Doc
• The back end changes will be done in a separate issue
• Update getMoneyRequestInformation to generate an optimistic transactionThreadReportID and createdReportActionIDForThread using ReportUtils.buildTransactionThread
  • Here's a commit with a proof-of-concept for these changes
    • We should make sure we're updating optimisticData with both values correctly
    • Ignore the comment // TODO add logic to create transaction thread for each DM between split participants
• Update getSendMoneyParams like getMoneyRequestInformation, send transactionThreadReportID and createdReportActionIDForThread, and add this to the request's optimisticData
• For the case of CompleteSplitBill where the user enters details manually before SmartScan finishes, we’ll generate random reportIDs in the client
  • Create a transactionThread and a created report action optimistically before the call to buildOnyxDataForMoneyRequest inside completeSplitBill like we do in getMoneyRequestInformation
  • Pass optimisticCreatedAction.reportID as the last param to the buildOnyxDataForMoneyRequest call
  • Add transactionThreadReportID: transactionThread.reportID and createdReportActionIDForThread: optimisticCreatedAction.reportActionID to the params object we pass to API.write(‘CompleteSplitBill’, {params})

[cead22]

@cdanwards can you comment here so I can assign you?

[cdanwards]

Commenting for Assignment

[melvin-bot]

@cdanwards Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@cdanwards Eep! 4 days overdue now. Issues have feelings too...

[cdanwards]

No update.

[melvin-bot]

@cdanwards Huh... This is 4 days overdue. Who can take care of this?

[cdanwards]

Next in line for work

[cdanwards]

Beginning work on this

[melvin-bot]

@cdanwards Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@cdanwards Eep! 4 days overdue now. Issues have feelings too...

[cdanwards]

Getting ready for PR

[paultsimura]

@cead22 @cdanwards can we assume these changes will fix the following issue?
A scan request is placed with an optimistic transactionThreadReportID, but when the RequestMoney API response comes, this transactionThreadReportID is replaced with another (which comes from BE). As a result, if the optimistic transaction thread report was opened, it becomes invalid and greyed out:

Screen.Recording.2023-12-06.at.17.24.40-compressed.mp4

[cead22]

Yes, we're in the process of updating the RequestMoney end point to accept and use the new parameters transactionThreadReportID and createdReportActionIDForThread

[cdanwards]

@cead22 I've been having some issues with the tests for IOU while implementing this. I made an assumption that we'd also need to pass both optimisticCreatedActionReportID and optimisticCreatedActionIDForThread to buildOnyxDataForMoneyRequest. Like so:

function buildOnyxDataForMoneyRequest(
   ...
   transactionThread,
    transactionThreadReportAction,
) {
  const optimisticData = [
  ...
        {
            onyxMethod: Onyx.METHOD.SET,
            key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThread.reportID}`,
            value: {
                [transactionThreadReportAction.reportActionID]: transactionThreadReportAction,
            },
        },
  ...

And then update the necessary success and failure data.

So a few questions:

1. Is this the correct way to implement this?
2. If so, is there a good way to isolate the created transaction thread for testing like we do here for iou and chat reports?
3. If not, what report should the created action be attributed to?

[cead22]

Started a thread on slack to discuss in real time https://expensify.slack.com/archives/C01GTK53T8Q/p1702070081420789

[melvin-bot]

@cdanwards Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[cdanwards]

Was prioritizing some other issues but this is next to finish

[lindboe]

@cead22 @cdanwards and I had a question about this part:

Add transactionThreadReportID: transactionThread.reportID and createdReportActionIDForThread: optimisticCreatedAction.reportActionID to the params object we pass to API.write(‘CompleteSplitBill’, {params})

We're creating multiple transaction threads for splits in a loop, but we only send one params object. It looks like for things we create multiples of on this command, there's a splits key. Is that where we should be adding the IDs? So it would look like:

API.write(
  'CompleteSplitBill,
  {
    ...
    splits:
    [{
      transactionThreadReportID: ...,
      createdReportActionIDForThread: ...,
     },
     {
     ...
     }],
  });

edit; I think the same would apply for SplitBill and SplitBillAndOpenReport (via createSplitsAndOnyxData).

[melvin-bot]

@lindboe, @cdanwards Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[cdanwards]

Working on finalizing and getting up a PR!

[melvin-bot]

Triggered auto assignment to @johncschuster (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[cead22]

@johncschuster please issue payment to C+ @situchan for PR review. Thanks!

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[johncschuster]

@situchan I've extended an invite, here. Can you accept that? Thanks!

[johncschuster]

@cead22 it looks like this issue was linked to a deploy blocker. Has that been resolved, or do I need to wait on issuing payment?

[situchan]

This issue was mentioned here which made Melvin post above message. Not regression.

[johncschuster]

Payment Summary:

Contributor: @cdanwards - Contractor
C+ Reviewer: @situchan - $500 - Upwork job offer

[johncschuster]

Paid!

[BiswaJK]

Triggered auto assignment to @cead22 , see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[melvin-bot]

📣 @BiswaJK! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>