Notification feature #358
Open
Notification feature #358
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
caffix
requested changes
Feb 16, 2020
caffix
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This correponds to the first PR implementing the Notification feature.
We implement here the smtp notification. The SMTP provider should support an API key to allow the feature to work properly (it has been tested with gmail).
The tricky part for me right now is the unit test implementation with our current framework.
To sum up:
I added a new protocol smtp.
Then, I naturally added the send_email function in smtp.go.
The problem is that this function requires API keys. I decided to allow the user to provide them through the config file. This does not seem to match our current model where the config file is mainly used for user sources API keys though. Besides, the testing framework provides a setupconfig function which is also only accessible in the services package.
So, right now, the quickest solution to start implemeting my tests would be to import the services package in my smtp_test.go file. if you agree, I'll start to work on it this way.
Otherwise, I'm open to any suggestion to get a better solution.
PS: This PR is still work in progress. I still need to handle 2 more cases and finish the tests to be able to set it as Ready.