Small code improvements and add migration specissue_27168_2

8 files changed, 67 insertions, 18 deletions

diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index 2b81ab2d28a..9cba20b7dc7 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -118,9 +118,8 @@ module Ci
     end
 
     def update_merge_requests_head_pipeline
-      merge_requests = MergeRequest.where(source_branch: @pipeline.ref, source_project: @pipeline.project)
-
-      merge_requests.update_all(head_pipeline_id: @pipeline.id) if merge_requests.any?
+      MergeRequest.where(source_branch: @pipeline.ref, source_project: @pipeline.project).
+        update_all(head_pipeline_id: @pipeline.id)
     end
 
     def error(message, save: false)
diff --git a/db/migrate/20170427205316_add_head_pipeline_id_to_merge_requests.rb b/db/migrate/20170504205316_add_head_pipeline_id_to_merge_requests.rb
index 8fc6e380a77..8fc6e380a77 100644
--- a/db/migrate/20170427205316_add_head_pipeline_id_to_merge_requests.rb
+++ b/db/migrate/20170504205316_add_head_pipeline_id_to_merge_requests.rb
diff --git a/db/post_migrate/20170428170547_add_head_pipeline_for_each_merge_request.rb b/db/post_migrate/20170505170547_add_head_pipeline_for_each_merge_request.rb
index bc3850c0c23..bc3850c0c23 100644
--- a/db/post_migrate/20170428170547_add_head_pipeline_for_each_merge_request.rb
+++ b/db/post_migrate/20170505170547_add_head_pipeline_for_each_merge_request.rb
diff --git a/db/schema.rb b/db/schema.rb
index 8da50ffaacb..b12257dcd8c 100644
--- a/db/schema.rb
+++ b/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: 20170504102911) do
+ActiveRecord::Schema.define(version: 20170505170547) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/spec/features/cycle_analytics_spec.rb b/spec/features/cycle_analytics_spec.rb
index 47a73011673..328b61a9ef9 100644
--- a/spec/features/cycle_analytics_spec.rb
+++ b/spec/features/cycle_analytics_spec.rb
@@ -9,10 +9,6 @@ feature 'Cycle Analytics', feature: true, js: true do
   let(:mr) { create_merge_request_closing_issue(issue) }
   let(:pipeline) { create(:ci_empty_pipeline, status: 'created', project: project, ref: mr.source_branch, sha: mr.source_branch_sha) }
 
-  before do
-    allow_any_instance_of(Gitlab::ReferenceExtractor).to receive(:issues).and_return([issue])
-  end
-
   context 'as an allowed user' do
     context 'when project is new' do
       before  do
@@ -36,8 +32,10 @@ feature 'Cycle Analytics', feature: true, js: true do
 
     context "when there's cycle analytics data" do
       before do
+        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
 
diff --git a/spec/migrations/add_head_pipeline_for_each_merge_request_spec.rb b/spec/migrations/add_head_pipeline_for_each_merge_request_spec.rb
new file mode 100644
index 00000000000..7f1cf956769
--- /dev/null
+++ b/spec/migrations/add_head_pipeline_for_each_merge_request_spec.rb
@@ -0,0 +1,29 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170505170547_add_head_pipeline_for_each_merge_request.rb')
+
+describe AddHeadPipelineForEachMergeRequest do
+  let(:migration) { described_class.new }
+
+  let!(:project) { create(:empty_project) }
+  let!(:forked_project_link) { create(:forked_project_link, forked_from_project: project) }
+  let!(:other_project) { forked_project_link.forked_to_project }
+
+  let!(:pipeline_1) { create(:ci_pipeline, project: project, ref: "branch_1") }
+  let!(:pipeline_2) { create(:ci_pipeline, project: other_project, ref: "branch_1") }
+  let!(:pipeline_3) { create(:ci_pipeline, project: other_project, ref: "branch_1") }
+  let!(:pipeline_4) { create(:ci_pipeline, project: project, ref: "branch_2") }
+
+  let!(:mr_1) { create(:merge_request, source_project: project, target_project: project, source_branch: "branch_1", target_branch: "target_1") }
+  let!(:mr_2) { create(:merge_request, source_project: other_project, target_project: project, source_branch: "branch_1", target_branch: "target_2") }
+  let!(:mr_3) { create(:merge_request, source_project: project, target_project: project, source_branch: "branch_2", target_branch: "master") }
+
+  context "#up" do
+    it "correctly sets head_pipeline_id for each merge request" do
+      migration.up
+
+      expect(mr_1.reload.head_pipeline_id).to eq(pipeline_1.id)
+      expect(mr_2.reload.head_pipeline_id).to eq(pipeline_3.id)
+      expect(mr_3.reload.head_pipeline_id).to eq(pipeline_4.id)
+    end
+  end
+end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index c95b1c59bcb..235530f0ebe 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -689,8 +689,7 @@ describe MergeRequest, models: true do
   describe '#head_pipeline' do
     describe 'when the source project exists' do
       it 'returns the latest pipeline' do
-        sha = "123abc"
-        pipeline = create(:ci_empty_pipeline, project: subject.source_project, ref: 'master', status: 'running', sha: sha)
+        pipeline = create(:ci_empty_pipeline, project: subject.source_project, ref: 'master', status: 'running', sha: "123abc")
         subject.update(head_pipeline: pipeline)
 
         expect(subject.head_pipeline).to eq(pipeline)
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index f2ed66b5e8e..1ff1438ba06 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -34,16 +34,40 @@ describe Ci::CreatePipelineService, services: true do
       it { expect(pipeline).to have_attributes(status: 'pending') }
       it { expect(pipeline.builds.first).to be_kind_of(Ci::Build) }
 
-      it 'updates head pipeline of each merge request' do
-        merge_request_1 = create(:merge_request, source_branch: 'master', target_branch: "branch_1", source_project: project)
-        merge_request_2 = create(:merge_request, source_branch: 'master', target_branch: "branch_2", source_project: project)
-        merge_request_3 = create(:merge_request, source_branch: 'other_branch', target_branch: "branch_2", source_project: project)
+      context '#update_merge_requests_head_pipeline' do
+        it 'updates head pipeline of each merge request' do
+          merge_request_1 = create(:merge_request, source_branch: 'master', target_branch: "branch_1", source_project: project)
+          merge_request_2 = create(:merge_request, source_branch: 'master', target_branch: "branch_2", source_project: project)
 
-        head_pipeline = pipeline
+          head_pipeline = pipeline
 
-        expect(merge_request_1.reload.head_pipeline).to eq(head_pipeline)
-        expect(merge_request_2.reload.head_pipeline).to eq(head_pipeline)
-        expect(merge_request_3.reload.head_pipeline).to be_nil
+          expect(merge_request_1.reload.head_pipeline).to eq(head_pipeline)
+          expect(merge_request_2.reload.head_pipeline).to eq(head_pipeline)
+        end
+
+        context 'when there is no pipeline for source branch' do
+          it "does not update merge request head pipeline" do
+            merge_request = create(:merge_request, source_branch: 'other_branch', target_branch: "branch_1", source_project: project)
+
+            head_pipeline = pipeline
+
+            expect(merge_request.reload.head_pipeline).not_to eq(head_pipeline)
+          end
+        end
+
+        context 'when merge request target project is different from source project' do
+          let!(:target_project) { create(:empty_project) }
+          let!(:forked_project_link) { create(:forked_project_link, forked_to_project: project, forked_from_project: target_project) }
+
+          it 'updates head pipeline for merge request' do
+            merge_request =
+              create(:merge_request, source_branch: 'master', target_branch: "branch_1", source_project: project, target_project: target_project)
+
+            head_pipeline = pipeline
+
+            expect(merge_request.reload.head_pipeline).to eq(head_pipeline)
+          end
+        end
       end
 
       context 'auto-cancel enabled' do