Persist IOU data in Onyx

[Jag96]

Details

This PR adds a new API command for GetIOUReport and starts persisting the IOUReport collection in Onyx.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/157104

Tests

1. On web, log out of e.cash and open the console (cmd + option + j)
2. Open the Application tab and select localhost:8080 under Local Storage
   
3. Log into an account without an IOU and check the LocalStorage keys, confirm there is nothing shown if you filter by reportIOU
   
4. Use the CreateIOUTransaction Curl request from https://github.com/Expensify/Expensify/issues/157104#issue-829068854 to create an IOU transaction. Ensure the debtorEmail is different from the email/accountID/authToken you set in the cookie field
5. Log into cash as the account of the person who created the IOU. Confirm that you see a value in localStorage for reportIOU_ that contains the IOU reportID. For an example of the full data, see https://github.com/Expensify/Expensify/issues/157104#issuecomment-799886417

6. Log out, and log into cash using the debtorEmail value you used in step 4, confirm you see a value in localStorage for reportIOU_ that contains the IOU reportID
7. Create an IOUTransaction from an account A that has been the only one to comment on a chat report and User B is the debtor.
8. Confirm that when logging in with UserA, the reportIOU collection contains that report.

Additionally, confirm that there are no regressions with sending messages to other accounts

Tested On

• Web
• Mobile Web
• Desktop
• iOS
• Android

Screenshots

N/A

[Julesssss]

Looks great, approving for now in case no additional changes are pushed 👍

[Jag96]

Updated, #1785 (comment) is the only unresolved thread at the moment

[Jag96]

#1785 (comment) has been resolved, this is ready for review again!

[Julesssss]

I noticed two issues when running the test:

1. I'm seeing an extra undefined 'actorEmail' in otherParticipants
   
   This makes the (otherParticipants.length > 1) condition true, preventing the getIOUReport request. Once I added .without(undefined), I was able to verify that the data was persisted to Onyx. Didn't look further into why this occurs or a better fix.

2. I'm only seeing IOU data when logged into the account of the IOU creator
   I think this is because we're using actorEmail, which is the same value for both users. Perhaps we should use managerEmail or ownerEmail (whichever does not match the current user). These will swap depending on who owes who at the current point in time, but both emails should be available this way.

[Julesssss]

Looking good, but a couple of issues mentioned above need addressing

[Jag96]

I'm seeing an extra undefined 'actorEmail' in otherParticipants

I'm unable to reproduce this, what is the account setup you have for IOU testing?

I'm only seeing IOU data when logged into the account of the IOU creator

I'm also unable to reproduce this, I see the IOU data get populated when logged in as both the IOU creator and the debtor. I will try testing with some additional cases (no report_actions from the debtor, create a report with IOUs from both parties) to try to reproduce.

That testing brings up a good point however, since we're using the actorEmail to build the list of participants we may not be able to get the debtorEmail if the only person who has chatted on the report is the requestor. I'll have a look at that test case and if it is a problem, look into getting the debtorEmail another way.

[Jag96]

After some testing, I confirmed the issues above exist if the requestor is the only one with chats on a chat report. Therefore, using the actorEmail values in the reportHistory isn't enough to get the participants on a report.

Perhaps we should use managerEmail or ownerEmail (whichever does not match the current user)

The problem with this is that we don't have a managerEmail or ownerEmail in the response from getHistory. Additionally, fetching actions happens separately from where we fetch the rest of the report data, so we don't have access to the participants inside getActions where we're filtering by the "IOU" actionName.

I looked into creating a local object that keeps track of the participants of each report, similar to how we keep track of lastReadSequenceNumbers or reportMaxSequenceNumbers. Then, in handleReportChanged, I tried setting the value of chatReportParticipants[report.reportID] = report.participants. However, there is still a race condition here since the actions can fetch before the participants are updated.

Forcing the reportActions to wait until we are finished fetching the reportData is one way to fix this, but I'd rather not add that limitation unless we need to. Alternatively, we can make updates to the API to include participant data in one of the other API calls (GetHistory?). Bringing this to slack to chat about alternatives.

[Jag96]

I've created https://github.com/Expensify/Web-Expensify/pull/30506 and updated this PR so that GetIOUReport now takes reportID instead of debtorEmail. I've also updated the tests to account for the failing case as well as the Web branch required for testing.

In slack we agreed that the ideal solution would be to have Auth or the API handle figuring out the participants for a chat report, and to return all the necessary data, which will be handled in https://github.com/Expensify/Expensify/issues/157722.

This is ready for review, but now requires https://github.com/Expensify/Web-Expensify/pull/30506 to work properly

[Jag96]

Thanks for all the comments and discussion! Updated this based on the discussion in slack. Since we're already fetching reportActions as part of fetchChatReportsByIDs when we call GetReportStuff, we can grab the participants and reportActionList there, which is all that is required to call GetReportIOU.

I've also added some error handling for non-200 errors, empty reportList responses, and reportIDs of 0. I ran through the tests again and confirmed everything is working. This is ready for review again.

[Julesssss]

Agree with a a few previously mentioned comments.

Ran through the tests with no issues this time, Great work!

[Jag96]

@iwiznia @tgolen Updated

[Jag96]

Updated and re-tested

[OSBotify]

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

[tgolen]

Sorry about not merging it! I was waiting for a test to finish... and apparently the GH page never updated when it was done.

[Jag96]

No worries! Those tests take a while!

[isagoico]

@Jag96 Hello! This one looks like it's internal QA. If so, can you go ahead with the testing? When done please let me know so I check the PR of the list. Thanks in advance!

[Jag96]

Checked this off 👍

[Julesssss]

This morning @trjExpensify and I noticed a potential regression with the Onyx changes:

The iou loading state values that we initialize here are returned null here when opening the IOUModal. The data is valid within local storage and this is only reproducible about 1 in 4 attempts.

While the simple solution would be to add a conditional check to confirm the iou object is valid, we need to confirm whether this requires a fix, or is now expected behavior.

CC @kidroca

IOURequestOnyxCrash.mov

[Julesssss]

Moving this discussion to the PR, so that @kidroca can participate