diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml index f1ed1311195525855b1aefb04ea75b9cbdf094c4..0c416d57b7587295a7d162cb83f70472f457a6df 100644 --- a/app/views/groups/edit.html.haml +++ b/app/views/groups/edit.html.haml @@ -28,6 +28,7 @@ .settings-content = render 'groups/settings/permissions' += render_if_exists 'groups/merge_requests', expanded: expanded, group: @group = render_if_exists 'groups/merge_request_approval_settings', expanded: expanded, group: @group, user: current_user = render_if_exists 'groups/insights', expanded: expanded = render_if_exists 'groups/analytics_dashboards', expanded: expanded diff --git a/app/views/projects/_merge_request_pipelines_and_threads_options.html.haml b/app/views/projects/_merge_request_pipelines_and_threads_options.html.haml index 94f8d3cc4a387e9f3d472a9048ad87597d25a154..a9609434f153f52b51327bc481e2d1b832fba0fa 100644 --- a/app/views/projects/_merge_request_pipelines_and_threads_options.html.haml +++ b/app/views/projects/_merge_request_pipelines_and_threads_options.html.haml @@ -1,13 +1,13 @@ - form = local_assigns.fetch(:form) = form.gitlab_ui_checkbox_component :only_allow_merge_if_pipeline_succeeds, - s_('ProjectSettings|Pipelines must succeed'), - help_text: s_("ProjectSettings|Merge requests can't be merged if the latest pipeline did not succeed or is still running.") + s_('MergeChecks|Pipelines must succeed'), + help_text: s_("MergeChecks|Merge requests can't be merged if the latest pipeline did not succeed or is still running.") .gl-pl-6 = form.gitlab_ui_checkbox_component :allow_merge_on_skipped_pipeline, - s_('ProjectSettings|Skipped pipelines are considered successful'), - help_text: s_('ProjectSettings|Introduces the risk of merging changes that do not pass the pipeline.'), + s_('MergeChecks|Skipped pipelines are considered successful'), + help_text: s_('MergeChecks|Introduces the risk of merging changes that do not pass the pipeline.'), checkbox_options: { class: 'gl-pl-6' } = form.gitlab_ui_checkbox_component :only_allow_merge_if_all_discussions_are_resolved, - s_('ProjectSettings|All threads must be resolved'), - checkbox_options: { data: { qa_selector: 'allow_merge_if_all_discussions_are_resolved_checkbox' } } + s_('MergeChecks|All threads must be resolved'), + checkbox_options: { data: { qa_selector: 'only_allow_merge_if_all_discussions_are_resolved_checkbox' } } diff --git a/ee/app/assets/javascripts/approvals/components/approval_settings_checkbox.vue b/ee/app/assets/javascripts/approvals/components/approval_settings_checkbox.vue index 164e55450eb456d6e551c02c580278d0d78dbb6d..dbd322b751a2524b9c3bdf8a1f2f45de86d2e0df 100644 --- a/ee/app/assets/javascripts/approvals/components/approval_settings_checkbox.vue +++ b/ee/app/assets/javascripts/approvals/components/approval_settings_checkbox.vue @@ -40,5 +40,8 @@ export default { {{ label }} + diff --git a/ee/app/assets/javascripts/approvals/components/approval_settings_locked_icon.vue b/ee/app/assets/javascripts/approvals/components/approval_settings_locked_icon.vue index b536edffca8da75a46c79857f6644d2e7d1b0815..bc3baa3c298ce661567588cdeb648fb533bd6912 100644 --- a/ee/app/assets/javascripts/approvals/components/approval_settings_locked_icon.vue +++ b/ee/app/assets/javascripts/approvals/components/approval_settings_locked_icon.vue @@ -1,6 +1,6 @@ + diff --git a/ee/app/assets/javascripts/merge_checks/constants.js b/ee/app/assets/javascripts/merge_checks/constants.js new file mode 100644 index 0000000000000000000000000000000000000000..d791213a97351d58b15060cf57e16ad95757bacd --- /dev/null +++ b/ee/app/assets/javascripts/merge_checks/constants.js @@ -0,0 +1,19 @@ +import { s__ } from '~/locale'; + +export const I18N = { + pipelineMustSucceed: { + label: s__('MergeChecks|Pipelines must succeed'), + help: s__( + "MergeChecks|Merge requests can't be merged if the latest pipeline did not succeed or is still running.", + ), + }, + allowMergeOnSkipped: { + label: s__('MergeChecks|Skipped pipelines are considered successful'), + help: s__('MergeChecks|Introduces the risk of merging changes that do not pass the pipeline.'), + }, + onlyMergeWhenAllResolvedLabel: s__('MergeChecks|All threads must be resolved'), + lockedText: s__( + 'MergeChecks|This setting is configured in group %{groupName} and can only be changed in the group settings by an administrator or group owner.', + ), + lockedUponPipelineMustSucceed: s__('MergeChecks|Enable "Pipelines must succeed" first.'), +}; diff --git a/ee/app/assets/javascripts/merge_checks/index.js b/ee/app/assets/javascripts/merge_checks/index.js new file mode 100644 index 0000000000000000000000000000000000000000..dfcd4966013522b81e03eff080516125a595260d --- /dev/null +++ b/ee/app/assets/javascripts/merge_checks/index.js @@ -0,0 +1,36 @@ +import Vue from 'vue'; +import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; + +export const initMergeRequestMergeChecksApp = async () => { + const el = document.querySelector('.js-merge-request-merge-checks'); + + if (!el) { + return false; + } + + const { default: MergeChecksApp } = await import( + /* webpackChunkName: 'mergeChecksApp' */ './components/merge_checks_app.vue' + ); + const { sourceType, settings, groupName } = el.dataset; + + const { + allowMergeOnSkippedPipeline, + onlyAllowMergeIfAllResolved, + pipelineMustSucceed, + } = convertObjectPropsToCamelCase(JSON.parse(settings)); + + return new Vue({ + el, + name: 'MergeChecksRoot', + provide: { + groupName, + sourceType, + allowMergeOnSkippedPipeline, + onlyAllowMergeIfAllResolved, + pipelineMustSucceed, + }, + render(createElement) { + return createElement(MergeChecksApp); + }, + }); +}; diff --git a/ee/app/assets/javascripts/pages/groups/edit/index.js b/ee/app/assets/javascripts/pages/groups/edit/index.js index 4540711abec9a2fcfac9fdfff0db622eeccec428..80f8c010f54fbe2b7b9abab072aa7a0482cb3690 100644 --- a/ee/app/assets/javascripts/pages/groups/edit/index.js +++ b/ee/app/assets/javascripts/pages/groups/edit/index.js @@ -4,6 +4,7 @@ import validateRestrictedIpAddress from 'ee/groups/settings/access_restriction_f import { initGroupPermissionsFormSubmit } from 'ee/groups/settings/permissions'; import { initServicePingSettingsClickTracking } from 'ee/registration_features_discovery_message'; import { createAlert } from '~/flash'; +import { initMergeRequestMergeChecksApp } from 'ee/merge_checks'; import { __ } from '~/locale'; if (gon.features.saasUserCapsAutoApprovePendingUsersOnCapIncrease) { @@ -61,3 +62,4 @@ if (mergeRequestApprovalSetting) { } initServicePingSettingsClickTracking(); +initMergeRequestMergeChecksApp(); diff --git a/ee/app/assets/javascripts/pages/projects/settings/merge_requests/index.js b/ee/app/assets/javascripts/pages/projects/settings/merge_requests/index.js index d025783689b8c41ec50525f12c37dfbf79d1c12d..fa13b027aa48752e11d83e8397ee31294d3a257e 100644 --- a/ee/app/assets/javascripts/pages/projects/settings/merge_requests/index.js +++ b/ee/app/assets/javascripts/pages/projects/settings/merge_requests/index.js @@ -1,9 +1,11 @@ import '~/pages/projects/settings/merge_requests'; import mountApprovals from 'ee/approvals/mount_project_settings'; import { initMergeOptionSettings } from 'ee/pages/projects/edit/merge_options'; +import { initMergeRequestMergeChecksApp } from 'ee/merge_checks'; import mountStatusChecks from 'ee/status_checks/mount'; mountApprovals(document.getElementById('js-mr-approvals-settings')); mountStatusChecks(document.getElementById('js-status-checks-settings')); initMergeOptionSettings(); +initMergeRequestMergeChecksApp(); diff --git a/ee/app/helpers/merge_checks_helper.rb b/ee/app/helpers/merge_checks_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..f087a801407cd6dcaa2a634ff4f86bb44ba5d884 --- /dev/null +++ b/ee/app/helpers/merge_checks_helper.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module MergeChecksHelper + def merge_checks(source) + case source + when Group + target = source.namespace_settings + source_type = 'namespace_setting' + group_name = source.name + when Project + target = source + source_type = 'project' + # When the project has no group it should be an empty strings + group_name = source.group.nil? ? '' : source.group.name + end + + { + source_type: source_type, + settings: { + pipeline_must_succeed: { + locked: target.only_allow_merge_if_pipeline_succeeds_locked?, + value: target.only_allow_merge_if_pipeline_succeeds?(inherit_group_setting: true) + }, + allow_merge_on_skipped_pipeline: { + locked: target.allow_merge_on_skipped_pipeline_locked?, + value: target.allow_merge_on_skipped_pipeline?(inherit_group_setting: true) + }, + only_allow_merge_if_all_resolved: { + locked: target.only_allow_merge_if_all_discussions_are_resolved_locked?, + value: target.only_allow_merge_if_all_discussions_are_resolved?(inherit_group_setting: true) + } + }.to_json, + group_name: group_name + } + end +end diff --git a/ee/app/views/groups/_merge_requests.html.haml b/ee/app/views/groups/_merge_requests.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..3fbde12a8425e117b6c1f5fa1cbf9bf3043bf7a5 --- /dev/null +++ b/ee/app/views/groups/_merge_requests.html.haml @@ -0,0 +1,10 @@ +- if Feature.enabled?(:support_group_level_merge_checks_setting, @group) && @group.licensed_feature_available?(:group_level_merge_checks_setting) + %section.settings.no-animate#js-merge-requests-settings{ class: ('expanded' if expanded) } + .settings-header + %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' } + = _('Merge requests') + = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do + = expanded ? _('Collapse') : _('Expand') + %p= s_('GroupSettings|Choose the merge request checks for projects in this group. This setting overrides the same settings configured on each project in this group.') + .settings-content + = render 'groups/settings/merge_request_pipelines_and_threads_options' diff --git a/ee/app/views/groups/settings/_merge_request_pipelines_and_threads_options.html.haml b/ee/app/views/groups/settings/_merge_request_pipelines_and_threads_options.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..e4887be42aaa2c73d20dd81d953a5321531df377 --- /dev/null +++ b/ee/app/views/groups/settings/_merge_request_pipelines_and_threads_options.html.haml @@ -0,0 +1,10 @@ += gitlab_ui_form_for @group.namespace_settings, url: group_settings_merge_requests_path(@group), html: { multipart: true, class: 'merge-request-settings-form js-mr-settings-form' }, authenticity_token: true do |f| + %input{ name: 'update_section', type: 'hidden', value: 'js-merge-request-settings' } + + %fieldset.form-group.gl-form-group + %h5= s_('ProjectSettings|Merge checks') + %p.text-secondary= s_('ProjectSettings|These checks must pass before merge requests can be merged.') + + .js-merge-request-merge-checks{ data: merge_checks(@group) } + + = f.submit _('Save changes'), data: { testid: 'save_merge_request_changes' }, pajamas_button: true diff --git a/ee/app/views/projects/_merge_request_pipelines_and_threads_options.html.haml b/ee/app/views/projects/_merge_request_pipelines_and_threads_options.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..a15e1ae2018476e6821c447d5ce8278894dcbf84 --- /dev/null +++ b/ee/app/views/projects/_merge_request_pipelines_and_threads_options.html.haml @@ -0,0 +1,6 @@ +- support_group_level_mr_setting = Feature.enabled?(:support_group_level_merge_checks_setting, @project) && @project.licensed_feature_available?(:group_level_merge_checks_setting) + +- if !support_group_level_mr_setting + = render_ce 'projects/merge_request_pipelines_and_threads_options', form: form, project: project +- else + .js-merge-request-merge-checks{ data: merge_checks(@project) } diff --git a/ee/spec/features/groups/settings/merge_requests_settings_spec.rb b/ee/spec/features/groups/settings/merge_requests_settings_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..6b95c1edfe11c53c5981ee42c69d81618195ee90 --- /dev/null +++ b/ee/spec/features/groups/settings/merge_requests_settings_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Group settings > [EE] General', feature_category: :code_review_workflow do + let_it_be(:group) { create(:group) } + let_it_be(:sub_group) { create(:group, parent: group) } + let(:merge_requests_settings_path) { edit_group_path(sub_group) } + + it_behaves_like 'MR checks settings' +end diff --git a/ee/spec/features/projects/settings/merge_requests_settings_spec.rb b/ee/spec/features/projects/settings/merge_requests_settings_spec.rb index d8e51b7fd2541e402d45f053600bd78c0805d30f..5ee220a4bbc5857fae4a81b93c24c044d2961c81 100644 --- a/ee/spec/features/projects/settings/merge_requests_settings_spec.rb +++ b/ee/spec/features/projects/settings/merge_requests_settings_spec.rb @@ -5,14 +5,14 @@ include GitlabRoutingHelper let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project) } let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :public, namespace: user.namespace, path: 'gitlab', name: 'sample', group: group) } let_it_be(:group_member) { create(:user) } before do sign_in(user) project.add_maintainer(user) - group.add_developer(user) + group.add_owner(user) group.add_developer(group_member) end @@ -130,4 +130,10 @@ end end end + + context 'MR checks' do + let(:merge_requests_settings_path) { project_settings_merge_requests_path(project) } + + it_behaves_like 'MR checks settings' + end end diff --git a/ee/spec/frontend/approvals/components/approval_settings_checkbox_spec.js b/ee/spec/frontend/approvals/components/approval_settings_checkbox_spec.js index 81c1e132017e7d562c8414b0bffb30d585bdd554..0a4ea5c2acd1d156bd5b66ce5f9a2293d54d4e0e 100644 --- a/ee/spec/frontend/approvals/components/approval_settings_checkbox_spec.js +++ b/ee/spec/frontend/approvals/components/approval_settings_checkbox_spec.js @@ -1,10 +1,8 @@ import { GlFormCheckbox } from '@gitlab/ui'; -import { shallowMount } from '@vue/test-utils'; import ApprovalSettingsCheckbox from 'ee/approvals/components/approval_settings_checkbox.vue'; import ApprovalSettingsLockedIcon from 'ee/approvals/components/approval_settings_locked_icon.vue'; -import { stubComponent } from 'helpers/stub_component'; -import { extendedWrapper } from 'helpers/vue_test_utils_helper'; +import { shallowMountExtended, mountExtended } from 'helpers/vue_test_utils_helper'; describe('ApprovalSettingsCheckbox', () => { const label = 'Foo'; @@ -12,17 +10,17 @@ describe('ApprovalSettingsCheckbox', () => { let wrapper; - const createWrapper = (props = {}) => { - wrapper = extendedWrapper( - shallowMount(ApprovalSettingsCheckbox, { - propsData: { label, lockedText, ...props }, - stubs: { - GlFormCheckbox: stubComponent(GlFormCheckbox, { - props: ['checked'], - }), + const createWrapper = ({ mountFn = shallowMountExtended, props = {}, slots = {} } = {}) => { + wrapper = mountFn(ApprovalSettingsCheckbox, { + propsData: { label, lockedText, ...props }, + slots, + stubs: { + GlFormCheckbox: { + ...GlFormCheckbox, + props: ['checked', 'disabled'], }, - }), - ); + }, + }); }; const findCheckbox = () => wrapper.findComponent(GlFormCheckbox); @@ -50,7 +48,7 @@ describe('ApprovalSettingsCheckbox', () => { }); it('sets the checkbox to `true` when checked is `true`', () => { - createWrapper({ checked: true }); + createWrapper({ props: { checked: true } }); expect(findCheckbox().props('checked')).toBe(true); }); @@ -81,11 +79,11 @@ describe('ApprovalSettingsCheckbox', () => { describe('when the setting is locked', () => { beforeEach(() => { - createWrapper({ locked: true }); + createWrapper({ props: { locked: true } }); }); it('disables the input', () => { - expect(findCheckbox().attributes('disabled')).toBe('disabled'); + expect(findCheckbox().props('disabled')).toBe(true); }); it('renders locked_icon', () => { @@ -98,4 +96,18 @@ describe('ApprovalSettingsCheckbox', () => { }); }); }); + + describe('#slot', () => { + it('should render a default slot', () => { + const slotContent = 'test slot content'; + createWrapper({ + mountFn: mountExtended, + slots: { + help: slotContent, + }, + }); + + expect(wrapper.text()).toContain(slotContent); + }); + }); }); diff --git a/ee/spec/frontend/approvals/components/approval_settings_locked_icon_spec.js b/ee/spec/frontend/approvals/components/approval_settings_locked_icon_spec.js index 853e567866f00c932f5781c819a2c05dcef3a8c9..4471ca46b101edb116e4bcfbc35904d5e48b30c9 100644 --- a/ee/spec/frontend/approvals/components/approval_settings_locked_icon_spec.js +++ b/ee/spec/frontend/approvals/components/approval_settings_locked_icon_spec.js @@ -3,12 +3,9 @@ import { shallowMount } from '@vue/test-utils'; import ApprovalSettingsLockedIcon from 'ee/approvals/components/approval_settings_locked_icon.vue'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; -import { slugify } from '~/lib/utils/text_utility'; describe('ApprovalSettingsLockedIcon', () => { const label = 'Foo'; - const lockIconId = `approval-settings-lock-icon-${slugify(label)}`; - let wrapper; const createWrapper = (props = {}) => { @@ -36,11 +33,11 @@ describe('ApprovalSettingsLockedIcon', () => { it('shows a lock icon', () => { expect(findLockIcon().props('name')).toBe('lock'); - expect(findLockIcon().attributes('id')).toBe(lockIconId); + expect(findLockIcon().attributes('id')).toMatch(/approval-settings-lock-icon-\d+/); }); it('shows a popover for the lock icon', () => { - expect(findPopover().props('target')).toBe(lockIconId); + expect(findPopover().props('target')).toBe(findLockIcon().attributes('id')); }); it('configures how and when the popover should show', () => { diff --git a/ee/spec/frontend/merge_checks/components/merge_checks_app_spec.js b/ee/spec/frontend/merge_checks/components/merge_checks_app_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..911b907ebefaebc93a6a5db286c7988f1e864970 --- /dev/null +++ b/ee/spec/frontend/merge_checks/components/merge_checks_app_spec.js @@ -0,0 +1,134 @@ +import { sprintf } from '~/locale'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import MergeChecksApp from 'ee/merge_checks/components/merge_checks_app.vue'; +import { I18N } from 'ee/merge_checks/constants'; + +describe('MergeChecksApp', () => { + const defaultType = 'project'; + const defaultGroupName = 'test'; + const defaultCheckValue = { + locked: false, + value: false, + }; + + let wrapper; + + const createWrapper = (provides = {}) => { + wrapper = shallowMountExtended(MergeChecksApp, { + provide: { + sourceType: defaultType, + groupName: defaultGroupName, + pipelineMustSucceed: defaultCheckValue, + allowMergeOnSkippedPipeline: defaultCheckValue, + onlyAllowMergeIfAllResolved: defaultCheckValue, + ...provides, + }, + }); + }; + + const findOnlyAllowMergeWhenPipelineSucceeds = () => + wrapper.findByTestId('allow_merge_if_pipeline_succeeds_checkbox'); + + const findOnlyAllowMergeWhenPipelineSucceedsInput = () => + wrapper.find('input[name$="[only_allow_merge_if_pipeline_succeeds]"]'); + + describe('sourceType', () => { + it('with default sourceType', () => { + createWrapper(); + expect(findOnlyAllowMergeWhenPipelineSucceedsInput().attributes('name')).toBe( + `${defaultType}[only_allow_merge_if_pipeline_succeeds]`, + ); + }); + + it('with other sourceType', () => { + const sourceType = 'new-type'; + createWrapper({ + sourceType, + }); + + expect(findOnlyAllowMergeWhenPipelineSucceedsInput().attributes('name')).toBe( + `${sourceType}[only_allow_merge_if_pipeline_succeeds]`, + ); + }); + }); + + describe('group name', () => { + it('with default groupName', () => { + createWrapper(); + + expect(findOnlyAllowMergeWhenPipelineSucceeds().attributes('lockedtext')).toBe( + sprintf(I18N.lockedText, { groupName: defaultGroupName }), + ); + }); + + it('with other groupName', () => { + const groupName = 'new-name'; + createWrapper({ groupName }); + + expect(findOnlyAllowMergeWhenPipelineSucceeds().attributes('lockedtext')).toBe( + sprintf(I18N.lockedText, { groupName }), + ); + }); + }); + + describe('binding value', () => { + it('should be able to bind and mutate value', async () => { + createWrapper(); + const checkbox = findOnlyAllowMergeWhenPipelineSucceeds(); + + expect(checkbox.props('checked')).toBe(defaultCheckValue.value); + + await checkbox.vm.$emit('input'); + expect(checkbox.props('checked')).toBe(true); + await checkbox.vm.$emit('input'); + expect(checkbox.props('checked')).toBe(false); + }); + }); + + describe('skipped pipeline', () => { + const findSkippedPipelineCheckbox = () => + wrapper.findByTestId('allow_merge_on_skipped_pipeline_checkbox'); + + it('should be corresponding with pipeline must succeeds', () => { + createWrapper(); + + expect(findSkippedPipelineCheckbox().props('locked')).toBe(true); + expect(findSkippedPipelineCheckbox().props('lockedText')).toBe( + I18N.lockedUponPipelineMustSucceed, + ); + + createWrapper({ + pipelineMustSucceed: { + value: true, + locked: false, + }, + }); + + expect(findSkippedPipelineCheckbox().props('locked')).toBe(false); + expect(findSkippedPipelineCheckbox().props('lockedText')).toBe( + sprintf(I18N.lockedText, { groupName: defaultGroupName }), + ); + }); + + it('should be locked and unchecked when preceding condition unmet', async () => { + createWrapper({ + pipelineMustSucceed: { + value: true, + locked: false, + }, + allowMergeOnSkippedPipeline: { + value: true, + locked: false, + }, + }); + + expect(findSkippedPipelineCheckbox().props('locked')).toBe(false); + expect(findSkippedPipelineCheckbox().props('checked')).toBe(true); + + await findOnlyAllowMergeWhenPipelineSucceeds().vm.$emit('input'); + + expect(findSkippedPipelineCheckbox().props('locked')).toBe(true); + expect(findSkippedPipelineCheckbox().props('checked')).toBe(false); + }); + }); +}); diff --git a/ee/spec/helpers/merge_checks_helper_spec.rb b/ee/spec/helpers/merge_checks_helper_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..853fbcb1b4ad570dd21b8224e2331d24f5c40c21 --- /dev/null +++ b/ee/spec/helpers/merge_checks_helper_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeChecksHelper, type: :helper, feature_category: :code_review_workflow do + let_it_be(:group) { create_default(:group, :public) } + let_it_be(:project) { create_default(:project, group: group) } + + before do + group.namespace_settings.update!(allow_merge_on_skipped_pipeline: true) + stub_licensed_features(group_level_merge_checks_setting: true) + end + + describe '#merge_checks' do + context 'when source is group' do + let(:source) { group } + + it 'returns the correct settings' do + expect(helper.merge_checks(source)).to eq({ + source_type: 'namespace_setting', + settings: { + pipeline_must_succeed: { + locked: false, + value: false + }, + allow_merge_on_skipped_pipeline: { + locked: false, + value: true + }, + only_allow_merge_if_all_resolved: { + locked: false, + value: false + } + }.to_json, + group_name: group.name + }) + end + end + + context 'when source is project' do + let(:source) { project } + + it 'returns the correct settings' do + expect(helper.merge_checks(source)).to eq({ + source_type: 'project', + settings: { + pipeline_must_succeed: { + locked: false, + value: false + }, + allow_merge_on_skipped_pipeline: { + locked: true, + value: true + }, + only_allow_merge_if_all_resolved: { + locked: false, + value: false + } + }.to_json, + group_name: group.name + }) + end + end + end +end diff --git a/ee/spec/support/shared_examples/features/merge_requests_settings_shared_examples.rb b/ee/spec/support/shared_examples/features/merge_requests_settings_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..5f4e248007e75ddccb58a8e515386365c053069c --- /dev/null +++ b/ee/spec/support/shared_examples/features/merge_requests_settings_shared_examples.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +RSpec.shared_examples_for 'MR checks settings' do + let_it_be(:user) { create(:user) } + + before do + sign_in(user) + group.add_owner(user) + stub_licensed_features(group_level_merge_checks_setting: true) + end + + context 'when checkboxes are not locked', :js do + it 'shows initial status' do + visit(merge_requests_settings_path) + + expect(page).not_to have_selector( + '[data-testid="allow_merge_if_pipeline_succeeds_checkbox"]>input[disabled]' + ) + expect(page).to have_selector('[data-testid="allow_merge_on_skipped_pipeline_checkbox"]>input[disabled]') + expect(page).not_to have_selector( + '[data-testid="allow_merge_if_all_discussions_are_resolved_checkbox"]>input[disabled]' + ) + end + end + + context 'when checkboxes are locked', :js do + before do + group.namespace_settings.update!( + only_allow_merge_if_pipeline_succeeds: true, + allow_merge_on_skipped_pipeline: true, + only_allow_merge_if_all_discussions_are_resolved: true + ) + end + + it 'shows disabled status' do + checkboxs_selectors = %w[ + [data-testid="allow_merge_if_pipeline_succeeds_checkbox"]>input[disabled] + [data-testid="allow_merge_on_skipped_pipeline_checkbox"]>input[disabled] + [data-testid="allow_merge_if_all_discussions_are_resolved_checkbox"]>input[disabled] + ] + + visit(merge_requests_settings_path) + + checkboxs_selectors.each do |selector| + expect(page).to have_selector(selector) + expect(page.find(selector)).to be_checked + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4d3a73dc3a6740219e2636fe36c89df376c1d7d6..1b643d9b9421bd84ee41fc37d124493f88ddeb1b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -20108,6 +20108,9 @@ msgstr "" msgid "GroupSettings|Choose a group path that does not start with a dash or end with a period. It can also contain alphanumeric characters and underscores." msgstr "" +msgid "GroupSettings|Choose the merge request checks for projects in this group. This setting overrides the same settings configured on each project in this group." +msgstr "" + msgid "GroupSettings|Compliance frameworks" msgstr "" @@ -26548,6 +26551,27 @@ msgstr "" msgid "Merge..." msgstr "" +msgid "MergeChecks|All threads must be resolved" +msgstr "" + +msgid "MergeChecks|Enable \"Pipelines must succeed\" first." +msgstr "" + +msgid "MergeChecks|Introduces the risk of merging changes that do not pass the pipeline." +msgstr "" + +msgid "MergeChecks|Merge requests can't be merged if the latest pipeline did not succeed or is still running." +msgstr "" + +msgid "MergeChecks|Pipelines must succeed" +msgstr "" + +msgid "MergeChecks|Skipped pipelines are considered successful" +msgstr "" + +msgid "MergeChecks|This setting is configured in group %{groupName} and can only be changed in the group settings by an administrator or group owner." +msgstr "" + msgid "MergeConflict|Commit to source branch" msgstr "" @@ -33475,9 +33499,6 @@ msgstr "" msgid "ProjectSettings|All merge requests and commits are made against this branch unless you specify a different one." msgstr "" -msgid "ProjectSettings|All threads must be resolved" -msgstr "" - msgid "ProjectSettings|Allow" msgstr "" @@ -33628,9 +33649,6 @@ msgstr "" msgid "ProjectSettings|Internal" msgstr "" -msgid "ProjectSettings|Introduces the risk of merging changes that do not pass the pipeline." -msgstr "" - msgid "ProjectSettings|Issues" msgstr "" @@ -33673,9 +33691,6 @@ msgstr "" msgid "ProjectSettings|Merge requests approved for merge are queued, and pipelines validate the combined results of the source and target branches before merge. %{link_start}What are merge trains?%{link_end}" msgstr "" -msgid "ProjectSettings|Merge requests can't be merged if the latest pipeline did not succeed or is still running." -msgstr "" - msgid "ProjectSettings|Merge suggestions" msgstr "" @@ -33712,9 +33727,6 @@ msgstr "" msgid "ProjectSettings|Pages for project documentation." msgstr "" -msgid "ProjectSettings|Pipelines must succeed" -msgstr "" - msgid "ProjectSettings|Prevents direct linking to potentially sensitive media files" msgstr "" @@ -33784,9 +33796,6 @@ msgstr "" msgid "ProjectSettings|Show link to create or view a merge request when pushing from the command line" msgstr "" -msgid "ProjectSettings|Skipped pipelines are considered successful" -msgstr "" - msgid "ProjectSettings|Snippets" msgstr "" diff --git a/qa/qa/ee/page/project/settings/merge_request.rb b/qa/qa/ee/page/project/settings/merge_request.rb index d32cb2cba669b21fdec1cc0ac2133ea85be23505..588e5d893cc67d45f4677ef4d5b5677782ef3f81 100644 --- a/qa/qa/ee/page/project/settings/merge_request.rb +++ b/qa/qa/ee/page/project/settings/merge_request.rb @@ -25,6 +25,10 @@ def self.prepended(base) view 'ee/app/views/projects/_merge_trains_settings.html.haml' do element :merge_trains_checkbox end + + view 'ee/app/assets/javascripts/merge_checks/components/merge_checks_app.vue' do + element :only_allow_merge_if_all_discussions_are_resolved_checkbox + end end end diff --git a/qa/qa/page/project/settings/merge_request.rb b/qa/qa/page/project/settings/merge_request.rb index ae6a04028c9d1ae4781627a42765b3fc5386cf30..c42a4e0bebed3e5e19e90a5b2125df1f6cc07c86 100644 --- a/qa/qa/page/project/settings/merge_request.rb +++ b/qa/qa/page/project/settings/merge_request.rb @@ -16,7 +16,7 @@ class MergeRequest < QA::Page::Base end view 'app/views/projects/_merge_request_pipelines_and_threads_options.html.haml' do - element :allow_merge_if_all_discussions_are_resolved_checkbox + element :only_allow_merge_if_all_discussions_are_resolved_checkbox end def click_save_changes @@ -29,7 +29,7 @@ def enable_ff_only end def enable_merge_if_all_disscussions_are_resolved - check_element(:allow_merge_if_all_discussions_are_resolved_checkbox, true) + check_element(:only_allow_merge_if_all_discussions_are_resolved_checkbox, true) click_save_changes end end