diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index a2225cc83433f9b6700a3c96862ad6b7742cbe6f..f47df8b623bdcc3835774c7de818f287099f89c2 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -325,16 +325,16 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_request.update(merge_error: nil) if params[:merge_when_build_succeeds].present? - unless @merge_request.pipeline + unless @merge_request.head_pipeline @status = :failed return end - if @merge_request.pipeline.active? + if @merge_request.head_pipeline.active? MergeRequests::MergeWhenBuildSucceedsService.new(@project, current_user, merge_params) .execute(@merge_request) @status = :merge_when_build_succeeds - elsif @merge_request.pipeline.success? + elsif @merge_request.head_pipeline.success? # This can be triggered when a user clicks the auto merge button while # the tests finish at about the same time MergeWorker.perform_async(@merge_request.id, current_user.id, params) @@ -398,7 +398,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def ci_status - pipeline = @merge_request.pipeline + pipeline = @merge_request.head_pipeline + if pipeline status = pipeline.status coverage = pipeline.try(:coverage) @@ -534,7 +535,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def define_widget_vars - @pipeline = @merge_request.pipeline + @pipeline = @merge_request.head_pipeline end def define_commit_vars @@ -563,7 +564,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController def define_pipelines_vars @pipelines = @merge_request.all_pipelines - @pipeline = @merge_request.pipeline + @pipeline = @merge_request.head_pipeline @statuses_count = @pipeline.present? ? @pipeline.statuses.relevant.count : 0 end @@ -631,7 +632,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController def merge_when_build_succeeds_active? params[:merge_when_build_succeeds].present? && - @merge_request.pipeline && @merge_request.pipeline.active? + @merge_request.head_pipeline && @merge_request.head_pipeline.active? end def build_merge_request diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 4294a10e9e3f8cbd0e5da1570043c89ac9997ee4..fabbf97d4db3e9b5df683e69976756a294302924 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -317,7 +317,7 @@ module Ci def merge_requests @merge_requests ||= project.merge_requests .where(source_branch: self.ref) - .select { |merge_request| merge_request.pipeline.try(:id) == self.id } + .select { |merge_request| merge_request.head_pipeline.try(:id) == self.id } end private diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 38d8c15e6b05cdb44eecc35c9a4fe911fe79a8bd..64990f8134e8b38068cf9c7f235c44b33bf23dae 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -678,7 +678,7 @@ class MergeRequest < ActiveRecord::Base def mergeable_ci_state? return true unless project.only_allow_merge_if_build_succeeds? - !pipeline || pipeline.success? || pipeline.skipped? + !head_pipeline || head_pipeline.success? || head_pipeline.skipped? end def environments @@ -774,10 +774,10 @@ class MergeRequest < ActiveRecord::Base commits.map(&:sha) end - def pipeline + def head_pipeline return unless diff_head_sha && source_project - @pipeline ||= source_project.pipeline_for(source_branch, diff_head_sha) + @head_pipeline ||= source_project.pipeline_for(source_branch, diff_head_sha) end def all_pipelines diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 58f69a41e14bb4d5281dd8a6a7085e1e84342caa..800fd39c42490a9ca025d68e6a0e9e4c616cde84 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -55,7 +55,7 @@ module MergeRequests def pipeline_merge_requests(pipeline) merge_requests_for(pipeline.ref).each do |merge_request| - next unless pipeline == merge_request.pipeline + next unless pipeline == merge_request.head_pipeline yield merge_request end @@ -63,7 +63,7 @@ module MergeRequests def commit_status_merge_requests(commit_status) merge_requests_for(commit_status.ref).each do |merge_request| - pipeline = merge_request.pipeline + pipeline = merge_request.head_pipeline next unless pipeline next unless pipeline.sha == commit_status.sha diff --git a/app/views/projects/issues/_merge_requests.html.haml b/app/views/projects/issues/_merge_requests.html.haml index 747bfa554cba540f684ff6885dc569678ee90eff..d48923b422a3e12b0f7f04e461c75c9efa5bf20c 100644 --- a/app/views/projects/issues/_merge_requests.html.haml +++ b/app/views/projects/issues/_merge_requests.html.haml @@ -2,12 +2,12 @@ %h2.merge-requests-title = pluralize(@merge_requests.count, 'Related Merge Request') %ul.unstyled-list.related-merge-requests - - has_any_ci = @merge_requests.any?(&:pipeline) + - has_any_ci = @merge_requests.any?(&:head_pipeline) - @merge_requests.each do |merge_request| %li %span.merge-request-ci-status - - if merge_request.pipeline - = render_pipeline_status(merge_request.pipeline) + - if merge_request.head_pipeline + = render_pipeline_status(merge_request.head_pipeline) - elsif has_any_ci = icon('blank fw') %span.merge-request-id diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index 9ffcc48eb803a82283016ed0575ff2acfb7ad56e..fa189ae62d824073952a3d88b704994e99ac14ff 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -15,9 +15,9 @@ = icon('ban') CLOSED - - if merge_request.pipeline + - if merge_request.head_pipeline %li - = render_pipeline_status(merge_request.pipeline) + = render_pipeline_status(merge_request.head_pipeline) - if merge_request.open? && merge_request.broken? %li diff --git a/app/views/projects/merge_requests/widget/_show.html.haml b/app/views/projects/merge_requests/widget/_show.html.haml index 608fdf1c5f5eb3d60c1a12291e6ca7f96e03b49e..a8918c85dde36f4957a59fbc1b11a857a29cd664 100644 --- a/app/views/projects/merge_requests/widget/_show.html.haml +++ b/app/views/projects/merge_requests/widget/_show.html.haml @@ -14,7 +14,7 @@ ci_status_url: "#{ci_status_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}", ci_environments_status_url: "#{ci_environments_status_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}", gitlab_icon: "#{asset_path 'gitlab_logo.png'}", - ci_status: "#{@merge_request.pipeline ? @merge_request.pipeline.status : ''}", + ci_status: "#{@merge_request.head_pipeline ? @merge_request.head_pipeline.status : ''}", ci_message: { normal: "Build {{status}} for \"{{title}}\"", preparing: "{{status}} build for \"{{title}}\"" diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 90fa588b455df83979ac70f1ef2d2c5b66123f9f..97baebc1d27ba2ed1f60c3b2af9f86883746d1d6 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -192,7 +192,7 @@ module API should_remove_source_branch: params[:should_remove_source_branch] } - if params[:merge_when_build_succeeds] && merge_request.pipeline && merge_request.pipeline.active? + if params[:merge_when_build_succeeds] && merge_request.head_pipeline && merge_request.head_pipeline.active? ::MergeRequests::MergeWhenBuildSucceedsService.new(merge_request.target_project, current_user, merge_params). execute(merge_request) else diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 58ccd056328319bef1aa53f6c61670ef6e4cdf11..26034cb1c7b858690dce84a84823eef3928283a4 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -570,7 +570,7 @@ describe MergeRequest, models: true do end end - describe '#pipeline' do + describe '#head_pipeline' do describe 'when the source project exists' do it 'returns the latest pipeline' do pipeline = double(:ci_pipeline, ref: 'master') @@ -581,7 +581,7 @@ describe MergeRequest, models: true do with('master', '123abc'). and_return(pipeline) - expect(subject.pipeline).to eq(pipeline) + expect(subject.head_pipeline).to eq(pipeline) end end @@ -589,7 +589,7 @@ describe MergeRequest, models: true do it 'returns nil' do allow(subject).to receive(:source_project).and_return(nil) - expect(subject.pipeline).to be_nil + expect(subject.head_pipeline).to be_nil end end end @@ -857,7 +857,7 @@ describe MergeRequest, models: true do context 'and a failed pipeline is associated' do before do pipeline.update(status: 'failed') - allow(subject).to receive(:pipeline) { pipeline } + allow(subject).to receive(:head_pipeline) { pipeline } end it { expect(subject.mergeable_ci_state?).to be_falsey } @@ -866,7 +866,7 @@ describe MergeRequest, models: true do context 'and a successful pipeline is associated' do before do pipeline.update(status: 'success') - allow(subject).to receive(:pipeline) { pipeline } + allow(subject).to receive(:head_pipeline) { pipeline } end it { expect(subject.mergeable_ci_state?).to be_truthy } @@ -875,7 +875,7 @@ describe MergeRequest, models: true do context 'and a skipped pipeline is associated' do before do pipeline.update(status: 'skipped') - allow(subject).to receive(:pipeline) { pipeline } + allow(subject).to receive(:head_pipeline) { pipeline } end it { expect(subject.mergeable_ci_state?).to be_truthy } @@ -883,7 +883,7 @@ describe MergeRequest, models: true do context 'when no pipeline is associated' do before do - allow(subject).to receive(:pipeline) { nil } + allow(subject).to receive(:head_pipeline) { nil } end it { expect(subject.mergeable_ci_state?).to be_truthy } @@ -896,7 +896,7 @@ describe MergeRequest, models: true do context 'and a failed pipeline is associated' do before do pipeline.statuses << create(:commit_status, status: 'failed', project: project) - allow(subject).to receive(:pipeline) { pipeline } + allow(subject).to receive(:head_pipeline) { pipeline } end it { expect(subject.mergeable_ci_state?).to be_truthy } @@ -904,7 +904,7 @@ describe MergeRequest, models: true do context 'when no pipeline is associated' do before do - allow(subject).to receive(:pipeline) { nil } + allow(subject).to receive(:head_pipeline) { nil } end it { expect(subject.mergeable_ci_state?).to be_truthy } diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 3ecf3eea5f561c55a3d0605f0ea6008a4e730a42..edc985b765bae0eee1fb1974b1c91db81818d9df 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -466,7 +466,7 @@ describe API::API, api: true do end it "enables merge when build succeeds if the ci is active" do - allow_any_instance_of(MergeRequest).to receive(:pipeline).and_return(pipeline) + allow_any_instance_of(MergeRequest).to receive(:head_pipeline).and_return(pipeline) allow(pipeline).to receive(:active?).and_return(true) put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), merge_when_build_succeeds: true diff --git a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb index a44312dd36390e4b2920e9a4a074a3ab5aa398b3..bb7830c7eea782e79f356a08b6a9e1ab4b1a3135 100644 --- a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb +++ b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb @@ -20,13 +20,19 @@ describe MergeRequests::AddTodoWhenBuildFailsService do let(:todo_service) { TodoService.new } let(:merge_request) do - create(:merge_request, merge_user: user, source_branch: 'master', - target_branch: 'feature', source_project: project, target_project: project, + create(:merge_request, merge_user: user, + source_branch: 'master', + target_branch: 'feature', + source_project: project, + target_project: project, state: 'opened') end before do - allow_any_instance_of(MergeRequest).to receive(:pipeline).and_return(pipeline) + allow_any_instance_of(MergeRequest) + .to receive(:head_pipeline) + .and_return(pipeline) + allow(service).to receive(:todo_service).and_return(todo_service) end diff --git a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb index c0164138713560facb2f5d8a0cb2bd878efb1f81..963d9573ac410f72fa13e882765cbad9703863f1 100644 --- a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb +++ b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb @@ -21,7 +21,10 @@ describe MergeRequests::MergeWhenBuildSucceedsService do context 'first time enabling' do before do - allow(merge_request).to receive(:pipeline).and_return(pipeline) + allow(merge_request) + .to receive(:head_pipeline) + .and_return(pipeline) + service.execute(merge_request) end @@ -43,8 +46,12 @@ describe MergeRequests::MergeWhenBuildSucceedsService do let(:build) { create(:ci_build, ref: mr_merge_if_green_enabled.source_branch) } before do - allow(mr_merge_if_green_enabled).to receive(:pipeline).and_return(pipeline) - allow(mr_merge_if_green_enabled).to receive(:mergeable?).and_return(true) + allow(mr_merge_if_green_enabled).to receive(:head_pipeline) + .and_return(pipeline) + + allow(mr_merge_if_green_enabled).to receive(:mergeable?) + .and_return(true) + allow(pipeline).to receive(:success?).and_return(true) end @@ -138,9 +145,12 @@ describe MergeRequests::MergeWhenBuildSucceedsService do before do # This behavior of MergeRequest: we instantiate a new object - allow_any_instance_of(MergeRequest).to receive(:pipeline).and_wrap_original do - Ci::Pipeline.find(pipeline.id) - end + # + allow_any_instance_of(MergeRequest) + .to receive(:head_pipeline) + .and_wrap_original do + Ci::Pipeline.find(pipeline.id) + end end it "doesn't merge if any of stages failed" do