[HOLD for payment 2024-06-28] [$250] IOU-User is shown option to submit expense using group.

[izarutskaya]

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.74-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Launch app
2. Tap start chat
3. Add a user and create group
4. Tap header -- member -- invite
5. Invite user A
6. Login in mweb as user A
7. As user A, note group chat in lhn
8. As group admin, in Android app remove another user (not user A)
9. In mweb, as member note green dot shown in LHN for removed user
10. In mweb, tap on the chat and navigate to members page
11. In Android app, as group admin tap back and "Leave group"
12. In mweb, note in members page, now member is shown as "admin"
13. Navigate to LHN
14. Tap fab -- submit expense
15. Enter an amount and tap next
16. Select group shown in recent
17. Tap submit

Expected Result:

User must not be shown option to submit expense using group.

In group user's LHN, when user removed green dot is shown.

Actual Result:

User is shown option to submit expense using group.

In group user's LHN, when user removed green dot must not be shown.

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

Bug6482069_1715809572377.rename.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01099fb6a88b2692fc
• Upwork Job ID: 1791216875339001856
• Last Price Increase: 2024-05-16
• Automatic offers:
• ishpaul777 | Reviewer | 102546813
• nkdengineer | Contributor | 102546815

Issue Owner

Current Issue Owner: @sakluger

[melvin-bot]

Triggered auto assignment to @sakluger (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.

[izarutskaya]

We think this issue might be related to the #vip-vsb.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01099fb6a88b2692fc

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @joekaufmanexpensify (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.

[sakluger]

@joekaufmanexpensify I'm assigning to you while I'm OOO for the next two weeks. I'll take it back over if it's still open after May 31.

[nkdengineer]

Proposal

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

1. In group user's LHN, when user removed green dot must not be shown.
2. User is shown option to submit expense using group.

What is the root cause of that problem?

1. This is a back-end issue, in a group of user [A, B, C], A is the admin, C is the logged in user.

When user A removes user B, it will show the "removed B" text when user C views it. The problem here is the lastMentionedTime of the group chat is pushed by Pusher in user C's group chat view when "removed B" report action comes. This is wrong because lastMentionedTime is supposed to be the time when "current user is mentioned", not "any user is mentioned".

This causes the green dot to show because we have this condition when checking requiresAttentionFromCurrentUser.

When reloading and openReport runs, the lastMentionedTime will not be there and the green dot disappear.

2. When we generate the option from report here, we're setting the login incorrectly.

In the case of group chat with 1 member, hasMultipleParticipants is false but it's still a group chat so should not have login or accountID in the result like 1-1 chat.

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

1. We need to fix back-end so that if user B is removed, back-end will not push (via Pusher) lastMentionedTime to user C's report because user C is not mentioned.

2. Update this to

if (!hasMultipleParticipants && !ReportUtils.isGroupChat(report)) {

We need to also check other report types (like room) to make sure the same case doesn't happen, and similar fix can be applied if there's problem.

What alternative solutions did you explore? (Optional)

NA

[joekaufmanexpensify]

I can reproduce both parts of the bug. Fixing both makes sense to me!

[joekaufmanexpensify]

Waiting for @ishpaul777 review

[ishpaul777]

on my radar, expect to review in 4-5 hours

[ishpaul777]

I can reproduce both issues @nkdengineer Can you please retest again.

Screen.Recording.2024-05-21.at.2.07.15.AM.mov

Your proposal for the second issue makes sense to me 👍

[nkdengineer]

Thanks for the video @ishpaul777, I can reproduce now.

That 1st issue's a back-end issue. Proposal updated to mention how to fix it.

[joekaufmanexpensify]

@ishpaul777 So seems like we need to both hire someone for the FE piece, and then find a volunteer for the BE?

[ishpaul777]

I'll confirm this in few minutes @joekaufmanexpensify

[ishpaul777]

Yes the first issue seems to be backend related. For the second issue Proposal from @nkdengineer LGTM!

🎀 👀 🎀 C+ reviewed

[joekaufmanexpensify]

@sakluger I see you're back from OOO, so gonna let you take this one back over. Here's a summary of where we're at:

• We determined that this bug has both a frontend and backend portion.
• We selected a proposal for the frontend portion, and a PR is now in review.
• We're determining how to proceed with the backend portion.

LMK if there's anything else I can help with!

[chiragsalian]

is the bug not fixed until both of these issues are handled?

Correct, both of these mentioned here.

since otherwise the bug won't necessarily be fixed when we close the external portion.

Correct, the bug won't be fixed when we finish just the external portion. But this issue is the externally marked issue so i don't quite follow the statement here.

If so, I kinda feel like it makes sense to handle them both in this issue

I think i disagree because this issue is marked external so in my K2 it shows up as greyed out. So during busy times its very easy to overlook this issue. Since its gray in my K2 i feel like there is nothing immediately actionable for me and so i miss it easily. But now with the fire's hopefully dying out i don't mind either.

unless you wanted to tackle it

I don't think i can get to it until late of next week or the week after. If its needed sooner we might have to look for someone else.

[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.84-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix User is shown option to submit expense using group #42977

If no regressions arise, payment will be issued on 2024-06-24. 🎊

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

• @ishpaul777 requires payment automatic offer (Reviewer)
• @nkdengineer requires payment automatic offer (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:

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

[melvin-bot]

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

• Fix User is shown option to submit expense using group #42977

If no regressions arise, payment will be issued on 2024-06-28. 🎊

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

• @ishpaul777 requires payment automatic offer (Reviewer)
• @nkdengineer requires payment automatic offer (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:

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

[chiragsalian]

Not overdue, hold for payment.

[sakluger]

The actual payment date is 6/28.

[sakluger]

Summarizing payment on this issue:

Contributor: @nkdengineer $250, paid via Upwork
Contributor+: @ishpaul777 $250, paid via Upwork

@ishpaul777 could you please complete the BZ checklist today? Thanks!

[ishpaul777]

[@ishpaul777] The PR that introduced the bug has been identified. Link to the PR: I was only able to track this to a very old PR -#10784
[@ishpaul777] 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: I am not sure if the bug from that PR since group chat is a feature released after that so not commenting.
[@ishpaul777] 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: Not required
[@ishpaul777] Determine if we should create a regression test for this bug.- No , this was a very rare edge case.
[@ishpaul777] 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.- No

[sakluger]

Thank you!