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 notifications & notification method updates #4114

Merged
merged 7 commits into from
Dec 31, 2022

Conversation

matmair
Copy link
Member

@matmair matmair commented Dec 28, 2022

This PR:

  • Adds a slack sending method
  • Adds a mixin for adding settings conent
  • Adds a central panel for global notification settings
    - [ ] Extends notification methods to include registering

Fixes #3843

Add slack sending method
Fixes inventree#3843
@matmair matmair added enhancement This is an suggested enhancement or new feature plugin Plugin ecosystem api Relates to the API labels Dec 28, 2022
@matmair matmair added this to the 0.10.0 milestone Dec 28, 2022
@matmair matmair self-assigned this Dec 28, 2022
@matmair matmair marked this pull request as draft December 28, 2022 18:20
@miggland
Copy link
Contributor

@matmair: I just stumbled on this package again. Have used it earlier (CLI only) for Gotify. However, it may give the possibility of having general notifications, without explicitly adding every service independently..
https://github.com/caronc/apprise

@matmair
Copy link
Member Author

matmair commented Dec 30, 2022

@miggland looks cool, I created a new FR for it. While it is easy to send simple notifications I prefer to use native integrations as they allow for the use of platform-specific features (like Slack Block Kit for example).

@matmair matmair marked this pull request as ready for review December 30, 2022 22:41
@SchrodingersGat
Copy link
Member

@matmair nice work this looks really good.

Do you think it also closes out #4073 ?

@matmair
Copy link
Member Author

matmair commented Dec 31, 2022

@SchrodingersGat I would have loved a simple one-click add button for setup but that would require me publishing a central app for slack (and therefore everyone trusting me not screwing with that access).

Not sure if everything Lukas envisioned is covered, I think we should let him decide. @wolflu05

@wolflu05
Copy link
Contributor

@matmair Sorry, didn't get the context why you mentioned me. Or do you mean the new settings content mixin which gets added by this pr which is not included in the title? Form my side, I would preferre an interface more like the panels mixin which also adds the ability to render custom js templates directly and also render html templates via filename. I know this could be made already via some custom logic, but I think I'm not the only one which will try to add custom templates instead of html strings.

@matmair
Copy link
Member Author

matmair commented Dec 31, 2022

@wolflu05 yeah you were mentioned for that / #4073
@SchrodingersGat so this won't close that. I do not intend to extend the current PR as the whole custom rendering is not really working well with #3901 and would add another work site for the transition.

@SchrodingersGat SchrodingersGat merged commit 8b2e2a2 into inventree:master Dec 31, 2022
@SchrodingersGat
Copy link
Member

@matmair nice work. A small docs PR would be great if you have a chance

@wolflu05
Copy link
Contributor

wolflu05 commented Jan 1, 2023

Yeah, the whole Django rendering part with templates, plugins provide will be tricky in the react ui. Maybe it can be just embedded via iframes?

@matmair matmair deleted the matmair/issue3843 branch January 1, 2023 16:10
@matmair
Copy link
Member Author

matmair commented Jan 1, 2023

@wolflu05 there are some limitations with iframes from browsers so I am still trying to find another way.

@wolflu05
Copy link
Contributor

wolflu05 commented Jan 1, 2023

@matmair from what limitations are you talking?

@matmair
Copy link
Member Author

matmair commented Jan 1, 2023

@wolflu05 some APIs can not be called by js code in an iframe.

@wolflu05
Copy link
Contributor

wolflu05 commented Jan 1, 2023

@matmair And what about using dangerouslysetinnerhtml for legacy plugins, and current plugins can just provide an bundle file which has a default export which is included via runtime import() and rendered as component?

@matmair
Copy link
Member Author

matmair commented Jan 1, 2023

@wolflu05 I think we should take this conversation into #3901 or the subrepo - it does not really have anything to do with this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Relates to the API enhancement This is an suggested enhancement or new feature plugin Plugin ecosystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] Slack notification
4 participants