Update the "SES" and "sendmail" mailers for the new API; remove "encoding"
ClosedPublic
Actions

Authored by epriestley on Jan 14 2019, 5:02 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T920: Provide SMS Support
T13222: 2018 Week 48-51 Bonus Content

Commits

rPc3cafffed726: Update the "SES" and "sendmail" mailers for the new API; remove "encoding"

Summary

Ref T13222. Ref T920. This is the last of the upstream adapter updates.

Test Plan

Sent mail with SES.
Sent mail with "sendmail". I don't have sendmail actually configured to an upstream MTA so I'm not 100% sure this worked, but the sendmail binary didn't complain and almost all of the code is shared with SES, so I'm reasonably confident this actually works.

Diff Detail

Repository

rP Phabricator

Branch

mfa10

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 21557
Build 29379: Run Core Tests
Build 29378: arc lint + arc unit

Event Timeline

epriestley created this revision.Jan 14 2019, 5:02 PM

Harbormaster completed remote builds in B21511: Diff 47659.Jan 14 2019, 5:33 PM

epriestley requested review of this revision.Jan 14 2019, 5:33 PM

epriestley mentioned this in D19956: Update Postmark adapter for multiple mail media.Jan 14 2019, 10:15 PM

I definitely reviewed almost this exact same code, but I have completely lost it in the stack of email-related changes recently. If I recall, I complained about the unused variable, checking for truthiness instead of using strlen for all the fields that PHPMailer wants, and the base64 attachment encoding thing. The truthiness checks make sense now that all those properties are objects instead of bare strings.

externals/phpmailer/class.phpmailer-lite.php
98	Still an unused variable.
src/applications/metamta/adapter/PhabricatorMailSendmailAdapter.php
3	Hooray!

This revision is now accepted and ready to land.Jan 16 2019, 6:59 PM

Aha, this is what triggered my deja vu: D19959.

Don't tell anyone, but I copy/pasted all of these.

bwahaha

Remove unused variable.

Harbormaster completed remote builds in B21557: Diff 47705.Jan 16 2019, 9:14 PM

Closed by commit rPc3cafffed726: Update the "SES" and "sendmail" mailers for the new API; remove "encoding" (authored by epriestley). · Explain WhyJan 16 2019, 9:19 PM

This revision was automatically updated to reflect the committed changes.

epriestley added inline comments.Jan 16 2019, 9:22 PM

src/applications/metamta/adapter/PhabricatorMailSendmailAdapter.php
101–105	Oh, and I believe this ("..does NOT support..") wasn't right, the way to do it is just a little tricky (`->AltBody`). Or maybe this was true when written, then we updated `PHPMailerLite`, then it became untrue? In either case, we build HTML + Text mail now.