[7.x] [Enterprise Search] Fix bug in Add Schema modal (#104024) (#104702

) * [Enterprise Search] Fix bug in Add Schema modal (#104024) * [Enterprise Search] Fix bug in Add Schema modal This PR fixes a bug where an error passed from the server was not rendering and was causing the UI to hang. The problem is that the `body` property was missed in the error object. * Use default message when error response for actions.setServerField has no message * i18n-ize default error message * Remove TODO Co-authored-by: Byron Hulcher <byronhulcher@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> * [Workplace Search] Fix bug where error was behind modal stuck in loading state (#104360) * Fix an issue from previous PR In #104024, the error handling incorrectly used the `message` property on the response, when it should have been the attributes.errors array. * Use inline error for duplicate name Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co> Co-authored-by: Byron Hulcher <byronhulcher@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
elastic · Jul 7, 2021 · de121c2 · de121c2
1 parent a6c80dd
commit de121c2
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 10 deletions.
diff --git a/.../plugins/enterprise_search/public/applications/shared/flash_messages/handle_api_errors.ts b/.../plugins/enterprise_search/public/applications/shared/flash_messages/handle_api_errors.ts
@@ -5,6 +5,8 @@
  * 2.0.
  */
 
+import { i18n } from '@kbn/i18n';
+
 import { HttpResponse } from 'src/core/public';
 
 import { FlashMessagesLogic } from './flash_messages_logic';
@@ -31,12 +33,17 @@ interface Options {
   isQueued?: boolean;
 }
 
+export const defaultErrorMessage = i18n.translate(
+  'xpack.enterpriseSearch.shared.flashMessages.defaultErrorMessage',
+  {
+    defaultMessage: 'An unexpected error occurred',
+  }
+);
+
 /**
  * Converts API/HTTP errors into user-facing Flash Messages
  */
 export const flashAPIErrors = (error: HttpResponse<ErrorResponse>, { isQueued }: Options = {}) => {
-  const defaultErrorMessage = 'An unexpected error occurred';
-
   const errorFlashMessages: IFlashMessage[] = Array.isArray(error?.body?.attributes?.errors)
     ? error.body!.attributes.errors.map((message) => ({ type: 'error', message }))
     : [{ type: 'error', message: error?.body?.message || defaultErrorMessage }];

diff --git a/...pplications/workplace_search/views/content_sources/components/schema/schema_logic.test.ts b/...pplications/workplace_search/views/content_sources/components/schema/schema_logic.test.ts
@@ -27,6 +27,7 @@ const spyScrollTo = jest.fn();
 Object.defineProperty(global.window, 'scrollTo', { value: spyScrollTo });
 
 import { ADD, UPDATE } from '../../../../../shared/constants/operations';
+import { defaultErrorMessage } from '../../../../../shared/flash_messages/handle_api_errors';
 import { SchemaType } from '../../../../../shared/schema/types';
 import { AppLogic } from '../../../../app_logic';
 
@@ -324,10 +325,11 @@ describe('SchemaLogic', () => {
       });
 
       it('handles duplicate', () => {
+        const onSchemaSetFormErrorsSpy = jest.spyOn(SchemaLogic.actions, 'onSchemaSetFormErrors');
         SchemaLogic.actions.onInitializeSchema(serverResponse);
         SchemaLogic.actions.addNewField('foo', SchemaType.Number);
 
-        expect(setErrorMessage).toHaveBeenCalledWith('New field already exists: foo.');
+        expect(onSchemaSetFormErrorsSpy).toHaveBeenCalledWith(['New field already exists: foo.']);
       });
     });
 
@@ -390,13 +392,27 @@ describe('SchemaLogic', () => {
           expect(onSchemaSetSuccessSpy).toHaveBeenCalledWith(serverResponse);
         });
 
-        it('handles error', async () => {
+        it('handles error with message', async () => {
+          const onSchemaSetFormErrorsSpy = jest.spyOn(SchemaLogic.actions, 'onSchemaSetFormErrors');
+          // We expect body.attributes.errors to be a string[] when it is present
+          http.post.mockReturnValue(
+            Promise.reject({ body: { attributes: { errors: ['this is an error'] } } })
+          );
+          SchemaLogic.actions.setServerField(schema, ADD);
+          await nextTick();
+
+          expect(onSchemaSetFormErrorsSpy).toHaveBeenCalledWith(['this is an error']);
+          expect(spyScrollTo).toHaveBeenCalledWith(0, 0);
+        });
+
+        it('handles error with no message', async () => {
           const onSchemaSetFormErrorsSpy = jest.spyOn(SchemaLogic.actions, 'onSchemaSetFormErrors');
-          http.post.mockReturnValue(Promise.reject({ message: 'this is an error' }));
+          http.post.mockReturnValue(Promise.reject());
           SchemaLogic.actions.setServerField(schema, ADD);
           await nextTick();
 
-          expect(onSchemaSetFormErrorsSpy).toHaveBeenCalledWith('this is an error');
+          expect(onSchemaSetFormErrorsSpy).toHaveBeenCalledWith([defaultErrorMessage]);
+          expect(spyScrollTo).toHaveBeenCalledWith(0, 0);
         });
       });
 

diff --git a/...lic/applications/workplace_search/views/content_sources/components/schema/schema_logic.ts b/...lic/applications/workplace_search/views/content_sources/components/schema/schema_logic.ts
@@ -17,6 +17,7 @@ import {
   setErrorMessage,
   clearFlashMessages,
 } from '../../../../../shared/flash_messages';
+import { defaultErrorMessage } from '../../../../../shared/flash_messages/handle_api_errors';
 import { HttpLogic } from '../../../../../shared/http';
 import {
   IndexJob,
@@ -300,15 +301,15 @@ export const SchemaLogic = kea<MakeLogicType<SchemaValues, SchemaActions>>({
     addNewField: ({ fieldName, newFieldType }) => {
       if (fieldName in values.activeSchema) {
         window.scrollTo(0, 0);
-        setErrorMessage(
+        actions.onSchemaSetFormErrors([
           i18n.translate(
             'xpack.enterpriseSearch.workplaceSearch.contentSource.schema.newFieldExists.message',
             {
               defaultMessage: 'New field already exists: {fieldName}.',
               values: { fieldName },
             }
-          )
-        );
+          ),
+        ]);
       } else {
         const schema = cloneDeep(values.activeSchema);
         schema[fieldName] = newFieldType;
@@ -349,7 +350,9 @@ export const SchemaLogic = kea<MakeLogicType<SchemaValues, SchemaActions>>({
       } catch (e) {
         window.scrollTo(0, 0);
         if (isAdding) {
-          actions.onSchemaSetFormErrors(e?.message);
+          // We expect body.attributes.errors to be a string[] for actions.onSchemaSetFormErrors
+          const message: string[] = e?.body?.attributes?.errors || [defaultErrorMessage];
+          actions.onSchemaSetFormErrors(message);
         } else {
           flashAPIErrors(e);
         }