[HOLD for payment 2024-08-02] [$250] mweb-LHN-Mark as unread a message after deleting self mention displays green dot in LHN.

[m-natarajan]

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

---

Version Number: 1.4.63-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: n/a
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. Go to https://staging.new.expensify.com/
2. Open self DM report
3. Mention self and send it
4. Send another message
5. Delete the previous DM message
6. Navigate to LHN
7. Long press the report in LHN and select mark as unread

Expected Result:

Mark as unread a message after deleting self mention must not display green dot in LHN.

Actual Result:

Mark as unread a message after deleting self mention displays green dot in LHN.

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

Bug6453516_1713452138014.self_email.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e421f5653f9e5c16
• Upwork Job ID: 1783272895128035328
• Last Price Increase: 2024-04-24
• Automatic offers:
• hungvu193 | Reviewer | 0
• tienifr | Contributor | 0

Issue Owner

Current Issue Owner: @twisterdotcom

[melvin-bot]

Triggered auto assignment to @twisterdotcom (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[m-natarajan]

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

[m-natarajan]

We think this bug might be related to #vip-vsb

[twisterdotcom]

A mobile web bug for an uncommon flow, this is niche.

[melvin-bot]

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

[melvin-bot]

@twisterdotcom Eep! 4 days overdue now. Issues have feelings too...

[twisterdotcom]

Not just mweb:
https://github.com/Expensify/App/assets/9133401/38e7564c-bcc8-4fcd-8a9a-db349a0e8ea2

[melvin-bot]

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

[melvin-bot]

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

[tienifr]

Proposal

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

Mark as unread a message after deleting self mention displays green dot in LHN.

What is the root cause of that problem?

When we delete a mention report action, we're not deleting/reseting the lastMentionedTime of the report, so this condition is still met and the GBR is displayed because it's thinking that the report is "unread with mention"

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

When we delete a mentioned report action:

1. We need to check if the mentioned user in that report action is the current user
2. If yes, check if the created time of the mention report action is equal to the lastMentionedTime of the report
3. If yes, we know now that the mention report action is the latest mention report action, it's deleted so we need to either:
   a. Set the lastMentionedTime optimistically back to null/empty (This is the easiest but there'll be an edge case where the user is mentioned twice, the latest mention is deleted, then the user marks the second latest mention as unread -> it will not show GBR. If we want this edge case to work, we need the approach in b, otherwise a. will work just fine)
   b. Or look for the latest mention report action that mentions this user, and optimistically set the lastMentionedTime to that one instead. If there's no such mention report action, set lastMentionedTime to null/empty

We might need to make similar changes in back-end.

What alternative solutions did you explore? (Optional)

In step 2, we can improve on it by having a lastMentionedReportActionId in the report, and check mention report action equality by that instead of using the date lastMentionedTime.

Out-of-scope observation: We're not optimistically setting the lastMentionedTime when a user mentioned themselves, we might want to do that if we detect they're sending a mention report action that mentions themselves.

[hungvu193]

I'll review this one tomorrow.

[hungvu193]

@tienifr 's main solution here looks good to me! 🎀 👀 🎀 C+ reviewed.

[melvin-bot]

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

[twisterdotcom]

Bump @neil-marcellini

[melvin-bot]

@twisterdotcom @hungvu193 @neil-marcellini this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[hungvu193]

Bump @neil-marcellini

[neil-marcellini]

Yesterday I wrote a test for a slightly more complicated case and updated the query to set the last mentioned time for all users in the mention. Finding the last read time and doing the update now happens in one query. I'm also changing it to only send the Onyx update when a user marks the report as unread, because that's the only flow where it's used in App right now and OpenReport will update lastMentionedTime as well.

Before I write more tests I should get the query timed to make sure it's going to be fast enough. I'm concerned that it will not be since I think it involves scanning over all report actions. If it's too slow we might need a totally different approach towards mention data storage.

[twisterdotcom]

How is this going @neil-marcellini?

[neil-marcellini]

I've been concerned that the query to fix this on the backend won't scale, so I brought that up in #engineering-chat today. After some discussion we decided that the only way to make this efficient is to change the storage format for mentions, and agreed that it's not worth doing so to fix this edge case. So we're going to do nothing on the backend.

However, I think we can still go ahead with the frontend fix because it looks like it solves this exact case and only has a bug in an even more extreme edge case described here.

[neil-marcellini]

@tienifr @hungvu193 would you please get the conflicts resolved, retest the PR, and get it ready to merge?

[hungvu193]

@neil-marcellini Conflicts resolved, can you take a look at the PR please?

[neil-marcellini]

Taking a look!

[twisterdotcom]

What are we waiting on here?

[hungvu193]

We found a bug during our review so we're trying to fix it

[neil-marcellini]

Turns out it's a backend bug though, and we already said above we're not going to fix that. Hopefully we can get this merged soon.

[neil-marcellini]

The PR was merged today, finally!

[melvin-bot]

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

• fix message after deleting self mention displays green dot in LHN #41888

If no regressions arise, payment will be issued on 2024-08-02. 🎊

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

• @hungvu193 requires payment through NewDot Manual Requests (Reviewer)
• @tienifr requires payment through NewDot Manual Requests (Contributor)

[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:

• [@hungvu193] The PR that introduced the bug has been identified. Link to the PR:
• [@hungvu193] 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:
• [@hungvu193] 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:
• [@hungvu193] Determine if we should create a regression test for this bug.
• [@hungvu193] 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.
• [@twisterdotcom] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[hungvu193]

• The PR that introduced the bug has been identified. Link to the PR: I can't really find the offending PR as this is an extremely edge case. See this comment.
• 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: N/A
• 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
• Determine if we should create a regression test for this bug: Yes

Regression Test:

1. Open App.
2. Open self DM report.
3. Mention yourself and send it.
4. Send another message.
5. Delete the previous DM message.
6. Navigate to LHN.
7. Long press the report in LHN and select mark as unread.
8. Verify mark as unread a message after deleting self mention must not display green dot in LHN.

Do we 👍 or 👎 ?

[twisterdotcom]

Both eligible for Manual Requests. Payment Summary:

• @hungvu193 paid $250 via NewDot Manual Request (Reviewer)
• @tienifr paid $250 via NewDot Manual Request (Contributor)

[JmillsExpensify]

$250 approved for @hungvu193

[JmillsExpensify]

$250 approved for @tienifr