Commit 3fcabe79 authored by Nathan Friend's avatar Nathan Friend Committed by Jose Ivan Vargas

Update tag name field to be required

This commit updates the New Release to make the "Tag name" field
required (using frontend validation).
parent b1e4c446
<script> <script>
import { mapState, mapActions } from 'vuex'; import { mapState, mapActions, mapGetters } from 'vuex';
import { GlFormGroup, GlFormInput } from '@gitlab/ui'; import { GlFormGroup, GlFormInput } from '@gitlab/ui';
import { uniqueId } from 'lodash'; import { uniqueId } from 'lodash';
import { __ } from '~/locale'; import { __ } from '~/locale';
...@@ -9,8 +9,17 @@ import FormFieldContainer from './form_field_container.vue'; ...@@ -9,8 +9,17 @@ import FormFieldContainer from './form_field_container.vue';
export default { export default {
name: 'TagFieldNew', name: 'TagFieldNew',
components: { GlFormGroup, GlFormInput, RefSelector, FormFieldContainer }, components: { GlFormGroup, GlFormInput, RefSelector, FormFieldContainer },
data() {
return {
// Keeps track of whether or not the user has interacted with
// the input field. This is used to avoid showing validation
// errors immediately when the page loads.
isInputDirty: false,
};
},
computed: { computed: {
...mapState('detail', ['projectId', 'release', 'createFrom']), ...mapState('detail', ['projectId', 'release', 'createFrom']),
...mapGetters('detail', ['validationErrors']),
tagName: { tagName: {
get() { get() {
return this.release.tagName; return this.release.tagName;
...@@ -27,6 +36,9 @@ export default { ...@@ -27,6 +36,9 @@ export default {
this.updateCreateFrom(createFrom); this.updateCreateFrom(createFrom);
}, },
}, },
showTagNameValidationError() {
return this.isInputDirty && this.validationErrors.isTagNameEmpty;
},
tagNameInputId() { tagNameInputId() {
return uniqueId('tag-name-input-'); return uniqueId('tag-name-input-');
}, },
...@@ -36,6 +48,9 @@ export default { ...@@ -36,6 +48,9 @@ export default {
}, },
methods: { methods: {
...mapActions('detail', ['updateReleaseTagName', 'updateCreateFrom']), ...mapActions('detail', ['updateReleaseTagName', 'updateCreateFrom']),
markInputAsDirty() {
this.isInputDirty = true;
},
}, },
translations: { translations: {
noRefSelected: __('No source selected'), noRefSelected: __('No source selected'),
...@@ -46,9 +61,22 @@ export default { ...@@ -46,9 +61,22 @@ export default {
</script> </script>
<template> <template>
<div> <div>
<gl-form-group :label="__('Tag name')" :label-for="tagNameInputId" data-testid="tag-name-field"> <gl-form-group
:label="__('Tag name')"
:label-for="tagNameInputId"
data-testid="tag-name-field"
:state="!showTagNameValidationError"
:invalid-feedback="__('Tag name is required')"
>
<form-field-container> <form-field-container>
<gl-form-input :id="tagNameInputId" v-model="tagName" type="text" class="form-control" /> <gl-form-input
:id="tagNameInputId"
v-model="tagName"
:state="!showTagNameValidationError"
type="text"
class="form-control"
@blur.once="markInputAsDirty"
/>
</form-field-container> </form-field-container>
</gl-form-group> </gl-form-group>
<gl-form-group <gl-form-group
......
...@@ -47,6 +47,10 @@ export const validationErrors = state => { ...@@ -47,6 +47,10 @@ export const validationErrors = state => {
return errors; return errors;
} }
if (!state.release.tagName?.trim?.().length) {
errors.isTagNameEmpty = true;
}
// Each key of this object is a URL, and the value is an // Each key of this object is a URL, and the value is an
// array of Release link objects that share this URL. // array of Release link objects that share this URL.
// This is used for detecting duplicate URLs. // This is used for detecting duplicate URLs.
...@@ -96,5 +100,6 @@ export const validationErrors = state => { ...@@ -96,5 +100,6 @@ export const validationErrors = state => {
/** Returns whether or not the release object is valid */ /** Returns whether or not the release object is valid */
export const isValid = (_state, getters) => { export const isValid = (_state, getters) => {
return Object.values(getters.validationErrors.assets.links).every(isEmpty); const errors = getters.validationErrors;
return Object.values(errors.assets.links).every(isEmpty) && !errors.isTagNameEmpty;
}; };
...@@ -23677,6 +23677,9 @@ msgstr "" ...@@ -23677,6 +23677,9 @@ msgstr ""
msgid "Tag name" msgid "Tag name"
msgstr "" msgstr ""
msgid "Tag name is required"
msgstr ""
msgid "Tag this commit." msgid "Tag this commit."
msgstr "" msgstr ""
......
import { shallowMount } from '@vue/test-utils'; import { mount, shallowMount } from '@vue/test-utils';
import { GlFormInput } from '@gitlab/ui'; import { GlFormInput } from '@gitlab/ui';
import TagFieldNew from '~/releases/components/tag_field_new.vue'; import TagFieldNew from '~/releases/components/tag_field_new.vue';
import createStore from '~/releases/stores'; import createStore from '~/releases/stores';
...@@ -16,6 +16,9 @@ describe('releases/components/tag_field_new', () => { ...@@ -16,6 +16,9 @@ describe('releases/components/tag_field_new', () => {
const createComponent = (mountFn = shallowMount) => { const createComponent = (mountFn = shallowMount) => {
wrapper = mountFn(TagFieldNew, { wrapper = mountFn(TagFieldNew, {
store, store,
stubs: {
RefSelector: true,
},
}); });
}; };
...@@ -32,6 +35,9 @@ describe('releases/components/tag_field_new', () => { ...@@ -32,6 +35,9 @@ describe('releases/components/tag_field_new', () => {
store.state.detail.release = { store.state.detail.release = {
tagName: TEST_TAG_NAME, tagName: TEST_TAG_NAME,
assets: {
links: [],
},
}; };
}); });
...@@ -42,11 +48,13 @@ describe('releases/components/tag_field_new', () => { ...@@ -42,11 +48,13 @@ describe('releases/components/tag_field_new', () => {
const findTagNameFormGroup = () => wrapper.find('[data-testid="tag-name-field"]'); const findTagNameFormGroup = () => wrapper.find('[data-testid="tag-name-field"]');
const findTagNameGlInput = () => findTagNameFormGroup().find(GlFormInput); const findTagNameGlInput = () => findTagNameFormGroup().find(GlFormInput);
const findTagNameInput = () => findTagNameFormGroup().find('input');
const findCreateFromFormGroup = () => wrapper.find('[data-testid="create-from-field"]'); const findCreateFromFormGroup = () => wrapper.find('[data-testid="create-from-field"]');
const findCreateFromDropdown = () => findCreateFromFormGroup().find(RefSelector); const findCreateFromDropdown = () => findCreateFromFormGroup().find(RefSelector);
describe('"Tag name" field', () => { describe('"Tag name" field', () => {
describe('rendering and behavior', () => {
beforeEach(createComponent); beforeEach(createComponent);
it('renders a label', () => { it('renders a label', () => {
...@@ -65,6 +73,56 @@ describe('releases/components/tag_field_new', () => { ...@@ -65,6 +73,56 @@ describe('releases/components/tag_field_new', () => {
}); });
}); });
describe('validation', () => {
beforeEach(() => {
createComponent(mount);
});
/**
* Utility function to test the visibility of the validation message
* @param {'shown' | 'hidden'} state The expected state of the validation message.
* Should be passed either 'shown' or 'hidden'
*/
const expectValidationMessageToBe = state => {
return wrapper.vm.$nextTick().then(() => {
expect(findTagNameFormGroup().element).toHaveClass(
state === 'shown' ? 'is-invalid' : 'is-valid',
);
expect(findTagNameFormGroup().element).not.toHaveClass(
state === 'shown' ? 'is-valid' : 'is-invalid',
);
});
};
describe('when the user has not yet interacted with the component', () => {
it('does not display a validation error', () => {
findTagNameInput().setValue('');
return expectValidationMessageToBe('hidden');
});
});
describe('when the user has interacted with the component and the value is not empty', () => {
it('does not display validation error', () => {
findTagNameInput().trigger('blur');
return expectValidationMessageToBe('hidden');
});
});
describe('when the user has interacted with the component and the value is empty', () => {
it('displays a validation error', () => {
const tagNameInput = findTagNameInput();
tagNameInput.setValue('');
tagNameInput.trigger('blur');
return expectValidationMessageToBe('shown');
});
});
});
});
describe('"Create from" field', () => { describe('"Create from" field', () => {
beforeEach(createComponent); beforeEach(createComponent);
......
...@@ -76,6 +76,7 @@ describe('Release detail getters', () => { ...@@ -76,6 +76,7 @@ describe('Release detail getters', () => {
it('returns no validation errors', () => { it('returns no validation errors', () => {
const state = { const state = {
release: { release: {
tagName: 'test-tag-name',
assets: { assets: {
links: [ links: [
{ id: 1, url: 'https://example.com/valid', name: 'Link 1' }, { id: 1, url: 'https://example.com/valid', name: 'Link 1' },
...@@ -110,6 +111,9 @@ describe('Release detail getters', () => { ...@@ -110,6 +111,9 @@ describe('Release detail getters', () => {
beforeEach(() => { beforeEach(() => {
const state = { const state = {
release: { release: {
// empty tag name
tagName: '',
assets: { assets: {
links: [ links: [
// Duplicate URLs // Duplicate URLs
...@@ -138,7 +142,15 @@ describe('Release detail getters', () => { ...@@ -138,7 +142,15 @@ describe('Release detail getters', () => {
actualErrors = getters.validationErrors(state); actualErrors = getters.validationErrors(state);
}); });
it('returns a validation errors if links share a URL', () => { it('returns a validation error if the tag name is empty', () => {
const expectedErrors = {
isTagNameEmpty: true,
};
expect(actualErrors).toMatchObject(expectedErrors);
});
it('returns a validation error if links share a URL', () => {
const expectedErrors = { const expectedErrors = {
assets: { assets: {
links: { links: {
...@@ -196,7 +208,8 @@ describe('Release detail getters', () => { ...@@ -196,7 +208,8 @@ describe('Release detail getters', () => {
// the value of state is not actually used by this getter // the value of state is not actually used by this getter
const state = {}; const state = {};
it('returns true when the form is valid', () => { describe('when the form is valid', () => {
it('returns true', () => {
const mockGetters = { const mockGetters = {
validationErrors: { validationErrors: {
assets: { assets: {
...@@ -209,8 +222,10 @@ describe('Release detail getters', () => { ...@@ -209,8 +222,10 @@ describe('Release detail getters', () => {
expect(getters.isValid(state, mockGetters)).toBe(true); expect(getters.isValid(state, mockGetters)).toBe(true);
}); });
});
it('returns false when the form is invalid', () => { describe('when an asset link contains a validation error', () => {
it('returns false', () => {
const mockGetters = { const mockGetters = {
validationErrors: { validationErrors: {
assets: { assets: {
...@@ -224,4 +239,22 @@ describe('Release detail getters', () => { ...@@ -224,4 +239,22 @@ describe('Release detail getters', () => {
expect(getters.isValid(state, mockGetters)).toBe(false); expect(getters.isValid(state, mockGetters)).toBe(false);
}); });
}); });
describe('when the tag name is empty', () => {
it('returns false', () => {
const mockGetters = {
validationErrors: {
isTagNameEmpty: true,
assets: {
links: {
1: {},
},
},
},
};
expect(getters.isValid(state, mockGetters)).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