diff options
author | Filipa Lacerda <filipa@gitlab.com> | 2019-03-21 15:31:10 +0000 |
---|---|---|
committer | Filipa Lacerda <filipa@gitlab.com> | 2019-03-21 15:31:10 +0000 |
commit | 785b5f0f3bb9737f177956113bf63d3c94072db8 (patch) | |
tree | 79c320971de9ff6e4e17260035a04b6457413833 | |
parent | 1e8cd7f9d9cbb53614f9ba5aee00e67e56a4851e (diff) | |
parent | 7e6b57499f838245136e0657069f7559b1eba02f (diff) | |
download | gitlab-ce-785b5f0f3bb9737f177956113bf63d3c94072db8.tar.gz |
Merge branch 'nfriend-update-merge-request-widget-pipeline-block' into 'master'
Update merge request widget pipeline block to accommodate post-merge pipelines
See merge request gitlab-org/gitlab-ce!25745
-rw-r--r-- | app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue | 70 | ||||
-rw-r--r-- | app/models/ci/pipeline.rb | 4 | ||||
-rw-r--r-- | app/models/merge_request.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/nfriend-update-merge-request-widget-pipeline-block.yml | 6 | ||||
-rw-r--r-- | locale/gitlab.pot | 15 | ||||
-rw-r--r-- | spec/features/merge_request/user_sees_merge_widget_spec.rb | 113 | ||||
-rw-r--r-- | spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js | 83 | ||||
-rw-r--r-- | spec/javascripts/vue_mr_widget/mock_data.js | 11 | ||||
-rw-r--r-- | spec/models/ci/pipeline_spec.rb | 24 | ||||
-rw-r--r-- | spec/models/environment_status_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 14 |
11 files changed, 328 insertions, 18 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue index 9e63aa00341..f5a1ff2f6fd 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue @@ -1,5 +1,6 @@ <script> /* eslint-disable vue/require-default-prop */ +import { GlTooltipDirective, GlLink } from '@gitlab/ui'; import { sprintf, __ } from '~/locale'; import PipelineStage from '~/pipelines/components/stage.vue'; import CiIcon from '~/vue_shared/components/ci_icon.vue'; @@ -14,9 +15,13 @@ export default { CiIcon, Icon, TooltipOnTruncate, + GlLink, LinkedPipelinesMiniList: () => import('ee_component/vue_shared/components/linked_pipelines_mini_list.vue'), }, + directives: { + GlTooltip: GlTooltipDirective, + }, mixins: [mrWidgetPipelineMixin], props: { pipeline: { @@ -78,12 +83,18 @@ export default { false, ); }, + isTriggeredByMergeRequest() { + return Boolean(this.pipeline.merge_request); + }, + isMergeRequestPipeline() { + return Boolean(this.pipeline.flags && this.pipeline.flags.merge_request_pipeline); + }, }, }; </script> <template> - <div v-if="hasPipeline || hasCIError" class="ci-widget media"> + <div v-if="hasPipeline || hasCIError" class="ci-widget media js-ci-widget"> <template v-if="hasCIError"> <div class="add-border ci-status-icon ci-status-icon-failed ci-error js-ci-error append-right-default" @@ -99,21 +110,58 @@ export default { <div class="ci-widget-container d-flex"> <div class="ci-widget-content"> <div class="media-body"> - <div class="font-weight-bold"> - Pipeline - <a :href="pipeline.path" class="pipeline-id font-weight-normal pipeline-number" - >#{{ pipeline.id }}</a + <div class="font-weight-bold js-pipeline-info-container"> + {{ s__('Pipeline|Pipeline') }} + <gl-link :href="pipeline.path" class="pipeline-id font-weight-normal pipeline-number" + >#{{ pipeline.id }}</gl-link > {{ pipeline.details.status.label }} <template v-if="hasCommitInfo"> - for - <a + {{ s__('Pipeline|for') }} + <gl-link :href="pipeline.commit.commit_path" class="commit-sha js-commit-link font-weight-normal" - >{{ pipeline.commit.short_id }}</a + >{{ pipeline.commit.short_id }}</gl-link > - on + {{ s__('Pipeline|on') }} + <template v-if="isTriggeredByMergeRequest"> + <gl-link + v-gl-tooltip + :href="pipeline.merge_request.path" + :title="pipeline.merge_request.title" + class="font-weight-normal" + >!{{ pipeline.merge_request.iid }}</gl-link + > + {{ s__('Pipeline|with') }} + <tooltip-on-truncate + :title="pipeline.merge_request.source_branch" + truncate-target="child" + class="label-branch label-truncate" + > + <gl-link + :href="pipeline.merge_request.source_branch_path" + class="font-weight-normal" + >{{ pipeline.merge_request.source_branch }}</gl-link + > + </tooltip-on-truncate> + + <template v-if="isMergeRequestPipeline"> + {{ s__('Pipeline|into') }} + <tooltip-on-truncate + :title="pipeline.merge_request.target_branch" + truncate-target="child" + class="label-branch label-truncate" + > + <gl-link + :href="pipeline.merge_request.target_branch_path" + class="font-weight-normal" + >{{ pipeline.merge_request.target_branch }}</gl-link + > + </tooltip-on-truncate> + </template> + </template> <tooltip-on-truncate + v-else :title="sourceBranch" truncate-target="child" class="label-branch label-truncate" @@ -121,7 +169,9 @@ export default { /> </template> </div> - <div v-if="pipeline.coverage" class="coverage">Coverage {{ pipeline.coverage }}%</div> + <div v-if="pipeline.coverage" class="coverage"> + {{ s__('Pipeline|Coverage') }} {{ pipeline.coverage }}% + </div> </div> </div> <div> diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index ae74f569415..826b3f82bbf 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -746,6 +746,10 @@ module Ci triggered_by_merge_request? && target_sha == merge_request.target_branch_sha end + def matches_sha_or_source_sha?(sha) + self.sha == sha || self.source_sha == sha + end + private def ci_yaml_from_repo diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index aeb6acf0ac0..5f6d5095bcc 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -232,7 +232,7 @@ class MergeRequest < ActiveRecord::Base # branch head commit, for example checking if a merge request can be merged. # For more information check: https://gitlab.com/gitlab-org/gitlab-ce/issues/40004 def actual_head_pipeline - head_pipeline&.sha == diff_head_sha ? head_pipeline : nil + head_pipeline&.matches_sha_or_source_sha?(diff_head_sha) ? head_pipeline : nil end def merge_pipeline diff --git a/changelogs/unreleased/nfriend-update-merge-request-widget-pipeline-block.yml b/changelogs/unreleased/nfriend-update-merge-request-widget-pipeline-block.yml new file mode 100644 index 00000000000..bd4120eb06f --- /dev/null +++ b/changelogs/unreleased/nfriend-update-merge-request-widget-pipeline-block.yml @@ -0,0 +1,6 @@ +--- +title: Update pipeline block on merge request page to accommodate post-merge pipeline + information +merge_request: 25745 +author: +type: added diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5ab7de81bb6..fe607e36808 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5791,6 +5791,9 @@ msgstr "" msgid "Pipeline|Commit" msgstr "" +msgid "Pipeline|Coverage" +msgstr "" + msgid "Pipeline|Create for" msgstr "" @@ -5836,9 +5839,21 @@ msgstr "" msgid "Pipeline|all" msgstr "" +msgid "Pipeline|for" +msgstr "" + +msgid "Pipeline|into" +msgstr "" + +msgid "Pipeline|on" +msgstr "" + msgid "Pipeline|success" msgstr "" +msgid "Pipeline|with" +msgstr "" + msgid "Pipeline|with stage" msgstr "" diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb index afb978d7c45..2609546990d 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -145,6 +145,119 @@ describe 'Merge request > User sees merge widget', :js do end end + context 'when merge request has a branch pipeline as the head pipeline' do + let!(:pipeline) do + create(:ci_pipeline, + ref: merge_request.source_branch, + sha: merge_request.source_branch_sha, + project: merge_request.source_project) + end + + before do + merge_request.update_head_pipeline + visit project_merge_request_path(project, merge_request) + end + + it 'shows head pipeline information' do + within '.ci-widget-content' do + expect(page).to have_content("Pipeline ##{pipeline.id} pending " \ + "for #{pipeline.short_sha} " \ + "on #{pipeline.ref}") + end + end + end + + context 'when merge request has a detached merge request pipeline as the head pipeline' do + let(:merge_request) do + create(:merge_request, + :with_detached_merge_request_pipeline, + source_project: source_project, + target_project: target_project) + end + + let!(:pipeline) do + merge_request.all_pipelines.last + end + + let(:source_project) { project } + let(:target_project) { project } + + before do + merge_request.update_head_pipeline + visit project_merge_request_path(project, merge_request) + end + + it 'shows head pipeline information' do + within '.ci-widget-content' do + expect(page).to have_content("Pipeline ##{pipeline.id} pending " \ + "for #{pipeline.short_sha} " \ + "on #{merge_request.to_reference} " \ + "with #{merge_request.source_branch}") + end + end + + context 'when source project is a forked project' do + let(:source_project) { fork_project(project, user, repository: true) } + + it 'shows head pipeline information' do + within '.ci-widget-content' do + expect(page).to have_content("Pipeline ##{pipeline.id} pending " \ + "for #{pipeline.short_sha} " \ + "on #{merge_request.to_reference} " \ + "with #{merge_request.source_branch}") + end + end + end + end + + context 'when merge request has a merge request pipeline as the head pipeline' do + let(:merge_request) do + create(:merge_request, + :with_merge_request_pipeline, + source_project: source_project, + target_project: target_project, + merge_sha: merge_sha) + end + + let!(:pipeline) do + merge_request.all_pipelines.last + end + + let(:source_project) { project } + let(:target_project) { project } + let(:merge_sha) { project.commit.sha } + + before do + merge_request.update_head_pipeline + visit project_merge_request_path(project, merge_request) + end + + it 'shows head pipeline information' do + within '.ci-widget-content' do + expect(page).to have_content("Pipeline ##{pipeline.id} pending " \ + "for #{pipeline.short_sha} " \ + "on #{merge_request.to_reference} " \ + "with #{merge_request.source_branch} " \ + "into #{merge_request.target_branch}") + end + end + + context 'when source project is a forked project' do + let(:source_project) { fork_project(project, user, repository: true) } + let(:merge_sha) { source_project.commit.sha } + + it 'shows head pipeline information' do + within '.ci-widget-content' do + expect(page).to have_content("Pipeline ##{pipeline.id} pending " \ + "for #{pipeline.short_sha} " \ + "on #{merge_request.to_reference} " \ + "with #{merge_request.source_branch} " \ + "into #{merge_request.target_branch}") + end + end + end + end + context 'view merge request with MWBS button' do before do commit_status = create(:commit_status, project: project, status: 'pending') diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js index d905bbe4040..de213210cfc 100644 --- a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js @@ -1,6 +1,7 @@ import Vue from 'vue'; import pipelineComponent from '~/vue_merge_request_widget/components/mr_widget_pipeline.vue'; import mountComponent from 'spec/helpers/vue_mount_component_helper'; +import { trimText } from 'spec/helpers/vue_component_helper'; import mockData from '../mock_data'; describe('MRWidgetPipeline', () => { @@ -123,7 +124,7 @@ describe('MRWidgetPipeline', () => { describe('without commit path', () => { beforeEach(() => { - const mockCopy = Object.assign({}, mockData); + const mockCopy = JSON.parse(JSON.stringify(mockData)); delete mockCopy.pipeline.commit; vm = mountComponent(Component, { @@ -164,7 +165,7 @@ describe('MRWidgetPipeline', () => { describe('without coverage', () => { it('should not render a coverage', () => { - const mockCopy = Object.assign({}, mockData); + const mockCopy = JSON.parse(JSON.stringify(mockData)); delete mockCopy.pipeline.coverage; vm = mountComponent(Component, { @@ -180,7 +181,7 @@ describe('MRWidgetPipeline', () => { describe('without a pipeline graph', () => { it('should not render a pipeline graph', () => { - const mockCopy = Object.assign({}, mockData); + const mockCopy = JSON.parse(JSON.stringify(mockData)); delete mockCopy.pipeline.details.stages; vm = mountComponent(Component, { @@ -193,5 +194,81 @@ describe('MRWidgetPipeline', () => { expect(vm.$el.querySelector('.js-mini-pipeline-graph')).toEqual(null); }); }); + + describe('without pipeline.merge_request', () => { + it('should render info that includes the commit and branch details', () => { + const mockCopy = JSON.parse(JSON.stringify(mockData)); + delete mockCopy.pipeline.merge_request; + const { pipeline } = mockCopy; + + vm = mountComponent(Component, { + pipeline, + hasCi: true, + ciStatus: 'success', + troubleshootingDocsPath: 'help', + sourceBranchLink: mockCopy.source_branch_link, + }); + + const expected = `Pipeline #${pipeline.id} ${pipeline.details.status.label} for ${ + pipeline.commit.short_id + } on ${mockCopy.source_branch_link}`; + + const actual = trimText(vm.$el.querySelector('.js-pipeline-info-container').innerText); + + expect(actual).toBe(expected); + }); + }); + + describe('with pipeline.merge_request and flags.merge_request_pipeline', () => { + it('should render info that includes the commit, MR, source branch, and target branch details', () => { + const mockCopy = JSON.parse(JSON.stringify(mockData)); + const { pipeline } = mockCopy; + pipeline.flags.merge_request_pipeline = true; + pipeline.flags.detached_merge_request_pipeline = false; + + vm = mountComponent(Component, { + pipeline, + hasCi: true, + ciStatus: 'success', + troubleshootingDocsPath: 'help', + sourceBranchLink: mockCopy.source_branch_link, + }); + + const expected = `Pipeline #${pipeline.id} ${pipeline.details.status.label} for ${ + pipeline.commit.short_id + } on !${pipeline.merge_request.iid} with ${pipeline.merge_request.source_branch} into ${ + pipeline.merge_request.target_branch + }`; + + const actual = trimText(vm.$el.querySelector('.js-pipeline-info-container').innerText); + + expect(actual).toBe(expected); + }); + }); + + describe('with pipeline.merge_request and flags.detached_merge_request_pipeline', () => { + it('should render info that includes the commit, MR, and source branch details', () => { + const mockCopy = JSON.parse(JSON.stringify(mockData)); + const { pipeline } = mockCopy; + pipeline.flags.merge_request_pipeline = false; + pipeline.flags.detached_merge_request_pipeline = true; + + vm = mountComponent(Component, { + pipeline, + hasCi: true, + ciStatus: 'success', + troubleshootingDocsPath: 'help', + sourceBranchLink: mockCopy.source_branch_link, + }); + + const expected = `Pipeline #${pipeline.id} ${pipeline.details.status.label} for ${ + pipeline.commit.short_id + } on !${pipeline.merge_request.iid} with ${pipeline.merge_request.source_branch}`; + + const actual = trimText(vm.$el.querySelector('.js-pipeline-info-container').innerText); + + expect(actual).toBe(expected); + }); + }); }); }); diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index 6ef07f81705..7ab203a6011 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -134,6 +134,8 @@ export default { yaml_errors: false, retryable: true, cancelable: false, + merge_request_pipeline: false, + detached_merge_request_pipeline: true, }, ref: { name: 'daaaa', @@ -141,6 +143,15 @@ export default { tag: false, branch: true, }, + merge_request: { + iid: 1, + path: '/root/detached-merge-request-pipelines/merge_requests/1', + title: 'Update README.md', + source_branch: 'feature-1', + source_branch_path: '/root/detached-merge-request-pipelines/branches/feature-1', + target_branch: 'master', + target_branch_path: '/root/detached-merge-request-pipelines/branches/master', + }, commit: { id: '104096c51715e12e7ae41f9333e9fa35b73f385d', short_id: '104096c5', diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 2ac056f63b2..5b8097621e0 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -362,6 +362,30 @@ describe Ci::Pipeline, :mailer do end end + describe '#matches_sha_or_source_sha?' do + subject { pipeline.matches_sha_or_source_sha?(sample_sha) } + + let(:sample_sha) { Digest::SHA1.hexdigest(SecureRandom.hex) } + + context 'when sha matches' do + let(:pipeline) { build(:ci_pipeline, sha: sample_sha) } + + it { is_expected.to be_truthy } + end + + context 'when source_sha matches' do + let(:pipeline) { build(:ci_pipeline, source_sha: sample_sha) } + + it { is_expected.to be_truthy } + end + + context 'when both sha and source_sha do not matche' do + let(:pipeline) { build(:ci_pipeline, sha: 'test', source_sha: 'test') } + + it { is_expected.to be_falsy } + end + end + describe '.triggered_for_branch' do subject { described_class.triggered_for_branch(ref) } diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index 9da16dea929..2576a9aba06 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -64,8 +64,8 @@ describe EnvironmentStatus do end describe '.for_merge_request' do - let(:admin) { create(:admin) } - let(:pipeline) { create(:ci_pipeline, sha: sha) } + let(:admin) { create(:admin) } + let!(:pipeline) { create(:ci_pipeline, sha: sha, merge_requests_as_head_pipeline: [merge_request]) } it 'is based on merge_request.diff_head_sha' do expect(merge_request).to receive(:diff_head_sha) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 42c49e330cc..22998bc5b6a 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1187,8 +1187,10 @@ describe MergeRequest do end context 'head pipeline' do + let(:diff_head_sha) { Digest::SHA1.hexdigest(SecureRandom.hex) } + before do - allow(subject).to receive(:diff_head_sha).and_return('lastsha') + allow(subject).to receive(:diff_head_sha).and_return(diff_head_sha) end describe '#head_pipeline' do @@ -1216,7 +1218,15 @@ describe MergeRequest do end it 'returns the pipeline for MR with recent pipeline' do - pipeline = create(:ci_empty_pipeline, sha: 'lastsha') + pipeline = create(:ci_empty_pipeline, sha: diff_head_sha) + subject.update_attribute(:head_pipeline_id, pipeline.id) + + expect(subject.actual_head_pipeline).to eq(subject.head_pipeline) + expect(subject.actual_head_pipeline).to eq(pipeline) + end + + it 'returns the pipeline for MR with recent merge request pipeline' do + pipeline = create(:ci_empty_pipeline, sha: 'merge-sha', source_sha: diff_head_sha) subject.update_attribute(:head_pipeline_id, pipeline.id) expect(subject.actual_head_pipeline).to eq(subject.head_pipeline) |