Remove url_for_new_issue
helper
Now we link to the standard `IssuesController#new` action, and let it redirect if we're using an external tracker.
This commit is contained in:
parent
a70431f874
commit
901d4d2ca5
5 changed files with 24 additions and 66 deletions
|
@ -5,7 +5,7 @@ class Projects::IssuesController < Projects::ApplicationController
|
||||||
include ToggleAwardEmoji
|
include ToggleAwardEmoji
|
||||||
include IssuableCollections
|
include IssuableCollections
|
||||||
|
|
||||||
before_action :redirect_to_external_issue_tracker, only: [:index]
|
before_action :redirect_to_external_issue_tracker, only: [:index, :new]
|
||||||
before_action :module_enabled
|
before_action :module_enabled
|
||||||
before_action :issue, only: [:edit, :update, :show, :referenced_merge_requests,
|
before_action :issue, only: [:edit, :update, :show, :referenced_merge_requests,
|
||||||
:related_branches, :can_create_branch]
|
:related_branches, :can_create_branch]
|
||||||
|
@ -203,9 +203,15 @@ class Projects::IssuesController < Projects::ApplicationController
|
||||||
end
|
end
|
||||||
|
|
||||||
def redirect_to_external_issue_tracker
|
def redirect_to_external_issue_tracker
|
||||||
return unless @project.external_issue_tracker
|
external = @project.external_issue_tracker
|
||||||
|
|
||||||
redirect_to @project.external_issue_tracker.issues_url
|
return unless external
|
||||||
|
|
||||||
|
if action_name == 'new'
|
||||||
|
redirect_to external.new_issue_path
|
||||||
|
else
|
||||||
|
redirect_to external.issues_url
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# Since iids are implemented only in 6.1
|
# Since iids are implemented only in 6.1
|
||||||
|
|
|
@ -13,22 +13,6 @@ module IssuesHelper
|
||||||
OpenStruct.new(id: 0, title: 'None (backlog)', name: 'Unassigned')
|
OpenStruct.new(id: 0, title: 'None (backlog)', name: 'Unassigned')
|
||||||
end
|
end
|
||||||
|
|
||||||
def url_for_new_issue(project = @project, options = {})
|
|
||||||
return '' if project.nil?
|
|
||||||
|
|
||||||
url =
|
|
||||||
if options[:only_path]
|
|
||||||
project.issues_tracker.new_issue_path
|
|
||||||
else
|
|
||||||
project.issues_tracker.new_issue_url
|
|
||||||
end
|
|
||||||
|
|
||||||
# Ensure we return a valid URL to prevent possible XSS.
|
|
||||||
URI.parse(url).to_s
|
|
||||||
rescue URI::InvalidURIError
|
|
||||||
''
|
|
||||||
end
|
|
||||||
|
|
||||||
def url_for_issue(issue_iid, project = @project, options = {})
|
def url_for_issue(issue_iid, project = @project, options = {})
|
||||||
return '' if project.nil?
|
return '' if project.nil?
|
||||||
|
|
||||||
|
|
|
@ -9,7 +9,7 @@
|
||||||
|
|
||||||
- if can_create_issue
|
- if can_create_issue
|
||||||
%li
|
%li
|
||||||
= link_to url_for_new_issue(@project, only_path: true) do
|
= link_to new_namespace_project_issue_path(@project.namespace, @project) do
|
||||||
= icon('exclamation-circle fw')
|
= icon('exclamation-circle fw')
|
||||||
New issue
|
New issue
|
||||||
|
|
||||||
|
|
|
@ -54,6 +54,20 @@ describe Projects::IssuesController do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe 'GET #new' do
|
||||||
|
context 'external issue tracker' do
|
||||||
|
it 'redirects to the external issue tracker' do
|
||||||
|
external = double(new_issue_path: 'https://example.com/issues/new')
|
||||||
|
allow(project).to receive(:external_issue_tracker).and_return(external)
|
||||||
|
controller.instance_variable_set(:@project, project)
|
||||||
|
|
||||||
|
get :new, namespace_id: project.namespace.path, project_id: project
|
||||||
|
|
||||||
|
expect(response).to redirect_to('https://example.com/issues/new')
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe 'PUT #update' do
|
describe 'PUT #update' do
|
||||||
context 'when moving issue to another private project' do
|
context 'when moving issue to another private project' do
|
||||||
let(:another_project) { create(:project, :private) }
|
let(:another_project) { create(:project, :private) }
|
||||||
|
|
|
@ -51,52 +51,6 @@ describe IssuesHelper do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'url_for_new_issue' do
|
|
||||||
let(:issues_url) { ext_project.external_issue_tracker.new_issue_url }
|
|
||||||
let(:ext_expected) { issues_url.gsub(':project_id', ext_project.id.to_s) }
|
|
||||||
let(:int_expected) { new_namespace_project_issue_path(project.namespace, project) }
|
|
||||||
|
|
||||||
it "should return internal path if used internal tracker" do
|
|
||||||
@project = project
|
|
||||||
expect(url_for_new_issue).to match(int_expected)
|
|
||||||
end
|
|
||||||
|
|
||||||
it "should return path to external tracker" do
|
|
||||||
@project = ext_project
|
|
||||||
|
|
||||||
expect(url_for_new_issue).to match(ext_expected)
|
|
||||||
end
|
|
||||||
|
|
||||||
it "should return empty string if project nil" do
|
|
||||||
@project = nil
|
|
||||||
|
|
||||||
expect(url_for_new_issue).to eq ""
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'returns an empty string if issue_url is invalid' do
|
|
||||||
expect(project).to receive_message_chain('issues_tracker.new_issue_url') { 'javascript:alert("foo");' }
|
|
||||||
|
|
||||||
expect(url_for_new_issue(project)).to eq ''
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'returns an empty string if issue_path is invalid' do
|
|
||||||
expect(project).to receive_message_chain('issues_tracker.new_issue_path') { 'javascript:alert("foo");' }
|
|
||||||
|
|
||||||
expect(url_for_new_issue(project, only_path: true)).to eq ''
|
|
||||||
end
|
|
||||||
|
|
||||||
describe "when external tracker was enabled and then config removed" do
|
|
||||||
before do
|
|
||||||
@project = ext_project
|
|
||||||
allow(Gitlab.config).to receive(:issues_tracker).and_return(nil)
|
|
||||||
end
|
|
||||||
|
|
||||||
it "should return internal path" do
|
|
||||||
expect(url_for_new_issue).to match(ext_expected)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
describe "merge_requests_sentence" do
|
describe "merge_requests_sentence" do
|
||||||
subject { merge_requests_sentence(merge_requests)}
|
subject { merge_requests_sentence(merge_requests)}
|
||||||
let(:merge_requests) do
|
let(:merge_requests) do
|
||||||
|
|
Loading…
Reference in a new issue