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

style(alerts): Change copy of mute alert link #73803

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

mifu67
Copy link
Contributor

@mifu67 mifu67 commented Jul 3, 2024

Change the link text for muting email alerts from "Mute this alert" to "Mute this alert for everyone."

Fixes GH-69792

@mifu67 mifu67 requested a review from a team as a code owner July 3, 2024 22:22
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 3, 2024
Copy link

codecov bot commented Jul 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.08%. Comparing base (b4e9f3f) to head (e906732).
Report is 65 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #73803      +/-   ##
==========================================
+ Coverage   74.95%   78.08%   +3.12%     
==========================================
  Files        6654     6648       -6     
  Lines      297330   297435     +105     
  Branches    51166    51195      +29     
==========================================
+ Hits       222859   232239    +9380     
+ Misses      67722    58812    -8910     
+ Partials     6749     6384     -365     

see 703 files with indirect coverage changes

schew2381

This comment was marked as abuse.

@mifu67 mifu67 merged commit 578516f into master Jul 8, 2024
48 checks passed
@mifu67 mifu67 deleted the mifu67/email-mute-options branch July 8, 2024 16:06
@ceorourke
Copy link
Member

ceorourke commented Jul 8, 2024

Hi I'm sorry I didn't see this before, but this text change is incorrect. You can see the original implementation here for the front end, and here for the issue alert email. (digests and metric alerts) We pass the query param mute:1 in the link and the front end reads that as "mute for me".

As far as I can see this hasn't changed since it was originally added so all emails would have the mute:1 query parameter. I think we left the text a little ambiguous on purpose because there are certain alert configurations where the only choice is to mute for everyone, like if it's set to notify a team (this may not be the correct example, it's hard to remember). If we really wanted to we could change it so as we build the emails it changes the text and query param, but we thought this was ok as is because when you do click to mute, there is a text banner that says either "You muted this alert so you won't get these notifications in the future." or "You muted this alert for everyone so you won't get these notifications in the future."

@mifu67 mifu67 added the Trigger: Revert Add to a merged PR to revert it (skips CI) label Jul 8, 2024
@getsentry-bot
Copy link
Contributor

PR reverted: 83e5b72

getsentry-bot added a commit that referenced this pull request Jul 8, 2024
This reverts commit 578516f.

Co-authored-by: mifu67 <83109586+mifu67@users.noreply.github.com>
priscilawebdev pushed a commit that referenced this pull request Jul 11, 2024
Change the link text for muting email alerts from "Mute this alert" to
"Mute this alert for everyone."

Fixes GH-69792
priscilawebdev pushed a commit that referenced this pull request Jul 11, 2024
This reverts commit 578516f.

Co-authored-by: mifu67 <83109586+mifu67@users.noreply.github.com>
@mifu67 mifu67 added the Do Not Merge Don't merge label Jul 15, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Do Not Merge Don't merge Scope: Backend Automatically applied to PRs that change backend components Trigger: Revert Add to a merged PR to revert it (skips CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify Sentry alert email - Mute this alert button - confusion
4 participants