Skip to content

Parameter from_email to send_mail() should be string - #3417

Merged
symbolist merged 1 commit into
masterfrom
usman/lms2518-refund-emails
May 2, 2014
Merged

Parameter from_email to send_mail() should be string#3417
symbolist merged 1 commit into
masterfrom
usman/lms2518-refund-emails

Conversation

@symbolist

Copy link
Copy Markdown
Contributor

No description provided.

@symbolist

Copy link
Copy Markdown
Contributor Author

@symbolist

Copy link
Copy Markdown
Contributor Author

@adampalay

@adampalay

Copy link
Copy Markdown
Contributor

👍

@flowerhack

Copy link
Copy Markdown
Contributor

Could we get a unit test added to shoppingcart/tests/test_models.py for this change?

Also, if it's not too much trouble, I'd really love to fix logging in this PR as well. Currently we log if an email raises a SMTPException or a BotoServerError. Maybe we should also log for whatever error went uncaught in this case / maybe just have a broad except that logs if there's any kind of problem with the send mail call?

Thanks for fixing this up!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This will result in logs like:

2014-05-02 11:16:32,719 ERROR 11860 [shoppingcart] models.py:57 - Failed sending email to billing request a refund for verified certiciate (User student, Course edx/math101/2014, CourseEnrollmentID 509, Order #136)
BotoServerError: 403 Forbidden
<ErrorResponse xmlns="http://ses.amazonaws.com/doc/2010-12-01/">
  <Error>
    <Type>Sender</Type>
    <Code>InvalidClientTokenId</Code>
    <Message>The security token included in the request is invalid</Message>
  </Error>
  <RequestId>bcd087b1-d20c-11e3-88cf-4b4f31b54ed5</RequestId>
</ErrorResponse>

@flowerhack

Copy link
Copy Markdown
Contributor

Looks great! 👍 , ship it, etc

Thanks for this.

symbolist added a commit that referenced this pull request May 2, 2014
Parameter from_email to send_mail() should be string
@symbolist
symbolist merged commit 5903ec5 into master May 2, 2014
@symbolist
symbolist deleted the usman/lms2518-refund-emails branch May 2, 2014 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants