From c4ded595ccf520bc30bde90403366ad14ba8b594 Mon Sep 17 00:00:00 2001 From: David Wagner Date: Fri, 18 Nov 2016 12:57:25 +0100 Subject: [PATCH 1/2] Fix broken external links in help/index.html An external link was recently added but was broken because 'https://gitlab.com/help/' was prepended to every link in the page. Since no link in the main help readme begins with "help" and since doing so wouldn't make sense, the substitution conditionaly prepending "help" can be simplified and reused. Signed-off-by: David Wagner --- app/controllers/help_controller.rb | 6 +++--- spec/controllers/help_controller_spec.rb | 14 +++----------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb index 4b3c71874be..a10cdcce72b 100644 --- a/app/controllers/help_controller.rb +++ b/app/controllers/help_controller.rb @@ -6,9 +6,9 @@ class HelpController < ApplicationController def index @help_index = File.read(Rails.root.join('doc', 'README.md')) - # Prefix Markdown links with `help/` unless they already have been - # See http://rubular.com/r/ie2MlpdUMq - @help_index.gsub!(/(\]\()(\/?help\/)?([^\)\(]+\))/, '\1/help/\3') + # Prefix Markdown links with `help/` unless they are external links + # See http://rubular.com/r/MioSrVLK3S + @help_index.gsub!(%r{(\]\()(?!.+://)([^\)\(]+\))}, '\1/help/\2') end def show diff --git a/spec/controllers/help_controller_spec.rb b/spec/controllers/help_controller_spec.rb index 6fc6ea95e13..cffed987f6b 100644 --- a/spec/controllers/help_controller_spec.rb +++ b/spec/controllers/help_controller_spec.rb @@ -16,14 +16,6 @@ describe HelpController do end end - context 'when url prefixed with help/' do - it 'will be an absolute path' do - stub_readme("[API](help/api/README.md)") - get :index - expect(assigns[:help_index]).to eq '[API](/help/api/README.md)' - end - end - context 'when url prefixed with help' do it 'will be an absolute path' do stub_readme("[API](helpful_hints/README.md)") @@ -32,11 +24,11 @@ describe HelpController do end end - context 'when url prefixed with /help/' do + context 'when url is an external link' do it 'will not be changed' do - stub_readme("[API](/help/api/README.md)") + stub_readme("[external](https://some.external.link)") get :index - expect(assigns[:help_index]).to eq '[API](/help/api/README.md)' + expect(assigns[:help_index]).to eq '[external](https://some.external.link)' end end end From de0a7378eb0e08ab532a6ba80550abf9fe9db875 Mon Sep 17 00:00:00 2001 From: David Wagner Date: Fri, 25 Nov 2016 18:26:24 +0100 Subject: [PATCH 2/2] Check that both '/help' and '/help/' URLs have the same behaviour The links in the help page may be modified. This new test checks that URLs in this page are absolute and do not depend on the presence of a trailing slash in the URL. Signed-off-by: David Wagner --- spec/features/help_pages_spec.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/features/help_pages_spec.rb b/spec/features/help_pages_spec.rb index e2101b333e2..73d03837144 100644 --- a/spec/features/help_pages_spec.rb +++ b/spec/features/help_pages_spec.rb @@ -10,4 +10,28 @@ describe 'Help Pages', feature: true do expect(page).to have_content("ssh-keygen -t rsa -C \"#{@user.email}\"") end end + + describe 'Get the main help page' do + shared_examples_for 'help page' do + it 'prefixes links correctly' do + expect(page).to have_selector('div.documentation-index > ul a[href="/help/api/README.md"]') + end + end + + context 'without a trailing slash' do + before do + visit help_path + end + + it_behaves_like 'help page' + end + + context 'with a trailing slash' do + before do + visit help_path + '/' + end + + it_behaves_like 'help page' + end + end end