Merge pull request #40 from FortisRiders/custom-content-disposition

Support custom 'Content-Disposition' options

CHANGELOG.md

@@ -2,11 +2,15 @@
 
 * `Unreleased`
   - Nothing yet
-
+
+* `v1.3.0 - Unreleased`
+  - [PR #40](https://github.com/cortiz/prawn-rails/pull/40) - Add `:filename` and `:disposition` options for `prawn_document
+  - [PR #40](https://github.com/cortiz/prawn-rails/pull/40) - Do not override existing`Content-Disposition` headers
+
 * `v1.2.1`
   - [PR #39](https://github.com/cortiz/prawn-rails/pull/39) - Fix Rails 6 deprecation warning for single arity template handlers
   - Remove unnecessary option logic from `prawn_document` method
-  
+
 * `v1.2.0`
   - [PR #31](https://github.com/cortiz/prawn-rails/pull/31) - Use Prawn::Document.extensions in favor of custom plug-in loading. This removes all changes made in PR #29 and should behave much more appropriately now.
   - [Undo PR #29](https://github.com/cortiz/prawn-rails/pull/31) - Use Prawn::Document.extensions in favor of custom plug-in loading
@@ -28,5 +32,5 @@
 
 * `v0.1.1`
 * `v0.1.0`
-* `v0.0.2` 
+* `v0.0.2`
 * `v0.0.1`

README.md

@@ -1,7 +1,7 @@
 # Prawn-Rails
 <a href="https://badge.fury.io/rb/prawn-rails" target="_blank"><img height="21" style='border:0px;height:21px;' border='0' src="https://badge.fury.io/rb/prawn-rails.svg" alt="Gem Version"></a>
 <a href='https://rubygems.org/gems/prawn-rails' target='_blank'><img height='21' style='border:0px;height:21px;' src='https://ruby-gem-downloads-badge.herokuapp.com/prawn-rails?label=rubygems&type=total&total_label=downloads&color=brightgreen' border='0' alt='RubyGems Downloads' /></a>
-<a href='https://ko-fi.com/A5071NK' target='_blank'><img height='22' style='border:0px;height:22px;' src='https://az743702.vo.msecnd.net/cdn/kofi1.png?v=a' border='0' alt='Buy Me a Coffee' /></a> 
+<a href='https://ko-fi.com/A5071NK' target='_blank'><img height='22' style='border:0px;height:22px;' src='https://az743702.vo.msecnd.net/cdn/kofi1.png?v=a' border='0' alt='Buy Me a Coffee' /></a>
 
 
 # Install
@@ -12,7 +12,7 @@ gem 'prawn-rails'
 Note: `prawn` and `prawn-table` are dependencies of `prawn-rails` so there is no need to mention it in the projects Gemfile unless you want to use a specific version of either of those libraries.
 
 # Usage
-Create a view with `pdf` as format and `prawn` as handler so filename should look like `example.pdf.prawn`
+Create a view with `pdf` as format and `prawn` as handler so filename should look like `example.pdf.prawn`.
 
 It provides a helper called `prawn_document` which builds a PrawnRails::Document with default options. You can override any options as you please. Example:
 
@@ -22,19 +22,41 @@ prawn_document do |pdf|
 end
 ```
 
-No need to call `pdf.render`, it is called by `prawn_document`
+No need to call `pdf.render`, it is called by `prawn_document`.
 
-If you want to customize the name of the file should a user try to save it, you can specify the filename in your action:
+You can customize the name of the generated PDF and the file's delivery format with the `:filename` and `:disposition` keys:
+
+```ruby
+prawn_document(filename: "my-file.pdf", disposition: "attachment") do |pdf|
+  pdf.text "Direct download incoming!"
+end
+```
+
+You can also override the content disposition using the [`Content-Disposition` HTTP header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition#As_a_response_header_for_the_main_body). If you've already set this, `prawn-rails` will *not* override it. Here's an example of setting the header directly:
+
+```ruby
+def download
+  respond_to do |format|
+    format.pdf do
+      headers["Content-Disposition"] = "attachment; filename=\"charts-#{Time.now.to_i}.pdf\""
+    end
+  end
+end
+```
+
+You can also override the file's name from the controller via `@filename`:
 
 ```ruby
 def show
-  @filename = 'my_report.pdf'
+  @filename = "my_report.pdf"
 end
 ```
 
+If no options are given, the file's name will match to your browser's default and the delivery format will default to `inline`.
+
 # Default configuration
 
-Add a `prawn-rails.rb` config to your Rails app under `config/initializers` like this
+Add a `prawn-rails.rb` config to your Rails app under `config/initializers` like this:
 
 ```ruby
 PrawnRails.config do |config|
@@ -44,11 +66,11 @@ PrawnRails.config do |config|
 end
 ```
 
-Please note that these are the defaults. 
+Please note that these are the defaults.
 
-For a list of all available options: [http://www.rubydoc.info/gems/prawn/Prawn%2FDocument:initialize](http://www.rubydoc.info/gems/prawn/Prawn%2FDocument:initialize). 
+For a list of all available options: [http://www.rubydoc.info/gems/prawn/Prawn%2FDocument:initialize](http://www.rubydoc.info/gems/prawn/Prawn%2FDocument:initialize).
 
-For a list of all metadata the the `:info` option supports, please see [https://github.com/prawnpdf/prawn/blob/master/manual/document_and_page_options/metadata.rb](https://github.com/prawnpdf/prawn/blob/master/manual/document_and_page_options/metadata.rb) 
+For a list of all metadata the the `:info` option supports, please see [https://github.com/prawnpdf/prawn/blob/master/manual/document_and_page_options/metadata.rb](https://github.com/prawnpdf/prawn/blob/master/manual/document_and_page_options/metadata.rb).
 
 If `skip_page_creation` is set to true then you have to create the first page yourself. Example:
 
@@ -62,7 +84,7 @@ pdf.start_new_page size: "A4", page_layout: :portrait
 
 #### Hello World
 
-```ruby	
+```ruby
 # hello.pdf.prawn
 
 prawn_document do |pdf|

lib/prawn-rails/rails_helper.rb

@@ -4,18 +4,35 @@ module PrawnRails
   module RailsHelper
 
     def prawn_document(options={})
-      options.reverse_merge!({
-        info: {
-          Title: @filename.sub(/\.(p|P)(d|D)(f|F)$/, '')
-        }
-      })
+      @filename ||= options[:filename]
+
+      options.reverse_merge!(get_metadata)
 
       pdf = PrawnRails::Document.new(options)
 
+      # Ignore headers when we're not rendering from an ActionController context.
+      if defined?(controller) && controller.respond_to?(:response) && !controller.response.nil?
+        disposition = options[:disposition] || "inline"
+        disposition += "; filename=\"#{@filename}\"" if @filename
+
+        # Don't override 'Content-Disposition' if we've chosen to set it elsewhere.
+        controller.response.headers['Content-Disposition'] ||= disposition
+      end
+
       yield pdf if block_given?
 
       pdf.render
     end
 
+    def get_metadata
+      return {} unless @filename
+
+      {
+        info: {
+          Title: @filename.sub(/\.pdf$/i, '')
+        }
+      }
+    end
+
   end
 end

lib/prawn-rails/renderer.rb

@@ -3,17 +3,8 @@
 module PrawnRails
   class Renderer
 
-    ### WARNING: BE VERY CAREFUL IF EDITING THIS METHOD
     def self.call(template, source = nil)
-      %{
-        @filename ||= "\#{controller.action_name}.pdf"
-
-        if controller.respond_to?(:response) && !controller.response.nil?
-          controller.response.headers['Content-Disposition'] = "inline; filename=\\\"\#{@filename}\\\""
-        end
-
-        #{(source || template.source).strip}
-      }
+      (source || template.source).strip
     end
 
   end

lib/prawn-rails/version.rb

@@ -1,3 +1,3 @@
 module PrawnRails
-  VERSION = "1.2.1"
+  VERSION = "1.3.0"
 end

test/dummy_app/app/controllers/reports_controller.rb

@@ -1,5 +1,5 @@
 class ReportsController < ApplicationController
-  
+
   def sample
     @items = [
       {name: "Hello"},
@@ -14,5 +14,24 @@ def table
       [7,8,9],
     ]
   end
-
+
+  def custom_filename
+  end
+
+  def custom_disposition
+  end
+
+  def custom
+  end
+
+  def ivar_filename
+    @filename = "ivar-filename.pdf"
+    render :custom
+  end
+
+  def custom_headers
+    headers['Content-Disposition'] = "attachment;filename=\"custom-headers.pdf\""
+    render :custom
+  end
+
 end

test/dummy_app/app/views/reports/custom.pdf.prawn

@@ -0,0 +1,3 @@
+prawn_document(filename: "from-options.pdf", disposition: "attachment") do |pdf|
+  pdf.text "Hello World!"
+end

test/dummy_app/app/views/reports/custom_disposition.pdf.prawn

@@ -0,0 +1,3 @@
+prawn_document(disposition: "attachment") do |pdf|
+  pdf.text "Hello World!"
+end

test/dummy_app/app/views/reports/custom_filename.pdf.prawn

@@ -0,0 +1,3 @@
+prawn_document(filename: "from-options.pdf") do |pdf|
+  pdf.text "Hello World!"
+end

test/dummy_app/config/routes.rb

@@ -1,4 +1,9 @@
 Dummy::Application.routes.draw do
+  get "reports/custom"
+  get "reports/custom_disposition"
+  get "reports/custom_filename"
+  get "reports/custom_headers"
+  get "reports/ivar_filename"
   get "reports/sample"
   get "reports/table"
 end

test/integration/prawn_rails_test.rb

@@ -6,6 +6,15 @@
 class NavigationTest < ActionDispatch::IntegrationTest
   fixtures :all
 
+  def confirm_pdf_format(source)
+    reader = PDF::Reader.new(StringIO.new(source))
+    assert_not_nil(reader)
+    assert_equal(1, reader.page_count)
+    page_str = reader.pages[0].to_s
+    assert page_str.include?('Hello World!')
+    assert_not page_str.include?("<h1>Hello World!</h1>")
+  end
+
   test "Registers :pdf mime type" do
     assert Mime::Type.lookup_by_extension(:pdf)
   end
@@ -17,35 +26,20 @@ class NavigationTest < ActionDispatch::IntegrationTest
   test "Renders html action" do
     get '/reports/sample'
     assert_response :success
-    #assert @response.body.include?("<h1>Hello World!</h1>")
     assert_match("<h1>Hello World!</h1>", @response.body)
   end
 
   test "Renders pdf to string" do
     pdf_str = ApplicationController.new.render_to_string("reports/sample.pdf", locals: {:@items => []})
 
-    reader = PDF::Reader.new(StringIO.new(pdf_str))
-    assert_not_nil(reader)
-
-    page_str = reader.pages[0].to_s
-    assert page_str.include?('Hello World!')
-    assert_not page_str.include?("<h1>Hello World!</h1>")
+    confirm_pdf_format(pdf_str)
   end
 
   test "Renders sample pdf action" do
     get '/reports/sample', params: {format: :pdf}
     assert_response :success
-    assert_not @response.body.include?("<h1>Hello World!</h1>")
-
-    reader = PDF::Reader.new(StringIO.new(@response.body))
-    assert_not_nil(reader)
-
-    assert_equal(1, reader.page_count)
 
-    page_str = reader.pages[0].to_s
-
-    assert page_str.include?('Hello World!')
-    assert_not page_str.include?("<h1>Hello World!</h1>")
+    confirm_pdf_format(@response.body)
   end
 
   test "Renders table pdf action using auto-required plugin Prawn-Table" do
@@ -62,17 +56,50 @@ class NavigationTest < ActionDispatch::IntegrationTest
     assert_equal(page_str, "1 2 3\n\n4 5 6\n\n7 8 9")
   end
 
+  test "Sets file name from '@filename' when present" do
+    get '/reports/ivar_filename.pdf'
+
+    disposition_header = @response.headers["Content-Disposition"]
+    assert disposition_header.include?("attachment")
+    assert disposition_header.include?("ivar-filename.pdf")
+  end
+
+  test "Maintains existing 'Content-Disposition' header" do
+    get '/reports/custom_headers.pdf'
+
+    disposition_header = @response.headers["Content-Disposition"]
+    assert disposition_header.include?("attachment")
+    assert disposition_header.include?("custom-headers.pdf")
+  end
+
+  test "Respects the 'filename' option alone" do
+    get '/reports/custom_filename.pdf'
+
+    disposition_header = @response.headers["Content-Disposition"]
+    assert disposition_header.include?("inline")
+    assert disposition_header.include?("from-options.pdf")
+  end
+
+  test "Respects the 'disposition' option alone" do
+    get '/reports/custom_disposition.pdf'
+
+    disposition_header = @response.headers["Content-Disposition"]
+    assert disposition_header.include?("attachment")
+    assert_not disposition_header.include?("filename")
+  end
+
+  test "Respects both options on 'prawn-document' together" do
+    get '/reports/custom.pdf'
+
+    disposition_header = @response.headers["Content-Disposition"]
+    assert disposition_header.include?("attachment")
+    assert disposition_header.include?("from-options.pdf")
+  end
+
   test "render_to_string in mailer" do
     mail = ReportsMailer.send_report
     assert_equal(1, mail.attachments.size)
 
-    reader = PDF::Reader.new(StringIO.new(mail.attachments["report.pdf"].body.raw_source))
-    assert_not_nil(reader)
-    assert_equal(1, reader.page_count)
-
-    page_str = reader.pages[0].to_s
-
-    assert page_str.include?('Hello World!')
-    assert_not page_str.include?("<h1>Hello World!</h1>")
+    confirm_pdf_format(mail.attachments["report.pdf"].body.raw_source)
   end
 end

test/prawn_rails_test.rb

@@ -7,8 +7,6 @@ class PrawnRailsTest < ActiveSupport::TestCase
   include PrawnRails::RailsHelper
 
   test "text can take non strings" do
-    @filename = ""
-
     output = prawn_document do |pdf|
       pdf.text "Number:"
       pdf.text 10
@@ -20,4 +18,11 @@ class PrawnRailsTest < ActiveSupport::TestCase
     page = reader.page(1).to_s
     assert page.include?("Number:\n10")
   end
+
+  test "matches .PDF extension regardless of case" do
+    ["pDf", "pdf", "PDF"].each do |ext|
+      @filename = "test.#{ext}"
+      assert_equal get_metadata[:info][:Title], "test"
+    end
+  end
 end