Commit c01687e0 authored by Nathan Friend's avatar Nathan Friend Committed by Filipa Lacerda

Add two warning messages to the MR widget

This commit adds two new warning messages to the MR widget that handle
cases involving merge request pipelines.
parent dcba84ce
<script>
import { GlLink } from '@gitlab/ui';
import Icon from '~/vue_shared/components/icon.vue';
import { WARNING, DANGER, WARNING_MESSAGE_CLASS, DANGER_MESSAGE_CLASS } from '../constants';
export default {
name: 'MrWidgetAlertMessage',
components: {
GlLink,
Icon,
},
props: {
type: {
type: String,
required: false,
default: DANGER,
validator: value => [WARNING, DANGER].includes(value),
},
helpPath: {
type: String,
required: false,
default: undefined,
},
},
computed: {
messageClass() {
if (this.type === WARNING) {
return WARNING_MESSAGE_CLASS;
} else if (this.type === DANGER) {
return DANGER_MESSAGE_CLASS;
}
return '';
},
},
};
</script>
<template>
<div class="m-3 ml-5" :class="messageClass">
<slot></slot>
<gl-link v-if="helpPath" :href="helpPath" target="_blank">
<icon :size="16" name="question-o" class="align-middle" />
</gl-link>
</div>
</template>
export const WARNING = 'warning';
export const DANGER = 'danger';
export const WARNING_MESSAGE_CLASS = 'warning_message';
export const DANGER_MESSAGE_CLASS = 'danger_message';
...@@ -12,6 +12,7 @@ import WidgetMergeHelp from './components/mr_widget_merge_help.vue'; ...@@ -12,6 +12,7 @@ import WidgetMergeHelp from './components/mr_widget_merge_help.vue';
import MrWidgetPipelineContainer from './components/mr_widget_pipeline_container.vue'; import MrWidgetPipelineContainer from './components/mr_widget_pipeline_container.vue';
import Deployment from './components/deployment.vue'; import Deployment from './components/deployment.vue';
import WidgetRelatedLinks from './components/mr_widget_related_links.vue'; import WidgetRelatedLinks from './components/mr_widget_related_links.vue';
import MrWidgetAlertMessage from './components/mr_widget_alert_message.vue';
import MergedState from './components/states/mr_widget_merged.vue'; import MergedState from './components/states/mr_widget_merged.vue';
import ClosedState from './components/states/mr_widget_closed.vue'; import ClosedState from './components/states/mr_widget_closed.vue';
import MergingState from './components/states/mr_widget_merging.vue'; import MergingState from './components/states/mr_widget_merging.vue';
...@@ -46,6 +47,7 @@ export default { ...@@ -46,6 +47,7 @@ export default {
MrWidgetPipelineContainer, MrWidgetPipelineContainer,
Deployment, Deployment,
'mr-widget-related-links': WidgetRelatedLinks, 'mr-widget-related-links': WidgetRelatedLinks,
MrWidgetAlertMessage,
'mr-widget-merged': MergedState, 'mr-widget-merged': MergedState,
'mr-widget-closed': ClosedState, 'mr-widget-closed': ClosedState,
'mr-widget-merging': MergingState, 'mr-widget-merging': MergingState,
...@@ -110,6 +112,18 @@ export default { ...@@ -110,6 +112,18 @@ export default {
shouldRenderMergedPipeline() { shouldRenderMergedPipeline() {
return this.mr.state === 'merged' && !_.isEmpty(this.mr.mergePipeline); return this.mr.state === 'merged' && !_.isEmpty(this.mr.mergePipeline);
}, },
showMergePipelineForkWarning() {
return Boolean(
this.mr.mergePipelinesEnabled && this.mr.sourceProjectId !== this.mr.targetProjectId,
);
},
showTargetBranchAdvancedError() {
return Boolean(
this.mr.pipeline &&
this.mr.pipeline.target_sha &&
this.mr.pipeline.target_sha !== this.mr.targetBranchSha,
);
},
}, },
watch: { watch: {
state(newVal, oldVal) { state(newVal, oldVal) {
...@@ -328,6 +342,30 @@ export default { ...@@ -328,6 +342,30 @@ export default {
:related-links="mr.relatedLinks" :related-links="mr.relatedLinks"
/> />
<mr-widget-alert-message
v-if="showMergePipelineForkWarning"
type="warning"
:help-path="mr.mergeRequestPipelinesHelpPath"
>
{{
s__(
'mrWidget|Fork merge requests do not create merge request pipelines which validate a post merge result',
)
}}
</mr-widget-alert-message>
<mr-widget-alert-message
v-if="showTargetBranchAdvancedError"
type="danger"
:help-path="mr.mergeRequestPipelinesHelpPath"
>
{{
s__(
'mrWidget|The target branch has advanced, which invalidates the merge request pipeline. Please update the source branch and retry merging',
)
}}
</mr-widget-alert-message>
<source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" /> <source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" />
</div> </div>
<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>
......
...@@ -28,9 +28,11 @@ export default class MergeRequestStore { ...@@ -28,9 +28,11 @@ export default class MergeRequestStore {
this.iid = data.iid; this.iid = data.iid;
this.title = data.title; this.title = data.title;
this.targetBranch = data.target_branch; this.targetBranch = data.target_branch;
this.targetBranchSha = data.target_branch_sha;
this.sourceBranch = data.source_branch; this.sourceBranch = data.source_branch;
this.sourceBranchProtected = data.source_branch_protected; this.sourceBranchProtected = data.source_branch_protected;
this.conflictsDocsPath = data.conflicts_docs_path; this.conflictsDocsPath = data.conflicts_docs_path;
this.mergeRequestPipelinesHelpPath = data.merge_request_pipelines_docs_path;
this.mergeStatus = data.merge_status; this.mergeStatus = data.merge_status;
this.commitMessage = data.default_merge_commit_message; this.commitMessage = data.default_merge_commit_message;
this.shortMergeCommitSha = data.short_merge_commit_sha; this.shortMergeCommitSha = data.short_merge_commit_sha;
...@@ -99,6 +101,9 @@ export default class MergeRequestStore { ...@@ -99,6 +101,9 @@ export default class MergeRequestStore {
this.allowCollaboration = data.allow_collaboration; this.allowCollaboration = data.allow_collaboration;
this.targetProjectFullPath = data.target_project_full_path; this.targetProjectFullPath = data.target_project_full_path;
this.sourceProjectFullPath = data.source_project_full_path; this.sourceProjectFullPath = data.source_project_full_path;
this.sourceProjectId = data.source_project_id;
this.targetProjectId = data.target_project_id;
this.mergePipelinesEnabled = data.merge_pipelines_enabled;
// Cherry-pick and Revert actions related // Cherry-pick and Revert actions related
this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false; this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false;
......
...@@ -200,12 +200,12 @@ li.note { ...@@ -200,12 +200,12 @@ li.note {
} }
} }
.warning_message { @mixin message($background-color, $border-color, $text-color) {
border-left: 4px solid $orange-200; border-left: 4px solid $border-color;
color: $orange-700; color: $text-color;
padding: 10px; padding: 10px;
margin-bottom: 10px; margin-bottom: 10px;
background: $orange-100; background: $background-color;
padding-left: 20px; padding-left: 20px;
&.centered { &.centered {
...@@ -213,6 +213,14 @@ li.note { ...@@ -213,6 +213,14 @@ li.note {
} }
} }
.warning_message {
@include message($orange-100, $orange-200, $orange-700);
}
.danger_message {
@include message($red-100, $red-200, $red-900);
}
.gitlab-promo { .gitlab-promo {
a { a {
color: $gl-gray-350; color: $gl-gray-350;
......
...@@ -212,6 +212,10 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated ...@@ -212,6 +212,10 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
help_page_path('user/project/merge_requests/resolve_conflicts.md') help_page_path('user/project/merge_requests/resolve_conflicts.md')
end end
def merge_request_pipelines_docs_path
help_page_path('ci/merge_request_pipelines/index.md')
end
private private
def cached_can_be_reverted? def cached_can_be_reverted?
......
...@@ -258,6 +258,10 @@ class MergeRequestWidgetEntity < IssuableEntity ...@@ -258,6 +258,10 @@ class MergeRequestWidgetEntity < IssuableEntity
presenter(merge_request).conflicts_docs_path presenter(merge_request).conflicts_docs_path
end end
expose :merge_request_pipelines_docs_path do |merge_request|
presenter(merge_request).merge_request_pipelines_docs_path
end
private private
delegate :current_user, to: :request delegate :current_user, to: :request
......
---
title: Add two new warning messages to the MR widget about merge request pipelines
merge_request: 25983
author:
type: added
...@@ -298,6 +298,31 @@ export default { ...@@ -298,6 +298,31 @@ export default {
:state="mr.state" :state="mr.state"
:related-links="mr.relatedLinks" :related-links="mr.relatedLinks"
/> />
<mr-widget-alert-message
v-if="showMergePipelineForkWarning"
type="warning"
:help-path="mr.mergeRequestPipelinesHelpPath"
>
{{
s__(
'mrWidget|Fork merge requests do not create merge request pipelines which validate a post merge result',
)
}}
</mr-widget-alert-message>
<mr-widget-alert-message
v-if="showTargetBranchAdvancedError"
type="danger"
:help-path="mr.mergeRequestPipelinesHelpPath"
>
{{
s__(
'mrWidget|The target branch has advanced, which invalidates the merge request pipeline. Please update the source branch and retry merging',
)
}}
</mr-widget-alert-message>
<source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" /> <source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" />
</div> </div>
<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>
......
# frozen_string_literal: true
require 'rails_helper'
describe 'Merge request > User sees merge widget', :js do
include ProjectForksHelper
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
before do
project.add_developer(user)
sign_in(user)
end
context 'when merge pipelines option is enabled at project level configuration' do
before do
stub_licensed_features(merge_pipelines: true)
project.update!(merge_pipelines_enabled: true)
end
let!(:merge_request) do
create(:merge_request,
*traits,
source_project: source_project,
source_branch: 'feature',
target_project: target_project,
target_branch: 'master',
**options)
end
let(:target_project) { project }
let(:source_project) { project }
context 'when the head pipeline is merge request pipeline' do
let(:traits) { [:with_merge_request_pipeline] }
let(:options) { {} }
before do
merge_request.update_head_pipeline
merge_request.all_pipelines.first.succeed!
end
it 'does not show any warnings' do
visit project_merge_request_path(project, merge_request)
expect(page).not_to have_css('.danger_message')
end
context 'when target branch is advanced' do
let(:options) { { target_sha: 'old-commit' } }
it 'shows a warning that fork project cannot create merge request pipelines' do
visit project_merge_request_path(project, merge_request)
within('.danger_message') do
expect(page)
.to have_content('The target branch has advanced, which invalidates ' \
'the merge request pipeline. Please update the source ' \
'branch and retry merging')
end
end
end
end
context 'when merge request is submitted from a forked project' do
let(:source_project) { fork_project(project, user, repository: true) }
let(:traits) { [:with_detached_merge_request_pipeline] }
let(:options) { {} }
it 'shows a warning that fork project cannot create merge request pipelines' do
visit project_merge_request_path(project, merge_request)
within('.warning_message') do
expect(page)
.to have_content('Fork merge requests do not create merge request' \
' pipelines which validate a post merge result')
end
end
end
end
end
...@@ -14353,6 +14353,9 @@ msgstr "" ...@@ -14353,6 +14353,9 @@ msgstr ""
msgid "mrWidget|Fast-forward merge is not possible. To merge this request, first rebase locally." msgid "mrWidget|Fast-forward merge is not possible. To merge this request, first rebase locally."
msgstr "" msgstr ""
msgid "mrWidget|Fork merge requests do not create merge request pipelines which validate a post merge result"
msgstr ""
msgid "mrWidget|If the %{branch} branch exists in your local repository, you can merge this merge request manually using the" msgid "mrWidget|If the %{branch} branch exists in your local repository, you can merge this merge request manually using the"
msgstr "" msgstr ""
...@@ -14477,6 +14480,9 @@ msgstr "" ...@@ -14477,6 +14480,9 @@ msgstr ""
msgid "mrWidget|The source branch will not be deleted" msgid "mrWidget|The source branch will not be deleted"
msgstr "" msgstr ""
msgid "mrWidget|The target branch has advanced, which invalidates the merge request pipeline. Please update the source branch and retry merging"
msgstr ""
msgid "mrWidget|There are merge conflicts" msgid "mrWidget|There are merge conflicts"
msgstr "" msgstr ""
......
...@@ -125,6 +125,7 @@ ...@@ -125,6 +125,7 @@
"test_reports_path": { "type": ["string", "null"] }, "test_reports_path": { "type": ["string", "null"] },
"can_receive_suggestion": { "type": "boolean" }, "can_receive_suggestion": { "type": "boolean" },
"source_branch_protected": { "type": "boolean" }, "source_branch_protected": { "type": "boolean" },
"conflicts_docs_path": { "type": ["string", "null"] } "conflicts_docs_path": { "type": ["string", "null"] },
"merge_request_pipelines_docs_path": { "type": ["string", "null"] }
} }
} }
import { shallowMount, createLocalVue } from '@vue/test-utils';
import MrWidgetAlertMessage from '~/vue_merge_request_widget/components/mr_widget_alert_message.vue';
import { GlLink } from '@gitlab/ui';
describe('MrWidgetAlertMessage', () => {
let wrapper;
beforeEach(() => {
const localVue = createLocalVue();
wrapper = shallowMount(localVue.extend(MrWidgetAlertMessage), {
propsData: {},
localVue,
});
});
afterEach(() => {
wrapper.destroy();
});
describe('when type is not provided', () => {
it('should render a red message', () => {
expect(wrapper.classes()).toContain('danger_message');
expect(wrapper.classes()).not.toContain('warning_message');
});
});
describe('when type === "danger"', () => {
it('should render a red message', () => {
wrapper.setProps({ type: 'danger' });
expect(wrapper.classes()).toContain('danger_message');
expect(wrapper.classes()).not.toContain('warning_message');
});
});
describe('when type === "warning"', () => {
it('should render a red message', () => {
wrapper.setProps({ type: 'warning' });
expect(wrapper.classes()).toContain('warning_message');
expect(wrapper.classes()).not.toContain('danger_message');
});
});
describe('when helpPath is not provided', () => {
it('should not render a help icon/link', () => {
const link = wrapper.find(GlLink);
expect(link.exists()).toBe(false);
});
});
describe('when helpPath is provided', () => {
it('should render a help icon/link', () => {
wrapper.setProps({ helpPath: '/path/to/help/docs' });
const link = wrapper.find(GlLink);
expect(link.exists()).toBe(true);
expect(link.attributes().href).toBe('/path/to/help/docs');
});
});
});
...@@ -243,6 +243,7 @@ export default { ...@@ -243,6 +243,7 @@ export default {
merge_commit_path: merge_commit_path:
'http://localhost:3000/root/acets-app/commit/53027d060246c8f47e4a9310fb332aa52f221775', 'http://localhost:3000/root/acets-app/commit/53027d060246c8f47e4a9310fb332aa52f221775',
troubleshooting_docs_path: 'help', troubleshooting_docs_path: 'help',
merge_request_pipelines_docs_path: '/help/ci/merge_request_pipelines/index.md',
squash: true, squash: true,
}; };
// Codeclimate // Codeclimate
......
...@@ -183,6 +183,85 @@ describe('mrWidgetOptions', () => { ...@@ -183,6 +183,85 @@ describe('mrWidgetOptions', () => {
}); });
}); });
}); });
describe('showMergePipelineForkWarning', () => {
describe('when the source project and target project are the same', () => {
beforeEach(done => {
Vue.set(vm.mr, 'mergePipelinesEnabled', true);
Vue.set(vm.mr, 'sourceProjectId', 1);
Vue.set(vm.mr, 'targetProjectId', 1);
vm.$nextTick(done);
});
it('should be false', () => {
expect(vm.showMergePipelineForkWarning).toEqual(false);
});
});
describe('when merge pipelines are not enabled', () => {
beforeEach(done => {
Vue.set(vm.mr, 'mergePipelinesEnabled', false);
Vue.set(vm.mr, 'sourceProjectId', 1);
Vue.set(vm.mr, 'targetProjectId', 2);
vm.$nextTick(done);
});
it('should be false', () => {
expect(vm.showMergePipelineForkWarning).toEqual(false);
});
});
describe('when merge pipelines are enabled _and_ the source project and target project are different', () => {
beforeEach(done => {
Vue.set(vm.mr, 'mergePipelinesEnabled', true);
Vue.set(vm.mr, 'sourceProjectId', 1);
Vue.set(vm.mr, 'targetProjectId', 2);
vm.$nextTick(done);
});
it('should be true', () => {
expect(vm.showMergePipelineForkWarning).toEqual(true);
});
});
});
describe('showTargetBranchAdvancedError', () => {
describe(`when the pipeline's target_sha property doesn't exist`, () => {
beforeEach(done => {
Vue.set(vm.mr.pipeline, 'target_sha', undefined);
Vue.set(vm.mr, 'targetBranchSha', 'abcd');
vm.$nextTick(done);
});
it('should be false', () => {
expect(vm.showTargetBranchAdvancedError).toEqual(false);
});
});
describe(`when the pipeline's target_sha matches the target branch's sha`, () => {
beforeEach(done => {
Vue.set(vm.mr.pipeline, 'target_sha', 'abcd');
Vue.set(vm.mr, 'targetBranchSha', 'abcd');
vm.$nextTick(done);
});
it('should be false', () => {
expect(vm.showTargetBranchAdvancedError).toEqual(false);
});
});
describe(`when the pipeline's target_sha does not match the target branch's sha`, () => {
beforeEach(done => {
Vue.set(vm.mr.pipeline, 'target_sha', 'abcd');
Vue.set(vm.mr, 'targetBranchSha', 'bcde');
vm.$nextTick(done);
});
it('should be true', () => {
expect(vm.showTargetBranchAdvancedError).toEqual(true);
});
});
});
}); });
describe('methods', () => { describe('methods', () => {
......
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