Merge branch 'issue_27166_2' into 'master'

Avoid repeated queries for pipeline builds on merge requests

See merge request !11888

app/models/ci/pipeline.rb

@@ -23,8 +23,8 @@ module Ci
     has_many :pending_builds, -> { pending }, foreign_key: :commit_id, class_name: 'Ci::Build'
     has_many :retryable_builds, -> { latest.failed_or_canceled }, foreign_key: :commit_id, class_name: 'Ci::Build'
     has_many :cancelable_statuses, -> { cancelable }, foreign_key: :commit_id, class_name: 'CommitStatus'
-    has_many :manual_actions, -> { latest.manual_actions }, foreign_key: :commit_id, class_name: 'Ci::Build'
-    has_many :artifacts, -> { latest.with_artifacts_not_expired }, foreign_key: :commit_id, class_name: 'Ci::Build'
+    has_many :manual_actions, -> { latest.manual_actions.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build'
+    has_many :artifacts, -> { latest.with_artifacts_not_expired.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build'
 
     has_many :auto_canceled_pipelines, class_name: 'Ci::Pipeline', foreign_key: 'auto_canceled_by_id'
     has_many :auto_canceled_jobs, class_name: 'CommitStatus', foreign_key: 'auto_canceled_by_id'

app/serializers/pipeline_serializer.rb

@@ -13,14 +13,15 @@ class PipelineSerializer < BaseSerializer
 
   def represent(resource, opts = {})
     if resource.is_a?(ActiveRecord::Relation)
+
       resource = resource.preload([
         :retryable_builds,
         :cancelable_statuses,
         :trigger_requests,
         :project,
-        { pending_builds: :project },
-        { manual_actions: :project },
-        { artifacts: :project }
+        :manual_actions,
+        :artifacts,
+        { pending_builds: :project }
       ])
     end
 

changelogs/unreleased/issue_27166_2.yml

+---
+title: Avoid repeated queries for pipeline builds on merge requests
+merge_request:
+author:

spec/features/merge_requests/mini_pipeline_graph_spec.rb

@@ -12,13 +12,39 @@ feature 'Mini Pipeline Graph', :js, :feature do
     build.run
 
     login_as(user)
-    visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+    visit_merge_request
+  end
+
+  def visit_merge_request(format = :html)
+    visit namespace_project_merge_request_path(project.namespace, project, merge_request, format: format)
   end
 
   it 'should display a mini pipeline graph' do
     expect(page).to have_selector('.mr-widget-pipeline-graph')
   end
 
+  context 'as json' do
+    let(:artifacts_file1) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') }
+    let(:artifacts_file2) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/png') }
+
+    before do
+      create(:ci_build, pipeline: pipeline, artifacts_file: artifacts_file1)
+      create(:ci_build, pipeline: pipeline, when: 'manual')
+    end
+
+    it 'avoids repeated database queries' do
+      before = ActiveRecord::QueryRecorder.new { visit_merge_request(:json) }
+
+      create(:ci_build, pipeline: pipeline, artifacts_file: artifacts_file2)
+      create(:ci_build, pipeline: pipeline, when: 'manual')
+
+      after = ActiveRecord::QueryRecorder.new { visit_merge_request(:json) }
+
+      expect(before.count).to eq(after.count)
+      expect(before.cached_count).to eq(after.cached_count)
+    end
+  end
+
   describe 'build list toggle' do
     let(:toggle) do
       find('.mini-pipeline-graph-dropdown-toggle')