Merge branch 'email-full-url'

CHANGELOG

@@ -48,6 +48,7 @@ v 7.10.0 (unreleased)
   - Prevent note form from being cleared when submitting failed.
   - Improve file icons rendering on tree (Sullivan Sénéchal)
   - API: Add pagination to project events
+  - Get issue links in notification mail to work again.
 
 v 7.9.0
   - Send EmailsOnPush email when branch or tag is created or deleted.

app/helpers/issues_helper.rb

@@ -13,23 +13,35 @@ 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?
 
+    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?
 
+    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?
 
+    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)
     return '' if project.nil?

app/models/project_services/gitlab_issue_tracker_service.rb

@@ -20,8 +20,13 @@
 
 class GitlabIssueTrackerService < IssueTrackerService
   include Rails.application.routes.url_helpers
-  prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url
 
+  default_url_options[:host]     = Gitlab.config.gitlab.host
+  default_url_options[:protocol] = Gitlab.config.gitlab.protocol
+  default_url_options[:port]     = Gitlab.config.gitlab.port unless Gitlab.config.gitlab_on_standard_port?
+  default_url_options[:script_name] = Gitlab.config.gitlab.relative_url_root
+
+  prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url
 
   def default?
     true
@@ -32,20 +37,26 @@ class GitlabIssueTrackerService < IssueTrackerService
   end
 
   def project_url
-    "#{gitlab_url}#{namespace_project_issues_path(project.namespace, project)}"
+    namespace_project_issues_url(project.namespace, project)
   end
 
   def new_issue_url
-    "#{gitlab_url}#{new_namespace_project_issue_path(namespace_id: project.namespace, project_id: project)}"
+    new_namespace_project_issue_url(namespace_id: project.namespace, project_id: project)
   end
 
   def issue_url(iid)
-    "#{gitlab_url}#{namespace_project_issue_path(namespace_id: project.namespace, project_id: project, id: iid)}"
+    namespace_project_issue_url(namespace_id: project.namespace, project_id: project, id: iid)
   end
 
-  private
+  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 gitlab_url
-    Gitlab.config.gitlab.relative_url_root.chomp("/") if Gitlab.config.gitlab.relative_url_root
+  def issue_path(iid)
+    namespace_project_issue_path(namespace_id: project.namespace, project_id: project, id: iid)
   end
 end

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 },

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

app/views/notify/_note_message.html.haml

 %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)

app/views/notify/new_issue_email.html.haml

 -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

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)

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

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.

spec/helpers/gitlab_markdown_helper_spec.rb

@@ -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("<p>why not <a href=\"http://example.tld/#!#{merge_request.iid}\">inspect </a><a class=\"gfm gfm-merge_request \" href=\"#{namespace_project_merge_request_url(project.namespace, project, merge_request)}\" title=\"Merge Request: #{merge_request.title}\">!#{merge_request.iid}</a><a href=\"http://example.tld/#!#{merge_request.iid}\"></a></p>\n")
+      expect(markdown("why not [inspect !#{merge_request.iid}](http://example.tld/#!#{merge_request.iid})")).to eq("<p>why not <a href=\"http://example.tld/#!#{merge_request.iid}\">inspect </a><a class=\"gfm gfm-merge_request \" href=\"#{namespace_project_merge_request_path(project.namespace, project, merge_request)}\" title=\"Merge Request: #{merge_request.title}\">!#{merge_request.iid}</a><a href=\"http://example.tld/#!#{merge_request.iid}\"></a></p>\n")
     end
 
     it "should leave ref-like src of images untouched" do

spec/models/project_services/gitlab_issue_tracker_service_spec.rb

@@ -31,6 +31,7 @@ describe GitlabIssueTrackerService do
 
     context 'with absolute urls' do
       before do
+        GitlabIssueTrackerService.default_url_options[:script_name] = "/gitlab/root"
         @service = project.create_gitlab_issue_tracker_service(active: true)
       end
 
@@ -39,15 +40,15 @@ describe GitlabIssueTrackerService do
       end
 
       it 'should give the correct path' do
-        expect(@service.project_url).to eq("/#{project.path_with_namespace}/issues")
-        expect(@service.new_issue_url).to eq("/#{project.path_with_namespace}/issues/new")
-        expect(@service.issue_url(432)).to eq("/#{project.path_with_namespace}/issues/432")
+        expect(@service.project_url).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues")
+        expect(@service.new_issue_url).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues/new")
+        expect(@service.issue_url(432)).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues/432")
       end
     end
 
-    context 'with enabled relative urls' do
+    context 'with relative urls' do
       before do
-        Settings.gitlab.stub(:relative_url_root).and_return("/gitlab/root")
+        GitlabIssueTrackerService.default_url_options[:script_name] = "/gitlab/root"
         @service = project.create_gitlab_issue_tracker_service(active: true)
       end
 
@@ -56,9 +57,9 @@ describe GitlabIssueTrackerService do
       end
 
       it 'should give the correct path' do
-        expect(@service.project_url).to eq("/gitlab/root/#{project.path_with_namespace}/issues")
-        expect(@service.new_issue_url).to eq("/gitlab/root/#{project.path_with_namespace}/issues/new")
-        expect(@service.issue_url(432)).to eq("/gitlab/root/#{project.path_with_namespace}/issues/432")
+        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