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

Fixes #3113: Adds from name to email #3174

Merged
merged 1 commit into from
Feb 11, 2017
Merged

Fixes #3113: Adds from name to email #3174

merged 1 commit into from
Feb 11, 2017

Conversation

abhinavk96
Copy link
Contributor

Fixes #3113

Specifies from "Name" for emails in admin dashboard

image

@abhinavk96 abhinavk96 changed the title Adds from name to email Fixes #3113: Adds from name to email Feb 10, 2017
Copy link
Member

@niranjan94 niranjan94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CosmicCoder96

Don't implement the name only for SMTP.

keep the name common to SMTP and Sendgrid and also implement on sendgrid.

@niranjan94
Copy link
Member

And @CosmicCoder96 ... please comment on the issue before you start working on it so that we can ensure multiple people are not working on the same issue

@abhinavk96
Copy link
Contributor Author

abhinavk96 commented Feb 10, 2017

@niranjan94
Thanks for the review. I will be sure to keep that in mind.
I have now placed the from name text field on the upper part of the form to make it common to both. And since the 'payload' in helpers.py is common to both smtp and sendgrid, I believe this should suffice. Please review again.

@niranjan94
Copy link
Member

@CosmicCoder96 no. this would fail for sendgrid. Sendgrid expects only an email ID in the from field. For sendgrid you'd need to put the name in the fromname field.

Please see https://sendgrid.com/docs/API_Reference/Web_API/mail.html

@niranjan94
Copy link
Member

When using a third-party API/sdk, never assume a behavior. Always check the appropriate documentation and confirm.

@abhinavk96
Copy link
Contributor Author

abhinavk96 commented Feb 10, 2017

@niranjan94
Thanks for the advice. I have made the necessary changes.
This time I have tested by sending a mail using sendgrid API too.

@niranjan94 niranjan94 merged commit bebafaf into fossasia:development Feb 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants