diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.js index f9cb79a0bc12d47b5da9ec3be8764a4208d42c2f..f2245a532f4e88155a72b0204a632ec6aa559871 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.js @@ -10,27 +10,40 @@ export default { }, template: `
- +
- - There are merge conflicts. - - Resolve these conflicts or ask someone with write access to this repository to merge it locally + +
`, 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 46a2c35e4e35058b877596660e8822ad2e8e28f1..dc3e557520846d952cc5d17c662675a1b8038af1 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 @@ -59,6 +59,7 @@ export default class MergeRequestStore { this.onlyAllowMergeIfPipelineSucceeds = data.only_allow_merge_if_pipeline_succeeds || false; this.mergeWhenPipelineSucceeds = data.merge_when_pipeline_succeeds || false; this.mergePath = data.merge_path; + this.ffOnlyEnabled = data.ff_only_enabled; this.statusPath = data.status_path; this.emailPatchesPath = data.email_patches_path; this.plainDiffPath = data.plain_diff_path; diff --git a/app/views/projects/_merge_request_fast_forward_settings.html.haml b/app/views/projects/_merge_request_fast_forward_settings.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..9d357293a2f0cc7ff9db12eeacbf5b4bf96f6847 --- /dev/null +++ b/app/views/projects/_merge_request_fast_forward_settings.html.haml @@ -0,0 +1,13 @@ +- form = local_assigns.fetch(:form) +- project = local_assigns.fetch(:project) + +.radio + = label_tag :project_merge_method_ff do + = form.radio_button :merge_method, :ff, class: "js-merge-method-radio" + %strong Fast-forward merge + %br + %span.descr + No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch could be fast-forwarded. + %br + %span.descr + When fast-forward merge is not possible, the user must first rebase locally. diff --git a/app/views/projects/_merge_request_settings.html.haml b/app/views/projects/_merge_request_settings.html.haml index ccbf4e70404c11555f3e12094f7589aec8a67415..939750ed1e5ba2cb4c1acb6e48d4fcfc348f3e00 100644 --- a/app/views/projects/_merge_request_settings.html.haml +++ b/app/views/projects/_merge_request_settings.html.haml @@ -1,5 +1,16 @@ - form = local_assigns.fetch(:form) -= render 'projects/ee/merge_request_settings', form: form, project: @project +.form-group + = label_tag :merge_method_merge, class: 'label-light' do + Merge method + .radio + = label_tag :project_merge_method_merge do + = form.radio_button :merge_method, :merge, class: "js-merge-method-radio" + %strong Merge commit + %br + %span.descr + A merge commit is created for every merge, and merging is allowed as long as there are no conflicts. + + = render 'merge_request_fast_forward_settings', project: @project, form: form = render 'projects/merge_request_merge_settings', form: form diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 8a4130fd526cd9ca054d62d0b04e9a3787721975..0a716159babb2afe9f85ddee191fcda27aaf89fc 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -97,7 +97,7 @@ = render 'shared/promotions/promote_mr_features' = form_for [@project.namespace.becomes(Namespace), @project], remote: true, html: { multipart: true, class: "merge-request-settings-form" }, authenticity_token: true do |f| - = render 'merge_request_settings', form: f + = render 'projects/ee/merge_request_settings', form: f = f.submit 'Save changes', class: "btn btn-save" = render 'projects/ee/service_desk_settings' diff --git a/ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index c241b375d008a3fc78b07c12dae738c627fe14ce..810469942ab3031fa1542ccf63e6305ed8ea56fe 100644 --- a/ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -28,7 +28,6 @@ export default class MergeRequestStore extends CEMergeRequestStore { this.rebaseInProgress = data.rebase_in_progress; this.approvalsLeft = !data.approved; this.rebasePath = data.rebase_path; - this.ffOnlyEnabled = data.ff_only_enabled; } initGeo(data) { diff --git a/ee/app/views/projects/ee/_merge_request_fast_forward_settings.html.haml b/ee/app/views/projects/ee/_merge_request_fast_forward_settings.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..85e0c54614060c4e562cb85614307a96e9e9223d --- /dev/null +++ b/ee/app/views/projects/ee/_merge_request_fast_forward_settings.html.haml @@ -0,0 +1,14 @@ +- form = local_assigns.fetch(:form) +- project = local_assigns.fetch(:project) + +.radio + = label_tag :project_merge_method_ff do + = form.radio_button :merge_method, :ff, class: "js-merge-method-radio" + %strong Fast-forward merge + %br + %span.descr + No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch could be fast-forwarded. + - if project.feature_available?(:merge_request_rebase) + %br + %span.descr + When fast-forward merge is not possible, the user is given the option to rebase. diff --git a/ee/app/views/projects/ee/_merge_request_rebase_settings.html.haml b/ee/app/views/projects/ee/_merge_request_rebase_settings.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..54e0b73d24c3fab8ed1c7cf91153e325803722e4 --- /dev/null +++ b/ee/app/views/projects/ee/_merge_request_rebase_settings.html.haml @@ -0,0 +1,13 @@ +- form = local_assigns.fetch(:form) + +.radio + = label_tag :project_merge_method_rebase_merge do + = form.radio_button :merge_method, :rebase_merge, class: "js-merge-method-radio" + %strong Merge commit with semi-linear history + %br + %span.descr + A merge commit is created for every merge, but merging is only allowed if fast-forward merge is possible. + This way you could make sure that if this merge request would build, after merging to target branch it would also build. + %br + %span.descr + When fast-forward merge is not possible, the user is given the option to rebase. diff --git a/ee/app/views/projects/ee/_merge_request_settings.html.haml b/ee/app/views/projects/ee/_merge_request_settings.html.haml index aef37b02fd680f88a61f6350cce89e8a11f61c30..c003f442ba09dbe4d0f1dfed8118ea299ac541e2 100644 --- a/ee/app/views/projects/ee/_merge_request_settings.html.haml +++ b/ee/app/views/projects/ee/_merge_request_settings.html.haml @@ -1,43 +1,21 @@ - form = local_assigns.fetch(:form) -- project = local_assigns.fetch(:project) -- if project.feature_available?(:merge_request_rebase) || project.feature_available?(:fast_forward_merge) - .form-group - = label_tag :merge_method_merge, class: 'label-light' do - Merge method - .radio - = label_tag :project_merge_method_merge do - = form.radio_button :merge_method, :merge, class: "js-merge-method-radio" - %strong Merge commit - %br - %span.descr - A merge commit is created for every merge, and merging is allowed as long as there are no conflicts. +.form-group + = label_tag :merge_method_merge, class: 'label-light' do + Merge method + .radio + = label_tag :project_merge_method_merge do + = form.radio_button :merge_method, :merge, class: "js-merge-method-radio" + %strong Merge commit + %br + %span.descr + A merge commit is created for every merge, and merging is allowed as long as there are no conflicts. + + - if @project.feature_available?(:merge_request_rebase) + = render 'projects/ee/merge_request_rebase_settings', project: @project, form: form - - if project.feature_available?(:merge_request_rebase) - .radio - = label_tag :project_merge_method_rebase_merge do - = form.radio_button :merge_method, :rebase_merge, class: "js-merge-method-radio" - %strong Merge commit with semi-linear history - %br - %span.descr - A merge commit is created for every merge, but merging is only allowed if fast-forward merge is possible. - This way you could make sure that if this merge request would build, after merging to target branch it would also build. - %br - %span.descr - When fast-forward merge is not possible, the user is given the option to rebase. + = render 'projects/ee/merge_request_fast_forward_settings', project: @project, form: form - - if project.feature_available?(:fast_forward_merge) - .radio - = label_tag :project_merge_method_ff do - = form.radio_button :merge_method, :ff, class: "js-merge-method-radio" - %strong Fast-forward merge - %br - %span.descr - No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch could be fast-forwarded. - - if project.feature_available?(:merge_request_rebase) - %br - %span.descr - When fast-forward merge is not possible, the user is given the option to rebase. - if @project.feature_available?(:issuable_default_templates) .form-group @@ -48,4 +26,6 @@ .hint Description parsed with #{link_to "GitLab Flavored Markdown", help_page_path('user/markdown'), target: '_blank'}. -= render 'projects/ee/merge_request_approvals_settings', project: project, form: form += render 'projects/ee/merge_request_approvals_settings', project: @project, form: form + += render 'projects/merge_request_merge_settings', form: form 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 3b7b7d936624df7b44c67cbc435f97fbd2ed85d5..d4b18703b258d646074cf95129490ca1a4e33a4d 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 @@ -2,17 +2,16 @@ import Vue from 'vue'; import conflictsComponent from '~/vue_merge_request_widget/components/states/mr_widget_conflicts'; const path = '/conflicts'; -const createComponent = () => { +const createComponent = (customConfig = {}) => { const Component = Vue.extend(conflictsComponent); + const config = Object.assign({ + mr: {}, + }, customConfig); + return new Component({ el: document.createElement('div'), - propsData: { - mr: { - canMerge: true, - conflictResolutionPath: path, - }, - }, + propsData: config, }); }; @@ -27,44 +26,78 @@ describe('MRWidgetConflicts', () => { }); describe('template', () => { - it('should have correct elements', () => { - const el = createComponent().$el; - const resolveButton = el.querySelector('.js-resolve-conflicts-button'); - const mergeButton = el.querySelector('.mr-widget-body .btn'); - const mergeLocallyButton = el.querySelector('.js-merge-locally-button'); - - expect(el.textContent).toContain('There are merge conflicts'); - expect(el.textContent).not.toContain('ask someone with write access'); - expect(el.querySelector('.btn-success').disabled).toBeTruthy(); - expect(resolveButton.textContent).toContain('Resolve conflicts'); - expect(resolveButton.getAttribute('href')).toEqual(path); - expect(mergeButton.textContent).toContain('Merge'); - expect(mergeLocallyButton.textContent).toContain('Merge locally'); + describe('when allowed to merge', () => { + let vm; + + beforeEach(() => { + vm = createComponent({ + mr: { + canMerge: true, + conflictResolutionPath: path, + }, + }); + }); + + 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'); + }); + + it('should allow you to resolve the conflicts', () => { + const resolveButton = vm.$refs.resolveConflictsButton; + + expect(resolveButton.textContent).toContain('Resolve conflicts'); + expect(resolveButton.getAttribute('href')).toEqual(path); + }); + + it('should have merge buttons', () => { + const mergeButton = vm.$refs.statusIcon.$refs.mergeButton; + const mergeLocallyButton = vm.$refs.mergeLocallyButton; + + expect(mergeButton.textContent).toContain('Merge'); + expect(mergeButton.disabled).toBeTruthy(); + expect(mergeButton.classList.contains('btn-success')).toBeTruthy(); + expect(mergeLocallyButton.textContent).toContain('Merge locally'); + }); }); describe('when user does not have permission to merge', () => { let vm; beforeEach(() => { - vm = createComponent(); - vm.mr.canMerge = false; + vm = createComponent({ + mr: { + canMerge: false, + }, + }); }); - it('should show proper message', (done) => { - Vue.nextTick(() => { - expect(vm.$el.textContent).toContain('ask someone with write access'); - done(); - }); + it('should show proper message', () => { + expect(vm.$el.textContent).toContain('ask someone with write access'); + }); + + it('should not have action buttons', () => { + expect(vm.$refs.statusIcon.$refs.mergeButton).toBeDefined(); + expect(vm.$refs.resolveConflictsButton).toBeUndefined(); + expect(vm.$refs.mergeLocallyButton).toBeUndefined(); }); + }); + + describe('when fast-forward merge enabled', () => { + let vm; - it('should not have action buttons', (done) => { - Vue.nextTick(() => { - expect(vm.$el.querySelectorAll('.btn').length).toBe(1); - expect(vm.$el.querySelector('.js-resolve-conflicts-button')).toEqual(null); - expect(vm.$el.querySelector('.js-merge-locally-button')).toEqual(null); - done(); + beforeEach(() => { + vm = createComponent({ + mr: { + ffOnlyEnabled: true, + }, }); }); + + it('should tell you to rebase locally', () => { + expect(vm.$el.textContent).toContain('Fast-forward merge is not possible.'); + expect(vm.$el.textContent).toContain('To merge this request, first rebase locally'); + }); }); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index c607c9746a4c210376a9fd951547bb0e21a090bc..2e84d8db946f2b24e7af6d12e71455bf9948141d 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -169,36 +169,6 @@ describe('MRWidgetReadyToMerge', () => { expect(vm.isMergeButtonDisabled).toBeTruthy(); }); }); - - describe('Remove source branch checkbox', () => { - describe('when user can merge but cannot delete branch', () => { - it('isRemoveSourceBranchButtonDisabled should be true', () => { - expect(vm.isRemoveSourceBranchButtonDisabled).toBe(true); - }); - - it('should be disabled in the rendered output', () => { - const checkboxElement = vm.$el.querySelector('#remove-source-branch-input'); - expect(checkboxElement.getAttribute('disabled')).toBe('disabled'); - }); - }); - - describe('when user can merge and can delete branch', () => { - beforeEach(() => { - this.customVm = createComponent({ - mr: { canRemoveSourceBranch: true }, - }); - }); - - it('isRemoveSourceBranchButtonDisabled should be false', () => { - expect(this.customVm.isRemoveSourceBranchButtonDisabled).toBe(false); - }); - - it('should be enabled in rendered output', () => { - const checkboxElement = this.customVm.$el.querySelector('#remove-source-branch-input'); - expect(checkboxElement.getAttribute('disabled')).toBeNull(); - }); - }); - }); }); describe('methods', () => { @@ -419,4 +389,54 @@ describe('MRWidgetReadyToMerge', () => { }); }); }); + + describe('Remove source branch checkbox', () => { + describe('when user can merge but cannot delete branch', () => { + it('isRemoveSourceBranchButtonDisabled should be true', () => { + expect(vm.isRemoveSourceBranchButtonDisabled).toBe(true); + }); + + it('should be disabled in the rendered output', () => { + const checkboxElement = vm.$el.querySelector('#remove-source-branch-input'); + expect(checkboxElement.getAttribute('disabled')).toBe('disabled'); + }); + }); + + describe('when user can merge and can delete branch', () => { + beforeEach(() => { + this.customVm = createComponent({ + mr: { canRemoveSourceBranch: true }, + }); + }); + + it('isRemoveSourceBranchButtonDisabled should be false', () => { + expect(this.customVm.isRemoveSourceBranchButtonDisabled).toBe(false); + }); + + it('should be enabled in rendered output', () => { + const checkboxElement = this.customVm.$el.querySelector('#remove-source-branch-input'); + expect(checkboxElement.getAttribute('disabled')).toBeNull(); + }); + }); + }); + + describe('Commit message area', () => { + it('when using merge commits, should show "Modify commit message" button', () => { + const customVm = createComponent({ + mr: { ffOnlyEnabled: false }, + }); + + expect(customVm.$refs.fastForwardMessage).toBeUndefined(); + expect(customVm.$refs.modifyCommitMessageButton).toBeDefined(); + }); + + it('when fast-forward merge is enabled, only show fast-forward message', () => { + const customVm = createComponent({ + mr: { ffOnlyEnabled: true }, + }); + + expect(customVm.$refs.fastForwardMessage).toBeDefined(); + expect(customVm.$refs.modifyCommitMessageButton).toBeUndefined(); + }); + }); });