Commit 11f1305e authored by Nathan Friend's avatar Nathan Friend

Make MR pipeline widget text more descriptive (CE)

This change updates the text of the pipeline widget that appears on the
merge request page. The text has been made more consistent between
different types of pipelines; this makes the front-end implementation
simpler and more maintainable.  In addition, the type of pipeline is
(i.e. regular pipeline, merge request pipeline, detached pipeline)
included in the text, making this type more obvious to the end user.

Some information has been removed from the widget as part of this
change; however, any information that was removed already appears
elsewhere on the merge request page.
parent 1ba9f029
<script> <script>
/* eslint-disable vue/require-default-prop */ /* eslint-disable vue/require-default-prop */
import { GlTooltipDirective, GlLink } from '@gitlab/ui'; import { GlTooltipDirective, GlLink } from '@gitlab/ui';
import { sprintf, __ } from '~/locale'; import { sprintf, s__ } 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';
import Icon from '~/vue_shared/components/icon.vue'; import Icon from '~/vue_shared/components/icon.vue';
...@@ -73,8 +73,8 @@ export default { ...@@ -73,8 +73,8 @@ export default {
}, },
errorText() { errorText() {
return sprintf( return sprintf(
__( s__(
'Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation.%{linkEnd}', 'Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation.%{linkEnd}',
), ),
{ {
linkStart: `<a href="${this.troubleshootingDocsPath}">`, linkStart: `<a href="${this.troubleshootingDocsPath}">`,
...@@ -89,6 +89,9 @@ export default { ...@@ -89,6 +89,9 @@ export default {
isMergeRequestPipeline() { isMergeRequestPipeline() {
return Boolean(this.pipeline.flags && this.pipeline.flags.merge_request_pipeline); return Boolean(this.pipeline.flags && this.pipeline.flags.merge_request_pipeline);
}, },
showSourceBranch() {
return Boolean(this.pipeline.ref.branch);
},
}, },
}; };
</script> </script>
...@@ -109,7 +112,7 @@ export default { ...@@ -109,7 +112,7 @@ export default {
<div class="ci-widget-content"> <div class="ci-widget-content">
<div class="media-body"> <div class="media-body">
<div class="font-weight-bold js-pipeline-info-container"> <div class="font-weight-bold js-pipeline-info-container">
{{ s__('Pipeline|Pipeline') }} {{ pipeline.details.name }}
<gl-link :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 }}</gl-link >#{{ pipeline.id }}</gl-link
> >
...@@ -121,48 +124,13 @@ export default { ...@@ -121,48 +124,13 @@ export default {
class="commit-sha js-commit-link font-weight-normal" class="commit-sha js-commit-link font-weight-normal"
>{{ pipeline.commit.short_id }}</gl-link >{{ pipeline.commit.short_id }}</gl-link
> >
{{ 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> </template>
<template v-if="showSourceBranch">
{{ s__('Pipeline|on') }}
<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 font-weight-normal"
v-html="sourceBranchLink" v-html="sourceBranchLink"
/> />
</template> </template>
......
...@@ -835,12 +835,12 @@ module Ci ...@@ -835,12 +835,12 @@ module Ci
return unless merge_request_event? return unless merge_request_event?
strong_memoize(:merge_request_event_type) do strong_memoize(:merge_request_event_type) do
if detached_merge_request_pipeline? if merge_train_pipeline?
:detached :merge_train
elsif merge_request_pipeline? elsif merge_request_pipeline?
:merged_result :merged_result
elsif merge_train_pipeline? elsif detached_merge_request_pipeline?
:merge_train :detached
end end
end end
end end
......
---
title: Make MR pipeline widget text more descriptive
merge_request: 32025
author:
type: changed
...@@ -3443,9 +3443,6 @@ msgstr "" ...@@ -3443,9 +3443,6 @@ msgstr ""
msgid "Could not remove the trigger." msgid "Could not remove the trigger."
msgstr "" msgstr ""
msgid "Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation.%{linkEnd}"
msgstr ""
msgid "Could not revoke impersonation token %{token_name}." msgid "Could not revoke impersonation token %{token_name}."
msgstr "" msgstr ""
...@@ -5439,7 +5436,6 @@ msgstr "" ...@@ -5439,7 +5436,6 @@ msgstr ""
msgid "Go to file (MRs only)" msgid "Go to file (MRs only)"
msgstr "" msgstr ""
msgid "Go to file permalink (while viewing a file)" msgid "Go to file permalink (while viewing a file)"
msgstr "" msgstr ""
...@@ -8234,6 +8230,9 @@ msgstr "" ...@@ -8234,6 +8230,9 @@ msgstr ""
msgid "Pipeline|Commit" msgid "Pipeline|Commit"
msgstr "" msgstr ""
msgid "Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation.%{linkEnd}"
msgstr ""
msgid "Pipeline|Coverage" msgid "Pipeline|Coverage"
msgstr "" msgstr ""
...@@ -8300,18 +8299,12 @@ msgstr "" ...@@ -8300,18 +8299,12 @@ msgstr ""
msgid "Pipeline|for" msgid "Pipeline|for"
msgstr "" msgstr ""
msgid "Pipeline|into"
msgstr ""
msgid "Pipeline|on" msgid "Pipeline|on"
msgstr "" msgstr ""
msgid "Pipeline|success" msgid "Pipeline|success"
msgstr "" msgstr ""
msgid "Pipeline|with"
msgstr ""
msgid "Pipeline|with stage" msgid "Pipeline|with stage"
msgstr "" msgstr ""
......
...@@ -189,26 +189,20 @@ describe 'Merge request > User sees merge widget', :js do ...@@ -189,26 +189,20 @@ describe 'Merge request > User sees merge widget', :js do
visit project_merge_request_path(project, merge_request) visit project_merge_request_path(project, merge_request)
end end
shared_examples 'pipeline widget' do
it 'shows head pipeline information' do it 'shows head pipeline information' do
within '.ci-widget-content' do within '.ci-widget-content' do
expect(page).to have_content("Pipeline ##{pipeline.id} pending " \ expect(page).to have_content("Detached merge request pipeline ##{pipeline.id} pending for #{pipeline.short_sha}")
"for #{pipeline.short_sha} " \ end
"on #{merge_request.to_reference} " \
"with #{merge_request.source_branch}")
end end
end end
include_examples 'pipeline widget'
context 'when source project is a forked project' do context 'when source project is a forked project' do
let(:source_project) { fork_project(project, user, repository: true) } let(:source_project) { fork_project(project, user, repository: true) }
it 'shows head pipeline information' do include_examples 'pipeline widget'
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
end end
...@@ -234,29 +228,21 @@ describe 'Merge request > User sees merge widget', :js do ...@@ -234,29 +228,21 @@ describe 'Merge request > User sees merge widget', :js do
visit project_merge_request_path(project, merge_request) visit project_merge_request_path(project, merge_request)
end end
shared_examples 'pipeline widget' do
it 'shows head pipeline information' do it 'shows head pipeline information' do
within '.ci-widget-content' do within '.ci-widget-content' do
expect(page).to have_content("Pipeline ##{pipeline.id} pending " \ expect(page).to have_content("Merged result pipeline ##{pipeline.id} pending for #{pipeline.short_sha}")
"for #{pipeline.short_sha} " \ end
"on #{merge_request.to_reference} " \
"with #{merge_request.source_branch} " \
"into #{merge_request.target_branch}")
end end
end end
include_examples 'pipeline widget'
context 'when source project is a forked project' do context 'when source project is a forked project' do
let(:source_project) { fork_project(project, user, repository: true) } let(:source_project) { fork_project(project, user, repository: true) }
let(:merge_sha) { source_project.commit.sha } let(:merge_sha) { source_project.commit.sha }
it 'shows head pipeline information' do include_examples 'pipeline widget'
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
end end
......
...@@ -69,7 +69,6 @@ describe('MRWidgetPipeline', () => { ...@@ -69,7 +69,6 @@ describe('MRWidgetPipeline', () => {
vm = mountComponent(Component, { vm = mountComponent(Component, {
pipeline: mockData.pipeline, pipeline: mockData.pipeline,
hasCi: true, hasCi: true,
ciStatus: null,
troubleshootingDocsPath: 'help', troubleshootingDocsPath: 'help',
}); });
...@@ -208,72 +207,67 @@ describe('MRWidgetPipeline', () => { ...@@ -208,72 +207,67 @@ describe('MRWidgetPipeline', () => {
}); });
}); });
describe('without pipeline.merge_request', () => { describe('for each type of pipeline', () => {
it('should render info that includes the commit and branch details', () => { let pipeline;
const mockCopy = JSON.parse(JSON.stringify(mockData));
delete mockCopy.pipeline.merge_request; beforeEach(() => {
const { pipeline } = mockCopy; ({ pipeline } = JSON.parse(JSON.stringify(mockData)));
pipeline.details.name = 'Pipeline';
pipeline.merge_request_event_type = undefined;
pipeline.ref.tag = false;
pipeline.ref.branch = false;
});
const factory = () => {
vm = mountComponent(Component, { vm = mountComponent(Component, {
pipeline, pipeline,
hasCi: true, hasCi: true,
ciStatus: 'success', ciStatus: 'success',
troubleshootingDocsPath: 'help', troubleshootingDocsPath: 'help',
sourceBranchLink: mockCopy.source_branch_link, sourceBranchLink: mockData.source_branch_link,
}); });
};
const expected = `Pipeline #${pipeline.id} ${pipeline.details.status.label} for ${pipeline.commit.short_id} on ${mockCopy.source_branch_link}`; describe('for a branch pipeline', () => {
it('renders a pipeline widget that reads "Pipeline <ID> <status> for <SHA> on <branch>"', () => {
pipeline.ref.branch = true;
factory();
const expected = `Pipeline #${pipeline.id} ${pipeline.details.status.label} for ${pipeline.commit.short_id} on ${mockData.source_branch_link}`;
const actual = trimText(vm.$el.querySelector('.js-pipeline-info-container').innerText); const actual = trimText(vm.$el.querySelector('.js-pipeline-info-container').innerText);
expect(actual).toBe(expected); expect(actual).toBe(expected);
}); });
}); });
describe('with pipeline.merge_request and flags.merge_request_pipeline', () => { describe('for a tag pipeline', () => {
it('should render info that includes the commit, MR, source branch, and target branch details', () => { it('renders a pipeline widget that reads "Pipeline <ID> <status> for <SHA> on <branch>"', () => {
const mockCopy = JSON.parse(JSON.stringify(mockData)); pipeline.ref.tag = true;
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}`; factory();
const expected = `Pipeline #${pipeline.id} ${pipeline.details.status.label} for ${pipeline.commit.short_id}`;
const actual = trimText(vm.$el.querySelector('.js-pipeline-info-container').innerText); const actual = trimText(vm.$el.querySelector('.js-pipeline-info-container').innerText);
expect(actual).toBe(expected); expect(actual).toBe(expected);
}); });
}); });
describe('with pipeline.merge_request and flags.detached_merge_request_pipeline', () => { describe('for a detached merge request pipeline', () => {
it('should render info that includes the commit, MR, and source branch details', () => { it('renders a pipeline widget that reads "Detached merge request pipeline <ID> <status> for <SHA>"', () => {
const mockCopy = JSON.parse(JSON.stringify(mockData)); pipeline.details.name = 'Detached merge request pipeline';
const { pipeline } = mockCopy; pipeline.merge_request_event_type = 'detached';
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}`; factory();
const expected = `Detached merge request pipeline #${pipeline.id} ${pipeline.details.status.label} for ${pipeline.commit.short_id}`;
const actual = trimText(vm.$el.querySelector('.js-pipeline-info-container').innerText); const actual = trimText(vm.$el.querySelector('.js-pipeline-info-container').innerText);
expect(actual).toBe(expected); expect(actual).toBe(expected);
}); });
}); });
}); });
});
}); });
...@@ -259,6 +259,8 @@ export const mockStore = { ...@@ -259,6 +259,8 @@ export const mockStore = {
tooltip: 'passed', tooltip: 'passed',
}, },
}, },
flags: {},
ref: {},
}, },
mergePipeline: { mergePipeline: {
id: 1, id: 1,
...@@ -276,6 +278,8 @@ export const mockStore = { ...@@ -276,6 +278,8 @@ export const mockStore = {
tooltip: 'passed', tooltip: 'passed',
}, },
}, },
flags: {},
ref: {},
}, },
targetBranch: 'target-branch', targetBranch: 'target-branch',
sourceBranch: 'source-branch', sourceBranch: 'source-branch',
......
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