Commit 21b90ef8 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'fix/rename-merge-request-head-pipeline' into 'master'

Rename `MergeRequest#pipeline` to `head_pipeline`

## What does this MR do?

This MR renames `MergeRequest#pipeline` to `MergeRequest#head_pipeline`

## Does this MR meet the acceptance criteria?

- [x] All builds are passing

## What are the relevant issue numbers?

Closes #24810

See merge request !7783
parents 50a410a9 a49e9949
...@@ -325,16 +325,16 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -325,16 +325,16 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@merge_request.update(merge_error: nil) @merge_request.update(merge_error: nil)
if params[:merge_when_build_succeeds].present? if params[:merge_when_build_succeeds].present?
unless @merge_request.pipeline unless @merge_request.head_pipeline
@status = :failed @status = :failed
return return
end end
if @merge_request.pipeline.active? if @merge_request.head_pipeline.active?
MergeRequests::MergeWhenBuildSucceedsService.new(@project, current_user, merge_params) MergeRequests::MergeWhenBuildSucceedsService.new(@project, current_user, merge_params)
.execute(@merge_request) .execute(@merge_request)
@status = :merge_when_build_succeeds @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 # This can be triggered when a user clicks the auto merge button while
# the tests finish at about the same time # the tests finish at about the same time
MergeWorker.perform_async(@merge_request.id, current_user.id, params) MergeWorker.perform_async(@merge_request.id, current_user.id, params)
...@@ -398,7 +398,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -398,7 +398,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end end
def ci_status def ci_status
pipeline = @merge_request.pipeline pipeline = @merge_request.head_pipeline
if pipeline if pipeline
status = pipeline.status status = pipeline.status
coverage = pipeline.try(:coverage) coverage = pipeline.try(:coverage)
...@@ -534,7 +535,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -534,7 +535,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end end
def define_widget_vars def define_widget_vars
@pipeline = @merge_request.pipeline @pipeline = @merge_request.head_pipeline
end end
def define_commit_vars def define_commit_vars
...@@ -563,7 +564,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -563,7 +564,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
def define_pipelines_vars def define_pipelines_vars
@pipelines = @merge_request.all_pipelines @pipelines = @merge_request.all_pipelines
@pipeline = @merge_request.pipeline @pipeline = @merge_request.head_pipeline
@statuses_count = @pipeline.present? ? @pipeline.statuses.relevant.count : 0 @statuses_count = @pipeline.present? ? @pipeline.statuses.relevant.count : 0
end end
...@@ -631,7 +632,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -631,7 +632,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
def merge_when_build_succeeds_active? def merge_when_build_succeeds_active?
params[:merge_when_build_succeeds].present? && params[:merge_when_build_succeeds].present? &&
@merge_request.pipeline && @merge_request.pipeline.active? @merge_request.head_pipeline && @merge_request.head_pipeline.active?
end end
def build_merge_request def build_merge_request
......
...@@ -317,7 +317,7 @@ module Ci ...@@ -317,7 +317,7 @@ module Ci
def merge_requests def merge_requests
@merge_requests ||= project.merge_requests @merge_requests ||= project.merge_requests
.where(source_branch: self.ref) .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 end
private private
......
...@@ -678,7 +678,7 @@ class MergeRequest < ActiveRecord::Base ...@@ -678,7 +678,7 @@ class MergeRequest < ActiveRecord::Base
def mergeable_ci_state? def mergeable_ci_state?
return true unless project.only_allow_merge_if_build_succeeds? return true unless project.only_allow_merge_if_build_succeeds?
!pipeline || pipeline.success? || pipeline.skipped? !head_pipeline || head_pipeline.success? || head_pipeline.skipped?
end end
def environments def environments
...@@ -774,10 +774,10 @@ class MergeRequest < ActiveRecord::Base ...@@ -774,10 +774,10 @@ class MergeRequest < ActiveRecord::Base
commits.map(&:sha) commits.map(&:sha)
end end
def pipeline def head_pipeline
return unless diff_head_sha && source_project 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 end
def all_pipelines def all_pipelines
......
...@@ -55,7 +55,7 @@ module MergeRequests ...@@ -55,7 +55,7 @@ module MergeRequests
def pipeline_merge_requests(pipeline) def pipeline_merge_requests(pipeline)
merge_requests_for(pipeline.ref).each do |merge_request| 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 yield merge_request
end end
...@@ -63,7 +63,7 @@ module MergeRequests ...@@ -63,7 +63,7 @@ module MergeRequests
def commit_status_merge_requests(commit_status) def commit_status_merge_requests(commit_status)
merge_requests_for(commit_status.ref).each do |merge_request| 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
next unless pipeline.sha == commit_status.sha next unless pipeline.sha == commit_status.sha
......
...@@ -2,12 +2,12 @@ ...@@ -2,12 +2,12 @@
%h2.merge-requests-title %h2.merge-requests-title
= pluralize(@merge_requests.count, 'Related Merge Request') = pluralize(@merge_requests.count, 'Related Merge Request')
%ul.unstyled-list.related-merge-requests %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| - @merge_requests.each do |merge_request|
%li %li
%span.merge-request-ci-status %span.merge-request-ci-status
- if merge_request.pipeline - if merge_request.head_pipeline
= render_pipeline_status(merge_request.pipeline) = render_pipeline_status(merge_request.head_pipeline)
- elsif has_any_ci - elsif has_any_ci
= icon('blank fw') = icon('blank fw')
%span.merge-request-id %span.merge-request-id
......
...@@ -15,9 +15,9 @@ ...@@ -15,9 +15,9 @@
= icon('ban') = icon('ban')
CLOSED CLOSED
- if merge_request.pipeline - if merge_request.head_pipeline
%li %li
= render_pipeline_status(merge_request.pipeline) = render_pipeline_status(merge_request.head_pipeline)
- if merge_request.open? && merge_request.broken? - if merge_request.open? && merge_request.broken?
%li %li
......
...@@ -14,7 +14,7 @@ ...@@ -14,7 +14,7 @@
ci_status_url: "#{ci_status_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}", 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)}", ci_environments_status_url: "#{ci_environments_status_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}",
gitlab_icon: "#{asset_path 'gitlab_logo.png'}", 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: { ci_message: {
normal: "Build {{status}} for \"{{title}}\"", normal: "Build {{status}} for \"{{title}}\"",
preparing: "{{status}} build for \"{{title}}\"" preparing: "{{status}} build for \"{{title}}\""
......
...@@ -192,7 +192,7 @@ module API ...@@ -192,7 +192,7 @@ module API
should_remove_source_branch: params[:should_remove_source_branch] 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). ::MergeRequests::MergeWhenBuildSucceedsService.new(merge_request.target_project, current_user, merge_params).
execute(merge_request) execute(merge_request)
else else
......
...@@ -570,7 +570,7 @@ describe MergeRequest, models: true do ...@@ -570,7 +570,7 @@ describe MergeRequest, models: true do
end end
end end
describe '#pipeline' do describe '#head_pipeline' do
describe 'when the source project exists' do describe 'when the source project exists' do
it 'returns the latest pipeline' do it 'returns the latest pipeline' do
pipeline = double(:ci_pipeline, ref: 'master') pipeline = double(:ci_pipeline, ref: 'master')
...@@ -581,7 +581,7 @@ describe MergeRequest, models: true do ...@@ -581,7 +581,7 @@ describe MergeRequest, models: true do
with('master', '123abc'). with('master', '123abc').
and_return(pipeline) and_return(pipeline)
expect(subject.pipeline).to eq(pipeline) expect(subject.head_pipeline).to eq(pipeline)
end end
end end
...@@ -589,7 +589,7 @@ describe MergeRequest, models: true do ...@@ -589,7 +589,7 @@ describe MergeRequest, models: true do
it 'returns nil' do it 'returns nil' do
allow(subject).to receive(:source_project).and_return(nil) 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 end
end end
...@@ -857,7 +857,7 @@ describe MergeRequest, models: true do ...@@ -857,7 +857,7 @@ describe MergeRequest, models: true do
context 'and a failed pipeline is associated' do context 'and a failed pipeline is associated' do
before do before do
pipeline.update(status: 'failed') pipeline.update(status: 'failed')
allow(subject).to receive(:pipeline) { pipeline } allow(subject).to receive(:head_pipeline) { pipeline }
end end
it { expect(subject.mergeable_ci_state?).to be_falsey } it { expect(subject.mergeable_ci_state?).to be_falsey }
...@@ -866,7 +866,7 @@ describe MergeRequest, models: true do ...@@ -866,7 +866,7 @@ describe MergeRequest, models: true do
context 'and a successful pipeline is associated' do context 'and a successful pipeline is associated' do
before do before do
pipeline.update(status: 'success') pipeline.update(status: 'success')
allow(subject).to receive(:pipeline) { pipeline } allow(subject).to receive(:head_pipeline) { pipeline }
end end
it { expect(subject.mergeable_ci_state?).to be_truthy } it { expect(subject.mergeable_ci_state?).to be_truthy }
...@@ -875,7 +875,7 @@ describe MergeRequest, models: true do ...@@ -875,7 +875,7 @@ describe MergeRequest, models: true do
context 'and a skipped pipeline is associated' do context 'and a skipped pipeline is associated' do
before do before do
pipeline.update(status: 'skipped') pipeline.update(status: 'skipped')
allow(subject).to receive(:pipeline) { pipeline } allow(subject).to receive(:head_pipeline) { pipeline }
end end
it { expect(subject.mergeable_ci_state?).to be_truthy } it { expect(subject.mergeable_ci_state?).to be_truthy }
...@@ -883,7 +883,7 @@ describe MergeRequest, models: true do ...@@ -883,7 +883,7 @@ describe MergeRequest, models: true do
context 'when no pipeline is associated' do context 'when no pipeline is associated' do
before do before do
allow(subject).to receive(:pipeline) { nil } allow(subject).to receive(:head_pipeline) { nil }
end end
it { expect(subject.mergeable_ci_state?).to be_truthy } it { expect(subject.mergeable_ci_state?).to be_truthy }
...@@ -896,7 +896,7 @@ describe MergeRequest, models: true do ...@@ -896,7 +896,7 @@ describe MergeRequest, models: true do
context 'and a failed pipeline is associated' do context 'and a failed pipeline is associated' do
before do before do
pipeline.statuses << create(:commit_status, status: 'failed', project: project) pipeline.statuses << create(:commit_status, status: 'failed', project: project)
allow(subject).to receive(:pipeline) { pipeline } allow(subject).to receive(:head_pipeline) { pipeline }
end end
it { expect(subject.mergeable_ci_state?).to be_truthy } it { expect(subject.mergeable_ci_state?).to be_truthy }
...@@ -904,7 +904,7 @@ describe MergeRequest, models: true do ...@@ -904,7 +904,7 @@ describe MergeRequest, models: true do
context 'when no pipeline is associated' do context 'when no pipeline is associated' do
before do before do
allow(subject).to receive(:pipeline) { nil } allow(subject).to receive(:head_pipeline) { nil }
end end
it { expect(subject.mergeable_ci_state?).to be_truthy } it { expect(subject.mergeable_ci_state?).to be_truthy }
......
...@@ -466,7 +466,7 @@ describe API::API, api: true do ...@@ -466,7 +466,7 @@ describe API::API, api: true do
end end
it "enables merge when build succeeds if the ci is active" do 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) 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 put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), merge_when_build_succeeds: true
......
...@@ -20,13 +20,19 @@ describe MergeRequests::AddTodoWhenBuildFailsService do ...@@ -20,13 +20,19 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
let(:todo_service) { TodoService.new } let(:todo_service) { TodoService.new }
let(:merge_request) do let(:merge_request) do
create(:merge_request, merge_user: user, source_branch: 'master', create(:merge_request, merge_user: user,
target_branch: 'feature', source_project: project, target_project: project, source_branch: 'master',
target_branch: 'feature',
source_project: project,
target_project: project,
state: 'opened') state: 'opened')
end end
before do 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) allow(service).to receive(:todo_service).and_return(todo_service)
end end
......
...@@ -21,7 +21,10 @@ describe MergeRequests::MergeWhenBuildSucceedsService do ...@@ -21,7 +21,10 @@ describe MergeRequests::MergeWhenBuildSucceedsService do
context 'first time enabling' do context 'first time enabling' do
before 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) service.execute(merge_request)
end end
...@@ -43,8 +46,12 @@ describe MergeRequests::MergeWhenBuildSucceedsService do ...@@ -43,8 +46,12 @@ describe MergeRequests::MergeWhenBuildSucceedsService do
let(:build) { create(:ci_build, ref: mr_merge_if_green_enabled.source_branch) } let(:build) { create(:ci_build, ref: mr_merge_if_green_enabled.source_branch) }
before do before do
allow(mr_merge_if_green_enabled).to receive(:pipeline).and_return(pipeline) allow(mr_merge_if_green_enabled).to receive(:head_pipeline)
allow(mr_merge_if_green_enabled).to receive(:mergeable?).and_return(true) .and_return(pipeline)
allow(mr_merge_if_green_enabled).to receive(:mergeable?)
.and_return(true)
allow(pipeline).to receive(:success?).and_return(true) allow(pipeline).to receive(:success?).and_return(true)
end end
...@@ -138,9 +145,12 @@ describe MergeRequests::MergeWhenBuildSucceedsService do ...@@ -138,9 +145,12 @@ describe MergeRequests::MergeWhenBuildSucceedsService do
before do before do
# This behavior of MergeRequest: we instantiate a new object # 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) allow_any_instance_of(MergeRequest)
end .to receive(:head_pipeline)
.and_wrap_original do
Ci::Pipeline.find(pipeline.id)
end
end end
it "doesn't merge if any of stages failed" do it "doesn't merge if any of stages failed" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment