From e8959ec75bfa59f077247b50f066cc58ae754568 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 29 Mar 2019 19:26:38 +0700 Subject: [PATCH] Expose parameters of merge request pipelines for MR widget Add changelog ok add ok Simplify Add some spec remove unnecessary spec Add spec --- app/models/ci/pipeline.rb | 8 ---- .../merge_request_widget_entity.rb | 1 + app/serializers/pipeline_entity.rb | 2 + ee/app/models/ee/project.rb | 4 ++ .../ee/merge_request_widget_entity.rb | 4 ++ ...r-pipeline-parameters-for-merge-widget.yml | 5 +++ ee/spec/models/project_spec.rb | 39 +++++++++++++++++ spec/models/ci/pipeline_spec.rb | 42 ------------------- .../merge_request_widget_entity_spec.rb | 4 ++ spec/serializers/pipeline_entity_spec.rb | 10 +++++ 10 files changed, 69 insertions(+), 50 deletions(-) create mode 100644 ee/changelogs/unreleased/expose-mr-pipeline-parameters-for-merge-widget.yml diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 4034c100040de0..fb2a3d2536e94f 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -206,10 +206,6 @@ class Pipeline < ApplicationRecord triggered_by_merge_request(merge_request).for_source_sha(source_sha) end - scope :mergeable_merge_request_pipelines, -> (merge_request) do - triggered_by_merge_request(merge_request).where(target_sha: merge_request.target_branch_sha) - end - scope :triggered_for_branch, -> (ref) do where(source: branch_pipeline_sources).where(ref: ref, tag: false) end @@ -746,10 +742,6 @@ def merge_request_pipeline? triggered_by_merge_request? && target_sha.present? end - def mergeable_merge_request_pipeline? - triggered_by_merge_request? && target_sha == merge_request.target_branch_sha - end - def merge_request_ref? MergeRequest.merge_request_ref?(ref) end diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index a1ffd9c706de03..685459690092f5 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -22,6 +22,7 @@ class MergeRequestWidgetEntity < IssuableEntity end expose :squash expose :target_branch + expose :target_branch_sha expose :target_project_id expose :target_project_full_path do |merge_request| merge_request.project&.full_path diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb index fba724102171ee..8fe5df81e6c7ce 100644 --- a/app/serializers/pipeline_entity.rb +++ b/app/serializers/pipeline_entity.rb @@ -59,6 +59,8 @@ class PipelineEntity < Grape::Entity end expose :commit, using: CommitEntity + 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| diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 84a68e063aa0c3..d74f5695b49eac 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -531,6 +531,10 @@ def protected_environments_feature_available? feature_available?(:protected_environments) end + def merge_pipelines_enabled? + feature_available?(:merge_pipelines) && super + end + # Because we use default_value_for we need to be sure # packages_enabled= method does exist even if we rollback migration. # Otherwise many tests from spec/migrations will fail. diff --git a/ee/app/serializers/ee/merge_request_widget_entity.rb b/ee/app/serializers/ee/merge_request_widget_entity.rb index 479f0bf4b20491..5944d7923329bd 100644 --- a/ee/app/serializers/ee/merge_request_widget_entity.rb +++ b/ee/app/serializers/ee/merge_request_widget_entity.rb @@ -119,6 +119,10 @@ module MergeRequestWidgetEntity expose :rebase_commit_sha expose :rebase_in_progress?, as: :rebase_in_progress + expose :merge_pipelines_enabled?, as: :merge_pipelines_enabled do |merge_request| + merge_request.target_project.merge_pipelines_enabled? + end + expose :can_push_to_source_branch do |merge_request| presenter(merge_request).can_push_to_source_branch? end diff --git a/ee/changelogs/unreleased/expose-mr-pipeline-parameters-for-merge-widget.yml b/ee/changelogs/unreleased/expose-mr-pipeline-parameters-for-merge-widget.yml new file mode 100644 index 00000000000000..d230db2d4cb48a --- /dev/null +++ b/ee/changelogs/unreleased/expose-mr-pipeline-parameters-for-merge-widget.yml @@ -0,0 +1,5 @@ +--- +title: Expose merge request pipeline parameters for MR widget +merge_request: 10502 +author: +type: added diff --git a/ee/spec/models/project_spec.rb b/ee/spec/models/project_spec.rb index be43a6073b04b8..a16ad6eeaf00a4 100644 --- a/ee/spec/models/project_spec.rb +++ b/ee/spec/models/project_spec.rb @@ -1735,6 +1735,45 @@ def stub_find_remote_root_ref(project, ref:) end end + describe '#merge_pipelines_enabled?' do + subject { project.merge_pipelines_enabled? } + + let(:project) { create(:project) } + let(:merge_pipelines_enabled) { true } + + before do + project.merge_pipelines_enabled = merge_pipelines_enabled + end + + context 'when Merge pipelines (EEP) is available' do + before do + stub_licensed_features(merge_pipelines: true) + end + + it { is_expected.to be_truthy } + + context 'when project setting is disabled' do + let(:merge_pipelines_enabled) { false } + + it { is_expected.to be_falsy } + end + end + + context 'when Merge pipelines (EEP) is unavailable' do + before do + stub_licensed_features(merge_pipelines: false) + end + + it { is_expected.to be_falsy } + + context 'when project setting is disabled' do + let(:merge_pipelines_enabled) { false } + + it { is_expected.to be_falsy } + end + end + end + describe "#insights_config" do context 'when project has no Insights config file' do it 'returns nil' do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index c1eba3bde99dd7..1b080d631e33ba 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -326,48 +326,6 @@ end end - describe '.mergeable_merge_request_pipelines' do - subject { described_class.mergeable_merge_request_pipelines(merge_request) } - - let!(:pipeline) do - create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, target_sha: target_sha) - end - - let(:merge_request) { create(:merge_request) } - let(:target_sha) { merge_request.target_branch_sha } - - it 'returns mergeable merge pipelines' do - is_expected.to eq([pipeline]) - end - - context 'when target sha does not point the head of the target branch' do - let(:target_sha) { merge_request.diff_head_sha } - - it 'returns empty array' do - is_expected.to be_empty - end - end - end - - describe '#mergeable_merge_request_pipeline?' do - subject { pipeline.mergeable_merge_request_pipeline? } - - let!(:pipeline) do - create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, target_sha: target_sha) - end - - let(:merge_request) { create(:merge_request) } - let(:target_sha) { merge_request.target_branch_sha } - - it { is_expected.to be_truthy } - - context 'when target sha does not point the head of the target branch' do - let(:target_sha) { merge_request.diff_head_sha } - - it { is_expected.to be_falsy } - end - end - describe '#merge_request_ref?' do subject { pipeline.merge_request_ref? } diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb index 727fd8951f29af..0e99ef38d2f341 100644 --- a/spec/serializers/merge_request_widget_entity_spec.rb +++ b/spec/serializers/merge_request_widget_entity_spec.rb @@ -13,6 +13,10 @@ described_class.new(resource, request: request).as_json end + it 'has the latest sha of the target branch' do + is_expected.to include(:target_branch_sha) + end + describe 'source_project_full_path' do it 'includes the full path of the source project' do expect(subject[:source_project_full_path]).to be_present diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb index 1d992e8a483a78..dba7fd9174750b 100644 --- a/spec/serializers/pipeline_entity_spec.rb +++ b/spec/serializers/pipeline_entity_spec.rb @@ -143,6 +143,11 @@ expect(subject[:flags][:detached_merge_request_pipeline]).to be_truthy end + it 'does not expose source sha and target sha' do + expect(subject[:source_sha]).to be_nil + expect(subject[:target_sha]).to be_nil + end + context 'when user is a developer' do before do project.add_developer(user) @@ -189,6 +194,11 @@ it 'makes atached flag true' do expect(subject[:flags][:merge_request_pipeline]).to be_truthy end + + it 'exposes source sha and target sha' do + expect(subject[:source_sha]).to be_present + expect(subject[:target_sha]).to be_present + end end end end -- GitLab