-
Notifications
You must be signed in to change notification settings - Fork 443
Add generic notification subscriptions through webhooks #1137
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
base: master
Are you sure you want to change the base?
Conversation
|
I think that this is a cool concept, but perhaps it could be extended to work for folders other than inbox as is currently hardcoded? I think that that would make it more extensible/useful. What are your thoughts, @sdelgadoc? Edit: I see that sdelgadoc is potentially inactive. @alejcas what are your thoughts on this PR as a whole? If it's accepted I would be happy to put in a PR then doing the above. |
|
Hey @kwilsonmg, I never finished this, but still find it useful to implement. There's also some ways I've figured out how to make it more robust. If @alejcas and you would find it useful, I could finish it and submit. Let me know! |
|
I have immediate use cases in mind as well for a feature like this. Curious alejcas' thoughts. I would love to see this completed. |
|
This should be on it's own file (subscriptions.py) and be accessible through the account object. A subscription can be set to any ms graph element. I would like to merge it but not in it's current state. |
|
Thanks @alejcas, your architectural suggestion makes sense. Give me a few days and I'll send out a PR with the changes for @kwilsonmg and you to test and review. |
|
Hey @alejcas and @kwilsonmg, please find here completed the PR to add subscriptions and notifications to the library. I added an example to demonstrate usage, and for testing. Let me know if you have any questions or edits. Otherwise, this is ready to merge. |
|
I can merge it as it is although it lacks:
At least the list method should be available. |
|
Thanks @alejcas. I think that adding the three endpoints you listed is reasonable. However, with the holidays coming up, I will likely not be able to start implementing until early to mid December. So, depending on @kwilsonmg and other users' timeline, we can either merge this PR to give folks basic functionality, and I can add the additional endpoints later, or wait and merge all the functionality together in a few weeks. Either works for me. |
|
This is certainly a "nice to have" for the library, but I can honestly wait a while longer. I'm not in a huge rush, just excited by the possibilities this will all unlock once completed. (TLDR: I'm indifferent) |
No description provided.