Fix N+1 query in Epics search for guest in private group

Prior to this change a guest in a private group would end up in the
`groups_user_can_read_epics` branch of logic in
`#permissioned_related_groups`. This loops through each subgroup one at
a time checking permissions. Instead of unwinding that logic for now we
are just going to add a logical short circuit for a common specific
case.

The `#permissioned_related_groups` only needs to check if the user has
the ability to view all epics in all subgroups in a group. Previously we
were using the `read_confidential_epic` permission to confirm that the
user could access all epics in the subgroups as well. But this is not
necessary because this class will already filter confidential epics
later in `with_confidentiality_access_check` so really we just need to
answer the question "Can a user view all epics in this group and all of
it's subgroups". This seems deceptively like we could just check the
`read_epic` permission but there is a weird edge case for public groups.
A user can view epics in a public group even if they aren't a member but
that public group can contain private subgroups and they won't be able
to view those epics. So we can conclude that if a group is `private?` as
well as the user having the ability to `read_epic` in that group then
they will also be able to `read_epic` in all child groups as well.

This MR also introduces a new explicit test for `guest` permissions with
confidential issues. The reason being that we don't appear to have any
such tests about guest users already and it may give us more confidence
that confidentiality filtering is working correctly prior to
implementing this optimization.

ee/app/finders/epics_finder.rb

@@ -95,7 +95,7 @@ class EpicsFinder < IssuableFinder
 
       # if user is member of top-level related group, he can automatically read
       # all epics in all subgroups
-      next groups if can_read_all_epics_in_related_groups?(groups)
+      next groups if can_read_all_epics_in_related_groups?(groups, include_confidential: false)
 
       groups_user_can_read_epics(groups)
     end
@@ -204,7 +204,14 @@ class EpicsFinder < IssuableFinder
     GroupMember.by_group_ids(group_ids).by_user_id(current_user).non_guests.select(:source_id)
   end
 
-  def can_read_all_epics_in_related_groups?(groups)
+  # @param include_confidential [Boolean] if this method should factor in
+  # confidential issues. Setting this to `false` will mean that it only checks
+  # the user can view all non-confidential epics within all of these groups. It
+  # does not check that they can view confidential epics and as such may return
+  # `true` even if `groups` contains a group where the user cannot view
+  # confidential epics. As such you should only call this with `false` if you
+  # are planning on filtering out confidential epics separately.
+  def can_read_all_epics_in_related_groups?(groups, include_confidential: true)
     return true if @skip_visibility_check
     return false unless current_user
 
@@ -219,7 +226,19 @@ class EpicsFinder < IssuableFinder
     # descending order (so top-level first), except if we fetch ancestors
     # - in that case top-level group is group's root parent
     parent = params.fetch(:include_ancestor_groups, false) ? groups.first.root_ancestor : group
-    Ability.allowed?(current_user, :read_confidential_epic, parent)
+
+    # If they can view confidential epics in this parent group they can
+    # definitely view confidential epics in subgroups.
+    return true if Ability.allowed?(current_user, :read_confidential_epic, parent)
+
+    # If we don't account for confidential (assume it will be filtered later by
+    # with_confidentiality_access_check) then as long as the user can see all
+    # epics in this group they can see in all subgroups. This is only true for
+    # private top level groups because it's possible that a top level public
+    # group has private subgroups and therefore they would not necessarily be
+    # able to read epics in the private subgroup even though they can in the
+    # parent group.
+    !include_confidential && parent.private? && Ability.allowed?(current_user, :read_epic, parent)
   end
 
   def by_confidential(items)

ee/changelogs/unreleased/327106-fix-epics-search-count-n-plus-1-queries.yml

+---
+title: Fix N+1 query in Epics search for guest in private group
+merge_request: 58863
+author:
+type: performance

ee/spec/finders/epics_finder_spec.rb

@@ -184,6 +184,19 @@ RSpec.describe EpicsFinder do
                 it { is_expected.to contain_exactly(subgroup_epic, subgroup2_epic, epic1, epic2, epic3) }
               end
             end
+
+            context 'when user is a guest of top level group' do
+              it 'does not have N+1 queries for subgroups' do
+                GroupMember.where(user_id: search_user.id).delete_all
+                group.add_guest(search_user)
+
+                control = ActiveRecord::QueryRecorder.new(skip_cached: false) { epics.to_a }
+
+                create_list(:group, 5, :private, parent: group)
+
+                expect { epics.to_a }.not_to exceed_all_query_limit(control)
+              end
+            end
           end
 
           it 'does not execute more than 5 SQL queries' do
@@ -590,6 +603,16 @@ RSpec.describe EpicsFinder do
             end
           end
 
+          context 'when user is a guest in the base group' do
+            before do
+              base_group.add_guest(search_user)
+            end
+
+            it 'does not return any confidential epics in the base or subgroups' do
+              expect(subject).to match_array([base_epic2, private_epic1, public_epic1])
+            end
+          end
+
           context 'when user is member of public subgroup' do
             before do
               public_group1.add_developer(search_user)