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

Attachments: Support file data in attachment struct + mailgun attachment support #292

Merged
merged 3 commits into from
Jul 13, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/bamboo/adapters/mandrill_adapter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,11 @@ defmodule Bamboo.MandrillAdapter do
defp attachments(%{attachments: attachments}) do
attachments
|> Enum.reverse
|> Enum.map(fn(att) ->
|> Enum.map(fn(attachment) ->
%{
name: att.filename,
type: att.content_type,
content: Base.encode64(File.read!(att.path))
name: attachment.filename,
type: attachment.content_type,
content: Base.encode64(attachment.data)
}
end)
end
Expand Down
3 changes: 3 additions & 0 deletions lib/bamboo/adapters/test_adapter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,7 @@ defmodule Bamboo.TestAdapter do
def clean_assigns(email) do
%{email | assigns: :assigns_removed_for_testing}
end

@doc false
def supports_attachments?, do: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch!

end
13 changes: 7 additions & 6 deletions lib/bamboo/attachment.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@ defmodule Bamboo.Attachment do
@moduledoc """
"""

defstruct filename: nil, content_type: nil, path: nil
defstruct filename: nil, content_type: nil, path: nil, data: nil

@doc ~S"""
Creates a new Attachment

Examples:
Attachment.new("/path/to/attachment.png")
Attachment.new("/path/to/attachment.png", filename: "image.png")
Attachment.new("/path/to/attachment.png", filename: "image.png", content_type: "image/png")
Attachment.new(params["file"]) # Where params["file"] is a %Plug.Upload
Bamboo.Attachment.new("/path/to/attachment.png")
Bamboo.Attachment.new("/path/to/attachment.png", filename: "image.png")
Bamboo.Attachment.new("/path/to/attachment.png", filename: "image.png", content_type: "image/png")
Bamboo.Attachment.new(params["file"]) # Where params["file"] is a %Plug.Upload
"""
def new(path, opts \\ [])
if Code.ensure_loaded?(Plug) do
Expand All @@ -21,7 +21,8 @@ defmodule Bamboo.Attachment do
def new(path, opts) do
filename = opts[:filename] || Path.basename(path)
content_type = opts[:content_type] || determine_content_type(path)
%__MODULE__{path: path, filename: filename, content_type: content_type}
data = File.read!(path)
%__MODULE__{path: path, data: data, filename: filename, content_type: content_type}
end

defp determine_content_type(path) do
Expand Down
24 changes: 22 additions & 2 deletions lib/bamboo/email.ex
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ defmodule Bamboo.Email do
assigns: %{},
private: %{}

alias Bamboo.Email
alias Bamboo.{Email, Attachment}

@address_functions ~w(from to cc bcc)a
@attribute_pipe_functions ~w(subject text_body html_body)a
Expand Down Expand Up @@ -203,7 +203,27 @@ defmodule Bamboo.Email do
end

@doc ~S"""
Adds an attachment to the email
Adds an data attachment to the email

## Example
put_attachment(email, %Bamboo.Attachment{})

Requires the fields filename and data of the %Bamboo.Attachment{} struct to be set.

## Example
def create(conn, params) do
#...
email
|> put_attachment(%Bamboo.Attachment{filname: "event.ics", data: "BEGIN:VCALENDAR..."})
#...
end
"""
def put_attachment(%__MODULE__{attachments: attachments} = email, %Attachment{filename: _filename, data: _data} = attachment) do
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you can shorten the pattern match of the attach meant to %Attachment{} since you wouldn't be able to initialize that struct without those fields

Copy link
Contributor Author

@gitviola gitviola Jul 13, 2017

Choose a reason for hiding this comment

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

You actually can. Then the values are set to nil because we don't specify anything as enforced keys:

iex(1)> %Bamboo.Attachment{}
%Bamboo.Attachment{content_type: nil, data: nil, filename: nil, path: nil}

But you're right - since we don't check the actual value it doesn't make sense for that check. But it would only make sense to put pass an Attachment struct with both keys (filename and data) specified in there. Do you think we should throw an error if one of it is empty?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we have 2 functions. One for the happy path and another for attachments that are missing data and filename

def put_attachment(_email, %Attachment{filename: nil, data: nil} = attachment) do
  raise "You must pass a valid attachment, instead got: #{inspect attachment}"
end

def put_attachment(%__MODULE__{attachments: attachments} = email, %Attachment{} = attachment) do
  # happy path!
end

I think this would cover the common error case. We can add different error cases later if people run into things often. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@paulcsmith I've made the error messages more specific since it would still jump into the happy path if one of the params is nil. What do you think?

%{email | attachments: [attachment | attachments]}
end

@doc ~S"""
Adds an file attachment to the email

## Example
put_attachment(email, path, opts \\ [])
Expand Down
6 changes: 3 additions & 3 deletions test/lib/bamboo/attachments_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ defmodule Bamboo.AttachmentTest do
attachment = Attachment.new(path)
assert attachment.content_type == "application/vnd.openxmlformats-officedocument.wordprocessingml.document"
assert attachment.filename == "attachment.docx"
assert attachment.path == path
assert attachment.data
end

test "create an attachment with an unknown content type" do
Expand Down Expand Up @@ -37,7 +37,7 @@ defmodule Bamboo.AttachmentTest do
attachment = Attachment.new(upload)
assert attachment.content_type == "application/msword"
assert attachment.filename == "test.docx"
assert attachment.path == path
assert attachment.data
end

test "create an attachment from a Plug Upload struct with overrides" do
Expand All @@ -48,6 +48,6 @@ defmodule Bamboo.AttachmentTest do
attachment = Attachment.new(upload, filename: "my-attachment.doc", content_type: "application/other")
assert attachment.content_type == "application/other"
assert attachment.filename == "my-attachment.doc"
assert attachment.path == path
assert attachment.data
end
end
9 changes: 8 additions & 1 deletion test/lib/bamboo/email_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,14 @@ defmodule Bamboo.EmailTest do
assert email.private["foo"] == "bar"
end

test "put_attachment/3 atts an attachment to the attachments list" do
test "put_attachment/2 adds an attachment to the attachments list" do
attachment = %Bamboo.Attachment{filename: "attachment.docx", data: "content"}
email = new_email() |> put_attachment(attachment)

assert [%Bamboo.Attachment{filename: "attachment.docx"}] = email.attachments
end

test "put_attachment/3 adds an attachment to the attachments list" do
path = Path.join(__DIR__, "../../support/attachment.docx")
email = new_email() |> put_attachment(path)

Expand Down