diff --git a/app/finders/clusters/agents/authorizations/ci_access/finder.rb b/app/finders/clusters/agents/authorizations/ci_access/finder.rb index 97d378669a4c726e0679e5ff9bb676af2c5425fb..215fa76ac605a03aedaf0f480427971c5bb713e4 100644 --- a/app/finders/clusters/agents/authorizations/ci_access/finder.rb +++ b/app/finders/clusters/agents/authorizations/ci_access/finder.rb @@ -66,7 +66,7 @@ def group_authorizations # rubocop: enable CodeReuse/ActiveRecord def all_namespace_ids - project.root_ancestor.self_and_descendants.select(:id) + project.root_ancestor.self_and_descendant_ids end end end diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index a6c7588d19dd735a0df88a6deba24e36ffd8122d..2b14e67bed100cf61dba971d162fbb3e4316d896 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -162,7 +162,7 @@ def projects if group? @projects = if params[:include_descendant_groups] - @projects.in_namespace(group.self_and_descendants.select(:id)) + @projects.in_namespace(group.self_and_descendant_ids) else @projects.in_namespace(group.id) end diff --git a/app/models/group.rb b/app/models/group.rb index cac322ea7ebe9e255451ebffc1c4363765503a7c..a98611b9f3333f99557289f92a6f85966a9f20a9 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -681,7 +681,7 @@ def members_from_self_and_ancestors_with_effective_access_level def members_with_descendants GroupMember .active_without_invites_and_requests - .where(source_id: self_and_descendants.reorder(nil).select(:id)) + .where(source_id: self_and_descendant_ids) end # Returns all members that are part of the group, it's subgroups, and ancestor groups diff --git a/app/models/project.rb b/app/models/project.rb index 6cac6f680167397673de0eb4f1ab4a4b3c52088e..c9cfaa934bb08f3ba833e2cd65b3c47d4148d3b2 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -946,7 +946,7 @@ def self.inactive # We require an alias to the project_mirror_data_table in order to use import_state in our queries scope :joins_import_state, -> { joins("INNER JOIN project_mirror_data import_state ON import_state.project_id = projects.id") } scope :for_group, -> (group) { where(group: group) } - scope :for_group_and_its_subgroups, ->(group) { where(namespace_id: group.self_and_descendants.select(:id)) } + scope :for_group_and_its_subgroups, ->(group) { where(namespace_id: group.self_and_descendant_ids) } scope :for_group_and_its_ancestor_groups, ->(group) { where(namespace_id: group.self_and_ancestors.select(:id)) } scope :is_importing, -> { with_import_state.where(import_state: { status: %w[started scheduled] }) } diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb index 387c5ce063a89f893b6bb50d9e29b8c57d12048d..cc5f3fe4e1320cc7bc2494b988458e54f47efefa 100644 --- a/app/services/todos/destroy/entity_leave_service.rb +++ b/app/services/todos/destroy/entity_leave_service.rb @@ -125,7 +125,7 @@ def unauthorized_private_groups # rubocop: enable CodeReuse/ActiveRecord def non_authorized_reporter_groups - entity.self_and_descendants.select(:id) + entity.self_and_descendant_ids .id_not_in(authorized_reporter_groups) end diff --git a/doc/development/database/efficient_in_operator_queries.md b/doc/development/database/efficient_in_operator_queries.md index c426781d71100b46468e0644d28b96a7a18854ce..6c11b6eadf33c5ef1e60c2a2784d513836b1e2a0 100644 --- a/doc/development/database/efficient_in_operator_queries.md +++ b/doc/development/database/efficient_in_operator_queries.md @@ -633,7 +633,7 @@ order = Gitlab::Pagination::Keyset::Order.build([ records = Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new( scope: Epic.where.not(closed_at: nil).reorder(order), # filter out NULL values - array_scope: Group.find(9970).self_and_descendants.select(:id), + array_scope: Group.find(9970).self_and_descendant_ids, array_mapping_scope: -> (id_expression) { Epic.where(Epic.arel_table[:group_id].eq(id_expression)) } ).execute.limit(20) @@ -714,7 +714,7 @@ scope = cte.apply_to(Issue.where({}).reorder(order)) opts = { scope: scope, - array_scope: Project.where(namespace_id: top_level_group.self_and_descendants.select(:id)).select(:id), + array_scope: Project.where(namespace_id: top_level_group.self_and_descendant_ids).select(:id), array_mapping_scope: -> (id_expression) { Issue.where(Issue.arel_table[:project_id].eq(id_expression)) } } diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 02f257ba6e7f5e3ece340ab463dc90665e6771b6..0a7044dc014f9a0118face79ad3c728541e7cc17 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -642,7 +642,7 @@ def vulnerabilities def vulnerability_reads(use_traversal_ids: false) return ::Vulnerabilities::Read.by_group(self) if use_traversal_ids - ::Vulnerabilities::Read.where(namespace_id: self_and_descendants.select(:id)) + ::Vulnerabilities::Read.where(namespace_id: self_and_descendant_ids) end def next_traversal_ids @@ -746,7 +746,7 @@ def users_count end def releases_count - ::Release.by_namespace_id(self_and_descendants.select(:id)).count + ::Release.by_namespace_id(self_and_descendant_ids).count end def releases_percentage @@ -758,7 +758,7 @@ def releases_percentage self.class.count_by_sql( ::Project.select(calculate_sql) - .where(namespace_id: self_and_descendants.select(:id)).to_sql + .where(namespace_id: self_and_descendant_ids).to_sql ) end @@ -1016,7 +1016,7 @@ def disable_personal_access_tokens? def active_project_tokens_of_root_ancestor root_ancestor_and_descendants_project_bots = ::User .joins(projects: :group) - .where(namespaces: { id: root_ancestor.self_and_descendants.select(:id) }) + .where(namespaces: { id: root_ancestor.self_and_descendant_ids }) .project_bot .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/428542") diff --git a/ee/lib/gitlab/auth/group_saml/membership_updater.rb b/ee/lib/gitlab/auth/group_saml/membership_updater.rb index d0a7bba9a9ffb64f1aba607b242b25d101af6331..87628e28364d89e242d9332a32bb179222948519 100644 --- a/ee/lib/gitlab/auth/group_saml/membership_updater.rb +++ b/ee/lib/gitlab/auth/group_saml/membership_updater.rb @@ -79,7 +79,7 @@ def any_group_links_in_hierarchy? end def group_ids_in_hierarchy - group.self_and_descendants.select(:id) + group.self_and_descendant_ids end # rubocop:enable CodeReuse/ActiveRecord diff --git a/ee/spec/models/ee/release_spec.rb b/ee/spec/models/ee/release_spec.rb index 1fd256e37a09a08c1b5220de12976536595b6124..be561a51c0088533885ba1f87719f948951ab5d0 100644 --- a/ee/spec/models/ee/release_spec.rb +++ b/ee/spec/models/ee/release_spec.rb @@ -27,7 +27,7 @@ end context 'when an array of namespace ids is passed' do - let(:ns_id) { group.self_and_descendants.select(:id) } + let(:ns_id) { group.self_and_descendant_ids } it 'returns releases associated to projects of all provided groups' do expect(described_class.by_namespace_id(ns_id)).to match_array( diff --git a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb index 1c67c9e0b95125e0bbc5499947e3c06d9ab4eb7f..c471a6f0370969990d7b588648117129779be884 100644 --- a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb @@ -97,7 +97,7 @@ let(:in_operator_optimization_options) do { - array_scope: Project.where(namespace_id: top_level_group.self_and_descendants.select(:id)).select(:id), + array_scope: Project.where(namespace_id: top_level_group.self_and_descendant_ids).select(:id), array_mapping_scope: -> (id_expression) { ignored_column_model.where(ignored_column_model.arel_table[:project_id].eq(id_expression)) }, finder_query: -> (id_expression) { ignored_column_model.where(ignored_column_model.arel_table[:id].eq(id_expression)) } } @@ -146,7 +146,7 @@ let(:in_operator_optimization_options) do { - array_scope: Project.where(namespace_id: top_level_group.self_and_descendants.select(:id)).select(:id), + array_scope: Project.where(namespace_id: top_level_group.self_and_descendant_ids).select(:id), array_mapping_scope: -> (id_expression) { Issue.where(Issue.arel_table[:project_id].eq(id_expression)) }, finder_query: -> (id_expression) { Issue.where(Issue.arel_table[:id].eq(id_expression)) } } @@ -200,7 +200,7 @@ let(:in_operator_optimization_options) do { - array_scope: Project.where(namespace_id: top_level_group.self_and_descendants.select(:id)).select(:id), + array_scope: Project.where(namespace_id: top_level_group.self_and_descendant_ids).select(:id), array_mapping_scope: -> (id_expression) { Issue.where(Issue.arel_table[:project_id].eq(id_expression)) }, finder_query: -> (_relative_position_expression, id_expression) { Issue.where(Issue.arel_table[:id].eq(id_expression)) } } @@ -224,7 +224,7 @@ let(:in_operator_optimization_options) do { - array_scope: Project.where(namespace_id: top_level_group.self_and_descendants.select(:id)).select(:id), + array_scope: Project.where(namespace_id: top_level_group.self_and_descendant_ids).select(:id), array_mapping_scope: -> (id_expression) { Issue.merge(base_scope.dup).where(Issue.arel_table[:project_id].eq(id_expression)) }, finder_query: -> (_relative_position_expression, id_expression) { Issue.where(Issue.arel_table[:id].eq(id_expression)) } } @@ -250,7 +250,7 @@ let(:in_operator_optimization_options) do { - array_scope: Project.where(namespace_id: top_level_group.self_and_descendants.select(:id)).select(:id), + array_scope: Project.where(namespace_id: top_level_group.self_and_descendant_ids).select(:id), array_mapping_scope: -> (id_expression) { Issue.where(Issue.arel_table[:project_id].eq(id_expression)) }, finder_query: -> (_created_at_expression, id_expression) { Issue.where(Issue.arel_table[:id].eq(id_expression)) } } @@ -282,7 +282,7 @@ let(:options) do { scope: scope, - array_scope: Project.where(namespace_id: top_level_group.self_and_descendants.select(:id)).select(:id), + array_scope: Project.where(namespace_id: top_level_group.self_and_descendant_ids).select(:id), array_mapping_scope: -> (id_expression) { Issue.where(Issue.arel_table[:project_id].eq(id_expression)) }, finder_query: -> (id_expression) { Issue.where(Issue.arel_table[:id].eq(id_expression)) } } @@ -330,7 +330,7 @@ def paginator(cursor = nil) options = { scope: scope, - array_scope: Project.where(namespace_id: top_level_group.self_and_descendants.select(:id)).select(:id), + array_scope: Project.where(namespace_id: top_level_group.self_and_descendant_ids).select(:id), array_mapping_scope: -> (id_expression) { Issue.where(Issue.arel_table[:project_id].eq(id_expression)) }, finder_query: -> (id_expression) { Issue.where(Issue.arel_table[:id].eq(id_expression)) } } @@ -360,7 +360,7 @@ def paginator(cursor = nil) let(:in_operator_optimization_options) do { - array_scope: Project.where(namespace_id: top_level_group.self_and_descendants.select(:id)).select(:id), + array_scope: Project.where(namespace_id: top_level_group.self_and_descendant_ids).select(:id), array_mapping_scope: -> (id_expression) { Issue.where(Issue.arel_table[:project_id].eq(id_expression)) } } end @@ -402,7 +402,7 @@ def paginator(cursor = nil) let(:in_operator_optimization_options) do { - array_scope: Project.where(namespace_id: top_level_group.self_and_descendants.select(:id)).select(:id), + array_scope: Project.where(namespace_id: top_level_group.self_and_descendant_ids).select(:id), array_mapping_scope: -> (id_expression) { Issue.where(Issue.arel_table[:project_id].eq(id_expression)) } } end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 58923fe4d04e75ec74eb0ade7261eb2dee6e767a..86df51d947bc8d0237d36c1aaaaeaf213e277647 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -3936,4 +3936,20 @@ def define_cache_expectations(cache_key) expect(group.group_readme).to be(nil) end end + + describe '#members_with_descendants' do + it 'returns members from root and descendant groups' do + group = create(:group) + subgroup = create(:group, parent: group) + subsubgroup = create(:group, parent: subgroup) + + group_member1 = create(:group_member, source: group) + group_member2 = create(:group_member, source: subgroup) + group_member3 = create(:group_member, source: subsubgroup) + create(:group_member) # member outside of the hierarchy + + expect(group.members_with_descendants).to match_array([group_member1, group_member2, group_member3]) + expect(subgroup.members_with_descendants).to match_array([group_member2, group_member3]) + end + end end