From 8c21610c79d2737c9cd728964f499d793e6a1279 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 28 Aug 2019 14:51:26 +0700 Subject: [PATCH] Add pipeline.type key to PipelineEntity This commit adds pipeline.type key to PipelineEntity. This key will be used in MR widget in the next iteration. --- app/models/ci/pipeline.rb | 23 +++++++ app/models/merge_request.rb | 4 ++ app/presenters/ci/pipeline_presenter.rb | 12 ++++ app/serializers/pipeline_entity.rb | 16 +++-- locale/gitlab.pot | 9 +++ spec/fixtures/api/schemas/pipeline.json | 8 +++ spec/models/ci/pipeline_spec.rb | 64 ++++++++++++++++++- spec/models/merge_request_spec.rb | 34 ++++++++++ spec/presenters/ci/pipeline_presenter_spec.rb | 34 ++++++++++ spec/serializers/pipeline_entity_spec.rb | 6 +- 10 files changed, 204 insertions(+), 6 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 64e372878e6..c44cdb5a70d 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -670,6 +670,7 @@ module Ci variables.append(key: 'CI_COMMIT_REF_PROTECTED', value: (!!protected_ref?).to_s) if merge_request_event? && merge_request + variables.append(key: 'CI_MERGE_REQUEST_EVENT_TYPE', value: merge_request_event_type.to_s) variables.append(key: 'CI_MERGE_REQUEST_SOURCE_BRANCH_SHA', value: source_sha.to_s) variables.append(key: 'CI_MERGE_REQUEST_TARGET_BRANCH_SHA', value: target_sha.to_s) variables.concat(merge_request.predefined_variables) @@ -772,10 +773,18 @@ module Ci triggered_by_merge_request? && target_sha.present? end + def merge_train_pipeline? + merge_request_pipeline? && merge_train_ref? + end + def merge_request_ref? MergeRequest.merge_request_ref?(ref) end + def merge_train_ref? + MergeRequest.merge_train_ref?(ref) + end + def matches_sha_or_source_sha?(sha) self.sha == sha || self.source_sha == sha end @@ -804,6 +813,20 @@ module Ci errors ? errors.full_messages.to_sentence : "" end + def merge_request_event_type + return unless merge_request_event? + + strong_memoize(:merge_request_event_type) do + if detached_merge_request_pipeline? + :detached + elsif merge_request_pipeline? + :merged_result + elsif merge_train_pipeline? + :merge_train + end + end + end + private def ci_yaml_from_repo diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index bfd636fa62a..28e450f9b30 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1142,6 +1142,10 @@ class MergeRequest < ApplicationRecord ref.start_with?("refs/#{Repository::REF_MERGE_REQUEST}/") end + def self.merge_train_ref?(ref) + %r{\Arefs/#{Repository::REF_MERGE_REQUEST}/\d+/train\z}.match?(ref) + end + def in_locked_state begin lock_mr diff --git a/app/presenters/ci/pipeline_presenter.rb b/app/presenters/ci/pipeline_presenter.rb index 358473d0a74..a96f97988b2 100644 --- a/app/presenters/ci/pipeline_presenter.rb +++ b/app/presenters/ci/pipeline_presenter.rb @@ -34,6 +34,18 @@ module Ci end end + NAMES = { + merge_train: s_('Pipeline|Merge train pipeline'), + merged_result: s_('Pipeline|Merged result pipeline'), + detached: s_('Pipeline|Detached merge request pipeline') + }.freeze + + def name + # Currently, `merge_request_event_type` is the only source to name pipelines + # but this could be extended with the other types in the future. + NAMES.fetch(pipeline.merge_request_event_type, s_('Pipeline|Pipeline')) + end + def ref_text if pipeline.detached_merge_request_pipeline? _("for %{link_to_merge_request} with %{link_to_merge_request_source_branch}").html_safe % { link_to_merge_request: link_to_merge_request, link_to_merge_request_source_branch: link_to_merge_request_source_branch } diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb index 9ef93b2387f..94e8b174f0f 100644 --- a/app/serializers/pipeline_entity.rb +++ b/app/serializers/pipeline_entity.rb @@ -2,6 +2,9 @@ class PipelineEntity < Grape::Entity include RequestAwareEntity + include Gitlab::Utils::StrongMemoize + + delegate :name, :failure_reason, to: :presented_pipeline expose :id expose :user, using: UserEntity @@ -36,6 +39,7 @@ class PipelineEntity < Grape::Entity expose :ordered_stages, as: :stages, using: StageEntity expose :duration expose :finished_at + expose :name end expose :merge_request, if: -> (*) { has_presentable_merge_request? }, with: MergeRequestForPipelineEntity do |pipeline| @@ -59,13 +63,11 @@ class PipelineEntity < Grape::Entity end expose :commit, using: CommitEntity + expose :merge_request_event_type, if: -> (pipeline, _) { pipeline.merge_request_event? } expose :source_sha, if: -> (pipeline, _) { pipeline.merge_request_pipeline? } expose :target_sha, if: -> (pipeline, _) { pipeline.merge_request_pipeline? } expose :yaml_errors, if: -> (pipeline, _) { pipeline.has_yaml_errors? } - - expose :failure_reason, if: -> (pipeline, _) { pipeline.failure_reason? } do |pipeline| - pipeline.present.failure_reason - end + expose :failure_reason, if: -> (pipeline, _) { pipeline.failure_reason? } expose :retry_path, if: -> (*) { can_retry? } do |pipeline| retry_project_pipeline_path(pipeline.project, pipeline) @@ -97,4 +99,10 @@ class PipelineEntity < Grape::Entity def detailed_status pipeline.detailed_status(request.current_user) end + + def presented_pipeline + strong_memoize(:presented_pipeline) do + pipeline.present + end + end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index dbf28db91dc..a5619079988 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8182,6 +8182,9 @@ msgstr "" msgid "Pipeline|Coverage" msgstr "" +msgid "Pipeline|Detached merge request pipeline" +msgstr "" + msgid "Pipeline|Duration" msgstr "" @@ -8191,6 +8194,12 @@ msgstr "" msgid "Pipeline|Key" msgstr "" +msgid "Pipeline|Merge train pipeline" +msgstr "" + +msgid "Pipeline|Merged result pipeline" +msgstr "" + msgid "Pipeline|Pipeline" msgstr "" diff --git a/spec/fixtures/api/schemas/pipeline.json b/spec/fixtures/api/schemas/pipeline.json index b6e30c40f13..d9ffad8fbab 100644 --- a/spec/fixtures/api/schemas/pipeline.json +++ b/spec/fixtures/api/schemas/pipeline.json @@ -97,6 +97,10 @@ "id": "/properties/details/properties/finished_at", "type": "string" }, + "name": { + "id": "/properties/details/properties/name", + "type": "string" + }, "manual_actions": { "id": "/properties/details/properties/manual_actions", "items": {}, @@ -323,6 +327,10 @@ "id": "/properties/web_url", "type": "string" }, + "merge_request_event_type": { + "id": "/properties/merge_request_event_type", + "type": "string" + }, "user": { "id": "/properties/user", "properties": { diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 7d84d094bdf..63ca383ac4b 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -323,6 +323,25 @@ describe Ci::Pipeline, :mailer do end end + describe '#merge_train_pipeline?' do + subject { pipeline.merge_train_pipeline? } + + let!(:pipeline) do + create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, ref: ref, target_sha: 'xxx') + end + + let(:merge_request) { create(:merge_request) } + let(:ref) { 'refs/merge-requests/1/train' } + + it { is_expected.to be_truthy } + + context 'when ref is merge ref' do + let(:ref) { 'refs/merge-requests/1/merge' } + + it { is_expected.to be_falsy } + end + end + describe '#merge_request_ref?' do subject { pipeline.merge_request_ref? } @@ -333,6 +352,48 @@ describe Ci::Pipeline, :mailer do end end + describe '#merge_train_ref?' do + subject { pipeline.merge_train_ref? } + + it 'calls Mergetrain#merge_train_ref?' do + expect(MergeRequest).to receive(:merge_train_ref?).with(pipeline.ref) + + subject + end + end + + describe '#merge_request_event_type' do + subject { pipeline.merge_request_event_type } + + before do + allow(pipeline).to receive(:merge_request_event?) { true } + end + + context 'when pipeline is merge train pipeline' do + before do + allow(pipeline).to receive(:merge_train_pipeline?) { true } + end + + it { is_expected.to eq(:merge_train) } + end + + context 'when pipeline is merge request pipeline' do + before do + allow(pipeline).to receive(:merge_request_pipeline?) { true } + end + + it { is_expected.to eq(:merged_result) } + end + + context 'when pipeline is detached merge request pipeline' do + before do + allow(pipeline).to receive(:detached_merge_request_pipeline?) { true } + end + + it { is_expected.to eq(:detached) } + end + end + describe '#legacy_detached_merge_request_pipeline?' do subject { pipeline.legacy_detached_merge_request_pipeline? } @@ -782,7 +843,8 @@ describe Ci::Pipeline, :mailer do 'CI_MERGE_REQUEST_TITLE' => merge_request.title, 'CI_MERGE_REQUEST_ASSIGNEES' => merge_request.assignee_username_list, 'CI_MERGE_REQUEST_MILESTONE' => milestone.title, - 'CI_MERGE_REQUEST_LABELS' => labels.map(&:title).join(',')) + 'CI_MERGE_REQUEST_LABELS' => labels.map(&:title).join(','), + 'CI_MERGE_REQUEST_EVENT_TYPE' => pipeline.merge_request_event_type.to_s) end context 'when source project does not exist' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d344a6d0f0d..11234982dd4 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3195,6 +3195,40 @@ describe MergeRequest do end end + describe '.merge_train_ref?' do + subject { described_class.merge_train_ref?(ref) } + + context 'when ref is ref name of a branch' do + let(:ref) { 'feature' } + + it { is_expected.to be_falsey } + end + + context 'when ref is HEAD ref path of a branch' do + let(:ref) { 'refs/heads/feature' } + + it { is_expected.to be_falsey } + end + + context 'when ref is HEAD ref path of a merge request' do + let(:ref) { 'refs/merge-requests/1/head' } + + it { is_expected.to be_falsey } + end + + context 'when ref is merge ref path of a merge request' do + let(:ref) { 'refs/merge-requests/1/merge' } + + it { is_expected.to be_falsey } + end + + context 'when ref is train ref path of a merge request' do + let(:ref) { 'refs/merge-requests/1/train' } + + it { is_expected.to be_truthy } + end + end + describe '#cleanup_refs' do subject { merge_request.cleanup_refs(only: only) } diff --git a/spec/presenters/ci/pipeline_presenter_spec.rb b/spec/presenters/ci/pipeline_presenter_spec.rb index cda07a0ae09..7e8bbedcf6d 100644 --- a/spec/presenters/ci/pipeline_presenter_spec.rb +++ b/spec/presenters/ci/pipeline_presenter_spec.rb @@ -77,6 +77,40 @@ describe Ci::PipelinePresenter do end end + describe '#name' do + subject { presenter.name } + + context 'when pipeline is detached merge request pipeline' do + let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) } + let(:pipeline) { merge_request.all_pipelines.last } + + it { is_expected.to eq('Detached merge request pipeline') } + end + + context 'when pipeline is merge request pipeline' do + let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) } + let(:pipeline) { merge_request.all_pipelines.last } + + it { is_expected.to eq('Merged result pipeline') } + end + + context 'when pipeline is merge train pipeline' do + let(:pipeline) { create(:ci_pipeline, project: project) } + + before do + allow(pipeline).to receive(:merge_request_event_type) { :merge_train } + end + + it { is_expected.to eq('Merge train pipeline') } + end + + context 'when pipeline is branch pipeline' do + let(:pipeline) { create(:ci_pipeline, project: project) } + + it { is_expected.to eq('Pipeline') } + end + end + describe '#ref_text' do subject { presenter.ref_text } diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb index 6be612ec226..eb9972d3e4d 100644 --- a/spec/serializers/pipeline_entity_spec.rb +++ b/spec/serializers/pipeline_entity_spec.rb @@ -41,7 +41,7 @@ describe PipelineEntity do it 'contains details' do expect(subject).to include :details expect(subject[:details]) - .to include :duration, :finished_at + .to include :duration, :finished_at, :name expect(subject[:details][:status]).to include :icon, :favicon, :text, :label, :tooltip end @@ -211,6 +211,10 @@ describe PipelineEntity do expect(subject[:source_sha]).to be_present expect(subject[:target_sha]).to be_present end + + it 'exposes merge request event type' do + expect(subject[:merge_request_event_type]).to be_present + end end end end