diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 7f5c3ff3d6a..cb1e514c60e 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -5,6 +5,7 @@ class Projects::IssuesController < Projects::ApplicationController include ToggleAwardEmoji include IssuableCollections + before_action :redirect_to_external_issue_tracker, only: [:index] before_action :module_enabled before_action :issue, only: [:edit, :update, :show, :referenced_merge_requests, :related_branches, :can_create_branch] @@ -201,6 +202,12 @@ class Projects::IssuesController < Projects::ApplicationController return render_404 unless @project.issues_enabled && @project.default_issues_tracker? end + def redirect_to_external_issue_tracker + return unless @project.external_issue_tracker + + redirect_to @project.external_issue_tracker.issues_url + end + # Since iids are implemented only in 6.1 # user may navigate to issue page using old global ids. # diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 2b0defd1dda..5061ccb93a4 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -13,22 +13,6 @@ module IssuesHelper OpenStruct.new(id: 0, title: 'None (backlog)', name: 'Unassigned') end - def url_for_project_issues(project = @project, options = {}) - return '' if project.nil? - - url = - if options[:only_path] - project.issues_tracker.project_path - else - project.issues_tracker.project_url - end - - # Ensure we return a valid URL to prevent possible XSS. - URI.parse(url).to_s - rescue URI::InvalidURIError - '' - end - def url_for_new_issue(project = @project, options = {}) return '' if project.nil? diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 9e65d94186b..1d3b8fc3683 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -66,7 +66,7 @@ - if project_nav_tab? :issues = nav_link(controller: [:issues, :labels, :milestones]) do - = link_to url_for_project_issues(@project, only_path: true), title: 'Issues', class: 'shortcuts-issues' do + = link_to namespace_project_issues_path(@project.namespace, @project), title: 'Issues', class: 'shortcuts-issues' do %span Issues - if @project.default_issues_tracker? diff --git a/app/views/projects/issues/_head.html.haml b/app/views/projects/issues/_head.html.haml index 403adb7426b..60b45115b73 100644 --- a/app/views/projects/issues/_head.html.haml +++ b/app/views/projects/issues/_head.html.haml @@ -2,7 +2,7 @@ %ul{ class: (container_class) } - if project_nav_tab?(:issues) && !current_controller?(:merge_requests) = nav_link(controller: :issues) do - = link_to url_for_project_issues(@project, only_path: true), title: 'Issues' do + = link_to namespace_project_issues_path(@project.namespace, @project), title: 'Issues' do %span Issues diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 77f65057f71..ed31f689d3d 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -6,37 +6,51 @@ describe Projects::IssuesController do let(:issue) { create(:issue, project: project) } describe "GET #index" do - before do - sign_in(user) - project.team << [user, :developer] + context 'external issue tracker' do + it 'redirects to the external issue tracker' do + external = double(issues_url: 'https://example.com/issues') + allow(project).to receive(:external_issue_tracker).and_return(external) + controller.instance_variable_set(:@project, project) + + get :index, namespace_id: project.namespace.path, project_id: project + + expect(response).to redirect_to('https://example.com/issues') + end end - it "returns index" do - get :index, namespace_id: project.namespace.path, project_id: project.path + context 'internal issue tracker' do + before do + sign_in(user) + project.team << [user, :developer] + end - expect(response).to have_http_status(200) - end + it "returns index" do + get :index, namespace_id: project.namespace.path, project_id: project.path - it "return 301 if request path doesn't match project path" do - get :index, namespace_id: project.namespace.path, project_id: project.path.upcase + expect(response).to have_http_status(200) + end - expect(response).to redirect_to(namespace_project_issues_path(project.namespace, project)) - end + it "return 301 if request path doesn't match project path" do + get :index, namespace_id: project.namespace.path, project_id: project.path.upcase - it "returns 404 when issues are disabled" do - project.issues_enabled = false - project.save + expect(response).to redirect_to(namespace_project_issues_path(project.namespace, project)) + end - get :index, namespace_id: project.namespace.path, project_id: project.path - expect(response).to have_http_status(404) - end + it "returns 404 when issues are disabled" do + project.issues_enabled = false + project.save - 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.path, project_id: project.path + expect(response).to have_http_status(404) + end - get :index, namespace_id: project.namespace.path, project_id: project.path - expect(response).to have_http_status(404) + 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.path, project_id: project.path + expect(response).to have_http_status(404) + end end end diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index 831ae7fb69c..ca4aea47413 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -5,52 +5,6 @@ describe IssuesHelper do let(:issue) { create :issue, project: project } let(:ext_project) { create :redmine_project } - describe "url_for_project_issues" do - let(:project_url) { ext_project.external_issue_tracker.project_url } - let(:ext_expected) { project_url.gsub(':project_id', ext_project.id.to_s) } - let(:int_expected) { polymorphic_path([@project.namespace, project]) } - - it "should return internal path if used internal tracker" do - @project = project - expect(url_for_project_issues).to match(int_expected) - end - - it "should return path to external tracker" do - @project = ext_project - - expect(url_for_project_issues).to match(ext_expected) - end - - it "should return empty string if project nil" do - @project = nil - - expect(url_for_project_issues).to eq "" - end - - it 'returns an empty string if project_url is invalid' do - expect(project).to receive_message_chain('issues_tracker.project_url') { 'javascript:alert("foo");' } - - expect(url_for_project_issues(project)).to eq '' - end - - it 'returns an empty string if project_path is invalid' do - expect(project).to receive_message_chain('issues_tracker.project_path') { 'javascript:alert("foo");' } - - expect(url_for_project_issues(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 path to external tracker" do - expect(url_for_project_issues).to match(ext_expected) - end - end - end - 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) }