Merge branch '32557-convert-generic-epic-error-banners-to-form-validation-messages' into 'master'

Convert flash epic error to form validation error

See merge request gitlab-org/gitlab!20130

changelogs/unreleased/32557-convert-generic-epic-error-banners-to-form-validation-messages.yml

+---
+title: Convert flash epic error to form validation error
+merge_request: 20130
+author:
+type: changed

ee/app/assets/javascripts/related_issues/components/add_issuable_form.vue

@@ -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" />

ee/app/assets/javascripts/related_issues/components/related_issuable_input.vue

@@ -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"
   >

ee/app/assets/javascripts/related_issues/constants.js

@@ -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.',
+  ),
 };
 
 /**

ee/app/assets/javascripts/related_items_tree/components/related_items_tree_app.vue

@@ -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"

ee/app/assets/javascripts/related_items_tree/store/actions.js

@@ -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');
+      }
     });
 };
 

ee/app/assets/javascripts/related_items_tree/store/mutations.js

@@ -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) {

ee/app/assets/javascripts/related_items_tree/store/state.js

@@ -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,

ee/app/assets/stylesheets/pages/issuable.scss

@@ -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 {

ee/spec/features/epics/epic_issues_spec.rb

@@ -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)

ee/spec/frontend/related_items_tree/store/mutations_spec.js

@@ -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');
         });
       });
 

ee/spec/javascripts/related_items_tree/store/actions_spec.js

@@ -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,

locale/gitlab.pot

@@ -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 ""