Commit 785b5f0f authored by Filipa Lacerda's avatar Filipa Lacerda

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
parents 1e8cd7f9 7e6b5749
<script> <script>
/* eslint-disable vue/require-default-prop */ /* eslint-disable vue/require-default-prop */
import { GlTooltipDirective, GlLink } from '@gitlab/ui';
import { sprintf, __ } from '~/locale'; import { sprintf, __ } from '~/locale';
import PipelineStage from '~/pipelines/components/stage.vue'; import PipelineStage from '~/pipelines/components/stage.vue';
import CiIcon from '~/vue_shared/components/ci_icon.vue'; import CiIcon from '~/vue_shared/components/ci_icon.vue';
...@@ -14,9 +15,13 @@ export default { ...@@ -14,9 +15,13 @@ export default {
CiIcon, CiIcon,
Icon, Icon,
TooltipOnTruncate, TooltipOnTruncate,
GlLink,
LinkedPipelinesMiniList: () => LinkedPipelinesMiniList: () =>
import('ee_component/vue_shared/components/linked_pipelines_mini_list.vue'), import('ee_component/vue_shared/components/linked_pipelines_mini_list.vue'),
}, },
directives: {
GlTooltip: GlTooltipDirective,
},
mixins: [mrWidgetPipelineMixin], mixins: [mrWidgetPipelineMixin],
props: { props: {
pipeline: { pipeline: {
...@@ -78,12 +83,18 @@ export default { ...@@ -78,12 +83,18 @@ export default {
false, false,
); );
}, },
isTriggeredByMergeRequest() {
return Boolean(this.pipeline.merge_request);
},
isMergeRequestPipeline() {
return Boolean(this.pipeline.flags && this.pipeline.flags.merge_request_pipeline);
},
}, },
}; };
</script> </script>
<template> <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"> <template v-if="hasCIError">
<div <div
class="add-border ci-status-icon ci-status-icon-failed ci-error js-ci-error append-right-default" class="add-border ci-status-icon ci-status-icon-failed ci-error js-ci-error append-right-default"
...@@ -99,21 +110,58 @@ export default { ...@@ -99,21 +110,58 @@ export default {
<div class="ci-widget-container d-flex"> <div class="ci-widget-container d-flex">
<div class="ci-widget-content"> <div class="ci-widget-content">
<div class="media-body"> <div class="media-body">
<div class="font-weight-bold"> <div class="font-weight-bold js-pipeline-info-container">
Pipeline {{ s__('Pipeline|Pipeline') }}
<a :href="pipeline.path" class="pipeline-id font-weight-normal pipeline-number" <gl-link :href="pipeline.path" class="pipeline-id font-weight-normal pipeline-number"
>#{{ pipeline.id }}</a >#{{ pipeline.id }}</gl-link
> >
{{ pipeline.details.status.label }} {{ pipeline.details.status.label }}
<template v-if="hasCommitInfo"> <template v-if="hasCommitInfo">
for {{ s__('Pipeline|for') }}
<a <gl-link
:href="pipeline.commit.commit_path" :href="pipeline.commit.commit_path"
class="commit-sha js-commit-link font-weight-normal" 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 <tooltip-on-truncate
v-else
:title="sourceBranch" :title="sourceBranch"
truncate-target="child" truncate-target="child"
class="label-branch label-truncate" class="label-branch label-truncate"
...@@ -121,7 +169,9 @@ export default { ...@@ -121,7 +169,9 @@ export default {
/> />
</template> </template>
</div> </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> </div>
<div> <div>
......
...@@ -746,6 +746,10 @@ module Ci ...@@ -746,6 +746,10 @@ module Ci
triggered_by_merge_request? && target_sha == merge_request.target_branch_sha triggered_by_merge_request? && target_sha == merge_request.target_branch_sha
end end
def matches_sha_or_source_sha?(sha)
self.sha == sha || self.source_sha == sha
end
private private
def ci_yaml_from_repo def ci_yaml_from_repo
......
...@@ -232,7 +232,7 @@ class MergeRequest < ActiveRecord::Base ...@@ -232,7 +232,7 @@ class MergeRequest < ActiveRecord::Base
# branch head commit, for example checking if a merge request can be merged. # 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 # For more information check: https://gitlab.com/gitlab-org/gitlab-ce/issues/40004
def actual_head_pipeline 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 end
def merge_pipeline def merge_pipeline
......
---
title: Update pipeline block on merge request page to accommodate post-merge pipeline
information
merge_request: 25745
author:
type: added
...@@ -5791,6 +5791,9 @@ msgstr "" ...@@ -5791,6 +5791,9 @@ msgstr ""
msgid "Pipeline|Commit" msgid "Pipeline|Commit"
msgstr "" msgstr ""
msgid "Pipeline|Coverage"
msgstr ""
msgid "Pipeline|Create for" msgid "Pipeline|Create for"
msgstr "" msgstr ""
...@@ -5836,9 +5839,21 @@ msgstr "" ...@@ -5836,9 +5839,21 @@ msgstr ""
msgid "Pipeline|all" msgid "Pipeline|all"
msgstr "" msgstr ""
msgid "Pipeline|for"
msgstr ""
msgid "Pipeline|into"
msgstr ""
msgid "Pipeline|on"
msgstr ""
msgid "Pipeline|success" msgid "Pipeline|success"
msgstr "" msgstr ""
msgid "Pipeline|with"
msgstr ""
msgid "Pipeline|with stage" msgid "Pipeline|with stage"
msgstr "" msgstr ""
......
...@@ -145,6 +145,119 @@ describe 'Merge request > User sees merge widget', :js do ...@@ -145,6 +145,119 @@ describe 'Merge request > User sees merge widget', :js do
end end
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 context 'view merge request with MWBS button' do
before do before do
commit_status = create(:commit_status, project: project, status: 'pending') commit_status = create(:commit_status, project: project, status: 'pending')
......
import Vue from 'vue'; import Vue from 'vue';
import pipelineComponent from '~/vue_merge_request_widget/components/mr_widget_pipeline.vue'; import pipelineComponent from '~/vue_merge_request_widget/components/mr_widget_pipeline.vue';
import mountComponent from 'spec/helpers/vue_mount_component_helper'; import mountComponent from 'spec/helpers/vue_mount_component_helper';
import { trimText } from 'spec/helpers/vue_component_helper';
import mockData from '../mock_data'; import mockData from '../mock_data';
describe('MRWidgetPipeline', () => { describe('MRWidgetPipeline', () => {
...@@ -123,7 +124,7 @@ describe('MRWidgetPipeline', () => { ...@@ -123,7 +124,7 @@ describe('MRWidgetPipeline', () => {
describe('without commit path', () => { describe('without commit path', () => {
beforeEach(() => { beforeEach(() => {
const mockCopy = Object.assign({}, mockData); const mockCopy = JSON.parse(JSON.stringify(mockData));
delete mockCopy.pipeline.commit; delete mockCopy.pipeline.commit;
vm = mountComponent(Component, { vm = mountComponent(Component, {
...@@ -164,7 +165,7 @@ describe('MRWidgetPipeline', () => { ...@@ -164,7 +165,7 @@ describe('MRWidgetPipeline', () => {
describe('without coverage', () => { describe('without coverage', () => {
it('should not render a coverage', () => { it('should not render a coverage', () => {
const mockCopy = Object.assign({}, mockData); const mockCopy = JSON.parse(JSON.stringify(mockData));
delete mockCopy.pipeline.coverage; delete mockCopy.pipeline.coverage;
vm = mountComponent(Component, { vm = mountComponent(Component, {
...@@ -180,7 +181,7 @@ describe('MRWidgetPipeline', () => { ...@@ -180,7 +181,7 @@ describe('MRWidgetPipeline', () => {
describe('without a pipeline graph', () => { describe('without a pipeline graph', () => {
it('should not render 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; delete mockCopy.pipeline.details.stages;
vm = mountComponent(Component, { vm = mountComponent(Component, {
...@@ -193,5 +194,81 @@ describe('MRWidgetPipeline', () => { ...@@ -193,5 +194,81 @@ describe('MRWidgetPipeline', () => {
expect(vm.$el.querySelector('.js-mini-pipeline-graph')).toEqual(null); 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);
});
});
}); });
}); });
...@@ -134,6 +134,8 @@ export default { ...@@ -134,6 +134,8 @@ export default {
yaml_errors: false, yaml_errors: false,
retryable: true, retryable: true,
cancelable: false, cancelable: false,
merge_request_pipeline: false,
detached_merge_request_pipeline: true,
}, },
ref: { ref: {
name: 'daaaa', name: 'daaaa',
...@@ -141,6 +143,15 @@ export default { ...@@ -141,6 +143,15 @@ export default {
tag: false, tag: false,
branch: true, 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: { commit: {
id: '104096c51715e12e7ae41f9333e9fa35b73f385d', id: '104096c51715e12e7ae41f9333e9fa35b73f385d',
short_id: '104096c5', short_id: '104096c5',
......
...@@ -362,6 +362,30 @@ describe Ci::Pipeline, :mailer do ...@@ -362,6 +362,30 @@ describe Ci::Pipeline, :mailer do
end end
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 describe '.triggered_for_branch' do
subject { described_class.triggered_for_branch(ref) } subject { described_class.triggered_for_branch(ref) }
......
...@@ -64,8 +64,8 @@ describe EnvironmentStatus do ...@@ -64,8 +64,8 @@ describe EnvironmentStatus do
end end
describe '.for_merge_request' do describe '.for_merge_request' do
let(:admin) { create(:admin) } let(:admin) { create(:admin) }
let(:pipeline) { create(:ci_pipeline, sha: sha) } let!(:pipeline) { create(:ci_pipeline, sha: sha, merge_requests_as_head_pipeline: [merge_request]) }
it 'is based on merge_request.diff_head_sha' do it 'is based on merge_request.diff_head_sha' do
expect(merge_request).to receive(:diff_head_sha) expect(merge_request).to receive(:diff_head_sha)
......
...@@ -1187,8 +1187,10 @@ describe MergeRequest do ...@@ -1187,8 +1187,10 @@ describe MergeRequest do
end end
context 'head pipeline' do context 'head pipeline' do
let(:diff_head_sha) { Digest::SHA1.hexdigest(SecureRandom.hex) }
before do 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 end
describe '#head_pipeline' do describe '#head_pipeline' do
...@@ -1216,7 +1218,15 @@ describe MergeRequest do ...@@ -1216,7 +1218,15 @@ describe MergeRequest do
end end
it 'returns the pipeline for MR with recent pipeline' do 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) 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(subject.head_pipeline)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment