IOU Details Modal

[Julesssss]

Details

• Introduces the IOU Details Modal, which allows users to view both active and settled IOU Reports and to settle them. Users can open IOU reports by tapping 'Pay' or 'View Details' from a chat with an outstanding IOU report
• I have also split ReportActionItemIOUPreview into two distinct components so that we can reuse them individually within IOU Details. ReportActionItemIOUAction now sits above ...IOUPreview and ...IOUQuote and selectively displays one or both of these components

Not included in this PR

• Rejecting transactions
• Displaying the transaction canceled/declined message
• Displaying the IOU settled message

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/158811
Fixes https://github.com/Expensify/Expensify/issues/163036

Tests

NOTE: If you are unable to build to iOS and are on the latest OSX version, you should disable Flipper locally
NOTE: You should test against the latest web and Auth commits
NOTE: IOU Report logic has recently changed, you should create new IOUs for testing
NOTE: This can occasionally be flakey on dev, due to requests timing out -- before creating a transaction, or paying an iouReport, please clear the network tab in devtools and ensure that the request succeeds_

A) Test the View Details and Pay opens the IOU

1. Create an IOU Request of any amount, you should see the following, with 'user owes user' message in the Preview Component

2. As the creator, click 'View Details' on the Preview component that is displayed
3. The Details modal should be displayed

4. You should not see a green 'I'll settle up elsewhere' button
5. Now login to the receiving user, navigate to the same chat report
6. you should see the following Preview Component

7. click on 'Pay' AND 'View Details', both should launch the Details Modal

8. Tap the I'll settle up elsewhere button, the button should show a loading indicator, and eventually disappear

9. The Preview message should now read 'user paid user' and the 'Ill settle up elsewhere' button should not be displayed

B) Test that paid reports are displayed correctly
10. Log out, and back into the app as either user
11. To avoid an external issue, refresh the page
12. Navigate to a chatReport with a paid IOU, you should see a 'Settled up elsewhere' message

13. Tap the 'View Details' button
14. You should see a loading indicator while the paid report is retrieved

15. It should then display, without the green 'I'll settle up elsewhere' button

C) Test that View Details is not displayed for group Bill Splits
16. Create a New Group from the global menu
17. WIthin this new group, click the + icon and select 'Split Bill'
18. Create an IOU Split of any amount
19. Locate the message that is generated ('Split $2.00 with Jules... User and User10')

20. You SHOULD NOT see a 'View Details' link beneath the message
    (notice there is no View Details link)

QA Steps

• Run the above tests

Tested On

• Web
• Mobile Web
• Desktop
• iOS
• Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

[NikkiWines]

Retested and didn't experience either of the errors that @marcaaron ran into. Looks good.

[NikkiWines]

Actual: Sometimes only the report action quoted text is displayed and the preview component does not display at all.

FWIW, I've experienced this sporadically as well but it seems to be tied to VM slowness from what I could tell - though I didn't investigate further than to retry several times to see if I could repro consistently (which I couldn't)

[Julesssss]

FWIW, I've experienced this sporadically as well but it seems to be tied to VM slowness from what I could tell - though I didn't investigate further than to retry several times to see if I could repro consistently (which I couldn't)

I'm certain this is the case. During development I would wait until all network requests had completed before creating a transaction or settling a report. Even then, they would fail 50% of the time 😩

[Julesssss]

@marcaaron I've fixed the LoadingIndicator issue, so you should now see a loading icon when a paid report is loaded. Also, I've rewritten the tests to simplify them and added screenshots for reference.

Would you mind retesting, thanks 🙏

When testing for the sporadic issue where Preview component wouldn't display -- it would always be associated with a failed network request, usually 555.

[marcaaron]

Even then, they would fail 50% of the time

So the network request returned 200 but failed in some other way? In the cases where I observed this the network requests eventually returned 200. So it made me think that the UI is not updating correctly or dependent on something happening "fast". But maybe that's not the case?

[marcaaron]

When testing for the sporadic issue where Preview component wouldn't display -- it would always be associated with a failed network request, usually 555.

I'll look for this when re-testing. Didn't see anything like that in the first go.

[Julesssss]

In this example, the IOU was sent by test account to Marcus and then paid by Marcus but the preview component says that the tester paid Marcus.

Finally, I also believe this issue is due to a failed request, as the report hasn't updated locally. Could you please try this again?

[Julesssss]

As you can imagine, it has been an absolute nightmare to develop against an API that failed 30% of the time

[marcaaron]

Sorry I re-tested and it's still not correct

[marcaaron]

Also observed the inconsistency again during re-test + no errors at all in the networking tab

CreateIOUTransaction succeeded

Get&rvl=reportStuff succeeded

No preview component appears.

[marcaaron]

Sorry, I know it's been a long review @Julesssss. To clarify, I am OK with merging this after we fix the incorrect text that shows who owes/paid who. If the inconsistency surfaces in production we can address it. Seems like I am the only one experiencing that particular bug.

[Julesssss]

@marcaaron I've spent some time looking into the Preview issue you mentioned. I can reproduce it with new conversations (the creator will not see the Preview Component, but the receiver will). I'm unable to make any further progress so have had to separate this out to a new issue.

I've reversed the user 'paid' user message (I accidentally reversed this when implementing translations 😩)

[tgolen]

I'm definitely not a fan of this promise being returned, and I think there are several other patterns that we have which can remove the need to return the promise here. For sake of #urgency, I am fine merging this as-is and then cleaning it up later. Here are some of the solutions that I think would ultimately make the code cleaner (in order of preference).

1. Solve more of the issues in the API layer rather than forcing the front-end to do a lot of work to keep all the data happy. For example, rather than expecting the front-end to try and keep the reportIDs in sync, the server should push data to e.cash that is just put straight into Onyx using pusher (this was the original dream).
2. Rather than having two methods (fetchIOUReportByID and fetchIOUReportByIDAndUpdateChatReport), add a third parameter to this method called shouldUpdateChatReport and move the logic into this method.
3. Have a callback connected to Onyx which looks for changes to the IOU report (eg. like after it's fetched from the server) and then it does the appropriate actions to keep the reportIDs in sync. This is similar to the pattern that we have for fetching new report data (here).

[Julesssss]

Fair points. The original PR used suggestion #2, but this led to other issues as the logic became quite complex and I switched to two functions in an attempt to make the code simpler to understand.

I am already planning to do #1, but I have added #3 to the post-launch improvements on the IOU tracker as this seems a worthy improvement.

Thanks for the review!

[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.

[kavimuru]

White screen displayed after clicking 'View Details"

Expected Result:

The Details modal should be displayed

Actual Result:

White screen displayed after clicking 'View Details"

Actions Performed:

1. Lunch the app
2. Log in with expensifail account
3. Select any user
4. Click on plus button and click Request Money
5. Put amount and click next
6. As the creator, click 'View Details' on the Preview component that is displayed.

Platform:

Web ✔️
Desktop ❌
iOS ✔️
mWeb ✔
Android ✔️

Build:

1.0.49-0

Notes/Images/Video:

Bug5076215_Screen_Recording_20210519-140913_Expensifycash.mp4

[OSBotify]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[Julesssss]

Created a new issue for this bug, but it was already fixed as part of this IOU PR

I'm removing the deploy blocker label, as this feature is behind a beta and was not working prior to this PR - so nothing has really changed.