Skip to content

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

Closed

Description

@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

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jul 27, 2023
  2. melvin-bot commented on Jul 27, 2023

    @melvin-bot

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

  3. melvin-bot commented on Jul 27, 2023

    @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
  4. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Jul 27, 2023
  5. changed the title [-]Chat is not marked as read when receiving messages while in the report[/-] [+][$1000] Chat is not marked as read when receiving messages while in the report[/+] on Jul 27, 2023
  6. melvin-bot commented on Jul 27, 2023

    @melvin-bot
  7. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Jul 27, 2023
  8. melvin-bot commented on Jul 27, 2023

    @melvin-bot

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

  9. melvin-bot commented on Jul 27, 2023

    @melvin-bot

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

  10. mountiny commented on Jul 27, 2023

    @mountiny
    ContributorAuthor

    Follow up to #23729

  11. Talha345 commented on Jul 27, 2023

    @Talha345
    Contributor

    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

  12. hannojg commented on Jul 28, 2023

    @hannojg
    Contributor

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

    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.

  13. 42 remaining items

  14. melvin-bot commented on Aug 21, 2023

    @melvin-bot

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

  15. situchan commented on Aug 21, 2023

    @situchan
    Contributor

    @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
  16. Talha345 commented on Aug 21, 2023

    @Talha345
    Contributor

    @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.

  17. Talha345 commented on Aug 21, 2023

    @Talha345
    Contributor

    @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.

  18. melvin-bot commented on Aug 24, 2023

    @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!

  19. added
    InternalRequires API changes or must be handled by Expensify staff
    and removed
    ExternalAdded to denote the issue can be worked on by a contributor
    Help WantedApply this label when an issue is open to proposals by contributors
    on Aug 24, 2023
  20. melvin-bot commented on Aug 24, 2023

    @melvin-bot

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

  21. melvin-bot commented on Aug 25, 2023

    @melvin-bot

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

  22. sonialiap commented on Aug 28, 2023

    @sonialiap
    Contributor

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

    Please reopen if the issue shows up again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.DailyKSv2InternalRequires API changes or must be handled by Expensify staff

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions