[HOLD #46835] [CRITICAL] Notification - User received desktop notification for a room a visited but didn't join

[isagoico]

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: Version 1.4.26-1
Reproducible in staging?: Unknown
Reproducible in production?: Unknown
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
Expensify/Expensify Issue URL:
Issue reported by: @mallenexpensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1705535848446499

Updated details from this thread on July 23rd, inc. rVNP data and more logs.

Action Performed:

Precondition: user is part of a workspace that has at least a room that is visible in the workspace
Employee

1. Navigate to a workspace room but do not join the room
2. Navigate away

Another user or workspace admin

1. As the workspace admin or another user that is part of the workspace room - Send a message

@mallenexpensify reproduction steps:

1. Days ago I visited #big-rig room in Expensify Chat workspace, I did not join the room nor was invited nor commented.

Expected Result:

To not receive desktop/browser notifications when new posts are added to the room

Actual Result:

User received a notification from the room that they did not join.

Workaround:

N/A

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0125cf67df8d5a6dca
• Upwork Job ID: 1749887918200393728
• Last Price Increase: 2024-02-13

Issue Owner

Current Issue Owner: @ishpaul777

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0125cf67df8d5a6dca

[melvin-bot]

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

[melvin-bot]

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

[abekkala]

@mallenexpensify Reproduced!

[mallenexpensify]

@abdulrahuman5196 any reason you can think that this shouldn't be External?

[melvin-bot]

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

[abdulrahuman5196]

Sorry the delay. Will check and update before my morning.

[melvin-bot]

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

[mallenexpensify]

Just happened again. Got a notification which showed as from Sheena (didn't get screenshot), clicked the notification and it opened #expensipups.

@abdulrahuman5196 can you see if you can find a reproduce and provide the steps, if so? Thx

[abekkala]

@abdulrahuman5196 can you take a look here please and address?

[abdulrahuman5196]

I didn't see the issue. Will double confirm again and update here

[melvin-bot]

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

[melvin-bot]

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

[abekkala]

@abdulrahuman5196 do you have an update?

[melvin-bot]

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

[mallenexpensify]

Think we're all set here, so closing.
If we need a regression test, this will get commented on (per here) and I'll reopen.

[puneetlath]

Actually @mallenexpensify I think it'd be good to create a regression test specifically for that case here.

[mallenexpensify]

@abdulrahuman5196 @ishpaul777 as you're the C+ on this PR, complete the BZ checklist since we want a test for this. Thx for chiming in Puneet.

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:

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

[mallenexpensify]

@ishpaul777 , 👀 above please. Also, I'm assuming payment isn't due on this issue and that you'll manage that in the main one. Thx

[ishpaul777]

Also, I'm assuming payment isn't due on this issue and that you'll manage that inhttps://github.com//issues/46835

correct

sure i'll fill checklist in a bit..

[ishpaul777]

1. [@] The PR that introduced the bug has been identified. Link to the PR: Honestly no idea which PR introduced could be introduced when we migrated from report.visibleParticapantIDs to report.notificationPreference, again I could not find which PR did that.
2. [@] 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
3. [@] 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
4. [@] Determine if we should create a regression test for this bug. yes
5. [@] 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.

Test Proposal

Precondition: user is part of a workspace that has at least a room that is visible in the workspace

As Employee

1. Navigate to a workspace room but do not join the room
2. Navigate away

as Another user (member of room) or workspace admin

3. Send a message

Expected Result:

To not receive desktop/browser notifications when new posts are added to the room

Do we agree 👍 or 👎 ?

Sidenote:
Before we start testing this in QA process we should wait for #47577 and also we should test this again with existing room with this adhoc build #47577 (comment) with accounts for which this is still producable to be sure that this isssue is fixed

[mallenexpensify]

Thanks @ishpaul777 , test case created

• https://github.com/Expensify/Expensify/issues/426642

[muttmuure]

This is still happening as reported here: https://expensify.slack.com/archives/C05LX9D6E07/p1725908484386969?thread_ts=1725899304.608239&cid=C05LX9D6E07

@shubham1206agra could you let us know when your fix is ready?

[mallenexpensify]

@puneetlath I'm out til Tuesday, can you keep 👀 on plz? You're already assigned, have context and you're a BZ, so it makes sense to me to not rando-assign to another BZ.

[melvin-bot]

@puneetlath, @mallenexpensify, @abdulrahuman5196, @ishpaul777 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[ishpaul777]

PR was deployed yesterday

[melvin-bot]

@puneetlath, @mallenexpensify, @abdulrahuman5196, @ishpaul777 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[ishpaul777]

PR deployed 5days ago i think this is Good to close

[mallenexpensify]

Closing, just checked Desktop and haven't seen any notifications/messages from rooms I haven't joined. Thx @ishpaul777