Convert markdown to HTML on demand

We were converting markdown to HTML every time we saved a record, which
has the same problems as sanitizing HTML before saving it to the
database, particularly because the body of a legislation draft is stored
in a translations table.

Performance-wise this isn't a problem: converting a text with more than
200_000 characters takes about a milisecond on my machine.

Note we need to modify a migration generated by globalize, since the
method `create_translation_table!` would fail now that we don't define
`translates :body_html` in the model.

app/models/legislation/draft_version.rb

@@ -7,8 +7,6 @@ class Legislation::DraftVersion < ApplicationRecord
   translates :title,     touch: true
   translates :changelog, touch: true
   translates :body,      touch: true
-  translates :body_html, touch: true
-  translates :toc_html,  touch: true
   include Globalizable
 
   belongs_to :process, class_name: "Legislation::Process", foreign_key: "legislation_process_id"
@@ -20,23 +18,16 @@ class Legislation::DraftVersion < ApplicationRecord
 
   scope :published, -> { where(status: "published").order("id DESC") }
 
-  before_save :render_html
-
-  def render_html
+  def body_html
     renderer = Redcarpet::Render::HTML.new(with_toc_data: true)
-    toc_renderer = Redcarpet::Render::HTML_TOC.new(with_toc_data: true)
 
-    if body_changed?
-      self.body_html = Redcarpet::Markdown.new(renderer).render(body)
-      self.toc_html = Redcarpet::Markdown.new(toc_renderer).render(body)
-    end
+    Redcarpet::Markdown.new(renderer).render(body)
+  end
 
-    translations.each do |translation|
-      if translation.body_changed?
-        translation.body_html = Redcarpet::Markdown.new(renderer).render(translation.body)
-        translation.toc_html = Redcarpet::Markdown.new(toc_renderer).render(translation.body)
-      end
-    end
+  def toc_html
+    renderer = Redcarpet::Render::HTML_TOC.new(with_toc_data: true)
+
+    Redcarpet::Markdown.new(renderer).render(body)
   end
 
   def display_title

app/views/legislation/draft_versions/show.html.erb

@@ -66,7 +66,7 @@
                    data-legislation-annotatable-base-url="<%= legislation_process_draft_version_path(@process, @draft_version) %>"
                    data-legislation-open-phase="<%= @process.allegations_phase.open? %>">
           <% end %>
-            <%= sanitize(@draft_version.body_html) %>
+            <%= sanitize(@draft_version.body_html, { attributes: ["id"] }) %>
           </section>
         </div>
       </div>

db/migrate/20180801140800_add_collaborative_legislation_translations.rb

@@ -20,12 +20,14 @@ class AddCollaborativeLegislationTranslations < ActiveRecord::Migration[4.2]
       {
         title:     :string,
         changelog: :text,
-        body:      :text,
-        body_html: :text,
-        toc_html:  :text
+        body:      :text
       },
       { migrate_data: true }
     )
+
+    add_column :legislation_draft_version_translations, :body_html, :text
+    add_column :legislation_draft_version_translations, :toc_html, :text
+
     Legislation::QuestionOption.create_translation_table!(
       { value: :string },
       { migrate_data: true }

db/migrate/20191006114603_remove_html_fields_form_legislation_draft_version.rb

@@ -0,0 +1,8 @@
+class RemoveHtmlFieldsFormLegislationDraftVersion < ActiveRecord::Migration[5.0]
+  def change
+    remove_column :legislation_draft_versions, :body_html
+    remove_column :legislation_draft_version_translations, :body_html
+    remove_column :legislation_draft_versions, :toc_html
+    remove_column :legislation_draft_version_translations, :toc_html
+  end
+end

db/schema.rb

@@ -759,8 +759,6 @@ ActiveRecord::Schema.define(version: 20191020153455) do
     t.string   "title"
     t.text     "changelog"
     t.text     "body"
-    t.text     "body_html"
-    t.text     "toc_html"
     t.datetime "hidden_at"
     t.index ["hidden_at"], name: "index_legislation_draft_version_translations_on_hidden_at", using: :btree
     t.index ["legislation_draft_version_id"], name: "index_900e5ba94457606e69e89193db426e8ddff809bc", using: :btree
@@ -777,8 +775,6 @@ ActiveRecord::Schema.define(version: 20191020153455) do
     t.datetime "hidden_at"
     t.datetime "created_at",                               null: false
     t.datetime "updated_at",                               null: false
-    t.text     "body_html"
-    t.text     "toc_html"
     t.index ["hidden_at"], name: "index_legislation_draft_versions_on_hidden_at", using: :btree
     t.index ["legislation_process_id"], name: "index_legislation_draft_versions_on_legislation_process_id", using: :btree
     t.index ["status"], name: "index_legislation_draft_versions_on_status", using: :btree

spec/features/xss_spec.rb

@@ -153,4 +153,13 @@ describe "Cross-Site Scripting protection", :js do
 
     expect(page.text).not_to be_empty
   end
+
+  scenario "legislation version body filters script tags but not header IDs" do
+    version = create(:legislation_draft_version, :published, body: "# Title 1\n#{attack_code}")
+
+    visit legislation_process_draft_version_path(version.process, version)
+
+    expect(page.text).not_to be_empty
+    expect(page).to have_css "h1#title-1", text: "Title 1"
+  end
 end

spec/models/legislation/draft_version_spec.rb

@@ -10,7 +10,7 @@ describe Legislation::DraftVersion do
     expect(legislation_draft_version).to be_valid
   end
 
-  it "renders and saves the html from the markdown body field" do
+  it "renders the html from the markdown body field" do
     legislation_draft_version.body = body_markdown
 
     legislation_draft_version.save!
@@ -19,16 +19,6 @@ describe Legislation::DraftVersion do
     expect(legislation_draft_version.toc_html).to eq(toc_html)
   end
 
-  it "renders and saves the html from the markdown body field with alternative translation" do
-    legislation_draft_version.title_fr = "Français"
-    legislation_draft_version.body_fr = body_markdown
-
-    legislation_draft_version.save!
-
-    expect(legislation_draft_version.body_html_fr).to eq(body_html)
-    expect(legislation_draft_version.toc_html_fr).to eq(toc_html)
-  end
-
   def body_markdown
     <<-BODY_MARKDOWN
 # Title 1