App icon indicating copy to clipboard operation
App copied to clipboard

[HOLD #31097 ] Chat - User taken to concierge chat page when start chat with same user from 2nd time onwards

Open lanitochka17 opened this issue 1 year ago β€’ 21 comments

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


Version Number: 1.3.98-1 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 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:

Action Performed:

  1. Navigate to staging.new.expensify.com
  2. Tap '+' icon >> Start chat option
  3. Enter email address & tap on the result displayed (Note: User taken to correct chat history page)
  4. Tap back button to go back to LHN
  5. Tap '+' icon >> Start chat option
  6. Enter same email address used in step 3 & tap on the result displayed

Expected Result:

User should be taken to correct user's conversation history page

Actual Result:

User taken to concierge chat page when start chat with same user from 2nd time onwards Note : Issue happens when start chat with a new non-existing account & users with previous chat history (2 different videos attached)

Workaround:

Unknown

Platforms:

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

  • [ ] Android: Native
  • [x] Android: mWeb Chrome
  • [x] iOS: Native
  • [x] iOS: mWeb Safari
  • [ ] MacOS: Chrome / Safari
  • [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/f872f94d-597b-48c1-93dc-f43829bc9a07

https://github.com/Expensify/App/assets/78819774/6324b981-73e3-48d1-854b-de9f726e39db

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01ec45a3e6b8c6cadc
  • Upwork Job ID: 1723101218860847104
  • Last Price Increase: 2023-11-10
  • Automatic offers:
    • s77rt | Contributor | 28105216

lanitochka17 avatar Nov 10 '23 22:11 lanitochka17

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

melvin-bot[bot] avatar Nov 10 '23 22:11 melvin-bot[bot]

Job added to Upwork: https://www.upwork.com/jobs/~01ec45a3e6b8c6cadc

melvin-bot[bot] avatar Nov 10 '23 22:11 melvin-bot[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[bot] avatar Nov 10 '23 22:11 melvin-bot[bot]

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

melvin-bot[bot] avatar Nov 10 '23 22:11 melvin-bot[bot]

Was able to reproduce, bizarre

mallenexpensify avatar Nov 10 '23 23:11 mallenexpensify

While solving this bug, I checked for duplicates and found https://github.com/Expensify/App/issues/30062. I submitted a proposal on the other issue because I tested my prospal on it and it worked. Can please someone check if it is a duplicate?

Tony-MK avatar Nov 11 '23 05:11 Tony-MK

Proposal

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

Secondary login chat after redirecting to primary login -redirects user to Concierge chat

What is the root cause of that problem?

The ReportScreen has to re-render a minimum of two times because of firstRenderRef. Read the two snippets below from the NavigationRoot.js and ReportScreen.js files to further illustrate the effects of the width of the screen.

https://github.com/Expensify/App/blob/fcee2073f096ad60dd9f30c536342de7d3893282/src/libs/Navigation/NavigationRoot.js#L62-L74

https://github.com/Expensify/App/blob/fcee2073f096ad60dd9f30c536342de7d3893282/src/pages/home/ReportScreen.js#L301-L305

Devices with large screen widths re-render one additional time, which allows report.reportID to become present, thus being able to produce the expected result. Here is a snippet to explain the possibilities of report.reportID being unavailable.

https://github.com/Expensify/App/blob/fcee2073f096ad60dd9f30c536342de7d3893282/src/pages/home/ReportScreen.js#L238-L241

Finally, the root cause of the problem is that, in this specific case, devices with small screen widths only re-render twice because report.reportID is not present. This will make the logic below navigate the Concierge chat because, in this situation, prevReport.parentReportID doesn't have a value.

https://github.com/Expensify/App/blob/fcee2073f096ad60dd9f30c536342de7d3893282/src/pages/home/ReportScreen.js#L307-L331

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

To produce the expected result accurately, we should try to re-render the page for small devices so they mimic the control flow of the larger devices. Therefore, adding an additional condition in this useEffect hook to force the function to run one more time for small devices should solve the problem. This will ensure that report.reportID is present to allow the fragile logic in the hook to perform as intended without changing it.

Tony-MK avatar Nov 11 '23 05:11 Tony-MK

Proposal

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

User taken to concierge chat page when start chat with same user from 2nd time onwards

What is the root cause of that problem?

The first time the user creates a new chat with another user, it will succeed normally.

The second time, it will return failure with a preexistingReportID, since the report was already created between the 2 users and preexistingReportID will be for navigating to the correct report.

We have that navigation logic here. The problem is that, in here, we remove the duplicate optimistic report before the redirection completes. At the time, the report screen with the duplicate optimistic report is still mounted and this logic kicks in, causing the navigation to Concierge, because it basically looks the same as the report was removed from another device.

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

We need to make sure we finish navigating to the correct report first before removing the duplicate optimistic report. We need to move this line to after this line, and run it inside InteractionManager.runAfterInteractions so that it will delete the report from Onyx after the navigation completes.

InteractionManager.runAfterInteractions(() => {
    Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, null);
});

What alternative solutions did you explore? (Optional)

Use a more sophisticated way to listen to the navigation completes, for example something similar to the useWaitForNavigation hook, or a timeout. But I find InteractionManager.runAfterInteractions to be the simplest and it works well.

tienifr avatar Nov 11 '23 08:11 tienifr

@robertKozik I think we should hold this for https://github.com/Expensify/App/pull/31097 since we're likely to change some parts that relate to this issue.

getusha avatar Nov 13 '23 16:11 getusha

Thanks for heads up @getusha. @mallenexpensify can we put this issue on hold until #31097 would be merged, per @getusha comment

robertKozik avatar Nov 13 '23 16:11 robertKozik

Thanks @getusha , put on hold pending the below, removed Help Wanted label too

  • https://github.com/Expensify/App/pull/31097

mallenexpensify avatar Nov 13 '23 19:11 mallenexpensify

@mallenexpensify shouldn't we keep this open while it's on hold?

tienifr avatar Nov 14 '23 11:11 tienifr

Yup! Thanks @tienifr , not sure how that happened (it's wayyy more likely I forget to close an issue vs prematurely doing so)

mallenexpensify avatar Nov 14 '23 22:11 mallenexpensify

@mallenexpensify, @robertKozik Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] avatar Nov 20 '23 13:11 melvin-bot[bot]

The issue this is held on is held on another issue, so it might be a bit, bumping to Weekly https://github.com/Expensify/App/issues/30868

mallenexpensify avatar Nov 21 '23 23:11 mallenexpensify

Still double held!

mallenexpensify avatar Dec 05 '23 23:12 mallenexpensify

Making a monthly cuz it's double held, this is the one that needs fixing first

  • https://github.com/Expensify/App/pull/31424 then
  • https://github.com/Expensify/App/pull/31097

mallenexpensify avatar Dec 20 '23 01:12 mallenexpensify

Will take this as C+ @mallenexpensify Please assign me here

s77rt avatar Jan 16 '24 23:01 s77rt

πŸ“£ @s77rt πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

melvin-bot[bot] avatar Jan 17 '24 02:01 melvin-bot[bot]

Done @s77rt

mallenexpensify avatar Jan 17 '24 02:01 mallenexpensify

The linked PRs are merged. The issue is no longer reproducible. We can close this cc @mallenexpensify

s77rt avatar Feb 05 '24 23:02 s77rt

Thanks @s77rt !

mallenexpensify avatar Feb 06 '24 23:02 mallenexpensify