Merge branch '8054-fix-jira-subgroup-links' into 'master'

Handle JIRA links with encoded namespace

Closes #8054

See merge request gitlab-org/gitlab-ee!8908

config/routes/project.rb

@@ -570,35 +570,3 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
     end
   end
 end
-
-# EE-specific
-scope path: '/-/jira', as: :jira do
-  scope path: '*namespace_id/:project_id',
-        namespace_id: Gitlab::Jira::Dvcs::ENCODED_ROUTE_REGEX,
-        project_id: Gitlab::Jira::Dvcs::ENCODED_ROUTE_REGEX do
-    get '/', to: redirect { |params, req|
-      ::Gitlab::Jira::Dvcs.restore_full_path(
-        namespace: params[:namespace_id],
-        project: params[:project_id]
-      )
-    }
-
-    get 'commit/:id', constraints: { id: /\h{7,40}/ }, to: redirect { |params, req|
-      project_full_path = ::Gitlab::Jira::Dvcs.restore_full_path(
-        namespace: params[:namespace_id],
-        project: params[:project_id]
-      )
-
-      "/#{project_full_path}/commit/#{params[:id]}"
-    }
-
-    get 'tree/*id', as: nil, to: redirect { |params, req|
-      project_full_path = ::Gitlab::Jira::Dvcs.restore_full_path(
-        namespace: params[:namespace_id],
-        project: params[:project_id]
-      )
-
-      "/#{project_full_path}/tree/#{params[:id]}"
-    }
-  end
-end

ee/changelogs/unreleased/8054-fix-jira-subgroup-links.yml

+---
+title: Fix JIRA Development Panel links with subgroups
+merge_request: 8908
+author:
+type: fixed

ee/config/routes/project.rb

@@ -43,3 +43,34 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
     end
   end
 end
+
+scope path: '(/-/jira)', constraints: ::Constraints::JiraEncodedUrlConstrainer.new, as: :jira do
+  scope path: '*namespace_id/:project_id',
+        namespace_id: Gitlab::Jira::Dvcs::ENCODED_ROUTE_REGEX,
+        project_id: Gitlab::Jira::Dvcs::ENCODED_ROUTE_REGEX do
+    get '/', to: redirect { |params, req|
+      ::Gitlab::Jira::Dvcs.restore_full_path(
+        namespace: params[:namespace_id],
+        project: params[:project_id]
+      )
+    }
+
+    get 'commit/:id', constraints: { id: /\h{7,40}/ }, to: redirect { |params, req|
+      project_full_path = ::Gitlab::Jira::Dvcs.restore_full_path(
+        namespace: params[:namespace_id],
+        project: params[:project_id]
+      )
+
+      "/#{project_full_path}/commit/#{params[:id]}"
+    }
+
+    get 'tree/*id', as: nil, to: redirect { |params, req|
+      project_full_path = ::Gitlab::Jira::Dvcs.restore_full_path(
+        namespace: params[:namespace_id],
+        project: params[:project_id]
+      )
+
+      "/#{project_full_path}/tree/#{params[:id]}"
+    }
+  end
+end

ee/lib/constraints/jira_encoded_url_constrainer.rb

+# frozen_string_literal: true
+
+module Constraints
+  class JiraEncodedUrlConstrainer
+    def matches?(request)
+      request.path.starts_with?('/-/jira') || request.path.include?(Gitlab::Jira::Dvcs::ENCODED_SLASH)
+    end
+  end
+end

ee/spec/lib/constraints/jira_encoded_url_constrainer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Constraints::JiraEncodedUrlConstrainer do
+  describe '#matches?' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:path, :match_result) do
+      "/-/jira/group/project"                                                  | true
+      "/-/jira/group/sub_group#{Gitlab::Jira::Dvcs::ENCODED_SLASH}sub_project" | true
+      "/group/sub_group#{Gitlab::Jira::Dvcs::ENCODED_SLASH}sub_project"        | true
+      "/group/project"                                                         | false
+    end
+
+    with_them do
+      it 'matches path with /-/jira prefix or encoded slash' do
+        request = build_request(path)
+
+        expect(subject.matches?(request)).to eq(match_result)
+      end
+    end
+  end
+
+  def build_request(path)
+    double(:request, path: path)
+  end
+end

ee/spec/requests/jira_routing_spec.rb

 require 'rails_helper'
 
 describe 'Jira referenced paths', type: :request do
+  using RSpec::Parameterized::TableSyntax
+
+  let(:user) { create(:user) }
+
   let(:group) { create(:group, name: 'group') }
   let(:sub_group) { create(:group, name: 'subgroup', parent: group) }
 
-  let(:group_project) { create(:project, name: 'group_project', namespace: group) }
-  let(:sub_group_project) { create(:project, name: 'sub_group_project', namespace: sub_group) }
+  let!(:group_project) { create(:project, name: 'group_project', namespace: group) }
+  let!(:sub_group_project) { create(:project, name: 'sub_group_project', namespace: sub_group) }
 
   before do
+    group.add_owner(user)
+
     login_as user
   end
 
-  describe 'redirects to projects#show' do
-    context 'without nested group' do
-      let(:user) { group_project.creator }
-
-      it 'redirects to project' do
-        get('/-/jira/group/group_project')
-
-        expect(response).to redirect_to('/group/group_project')
-      end
-    end
-
-    context 'with nested group' do
-      let(:user) { sub_group_project.creator }
-
-      it 'redirects to project for root group' do
-        get('/-/jira/group/group@sub_group@sub_group_project')
-
-        expect(response).to redirect_to('/group/sub_group/sub_group_project')
-      end
-
-      it 'redirects to project for nested group' do
-        get('/-/jira/group@sub_group/group@sub_group@sub_group_project')
+  def redirects_to_canonical_path(jira_path, redirect_path)
+    get(jira_path)
 
-        expect(response).to redirect_to('/group/sub_group/sub_group_project')
-      end
-    end
+    expect(response).to redirect_to(redirect_path)
   end
 
-  describe 'redirects to projects/commit#show' do
-    context 'without nested group' do
-      let(:user) { group_project.creator }
-
-      it 'redirects to commits' do
-        get('/-/jira/group/group_project/commit/1234567')
-
-        expect(response).to redirect_to('/group/group_project/commit/1234567')
-      end
+  context 'with encoded subgroup path' do
+    where(:jira_path, :redirect_path) do
+      '/group/group@sub_group@sub_group_project'                | '/group/sub_group/sub_group_project'
+      '/group@sub_group/group@sub_group@sub_group_project'      | '/group/sub_group/sub_group_project'
+      '/group/group@sub_group@sub_group_project/commit/1234567' | '/group/sub_group/sub_group_project/commit/1234567'
+      '/group/group@sub_group@sub_group_project/tree/1234567'   | '/group/sub_group/sub_group_project/tree/1234567'
     end
 
-    context 'with nested group' do
-      let(:user) { sub_group_project.creator }
-
-      it 'redirects to commits' do
-        get('/-/jira/group/group@sub_group@sub_group_project/commit/1234567')
+    with_them do
+      context 'with legacy prefix' do
+        it 'redirects to canonical path' do
+          redirects_to_canonical_path "/-/jira#{jira_path}", redirect_path
+        end
+      end
 
-        expect(response).to redirect_to('/group/sub_group/sub_group_project/commit/1234567')
+      it 'redirects to canonical path' do
+        redirects_to_canonical_path jira_path, redirect_path
       end
     end
   end
 
-  describe 'redirects to projects/tree#show' do
-    context 'without nested group' do
-      let(:user) { group_project.creator }
-
-      it 'redirects to tree' do
-        get('/-/jira/group/group_project/tree/1234567')
-
-        expect(response).to redirect_to('/group/group_project/tree/1234567')
-      end
+  context 'regular paths with legacy prefix' do
+    where(:jira_path, :redirect_path) do
+      '/-/jira/group/group_project'                | '/group/group_project'
+      '/-/jira/group/group_project/commit/1234567' | '/group/group_project/commit/1234567'
+      '/-/jira/group/group_project/tree/1234567'   | '/group/group_project/tree/1234567'
     end
 
-    context 'with nested group' do
-      let(:user) { sub_group_project.creator }
-
-      it 'redirects to tree' do
-        get('/-/jira/group/group@sub_group@sub_group_project/tree/1234567')
-
-        expect(response).to redirect_to('/group/sub_group/sub_group_project/tree/1234567')
+    with_them do
+      it 'redirects to canonical path' do
+        redirects_to_canonical_path jira_path, redirect_path
       end
     end
   end