Skip to content

Conversation

@driesvints
Copy link
Member

@driesvints driesvints commented Jan 28, 2022

No description provided.

@driesvints driesvints changed the title Add SesTransport decorator [9.x] Add SesTransport decorator Jan 28, 2022
@deleugpn
Copy link
Contributor

Hey @driesvints which implementation you prefer to maintain long-term on Laravel? I see this decorator as a small tech footprint as it just adapts Laravel config to Symfony Mailer while the other PR bring back the entire AWS SDK and go back to handling email within Laravel instead of through Symfony.

Which one should I try to test first?

* @param array $options
* @return void
*/
public function __construct(TransportInterface $transport, array $options = [])
Copy link
Contributor

Choose a reason for hiding this comment

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

Since Laravel 9 requires PHP 8, couldn't the type-hint here be SesApiAsyncAwsTransport|SesHttpAsyncAwsTransport instead and remove the exception below?

@driesvints
Copy link
Member Author

@deleugpn heya, the re-implementation of the old SesTransport: #40696

This is because that one already fully offers every feature for SES while the Amazon Mailer by Symfony is more limited. Plus that way we don't need to wait until Symfony does a new minor release to re-implement missing features.

@deleugpn
Copy link
Contributor

deleugpn commented Jan 31, 2022

This implementation works correctly and the ConfigurationSet is properly configured. However, when adding custom headers to the Email, they are being lost somewhere along the execution stack. I will not investigate this further because of the preference of the other PR, so I will just jump to testing that one instead. I can dig deeper into the headers being lost if there is desire to merge this one.

Tags functionality is also lost here.

Here is how I'm adding custom headers

$closure = function(Email $message) use ($tenant) {
    $message->getHeaders()->addTextHeader('X-ORG-TENANT', $tenant);
};

@driesvints driesvints closed this Feb 1, 2022
@driesvints driesvints deleted the extend-ses-transport branch February 1, 2022 15:49
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.

3 participants