diff options
author | Filipa Lacerda <filipa@gitlab.com> | 2019-04-16 14:21:50 +0000 |
---|---|---|
committer | Filipa Lacerda <filipa@gitlab.com> | 2019-04-16 14:21:50 +0000 |
commit | cd509d991ec43b05994af1c157ef724af7f04e8f (patch) | |
tree | 811b8c65b6e825db3af4e8ca932683b10a74322c | |
parent | 2dbbe7348b4156dea1f5229fd3e5db43ccad8834 (diff) | |
parent | 12170f8320c3bbe5b6bbdbda83ac7a163826b34a (diff) | |
download | gitlab-ce-cd509d991ec43b05994af1c157ef724af7f04e8f.tar.gz |
Merge branch 'nfriend-update-merge-request-widget-for-post-merge-pipelines' into 'master'
Update merge request widget to accommodate post-merge pipelines
See merge request gitlab-org/gitlab-ce!25983
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 6b3d60a9128..45708d78886 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 be0073559b6..dffd5e70edb 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -200,12 +200,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 { @@ -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 { a { color: $gl-gray-350; diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index 0c5369ba32c..3f7b5bebb74 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -212,6 +212,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 435f2408a9e..e6e161a2b27 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10947,6 +10947,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 "" @@ -11040,6 +11043,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', () => { |