diff --git a/app/assets/javascripts/groups/settings/api/access_dropdown_api.js b/app/assets/javascripts/groups/settings/api/access_dropdown_api.js
index 37f95a7ab30acf281a00481b39def4d7fcb48549..e232b1977f0a2a1ce66eb6a9d13c68d4f5f42a62 100644
--- a/app/assets/javascripts/groups/settings/api/access_dropdown_api.js
+++ b/app/assets/javascripts/groups/settings/api/access_dropdown_api.js
@@ -7,15 +7,16 @@ const buildUrl = (urlRoot, url) => {
return joinPaths(urlRoot, url);
};
-const defaultOptions = { includeParentDescendants: false };
+const defaultOptions = { includeParentDescendants: false, includeParentSharedGroups: false };
export const getSubGroups = (options = defaultOptions) => {
- const { includeParentDescendants } = options;
+ const { includeParentDescendants, includeParentSharedGroups } = options;
return axios.get(buildUrl(gon.relative_url_root || '', GROUP_SUBGROUPS_PATH), {
params: {
group_id: gon.current_group_id,
include_parent_descendants: includeParentDescendants,
+ include_parent_shared_groups: includeParentSharedGroups,
},
});
};
diff --git a/app/assets/javascripts/groups/settings/components/access_dropdown.vue b/app/assets/javascripts/groups/settings/components/access_dropdown.vue
index fee1383c8e25139e7635a06df215a7e2eaa1456a..327f73b88c20bb4fd9222d5ebb0db8565756ebad 100644
--- a/app/assets/javascripts/groups/settings/components/access_dropdown.vue
+++ b/app/assets/javascripts/groups/settings/components/access_dropdown.vue
@@ -96,7 +96,7 @@ export default {
Promise.all([
this.groups.length
? Promise.resolve({ data: this.groups })
- : getSubGroups({ includeParentDescendants: true }),
+ : getSubGroups({ includeParentDescendants: true, includeParentSharedGroups: true }),
])
.then(([groupsResponse]) => {
this.consolidateData(groupsResponse.data);
diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb
index ba691bcdf0f2968a1be82275acb95bdaccaaafbe..c89722327ecf00715f420346944e486b51539b92 100644
--- a/app/finders/groups_finder.rb
+++ b/app/finders/groups_finder.rb
@@ -16,6 +16,9 @@
# filter_group_ids: array of integers - only include groups from the specified list of ids
# include_parent_descendants: boolean (defaults to false) - includes descendant groups when
# filtering by parent. The parent param must be present.
+# include_parent_shared_groups: boolean (defaults to false) - includes shared groups of a parent group
+# when filtering by parent.
+# Both parent and include_parent_descendants params must be present.
# include_ancestors: boolean (defaults to true)
# organization: Scope the groups to the Organizations::Organization
#
@@ -118,17 +121,27 @@ def by_organization(groups)
groups.in_organization(organization)
end
- # rubocop: disable CodeReuse/ActiveRecord
def by_parent(groups)
return groups unless params[:parent]
- if params.fetch(:include_parent_descendants, false)
- groups.id_in(params[:parent].descendants)
+ if include_parent_descendants?
+ by_parent_descendants(groups, params[:parent])
else
- groups.where(parent: params[:parent])
+ by_parent_children(groups, params[:parent])
+ end
+ end
+
+ def by_parent_descendants(groups, parent)
+ if include_parent_shared_groups?
+ groups.descendants_with_shared_with_groups(parent)
+ else
+ groups.id_in(parent.descendants)
end
end
- # rubocop: enable CodeReuse/ActiveRecord
+
+ def by_parent_children(groups, parent)
+ groups.by_parent(parent)
+ end
def filter_group_ids(groups)
return groups unless params[:filter_group_ids]
@@ -154,6 +167,14 @@ def sort(groups)
groups.sort_by_attribute(params[:sort])
end
+ def include_parent_shared_groups?
+ params.fetch(:include_parent_shared_groups, false)
+ end
+
+ def include_parent_descendants?
+ params.fetch(:include_parent_descendants, false)
+ end
+
def min_access_level?
current_user && params[:min_access_level].present?
end
diff --git a/app/models/group.rb b/app/models/group.rb
index d8fc468e7344f0051617c57668b8c7ede927aa48..80fcd4470a04486d00741d4e4294575cd9f6bf95 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -239,6 +239,27 @@ def of_ancestors_and_self
.where(group_group_links: { shared_group_id: group.self_and_ancestors })
end
+ # Returns all groups that are shared with the given group (see :shared_with_group)
+ # and all descendents of the given group
+ # returns none if the given group is nil
+ scope :descendants_with_shared_with_groups, -> (group) do
+ return none if group.nil?
+
+ descendants_query = group.descendants.select(:id)
+ # since we're only interested in ids, we query GroupGroupLink directly instead of using :shared_with_group
+ # to avoid an extra JOIN in the resulting query
+ shared_groups_query = GroupGroupLink
+ .where(shared_group_id: group.id)
+ .select('shared_with_group_id AS id')
+
+ combined_query = Group
+ .from_union(descendants_query, shared_groups_query, alias_as: :combined)
+ .unscope(where: :type)
+ .select(:id)
+
+ id_in(combined_query)
+ end
+
# WARNING: This method should never be used on its own
# please do make sure the number of rows you are filtering is small
# enough for this query
diff --git a/ee/app/finders/autocomplete/group_subgroups_finder.rb b/ee/app/finders/autocomplete/group_subgroups_finder.rb
index d150de44d05286a6489d30b059b69d4f12382703..36bbd0524800113b0f226dfe072476f89af2edbc 100644
--- a/ee/app/finders/autocomplete/group_subgroups_finder.rb
+++ b/ee/app/finders/autocomplete/group_subgroups_finder.rb
@@ -15,6 +15,10 @@ def include_parent_descendants?
Gitlab::Utils.to_boolean(params[:include_parent_descendants])
end
+ def include_parent_shared_groups?
+ Gitlab::Utils.to_boolean(params[:include_parent_shared_groups])
+ end
+
def group_id
params[:group_id]
end
@@ -23,7 +27,8 @@ def group_id
def execute
group = ::Autocomplete::GroupFinder.new(current_user, nil, group_id: group_id).execute
GroupsFinder.new(current_user, parent: group,
- include_parent_descendants: include_parent_descendants?).execute.limit(LIMIT)
+ include_parent_descendants: include_parent_descendants?,
+ include_parent_shared_groups: include_parent_shared_groups?).execute.limit(LIMIT)
end
# rubocop: enable CodeReuse/Finder
end
diff --git a/ee/spec/finders/autocomplete/group_subgroups_finder_spec.rb b/ee/spec/finders/autocomplete/group_subgroups_finder_spec.rb
index bf2a3974c4c8ba2b952fb09507012c9f7a9201c2..ac416291199a88e54080323cf3e643226550dad3 100644
--- a/ee/spec/finders/autocomplete/group_subgroups_finder_spec.rb
+++ b/ee/spec/finders/autocomplete/group_subgroups_finder_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe Autocomplete::GroupSubgroupsFinder do
+RSpec.describe Autocomplete::GroupSubgroupsFinder, feature_category: :groups_and_projects do
describe '#execute' do
let_it_be(:group) { create(:group, :private) }
let_it_be(:subgroup_1) { create(:group, :private, parent: group) }
@@ -10,10 +10,17 @@
let_it_be(:grandchild_1) { create(:group, :private, parent: subgroup_1) }
let_it_be(:member_in_group) { create(:user).tap { |u| group.add_reporter(u) } }
let_it_be(:member_in_subgroup) { create(:user).tap { |u| subgroup_1.add_reporter(u) } }
+ let_it_be(:invited_to_group) { create(:group, :public) }
+ let_it_be(:invited_to_subgroup) { create(:group, :public) }
let(:params) { { group_id: group.id } }
let(:current_user) { member_in_group }
+ before do
+ group.shared_with_groups << invited_to_group
+ grandchild_1.shared_with_groups << invited_to_subgroup
+ end
+
subject { described_class.new(current_user, params).execute }
it 'returns subgroups', :aggregate_failures do
@@ -21,6 +28,22 @@
expect(subject).to contain_exactly(subgroup_1, subgroup_2)
end
+ context 'when include_parent_shared_groups parameter is true' do
+ before do
+ params[:include_parent_shared_groups] = true
+ params[:include_parent_descendants] = true
+ end
+
+ it 'returns subgroups and shared groups' do
+ expect(subject.count).to eq(4)
+ expect(subject).to contain_exactly(
+ subgroup_1,
+ subgroup_2,
+ grandchild_1,
+ invited_to_group)
+ end
+ end
+
context 'when include_parent_descendants parameter is true' do
before do
params[:include_parent_descendants] = true
diff --git a/spec/finders/groups_finder_spec.rb b/spec/finders/groups_finder_spec.rb
index b9add5b7db2de018e6133763426e804e93a6ae40..c1ca19e29ec44bcfd7aca03e915ed4161afaa7a5 100644
--- a/spec/finders/groups_finder_spec.rb
+++ b/spec/finders/groups_finder_spec.rb
@@ -180,17 +180,29 @@
let_it_be(:internal_sub_subgroup) { create(:group, :internal, parent: public_subgroup) }
let_it_be(:private_sub_subgroup) { create(:group, :private, parent: public_subgroup) }
let_it_be(:public_sub_subgroup) { create(:group, :public, parent: public_subgroup) }
+ let_it_be(:invited_to_group) { create(:group, :public) }
+ let_it_be(:invited_to_subgroup) { create(:group, :public) }
let(:params) { { include_parent_descendants: true, parent: parent_group } }
+ before do
+ parent_group.shared_with_groups << invited_to_group
+ public_subgroup.shared_with_groups << invited_to_subgroup
+ end
+
context 'with nil parent' do
- it 'returns all accessible groups' do
+ before do
params[:parent] = nil
+ end
+
+ it 'returns all accessible groups' do
expect(described_class.new(user, params).execute).to contain_exactly(
parent_group,
public_subgroup,
internal_sub_subgroup,
- public_sub_subgroup
+ public_sub_subgroup,
+ invited_to_group,
+ invited_to_subgroup
)
end
end
@@ -227,6 +239,22 @@
private_sub_subgroup
)
end
+
+ context 'when include shared groups is set' do
+ before do
+ params[:include_parent_shared_groups] = true
+ end
+
+ it 'returns all group descendants with shared groups' do
+ expect(described_class.new(user, params).execute).to contain_exactly(
+ public_subgroup,
+ public_sub_subgroup,
+ internal_sub_subgroup,
+ private_sub_subgroup,
+ invited_to_group
+ )
+ end
+ end
end
end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index f9f18a8b23e8acd0b73590a3f8b022f87bfb52f8..95c4af9c41c4b818c8edcd8f4efeb7e4e24c6bba 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -1306,6 +1306,37 @@
it { is_expected.to match_array(groups) }
end
+
+ describe 'descendants_with_shared_with_groups' do
+ subject { described_class.descendants_with_shared_with_groups(parent_group) }
+
+ let_it_be(:grand_parent_group) { create(:group, :public) }
+ let_it_be(:parent_group) { create(:group, :public, parent: grand_parent_group) }
+ let_it_be(:subgroup) { create(:group, :public, parent: parent_group) }
+ let_it_be(:subsubgroup) { create(:group, :public, parent: subgroup) }
+
+ let_it_be(:shared_to_group) { create(:group, :public) }
+ let_it_be(:shared_to_sub_group) { create(:group, :public) }
+
+ context 'when parent group is nil' do
+ let(:parent_group) { nil }
+
+ it { is_expected.to match_array([]) }
+ end
+
+ context 'when parent group is present and there are shared groups' do
+ before do
+ parent_group.shared_with_groups << shared_to_group
+ subgroup.shared_with_groups << shared_to_sub_group
+ end
+
+ it { is_expected.to match_array([subgroup, subsubgroup, shared_to_group]) }
+ end
+
+ context 'when parent group is present and there are no shared groups' do
+ it { is_expected.to match_array([subgroup, subsubgroup]) }
+ end
+ end
end
describe '#to_reference' do