[HOLD 19119][$1000] Offline - Infinite loading in case of Send a money to the user you have never chatted before

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Go offline
2. Send a money to the user you have never chatted before with settled up elsewhere

Expected Result:

Greyed out IOU should be displayed

Actual Result:

Infinite loading

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.16.5
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Bug6061287_RPReplay_Final1684511309.mp4

Expensify/Expensify Issue URL:
Issue reported by: applause internal team
Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010c9cbd6bae33acb1
• Upwork Job ID: 1660205816623935488
• Last Price Increase: 2023-05-21

[melvin-bot]

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~010c9cbd6bae33acb1

[melvin-bot]

Current assignee @dylanexpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @MonilBhavsar (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[hoangzinh]

Proposal

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

Offline - Infinite loading in case of Send a money to the user you have never chatted before

What is the root cause of that problem?

• One of condition to loading skeleton view for ReportActionsView is this line
  

  App/src/pages/home/ReportScreen.js

  Line 232 in 8180113

  const isLoadingInitialReportActions = _.isEmpty(this.props.reportActions) && this.props.report.isLoadingReportActions;

  
  The condition isLoadingReportActions is always true when we open a Report. So the issue here is why reportActions is empty. It should have some at least This is the beginning of your chat with (optimistic) report actions.

• When we build optimisticData for API SendMoneyElsewhere. There are a few changes since this PR:

  • The optimisticReportActionsData has been changed to IOUReport
  • Then, when we add optimisticCreatedAction, we added to reportActions of IOUReport (It should be added to reportActions of chatReport as before)

=> It's reason why reportActions of chatReport is empty => It shows skeleton reportActions view when we navigate to chatReport in offline mode.

• We also don't set optimistic reportAction Preview yet, so we can't see grayed IOU message in chatReport.

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

I think we need to replicate what we did for RequestMoney, both FE + BE. For more details:

• FE:

  • Build another optimisticCreatedActionForChat, same as existing optimisticCreatedAction. But I think we need to pass managerEmail instead of recipientEmail, because managerEmail is creator. Then add it to optimistic reportActions of chatReport. Then we need to pass it as an API request params same as RequestMoney API
  • For the existing optimisticCreatedAction, same as above, we need to pass managerEmail and pass it as an API request params same as RequestMoney API
  • Build reportPreviewAction same as here. But we need to call addition util ReportUtils.getIOUReportActionMessage to get default message. Then put it into optimistic reportActions of chatReport. Then we need to pass it as an API request params same as RequestMoney API

• BE:

  • BE needs to update so that it picks up those new params and create with those reportActionIDs in BE side. Otherwise:
    • It will create duplicated those report actions when we go online
    • Interact actions with optimistic reportAction (I.e emoji icon...) will be gone when we go online.

[fedirjh]

@hoangzinh Thank you for the proposal. However, I don't believe you are addressing the correct issue. I can see from your screenshot that the result differs from what was expected. Please double-check the expected result:

Greyed out IOU should be displayed

Actually IOU Preview is not displayed on offline mode, even If we have an existing chat report

Screen.Recording.2023-05-21.at.11.14.09.PM.mov

[hoangzinh]

@fedirjh Thanks for quickly review proposal. I'm confused on term "IOU". So what is our expected:
1.

2.

which one should be greyed out in chatReport?

[fedirjh]

@hoangzinh For chat report , I think any action taken offline should be grayed out same as new comment. "IOU" refers to "I owe you". I think our expected result is the second one for this case.

[tienifr]

Proposal

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

Infinite loading when sending money to another user that never chats before.

What is the root cause of that problem?

There're 2 reports that are of concern when sending money:

1. Regular chat report between 2 users
2. IOU report

In getSendMoneyParams here

App/src/libs/actions/IOU.js

Line 993 in 8180113

const optimisticData = [optimisticChatReportData, optimisticIOUReportData, optimisticReportActionsData, optimisticTransactionData];

, we're only constructing these optimistic report actions data for the IOU report:

• The IOU report action
• Chat created action (if it's a new chat)
  But we're not adding any optimistic report action for the regular chat report. There's no report actions in the regular chat report so it will show infinite loading as per this line:

  App/src/pages/home/ReportScreen.js

  Line 232 in 8180113

  const isLoadingInitialReportActions = _.isEmpty(this.props.reportActions) && this.props.report.isLoadingReportActions;

  .

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

We need to add these optimistic report actions for the Regular chat as well:

• The IOU report action (The one that says Settled up ${amount} elsewhere). This is quite similar to the IOU report action in the IOU report here, with the following differences:
  • This should use the REPORTPREVIEW action name
  • The message should be changed to the correct format
• Chat created action (if it's a new chat)
  • We can use the in buildOptimisticCreatedReportAction in

    App/src/libs/actions/IOU.js

    Line 327 in 8180113

    const optimisticCreatedActionForChat = ReportUtils.buildOptimisticCreatedReportAction(payeeEmail);

    , but needs to make another call so that the id is different from the one in IOU report

Also this line https://github.com/Expensify/App/blob/8180113819532fe5503bc3719ba46ffc566e4ead/src/libs/actions/IOU.js#LL987C9-L987C9 should be moved outside of the isNewChat block (this is causing the Skeleton loading above the IOU report action inside the IOU report when creating IOU offline with an existing user), since isNewChat is checking if the Regular chat exists, while IOU report is always a new chat so it should always have the chat created report.

What alternative solutions did you explore? (Optional)

• We need to double check other flows like Request money, Split bill, ... They might have the same issues.

• There's another issue where if we send money from A to B, when online, the OpenReport API will return the new Created chat and Report preview report actions generated from BE side in the main chat between A and B. We'll see the Settled up ... elsewhere report preview, but the sender is B (wrong actorEmail), this is not correct since the sender should be A.

  This should be fixed in the back-end to accept the reportActionIDs that are optimistically generated in front-end for both Created chat and Report preview report.

Result

[dylanexpensify]

Still receiving proposals! Nice!

[sophiepintoraetz]

Hmm, I feel like these proposals could also factor in this issue here?

[hoangzinh]

Proposal

Updated here #19318 (comment)

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[dylanexpensify]

I'm heading OOO tomorrow for about a week so adding another BZ member while I'm away to help keep the train moving! 🚂 Thanks @bfitzexpensify!!

[luacmartins]

This could be due to the fact that the only reportAction that exists in the chat report at that point is the reportPreview action and it's still not being added optimistically. If that's the case, yes https://github.com/Expensify/Expensify/issues/284588 should address that.

[MonilBhavsar]

Yes it is and this is the issue for frontend, so I think we should hold this issue on the frontend issue instead. cc @mountiny

[mountiny]

Yep I agree this should wait for the implementation from Cristi

[dylanexpensify]

Still on hold!

[dylanexpensify]

@cristipaval @mountiny any update here?

[cristipaval]

I'm making the PR ready for review today.

[dylanexpensify]

amazing, @cristipaval any updates?

[cristipaval]

the PR is still in review, almost there

[JmillsExpensify]

We deprecated send money. Are ya'll sure we should still move forward with this PR?

[dylanexpensify]

Ah great shout @JmillsExpensify - @cristipaval mind weighing in? I think we should actually hold on this PR

[cristipaval]

The PR that this issue is held on was deployed. The thing is that this issue can't be reproduced anyway, since the Send Money flow is disabled.

[dylanexpensify]

Interesting - so not reproducible because of it being disabled, but not necessarily meaning it doesn't still exist?

[cristipaval]

This should be fixed by my PR. I would close this issue for now, I'm sure we'll retest the entire Send money feature if we'll enable it again. Thoughts?