Make Mandrill MessageId available to Laravel + sub accounts/headers bugfix #39
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR is just for visibility. Will PR upstream once complete.
Make the Mandrill message `id` accessible from the Laravel via the sent event. Message id is now available via
* the `sentMessage` messageId property. `e.g. $event->sent->getMessageId()`
* the `X-Message-ID` header `$event->message->getHeaders()->get('X-Message-ID')`
The `X-Message-ID` convention is taken from Laravel maintained mail drivers including the base SES driver.
Additionally fixes headers not being sent as part of the Mail payload, causing `X-MC-Subaccount` and similar headers not to work as expected.
In theory this should resolve luisdalmolin#30 and luisdalmolin#31
Unit tests for the message _id retrieval and header submission have been added. This did require a minor change to the Transport in order to facilitate mocking of the underlying Guzzle instance.
luisdalmolin
approved these changes
Apr 6, 2023
|
I'll tag this as a beta release for now, and once we get a few confirmations it's stable, I can tag a stable release. |
|
Thank you! |
|
Cheers. I hope its been working well for people so far. |
|
That would be awesome, actually! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi @luisdalmolin, hope this PR all looks okay. Please let me know if there is anything you would like tweaking.
Key changes involved
Mandrill Message Id is now available via the event at
sentMessagemessageId property.e.g. $event->sent->getMessageId()X-Message-IDheader$event->message->getHeaders()->get('X-Message-ID')The
X-Message-IDconvention is taken from Laravel maintained mail drivers including the base SES driver.The Unit tests work via injecting a mocked Guzzle instance into MailChimp. Tests then just double check the event gets back what it expects from the mandrill API and that all headers are submitted as expected.
While checking headers i also noticed these did not appear to be being correctly passed to the Mandrill API (most notably
X-MC-Subaccount). I had to rework the logic a little to ensure these are set on the Message before the logic living in the abstract classes send method, as after this point the headers are updated on a copy of the data, not the actual data that will be sent via the API calls.With luck this should resolve #30 and #31