[HOLD for payment 2024-03-25] [$500] Chat - Unable to re-join thread after leaving thread and returning with back button

[lanitochka17]

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.4.46.0
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): [email protected]
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 any 1:1 chat conversation
2. Join a thread by replying to any sent message
3. Click on 3 dots in threaded message and leave conversation(User should return to original DM conversation)
4. Select back button(User should return to threaded message just left)
5. Select "Join" in header

Expected Result:

User should be able to join conversation and "join: in header disappears

Actual Result:

Tapping "join" button does nothing but flashes button

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6398425_1709308701289.RPReplay_Final1709302510.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014d212626b2be400f
• Upwork Job ID: 1763728742263644160
• Last Price Increase: 2024-03-03

[melvin-bot]

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

[lanitochka17]

We think that this bug might be related to #vip-vsp
CC @quinthar

[lanitochka17]

@mallenexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014d212626b2be400f

[melvin-bot]

Triggered auto assignment to Contributor Plus for review of internal employee PR - @jjcoffee (Internal)

[melvin-bot]

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

[mallenexpensify]

@jjcoffee think this can be External? I feel like it can be.
I was able to reproduce.

[dukenv0307]

Proposal

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

Tapping "join" button does nothing but flashes button

What is the root cause of that problem?

When the user leaves a thread, the user will lose access to the thread until they open the thread again. The problem here is that when the user goes back to the thread via back button, the route of that thread does not change so OpenReport is not called again. So when the user tries to join the thread, they won't be able to since at the time they still doesn't have access to the thread (not open it again yet).

This same problem doens't happen if we click on the x Replies in the main chat to open the thread, because in that case the OpenReport will always be called inside navigateToAndOpenChildReport here

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

Make sure the openReport will be called again if we go back to a HIDDEN thread (means the user has left the thread and go back). This is the same behavior as we already do when clicking on x Replies in the main chat to open the thread. This can be done in many ways, one of it is to relies on isFocused changing to true to call openThread here

useEffect(() => {
        if (isFocused && ReportUtils.isChatThread(report) && report.notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN) {
            Report.openReport(report.reportID);
        }
    }, [isFocused]);

(We can make the check stricter by using prevIsFocused and run openReport only if isFocused changes from false to true, if we do that we can also add the report to the dependency list of useEffect)

What alternative solutions did you explore? (Optional)

Instead of modifying the useEffect above we can put similar checks in here and here. But the main solution should be the same.

Currently when going back to the thread we'll see the thread content immediately, that's because we're keeping the thread content after we leave it, we can optionally change to using loading skeleton (until the report is opened again) by not keeping the thread content and also make sure it doesn't show not found page when going back. This will require similar condition changes.

[badeggg]

Proposal

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

Not able to join the thread when coming back from a chat report page.

What is the root cause of that problem?

The root cause is that we navigate to a new chat page when leave the thread rather than go back to the previous report page's route, this behavior is coming from #37355.
And I don't think the related issue's #35810 expected behavior is reasonable. I will detail it in a few minutes.

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

We should revert the code change in src/libs/actions/Report.ts:

What alternative solutions did you explore? (Optional)

If the team decide issue's #35810 expected behavior is reasonable, then maybe duke's solution can do the trick.

[badeggg]

Why I think issue's #35810 expected behavior is not reasonable. To brief #35810's expected behavior, navigate to a new chat report page when leave a thread, navigate back to the thread when click the back button. I believe the intuition is navigate back to the previous chat report page when leave a thread rather than go to a new chat report page. Plus it will cause a route stack 'loop':

Screen.Recording.2024-03-04.at.14.22.55.mov

[jjcoffee]

Hmm yeah I agree with @badeggg that the expected behaviour in #35810 seems a little off. I'd expect that explicitly choosing to leave the thread indicates the user doesn't want to go back there, and the following back button interaction should take you back to LHN (on mobile). It seems like they settled on this behaviour after a long discussion on Slack though, so I think we can consider it "expected" 😄

@dukenv0307's proposal almost makes sense to me, I'm just missing a bit more clarity on the RCA. Why does the Join button not work until the report is reopened via OpenReport? It seems like it probably should work and that that's the thing to fix. The proposed solution seems a bit like a workaround, but I'm not 100%!

[dukenv0307]

Why does the Join button not work until the report is reopened via OpenReport? It seems like it probably should work and that that's the thing to fix.

@jjcoffee The Join button is a thread-specific operation, the user can only do it if they're a participant in the thread. Once they left the thread, they are no longer a participant in the thread (and only re-become one if they OpenReport).

The proposed solution seems a bit like a workaround, but I'm not 100%!

@jjcoffee Even if we make the Join button work before the user becomes a participant in the thread, we still have to OpenReport if the user comes back to the thread, otherwise the user will not see real time messages on the thread (because the user is not a participant and our back-end won't bother sending live updates to non-participants). So I think doing OpenReport here is enough to solve both issues, and it's also consistent with what we're doing when clicking x Replies

[jjcoffee]

@dukenv0307 Thanks for the extra details! One last missing piece - what exactly does opening the thread with OpenReport do in terms of making them a participant? Does the server then start sending notifications (silently)?

Under your proposal it sounds like there would be no way to fully leave a thread since if you use the back button to navigate away from the parent chat, you always go to the thread you left first which (via OpenReport) would re-add you as a participant regardless of your intentions. So the server suddenly ends up sending (even silently) notifications for all threads where a user has left, for all eternity!

This is an "interesting" side-effect of implementing #35810, so I'm not 100% on whether or not to consider this a regression that the PR author there should fix.

[dukenv0307]

what exactly does opening the thread with OpenReport do in terms of making them a participant?

@jjcoffee It's the same as when we open thread by clicking "x Replies", the user will then see the thread content and real time messages on the thread

Does the server then start sending notifications (silently)?

If you mean notifications as in web/app native notifications then no, because the NOTIFICATION_PREFERENCE is still hidden. Just that they'll see real time messages if they are opening the report, just like any other reports. They will only receive real-time notifications if they "Join" the thread.

Under your proposal it sounds like there would be no way to fully leave a thread since if you use the back button to navigate away from the parent chat, you always go to the thread you left first which (via OpenReport) would re-add you as a participant regardless of your intentions.

"re-add you as a participant" doesn't have any effect really, except that the user will be able to see real-time messages if they are opening the thread. The behavior here is just the same as if we open the thread when clicking "x Replies", going back to get there or clicking "x Replies" to get there shouldn't be any different. Here I'm just making sure the behavior is consistent with opening the thread via clicking the "x Replies", not adding any new behavior.

I also don't think "you use the back button to navigate away from the parent chat" would be a very common user action after leaving the thread, if they do anything other than that then they will not be re-added as a participant of the thread. So "no way to fully leave a thread" doesn't sound right to me here 😄 , the user will only reopen the thread if they decide so by navigating to the thread.

This is an "interesting" side-effect of implementing #35810, so I'm not 100% on whether or not to consider this a regression that the PR author there should fix.

I'm not sure regression is appropriate since this "go back to thread" feature doesn't exist on prod so there's nothing to regress. It sounds more like an improvement to a new feature.

[badeggg]

Question, if the final decision is to consider this as a regression of #35810, will I get the bounty of this issue? @jjcoffee

[jjcoffee]

I also don't think "you use the back button to navigate away from the parent chat" would be a very common user action after leaving the thread, if they do anything other than that then they will not be re-added as a participant of the thread.

@dukenv0307 I don't think there's any other way to get back to LHN on mobile other than using the back button (which would always take you to the thread you just left)?

Question, if the final decision is to consider this as a regression of #35810, will I get the bounty of this issue?

@badeggg We usually don't reward bounties for pointing out regressions. Bounties only really apply where a proposal was selected to fix an issue and a PR raised + merged.

[badeggg]

ok

[dukenv0307]

@dukenv0307 I don't think there's any other way to get back to LHN on mobile other than using the back button (which would always take you to the thread you just left)?

@jjcoffee Yeah I see, I was talking about Desktop.

But I don't think it's an issue because there's no notifications that's being sent by the back-end if the user didn't "join" the thread.

[jjcoffee]

Thanks for the back and forth @dukenv0307! I'm clarifying this on Slack.

[mallenexpensify]

Question, if the final decision is to consider this as a regression of #35810, will I get the bounty of this issue? @jjcoffee

@badeggg , good question! Let's keep moving this issue forward to get it fixed then discuss after the PR's been merged for 7 days.

[melvin-bot]

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

[melvin-bot]

❌ There was an error making the offer to @jjcoffee for the Reviewer role. The BZ member will need to manually hire the contributor.

[melvin-bot]

❌ There was an error making the offer to @dukenv0307 for the Contributor role. The BZ member will need to manually hire the contributor.

[mallenexpensify]

Added to #vip-vsb, will hire in Upwork soon if automation doesn't magically start working. @MariaHCD , can you review the recommended proposal above? Thx

[mallenexpensify]

@dukenv0307 @jjcoffee , can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01932d25804c11bb6a

[dukenv0307]

I'm working on this issue and will raise PR soon

[jjcoffee]

@mallenexpensify Accepted! 🙏

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.53-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: re-join thread after leaving thread and returning with back button #38027

If no regressions arise, payment will be issued on 2024-03-25. 🎊

For reference, here are some details about the assignees on this issue:

• @jjcoffee requires payment (Needs manual offer from BZ)
• @dukenv0307 requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@jjcoffee / @dukenv0307] The PR that introduced the bug has been identified. Link to the PR:
• [@jjcoffee / @dukenv0307] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@jjcoffee / @dukenv0307] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@jjcoffee / @dukenv0307] Determine if we should create a regression test for this bug.
• [@jjcoffee / @dukenv0307] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: Fix-unable to navigate back after leaving chat threads #37355
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: Fix-unable to navigate back after leaving chat threads #37355 (comment)
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A - just needed extra testing
• Determine if we should create a regression test for this bug. Yes
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. Navigate to any 1:1 chat conversation
2. Start a thread by replying to any message
3. Click on 3 dots in threaded message and leave conversation
4. Tap back button to navigate back to the thread
5. Tap "Join" in the header
6. Verify that the button disappears to indicate you have rejoined the thread successfully

Do we agree 👍 or 👎

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[mallenexpensify]

Contributor: @dukenv0307 paid $500 via Upwork
Contributor+: @jjcoffee paid $500 via Upwork

TR GH https://github.com/Expensify/Expensify/issues/382664

Thanks!