Commit 6d13bc58 authored by Sean McGivern's avatar Sean McGivern

Merge branch '33097-issue-tracker' into 'master'

Associate Issues tab only with internal issues tracker

Closes #33097

See merge request !12130
parents 7eaef130 6774a037
...@@ -8,7 +8,6 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -8,7 +8,6 @@ class Projects::IssuesController < Projects::ApplicationController
prepend_before_action :authenticate_user!, only: [:new] prepend_before_action :authenticate_user!, only: [:new]
before_action :redirect_to_external_issue_tracker, only: [:index, :new]
before_action :check_issues_available! before_action :check_issues_available!
before_action :issue, except: [:index, :new, :create, :bulk_update] before_action :issue, except: [:index, :new, :create, :bulk_update]
...@@ -243,19 +242,19 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -243,19 +242,19 @@ class Projects::IssuesController < Projects::ApplicationController
end end
def authorize_update_issue! def authorize_update_issue!
return render_404 unless can?(current_user, :update_issue, @issue) render_404 unless can?(current_user, :update_issue, @issue)
end end
def authorize_admin_issues! def authorize_admin_issues!
return render_404 unless can?(current_user, :admin_issue, @project) render_404 unless can?(current_user, :admin_issue, @project)
end end
def authorize_create_merge_request! def authorize_create_merge_request!
return render_404 unless can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user) render_404 unless can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user)
end end
def check_issues_available! def check_issues_available!
return render_404 unless @project.feature_available?(:issues, current_user) && @project.default_issues_tracker? return render_404 unless @project.feature_available?(:issues, current_user)
end end
def redirect_to_external_issue_tracker def redirect_to_external_issue_tracker
......
...@@ -17,10 +17,10 @@ module IssuesHelper ...@@ -17,10 +17,10 @@ module IssuesHelper
return '' if project.nil? return '' if project.nil?
url = url =
if options[:only_path] if options[:internal]
project.issues_tracker.issue_path(issue_iid) url_for_internal_issue(issue_iid, project, options)
else else
project.issues_tracker.issue_url(issue_iid) url_for_tracker_issue(issue_iid, project, options)
end end
# Ensure we return a valid URL to prevent possible XSS. # Ensure we return a valid URL to prevent possible XSS.
...@@ -29,6 +29,24 @@ module IssuesHelper ...@@ -29,6 +29,24 @@ module IssuesHelper
'' ''
end 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 def bulk_update_milestone_options
milestones = @project.milestones.active.reorder(due_date: :asc, title: :asc).to_a milestones = @project.milestones.active.reorder(due_date: :asc, title: :asc).to_a
milestones.unshift(Milestone::None) milestones.unshift(Milestone::None)
...@@ -158,4 +176,6 @@ module IssuesHelper ...@@ -158,4 +176,6 @@ module IssuesHelper
# Required for Banzai::Filter::IssueReferenceFilter # Required for Banzai::Filter::IssueReferenceFilter
module_function :url_for_issue module_function :url_for_issue
module_function :url_for_internal_issue
module_function :url_for_tracker_issue
end end
...@@ -596,7 +596,7 @@ class MergeRequest < ActiveRecord::Base ...@@ -596,7 +596,7 @@ class MergeRequest < ActiveRecord::Base
# running `ReferenceExtractor` on each of them separately. # running `ReferenceExtractor` on each of them separately.
# This optimization does not apply to issues from external sources. # This optimization does not apply to issues from external sources.
def cache_merge_request_closes_issues!(current_user) def cache_merge_request_closes_issues!(current_user)
return if project.has_external_issue_tracker? return unless project.issues_enabled?
transaction do transaction do
self.merge_requests_closing_issues.delete_all self.merge_requests_closing_issues.delete_all
......
...@@ -734,9 +734,11 @@ class Project < ActiveRecord::Base ...@@ -734,9 +734,11 @@ class Project < ActiveRecord::Base
end end
def get_issue(issue_id, current_user) def get_issue(issue_id, current_user)
if default_issues_tracker? issue = IssuesFinder.new(current_user, project_id: id).find_by(iid: issue_id) if issues_enabled?
IssuesFinder.new(current_user, project_id: id).find_by(iid: issue_id)
else if issue
issue
elsif external_issue_tracker
ExternalIssue.new(issue_id, self) ExternalIssue.new(issue_id, self)
end end
end end
...@@ -758,7 +760,7 @@ class Project < ActiveRecord::Base ...@@ -758,7 +760,7 @@ class Project < ActiveRecord::Base
end end
def external_issue_reference_pattern def external_issue_reference_pattern
external_issue_tracker.class.reference_pattern external_issue_tracker.class.reference_pattern(only_long: issues_enabled?)
end end
def default_issues_tracker? def default_issues_tracker?
......
...@@ -8,8 +8,12 @@ class IssueTrackerService < Service ...@@ -8,8 +8,12 @@ class IssueTrackerService < Service
# This pattern does not support cross-project references # This pattern does not support cross-project references
# The other code assumes that this pattern is a superset of all # The other code assumes that this pattern is a superset of all
# overriden patterns. See ReferenceRegexes::EXTERNAL_PATTERN # overriden patterns. See ReferenceRegexes::EXTERNAL_PATTERN
def self.reference_pattern def self.reference_pattern(only_long: false)
@reference_pattern ||= %r{(\b[A-Z][A-Z0-9_]+-|#{Issue.reference_prefix})(?<issue>\d+)} 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 end
def default? def default?
......
...@@ -18,7 +18,7 @@ class JiraService < IssueTrackerService ...@@ -18,7 +18,7 @@ class JiraService < IssueTrackerService
end end
# {PROJECT-KEY}-{NUMBER} Examples: JIRA-1, PROJECT-1 # {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+)} @reference_pattern ||= %r{(?<issue>\b([A-Z][A-Z0-9_]+-)\d+)}
end end
......
...@@ -287,9 +287,6 @@ class ProjectPolicy < BasePolicy ...@@ -287,9 +287,6 @@ class ProjectPolicy < BasePolicy
prevent :create_issue prevent :create_issue
prevent :update_issue prevent :update_issue
prevent :admin_issue prevent :admin_issue
end
rule { issues_disabled & default_issues_tracker }.policy do
prevent :read_issue prevent :read_issue
end end
......
...@@ -16,13 +16,13 @@ module Issues ...@@ -16,13 +16,13 @@ module Issues
# The code calling this method is responsible for ensuring that a user is # The code calling this method is responsible for ensuring that a user is
# allowed to close the given issue. # allowed to close the given issue.
def close_issue(issue, commit: nil, notifications: true, system_note: true) 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) project.jira_service.close_issue(commit, issue)
todo_service.close_issue(issue, current_user) todo_service.close_issue(issue, current_user)
return issue return issue
end end
if project.default_issues_tracker? && issue.close if project.issues_enabled? && issue.close
event_service.close_issue(issue, current_user) event_service.close_issue(issue, current_user)
create_note(issue, commit) if system_note create_note(issue, commit) if system_note
notification_service.close_issue(issue, current_user) if notifications notification_service.close_issue(issue, current_user) if notifications
......
...@@ -75,10 +75,10 @@ ...@@ -75,10 +75,10 @@
Registry Registry
- if project_nav_tab? :issues - 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 = link_to project_issues_path(@project), title: 'Issues', class: 'shortcuts-issues' do
%span %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) %span.badge.count.issue_counter= number_with_delimiter(IssuesFinder.new(current_user, project_id: @project.id).execute.opened.count)
Issues Issues
...@@ -113,7 +113,7 @@ ...@@ -113,7 +113,7 @@
Milestones Milestones
- if project_nav_tab? :merge_requests - 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 = link_to project_merge_requests_path(@project), title: 'Merge Requests', class: 'shortcuts-merge_requests' do
%span %span
%span.badge.count.merge_counter.js-merge-counter= number_with_delimiter(MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.count) %span.badge.count.merge_counter.js-merge-counter= number_with_delimiter(MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.count)
......
...@@ -23,16 +23,16 @@ ...@@ -23,16 +23,16 @@
Registry Registry
- if project_nav_tab? :issues - 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 = link_to project_issues_path(@project), title: 'Issues', class: 'shortcuts-issues' do
%span %span
Issues 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))) %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 - if project_nav_tab? :merge_requests
- controllers = [:merge_requests, 'projects/merge_requests/conflicts'] - 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 = nav_link(controller: controllers) do
= link_to project_merge_requests_path(@project), title: 'Merge Requests', class: 'shortcuts-merge_requests' do = link_to project_merge_requests_path(@project), title: 'Merge Requests', class: 'shortcuts-merge_requests' do
%span %span
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
- new_merge_request_path = project_new_merge_request_path(merge_project) if merge_project - new_merge_request_path = project_new_merge_request_path(merge_project) if merge_project
- page_title "Merge Requests" - page_title "Merge Requests"
- unless @project.default_issues_tracker? - unless @project.issues_enabled?
= content_for :sub_nav do = content_for :sub_nav do
= render "projects/merge_requests/head" = render "projects/merge_requests/head"
......
- if @project.default_issues_tracker? - if @project.issues_enabled?
= render "projects/issues/head" = render "projects/issues/head"
- else - else
= render "projects/merge_requests/head" = render "projects/merge_requests/head"
---
title: Associate Issues tab only with internal issues tracker
merge_request:
author:
...@@ -4,14 +4,12 @@ GitLab has a great issue tracker but you can also use an external one such as ...@@ -4,14 +4,12 @@ GitLab has a great issue tracker but you can also use an external one such as
Jira, Redmine, or Bugzilla. Issue trackers are configurable per GitLab project and allow Jira, Redmine, or Bugzilla. Issue trackers are configurable per GitLab project and allow
you to do the following: you to do the following:
- the **Issues** link on the GitLab project pages takes you to the appropriate
issue index of the external tracker
- clicking **New issue** on the project dashboard creates a new issue on the
external tracker
- you can reference these external issues inside GitLab interface - you can reference these external issues inside GitLab interface
(merge requests, commits, comments) and they will be automatically converted (merge requests, commits, comments) and they will be automatically converted
into links into links
You can have enabled both external and internal GitLab issue trackers in parallel. The **Issues** link always opens the internal issue tracker and in case the internal issue tracker is disabled the link is not visible in the menu.
## Configuration ## Configuration
The configuration is done via a project's **Services**. The configuration is done via a project's **Services**.
......
...@@ -20,10 +20,12 @@ Once you have configured and enabled Bugzilla: ...@@ -20,10 +20,12 @@ Once you have configured and enabled Bugzilla:
## Referencing issues in Bugzilla ## Referencing issues in Bugzilla
Issues in Bugzilla can be referenced in two alternative ways: Issues in Bugzilla can be referenced in two alternative ways:
1. `#<ID>` where `<ID>` is a number (example `#143`) 1. `#<ID>` where `<ID>` is a number (example `#143`).
2. `<PROJECT>-<ID>` where `<PROJECT>` starts with a capital letter which is 2. `<PROJECT>-<ID>` where `<PROJECT>` starts with a capital letter which is
then followed by capital letters, numbers or underscores, and `<ID>` is then followed by capital letters, numbers or underscores, and `<ID>` is
a number (example `API_32-143`). a number (example `API_32-143`).
We suggest using the longer format if you have both internal and external issue trackers enabled. If you use the shorter format and an issue with the same ID exists in the internal issue tracker the internal issue will be linked.
Please note that `<PROJECT>` part is ignored and links always point to the Please note that `<PROJECT>` part is ignored and links always point to the
address specified in `issues_url`. address specified in `issues_url`.
...@@ -30,5 +30,7 @@ Issues in Redmine can be referenced in two alternative ways: ...@@ -30,5 +30,7 @@ Issues in Redmine can be referenced in two alternative ways:
then followed by capital letters, numbers or underscores, and `<ID>` is then followed by capital letters, numbers or underscores, and `<ID>` is
a number (example `API_32-143`). a number (example `API_32-143`).
We suggest using the longer format if you have both internal and external issue trackers enabled. If you use the shorter format and an issue with the same ID exists in the internal issue tracker the internal issue will be linked.
Please note that `<PROJECT>` part is ignored and links always point to the Please note that `<PROJECT>` part is ignored and links always point to the
address specified in `issues_url`. address specified in `issues_url`.
...@@ -109,7 +109,7 @@ module API ...@@ -109,7 +109,7 @@ module API
user.avatar_url(only_path: false) user.avatar_url(only_path: false)
end end
expose :star_count, :forks_count 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 :runners_token, if: lambda { |_project, options| options[:user_can_admin_project] }
expose :public_builds, as: :public_jobs expose :public_builds, as: :public_jobs
expose :ci_config_path expose :ci_config_path
......
...@@ -29,14 +29,6 @@ module API ...@@ -29,14 +29,6 @@ module API
render_api_error!(errors, 400) render_api_error!(errors, 400)
end end
def issue_entity(project)
if project.has_external_issue_tracker?
Entities::ExternalIssue
else
Entities::IssueBasic
end
end
def find_merge_requests(args = {}) def find_merge_requests(args = {})
args = params.merge(args) args = params.merge(args)
...@@ -278,7 +270,14 @@ module API ...@@ -278,7 +270,14 @@ module API
get ':id/merge_requests/:merge_request_iid/closes_issues' do get ':id/merge_requests/:merge_request_iid/closes_issues' do
merge_request = find_merge_request_with_access(params[:merge_request_iid]) merge_request = find_merge_request_with_access(params[:merge_request_iid])
issues = ::Kaminari.paginate_array(merge_request.closes_issues(current_user)) 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 end
end end
......
...@@ -20,7 +20,7 @@ module Banzai ...@@ -20,7 +20,7 @@ module Banzai
end end
def url_for_object(issue, project) 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 end
def project_from_ref(ref) def project_from_ref(ref)
......
...@@ -21,10 +21,14 @@ module Banzai ...@@ -21,10 +21,14 @@ module Banzai
gather_attributes_per_project(nodes, self.class.data_attribute) gather_attributes_per_project(nodes, self.class.data_attribute)
end 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) 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 end
end end
......
...@@ -33,7 +33,12 @@ module Gitlab ...@@ -33,7 +33,12 @@ module Gitlab
def issues def issues
if project && project.jira_tracker? 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 else
@references[:issue] ||= references(:issue) @references[:issue] ||= references(:issue)
end end
......
...@@ -2,7 +2,7 @@ module Gitlab ...@@ -2,7 +2,7 @@ module Gitlab
module SlashCommands module SlashCommands
class IssueCommand < BaseCommand class IssueCommand < BaseCommand
def self.available?(project) def self.available?(project)
project.issues_enabled? && project.default_issues_tracker? project.issues_enabled?
end end
def collection def collection
......
...@@ -7,16 +7,30 @@ describe Projects::IssuesController do ...@@ -7,16 +7,30 @@ describe Projects::IssuesController do
describe "GET #index" do describe "GET #index" do
context 'external issue tracker' do context 'external issue tracker' do
let!(:service) do before do
create(:custom_issue_tracker_service, project: project, title: 'Custom Issue Tracker', project_url: 'http://test.com') sign_in(user)
project.add_developer(user)
create(:jira_service, project: project)
end end
it 'redirects to the external issue tracker' do context 'when GitLab issues disabled' do
controller.instance_variable_set(:@project, project) it 'returns 404 status' do
project.issues_enabled = false
project.save!
get :index, namespace_id: project.namespace, project_id: project
expect(response).to have_http_status(404)
end
end
context 'when GitLab issues enabled' do
it 'renders the "index" template' do
get :index, namespace_id: project.namespace, project_id: project get :index, namespace_id: project.namespace, project_id: project
expect(response).to redirect_to(service.issue_tracker_path) expect(response).to have_http_status(200)
expect(response).to render_template(:index)
end
end end
end end
...@@ -42,15 +56,7 @@ describe Projects::IssuesController do ...@@ -42,15 +56,7 @@ describe Projects::IssuesController do
it "returns 404 when issues are disabled" do it "returns 404 when issues are disabled" do
project.issues_enabled = false project.issues_enabled = false
project.save project.save!
get :index, namespace_id: project.namespace, project_id: project
expect(response).to have_http_status(404)
end
it "returns 404 when external issue tracker is enabled" do
controller.instance_variable_set(:@project, project)
allow(project).to receive(:default_issues_tracker?).and_return(false)
get :index, namespace_id: project.namespace, project_id: project get :index, namespace_id: project.namespace, project_id: project
expect(response).to have_http_status(404) expect(response).to have_http_status(404)
...@@ -148,14 +154,29 @@ describe Projects::IssuesController do ...@@ -148,14 +154,29 @@ describe Projects::IssuesController do
before do before do
sign_in(user) sign_in(user)
project.team << [user, :developer] project.team << [user, :developer]
external = double
allow(project).to receive(:external_issue_tracker).and_return(external)
end end
it 'redirects to the external issue tracker' do context 'when GitLab issues disabled' do
controller.instance_variable_set(:@project, project) it 'returns 404 status' do
project.issues_enabled = false
project.save!
get :new, namespace_id: project.namespace, project_id: project
expect(response).to have_http_status(404)
end
end
context 'when GitLab issues enabled' do
it 'renders the "new" template' do
get :new, namespace_id: project.namespace, project_id: project get :new, namespace_id: project.namespace, project_id: project
expect(response).to redirect_to('http://test.com') expect(response).to have_http_status(200)
expect(response).to render_template(:new)
end
end end
end end
end end
......
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
...@@ -39,14 +39,25 @@ describe 'Edit Project Settings', feature: true do ...@@ -39,14 +39,25 @@ describe 'Edit Project Settings', feature: true do
end end
end end
context "When external issue tracker is enabled" do context 'When external issue tracker is enabled and issues enabled on project settings' do
it "does not hide issues tab" do it 'does not hide issues tab' do
project.project_feature.update(issues_access_level: ProjectFeature::DISABLED)
allow_any_instance_of(Project).to receive(:external_issue_tracker).and_return(JiraService.new) allow_any_instance_of(Project).to receive(:external_issue_tracker).and_return(JiraService.new)
visit project_path(project) visit project_path(project)
expect(page).to have_selector(".shortcuts-issues") expect(page).to have_selector('.shortcuts-issues')
end
end
context 'When external issue tracker is enabled and issues disabled on project settings' do
it 'hides issues tab' do
project.issues_enabled = false
project.save!
allow_any_instance_of(Project).to receive(:external_issue_tracker).and_return(JiraService.new)
visit project_path(project)
expect(page).not_to have_selector('.shortcuts-issues')
end end
end end
......
...@@ -8,7 +8,7 @@ describe IssuesHelper do ...@@ -8,7 +8,7 @@ describe IssuesHelper do
describe "url_for_issue" do describe "url_for_issue" do
let(:issues_url) { ext_project.external_issue_tracker.issues_url} 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(: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 it "returns internal path if used internal tracker" do
@project = project @project = project
...@@ -22,6 +22,12 @@ describe IssuesHelper do ...@@ -22,6 +22,12 @@ describe IssuesHelper do
expect(url_for_issue(issue.iid)).to match(ext_expected) expect(url_for_issue(issue.iid)).to match(ext_expected)
end 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 it "returns empty string if project nil" do
@project = nil @project = nil
......
...@@ -108,6 +108,11 @@ describe Banzai::Filter::ExternalIssueReferenceFilter, lib: true do ...@@ -108,6 +108,11 @@ describe Banzai::Filter::ExternalIssueReferenceFilter, lib: true do
let(:issue) { ExternalIssue.new("#123", project) } let(:issue) { ExternalIssue.new("#123", project) }
let(:reference) { issue.to_reference } let(:reference) { issue.to_reference }
before do
project.issues_enabled = false
project.save!
end
it_behaves_like "external issue tracker" it_behaves_like "external issue tracker"
end end
......
...@@ -4,6 +4,57 @@ describe Banzai::Pipeline::GfmPipeline do ...@@ -4,6 +4,57 @@ describe Banzai::Pipeline::GfmPipeline do
describe 'integration between parsing regular and external issue references' do describe 'integration between parsing regular and external issue references' do
let(:project) { create(:redmine_project, :public) } let(:project) { create(:redmine_project, :public) }
context 'when internal issue tracker is enabled' do
context 'when shorthand pattern #ISSUE_ID is used' do
it 'links an internal issue if it exists' do
issue = create(:issue, project: project)
markdown = issue.to_reference(project, full: true)
result = described_class.call(markdown, project: project)[:output]
link = result.css('a').first
expect(link['href']).to eq(
Gitlab::Routing.url_helpers.project_issue_path(project, issue)
)
end
it 'does not link any issue if it does not exist on GitLab' do
markdown = '#12'
result = described_class.call(markdown, project: project)[:output]
expect(result.css('a')).to be_empty
end
end
it 'allows to use long external reference syntax for Redmine' do
markdown = 'API_32-12'
result = described_class.call(markdown, project: project)[:output]
link = result.css('a').first
expect(link['href']).to eq 'http://redmine/projects/project_name_in_redmine/issues/12'
end
it 'parses cross-project references to regular issues' do
other_project = create(:empty_project, :public)
issue = create(:issue, project: other_project)
markdown = issue.to_reference(project, full: true)
result = described_class.call(markdown, project: project)[:output]
link = result.css('a').first
expect(link['href']).to eq(
Gitlab::Routing.url_helpers.project_issue_path(other_project, issue)
)
end
end
context 'when internal issue tracker is disabled' do
before do
project.issues_enabled = false
project.save!
end
it 'allows to use shorthand external reference syntax for Redmine' do it 'allows to use shorthand external reference syntax for Redmine' do
markdown = '#12' markdown = '#12'
...@@ -13,6 +64,15 @@ describe Banzai::Pipeline::GfmPipeline do ...@@ -13,6 +64,15 @@ describe Banzai::Pipeline::GfmPipeline do
expect(link['href']).to eq 'http://redmine/projects/project_name_in_redmine/issues/12' expect(link['href']).to eq 'http://redmine/projects/project_name_in_redmine/issues/12'
end end
it 'allows to use long external reference syntax for Redmine' do
markdown = 'API_32-12'
result = described_class.call(markdown, project: project)[:output]
link = result.css('a').first
expect(link['href']).to eq 'http://redmine/projects/project_name_in_redmine/issues/12'
end
it 'parses cross-project references to regular issues' do it 'parses cross-project references to regular issues' do
other_project = create(:empty_project, :public) other_project = create(:empty_project, :public)
issue = create(:issue, project: other_project) issue = create(:issue, project: other_project)
...@@ -26,4 +86,5 @@ describe Banzai::Pipeline::GfmPipeline do ...@@ -26,4 +86,5 @@ describe Banzai::Pipeline::GfmPipeline do
) )
end end
end end
end
end end
...@@ -183,14 +183,37 @@ describe Gitlab::ReferenceExtractor, lib: true do ...@@ -183,14 +183,37 @@ describe Gitlab::ReferenceExtractor, lib: true do
context 'with an external issue tracker' do context 'with an external issue tracker' do
let(:project) { create(:jira_project) } let(:project) { create(:jira_project) }
let(:issue) { create(:issue, project: project) }
context 'when GitLab issues are enabled' do
it 'returns both JIRA and internal issues' do
subject.analyze("JIRA-123 and FOOBAR-4567 and #{issue.to_reference}")
expect(subject.issues).to eq [ExternalIssue.new('JIRA-123', project),
ExternalIssue.new('FOOBAR-4567', project),
issue]
end
it 'returns JIRA issues for a JIRA-integrated project' do it 'returns only JIRA issues if the internal one does not exists' do
subject.analyze('JIRA-123 and FOOBAR-4567') subject.analyze("JIRA-123 and FOOBAR-4567 and #999")
expect(subject.issues).to eq [ExternalIssue.new('JIRA-123', project), expect(subject.issues).to eq [ExternalIssue.new('JIRA-123', project),
ExternalIssue.new('FOOBAR-4567', project)] ExternalIssue.new('FOOBAR-4567', project)]
end end
end end
context 'when GitLab issues are disabled' do
before do
project.issues_enabled = false
project.save!
end
it 'returns only JIRA issues' do
subject.analyze("JIRA-123 and FOOBAR-4567 and #{issue.to_reference}")
expect(subject.issues).to eq [ExternalIssue.new('JIRA-123', project),
ExternalIssue.new('FOOBAR-4567', project)]
end
end
end
context 'with a project with an underscore' do context 'with a project with an underscore' do
let(:other_project) { create(:empty_project, path: 'test_project') } let(:other_project) { create(:empty_project, path: 'test_project') }
let(:issue) { create(:issue, project: other_project) } let(:issue) { create(:issue, project: other_project) }
......
...@@ -174,25 +174,25 @@ describe Commit, 'Mentionable' do ...@@ -174,25 +174,25 @@ describe Commit, 'Mentionable' do
it "is false when message doesn't reference anything" do it "is false when message doesn't reference anything" do
allow(commit.raw).to receive(:message).and_return "WIP: Do something" allow(commit.raw).to receive(:message).and_return "WIP: Do something"
expect(commit.matches_cross_reference_regex?).to be false expect(commit.matches_cross_reference_regex?).to be_falsey
end end
it 'is true if issue #number mentioned in title' do it 'is true if issue #number mentioned in title' do
allow(commit.raw).to receive(:message).and_return "#1" allow(commit.raw).to receive(:message).and_return "#1"
expect(commit.matches_cross_reference_regex?).to be true expect(commit.matches_cross_reference_regex?).to be_truthy
end end
it 'is true if references an MR' do it 'is true if references an MR' do
allow(commit.raw).to receive(:message).and_return "See merge request !12" allow(commit.raw).to receive(:message).and_return "See merge request !12"
expect(commit.matches_cross_reference_regex?).to be true expect(commit.matches_cross_reference_regex?).to be_truthy
end end
it 'is true if references a commit' do it 'is true if references a commit' do
allow(commit.raw).to receive(:message).and_return "a1b2c3d4" allow(commit.raw).to receive(:message).and_return "a1b2c3d4"
expect(commit.matches_cross_reference_regex?).to be true expect(commit.matches_cross_reference_regex?).to be_truthy
end end
it 'is true if issue referenced by url' do it 'is true if issue referenced by url' do
...@@ -200,7 +200,7 @@ describe Commit, 'Mentionable' do ...@@ -200,7 +200,7 @@ describe Commit, 'Mentionable' do
allow(commit.raw).to receive(:message).and_return Gitlab::UrlBuilder.build(issue) allow(commit.raw).to receive(:message).and_return Gitlab::UrlBuilder.build(issue)
expect(commit.matches_cross_reference_regex?).to be true expect(commit.matches_cross_reference_regex?).to be_truthy
end end
context 'with external issue tracker' do context 'with external issue tracker' do
...@@ -209,7 +209,13 @@ describe Commit, 'Mentionable' do ...@@ -209,7 +209,13 @@ describe Commit, 'Mentionable' do
it 'is true if external issues referenced' do it 'is true if external issues referenced' do
allow(commit.raw).to receive(:message).and_return 'JIRA-123' allow(commit.raw).to receive(:message).and_return 'JIRA-123'
expect(commit.matches_cross_reference_regex?).to be true expect(commit.matches_cross_reference_regex?).to be_truthy
end
it 'is true if internal issues referenced' do
allow(commit.raw).to receive(:message).and_return '#123'
expect(commit.matches_cross_reference_regex?).to be_truthy
end end
end end
end end
......
...@@ -155,14 +155,54 @@ describe MergeRequest, models: true do ...@@ -155,14 +155,54 @@ describe MergeRequest, models: true do
expect { subject.cache_merge_request_closes_issues!(subject.author) }.to change(subject.merge_requests_closing_issues, :count).by(1) expect { subject.cache_merge_request_closes_issues!(subject.author) }.to change(subject.merge_requests_closing_issues, :count).by(1)
end end
context 'when both internal and external issue trackers are enabled' do
before do
subject.project.has_external_issue_tracker = true
subject.project.save!
end
it 'does not cache issues from external trackers' do
issue = ExternalIssue.new('JIRA-123', subject.project)
commit = double('commit1', safe_message: "Fixes #{issue.to_reference}")
allow(subject).to receive(:commits).and_return([commit])
expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to change(subject.merge_requests_closing_issues, :count)
end
it 'caches an internal issue' do
issue = create(:issue, project: subject.project)
commit = double('commit1', safe_message: "Fixes #{issue.to_reference}")
allow(subject).to receive(:commits).and_return([commit])
expect { subject.cache_merge_request_closes_issues!(subject.author) }
.to change(subject.merge_requests_closing_issues, :count).by(1)
end
end
context 'when only external issue tracker enabled' do
before do
subject.project.has_external_issue_tracker = true
subject.project.issues_enabled = false
subject.project.save!
end
it 'does not cache issues from external trackers' do it 'does not cache issues from external trackers' do
subject.project.update_attribute(:has_external_issue_tracker, true)
issue = ExternalIssue.new('JIRA-123', subject.project) issue = ExternalIssue.new('JIRA-123', subject.project)
commit = double('commit1', safe_message: "Fixes #{issue.to_reference}") commit = double('commit1', safe_message: "Fixes #{issue.to_reference}")
allow(subject).to receive(:commits).and_return([commit]) allow(subject).to receive(:commits).and_return([commit])
expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to change(subject.merge_requests_closing_issues, :count) expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to change(subject.merge_requests_closing_issues, :count)
end end
it 'does not cache an internal issue' do
issue = create(:issue, project: subject.project)
commit = double('commit1', safe_message: "Fixes #{issue.to_reference}")
allow(subject).to receive(:commits).and_return([commit])
expect { subject.cache_merge_request_closes_issues!(subject.author) }
.not_to change(subject.merge_requests_closing_issues, :count)
end
end
end end
describe '#source_branch_sha' do describe '#source_branch_sha' do
......
...@@ -533,15 +533,48 @@ describe Project, models: true do ...@@ -533,15 +533,48 @@ describe Project, models: true do
end end
context 'with external issues tracker' do context 'with external issues tracker' do
let!(:internal_issue) { create(:issue, project: project) }
before do before do
allow(project).to receive(:default_issues_tracker?).and_return(false) allow(project).to receive(:external_issue_tracker).and_return(true)
end end
it 'returns an ExternalIssue' do context 'when internal issues are enabled' do
it 'returns interlan issue' do
issue = project.get_issue(internal_issue.iid, user)
expect(issue).to be_kind_of(Issue)
expect(issue.iid).to eq(internal_issue.iid)
expect(issue.project).to eq(project)
end
it 'returns an ExternalIssue when internal issue does not exists' do
issue = project.get_issue('FOO-1234', user)
expect(issue).to be_kind_of(ExternalIssue)
expect(issue.iid).to eq('FOO-1234')
expect(issue.project).to eq(project)
end
end
context 'when internal issues are disabled' do
before do
project.issues_enabled = false
project.save!
end
it 'returns always an External issues' do
issue = project.get_issue(internal_issue.iid, user)
expect(issue).to be_kind_of(ExternalIssue)
expect(issue.iid).to eq(internal_issue.iid.to_s)
expect(issue.project).to eq(project)
end
it 'returns an ExternalIssue when internal issue does not exists' do
issue = project.get_issue('FOO-1234', user) issue = project.get_issue('FOO-1234', user)
expect(issue).to be_kind_of(ExternalIssue) expect(issue).to be_kind_of(ExternalIssue)
expect(issue.iid).to eq 'FOO-1234' expect(issue.iid).to eq('FOO-1234')
expect(issue.project).to eq project expect(issue.project).to eq(project)
end
end end
end end
end end
......
...@@ -103,6 +103,30 @@ describe ProjectPolicy, models: true do ...@@ -103,6 +103,30 @@ describe ProjectPolicy, models: true do
end end
end end
context 'issues feature' do
subject { described_class.new(owner, project) }
context 'when the feature is disabled' do
it 'does not include the issues permissions' do
project.issues_enabled = false
project.save!
expect_disallowed :read_issue, :create_issue, :update_issue, :admin_issue
end
end
context 'when the feature is disabled and external tracker configured' do
it 'does not include the issues permissions' do
create(:jira_service, project: project)
project.issues_enabled = false
project.save!
expect_disallowed :read_issue, :create_issue, :update_issue, :admin_issue
end
end
end
context 'abilities for non-public projects' do context 'abilities for non-public projects' do
let(:project) { create(:empty_project, namespace: owner.namespace) } let(:project) { create(:empty_project, namespace: owner.namespace) }
......
...@@ -794,18 +794,24 @@ describe API::MergeRequests do ...@@ -794,18 +794,24 @@ describe API::MergeRequests do
it 'handles external issues' do it 'handles external issues' do
jira_project = create(:jira_project, :public, name: 'JIR_EXT1') jira_project = create(:jira_project, :public, name: 'JIR_EXT1')
issue = ExternalIssue.new("#{jira_project.name}-123", jira_project) ext_issue = ExternalIssue.new("#{jira_project.name}-123", jira_project)
merge_request = create(:merge_request, :simple, author: user, assignee: user, source_project: jira_project) issue = create(:issue, project: jira_project)
merge_request.update_attribute(:description, "Closes #{issue.to_reference(jira_project)}") description = "Closes #{ext_issue.to_reference(jira_project)}\ncloses #{issue.to_reference}"
merge_request = create(:merge_request,
:simple, author: user, assignee: user, source_project: jira_project, description: description)
get api("/projects/#{jira_project.id}/merge_requests/#{merge_request.iid}/closes_issues", user) get api("/projects/#{jira_project.id}/merge_requests/#{merge_request.iid}/closes_issues", user)
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(response).to include_pagination_headers expect(response).to include_pagination_headers
expect(json_response).to be_an Array expect(json_response).to be_an Array
expect(json_response.length).to eq(1) expect(json_response.length).to eq(2)
expect(json_response.second['title']).to eq(ext_issue.title)
expect(json_response.second['id']).to eq(ext_issue.id)
expect(json_response.second['confidential']).to be_nil
expect(json_response.first['title']).to eq(issue.title) expect(json_response.first['title']).to eq(issue.title)
expect(json_response.first['id']).to eq(issue.id) expect(json_response.first['id']).to eq(issue.id)
expect(json_response.first['confidential']).not_to be_nil
end end
it 'returns 403 if the user has no access to the merge request' do it 'returns 403 if the user has no access to the merge request' do
......
...@@ -159,6 +159,31 @@ describe API::Projects do ...@@ -159,6 +159,31 @@ describe API::Projects do
expect(json_response.first).to include 'statistics' expect(json_response.first).to include 'statistics'
end end
context 'when external issue tracker is enabled' do
let!(:jira_service) { create(:jira_service, project: project) }
it 'includes open_issues_count' do
get api('/projects', user)
expect(response.status).to eq 200
expect(response).to include_pagination_headers
expect(json_response).to be_an Array
expect(json_response.first.keys).to include('open_issues_count')
expect(json_response.find { |hash| hash['id'] == project.id }.keys).to include('open_issues_count')
end
it 'does not include open_issues_count if issues are disabled' do
project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED)
get api('/projects', user)
expect(response.status).to eq 200
expect(response).to include_pagination_headers
expect(json_response).to be_an Array
expect(json_response.find { |hash| hash['id'] == project.id }.keys).not_to include('open_issues_count')
end
end
context 'and with simple=true' do context 'and with simple=true' do
it 'returns a simplified version of all the projects' do it 'returns a simplified version of all the projects' do
expected_keys = %w(id http_url_to_repo web_url name name_with_namespace path path_with_namespace) expected_keys = %w(id http_url_to_repo web_url name name_with_namespace path path_with_namespace)
......
...@@ -488,7 +488,12 @@ describe GitPushService, services: true do ...@@ -488,7 +488,12 @@ describe GitPushService, services: true do
end end
end end
context "using wrong markdown" do context "using internal issue reference" do
context 'when internal issues are disabled' do
before do
project.issues_enabled = false
project.save!
end
let(:message) { "this is some work.\n\ncloses #1" } let(:message) { "this is some work.\n\ncloses #1" }
it "does not initiates one api call to jira server to close the issue" do it "does not initiates one api call to jira server to close the issue" do
...@@ -505,6 +510,37 @@ describe GitPushService, services: true do ...@@ -505,6 +510,37 @@ describe GitPushService, services: true do
).once ).once
end end
end end
context 'when internal issues are enabled' do
let(:issue) { create(:issue, project: project) }
let(:message) { "this is some work.\n\ncloses JIRA-1 \n\n closes #{issue.to_reference}" }
it "initiates one api call to jira server to close the jira issue" do
execute_service(project, commit_author, @oldrev, @newrev, @ref )
expect(WebMock).to have_requested(:post, jira_api_transition_url('JIRA-1')).once
end
it "initiates one api call to jira server to comment on the jira issue" do
execute_service(project, commit_author, @oldrev, @newrev, @ref )
expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with(
body: comment_body
).once
end
it "closes the internal issue" do
execute_service(project, commit_author, @oldrev, @newrev, @ref )
expect(issue.reload).to be_closed
end
it "adds a note indicating that the issue is now closed" do
expect(SystemNoteService).to receive(:change_status)
.with(issue, project, commit_author, "closed", closing_commit)
execute_service(project, commit_author, @oldrev, @newrev, @ref )
end
end
end
end end
end end
end end
......
...@@ -98,13 +98,13 @@ describe Issues::CloseService, services: true do ...@@ -98,13 +98,13 @@ describe Issues::CloseService, services: true do
end end
end end
context 'external issue tracker' do context 'internal issues disabled' do
before do before do
allow(project).to receive(:default_issues_tracker?).and_return(false) project.issues_enabled = false
described_class.new(project, user).close_issue(issue) project.save!
end end
it 'closes the issue' do it 'does not close the issue' do
expect(issue).to be_valid expect(issue).to be_valid
expect(issue).to be_opened expect(issue).to be_opened
expect(todo.reload).to be_pending expect(todo.reload).to be_pending
......
...@@ -207,7 +207,7 @@ describe MergeRequests::BuildService, services: true do ...@@ -207,7 +207,7 @@ describe MergeRequests::BuildService, services: true do
let(:source_branch) { '12345-fix-issue' } let(:source_branch) { '12345-fix-issue' }
before do before do
allow(project).to receive(:default_issues_tracker?).and_return(false) allow(project).to receive(:external_issue_tracker).and_return(true)
end end
it 'sets the title to: Resolves External Issue $issue-iid' do it 'sets the title to: Resolves External Issue $issue-iid' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment