Commit a469111a authored by Jiaan Louw's avatar Jiaan Louw Committed by Enrique Alcántara

Update approval settings to use objects

This updates the approvals settings app to use object based
setting values instead of booleans in preperation for the new
API endpoints that will use object settings.
parent eae4fd30
......@@ -3,7 +3,6 @@ import { GlAlert, GlButton, GlForm, GlFormGroup, GlLoadingIcon, GlLink } from '@
import { isEmpty } from 'lodash';
import { mapActions, mapGetters, mapState } from 'vuex';
import { helpPagePath } from '~/helpers/help_page_helper';
import { mapComputed } from '~/vuex_shared/bindings';
import { APPROVAL_SETTINGS_I18N } from '../constants';
import ApprovalSettingsCheckbox from './approval_settings_checkbox.vue';
......@@ -48,18 +47,14 @@ export default {
isUpdated: (state) => state.approvalSettings.isUpdated,
settings: (state) => state.approvalSettings.settings,
errorMessage: (state) => state.approvalSettings.errorMessage,
preventAuthorApproval: (state) => state.approvalSettings.settings.preventAuthorApproval,
preventCommittersApproval: (state) =>
state.approvalSettings.settings.preventCommittersApproval,
preventMrApprovalRuleEdit: (state) =>
state.approvalSettings.settings.preventMrApprovalRuleEdit,
removeApprovalsOnPush: (state) => state.approvalSettings.settings.removeApprovalsOnPush,
requireUserPassword: (state) => state.approvalSettings.settings.requireUserPassword,
}),
...mapComputed(
[
{ key: 'preventAuthorApproval', updateFn: 'setPreventAuthorApproval' },
{ key: 'preventCommittersApproval', updateFn: 'setPreventCommittersApproval' },
{ key: 'preventMrApprovalRuleEdit', updateFn: 'setPreventMrApprovalRuleEdit' },
{ key: 'removeApprovalsOnPush', updateFn: 'setRemoveApprovalsOnPush' },
{ key: 'requireUserPassword', updateFn: 'setRequireUserPassword' },
],
undefined,
(state) => state.approvalSettings.settings,
),
...mapGetters(['settingChanged']),
hasSettings() {
return !isEmpty(this.settings);
......@@ -77,6 +72,11 @@ export default {
'updateSettings',
'dismissErrorMessage',
'dismissSuccessMessage',
'setPreventAuthorApproval',
'setPreventCommittersApproval',
'setPreventMrApprovalRuleEdit',
'setRemoveApprovalsOnPush',
'setRequireUserPassword',
]),
async onSubmit() {
await this.updateSettings(this.approvalSettingsPath);
......@@ -124,35 +124,44 @@ export default {
</p>
<gl-form-group>
<approval-settings-checkbox
v-model="preventAuthorApproval"
:checked="preventAuthorApproval.value"
:label="settingsLabels.authorApprovalLabel"
:locked="!canPreventAuthorApproval"
:locked="!canPreventAuthorApproval || preventAuthorApproval.locked"
:locked-text="$options.i18n.lockedByAdmin"
data-testid="prevent-author-approval"
@input="setPreventAuthorApproval"
/>
<approval-settings-checkbox
v-model="preventCommittersApproval"
:checked="preventCommittersApproval.value"
:label="settingsLabels.preventCommittersApprovalLabel"
:locked="!canPreventCommittersApproval"
:locked="!canPreventCommittersApproval || preventCommittersApproval.locked"
:locked-text="$options.i18n.lockedByAdmin"
data-testid="prevent-committers-approval"
@input="setPreventCommittersApproval"
/>
<approval-settings-checkbox
v-model="preventMrApprovalRuleEdit"
:checked="preventMrApprovalRuleEdit.value"
:label="settingsLabels.preventMrApprovalRuleEditLabel"
:locked="!canPreventMrApprovalRuleEdit"
:locked="!canPreventMrApprovalRuleEdit || preventMrApprovalRuleEdit.locked"
:locked-text="$options.i18n.lockedByAdmin"
data-testid="prevent-mr-approval-rule-edit"
@input="setPreventMrApprovalRuleEdit"
/>
<approval-settings-checkbox
v-model="requireUserPassword"
:checked="requireUserPassword.value"
:label="settingsLabels.requireUserPasswordLabel"
:locked="requireUserPassword.locked"
:locked-text="$options.i18n.lockedByAdmin"
data-testid="require-user-password"
@input="setRequireUserPassword"
/>
<approval-settings-checkbox
v-model="removeApprovalsOnPush"
:checked="removeApprovalsOnPush.value"
:label="settingsLabels.removeApprovalsOnPushLabel"
:locked="removeApprovalsOnPush.locked"
:locked-text="$options.i18n.lockedByAdmin"
data-testid="remove-approvals-on-push"
@input="setRemoveApprovalsOnPush"
/>
</gl-form-group>
<gl-button
......
......@@ -14,7 +14,7 @@ export default {
type: String,
required: true,
},
value: {
checked: {
type: Boolean,
required: false,
default: false,
......@@ -47,7 +47,7 @@ export default {
</script>
<template>
<gl-form-checkbox :disabled="locked" :checked="value" @input="input">
<gl-form-checkbox :disabled="locked" :checked="checked" @input="input">
{{ label }}
<template v-if="locked">
<gl-icon :id="lockIconId" data-testid="lock-icon" name="lock" />
......
......@@ -100,36 +100,40 @@ export const mapMRApprovalSettingsResponse = (res) => {
};
};
const invertApprovalSetting = ({ value, ...rest }) => ({ value: !value, ...rest });
export const groupApprovalsMappers = {
mapDataToState: (data) => ({
preventAuthorApproval: !data.allow_author_approval.value,
preventMrApprovalRuleEdit: !data.allow_overrides_to_approver_list_per_merge_request.value,
requireUserPassword: data.require_password_to_approve.value,
removeApprovalsOnPush: !data.retain_approvals_on_push.value,
preventCommittersApproval: !data.allow_committer_approval.value,
preventAuthorApproval: invertApprovalSetting(data.allow_author_approval),
preventMrApprovalRuleEdit: invertApprovalSetting(
data.allow_overrides_to_approver_list_per_merge_request,
),
requireUserPassword: data.require_password_to_approve,
removeApprovalsOnPush: invertApprovalSetting(data.retain_approvals_on_push),
preventCommittersApproval: invertApprovalSetting(data.allow_committer_approval),
}),
mapStateToPayload: (state) => ({
allow_author_approval: !state.settings.preventAuthorApproval,
allow_overrides_to_approver_list_per_merge_request: !state.settings.preventMrApprovalRuleEdit,
require_password_to_approve: state.settings.requireUserPassword,
retain_approvals_on_push: !state.settings.removeApprovalsOnPush,
allow_committer_approval: !state.settings.preventCommittersApproval,
mapStateToPayload: ({ settings }) => ({
allow_author_approval: !settings.preventAuthorApproval.value,
allow_overrides_to_approver_list_per_merge_request: !settings.preventMrApprovalRuleEdit.value,
require_password_to_approve: settings.requireUserPassword.value,
retain_approvals_on_push: !settings.removeApprovalsOnPush.value,
allow_committer_approval: !settings.preventCommittersApproval.value,
}),
};
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,
preventAuthorApproval: { value: !data.merge_requests_author_approval },
preventMrApprovalRuleEdit: { value: data.disable_overriding_approvers_per_merge_request },
requireUserPassword: { value: data.require_password_to_approve },
removeApprovalsOnPush: { value: data.reset_approvals_on_push },
preventCommittersApproval: { value: 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,
mapStateToPayload: ({ settings }) => ({
merge_requests_author_approval: !settings.preventAuthorApproval.value,
disable_overriding_approvers_per_merge_request: settings.preventMrApprovalRuleEdit.value,
require_password_to_approve: settings.requireUserPassword.value,
reset_approvals_on_push: settings.removeApprovalsOnPush.value,
merge_requests_disable_committers_approval: settings.preventCommittersApproval.value,
}),
};
......@@ -46,23 +46,23 @@ export default (mapStateToPayload, updateMethod = 'put') => ({
commit(types.DISMISS_ERROR_MESSAGE);
},
setPreventAuthorApproval({ commit }, { preventAuthorApproval }) {
commit(types.SET_PREVENT_AUTHOR_APPROVAL, preventAuthorApproval);
setPreventAuthorApproval({ commit }, value) {
commit(types.SET_PREVENT_AUTHOR_APPROVAL, value);
},
setPreventCommittersApproval({ commit }, { preventCommittersApproval }) {
commit(types.SET_PREVENT_COMMITTERS_APPROVAL, preventCommittersApproval);
setPreventCommittersApproval({ commit }, value) {
commit(types.SET_PREVENT_COMMITTERS_APPROVAL, value);
},
setPreventMrApprovalRuleEdit({ commit }, { preventMrApprovalRuleEdit }) {
commit(types.SET_PREVENT_MR_APPROVAL_RULE_EDIT, preventMrApprovalRuleEdit);
setPreventMrApprovalRuleEdit({ commit }, value) {
commit(types.SET_PREVENT_MR_APPROVAL_RULE_EDIT, value);
},
setRemoveApprovalsOnPush({ commit }, { removeApprovalsOnPush }) {
commit(types.SET_REMOVE_APPROVALS_ON_PUSH, removeApprovalsOnPush);
setRemoveApprovalsOnPush({ commit }, value) {
commit(types.SET_REMOVE_APPROVALS_ON_PUSH, value);
},
setRequireUserPassword({ commit }, { requireUserPassword }) {
commit(types.SET_REQUIRE_USER_PASSWORD, requireUserPassword);
setRequireUserPassword({ commit }, value) {
commit(types.SET_REQUIRE_USER_PASSWORD, value);
},
});
export const settingChanged = ({ settings, initialSettings }) => {
return Object.entries(settings).findIndex(([key, value]) => initialSettings[key] !== value) > -1;
return (
Object.entries(settings).findIndex(([key, { value }]) => initialSettings[key].value !== value) >
-1
);
};
import { cloneDeep } from 'lodash';
import { APPROVAL_SETTINGS_I18N } from '../../../constants';
import * as types from './mutation_types';
......@@ -8,7 +9,7 @@ export default (mapDataToState) => ({
},
[types.RECEIVE_SETTINGS_SUCCESS](state, data) {
state.settings = { ...mapDataToState(data) };
state.initialSettings = { ...state.settings };
state.initialSettings = cloneDeep(state.settings);
state.isLoading = false;
},
[types.RECEIVE_SETTINGS_ERROR](state) {
......@@ -22,7 +23,7 @@ export default (mapDataToState) => ({
},
[types.UPDATE_SETTINGS_SUCCESS](state, data) {
state.settings = { ...mapDataToState(data) };
state.initialSettings = { ...state.settings };
state.initialSettings = cloneDeep(state.settings);
state.isLoading = false;
state.isUpdated = true;
},
......@@ -36,19 +37,19 @@ export default (mapDataToState) => ({
[types.DISMISS_ERROR_MESSAGE](state) {
state.errorMessage = '';
},
[types.SET_PREVENT_AUTHOR_APPROVAL](state, preventAuthorApproval) {
state.settings.preventAuthorApproval = preventAuthorApproval;
[types.SET_PREVENT_AUTHOR_APPROVAL](state, value) {
state.settings.preventAuthorApproval.value = value;
},
[types.SET_PREVENT_COMMITTERS_APPROVAL](state, preventCommittersApproval) {
state.settings.preventCommittersApproval = preventCommittersApproval;
[types.SET_PREVENT_COMMITTERS_APPROVAL](state, value) {
state.settings.preventCommittersApproval.value = value;
},
[types.SET_PREVENT_MR_APPROVAL_RULE_EDIT](state, preventMrApprovalRuleEdit) {
state.settings.preventMrApprovalRuleEdit = preventMrApprovalRuleEdit;
[types.SET_PREVENT_MR_APPROVAL_RULE_EDIT](state, value) {
state.settings.preventMrApprovalRuleEdit.value = value;
},
[types.SET_REMOVE_APPROVALS_ON_PUSH](state, removeApprovalsOnPush) {
state.settings.removeApprovalsOnPush = removeApprovalsOnPush;
[types.SET_REMOVE_APPROVALS_ON_PUSH](state, value) {
state.settings.removeApprovalsOnPush.value = value;
},
[types.SET_REQUIRE_USER_PASSWORD](state, requireUserPassword) {
state.settings.requireUserPassword = requireUserPassword;
[types.SET_REQUIRE_USER_PASSWORD](state, value) {
state.settings.requireUserPassword.value = value;
},
});
......@@ -45,15 +45,15 @@ describe('ApprovalSettingsCheckbox', () => {
});
});
describe('value', () => {
it('defaults to false when no value is given', () => {
describe('checked', () => {
it('defaults to false when no checked value is given', () => {
createWrapper();
expect(findCheckbox().props('checked')).toBe(false);
});
it('sets the checkbox to `true` when a `true` value is given', () => {
createWrapper({ value: true });
it('sets the checkbox to `true` when checked is `true`', () => {
createWrapper({ checked: true });
expect(findCheckbox().props('checked')).toBe(true);
});
......
......@@ -12,7 +12,7 @@ import createStore from 'ee/approvals/stores';
import approvalSettingsModule from 'ee/approvals/stores/modules/approval_settings/';
import { extendedWrapper } from 'helpers/vue_test_utils_helper';
import waitForPromises from 'helpers/wait_for_promises';
import { createGroupApprovalsPayload } from '../mocks';
import { createGroupApprovalsPayload, createGroupApprovalsState } from '../mocks';
const localVue = createLocalVue();
localVue.use(Vuex);
......@@ -101,13 +101,7 @@ describe('ApprovalSettings', () => {
});
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,
};
const { settings } = createGroupApprovalsState();
beforeEach(() => {
setupStore(settings);
......@@ -146,7 +140,15 @@ describe('ApprovalSettings', () => {
});
it('renders the button as enabled when a setting was changed', async () => {
setupStore({ ...settings, allow_author_approval: true }, settings);
const changedSettings = {
...settings,
preventAuthorApproval: {
...settings.preventAuthorApproval,
value: true,
},
};
setupStore(changedSettings, settings);
createWrapper();
await waitForPromises();
......@@ -203,11 +205,18 @@ describe('ApprovalSettings', () => {
expect(checkbox.props('label')).toBe(PROJECT_APPROVAL_SETTINGS_LABELS_I18N[labelKey]);
});
it('sets the locked and lockedText based on the setting values', () => {
expect(checkbox.props()).toMatchObject({
locked: settings[setting].locked,
lockedText: APPROVAL_SETTINGS_I18N.lockedByAdmin,
});
});
it(`triggers the action ${action} when the value is changed`, async () => {
await checkbox.vm.$emit('input', true);
await waitForPromises();
expect(store.dispatch).toHaveBeenLastCalledWith(action, { [setting]: true });
expect(store.dispatch).toHaveBeenLastCalledWith(action, true);
});
});
......@@ -263,6 +272,16 @@ describe('ApprovalSettings', () => {
});
describe('locked settings', () => {
beforeEach(() => {
setupStore({
...settings,
preventAuthorApproval: {
...settings.preventAuthorApproval,
locked: false,
},
});
});
it.each`
property | value | locked | testid
${'canPreventAuthorApproval'} | ${true} | ${false} | ${'prevent-author-approval'}
......@@ -276,10 +295,7 @@ describe('ApprovalSettings', () => {
({ property, value, locked, testid }) => {
createWrapper({ [property]: value });
expect(wrapper.findByTestId(testid).props()).toMatchObject({
locked,
lockedText: APPROVAL_SETTINGS_I18N.lockedByAdmin,
});
expect(wrapper.findByTestId(testid).props('locked')).toBe(locked);
},
);
});
......
import { groupApprovalsMappers } from 'ee/approvals/mappers';
import { createGroupApprovalsPayload } from './mocks';
import { createGroupApprovalsPayload, createGroupApprovalsState } from './mocks';
describe('approvals mappers', () => {
describe('groupApprovalsMappers', () => {
const groupFetchPayload = createGroupApprovalsPayload();
const groupUpdatePayload = {
const approvalsState = createGroupApprovalsState();
const approvalsFetchPayload = createGroupApprovalsPayload();
const approvalsUpdatePayload = {
allow_author_approval: true,
allow_overrides_to_approver_list_per_merge_request: true,
require_password_to_approve: true,
retain_approvals_on_push: true,
allow_committer_approval: true,
};
const groupState = {
settings: {
preventAuthorApproval: false,
preventMrApprovalRuleEdit: false,
requireUserPassword: true,
removeApprovalsOnPush: false,
preventCommittersApproval: false,
},
};
it('maps data to state', () => {
expect(groupApprovalsMappers.mapDataToState(groupFetchPayload)).toStrictEqual(
groupState.settings,
expect(groupApprovalsMappers.mapDataToState(approvalsFetchPayload)).toStrictEqual(
approvalsState.settings,
);
});
it('maps state to payload', () => {
expect(groupApprovalsMappers.mapStateToPayload(groupState)).toStrictEqual(groupUpdatePayload);
expect(groupApprovalsMappers.mapStateToPayload(approvalsState)).toStrictEqual(
approvalsUpdatePayload,
);
});
});
});
......@@ -63,3 +63,33 @@ export const createGroupApprovalsPayload = () => ({
inherited_from: null,
},
});
export const createGroupApprovalsState = () => ({
settings: {
preventAuthorApproval: {
inherited_from: 'instance',
locked: true,
value: false,
},
preventCommittersApproval: {
inherited_from: null,
locked: false,
value: false,
},
preventMrApprovalRuleEdit: {
inherited_from: null,
locked: false,
value: false,
},
removeApprovalsOnPush: {
inherited_from: null,
locked: null,
value: false,
},
requireUserPassword: {
inherited_from: null,
locked: null,
value: true,
},
},
});
......@@ -78,11 +78,11 @@ describe('EE approvals group settings module actions', () => {
describe('on success', () => {
it('dispatches the request and updates payload', () => {
const data = {
allow_author_approval: true,
allow_committer_approval: true,
allow_overrides_to_approver_list_per_merge_request: true,
require_password_to_approve: true,
retain_approvals_on_push: true,
allow_author_approval: { value: true },
allow_committer_approval: { value: true },
allow_overrides_to_approver_list_per_merge_request: { value: true },
require_password_to_approve: { value: true },
retain_approvals_on_push: { value: true },
};
mock[onMethod](approvalSettingsPath).replyOnce(httpStatus.OK, data);
......@@ -142,17 +142,15 @@ describe('EE approvals group settings module actions', () => {
});
describe.each`
action | type | prop
${'setPreventAuthorApproval'} | ${types.SET_PREVENT_AUTHOR_APPROVAL} | ${'preventAuthorApproval'}
${'setPreventCommittersApproval'} | ${types.SET_PREVENT_COMMITTERS_APPROVAL} | ${'preventCommittersApproval'}
${'setPreventMrApprovalRuleEdit'} | ${types.SET_PREVENT_MR_APPROVAL_RULE_EDIT} | ${'preventMrApprovalRuleEdit'}
${'setRemoveApprovalsOnPush'} | ${types.SET_REMOVE_APPROVALS_ON_PUSH} | ${'removeApprovalsOnPush'}
${'setRequireUserPassword'} | ${types.SET_REQUIRE_USER_PASSWORD} | ${'requireUserPassword'}
`('$action', ({ action, type, prop }) => {
action | type
${'setPreventAuthorApproval'} | ${types.SET_PREVENT_AUTHOR_APPROVAL}
${'setPreventCommittersApproval'} | ${types.SET_PREVENT_COMMITTERS_APPROVAL}
${'setPreventMrApprovalRuleEdit'} | ${types.SET_PREVENT_MR_APPROVAL_RULE_EDIT}
${'setRemoveApprovalsOnPush'} | ${types.SET_REMOVE_APPROVALS_ON_PUSH}
${'setRequireUserPassword'} | ${types.SET_REQUIRE_USER_PASSWORD}
`('$action', ({ action, type }) => {
it(`commits ${type}`, () => {
const payload = { [prop]: true };
return testAction(actions[action], payload, state, [{ type, payload: true }], []);
return testAction(actions[action], true, state, [{ type, payload: true }], []);
});
});
});
import { cloneDeep } from 'lodash';
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,
preventAuthorApproval: { value: true },
preventCommittersApproval: { value: true },
preventMrApprovalRuleEdit: { value: true },
requireUserPassword: { value: true },
removeApprovalsOnPush: { value: false },
};
beforeEach(() => {
settings = { ...initialSettings };
settings = cloneDeep(initialSettings);
});
describe('settingChanged', () => {
it('returns true when a setting is changed', () => {
settings.preventAuthorApproval = false;
settings.preventAuthorApproval.value = false;
expect(getters.settingChanged({ settings, initialSettings })).toBe(true);
});
......
......@@ -8,10 +8,11 @@ describe('Group settings store mutations', () => {
const mapperFn = jest.fn((data) => data);
const mutations = mutationsFactory(mapperFn);
const settings = {
preventAuthorApproval: true,
preventMrApprovalRuleEdit: true,
requireUserPassword: true,
removeApprovalsOnPush: true,
preventAuthorApproval: { value: false },
preventCommittersApproval: { value: false },
preventMrApprovalRuleEdit: { value: false },
requireUserPassword: { value: false },
removeApprovalsOnPush: { value: false },
};
beforeEach(() => {
......@@ -100,10 +101,14 @@ describe('Group settings store mutations', () => {
${'SET_REMOVE_APPROVALS_ON_PUSH'} | ${'removeApprovalsOnPush'}
${'SET_REQUIRE_USER_PASSWORD'} | ${'requireUserPassword'}
`('$mutation', ({ mutation, prop }) => {
beforeEach(() => {
mutations.RECEIVE_SETTINGS_SUCCESS(state, settings);
});
it(`sets the ${prop}`, () => {
mutations[mutation](state, true);
expect(state.settings[prop]).toBe(true);
expect(state.settings[prop].value).toBe(true);
});
});
});
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