-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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: support default email feature via portal config #3415
Conversation
SMTPTransport t = null; | ||
try { | ||
Properties prop = System.getProperties(); | ||
prop.put("mail.smtp.auth", "true"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this mandatory? I noticed the following instruction from the javadoc
To use SMTP authentication you'll need to set the mail.smtp.auth property (see below) or provide the SMTP Transport with a username and password when connecting to the SMTP server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I just remove this line. The test is fine. Please take a look.
d617268
to
d25df08
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one nit, otherwise looks good to me
d25df08
to
72a4825
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What's the purpose of this PR
Apollo already has supported to integration email feature, but it needs developers to implement it by themselves.
For easier use of Apollo, I modified the
DefaultEmailService
, so users could enjoy the email notification feature by just add some configurations at the portal.Properties users concerned as follows:
true
to enable email feature.And the properties as follows I think they should have default values unless they want to modify:
email.config.user
Which issue(s) this PR fixes:
Brief changelog
javax.mail
in the portal.Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean test
to make sure this pull request doesn't break anything.