diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index f919cfe697e..29f36570912 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -493,7 +493,7 @@ class MergeRequest < ActiveRecord::Base def mergeable_ci_state? return true unless project.only_allow_merge_if_build_succeeds? - !ci_commit || ci_commit.success? + !pipeline || pipeline.success? end def state_human_name diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml index 84fbf618ec6..0e0af57d76e 100644 --- a/app/views/projects/merge_requests/widget/_open.html.haml +++ b/app/views/projects/merge_requests/widget/_open.html.haml @@ -17,7 +17,7 @@ = render 'projects/merge_requests/widget/open/merge_when_build_succeeds' - elsif !@merge_request.can_be_merged_by?(current_user) = render 'projects/merge_requests/widget/open/not_allowed' - - elsif !@merge_request.mergeable_ci_state? && @ci_commit && @ci_commit.failed? + - elsif !@merge_request.mergeable_ci_state? && @pipeline && @pipeline.failed? = render 'projects/merge_requests/widget/open/build_failed' - elsif @merge_request.can_be_merged? = render 'projects/merge_requests/widget/open/accept' diff --git a/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb b/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb index 52612c91824..65e9185ec24 100644 --- a/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb +++ b/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb @@ -19,7 +19,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature: end context 'when project has CI enabled' do - let(:ci_commit) { create(:ci_empty_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) } + let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) } context 'when merge requests can only be merged if the build succeeds' do before do @@ -27,7 +27,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature: end context 'when CI is running' do - before { ci_commit.update_column(:status, :running) } + before { pipeline.update_column(:status, :running) } it 'does not allow to merge immediately' do visit_merge_request(merge_request) @@ -38,7 +38,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature: end context 'when CI failed' do - before { ci_commit.update_column(:status, :failed) } + before { pipeline.update_column(:status, :failed) } it 'does not allow MR to be merged' do visit_merge_request(merge_request) @@ -49,7 +49,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature: end context 'when CI succeeded' do - before { ci_commit.update_column(:status, :success) } + before { pipeline.update_column(:status, :success) } it 'allows MR to be merged' do visit_merge_request(merge_request) @@ -65,7 +65,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature: end context 'when CI is running' do - before { ci_commit.update_column(:status, :running) } + before { pipeline.update_column(:status, :running) } it 'allows MR to be merged immediately', js: true do visit_merge_request(merge_request) @@ -78,7 +78,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature: end context 'when CI failed' do - before { ci_commit.update_column(:status, :failed) } + before { pipeline.update_column(:status, :failed) } it 'allows MR to be merged' do visit_merge_request(merge_request) @@ -88,7 +88,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature: end context 'when CI succeeded' do - before { ci_commit.update_column(:status, :success) } + before { pipeline.update_column(:status, :success) } it 'allows MR to be merged' do visit_merge_request(merge_request) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c543cbcfabd..3b199f4d98d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -564,23 +564,23 @@ describe MergeRequest, models: true do describe '#mergeable_ci_state?' do let(:project) { create(:empty_project, only_allow_merge_if_build_succeeds: true) } - let(:ci_commit) { create(:ci_empty_pipeline) } + let(:pipeline) { create(:ci_empty_pipeline) } subject { build(:merge_request, target_project: project) } context 'when it is only allowed to merge when build is green' do - context 'and a failed ci_commit is associated' do + context 'and a failed pipeline is associated' do before do - ci_commit.statuses << create(:commit_status, status: 'failed', project: project) - allow(subject).to receive(:ci_commit) { ci_commit } + pipeline.statuses << create(:commit_status, status: 'failed', project: project) + allow(subject).to receive(:pipeline) { pipeline } end it { expect(subject.mergeable_ci_state?).to be_falsey } end - context 'when no ci_commit is associated' do + context 'when no pipeline is associated' do before do - allow(subject).to receive(:ci_commit) { nil } + allow(subject).to receive(:pipeline) { nil } end it { expect(subject.mergeable_ci_state?).to be_truthy } @@ -590,18 +590,18 @@ describe MergeRequest, models: true do context 'when merges are not restricted to green builds' do subject { build(:merge_request, target_project: build(:empty_project, only_allow_merge_if_build_succeeds: false)) } - context 'and a failed ci_commit is associated' do + context 'and a failed pipeline is associated' do before do - ci_commit.statuses << create(:commit_status, status: 'failed', project: project) - allow(subject).to receive(:ci_commit) { ci_commit } + pipeline.statuses << create(:commit_status, status: 'failed', project: project) + allow(subject).to receive(:pipeline) { pipeline } end it { expect(subject.mergeable_ci_state?).to be_truthy } end - context 'when no ci_commit is associated' do + context 'when no pipeline is associated' do before do - allow(subject).to receive(:ci_commit) { nil } + allow(subject).to receive(:pipeline) { nil } end it { expect(subject.mergeable_ci_state?).to be_truthy }