diff options
7 files changed, 181 insertions, 43 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue index 27352e0b2b1..f6f445c1cef 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue @@ -1,10 +1,14 @@ <script> -import statusIcon from '../mr_widget_status_icon.vue'; +import $ from 'jquery'; +import _ from 'underscore'; +import { s__, sprintf } from '~/locale'; +import { mouseenter, debouncedMouseleave, togglePopover } from '~/shared/popover'; +import StatusIcon from '../mr_widget_status_icon.vue'; export default { name: 'MRWidgetConflicts', components: { - statusIcon, + StatusIcon, }, props: { /* TODO: This is providing all store and service down when it @@ -15,6 +19,52 @@ export default { default: () => ({}), }, }, + computed: { + popoverTitle() { + return s__( + 'mrWidget|This feature merges changes from the target branch to the source branch. You cannot use this feature since the source branch is protected.', + ); + }, + showResolveButton() { + return this.mr.conflictResolutionPath && this.mr.canMerge; + }, + showPopover() { + return this.showResolveButton && this.mr.sourceBranchProtected; + }, + }, + mounted() { + if (this.showPopover) { + const $el = $(this.$refs.popover); + + $el + .popover({ + html: true, + trigger: 'focus', + container: 'body', + placement: 'top', + template: + '<div class="popover" role="tooltip"><div class="arrow"></div><p class="popover-header"></p><div class="popover-body"></div></div>', + title: s__( + 'mrWidget|This feature merges changes from the target branch to the source branch. You cannot use this feature since the source branch is protected.', + ), + content: sprintf( + s__('mrWidget|%{link_start}Learn more about resolving conflicts%{link_end}'), + { + link_start: `<a href="${_.escape( + this.mr.conflictsDocsPath, + )}" target="_blank" rel="noopener noreferrer">`, + link_end: '</a>', + }, + false, + ), + }) + .on('mouseenter', mouseenter) + .on('mouseleave', debouncedMouseleave(300)) + .on('show.bs.popover', () => { + window.addEventListener('scroll', togglePopover.bind($el, false), { once: true }); + }); + } + }, }; </script> <template> @@ -38,13 +88,15 @@ To merge this request, first rebase locally.`) }} </span> </span> - <a - v-if="mr.canMerge && mr.conflictResolutionPath" - :href="mr.conflictResolutionPath" - class="js-resolve-conflicts-button btn btn-default btn-sm" - > - {{ s__('mrWidget|Resolve conflicts') }} - </a> + <span v-if="showResolveButton" ref="popover"> + <a + :href="mr.conflictResolutionPath" + :disabled="mr.sourceBranchProtected" + class="js-resolve-conflicts-button btn btn-default btn-sm" + > + {{ s__('mrWidget|Resolve conflicts') }} + </a> + </span> <button v-if="mr.canMerge" class="js-merge-locally-button btn btn-default btn-sm" 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 c777bcca0fa..e5a52c6a7f6 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 @@ -29,6 +29,8 @@ export default class MergeRequestStore { this.title = data.title; this.targetBranch = data.target_branch; this.sourceBranch = data.source_branch; + this.sourceBranchProtected = data.source_branch_protected; + this.conflictsDocsPath = data.conflicts_docs_path; this.mergeStatus = data.merge_status; this.commitMessage = data.merge_commit_message; this.shortMergeCommitSha = data.short_merge_commit_sha; diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index 1db6c9eff36..44b6ca299ae 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -189,6 +189,10 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated merge_request.subscribed?(current_user, merge_request.target_project) end + def conflicts_docs_path + help_page_path('user/project/merge_requests/resolve_conflicts.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 9731b52f1ad..9361c9f987b 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -11,6 +11,9 @@ class MergeRequestWidgetEntity < IssuableEntity expose :merge_user_id expose :merge_when_pipeline_succeeds expose :source_branch + expose :source_branch_protected do |merge_request| + merge_request.source_project.present? && ProtectedBranch.protected?(merge_request.source_project, merge_request.source_branch) + end expose :source_project_id expose :source_project_full_path do |merge_request| merge_request.source_project&.full_path @@ -240,6 +243,10 @@ class MergeRequestWidgetEntity < IssuableEntity expose :supports_suggestion?, as: :can_receive_suggestion + expose :conflicts_docs_path do |merge_request| + presenter(merge_request).conflicts_docs_path + end + private delegate :current_user, to: :request diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c4bd2636fb1..2f8a1c8b03d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8076,6 +8076,9 @@ msgstr[1] "" msgid "mrWidget| Please restore it or use a different %{missingBranchName} branch" msgstr "" +msgid "mrWidget|%{link_start}Learn more about resolving conflicts%{link_end}" +msgstr "" + msgid "mrWidget|%{metricsLinkStart} Memory %{metricsLinkEnd} usage %{emphasisStart} decreased %{emphasisEnd} from %{memoryFrom}MB to %{memoryTo}MB" msgstr "" @@ -8235,6 +8238,9 @@ msgstr "" msgid "mrWidget|There are unresolved discussions. Please resolve these discussions" msgstr "" +msgid "mrWidget|This feature merges changes from the target branch to the source branch. You cannot use this feature since the source branch is protected." +msgstr "" + msgid "mrWidget|This merge request failed to be merged automatically" msgstr "" diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 193ab6821a5..1bd39a46830 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -120,7 +120,9 @@ "rebase_path": { "type": ["string", "null"] }, "squash": { "type": "boolean" }, "test_reports_path": { "type": ["string", "null"] }, - "can_receive_suggestion": { "type": "boolean" } + "can_receive_suggestion": { "type": "boolean" }, + "source_branch_protected": { "type": "boolean" }, + "conflicts_docs_path": { "type": ["string", "null"] } }, "additionalProperties": false } diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js index f9cd5c8bd3c..0ddbdf67d8b 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js @@ -1,89 +1,154 @@ -import Vue from 'vue'; -import conflictsComponent from '~/vue_merge_request_widget/components/states/mr_widget_conflicts.vue'; -import mountComponent from 'spec/helpers/vue_mount_component_helper'; +import $ from 'jquery'; +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import ConflictsComponent from '~/vue_merge_request_widget/components/states/mr_widget_conflicts.vue'; import { removeBreakLine } from 'spec/helpers/vue_component_helper'; describe('MRWidgetConflicts', () => { - let Component; let vm; const path = '/conflicts'; + function createComponent(propsData = {}) { + const localVue = createLocalVue(); + + vm = shallowMount(localVue.extend(ConflictsComponent), { + propsData, + }); + } + beforeEach(() => { - Component = Vue.extend(conflictsComponent); + spyOn($.fn, 'popover').and.callThrough(); }); afterEach(() => { - vm.$destroy(); + vm.destroy(); }); describe('when allowed to merge', () => { beforeEach(() => { - vm = mountComponent(Component, { + createComponent({ mr: { canMerge: true, conflictResolutionPath: path, + conflictsDocsPath: '', }, }); }); it('should tell you about conflicts without bothering other people', () => { - expect(vm.$el.textContent).toContain('There are merge conflicts'); - expect(vm.$el.textContent).not.toContain('ask someone with write access'); + expect(vm.text()).toContain('There are merge conflicts'); + expect(vm.text()).not.toContain('ask someone with write access'); }); it('should allow you to resolve the conflicts', () => { - const resolveButton = vm.$el.querySelector('.js-resolve-conflicts-button'); + const resolveButton = vm.find('.js-resolve-conflicts-button'); - expect(resolveButton.textContent).toContain('Resolve conflicts'); - expect(resolveButton.getAttribute('href')).toEqual(path); + expect(resolveButton.text()).toContain('Resolve conflicts'); + expect(resolveButton.attributes('href')).toEqual(path); }); it('should have merge buttons', () => { - const mergeButton = vm.$el.querySelector('.js-disabled-merge-button'); - const mergeLocallyButton = vm.$el.querySelector('.js-merge-locally-button'); + const mergeLocallyButton = vm.find('.js-merge-locally-button'); - expect(mergeButton.textContent).toContain('Merge'); - expect(mergeButton.disabled).toBeTruthy(); - expect(mergeButton.classList.contains('btn-success')).toEqual(true); - expect(mergeLocallyButton.textContent).toContain('Merge locally'); + expect(mergeLocallyButton.text()).toContain('Merge locally'); }); }); describe('when user does not have permission to merge', () => { - beforeEach(() => { - vm = mountComponent(Component, { + it('should show proper message', () => { + createComponent({ mr: { canMerge: false, + conflictsDocsPath: '', }, }); - }); - it('should show proper message', () => { - expect(vm.$el.textContent.trim().replace(/\s\s+/g, ' ')).toContain( - 'ask someone with write access', - ); + expect( + vm + .text() + .trim() + .replace(/\s\s+/g, ' '), + ).toContain('ask someone with write access'); }); it('should not have action buttons', () => { - expect(vm.$el.querySelector('.js-disabled-merge-button')).toBeDefined(); - expect(vm.$el.querySelector('.js-resolve-conflicts-button')).toBeNull(); - expect(vm.$el.querySelector('.js-merge-locally-button')).toBeNull(); + createComponent({ + mr: { + canMerge: false, + conflictsDocsPath: '', + }, + }); + + expect(vm.contains('.js-resolve-conflicts-button')).toBe(false); + expect(vm.contains('.js-merge-locally-button')).toBe(false); + }); + + it('should not have resolve button when no conflict resolution path', () => { + createComponent({ + mr: { + canMerge: true, + conflictResolutionPath: null, + conflictsDocsPath: '', + }, + }); + + expect(vm.contains('.js-resolve-conflicts-button')).toBe(false); }); }); describe('when fast-forward or semi-linear merge enabled', () => { - beforeEach(() => { - vm = mountComponent(Component, { + it('should tell you to rebase locally', () => { + createComponent({ mr: { shouldBeRebased: true, + conflictsDocsPath: '', }, }); - }); - it('should tell you to rebase locally', () => { - expect(removeBreakLine(vm.$el.textContent).trim()).toContain( + expect(removeBreakLine(vm.text()).trim()).toContain( 'Fast-forward merge is not possible. To merge this request, first rebase locally.', ); }); }); + + describe('when source branch protected', () => { + beforeEach(() => { + createComponent({ + mr: { + canMerge: true, + conflictResolutionPath: gl.TEST_HOST, + sourceBranchProtected: true, + conflictsDocsPath: '', + }, + }); + }); + + it('sets resolve button as disabled', () => { + expect(vm.find('.js-resolve-conflicts-button').attributes('disabled')).toBe('disabled'); + }); + + it('renders popover', () => { + expect($.fn.popover).toHaveBeenCalled(); + }); + }); + + describe('when source branch not protected', () => { + beforeEach(() => { + createComponent({ + mr: { + canMerge: true, + conflictResolutionPath: gl.TEST_HOST, + sourceBranchProtected: false, + conflictsDocsPath: '', + }, + }); + }); + + it('sets resolve button as disabled', () => { + expect(vm.find('.js-resolve-conflicts-button').attributes('disabled')).toBe(undefined); + }); + + it('renders popover', () => { + expect($.fn.popover).not.toHaveBeenCalled(); + }); + }); }); |