From 5f9851647095f2c4792e978dcc384ac5a574798a Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Mon, 6 Apr 2020 16:14:26 +0200 Subject: [PATCH] [ML] Fix error messages. (#62575) (#62599) Fixes two regressions: - The migration to NP and its related http service changed the format of error messages, because of this, the output in toast error messages ended up less informational. This PR fixes it by reusing the ML plugin's updated getErrorMessage() function. - The migration to NP and move to NP's useKibanaContext() introduced a regression where the modal overlays of error toast with long error message would no longer work anymore. This PR fixes it by passing on overlays as a component prop instead of using the context hook within the component. --- .../toast_notification_text.test.tsx | 4 ++++ .../app/components/toast_notification_text.tsx | 13 +++++++++---- .../public/app/hooks/use_delete_transform.tsx | 9 +++++---- .../step_create/step_create_form.tsx | 18 ++++++++++++++---- .../step_details/step_details_form.tsx | 14 +++++++++++--- .../plugins/transform/public/shared_imports.ts | 2 ++ 6 files changed, 45 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/transform/public/app/components/toast_notification_text.test.tsx b/x-pack/plugins/transform/public/app/components/toast_notification_text.test.tsx index e51119d67d567..40fcf789b30bb 100644 --- a/x-pack/plugins/transform/public/app/components/toast_notification_text.test.tsx +++ b/x-pack/plugins/transform/public/app/components/toast_notification_text.test.tsx @@ -7,6 +7,8 @@ import React from 'react'; import { render } from '@testing-library/react'; +import { CoreStart } from 'src/core/public'; + import { ToastNotificationText } from './toast_notification_text'; jest.mock('../../shared_imports'); @@ -15,6 +17,7 @@ jest.mock('../../app/app_dependencies'); describe('ToastNotificationText', () => { test('should render the text as plain text', () => { const props = { + overlays: {} as CoreStart['overlays'], text: 'a short text message', }; const { container } = render(); @@ -23,6 +26,7 @@ describe('ToastNotificationText', () => { test('should render the text within a modal', () => { const props = { + overlays: {} as CoreStart['overlays'], text: 'a text message that is longer than 140 characters. a text message that is longer than 140 characters. a text message that is longer than 140 characters. ', }; diff --git a/x-pack/plugins/transform/public/app/components/toast_notification_text.tsx b/x-pack/plugins/transform/public/app/components/toast_notification_text.tsx index 44927e61a42c4..1044081670523 100644 --- a/x-pack/plugins/transform/public/app/components/toast_notification_text.tsx +++ b/x-pack/plugins/transform/public/app/components/toast_notification_text.tsx @@ -18,15 +18,20 @@ import { import { i18n } from '@kbn/i18n'; -import { toMountPoint } from '../../../../../../src/plugins/kibana_react/public'; +import { CoreStart } from 'src/core/public'; -import { useAppDependencies } from '../app_dependencies'; +import { toMountPoint } from '../../../../../../src/plugins/kibana_react/public'; const MAX_SIMPLE_MESSAGE_LENGTH = 140; -export const ToastNotificationText: FC<{ text: any }> = ({ text }) => { - const { overlays } = useAppDependencies(); +// Because of the use of `toMountPoint`, `useKibanaContext` doesn't work via `useAppDependencies`. +// That's why we need to pass in `overlays` as a prop cannot get it via context. +interface ToastNotificationTextProps { + overlays: CoreStart['overlays']; + text: any; +} +export const ToastNotificationText: FC = ({ overlays, text }) => { if (typeof text === 'string' && text.length <= MAX_SIMPLE_MESSAGE_LENGTH) { return text; } diff --git a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx index 6210c72ef9d05..23693cd7222c8 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx +++ b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx @@ -11,13 +11,16 @@ import { toMountPoint } from '../../../../../../src/plugins/kibana_react/public' import { TransformEndpointRequest, TransformEndpointResult } from '../../../common'; -import { useToastNotifications } from '../app_dependencies'; +import { getErrorMessage } from '../../shared_imports'; + +import { useAppDependencies, useToastNotifications } from '../app_dependencies'; import { TransformListRow, refreshTransformList$, REFRESH_TRANSFORM_LIST_STATE } from '../common'; import { ToastNotificationText } from '../components'; import { useApi } from './use_api'; export const useDeleteTransforms = () => { + const { overlays } = useAppDependencies(); const toastNotifications = useToastNotifications(); const api = useApi(); @@ -56,9 +59,7 @@ export const useDeleteTransforms = () => { title: i18n.translate('xpack.transform.transformList.deleteTransformGenericErrorMessage', { defaultMessage: 'An error occurred calling the API endpoint to delete transforms.', }), - text: toMountPoint( - - ), + text: toMountPoint(), }); } }; diff --git a/x-pack/plugins/transform/public/app/sections/create_transform/components/step_create/step_create_form.tsx b/x-pack/plugins/transform/public/app/sections/create_transform/components/step_create/step_create_form.tsx index 5dcaece28bdde..5e5aace4139f0 100644 --- a/x-pack/plugins/transform/public/app/sections/create_transform/components/step_create/step_create_form.tsx +++ b/x-pack/plugins/transform/public/app/sections/create_transform/components/step_create/step_create_form.tsx @@ -32,6 +32,8 @@ import { toMountPoint } from '../../../../../../../../../src/plugins/kibana_reac import { PROGRESS_REFRESH_INTERVAL_MS } from '../../../../../../common/constants'; +import { getErrorMessage } from '../../../../../shared_imports'; + import { getTransformProgress, getDiscoverUrl } from '../../../../common'; import { useApi } from '../../../../hooks/use_api'; import { useAppDependencies, useToastNotifications } from '../../../../app_dependencies'; @@ -116,7 +118,9 @@ export const StepCreateForm: FC = React.memo( defaultMessage: 'An error occurred creating the transform {transformId}:', values: { transformId }, }), - text: toMountPoint(), + text: toMountPoint( + + ), }); setCreated(false); setLoading(false); @@ -157,7 +161,9 @@ export const StepCreateForm: FC = React.memo( defaultMessage: 'An error occurred starting the transform {transformId}:', values: { transformId }, }), - text: toMountPoint(), + text: toMountPoint( + + ), }); setStarted(false); setLoading(false); @@ -223,7 +229,9 @@ export const StepCreateForm: FC = React.memo( 'An error occurred creating the Kibana index pattern {indexPatternName}:', values: { indexPatternName }, }), - text: toMountPoint(), + text: toMountPoint( + + ), }); setLoading(false); return false; @@ -260,7 +268,9 @@ export const StepCreateForm: FC = React.memo( title: i18n.translate('xpack.transform.stepCreateForm.progressErrorMessage', { defaultMessage: 'An error occurred getting the progress percentage:', }), - text: toMountPoint(), + text: toMountPoint( + + ), }); clearInterval(interval); } diff --git a/x-pack/plugins/transform/public/app/sections/create_transform/components/step_details/step_details_form.tsx b/x-pack/plugins/transform/public/app/sections/create_transform/components/step_details/step_details_form.tsx index d47af47214851..9a39616fb0989 100644 --- a/x-pack/plugins/transform/public/app/sections/create_transform/components/step_details/step_details_form.tsx +++ b/x-pack/plugins/transform/public/app/sections/create_transform/components/step_details/step_details_form.tsx @@ -16,6 +16,8 @@ import { toMountPoint } from '../../../../../../../../../src/plugins/kibana_reac import { TransformId } from '../../../../../../common'; import { isValidIndexName } from '../../../../../../common/utils/es_utils'; +import { getErrorMessage } from '../../../../../shared_imports'; + import { useAppDependencies, useToastNotifications } from '../../../../app_dependencies'; import { ToastNotificationText } from '../../../../components'; import { useDocumentationLinks } from '../../../../hooks/use_documentation_links'; @@ -116,7 +118,9 @@ export const StepDetailsForm: FC = React.memo( title: i18n.translate('xpack.transform.stepDetailsForm.errorGettingTransformList', { defaultMessage: 'An error occurred getting the existing transform IDs:', }), - text: toMountPoint(), + text: toMountPoint( + + ), }); } @@ -127,7 +131,9 @@ export const StepDetailsForm: FC = React.memo( title: i18n.translate('xpack.transform.stepDetailsForm.errorGettingIndexNames', { defaultMessage: 'An error occurred getting the existing index names:', }), - text: toMountPoint(), + text: toMountPoint( + + ), }); } @@ -141,7 +147,9 @@ export const StepDetailsForm: FC = React.memo( defaultMessage: 'An error occurred getting the existing index pattern titles:', } ), - text: toMountPoint(), + text: toMountPoint( + + ), }); } })(); diff --git a/x-pack/plugins/transform/public/shared_imports.ts b/x-pack/plugins/transform/public/shared_imports.ts index 938ec77344d8f..8eb42ad677c0f 100644 --- a/x-pack/plugins/transform/public/shared_imports.ts +++ b/x-pack/plugins/transform/public/shared_imports.ts @@ -15,3 +15,5 @@ export { UseRequestConfig, useRequest, } from '../../../../src/plugins/es_ui_shared/public/request/np_ready_request'; + +export { getErrorMessage } from '../../ml/common/util/errors';