Merge pull request #7982 from Soullivaneuh/help-images

Fix and improve help rendering

CHANGELOG

@@ -56,6 +56,7 @@ v 7.10.0 (unreleased)
   - Fix "Hello @username." references not working by no longer allowing usernames to end in period.
   - Archive repositories in background worker.
   - Import GitHub, Bitbucket or GitLab.com projects owned by authenticated user into current namespace.
+  - Fix and improve help rendering (Sullivan Sénéchal)
 
 
 v 7.9.2

app/controllers/help_controller.rb

@@ -3,17 +3,35 @@ class HelpController < ApplicationController
   end
 
   def show
-    @category = params[:category]
-    @file = params[:file]
+    @filepath = params[:filepath]
+    @format = params[:format]
 
-    if File.exists?(Rails.root.join('doc', @category, @file + '.md'))
-      render 'show'
+    respond_to do |format|
+      format.md { render_doc }
+      format.all { send_file_data }
+    end
+  end
+
+  def shortcuts
+  end
+
+  private
+
+  def render_doc
+    if File.exists?(Rails.root.join('doc', @filepath + '.md'))
+      render 'show.html.haml'
     else
       not_found!
     end
   end
 
-  def shortcuts
+  def send_file_data
+    path = Rails.root.join('doc', "#{@filepath}.#{@format}")
+    if File.exists?(path)
+      send_file(path, disposition: 'inline')
+    else
+      head :not_found
+    end
   end
 
   def ui

app/views/help/show.html.haml

 .documentation.wiki
-  = markdown File.read(Rails.root.join('doc', @category, @file + '.md')).gsub("$your_email", current_user.email)
+  = markdown File.read(Rails.root.join('doc', @filepath + '.md')).gsub("$your_email", current_user.email)

config/initializers/mime_types.rb

@@ -6,3 +6,4 @@
 
 Mime::Type.register_alias "text/plain", :diff
 Mime::Type.register_alias "text/plain", :patch
+Mime::Type.register_alias 'text/html', :md

config/routes.rb

@@ -39,9 +39,9 @@ Gitlab::Application.routes.draw do
 
   # Help
   get 'help'                  => 'help#index'
-  get 'help/:category/:file'  => 'help#show', as: :help_page
   get 'help/shortcuts'
   get 'help/ui'               => 'help#ui'
+  get 'help/:filepath'        => 'help#show', as: :help_page, constraints: { filepath: /[^\.]+/ }
 
   #
   # Global snippets

features/steps/dashboard/help.rb

@@ -8,7 +8,7 @@ class Spinach::Features::DashboardHelp < Spinach::FeatureSteps
   end
 
   step 'I visit the "Rake Tasks" help page' do
-    visit help_page_path("raketasks", "maintenance")
+    visit help_page_path('raketasks/maintenance', format: 'md')
   end
 
   step 'I should see "Rake Tasks" page markdown rendered' do

spec/features/help_pages_spec.rb

@@ -6,7 +6,7 @@ describe 'Help Pages', feature: true do
       login_as :user
     end
     it 'replace the variable $your_email with the email of the user' do
-      visit help_page_path(category: 'ssh', file: 'README.md')
+      visit help_page_path(filepath: 'ssh/README', format: 'md')
       expect(page).to have_content("ssh-keygen -t rsa -C \"#{@user.email}\"")
     end
   end

spec/routing/routing_spec.rb

@@ -73,41 +73,41 @@ end
 #     help_markdown GET    /help/markdown(.:format)     help#markdown
 #          help_ssh GET    /help/ssh(.:format)          help#ssh
 #    help_raketasks GET    /help/raketasks(.:format)    help#raketasks
-describe HelpController, "routing" do
-  it "to #index" do
-    expect(get("/help")).to route_to('help#index')
+describe HelpController, 'routing' do
+  it 'to #index' do
+    expect(get('/help')).to route_to('help#index')
   end
 
-  it "to #permissions" do
-    expect(get("/help/permissions/permissions")).to route_to('help#show', category: "permissions", file: "permissions")
+  it 'to #permissions' do
+    expect(get('/help/permissions/permissions')).to route_to('help#show', filepath: 'permissions/permissions')
   end
 
-  it "to #workflow" do
-    expect(get("/help/workflow/README")).to route_to('help#show', category: "workflow", file: "README")
+  it 'to #workflow' do
+    expect(get('/help/workflow/README')).to route_to('help#show', filepath: 'workflow/README')
   end
 
-  it "to #api" do
-    expect(get("/help/api/README")).to route_to('help#show', category: "api", file: "README")
+  it 'to #api' do
+    expect(get('/help/api/README')).to route_to('help#show', filepath: 'api/README')
   end
 
-  it "to #web_hooks" do
-    expect(get("/help/web_hooks/web_hooks")).to route_to('help#show', category: "web_hooks", file: "web_hooks")
+  it 'to #web_hooks' do
+    expect(get('/help/web_hooks/web_hooks')).to route_to('help#show', filepath: 'web_hooks/web_hooks')
   end
 
-  it "to #system_hooks" do
-    expect(get("/help/system_hooks/system_hooks")).to route_to('help#show', category: "system_hooks", file: "system_hooks")
+  it 'to #system_hooks' do
+    expect(get('/help/system_hooks/system_hooks')).to route_to('help#show', filepath: 'system_hooks/system_hooks')
   end
 
-  it "to #markdown" do
-    expect(get("/help/markdown/markdown")).to route_to('help#show',category: "markdown", file: "markdown")
+  it 'to #markdown' do
+    expect(get('/help/markdown/markdown')).to route_to('help#show',filepath: 'markdown/markdown')
   end
 
-  it "to #ssh" do
-    expect(get("/help/ssh/README")).to route_to('help#show', category: "ssh", file: "README")
+  it 'to #ssh' do
+    expect(get('/help/ssh/README')).to route_to('help#show', filepath: 'ssh/README')
   end
 
-  it "to #raketasks" do
-    expect(get("/help/raketasks/README")).to route_to('help#show', category: "raketasks", file: "README")
+  it 'to #raketasks' do
+    expect(get('/help/raketasks/README')).to route_to('help#show', filepath: 'raketasks/README')
   end
 end