Commit 5dde0536 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Free up `avatar`, `group_members` and `milestones` as paths

parent 5d14337b
...@@ -20,20 +20,6 @@ constraints(GroupUrlConstrainer.new) do ...@@ -20,20 +20,6 @@ constraints(GroupUrlConstrainer.new) do
module: :groups, module: :groups,
as: :group, as: :group,
constraints: { group_id: Gitlab::PathRegex.full_namespace_route_regex }) do constraints: { group_id: Gitlab::PathRegex.full_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
end
resource :avatar, only: [:destroy]
resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :edit, :update, :new, :create] do
member do
get :merge_requests
get :participants
get :labels
end
end
scope path: '-' do scope path: '-' do
namespace :settings do namespace :settings do
resource :ci_cd, only: [:show], controller: 'ci_cd' resource :ci_cd, only: [:show], controller: 'ci_cd'
...@@ -46,6 +32,21 @@ constraints(GroupUrlConstrainer.new) do ...@@ -46,6 +32,21 @@ constraints(GroupUrlConstrainer.new) do
resources :labels, except: [:show] do resources :labels, except: [:show] do
post :toggle_subscription, on: :member post :toggle_subscription, on: :member
end end
resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :edit, :update, :new, :create] do
member do
get :merge_requests
get :participants
get :labels
end
end
resource :avatar, only: [:destroy]
resources :group_members, only: [:index, :create, :update, :destroy], concerns: :access_requestable do
post :resend_invite, on: :member
delete :leave, on: :collection
end
end end
end end
...@@ -62,6 +63,6 @@ constraints(GroupUrlConstrainer.new) do ...@@ -62,6 +63,6 @@ constraints(GroupUrlConstrainer.new) do
# Legacy paths should be defined last, so they would be ignored if routes with # Legacy paths should be defined last, so they would be ignored if routes with
# one of the previously reserved words exist. # one of the previously reserved words exist.
scope(path: 'groups/*group_id') do scope(path: 'groups/*group_id') do
Gitlab::Routing.redirect_legacy_paths(self, :labels) Gitlab::Routing.redirect_legacy_paths(self, :labels, :milestones, :group_members)
end end
end end
...@@ -115,15 +115,12 @@ module Gitlab ...@@ -115,15 +115,12 @@ module Gitlab
activity activity
analytics analytics
audit_events audit_events
avatar
edit edit
group_members
hooks hooks
issues issues
ldap ldap
ldap_group_links ldap_group_links
merge_requests merge_requests
milestones
notification_setting notification_setting
pipeline_quota pipeline_quota
projects projects
......
...@@ -225,8 +225,6 @@ describe Gitlab::PathRegex do ...@@ -225,8 +225,6 @@ describe Gitlab::PathRegex do
it 'accepts group routes' do it 'accepts group routes' do
expect(subject).to match('activity/') expect(subject).to match('activity/')
expect(subject).to match('group_members/')
expect(subject).to match('labels/')
end end
it 'is not case sensitive' do it 'is not case sensitive' do
...@@ -258,8 +256,6 @@ describe Gitlab::PathRegex do ...@@ -258,8 +256,6 @@ describe Gitlab::PathRegex do
it 'accepts group routes' do it 'accepts group routes' do
expect(subject).to match('activity/') expect(subject).to match('activity/')
expect(subject).to match('group_members/')
expect(subject).to match('labels/')
end end
end end
...@@ -280,8 +276,6 @@ describe Gitlab::PathRegex do ...@@ -280,8 +276,6 @@ describe Gitlab::PathRegex do
it 'accepts group routes' do it 'accepts group routes' do
expect(subject).to match('activity/more/') expect(subject).to match('activity/more/')
expect(subject).to match('group_members/more/')
expect(subject).to match('labels/more/')
end end
end end
end end
...@@ -304,8 +298,6 @@ describe Gitlab::PathRegex do ...@@ -304,8 +298,6 @@ describe Gitlab::PathRegex do
it 'rejects group routes' do it 'rejects group routes' do
expect(subject).not_to match('root/activity/') expect(subject).not_to match('root/activity/')
expect(subject).not_to match('root/group_members/')
expect(subject).not_to match('root/labels/')
end end
end end
...@@ -326,8 +318,6 @@ describe Gitlab::PathRegex do ...@@ -326,8 +318,6 @@ describe Gitlab::PathRegex do
it 'rejects group routes' do it 'rejects group routes' do
expect(subject).not_to match('root/activity/more/') 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 end
end end
...@@ -361,8 +351,6 @@ describe Gitlab::PathRegex do ...@@ -361,8 +351,6 @@ describe Gitlab::PathRegex do
it 'accepts group routes' do it 'accepts group routes' do
expect(subject).to match('activity/') expect(subject).to match('activity/')
expect(subject).to match('group_members/')
expect(subject).to match('labels/')
end end
it 'is not case sensitive' do it 'is not case sensitive' do
...@@ -394,8 +382,6 @@ describe Gitlab::PathRegex do ...@@ -394,8 +382,6 @@ describe Gitlab::PathRegex do
it 'accepts group routes' do it 'accepts group routes' do
expect(subject).to match('root/activity/') expect(subject).to match('root/activity/')
expect(subject).to match('root/group_members/')
expect(subject).to match('root/labels/')
end end
it 'is not case sensitive' do it 'is not case sensitive' do
......
require 'spec_helper' require 'spec_helper'
describe 'group routing' do describe "Groups", "routing" do
let!(:existing_group) { create(:group, parent: create(:group, path: 'gitlab-org'), path: 'infra') } let(:group_path) { 'complex.group-namegit' }
let!(:group) { create(:group, path: group_path) }
describe 'GET #labels' do
it 'routes to the correct controller' do it "to #show" do
expect(get('/groups/gitlab-org/infra/-/labels')) expect(get("/groups/#{group_path}")).to route_to('groups#show', id: group_path)
.to route_to(group_id: 'gitlab-org/infra', end
controller: 'groups/labels',
action: 'index') 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 end
it_behaves_like 'redirecting a legacy path', '/groups/gitlab-org/infra/labels', '/groups/gitlab-org/infra/-/labels' do describe 'avatar' do
let(:resource) { create(:group, parent: existing_group, path: 'labels') } 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 end
end end
...@@ -278,36 +278,6 @@ describe "Authentication", "routing" do ...@@ -278,36 +278,6 @@ describe "Authentication", "routing" do
end end
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 describe HealthCheckController, 'routing' do
it 'to #index' do it 'to #index' do
expect(get('/health_check')).to route_to('health_check#index') expect(get('/health_check')).to route_to('health_check#index')
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment