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

feat: Enable inline attachments for mailgun. #551

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

astjohn
Copy link

@astjohn astjohn commented Sep 19, 2020

This PR enables inline attachments for mailgun.

@astjohn
Copy link
Author

astjohn commented Sep 19, 2020

At the time of writing, mailgun sets the content-id of the attachment to the supplied filename. I have an open ticket with them to see if there is a way to override this. If not, it's not 100% ideal, but it is a working solution.

@astjohn
Copy link
Author

astjohn commented Sep 26, 2020

Official stance from mailgun is that they don't currently support specifying content-id. Although not ideal, this remains a working solution until the day they decide to support something like that.

@astjohn astjohn changed the title WIP: Enable inline attachments for mailgun. Enable inline attachments for mailgun. Sep 26, 2020
@germsvel
Copy link
Collaborator

germsvel commented Oct 9, 2020

Thanks for opening this PR @astjohn.

Do you have any links to Mailgun documentation that I could reference to help me review this PR?

@astjohn
Copy link
Author

astjohn commented May 26, 2021

It's been a while. Updating our own branch and coming back around to this. Here's the mailgun docs:
https://documentation.mailgun.com/en/latest/api_reference.html#api-reference
https://documentation.mailgun.com/en/latest/api-sending.html#sending

If you'd like to verify what I went through with Mailgun, feel free to email them. As far as I know, their stance hasn't changed on the matter.

@germsvel
Copy link
Collaborator

@astjohn, thanks so much for looping back and rebasing the PR to be up to date!

Would you mind adding a test for the MailgunAdapter so the changes are tested?

I think once we have that, this is good to go. 👍

@doomspork
Copy link
Member

Hi @astjohn 👋 Now that BEAM is managing Bamboo I'm trying to work through all open PRs. Can you please let us know the status of this? Can you please add the test coverage requested by @germsvel?

If this isn't a change you're interested in continuing to move forward we can close the PR 👍

@doomspork doomspork changed the base branch from master to main September 30, 2024 16:57
@doomspork doomspork requested a review from a team as a code owner September 30, 2024 16:57
@doomspork doomspork changed the title Enable inline attachments for mailgun. feat: Enable inline attachments for mailgun. Sep 30, 2024
Copy link
Contributor

This pull request has been automatically marked as "stale:discard". If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated!.

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.

3 participants