Skip to content

G
gitlab-ce
Commit 925655ed authored by Robert Hunt's avatar Robert Hunt Committed by Savas Vedova
Browse files
Options

Add loading icon and form hiding on error to group-level merge request settings

parent c83cf1f8
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 358 additions and 181 deletions
ee/app/assets/javascripts/approvals/components/approval_settings.vue
View file @ 925655ed
<script>
import { GlButton, GlForm, GlFormGroup } from '@gitlab/ui';
import { GlAlert, GlButton, GlForm, GlFormGroup, GlLoadingIcon } from '@gitlab/ui';
import { isEmpty } from 'lodash';
import { mapActions, mapState } from 'vuex';
import { mapComputed } from '~/vuex_shared/bindings';
import { APPROVAL_SETTINGS_I18N } from '../constants';
......@@ -8,9 +9,11 @@ import ApprovalSettingsCheckbox from './approval_settings_checkbox.vue';
export default {
components: {
ApprovalSettingsCheckbox,
GlAlert,
GlButton,
GlForm,
GlFormGroup,
GlLoadingIcon,
},
props: {
approvalSettingsPath: {
......@@ -18,14 +21,12 @@ export default {
required: true,
},
},
data() {
return {
hasFormLoaded: false,
};
},
computed: {
...mapState({
isLoading: (state) => state.approvals.isLoading,
isUpdated: (state) => state.approvals.isUpdated,
settings: (state) => state.approvals.settings,
errorMessage: (state) => state.approvals.errorMessage,
}),
...mapComputed(
[
......@@ -38,15 +39,25 @@ export default {
undefined,
(state) => state.approvals.settings,
),
hasSettings() {
return !isEmpty(this.settings);
},
isLoaded() {
return this.hasSettings || this.errorMessage;
},
},
async created() {
await this.fetchSettings(this.approvalSettingsPath);
this.hasFormLoaded = true;
created() {
this.fetchSettings(this.approvalSettingsPath);
},
methods: {
...mapActions(['fetchSettings', 'updateSettings']),
onSubmit() {
this.updateSettings(this.approvalSettingsPath);
...mapActions([
'fetchSettings',
'updateSettings',
'dismissErrorMessage',
'dismissSuccessMessage',
]),
async onSubmit() {
await this.updateSettings(this.approvalSettingsPath);
},
},
links: {
......@@ -62,41 +73,66 @@ export default {
</script>
<template>
<gl-form v-if="hasFormLoaded" @submit.prevent="onSubmit">
<gl-form-group>
<approval-settings-checkbox
v-model="preventAuthorApproval"
:label="$options.i18n.authorApprovalLabel"
:anchor="$options.links.preventAuthorApprovalDocsAnchor"
data-testid="prevent-author-approval"
/>
<approval-settings-checkbox
v-model="preventMrApprovalRuleEdit"
:label="$options.i18n.preventMrApprovalRuleEditLabel"
:anchor="$options.links.preventMrApprovalRuleEditDocsAnchor"
data-testid="prevent-mr-approval-rule-edit"
/>
<approval-settings-checkbox
v-model="requireUserPassword"
:label="$options.i18n.requireUserPasswordLabel"
:anchor="$options.links.requireUserPasswordDocsAnchor"
data-testid="require-user-password"
/>
<approval-settings-checkbox
v-model="removeApprovalsOnPush"
:label="$options.i18n.removeApprovalsOnPushLabel"
:anchor="$options.links.removeApprovalsOnPushDocsAnchor"
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-button type="submit" variant="success" category="primary" :disabled="isLoading">
{{ $options.i18n.saveChanges }}
</gl-button>
</gl-form>
<div>
<gl-loading-icon v-if="!isLoaded" size="lg" />
<gl-alert
v-if="errorMessage"
variant="danger"
:dismissible="hasSettings"
:contained="true"
:class="{ 'gl-mb-6': hasSettings }"
data-testid="error-alert"
@dismiss="dismissErrorMessage"
>
{{ errorMessage }}
</gl-alert>
<gl-alert
v-if="isUpdated"
variant="success"
:dismissible="true"
:contained="true"
class="gl-mb-6"
data-testid="success-alert"
@dismiss="dismissSuccessMessage"
>
{{ $options.i18n.savingSuccessMessage }}
</gl-alert>
<gl-form v-if="hasSettings" @submit.prevent="onSubmit">
<gl-form-group>
<approval-settings-checkbox
v-model="preventAuthorApproval"
:label="$options.i18n.authorApprovalLabel"
:anchor="$options.links.preventAuthorApprovalDocsAnchor"
data-testid="prevent-author-approval"
/>
<approval-settings-checkbox
v-model="preventMrApprovalRuleEdit"
:label="$options.i18n.preventMrApprovalRuleEditLabel"
:anchor="$options.links.preventMrApprovalRuleEditDocsAnchor"
data-testid="prevent-mr-approval-rule-edit"
/>
<approval-settings-checkbox
v-model="requireUserPassword"
:label="$options.i18n.requireUserPasswordLabel"
:anchor="$options.links.requireUserPasswordDocsAnchor"
data-testid="require-user-password"
/>
<approval-settings-checkbox
v-model="removeApprovalsOnPush"
:label="$options.i18n.removeApprovalsOnPushLabel"
:anchor="$options.links.removeApprovalsOnPushDocsAnchor"
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-button type="submit" variant="confirm" category="primary" :loading="isLoading">
{{ $options.i18n.saveChanges }}
</gl-button>
</gl-form>
</div>
</template>
ee/app/assets/javascripts/approvals/constants.js
View file @ 925655ed
......@@ -50,14 +50,23 @@ export const APPROVAL_RULE_CONFIGS = {
export const APPROVALS_HELP_PATH = 'user/project/merge_requests/merge_request_approvals';
export const APPROVAL_SETTINGS_I18N = {
authorApprovalLabel: __('Prevent MR approvals by the author.'),
preventMrApprovalRuleEditLabel: __('Prevent users from modifying MR approval rules.'),
preventCommittersApprovalLabel: __(
'Prevent approval of merge requests by merge request committers.',
authorApprovalLabel: s__('ApprovalSettings|Prevent MR approvals by the author.'),
preventMrApprovalRuleEditLabel: s__(
'ApprovalSettings|Prevent users from modifying MR approval rules.',
),
requireUserPasswordLabel: __('Require user password for approvals.'),
removeApprovalsOnPushLabel: __(
'Remove all approvals in a merge request when new commits are pushed to its source branch.',
preventCommittersApprovalLabel: s__(
'ApprovalSettings|Prevent approval of merge requests by merge request committers.',
),
requireUserPasswordLabel: s__('ApprovalSettings|Require user password for approvals.'),
removeApprovalsOnPushLabel: s__(
'ApprovalSettings|Remove all approvals in a merge request when new commits are pushed to its source branch.',
),
saveChanges: __('Save changes'),
loadingErrorMessage: s__(
'ApprovalSettings|There was an error loading merge request approval settings.',
),
savingErrorMessage: s__(
'ApprovalSettings|There was an error updating merge request approval settings.',
),
savingSuccessMessage: s__('ApprovalSettings|Merge request approval settings have been updated.'),
};
ee/app/assets/javascripts/approvals/stores/modules/group_settings/actions.js
View file @ 925655ed
import createFlash from '~/flash';
import * as Sentry from '@sentry/browser';
import axios from '~/lib/utils/axios_utils';
import { __ } from '~/locale';
import * as types from './mutation_types';
const mapStateToPayload = (state) => ({
......@@ -19,15 +18,11 @@ export const fetchSettings = ({ commit }, endpoint) => {
.then(({ data }) => {
commit(types.RECEIVE_SETTINGS_SUCCESS, data);
})
.catch(({ response }) => {
const error = response?.data?.message;
.catch((e) => {
const error = e?.response?.data?.message || e;
commit(types.RECEIVE_SETTINGS_ERROR, error);
createFlash({
message: __('There was an error loading merge request approval settings.'),
captureError: true,
error,
});
Sentry.captureException(error);
commit(types.RECEIVE_SETTINGS_ERROR);
});
};
......@@ -38,23 +33,23 @@ export const updateSettings = ({ commit, state }, endpoint) => {
.put(endpoint, { ...mapStateToPayload(state) })
.then(({ data }) => {
commit(types.UPDATE_SETTINGS_SUCCESS, data);
createFlash({
message: __('Merge request approval settings have been updated.'),
type: 'notice',
});
})
.catch(({ response }) => {
const error = response?.data?.message;
.catch((e) => {
const error = e?.response?.data?.message || e;
commit(types.UPDATE_SETTINGS_ERROR, error);
createFlash({
message: __('There was an error updating merge request approval settings.'),
captureError: true,
error,
});
Sentry.captureException(error);
commit(types.UPDATE_SETTINGS_ERROR);
});
};
export const dismissSuccessMessage = ({ commit }) => {
commit(types.DISMISS_SUCCESS_MESSAGE);
};
export const dismissErrorMessage = ({ commit }) => {
commit(types.DISMISS_ERROR_MESSAGE);
};
export const setPreventAuthorApproval = ({ commit }, { preventAuthorApproval }) => {
commit(types.SET_PREVENT_AUTHOR_APPROVAL, preventAuthorApproval);
};
......
ee/app/assets/javascripts/approvals/stores/modules/group_settings/mutation_types.js
View file @ 925655ed
......@@ -6,6 +6,8 @@ export const RECEIVE_SETTINGS_ERROR = 'RECEIVE_SETTINGS_ERROR';
export const REQUEST_UPDATE_SETTINGS = 'REQUEST_UPDATE_SETTINGS';
export const UPDATE_SETTINGS_SUCCESS = 'UPDATE_SETTINGS_SUCCESS';
export const UPDATE_SETTINGS_ERROR = 'UPDATE_SETTINGS_ERROR';
export const DISMISS_SUCCESS_MESSAGE = 'DISMISS_SUCCESS_MESSAGE';
export const DISMISS_ERROR_MESSAGE = 'DISMISS_ERROR_MESSAGE';
export const SET_PREVENT_AUTHOR_APPROVAL = 'SET_PREVENT_AUTHOR_APPROVAL';
export const SET_PREVENT_COMMITTERS_APPROVAL = 'SET_PREVENT_COMMITTERS_APPROVAL';
......
ee/app/assets/javascripts/approvals/stores/modules/group_settings/mutations.js
View file @ 925655ed
import { APPROVAL_SETTINGS_I18N } from '../../../constants';
import * as types from './mutation_types';
const mapDataToState = (data) => ({
......@@ -11,6 +12,7 @@ const mapDataToState = (data) => ({
export default {
[types.REQUEST_SETTINGS](state) {
state.isLoading = true;
state.errorMessage = '';
},
[types.RECEIVE_SETTINGS_SUCCESS](state, data) {
state.settings = { ...mapDataToState(data) };
......@@ -18,16 +20,27 @@ export default {
},
[types.RECEIVE_SETTINGS_ERROR](state) {
state.isLoading = false;
state.errorMessage = APPROVAL_SETTINGS_I18N.loadingErrorMessage;
},
[types.REQUEST_UPDATE_SETTINGS](state) {
state.isLoading = true;
state.isUpdated = false;
state.errorMessage = '';
},
[types.UPDATE_SETTINGS_SUCCESS](state, data) {
state.settings = { ...mapDataToState(data) };
state.isLoading = false;
state.isUpdated = true;
},
[types.UPDATE_SETTINGS_ERROR](state) {
state.isLoading = false;
state.errorMessage = APPROVAL_SETTINGS_I18N.savingErrorMessage;
},
[types.DISMISS_SUCCESS_MESSAGE](state) {
state.isUpdated = false;
},
[types.DISMISS_ERROR_MESSAGE](state) {
state.errorMessage = '';
},
[types.SET_PREVENT_AUTHOR_APPROVAL](state, preventAuthorApproval) {
state.settings.preventAuthorApproval = preventAuthorApproval;
......
ee/app/assets/javascripts/approvals/stores/modules/group_settings/state.js
View file @ 925655ed
export default () => ({
settings: {},
isLoading: false,
isUpdated: false,
errorMessage: '',
});
ee/spec/frontend/approvals/stores/modules/group_settings/actions_spec.js
View file @ 925655ed
import * as Sentry from '@sentry/browser';
import MockAdapter from 'axios-mock-adapter';
import * as actions from 'ee/approvals/stores/modules/group_settings/actions';
import * as types from 'ee/approvals/stores/modules/group_settings/mutation_types';
import getInitialState from 'ee/approvals/stores/modules/group_settings/state';
import testAction from 'helpers/vuex_action_helper';
import createFlash from '~/flash';
import axios from '~/lib/utils/axios_utils';
import httpStatus from '~/lib/utils/http_status';
jest.mock('~/flash');
describe('EE approvals group settings module actions', () => {
let state;
let mock;
......@@ -18,10 +16,10 @@ describe('EE approvals group settings module actions', () => {
beforeEach(() => {
state = getInitialState();
mock = new MockAdapter(axios);
jest.spyOn(Sentry, 'captureException');
});
afterEach(() => {
createFlash.mockClear();
mock.restore();
});
......@@ -53,17 +51,10 @@ describe('EE approvals group settings module actions', () => {
actions.fetchSettings,
approvalSettingsPath,
state,
[
{ type: types.REQUEST_SETTINGS },
{ type: types.RECEIVE_SETTINGS_ERROR, payload: data.message },
],
[{ type: types.REQUEST_SETTINGS }, { type: types.RECEIVE_SETTINGS_ERROR }],
[],
).then(() => {
expect(createFlash).toHaveBeenCalledWith({
message: 'There was an error loading merge request approval settings.',
captureError: true,
error: 'Internal Server Error',
});
expect(Sentry.captureException.mock.calls[0][0]).toBe(data.message);
});
});
});
......@@ -102,12 +93,7 @@ describe('EE approvals group settings module actions', () => {
{ type: types.UPDATE_SETTINGS_SUCCESS, payload: data },
],
[],
).then(() => {
expect(createFlash).toHaveBeenCalledWith({
message: 'Merge request approval settings have been updated.',
type: 'notice',
});
});
);
});
});
......@@ -120,22 +106,39 @@ describe('EE approvals group settings module actions', () => {
actions.updateSettings,
approvalSettingsPath,
state,
[
{ type: types.REQUEST_UPDATE_SETTINGS },
{ type: types.UPDATE_SETTINGS_ERROR, payload: data.message },
],
[{ type: types.REQUEST_UPDATE_SETTINGS }, { type: types.UPDATE_SETTINGS_ERROR }],
[],
).then(() => {
expect(createFlash).toHaveBeenCalledWith({
message: 'There was an error updating merge request approval settings.',
captureError: true,
error: 'Internal Server Error',
});
expect(Sentry.captureException.mock.calls[0][0]).toBe(data.message);
});
});
});
});
describe('dismissSuccessMessage', () => {
it('commits DISMISS_SUCCESS_MESSAGE', () => {
return testAction(
actions.dismissSuccessMessage,
{},
state,
[{ type: types.DISMISS_SUCCESS_MESSAGE }],
[],
);
});
});
describe('dismissErrorMessage', () => {
it('commits DISMISS_ERROR_MESSAGE', () => {
return testAction(
actions.dismissErrorMessage,
{},
state,
[{ type: types.DISMISS_ERROR_MESSAGE }],
[],
);
});
});
describe.each`
action | type | prop
${'setPreventAuthorApproval'} | ${types.SET_PREVENT_AUTHOR_APPROVAL} | ${'preventAuthorApproval'}
......
ee/spec/frontend/approvals/stores/modules/group_settings/mutations_spec.js
View file @ 925655ed
import { APPROVAL_SETTINGS_I18N } from 'ee/approvals/constants';
import mutations from 'ee/approvals/stores/modules/group_settings/mutations';
import getInitialState from 'ee/approvals/stores/modules/group_settings/state';
......@@ -13,6 +14,7 @@ describe('Group settings store mutations', () => {
mutations.REQUEST_SETTINGS(state);
expect(state.isLoading).toBe(true);
expect(state.errorMessage).toBe('');
});
});
......@@ -40,6 +42,7 @@ describe('Group settings store mutations', () => {
mutations.RECEIVE_SETTINGS_ERROR(state);
expect(state.isLoading).toBe(false);
expect(state.errorMessage).toBe(APPROVAL_SETTINGS_I18N.loadingErrorMessage);
});
});
......@@ -48,6 +51,8 @@ describe('Group settings store mutations', () => {
mutations.REQUEST_UPDATE_SETTINGS(state);
expect(state.isLoading).toBe(true);
expect(state.isUpdated).toBe(false);
expect(state.errorMessage).toBe('');
});
});
......@@ -65,6 +70,7 @@ describe('Group settings store mutations', () => {
expect(state.settings.requireUserPassword).toBe(true);
expect(state.settings.removeApprovalsOnPush).toBe(false);
expect(state.isLoading).toBe(false);
expect(state.isUpdated).toBe(true);
});
});
......@@ -73,6 +79,23 @@ describe('Group settings store mutations', () => {
mutations.UPDATE_SETTINGS_ERROR(state);
expect(state.isLoading).toBe(false);
expect(state.errorMessage).toBe(APPROVAL_SETTINGS_I18N.savingErrorMessage);
});
});
describe('DISMISS_SUCCESS_MESSAGE', () => {
it('resets isUpdated', () => {
mutations.DISMISS_SUCCESS_MESSAGE(state);
expect(state.isUpdated).toBe(false);
});
});
describe('DISMISS_ERROR_MESSAGE', () => {
it('resets errorMessage', () => {
mutations.DISMISS_ERROR_MESSAGE(state);
expect(state.errorMessage).toBe('');
});
});
......
locale/gitlab.pot
View file @ 925655ed
......@@ -4160,6 +4160,30 @@ msgstr ""
msgid "ApprovalRule|Target branch"
msgstr ""
msgid "ApprovalSettings|Merge request approval settings have been updated."
msgstr ""
msgid "ApprovalSettings|Prevent MR approvals by the author."
msgstr ""
msgid "ApprovalSettings|Prevent approval of merge requests by merge request committers."
msgstr ""
msgid "ApprovalSettings|Prevent users from modifying MR approval rules."
msgstr ""
msgid "ApprovalSettings|Remove all approvals in a merge request when new commits are pushed to its source branch."
msgstr ""
msgid "ApprovalSettings|Require user password for approvals."
msgstr ""
msgid "ApprovalSettings|There was an error loading merge request approval settings."
msgstr ""
msgid "ApprovalSettings|There was an error updating merge request approval settings."
msgstr ""
msgid "ApprovalStatusTooltip|Adheres to separation of duties"
msgstr ""
......@@ -20486,9 +20510,6 @@ msgstr ""
msgid "Merge request analytics"
msgstr ""
msgid "Merge request approval settings have been updated."
msgstr ""
msgid "Merge request approvals"
msgstr ""
......@@ -24764,9 +24785,6 @@ msgstr ""
msgid "Prevent adding new members to project membership within this group"
msgstr ""
msgid "Prevent approval of merge requests by merge request committers."
msgstr ""
msgid "Prevent environment from auto-stopping"
msgstr ""
......@@ -24782,9 +24800,6 @@ msgstr ""
msgid "Prevent users from modifying MR approval rules in projects and merge requests."
msgstr ""
msgid "Prevent users from modifying MR approval rules."
msgstr ""
msgid "Prevent users from performing write operations on GitLab while performing maintenance."
msgstr ""
......@@ -27123,9 +27138,6 @@ msgstr ""
msgid "Remove access"
msgstr ""
msgid "Remove all approvals in a merge request when new commits are pushed to its source branch."
msgstr ""
msgid "Remove all or specific assignee(s)"
msgstr ""
......@@ -33123,9 +33135,6 @@ msgstr ""
msgid "There was an error importing the Jira project."
msgstr ""
msgid "There was an error loading merge request approval settings."
msgstr ""
msgid "There was an error loading related feature flags"
msgstr ""
......@@ -33165,9 +33174,6 @@ 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 ""
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7