diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 17d40cedd5d88640880042fbcb9572f9fdd2b19c..c967488fff4572ec6347406a269ebccd2115ac22 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -61,7 +61,11 @@ class GroupsController < Groups::ApplicationController end def children - parent = Group.find_by(parent_id: params[:parent_id]) || @group + parent = if params[:parent_id].present? + Group.find(params[:parent_id]) + else + @group + end if parent.nil? || !can?(current_user, :read_group, parent) render_404 end @@ -70,7 +74,7 @@ class GroupsController < Groups::ApplicationController respond_to do |format| format.json do - render json: GroupChildrenSerializer + render json: GroupChildSerializer .new(current_user: current_user) .with_pagination(request, response) .represent(@children) diff --git a/config/routes/group.rb b/config/routes/group.rb index e71a3c3b190524452574603edfd4f34f7337b390..0f0ece61a38e7d8756636fd7c546893407df4209 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -41,6 +41,9 @@ scope(path: 'groups/*id', get :merge_requests, as: :merge_requests_group get :projects, as: :projects_group get :activity, as: :activity_group + scope(path: '-') do + get :children, as: :group_children + end get '/', action: :show, as: :group_canonical end diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index 7c02c9c5c485f700c122773bf0d8d1ded0b4bcdc..e2fbcefdb74d41f150f7b9da60895fa6f54fda23 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -129,7 +129,6 @@ module Gitlab notification_setting pipeline_quota projects - subgroups ].freeze ILLEGAL_PROJECT_PATH_WORDS = PROJECT_WILDCARD_ROUTES diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 83a2e82d78ce465f4c71e40c77d88f0c411b6c3e..c96a44d6186e1a86b51fb55334eb984718d01b49 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -150,39 +150,79 @@ describe GroupsController do end end - describe 'GET #subgroups', :nested_groups do - let!(:public_subgroup) { create(:group, :public, parent: group) } - let!(:private_subgroup) { create(:group, :private, parent: group) } + describe 'GET #children' do + context 'for projects' do + let!(:public_project) { create(:project, :public, namespace: group) } + let!(:private_project) { create(:project, :private, namespace: group) } - context 'as a user' do - before do - sign_in(user) - pending('spec the children path instead') + context 'as a user' do + before do + sign_in(user) + end + + it 'shows all children' do + get :children, id: group.to_param, format: :json + + expect(assigns(:children)).to contain_exactly(public_project, private_project) + end + + context 'being member of private subgroup' do + it 'shows public and private children the user is member of' do + group_member.destroy! + private_project.add_guest(user) + + get :children, id: group.to_param, format: :json + + expect(assigns(:children)).to contain_exactly(public_project, private_project) + end + end end - it 'shows all subgroups' do - get :subgroups, id: group.to_param + context 'as a guest' do + it 'shows the public children' do + get :children, id: group.to_param, format: :json - expect(assigns(:nested_groups)).to contain_exactly(public_subgroup, private_subgroup) + expect(assigns(:children)).to contain_exactly(public_project) + end end + end - context 'being member of private subgroup' do - it 'shows public and private subgroups the user is member of' do - group_member.destroy! - private_subgroup.add_guest(user) + context 'for subgroups', :nested_groups do + let!(:public_subgroup) { create(:group, :public, parent: group) } + let!(:private_subgroup) { create(:group, :private, parent: group) } + let!(:public_project) { create(:project, :public, namespace: group) } + let!(:private_project) { create(:project, :private, namespace: group) } - get :subgroups, id: group.to_param + context 'as a user' do + before do + sign_in(user) + end - expect(assigns(:nested_groups)).to contain_exactly(public_subgroup, private_subgroup) + it 'shows all children' do + get :children, id: group.to_param, format: :json + + expect(assigns(:children)).to contain_exactly(public_subgroup, private_subgroup, public_project, private_project) + end + + context 'being member of private subgroup' do + it 'shows public and private children the user is member of' do + group_member.destroy! + private_subgroup.add_guest(user) + private_project.add_guest(user) + + get :children, id: group.to_param, format: :json + + expect(assigns(:children)).to contain_exactly(public_subgroup, private_subgroup, public_project, private_project) + end end end - end - context 'as a guest' do - it 'shows the public subgroups' do - get :subgroups, id: group.to_param + context 'as a guest' do + it 'shows the public children' do + get :children, id: group.to_param, format: :json - expect(assigns(:nested_groups)).to contain_exactly(public_subgroup) + expect(assigns(:children)).to contain_exactly(public_subgroup, public_project) + end end end end diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb index 1f1c48ee9b5bf17d3f0d937c140490cdebd63e9e..f1f188cbfb53842420809884d2a25bbaee1fa2db 100644 --- a/spec/lib/gitlab/path_regex_spec.rb +++ b/spec/lib/gitlab/path_regex_spec.rb @@ -213,7 +213,7 @@ describe Gitlab::PathRegex do it 'accepts group routes' do expect(subject).to match('activity/') expect(subject).to match('group_members/') - expect(subject).to match('subgroups/') + expect(subject).to match('labels/') end it 'is not case sensitive' do @@ -246,7 +246,7 @@ describe Gitlab::PathRegex do it 'accepts group routes' do expect(subject).to match('activity/') expect(subject).to match('group_members/') - expect(subject).to match('subgroups/') + expect(subject).to match('labels/') end end @@ -268,7 +268,7 @@ 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('subgroups/more/') + expect(subject).to match('labels/more/') end end end @@ -292,7 +292,7 @@ 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/subgroups/') + expect(subject).not_to match('root/labels/') end end @@ -314,7 +314,7 @@ 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/subgroups/more/') + expect(subject).not_to match('root/labels/more/') end end end @@ -349,7 +349,7 @@ describe Gitlab::PathRegex do it 'accepts group routes' do expect(subject).to match('activity/') expect(subject).to match('group_members/') - expect(subject).to match('subgroups/') + expect(subject).to match('labels/') end it 'is not case sensitive' do @@ -382,7 +382,7 @@ 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/subgroups/') + expect(subject).to match('root/labels/') end it 'is not case sensitive' do