App
App copied to clipboard
[HOLD #31097 ] Chat - User taken to concierge chat page when start chat with same user from 2nd time onwards
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:
- Navigate to staging.new.expensify.com
- Tap '+' icon >> Start chat option
- Enter email address & tap on the result displayed (Note: User taken to correct chat history page)
- Tap back button to go back to LHN
- Tap '+' icon >> Start chat option
- 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
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
Triggered auto assignment to @mallenexpensify (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Job added to Upwork: https://www.upwork.com/jobs/~01ec45a3e6b8c6cadc
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
Triggered auto assignment to Contributor-plus team member for initial proposal review - @robertKozik (External
)
Was able to reproduce, bizarre
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?
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.
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.
@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.
Thanks for heads up @getusha. @mallenexpensify can we put this issue on hold until #31097 would be merged, per @getusha comment
Thanks @getusha , put on hold pending the below, removed Help Wanted
label too
- https://github.com/Expensify/App/pull/31097
@mallenexpensify shouldn't we keep this open while it's on hold?
Yup! Thanks @tienifr , not sure how that happened (it's wayyy more likely I forget to close an issue vs prematurely doing so)
@mallenexpensify, @robertKozik Huh... This is 4 days overdue. Who can take care of this?
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
Still double held!
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
Will take this as C+ @mallenexpensify Please assign me here
π£ @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 π
Done @s77rt
The linked PRs are merged. The issue is no longer reproducible. We can close this cc @mallenexpensify
Thanks @s77rt !