diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb index 0080123407d..7d419103b1c 100644 --- a/app/finders/groups_finder.rb +++ b/app/finders/groups_finder.rb @@ -11,6 +11,7 @@ # parent: Group # all_available: boolean (defaults to true) # min_access_level: integer +# exclude_group_ids: array of integers # # Users with full private access can see all groups. The `owned` and `parent` # params can be used to restrict the groups that are returned. @@ -29,6 +30,7 @@ class GroupsFinder < UnionFinder items = all_groups.map do |item| item = by_parent(item) item = by_custom_attributes(item) + item = exclude_group_ids(item) item end @@ -72,6 +74,12 @@ class GroupsFinder < UnionFinder end # rubocop: enable CodeReuse/ActiveRecord + def exclude_group_ids(groups) + return groups unless params[:exclude_group_ids] + + groups.id_not_in(params[:exclude_group_ids]) + end + # rubocop: disable CodeReuse/ActiveRecord def by_parent(groups) return groups unless params[:parent] diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 9d028dccad7..7af766c8544 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -118,11 +118,12 @@ module GroupsHelper end def parent_group_options(current_group) - groups = current_user.owned_groups.sort_by(&:human_name).map do |group| + exclude_groups = current_group.self_and_descendants.pluck_primary_key + exclude_groups << current_group.parent_id if current_group.parent_id + groups = GroupsFinder.new(current_user, min_access_level: Gitlab::Access::OWNER, exclude_group_ids: exclude_groups).execute.sort_by(&:human_name).map do |group| { id: group.id, text: group.human_name } end - groups.delete_if { |group| group[:id] == current_group.id } groups.to_json end diff --git a/changelogs/unreleased/fix-transfer-group-possibilities.yml b/changelogs/unreleased/fix-transfer-group-possibilities.yml new file mode 100644 index 00000000000..ebefb47b3da --- /dev/null +++ b/changelogs/unreleased/fix-transfer-group-possibilities.yml @@ -0,0 +1,5 @@ +--- +title: Fix group transfer selection possibilities +merge_request: 26123 +author: Peter Marko +type: fixed diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 91541a16c13..1763c46389a 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -229,4 +229,37 @@ describe GroupsHelper do expect(helper.group_sidebar_links).not_to include(*cross_project_features) end end + + describe 'parent_group_options', :nested_groups do + let(:current_user) { create(:user) } + let(:group) { create(:group, name: 'group') } + let(:group2) { create(:group, name: 'group2') } + + before do + group.add_owner(current_user) + group2.add_owner(current_user) + end + + it 'includes explicitly owned groups except self' do + expect(parent_group_options(group2)).to eq([{ id: group.id, text: group.human_name }].to_json) + end + + it 'excludes parent group' do + subgroup = create(:group, parent: group2) + + expect(parent_group_options(subgroup)).to eq([{ id: group.id, text: group.human_name }].to_json) + end + + it 'includes subgroups with inherited ownership' do + subgroup = create(:group, parent: group) + + expect(parent_group_options(group2)).to eq([{ id: group.id, text: group.human_name }, { id: subgroup.id, text: subgroup.human_name }].to_json) + end + + it 'excludes own subgroups' do + create(:group, parent: group2) + + expect(parent_group_options(group2)).to eq([{ id: group.id, text: group.human_name }].to_json) + end + end end