diff options
-rw-r--r-- | app/models/ci/pipeline.rb | 18 | ||||
-rw-r--r-- | app/models/commit.rb | 12 | ||||
-rw-r--r-- | app/services/ci/image_for_build_service.rb | 8 | ||||
-rw-r--r-- | changelogs/unreleased/show-commit-status-from-latest-pipeline.yml | 4 | ||||
-rw-r--r-- | lib/gitlab/badge/build/status.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/badge/build/status_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/ci/pipeline_spec.rb | 59 | ||||
-rw-r--r-- | spec/models/commit_spec.rb | 29 |
8 files changed, 105 insertions, 33 deletions
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 54f73171fd4..48354cdbefb 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -88,8 +88,24 @@ module Ci end # ref can't be HEAD or SHA, can only be branch/tag name + scope :latest, ->(ref = nil) do + max_id = unscope(:select) + .select("max(#{quoted_table_name}.id)") + .group(:ref, :sha) + + if ref + where(id: max_id, ref: ref) + else + where(id: max_id) + end + end + + def self.latest_status(ref = nil) + latest(ref).status + end + def self.latest_successful_for(ref) - where(ref: ref).order(id: :desc).success.first + success.latest(ref).first end def self.truncate_sha(sha) diff --git a/app/models/commit.rb b/app/models/commit.rb index 1831cc7e175..69cfc47f5bf 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -228,13 +228,9 @@ class Commit def status(ref = nil) @statuses ||= {} - if @statuses.key?(ref) - @statuses[ref] - elsif ref - @statuses[ref] = pipelines.where(ref: ref).status - else - @statuses[ref] = pipelines.status - end + return @statuses[ref] if @statuses.key?(ref) + + @statuses[ref] = pipelines.latest_status(ref) end def revert_branch_name @@ -270,7 +266,7 @@ class Commit @merged_merge_request_hash ||= Hash.new do |hash, user| hash[user] = merged_merge_request_no_cache(user) end - + @merged_merge_request_hash[current_user] end diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb index 75d847d5bee..240ddabec36 100644 --- a/app/services/ci/image_for_build_service.rb +++ b/app/services/ci/image_for_build_service.rb @@ -1,13 +1,13 @@ module Ci class ImageForBuildService def execute(project, opts) - sha = opts[:sha] || ref_sha(project, opts[:ref]) - + ref = opts[:ref] + sha = opts[:sha] || ref_sha(project, ref) pipelines = project.pipelines.where(sha: sha) - pipelines = pipelines.where(ref: opts[:ref]) if opts[:ref] - image_name = image_for_status(pipelines.status) + image_name = image_for_status(pipelines.latest_status(ref)) image_path = Rails.root.join('public/ci', image_name) + OpenStruct.new(path: image_path, name: image_name) end diff --git a/changelogs/unreleased/show-commit-status-from-latest-pipeline.yml b/changelogs/unreleased/show-commit-status-from-latest-pipeline.yml new file mode 100644 index 00000000000..bbd7a217493 --- /dev/null +++ b/changelogs/unreleased/show-commit-status-from-latest-pipeline.yml @@ -0,0 +1,4 @@ +--- +title: Show commit status from latest pipeline +merge_request: 7333 +author: diff --git a/lib/gitlab/badge/build/status.rb b/lib/gitlab/badge/build/status.rb index 50aa45e5406..b762d85b6e5 100644 --- a/lib/gitlab/badge/build/status.rb +++ b/lib/gitlab/badge/build/status.rb @@ -20,8 +20,8 @@ module Gitlab def status @project.pipelines - .where(sha: @sha, ref: @ref) - .status || 'unknown' + .where(sha: @sha) + .latest_status(@ref) || 'unknown' end def metadata diff --git a/spec/lib/gitlab/badge/build/status_spec.rb b/spec/lib/gitlab/badge/build/status_spec.rb index 38eebb2a176..70f03021d36 100644 --- a/spec/lib/gitlab/badge/build/status_spec.rb +++ b/spec/lib/gitlab/badge/build/status_spec.rb @@ -69,8 +69,8 @@ describe Gitlab::Badge::Build::Status do new_build.success! end - it 'reports the compound status' do - expect(badge.status).to eq 'failed' + it 'does not take outdated pipeline into account' do + expect(badge.status).to eq 'success' end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index e78ae14b737..52dd41065e9 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -381,6 +381,65 @@ describe Ci::Pipeline, models: true do end end + shared_context 'with some outdated pipelines' do + before do + create_pipeline(:canceled, 'ref', 'A') + create_pipeline(:success, 'ref', 'A') + create_pipeline(:failed, 'ref', 'B') + create_pipeline(:skipped, 'feature', 'C') + end + + def create_pipeline(status, ref, sha) + create(:ci_empty_pipeline, status: status, ref: ref, sha: sha) + end + end + + describe '.latest' do + include_context 'with some outdated pipelines' + + context 'when no ref is specified' do + let(:pipelines) { described_class.latest.all } + + it 'returns the latest pipeline for the same ref and different sha' do + expect(pipelines.map(&:sha)).to contain_exactly('A', 'B', 'C') + expect(pipelines.map(&:status)). + to contain_exactly('success', 'failed', 'skipped') + end + end + + context 'when ref is specified' do + let(:pipelines) { described_class.latest('ref').all } + + it 'returns the latest pipeline for ref and different sha' do + expect(pipelines.map(&:sha)).to contain_exactly('A', 'B') + expect(pipelines.map(&:status)). + to contain_exactly('success', 'failed') + end + end + end + + describe '.latest_status' do + include_context 'with some outdated pipelines' + + context 'when no ref is specified' do + let(:latest_status) { described_class.latest_status } + + it 'returns the latest status for the same ref and different sha' do + expect(latest_status).to eq(described_class.latest.status) + expect(latest_status).to eq('failed') + end + end + + context 'when ref is specified' do + let(:latest_status) { described_class.latest_status('ref') } + + it 'returns the latest status for ref and different sha' do + expect(latest_status).to eq(described_class.latest_status('ref')) + expect(latest_status).to eq('failed') + end + end + end + describe '#status' do let!(:build) { create(:ci_build, :created, pipeline: pipeline, name: 'test') } diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 0935fc0561c..74b50d2908d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -213,23 +213,19 @@ eos end describe '#status' do - context 'without arguments for compound status' do - shared_examples 'giving the status from pipeline' do - it do - expect(commit.status).to eq(Ci::Pipeline.status) - end - end - - context 'with pipelines' do - let!(:pipeline) do - create(:ci_empty_pipeline, project: project, sha: commit.sha) + context 'without ref argument' do + before do + %w[success failed created pending].each do |status| + create(:ci_empty_pipeline, + project: project, + sha: commit.sha, + status: status) end - - it_behaves_like 'giving the status from pipeline' end - context 'without pipelines' do - it_behaves_like 'giving the status from pipeline' + it 'gives compound status from latest pipelines' do + expect(commit.status).to eq(Ci::Pipeline.latest_status) + expect(commit.status).to eq('pending') end end @@ -255,8 +251,9 @@ eos expect(commit.status('fix')).to eq(pipeline_from_fix.status) end - it 'gives compound status if ref is nil' do - expect(commit.status(nil)).to eq(commit.status) + it 'gives compound status from latest pipelines if ref is nil' do + expect(commit.status(nil)).to eq(Ci::Pipeline.latest_status) + expect(commit.status(nil)).to eq('failed') end end end |