Merge branch 'patch/fix-markdown-preview-wikis' into 'master'

Wiki preview URL converting problem [via Markdown]

Current implementation when rendering the preview, thinks relative links are for project repository files.

We are creating a new preview route that will define correct context data to render for wikis instead.

Fixes #2380, #1184

See merge request !3461

10 files changed, 105 insertions, 18 deletions

diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb
index 02ceb8f4334..9f3a4a69721 100644
--- a/app/controllers/projects/wikis_controller.rb
+++ b/app/controllers/projects/wikis_controller.rb
@@ -88,6 +88,20 @@ class Projects::WikisController < Projects::ApplicationController
     )
   end
 
+  def markdown_preview
+    text = params[:text]
+
+    ext = Gitlab::ReferenceExtractor.new(@project, current_user, current_user)
+    ext.analyze(text)
+
+    render json: {
+      body: view_context.markdown(text, pipeline: :wiki, project_wiki: @project_wiki),
+      references: {
+        users: ext.users.map(&:username)
+      }
+    }
+  end
+
   def git_access
   end
 
diff --git a/app/views/layouts/project.html.haml b/app/views/layouts/project.html.haml
index ab527e8e438..a7ef31acd3d 100644
--- a/app/views/layouts/project.html.haml
+++ b/app/views/layouts/project.html.haml
@@ -5,10 +5,14 @@
 
 - content_for :scripts_body_top do
   - project = @target_project || @project
+  - if @project_wiki
+    - markdown_preview_path = namespace_project_wikis_markdown_preview_path(project.namespace, project)
+  - else
+    - markdown_preview_path = markdown_preview_namespace_project_path(project.namespace, project)
   - if current_user
     :javascript
       window.project_uploads_path = "#{namespace_project_uploads_path project.namespace,project}";
-      window.markdown_preview_path = "#{markdown_preview_namespace_project_path(project.namespace, project)}";
+      window.markdown_preview_path = "#{markdown_preview_path}";
 
 - content_for :scripts_body do
   = render "layouts/init_auto_complete" if current_user
diff --git a/config/routes.rb b/config/routes.rb
index 6bf22fb4456..e57c04595f6 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -575,6 +575,7 @@ Rails.application.routes.draw do
           # Order matters to give priority to these matches
           get '/wikis/git_access', to: 'wikis#git_access'
           get '/wikis/pages', to: 'wikis#pages', as: 'wiki_pages'
+          post '/wikis/markdown_preview', to:'wikis#markdown_preview'
           post '/wikis', to: 'wikis#create'
 
           get '/wikis/*id/history', to: 'wikis#history', as: 'wiki_history', constraints: WIKI_SLUG_ID
diff --git a/lib/banzai/filter/gollum_tags_filter.rb b/lib/banzai/filter/gollum_tags_filter.rb
index 7ce26db1b90..d08267a9d6c 100644
--- a/lib/banzai/filter/gollum_tags_filter.rb
+++ b/lib/banzai/filter/gollum_tags_filter.rb
@@ -118,7 +118,7 @@ module Banzai
         end
 
         if path
-          content_tag(:img, nil, src: path)
+          content_tag(:img, nil, src: path, class: 'gfm')
         end
       end
 
@@ -144,12 +144,18 @@ module Banzai
       # if it is not.
       def process_page_link_tag(parts)
         if parts.size == 1
-          url = parts[0].strip
+          reference = parts[0].strip
         else
-          name, url = *parts.compact.map(&:strip)
+          name, reference = *parts.compact.map(&:strip)
         end
 
-        content_tag(:a, name || url, href: url)
+        if url?(reference)
+          href = reference
+        else
+          href = ::File.join(project_wiki_base_path, reference)
+        end
+
+        content_tag(:a, name || reference, href: href, class: 'gfm')
       end
 
       def project_wiki
diff --git a/lib/banzai/filter/wiki_link_filter.rb b/lib/banzai/filter/wiki_link_filter.rb
new file mode 100644
index 00000000000..06d10c98501
--- /dev/null
+++ b/lib/banzai/filter/wiki_link_filter.rb
@@ -0,0 +1,56 @@
+require 'uri'
+
+module Banzai
+  module Filter
+    # HTML filter that "fixes" relative links to files in a repository.
+    #
+    # Context options:
+    #   :project_wiki
+    class WikiLinkFilter < HTML::Pipeline::Filter
+
+      def call
+        return doc unless project_wiki?
+
+        doc.search('a:not(.gfm)').each do |el|
+          process_link_attr el.attribute('href')
+        end
+
+        doc
+      end
+
+      protected
+
+      def project_wiki?
+        !context[:project_wiki].nil?
+      end
+
+      def process_link_attr(html_attr)
+        return if html_attr.blank? || file_reference?(html_attr)
+
+        uri = URI(html_attr.value)
+        if uri.relative? && uri.path.present?
+          html_attr.value = rebuild_wiki_uri(uri).to_s
+        end
+      rescue URI::Error
+        # noop
+      end
+
+      def rebuild_wiki_uri(uri)
+        uri.path = ::File.join(project_wiki_base_path, uri.path)
+        uri
+      end
+
+      def file_reference?(html_attr)
+        !File.extname(html_attr.value).blank?
+      end
+
+      def project_wiki
+        context[:project_wiki]
+      end
+
+      def project_wiki_base_path
+        project_wiki && project_wiki.wiki_base_path
+      end
+    end
+  end
+end
diff --git a/lib/banzai/pipeline/wiki_pipeline.rb b/lib/banzai/pipeline/wiki_pipeline.rb
index 0b5a9e0b2b8..c37b8e71cb0 100644
--- a/lib/banzai/pipeline/wiki_pipeline.rb
+++ b/lib/banzai/pipeline/wiki_pipeline.rb
@@ -2,8 +2,10 @@ module Banzai
   module Pipeline
     class WikiPipeline < FullPipeline
       def self.filters
-        @filters ||= super.insert_after(Filter::TableOfContentsFilter,
-                                        Filter::GollumTagsFilter)
+        @filters ||= begin
+          super.insert_after(Filter::TableOfContentsFilter, Filter::GollumTagsFilter)
+               .insert_before(Filter::TaskListFilter, Filter::WikiLinkFilter)
+        end
       end
     end
   end
diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown_spec.rb
index 12fd8d37210..3d0d0e59fd7 100644
--- a/spec/features/markdown_spec.rb
+++ b/spec/features/markdown_spec.rb
@@ -39,7 +39,7 @@ describe 'GitLab Markdown', feature: true do
   end
 
   def doc(html = @html)
-    Nokogiri::HTML::DocumentFragment.parse(html)
+    @doc ||= Nokogiri::HTML::DocumentFragment.parse(html)
   end
 
   # Shared behavior that all pipelines should exhibit
@@ -230,6 +230,7 @@ describe 'GitLab Markdown', feature: true do
       file = Gollum::File.new(@project_wiki.wiki)
       expect(file).to receive(:path).and_return('images/example.jpg')
       expect(@project_wiki).to receive(:find_file).with('images/example.jpg').and_return(file)
+      allow(@project_wiki).to receive(:wiki_base_path) { '/namespace1/gitlabhq/wikis' }
 
       @html = markdown(@feat.raw_markdown, { pipeline: :wiki, project_wiki: @project_wiki })
     end
diff --git a/spec/lib/banzai/filter/gollum_tags_filter_spec.rb b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb
index 5e23c5c319a..fe2ce092e6b 100644
--- a/spec/lib/banzai/filter/gollum_tags_filter_spec.rb
+++ b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb
@@ -70,20 +70,22 @@ describe Banzai::Filter::GollumTagsFilter, lib: true do
   end
 
   context 'linking internal resources' do
-    it "the created link's text will be equal to the resource's text" do
+    it "the created link's text includes the resource's text and wiki base path" do
       tag = '[[wiki-slug]]'
       doc = filter("See #{tag}", project_wiki: project_wiki)
+      expected_path = ::File.join(project_wiki.wiki_base_path, 'wiki-slug')
 
       expect(doc.at_css('a').text).to eq 'wiki-slug'
-      expect(doc.at_css('a')['href']).to eq 'wiki-slug'
+      expect(doc.at_css('a')['href']).to eq expected_path
     end
 
     it "the created link's text will be link-text" do
       tag = '[[link-text|wiki-slug]]'
       doc = filter("See #{tag}", project_wiki: project_wiki)
+      expected_path = ::File.join(project_wiki.wiki_base_path, 'wiki-slug')
 
       expect(doc.at_css('a').text).to eq 'link-text'
-      expect(doc.at_css('a')['href']).to eq 'wiki-slug'
+      expect(doc.at_css('a')['href']).to eq expected_path
     end
   end
 
diff --git a/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb b/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb
index 3e25406e498..7aa1b4a3bf6 100644
--- a/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb
+++ b/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb
@@ -11,7 +11,7 @@ describe Banzai::Pipeline::WikiPipeline do
           Foo
       MD
 
-      result = described_class.call(markdown, project: spy, project_wiki: double)
+      result = described_class.call(markdown, project: spy, project_wiki: spy)
 
       aggregate_failures do
         expect(result[:output].text).not_to include '[['
@@ -29,7 +29,7 @@ describe Banzai::Pipeline::WikiPipeline do
           Foo
       MD
 
-      output = described_class.to_html(markdown, project: spy, project_wiki: double)
+      output = described_class.to_html(markdown, project: spy, project_wiki: spy)
 
       expect(output).to include('[[<em>toc</em>]]')
     end
@@ -42,7 +42,7 @@ describe Banzai::Pipeline::WikiPipeline do
           Foo
       MD
 
-      output = described_class.to_html(markdown, project: spy, project_wiki: double)
+      output = described_class.to_html(markdown, project: spy, project_wiki: spy)
 
       aggregate_failures do
         expect(output).not_to include('<ul>')
diff --git a/spec/support/matchers/markdown_matchers.rb b/spec/support/matchers/markdown_matchers.rb
index 1d52489e804..43cb6ef43f2 100644
--- a/spec/support/matchers/markdown_matchers.rb
+++ b/spec/support/matchers/markdown_matchers.rb
@@ -13,7 +13,7 @@ module MarkdownMatchers
     set_default_markdown_messages
 
     match do |actual|
-      link  = actual.at_css('a:contains("Relative Link")')
+      link = actual.at_css('a:contains("Relative Link")')
       image = actual.at_css('img[alt="Relative Image"]')
 
       expect(link['href']).to end_with('master/doc/README.md')
@@ -72,14 +72,15 @@ module MarkdownMatchers
       have_css("img[src$='#{src}']")
     end
 
+    prefix = '/namespace1/gitlabhq/wikis'
     set_default_markdown_messages
 
     match do |actual|
-      expect(actual).to have_link('linked-resource', href: 'linked-resource')
-      expect(actual).to have_link('link-text', href: 'linked-resource')
+      expect(actual).to have_link('linked-resource', href: "#{prefix}/linked-resource")
+      expect(actual).to have_link('link-text', href: "#{prefix}/linked-resource")
       expect(actual).to have_link('http://example.com', href: 'http://example.com')
       expect(actual).to have_link('link-text', href: 'http://example.com/pdfs/gollum.pdf')
-      expect(actual).to have_image('/gitlabhq/wikis/images/example.jpg')
+      expect(actual).to have_image("#{prefix}/images/example.jpg")
       expect(actual).to have_image('http://example.com/images/example.jpg')
     end
   end