Commit f988cafd authored by Maxime Orefice's avatar Maxime Orefice Committed by Shinya Maeda

Refactor merge request pipeline

This commit renames our merge_request_pipeline as
merged_request_pipeline. This is part of our effort to consolidate
our naming with merge request pipeline.
parent dbe5012f
...@@ -1117,7 +1117,7 @@ module Ci ...@@ -1117,7 +1117,7 @@ module Ci
detached_merge_request_pipeline? && !merge_request_ref? detached_merge_request_pipeline? && !merge_request_ref?
end end
def merge_request_pipeline? def merged_result_pipeline?
merge_request? && target_sha.present? merge_request? && target_sha.present?
end end
...@@ -1157,7 +1157,7 @@ module Ci ...@@ -1157,7 +1157,7 @@ module Ci
return unless merge_request? return unless merge_request?
strong_memoize(:merge_request_event_type) do strong_memoize(:merge_request_event_type) do
if merge_request_pipeline? if merged_result_pipeline?
:merged_result :merged_result
elsif detached_merge_request_pipeline? elsif detached_merge_request_pipeline?
:detached :detached
...@@ -1165,10 +1165,6 @@ module Ci ...@@ -1165,10 +1165,6 @@ module Ci
end end
end end
def for_merged_result?
merge_request_event_type == :merged_result
end
def persistent_ref def persistent_ref
@persistent_ref ||= PersistentRef.new(pipeline: self) @persistent_ref ||= PersistentRef.new(pipeline: self)
end end
...@@ -1251,7 +1247,7 @@ module Ci ...@@ -1251,7 +1247,7 @@ module Ci
def merge_request_diff_sha def merge_request_diff_sha
return unless merge_request? return unless merge_request?
if merge_request_pipeline? if merged_result_pipeline?
source_sha source_sha
else else
sha sha
......
...@@ -61,7 +61,7 @@ module Ci ...@@ -61,7 +61,7 @@ module Ci
link_to_merge_request: link_to_merge_request, link_to_merge_request: link_to_merge_request,
link_to_merge_request_source_branch: link_to_merge_request_source_branch link_to_merge_request_source_branch: link_to_merge_request_source_branch
} }
elsif pipeline.merge_request_pipeline? elsif pipeline.merged_result_pipeline?
_("for %{link_to_merge_request} with %{link_to_merge_request_source_branch} into %{link_to_merge_request_target_branch}") _("for %{link_to_merge_request} with %{link_to_merge_request_source_branch} into %{link_to_merge_request_target_branch}")
.html_safe % { .html_safe % {
link_to_merge_request: link_to_merge_request, link_to_merge_request: link_to_merge_request,
......
...@@ -31,7 +31,7 @@ class Ci::PipelineEntity < Grape::Entity ...@@ -31,7 +31,7 @@ class Ci::PipelineEntity < Grape::Entity
expose :can_cancel?, as: :cancelable expose :can_cancel?, as: :cancelable
expose :failure_reason?, as: :failure_reason expose :failure_reason?, as: :failure_reason
expose :detached_merge_request_pipeline?, as: :detached_merge_request_pipeline expose :detached_merge_request_pipeline?, as: :detached_merge_request_pipeline
expose :merge_request_pipeline?, as: :merge_request_pipeline expose :merged_result_pipeline?, as: :merge_request_pipeline
end end
expose :details do expose :details do
...@@ -64,8 +64,8 @@ class Ci::PipelineEntity < Grape::Entity ...@@ -64,8 +64,8 @@ class Ci::PipelineEntity < Grape::Entity
expose :commit, using: CommitEntity expose :commit, using: CommitEntity
expose :merge_request_event_type, if: -> (pipeline, _) { pipeline.merge_request? } expose :merge_request_event_type, if: -> (pipeline, _) { pipeline.merge_request? }
expose :source_sha, if: -> (pipeline, _) { pipeline.merge_request_pipeline? } expose :source_sha, if: -> (pipeline, _) { pipeline.merged_result_pipeline? }
expose :target_sha, if: -> (pipeline, _) { pipeline.merge_request_pipeline? } expose :target_sha, if: -> (pipeline, _) { pipeline.merged_result_pipeline? }
expose :yaml_errors, if: -> (pipeline, _) { pipeline.has_yaml_errors? } expose :yaml_errors, if: -> (pipeline, _) { pipeline.has_yaml_errors? }
expose :failure_reason, if: -> (pipeline, _) { pipeline.failure_reason? } expose :failure_reason, if: -> (pipeline, _) { pipeline.failure_reason? }
......
...@@ -157,7 +157,7 @@ class MergeRequestWidgetEntity < Grape::Entity ...@@ -157,7 +157,7 @@ class MergeRequestWidgetEntity < Grape::Entity
end end
def use_merge_base_with_merged_results? def use_merge_base_with_merged_results?
object.actual_head_pipeline&.for_merged_result? object.actual_head_pipeline&.merged_result_pipeline?
end end
def head_pipeline_downloadable_path_for_report_type(file_type) def head_pipeline_downloadable_path_for_report_type(file_type)
......
...@@ -11,7 +11,7 @@ class MergeRequests::PipelineEntity < Grape::Entity ...@@ -11,7 +11,7 @@ class MergeRequests::PipelineEntity < Grape::Entity
end end
expose :flags do expose :flags do
expose :merge_request_pipeline?, as: :merge_request_pipeline expose :merged_result_pipeline?, as: :merge_request_pipeline
end end
expose :commit, using: CommitEntity expose :commit, using: CommitEntity
......
...@@ -129,8 +129,8 @@ module EE ...@@ -129,8 +129,8 @@ module EE
## ##
# Check if it's a merge request pipeline with the HEAD of source and target branches # Check if it's a merge request pipeline with the HEAD of source and target branches
# TODO: Make `Ci::Pipeline#latest?` compatible with merge request pipelines and remove this method. # TODO: Make `Ci::Pipeline#latest?` compatible with merge request pipelines and remove this method.
def latest_merge_request_pipeline? def latest_merged_result_pipeline?
merge_request_pipeline? && merged_result_pipeline? &&
source_sha == merge_request.diff_head_sha && source_sha == merge_request.diff_head_sha &&
target_sha == merge_request.target_branch_sha target_sha == merge_request.target_branch_sha
end end
...@@ -146,7 +146,7 @@ module EE ...@@ -146,7 +146,7 @@ module EE
override :merge_train_pipeline? override :merge_train_pipeline?
def merge_train_pipeline? def merge_train_pipeline?
merge_request_pipeline? && merge_train_ref? merged_result_pipeline? && merge_train_ref?
end end
def latest_failed_security_builds def latest_failed_security_builds
......
...@@ -50,7 +50,7 @@ RSpec.describe 'Two merge requests on a merge train' do ...@@ -50,7 +50,7 @@ RSpec.describe 'Two merge requests on a merge train' do
end end
it 'creates a pipeline for merge request 1', :sidekiq_might_not_need_inline do it 'creates a pipeline for merge request 1', :sidekiq_might_not_need_inline do
expect(merge_request_1.merge_train.pipeline).to be_merge_request_pipeline expect(merge_request_1.merge_train.pipeline).to be_merged_result_pipeline
expect(merge_request_1.merge_train.pipeline.user).to eq(maintainer_1) expect(merge_request_1.merge_train.pipeline.user).to eq(maintainer_1)
expect(merge_request_1.merge_train.pipeline.ref).to eq(merge_request_1.train_ref_path) expect(merge_request_1.merge_train.pipeline.ref).to eq(merge_request_1.train_ref_path)
expect(merge_request_1.merge_train.pipeline.target_sha) expect(merge_request_1.merge_train.pipeline.target_sha)
...@@ -58,7 +58,7 @@ RSpec.describe 'Two merge requests on a merge train' do ...@@ -58,7 +58,7 @@ RSpec.describe 'Two merge requests on a merge train' do
end end
it 'creates a pipeline for merge request 2', :sidekiq_might_not_need_inline do it 'creates a pipeline for merge request 2', :sidekiq_might_not_need_inline do
expect(merge_request_2.merge_train.pipeline).to be_merge_request_pipeline expect(merge_request_2.merge_train.pipeline).to be_merged_result_pipeline
expect(merge_request_2.merge_train.pipeline.user).to eq(maintainer_2) expect(merge_request_2.merge_train.pipeline.user).to eq(maintainer_2)
expect(merge_request_2.merge_train.pipeline.ref).to eq(merge_request_2.train_ref_path) expect(merge_request_2.merge_train.pipeline.ref).to eq(merge_request_2.train_ref_path)
expect(merge_request_2.merge_train.pipeline.target_sha) expect(merge_request_2.merge_train.pipeline.target_sha)
......
...@@ -420,8 +420,8 @@ RSpec.describe Ci::Pipeline do ...@@ -420,8 +420,8 @@ RSpec.describe Ci::Pipeline do
end end
end end
describe '#latest_merge_request_pipeline?' do describe '#latest_merged_result_pipeline?' do
subject { pipeline.latest_merge_request_pipeline? } subject { pipeline.latest_merged_result_pipeline? }
let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) } let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) }
let(:pipeline) { merge_request.all_pipelines.first } let(:pipeline) { merge_request.all_pipelines.first }
......
...@@ -49,7 +49,7 @@ RSpec.describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_ ...@@ -49,7 +49,7 @@ RSpec.describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_
subject subject
expect(merge_request.all_pipelines.count).to eq(1) expect(merge_request.all_pipelines.count).to eq(1)
expect(merge_request.all_pipelines.last).not_to be_merge_request_pipeline expect(merge_request.all_pipelines.last).not_to be_merged_result_pipeline
expect(merge_request.all_pipelines.last).to be_detached_merge_request_pipeline expect(merge_request.all_pipelines.last).to be_detached_merge_request_pipeline
end end
end end
...@@ -58,7 +58,7 @@ RSpec.describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_ ...@@ -58,7 +58,7 @@ RSpec.describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_
subject subject
expect(merge_request.all_pipelines.count).to eq(1) expect(merge_request.all_pipelines.count).to eq(1)
expect(merge_request.all_pipelines.last).to be_merge_request_pipeline expect(merge_request.all_pipelines.last).to be_merged_result_pipeline
expect(merge_request.all_pipelines.last).not_to be_detached_merge_request_pipeline expect(merge_request.all_pipelines.last).not_to be_detached_merge_request_pipeline
end end
......
...@@ -274,7 +274,7 @@ RSpec.describe MergeRequests::RefreshService do ...@@ -274,7 +274,7 @@ RSpec.describe MergeRequests::RefreshService do
expect { subject } expect { subject }
.to change { merge_request.pipelines_for_merge_request.count }.by(1) .to change { merge_request.pipelines_for_merge_request.count }.by(1)
expect(merge_request.all_pipelines.last).to be_merge_request_pipeline expect(merge_request.all_pipelines.last).to be_merged_result_pipeline
end end
context 'when MergeRequestUpdateWorker is retried by an exception' do context 'when MergeRequestUpdateWorker is retried by an exception' do
......
...@@ -373,8 +373,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ...@@ -373,8 +373,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end end
end end
describe '#merge_request_pipeline?' do describe '#merged_result_pipeline?' do
subject { pipeline.merge_request_pipeline? } subject { pipeline.merged_result_pipeline? }
let!(:pipeline) do let!(:pipeline) do
create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, target_sha: target_sha) create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, target_sha: target_sha)
...@@ -422,24 +422,6 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ...@@ -422,24 +422,6 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end end
end end
describe '#for_merged_result?' do
subject { pipeline.for_merged_result? }
let(:pipeline) { merge_request.all_pipelines.last }
context 'when pipeline is merge request pipeline' do
let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) }
it { is_expected.to be_truthy }
end
context 'when pipeline is detached merge request pipeline' do
let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) }
it { is_expected.to be_falsey }
end
end
describe '#legacy_detached_merge_request_pipeline?' do describe '#legacy_detached_merge_request_pipeline?' do
subject { pipeline.legacy_detached_merge_request_pipeline? } subject { pipeline.legacy_detached_merge_request_pipeline? }
......
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