Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$1000] Chat - Room admin sees all welcome messages to invited users (appears after page refresh) #19689

Closed
3 of 6 tasks
kbecciv opened this issue May 26, 2023 · 15 comments
Closed
3 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kbecciv
Copy link

kbecciv commented May 26, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Issue found when executing PR #18662

Action Performed:

  1. Login to NewDot
  2. As user A create a public room and add a welcome message
  3. As a user A invite user B to public room
  4. As user B accept invitation to the room (enter the room). Observe you see welcome message.
  5. As a user A refresh the room page

Expected Result:

Welcome messages should be visible only to invited users

Actual Result:

Welcome message to every user that joined the room is visible to admin (with tag, that message is visible only to that joining user)

Workaround:

Unknown

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

Reproducible in staging?: yes

Reproducible in production?: new feature

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

https://platform.applause.com/services/links/v1/external/86a90ede63d027d507e3b0d7caffd19534dbaef8b6bd1ac1d30e8c226d23d120

Expensify/Expensify Issue URL:

Issue reported by: Applause - Infernal team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01454f0d4365208af8
  • Upwork Job ID: 1664740336325787648
  • Last Price Increase: 2023-06-02
@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels May 26, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 26, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented May 26, 2023

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 melvin-bot bot added the Overdue label May 29, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 29, 2023

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

@huzaifa-99
Copy link
Contributor

huzaifa-99 commented May 31, 2023

Proposal

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

We want the welcome messages to show only for intended users

What is the root cause of that problem?

We are not filtering welcome messages for non-intended users.

When User A creates a room, sets a welcome message and then User B joins, the api creates a new reportAction with the welcome message and a whisperedTo key that contains the email of User B. When User A reloads the page and gets fresh reportActions data, the new message that api created will also be included in report actions which is why the admin or User A sees the welcome message.

Similarly when another user (User C) joins, the api will create a new reportAction with the welcome message in which whisperedTo will contain the email of User C.

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

We should filter welcome message and show only to intended users. In shouldReportActionBeVisible(), we should add a condition to show whisper messages when the email of the login user is included in the whisper message. Something like this

// filters out messages where the login user is not a participant (not in whisperedTo array)
const whisperedTo = lodashGet(reportAction, 'whisperedTo', []);
if (whisperedTo.length > 0 && !_.includes(whisperedTo, sessionEmail)) {
    return false
}

The session email we can get from onyx in ReportActionsUtils

let sessionEmail;
Onyx.connect({
    key: ONYXKEYS.SESSION,
    callback: (val) => (sessionEmail = val ? val.email : null),
});
Result
Screen.Recording.2023-05-31.at.3.44.45.PM.mov

Optional: We can also check for report type i.e with CONST.REPORT.ACTIONS.TYPE and check filter whisperedTo based on type

What alternative solutions did you explore? (Optional)

N/A

@michaelhaxhiu
Copy link
Contributor

michaelhaxhiu commented May 31, 2023

Ya this is strange UX for sure. I agree with the expected outcome.

edit: Wow Seems I failed to add the external label after reproducing 2 days ago. 😶‍🌫️ that was dumb. fixed.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels May 31, 2023
@michaelhaxhiu michaelhaxhiu added the External Added to denote the issue can be worked on by a contributor label Jun 2, 2023
@melvin-bot melvin-bot bot changed the title Chat - Room admin sees all welcome messages to invited users (appears after page refresh) [$1000] Chat - Room admin sees all welcome messages to invited users (appears after page refresh) Jun 2, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 2, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01454f0d4365208af8

@melvin-bot
Copy link

melvin-bot bot commented Jun 2, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jun 2, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 2, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 2, 2023

Triggered auto assignment to @dangrous (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@michaelhaxhiu
Copy link
Contributor

1 proposal already added

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jun 2, 2023
@dangrous
Copy link
Contributor

dangrous commented Jun 5, 2023

@abdulrahuman5196 if you have time to take a look at @huzaifa-99's proposal that would be great!

@melvin-bot melvin-bot bot removed the Overdue label Jun 5, 2023
@abdulrahuman5196
Copy link
Contributor

@dangrous Based on the proposal I checked on the action items. We are getting the unwanted action from API itself.
Can we filter the unwanted reportActions at server itself? Any specific reason we should do it in frontend?

@dangrous
Copy link
Contributor

dangrous commented Jun 5, 2023

Oh hm, yeah whispers shouldn't be sent at all to anyone who isn't the recipient, i don't think.

I'll cc @pecanoro here in case she has some ideas, but otherwise I'll take a look at the backend and see what I can figure out.

@pecanoro
Copy link
Contributor

pecanoro commented Jun 5, 2023

This is by design, whoever sent the whisper can see the message as well so they know who was sent to (both are aware). However, it's true this can be a bit more annoying for welcome messages as they can be sent to a lot of people if they join a big room. I vote to close it for now and deal with it later. Or maybe just bring up the discussion to Slack in product to see how we should approach it. We would need some kind of flag to tell welcome messages apart from normal whispers.

@dangrous
Copy link
Contributor

dangrous commented Jun 5, 2023

Oh interesting. I have started a discussion in Slack, and closed this for now. Thank you for the details @pecanoro!

@dangrous dangrous closed this as completed Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

6 participants