Merge branch '324075-vsa-pagination' into 'master'

Add pagination for VSA records

See merge request gitlab-org/gitlab!57843

ee/app/controllers/groups/analytics/cycle_analytics/stages_controller.rb

@@ -56,7 +56,11 @@ module Groups
         def records
           return render_403 unless can?(current_user, :read_group_stage, @group)
 
-          render json: data_collector.serialized_records
+          serialized_records = data_collector.serialized_records do |relation|
+            add_pagination_headers(relation)
+          end
+
+          render json: serialized_records
         end
 
         def duration_chart
@@ -135,6 +139,17 @@ module Groups
             @value_stream = @group.value_streams.find(params[:value_stream_id])
           end
         end
+
+        def add_pagination_headers(relation)
+          Gitlab::Pagination::OffsetHeaderBuilder.new(
+            request_context: self,
+            per_page: relation.limit_value,
+            page: relation.current_page,
+            next_page: relation.next_page,
+            prev_page: relation.prev_page,
+            params: permitted_cycle_analytics_params
+          ).execute(exclude_total_headers: true, data_without_counts: true)
+        end
       end
     end
   end

ee/lib/gitlab/analytics/cycle_analytics/request_params.rb

@@ -18,6 +18,7 @@ module Gitlab
           :milestone_title,
           :sort,
           :direction,
+          :page,
           label_name: [].freeze,
           assignee_username: [].freeze,
           project_ids: [].freeze
@@ -39,6 +40,7 @@ module Gitlab
         attribute :value_stream
         attribute :sort
         attribute :direction
+        attribute :page
 
         FINDER_PARAM_NAMES.each do |param_name|
           attribute param_name
@@ -68,7 +70,8 @@ module Gitlab
             to: created_before,
             project_ids: project_ids,
             sort: sort&.to_sym,
-            direction: direction&.to_sym
+            direction: direction&.to_sym,
+            page: page
           }.merge(attributes.symbolize_keys.slice(*FINDER_PARAM_NAMES))
         end
 

ee/spec/support/shared_examples/controllers/analytics/cycle_analytics/shared_stage_shared_examples.rb

@@ -234,6 +234,19 @@ RSpec.shared_examples 'Value Stream Analytics Stages controller' do
           expect(response).to have_gitlab_http_status(:ok)
         end
       end
+
+      context 'pagination' do
+        it 'exposes pagination headers' do
+          create_list(:merge_request, 3)
+          stub_const('Gitlab::Analytics::CycleAnalytics::RecordsFetcher::MAX_RECORDS', 2)
+          allow_any_instance_of(Gitlab::Analytics::CycleAnalytics::RecordsFetcher).to receive(:query).and_return(MergeRequest.join_metrics.all)
+
+          subject
+
+          expect(response.headers['X-Next-Page']).to eq('2')
+          expect(response.headers['Link']).to include('rel="next"')
+        end
+      end
     end
 
     describe 'GET #duration_chart' do

lib/gitlab/analytics/cycle_analytics/records_fetcher.rb

@@ -31,14 +31,34 @@ module Gitlab
           @params = params
           @sort = params[:sort] || :end_event
           @direction = params[:direction] || :desc
+          @page = params[:page] || 1
+          @per_page = MAX_RECORDS
         end
 
+        # rubocop: disable CodeReuse/ActiveRecord
         def serialized_records
           strong_memoize(:serialized_records) do
             # special case (legacy): 'Test' and 'Staging' stages should show Ci::Build records
             if default_test_stage? || default_staging_stage?
+              ci_build_join = mr_metrics_table
+                .join(build_table)
+                .on(mr_metrics_table[:pipeline_id].eq(build_table[:commit_id]))
+                .join_sources
+
+              records = ordered_and_limited_query
+                .joins(ci_build_join)
+                .select(build_table[:id], *time_columns)
+
+              yield records if block_given?
+              ci_build_records = preload_ci_build_associations(records)
+
               AnalyticsBuildSerializer.new.represent(ci_build_records.map { |e| e['build'] })
             else
+              records = ordered_and_limited_query.select(*columns, *time_columns)
+
+              yield records if block_given?
+              records = preload_associations(records)
+
               records.map do |record|
                 project = record.project
                 attributes = record.attributes.merge({
@@ -51,10 +71,11 @@ module Gitlab
             end
           end
         end
+        # rubocop: enable CodeReuse/ActiveRecord
 
         private
 
-        attr_reader :stage, :query, :params, :sort, :direction
+        attr_reader :stage, :query, :params, :sort, :direction, :page, :per_page
 
         def columns
           MAPPINGS.fetch(subject_class).fetch(:columns_for_select).map do |column_name|
@@ -74,41 +95,32 @@ module Gitlab
           MAPPINGS.fetch(subject_class).fetch(:serializer_class).new
         end
 
-        # Loading Ci::Build records instead of MergeRequest records
         # rubocop: disable CodeReuse/ActiveRecord
-        def ci_build_records
-          ci_build_join = mr_metrics_table
-            .join(build_table)
-            .on(mr_metrics_table[:pipeline_id].eq(build_table[:commit_id]))
-            .join_sources
-
-          q = ordered_and_limited_query
-            .joins(ci_build_join)
-            .select(build_table[:id], *time_columns)
-
-          results = execute_query(q).to_a
+        def preload_ci_build_associations(records)
+          results = records.map(&:attributes)
 
           Gitlab::CycleAnalytics::Updater.update!(results, from: 'id', to: 'build', klass: ::Ci::Build.includes({ project: [:namespace], user: [], pipeline: [] }))
         end
+        # rubocop: enable CodeReuse/ActiveRecord
 
         def ordered_and_limited_query
-          order_by(query, sort, direction, columns).limit(MAX_RECORDS)
+          strong_memoize(:ordered_and_limited_query) do
+            order_by(query, sort, direction, columns).page(page).per(per_page).without_count
+          end
         end
 
-        def records
-          results = ordered_and_limited_query
-            .select(*columns, *time_columns)
-
+        # rubocop: disable CodeReuse/ActiveRecord
+        def preload_associations(records)
           # using preloader instead of includes to avoid AR generating a large column list
           ActiveRecord::Associations::Preloader.new.preload(
-            results,
+            records,
             MAPPINGS.fetch(subject_class).fetch(:includes_for_query)
           )
 
-          results
+          records
         end
-        # rubocop: enable CodeReuse/ActiveRecord
 
+        # rubocop: enable CodeReuse/ActiveRecord
         def time_columns
           [
             stage.start_event.timestamp_projection.as('start_event_timestamp'),

lib/gitlab/pagination/offset_header_builder.rb

@@ -5,9 +5,9 @@ module Gitlab
     class OffsetHeaderBuilder
       attr_reader :request_context, :per_page, :page, :next_page, :prev_page, :total, :total_pages
 
-      delegate :params, :header, :request, to: :request_context
+      delegate :request, to: :request_context
 
-      def initialize(request_context:, per_page:, page:, next_page:, prev_page: nil, total:, total_pages:)
+      def initialize(request_context:, per_page:, page:, next_page:, prev_page: nil, total: nil, total_pages: nil, params: nil)
         @request_context = request_context
         @per_page = per_page
         @page = page
@@ -15,6 +15,7 @@ module Gitlab
         @prev_page = prev_page
         @total = total
         @total_pages = total_pages
+        @params = params
       end
 
       def execute(exclude_total_headers: false, data_without_counts: false)
@@ -56,10 +57,24 @@ module Gitlab
       end
 
       def page_href(next_page_params = {})
-        query_params = params.merge(**next_page_params, per_page: params[:per_page]).to_query
+        query_params = params.merge(**next_page_params, per_page: per_page).to_query
 
         build_page_url(query_params: query_params)
       end
+
+      def params
+        @params || request_context.params
+      end
+
+      def header(name, value)
+        if request_context.respond_to?(:header)
+          # For Grape API
+          request_context.header(name, value)
+        else
+          # For rails controllers
+          request_context.response.headers[name] = value
+        end
+      end
     end
   end
 end

spec/lib/gitlab/analytics/cycle_analytics/records_fetcher_spec.rb

@@ -7,16 +7,15 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RecordsFetcher do
     Timecop.freeze { example.run }
   end
 
+  let(:params) { { from: 1.year.ago, current_user: user } }
+
   let_it_be(:project) { create(:project, :empty_repo) }
   let_it_be(:user) { create(:user) }
 
   subject do
     Gitlab::Analytics::CycleAnalytics::DataCollector.new(
       stage: stage,
-      params: {
-        from: 1.year.ago,
-        current_user: user
-      }
+      params: params
     ).records_fetcher.serialized_records
   end
 
@@ -131,4 +130,40 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RecordsFetcher do
       end
     end
   end
+
+  describe 'pagination' do
+    let_it_be(:issue1) { create(:issue, project: project) }
+    let_it_be(:issue2) { create(:issue, project: project) }
+    let_it_be(:issue3) { create(:issue, project: project) }
+
+    let(:stage) do
+      build(:cycle_analytics_project_stage, {
+        start_event_identifier: :plan_stage_start,
+        end_event_identifier: :issue_first_mentioned_in_commit,
+        project: project
+      })
+    end
+
+    before(:all) do
+      issue1.metrics.update(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
+      issue2.metrics.update(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
+      issue3.metrics.update(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
+    end
+
+    before do
+      project.add_user(user, Gitlab::Access::DEVELOPER)
+
+      stub_const('Gitlab::Analytics::CycleAnalytics::RecordsFetcher::MAX_RECORDS', 2)
+    end
+
+    it 'limits the results' do
+      expect(subject.size).to eq(2)
+    end
+
+    it 'loads the record for the next page' do
+      params[:page] = 2
+
+      expect(subject.size).to eq(1)
+    end
+  end
 end

spec/lib/gitlab/pagination/offset_header_builder_with_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Pagination::OffsetHeaderBuilder, type: :controller do
+  controller(ActionController::Base) do
+    def index
+      relation = Project.where(archived: params[:archived]).page(params[:page]).per(1)
+
+      params_for_pagination = { archived: params[:archived], page: params[:page] }
+
+      Gitlab::Pagination::OffsetHeaderBuilder.new(
+        request_context: self,
+        per_page: relation.limit_value,
+        page: relation.current_page,
+        next_page: relation.next_page,
+        prev_page: relation.prev_page,
+        params: params_for_pagination
+      ).execute(exclude_total_headers: true, data_without_counts: true)
+
+      render json: relation.map(&:id)
+    end
+  end
+
+  let_it_be(:projects) { create_list(:project, 2, archived: true) }
+
+  describe 'pagination' do
+    it 'returns correct result for the first page' do
+      get :index, params: { page: 1, archived: true }
+
+      expect(json_response).to eq([projects.first.id])
+    end
+
+    it 'returns correct result for the second page' do
+      get :index, params: { page: 2, archived: true }
+
+      expect(json_response).to eq([projects.last.id])
+    end
+  end
+
+  describe 'pagination heders' do
+    it 'adds next page header' do
+      get :index, params: { page: 1, archived: true }
+
+      expect(response.headers['X-Next-Page']).to eq('2')
+    end
+
+    it 'adds only the specified params to the lnk' do
+      get :index, params: { page: 1, archived: true, some_param: '1' }
+
+      expect(response.headers['Link']).not_to include('some_param')
+    end
+  end
+end