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 Issue 451 #454

Merged
merged 3 commits into from
May 30, 2018
Merged

Fixes Issue 451 #454

merged 3 commits into from
May 30, 2018

Conversation

dsouzarc
Copy link
Contributor

@dsouzarc dsouzarc commented Oct 27, 2017

Fixes #451 with test cases

Issue Summary
Requests to send mail with both plain text and HTML content fail if the HTML content is specified first.

@mbernier edited the link to the issue

@codecov-io
Copy link

codecov-io commented Oct 27, 2017

Codecov Report

Merging #454 into master will decrease coverage by 0.16%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #454      +/-   ##
=========================================
- Coverage   88.26%   88.1%   -0.17%     
=========================================
  Files          30      30              
  Lines         929     933       +4     
  Branches      112     114       +2     
=========================================
+ Hits          820     822       +2     
- Misses         53      54       +1     
- Partials       56      57       +1
Impacted Files Coverage Δ
sendgrid/helpers/mail/mail.py 96.02% <60%> (-1.26%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 718ff6a...aefb3bd. Read the comment docs.

self._contents.append(content)

#Fix for issue-451: text content should be before html content
if content._type == "text/plain":
Copy link
Contributor

Choose a reason for hiding this comment

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

What will happen here if someone provides only html content?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hugomallinson who reported the issue mentioned that it was only when there is both plain text and HTML content and the HTML content is specified first.

If there's only HTML content, there shouldn't be any error and order wouldn't matter/make a difference

@mbernier mbernier added hacktoberfest difficulty: medium fix is medium in difficulty status: code review request requesting a community code review or review from Twilio and removed status: code review request requesting a community code review or review from Twilio labels Oct 28, 2017
@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 29, 2017
@thinkingserious thinkingserious added the type: community enhancement feature request not on Twilio's roadmap label Feb 27, 2018
@thinkingserious thinkingserious merged commit 0007d66 into sendgrid:master May 30, 2018
@thinkingserious
Copy link
Contributor

Hello @dsouzarc,

Thanks again for the PR!

We appreciate your contribution and look forward to continued collaboration. Thanks!

Team SendGrid DX

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium fix is medium in difficulty status: code review request requesting a community code review or review from Twilio type: community enhancement feature request not on Twilio's roadmap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants