Skip to content

G
gitlab-ce
Commit 4d0aa962 authored by Natalia Tepluhina's avatar Natalia Tepluhina
Browse files
Options

Merge branch '323729-follow-up-update-ruleform-inputs-to-use-glinput-and-glformgroup' into 'master' 

Update RuleForm inputs to use GlFormInput and GlFormGroup

See merge request gitlab-org/gitlab!56319
parents e92d10e4 a558f7f8
Hide whitespace changes
Inline Side-by-side
Showing with 305 additions and 323 deletions
ee/app/assets/javascripts/approvals/components/rule_form.vue
View file @ 4d0aa962
<script> <script>
import { GlFormGroup, GlFormInput } from '@gitlab/ui';
import { groupBy, isNumber } from 'lodash'; import { groupBy, isNumber } from 'lodash';
import { mapState, mapActions } from 'vuex'; import { mapState, mapActions } from 'vuex';
import { isSafeURL } from '~/lib/utils/url_utility'; import { isSafeURL } from '~/lib/utils/url_utility';
...@@ -27,10 +28,12 @@ function mapServerResponseToValidationErrors(messages) { ...@@ -27,10 +28,12 @@ function mapServerResponseToValidationErrors(messages) {
export default { export default {
components: { components: {
ApproverTypeSelect,
ApproversList, ApproversList,
ApproversSelect, ApproversSelect,
BranchesSelect, BranchesSelect,
ApproverTypeSelect, GlFormGroup,
GlFormInput,
}, },
mixins: [glFeatureFlagsMixin()], mixins: [glFeatureFlagsMixin()],
props: { props: {
...@@ -95,7 +98,9 @@ export default { ...@@ -95,7 +98,9 @@ export default {
invalidApprovalGateUrl() { invalidApprovalGateUrl() {
if (this.serverValidationErrors.includes('External url has already been taken')) { if (this.serverValidationErrors.includes('External url has already been taken')) {
return this.$options.i18n.validations.externalUrlTaken; return this.$options.i18n.validations.externalUrlTaken;
} else if (!this.externalUrl || !isSafeURL(this.externalUrl)) { }
if (!this.externalUrl || !isSafeURL(this.externalUrl)) {
return this.$options.i18n.validations.invalidUrl; return this.$options.i18n.validations.invalidUrl;
} }
...@@ -266,7 +271,7 @@ export default { ...@@ -266,7 +271,7 @@ export default {
* - Single rule? * - Single rule?
* - Multi rule? * - Multi rule?
*/ */
submit() { async submit() {
let submission; let submission;
this.serverValidationErrors = []; this.serverValidationErrors = [];
...@@ -275,16 +280,18 @@ export default { ...@@ -275,16 +280,18 @@ export default {
const valid = this.isExternalApprovalRule ? this.isValidExternalApprovalRule : this.isValid; const valid = this.isExternalApprovalRule ? this.isValidExternalApprovalRule : this.isValid;
if (!valid) { if (!valid) {
submission = Promise.resolve(); submission = Promise.resolve;
} else if (this.isFallbackSubmission) { } else if (this.isFallbackSubmission) {
submission = this.submitFallback(); submission = this.submitFallback;
} else if (!this.isMultiSubmission) { } else if (!this.isMultiSubmission) {
submission = this.submitSingleRule(); submission = this.submitSingleRule;
} else { } else {
submission = this.submitRule(); submission = this.submitRule;
} }
submission.catch((failureResponse) => { try {
await submission();
} catch (failureResponse) {
if (this.isExternalApprovalRule) { if (this.isExternalApprovalRule) {
this.serverValidationErrors = failureResponse?.response?.data?.message || []; this.serverValidationErrors = failureResponse?.response?.data?.message || [];
} else { } else {
...@@ -292,9 +299,7 @@ export default { ...@@ -292,9 +299,7 @@ export default {
failureResponse?.response?.data?.message || {}, failureResponse?.response?.data?.message || {},
); );
} }
}); }
return submission;
}, },
/** /**
* Submit the rule, by either put-ing or post-ing. * Submit the rule, by either put-ing or post-ing.
...@@ -304,10 +309,13 @@ export default { ...@@ -304,10 +309,13 @@ export default {
const data = this.externalRuleSubmissionData; const data = this.externalRuleSubmissionData;
return data.id ? this.putExternalApprovalRule(data) : this.postExternalApprovalRule(data); return data.id ? this.putExternalApprovalRule(data) : this.postExternalApprovalRule(data);
} }
const data = this.submissionData; const data = this.submissionData;
if (!this.settings.allowMultiRule && this.settings.prefix === 'mr-edit') { if (!this.settings.allowMultiRule && this.settings.prefix === 'mr-edit') {
return data.id ? this.putRule(data) : this.postRegularRule(data); return data.id ? this.putRule(data) : this.postRegularRule(data);
} }
return data.id ? this.putRule(data) : this.postRule(data); return data.id ? this.putRule(data) : this.postRule(data);
}, },
/** /**
...@@ -416,86 +424,92 @@ export default { ...@@ -416,86 +424,92 @@ export default {
<template> <template>
<form novalidate @submit.prevent.stop="submit"> <form novalidate @submit.prevent.stop="submit">
<div v-if="showName" class="form-group gl-form-group"> <gl-form-group
<label class="col-form-label">{{ $options.i18n.form.nameLabel }}</label> v-if="showName"
<input :label="$options.i18n.form.nameLabel"
:description="$options.i18n.form.nameDescription"
:state="isValidName"
:invalid-feedback="invalidName"
data-testid="name-group"
>
<gl-form-input
v-model="name" v-model="name"
:class="{ 'is-invalid': !isValidName }"
:disabled="isNameDisabled" :disabled="isNameDisabled"
class="gl-form-input form-control" :state="isValidName"
name="name"
type="text"
data-qa-selector="rule_name_field" data-qa-selector="rule_name_field"
data-testid="name"
/> />
<span class="invalid-feedback">{{ isValidName ? '' : invalidName }}</span> </gl-form-group>
<small class="form-text text-gl-muted"> <gl-form-group
{{ $options.i18n.form.nameDescription }} v-if="showProtectedBranch"
</small> :label="$options.i18n.form.protectedBranchLabel"
</div> :description="$options.i18n.form.protectedBranchDescription"
<div v-if="showProtectedBranch" class="form-group gl-form-group"> :state="isValidBranches"
<label class="col-form-label">{{ $options.i18n.form.protectedBranchLabel }}</label> :invalid-feedback="invalidBranches"
data-testid="branches-group"
>
<branches-select <branches-select
v-model="branchesToAdd" v-model="branchesToAdd"
:project-id="settings.projectId" :project-id="settings.projectId"
:is-invalid="!isValidBranches" :is-invalid="!isValidBranches"
:init-rule="rule" :init-rule="rule"
/> />
<span class="invalid-feedback">{{ isValidBranches ? '' : invalidBranches }}</span> </gl-form-group>
<small class="form-text text-gl-muted"> <gl-form-group v-if="showApproverTypeSelect" :label="$options.i18n.form.approvalTypeLabel">
{{ $options.i18n.form.protectedBranchDescription }}
</small>
</div>
<div v-if="showApproverTypeSelect" class="form-group gl-form-group">
<label class="col-form-label">{{ $options.i18n.form.approvalTypeLabel }}</label>
<approver-type-select <approver-type-select
v-model="ruleType" v-model="ruleType"
:approver-type-options="$options.approverTypeOptions" :approver-type-options="$options.approverTypeOptions"
/> />
</div> </gl-form-group>
<div v-if="!isExternalApprovalRule" class="form-group gl-form-group"> <template v-if="!isExternalApprovalRule">
<label class="col-form-label">{{ $options.i18n.form.approvalsRequiredLabel }}</label> <gl-form-group
<input :label="$options.i18n.form.approvalsRequiredLabel"
v-model.number="approvalsRequired" :state="isValidApprovalsRequired"
:class="{ 'is-invalid': !isValidApprovalsRequired }" :invalid-feedback="invalidApprovalsRequired"
class="gl-form-input form-control mw-6em" data-testid="approvals-required-group"
name="approvals_required" >
type="number" <gl-form-input
:min="minApprovalsRequired" v-model.number="approvalsRequired"
data-qa-selector="approvals_required_field" :state="isValidApprovalsRequired"
/> :min="minApprovalsRequired"
<span class="invalid-feedback">{{ class="mw-6em"
isValidApprovalsRequired ? '' : invalidApprovalsRequired type="number"
}}</span> data-testid="approvals-required"
</div> data-qa-selector="approvals_required_field"
<div v-if="!isExternalApprovalRule" class="form-group gl-form-group"> />
<label class="col-form-label">{{ $options.i18n.form.approversLabel }}</label> </gl-form-group>
<approvers-select <gl-form-group
v-model="approversToAdd" :label="$options.i18n.form.approversLabel"
:project-id="settings.projectId" :state="isValidApprovers"
:skip-user-ids="userIds" :invalid-feedback="invalidApprovers"
:skip-group-ids="groupIds" data-testid="approvers-group"
:is-invalid="!isValidApprovers" >
data-qa-selector="member_select_field" <approvers-select
/> v-model="approversToAdd"
<span class="invalid-feedback">{{ isValidApprovers ? '' : invalidApprovers }}</span> :project-id="settings.projectId"
</div> :skip-user-ids="userIds"
<div v-if="isExternalApprovalRule" class="form-group gl-form-group"> :skip-group-ids="groupIds"
<label class="col-form-label">{{ approvalGateLabel }}</label> :is-invalid="!isValidApprovers"
<input data-qa-selector="member_select_field"
/>
</gl-form-group>
</template>
<gl-form-group
v-if="isExternalApprovalRule"
:label="approvalGateLabel"
:description="$options.i18n.form.approvalGateDescription"
:state="isValidApprovalGateUrl"
:invalid-feedback="invalidApprovalGateUrl"
data-testid="approval-gate-url-group"
>
<gl-form-input
v-model="externalUrl" v-model="externalUrl"
:class="{ 'is-invalid': !isValidApprovalGateUrl }" :state="isValidApprovalGateUrl"
class="gl-form-input form-control"
name="approval_gate_url"
type="url" type="url"
data-qa-selector="external_url_field" data-qa-selector="external_url_field"
data-testid="approval-gate-url"
/> />
<span class="invalid-feedback">{{ </gl-form-group>
isValidApprovalGateUrl ? '' : invalidApprovalGateUrl
}}</span>
<small class="form-text text-gl-muted">
{{ $options.i18n.form.approvalGateDescription }}
</small>
</div>
<div v-if="!isExternalApprovalRule" class="bordered-box overflow-auto h-12em"> <div v-if="!isExternalApprovalRule" class="bordered-box overflow-auto h-12em">
<approvers-list v-model="approvers" /> <approvers-list v-model="approvers" />
</div> </div>
......
ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb
View file @ 4d0aa962
...@@ -62,7 +62,9 @@ RSpec.describe 'Merge request > User edits MR with approval rules', :js do ...@@ -62,7 +62,9 @@ RSpec.describe 'Merge request > User edits MR with approval rules', :js do
click_button "Add approval rule" click_button "Add approval rule"
fill_in "name", with: rule_name within_fieldset('Rule name') do
fill_in with: rule_name
end
add_approval_rule_member('user', approver.name) add_approval_rule_member('user', approver.name)
......
ee/spec/features/merge_request/user_sets_approvers_spec.rb
View file @ 4d0aa962
...@@ -228,9 +228,11 @@ RSpec.describe 'Merge request > User sets approvers', :js do ...@@ -228,9 +228,11 @@ RSpec.describe 'Merge request > User sets approvers', :js do
find('.merge-request').click_on 'Edit' find('.merge-request').click_on 'Edit'
open_modal open_modal
expect(page).to have_field 'approvals_required', exact: 2 within_fieldset('Approvals required') do
expect(find_field.value).to eq '2'
fill_in 'approvals_required', with: '3' fill_in with: '3'
end
click_button 'Update approval rule' click_button 'Update approval rule'
click_on('Save changes') click_on('Save changes')
...@@ -245,7 +247,9 @@ RSpec.describe 'Merge request > User sets approvers', :js do ...@@ -245,7 +247,9 @@ RSpec.describe 'Merge request > User sets approvers', :js do
open_modal open_modal
expect(page).to have_field 'approvals_required', exact: 3 within_fieldset('Approvals required') do
expect(find_field.value).to eq '3'
end
end end
end end
end end
......
ee/spec/frontend/approvals/components/rule_form_spec.js
View file @ 4d0aa962
import { GlFormGroup, GlFormInput } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils'; import { shallowMount } from '@vue/test-utils';
import Vue from 'vue'; import Vue, { nextTick } from 'vue';
import Vuex from 'vuex'; import Vuex from 'vuex';
import ApproverTypeSelect from 'ee/approvals/components/approver_type_select.vue'; import ApproverTypeSelect from 'ee/approvals/components/approver_type_select.vue';
import ApproversList from 'ee/approvals/components/approvers_list.vue'; import ApproversList from 'ee/approvals/components/approvers_list.vue';
...@@ -14,6 +15,8 @@ import { ...@@ -14,6 +15,8 @@ import {
} from 'ee/approvals/constants'; } from 'ee/approvals/constants';
import { createStoreOptions } from 'ee/approvals/stores'; import { createStoreOptions } from 'ee/approvals/stores';
import projectSettingsModule from 'ee/approvals/stores/modules/project_settings'; import projectSettingsModule from 'ee/approvals/stores/modules/project_settings';
import { stubComponent } from 'helpers/stub_component';
import { extendedWrapper } from 'helpers/vue_test_utils_helper';
import waitForPromises from 'helpers/wait_for_promises'; import waitForPromises from 'helpers/wait_for_promises';
import { createExternalRule } from '../mocks'; import { createExternalRule } from '../mocks';
...@@ -25,7 +28,7 @@ const TEST_RULE = { ...@@ -25,7 +28,7 @@ const TEST_RULE = {
users: [{ id: 1 }, { id: 2 }, { id: 3 }], users: [{ id: 1 }, { id: 2 }, { id: 3 }],
groups: [{ id: 1 }, { id: 2 }], groups: [{ id: 1 }, { id: 2 }],
}; };
const TEST_PROTECTED_BRANCHES = [{ id: 2 }]; const TEST_PROTECTED_BRANCHES = [{ id: 2 }, { id: 3 }, { id: 4 }];
const TEST_RULE_WITH_PROTECTED_BRANCHES = { const TEST_RULE_WITH_PROTECTED_BRANCHES = {
...TEST_RULE, ...TEST_RULE,
protectedBranches: TEST_PROTECTED_BRANCHES, protectedBranches: TEST_PROTECTED_BRANCHES,
...@@ -67,36 +70,48 @@ describe('EE Approvals RuleForm', () => { ...@@ -67,36 +70,48 @@ describe('EE Approvals RuleForm', () => {
let store; let store;
let actions; let actions;
const createComponent = (props = {}, options = {}) => { const createComponent = (props = {}, features = {}) => {
wrapper = shallowMount(RuleForm, { wrapper = extendedWrapper(
propsData: props, shallowMount(RuleForm, {
store: new Vuex.Store(store), propsData: props,
provide: { store: new Vuex.Store(store),
glFeatures: {
ffComplianceApprovalGates: true, provide: {
scopedApprovalRules: true, glFeatures: {
...options.provide?.glFeatures, ffComplianceApprovalGates: true,
scopedApprovalRules: true,
...features,
},
}, },
}, stubs: {
}); GlFormGroup: stubComponent(GlFormGroup, {
props: ['state', 'invalidFeedback'],
}),
GlFormInput: stubComponent(GlFormInput, {
props: ['state', 'disabled', 'value'],
template: `<input />`,
}),
},
}),
);
}; };
const findValidation = (node, hasProps = false) => ({
feedback: node.element.nextElementSibling.textContent,
isValid: hasProps ? !node.props('isInvalid') : !node.classes('is-invalid'),
});
const findNameInput = () => wrapper.find('input[name=name]'); const findForm = () => wrapper.find('form');
const findNameValidation = () => findValidation(findNameInput(), false); const findNameInput = () => wrapper.findByTestId('name');
const findApprovalsRequiredInput = () => wrapper.find('input[name=approvals_required]'); const findNameValidation = () => wrapper.findByTestId('name-group');
const findApprovalsRequiredValidation = () => findValidation(findApprovalsRequiredInput(), false); const findApprovalsRequiredInput = () => wrapper.findByTestId('approvals-required');
const findApproversSelect = () => wrapper.find(ApproversSelect); const findApprovalsRequiredValidation = () => wrapper.findByTestId('approvals-required-group');
const findApproversValidation = () => findValidation(findApproversSelect(), true); const findApproversSelect = () => wrapper.findComponent(ApproversSelect);
const findApproversList = () => wrapper.find(ApproversList); const findApproversValidation = () => wrapper.findByTestId('approvers-group');
const findBranchesSelect = () => wrapper.find(BranchesSelect); const findApproversList = () => wrapper.findComponent(ApproversList);
const findBranchesSelect = () => wrapper.findComponent(BranchesSelect);
const findApproverTypeSelect = () => wrapper.findComponent(ApproverTypeSelect); const findApproverTypeSelect = () => wrapper.findComponent(ApproverTypeSelect);
const findExternalUrlInput = () => wrapper.find('input[name=approval_gate_url'); const findExternalUrlInput = () => wrapper.findByTestId('approval-gate-url');
const findExternalUrlValidation = () => findValidation(findExternalUrlInput(), false); const findExternalUrlValidation = () => wrapper.findByTestId('approval-gate-url-group');
const findBranchesValidation = () => findValidation(findBranchesSelect(), true); const findBranchesValidation = () => wrapper.findByTestId('branches-group');
const inputsAreValid = (inputs) => inputs.every((x) => x.props('state'));
const findValidations = () => [ const findValidations = () => [
findNameValidation(), findNameValidation(),
findApprovalsRequiredValidation(), findApprovalsRequiredValidation(),
...@@ -147,48 +162,51 @@ describe('EE Approvals RuleForm', () => { ...@@ -147,48 +162,51 @@ describe('EE Approvals RuleForm', () => {
}); });
it('on load, it populates initial protected branch ids', () => { it('on load, it populates initial protected branch ids', () => {
expect(wrapper.vm.branches).toEqual(TEST_PROTECTED_BRANCHES.map((x) => x.id)); expect(findBranchesSelect().props('initRule').protectedBranches).toEqual(
TEST_PROTECTED_BRANCHES,
);
}); });
}); });
describe('without initRule', () => { describe('without initRule', () => {
beforeEach(() => { beforeEach(() => {
store.state.settings.protectedBranches = TEST_PROTECTED_BRANCHES; store.state.settings.protectedBranches = TEST_PROTECTED_BRANCHES;
createComponent({
isMrEdit: false,
});
}); });
it('at first, shows no validation', () => { it('at first, shows no validation', () => {
const inputs = findValidationsWithBranch(); createComponent({
const invalidInputs = inputs.filter((x) => !x.isValid); isMrEdit: false,
const feedbacks = inputs.map((x) => x.feedback); });
expect(invalidInputs.length).toBe(0); expect(inputsAreValid(findValidationsWithBranch())).toBe(true);
expect(feedbacks.every((str) => !str.length)).toBe(true);
}); });
it('on submit, shows branches validation', (done) => { it('on submit, shows branches validation', async () => {
wrapper.vm.branches = ['3']; createComponent({
wrapper.vm.submit(); isMrEdit: false,
});
await findBranchesSelect().vm.$emit('input', '3');
await findForm().trigger('submit');
await nextTick();
Vue.nextTick() const branchesGroup = findBranchesValidation();
.then(() => { expect(branchesGroup.props('state')).toBe(false);
expect(findBranchesValidation()).toEqual({ expect(branchesGroup.props('invalidFeedback')).toBe(
isValid: false, 'Please select a valid target branch',
feedback: 'Please select a valid target branch', );
});
})
.then(done)
.catch(done.fail);
}); });
it('on submit with data, posts rule', () => { it('on submit with data, posts rule', async () => {
createComponent({
isMrEdit: false,
});
const users = [1, 2]; const users = [1, 2];
const groups = [2, 3]; const groups = [2, 3];
const userRecords = users.map((id) => ({ id, type: TYPE_USER })); const userRecords = users.map((id) => ({ id, type: TYPE_USER }));
const groupRecords = groups.map((id) => ({ id, type: TYPE_GROUP })); const groupRecords = groups.map((id) => ({ id, type: TYPE_GROUP }));
const branches = TEST_PROTECTED_BRANCHES.map((x) => x.id); const branches = [TEST_PROTECTED_BRANCHES[0].id];
const expected = { const expected = {
id: null, id: null,
name: 'Lorem', name: 'Lorem',
...@@ -201,12 +219,11 @@ describe('EE Approvals RuleForm', () => { ...@@ -201,12 +219,11 @@ describe('EE Approvals RuleForm', () => {
protectedBranchIds: branches, protectedBranchIds: branches,
}; };
findNameInput().setValue(expected.name); await findNameInput().vm.$emit('input', expected.name);
findApprovalsRequiredInput().setValue(expected.approvalsRequired); await findApprovalsRequiredInput().vm.$emit('input', expected.approvalsRequired);
wrapper.vm.approvers = groupRecords.concat(userRecords); await findApproversList().vm.$emit('input', [...groupRecords, ...userRecords]);
wrapper.vm.branches = expected.protectedBranchIds; await findBranchesSelect().vm.$emit('input', branches[0]);
await findForm().trigger('submit');
wrapper.vm.submit();
expect(actions.postRule).toHaveBeenCalledWith(expect.anything(), expected); expect(actions.postRule).toHaveBeenCalledWith(expect.anything(), expected);
}); });
...@@ -227,7 +244,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -227,7 +244,7 @@ describe('EE Approvals RuleForm', () => {
}); });
it('on load, it populates the external URL', () => { it('on load, it populates the external URL', () => {
expect(findExternalUrlInput().element.value).toBe( expect(findExternalUrlInput().props('value')).toBe(
TEST_EXTERNAL_APPROVAL_RULE.externalUrl, TEST_EXTERNAL_APPROVAL_RULE.externalUrl,
); );
}); });
...@@ -258,35 +275,28 @@ describe('EE Approvals RuleForm', () => { ...@@ -258,35 +275,28 @@ describe('EE Approvals RuleForm', () => {
}); });
it('at first, shows no validation', () => { it('at first, shows no validation', () => {
const inputs = findValidationForExternal(); expect(inputsAreValid(findValidationForExternal())).toBe(true);
const invalidInputs = inputs.filter((x) => !x.isValid);
const feedbacks = inputs.map((x) => x.feedback);
expect(invalidInputs.length).toBe(0);
expect(feedbacks.every((str) => !str.length)).toBe(true);
}); });
it('on submit, does not dispatch action', () => { it('on submit, does not dispatch action', async () => {
wrapper.vm.submit(); await findForm().trigger('submit');
expect(actions.postExternalApprovalRule).not.toHaveBeenCalled(); expect(actions.postExternalApprovalRule).not.toHaveBeenCalled();
}); });
it('on submit, shows name validation', async () => { it('on submit, shows external URL validation', async () => {
findExternalUrlInput().setValue(''); findNameInput().setValue('');
wrapper.vm.submit();
await Vue.nextTick(); await findForm().trigger('submit');
await nextTick();
expect(findExternalUrlValidation()).toEqual({ const externalUrlGroup = findExternalUrlValidation();
isValid: false, expect(externalUrlGroup.props('state')).toBe(false);
feedback: 'Please provide a valid URL', expect(externalUrlGroup.props('invalidFeedback')).toBe('Please provide a valid URL');
});
}); });
describe('with valid data', () => { describe('with valid data', () => {
const branches = TEST_PROTECTED_BRANCHES.map((x) => x.id); const branches = [TEST_PROTECTED_BRANCHES[0].id];
const expected = { const expected = {
id: null, id: null,
name: 'Lorem', name: 'Lorem',
...@@ -294,14 +304,14 @@ describe('EE Approvals RuleForm', () => { ...@@ -294,14 +304,14 @@ describe('EE Approvals RuleForm', () => {
protectedBranchIds: branches, protectedBranchIds: branches,
}; };
beforeEach(() => { beforeEach(async () => {
findNameInput().setValue(expected.name); await findNameInput().vm.$emit('input', expected.name);
findExternalUrlInput().setValue(expected.externalUrl); await findExternalUrlInput().vm.$emit('input', expected.externalUrl);
wrapper.vm.branches = expected.protectedBranchIds; await findBranchesSelect().vm.$emit('input', branches[0]);
}); });
it('on submit, posts external approval rule', () => { it('on submit, posts external approval rule', async () => {
wrapper.vm.submit(); await findForm().trigger('submit');
expect(actions.postExternalApprovalRule).toHaveBeenCalledWith( expect(actions.postExternalApprovalRule).toHaveBeenCalledWith(
expect.anything(), expect.anything(),
...@@ -311,16 +321,16 @@ describe('EE Approvals RuleForm', () => { ...@@ -311,16 +321,16 @@ describe('EE Approvals RuleForm', () => {
it('when submitted with a duplicate external URL, shows the "url already taken" validation', async () => { it('when submitted with a duplicate external URL, shows the "url already taken" validation', async () => {
store.state.settings.prefix = 'project-settings'; store.state.settings.prefix = 'project-settings';
jest.spyOn(wrapper.vm, 'postExternalApprovalRule').mockRejectedValueOnce(urlTakenError); actions.postExternalApprovalRule.mockRejectedValueOnce(urlTakenError);
wrapper.vm.submit();
await findForm().trigger('submit');
await waitForPromises(); await waitForPromises();
expect(findExternalUrlValidation()).toEqual({ const externalUrlGroup = findExternalUrlValidation();
isValid: false, expect(externalUrlGroup.props('state')).toBe(false);
feedback: 'External url has already been taken', expect(externalUrlGroup.props('invalidFeedback')).toBe(
}); 'External url has already been taken',
);
}); });
}); });
}); });
...@@ -328,69 +338,50 @@ describe('EE Approvals RuleForm', () => { ...@@ -328,69 +338,50 @@ describe('EE Approvals RuleForm', () => {
describe('without initRule', () => { describe('without initRule', () => {
beforeEach(() => { beforeEach(() => {
createComponent(); createComponent({ isMrEdit: false });
}); });
it('at first, shows no validation', () => { it('at first, shows no validation', () => {
const inputs = findValidations(); expect(inputsAreValid(findValidationsWithBranch())).toBe(true);
const invalidInputs = inputs.filter((x) => !x.isValid);
const feedbacks = inputs.map((x) => x.feedback);
expect(invalidInputs.length).toBe(0);
expect(feedbacks.every((str) => !str.length)).toBe(true);
}); });
it('on submit, does not dispatch action', () => { it('on submit, does not dispatch action', async () => {
wrapper.vm.submit(); await findForm().trigger('submit');
expect(actions.postRule).not.toHaveBeenCalled(); expect(actions.postRule).not.toHaveBeenCalled();
}); });
it('on submit, shows name validation', (done) => { it('on submit, shows name validation', async () => {
findNameInput().setValue(''); findNameInput().setValue('');
wrapper.vm.submit(); await findForm().trigger('submit');
await nextTick();
Vue.nextTick() const nameGroup = findNameValidation();
.then(() => { expect(nameGroup.props('state')).toBe(false);
expect(findNameValidation()).toEqual({ expect(nameGroup.props('invalidFeedback')).toBe('Please provide a name');
isValid: false,
feedback: 'Please provide a name',
});
})
.then(done)
.catch(done.fail);
}); });
it('on submit, shows approvalsRequired validation', (done) => { it('on submit, shows approvalsRequired validation', async () => {
findApprovalsRequiredInput().setValue(-1); await findApprovalsRequiredInput().vm.$emit('input', -1);
await findForm().trigger('submit');
wrapper.vm.submit(); await nextTick();
Vue.nextTick() const approvalsRequiredGroup = findApprovalsRequiredValidation();
.then(() => { expect(approvalsRequiredGroup.props('state')).toBe(false);
expect(findApprovalsRequiredValidation()).toEqual({ expect(approvalsRequiredGroup.props('invalidFeedback')).toBe(
isValid: false, 'Please enter a non-negative number',
feedback: 'Please enter a non-negative number', );
});
})
.then(done)
.catch(done.fail);
}); });
it('on submit, shows approvers validation', (done) => { it('on submit, shows approvers validation', async () => {
wrapper.vm.approvers = []; await findApproversList().vm.$emit('input', []);
wrapper.vm.submit(); await findForm().trigger('submit');
await nextTick();
Vue.nextTick() const approversGroup = findApproversValidation();
.then(() => { expect(approversGroup.props('state')).toBe(false);
expect(findApproversValidation()).toEqual({ expect(approversGroup.props('invalidFeedback')).toBe('Please select and add a member');
isValid: false,
feedback: 'Please select and add a member',
});
})
.then(done)
.catch(done.fail);
}); });
describe('with valid data', () => { describe('with valid data', () => {
...@@ -398,7 +389,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -398,7 +389,7 @@ describe('EE Approvals RuleForm', () => {
const groups = [2, 3]; const groups = [2, 3];
const userRecords = users.map((id) => ({ id, type: TYPE_USER })); const userRecords = users.map((id) => ({ id, type: TYPE_USER }));
const groupRecords = groups.map((id) => ({ id, type: TYPE_GROUP })); const groupRecords = groups.map((id) => ({ id, type: TYPE_GROUP }));
const branches = TEST_PROTECTED_BRANCHES.map((x) => x.id); const branches = [TEST_PROTECTED_BRANCHES[0].id];
const expected = { const expected = {
id: null, id: null,
name: 'Lorem', name: 'Lorem',
...@@ -411,46 +402,44 @@ describe('EE Approvals RuleForm', () => { ...@@ -411,46 +402,44 @@ describe('EE Approvals RuleForm', () => {
protectedBranchIds: branches, protectedBranchIds: branches,
}; };
beforeEach(() => { beforeEach(async () => {
findNameInput().setValue(expected.name); await findNameInput().vm.$emit('input', expected.name);
findApprovalsRequiredInput().setValue(expected.approvalsRequired); await findApprovalsRequiredInput().vm.$emit('input', expected.approvalsRequired);
wrapper.vm.approvers = groupRecords.concat(userRecords); await findApproversList().vm.$emit('input', [...groupRecords, ...userRecords]);
wrapper.vm.branches = expected.protectedBranchIds; await findBranchesSelect().vm.$emit('input', branches[0]);
}); });
it('on submit, posts rule', () => { it('on submit, posts rule', async () => {
wrapper.vm.submit(); await findForm().trigger('submit');
expect(actions.postRule).toHaveBeenCalledWith(expect.anything(), expected); expect(actions.postRule).toHaveBeenCalledWith(expect.anything(), expected);
}); });
it('when submitted with a duplicate name, shows the "taken name" validation', async () => { it('when submitted with a duplicate name, shows the "taken name" validation', async () => {
store.state.settings.prefix = 'project-settings'; store.state.settings.prefix = 'project-settings';
jest.spyOn(wrapper.vm, 'postRule').mockRejectedValueOnce(nameTakenError); actions.postRule.mockRejectedValueOnce(nameTakenError);
wrapper.vm.submit();
await wrapper.vm.$nextTick(); await findForm().trigger('submit');
await nextTick();
// We have to wait for two ticks because the promise needs to resolve // We have to wait for two ticks because the promise needs to resolve
// AND the result has to update into the UI // AND the result has to update into the UI
await wrapper.vm.$nextTick(); await nextTick();
expect(findNameValidation()).toEqual({ const nameGroup = findNameValidation();
isValid: false, expect(nameGroup.props('state')).toBe(false);
feedback: 'Rule name is already taken.', expect(nameGroup.props('invalidFeedback')).toBe('Rule name is already taken.');
});
}); });
}); });
it('adds selected approvers on selection', () => { it('adds selected approvers on selection', async () => {
const orig = [{ id: 7, type: TYPE_GROUP }]; const orig = [{ id: 7, type: TYPE_GROUP }];
const selected = [{ id: 2, type: TYPE_USER }]; const selected = [{ id: 2, type: TYPE_USER }];
const expected = [...orig, ...selected]; const expected = [...orig, ...selected];
wrapper.setData({ approvers: orig }); await findApproversSelect().vm.$emit('input', orig);
wrapper.vm.$options.watch.approversToAdd.call(wrapper.vm, selected); await findApproversSelect().vm.$emit('input', selected);
expect(wrapper.vm.approvers).toEqual(expected); expect(findApproversList().props('value')).toEqual(expected);
}); });
}); });
...@@ -458,6 +447,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -458,6 +447,7 @@ describe('EE Approvals RuleForm', () => {
beforeEach(() => { beforeEach(() => {
createComponent({ createComponent({
initRule: TEST_RULE, initRule: TEST_RULE,
isMrEdit: false,
}); });
}); });
...@@ -490,34 +480,22 @@ describe('EE Approvals RuleForm', () => { ...@@ -490,34 +480,22 @@ describe('EE Approvals RuleForm', () => {
protectedBranchIds: [], protectedBranchIds: [],
}; };
beforeEach(() => { it('on submit, puts rule', async () => {
findNameInput().setValue(expected.name); await findForm().trigger('submit');
findApprovalsRequiredInput().setValue(expected.approvalsRequired);
wrapper.vm.approvers = groupRecords.concat(userRecords);
wrapper.vm.branches = expected.protectedBranchIds;
});
it('on submit, puts rule', () => {
wrapper.vm.submit();
expect(actions.putRule).toHaveBeenCalledWith(expect.anything(), expected); expect(actions.putRule).toHaveBeenCalledWith(expect.anything(), expected);
}); });
it('when submitted with a duplicate name, shows the "taken name" validation', async () => { it('when submitted with a duplicate name, shows the "taken name" validation', async () => {
store.state.settings.prefix = 'project-settings'; store.state.settings.prefix = 'project-settings';
jest.spyOn(wrapper.vm, 'putRule').mockRejectedValueOnce(nameTakenError); actions.putRule.mockRejectedValueOnce(nameTakenError);
wrapper.vm.submit(); await findForm().trigger('submit');
await waitForPromises();
await wrapper.vm.$nextTick(); const nameGroup = findNameValidation();
// We have to wait for two ticks because the promise needs to resolve expect(nameGroup.props('state')).toBe(false);
// AND the result has to update into the UI expect(nameGroup.props('invalidFeedback')).toBe('Rule name is already taken.');
await wrapper.vm.$nextTick();
expect(findNameValidation()).toEqual({
isValid: false,
feedback: 'Rule name is already taken.',
});
}); });
}); });
}); });
...@@ -528,15 +506,14 @@ describe('EE Approvals RuleForm', () => { ...@@ -528,15 +506,14 @@ describe('EE Approvals RuleForm', () => {
initRule: TEST_FALLBACK_RULE, initRule: TEST_FALLBACK_RULE,
}); });
wrapper.vm.name = ''; findNameInput().vm.$emit('input', '');
wrapper.vm.approvers = []; findApprovalsRequiredInput().vm.$emit('input', TEST_APPROVALS_REQUIRED);
wrapper.vm.approvalsRequired = TEST_APPROVALS_REQUIRED; findApproversList().vm.$emit('input', []);
}); });
describe('with empty name and empty approvers', () => { describe('with empty name and empty approvers', () => {
beforeEach((done) => { beforeEach(() => {
wrapper.vm.submit(); findForm().trigger('submit');
Vue.nextTick(done);
}); });
it('does not post rule', () => { it('does not post rule', () => {
...@@ -550,16 +527,14 @@ describe('EE Approvals RuleForm', () => { ...@@ -550,16 +527,14 @@ describe('EE Approvals RuleForm', () => {
}); });
it('does not show any validation errors', () => { it('does not show any validation errors', () => {
expect(findValidations().every((x) => x.isValid)).toBe(true); expect(inputsAreValid(findValidations())).toBe(true);
}); });
}); });
describe('with name and empty approvers', () => { describe('with name and empty approvers', () => {
beforeEach((done) => { beforeEach(() => {
wrapper.vm.name = 'Lorem'; findNameInput().vm.$emit('input', 'Lorem');
wrapper.vm.submit(); findForm().trigger('submit');
Vue.nextTick(done);
}); });
it('does not put fallback rule', () => { it('does not put fallback rule', () => {
...@@ -567,16 +542,14 @@ describe('EE Approvals RuleForm', () => { ...@@ -567,16 +542,14 @@ describe('EE Approvals RuleForm', () => {
}); });
it('shows approvers validation error', () => { it('shows approvers validation error', () => {
expect(findApproversValidation().isValid).toBe(false); expect(findApproversValidation().props('state')).toBe(false);
}); });
}); });
describe('with empty name and approvers', () => { describe('with empty name and approvers', () => {
beforeEach((done) => { beforeEach(() => {
wrapper.vm.approvers = TEST_APPROVERS; findApproversList().vm.$emit('input', TEST_APPROVERS);
wrapper.vm.submit(); findForm().trigger('submit');
Vue.nextTick(done);
}); });
it('does not put fallback rule', () => { it('does not put fallback rule', () => {
...@@ -584,17 +557,15 @@ describe('EE Approvals RuleForm', () => { ...@@ -584,17 +557,15 @@ describe('EE Approvals RuleForm', () => {
}); });
it('shows name validation error', () => { it('shows name validation error', () => {
expect(findNameValidation().isValid).toBe(false); expect(findNameValidation().props('state')).toBe(false);
}); });
}); });
describe('with name and approvers', () => { describe('with name and approvers', () => {
beforeEach((done) => { beforeEach(() => {
wrapper.vm.approvers = [{ id: 7, type: TYPE_USER }]; findApproversList().vm.$emit('input', [{ id: 7, type: TYPE_USER }]);
wrapper.vm.name = 'Lorem'; findNameInput().vm.$emit('input', 'Lorem');
wrapper.vm.submit(); findForm().trigger('submit');
Vue.nextTick(done);
}); });
it('does not put fallback rule', () => { it('does not put fallback rule', () => {
...@@ -627,8 +598,8 @@ describe('EE Approvals RuleForm', () => { ...@@ -627,8 +598,8 @@ describe('EE Approvals RuleForm', () => {
]); ]);
}); });
it('on submit, does not remove hidden groups', () => { it('on submit, does not remove hidden groups', async () => {
wrapper.vm.submit(); await findForm().trigger('submit');
expect(actions.putRule).toHaveBeenCalledWith( expect(actions.putRule).toHaveBeenCalledWith(
expect.anything(), expect.anything(),
...@@ -640,11 +611,16 @@ describe('EE Approvals RuleForm', () => { ...@@ -640,11 +611,16 @@ describe('EE Approvals RuleForm', () => {
describe('and hidden groups removed', () => { describe('and hidden groups removed', () => {
beforeEach(() => { beforeEach(() => {
wrapper.vm.approvers = wrapper.vm.approvers.filter((x) => x.type !== TYPE_HIDDEN_GROUPS); findApproversList().vm.$emit(
'input',
findApproversList()
.props('value')
.filter((x) => x.type !== TYPE_HIDDEN_GROUPS),
);
}); });
it('on submit, removes hidden groups', () => { it('on submit, removes hidden groups', async () => {
wrapper.vm.submit(); await findForm().trigger('submit');
expect(actions.putRule).toHaveBeenCalledWith( expect(actions.putRule).toHaveBeenCalledWith(
expect.anything(), expect.anything(),
...@@ -679,9 +655,9 @@ describe('EE Approvals RuleForm', () => { ...@@ -679,9 +655,9 @@ describe('EE Approvals RuleForm', () => {
describe('with approval suggestions', () => { describe('with approval suggestions', () => {
describe.each` describe.each`
defaultRuleName | expectedDisabledAttribute | approverTypeSelect defaultRuleName | expectedDisabledAttribute | approverTypeSelect
${'Vulnerability-Check'} | ${'disabled'} | ${false} ${'Vulnerability-Check'} | ${true} | ${false}
${'License-Check'} | ${'disabled'} | ${false} ${'License-Check'} | ${true} | ${false}
${'Foo Bar Baz'} | ${undefined} | ${true} ${'Foo Bar Baz'} | ${false} | ${true}
`( `(
'with defaultRuleName set to $defaultRuleName', 'with defaultRuleName set to $defaultRuleName',
({ defaultRuleName, expectedDisabledAttribute, approverTypeSelect }) => { ({ defaultRuleName, expectedDisabledAttribute, approverTypeSelect }) => {
...@@ -696,7 +672,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -696,7 +672,7 @@ describe('EE Approvals RuleForm', () => {
it(`it ${ it(`it ${
expectedDisabledAttribute ? 'disables' : 'does not disable' expectedDisabledAttribute ? 'disables' : 'does not disable'
} the name text field`, () => { } the name text field`, () => {
expect(findNameInput().attributes('disabled')).toBe(expectedDisabledAttribute); expect(findNameInput().props('disabled')).toBe(expectedDisabledAttribute);
}); });
it(`${ it(`${
...@@ -716,7 +692,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -716,7 +692,7 @@ describe('EE Approvals RuleForm', () => {
}); });
it('does not disable the name text field', () => { it('does not disable the name text field', () => {
expect(findNameInput().attributes('disabled')).toBe(undefined); expect(findNameInput().props('disabled')).toBe(false);
}); });
}); });
...@@ -728,7 +704,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -728,7 +704,7 @@ describe('EE Approvals RuleForm', () => {
}); });
it('does not disable the name text field', () => { it('does not disable the name text field', () => {
expect(findNameInput().attributes('disabled')).toBe(undefined); expect(findNameInput().props('disabled')).toBe(false);
}); });
}); });
...@@ -740,7 +716,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -740,7 +716,7 @@ describe('EE Approvals RuleForm', () => {
}); });
it('disables the name text field', () => { it('disables the name text field', () => {
expect(findNameInput().attributes('disabled')).toBe('disabled'); expect(findNameInput().props('disabled')).toBe(true);
}); });
}); });
...@@ -752,7 +728,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -752,7 +728,7 @@ describe('EE Approvals RuleForm', () => {
}); });
it('disables the name text field', () => { it('disables the name text field', () => {
expect(findNameInput().attributes('disabled')).toBe('disabled'); expect(findNameInput().props('disabled')).toBe(true);
}); });
}); });
}); });
...@@ -781,15 +757,13 @@ describe('EE Approvals RuleForm', () => { ...@@ -781,15 +757,13 @@ describe('EE Approvals RuleForm', () => {
beforeEach(() => { beforeEach(() => {
store.state.settings.lockedApprovalsRuleName = lockedRuleName; store.state.settings.lockedApprovalsRuleName = lockedRuleName;
createComponent(); createComponent();
wrapper.vm.approvalsRequired = TEST_APPROVALS_REQUIRED; findApprovalsRequiredInput().vm.$emit('input', TEST_APPROVALS_REQUIRED);
}); });
describe('with approvers selected', () => { describe('with approvers selected', () => {
beforeEach(() => { beforeEach(() => {
wrapper.vm.approvers = TEST_APPROVERS; findApproversList().vm.$emit('input', TEST_APPROVERS);
wrapper.vm.submit(); findForm().trigger('submit');
return Vue.nextTick();
}); });
it('posts new rule', () => { it('posts new rule', () => {
...@@ -806,9 +780,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -806,9 +780,7 @@ describe('EE Approvals RuleForm', () => {
describe('without approvers', () => { describe('without approvers', () => {
beforeEach(() => { beforeEach(() => {
wrapper.vm.submit(); findForm().trigger('submit');
return Vue.nextTick();
}); });
it('puts fallback rule', () => { it('puts fallback rule', () => {
...@@ -826,20 +798,17 @@ describe('EE Approvals RuleForm', () => { ...@@ -826,20 +798,17 @@ describe('EE Approvals RuleForm', () => {
`('with init rule', ({ lockedRuleName, inputName, expectedNameSubmitted }) => { `('with init rule', ({ lockedRuleName, inputName, expectedNameSubmitted }) => {
beforeEach(() => { beforeEach(() => {
store.state.settings.lockedApprovalsRuleName = lockedRuleName; store.state.settings.lockedApprovalsRuleName = lockedRuleName;
createComponent({
initRule: TEST_RULE,
});
wrapper.vm.approvalsRequired = TEST_APPROVALS_REQUIRED;
}); });
describe('with empty name and empty approvers', () => { describe('with empty name and empty approvers', () => {
beforeEach(() => { beforeEach(() => {
wrapper.vm.name = ''; createComponent({
wrapper.vm.approvers = []; initRule: { ...TEST_RULE, name: '' },
});
wrapper.vm.submit(); findApprovalsRequiredInput().vm.$emit('input', TEST_APPROVALS_REQUIRED);
findApproversList().vm.$emit('input', []);
return Vue.nextTick(); findForm().trigger('submit');
}); });
it('deletes rule', () => { it('deletes rule', () => {
...@@ -854,12 +823,14 @@ describe('EE Approvals RuleForm', () => { ...@@ -854,12 +823,14 @@ describe('EE Approvals RuleForm', () => {
}); });
describe('with name and approvers', () => { describe('with name and approvers', () => {
beforeEach((done) => { beforeEach(() => {
wrapper.vm.name = inputName; createComponent({
wrapper.vm.approvers = TEST_APPROVERS; initRule: { ...TEST_RULE, name: inputName },
wrapper.vm.submit(); });
findApprovalsRequiredInput().vm.$emit('input', TEST_APPROVALS_REQUIRED);
findApproversList().vm.$emit('input', TEST_APPROVERS);
Vue.nextTick(done); findForm().trigger('submit');
}); });
it('puts rule', () => { it('puts rule', () => {
...@@ -879,18 +850,9 @@ describe('EE Approvals RuleForm', () => { ...@@ -879,18 +850,9 @@ describe('EE Approvals RuleForm', () => {
describe('when the approval gates feature is disabled', () => { describe('when the approval gates feature is disabled', () => {
it('does not render the approver type select input', async () => { it('does not render the approver type select input', async () => {
createComponent( createComponent({ isMrEdit: false }, { ffComplianceApprovalGates: false });
{ isMrEdit: false },
{
provide: {
glFeatures: {
ffComplianceApprovalGates: false,
},
},
},
);
await Vue.nextTick(); await nextTick();
expect(findApproverTypeSelect().exists()).toBe(false); expect(findApproverTypeSelect().exists()).toBe(false);
}); });
......
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