Merge branch…

Merge branch '40230-groups-gitlab-org-labels-json-redirects-to-groups-gitlab-org-labels-json' into 'master' Resolve "/groups/gitlab-org/labels.json redirects to /groups/gitlab-org/-/labels/.json" Closes #40230 See merge request gitlab-org/gitlab-ce!15427 (cherry picked from commit 7142af49) 9f921b73 Don't add a trailing slash in group redirects

Merge branch…
616242a7 · Douwe Maan · Winnie Hellmann · 1fe4240c · 616242a7 · 616242a7
Commit 616242a7 authored Nov 17, 2017 by Douwe Maan Committed by Winnie Hellmann Nov 17, 2017
Hide whitespace changes
Inline Side-by-side

Showing with 20 additions and 14 deletions

routing.rb lib/gitlab/routing.rb +3 -3

group_routing_spec.rb spec/routing/group_routing_spec.rb +17 -11

No files found.
--- a/lib/gitlab/routing.rb
+++ b/lib/gitlab/routing.rb
@@ -46,10 +46,10 @@ module Gitlab
        # Only replace the last occurence of `path`.
        #
        # `request.fullpath` includes the querystring
-        path = request.path.sub(%r{/#{path}/*(?!.*#{path})}, "/-/#{path}/")
-        path << "?#{request.query_string}" if request.query_string.present?
+        new_path = request.path.sub(%r{/#{path}(/*)(?!.*#{path})}, "/-/#{path}\\1")
+        new_path << "?#{request.query_string}" if request.query_string.present?

-        path
+        new_path
      end

      paths.each do |path|

--- a/spec/routing/group_routing_spec.rb
+++ b/spec/routing/group_routing_spec.rb
@@ -39,13 +39,19 @@ describe "Groups", "routing" do

  describe 'legacy redirection' do
    describe 'labels' do
-      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/labels", "/groups/complex.group-namegit/-/labels/" do
+      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/labels", "/groups/complex.group-namegit/-/labels" do
        let(:resource) { create(:group, parent: group, path: 'labels') }
      end
+
+      context 'when requesting JSON' do
+        it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/labels.json", "/groups/complex.group-namegit/-/labels.json" do
+          let(:resource) { create(:group, parent: group, path: 'labels') }
+        end
+      end
    end

    describe 'group_members' do
-      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/group_members", "/groups/complex.group-namegit/-/group_members/" do
+      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/group_members", "/groups/complex.group-namegit/-/group_members" do
        let(:resource) { create(:group, parent: group, path: 'group_members') }
      end
    end
@@ -60,7 +66,7 @@ describe "Groups", "routing" do
    end

    describe 'milestones' do
-      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones", "/groups/complex.group-namegit/-/milestones/" do
+      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones", "/groups/complex.group-namegit/-/milestones" do
        let(:resource) { create(:group, parent: group, path: 'milestones') }
      end

@@ -76,18 +82,18 @@ describe "Groups", "routing" do
      end

      context 'with a query string' do
-        it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?hello=world", "/groups/complex.group-namegit/-/milestones/?hello=world" do
+        it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?hello=world", "/groups/complex.group-namegit/-/milestones?hello=world" do
          let(:resource) { create(:group, parent: group, path: 'milestones') }
        end

-        it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?milestones=/milestones", "/groups/complex.group-namegit/-/milestones/?milestones=/milestones" do
+        it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?milestones=/milestones", "/groups/complex.group-namegit/-/milestones?milestones=/milestones" do
          let(:resource) { create(:group, parent: group, path: 'milestones') }
        end
      end
    end

    describe 'edit' do
-      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/edit", "/groups/complex.group-namegit/-/edit/" do
+      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/edit", "/groups/complex.group-namegit/-/edit" do
        let(:resource) do
          pending('still rejected because of the wildcard reserved word')
          create(:group, parent: group, path: 'edit')
@@ -96,29 +102,29 @@ describe "Groups", "routing" do
    end

    describe 'issues' do
-      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/issues", "/groups/complex.group-namegit/-/issues/" do
+      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/issues", "/groups/complex.group-namegit/-/issues" do
        let(:resource) { create(:group, parent: group, path: 'issues') }
      end
    end

    describe 'merge_requests' do
-      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/merge_requests", "/groups/complex.group-namegit/-/merge_requests/" do
+      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/merge_requests", "/groups/complex.group-namegit/-/merge_requests" do
        let(:resource) { create(:group, parent: group, path: 'merge_requests') }
      end
    end

    describe 'projects' do
-      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/projects", "/groups/complex.group-namegit/-/projects/" do
+      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/projects", "/groups/complex.group-namegit/-/projects" do
        let(:resource) { create(:group, parent: group, path: 'projects') }
      end
    end

    describe 'activity' do
-      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/activity", "/groups/complex.group-namegit/-/activity/" do
+      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/activity", "/groups/complex.group-namegit/-/activity" do
        let(:resource) { create(:group, parent: group, path: 'activity') }
      end

-      it_behaves_like 'redirecting a legacy path', "/groups/activity/activity", "/groups/activity/-/activity/" do
+      it_behaves_like 'redirecting a legacy path', "/groups/activity/activity", "/groups/activity/-/activity" do
        let!(:parent) { create(:group, path: 'activity') }
        let(:resource) { create(:group, parent: parent, path: 'activity') }
      end