Merge branch '356131-resolve-minor-technical-debts-of-groupreleasesfinder' into 'master'

Resolve "Resolve minor technical debts of `GroupReleasesFinder`"

See merge request gitlab-org/gitlab!83295

app/finders/releases/group_releases_finder.rb

@@ -6,9 +6,8 @@ module Releases
   #
   # order_by - only ordering by released_at is supported
   # filter by tag - currently not supported
+  # include_subgroups - always true for group releases finder
   class GroupReleasesFinder
-    include Gitlab::Utils::StrongMemoize
-
     attr_reader :parent, :current_user, :params
 
     def initialize(parent, current_user = nil, params = {})
@@ -25,45 +24,26 @@ module Releases
     def execute(preload: true)
       return Release.none unless Ability.allowed?(current_user, :read_release, parent)
 
-      releases = get_releases(preload: preload)
-
+      releases = get_releases
+      releases.preloaded if preload
       paginate_releases(releases)
     end
 
     private
 
-    def include_subgroups?
-      params.fetch(:include_subgroups, false)
-    end
-
-    def accessible_projects_scope
-      if include_subgroups?
-        Project.for_group_and_its_subgroups(parent)
-      else
-        parent.projects
-      end
-    end
-
     # rubocop: disable CodeReuse/ActiveRecord
-    def get_releases(preload: true)
+    def get_releases
       Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
-        scope: releases_scope(preload: preload),
-        array_scope: accessible_projects_scope.select(:id),
+        scope: releases_scope,
+        array_scope:  Project.for_group_and_its_subgroups(parent).select(:id),
         array_mapping_scope: -> (project_id_expression) { Release.where(Release.arel_table[:project_id].eq(project_id_expression)) },
         finder_query: -> (order_by, id_expression) { Release.where(Release.arel_table[:id].eq(id_expression)) }
       )
       .execute
     end
 
-    def releases_scope(preload: true)
-      scope = Release.all
-      scope = order_releases(scope)
-      scope = scope.preloaded if preload
-      scope
-    end
-
-    def order_releases(scope)
-      scope.sort_by_attribute("released_at_#{params[:sort]}").order(id: params[:sort])
+    def releases_scope
+      Release.sort_by_attribute("released_at_#{params[:sort]}").order(id: params[:sort])
     end
 
     def paginate_releases(releases)

spec/finders/releases/group_releases_finder_spec.rb

@@ -95,8 +95,6 @@ RSpec.describe Releases::GroupReleasesFinder do
     end
 
     describe 'with subgroups' do
-      let(:params) { { include_subgroups: true } }
-
       subject(:releases) { described_class.new(group, user, params).execute(**args) }
 
       context 'with a single-level subgroup' do
@@ -164,22 +162,12 @@ RSpec.describe Releases::GroupReleasesFinder do
           end
         end
 
-        context 'when the user a guest on the group' do
-          before do
-            group.add_guest(user)
-          end
-
-          it 'returns all releases' do
-            expect(releases).to match_array([v1_1_1, v1_1_0, v6, v1_0_0, p3])
-          end
-        end
-
         context 'performance testing' do
           shared_examples 'avoids N+1 queries' do |query_params = {}|
             context 'with subgroups' do
               let(:params) { query_params }
 
-              it 'include_subgroups avoids N+1 queries' do
+              it 'subgroups avoids N+1 queries' do
                 control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do
                   releases
                 end.count
@@ -196,7 +184,6 @@ RSpec.describe Releases::GroupReleasesFinder do
           end
 
           it_behaves_like 'avoids N+1 queries'
-          it_behaves_like 'avoids N+1 queries', { simple: true }
         end
       end
     end