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

[HOLD #336130][$500] Workspace chat notifications is ignored for requests #32072

Closed
1 of 6 tasks
m-natarajan opened this issue Nov 27, 2023 · 30 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff

Comments

@m-natarajan
Copy link

m-natarajan commented Nov 27, 2023

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:
Reproducible in staging?: needs reproduction
Reproducible in production?: needs reproduction
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: @quinthar
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1700919080607879

Action Performed:

  1. Create a workspace in account A
  2. Click on the new workspace in the LHN > click on the workspace name at the top of the page
  3. Click on Settings
  4. Set notification preference for workspace chat to Daily or Mute
  5. Go back to the workspace through the Profile > click Members > Invite member B
  6. As member B request money from the workspace chat

Expected Result:

Should notify Account A via the standard UnreadMessageSummary logic, which honors the notification preference.

Actual Result:

A is sent an email/text immediately, via a dedicated notification, ignoring the A's workspace chat notification setting.

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

View all open jobs on GitHub

2023-11-28_16-01-33 (4)

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01055a3f6b1ce74d71
  • Upwork Job ID: 1729640666933612544
  • Last Price Increase: 2023-11-28
@m-natarajan m-natarajan added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels Nov 27, 2023
Copy link

melvin-bot bot commented Nov 27, 2023

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

Copy link

melvin-bot bot commented Nov 27, 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

@Christinadobrzyn Christinadobrzyn removed the Needs Reproduction Reproducible steps needed label Nov 28, 2023
@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Nov 28, 2023

I think I got this right in the OP video (sorry the pixelation is so bad, it's a heavy video). It's true the notifications shouldn't show in the LHN if the settings are set to something other than immediate.

I think this can be external?

@Christinadobrzyn Christinadobrzyn added the External Added to denote the issue can be worked on by a contributor label Nov 28, 2023
@melvin-bot melvin-bot bot changed the title Workspace chat notifications is ignored for requests [$500] Workspace chat notifications is ignored for requests Nov 28, 2023
Copy link

melvin-bot bot commented Nov 28, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01055a3f6b1ce74d71

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

melvin-bot bot commented Nov 28, 2023

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

@unidev727
Copy link
Contributor

@m-natarajan
What is the UnreadMessageSummary?

@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Nov 29, 2023

@aldo-expensify it looks like you've been working in the product channel on the UnreadMessageSummary. This is what I understand of this behaviour but I don't know if it actually answers @unicorndev-727's above question.

  1. Unread message can create a notification (UnreadMessageSummary). These notification are not sent if the workspace setting is set to Mute (or Daily) messages.
  2. Creating/modifying reports/expense can create a different notification. These notifications are always sent.
  • We're working on changes if the user doesn't' want to receive the 2nd notification type.

Maybe this should be internal?

Asking for some guidance - https://expensify.slack.com/archives/C03U7DCU4/p1701224698357089?thread_ts=1700867114.980389&cid=C03U7DCU4

@aldo-expensify
Copy link
Contributor

aldo-expensify commented Nov 29, 2023

@Christinadobrzyn yes, this is internal. Also, I think this falls within the scope of https://github.com/Expensify/Expensify/issues/336130

@Christinadobrzyn Christinadobrzyn added Internal Requires API changes or must be handled by Expensify staff and removed External Added to denote the issue can be worked on by a contributor labels Nov 30, 2023
Copy link

melvin-bot bot commented Nov 30, 2023

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

@Christinadobrzyn
Copy link
Contributor

Awesome! Thanks @aldo-expensify! I'll put this on hold for https://github.com/Expensify/Expensify/issues/336130 just to make sure it's resolved with that job!

@Christinadobrzyn Christinadobrzyn changed the title [$500] Workspace chat notifications is ignored for requests [HOLD 336130][$500] Workspace chat notifications is ignored for requests Nov 30, 2023
@Christinadobrzyn Christinadobrzyn changed the title [HOLD 336130][$500] Workspace chat notifications is ignored for requests [HOLD #336130][$500] Workspace chat notifications is ignored for requests Nov 30, 2023
@Christinadobrzyn Christinadobrzyn added Weekly KSv2 and removed Daily KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors labels Nov 30, 2023
@Christinadobrzyn
Copy link
Contributor

1 similar comment
@Christinadobrzyn
Copy link
Contributor

@melvin-bot melvin-bot bot added the Overdue label Dec 21, 2023
@Christinadobrzyn
Copy link
Contributor

@Christinadobrzyn
Copy link
Contributor

@melvin-bot melvin-bot bot removed the Overdue label Jan 16, 2024
@Christinadobrzyn
Copy link
Contributor

@melvin-bot melvin-bot bot added the Overdue label Feb 1, 2024
@Christinadobrzyn
Copy link
Contributor

@melvin-bot melvin-bot bot removed the Overdue label Feb 1, 2024
@Christinadobrzyn
Copy link
Contributor

@melvin-bot melvin-bot bot added the Overdue label Feb 15, 2024
@Christinadobrzyn
Copy link
Contributor

@melvin-bot melvin-bot bot removed the Overdue label Feb 16, 2024
@aldo-expensify
Copy link
Contributor

I think this may be resolved, the following changes have been made since this was created:

  • Mute notification preference is respected now, and
  • The OldDot submit notification is not send anymore if the manager is an account that only uses NewDot (this is checked by looking at the NVP tryNewDot)

So.. if the workspace admin is a NewDot only user, the only notification that we will send if about "offline activity", but that will be cancelled because the workspace is muted.

@melvin-bot melvin-bot bot added the Overdue label Feb 26, 2024
@Christinadobrzyn
Copy link
Contributor

Oh great! I'll test this tomorrow! Thanks for the insight @aldo-expensify!

@melvin-bot melvin-bot bot removed the Overdue label Feb 27, 2024
@Christinadobrzyn Christinadobrzyn added Daily KSv2 Overdue and removed Weekly KSv2 labels Feb 27, 2024
@melvin-bot melvin-bot bot removed the Overdue label Feb 27, 2024
@Christinadobrzyn
Copy link
Contributor

Hey @aldo-expensify I'm not seeing 'notification' preferences in our NewDot workspace - do you know where this exists?

image

@aldo-expensify
Copy link
Contributor

It is available in the policyExpenseChat of each user, here:

image

@aldo-expensify
Copy link
Contributor

I don't think you can disable for the whole workspace

@Christinadobrzyn
Copy link
Contributor

Okay testing this Admin A for me [email protected] Employee [email protected].

Changed notification to daily so I'll see if I get a notification on this tomorrow.

@Christinadobrzyn
Copy link
Contributor

I did receive an email when the workspace was created but it was sent around the same time that I created the workspace. So I don't know if this aligns with our 'daily' frequency. Going to try again by muting the messages.

image

Testing again...

@Christinadobrzyn
Copy link
Contributor

Okay, testing on [email protected] (admin) and [email protected](member).

I created a workspace and set the notifications as mute.

image

When the new workspace was created, both users got an email notification about joining the workspace

image

But there was no message sent about the request money.

@aldo-expensify do you know if this is expected?
• Emails about joining a new workspace do not follow the notification rules of a chat?
• Request money notifications follow the notification rules?

I think this is correct but can you confirm?

@aldo-expensify
Copy link
Contributor

aldo-expensify commented Mar 1, 2024

@aldo-expensify do you know if this is expected?
• Emails about joining a new workspace do not follow the notification rules of a chat?

I haven't looked into that notification, but considering that the mute is set in the policyExpenseChat of the employee, I think that is not going to be able to prevent this notification from being sent to the admin. That notification goes out before you set it to mute

• Request money notifications follow the notification rules?

Yes, to my understanding this one should follow not be sent because of the mute

@melvin-bot melvin-bot bot added the Overdue label Mar 4, 2024
Copy link

melvin-bot bot commented Mar 4, 2024

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

@Christinadobrzyn
Copy link
Contributor

Ah okay so it does sound like this is behaving as expected based on #32072 (comment)

Going to close! @aldo-expensify or @c3024 let me know if you think otherwise!

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 Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests

5 participants