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 inline images in mandrill adapter #608

Merged

Conversation

agutierrezrodriguez
Copy link
Contributor

This PR adds support to send to mandrill inline images.

Copy link
Collaborator

@germsvel germsvel left a comment

Choose a reason for hiding this comment

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

Thank you for opening this pull request!

I took a quick glance and left a comment on the changes. So that I can better review the pull request, do you have any links to Mandrill documentation on inline attachments?


mandrill_message
|> Map.put(:attachments, format_attachments(files))
|> Map.put(:images, format_attachments(images))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you know why we're calling this images? Will inline attachments always be images or is that how mandrill categorizes inline attachments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, of course! https://mailchimp.com/developer/transactional/api/messages/send-new-message/
Inside message attribute at the end.

Is similar to attachments with the difference of send the cid instead of the filename.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for sending that link. That's very helpful! This is the section I'm looking at:

Screen Shot 2021-06-11 at 8 56 45 AM

Based on what I'm seeing, the changes in this PR seem good.

My only lingering question is: do you know if it's possible to add non-image files as inline attachments?

  • If that's the case, then someone could add a non-image attachment with content-id but we would incorrectly put it in the :images field.
  • If Mandrill only allows for images as inline attachments, then I think this PR is good to go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a new commit to check the content_type too before add it as inline image.

Copy link
Collaborator

@germsvel germsvel left a comment

Choose a reason for hiding this comment

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

Thanks so much for all this work! Looks good to me.

@germsvel germsvel merged commit b82c988 into beam-community:master Jun 25, 2021
@agutierrezrodriguez
Copy link
Contributor Author

Thanks a lot for you @germsvel!! 🍻

@agutierrezrodriguez agutierrezrodriguez deleted the add-inline-images-to-mandrill branch July 19, 2021 08:17
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.

2 participants