-
Notifications
You must be signed in to change notification settings - Fork 72
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
use HTML property, not text, when dispatching Mailchimp Transactional emails #2901
Conversation
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.
this looks good. just noting, per @seanpreston , this was tested manually by a customer. we don't have any automated integration tests with mailchimp transactional so it'd be difficult to have automated coverage for this code path at this point.
@seanpreston my only concern is related to your question before about whether we have any non-html templates. can you think of anything that would make this a bit more robust/fail more obviously in the case that we do get a non-HTML template into our code base? maybe some automated test around that?
not gonna hold this up for that though :)
Passing run #958 ↗︎
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #2901 +/- ##
==========================================
- Coverage 86.73% 86.60% -0.13%
==========================================
Files 295 299 +4
Lines 16760 16814 +54
Branches 2146 2148 +2
==========================================
+ Hits 14537 14562 +25
- Misses 1821 1841 +20
- Partials 402 411 +9
... and 40 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
These are good questions. It's my understanding that if you pass text into |
Hey @adamsachs, I can confirm what I expected is the behaviour: Both of those emails were sent using the |
Thank you for the diligence @seanpreston!!! great to see that we're able to get testing done internally too. 0 caveats or reservations on getting this merged now 😄 |
Description
This PR updates the Mailchimp Transactional adaptor to use the
html
attribute, and subsequently render html correctly in emails.