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.

Sign up for GitHub

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

Jump to bottom

feat(notif): prevent manage-request users receiving auto-approve notif from their requests #1709

Merged
merged 1 commit into from
May 29, 2021
Merged

feat(notif): prevent manage-request users receiving auto-approve notif from their requests #1709

merged 1 commit into from
May 29, 2021

Conversation

dphildebrandt
Copy link
Contributor

Description

Email and webpush agents will filter out these users. Discord agent won't @ them.

To-Dos

  • Successful build yarn build
  • Translation keys yarn i18n:extract

Issues Fixed or Closed

@dphildebrandt
feat(notif): prevent manage-request users receiving auto-approve noti…
1365b81 
…f from their requests (#1707)

Email and webpush agents will filter out these users. Discord agent won't @ them.
@dphildebrandt dphildebrandt requested a review from sct as a code owner May 28, 2021 21:47
@dphildebrandt dphildebrandt changed the title feat(notif): prevent manage-request users receiving auto-approve notif from their requests (#1707) feat(notif): prevent manage-request users receiving auto-approve notif from their requests May 28, 2021
@TheCatLady TheCatLady self-requested a review May 28, 2021 22:20
@TheCatLady
Copy link
Collaborator

TheCatLady commented May 28, 2021 •
edited

@dphildebrandt, the Telegram agent also supports per-user notifications (in addition to "system" notifications), so we should probably add this same logic to that agent as well. Otherwise, LGTM! Oops, forgot that this wasn't in develop yet, sorry! LGTM!

@sct, if we want to merge this PR first, I can modify the new user settings notification type descriptions in #1512 to reflect that the notifications are for other users' auto-approved requests.

sct
sct approved these changes May 29, 2021
View reviewed changes
Copy link
Owner

@sct sct left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thanks for this!

@sct sct merged commit 9ead8bb into sct:develop May 29, 2021
@sct
Copy link
Owner

sct commented May 29, 2021

@all-contributors please add @dphildebrandt for code

@allcontributors allcontributors bot mentioned this pull request May 29, 2021
Merged
@allcontributors
Copy link
Contributor

@sct

I've put up a pull request to add @dphildebrandt! 🎉

@dphildebrandt dphildebrandt deleted the feat/notif/self-auto-approve branch June 7, 2021 01:17
@dphildebrandt dphildebrandt restored the feat/notif/self-auto-approve branch June 7, 2021 01:17
@dphildebrandt dphildebrandt deleted the feat/notif/self-auto-approve branch June 7, 2021 01:18
@github-actions
Copy link

🎉 This PR is included in version 1.25.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sct sct sct approved these changes

@TheCatLady TheCatLady TheCatLady approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Prevent Automatically Approved notifications for those with Manage Requests
3 participants
@dphildebrandt @TheCatLady @sct