Merge branch...

Merge branch '12254-error-when-viewing-page-2-or-later-of-elasticsearch-search-results' into 'master'

Fix ElasticSearch search results pagination

Closes #12254

See merge request gitlab-org/gitlab-ee!14258

ee/lib/gitlab/elastic/search_results.rb

@@ -22,7 +22,9 @@ module Gitlab
         @public_and_internal_projects = public_and_internal_projects
       end
 
-      def objects(scope, page = nil)
+      def objects(scope, page = 1)
+        page = (page || 1).to_i
+
         case scope
         when 'projects'
           eager_load(projects, page, eager: [:route, :namespace])
@@ -45,21 +47,6 @@ module Gitlab
         end
       end
 
-      # Apply some eager loading to the `records` of an ES result object without
-      # losing pagination information
-      def eager_load(es_result, page, eager:)
-        page ||= 1
-        paginated_base = es_result.page(page).per(per_page)
-        relation = paginated_base.records.includes(eager) # rubocop:disable CodeReuse/ActiveRecord
-
-        Kaminari.paginate_array(
-          relation,
-          total_count: paginated_base.total_count,
-          limit: per_page,
-          offset: per_page * (page - 1)
-        )
-      end
-
       def generic_search_results
         @generic_search_results ||= Gitlab::SearchResults.new(current_user, limit_projects, query)
       end
@@ -150,6 +137,20 @@ module Gitlab
 
       private
 
+      # Apply some eager loading to the `records` of an ES result object without
+      # losing pagination information
+      def eager_load(es_result, page, eager:)
+        paginated_base = es_result.page(page).per(per_page)
+        relation = paginated_base.records.includes(eager) # rubocop:disable CodeReuse/ActiveRecord
+
+        Kaminari.paginate_array(
+          relation,
+          total_count: paginated_base.total_count,
+          limit: per_page,
+          offset: per_page * (page - 1)
+        )
+      end
+
       def base_options
         {
           current_user: current_user,

ee/spec/lib/gitlab/elastic/search_results_spec.rb

@@ -25,6 +25,20 @@ describe Gitlab::Elastic::SearchResults, :elastic do
     end
   end
 
+  shared_examples_for 'a paginated object' do |object_type|
+    let(:results) { described_class.new(user, 'hello world', limit_project_ids) }
+
+    it 'does not explode when given a page as a string' do
+      expect { results.objects(object_type, "2") }.not_to raise_error
+    end
+
+    it 'paginates' do
+      objects = results.objects(object_type, 2)
+      expect(objects).to respond_to(:total_count, :limit, :offset)
+      expect(objects.offset_value).to eq(20)
+    end
+  end
+
   describe 'parse_search_result' do
     let(:blob) do
       {
@@ -93,6 +107,8 @@ describe Gitlab::Elastic::SearchResults, :elastic do
       Gitlab::Elastic::Helper.refresh_index
     end
 
+    it_behaves_like 'a paginated object', 'issues'
+
     it 'lists found issues' do
       results = described_class.new(user, 'hello world', limit_project_ids)
       issues = results.objects('issues')
@@ -349,6 +365,8 @@ describe Gitlab::Elastic::SearchResults, :elastic do
       Gitlab::Elastic::Helper.refresh_index
     end
 
+    it_behaves_like 'a paginated object', 'merge_requests'
+
     it 'lists found merge requests' do
       results = described_class.new(user, 'hello world', limit_project_ids)
       merge_requests = results.objects('merge_requests')
@@ -435,6 +453,8 @@ describe Gitlab::Elastic::SearchResults, :elastic do
       end
     end
 
+    it_behaves_like 'a paginated object', 'blobs'
+
     it 'finds blobs' do
       results = described_class.new(user, 'def', limit_project_ids)
       blobs = results.objects('blobs')
@@ -556,6 +576,8 @@ describe Gitlab::Elastic::SearchResults, :elastic do
       Gitlab::Elastic::Helper.refresh_index
     end
 
+    it_behaves_like 'a paginated object', 'wiki_blobs'
+
     it 'finds wiki blobs' do
       blobs = results.objects('wiki_blobs')
 
@@ -629,6 +651,8 @@ describe Gitlab::Elastic::SearchResults, :elastic do
       Gitlab::Elastic::Helper.refresh_index
     end
 
+    it_behaves_like 'a paginated object', 'commits'
+
     it 'finds commits' do
       results = described_class.new(user, 'add', limit_project_ids)
       commits = results.objects('commits')
@@ -704,6 +728,8 @@ describe Gitlab::Elastic::SearchResults, :elastic do
         Gitlab::Elastic::Helper.refresh_index
       end
 
+      it_behaves_like 'a paginated object', 'milestones'
+
       context 'when project ids are present' do
         context 'when authenticated' do
           context 'when user and merge requests are disabled in a project' do
@@ -811,6 +837,8 @@ describe Gitlab::Elastic::SearchResults, :elastic do
     end
 
     context 'Projects' do
+      it_behaves_like 'a paginated object', 'projects'
+
       it 'finds right set of projects' do
         internal_project
         private_project1