From 414c4e3fd83a6a3cae623dcc8135d6ef09a30562 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Thu, 26 Oct 2017 16:06:51 +0200
Subject: Add helper methods to redirect legacy paths

---
 spec/support/legacy_path_redirect_shared_examples.rb | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 spec/support/legacy_path_redirect_shared_examples.rb

(limited to 'spec')

diff --git a/spec/support/legacy_path_redirect_shared_examples.rb b/spec/support/legacy_path_redirect_shared_examples.rb
new file mode 100644
index 00000000000..f300bdd48b1
--- /dev/null
+++ b/spec/support/legacy_path_redirect_shared_examples.rb
@@ -0,0 +1,13 @@
+shared_examples 'redirecting a legacy path' do |source, target|
+  include RSpec::Rails::RequestExampleGroup
+
+  it "redirects #{source} to #{target} when the resource does not exist" do
+    expect(get(source)).to redirect_to(target)
+  end
+
+  it "does not redirect #{source} to #{target} when the resource exists" do
+    resource
+
+    expect(get(source)).not_to redirect_to(target)
+  end
+end
-- 
cgit v1.2.1


From 5d14337baff9c2ae8091b7b4ab954f8024449a52 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Thu, 26 Oct 2017 16:10:03 +0200
Subject: Free up `labels` as a group name

---
 spec/routing/group_routing_spec.rb | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 spec/routing/group_routing_spec.rb

(limited to 'spec')

diff --git a/spec/routing/group_routing_spec.rb b/spec/routing/group_routing_spec.rb
new file mode 100644
index 00000000000..39fbf082b4d
--- /dev/null
+++ b/spec/routing/group_routing_spec.rb
@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+describe 'group routing' do
+  let!(:existing_group) { create(:group, parent: create(:group, path: 'gitlab-org'), path: 'infra') }
+
+  describe 'GET #labels' do
+    it 'routes to the correct controller' do
+      expect(get('/groups/gitlab-org/infra/-/labels'))
+        .to route_to(group_id: 'gitlab-org/infra',
+                     controller: 'groups/labels',
+                     action: 'index')
+    end
+
+    it_behaves_like 'redirecting a legacy path', '/groups/gitlab-org/infra/labels', '/groups/gitlab-org/infra/-/labels' do
+      let(:resource) { create(:group, parent: existing_group, path: 'labels') }
+    end
+  end
+end
-- 
cgit v1.2.1


From 5dde0536c323d14fef2327a8d553b5f8a8a7b2d0 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Thu, 26 Oct 2017 17:49:07 +0200
Subject: Free up `avatar`, `group_members` and `milestones` as paths

---
 spec/lib/gitlab/path_regex_spec.rb | 14 -------
 spec/routing/group_routing_spec.rb | 82 +++++++++++++++++++++++++++++++++-----
 spec/routing/routing_spec.rb       | 30 --------------
 3 files changed, 71 insertions(+), 55 deletions(-)

(limited to 'spec')

diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb
index ee63c9338c5..c76084114cb 100644
--- a/spec/lib/gitlab/path_regex_spec.rb
+++ b/spec/lib/gitlab/path_regex_spec.rb
@@ -225,8 +225,6 @@ describe Gitlab::PathRegex do
 
     it 'accepts group routes' do
       expect(subject).to match('activity/')
-      expect(subject).to match('group_members/')
-      expect(subject).to match('labels/')
     end
 
     it 'is not case sensitive' do
@@ -258,8 +256,6 @@ describe Gitlab::PathRegex do
 
         it 'accepts group routes' do
           expect(subject).to match('activity/')
-          expect(subject).to match('group_members/')
-          expect(subject).to match('labels/')
         end
       end
 
@@ -280,8 +276,6 @@ describe Gitlab::PathRegex do
 
         it 'accepts group routes' do
           expect(subject).to match('activity/more/')
-          expect(subject).to match('group_members/more/')
-          expect(subject).to match('labels/more/')
         end
       end
     end
@@ -304,8 +298,6 @@ describe Gitlab::PathRegex do
 
         it 'rejects group routes' do
           expect(subject).not_to match('root/activity/')
-          expect(subject).not_to match('root/group_members/')
-          expect(subject).not_to match('root/labels/')
         end
       end
 
@@ -326,8 +318,6 @@ describe Gitlab::PathRegex do
 
         it 'rejects group routes' do
           expect(subject).not_to match('root/activity/more/')
-          expect(subject).not_to match('root/group_members/more/')
-          expect(subject).not_to match('root/labels/more/')
         end
       end
     end
@@ -361,8 +351,6 @@ describe Gitlab::PathRegex do
 
     it 'accepts group routes' do
       expect(subject).to match('activity/')
-      expect(subject).to match('group_members/')
-      expect(subject).to match('labels/')
     end
 
     it 'is not case sensitive' do
@@ -394,8 +382,6 @@ describe Gitlab::PathRegex do
 
     it 'accepts group routes' do
       expect(subject).to match('root/activity/')
-      expect(subject).to match('root/group_members/')
-      expect(subject).to match('root/labels/')
     end
 
     it 'is not case sensitive' do
diff --git a/spec/routing/group_routing_spec.rb b/spec/routing/group_routing_spec.rb
index 39fbf082b4d..b3e55f5e3c9 100644
--- a/spec/routing/group_routing_spec.rb
+++ b/spec/routing/group_routing_spec.rb
@@ -1,18 +1,78 @@
 require 'spec_helper'
 
-describe 'group routing' do
-  let!(:existing_group) { create(:group, parent: create(:group, path: 'gitlab-org'), path: 'infra') }
-
-  describe 'GET #labels' do
-    it 'routes to the correct controller' do
-      expect(get('/groups/gitlab-org/infra/-/labels'))
-        .to route_to(group_id: 'gitlab-org/infra',
-                     controller: 'groups/labels',
-                     action: 'index')
+describe "Groups", "routing" do
+  let(:group_path) { 'complex.group-namegit' }
+  let!(:group) { create(:group, path: group_path) }
+
+  it "to #show" do
+    expect(get("/groups/#{group_path}")).to route_to('groups#show', id: group_path)
+  end
+
+  it "also supports nested groups" do
+    nested_group = create(:group, parent: group)
+    expect(get("/#{group_path}/#{nested_group.path}")).to route_to('groups#show', id: "#{group_path}/#{nested_group.path}")
+  end
+
+  it "also display group#show on the short path" do
+    expect(get("/#{group_path}")).to route_to('groups#show', id: group_path)
+  end
+
+  it "to #activity" do
+    expect(get("/groups/#{group_path}/activity")).to route_to('groups#activity', id: group_path)
+  end
+
+  it "to #issues" do
+    expect(get("/groups/#{group_path}/issues")).to route_to('groups#issues', id: group_path)
+  end
+
+  it "to #members" do
+    expect(get("/groups/#{group_path}/-/group_members")).to route_to('groups/group_members#index', group_id: group_path)
+  end
+
+  describe 'legacy redirection' do
+    shared_examples 'canonical groups route' do |path|
+      it "#{path} routes to the correct controller" do
+        expect(get("/groups/#{group_path}/-/#{path}"))
+          .to route_to(group_id: group_path,
+                       controller: "groups/#{path}",
+                       action: 'index')
+      end
+
+      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/#{path}", "/groups/complex.group-namegit/-/#{path}/" do
+        let(:resource) { create(:group, parent: group, path: path) }
+      end
+    end
+
+    describe 'labels' do
+      it_behaves_like 'canonical groups route', 'labels'
+    end
+
+    describe 'group_members' do
+      it_behaves_like 'canonical groups route', 'group_members'
     end
 
-    it_behaves_like 'redirecting a legacy path', '/groups/gitlab-org/infra/labels', '/groups/gitlab-org/infra/-/labels' do
-      let(:resource) { create(:group, parent: existing_group, path: 'labels') }
+    describe 'avatar' do
+      it 'routes to the avatars controller' do
+        expect(delete("/groups/#{group_path}/-/avatar"))
+          .to route_to(group_id: group_path,
+                       controller: 'groups/avatars',
+                       action: 'destroy')
+      end
+    end
+
+    describe 'milestones' do
+      it_behaves_like 'canonical groups route', 'milestones'
+
+      context 'nested routes' do
+        include RSpec::Rails::RequestExampleGroup
+
+        let(:milestone) { create(:milestone, group: group) }
+
+        it 'redirects the nested routes' do
+          request = get("/groups/#{group_path}/milestones/#{milestone.id}/merge_requests")
+          expect(request).to redirect_to("/groups/#{group_path}/-/milestones/#{milestone.id}/merge_requests")
+        end
+      end
     end
   end
 end
diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb
index 609481603af..32aa6e5ad52 100644
--- a/spec/routing/routing_spec.rb
+++ b/spec/routing/routing_spec.rb
@@ -278,36 +278,6 @@ describe "Authentication", "routing" do
   end
 end
 
-describe "Groups", "routing" do
-  let(:name) { 'complex.group-namegit' }
-  let!(:group) { create(:group, name: name) }
-
-  it "to #show" do
-    expect(get("/groups/#{name}")).to route_to('groups#show', id: name)
-  end
-
-  it "also supports nested groups" do
-    nested_group = create(:group, parent: group)
-    expect(get("/#{name}/#{nested_group.name}")).to route_to('groups#show', id: "#{name}/#{nested_group.name}")
-  end
-
-  it "also display group#show on the short path" do
-    expect(get("/#{name}")).to route_to('groups#show', id: name)
-  end
-
-  it "to #activity" do
-    expect(get("/groups/#{name}/activity")).to route_to('groups#activity', id: name)
-  end
-
-  it "to #issues" do
-    expect(get("/groups/#{name}/issues")).to route_to('groups#issues', id: name)
-  end
-
-  it "to #members" do
-    expect(get("/groups/#{name}/group_members")).to route_to('groups/group_members#index', group_id: name)
-  end
-end
-
 describe HealthCheckController, 'routing' do
   it 'to #index' do
     expect(get('/health_check')).to route_to('health_check#index')
-- 
cgit v1.2.1


From 58d1d6a5c7e0a45c9aa8a9d4d1be24dbdce5a08a Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Fri, 27 Oct 2017 11:29:51 +0200
Subject: Free up some group reserved words

---
 spec/helpers/labels_helper_spec.rb |  2 +-
 spec/lib/gitlab/path_regex_spec.rb |  8 ++---
 spec/models/group_spec.rb          |  6 ----
 spec/routing/group_routing_spec.rb | 73 +++++++++++++++++++++++++++++---------
 4 files changed, 61 insertions(+), 28 deletions(-)

(limited to 'spec')

diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb
index 36d6e495ed0..4ac4302adfd 100644
--- a/spec/helpers/labels_helper_spec.rb
+++ b/spec/helpers/labels_helper_spec.rb
@@ -24,7 +24,7 @@ describe LabelsHelper do
       let(:group) { build(:group, name: 'bar') }
 
       it 'links to group issues page' do
-        expect(link_to_label(label, subject: group)).to match %r{<a href="/groups/bar/issues\?label_name%5B%5D=#{label.name}">.*</a>}
+        expect(link_to_label(label, subject: group)).to match %r{<a href="/groups/bar/-/issues\?label_name%5B%5D=#{label.name}">.*</a>}
       end
     end
 
diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb
index c76084114cb..98fcc84c416 100644
--- a/spec/lib/gitlab/path_regex_spec.rb
+++ b/spec/lib/gitlab/path_regex_spec.rb
@@ -297,7 +297,7 @@ describe Gitlab::PathRegex do
         end
 
         it 'rejects group routes' do
-          expect(subject).not_to match('root/activity/')
+          expect(subject).not_to match('root/analytics/')
         end
       end
 
@@ -317,7 +317,7 @@ describe Gitlab::PathRegex do
         end
 
         it 'rejects group routes' do
-          expect(subject).not_to match('root/activity/more/')
+          expect(subject).not_to match('root/analytics/more/')
         end
       end
     end
@@ -350,7 +350,7 @@ describe Gitlab::PathRegex do
     end
 
     it 'accepts group routes' do
-      expect(subject).to match('activity/')
+      expect(subject).to match('analytics/')
     end
 
     it 'is not case sensitive' do
@@ -381,7 +381,7 @@ describe Gitlab::PathRegex do
     end
 
     it 'accepts group routes' do
-      expect(subject).to match('root/activity/')
+      expect(subject).to match('root/analytics/')
     end
 
     it 'is not case sensitive' do
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index c8caa11b8b0..d4052a64570 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -65,12 +65,6 @@ describe Group do
 
         expect(group).not_to be_valid
       end
-
-      it 'rejects reserved group paths' do
-        group = build(:group, path: 'activity', parent: create(:group))
-
-        expect(group).not_to be_valid
-      end
     end
 
     describe '#visibility_level_allowed_by_parent' do
diff --git a/spec/routing/group_routing_spec.rb b/spec/routing/group_routing_spec.rb
index b3e55f5e3c9..3e56d34bc9d 100644
--- a/spec/routing/group_routing_spec.rb
+++ b/spec/routing/group_routing_spec.rb
@@ -18,37 +18,36 @@ describe "Groups", "routing" do
   end
 
   it "to #activity" do
-    expect(get("/groups/#{group_path}/activity")).to route_to('groups#activity', id: group_path)
+    expect(get("/groups/#{group_path}/-/activity")).to route_to('groups#activity', id: group_path)
   end
 
   it "to #issues" do
-    expect(get("/groups/#{group_path}/issues")).to route_to('groups#issues', id: group_path)
+    expect(get("/groups/#{group_path}/-/issues")).to route_to('groups#issues', id: group_path)
   end
 
   it "to #members" do
     expect(get("/groups/#{group_path}/-/group_members")).to route_to('groups/group_members#index', group_id: group_path)
   end
 
-  describe 'legacy redirection' do
-    shared_examples 'canonical groups route' do |path|
-      it "#{path} routes to the correct controller" do
-        expect(get("/groups/#{group_path}/-/#{path}"))
-          .to route_to(group_id: group_path,
-                       controller: "groups/#{path}",
-                       action: 'index')
-      end
+  it "to #labels" do
+    expect(get("/groups/#{group_path}/-/labels")).to route_to('groups/labels#index', group_id: group_path)
+  end
 
-      it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/#{path}", "/groups/complex.group-namegit/-/#{path}/" do
-        let(:resource) { create(:group, parent: group, path: path) }
-      end
-    end
+  it "to #milestones" do
+    expect(get("/groups/#{group_path}/-/milestones")).to route_to('groups/milestones#index', group_id: group_path)
+  end
 
+  describe 'legacy redirection' do
     describe 'labels' do
-      it_behaves_like 'canonical groups route', 'labels'
+      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
     end
 
     describe 'group_members' do
-      it_behaves_like 'canonical groups route', 'group_members'
+      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
 
     describe 'avatar' do
@@ -61,7 +60,9 @@ describe "Groups", "routing" do
     end
 
     describe 'milestones' do
-      it_behaves_like 'canonical groups route', 'milestones'
+      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
 
       context 'nested routes' do
         include RSpec::Rails::RequestExampleGroup
@@ -74,5 +75,43 @@ describe "Groups", "routing" do
         end
       end
     end
+
+    describe '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')
+        end
+      end
+    end
+
+    describe '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
+        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
+        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
+        let(:resource) { create(:group, parent: group, path: 'activity') }
+      end
+
+      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
+    end
   end
 end
-- 
cgit v1.2.1


From e070e216c80efb897eb6c5a7d13872e4762205c1 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Fri, 27 Oct 2017 16:47:08 +0200
Subject: Update failure message when finding new routes in `PathRegex`

---
 spec/lib/gitlab/path_regex_spec.rb | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

(limited to 'spec')

diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb
index 98fcc84c416..751cde79a94 100644
--- a/spec/lib/gitlab/path_regex_spec.rb
+++ b/spec/lib/gitlab/path_regex_spec.rb
@@ -45,21 +45,16 @@ describe Gitlab::PathRegex do
       Found new routes that could cause conflicts with existing namespaced routes
       for groups or projects.
 
-      Add <#{missing_words.join(', ')}> to `Gitlab::PathRegex::#{constant_name}
-      to make sure no projects or namespaces can be created with those paths.
-
-      To rename any existing records with those paths you can use the
-      `Gitlab::Database::RenameReservedpathsMigration::<VERSION>.#{migration_helper}`
-      migration helper.
-
-      Make sure to make a note of the renamed records in the release blog post.
+      Nest <#{missing_words.join(', ')}> in a route containing `-`, that way
+      we know there will be no conflicts with groups or projects created with those
+      paths.
 
       MISSING
     end
 
     if additional_words.any?
       message += <<-ADDITIONAL
-      Why are <#{additional_words.join(', ')}> in `#{constant_name}`?
+      Is <#{additional_words.join(', ')}> in `#{constant_name}` required?
       If they are really required, update these specs to reflect that.
 
       ADDITIONAL
-- 
cgit v1.2.1


From 2414c69ee981cf6432e513dfdcf57a9badc5d51f Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Tue, 7 Nov 2017 13:04:46 +0100
Subject: Check redirecting with a querystring

---
 spec/routing/group_routing_spec.rb | 10 ++++++++++
 1 file changed, 10 insertions(+)

(limited to 'spec')

diff --git a/spec/routing/group_routing_spec.rb b/spec/routing/group_routing_spec.rb
index 3e56d34bc9d..7a4c8304e62 100644
--- a/spec/routing/group_routing_spec.rb
+++ b/spec/routing/group_routing_spec.rb
@@ -74,6 +74,16 @@ describe "Groups", "routing" do
           expect(request).to redirect_to("/groups/#{group_path}/-/milestones/#{milestone.id}/merge_requests")
         end
       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
+          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
+          let(:resource) { create(:group, parent: group, path: 'milestones') }
+        end
+      end
     end
 
     describe 'edit' do
-- 
cgit v1.2.1


From 9b0899cb809d826249bb3ad2eb35beec5bdf2190 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Tue, 7 Nov 2017 13:05:08 +0100
Subject: Remove EE-specific group paths

---
 spec/lib/gitlab/path_regex_spec.rb | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

(limited to 'spec')

diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb
index 751cde79a94..0ae90069b7f 100644
--- a/spec/lib/gitlab/path_regex_spec.rb
+++ b/spec/lib/gitlab/path_regex_spec.rb
@@ -152,16 +152,7 @@ describe Gitlab::PathRegex do
   let(:paths_after_group_id) do
     group_routes.map do |route|
       route.gsub(STARTING_WITH_GROUP, '').split('/').first
-    end.uniq + ee_paths_after_group_id
-  end
-
-  let(:ee_paths_after_group_id) do
-    %w(analytics
-       ldap
-       ldap_group_links
-       notification_setting
-       audit_events
-       pipeline_quota hooks)
+    end.uniq
   end
 
   describe 'TOP_LEVEL_ROUTES' do
@@ -292,7 +283,7 @@ describe Gitlab::PathRegex do
         end
 
         it 'rejects group routes' do
-          expect(subject).not_to match('root/analytics/')
+          expect(subject).not_to match('root/-/')
         end
       end
 
@@ -312,7 +303,7 @@ describe Gitlab::PathRegex do
         end
 
         it 'rejects group routes' do
-          expect(subject).not_to match('root/analytics/more/')
+          expect(subject).not_to match('root/-/')
         end
       end
     end
-- 
cgit v1.2.1