Merge pull request #5136 from Meet-Democracy/Legislation-draft-render-markdown-tables

Render markdown tables in legislation draft

app/assets/stylesheets/legislation_process.scss

@@ -459,6 +459,10 @@
         padding: rem-calc(32) rem-calc(32) rem-calc(32) rem-calc(48);
       }
 
+      table {
+        @include table-scroll;
+      }
+
       h2 {
         font-weight: 400;
         margin-bottom: rem-calc(32);

app/helpers/application_helper.rb

@@ -10,26 +10,8 @@ module ApplicationHelper
     %i[ar fa he].include?(locale)
   end
 
-  def markdown(text)
-    return text if text.blank?
-
-    # See https://github.com/vmg/redcarpet for options
-    render_options = {
-      filter_html:     false,
-      hard_wrap:       true,
-      link_attributes: {  target: "_blank" }
-    }
-    renderer = Redcarpet::Render::HTML.new(render_options)
-    extensions = {
-      autolink:           true,
-      fenced_code_blocks: true,
-      lax_spacing:        true,
-      no_intra_emphasis:  true,
-      strikethrough:      true,
-      superscript:        true
-    }
-
-    AdminLegislationSanitizer.new.sanitize(Redcarpet::Markdown.new(renderer, extensions).render(text))
+  def markdown(text, **render_options)
+    MarkdownConverter.new(text, **render_options).render
   end
 
   def wysiwyg(text)

app/models/legislation/draft_version.rb

@@ -22,15 +22,11 @@ class Legislation::DraftVersion < ApplicationRecord
   scope :published, -> { where(status: "published").order("id DESC") }
 
   def body_html
-    renderer = Redcarpet::Render::HTML.new(with_toc_data: true)
-
-    Redcarpet::Markdown.new(renderer).render(body)
+    MarkdownConverter.new(body, with_toc_data: true).render
   end
 
   def toc_html
-    renderer = Redcarpet::Render::HTML_TOC.new(with_toc_data: true)
-
-    Redcarpet::Markdown.new(renderer).render(body)
+    MarkdownConverter.new(body).render_toc
   end
 
   def display_title

lib/admin_legislation_sanitizer.rb

@@ -1,6 +1,6 @@
 class AdminLegislationSanitizer < WYSIWYGSanitizer
   def allowed_tags
-    super + %w[img h1 h4 h5 h6]
+    super + %w[img h1 h4 h5 h6 table thead tbody tr th td]
   end
 
   def allowed_attributes

lib/markdown_converter.rb

@@ -0,0 +1,48 @@
+class MarkdownConverter
+  attr_reader :text, :render_options
+
+  def initialize(text, **render_options)
+    @text = text
+    @render_options = render_options
+  end
+
+  def render
+    return text if text.blank?
+
+    AdminLegislationSanitizer.new.sanitize(Redcarpet::Markdown.new(renderer, extensions).render(text))
+  end
+
+  def render_toc
+    Redcarpet::Markdown.new(toc_renderer).render(text)
+  end
+
+  private
+
+    def renderer
+      Redcarpet::Render::HTML.new(default_render_options.merge(render_options))
+    end
+
+    def toc_renderer
+      Redcarpet::Render::HTML_TOC.new(with_toc_data: true)
+    end
+
+    def default_render_options
+      {
+        filter_html:     false,
+        hard_wrap:       true,
+        link_attributes: { target: "_blank" }
+      }
+    end
+
+    def extensions
+      {
+        autolink:           true,
+        fenced_code_blocks: true,
+        lax_spacing:        true,
+        no_intra_emphasis:  true,
+        strikethrough:      true,
+        superscript:        true,
+        tables:             true
+      }
+    end
+end

spec/factories/legislations.rb

@@ -127,6 +127,15 @@ FactoryBot.define do
     trait :final_version do
       final_version { true }
     end
+
+    trait :with_table do
+      body { <<~BODY_MARKDOWN }
+        | id | name    | age | gender |
+        |----|---------|-----|--------|
+        | 1  | Roberta | 39  | M      |
+        | 2  | Oliver  | 25  | F      |
+        BODY_MARKDOWN
+    end
   end
 
   factory :legislation_annotation, class: "Legislation::Annotation" do

spec/lib/admin_legislation_sanitizer_spec.rb

@@ -0,0 +1,65 @@
+require "rails_helper"
+
+describe AdminLegislationSanitizer do
+  let(:sanitizer) { AdminLegislationSanitizer.new }
+
+  describe "#sanitize" do
+    it "allows images" do
+      html = 'Dangerous<img src="/smile.png" alt="Smile"> image'
+      expect(sanitizer.sanitize(html)).to eq(html)
+    end
+
+    it "allows h1 to h6" do
+      html = '<h1>Heading 1</h1>
+              <h2>Heading 2</h2>
+              <h3>Heading 3</h3>
+              <h4>Heading 4</h4>
+              <h5>Heading 5</h5>
+              <h6>Heading 6</h6>'
+      expect(sanitizer.sanitize(html)).to eq(html)
+    end
+
+    it "allows tables" do
+      html = '<table>
+                <thead>
+                  <tr>
+                    <th>id</th>
+                    <th>name</th>
+                    <th>age</th>
+                    <th>gender</th>
+                  </tr>
+                </thead>
+                <tbody>
+                  <tr>
+                    <td>1</td>
+                    <td>Roberta</td>
+                    <td>39</td>
+                    <td>M</td>
+                  </tr>
+                  <tr>
+                    <td>2</td>
+                    <td>Oliver</td>
+                    <td>25</td>
+                    <td>F</td>
+                  </tr>
+                </tbody>
+              </table>'
+      expect(sanitizer.sanitize(html)).to eq(html)
+    end
+
+    it "allows alt src and id" do
+      html = 'Dangerous<img src="/smile.png" alt="Smile" id="smile"> image'
+      expect(sanitizer.sanitize(html)).to eq(html)
+    end
+
+    it "doesn't allow style" do
+      html = 'Dangerous<img src="/smile.png" alt="Smile" style="width:10px;"> image'
+      expect(sanitizer.sanitize(html)).not_to eq(html)
+    end
+
+    it "doesn't allow class" do
+      html = 'Dangerous<img src="/smile.png" alt="Smile" class="smile"> image'
+      expect(sanitizer.sanitize(html)).not_to eq(html)
+    end
+  end
+end

spec/models/legislation/draft_version_spec.rb

@@ -29,6 +29,15 @@ describe Legislation::DraftVersion do
     expect(legislation_draft_version.toc_html).to eq(toc_html)
   end
 
+  it "renders the tables from the markdown body field" do
+    legislation_draft_version.body = body_with_table_markdown
+
+    legislation_draft_version.save!
+
+    expect(legislation_draft_version.body_html).to eq(body_with_table_html)
+    expect(legislation_draft_version.toc_html).to eq(toc_html)
+  end
+
   def body_markdown
     <<~BODY_MARKDOWN
       # Title 1
@@ -50,6 +59,32 @@ describe Legislation::DraftVersion do
     BODY_MARKDOWN
   end
 
+  def body_with_table_markdown
+    <<~BODY_MARKDOWN
+      # Title 1
+
+      Some paragraph.
+
+      A list:
+
+      - item 1
+      - item 2
+
+      ## Subtitle
+
+      Another paragraph.
+
+      # Title 2
+
+      Something about this.
+
+      | id | name    | age | gender |
+      |----|---------|-----|--------|
+      | 1  | Roberta | 39  | M      |
+      | 2  | Oliver  | 25  | F      |
+    BODY_MARKDOWN
+  end
+
   def body_html
     <<~BODY_HTML
       <h1 id="title-1">Title 1</h1>
@@ -73,6 +108,54 @@ describe Legislation::DraftVersion do
     BODY_HTML
   end
 
+  def body_with_table_html
+    <<~BODY_HTML
+      <h1 id="title-1">Title 1</h1>
+
+      <p>Some paragraph.</p>
+
+      <p>A list:</p>
+
+      <ul>
+      <li>item 1</li>
+      <li>item 2</li>
+      </ul>
+
+      <h2 id="subtitle">Subtitle</h2>
+
+      <p>Another paragraph.</p>
+
+      <h1 id="title-2">Title 2</h1>
+
+      <p>Something about this.</p>
+
+      <table>
+      <thead>
+      <tr>
+      <th>id</th>
+      <th>name</th>
+      <th>age</th>
+      <th>gender</th>
+      </tr>
+      </thead>
+      <tbody>
+      <tr>
+      <td>1</td>
+      <td>Roberta</td>
+      <td>39</td>
+      <td>M</td>
+      </tr>
+      <tr>
+      <td>2</td>
+      <td>Oliver</td>
+      <td>25</td>
+      <td>F</td>
+      </tr>
+      </tbody>
+      </table>
+    BODY_HTML
+  end
+
   def toc_html
     <<~TOC_HTML
       <ul>

spec/system/legislation/draft_versions_spec.rb

@@ -397,4 +397,30 @@ describe "Legislation Draft Versions" do
       expect(page).to have_content "my other annotation"
     end
   end
+
+  context "See table from markdown" do
+    let(:draft_version) { create(:legislation_draft_version, :published, :with_table) }
+    let(:path) do
+      edit_admin_legislation_process_draft_version_path(draft_version.process, draft_version)
+    end
+
+    scenario "See table as a user" do
+      visit legislation_process_draft_version_path(draft_version.process, draft_version)
+
+      expect(page).to have_css("table")
+      expect(page).to have_content "Roberta"
+      expect(page).to have_content "25"
+    end
+
+    scenario "See table as an admin" do
+      login_as(administrator)
+
+      visit path
+      click_link "Launch text editor"
+
+      expect(page).to have_css("table")
+      expect(page).to have_content "Roberta"
+      expect(page).to have_content "25"
+    end
+  end
 end