Optionally decode attachment contents

[dceluis]

Hello, I was recently appending an attachment the following way:

attachments["report.xlsx"] = {
  mime_type: Mime[:xlsx],
  encoding: "base64",
  content: Base64.encode64(raw_xlsx)
}

and I realized the gem's default behavior is to write encoded attachments to file. There is a small issue with that though: downloading and trying to open the attachment may fail because the handling application may not be able to read the Base64-encoded file. I thought it would be nice to be able to optionally write the attachment decoded.

Sorry for not creating an issue, I thought the shortest route to start the discussion would be to present the proposed changes. Thank you.

Related: https://github.com/ryanb/letter_opener/issues/49 https://github.com/ryanb/letter_opener/issues/7

[dceluis]

Do you mean only selectively decode some attachments but not others?

[nashby]

@dceluis thanks for PR! One question though: can't we just always use attachment.decoded and fallback to raw content if we can't decode it? (catching this exception https://github.com/mikel/mail/blob/641060598f8f4be14d79bad8d703e9f2967e1cdb/lib/mail/body.rb#L181)

[dceluis]

@nashby Thank you for the suggestion. I think you mean that we should store the decoded attachment by default, without having to set a configuration option?

[nashby]

@dceluis yeah, unless it's a bad idea. Not sure if we might break anything with this change

[dceluis]

@nashby Good question. For an application to depend on the attachment remaining encoded, it would have to be programmatically opening an email and reading/processing its contents. That would defeat the whole purpose of letter_opener though, which is meant to take an email out of the application layer and be inspected by human users.

For users, the difference is that files like attached excel sheets (my use case) can be read without having to manually decode it first.

So I don't think we're breaking any dependency and we can make it the default.

[nashby]

@dceluis hey! I think I'll close it for now unless you want to keep working on it. Given that this issue wasn't mentioned since you opened this PR it's not something a lot of people desire. Though I'd merge it if you can make it work out of the box without any config and it doesn't break existing functionality. Thanks!