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

Add support to send mails using AWS SES #2877

Closed

Conversation

christiandavilakoobin
Copy link
Contributor

No description provided.

Copy link
Contributor

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Would it be possible to cover the new code with tests ?

@christiandavilakoobin
Copy link
Contributor Author

Hi @jrfnl. I'm not sure how to test this without an AWS account and without AWS SDK libraries installed. Also, I don't know how to warn anyone who wants to use this feature that they must have AWS SDK v3 installed.

@Synchro
Copy link
Member

Synchro commented Mar 1, 2023

While I can see that you've put a lot of effort in here (thank you for that), I'm not too keen on merging this. AWS SES is a proprietary thing, but it accepts a fully-formed RFC822 message, which you can get out of PHPMailer via getSentMIMEMessage(), so you could use PHPMailer to create the message, then hand it over to an entirely separate "companion" class that deals with AWS auth and submission over HTTP, all without having to alter the core classes at all.

@christiandavilakoobin
Copy link
Contributor Author

Hi @Synchro. Using getSentMIMEMessage() was my first aproach, but it requieres some extra adjustments to use PHPMailer with this, and this way the "bcc" headers are ignored because "preSend" doesn't include them on MIME. Finally I had to made those changes to be more friendly to people who are already using PHPMailer. Now you're only to mark as "$mail->useSes()" and it works same way than all others.

@Synchro
Copy link
Member

Synchro commented Mar 3, 2023

BCC addresses should never be in headers? Even then, you could easily insert an extra header via your own wrapper after fetching the built message. I still think there is too much going on in here for a single proprietary option and it would be better implemented in a subclass, where you would not even have to call useSes.

Put it like this, if I was using SES, this is how I would implement it anyway.

@Synchro Synchro closed this Aug 18, 2023
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.

None yet

3 participants