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

feat(alerting): discord provider message content config #712

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

GarrettRobinson
Copy link

Summary

Discord provider config entry for the Content field when posting messages to the service.

Discord limitations don't allow mentions inside embeds to ping/highlight.

Using the content field allows a mention to:

  • Ping the user/role with an alert
  • Increase the notification counter on the server/channel
  • Add a message to the notification inbox

Checklist

  • Tested and/or added tests to validate that the changes work as intended, if applicable.
  • Updated documentation in README.md, if applicable.

Copy link
Owner

@TwiN TwiN left a comment

Choose a reason for hiding this comment

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

Couple things:

  1. Why not just have a channel specifically for these types of alerts, and have people edit the notification settings for that channel so that they get notified for every message?
  2. Some of the indentations you made use spaces instead of tabs
  3. You'll have to update the tests or they'll fail
  4. In the documentation, I'd suggest you add a note that says:

optionally, you can use the alerting.discord.content field to ping specific users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants