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

Adding content_id for displaying attachements in HTML email #475

Closed
wants to merge 2 commits into from

Conversation

ttyerl
Copy link
Contributor

@ttyerl ttyerl commented Apr 18, 2019

Adding content_id for displaying attachements in HTML email. The corresponding changes to bamboo_smtp have been made and a PR created for it.

@ttyerl
Copy link
Contributor Author

ttyerl commented Apr 18, 2019

It says "mix format --check-formatted" failed but gives me no indication of why it fails.

@maymillerricci
Copy link
Contributor

@ttyerl Thanks for this!

If you run mix format locally, it should fix whatever formatting issues there are.

Also, is there a test we can add for this new behavior?

@ttyerl
Copy link
Contributor Author

ttyerl commented Apr 27, 2019

I ran this locally and everything passed. Not too sure how to fix this failing test. Hints appreciated.

Copy link
Contributor

@maymillerricci maymillerricci left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for this! I reran the test suite on CI and it passed - the failure must have been a fluke. I just have a few small comments about fixing up the documentation for this.

}

@doc ~S"""
Creates a new Attachment

context_id can be use to embed an image, attach it and reference in the message body by
Copy link
Contributor

Choose a reason for hiding this comment

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

context_id -> content_id
can be use -> can be used
and reference -> and reference it


<img src="cid:some-image-cid" alt="img" />

within a HTML email message.
Copy link
Contributor

Choose a reason for hiding this comment

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

within a -> within an

context_id can be use to embed an image, attach it and reference in the message body by
setting its CID (Content-ID) and using a standard HTML tag:

<img src="cid:some-image-cid" alt="img" />
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs 2 more spaces of indentation in order to render correctly as code on the docs.


email
|> put_html_layout({LayoutView, "email.html"})
|> put_attachment(%Bamboo.Attachment{content_type: "image/png", filename: "logo.png", data: get_content(), content_id: "2343333333"})
Copy link
Contributor

Choose a reason for hiding this comment

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

The code formatter would line the pipes up directly with email - so all the lines are directly in line. Also, what do you think about switching the data passed in in the example to some arbitrary data instead of referencing a get_content() function, to be as understandable as possible?

@maymillerricci
Copy link
Contributor

I made these changes and merged in 228827b.

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