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

[FR] Slack notification #3843

Closed
2 tasks done
larchen opened this issue Oct 24, 2022 · 5 comments · Fixed by #4114
Closed
2 tasks done

[FR] Slack notification #3843

larchen opened this issue Oct 24, 2022 · 5 comments · Fixed by #4114
Assignees
Labels
enhancement This is an suggested enhancement or new feature plugin Plugin ecosystem
Milestone

Comments

@larchen
Copy link

larchen commented Oct 24, 2022

Please verify that this feature request has NOT been suggested before.

  • I checked and didn't find similar feature request

Problem statement

I'd like to have slack as an alternative notification delivery method to email, with the ability to specify which notifications trigger a message, and which channels they should send the message to. Ideally this could all be configured through the settings page.

Suggested solution

It looks like this would involve adding a new plugin to handle the message send logic. If I understand correctly it seems that I can use the CoreNotificationsPlugin as a template. Do I also need to use the EventMixin?

Sending messages to slack channels is relatively straightforward with webhooks and the Slack API.

I'm relatively new to this so would appreciate any suggestions/advice on how best to proceed.

Describe alternatives you've considered

Open to other suggestions.

Examples of other systems

Prometheus/Alertmanager has nice integration with slack webhooks: https://prometheus.io/docs/alerting/latest/configuration/#slack_config

Do you want to develop this?

  • I want to develop this.
@larchen larchen added enhancement This is an suggested enhancement or new feature triage:not-checked Item was not checked by the core team labels Oct 24, 2022
@SchrodingersGat SchrodingersGat added plugin Plugin ecosystem and removed triage:not-checked Item was not checked by the core team labels Oct 24, 2022
@matmair
Copy link
Member

matmair commented Oct 24, 2022

@larchen that would be a pretty simple addition and I think we might be open to ship it as built-in.
You can basically copy the code from CoreNotificationsPlugin, as you will see there you only need to implement the sending itself.
The settings/notification framework enable you to use both general and user settings. That way you could for example send notifications to a general channel and to users directly.

Regarding events: Notifications are triggered directly and can be addressed (to the responsible user) while events are just database/ORM changes. Therefore working with events requires a bit more code as you need to filter and address target users yourself.
We ship with notifications for some common use cases like low stock. Those are notifications that get triggered (mostly) daily.

@larchen
Copy link
Author

larchen commented Oct 26, 2022

Sounds good I'll take a stab at this over the weekend.

One additional question is whether or not there is currently a builtin mechanism for routing notifications to different sources? Let's say I want to send all low stock notifications to one slack channel, purchase order notifications to a different channel, but server error notifications to email. I guess this could be done by having a filter on notifications for each notification method.

@matmair
Copy link
Member

matmair commented Oct 26, 2022

@larchen filtering based on category is currently not done but would be certainly possible; for the first try I would recommend getting the plugin and slack running, adding filtering ontop should be rather easy.

I think more setting for notifications (distribution channel per category/team/user, delayed delivery on some channels, aggregation) would be certainly welcome in core. From an architectural point I would prefer everything else then final channel delivery in the core notification code; that makes reuse simpler and keeps coverage high (delivery channels are not well tested in general).

@github-actions
Copy link
Contributor

This issue seems stale. Please react to show this is still important.

@github-actions github-actions bot added the inactive Indicates lack of activity label Dec 26, 2022
@matmair matmair removed the inactive Indicates lack of activity label Dec 28, 2022
@matmair matmair self-assigned this Dec 28, 2022
@matmair matmair added this to the 0.10.0 milestone Dec 28, 2022
@matmair
Copy link
Member

matmair commented Dec 28, 2022

@larchen I have a PR ready if you want to test.

matmair added a commit to matmair/InvenTree that referenced this issue Dec 28, 2022
Add slack sending method
Fixes inventree#3843
SchrodingersGat pushed a commit that referenced this issue Dec 31, 2022
* [FR] Slack notification
Add slack sending method
Fixes #3843

* Add global panel with notification methods

* add plugin information

* fix plugin lookup and link

* Add settings content mixin

* Add instructions for Slack setup

* fix rendering of custom settings content
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an suggested enhancement or new feature plugin Plugin ecosystem
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants