Fix 404 on some group pages when name contains dotdz-fix-group-name-dot

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

3 files changed, 25 insertions, 8 deletions

diff --git a/changelogs/unreleased/dz-fix-group-name-dot.yml b/changelogs/unreleased/dz-fix-group-name-dot.yml
new file mode 100644
index 00000000000..439cb229e6b
--- /dev/null
+++ b/changelogs/unreleased/dz-fix-group-name-dot.yml
@@ -0,0 +1,4 @@
+---
+title: Fix 404 on some group pages when name contains dot
+merge_request: 7614
+author: 
diff --git a/config/routes/group.rb b/config/routes/group.rb
index 068e0b6e843..a3a001178b4 100644
--- a/config/routes/group.rb
+++ b/config/routes/group.rb
@@ -14,7 +14,9 @@ end
 
 resources :groups, only: [:index, :new, :create]
 
-scope(path: 'groups/:id', controller: :groups) do
+scope(path: 'groups/:id',
+      controller: :groups,
+      constraints: { id: Gitlab::Regex.namespace_route_regex }) do
   get :edit, as: :edit_group
   get :issues, as: :issues_group
   get :merge_requests, as: :merge_requests_group
@@ -22,7 +24,10 @@ scope(path: 'groups/:id', controller: :groups) do
   get :activity, as: :activity_group
 end
 
-scope(path: 'groups/:group_id', module: :groups, as: :group) do
+scope(path: 'groups/:group_id',
+      module: :groups,
+      as: :group,
+      constraints: { group_id: Gitlab::Regex.namespace_route_regex }) do
   resources :group_members, only: [:index, :create, :update, :destroy], concerns: :access_requestable do
     post :resend_invite, on: :member
     delete :leave, on: :collection
@@ -37,4 +42,4 @@ scope(path: 'groups/:group_id', module: :groups, as: :group) do
 end
 
 # Must be last route in this file
-get 'groups/:id' => 'groups#show', as: :group_canonical
+get 'groups/:id' => 'groups#show', as: :group_canonical, constraints: { id: Gitlab::Regex.namespace_route_regex }
diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb
index 61dca5d5a62..7aba4f08088 100644
--- a/spec/routing/routing_spec.rb
+++ b/spec/routing/routing_spec.rb
@@ -261,20 +261,28 @@ describe "Authentication", "routing" do
 end
 
 describe "Groups", "routing" do
+  let(:name) { 'complex.group-name' }
+
   it "to #show" do
-    expect(get("/groups/1")).to route_to('groups#show', id: '1')
+    expect(get("/groups/#{name}")).to route_to('groups#show', id: name)
   end
 
   it "also display group#show on the short path" do
     allow(Group).to receive(:find_by).and_return(true)
 
-    expect(get('/1')).to route_to('groups#show', id: '1')
+    expect(get("/#{name}")).to route_to('groups#show', id: name)
   end
 
-  it "also display group#show with dot in the path" do
-    allow(Group).to receive(:find_by).and_return(true)
+  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
 
-    expect(get('/group.with.dot')).to route_to('groups#show', id: 'group.with.dot')
+  it "to #members" do
+    expect(get("/groups/#{name}/group_members")).to route_to('groups/group_members#index', group_id: name)
   end
 end