Merge branch 'issue_27168_2' into 'master'

Preloads head pipeline for each merge request

Closes #27168

See merge request !10064

app/controllers/concerns/issuable_collections.rb

@@ -47,7 +47,7 @@ module IssuableCollections
   end
   def merge_requests_collection
-    merge_requests_finder.execute.preload(:source_project, :target_project, :author, :assignee, :labels, :milestone, :merge_request_diff, target_project: :namespace)
+    merge_requests_finder.execute.preload(:source_project, :target_project, :author, :assignee, :labels, :milestone, :merge_request_diff, :head_pipeline, target_project: :namespace)
   end
   def issues_finder

app/models/ci/pipeline.rb

@@ -18,6 +18,10 @@ module Ci
     has_many :builds, foreign_key: :commit_id
     has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id
+    # Merge requests for which the current pipeline is running against
+    # the merge request's latest commit.
+    has_many :merge_requests, foreign_key: "head_pipeline_id"
+
     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'
@@ -381,14 +385,6 @@ module Ci
       project.execute_services(data, :pipeline_hooks)
     end
-    # Merge requests for which the current pipeline is running against
-    # the merge request's latest commit.
-    def merge_requests
-      @merge_requests ||= project.merge_requests
-        .where(source_branch: self.ref)
-        .select { |merge_request| merge_request.head_pipeline.try(:id) == self.id }
-    end
-
     # All the merge requests for which the current pipeline runs/ran against
     def all_merge_requests
       @all_merge_requests ||= project.merge_requests.where(source_branch: ref)

app/models/merge_request.rb

@@ -13,6 +13,8 @@ class MergeRequest < ActiveRecord::Base
   has_one :merge_request_diff,
     -> { order('merge_request_diffs.id DESC') }
+  belongs_to :head_pipeline, foreign_key: "head_pipeline_id", class_name: "Ci::Pipeline"
+
   has_many :events, as: :target, dependent: :destroy
   has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues', dependent: :delete_all
@@ -829,12 +831,6 @@ class MergeRequest < ActiveRecord::Base
     diverged_commits_count > 0
   end
-  def head_pipeline
-    return unless diff_head_sha && source_project
-
-    @head_pipeline ||= source_project.pipeline_for(source_branch, diff_head_sha)
-  end
-
   def all_pipelines
     return Ci::Pipeline.none unless source_project

app/services/ci/create_pipeline_service.rb

@@ -47,7 +47,7 @@ module Ci
       end
       Ci::Pipeline.transaction do
-        pipeline.save
+        update_merge_requests_head_pipeline if pipeline.save
         Ci::CreatePipelineBuildsService
           .new(project, current_user)
@@ -118,6 +118,11 @@ module Ci
       origin_sha && origin_sha != Gitlab::Git::BLANK_SHA
     end
+    def update_merge_requests_head_pipeline
+      MergeRequest.where(source_branch: @pipeline.ref, source_project: @pipeline.project).
+        update_all(head_pipeline_id: @pipeline.id)
+    end
+
     def error(message, save: false)
       pipeline.errors.add(:base, message)
       pipeline.drop if save

changelogs/unreleased/issue_27168_2.yml

+---
+title: Preloads head pipeline for merge request collection
+merge_request:
+author:

db/migrate/20170507205316_add_head_pipeline_id_to_merge_requests.rb

+class AddHeadPipelineIdToMergeRequests < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def change
+    add_column :merge_requests, :head_pipeline_id, :integer
+  end
+end

db/post_migrate/20170508170547_add_head_pipeline_for_each_merge_request.rb

+class AddHeadPipelineForEachMergeRequest < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    disable_statement_timeout
+
+    pipelines = Arel::Table.new(:ci_pipelines)
+    merge_requests = Arel::Table.new(:merge_requests)
+
+    head_id = pipelines.
+      project(Arel::Nodes::NamedFunction.new('max', [pipelines[:id]])).
+      from(pipelines).
+      where(pipelines[:ref].eq(merge_requests[:source_branch])).
+      where(pipelines[:project_id].eq(merge_requests[:source_project_id]))
+
+    sub_query = Arel::Nodes::SqlLiteral.new(Arel::Nodes::Grouping.new(head_id).to_sql)
+
+    update_column_in_batches(:merge_requests, :head_pipeline_id, sub_query)
+  end
+
+  def down
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20170506185517) do
+ActiveRecord::Schema.define(version: 20170508170547) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -690,6 +690,7 @@ ActiveRecord::Schema.define(version: 20170506185517) do
     t.integer "cached_markdown_version"
     t.datetime "last_edited_at"
     t.integer "last_edited_by_id"
+    t.integer "head_pipeline_id"
   end
   add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree

features/steps/project/merge_requests.rb

@@ -557,6 +557,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
     project = merge_request.source_project
     project.enable_ci
     pipeline = create :ci_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch
+    merge_request.update(head_pipeline: pipeline)
     create :ci_build, pipeline: pipeline
   end

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -345,7 +345,8 @@ describe Projects::MergeRequestsController do
         end
         before do
-          create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch)
+          pipeline = create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch)
+          merge_request.update(head_pipeline: pipeline)
         end
         it 'returns :merge_when_pipeline_succeeds' do
@@ -1159,7 +1160,10 @@ describe Projects::MergeRequestsController do
       let(:status) { pipeline.detailed_status(double('user')) }
-      before { get_pipeline_status }
+      before do
+        merge_request.update(head_pipeline: pipeline)
+        get_pipeline_status
+      end
       it 'return a detailed head_pipeline status in json' do
         expect(response).to have_http_status(:ok)

spec/features/cycle_analytics_spec.rb

@@ -12,8 +12,10 @@ feature 'Cycle Analytics', feature: true, js: true do
   context 'as an allowed user' do
     context 'when project is new' do
       before  do
-        project.team << [user, :master]
+        project.add_master(user)
+
         login_as(user)
+
         visit namespace_project_cycle_analytics_path(project.namespace, project)
         wait_for_ajax
       end
@@ -30,7 +32,9 @@ feature 'Cycle Analytics', feature: true, js: true do
     context "when there's cycle analytics data" do
       before do
-        project.team << [user, :master]
+        allow_any_instance_of(Gitlab::ReferenceExtractor).to receive(:issues).and_return([issue])
+        mr.update(head_pipeline: pipeline)
+        project.add_master(user)
         create_cycle
         deploy_master
@@ -84,7 +88,7 @@ feature 'Cycle Analytics', feature: true, js: true do
   context "as a guest" do
     before do
-      project.team << [guest, :guest]
+      project.add_guest(guest)
       allow_any_instance_of(Gitlab::ReferenceExtractor).to receive(:issues).and_return([issue])
       create_cycle

spec/features/issuables/issuable_list_spec.rb

@@ -52,6 +52,9 @@ describe 'issuable list', feature: true do
           create(:issue, project: project, author: user)
         else
           create(:merge_request, source_project: project, source_branch: generate(:branch))
+          source_branch = FFaker::Name.name
+          pipeline = create(:ci_empty_pipeline, project: project, ref: source_branch, status: %w(running failed success).sample, sha: 'any')
+          create(:merge_request, title: FFaker::Lorem.sentence, source_project: project, source_branch: source_branch, head_pipeline: pipeline)
         end
       2.times do

spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb

@@ -4,16 +4,18 @@ feature 'Merge immediately', :feature, :js do
   let(:user) { create(:user) }
   let(:project) { create(:project, :public) }
-  let(:merge_request) do
+  let!(:merge_request) do
     create(:merge_request_with_diffs, source_project: project,
                                       author: user,
-                                      title: 'Bug NS-04')
+                                      title: 'Bug NS-04',
+                                      head_pipeline: pipeline,
+                                      source_branch: pipeline.ref)
   end
   let(:pipeline) do
     create(:ci_pipeline, project: project,
-                         sha: merge_request.diff_head_sha,
-                         ref: merge_request.source_branch)
+                         ref: 'master',
+                         sha: project.repository.commit('master').id)
   end
   before { project.team << [user, :master] }

spec/features/merge_requests/merge_when_pipeline_succeeds_spec.rb

@@ -16,7 +16,10 @@ feature 'Merge When Pipeline Succeeds', :feature, :js do
                          ref: merge_request.source_branch)
   end
-  before { project.team << [user, :master] }
+  before do
+    project.add_master(user)
+    merge_request.update(head_pipeline_id: pipeline.id)
+  end
   context 'when there is active pipeline for merge request' do
     background do

spec/features/merge_requests/mini_pipeline_graph_spec.rb

@@ -3,7 +3,7 @@ require 'rails_helper'
 feature 'Mini Pipeline Graph', :js, :feature do
   let(:user) { create(:user) }
   let(:project) { create(:project, :public) }
-  let(:merge_request) { create(:merge_request, source_project: project) }
+  let(:merge_request) { create(:merge_request, source_project: project, head_pipeline: pipeline) }
   let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: 'master', status: 'running', sha: project.commit.id) }
   let(:build) { create(:ci_build, pipeline: pipeline, stage: 'test', commands: 'test') }

spec/features/merge_requests/only_allow_merge_if_build_succeeds_spec.rb

@@ -31,6 +31,8 @@ feature 'Only allow merge requests to be merged if the pipeline succeeds', featu
       status: status)
     end
+    before { merge_request.update(head_pipeline: pipeline) }
+
     context 'when merge requests can only be merged if the pipeline succeeds' do
       before do
         project.update_attribute(:only_allow_merge_if_pipeline_succeeds, true)

spec/features/merge_requests/widget_spec.rb

@@ -91,6 +91,8 @@ describe 'Merge request', :feature, :js do
                                       statuses: [commit_status])
       create(:ci_build, :pending, pipeline: pipeline)
+      merge_request.update(head_pipeline: pipeline)
+
       visit namespace_project_merge_request_path(project.namespace, project, merge_request)
     end
@@ -103,10 +105,15 @@ describe 'Merge request', :feature, :js do
   context 'when merge request is in the blocked pipeline state' do
     before do
-      create(:ci_pipeline, project: project,
-                           sha: merge_request.diff_head_sha,
-                           ref: merge_request.source_branch,
-                           status: :manual)
+      pipeline = create(
+        :ci_pipeline,
+        project: project,
+        sha: merge_request.diff_head_sha,
+        ref: merge_request.source_branch,
+        status: :manual
+      )
+
+      merge_request.update(head_pipeline: pipeline)
       visit namespace_project_merge_request_path(project.namespace,
                                                  project,
@@ -131,6 +138,8 @@ describe 'Merge request', :feature, :js do
                                       statuses: [commit_status])
       create(:ci_build, :pending, pipeline: pipeline)
+      merge_request.update(head_pipeline: pipeline)
+
       visit namespace_project_merge_request_path(project.namespace, project, merge_request)
     end

spec/lib/gitlab/cycle_analytics/events_spec.rb

@@ -130,6 +130,8 @@ describe 'cycle analytics events' do
     end
     before do
+      merge_request.update(head_pipeline: pipeline)
+
       create(:ci_build, pipeline: pipeline, status: :success, author: user)
       create(:ci_build, pipeline: pipeline, status: :success, author: user)
@@ -226,6 +228,8 @@ describe 'cycle analytics events' do
     end
     before do
+      merge_request.update(head_pipeline: pipeline)
+
       create(:ci_build, pipeline: pipeline, status: :success, author: user)
       create(:ci_build, pipeline: pipeline, status: :success, author: user)

spec/lib/gitlab/import_export/all_models.yml

@@ -85,6 +85,7 @@ merge_requests:
 - merge_requests_closing_issues
 - metrics
 - timelogs
+- head_pipeline
 merge_request_diff:
 - merge_request
 pipelines:
@@ -102,6 +103,7 @@ pipelines:
 - manual_actions
 - artifacts
 - pipeline_schedule
+- merge_requests
 statuses:
 - project
 - pipeline

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -158,6 +158,7 @@ MergeRequest:
 - time_estimate
 - last_edited_at
 - last_edited_by_id
+- head_pipeline_id
 MergeRequestDiff:
 - id
 - state