Commit 0b4791aa authored by Nathan Friend's avatar Nathan Friend

Add merge train helper text to MR widget (EE)

This commit adds a helper text that describes what will happen when a
user clicks the "Start/Add to merge train when pipeline succeeds"
button.  This text appears at the bottom of the merge request widget.

In addition, this commit renames the "merge_train_info" component to
"merge_train_position_indicator" to avoid ambiguity with the
"merge_train_helper_text" component.
parent 3302d1c4
<script>
import _ from 'underscore';
import { GlLink } from '@gitlab/ui';
import { s__, sprintf } from '~/locale';
export default {
name: 'MergeTrainHelperText',
components: {
GlLink,
},
props: {
pipelineId: {
type: Number,
required: true,
},
pipelineLink: {
type: String,
required: true,
},
mergeTrainWhenPipelineSucceedsDocsPath: {
type: String,
required: true,
},
mergeTrainLength: {
type: Number,
required: true,
},
},
computed: {
message() {
const text =
this.mergeTrainLength === 0
? s__(
'mrWidget|This merge request will start a merge train when pipeline %{linkStart}#%{pipelineId}%{linkEnd} succeeds.',
)
: s__(
'mrWidget|This merge request will be added to the merge train when pipeline %{linkStart}#%{pipelineId}%{linkEnd} succeeds.',
);
const sanitizedPipelineLink = _.escape(this.pipelineLink);
return sprintf(
text,
{
pipelineId: this.pipelineId,
linkStart: `<a class="js-pipeline-link" href="${sanitizedPipelineLink}">`,
linkEnd: '</a>',
},
false,
);
},
},
};
</script>
<template>
<section class="js-merge-train-helper-text mr-widget-help border-top">
<span v-html="message"></span>
<gl-link
:href="mergeTrainWhenPipelineSucceedsDocsPath"
target="_blank"
rel="noopener noreferrer"
class="js-documentation-link"
>
{{ s__('mrWidget|More information') }}
</gl-link>
</section>
</template>
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
import { s__, sprintf } from '~/locale'; import { s__, sprintf } from '~/locale';
export default { export default {
name: 'MergeTrainInfo', name: 'MergeTrainPositionIndicator',
props: { props: {
mergeTrainIndex: { mergeTrainIndex: {
type: Number, type: Number,
...@@ -20,7 +20,7 @@ export default { ...@@ -20,7 +20,7 @@ export default {
</script> </script>
<template> <template>
<div class="pt-2 pb-2 pl-3 plr-3 merge-train-info"> <div class="pt-2 pb-2 pl-3 plr-3 merge-train-position-indicator">
<div class="media-body"> <div class="media-body">
{{ message }} {{ message }}
</div> </div>
......
<script> <script>
import _ from 'underscore';
import ReportSection from '~/reports/components/report_section.vue'; import ReportSection from '~/reports/components/report_section.vue';
import GroupedSecurityReportsApp from 'ee/vue_shared/security_reports/grouped_security_reports_app.vue'; import GroupedSecurityReportsApp from 'ee/vue_shared/security_reports/grouped_security_reports_app.vue';
import GroupedMetricsReportsApp from 'ee/vue_shared/metrics_reports/grouped_metrics_reports_app.vue'; import GroupedMetricsReportsApp from 'ee/vue_shared/metrics_reports/grouped_metrics_reports_app.vue';
...@@ -11,9 +12,12 @@ import { n__, s__, __, sprintf } from '~/locale'; ...@@ -11,9 +12,12 @@ import { n__, s__, __, sprintf } from '~/locale';
import CEWidgetOptions from '~/vue_merge_request_widget/mr_widget_options.vue'; import CEWidgetOptions from '~/vue_merge_request_widget/mr_widget_options.vue';
import MrWidgetApprovals from './components/approvals/approvals.vue'; import MrWidgetApprovals from './components/approvals/approvals.vue';
import MrWidgetGeoSecondaryNode from './components/states/mr_widget_secondary_geo_node.vue'; import MrWidgetGeoSecondaryNode from './components/states/mr_widget_secondary_geo_node.vue';
import MergeTrainHelperText from './components/merge_train_helper_text.vue';
import { ATMTWPS_MERGE_STRATEGY } from '~/vue_merge_request_widget/constants';
export default { export default {
components: { components: {
MergeTrainHelperText,
MrWidgetLicenses, MrWidgetLicenses,
MrWidgetApprovals, MrWidgetApprovals,
MrWidgetGeoSecondaryNode, MrWidgetGeoSecondaryNode,
...@@ -131,6 +135,16 @@ export default { ...@@ -131,6 +135,16 @@ export default {
performanceStatus() { performanceStatus() {
return this.checkReportStatus(this.isLoadingPerformance, this.loadingPerformanceFailed); return this.checkReportStatus(this.isLoadingPerformance, this.loadingPerformanceFailed);
}, },
shouldRenderMergeTrainHelperText() {
return (
this.mr.pipeline &&
_.isNumber(this.mr.pipeline.id) &&
_.isString(this.mr.pipeline.path) &&
this.mr.preferredAutoMergeStrategy === ATMTWPS_MERGE_STRATEGY &&
!this.mr.autoMergeEnabled
);
},
}, },
created() { created() {
if (this.shouldRenderCodeQuality) { if (this.shouldRenderCodeQuality) {
...@@ -331,6 +345,13 @@ export default { ...@@ -331,6 +345,13 @@ export default {
<source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" /> <source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" />
</div> </div>
</div> </div>
<merge-train-helper-text
v-if="shouldRenderMergeTrainHelperText"
:pipeline-id="mr.pipeline.id"
:pipeline-link="mr.pipeline.path"
:merge-train-length="mr.mergeTrainsCount"
:merge-train-when-pipeline-succeeds-docs-path="mr.mergeTrainWhenPipelineSucceedsDocsPath"
/>
<div v-if="shouldRenderMergeHelp" class="mr-widget-footer"><mr-widget-merge-help /></div> <div v-if="shouldRenderMergeHelp" class="mr-widget-footer"><mr-widget-merge-help /></div>
</div> </div>
<mr-widget-pipeline-container <mr-widget-pipeline-container
......
...@@ -41,6 +41,10 @@ module EE ...@@ -41,6 +41,10 @@ module EE
end end
end end
def merge_train_when_pipeline_succeeds_docs_path
help_page_path('ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/index.md', anchor: 'startadd-to-merge-train-when-pipeline-succeeds')
end
def target_project def target_project
merge_request.target_project.present(current_user: current_user) merge_request.target_project.present(current_user: current_user)
end end
......
...@@ -161,6 +161,9 @@ module EE ...@@ -161,6 +161,9 @@ module EE
expose :api_unapprove_path do |merge_request| expose :api_unapprove_path do |merge_request|
presenter(merge_request).api_unapprove_path presenter(merge_request).api_unapprove_path
end end
expose :merge_train_when_pipeline_succeeds_docs_path do |merge_request|
presenter(merge_request).merge_train_when_pipeline_succeeds_docs_path
end
expose :blocking_merge_requests, if: -> (mr, _) { mr&.target_project&.feature_available?(:blocking_merge_requests) } expose :blocking_merge_requests, if: -> (mr, _) { mr&.target_project&.feature_available?(:blocking_merge_requests) }
......
---
title: Add merge train helper text to merge request widget
merge_request: 14097
author:
type: added
...@@ -12,6 +12,8 @@ describe 'User adds to merge train when pipeline succeeds', :js do ...@@ -12,6 +12,8 @@ describe 'User adds to merge train when pipeline succeeds', :js do
target_project: project, target_branch: 'master') target_project: project, target_branch: 'master')
end end
let(:pipeline) { merge_request.all_pipelines.first }
before do before do
stub_licensed_features(merge_pipelines: true, merge_trains: true) stub_licensed_features(merge_pipelines: true, merge_trains: true)
project.add_maintainer(user) project.add_maintainer(user)
...@@ -21,10 +23,16 @@ describe 'User adds to merge train when pipeline succeeds', :js do ...@@ -21,10 +23,16 @@ describe 'User adds to merge train when pipeline succeeds', :js do
sign_in(user) sign_in(user)
end end
it 'shows Start merge train when pipeline succeeds button' do it 'shows Start merge train when pipeline succeeds button and helper texts' do
visit project_merge_request_path(project, merge_request) visit project_merge_request_path(project, merge_request)
expect(page).to have_button('Start merge train when pipeline succeeds') expect(page).to have_button('Start merge train when pipeline succeeds')
within('.js-merge-train-helper-text') do
expect(page).to have_content("This merge request will start a merge train when pipeline ##{pipeline.id} succeeds.")
expect(page).to have_link('More information',
href: MergeRequestPresenter.new(merge_request).merge_train_when_pipeline_succeeds_docs_path)
end
end end
context 'when merge_trains EEP license is not available' do context 'when merge_trains EEP license is not available' do
......
import { shallowMount, createLocalVue } from '@vue/test-utils';
import { GlLink } from '@gitlab/ui';
import { trimText } from 'helpers/text_helper';
import MergeTrainHelperText from 'ee/vue_merge_request_widget/components/merge_train_helper_text.vue';
describe('MergeTrainHelperText', () => {
const localVue = createLocalVue();
let wrapper;
const factory = propsData => {
wrapper = shallowMount(localVue.extend(MergeTrainHelperText), {
propsData,
localVue,
sync: false,
});
};
afterEach(() => {
wrapper.destroy();
});
it('should return the "start" version of the message if there is no existing merge train', () => {
factory({
pipelineId: 123,
pipelineLink: 'path/to/pipeline',
mergeTrainWhenPipelineSucceedsDocsPath: 'path/to/help',
mergeTrainLength: 0,
});
expect(trimText(wrapper.text())).toBe(
'This merge request will start a merge train when pipeline #123 succeeds. More information',
);
});
it('should render the correct pipeline link in the helper text', () => {
factory({
pipelineId: 123,
pipelineLink: 'path/to/pipeline',
mergeTrainWhenPipelineSucceedsDocsPath: 'path/to/help',
mergeTrainLength: 2,
});
const pipelineLink = wrapper.find('.js-pipeline-link').element;
expect(pipelineLink).toExist();
expect(pipelineLink.textContent).toContain('#123');
expect(pipelineLink).toHaveAttr('href', 'path/to/pipeline');
});
it('should sanitize the pipeline link', () => {
factory({
pipelineId: 123,
pipelineLink: '"></a> <script>console.log("hacked!!")</script> <a href="',
mergeTrainWhenPipelineSucceedsDocsPath: 'path/to/help',
mergeTrainLength: 2,
});
const pipelineLink = wrapper.find('.js-pipeline-link').element;
expect(pipelineLink).toExist();
// The escaped characters are un-escaped when rendered by the DOM,
// so we expect the value of the "href" attr to be exactly the same
// as the input. If the link was not sanitized, the "href" attr
// would equal "".
expect(pipelineLink).toHaveAttr(
'href',
'"></a> <script>console.log("hacked!!")</script> <a href="',
);
});
it('should render the correct documentation link in the helper text', () => {
factory({
pipelineId: 123,
pipelineLink: 'path/to/pipeline',
mergeTrainWhenPipelineSucceedsDocsPath: 'path/to/help',
mergeTrainLength: 2,
});
const docLink = wrapper.find(GlLink);
expect(docLink.exists()).toBe(true);
expect(docLink.attributes().href).toBe('path/to/help');
});
});
import { shallowMount, createLocalVue } from '@vue/test-utils'; import { shallowMount, createLocalVue } from '@vue/test-utils';
import { trimText } from 'spec/helpers/text_helper'; import { trimText } from 'helpers/text_helper';
import MergeTrainInfo from 'ee/vue_merge_request_widget/components/merge_train_info.vue'; import MergeTrainPositionIndicator from 'ee/vue_merge_request_widget/components/merge_train_position_indicator.vue';
describe('MergeTrainInfo', () => { describe('MergeTrainPositionIndicator', () => {
const localVue = createLocalVue(); const localVue = createLocalVue();
let wrapper; let wrapper;
let vm; let vm;
const factory = propsData => { const factory = propsData => {
wrapper = shallowMount(localVue.extend(MergeTrainInfo), { wrapper = shallowMount(localVue.extend(MergeTrainPositionIndicator), {
propsData, propsData,
localVue, localVue,
sync: false, sync: false,
......
import { mount, createLocalVue } from '@vue/test-utils'; import { mount, createLocalVue } from '@vue/test-utils';
import MrWidgetPipelineContainer from '~/vue_merge_request_widget/components/mr_widget_pipeline_container.vue'; import MrWidgetPipelineContainer from '~/vue_merge_request_widget/components/mr_widget_pipeline_container.vue';
import { MT_MERGE_STRATEGY, MWPS_MERGE_STRATEGY } from '~/vue_merge_request_widget/constants'; import { MT_MERGE_STRATEGY, MWPS_MERGE_STRATEGY } from '~/vue_merge_request_widget/constants';
import MergeTrainInfo from 'ee/vue_merge_request_widget/components/merge_train_info.vue'; import MergeTrainPositionIndicator from 'ee/vue_merge_request_widget/components/merge_train_position_indicator.vue';
import { mockStore } from 'spec/vue_mr_widget/mock_data'; import { mockStore } from 'spec/vue_mr_widget/mock_data';
describe('MrWidgetPipelineContainer', () => { describe('MrWidgetPipelineContainer', () => {
...@@ -29,7 +29,7 @@ describe('MrWidgetPipelineContainer', () => { ...@@ -29,7 +29,7 @@ describe('MrWidgetPipelineContainer', () => {
autoMergeStrategy: MT_MERGE_STRATEGY, autoMergeStrategy: MT_MERGE_STRATEGY,
}); });
expect(wrapper.find(MergeTrainInfo).exists()).toBe(false); expect(wrapper.find(MergeTrainPositionIndicator).exists()).toBe(false);
}); });
it('should not render the merge train indicator if the MR is closed', () => { it('should not render the merge train indicator if the MR is closed', () => {
...@@ -38,7 +38,7 @@ describe('MrWidgetPipelineContainer', () => { ...@@ -38,7 +38,7 @@ describe('MrWidgetPipelineContainer', () => {
autoMergeStrategy: MT_MERGE_STRATEGY, autoMergeStrategy: MT_MERGE_STRATEGY,
}); });
expect(wrapper.find(MergeTrainInfo).exists()).toBe(false); expect(wrapper.find(MergeTrainPositionIndicator).exists()).toBe(false);
}); });
it('should not render the merge train indicator if the MR is not on the merge train', () => { it('should not render the merge train indicator if the MR is not on the merge train', () => {
...@@ -47,7 +47,7 @@ describe('MrWidgetPipelineContainer', () => { ...@@ -47,7 +47,7 @@ describe('MrWidgetPipelineContainer', () => {
autoMergeStrategy: MWPS_MERGE_STRATEGY, autoMergeStrategy: MWPS_MERGE_STRATEGY,
}); });
expect(wrapper.find(MergeTrainInfo).exists()).toBe(false); expect(wrapper.find(MergeTrainPositionIndicator).exists()).toBe(false);
}); });
}); });
}); });
...@@ -29,6 +29,7 @@ import { ...@@ -29,6 +29,7 @@ import {
sastBaseAllIssues, sastBaseAllIssues,
sastHeadAllIssues, sastHeadAllIssues,
} from 'ee_spec/vue_shared/security_reports/mock_data'; } from 'ee_spec/vue_shared/security_reports/mock_data';
import { ATMTWPS_MERGE_STRATEGY, MT_MERGE_STRATEGY } from '~/vue_merge_request_widget/constants';
describe('ee merge request widget options', () => { describe('ee merge request widget options', () => {
let vm; let vm;
...@@ -792,6 +793,20 @@ describe('ee merge request widget options', () => { ...@@ -792,6 +793,20 @@ describe('ee merge request widget options', () => {
expect(vm.shouldRenderApprovals).toBeTruthy(); expect(vm.shouldRenderApprovals).toBeTruthy();
}); });
}); });
describe('shouldRenderMergeTrainHelperText', () => {
it('should return true if ATMTWPS is available and the user has not yet pressed the ATMTWPS button', () => {
vm = mountComponent(Component, {
mrData: {
...mockData,
available_auto_merge_strategies: [ATMTWPS_MERGE_STRATEGY],
auto_merge_enabled: false,
},
});
expect(vm.shouldRenderMergeTrainHelperText).toBe(true);
});
});
}); });
describe('rendering source branch removal status', () => { describe('rendering source branch removal status', () => {
...@@ -889,6 +904,115 @@ describe('ee merge request widget options', () => { ...@@ -889,6 +904,115 @@ describe('ee merge request widget options', () => {
}); });
}); });
describe('merge train helper text', () => {
const getHelperTextElement = () => vm.$el.querySelector('.js-merge-train-helper-text');
it('does not render the merge train helpe text if the ATMTWPS strategy is not available', () => {
vm = mountComponent(Component, {
mrData: {
...mockData,
available_auto_merge_strategies: [MT_MERGE_STRATEGY],
pipeline: {
...mockData.pipeline,
active: true,
},
},
});
const helperText = getHelperTextElement();
expect(helperText).not.toExist();
});
it('renders the correct merge train helper text when there is an existing merge train', () => {
vm = mountComponent(Component, {
mrData: {
...mockData,
available_auto_merge_strategies: [ATMTWPS_MERGE_STRATEGY],
merge_trains_count: 2,
merge_train_when_pipeline_succeeds_docs_path: 'path/to/help',
pipeline: {
...mockData.pipeline,
id: 123,
active: true,
},
},
});
const helperText = getHelperTextElement();
expect(helperText).toExist();
expect(helperText).toContainText(
'This merge request will be added to the merge train when pipeline #123 succeeds.',
);
});
it('renders the correct merge train helper text when there is no existing merge train', () => {
vm = mountComponent(Component, {
mrData: {
...mockData,
available_auto_merge_strategies: [ATMTWPS_MERGE_STRATEGY],
merge_trains_count: 0,
merge_train_when_pipeline_succeeds_docs_path: 'path/to/help',
pipeline: {
...mockData.pipeline,
id: 123,
active: true,
},
},
});
const helperText = getHelperTextElement();
expect(helperText).toExist();
expect(helperText).toContainText(
'This merge request will start a merge train when pipeline #123 succeeds.',
);
});
it('renders the correct pipeline link inside the message', () => {
vm = mountComponent(Component, {
mrData: {
...mockData,
available_auto_merge_strategies: [ATMTWPS_MERGE_STRATEGY],
merge_train_when_pipeline_succeeds_docs_path: 'path/to/help',
pipeline: {
...mockData.pipeline,
id: 123,
path: 'path/to/pipeline',
active: true,
},
},
});
const pipelineLink = getHelperTextElement().querySelector('.js-pipeline-link');
expect(pipelineLink).toExist();
expect(pipelineLink).toContainText('#123');
expect(pipelineLink).toHaveAttr('href', 'path/to/pipeline');
});
it('renders the documentation link inside the message', () => {
vm = mountComponent(Component, {
mrData: {
...mockData,
available_auto_merge_strategies: [ATMTWPS_MERGE_STRATEGY],
merge_train_when_pipeline_succeeds_docs_path: 'path/to/help',
pipeline: {
...mockData.pipeline,
active: true,
},
},
});
const pipelineLink = getHelperTextElement().querySelector('.js-documentation-link');
expect(pipelineLink).toExist();
expect(pipelineLink).toContainText('More information');
expect(pipelineLink).toHaveAttr('href', 'path/to/help');
});
});
describe('data', () => { describe('data', () => {
it('passes approval api paths to service', () => { it('passes approval api paths to service', () => {
const paths = { const paths = {
......
...@@ -18261,6 +18261,9 @@ msgstr "" ...@@ -18261,6 +18261,9 @@ msgstr ""
msgid "mrWidget|Merged by" msgid "mrWidget|Merged by"
msgstr "" msgstr ""
msgid "mrWidget|More information"
msgstr ""
msgid "mrWidget|No approval required" msgid "mrWidget|No approval required"
msgstr "" msgstr ""
...@@ -18360,6 +18363,12 @@ msgstr "" ...@@ -18360,6 +18363,12 @@ msgstr ""
msgid "mrWidget|This merge request is in the process of being merged" msgid "mrWidget|This merge request is in the process of being merged"
msgstr "" msgstr ""
msgid "mrWidget|This merge request will be added to the merge train when pipeline %{linkStart}#%{pipelineId}%{linkEnd} succeeds."
msgstr ""
msgid "mrWidget|This merge request will start a merge train when pipeline %{linkStart}#%{pipelineId}%{linkEnd} succeeds."
msgstr ""
msgid "mrWidget|This project is archived, write access has been disabled" msgid "mrWidget|This project is archived, write access has been disabled"
msgstr "" msgstr ""
......
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