Attachment Support (#26) #36

wants to merge 15 commits into


None yet
4 participants

davidcornu commented May 10, 2012

Write attachments to an attachments directory alongside plain.html and rich.html


  • Modified delivery_method.rb to use the text_part and html_part methods provided by mail to avoid rendering attachments as the plain version
  • Modified message.rb to write out the attachments and added an @attachments variable to simplify the template
  • Added links to the attachments in message.html.erb and changed styling accordingly
  • Modified delivery_method_spec.rb to test added functionality

@cj cj added a commit to cj/letter_opener that referenced this pull request May 15, 2012

@cj cj Merge pull request #36 from davidcornu/master
Attachment Support (#26)

* davidcornu/master:
  Don't recreate files if already present
  Added test for attachments
  mail.attachments returns an array which evaluates to true, changed to a size check
  Forgot a pixel
  Always show attachment section
  only show rich/plain links if the parts exist
  Use mail's text_part and html_part methods to avoid rendering attachments as plain text
  Turns out there was a good reason for that
  Remove height constraint from headers and add more padding so more attachments can fit
  Better styling
  Require 'uri' for escaping filenames
  Tentative attachment support

davidcornu commented May 17, 2012

@cj are the changes working out for you?

I successively used to test attachments. Very nice.


Is there any technical reason this hasn't been merged?

Ditto: I successively used to test attachments. Very nice.

Suggest merging this.

ryanb referenced this pull request Oct 1, 2012


Attachments? #26


nashby commented Oct 2, 2012

@davidcornu hey, thanks for the pull request. Could you rebase it against master? Thanks!


davidcornu commented Oct 2, 2012

@nashby Will do


davidcornu commented Oct 7, 2012

@nashby, I've rebased davidcornu/letter_opener#master against ryanb/letter_opener#master and pushed it to davidcornu/letter_opener#rebased as pushing to my own master would cause all sorts of havoc.

Let me know if you'd like me to open another pull request with that branch.


nashby commented Oct 8, 2012

@davidcornu yeah, please open a new PR. Thanks.


davidcornu commented Oct 8, 2012

Closing this pull request.

davidcornu closed this Oct 8, 2012

@nashby nashby added a commit that referenced this pull request Oct 9, 2012

@nashby nashby Merge pull request #48 from davidcornu/rebased
Attachment Support - Rebased version of pull request #36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment