summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNathan Friend <nathan@gitlab.com>2019-09-10 15:18:39 -0300
committerNathan Friend <nathan@gitlab.com>2019-09-11 07:27:58 -0300
commit11f1305ec7b71721a24352ab2346d3ed865883c4 (patch)
treee714bad8becb45bc2249179c0d48a52027ef2ce4
parent1ba9f029e89e99a245a201c21a98f4f79dc7b302 (diff)
downloadgitlab-ce-ce-indicator-for-pipeline-for-merge-train.tar.gz
Make MR pipeline widget text more descriptive (CE)ce-indicator-for-pipeline-for-merge-train
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.
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue52
-rw-r--r--app/models/ci/pipeline.rb8
-rw-r--r--changelogs/unreleased/ce-indicator-for-pipeline-for-merge-train.yml5
-rw-r--r--locale/gitlab.pot13
-rw-r--r--spec/features/merge_request/user_sees_merge_widget_spec.rb46
-rw-r--r--spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js86
-rw-r--r--spec/javascripts/vue_mr_widget/mock_data.js4
7 files changed, 82 insertions, 132 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 40c095aa954..4b5201bbca7 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,7 +1,7 @@
<script>
/* eslint-disable vue/require-default-prop */
import { GlTooltipDirective, GlLink } from '@gitlab/ui';
-import { sprintf, __ } from '~/locale';
+import { sprintf, s__ } from '~/locale';
import PipelineStage from '~/pipelines/components/stage.vue';
import CiIcon from '~/vue_shared/components/ci_icon.vue';
import Icon from '~/vue_shared/components/icon.vue';
@@ -73,8 +73,8 @@ export default {
},
errorText() {
return sprintf(
- __(
- 'Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation.%{linkEnd}',
+ s__(
+ 'Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation.%{linkEnd}',
),
{
linkStart: `<a href="${this.troubleshootingDocsPath}">`,
@@ -89,6 +89,9 @@ export default {
isMergeRequestPipeline() {
return Boolean(this.pipeline.flags && this.pipeline.flags.merge_request_pipeline);
},
+ showSourceBranch() {
+ return Boolean(this.pipeline.ref.branch);
+ },
},
};
</script>
@@ -109,7 +112,7 @@ export default {
<div class="ci-widget-content">
<div class="media-body">
<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"
>#{{ pipeline.id }}</gl-link
>
@@ -121,48 +124,13 @@ export default {
class="commit-sha js-commit-link font-weight-normal"
>{{ pipeline.commit.short_id }}</gl-link
>
+ </template>
+ <template v-if="showSourceBranch">
{{ 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"
+ class="label-branch label-truncate font-weight-normal"
v-html="sourceBranchLink"
/>
</template>
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index d2271c1335c..4aaabed6b7b 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -835,12 +835,12 @@ module Ci
return unless merge_request_event?
strong_memoize(:merge_request_event_type) do
- if detached_merge_request_pipeline?
- :detached
+ if merge_train_pipeline?
+ :merge_train
elsif merge_request_pipeline?
:merged_result
- elsif merge_train_pipeline?
- :merge_train
+ elsif detached_merge_request_pipeline?
+ :detached
end
end
end
diff --git a/changelogs/unreleased/ce-indicator-for-pipeline-for-merge-train.yml b/changelogs/unreleased/ce-indicator-for-pipeline-for-merge-train.yml
new file mode 100644
index 00000000000..06d2f4b56f5
--- /dev/null
+++ b/changelogs/unreleased/ce-indicator-for-pipeline-for-merge-train.yml
@@ -0,0 +1,5 @@
+---
+title: Make MR pipeline widget text more descriptive
+merge_request: 32025
+author:
+type: changed
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index f2d3a39d593..1801a6d431a 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3443,9 +3443,6 @@ msgstr ""
msgid "Could not remove the trigger."
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}."
msgstr ""
@@ -5439,7 +5436,6 @@ msgstr ""
msgid "Go to file (MRs only)"
msgstr ""
-
msgid "Go to file permalink (while viewing a file)"
msgstr ""
@@ -8234,6 +8230,9 @@ msgstr ""
msgid "Pipeline|Commit"
msgstr ""
+msgid "Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation.%{linkEnd}"
+msgstr ""
+
msgid "Pipeline|Coverage"
msgstr ""
@@ -8300,18 +8299,12 @@ 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 3f2a676462b..d19835741e3 100644
--- a/spec/features/merge_request/user_sees_merge_widget_spec.rb
+++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb
@@ -189,26 +189,20 @@ describe 'Merge request > User sees merge widget', :js do
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}")
+ shared_examples 'pipeline widget' do
+ it 'shows head pipeline information' do
+ within '.ci-widget-content' do
+ expect(page).to have_content("Detached merge request pipeline ##{pipeline.id} pending for #{pipeline.short_sha}")
+ end
end
end
+ include_examples 'pipeline widget'
+
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
+ include_examples 'pipeline widget'
end
end
@@ -234,29 +228,21 @@ describe 'Merge request > User sees merge widget', :js do
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}")
+ shared_examples 'pipeline widget' do
+ it 'shows head pipeline information' do
+ within '.ci-widget-content' do
+ expect(page).to have_content("Merged result pipeline ##{pipeline.id} pending for #{pipeline.short_sha}")
+ end
end
end
+ include_examples 'pipeline widget'
+
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
+ include_examples 'pipeline widget'
end
end
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 fe831094ecf..67e85763fae 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
@@ -69,7 +69,6 @@ describe('MRWidgetPipeline', () => {
vm = mountComponent(Component, {
pipeline: mockData.pipeline,
hasCi: true,
- ciStatus: null,
troubleshootingDocsPath: 'help',
});
@@ -208,71 +207,66 @@ describe('MRWidgetPipeline', () => {
});
});
- 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}`;
+ describe('for each type of pipeline', () => {
+ let pipeline;
- const actual = trimText(vm.$el.querySelector('.js-pipeline-info-container').innerText);
+ beforeEach(() => {
+ ({ pipeline } = JSON.parse(JSON.stringify(mockData)));
- expect(actual).toBe(expected);
+ pipeline.details.name = 'Pipeline';
+ pipeline.merge_request_event_type = undefined;
+ pipeline.ref.tag = false;
+ pipeline.ref.branch = false;
});
- });
-
- 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;
+ const factory = () => {
vm = mountComponent(Component, {
pipeline,
hasCi: true,
ciStatus: 'success',
troubleshootingDocsPath: 'help',
- sourceBranchLink: mockCopy.source_branch_link,
+ sourceBranchLink: mockData.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;
- 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 actual = trimText(vm.$el.querySelector('.js-pipeline-info-container').innerText);
+ 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);
- expect(actual).toBe(expected);
+ 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;
+ describe('for a tag pipeline', () => {
+ it('renders a pipeline widget that reads "Pipeline <ID> <status> for <SHA> on <branch>"', () => {
+ pipeline.ref.tag = true;
- vm = mountComponent(Component, {
- pipeline,
- hasCi: true,
- ciStatus: 'success',
- troubleshootingDocsPath: 'help',
- sourceBranchLink: mockCopy.source_branch_link,
+ 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);
+
+ expect(actual).toBe(expected);
});
+ });
+
+ describe('for a detached merge request pipeline', () => {
+ it('renders a pipeline widget that reads "Detached merge request pipeline <ID> <status> for <SHA>"', () => {
+ pipeline.details.name = 'Detached merge request pipeline';
+ pipeline.merge_request_event_type = 'detached';
- 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 actual = trimText(vm.$el.querySelector('.js-pipeline-info-container').innerText);
+ 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);
- expect(actual).toBe(expected);
+ 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 a55d5537df7..2f79806652b 100644
--- a/spec/javascripts/vue_mr_widget/mock_data.js
+++ b/spec/javascripts/vue_mr_widget/mock_data.js
@@ -259,6 +259,8 @@ export const mockStore = {
tooltip: 'passed',
},
},
+ flags: {},
+ ref: {},
},
mergePipeline: {
id: 1,
@@ -276,6 +278,8 @@ export const mockStore = {
tooltip: 'passed',
},
},
+ flags: {},
+ ref: {},
},
targetBranch: 'target-branch',
sourceBranch: 'source-branch',