Commit 5025d85b authored by Jiaan Louw's avatar Jiaan Louw Committed by Andrew Fontaine

Update the project approval settings section to Vue

parent 1b85b853
...@@ -25,10 +25,6 @@ initProjectLoadingSpinner(); ...@@ -25,10 +25,6 @@ initProjectLoadingSpinner();
initProjectPermissionsSettings(); initProjectPermissionsSettings();
setupTransferEdit('.js-project-transfer-form', 'select.select2'); setupTransferEdit('.js-project-transfer-form', 'select.select2');
dirtySubmitFactory( dirtySubmitFactory(document.querySelectorAll('.js-general-settings-form, .js-mr-settings-form'));
document.querySelectorAll(
'.js-general-settings-form, .js-mr-settings-form, .js-mr-approvals-form',
),
);
initSearchSettings(); initSearchSettings();
<script> <script>
import { GlAlert, GlButton, GlForm, GlFormGroup, GlLoadingIcon } from '@gitlab/ui'; import { GlAlert, GlButton, GlForm, GlFormGroup, GlLoadingIcon } from '@gitlab/ui';
import { isEmpty } from 'lodash'; import { isEmpty } from 'lodash';
import { mapActions, mapState } from 'vuex'; import { mapActions, mapGetters, mapState } from 'vuex';
import { mapComputed } from '~/vuex_shared/bindings'; import { mapComputed } from '~/vuex_shared/bindings';
import { APPROVAL_SETTINGS_I18N } from '../constants'; import { APPROVAL_SETTINGS_I18N } from '../constants';
import ApprovalSettingsCheckbox from './approval_settings_checkbox.vue'; import ApprovalSettingsCheckbox from './approval_settings_checkbox.vue';
...@@ -20,6 +20,21 @@ export default { ...@@ -20,6 +20,21 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
canPreventMrApprovalRuleEdit: {
type: Boolean,
required: false,
default: true,
},
canPreventAuthorApproval: {
type: Boolean,
required: false,
default: true,
},
canPreventCommittersApproval: {
type: Boolean,
required: false,
default: true,
},
}, },
computed: { computed: {
...mapState({ ...mapState({
...@@ -39,6 +54,7 @@ export default { ...@@ -39,6 +54,7 @@ export default {
undefined, undefined,
(state) => state.approvalSettings.settings, (state) => state.approvalSettings.settings,
), ),
...mapGetters(['settingChanged']),
hasSettings() { hasSettings() {
return !isEmpty(this.settings); return !isEmpty(this.settings);
}, },
...@@ -61,12 +77,11 @@ export default { ...@@ -61,12 +77,11 @@ export default {
}, },
}, },
links: { links: {
preventAuthorApprovalDocsAnchor: preventAuthorApprovalDocsAnchor: 'prevent-authors-from-approving-their-own-work',
'allowing-merge-request-authors-to-approve-their-own-merge-requests', preventMrApprovalRuleEditDocsAnchor: 'prevent-overrides-of-default-approvals',
preventMrApprovalRuleEditDocsAnchor: 'editing--overriding-approval-rules-per-merge-request', requireUserPasswordDocsAnchor: 'require-authentication-for-approvals',
requireUserPasswordDocsAnchor: 'require-authentication-when-approving-a-merge-request', removeApprovalsOnPushDocsAnchor: 'reset-approvals-on-push',
removeApprovalsOnPushDocsAnchor: 'resetting-approvals-on-push', preventCommittersApprovalDocsAnchor: 'prevent-committers-from-approving-their-own-work',
preventCommittersApprovalAnchor: 'prevent-approval-of-merge-requests-by-their-committers',
}, },
i18n: APPROVAL_SETTINGS_I18N, i18n: APPROVAL_SETTINGS_I18N,
}; };
...@@ -103,12 +118,24 @@ export default { ...@@ -103,12 +118,24 @@ export default {
v-model="preventAuthorApproval" v-model="preventAuthorApproval"
:label="$options.i18n.authorApprovalLabel" :label="$options.i18n.authorApprovalLabel"
:anchor="$options.links.preventAuthorApprovalDocsAnchor" :anchor="$options.links.preventAuthorApprovalDocsAnchor"
:locked="!canPreventAuthorApproval"
:locked-text="$options.i18n.lockedByAdmin"
data-testid="prevent-author-approval" data-testid="prevent-author-approval"
/> />
<approval-settings-checkbox
v-model="preventCommittersApproval"
:label="$options.i18n.preventCommittersApprovalLabel"
:anchor="$options.links.preventCommittersApprovalDocsAnchor"
:locked="!canPreventCommittersApproval"
:locked-text="$options.i18n.lockedByAdmin"
data-testid="prevent-committers-approval"
/>
<approval-settings-checkbox <approval-settings-checkbox
v-model="preventMrApprovalRuleEdit" v-model="preventMrApprovalRuleEdit"
:label="$options.i18n.preventMrApprovalRuleEditLabel" :label="$options.i18n.preventMrApprovalRuleEditLabel"
:anchor="$options.links.preventMrApprovalRuleEditDocsAnchor" :anchor="$options.links.preventMrApprovalRuleEditDocsAnchor"
:locked="!canPreventMrApprovalRuleEdit"
:locked-text="$options.i18n.lockedByAdmin"
data-testid="prevent-mr-approval-rule-edit" data-testid="prevent-mr-approval-rule-edit"
/> />
<approval-settings-checkbox <approval-settings-checkbox
...@@ -123,14 +150,14 @@ export default { ...@@ -123,14 +150,14 @@ export default {
:anchor="$options.links.removeApprovalsOnPushDocsAnchor" :anchor="$options.links.removeApprovalsOnPushDocsAnchor"
data-testid="remove-approvals-on-push" data-testid="remove-approvals-on-push"
/> />
<approval-settings-checkbox
v-model="preventCommittersApproval"
:label="$options.i18n.preventCommittersApprovalLabel"
:anchor="$options.links.preventCommittersApprovalAnchor"
data-testid="prevent-committers-approval"
/>
</gl-form-group> </gl-form-group>
<gl-button type="submit" variant="confirm" category="primary" :loading="isLoading"> <gl-button
type="submit"
variant="confirm"
category="primary"
:disabled="!settingChanged"
:loading="isLoading"
>
{{ $options.i18n.saveChanges }} {{ $options.i18n.saveChanges }}
</gl-button> </gl-button>
</gl-form> </gl-form>
......
<script> <script>
import App from '../app.vue'; import App from '../app.vue';
import ProjectApprovalSettings from './project_approval_settings.vue';
import ProjectRules from './project_rules.vue'; import ProjectRules from './project_rules.vue';
export default { export default {
components: { components: {
App, App,
ProjectApprovalSettings,
ProjectRules, ProjectRules,
}, },
}; };
</script> </script>
<template> <template>
<app :is-mr-edit="false"> <div>
<template #rules> <app :is-mr-edit="false">
<project-rules /> <template #rules>
</template> <project-rules />
</app> </template>
</app>
<project-approval-settings class="gl-mt-5" />
</div>
</template> </template>
<script>
import { mapState } from 'vuex';
import { __ } from '~/locale';
import ApprovalSettings from '../approval_settings.vue';
export default {
components: {
ApprovalSettings,
},
computed: {
...mapState({
approvalsPath: (state) => state.settings.approvalsPath,
canPreventMrApprovalRuleEdit: (state) => state.settings.canEdit,
canModifyAuthorSettings: (state) => state.settings.canModifyAuthorSettings,
canModifyCommiterSettings: (state) => state.settings.canModifyCommiterSettings,
}),
},
i18n: {
projectSettingsHeader: __('Approval settings'),
},
};
</script>
<template>
<div data-testid="merge-request-approval-settings">
<label class="label-bold">
{{ $options.i18n.projectSettingsHeader }}
</label>
<approval-settings
:approval-settings-path="approvalsPath"
:can-prevent-author-approval="canModifyAuthorSettings"
:can-prevent-committers-approval="canModifyCommiterSettings"
:can-prevent-mr-approval-rule-edit="canPreventMrApprovalRuleEdit"
/>
</div>
</template>
...@@ -47,7 +47,7 @@ export const APPROVAL_RULE_CONFIGS = { ...@@ -47,7 +47,7 @@ export const APPROVAL_RULE_CONFIGS = {
}, },
}; };
export const APPROVALS_HELP_PATH = 'user/project/merge_requests/merge_request_approvals'; export const APPROVALS_HELP_PATH = 'user/project/merge_requests/approvals/settings';
export const APPROVAL_SETTINGS_I18N = { export const APPROVAL_SETTINGS_I18N = {
authorApprovalLabel: s__('ApprovalSettings|Prevent MR approvals by the author.'), authorApprovalLabel: s__('ApprovalSettings|Prevent MR approvals by the author.'),
...@@ -69,6 +69,9 @@ export const APPROVAL_SETTINGS_I18N = { ...@@ -69,6 +69,9 @@ export const APPROVAL_SETTINGS_I18N = {
'ApprovalSettings|There was an error updating merge request approval settings.', 'ApprovalSettings|There was an error updating merge request approval settings.',
), ),
savingSuccessMessage: s__('ApprovalSettings|Merge request approval settings have been updated.'), savingSuccessMessage: s__('ApprovalSettings|Merge request approval settings have been updated.'),
lockedByAdmin: s__(
'ApprovalSettings|This setting is configured at the instance level and can only be changed by an administrator.',
),
}; };
export const APPROVAL_DIALOG_I18N = { export const APPROVAL_DIALOG_I18N = {
......
...@@ -112,3 +112,20 @@ export const groupApprovalsMappers = { ...@@ -112,3 +112,20 @@ export const groupApprovalsMappers = {
allow_committer_approval: !state.settings.preventCommittersApproval, allow_committer_approval: !state.settings.preventCommittersApproval,
}), }),
}; };
export const projectApprovalsMappers = {
mapDataToState: (data) => ({
preventAuthorApproval: !data.merge_requests_author_approval,
preventMrApprovalRuleEdit: data.disable_overriding_approvers_per_merge_request,
requireUserPassword: data.require_password_to_approve,
removeApprovalsOnPush: data.reset_approvals_on_push,
preventCommittersApproval: data.merge_requests_disable_committers_approval,
}),
mapStateToPayload: (state) => ({
merge_requests_author_approval: !state.settings.preventAuthorApproval,
disable_overriding_approvers_per_merge_request: state.settings.preventMrApprovalRuleEdit,
require_password_to_approve: state.settings.requireUserPassword,
reset_approvals_on_push: state.settings.removeApprovalsOnPush,
merge_requests_disable_committers_approval: state.settings.preventCommittersApproval,
}),
};
import Vue from 'vue'; import Vue from 'vue';
import { parseBoolean } from '~/lib/utils/common_utils'; import { parseBoolean } from '~/lib/utils/common_utils';
import ProjectSettingsApp from './components/project_settings/app.vue'; import ProjectSettingsApp from './components/project_settings/app.vue';
import { projectApprovalsMappers } from './mappers';
import createStore from './stores'; import createStore from './stores';
import approvalSettingsModule from './stores/modules/approval_settings';
import projectSettingsModule from './stores/modules/project_settings'; import projectSettingsModule from './stores/modules/project_settings';
export default function mountProjectSettingsApprovals(el) { export default function mountProjectSettingsApprovals(el) {
...@@ -15,15 +17,19 @@ export default function mountProjectSettingsApprovals(el) { ...@@ -15,15 +17,19 @@ export default function mountProjectSettingsApprovals(el) {
coverageCheckHelpPagePath, coverageCheckHelpPagePath,
} = el.dataset; } = el.dataset;
const store = createStore( const modules = {
{ approvals: projectSettingsModule() }, approvalSettings: approvalSettingsModule({ updateMethod: 'post', ...projectApprovalsMappers }),
{ approvals: projectSettingsModule(),
...el.dataset, };
prefix: 'project-settings',
allowMultiRule: parseBoolean(el.dataset.allowMultiRule), const store = createStore(modules, {
canEdit: parseBoolean(el.dataset.canEdit), ...el.dataset,
}, prefix: 'project-settings',
); allowMultiRule: parseBoolean(el.dataset.allowMultiRule),
canEdit: parseBoolean(el.dataset.canEdit),
canModifyAuthorSettings: parseBoolean(el.dataset.canModifyAuthorSettings),
canModifyCommiterSettings: parseBoolean(el.dataset.canModifyCommiterSettings),
});
return new Vue({ return new Vue({
el, el,
......
...@@ -2,7 +2,7 @@ import * as Sentry from '@sentry/browser'; ...@@ -2,7 +2,7 @@ import * as Sentry from '@sentry/browser';
import axios from '~/lib/utils/axios_utils'; import axios from '~/lib/utils/axios_utils';
import * as types from './mutation_types'; import * as types from './mutation_types';
export default (mapStateToPayload) => ({ export default (mapStateToPayload, updateMethod = 'put') => ({
fetchSettings({ commit }, endpoint) { fetchSettings({ commit }, endpoint) {
commit(types.REQUEST_SETTINGS); commit(types.REQUEST_SETTINGS);
...@@ -22,8 +22,11 @@ export default (mapStateToPayload) => ({ ...@@ -22,8 +22,11 @@ export default (mapStateToPayload) => ({
updateSettings({ commit, state }, endpoint) { updateSettings({ commit, state }, endpoint) {
commit(types.REQUEST_UPDATE_SETTINGS); commit(types.REQUEST_UPDATE_SETTINGS);
return axios return axios({
.put(endpoint, { ...mapStateToPayload(state) }) method: updateMethod,
url: endpoint,
data: { ...mapStateToPayload(state) },
})
.then(({ data }) => { .then(({ data }) => {
commit(types.UPDATE_SETTINGS_SUCCESS, data); commit(types.UPDATE_SETTINGS_SUCCESS, data);
}) })
......
export const settingChanged = ({ settings, initialSettings }) => {
return Object.entries(settings).findIndex(([key, value]) => initialSettings[key] !== value) > -1;
};
import actionsFactory from './actions'; import actionsFactory from './actions';
import * as getters from './getters';
import mutationsFactory from './mutations'; import mutationsFactory from './mutations';
import createState from './state'; import createState from './state';
export default ({ mapStateToPayload, mapDataToState }) => ({ export default ({ mapStateToPayload, mapDataToState, updateMethod = 'put' }) => ({
state: createState(), state: createState(),
actions: actionsFactory(mapStateToPayload), actions: actionsFactory(mapStateToPayload, updateMethod),
mutations: mutationsFactory(mapDataToState), mutations: mutationsFactory(mapDataToState),
getters,
}); });
...@@ -8,6 +8,7 @@ export default (mapDataToState) => ({ ...@@ -8,6 +8,7 @@ export default (mapDataToState) => ({
}, },
[types.RECEIVE_SETTINGS_SUCCESS](state, data) { [types.RECEIVE_SETTINGS_SUCCESS](state, data) {
state.settings = { ...mapDataToState(data) }; state.settings = { ...mapDataToState(data) };
state.initialSettings = { ...state.settings };
state.isLoading = false; state.isLoading = false;
}, },
[types.RECEIVE_SETTINGS_ERROR](state) { [types.RECEIVE_SETTINGS_ERROR](state) {
...@@ -21,6 +22,7 @@ export default (mapDataToState) => ({ ...@@ -21,6 +22,7 @@ export default (mapDataToState) => ({
}, },
[types.UPDATE_SETTINGS_SUCCESS](state, data) { [types.UPDATE_SETTINGS_SUCCESS](state, data) {
state.settings = { ...mapDataToState(data) }; state.settings = { ...mapDataToState(data) };
state.initialSettings = { ...state.settings };
state.isLoading = false; state.isLoading = false;
state.isUpdated = true; state.isUpdated = true;
}, },
......
export default () => ({ export default () => ({
settings: {}, settings: {},
initialSettings: {},
isLoading: false, isLoading: false,
isUpdated: false, isUpdated: false,
errorMessage: '', errorMessage: '',
......
...@@ -72,8 +72,11 @@ module EE ...@@ -72,8 +72,11 @@ module EE
data: { data: {
'project_id': project.id, 'project_id': project.id,
'can_edit': can_modify_approvers.to_s, 'can_edit': can_modify_approvers.to_s,
'can_modify_author_settings': can_modify_author_settings.to_s,
'can_modify_commiter_settings': can_modify_commiter_settings.to_s,
'project_path': expose_path(api_v4_projects_path(id: project.id)), 'project_path': expose_path(api_v4_projects_path(id: project.id)),
'settings_path': expose_path(api_v4_projects_approval_settings_path(id: project.id)), 'settings_path': expose_path(api_v4_projects_approval_settings_path(id: project.id)),
'approvals_path': expose_path(api_v4_projects_approvals_path(id: project.id)),
'rules_path': expose_path(api_v4_projects_approval_settings_rules_path(id: project.id)), 'rules_path': expose_path(api_v4_projects_approval_settings_rules_path(id: project.id)),
'allow_multi_rule': project.multiple_approval_rules_available?.to_s, 'allow_multi_rule': project.multiple_approval_rules_available?.to_s,
'eligible_approvers_docs_path': help_page_path('user/project/merge_requests/approvals/rules', anchor: 'eligible-approvers'), 'eligible_approvers_docs_path': help_page_path('user/project/merge_requests/approvals/rules', anchor: 'eligible-approvers'),
...@@ -99,6 +102,14 @@ module EE ...@@ -99,6 +102,14 @@ module EE
can?(current_user, :modify_approvers_rules, project) can?(current_user, :modify_approvers_rules, project)
end end
def can_modify_author_settings(project = @project)
can?(current_user, :modify_merge_request_author_setting, project)
end
def can_modify_commiter_settings(project = @project)
can?(current_user, :modify_merge_request_committer_setting, project)
end
def permanent_delete_message(project) def permanent_delete_message(project)
message = _('This action will %{strongOpen}permanently delete%{strongClose} %{codeOpen}%{project}%{codeClose} %{strongOpen}immediately%{strongClose}, including its repositories and all related resources, including issues and merge requests.') message = _('This action will %{strongOpen}permanently delete%{strongClose} %{codeOpen}%{project}%{codeClose} %{strongOpen}immediately%{strongClose}, including its repositories and all related resources, including issues and merge requests.')
html_escape(message) % remove_message_data(project) html_escape(message) % remove_message_data(project)
......
...@@ -10,7 +10,4 @@ ...@@ -10,7 +10,4 @@
= link_to _("Learn more."), help_page_path("user/project/merge_requests/approvals/index.md"), target: '_blank' = link_to _("Learn more."), help_page_path("user/project/merge_requests/approvals/index.md"), target: '_blank'
.settings-content .settings-content
= form_for @project, html: { class: "merge-request-approval-settings-form js-mr-approvals-form" }, authenticity_token: true do |f| = render 'projects/merge_request_approvals_settings_form'
%input{ name: 'update_section', type: 'hidden', value: 'js-merge-request-approval-settings' }
= render 'projects/merge_request_approvals_settings_form', form: f, project: @project
= f.submit _("Save changes"), class: "btn gl-button btn-confirm gl-mt-4", data: { qa_selector: 'save_merge_request_approval_settings_button' }
- can_modify_merge_request_author_settings = can?(current_user, :modify_merge_request_author_setting, @project)
- can_modify_merge_request_committer_settings = can?(current_user, :modify_merge_request_committer_setting, @project)
.form-group .form-group
= form.label :approver_ids, class: 'label-bold' do = label :approver_ids, class: 'label-bold' do
= _("Approval rules") = _("Approval rules")
#js-mr-approvals-settings{ approvals_app_data } #js-mr-approvals-settings{ approvals_app_data }
.text-center.gl-mt-3 .text-center.gl-mt-3
= sprite_icon('spinner', size: 24, css_class: 'gl-spinner') = sprite_icon('spinner', size: 24, css_class: 'gl-spinner')
%fieldset.form-group
%legend.h5.gl-border-none
= _('Approval settings')
.gl-form-checkbox-group
.gl-form-checkbox.custom-control.custom-checkbox
= form.check_box(:disable_overriding_approvers_per_merge_request, { class: 'custom-control-input', disabled: !can_modify_approvers })
= form.label :disable_overriding_approvers_per_merge_request, class: 'custom-control-label' do
%span= _('Prevent users from modifying MR approval rules in merge requests.')
= link_to sprite_icon('question-o'), help_page_path('user/project/merge_requests/approvals/settings', anchor: 'prevent-overrides-of-default-approvals'), target: '_blank'
.gl-form-checkbox.custom-control.custom-checkbox
= form.check_box :reset_approvals_on_push, class: 'custom-control-input'
= form.label :reset_approvals_on_push, class: 'custom-control-label' do
%span= _('Require new approvals when new commits are added to an MR.')
.gl-form-checkbox.custom-control.custom-checkbox
= form.check_box :merge_requests_author_approval, { class: 'custom-control-input', disabled: !can_modify_merge_request_author_settings }, false, true
= form.label :merge_requests_author_approval, class: 'custom-control-label' do
%span= _('Prevent MR approvals by the author.')
= link_to sprite_icon('question-o'), help_page_path('user/project/merge_requests/approvals/settings',
anchor: 'prevent-authors-from-approving-their-own-work'), target: '_blank'
.gl-form-checkbox.custom-control.custom-checkbox
= form.check_box :merge_requests_disable_committers_approval, { disabled: !can_modify_merge_request_committer_settings, class: 'custom-control-input' }
= form.label :merge_requests_disable_committers_approval, class: 'custom-control-label' do
%span= _('Prevent MR approvals from users who make commits to the MR.')
= link_to sprite_icon('question-o'), help_page_path('user/project/merge_requests/approvals/settings',
anchor: 'prevent-committers-from-approving-their-own-work'), target: '_blank'
- if password_authentication_enabled_for_web?
.gl-form-checkbox.custom-control.custom-checkbox
= form.check_box :require_password_to_approve, class: 'custom-control-input'
= form.label :require_password_to_approve, class: 'custom-control-label' do
%span= _('Require user password for approvals.')
= link_to sprite_icon('question-o'), help_page_path('user/project/merge_requests/approvals/settings',
anchor: 'require-authentication-for-approvals'), target: '_blank'
...@@ -18,7 +18,7 @@ RSpec.describe 'Admin interacts with merge requests approvals settings' do ...@@ -18,7 +18,7 @@ RSpec.describe 'Admin interacts with merge requests approvals settings' do
visit(admin_push_rule_path) visit(admin_push_rule_path)
end end
it 'updates instance-level merge request approval settings and enforces project-level ones' do it 'updates instance-level merge request approval settings and enforces project-level ones', :js do
page.within('.merge-request-approval-settings') do page.within('.merge-request-approval-settings') do
check 'Prevent MR approvals by author.' check 'Prevent MR approvals by author.'
check 'Prevent MR approvals from users who make commits to the MR.' check 'Prevent MR approvals from users who make commits to the MR.'
...@@ -34,10 +34,10 @@ RSpec.describe 'Admin interacts with merge requests approvals settings' do ...@@ -34,10 +34,10 @@ RSpec.describe 'Admin interacts with merge requests approvals settings' do
visit edit_project_path(project) visit edit_project_path(project)
page.within('#js-merge-request-approval-settings') do page.within('[data-testid="merge-request-approval-settings"]') do
expect(find('#project_merge_requests_author_approval')).to be_disabled.and be_checked expect(find('[data-testid="prevent-author-approval"] > input')).to be_disabled.and be_checked
expect(find('#project_merge_requests_disable_committers_approval')).to be_disabled.and be_checked expect(find('[data-testid="prevent-committers-approval"] > input')).to be_disabled.and be_checked
expect(find('#project_disable_overriding_approvers_per_merge_request')).to be_disabled.and be_checked expect(find('[data-testid="prevent-mr-approval-rule-edit"] > input')).to be_disabled.and be_checked
end end
end end
end end
...@@ -138,12 +138,12 @@ RSpec.describe 'Projects > Audit Events', :js do ...@@ -138,12 +138,12 @@ RSpec.describe 'Projects > Audit Events', :js do
project.add_developer(pete) project.add_developer(pete)
end end
it "appears in the project's audit events" do it "appears in the project's audit events", :js do
visit edit_project_path(project) visit edit_project_path(project)
page.within('#js-merge-request-approval-settings') do page.within('[data-testid="merge-request-approval-settings"]') do
uncheck 'project_merge_requests_author_approval' find('[data-testid="prevent-author-approval"] > input').set(false)
check 'project_merge_requests_disable_committers_approval' find('[data-testid="prevent-committers-approval"] > input').set(true)
click_button 'Save changes' click_button 'Save changes'
end end
......
...@@ -20,10 +20,11 @@ describe('ApprovalSettings', () => { ...@@ -20,10 +20,11 @@ describe('ApprovalSettings', () => {
const approvalSettingsPath = 'groups/22/merge_request_approval_settings'; const approvalSettingsPath = 'groups/22/merge_request_approval_settings';
const setupStore = (data = {}) => { const setupStore = (data = {}, initialData) => {
const module = approvalSettingsModule(groupApprovalsMappers); const module = approvalSettingsModule(groupApprovalsMappers);
module.state.settings = data; module.state.settings = data;
module.state.initialSettings = initialData || data;
actions = module.actions; actions = module.actions;
jest.spyOn(actions, 'fetchSettings').mockImplementation(); jest.spyOn(actions, 'fetchSettings').mockImplementation();
jest.spyOn(actions, 'updateSettings').mockImplementation(); jest.spyOn(actions, 'updateSettings').mockImplementation();
...@@ -33,12 +34,16 @@ describe('ApprovalSettings', () => { ...@@ -33,12 +34,16 @@ describe('ApprovalSettings', () => {
store = createStore({ approvalSettings: module }); store = createStore({ approvalSettings: module });
}; };
const createWrapper = () => { const createWrapper = (props = {}) => {
wrapper = extendedWrapper( wrapper = extendedWrapper(
shallowMount(ApprovalSettings, { shallowMount(ApprovalSettings, {
localVue, localVue,
store, store,
propsData: { approvalSettingsPath }, propsData: {
approvalSettingsPath,
...props,
},
stubs: { GlButton },
}), }),
); );
}; };
...@@ -89,14 +94,16 @@ describe('ApprovalSettings', () => { ...@@ -89,14 +94,16 @@ describe('ApprovalSettings', () => {
}); });
describe('with settings', () => { describe('with settings', () => {
const settings = {
allow_author_approval: false,
allow_committer_approval: false,
allow_overrides_to_approver_list_per_merge_request: false,
require_password_to_approve: false,
retain_approvals_on_push: false,
};
beforeEach(() => { beforeEach(() => {
setupStore({ setupStore(settings);
allow_author_approval: false,
allow_committer_approval: false,
allow_overrides_to_approver_list_per_merge_request: false,
require_password_to_approve: false,
retain_approvals_on_push: false,
});
}); });
it('renders the form once successfully loaded', async () => { it('renders the form once successfully loaded', async () => {
...@@ -109,14 +116,14 @@ describe('ApprovalSettings', () => { ...@@ -109,14 +116,14 @@ describe('ApprovalSettings', () => {
expect(findForm().exists()).toBe(true); expect(findForm().exists()).toBe(true);
}); });
it('renders enabled button when not loading', async () => { it('renders the button as not loading when loaded', async () => {
createWrapper(); createWrapper();
await waitForPromises(); await waitForPromises();
expect(findSaveButton().props('loading')).toBe(false); expect(findSaveButton().props('loading')).toBe(false);
}); });
it('renders loading button when loading', async () => { it('renders the button as loading when updating', async () => {
createWrapper(); createWrapper();
await waitForPromises(); await waitForPromises();
await store.commit('REQUEST_UPDATE_SETTINGS'); await store.commit('REQUEST_UPDATE_SETTINGS');
...@@ -124,13 +131,29 @@ describe('ApprovalSettings', () => { ...@@ -124,13 +131,29 @@ describe('ApprovalSettings', () => {
expect(findSaveButton().props('loading')).toBe(true); expect(findSaveButton().props('loading')).toBe(true);
}); });
it('renders the button as disabled when setting are unchanged', async () => {
createWrapper();
await waitForPromises();
expect(findSaveButton().attributes('disabled')).toBe('true');
});
it('renders the button as enabled when a setting was changed', async () => {
setupStore({ ...settings, allow_author_approval: true }, settings);
createWrapper();
await waitForPromises();
expect(findSaveButton().attributes('disabled')).toBeUndefined();
});
describe.each` describe.each`
testid | action | setting | labelKey | anchor testid | action | setting | labelKey | anchor
${'prevent-author-approval'} | ${'setPreventAuthorApproval'} | ${'preventAuthorApproval'} | ${'authorApprovalLabel'} | ${'allowing-merge-request-authors-to-approve-their-own-merge-requests'} ${'prevent-author-approval'} | ${'setPreventAuthorApproval'} | ${'preventAuthorApproval'} | ${'authorApprovalLabel'} | ${'prevent-authors-from-approving-their-own-work'}
${'prevent-committers-approval'} | ${'setPreventCommittersApproval'} | ${'preventCommittersApproval'} | ${'preventCommittersApprovalLabel'} | ${'prevent-approval-of-merge-requests-by-their-committers'} ${'prevent-committers-approval'} | ${'setPreventCommittersApproval'} | ${'preventCommittersApproval'} | ${'preventCommittersApprovalLabel'} | ${'prevent-committers-from-approving-their-own-work'}
${'prevent-mr-approval-rule-edit'} | ${'setPreventMrApprovalRuleEdit'} | ${'preventMrApprovalRuleEdit'} | ${'preventMrApprovalRuleEditLabel'} | ${'editing--overriding-approval-rules-per-merge-request'} ${'prevent-mr-approval-rule-edit'} | ${'setPreventMrApprovalRuleEdit'} | ${'preventMrApprovalRuleEdit'} | ${'preventMrApprovalRuleEditLabel'} | ${'prevent-overrides-of-default-approvals'}
${'require-user-password'} | ${'setRequireUserPassword'} | ${'requireUserPassword'} | ${'requireUserPasswordLabel'} | ${'require-authentication-when-approving-a-merge-request'} ${'require-user-password'} | ${'setRequireUserPassword'} | ${'requireUserPassword'} | ${'requireUserPasswordLabel'} | ${'require-authentication-for-approvals'}
${'remove-approvals-on-push'} | ${'setRemoveApprovalsOnPush'} | ${'removeApprovalsOnPush'} | ${'removeApprovalsOnPushLabel'} | ${'resetting-approvals-on-push'} ${'remove-approvals-on-push'} | ${'setRemoveApprovalsOnPush'} | ${'removeApprovalsOnPush'} | ${'removeApprovalsOnPushLabel'} | ${'reset-approvals-on-push'}
`('with the $testid checkbox', ({ testid, action, setting, labelKey, anchor }) => { `('with the $testid checkbox', ({ testid, action, setting, labelKey, anchor }) => {
let checkbox = null; let checkbox = null;
...@@ -214,5 +237,27 @@ describe('ApprovalSettings', () => { ...@@ -214,5 +237,27 @@ describe('ApprovalSettings', () => {
}); });
}); });
}); });
describe('locked settings', () => {
it.each`
property | value | locked | testid
${'canPreventAuthorApproval'} | ${true} | ${false} | ${'prevent-author-approval'}
${'canPreventMrApprovalRuleEdit'} | ${true} | ${false} | ${'prevent-mr-approval-rule-edit'}
${'canPreventCommittersApproval'} | ${true} | ${false} | ${'prevent-committers-approval'}
${'canPreventAuthorApproval'} | ${false} | ${true} | ${'prevent-author-approval'}
${'canPreventMrApprovalRuleEdit'} | ${false} | ${true} | ${'prevent-mr-approval-rule-edit'}
${'canPreventCommittersApproval'} | ${false} | ${true} | ${'prevent-committers-approval'}
`(
`when $property is $value, then $testid has "locked" set to $locked`,
({ property, value, locked, testid }) => {
createWrapper({ [property]: value });
expect(wrapper.findByTestId(testid).props()).toMatchObject({
locked,
lockedText: APPROVAL_SETTINGS_I18N.lockedByAdmin,
});
},
);
});
}); });
}); });
import { shallowMount } from '@vue/test-utils';
import Vue from 'vue';
import Vuex from 'vuex';
import ApprovalSettings from 'ee/approvals/components/approval_settings.vue';
import ProjectApprovalSettings from 'ee/approvals/components/project_settings/project_approval_settings.vue';
import { projectApprovalsMappers } from 'ee/approvals/mappers';
import createStore from 'ee/approvals/stores';
import approvalSettingsModule from 'ee/approvals/stores/modules/approval_settings';
Vue.use(Vuex);
describe('ProjectApprovalSettings', () => {
let wrapper;
let store;
const findApprovalSettings = () => wrapper.find(ApprovalSettings);
const setupStore = (data = {}) => {
store = createStore({
approvalSettings: approvalSettingsModule(projectApprovalsMappers),
});
store.state.settings = data;
};
const createWrapper = () => {
wrapper = shallowMount(ProjectApprovalSettings, { store });
};
afterEach(() => {
wrapper.destroy();
store = null;
});
it('configures the app with the store values', () => {
setupStore({
approvalsPath: 'foo',
canEdit: true,
canModifyAuthorSettings: false,
canModifyCommiterSettings: true,
});
createWrapper();
expect(findApprovalSettings().props()).toMatchObject({
approvalSettingsPath: 'foo',
canPreventMrApprovalRuleEdit: true,
canPreventAuthorApproval: false,
canPreventCommittersApproval: true,
});
});
});
...@@ -61,11 +61,18 @@ describe('EE approvals group settings module actions', () => { ...@@ -61,11 +61,18 @@ describe('EE approvals group settings module actions', () => {
}); });
}); });
describe('updateSettings', () => { describe.each`
httpMethod | onMethod
${'put'} | ${'onPut'}
${'post'} | ${'onPost'}
`('updateSetting with $httpMethod', ({ httpMethod, onMethod }) => {
let actionsWithMethod;
beforeEach(() => { beforeEach(() => {
state = { state = {
settings: {}, settings: {},
}; };
actionsWithMethod = actionsFactory((data) => data, httpMethod);
}); });
describe('on success', () => { describe('on success', () => {
...@@ -77,10 +84,10 @@ describe('EE approvals group settings module actions', () => { ...@@ -77,10 +84,10 @@ describe('EE approvals group settings module actions', () => {
require_password_to_approve: true, require_password_to_approve: true,
retain_approvals_on_push: true, retain_approvals_on_push: true,
}; };
mock.onPut(approvalSettingsPath).replyOnce(httpStatus.OK, data); mock[onMethod](approvalSettingsPath).replyOnce(httpStatus.OK, data);
return testAction( return testAction(
actions.updateSettings, actionsWithMethod.updateSettings,
approvalSettingsPath, approvalSettingsPath,
state, state,
[ [
...@@ -95,10 +102,10 @@ describe('EE approvals group settings module actions', () => { ...@@ -95,10 +102,10 @@ describe('EE approvals group settings module actions', () => {
describe('on error', () => { describe('on error', () => {
it('dispatches the request, updates payload and sets error message', () => { it('dispatches the request, updates payload and sets error message', () => {
const data = { message: 'Internal Server Error' }; const data = { message: 'Internal Server Error' };
mock.onPut(approvalSettingsPath).replyOnce(httpStatus.INTERNAL_SERVER_ERROR, data); mock[onMethod](approvalSettingsPath).replyOnce(httpStatus.INTERNAL_SERVER_ERROR, data);
return testAction( return testAction(
actions.updateSettings, actionsWithMethod.updateSettings,
approvalSettingsPath, approvalSettingsPath,
state, state,
[{ type: types.REQUEST_UPDATE_SETTINGS }, { type: types.UPDATE_SETTINGS_ERROR }], [{ type: types.REQUEST_UPDATE_SETTINGS }, { type: types.UPDATE_SETTINGS_ERROR }],
......
import * as getters from 'ee/approvals/stores/modules/approval_settings/getters';
describe('Group settings store getters', () => {
let settings;
const initialSettings = {
preventAuthorApproval: true,
preventMrApprovalRuleEdit: true,
requireUserPassword: true,
removeApprovalsOnPush: true,
};
beforeEach(() => {
settings = { ...initialSettings };
});
describe('settingChanged', () => {
it('returns true when a setting is changed', () => {
settings.preventAuthorApproval = false;
expect(getters.settingChanged({ settings, initialSettings })).toBe(true);
});
it('returns false when the setting remains unchanged', () => {
expect(getters.settingChanged({ settings, initialSettings })).toBe(false);
});
});
});
...@@ -389,6 +389,9 @@ RSpec.describe ProjectsHelper do ...@@ -389,6 +389,9 @@ RSpec.describe ProjectsHelper do
expect(subject[:data]).to eq({ expect(subject[:data]).to eq({
project_id: project.id, project_id: project.id,
can_edit: 'true', can_edit: 'true',
can_modify_author_settings: 'true',
can_modify_commiter_settings: 'true',
approvals_path: expose_path(api_v4_projects_approvals_path(id: project.id)),
project_path: expose_path(api_v4_projects_path(id: project.id)), project_path: expose_path(api_v4_projects_path(id: project.id)),
settings_path: expose_path(api_v4_projects_approval_settings_path(id: project.id)), settings_path: expose_path(api_v4_projects_approval_settings_path(id: project.id)),
rules_path: expose_path(api_v4_projects_approval_settings_rules_path(id: project.id)), rules_path: expose_path(api_v4_projects_approval_settings_rules_path(id: project.id)),
......
...@@ -4227,6 +4227,9 @@ msgstr "" ...@@ -4227,6 +4227,9 @@ msgstr ""
msgid "ApprovalSettings|There was an error updating merge request approval settings." msgid "ApprovalSettings|There was an error updating merge request approval settings."
msgstr "" msgstr ""
msgid "ApprovalSettings|This setting is configured at the instance level and can only be changed by an administrator."
msgstr ""
msgid "ApprovalStatusTooltip|Adheres to separation of duties" msgid "ApprovalStatusTooltip|Adheres to separation of duties"
msgstr "" msgstr ""
...@@ -25047,9 +25050,6 @@ msgstr "" ...@@ -25047,9 +25050,6 @@ msgstr ""
msgid "Prevent MR approvals by author." msgid "Prevent MR approvals by author."
msgstr "" msgstr ""
msgid "Prevent MR approvals by the author."
msgstr ""
msgid "Prevent MR approvals from users who make commits to the MR." msgid "Prevent MR approvals from users who make commits to the MR."
msgstr "" msgstr ""
...@@ -25065,9 +25065,6 @@ msgstr "" ...@@ -25065,9 +25065,6 @@ msgstr ""
msgid "Prevent users from changing their profile name" msgid "Prevent users from changing their profile name"
msgstr "" msgstr ""
msgid "Prevent users from modifying MR approval rules in merge requests."
msgstr ""
msgid "Prevent users from modifying MR approval rules in projects and merge requests." msgid "Prevent users from modifying MR approval rules in projects and merge requests."
msgstr "" msgstr ""
...@@ -28134,12 +28131,6 @@ msgstr "" ...@@ -28134,12 +28131,6 @@ msgstr ""
msgid "Require all users to set up two-factor authentication" msgid "Require all users to set up two-factor authentication"
msgstr "" msgstr ""
msgid "Require new approvals when new commits are added to an MR."
msgstr ""
msgid "Require user password for approvals."
msgstr ""
msgid "Required approvals (%{approvals_given} given)" msgid "Required approvals (%{approvals_given} given)"
msgstr "" msgstr ""
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment