Commit 51d10834 authored by Florie Guibert's avatar Florie Guibert Committed by Natalia Tepluhina

Convert flash epic error to form validation error

Error on adding issue or epic to epic:
- Remove error banner
- Add validation error message on field
- Field is outlined in red
- Error gets cleared when clearing input tokens
parent b98072f4
---
title: Convert flash epic error to form validation error
merge_request: 20130
author:
type: changed
......@@ -2,7 +2,12 @@
/* eslint-disable @gitlab/vue-i18n/no-bare-strings */
import { GlLoadingIcon } from '@gitlab/ui';
import RelatedIssuableInput from './related_issuable_input.vue';
import { issuableTypesMap } from '../constants';
import {
issuableTypesMap,
itemAddFailureTypesMap,
addRelatedIssueErrorMap,
addRelatedItemErrorMap,
} from '../constants';
export default {
name: 'AddIssuableForm',
......@@ -39,6 +44,16 @@ export default {
required: false,
default: issuableTypesMap.ISSUE,
},
hasError: {
type: Boolean,
required: false,
default: false,
},
itemAddFailureType: {
type: String,
required: false,
default: itemAddFailureTypesMap.NOT_FOUND,
},
},
computed: {
isSubmitButtonDisabled() {
......@@ -46,6 +61,13 @@ export default {
(this.inputValue.length === 0 && this.pendingReferences.length === 0) || this.isSubmitting
);
},
addRelatedErrorMessage() {
if (this.itemAddFailureType === itemAddFailureTypesMap.NOT_FOUND) {
return addRelatedIssueErrorMap[this.issuableType];
}
// Only other failure is MAX_NUMBER_OF_CHILD_EPICS at the moment
return addRelatedItemErrorMap[this.itemAddFailureType];
},
},
methods: {
onPendingIssuableRemoveRequest(params) {
......@@ -83,12 +105,16 @@ export default {
@addIssuableFormBlur="onAddIssuableFormBlur"
@addIssuableFormInput="onAddIssuableFormInput"
/>
<p v-if="hasError" class="gl-field-error">
{{ addRelatedErrorMessage }}
</p>
<div class="add-issuable-form-actions clearfix">
<button
ref="addButton"
:disabled="isSubmitButtonDisabled"
type="submit"
class="js-add-issuable-form-add-button btn btn-success float-left qa-add-issue-button"
:class="{ disabled: isSubmitButtonDisabled }"
>
Add
<gl-loading-icon v-if="isSubmitting" ref="loadingIcon" :inline="true" />
......
......@@ -166,7 +166,7 @@ export default {
<div
ref="issuableFormWrapper"
:class="{ focus: isInputFocused }"
class="add-issuable-form-input-wrapper form-control"
class="add-issuable-form-input-wrapper form-control gl-field-error-outline"
role="button"
@click="onIssuableFormWrapperClick"
>
......
......@@ -35,9 +35,20 @@ export const pathIndeterminateErrorMap = {
[issuableTypesMap.EPIC]: __('We could not determine the path to remove the epic'),
};
export const itemAddFailureTypesMap = {
NOT_FOUND: 'not_found',
MAX_NUMBER_OF_CHILD_EPICS: 'conflict',
};
export const addRelatedIssueErrorMap = {
[issuableTypesMap.ISSUE]: __("We can't find an issue that matches what you are looking for."),
[issuableTypesMap.EPIC]: __("We can't find an epic that matches what you are looking for."),
[issuableTypesMap.ISSUE]: __('Issue cannot be found.'),
[issuableTypesMap.EPIC]: __('Epic cannot be found.'),
};
export const addRelatedItemErrorMap = {
[itemAddFailureTypesMap.MAX_NUMBER_OF_CHILD_EPICS]: __(
'This epic already has the maximum number of child epics.',
),
};
/**
......
......@@ -48,6 +48,8 @@ export default {
'itemsFetchInProgress',
'itemsFetchResultEmpty',
'itemAddInProgress',
'itemAddFailure',
'itemAddFailureType',
'itemCreateInProgress',
'showAddItemForm',
'showCreateEpicForm',
......@@ -172,7 +174,10 @@ export default {
v-if="visibleForm"
:active-slot-names="[visibleForm]"
class="card-body add-item-form-container"
:class="{ 'border-bottom-0': itemsFetchResultEmpty }"
:class="{
'border-bottom-0': itemsFetchResultEmpty,
'gl-show-field-errors': itemAddFailure,
}"
>
<add-item-form
:slot="$options.FORM_SLOTS.addItem"
......@@ -182,6 +187,8 @@ export default {
:pending-references="pendingReferences"
:auto-complete-sources="itemAutoCompleteSources"
:path-id-separator="itemPathIdSeparator"
:has-error="itemAddFailure"
:item-add-failure-type="itemAddFailureType"
@pendingIssuableRemoveRequest="handlePendingItemRemove"
@addIssuableFormInput="handleAddItemFormInput"
@addIssuableFormBlur="handleAddItemFormBlur"
......
......@@ -6,8 +6,8 @@ import httpStatusCodes from '~/lib/utils/http_status';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import {
addRelatedIssueErrorMap,
issuableTypesMap,
itemAddFailureTypesMap,
pathIndeterminateErrorMap,
relatedIssuesRemoveErrorMap,
} from 'ee/related_issues/constants';
......@@ -308,14 +308,8 @@ export const receiveAddItemSuccess = ({ dispatch, commit, getters }, { rawItems
dispatch('setItemInputValue', '');
dispatch('toggleAddItemForm', { toggleState: false });
};
export const receiveAddItemFailure = ({ commit, state }, data = {}) => {
commit(types.RECEIVE_ADD_ITEM_FAILURE);
let errorMessage = addRelatedIssueErrorMap[state.issuableType];
if (data.message) {
errorMessage = data.message;
}
flash(errorMessage);
export const receiveAddItemFailure = ({ commit }, { itemAddFailureType }) => {
commit(types.RECEIVE_ADD_ITEM_FAILURE, { itemAddFailureType });
};
export const addItem = ({ state, dispatch, getters }) => {
dispatch('requestAddItem');
......@@ -330,8 +324,23 @@ export const addItem = ({ state, dispatch, getters }) => {
rawItems: data.issuables.slice(0, state.pendingReferences.length),
});
})
.catch(({ data }) => {
dispatch('receiveAddItemFailure', data);
.catch(data => {
const { response } = data;
if (response.status === 404) {
dispatch('receiveAddItemFailure', { itemAddFailureType: itemAddFailureTypesMap.NOT_FOUND });
}
// Ignore 409 conflict when the issue or epic is already attached to epic
/* eslint-disable @gitlab/i18n/no-non-i18n-strings */
else if (
response.status === 409 &&
response.data.message === 'Epic hierarchy level too deep'
) {
dispatch('receiveAddItemFailure', {
itemAddFailureType: itemAddFailureTypesMap.MAX_NUMBER_OF_CHILD_EPICS,
});
} else {
dispatch('receiveAddItemFailure');
}
});
};
......
......@@ -147,6 +147,9 @@ export default {
state.pendingReferences = state.pendingReferences.filter(
(ref, index) => index !== indexToRemove,
);
if (state.pendingReferences.length === 0) {
state.itemAddFailure = false;
}
},
[types.SET_ITEM_INPUT_VALUE](state, itemInputValue) {
......@@ -161,8 +164,12 @@ export default {
state.itemAddInProgress = false;
state.itemsFetchResultEmpty = false;
},
[types.RECEIVE_ADD_ITEM_FAILURE](state) {
[types.RECEIVE_ADD_ITEM_FAILURE](state, { itemAddFailureType }) {
state.itemAddInProgress = false;
state.itemAddFailure = true;
if (itemAddFailureType) {
state.itemAddFailureType = itemAddFailureType;
}
},
[types.REQUEST_CREATE_ITEM](state) {
......
......@@ -21,12 +21,14 @@ export default () => ({
itemInputValue: '',
pendingReferences: [],
itemAutoCompleteSources: {},
itemAddFailureType: null,
// UI Flags
itemsFetchInProgress: false,
itemsFetchFailure: false,
itemsFetchResultEmpty: false,
itemAddInProgress: false,
itemAddFailure: false,
itemCreateInProgress: false,
showAddItemForm: false,
showCreateEpicForm: false,
......
......@@ -40,6 +40,10 @@
border-color: $blue-300;
box-shadow: 0 0 4px $dropdown-input-focus-shadow;
}
.gl-show-field-errors &.form-control:not(textarea) {
height: auto;
}
}
.add-issuable-form-input-token-list {
......
......@@ -135,8 +135,8 @@ describe 'Epic Issues', :js do
it 'user cannot add new issues to the epic from another group' do
add_issues("#{issue_invalid.to_reference(full: true)}")
expect(page).to have_selector('.content-wrapper .flash-text')
expect(find('.flash-alert')).to have_text("We can't find an issue that matches what you are looking for.")
expect(page).to have_selector('.gl-field-error')
expect(find('.gl-field-error')).to have_text("Issue cannot be found.")
end
it 'user can add new issues to the epic' do
......@@ -144,14 +144,56 @@ describe 'Epic Issues', :js do
add_issues(references)
expect(page).not_to have_selector('.content-wrapper .flash-text')
expect(page).not_to have_content("We can't find an issue that matches what you are looking for.")
expect(page).not_to have_selector('.gl-field-error')
expect(page).not_to have_content("Issue cannot be found.")
within('.related-items-tree-container ul.related-items-list') do
expect(page).to have_selector('li.js-item-type-issue', count: 3)
end
end
it 'user cannot add new issue that does not exist' do
add_issues("&123")
expect(page).to have_selector('.gl-field-error')
expect(find('.gl-field-error')).to have_text("Issue cannot be found.")
end
it 'user cannot add new epic that does not exist' do
add_epics("&123")
expect(page).to have_selector('.gl-field-error')
expect(find('.gl-field-error')).to have_text("Epic cannot be found.")
end
context 'when epics are nested too deep' do
let(:epic1) { create(:epic, group: group, parent_id: epic.id) }
let(:epic2) { create(:epic, group: group, parent_id: epic1.id) }
let(:epic3) { create(:epic, group: group, parent_id: epic2.id) }
let(:epic4) { create(:epic, group: group, parent_id: epic3.id) }
before do
stub_licensed_features(epics: true)
sign_in(user)
visit group_epic_path(group, epic4)
wait_for_requests
find('.js-epic-tabs-container #tree-tab').click
wait_for_requests
end
it 'user cannot add new epic when hierarchy level limit has been reached' do
references = "#{epic_to_add.to_reference(full: true)}"
add_epics(references)
expect(page).to have_selector('.gl-field-error')
expect(find('.gl-field-error')).to have_text("This epic already has the maximum number of child epics.")
end
end
context 'with epic_new_issue feature flag enabled' do
before do
stub_feature_flags(epic_new_issue: true)
......@@ -163,8 +205,8 @@ describe 'Epic Issues', :js do
add_issues(references, button_selector: '.js-issue-actions-split-button')
expect(page).not_to have_selector('.content-wrapper .flash-text')
expect(page).not_to have_content("We can't find an issue that matches what you are looking for.")
expect(page).not_to have_selector('.gl-field-error')
expect(page).not_to have_content("Issue cannot be found.")
within('.related-items-tree-container ul.related-items-list') do
expect(page).to have_selector('li.js-item-type-issue', count: 3)
......@@ -176,8 +218,8 @@ describe 'Epic Issues', :js do
references = "#{epic_to_add.to_reference(full: true)}"
add_epics(references)
expect(page).not_to have_selector('.content-wrapper .flash-text')
expect(page).not_to have_content("We can't find an epic that matches what you are looking for.")
expect(page).not_to have_selector('.gl-field-error')
expect(page).not_to have_content("Epic cannot be found.")
within('.related-items-tree-container ul.related-items-list') do
expect(page).to have_selector('li.js-item-type-epic', count: 3)
......
......@@ -431,6 +431,16 @@ describe('RelatedItemsTree', () => {
expect(state.pendingReferences).toEqual(expect.arrayContaining(['foo']));
});
it('should update `itemAddFailure` to false if no `pendingReferences` are left', () => {
state.pendingReferences = ['foo'];
state.itemAddFailure = true;
mutations[types.REMOVE_PENDING_REFERENCE](state, 0);
expect(state.pendingReferences.length).toEqual(0);
expect(state.itemAddFailure).toBe(false);
});
});
describe(types.SET_ITEM_INPUT_VALUE, () => {
......@@ -468,10 +478,12 @@ describe('RelatedItemsTree', () => {
});
describe(types.RECEIVE_ADD_ITEM_FAILURE, () => {
it('should set `itemAddInProgress` to false within state', () => {
mutations[types.RECEIVE_ADD_ITEM_FAILURE](state);
it('should set `itemAddInProgress` to false, `itemAddFailure` to true and `itemAddFailureType` value within state', () => {
mutations[types.RECEIVE_ADD_ITEM_FAILURE](state, { itemAddFailureType: 'bar' });
expect(state.itemAddInProgress).toBe(false);
expect(state.itemAddFailure).toBe(true);
expect(state.itemAddFailureType).toEqual('bar');
});
});
......
......@@ -6,7 +6,11 @@ import * as types from 'ee/related_items_tree/store/mutation_types';
import * as epicUtils from 'ee/related_items_tree/utils/epic_utils';
import { ChildType, ChildState } from 'ee/related_items_tree/constants';
import { issuableTypesMap, PathIdSeparator } from 'ee/related_issues/constants';
import {
issuableTypesMap,
itemAddFailureTypesMap,
PathIdSeparator,
} from 'ee/related_issues/constants';
import axios from '~/lib/utils/axios_utils';
import testAction from 'spec/helpers/vuex_action_helper';
......@@ -911,41 +915,21 @@ describe('RelatedItemTree', () => {
});
describe('receiveAddItemFailure', () => {
beforeEach(() => {
setFixtures('<div class="flash-container"></div>');
});
it('should set `state.itemAddInProgress` to false', done => {
testAction(
actions.receiveAddItemFailure,
{},
{ itemAddFailureType: itemAddFailureTypesMap.NOT_FOUND },
{},
[
{
type: types.RECEIVE_ADD_ITEM_FAILURE,
payload: { itemAddFailureType: itemAddFailureTypesMap.NOT_FOUND },
},
],
[],
done,
);
});
it('should show flash error with message "Something went wrong while adding item."', () => {
const message = 'Something went wrong while adding item.';
actions.receiveAddItemFailure(
{
commit: () => {},
state: { issuableType: issuableTypesMap.EPIC },
},
{
message,
},
);
expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(
message,
);
});
});
describe('addItem', () => {
......@@ -1003,6 +987,7 @@ describe('RelatedItemTree', () => {
},
{
type: 'receiveAddItemFailure',
payload: { itemAddFailureType: itemAddFailureTypesMap.NOT_FOUND },
},
],
done,
......
......@@ -6644,6 +6644,9 @@ msgstr ""
msgid "Epic"
msgstr ""
msgid "Epic cannot be found."
msgstr ""
msgid "Epic events"
msgstr ""
......@@ -9611,6 +9614,9 @@ msgstr ""
msgid "Issue board focus mode"
msgstr ""
msgid "Issue cannot be found."
msgstr ""
msgid "Issue events"
msgstr ""
......@@ -17678,6 +17684,9 @@ msgstr ""
msgid "This environment has no deployments yet."
msgstr ""
msgid "This epic already has the maximum number of child epics."
msgstr ""
msgid "This epic does not exist or you don't have sufficient permission."
msgstr ""
......@@ -19487,12 +19496,6 @@ msgstr ""
msgid "Want to see the data? Please ask an administrator for access."
msgstr ""
msgid "We can't find an epic that matches what you are looking for."
msgstr ""
msgid "We can't find an issue that matches what you are looking for."
msgstr ""
msgid "We could not determine the path to remove the epic"
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