[$1000] Chat is not marked as read when receiving messages while in the report

[mountiny]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

Break down in numbered steps

On mobile

1. User A: Open chat with user B
2. UserB: Send message into chat with user A
3. User A: Go back from chat to chat list.

Expected Result:

Describe what you think should've happened

The chat should be marked as read

Actual Result:

Describe what actually happened

The chat will still appear as unread

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: v1.3.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):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by: @hannojg
Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01fb57ede1a93ebb92
• Upwork Job ID: 1684558218046046208
• Last Price Increase: 2023-08-17

[melvin-bot]

Triggered auto assignment to @sonialiap (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/~01fb57ede1a93ebb92

[melvin-bot]

Current assignee @sonialiap 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)

[mountiny]

Follow up to #23729

[Talha345]

Proposal

Note: I was able to reproduce this issue on Web too, so it was not only on mobile.See below video.

Screen.Recording.2023-07-27.at.23.43.52.mov

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

When a user has a report opened and a new message is received, the message is not marked as read.

What is the root cause of that problem?

The RC of this issue is the following function showReportActionNotification logic as it is the entrypoint of any newly received message in a report. Whenever a new message is received, this function is triggered and it calculates if a notification should be shown to the user for that message or not using the shouldShowReportActionNotification method which acts as a guard clause.

Further down we can see the call to the method notifyNewAction , which executes the callback here which is responsible for performing certain actions including marking the message as read here.

If the guard clause mentioned above prevents the notification to be sent to the user, the function returns and notifyNewAction method is never called which in turn never calls the callback responsible for marking the message as read and is also the single source of truth for the "new action" event.

It can be seen here in shouldShowReportActionNotification that it returns false if a user is currently viewing the report which activates the guard clause and the message is never marked as read.

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

The call to the notifyNewAction method should be placed before the guard clause such that it is always executed irrespective of that a notification should be sent to the user or not.The updated code should look like this:

function showReportActionNotification(reportID, action) {
    notifyNewAction(reportID, action.actorAccountID, action.reportActionID);
    
    if (!shouldShowReportActionNotification(reportID, action)) {
        return;
    }

    Log.info('[LocalNotification] Creating notification');
    LocalNotification.showCommentNotification({
        report: allReports[reportID],
        reportAction: action,
        onClick: () => {
            // Navigate to this report onClick
            Navigation.navigate(ROUTES.getReportRoute(reportID));
        },
    });
}

What alternative solutions did you explore? (Optional)

N/A

[hannojg]

@Talha345 Can you please check if you are still able to reproduce on web, when having the changes from these PR in your branch:

• [CP Staging] Fix/regression unread messages #23729

I believe you haven't. I think when making the proposal its important to be up to date with the latest state of the branch.

[Talha345]

@Talha345 Can you please check if you are still able to reproduce on web, when having the changes from these PR in your branch:

• [CP Staging] Fix/regression unread messages #23729

I believe you haven't. I think when making the proposal its important to be up to date with the latest state of the branch.

@hannojg my local branch was updated with main and I had the changes from this PR of yours while testing.

The reproduction video for web I shared above in my proposal, already had these changes.

[Talha345]

@hannojg I am also able to reproduce this on STG web.See attached video below.

The RC of this bug has nothing to do with your changes and I have explained that in detail in my proposal here.Also took me a while to get to the RC 😅

Screen.Recording.2023-07-28.at.12.05.08.mov

[melvin-bot]

@sonialiap, @fedirjh Eep! 4 days overdue now. Issues have feelings too...

[fedirjh]

Reviewing shortly.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[hannojg]

Hey, I think this needs to be retested on the latest staging / production version!

[Talha345]

@hannojg Reproduced on latest staging just now.

unread-04-08.1.1.mov

[fedirjh]

I am unable to reproduce it in the latest staging build 🏷️ v1.3.50-2

CleanShot.2023-08-06.at.17.17.24-converted.mp4

[Talha345]

@fedirjh Pretty strange because I am able to reproduce on the same version of STG.I also tried it with a new account but couldn't reproduce on it so I am guessing it is a specific edge case.

Screen.Recording.2023-08-06.at.18.36.43.mov

mWeb:

WhatsApp.Video.2023-08-06.at.6.42.51.PM.mp4

[melvin-bot]

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

[melvin-bot]

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

[sonialiap]

Reassigned C+ per the request #23733 (comment)

[hannojg]

Hm okay, will double check! You are using the original reproduction steps or any other ones?

[Talha345]

Hm okay, will double check! You are using the original reproduction steps or any other ones?

Yes, I am using the reproduction steps mentioned in this issue and have also shared multiple videos in the comments above.You can also see my proposal as to what I think is causing this problem.

I just noticed the links to code in my proposal have deviated because the proposal is 3 weeks old and a lot of changes have been made in the codebase but I hope you will be able to understand it

[melvin-bot]

@sonialiap, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@sonialiap, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[situchan]

@Talha345 can you test again on latest main? I am also not able to reproduce.
Please explain in root cause why this is sometimes reproducible but sometimes not?

unread.mov

[Talha345]

@Talha345 can you test again on latest main? I am also not able to reproduce.
Please explain in root cause why this is sometimes reproducible but sometimes not?

@situchan Okay,I will update in a few hours.

[Talha345]

@situchan I am no longer able to reproduce this on either platform in dev on latest main but I am still able to reproduce it on STG.Lets wait a day or two and we can test again on STG to see if the issue is no longer present.

[melvin-bot]

@sonialiap @situchan this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot]

Current assignee @situchan is eligible for the Internal assigner, not assigning anyone new.

[melvin-bot]

@sonialiap, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

[sonialiap]

I cannot reproduce on staging or production either. Seems like the issue is resolved

Please reopen if the issue shows up again