diff --git a/ee/app/assets/javascripts/approvals/components/approval_settings.vue b/ee/app/assets/javascripts/approvals/components/approval_settings.vue
index fb048bba4aa96b402249294ac49d9de2e077b51b..f764e79a1c2bbf61985fa22c1c2391304bf909c5 100644
--- a/ee/app/assets/javascripts/approvals/components/approval_settings.vue
+++ b/ee/app/assets/javascripts/approvals/components/approval_settings.vue
@@ -21,7 +21,7 @@ export default {
},
computed: {
...mapState({
- preventAuthorApproval: (state) => state.approvals.preventAuthorApproval,
+ settings: (state) => state.approvals.settings,
isLoading: (state) => state.approvals.isLoading,
}),
},
@@ -29,7 +29,10 @@ export default {
this.fetchSettings(this.approvalSettingsPath);
},
methods: {
- ...mapActions(['fetchSettings', 'updatePreventAuthorApproval']),
+ ...mapActions(['fetchSettings', 'updateSettings']),
+ onSubmit() {
+ this.updateSettings(this.approvalSettingsPath);
+ },
},
links: {
preventAuthorApprovalDocsPath: helpPagePath(
@@ -48,12 +51,11 @@ export default {
</script>
<template>
- <gl-form>
+ <gl-form @submit.prevent="onSubmit">
<gl-form-group>
<gl-form-checkbox
- :checked="preventAuthorApproval"
+ v-model="settings.preventAuthorApproval"
data-testid="prevent-author-approval"
- @input="updatePreventAuthorApproval"
>
{{ $options.i18n.authorApprovalLabel }}
<gl-link :href="$options.links.preventAuthorApprovalDocsPath" target="_blank">
diff --git a/ee/app/assets/javascripts/approvals/components/group_settings/app.vue b/ee/app/assets/javascripts/approvals/components/group_settings/app.vue
index 18037c35756ae8caec9b227965d0296782ec3008..f7dce8fa3bebf0b8464917769c2977564739696b 100644
--- a/ee/app/assets/javascripts/approvals/components/group_settings/app.vue
+++ b/ee/app/assets/javascripts/approvals/components/group_settings/app.vue
@@ -34,7 +34,7 @@ export default {
</script>
<template>
- <settings-block :default-expanded="defaultExpanded">
+ <settings-block :default-expanded="defaultExpanded" data-testid="merge-request-approval-settings">
<template #title> {{ $options.i18n.groupSettingsHeader }}</template>
<template #description>
<gl-sprintf :message="$options.i18n.groupSettingsDescription">
diff --git a/ee/app/assets/javascripts/approvals/stores/modules/group_settings/actions.js b/ee/app/assets/javascripts/approvals/stores/modules/group_settings/actions.js
index 1486eabfee6c56e47f2dc96eb41b6b165003536d..30aed05af0debfa0c116a85e4fc57166c1d120fa 100644
--- a/ee/app/assets/javascripts/approvals/stores/modules/group_settings/actions.js
+++ b/ee/app/assets/javascripts/approvals/stores/modules/group_settings/actions.js
@@ -23,6 +23,26 @@ export const fetchSettings = ({ commit }, endpoint) => {
});
};
-export const updatePreventAuthorApproval = ({ commit }, preventAuthorApproval) => {
- commit(types.UPDATE_PREVENT_AUTHOR_APPROVAL, preventAuthorApproval);
+export const updateSettings = ({ commit, state }, endpoint) => {
+ const payload = {
+ allow_author_approval: !state.settings.preventAuthorApproval,
+ };
+
+ commit(types.REQUEST_UPDATE_SETTINGS);
+
+ return axios
+ .put(endpoint, payload)
+ .then(({ data }) => {
+ commit(types.UPDATE_SETTINGS_SUCCESS, data);
+ })
+ .catch(({ response }) => {
+ const error = response?.data?.message;
+
+ commit(types.UPDATE_SETTINGS_ERROR, error);
+ createFlash({
+ message: __('There was an error updating merge request approval settings.'),
+ captureError: true,
+ error,
+ });
+ });
};
diff --git a/ee/app/assets/javascripts/approvals/stores/modules/group_settings/mutation_types.js b/ee/app/assets/javascripts/approvals/stores/modules/group_settings/mutation_types.js
index 485d1aea763b8d535c6aa4ae00c05f1f11341ad5..de83844af767ce9820c82e877b8c38f0003d1a3f 100644
--- a/ee/app/assets/javascripts/approvals/stores/modules/group_settings/mutation_types.js
+++ b/ee/app/assets/javascripts/approvals/stores/modules/group_settings/mutation_types.js
@@ -3,4 +3,6 @@ export * from '../base/mutation_types';
export const REQUEST_SETTINGS = 'REQUEST_SETTINGS';
export const RECEIVE_SETTINGS_SUCCESS = 'RECEIVE_SETTINGS_SUCCESS';
export const RECEIVE_SETTINGS_ERROR = 'RECEIVE_SETTINGS_ERROR';
-export const UPDATE_PREVENT_AUTHOR_APPROVAL = 'UPDATE_PREVENT_AUTHOR_APPROVAL';
+export const REQUEST_UPDATE_SETTINGS = 'REQUEST_UPDATE_SETTINGS';
+export const UPDATE_SETTINGS_SUCCESS = 'UPDATE_SETTINGS_SUCCESS';
+export const UPDATE_SETTINGS_ERROR = 'UPDATE_SETTINGS_ERROR';
diff --git a/ee/app/assets/javascripts/approvals/stores/modules/group_settings/mutations.js b/ee/app/assets/javascripts/approvals/stores/modules/group_settings/mutations.js
index 55eb24b73fc77a373812cb8ed5824fcb805b88ce..60e240fd05e703480a6ebf9b84deb713fc7c625d 100644
--- a/ee/app/assets/javascripts/approvals/stores/modules/group_settings/mutations.js
+++ b/ee/app/assets/javascripts/approvals/stores/modules/group_settings/mutations.js
@@ -5,13 +5,20 @@ export default {
state.isLoading = true;
},
[types.RECEIVE_SETTINGS_SUCCESS](state, data) {
- state.preventAuthorApproval = !data.allow_author_approval;
+ state.settings.preventAuthorApproval = !data.allow_author_approval;
state.isLoading = false;
},
[types.RECEIVE_SETTINGS_ERROR](state) {
state.isLoading = false;
},
- [types.UPDATE_PREVENT_AUTHOR_APPROVAL](state, value) {
- state.preventAuthorApproval = value;
+ [types.REQUEST_UPDATE_SETTINGS](state) {
+ state.isLoading = true;
+ },
+ [types.UPDATE_SETTINGS_SUCCESS](state, data) {
+ state.settings.preventAuthorApproval = !data.allow_author_approval;
+ state.isLoading = false;
+ },
+ [types.UPDATE_SETTINGS_ERROR](state) {
+ state.isLoading = false;
},
};
diff --git a/ee/app/assets/javascripts/approvals/stores/modules/group_settings/state.js b/ee/app/assets/javascripts/approvals/stores/modules/group_settings/state.js
index c67440dae9a6f2336113698f31058336ebcae805..979c3c8cdcde5859cf81e95e4d0338c486f3ef8e 100644
--- a/ee/app/assets/javascripts/approvals/stores/modules/group_settings/state.js
+++ b/ee/app/assets/javascripts/approvals/stores/modules/group_settings/state.js
@@ -1,4 +1,4 @@
export default () => ({
- preventAuthorApproval: true,
+ settings: {},
isLoading: false,
});
diff --git a/ee/spec/features/groups/group_settings_spec.rb b/ee/spec/features/groups/group_settings_spec.rb
index 77758683a2c8e97b13201153833a9f403b0747b7..cc97423b55eeae09364cc04a58a1725f3079bef1 100644
--- a/ee/spec/features/groups/group_settings_spec.rb
+++ b/ee/spec/features/groups/group_settings_spec.rb
@@ -5,13 +5,16 @@
RSpec.describe 'Edit group settings' do
include Select2Helper
- let(:user) { create(:user) }
- let(:developer) { create(:user) }
- let(:group) { create(:group, path: 'foo') }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:developer) { create(:user) }
+ let_it_be(:group) { create(:group, path: 'foo') }
- before do
+ before_all do
group.add_owner(user)
group.add_developer(developer)
+ end
+
+ before do
sign_in(user)
end
@@ -275,16 +278,38 @@
end
describe 'merge request approval settings', :js do
+ let_it_be(:approval_settings) do
+ create(:group_merge_request_approval_setting, group: group, allow_author_approval: false)
+ end
+
context 'when feature flag is enabled and group is licensed' do
before do
stub_feature_flags(group_merge_request_approval_settings_feature_flag: true)
stub_licensed_features(group_merge_request_approval_settings: true)
end
- it 'is visible' do
+ it 'allows to save settings' do
visit edit_group_path(group)
+ wait_for_all_requests
expect(page).to have_content('Merge request approvals')
+
+ within('[data-testid="merge-request-approval-settings"]') do
+ click_button 'Expand'
+ expect(find('[data-testid="prevent-author-approval"]')).to be_checked
+
+ find('[data-testid="prevent-author-approval"]').set(false)
+ click_button 'Save changes'
+ wait_for_all_requests
+ end
+
+ visit edit_group_path(group)
+ wait_for_all_requests
+
+ within('[data-testid="merge-request-approval-settings"]') do
+ click_button 'Expand'
+ expect(find('[data-testid="prevent-author-approval"]')).not_to be_checked
+ end
end
end
diff --git a/ee/spec/frontend/approvals/components/approval_settings_spec.js b/ee/spec/frontend/approvals/components/approval_settings_spec.js
index 1a4d099f3def8c10c084a2baa8ae607bb7077976..9a6f02ebd12cce9be12dbd865c217bcae2f8a342 100644
--- a/ee/spec/frontend/approvals/components/approval_settings_spec.js
+++ b/ee/spec/frontend/approvals/components/approval_settings_spec.js
@@ -1,4 +1,4 @@
-import { GlButton } from '@gitlab/ui';
+import { GlButton, GlForm } from '@gitlab/ui';
import { createLocalVue, shallowMount } from '@vue/test-utils';
import Vuex from 'vuex';
@@ -24,6 +24,7 @@ describe('ApprovalSettings', () => {
});
};
+ const findForm = () => wrapper.findComponent(GlForm);
const findPreventAuthorApproval = () => wrapper.find('[data-testid="prevent-author-approval"]');
const findSaveButton = () => wrapper.findComponent(GlButton);
@@ -31,6 +32,7 @@ describe('ApprovalSettings', () => {
store = createStoreOptions(groupSettingsModule());
jest.spyOn(store.modules.approvals.actions, 'fetchSettings').mockImplementation();
+ jest.spyOn(store.modules.approvals.actions, 'updateSettings').mockImplementation();
({ actions } = store.modules.approvals);
});
@@ -52,8 +54,7 @@ describe('ApprovalSettings', () => {
const input = findPreventAuthorApproval();
await input.vm.$emit('input', false);
- expect(input.vm.$attrs.checked).toBe(false);
- expect(store.modules.approvals.state.preventAuthorApproval).toBe(false);
+ expect(store.modules.approvals.state.settings.preventAuthorApproval).toBe(false);
});
});
@@ -74,4 +75,14 @@ describe('ApprovalSettings', () => {
expect(findSaveButton().props('disabled')).toBe(true);
});
});
+
+ describe('form submission', () => {
+ it('update settings via API', async () => {
+ createWrapper();
+
+ await findForm().vm.$emit('submit', { preventDefault: () => {} });
+
+ expect(actions.updateSettings).toHaveBeenCalledWith(expect.any(Object), approvalSettingsPath);
+ });
+ });
});
diff --git a/ee/spec/frontend/approvals/stores/modules/group_settings/actions_spec.js b/ee/spec/frontend/approvals/stores/modules/group_settings/actions_spec.js
index ae7eb8ddeece38762d0a8e7747f10ebd3d33b638..b66b93849f7c2f091ac6ff553a0b043536625c3e 100644
--- a/ee/spec/frontend/approvals/stores/modules/group_settings/actions_spec.js
+++ b/ee/spec/frontend/approvals/stores/modules/group_settings/actions_spec.js
@@ -69,17 +69,55 @@ describe('EE approvals group settings module actions', () => {
});
});
- describe('updatePreventAuthorApproval', () => {
- it('updates payload', () => {
- const value = false;
+ describe('updateSettings', () => {
+ beforeEach(() => {
+ state = {
+ settings: {
+ preventAuthorApproval: false,
+ },
+ };
+ });
+
+ describe('on success', () => {
+ it('dispatches the request and updates payload', () => {
+ const data = { allow_author_approval: true };
+ mock.onPut(approvalSettingsPath).replyOnce(httpStatus.OK, data);
+
+ return testAction(
+ actions.updateSettings,
+ approvalSettingsPath,
+ state,
+ [
+ { type: types.REQUEST_UPDATE_SETTINGS },
+ { type: types.UPDATE_SETTINGS_SUCCESS, payload: data },
+ ],
+ [],
+ );
+ });
+ });
+
+ describe('on error', () => {
+ it('dispatches the request, updates payload and sets error message', () => {
+ const data = { message: 'Internal Server Error' };
+ mock.onPut(approvalSettingsPath).replyOnce(httpStatus.INTERNAL_SERVER_ERROR, data);
- return testAction(
- actions.updatePreventAuthorApproval,
- value,
- state,
- [{ type: types.UPDATE_PREVENT_AUTHOR_APPROVAL, payload: value }],
- [],
- );
+ return testAction(
+ actions.updateSettings,
+ approvalSettingsPath,
+ state,
+ [
+ { type: types.REQUEST_UPDATE_SETTINGS },
+ { type: types.UPDATE_SETTINGS_ERROR, payload: data.message },
+ ],
+ [],
+ ).then(() => {
+ expect(createFlash).toHaveBeenCalledWith({
+ message: 'There was an error updating merge request approval settings.',
+ captureError: true,
+ error: 'Internal Server Error',
+ });
+ });
+ });
});
});
});
diff --git a/ee/spec/frontend/approvals/stores/modules/group_settings/mutations_spec.js b/ee/spec/frontend/approvals/stores/modules/group_settings/mutations_spec.js
index 3c4c379b8509771b425d3101eeea5b8a46986776..c64ca9228fb607b28ae4452d7b4d710514559421 100644
--- a/ee/spec/frontend/approvals/stores/modules/group_settings/mutations_spec.js
+++ b/ee/spec/frontend/approvals/stores/modules/group_settings/mutations_spec.js
@@ -20,7 +20,7 @@ describe('Group settings store mutations', () => {
it('updates settings', () => {
mutations.RECEIVE_SETTINGS_SUCCESS(state, { allow_author_approval: true });
- expect(state.preventAuthorApproval).toBe(false);
+ expect(state.settings.preventAuthorApproval).toBe(false);
expect(state.isLoading).toBe(false);
});
});
@@ -33,11 +33,28 @@ describe('Group settings store mutations', () => {
});
});
- describe('UPDATE_PREVENT_AUTHOR_APPROVAL', () => {
- it('updates setting', () => {
- mutations.UPDATE_PREVENT_AUTHOR_APPROVAL(state, false);
+ describe('REQUEST_UPDATE_SETTINGS', () => {
+ it('sets loading state', () => {
+ mutations.REQUEST_UPDATE_SETTINGS(state);
+
+ expect(state.isLoading).toBe(true);
+ });
+ });
+
+ describe('UPDATE_SETTINGS_SUCCESS', () => {
+ it('updates settings', () => {
+ mutations.UPDATE_SETTINGS_SUCCESS(state, { allow_author_approval: true });
- expect(state.preventAuthorApproval).toBe(false);
+ expect(state.settings.preventAuthorApproval).toBe(false);
+ expect(state.isLoading).toBe(false);
+ });
+ });
+
+ describe('UPDATE_SETTINGS_ERROR', () => {
+ it('sets loading state', () => {
+ mutations.UPDATE_SETTINGS_ERROR(state);
+
+ expect(state.isLoading).toBe(false);
});
});
});
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 2b5a8e81a91bec89f7a30e370935c24e8696ef97..083a856101793268416ac32ff085730c31d6e59c 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -29701,6 +29701,9 @@ msgstr ""
msgid "There was an error trying to validate your query"
msgstr ""
+msgid "There was an error updating merge request approval settings."
+msgstr ""
+
msgid "There was an error updating the Geo Settings"
msgstr ""