diff options
| -rw-r--r-- | app/controllers/groups_controller.rb | 8 | ||||
| -rw-r--r-- | config/routes/group.rb | 3 | ||||
| -rw-r--r-- | lib/gitlab/path_regex.rb | 1 | ||||
| -rw-r--r-- | spec/controllers/groups_controller_spec.rb | 82 | ||||
| -rw-r--r-- | spec/lib/gitlab/path_regex_spec.rb | 14 |
5 files changed, 77 insertions, 31 deletions
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 17d40cedd5d..c967488fff4 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 e71a3c3b190..0f0ece61a38 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 7c02c9c5c48..e2fbcefdb74 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 83a2e82d78c..c96a44d6186 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 1f1c48ee9b5..f1f188cbfb5 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 |