diff options
author | Nathan Friend <nathan@gitlab.com> | 2019-04-15 16:25:48 +0300 |
---|---|---|
committer | Nathan Friend <nathan@gitlab.com> | 2019-04-15 16:25:48 +0300 |
commit | 12170f8320c3bbe5b6bbdbda83ac7a163826b34a (patch) | |
tree | b541b90a01ce8062ce5e0edd8da49e5ced6e7346 | |
parent | b99b6bb0960f749e1ba9a129be9c0365e306ed96 (diff) |
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.
13 files changed, 270 insertions, 5 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_alert_message.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_alert_message.vue new file mode 100644 index 00000000000..040315b3c66 --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_alert_message.vue @@ -0,0 +1,46 @@ +<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> diff --git a/app/assets/javascripts/vue_merge_request_widget/constants.js b/app/assets/javascripts/vue_merge_request_widget/constants.js new file mode 100644 index 00000000000..0a29d55fbd6 --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/constants.js @@ -0,0 +1,5 @@ +export const WARNING = 'warning'; +export const DANGER = 'danger'; + +export const WARNING_MESSAGE_CLASS = 'warning_message'; +export const DANGER_MESSAGE_CLASS = 'danger_message'; diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index 57c4dfbe3b7..aa4ecb0aac3 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -12,6 +12,7 @@ import WidgetMergeHelp from './components/mr_widget_merge_help.vue'; import MrWidgetPipelineContainer from './components/mr_widget_pipeline_container.vue'; import Deployment from './components/deployment.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 ClosedState from './components/states/mr_widget_closed.vue'; import MergingState from './components/states/mr_widget_merging.vue'; @@ -46,6 +47,7 @@ export default { MrWidgetPipelineContainer, Deployment, 'mr-widget-related-links': WidgetRelatedLinks, + MrWidgetAlertMessage, 'mr-widget-merged': MergedState, 'mr-widget-closed': ClosedState, 'mr-widget-merging': MergingState, @@ -110,6 +112,18 @@ export default { shouldRenderMergedPipeline() { 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: { state(newVal, oldVal) { @@ -328,6 +342,30 @@ export default { :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" /> </div> <div v-if="shouldRenderMergeHelp" class="mr-widget-footer"><mr-widget-merge-help /></div> diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 58363f632a9..5fa6ec9328c 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -28,9 +28,11 @@ export default class MergeRequestStore { this.iid = data.iid; this.title = data.title; this.targetBranch = data.target_branch; + this.targetBranchSha = data.target_branch_sha; this.sourceBranch = data.source_branch; this.sourceBranchProtected = data.source_branch_protected; this.conflictsDocsPath = data.conflicts_docs_path; + this.mergeRequestPipelinesHelpPath = data.merge_request_pipelines_docs_path; this.mergeStatus = data.merge_status; this.commitMessage = data.default_merge_commit_message; this.shortMergeCommitSha = data.short_merge_commit_sha; @@ -99,6 +101,9 @@ export default class MergeRequestStore { this.allowCollaboration = data.allow_collaboration; this.targetProjectFullPath = data.target_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 this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false; diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index d72597a6147..ff6a24be51f 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -205,12 +205,12 @@ li.note { } } -.warning_message { - border-left: 4px solid $orange-200; - color: $orange-700; +@mixin message($background-color, $border-color, $text-color) { + border-left: 4px solid $border-color; + color: $text-color; padding: 10px; margin-bottom: 10px; - background: $orange-100; + background: $background-color; padding-left: 20px; &.centered { @@ -218,6 +218,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 { a { color: $gl-gray-350; diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index 284b1ad9b55..55103c0a95c 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -209,6 +209,10 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated help_page_path('user/project/merge_requests/resolve_conflicts.md') end + def merge_request_pipelines_docs_path + help_page_path('ci/merge_request_pipelines/index.md') + end + private def cached_can_be_reverted? diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index 4831eb32c96..b130f447cce 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -256,6 +256,10 @@ class MergeRequestWidgetEntity < IssuableEntity presenter(merge_request).conflicts_docs_path end + expose :merge_request_pipelines_docs_path do |merge_request| + presenter(merge_request).merge_request_pipelines_docs_path + end + private delegate :current_user, to: :request diff --git a/changelogs/unreleased/nfriend-update-merge-request-widget-for-post-merge-pipelines.yml b/changelogs/unreleased/nfriend-update-merge-request-widget-for-post-merge-pipelines.yml new file mode 100644 index 00000000000..420c8f2923c --- /dev/null +++ b/changelogs/unreleased/nfriend-update-merge-request-widget-for-post-merge-pipelines.yml @@ -0,0 +1,5 @@ +--- +title: Add two new warning messages to the MR widget about merge request pipelines +merge_request: 25983 +author: +type: added diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9de34dd92ea..08e534ed59a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9755,6 +9755,9 @@ msgstr "" msgid "mrWidget|Fast-forward merge is not possible. To merge this request, first rebase locally." 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" msgstr "" @@ -9848,6 +9851,9 @@ msgstr "" msgid "mrWidget|The source branch will not be deleted" 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" msgstr "" diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 6b1cd60c25d..7018cb9a305 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -125,6 +125,7 @@ "test_reports_path": { "type": ["string", "null"] }, "can_receive_suggestion": { "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"] } } } diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_alert_message_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_alert_message_spec.js new file mode 100644 index 00000000000..f115cb457e5 --- /dev/null +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_alert_message_spec.js @@ -0,0 +1,63 @@ +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'); + }); + }); +}); diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index 7ab203a6011..dda16375103 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -233,6 +233,7 @@ export default { merge_commit_path: 'http://localhost:3000/root/acets-app/commit/53027d060246c8f47e4a9310fb332aa52f221775', troubleshooting_docs_path: 'help', + merge_request_pipelines_docs_path: '/help/ci/merge_request_pipelines/index.md', squash: true, }; diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index 3e8f73646c8..690fcd3e224 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -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', () => { |