From 5b9e801e819b6daf1804874ed962bc2f1650c8da Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Tue, 23 May 2017 18:36:57 -0300 Subject: [PATCH] Sanity check pipeline sha before saving merge request head pipeline --- app/services/ci/create_pipeline_service.rb | 10 ++++++++-- changelogs/unreleased/issue_32225_2.yml | 4 ++++ spec/services/ci/create_pipeline_service_spec.rb | 13 ++++++++++++- 3 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/issue_32225_2.yml diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 1f6c1f4a7f6..801ff410fb5 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -119,8 +119,14 @@ module Ci end def update_merge_requests_head_pipeline - MergeRequest.where(source_branch: @pipeline.ref, source_project: @pipeline.project). - update_all(head_pipeline_id: @pipeline.id) + merge_requests = MergeRequest.where(source_branch: @pipeline.ref, source_project: @pipeline.project) + + merge_requests_ids = + merge_requests.select do |mr| + mr.diff_head_sha == @pipeline.sha + end.map(&:id) + + MergeRequest.where(id: merge_requests_ids).update_all(head_pipeline_id: @pipeline.id) end def error(message, save: false) diff --git a/changelogs/unreleased/issue_32225_2.yml b/changelogs/unreleased/issue_32225_2.yml new file mode 100644 index 00000000000..80d2bdcd63c --- /dev/null +++ b/changelogs/unreleased/issue_32225_2.yml @@ -0,0 +1,4 @@ +--- +title: Sanity check for pipeline sha before saving merge request pipeline id +merge_request: +author: diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index b536103ed65..c31b3d63676 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -58,7 +58,7 @@ describe Ci::CreatePipelineService, services: true do end context 'when merge request target project is different from source project' do - let!(:target_project) { create(:empty_project) } + let!(:target_project) { create(: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 @@ -70,6 +70,17 @@ describe Ci::CreatePipelineService, services: true do expect(merge_request.reload.head_pipeline).to eq(head_pipeline) end end + + context 'when merge request head commit sha does not match pipeline sha' do + it 'does not update merge request head pipeline' do + merge_request = create(:merge_request, source_branch: 'master', target_branch: "branch_1", source_project: project) + allow_any_instance_of(MergeRequestDiff).to receive(:head_commit).and_return(double(id: 1234)) + + pipeline + + expect(merge_request.reload.head_pipeline).to be_nil + end + end end context 'auto-cancel enabled' do