diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index 10fc5d399b73f98a883f1d19166295d70b224e2f..4a105c8015419c3fb4ec85d359e937157142b34b 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -261,7 +261,7 @@ def set_pipeline_path
end
def latest_pipeline
- @project.latest_pipeline_for_ref(params['ref'])
+ @project.latest_pipeline(params['ref'])
&.present(current_user: current_user)
end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 6174fd139b3aba3be97f81f1b28eaeac2fc9be8f..ccf9f501799f9953c5235de156596776d785086d 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -364,7 +364,7 @@ def merge_pipeline
# when it is fast-forward there is no merge commit, so we must fall back to
# either the squash commit (if the MR was squashed) or the diff head commit.
sha = merge_commit_sha || squash_commit_sha || diff_head_sha
- target_project.pipeline_for(target_branch, sha)
+ target_project.latest_pipeline(target_branch, sha)
end
# Pattern used to extract `!123` merge request references from text
diff --git a/app/models/project.rb b/app/models/project.rb
index a40f3b35adde1a68ddb3ac2e8f93514724964fdb..6f0497d36467a453854823f2ec3d0e0e63d90a21 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -956,13 +956,12 @@ def latest_successful_build_for_ref!(job_name, ref = default_branch)
latest_successful_build_for_ref(job_name, ref) || raise(ActiveRecord::RecordNotFound.new("Couldn't find job #{job_name}"))
end
- def latest_pipeline_for_ref(ref = default_branch)
+ def latest_pipeline(ref = default_branch, sha = nil)
ref = ref.presence || default_branch
- sha = commit(ref)&.sha
-
+ sha ||= commit(ref)&.sha
return unless sha
- ci_pipelines.newest_first(ref: ref, sha: sha).first
+ ci_pipelines.newest_first(ref: ref, sha: sha).take
end
def merge_base_commit(first_commit_id, second_commit_id)
@@ -1671,21 +1670,6 @@ def allowed_to_share_with_group?
!namespace.share_with_group_lock
end
- def pipeline_for(ref, sha = nil, id = nil)
- sha ||= commit(ref).try(:sha)
- return unless sha
-
- if id.present?
- pipelines_for(ref, sha).find_by(id: id)
- else
- pipelines_for(ref, sha).take
- end
- end
-
- def pipelines_for(ref, sha)
- ci_pipelines.order(id: :desc).where(sha: sha, ref: ref)
- end
-
def latest_successful_pipeline_for_default_branch
if defined?(@latest_successful_pipeline_for_default_branch)
return @latest_successful_pipeline_for_default_branch
diff --git a/app/services/issues/related_branches_service.rb b/app/services/issues/related_branches_service.rb
index 46076218857861269e4a5a29de0022778081810a..98d8412102f866d446dfe494194f006939827d82 100644
--- a/app/services/issues/related_branches_service.rb
+++ b/app/services/issues/related_branches_service.rb
@@ -24,7 +24,7 @@ def pipeline_status(branch_name)
return unless target
- pipeline = project.pipeline_for(branch_name, target.sha)
+ pipeline = project.latest_pipeline(branch_name, target.sha)
pipeline.detailed_status(current_user) if can?(current_user, :read_pipeline, pipeline)
end
diff --git a/app/views/projects/buttons/_download.html.haml b/app/views/projects/buttons/_download.html.haml
index 1d0ad6dcde6585748d8a033bc0998ed42669eeb6..c04687bd8468b133510945a6b0e26a68480700d7 100644
--- a/app/views/projects/buttons/_download.html.haml
+++ b/app/views/projects/buttons/_download.html.haml
@@ -17,7 +17,7 @@
%section.border-top.pt-1.mt-1
%h5.m-0.dropdown-bold-header= _('Download artifacts')
- unless pipeline.latest?
- %span.unclickable= ci_status_for_statuseable(project.pipeline_for(ref))
+ %span.unclickable= ci_status_for_statuseable(project.latest_pipeline(ref))
%h6.m-0.dropdown-header= _('Previous Artifacts')
%ul
- pipeline.latest_builds_with_artifacts.each do |job|
diff --git a/ee/app/models/concerns/latest_pipeline_information.rb b/ee/app/models/concerns/latest_pipeline_information.rb
index b009478e33b933d27702ce1712bc82bccdecbb43..ea590576536b9cc19e4e634b502d02e44cf51cf8 100644
--- a/ee/app/models/concerns/latest_pipeline_information.rb
+++ b/ee/app/models/concerns/latest_pipeline_information.rb
@@ -29,7 +29,7 @@ def latest_security_builds
end
def latest_default_branch_pipeline
- strong_memoize(:pipeline) { latest_pipeline_for_ref }
+ strong_memoize(:pipeline) { latest_pipeline }
end
def auto_devops_source?
diff --git a/ee/app/presenters/projects/security/configuration_presenter.rb b/ee/app/presenters/projects/security/configuration_presenter.rb
index 36724b57d0a49e408bab7a36b60dd54825c86b38..97b3837246cc30e435d1e03bbcd2946ebb64cce1 100644
--- a/ee/app/presenters/projects/security/configuration_presenter.rb
+++ b/ee/app/presenters/projects/security/configuration_presenter.rb
@@ -90,7 +90,7 @@ def can_enable_auto_devops?
end
def gitlab_ci_present?
- latest_pipeline_for_ref.try(:config_path) == Gitlab::FileDetector::PATTERNS[:gitlab_ci]
+ latest_pipeline.try(:config_path) == Gitlab::FileDetector::PATTERNS[:gitlab_ci]
end
def features
diff --git a/ee/app/services/ci/subscribe_bridge_service.rb b/ee/app/services/ci/subscribe_bridge_service.rb
index dc756623c7158cc5ee6c8f25f61cabecbd4247d4..a0b0ed46a9aaeb741b9dcff318c715a4d81c9b18 100644
--- a/ee/app/services/ci/subscribe_bridge_service.rb
+++ b/ee/app/services/ci/subscribe_bridge_service.rb
@@ -35,7 +35,7 @@ def upstream_project
def upstream_pipeline
strong_memoize(:upstream_pipeline) do
- upstream_project.pipeline_for(upstream_project.default_branch)
+ upstream_project.latest_pipeline(upstream_project.default_branch)
end
end
end
diff --git a/ee/spec/presenters/projects/security/configuration_presenter_spec.rb b/ee/spec/presenters/projects/security/configuration_presenter_spec.rb
index a9b324bd438818b68013cd35601f0ed2439ade89..8ddcd7570366bab97b9a54a08b6d2b018fc6c104 100644
--- a/ee/spec/presenters/projects/security/configuration_presenter_spec.rb
+++ b/ee/spec/presenters/projects/security/configuration_presenter_spec.rb
@@ -190,7 +190,7 @@
end
it 'expects the gitlab_ci_presence to be false if the file is absent' do
- allow_any_instance_of(described_class).to receive(:latest_pipeline_for_ref).and_return(nil)
+ allow_any_instance_of(described_class).to receive(:latest_pipeline).and_return(nil)
expect(subject[:gitlab_ci_present]).to eq(false)
end
end
diff --git a/lib/api/ci/pipelines.rb b/lib/api/ci/pipelines.rb
index a010e0dd7615d52e72b857a3a9651cb1afebc36d..55089045e809d1f7a2e984276688141df8d61dc8 100644
--- a/lib/api/ci/pipelines.rb
+++ b/lib/api/ci/pipelines.rb
@@ -178,7 +178,7 @@ def pipeline
def latest_pipeline
strong_memoize(:latest_pipeline) do
- user_project.latest_pipeline_for_ref(params[:ref])
+ user_project.latest_pipeline(params[:ref])
end
end
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index efa2353259cdf83ecc15ff9c0e2b45ef93c1098d..89772a651c2e484f18d2b560e660591e3ff821c0 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1269,60 +1269,6 @@
end
end
- describe '#pipeline_for' do
- let(:project) { create(:project, :repository) }
-
- shared_examples 'giving the correct pipeline' do
- it { is_expected.to eq(pipeline) }
-
- context 'return latest' do
- let!(:pipeline2) { create_pipeline(project) }
-
- it { is_expected.to eq(pipeline2) }
- end
- end
-
- context 'with a matching pipeline' do
- let!(:pipeline) { create_pipeline(project) }
-
- context 'with explicit sha' do
- subject { project.pipeline_for('master', pipeline.sha) }
-
- it_behaves_like 'giving the correct pipeline'
-
- context 'with supplied id' do
- let!(:other_pipeline) { create_pipeline(project) }
-
- subject { project.pipeline_for('master', pipeline.sha, other_pipeline.id) }
-
- it { is_expected.to eq(other_pipeline) }
- end
- end
-
- context 'with implicit sha' do
- subject { project.pipeline_for('master') }
-
- it_behaves_like 'giving the correct pipeline'
- end
- end
-
- context 'when there is no matching pipeline' do
- subject { project.pipeline_for('master') }
-
- it { is_expected.to be_nil }
- end
- end
-
- describe '#pipelines_for' do
- let(:project) { create(:project, :repository) }
- let!(:pipeline) { create_pipeline(project) }
- let!(:other_pipeline) { create_pipeline(project) }
-
- subject { project.pipelines_for(project.default_branch, project.commit.sha) }
-
- it { is_expected.to contain_exactly(pipeline, other_pipeline) }
- end
-
describe '#builds_enabled' do
let(:project) { create(:project) }
@@ -2369,42 +2315,90 @@
end
end
- describe '#latest_pipeline_for_ref' do
+ describe '#latest_pipeline' do
let(:project) { create(:project, :repository) }
let(:second_branch) { project.repository.branches[2] }
let!(:pipeline_for_default_branch) do
- create(:ci_empty_pipeline, project: project, sha: project.commit.id,
- ref: project.default_branch)
+ create(:ci_pipeline, project: project, sha: project.commit.id,
+ ref: project.default_branch)
end
let!(:pipeline_for_second_branch) do
- create(:ci_empty_pipeline, project: project, sha: second_branch.target,
- ref: second_branch.name)
+ create(:ci_pipeline, project: project, sha: second_branch.target,
+ ref: second_branch.name)
end
- before do
- create(:ci_empty_pipeline, project: project, sha: project.commit.parent.id,
- ref: project.default_branch)
+ let!(:other_pipeline_for_default_branch) do
+ create(:ci_pipeline, project: project, sha: project.commit.parent.id,
+ ref: project.default_branch)
end
context 'default repository branch' do
- subject { project.latest_pipeline_for_ref(project.default_branch) }
+ context 'when explicitly provided' do
+ subject { project.latest_pipeline(project.default_branch) }
+
+ it { is_expected.to eq(pipeline_for_default_branch) }
+ end
+
+ context 'when not provided' do
+ subject { project.latest_pipeline }
+
+ it { is_expected.to eq(pipeline_for_default_branch) }
+ end
- it { is_expected.to eq(pipeline_for_default_branch) }
+ context 'with provided sha' do
+ subject { project.latest_pipeline(project.default_branch, project.commit.parent.id) }
+
+ it { is_expected.to eq(other_pipeline_for_default_branch) }
+ end
end
context 'provided ref' do
- subject { project.latest_pipeline_for_ref(second_branch.name) }
+ subject { project.latest_pipeline(second_branch.name) }
it { is_expected.to eq(pipeline_for_second_branch) }
+
+ context 'with provided sha' do
+ let!(:latest_pipeline_for_ref) do
+ create(:ci_pipeline, project: project, sha: pipeline_for_second_branch.sha,
+ ref: pipeline_for_second_branch.ref)
+ end
+
+ subject { project.latest_pipeline(second_branch.name, second_branch.target) }
+
+ it { is_expected.to eq(latest_pipeline_for_ref) }
+ end
end
context 'bad ref' do
- subject { project.latest_pipeline_for_ref(SecureRandom.uuid) }
+ before do
+ # ensure we don't skip the filter by ref by mistakenly return this pipeline
+ create(:ci_pipeline, project: project)
+ end
+
+ subject { project.latest_pipeline(SecureRandom.uuid) }
it { is_expected.to be_nil }
end
+
+ context 'on deleted ref' do
+ let(:branch) { project.repository.branches.last }
+
+ let!(:pipeline_on_deleted_ref) do
+ create(:ci_pipeline, project: project, sha: branch.target, ref: branch.name)
+ end
+
+ before do
+ project.repository.rm_branch(project.owner, branch.name)
+ end
+
+ subject { project.latest_pipeline(branch.name) }
+
+ it 'always returns nil despite a pipeline exists' do
+ expect(subject).to be_nil
+ end
+ end
end
describe '#latest_successful_build_for_sha' do
diff --git a/spec/services/issues/related_branches_service_spec.rb b/spec/services/issues/related_branches_service_spec.rb
index d79132d98db5a506fd00d35a3707c60b17fd89d9..1780023803a63d752449bd9954ca80b70159eb13 100644
--- a/spec/services/issues/related_branches_service_spec.rb
+++ b/spec/services/issues/related_branches_service_spec.rb
@@ -57,7 +57,7 @@ def make_branch
unreadable_branch_name => unreadable_pipeline
}.each do |name, pipeline|
allow(repo).to receive(:find_branch).with(name).and_return(make_branch)
- allow(project).to receive(:pipeline_for).with(name, sha).and_return(pipeline)
+ allow(project).to receive(:latest_pipeline).with(name, sha).and_return(pipeline)
end
allow(repo).to receive(:find_branch).with(missing_branch).and_return(nil)