From f810fac2120a2766fe563484ccfadf7ae4165fd7 Mon Sep 17 00:00:00 2001
From: Weston Ganger <westonganger@gmail.com>
Date: Wed, 13 Jun 2018 11:46:51 -0700
Subject: [PATCH] remove html_strip method, add legitimate tests, cleanup

---
 CHANGELOG.md                                  |  2 +-
 README.md                                     | 22 +++-----
 lib/prawn-rails/document.rb                   | 21 ++------
 lib/prawn-rails/extension.rb                  | 48 -----------------
 lib/prawn-rails/load_prawn_plugins.rb         | 11 ++++
 lib/prawn-rails/rails_helper.rb               | 18 ++++---
 lib/prawn-rails/renderer.rb                   |  3 ++
 .../app/controllers/reports_controller.rb     | 13 ++++-
 .../app/helpers/application_helper.rb         |  2 -
 test/dummy_app/app/helpers/pdf_helper.rb      |  2 -
 test/dummy_app/app/helpers/products_helper.rb |  2 -
 .../app/views/reports/sample.pdf.prawn        | 22 ++++----
 .../app/views/reports/table.pdf.prawn         |  7 +++
 test/dummy_app/config/application.rb          |  7 +++
 .../config/initializers/session_store.rb      |  2 +-
 .../config/initializers/wrap_parameters.rb    |  2 +-
 test/dummy_app/config/routes.rb               |  3 +-
 test/dummy_app/db/schema.rb                   |  2 +-
 test/integration/prawn_rails_test.rb          | 53 ++++++++++---------
 test/prawn_rails_test.rb                      |  8 +--
 20 files changed, 110 insertions(+), 140 deletions(-)
 delete mode 100644 lib/prawn-rails/extension.rb
 create mode 100644 lib/prawn-rails/load_prawn_plugins.rb
 delete mode 100644 test/dummy_app/app/helpers/application_helper.rb
 delete mode 100644 test/dummy_app/app/helpers/pdf_helper.rb
 delete mode 100644 test/dummy_app/app/helpers/products_helper.rb
 create mode 100644 test/dummy_app/app/views/reports/table.pdf.prawn

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9315ee2..ef72853 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,8 +1,8 @@
 # Changelog
 
 * `v1.1.0` - UNRELEASED
-  - Breaking Change - Remove `html_strip` helper method. Use `ActionView::Base.full_sanitizer.sanitize(html_string)` if you were using this feature.
   - [#29](https://github.com/cortiz/prawn-rails/pull/29) - Require all installed prawn extensions
+  - Removed unnecessary `html_strip` helper method. If you were using this feature, instead use Rails built in `strip_tags(html_str)` or `ActionView::Base.full_sanitizer.sanitize(html_str)`
   - Add legitimate tests
 
 * `v1.0.1`
diff --git a/README.md b/README.md
index ee2cfee..d029b4f 100644
--- a/README.md
+++ b/README.md
@@ -1,14 +1,14 @@
 # Prawn-Rails [![Gem Version](https://badge.fury.io/rb/prawn-rails.svg)](http://badge.fury.io/rb/prawn-rails)
 <a href='http://ko-fi.com/A552JBK' target='_blank'><img height='32' style='border:0px;height:32px;' src='https://az743702.vo.msecnd.net/cdn/kofi1.png?v=a' border='0' alt='Buy Me a Coffee' /></a> 
 
-## Install
+# Install
 ```ruby
 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
+# Usage
 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:
@@ -29,11 +29,7 @@ def show
 end
 ```
 
-## Built-in Helpers
-* *html_strip(html)*:
-Removes the html tags from a string
-
-## Default configuration
+# Default configuration
 
 Add a `prawn-rails.rb` config to your Rails app under `config/initializers` like this
 
@@ -59,7 +55,7 @@ pdf.start_new_page
 pdf.start_new_page size: "A4", page_layout: :portrait
 ```
 
-## Examples
+# Examples
 
 #### Hello World
 
@@ -92,12 +88,10 @@ prawn_document(page_layout: :landscape) do |pdf|
 end
 ```
 
-## Credits
-
-Created by Carlos Ortiz - @cortiz 
-
-<a href='http://ko-fi.com/A552JBK' target='_blank'><img height='32' style='border:0px;height:32px;' src='https://az743702.vo.msecnd.net/cdn/kofi1.png?v=a' border='0' alt='Buy Me a Coffee' /></a> 
+# Credits
 
 Maintained by Weston Ganger - @westonganger
 
-Special thanks to @rwilliams, @sigmike, @smber1, @iffyuva
+Created by Carlos Ortiz - @cortiz
+
+<a href='http://ko-fi.com/A552JBK' target='_blank'><img height='32' style='border:0px;height:32px;' src='https://az743702.vo.msecnd.net/cdn/kofi1.png?v=a' border='0' alt='Buy Me a Coffee' /></a> 
diff --git a/lib/prawn-rails/document.rb b/lib/prawn-rails/document.rb
index 9d0dd95..018ac43 100644
--- a/lib/prawn-rails/document.rb
+++ b/lib/prawn-rails/document.rb
@@ -1,21 +1,10 @@
 require 'prawn'
-require 'prawn-rails/extension'
-
-Gem.loaded_specs.keys.select{|spec_name| spec_name.starts_with?('prawn-')}.each do |gem_name|
-  next if gem_name == 'prawn-rails' # Prevent circular loading
-
-  file = gem_name.gsub('-', '/')
-
-  begin
-    require file
-  rescue LoadError => _
-    STDERR.puts "#{__FILE__}:#{__LINE__}: warning: prawn-rails could not require plugin `#{gem_name}`"
-  end
-end
+require 'prawn-rails/load_prawn_plugins'
 
 module PrawnRails
 
-  # This derives from Prawn::Document in order to override defaults. Note that the Prawn::Document behaviour itself shouldn't be changed.
+  # This derives from Prawn::Document in order to override defaults. 
+  # Note that the Prawn::Document behaviour itself shouldn't be changed.
   class Document < Prawn::Document
     def initialize(opts = {})
       if PrawnRails.config.respond_to?(:to_h)
@@ -26,8 +15,7 @@ def initialize(opts = {})
       super(default)
     end
 
-    # Typically text expects a string. But rails views have this interesting concept that they implicitly call 
-    # `to_s` on all the variables before rendering. So, passing an integer to text fails:
+    # Typically text expects a string. But Rails views have this interesting concept that they implicitly call `to_s` on all the variables before rendering. So, passing an integer to text fails:
     #
     # pdf.text 10       #=> fails because 10 is not a string
     # pdf.text 10.to_s  #=> works
@@ -38,5 +26,4 @@ def text(value, options = {})
     end
   end
 
-  Document.extensions << Extension
 end
diff --git a/lib/prawn-rails/extension.rb b/lib/prawn-rails/extension.rb
deleted file mode 100644
index f702b3a..0000000
--- a/lib/prawn-rails/extension.rb
+++ /dev/null
@@ -1,48 +0,0 @@
-module PrawnRails
-  module Extension
-    ##
-    # Removes all html tags from the string
-    # @param html[String] string to remove the tags
-    # @return [String] the string with out the tags
-    ##
-    def html_strip html
-      return html if html.nil?
-
-      text = html
-              .gsub(/(&nbsp;|\n|\s)+/im, ' ')
-              .squeeze(' ')
-              .strip
-              .gsub(/<([^\s]+)[^>]*(src|href)=\s*(.?)([^>\s]*)\3[^>]*>\4<\/\1>/i, '\4')
-
-      links = []
-      linkregex = /<[^>]*(src|href)=\s*(.?)([^>\s]*)\2[^>]*>\s*/i
-
-      while linkregex.match(text)
-        links << $~[3]
-        text.sub!(linkregex, "[#{links.size}]")
-      end
-                              
-      text = text
-              .gsub(/<(script|style)[^>]*>.*<\/\1>/im, '')
-              .gsub(/<!--.*-->/m, '')
-              .gsub(/<hr(| [^>]*)>/i, "___\n")
-              .gsub(/<li(| [^>]*)>/i, "\n* ")
-              .gsub(/<blockquote(| [^>]*)>/i, '> ')
-              .gsub(/<(br)(| [^>]*)>/i, "\n")
-              .gsub(/<(\/h[\d]+|p)(| [^>]*)>/i, "\n\n")
-              .gsub(/<[^>]*>/, '')
-
-      text = CGI.unescapeHTML(text).lstrip.gsub(/\n[ ]+/, "\n") + "\n"
-
-      links.each_with_index do |link, i|
-        unless links[i].nil?
-          text = text + "\n  [#{i+1}] <#{CGI.unescapeHTML(link)}>"
-        end
-      end
-
-      links = nil
-
-      return text
-    end
-  end
-end
diff --git a/lib/prawn-rails/load_prawn_plugins.rb b/lib/prawn-rails/load_prawn_plugins.rb
new file mode 100644
index 0000000..49648df
--- /dev/null
+++ b/lib/prawn-rails/load_prawn_plugins.rb
@@ -0,0 +1,11 @@
+Gem.loaded_specs.keys.select{|spec_name| spec_name.starts_with?('prawn-')}.each do |gem_name|
+  next if gem_name == 'prawn-rails' # Prevent circular loading
+
+  file = gem_name.gsub('-', '/')
+
+  begin
+    require file
+  rescue LoadError => _
+    STDERR.puts "#{__FILE__}:#{__LINE__}: warning: prawn-rails could not require plugin `#{gem_name}`"
+  end
+end
diff --git a/lib/prawn-rails/rails_helper.rb b/lib/prawn-rails/rails_helper.rb
index d66d398..ad7189d 100644
--- a/lib/prawn-rails/rails_helper.rb
+++ b/lib/prawn-rails/rails_helper.rb
@@ -2,14 +2,19 @@
 
 module PrawnRails
   module RailsHelper
+
     def prawn_document(options={})
-      options.reverse_merge!(page_layout: PrawnRails.config.page_layout,
-                             page_size: PrawnRails.config.page_size,
-                             info: {
-                               Title: @filename.sub(/\.(p|P)(d|D)(f|F)$/, '')
-                             })
+      options.reverse_merge!({
+        page_layout: PrawnRails.config.page_layout,
+        page_size: PrawnRails.config.page_size,
+        info: {
+          Title: @filename.sub(/\.(p|P)(d|D)(f|F)$/, '')
+        }
+      })
 
-      options.reverse_merge!(skip_page_creation: true) if PrawnRails.config.skip_page_creation
+      if PrawnRails.config.skip_page_creation
+        options.reverse_merge!(skip_page_creation: true) 
+      end
 
       pdf = PrawnRails::Document.new(options)
 
@@ -17,5 +22,6 @@ def prawn_document(options={})
 
       pdf.render
     end
+
   end
 end
diff --git a/lib/prawn-rails/renderer.rb b/lib/prawn-rails/renderer.rb
index 30e9a3c..9ef64c4 100644
--- a/lib/prawn-rails/renderer.rb
+++ b/lib/prawn-rails/renderer.rb
@@ -2,6 +2,8 @@
 
 module PrawnRails
   class Renderer
+
+    ### WARNING: BE VERY CAREFUL IF EDITING THIS METHOD
     def self.call(template)
       %{
         @filename ||= "\#{controller.action_name}.pdf"
@@ -13,5 +15,6 @@ def self.call(template)
         #{template.source.strip}
       }
     end
+
   end
 end
diff --git a/test/dummy_app/app/controllers/reports_controller.rb b/test/dummy_app/app/controllers/reports_controller.rb
index ac9582f..8e4a8ee 100644
--- a/test/dummy_app/app/controllers/reports_controller.rb
+++ b/test/dummy_app/app/controllers/reports_controller.rb
@@ -1,7 +1,18 @@
 class ReportsController < ApplicationController
   
   def sample
-    @items = [{:name=> "Hello"},{:name=> "World"}]
+    @items = [
+      {name: "Hello"},
+      {name: "World"},
+    ]
+  end
+
+  def table
+    @items = [
+      [1,2,3],
+      [4,5,6],
+      [7,8,9],
+    ]
   end
  
 end
diff --git a/test/dummy_app/app/helpers/application_helper.rb b/test/dummy_app/app/helpers/application_helper.rb
deleted file mode 100644
index de6be79..0000000
--- a/test/dummy_app/app/helpers/application_helper.rb
+++ /dev/null
@@ -1,2 +0,0 @@
-module ApplicationHelper
-end
diff --git a/test/dummy_app/app/helpers/pdf_helper.rb b/test/dummy_app/app/helpers/pdf_helper.rb
deleted file mode 100644
index 2f0bace..0000000
--- a/test/dummy_app/app/helpers/pdf_helper.rb
+++ /dev/null
@@ -1,2 +0,0 @@
-module PdfHelper
-end
diff --git a/test/dummy_app/app/helpers/products_helper.rb b/test/dummy_app/app/helpers/products_helper.rb
deleted file mode 100644
index ab5c42b..0000000
--- a/test/dummy_app/app/helpers/products_helper.rb
+++ /dev/null
@@ -1,2 +0,0 @@
-module ProductsHelper
-end
diff --git a/test/dummy_app/app/views/reports/sample.pdf.prawn b/test/dummy_app/app/views/reports/sample.pdf.prawn
index d0b54e0..23d1f6d 100644
--- a/test/dummy_app/app/views/reports/sample.pdf.prawn
+++ b/test/dummy_app/app/views/reports/sample.pdf.prawn
@@ -1,13 +1,15 @@
 prawn_document(page_layout: :portrait) do |pdf|
-  pdf.text "Hello There :D"
+  pdf.text "Hello World!"
   pdf.text "now it is here: #{pdf.cursor}"
-  pdf.text "Items are #{@items.inspect}"
+  pdf.text "Number of Items: #{@items.count}"
+
   pdf.font("Courier") do
     pdf.text "Written in Courier because we are inside the block."
   end
 
   pdf.text "with html<bold></bold>"
-  pdf.text pdf.html_strip("without html <bold>LD</bold>")
+  pdf.text strip_tags("without html <bold>LD</bold>")
+
   pdf.text "Let's see which is the current font_size: #{pdf.font_size.inspect}"
   pdf.move_down 10
   pdf.font_size 16
@@ -17,20 +19,22 @@ prawn_document(page_layout: :portrait) do |pdf|
   pdf.move_down 10
   pdf.text "Back to 16 again."
   pdf.move_down 10
-  pdf.text "Single line on 20 using the :size option.", :size => 20
+  pdf.text "Single line on 20 using the :size option.", size: 20
   pdf.move_down 10
   pdf.text "Back to 16 once more."
   pdf.move_down 10
-  pdf.font("Courier", :size => 10) do
+
+  pdf.font("Courier", size: 10) do
     pdf.text "Yeah, using Courier 10 courtesy of the font method."
   end
-  pdf.font("Helvetica", :size => 12) # back to normal
+
+  pdf.font("Helvetica", size: 12) # back to normal
 
   pdf.text "Default color is black"
   pdf.move_down 25
-  pdf.text "Changed to red", :color => "FF0000"
+  pdf.text "Changed to red", color: "FF0000"
   pdf.move_down 25
-  pdf.text "CMYK color", :color => [22, 55, 79, 30]
+  pdf.text "CMYK color", color: [22, 55, 79, 30]
   pdf.move_down 25
-  pdf.text "Also works with <color rgb='ff0000'>inline</color> formatting", :color => "0000FF", :inline_format => true
+  pdf.text "Also works with <color rgb='ff0000'>inline</color> formatting", color: "0000FF", inline_format: true
 end
diff --git a/test/dummy_app/app/views/reports/table.pdf.prawn b/test/dummy_app/app/views/reports/table.pdf.prawn
new file mode 100644
index 0000000..25c8c9d
--- /dev/null
+++ b/test/dummy_app/app/views/reports/table.pdf.prawn
@@ -0,0 +1,7 @@
+prawn_document(page_layout: :portrait) do |pdf|
+
+  pdf.table @items do
+
+  end
+
+end
diff --git a/test/dummy_app/config/application.rb b/test/dummy_app/config/application.rb
index 182a6c5..539e3ae 100644
--- a/test/dummy_app/config/application.rb
+++ b/test/dummy_app/config/application.rb
@@ -43,9 +43,16 @@ class Application < Rails::Application
     # Enable the asset pipeline
     config.assets.enabled = true
 
+    config.assets.quiet = true
+
     # Version of your assets, change this if you want to expire all your assets
     config.assets.version = '1.0'
 
+    config.generators.test_framework = false
+    config.generators.helper = false
+    config.generators.stylesheets = false
+    config.generators.javascripts = false
+
     if ActiveRecord.respond_to?(:gem_version)
       gem_version = ActiveRecord.gem_version
       if gem_version >= Gem::Version.new("5.2")
diff --git a/test/dummy_app/config/initializers/session_store.rb b/test/dummy_app/config/initializers/session_store.rb
index aa2f512..952473f 100644
--- a/test/dummy_app/config/initializers/session_store.rb
+++ b/test/dummy_app/config/initializers/session_store.rb
@@ -1,6 +1,6 @@
 # Be sure to restart your server when you modify this file.
 
-Dummy::Application.config.session_store :cookie_store, :key => '_dummy_session'
+Dummy::Application.config.session_store :cookie_store, key: '_dummy_session'
 
 # Use the database for sessions instead of the cookie-based default,
 # which shouldn't be used to store highly confidential information
diff --git a/test/dummy_app/config/initializers/wrap_parameters.rb b/test/dummy_app/config/initializers/wrap_parameters.rb
index da4fb07..999df20 100644
--- a/test/dummy_app/config/initializers/wrap_parameters.rb
+++ b/test/dummy_app/config/initializers/wrap_parameters.rb
@@ -5,7 +5,7 @@
 
 # Enable parameter wrapping for JSON. You can disable this by setting :format to an empty array.
 ActiveSupport.on_load(:action_controller) do
-  wrap_parameters :format => [:json]
+  wrap_parameters format: [:json]
 end
 
 # Disable root element in JSON by default.
diff --git a/test/dummy_app/config/routes.rb b/test/dummy_app/config/routes.rb
index 21bbb62..7f6c37f 100644
--- a/test/dummy_app/config/routes.rb
+++ b/test/dummy_app/config/routes.rb
@@ -1,5 +1,4 @@
 Dummy::Application.routes.draw do
-  resources :products
-
   get "reports/sample"
+  get "reports/table"
 end
diff --git a/test/dummy_app/db/schema.rb b/test/dummy_app/db/schema.rb
index 7a0d57a..2298094 100644
--- a/test/dummy_app/db/schema.rb
+++ b/test/dummy_app/db/schema.rb
@@ -10,6 +10,6 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20120221152352) do
+ActiveRecord::Schema.define(version: 20120221152352) do
 
 end
diff --git a/test/integration/prawn_rails_test.rb b/test/integration/prawn_rails_test.rb
index 02315f4..4412180 100644
--- a/test/integration/prawn_rails_test.rb
+++ b/test/integration/prawn_rails_test.rb
@@ -14,50 +14,51 @@ class NavigationTest < ActionDispatch::IntegrationTest
     assert ActionView::Template::Handlers.extensions.include?(:prawn)
   end
 
-  test "render html action" do
+  test "Renders html action" do
     get '/reports/sample'
     assert_response :success
-    assert @response.body.include?("<h1>Hello World!</h1>")
+    #assert @response.body.include?("<h1>Hello World!</h1>")
+    assert_match("<h1>Hello World!</h1>", @response.body)
   end
 
-  test "render pdf action" do
+  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>")
+  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>")
-  end
-
-  test "Missing pdf format" do
-    assert true
-  end
 
-  test "strip_tags" do
-    assert true
-  end
+    reader = PDF::Reader.new(StringIO.new(@response.body))
+    assert_not_nil(reader)
 
-  test "ActionView::Base.full_sanitizer.sanitize(html_string)" do
-    assert true
-  end
+    assert_equal(1, reader.page_count)
 
-  test "" do
-    assert true
-  end
+    page_str = reader.pages[0].to_s
 
-  test "legacy test - render pdf to string" do
-    ApplicationController.new.render_to_string("reports/sample.pdf", locals: {'@items' => []})
+    assert page_str.include?('Hello World!')
+    assert_not page_str.include?("<h1>Hello World!</h1>")
   end
 
-  test "legacy test - render pdf" do
-    get '/reports/sample', params: {format: :pdf}
+  test "Renders table pdf action using auto-required plugin Prawn-Table" do
+    get '/reports/table', params: {format: :pdf}
     assert_response :success
 
     reader = PDF::Reader.new(StringIO.new(@response.body))
     assert_not_nil(reader)
 
     assert_equal(1, reader.page_count)
-    assert_match(/Hello There :D/, reader.pages[0].to_s)
-    assert_no_match(/This should not be any where/, reader.pages[0].to_s)
-    assert_no_match(/ITEMS ARE nil/, reader.pages[0].to_s)
-    assert_match(/with html<bold><\/bold>/,reader.pages[0].to_s)
-    assert_match(/without html LD/,reader.pages[0].to_s)
+
+    page_str = reader.pages[0].to_s
+
+    assert_equal(page_str, "1 2 3\n\n4 5 6\n\n7 8 9")
   end
 end
diff --git a/test/prawn_rails_test.rb b/test/prawn_rails_test.rb
index 584c2aa..ea66fc5 100644
--- a/test/prawn_rails_test.rb
+++ b/test/prawn_rails_test.rb
@@ -4,14 +4,8 @@
 
 class PrawnRailsTest < ActiveSupport::TestCase
 
-  # TODO remove this test
-  include PrawnRails::Extension
-  test "html_strip" do
-    assert_nil html_strip(nil)
-    assert_not_nil html_strip("")
-  end
-
   test "config" do
     assert true
   end
+
 end