elixir-mail icon indicating copy to clipboard operation
elixir-mail copied to clipboard

Feature/inline attachments

Open jarrodmoldrich opened this issue 4 years ago • 5 comments
trafficstars

Changes proposed in this pull request

For parts with content-dispostion: inline its important to reorganize the multipart arrangement in the adapter. In particular it needs to be arrange liked this:

start multipart/mixed
  start multipart/related
    start multipart/alternative
      <text and html parts>
    end multipart/alternative
    <inline parts>
  end multipart/related
  <attachment parts>
end multipart/mixed

My changes reorganize the parts as shown above, while maintaining backwards compatibility. It also adds matching functions for querying for inline attachments as for regular attachments. This change was required for inline images and .ics files to work properly, particularly in Outlook webmail.

I've stopped short of adding tests to the test suite, and I'll wait for you to validate my approach first. I duplicated this code for kalys/bamboo_ses, where it currently functions well for my use cases: https://github.com/kalys/bamboo_ses/pull/43

Note: This PR also deprecates #133 as the original order of text parts should be maintained

jarrodmoldrich avatar Oct 22 '21 14:10 jarrodmoldrich

Hi there,

First, thanks for this library. I'm trying this branch, and it works ok; I'm testing it with a set of .eml files, and it is parsed correctly.

the only thing that is missing in this PR is the get_attachments function to be able to get both inline and attachments:

it is a little change though:

  def get_attachments(%Mail.Message{} = message) do
    walk_parts([message], {:cont, []}, fn message, acc ->
      case Mail.Message.is_any_attachment?(message) do
        true ->
          ##### THIS
          case Mail.Message.get_header(message, :content_disposition) do
            ["inline", {"filename", filename}] ->
              {:cont, List.insert_at(acc, -1, {filename, message.body})}
            ["attachment", {"filename", filename}] ->
              {:cont, List.insert_at(acc, -1, {filename, message.body})}
          end
          ###### 
        false ->
          {:cont, acc}
      end
    end)
    |> elem(1)
  end

michelson avatar Dec 11 '21 07:12 michelson

@jarrodmoldrich thank you very much apoligies this went stale. I know it's a bit rediculous to ask after years but if you could just add regression tests for the additional behavior added I can merge this in.

bcardarella avatar Feb 04 '24 14:02 bcardarella

+1

Hermanverschooten avatar May 08 '24 16:05 Hermanverschooten