Use proper markdown rendering for previews

8 files changed, 126 insertions, 10 deletions

diff --git a/spec/features/projects/wiki/markdown_preview_spec.rb b/spec/features/projects/wiki/markdown_preview_spec.rb
index ef15e7e1ff9..0bec5f185d6 100644
--- a/spec/features/projects/wiki/markdown_preview_spec.rb
+++ b/spec/features/projects/wiki/markdown_preview_spec.rb
@@ -9,6 +9,7 @@ describe 'Projects > Wiki > User previews markdown changes', :js do
 [relative link 1](../relative)
 [relative link 2](./relative)
 [relative link 3](./e/f/relative)
+[spaced link](title with spaces)
     HEREDOC
   end
 
@@ -42,6 +43,7 @@ describe 'Projects > Wiki > User previews markdown changes', :js do
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a/b/relative\">relative link 1</a>")
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a/b/c/relative\">relative link 2</a>")
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a/b/c/e/f/relative\">relative link 3</a>")
+        expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/title%20with%20spaces\">spaced link</a>")
       end
     end
 
@@ -64,6 +66,7 @@ describe 'Projects > Wiki > User previews markdown changes', :js do
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a-page/b-page/relative\">relative link 1</a>")
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a-page/b-page/c-page/relative\">relative link 2</a>")
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a-page/b-page/c-page/e/f/relative\">relative link 3</a>")
+        expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/title%20with%20spaces\">spaced link</a>")
       end
     end
 
@@ -86,6 +89,7 @@ describe 'Projects > Wiki > User previews markdown changes', :js do
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a-page/b-page/relative\">relative link 1</a>")
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a-page/b-page/c-page/relative\">relative link 2</a>")
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a-page/b-page/c-page/e/f/relative\">relative link 3</a>")
+        expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/title%20with%20spaces\">spaced link</a>")
       end
     end
   end
@@ -119,6 +123,7 @@ describe 'Projects > Wiki > User previews markdown changes', :js do
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a/b/relative\">relative link 1</a>")
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a/b/c/relative\">relative link 2</a>")
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a/b/c/e/f/relative\">relative link 3</a>")
+        expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/title%20with%20spaces\">spaced link</a>")
       end
     end
 
@@ -136,6 +141,7 @@ describe 'Projects > Wiki > User previews markdown changes', :js do
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a-page/b-page/relative\">relative link 1</a>")
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a-page/b-page/c-page/relative\">relative link 2</a>")
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a-page/b-page/c-page/e/f/relative\">relative link 3</a>")
+        expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/title%20with%20spaces\">spaced link</a>")
       end
     end
 
@@ -153,6 +159,7 @@ describe 'Projects > Wiki > User previews markdown changes', :js do
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a-page/b-page/relative\">relative link 1</a>")
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a-page/b-page/c-page/relative\">relative link 2</a>")
         expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/a-page/b-page/c-page/e/f/relative\">relative link 3</a>")
+        expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/title%20with%20spaces\">spaced link</a>")
       end
     end
   end
diff --git a/spec/features/snippets/show_spec.rb b/spec/features/snippets/show_spec.rb
index 74b693f3eff..f31457db92f 100644
--- a/spec/features/snippets/show_spec.rb
+++ b/spec/features/snippets/show_spec.rb
@@ -68,6 +68,26 @@ describe 'Snippet', :js do
         end
       end
 
+      context 'with cached Redcarpet html' do
+        let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_REDCARPET_VERSION) }
+        let(:file_name) { 'test.md' }
+        let(:content) { "1. one\n  - sublist\n" }
+
+        it 'renders correctly' do
+          expect(page).to have_xpath("//ol//li//ul")
+        end
+      end
+
+      context 'with cached CommonMark html' do
+        let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) }
+        let(:file_name) { 'test.md' }
+        let(:content) { "1. one\n  - sublist\n" }
+
+        it 'renders correctly' do
+          expect(page).not_to have_xpath("//ol//li//ul")
+        end
+      end
+
       context 'switching to the simple viewer' do
         before do
           find('.js-blob-viewer-switch-btn[data-viewer=simple]').click
diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb
index d246beb9888..f76ed4bfda4 100644
--- a/spec/helpers/issuables_helper_spec.rb
+++ b/spec/helpers/issuables_helper_spec.rb
@@ -184,6 +184,7 @@ describe IssuablesHelper do
         issuableRef: "##{issue.iid}",
         markdownPreviewPath: "/#{@project.full_path}/preview_markdown",
         markdownDocsPath: '/help/user/markdown',
+        markdownVersion: 11,
         issuableTemplates: [],
         projectPath: @project.path,
         projectNamespace: @project.namespace.path,
diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb
index 1a720aae55c..d5ed5c59c61 100644
--- a/spec/helpers/markup_helper_spec.rb
+++ b/spec/helpers/markup_helper_spec.rb
@@ -205,7 +205,9 @@ describe MarkupHelper do
     it "uses Wiki pipeline for markdown files" do
       allow(@wiki).to receive(:format).and_return(:markdown)
 
-      expect(helper).to receive(:markdown_unsafe).with('wiki content', pipeline: :wiki, project: project, project_wiki: @wiki, page_slug: "nested/page", issuable_state_filter_enabled: true)
+      expect(helper).to receive(:markdown_unsafe).with('wiki content',
+        pipeline: :wiki, project: project, project_wiki: @wiki, page_slug: "nested/page",
+        issuable_state_filter_enabled: true, markdown_engine: :redcarpet)
 
       helper.render_wiki_content(@wiki)
     end
@@ -236,19 +238,32 @@ describe MarkupHelper do
       expect(helper.markup('foo.rst', content).encoding.name).to eq('UTF-8')
     end
 
-    it "delegates to #markdown_unsafe when file name corresponds to Markdown" do
+    it 'delegates to #markdown_unsafe when file name corresponds to Markdown' do
       expect(helper).to receive(:gitlab_markdown?).with('foo.md').and_return(true)
       expect(helper).to receive(:markdown_unsafe).and_return('NOEL')
 
       expect(helper.markup('foo.md', content)).to eq('NOEL')
     end
 
-    it "delegates to #asciidoc_unsafe when file name corresponds to AsciiDoc" do
+    it 'delegates to #asciidoc_unsafe when file name corresponds to AsciiDoc' do
       expect(helper).to receive(:asciidoc?).with('foo.adoc').and_return(true)
       expect(helper).to receive(:asciidoc_unsafe).and_return('NOEL')
 
       expect(helper.markup('foo.adoc', content)).to eq('NOEL')
     end
+
+    it 'uses passed in rendered content' do
+      expect(helper).not_to receive(:gitlab_markdown?)
+      expect(helper).not_to receive(:markdown_unsafe)
+
+      expect(helper.markup('foo.md', content, rendered: '<p>NOEL</p>')).to eq('<p>NOEL</p>')
+    end
+
+    it 'defaults to Redcarpet' do
+      expect(helper).to receive(:markdown_unsafe).with(content, hash_including(markdown_engine: :redcarpet)).and_return('NOEL')
+
+      expect(helper.markup('foo.md', content)).to eq('NOEL')
+    end
   end
 
   describe '#first_line_in_markdown' do
diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js
index 1b040c924aa..be2a8ba67fe 100644
--- a/spec/javascripts/notes/mock_data.js
+++ b/spec/javascripts/notes/mock_data.js
@@ -165,6 +165,7 @@ export const note = {
   report_abuse_path:
     '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_546&user_id=1',
   path: '/gitlab-org/gitlab-ce/notes/546',
+  cached_markdown_version: 11,
 };
 
 export const discussionMock = {
diff --git a/spec/lib/banzai/filter/markdown_filter_spec.rb b/spec/lib/banzai/filter/markdown_filter_spec.rb
index ab14d77d552..a515d07b072 100644
--- a/spec/lib/banzai/filter/markdown_filter_spec.rb
+++ b/spec/lib/banzai/filter/markdown_filter_spec.rb
@@ -3,17 +3,61 @@ require 'spec_helper'
 describe Banzai::Filter::MarkdownFilter do
   include FilterSpecHelper
 
-  context 'code block' do
-    it 'adds language to lang attribute when specified' do
-      result = filter("```html\nsome code\n```")
+  describe 'markdown engine from context' do
+    it 'defaults to CommonMark' do
+      expect_any_instance_of(Banzai::Filter::MarkdownEngines::CommonMark).to receive(:render).and_return('test')
 
-      expect(result).to start_with("<pre><code lang=\"html\">")
+      filter('test')
     end
 
-    it 'does not add language to lang attribute when not specified' do
-      result = filter("```\nsome code\n```")
+    it 'uses Redcarpet' do
+      expect_any_instance_of(Banzai::Filter::MarkdownEngines::Redcarpet).to receive(:render).and_return('test')
 
-      expect(result).to start_with("<pre><code>")
+      filter('test', { markdown_engine: :redcarpet })
+    end
+
+    it 'uses CommonMark' do
+      expect_any_instance_of(Banzai::Filter::MarkdownEngines::CommonMark).to receive(:render).and_return('test')
+
+      filter('test', { markdown_engine: :common_mark })
+    end
+  end
+
+  describe 'code block' do
+    context 'using CommonMark' do
+      before do
+        stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :common_mark)
+      end
+
+      it 'adds language to lang attribute when specified' do
+        result = filter("```html\nsome code\n```")
+
+        expect(result).to start_with("<pre><code lang=\"html\">")
+      end
+
+      it 'does not add language to lang attribute when not specified' do
+        result = filter("```\nsome code\n```")
+
+        expect(result).to start_with("<pre><code>")
+      end
+    end
+
+    context 'using Redcarpet' do
+      before do
+        stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :redcarpet)
+      end
+
+      it 'adds language to lang attribute when specified' do
+        result = filter("```html\nsome code\n```")
+
+        expect(result).to start_with("\n<pre><code lang=\"html\">")
+      end
+
+      it 'does not add language to lang attribute when not specified' do
+        result = filter("```\nsome code\n```")
+
+        expect(result).to start_with("\n<pre><code>")
+      end
     end
   end
 end
diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index 2d75422ee68..da26d802688 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -370,4 +370,20 @@ describe CacheMarkdownField do
       end
     end
   end
+
+  describe CacheMarkdownField::MarkdownEngine do
+    subject { lambda { |version| CacheMarkdownField::MarkdownEngine.from_version(version) } }
+
+    it 'returns :common_mark as a default' do
+      expect(subject.call(nil)).to eq :common_mark
+    end
+
+    it 'returns :common_mark' do
+      expect(subject.call(CacheMarkdownField::CACHE_COMMONMARK_VERSION)).to eq :common_mark
+    end
+
+    it 'returns :redcarpet' do
+      expect(subject.call(CacheMarkdownField::CACHE_REDCARPET_VERSION)).to eq :redcarpet
+    end
+  end
 end
diff --git a/spec/services/preview_markdown_service_spec.rb b/spec/services/preview_markdown_service_spec.rb
index 64a9559791f..81dc7c57f4a 100644
--- a/spec/services/preview_markdown_service_spec.rb
+++ b/spec/services/preview_markdown_service_spec.rb
@@ -64,4 +64,16 @@ describe PreviewMarkdownService do
       expect(result[:commands]).to eq 'Sets time estimate to 2y.'
     end
   end
+
+  it 'sets correct markdown engine' do
+    service = described_class.new(project, user, { markdown_version: CacheMarkdownField::CACHE_REDCARPET_VERSION })
+    result  = service.execute
+
+    expect(result[:markdown_engine]).to eq :redcarpet
+
+    service = described_class.new(project, user, { markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION })
+    result  = service.execute
+
+    expect(result[:markdown_engine]).to eq :common_mark
+  end
 end