10000 [Mailer] Fix attachments and embedded images for Mailchimp API by michaelperrin · Pull Request #33996 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Mailer] Fix attachments and embedded images for Mailchimp API #33996

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

Conversation

michaelperrin
Copy link
Contributor
Q A
Branch? 4.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #33995
License MIT

Fix the Mandrill transport API payload for embedded images and attachments. Fixes #33995.

As mentioned in #33994, I will make another PR on Wednesday to add some tests on the payload for the 4.4 branch (tests on transports have been introduced for the 4.4 branch) once this gets merged. If you have any better idea, just let me know!

@OskarStark
Copy link
Contributor
OskarStark commented Oct 16, 2019

Looks like your commit email is not associated with your GitHub account ☝🏻
To get full credits you should add the mail via settings to your account.

Same for your other PR

@nicolas-grekas nicolas-grekas added this to the 4.3 milestone Oct 16, 2019
@michaelperrin michaelperrin force-pushed the mailer/fix-mandrill-api-attachments branch from 46aacba to d8f544d Compare October 16, 2019 13:30
@michaelperrin
Copy link
Contributor Author

Thank you @OskarStark for pointing this out! I was sure I did change my author email address, but seems like I didn't…

I pushed again with the correct information for this PR. For my other PR, that's too late as @fabpot already merged it, but that's ok as I associated the former email address to my Github account (didn't really want to do that but that's fine).

@michaelperrin
Copy link
Contributor Author

Could this make it for next Symfony 4.3 minor version (along with #33994 which has already been merged)? Thanks :)

@michaelperrin
Copy link
Contributor Author

Closing as #35173 got merged and does the same thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0