Support both internal and external issue trackers

app/controllers/projects/issues_controller.rb

@@ -269,10 +269,6 @@ class Projects::IssuesController < Projects::ApplicationController
     end
   end
-  def module_enabled
-    render_404 unless @project.feature_available?(:issues, current_user)
-  end
-
   def issue_params
     params.require(:issue).permit(*issue_params_attributes)
   end

app/helpers/issues_helper.rb

@@ -17,10 +17,10 @@ module IssuesHelper
     return '' if project.nil?
     url =
-      if options[:only_path]
-        project.issues_tracker.issue_path(issue_iid)
+      if options[:internal]
+        url_for_internal_issue(issue_iid, project, options)
       else
-        project.issues_tracker.issue_url(issue_iid)
+        url_for_tracker_issue(issue_iid, project, options)
       end
     # Ensure we return a valid URL to prevent possible XSS.
@@ -29,6 +29,24 @@ module IssuesHelper
     ''
   end
+  def url_for_tracker_issue(issue_iid, project, options)
+    if options[:only_path]
+      project.issues_tracker.issue_path(issue_iid)
+    else
+      project.issues_tracker.issue_url(issue_iid)
+    end
+  end
+
+  def url_for_internal_issue(issue_iid, project = @project, options = {})
+    helpers = Gitlab::Routing.url_helpers
+
+    if options[:only_path]
+      helpers.namespace_project_issue_path(namespace_id: project.namespace, project_id: project, id: issue_iid)
+    else
+      helpers.namespace_project_issue_url(namespace_id: project.namespace, project_id: project, id: issue_iid)
+    end
+  end
+
   def bulk_update_milestone_options
     milestones = @project.milestones.active.reorder(due_date: :asc, title: :asc).to_a
     milestones.unshift(Milestone::None)
@@ -158,4 +176,6 @@ module IssuesHelper
   # Required for Banzai::Filter::IssueReferenceFilter
   module_function :url_for_issue
+  module_function :url_for_internal_issue
+  module_function :url_for_tracker_issue
 end

app/models/merge_request.rb

@@ -596,7 +596,7 @@ class MergeRequest < ActiveRecord::Base
   # running `ReferenceExtractor` on each of them separately.
   # This optimization does not apply to issues from external sources.
   def cache_merge_request_closes_issues!(current_user)
-    return if project.has_external_issue_tracker?
+    return unless project.issues_enabled?
     transaction do
       self.merge_requests_closing_issues.delete_all

app/models/project.rb

@@ -734,9 +734,11 @@ class Project < ActiveRecord::Base
   end
   def get_issue(issue_id, current_user)
-    if default_issues_tracker?
-      IssuesFinder.new(current_user, project_id: id).find_by(iid: issue_id)
-    else
+    issue = IssuesFinder.new(current_user, project_id: id).find_by(iid: issue_id) if issues_enabled?
+
+    if issue
+      issue
+    elsif external_issue_tracker
       ExternalIssue.new(issue_id, self)
     end
   end
@@ -758,7 +760,7 @@ class Project < ActiveRecord::Base
   end
   def external_issue_reference_pattern
-    external_issue_tracker.class.reference_pattern
+    external_issue_tracker.class.reference_pattern(only_long: issues_enabled?)
   end
   def default_issues_tracker?

app/models/project_services/issue_tracker_service.rb

@@ -8,8 +8,12 @@ class IssueTrackerService < Service
   # This pattern does not support cross-project references
   # The other code assumes that this pattern is a superset of all
   # overriden patterns. See ReferenceRegexes::EXTERNAL_PATTERN
-  def self.reference_pattern
-    @reference_pattern ||= %r{(\b[A-Z][A-Z0-9_]+-|#{Issue.reference_prefix})(?<issue>\d+)}
+  def self.reference_pattern(only_long: false)
+    if only_long
+      %r{(\b[A-Z][A-Z0-9_]+-)(?<issue>\d+)}
+    else
+      %r{(\b[A-Z][A-Z0-9_]+-|#{Issue.reference_prefix})(?<issue>\d+)}
+    end
   end
   def default?

app/models/project_services/jira_service.rb

@@ -18,7 +18,7 @@ class JiraService < IssueTrackerService
   end
   # {PROJECT-KEY}-{NUMBER} Examples: JIRA-1, PROJECT-1
-  def self.reference_pattern
+  def self.reference_pattern(only_long: true)
     @reference_pattern ||= %r{(?<issue>\b([A-Z][A-Z0-9_]+-)\d+)}
   end

app/services/issues/close_service.rb

@@ -16,13 +16,13 @@ module Issues
     # The code calling this method is responsible for ensuring that a user is
     # allowed to close the given issue.
     def close_issue(issue, commit: nil, notifications: true, system_note: true)
-      if project.jira_tracker? && project.jira_service.active
+      if project.jira_tracker? && project.jira_service.active && issue.is_a?(ExternalIssue)
         project.jira_service.close_issue(commit, issue)
         todo_service.close_issue(issue, current_user)
         return issue
       end
-      if project.default_issues_tracker? && issue.close
+      if project.issues_enabled? && issue.close
         event_service.close_issue(issue, current_user)
         create_note(issue, commit) if system_note
         notification_service.close_issue(issue, current_user) if notifications

app/views/layouts/nav/_new_project_sidebar.html.haml

@@ -75,10 +75,10 @@
             Registry
     - if project_nav_tab? :issues
-      = nav_link(controller: @project.default_issues_tracker? ? [:issues, :labels, :milestones, :boards] : :issues) do
+      = nav_link(controller: @project.issues_enabled? ? [:issues, :labels, :milestones, :boards] : :issues) do
         = link_to project_issues_path(@project), title: 'Issues', class: 'shortcuts-issues' do
           %span
-            - if @project.default_issues_tracker?
+            - if @project.issues_enabled?
               %span.badge.count.issue_counter= number_with_delimiter(IssuesFinder.new(current_user, project_id: @project.id).execute.opened.count)
             Issues
@@ -113,7 +113,7 @@
                   Milestones
     - if project_nav_tab? :merge_requests
-      = nav_link(controller: @project.default_issues_tracker? ? :merge_requests : [:merge_requests, :labels, :milestones]) do
+      = nav_link(controller: @project.issues_enabled? ? :merge_requests : [:merge_requests, :labels, :milestones]) do
         = link_to project_merge_requests_path(@project), title: 'Merge Requests', class: 'shortcuts-merge_requests' do
           %span
             %span.badge.count.merge_counter.js-merge-counter= number_with_delimiter(MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.count)

app/views/layouts/nav/_project.html.haml

@@ -23,16 +23,16 @@
             Registry
     - if project_nav_tab? :issues
-      = nav_link(controller: @project.default_issues_tracker? ? [:issues, :labels, :milestones, :boards] : :issues) do
+      = nav_link(controller: @project.issues_enabled? ? [:issues, :labels, :milestones, :boards] : :issues) do
         = link_to project_issues_path(@project), title: 'Issues', class: 'shortcuts-issues' do
           %span
             Issues
-            - if @project.default_issues_tracker?
+            - if @project.issues_enabled?
               %span.badge.count.issue_counter= number_with_delimiter(issuables_count_for_state(:issues, :opened, finder: IssuesFinder.new(current_user, project_id: @project.id)))
     - if project_nav_tab? :merge_requests
       - controllers = [:merge_requests, 'projects/merge_requests/conflicts']
-      - controllers.push(:merge_requests, :labels, :milestones) unless @project.default_issues_tracker?
+      - controllers.push(:merge_requests, :labels, :milestones) unless @project.issues_enabled?
       = nav_link(controller: controllers) do
         = link_to project_merge_requests_path(@project), title: 'Merge Requests', class: 'shortcuts-merge_requests' do
           %span

app/views/projects/merge_requests/index.html.haml

@@ -4,7 +4,7 @@
 - new_merge_request_path = project_new_merge_request_path(merge_project) if merge_project
 - page_title "Merge Requests"
-- unless @project.default_issues_tracker?
+- unless @project.issues_enabled?
   = content_for :sub_nav do
     = render "projects/merge_requests/head"

app/views/shared/_mr_head.html.haml

-- if @project.default_issues_tracker?
+- if @project.issues_enabled?
   = render "projects/issues/head"
 - else
   = render "projects/merge_requests/head"

lib/api/entities.rb

@@ -109,7 +109,7 @@ module API
         user.avatar_url(only_path: false)
       end
       expose :star_count, :forks_count
-      expose :open_issues_count, if: lambda { |project, options| project.feature_available?(:issues, options[:current_user]) && project.default_issues_tracker? }
+      expose :open_issues_count, if: lambda { |project, options| project.feature_available?(:issues, options[:current_user]) }
       expose :runners_token, if: lambda { |_project, options| options[:user_can_admin_project] }
       expose :public_builds, as: :public_jobs
       expose :ci_config_path

lib/api/merge_requests.rb

@@ -29,14 +29,6 @@ module API
           render_api_error!(errors, 400)
         end
-        def issue_entity(project)
-          if project.has_external_issue_tracker?
-            Entities::ExternalIssue
-          else
-            Entities::IssueBasic
-          end
-        end
-
         def find_merge_requests(args = {})
           args = params.merge(args)
@@ -278,7 +270,14 @@ module API
       get ':id/merge_requests/:merge_request_iid/closes_issues' do
         merge_request = find_merge_request_with_access(params[:merge_request_iid])
         issues = ::Kaminari.paginate_array(merge_request.closes_issues(current_user))
-        present paginate(issues), with: issue_entity(user_project), current_user: current_user
+        issues = paginate(issues)
+
+        external_issues, internal_issues = issues.partition { |issue| issue.is_a?(ExternalIssue) }
+
+        data = Entities::IssueBasic.represent(internal_issues, current_user: current_user)
+        data += Entities::ExternalIssue.represent(external_issues, current_user: current_user)
+
+        data.as_json
       end
     end
   end

lib/banzai/filter/issue_reference_filter.rb

@@ -20,7 +20,7 @@ module Banzai
       end
       def url_for_object(issue, project)
-        IssuesHelper.url_for_issue(issue.iid, project, only_path: context[:only_path])
+        IssuesHelper.url_for_issue(issue.iid, project, only_path: context[:only_path], internal: true)
       end
       def project_from_ref(ref)

lib/banzai/reference_parser/external_issue_parser.rb

@@ -21,10 +21,14 @@ module Banzai
         gather_attributes_per_project(nodes, self.class.data_attribute)
       end
-      private
-
+      # we extract only external issue trackers references here, we don't extract cross-project references,
+      # so we don't need to do anything here.
       def can_read_reference?(user, ref_project, node)
-        can?(user, :read_issue, ref_project)
+        true
+      end
+
+      def nodes_visible_to_user(user, nodes)
+        nodes
       end
     end
   end

lib/gitlab/reference_extractor.rb

@@ -33,7 +33,12 @@ module Gitlab
     def issues
       if project && project.jira_tracker?
-        @references[:external_issue] ||= references(:external_issue)
+        if project.issues_enabled?
+          @references[:all_issues] ||= references(:external_issue) + references(:issue)
+        else
+          @references[:external_issue] ||= references(:external_issue) +
+            references(:issue).select { |i| i.project_id != project.id }
+        end
       else
         @references[:issue] ||= references(:issue)
       end

lib/gitlab/slash_commands/issue_command.rb

@@ -2,7 +2,7 @@ module Gitlab
   module SlashCommands
     class IssueCommand < BaseCommand
       def self.available?(project)
-        project.issues_enabled? && project.default_issues_tracker?
+        project.issues_enabled?
       end
       def collection

spec/features/issuables/markdown_references_spec.rb

+require 'rails_helper'
+
+describe 'Markdown References', :feature, :js do
+  let(:user)    { create(:user) }
+  let(:actual_project) { create(:project, :public) }
+  let(:merge_request) { create(:merge_request, target_project: actual_project, source_project: actual_project)}
+  let(:issue_actual_project) { create(:issue, project: actual_project) }
+  let!(:other_project) { create(:empty_project, :public) }
+  let!(:issue_other_project)   { create(:issue, project: other_project) }
+  let(:issues) { [issue_actual_project, issue_other_project] }
+
+  def build_note
+    markdown = "Referencing internal issue #{issue_actual_project.to_reference}, " +
+      "cross-project #{issue_other_project.to_reference(actual_project)} external JIRA-5 " +
+      "and non existing #999"
+
+    page.within('#diff-notes-app') do
+      fill_in 'note_note', with: markdown
+    end
+  end
+
+  shared_examples 'correct references' do
+    before do
+      remotelink = double(:remotelink, all: [], build: double(save!: true))
+
+      stub_request(:get, "https://jira.example.com/rest/api/2/issue/JIRA-5")
+      stub_request(:post, "https://jira.example.com/rest/api/2/issue/JIRA-5/comment")
+      allow_any_instance_of(JIRA::Resource::Issue).to receive(:remotelink).and_return(remotelink)
+
+      sign_in(user)
+      visit merge_request_path(merge_request)
+      build_note
+    end
+
+    def links_expectations
+      issues.each do |issue|
+        if referenced_issues.include?(issue)
+          expect(page).to have_link(issue.to_reference, href: issue_path(issue))
+        else
+          expect(page).not_to have_link(issue.to_reference, href: issue_path(issue))
+        end
+      end
+
+      if jira_referenced
+        expect(page).to have_link('JIRA-5', href: 'https://jira.example.com/browse/JIRA-5')
+      else
+        expect(page).not_to have_link('JIRA-5', href: 'https://jira.example.com/browse/JIRA-5')
+      end
+
+      expect(page).not_to have_link('#999')
+    end
+
+    it 'creates a link to the referenced issue on the preview' do
+      find('.js-md-preview-button').click
+      wait_for_requests
+
+      page.within('.md-preview-holder') do
+        links_expectations
+      end
+    end
+
+    it 'creates a link to the referenced issue after submit' do
+      click_button 'Comment'
+      wait_for_requests
+
+      page.within('#diff-notes-app') do
+        links_expectations
+      end
+    end
+
+    it 'creates a note on the referenced issues' do
+      click_button 'Comment'
+      wait_for_requests
+
+      if referenced_issues.include?(issue_actual_project)
+        visit issue_path(issue_actual_project)
+
+        page.within('#notes') do
+          expect(page).to have_content(
+            "#{user.to_reference} mentioned in merge request #{merge_request.to_reference}"
+          )
+        end
+      end
+
+      if referenced_issues.include?(issue_other_project)
+        visit issue_path(issue_other_project)
+
+        page.within('#notes') do
+          expect(page).to have_content(
+            "#{user.to_reference} mentioned in merge request #{merge_request.to_reference(other_project)}"
+          )
+        end
+      end
+    end
+  end
+
+  context 'when internal issues tracker is enabled for the other project' do
+    context 'when only internal issues tracker is enabled for the actual project' do
+      include_examples 'correct references' do
+        let(:referenced_issues) { [issue_actual_project, issue_other_project] }
+        let(:jira_referenced)   { false }
+      end
+    end
+
+    context 'when both external and internal issues trackers are enabled for the actual project' do
+      before do
+        create(:jira_service, project: actual_project)
+      end
+
+      include_examples 'correct references' do
+        let(:referenced_issues) { [issue_actual_project, issue_other_project] }
+        let(:jira_referenced)   { true }
+      end
+    end
+
+    context 'when only external issues tracker is enabled for the actual project' do
+      before do
+        create(:jira_service, project: actual_project)
+
+        actual_project.issues_enabled = false
+        actual_project.save!
+      end
+
+      include_examples 'correct references' do
+        let(:referenced_issues) { [issue_other_project] }
+        let(:jira_referenced)   { true }
+      end
+    end
+
+    context 'when no tracker is enabled for the actual project' do
+      before do
+        actual_project.issues_enabled = false
+        actual_project.save!
+      end
+
+      include_examples 'correct references' do
+        let(:referenced_issues) { [issue_other_project] }
+        let(:jira_referenced)   { false }
+      end
+    end
+  end
+
+  context 'when internal issues tracker is disabled for the other project' do
+    before do
+      other_project.issues_enabled = false
+      other_project.save!
+    end
+
+    context 'when only internal issues tracker is enabled for the actual project' do
+      include_examples 'correct references' do
+        let(:referenced_issues) { [issue_actual_project] }
+        let(:jira_referenced)   { false }
+      end
+    end
+
+    context 'when both external and internal issues trackers are enabled for the actual project' do
+      before do
+        create(:jira_service, project: actual_project)
+      end
+
+      include_examples 'correct references' do
+        let(:referenced_issues) { [issue_actual_project] }
+        let(:jira_referenced)   { true }
+      end
+    end
+
+    context 'when only external issues tracker is enabled for the actual project' do
+      before do
+        create(:jira_service, project: actual_project)
+
+        actual_project.issues_enabled = false
+        actual_project.save!
+      end
+
+      include_examples 'correct references' do
+        let(:referenced_issues) { [] }
+        let(:jira_referenced)   { true }
+      end
+    end
+
+    context 'when no issues tracker is enabled for the actual project' do
+      before do
+        actual_project.issues_enabled = false
+        actual_project.save!
+      end
+
+      include_examples 'correct references' do
+        let(:referenced_issues) { [] }
+        let(:jira_referenced)   { false }
+      end
+    end
+  end
+end

spec/features/projects/features_visibility_spec.rb

@@ -55,7 +55,7 @@ describe 'Edit Project Settings', feature: true do
         project.save!
         allow_any_instance_of(Project).to receive(:external_issue_tracker).and_return(JiraService.new)
-        visit namespace_project_path(project.namespace, project)
+        visit project_path(project)
         expect(page).not_to have_selector('.shortcuts-issues')
       end

spec/helpers/issues_helper_spec.rb

@@ -8,7 +8,7 @@ describe IssuesHelper do
   describe "url_for_issue" do
     let(:issues_url) { ext_project.external_issue_tracker.issues_url}
     let(:ext_expected) { issues_url.gsub(':id', issue.iid.to_s).gsub(':project_id', ext_project.id.to_s) }
-    let(:int_expected) { polymorphic_path([@project.namespace, project, issue]) }
+    let(:int_expected) { polymorphic_path([@project.namespace, @project, issue]) }
     it "returns internal path if used internal tracker" do
       @project = project
@@ -22,6 +22,12 @@ describe IssuesHelper do
       expect(url_for_issue(issue.iid)).to match(ext_expected)
     end
+    it "returns path to internal issue when internal option passed" do
+      @project = ext_project
+
+      expect(url_for_issue(issue.iid, ext_project, internal: true)).to match(int_expected)
+    end
+
     it "returns empty string if project nil" do
       @project = nil