Merge branch 'dz-fix-group-name-dot' into 'master'

Add constraints to group id in routing

To prevent 404 when visit page like https://gitlab.com/groups/group.with.dot/activity

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/24622

See merge request !7614

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: 

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 }

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
 
-    expect(get('/group.with.dot')).to route_to('groups#show', id: 'group.with.dot')
+  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