Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.52k stars 2.88k forks source link

[$250] Email notification received for expense report comments #41426

Closed m-natarajan closed 1 month ago

m-natarajan commented 6 months ago

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: 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: @puneetlath Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1714498891552929

Action Performed:

  1. Create and submit expense report as a employee to non admin approver
  2. Add comments to the expense report thread
  3. Check the email inbox as approver

    Expected Result:

    Since expense reports have the "hidden" notification preference by default, approver should not receive an email about this. Only email to be receive would be an unread message summary from the workspace chat when this expense was submitted.

Actual Result:

But email is sent to the approver and we are sending unread message summaries for expense reports, even for users they should be "hidden" for.

Clarification from Slack:

Two issues at play:

  1. Being notified about comments on a report which has the "hidden" notification preference
  2. The email seems to come from the expense report rather than from the workspace chat

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence Screenshot 2024-04-30 at 1 39 45 PM Screenshot 2024-04-30 at 1 39 53 PM

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019ccd32609048d078
  • Upwork Job ID: 1788198784674365440
  • Last Price Increase: 2024-05-08
melvin-bot[bot] commented 6 months ago

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

MelvinBot commented 6 months ago

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

CortneyOfstad commented 6 months ago

@m-natarajan I am having a really hard time understanding the expected and actual result sections — particularly the "hidden" message aspect. You mention "even for users they should be "hidden" for" — can you clarify who these users are exactly? Thanks!

CortneyOfstad commented 6 months ago

@m-natarajan bump ^^^ thanks!

CortneyOfstad commented 6 months ago

Asking in Slack here

CortneyOfstad commented 6 months ago

Alright, I got context in Slack for the issue at hand. There are two issues at play here:

  1. Being notified about comments on a report which has the "hidden" notification preference
  2. The email seems to come from the expense report rather than from the workspace chat

Going to get eyes on this 👍

melvin-bot[bot] commented 6 months ago

Job added to Upwork: https://www.upwork.com/jobs/~019ccd32609048d078

melvin-bot[bot] commented 6 months ago

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

CortneyOfstad commented 6 months ago

Still waiting on proposals 👍

puneetlath commented 5 months ago

@CortneyOfstad I think this will need to be internal as emails are generated by the back-end.

CortneyOfstad commented 5 months ago

Sounds good @puneetlath!

melvin-bot[bot] commented 5 months ago

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

melvin-bot[bot] commented 5 months ago

@eVoloshchak, @CortneyOfstad Huh... This is 4 days overdue. Who can take care of this?

CortneyOfstad commented 5 months ago

waiting for this to be picked up 👍

melvin-bot[bot] commented 5 months ago

@eVoloshchak @CortneyOfstad this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] commented 5 months ago

@eVoloshchak, @CortneyOfstad 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

CortneyOfstad commented 5 months ago

Not overdue — still waiting on proposals!

CortneyOfstad commented 5 months ago

Waiting for it to be picked up

CortneyOfstad commented 5 months ago

Still waiting

eVoloshchak commented 5 months ago

Not overdue, waiting on proposals

CortneyOfstad commented 5 months ago

^^^ still waiting on proposals.

CortneyOfstad commented 5 months ago

I know there is a similar issue with notifications regarding iOS and I wonder if there is something similar at play here in regards to notifications as a whole. Going to check with the team.

CortneyOfstad commented 5 months ago

@Julesssss You're working on the other GH with notifications (https://github.com/Expensify/App/issues/42379) do you think there could be something tying them together?

Julesssss commented 5 months ago

Hey, I don't think so in this case -- this issue seems to be linked to our wider nofity preference, but the iOS issue is due to a very specific piece of logic in the NewDot app.

CortneyOfstad commented 5 months ago

Gotcha, thank you @Julesssss!

CortneyOfstad commented 4 months ago

Still waiting on an engineer to snag this. Going to adjust the priority a bit, as the issue is pretty low in terms of overall functionality 👍

CortneyOfstad commented 4 months ago

Not overdue as this is still a hot pick. With Fast APIs being the priority, and most VIP-VSB being paused (outside of critical or high issues), going to adjust the frequency 👍 (More context here)

CortneyOfstad commented 3 months ago

Still not overdue because of the pause on VIP-VSB.

CortneyOfstad commented 2 months ago

I'm going to be retesting this over the next few days (have a few timely items that have to be addressed first) to see if the result is still the same here. Since it has been May and we've adjusted quite a few things within the app as a whole, I want to confirm this is still happening. Will follow up!

mvtglobally commented 1 month ago

Issue not reproducible during KI retests. (First week)

CortneyOfstad commented 1 month ago

I am not able to recreate this either. Going to close and if the issue pops up again, please reopen the GH and we can readdress. Thanks!