From e08d947e77fa79b723bb1a32794f99d3c39ee463 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 25 Mar 2015 10:03:55 +0100 Subject: [PATCH] Use relative URL for Markdown references, except in mails. --- app/helpers/issues_helper.rb | 24 +++-- .../gitlab_issue_tracker_service.rb | 12 +++ .../project_services/issue_tracker_service.rb | 12 +++ app/views/layouts/nav/_project.html.haml | 2 +- app/views/notify/_note_message.html.haml | 2 +- app/views/notify/new_issue_email.html.haml | 2 +- .../notify/new_merge_request_email.html.haml | 2 +- app/views/projects/_dropdown.html.haml | 2 +- lib/gitlab/markdown.rb | 90 +++++++++++-------- spec/helpers/gitlab_markdown_helper_spec.rb | 6 +- .../gitlab_issue_tracker_service_spec.rb | 33 +++---- 11 files changed, 120 insertions(+), 67 deletions(-) diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index a4bd4d30215..ad4a7612724 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -13,22 +13,34 @@ module IssuesHelper OpenStruct.new(id: 0, title: 'None (backlog)', name: 'Unassigned') end - def url_for_project_issues(project = @project) + def url_for_project_issues(project = @project, options = {}) return '' if project.nil? - project.issues_tracker.project_url + if options[:only_path] + project.issues_tracker.project_path + else + project.issues_tracker.project_url + end end - def url_for_new_issue(project = @project) + def url_for_new_issue(project = @project, options = {}) return '' if project.nil? - project.issues_tracker.new_issue_url + if options[:only_path] + project.issues_tracker.new_issue_path + else + project.issues_tracker.new_issue_url + end end - def url_for_issue(issue_iid, project = @project) + def url_for_issue(issue_iid, project = @project, options = {}) return '' if project.nil? - project.issues_tracker.issue_url(issue_iid) + if options[:only_path] + project.issues_tracker.issue_path(issue_iid) + else + project.issues_tracker.issue_url(issue_iid) + end end def title_for_issue(issue_iid, project = @project) diff --git a/app/models/project_services/gitlab_issue_tracker_service.rb b/app/models/project_services/gitlab_issue_tracker_service.rb index 4ff9f75fcc9..5f0553f3b0b 100644 --- a/app/models/project_services/gitlab_issue_tracker_service.rb +++ b/app/models/project_services/gitlab_issue_tracker_service.rb @@ -47,4 +47,16 @@ class GitlabIssueTrackerService < IssueTrackerService def issue_url(iid) namespace_project_issue_url(namespace_id: project.namespace, project_id: project, id: iid) end + + def project_path + namespace_project_issues_path(project.namespace, project) + end + + def new_issue_path + new_namespace_project_issue_path(namespace_id: project.namespace, project_id: project) + end + + def issue_path(iid) + namespace_project_issue_path(namespace_id: project.namespace, project_id: project, id: iid) + end end diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index 8e90c44d103..c8ab9d63b74 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -34,6 +34,18 @@ class IssueTrackerService < Service self.issues_url.gsub(':id', iid.to_s) end + def project_path + project_url + end + + def new_issue_path + new_issue_url + end + + def issue_path(iid) + issue_url(iid) + end + def fields [ { type: 'text', name: 'description', placeholder: description }, diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 52681865d64..6c13f30f627 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -53,7 +53,7 @@ - if project_nav_tab? :issues = nav_link(controller: :issues) do - = link_to url_for_project_issues, title: 'Issues', class: 'shortcuts-issues' do + = link_to url_for_project_issues(@project, only_path: true), title: 'Issues', class: 'shortcuts-issues' do %i.fa.fa-exclamation-circle %span Issues diff --git a/app/views/notify/_note_message.html.haml b/app/views/notify/_note_message.html.haml index 778a78acf56..e796353cec4 100644 --- a/app/views/notify/_note_message.html.haml +++ b/app/views/notify/_note_message.html.haml @@ -1,2 +1,2 @@ %div - = replace_image_links_with_base64(markdown(@note.note), @note.project) + = replace_image_links_with_base64(markdown(@note.note, reference_only_path: false), @note.project) diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml index 03cbee94608..d4d413b5b44 100644 --- a/app/views/notify/new_issue_email.html.haml +++ b/app/views/notify/new_issue_email.html.haml @@ -1,5 +1,5 @@ -if @issue.description - = replace_image_links_with_base64(markdown(@issue.description), @issue.project) + = replace_image_links_with_base64(markdown(@issue.description, reference_only_path: false), @issue.project) - if @issue.assignee_id.present? %p diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index 729a7bb505d..60e33227e01 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -6,4 +6,4 @@ Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name} -if @merge_request.description - = replace_image_links_with_base64(markdown(@merge_request.description), @merge_request.project) + = replace_image_links_with_base64(markdown(@merge_request.description, reference_only_path: false), @merge_request.project) diff --git a/app/views/projects/_dropdown.html.haml b/app/views/projects/_dropdown.html.haml index f4f4c2662cf..3adb3087289 100644 --- a/app/views/projects/_dropdown.html.haml +++ b/app/views/projects/_dropdown.html.haml @@ -5,7 +5,7 @@ %ul.dropdown-menu - if @project.issues_enabled && can?(current_user, :write_issue, @project) %li - = link_to url_for_new_issue, title: "New Issue" do + = link_to url_for_new_issue(@project, only_path: true), title: "New Issue" do New issue - if @project.merge_requests_enabled && can?(current_user, :write_merge_request, @project) %li diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 41bb8d08924..dbc1025855b 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -32,12 +32,12 @@ module Gitlab module Markdown include IssuesHelper - attr_reader :html_options + attr_reader :options, :html_options # Public: Parse the provided text with GitLab-Flavored Markdown # # text - the source text - # project - extra options for the reference links as given to link_to + # project - the project # html_options - extra options for the reference links as given to link_to def gfm(text, project = @project, html_options = {}) gfm_with_options(text, {}, project, html_options) @@ -46,9 +46,10 @@ module Gitlab # Public: Parse the provided text with GitLab-Flavored Markdown # # text - the source text - # options - parse_tasks: true - render tasks - # - xhtml: true - output XHTML instead of HTML - # project - extra options for the reference links as given to link_to + # options - parse_tasks - render tasks + # - xhtml - output XHTML instead of HTML + # - reference_only_path - Use relative path for reference links + # project - the project # html_options - extra options for the reference links as given to link_to def gfm_with_options(text, options = {}, project = @project, html_options = {}) return text if text.nil? @@ -58,6 +59,13 @@ module Gitlab # for gsub calls to work as we need them to. text = text.dup.to_str + options.reverse_merge!( + parse_tasks: false, + xhtml: false, + reference_only_path: true + ) + + @options = options @html_options = html_options # Extract pre blocks so they are not altered @@ -113,12 +121,13 @@ module Gitlab markdown_pipeline = HTML::Pipeline::Gitlab.new(filters).pipeline result = markdown_pipeline.call(text, markdown_context) - saveoptions = 0 + + save_options = 0 if options[:xhtml] - saveoptions |= Nokogiri::XML::Node::SaveOptions::AS_XHTML + save_options |= Nokogiri::XML::Node::SaveOptions::AS_XHTML end - text = result[:output].to_html(save_with: saveoptions) + text = result[:output].to_html(save_with: save_options) if options[:parse_tasks] text = parse_tasks(text) @@ -229,33 +238,37 @@ module Gitlab end def reference_user(identifier, project = @project, _ = nil) - options = html_options.merge( + link_options = html_options.merge( class: "gfm gfm-project_member #{html_options[:class]}" ) if identifier == "all" - link_to("@all", namespace_project_url(project.namespace, project), options) + link_to( + "@all", + namespace_project_url(project.namespace, project, only_path: options[:reference_only_path]), + link_options + ) elsif namespace = Namespace.find_by(path: identifier) url = if namespace.type == "Group" - group_url(identifier) + group_url(identifier, only_path: options[:reference_only_path]) else - user_url(identifier) + user_url(identifier, only_path: options[:reference_only_path]) end - link_to("@#{identifier}", url, options) + link_to("@#{identifier}", url, link_options) end end def reference_label(identifier, project = @project, _ = nil) if label = project.labels.find_by(id: identifier) - options = html_options.merge( + link_options = html_options.merge( class: "gfm gfm-label #{html_options[:class]}" ) link_to( render_colored_label(label), namespace_project_issues_path(project.namespace, project, label_name: label.name), - options + link_options ) end end @@ -263,14 +276,14 @@ module Gitlab def reference_issue(identifier, project = @project, prefix_text = nil) if project.default_issues_tracker? if project.issue_exists? identifier - url = url_for_issue(identifier, project) + url = url_for_issue(identifier, project, only_path: options[:reference_only_path]) title = title_for_issue(identifier, project) - options = html_options.merge( + link_options = html_options.merge( title: "Issue: #{title}", class: "gfm gfm-issue #{html_options[:class]}" ) - link_to("#{prefix_text}##{identifier}", url, options) + link_to("#{prefix_text}##{identifier}", url, link_options) end else if project.external_issue_tracker.present? @@ -280,44 +293,46 @@ module Gitlab end end - def reference_merge_request(identifier, project = @project, - prefix_text = nil) + def reference_merge_request(identifier, project = @project, prefix_text = nil) if merge_request = project.merge_requests.find_by(iid: identifier) - options = html_options.merge( + link_options = html_options.merge( title: "Merge Request: #{merge_request.title}", class: "gfm gfm-merge_request #{html_options[:class]}" ) url = namespace_project_merge_request_url(project.namespace, project, - merge_request) - link_to("#{prefix_text}!#{identifier}", url, options) + merge_request, + only_path: options[:reference_only_path]) + link_to("#{prefix_text}!#{identifier}", url, link_options) end end def reference_snippet(identifier, project = @project, _ = nil) if snippet = project.snippets.find_by(id: identifier) - options = html_options.merge( + link_options = html_options.merge( title: "Snippet: #{snippet.title}", class: "gfm gfm-snippet #{html_options[:class]}" ) link_to( "$#{identifier}", - namespace_project_snippet_url(project.namespace, project, snippet), - options + namespace_project_snippet_url(project.namespace, project, snippet, + only_path: options[:reference_only_path]), + link_options ) end end def reference_commit(identifier, project = @project, prefix_text = nil) if project.valid_repo? && commit = project.repository.commit(identifier) - options = html_options.merge( + link_options = html_options.merge( title: commit.link_title, class: "gfm gfm-commit #{html_options[:class]}" ) prefix_text = "#{prefix_text}@" if prefix_text link_to( "#{prefix_text}#{identifier}", - namespace_project_commit_url(project.namespace, project, commit), - options + namespace_project_commit_url( project.namespace, project, commit, + only_path: options[:reference_only_path]), + link_options ) end end @@ -332,7 +347,7 @@ module Gitlab from = project.repository.commit(from_id) && to = project.repository.commit(to_id) - options = html_options.merge( + link_options = html_options.merge( title: "Commits #{from_id} through #{to_id}", class: "gfm gfm-commit_range #{html_options[:class]}" ) @@ -340,22 +355,23 @@ module Gitlab link_to( "#{prefix_text}#{identifier}", - namespace_project_compare_url(project.namespace, project, from: from_id, to: to_id), - options + namespace_project_compare_url(project.namespace, project, + from: from_id, to: to_id, + only_path: options[:reference_only_path]), + link_options ) end end - def reference_external_issue(identifier, project = @project, - prefix_text = nil) - url = url_for_issue(identifier, project) + def reference_external_issue(identifier, project = @project, prefix_text = nil) + url = url_for_issue(identifier, project, only_path: options[:reference_only_path]) title = project.external_issue_tracker.title - options = html_options.merge( + link_options = html_options.merge( title: "Issue in #{title}", class: "gfm gfm-issue #{html_options[:class]}" ) - link_to("#{prefix_text}##{identifier}", url, options) + link_to("#{prefix_text}##{identifier}", url, link_options) end # Turn list items that start with "[ ]" into HTML checkbox inputs. diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index f415f126eea..0d06c6ffb82 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -522,7 +522,7 @@ describe GitlabMarkdownHelper do # First issue link expect(groups[1]). - to match(/href="#{namespace_project_issue_url(project.namespace, project, issues[0])}"/) + to match(/href="#{namespace_project_issue_path(project.namespace, project, issues[0])}"/) expect(groups[1]).to match(/##{issues[0].iid}$/) # Internal commit link @@ -531,7 +531,7 @@ describe GitlabMarkdownHelper do # Second issue link expect(groups[3]). - to match(/href="#{namespace_project_issue_url(project.namespace, project, issues[1])}"/) + to match(/href="#{namespace_project_issue_path(project.namespace, project, issues[1])}"/) expect(groups[3]).to match(/##{issues[1].iid}$/) # Trailing commit link @@ -651,7 +651,7 @@ describe GitlabMarkdownHelper do end it "should leave ref-like href of 'manual' links untouched" do - expect(markdown("why not [inspect !#{merge_request.iid}](http://example.tld/#!#{merge_request.iid})")).to eq("

why not inspect !#{merge_request.iid}

\n") + expect(markdown("why not [inspect !#{merge_request.iid}](http://example.tld/#!#{merge_request.iid})")).to eq("

why not inspect !#{merge_request.iid}

\n") end it "should leave ref-like src of images untouched" do diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb index bedb6ecf1b4..f94bef5c365 100644 --- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb +++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb @@ -30,22 +30,6 @@ describe GitlabIssueTrackerService do let(:project) { create(:project) } context 'with absolute urls' do - before do - @service = project.create_gitlab_issue_tracker_service(active: true) - end - - after do - @service.destroy! - end - - it 'should give the correct path' do - expect(@service.project_url).to eq("http://localhost/#{project.path_with_namespace}/issues") - expect(@service.new_issue_url).to eq("http://localhost/#{project.path_with_namespace}/issues/new") - expect(@service.issue_url(432)).to eq("http://localhost/#{project.path_with_namespace}/issues/432") - end - end - - context 'with enabled relative urls' do before do GitlabIssueTrackerService.default_url_options[:script_name] = "/gitlab/root" @service = project.create_gitlab_issue_tracker_service(active: true) @@ -61,5 +45,22 @@ describe GitlabIssueTrackerService do expect(@service.issue_url(432)).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues/432") end end + + context 'with relative urls' do + before do + GitlabIssueTrackerService.default_url_options[:script_name] = "/gitlab/root" + @service = project.create_gitlab_issue_tracker_service(active: true) + end + + after do + @service.destroy! + end + + it 'should give the correct path' do + expect(@service.project_path).to eq("/gitlab/root/#{project.path_with_namespace}/issues") + expect(@service.new_issue_path).to eq("/gitlab/root/#{project.path_with_namespace}/issues/new") + expect(@service.issue_path(432)).to eq("/gitlab/root/#{project.path_with_namespace}/issues/432") + end + end end end