From b891a48d2191f2cddf98966ee84da5fd4a947bc8 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 2 Nov 2018 16:52:45 -0700 Subject: [PATCH 1/8] Improve Rollup Job Wizard error handling. - Consistently format errors with showApiWarning and showApiError helpers. - Use fatalError for unexpected errors. - Localize errors. --- x-pack/plugins/rollup/public/crud_app/app.js | 7 ++- .../public/crud_app/services/api_errors.js | 42 +++++++++++++++++ .../rollup/public/crud_app/services/index.js | 5 ++ .../store/actions/change_job_status.js | 12 +++-- .../crud_app/store/actions/create_job.js | 47 ++++++++++++------- .../crud_app/store/actions/delete_jobs.js | 21 +++++++-- .../crud_app/store/actions/load_jobs.js | 13 +++-- .../crud_app/store/actions/refresh_jobs.js | 13 +++-- 8 files changed, 130 insertions(+), 30 deletions(-) create mode 100644 x-pack/plugins/rollup/public/crud_app/services/api_errors.js diff --git a/x-pack/plugins/rollup/public/crud_app/app.js b/x-pack/plugins/rollup/public/crud_app/app.js index e17f3155c0196..a84db00d43364 100644 --- a/x-pack/plugins/rollup/public/crud_app/app.js +++ b/x-pack/plugins/rollup/public/crud_app/app.js @@ -9,7 +9,7 @@ import PropTypes from 'prop-types'; import { Switch, Route, Redirect } from 'react-router-dom'; import { CRUD_APP_BASE_PATH } from './constants'; -import { registerRouter } from './services'; +import { registerRouter, setUserHasLeftApp } from './services'; import { JobList, JobCreate } from './sections'; export class App extends Component { @@ -33,6 +33,11 @@ export class App extends Component { registerRouter(router); } + componentWillUnmount() { + // Set internal flag so we can prevent reacting to route changes internally. + setUserHasLeftApp(true); + } + render() { return (
diff --git a/x-pack/plugins/rollup/public/crud_app/services/api_errors.js b/x-pack/plugins/rollup/public/crud_app/services/api_errors.js new file mode 100644 index 0000000000000..372b0a7228ebd --- /dev/null +++ b/x-pack/plugins/rollup/public/crud_app/services/api_errors.js @@ -0,0 +1,42 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { fatalError, toastNotifications } from 'ui/notify'; + +function createToast(error) { + // Expect an error in the shape provided by Angular's $http service. + if (error && error.data) { + const { error: errorString, statusCode, message } = error.data; + return { + title: `${statusCode}: ${errorString}`, + text: message, + }; + } +} + +export function showApiWarning(error, location) { + const toast = createToast(error); + + if (toast) { + return toastNotifications.addWarning(toast); + } + + // This error isn't an HTTP error, so let the fatal error screen tell the user something + // unexpected happened. + return fatalError(error, location); +} + +export function showApiError(error, location) { + const toast = createToast(error); + + if (toast) { + return toastNotifications.addDanger(toast); + } + + // This error isn't an HTTP error, so let the fatal error screen tell the user something + // unexpected happened. + return fatalError(error, location); +} diff --git a/x-pack/plugins/rollup/public/crud_app/services/index.js b/x-pack/plugins/rollup/public/crud_app/services/index.js index f1ed752528f74..d47fa0b20be4b 100644 --- a/x-pack/plugins/rollup/public/crud_app/services/index.js +++ b/x-pack/plugins/rollup/public/crud_app/services/index.js @@ -13,6 +13,11 @@ export { validateIndexPattern, } from './api'; +export { + showApiError, + showApiWarning, +} from './api_errors'; + export { cronExpressionToParts, cronPartsToExpression, diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/change_job_status.js b/x-pack/plugins/rollup/public/crud_app/store/actions/change_job_status.js index fe26807944569..0f1f6f8b0be04 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/change_job_status.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/change_job_status.js @@ -4,11 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ -import { toastNotifications } from 'ui/notify'; +import { i18n } from '@kbn/i18n'; + import { startJobs as sendStartJobsRequest, stopJobs as sendStopJobsRequest, createNoticeableDelay, + showApiError, } from '../../services'; import { @@ -31,7 +33,9 @@ export const startJobs = (jobIds) => async (dispatch) => { type: UPDATE_JOB_FAILURE, }); - return toastNotifications.addDanger(error.data.message); + return showApiError(error, i18n.translate('xpack.rollupJobs.api.errors.startJobsErrorLocation.text', { + defaultMessage: 'Rollup Job Wizard start jobs', + })); } dispatch({ @@ -53,7 +57,9 @@ export const stopJobs = (jobIds) => async (dispatch) => { type: UPDATE_JOB_FAILURE, }); - return toastNotifications.addDanger(error.data.message); + return showApiError(error, i18n.translate('xpack.rollupJobs.api.errors.stopJobsErrorLocation.text', { + defaultMessage: 'Rollup Job Wizard stop jobs', + })); } dispatch({ diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/create_job.js b/x-pack/plugins/rollup/public/crud_app/store/actions/create_job.js index 34b83f046fb76..ae20d4a5680b0 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/create_job.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/create_job.js @@ -4,6 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ +import { i18n } from '@kbn/i18n'; +import { fatalError } from 'ui/notify'; + import { CRUD_APP_BASE_PATH } from '../../constants'; import { createJob as sendCreateJobRequest, @@ -33,34 +36,46 @@ export const createJob = (jobConfig) => async (dispatch) => { new Promise(resolve => setTimeout(resolve, 500)), ]); } catch (error) { - const { statusCode, data } = error; + if (error) { + const { statusCode, data } = error; - // Some errors have statusCode directly available but some are under a data property. - switch (statusCode || data.statusCode) { - case 409: - dispatch({ - type: CREATE_JOB_FAILURE, - payload: { - error: { - message: `A job with ID '${jobConfig.id}' already exists.`, + // Expect an error in the shape provided by Angular's $http service. + if (data) { + // Some errors have statusCode directly available but some are under a data property. + if ((statusCode || (data && data.statusCode)) === 409) { + return dispatch({ + type: CREATE_JOB_FAILURE, + payload: { + error: { + message: i18n.translate('xpack.rollupJobs.api.errors.createJobAlreadyExistsError.text', { + defaultMessage: 'A job with ID \'{jobConfigId}\' already exists.', + values: { jobConfigId: jobConfig.id }, + }), + }, }, - }, - }); - break; + }); + } - default: - dispatch({ + return dispatch({ type: CREATE_JOB_FAILURE, payload: { error: { - message: `Request failed with a ${statusCode} error. ${data.message}`, + message: i18n.translate('xpack.rollupJobs.api.errors.createJobDefaultError.text', { + defaultMessage: 'Request failed with a {statusCode} error. {message}', + values: { statusCode, message: data.message }, + }), cause: data.cause, }, }, }); + } } - return; + // This error isn't an HTTP error, so let the fatal error screen tell the user something + // unexpected happened. + return fatalError(error, i18n.translate('xpack.rollupJobs.api.errors.createJobErrorLocation.text', { + defaultMessage: 'Rollup Job Wizard create job', + })); } dispatch({ diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/delete_jobs.js b/x-pack/plugins/rollup/public/crud_app/store/actions/delete_jobs.js index 6ab2913870b66..df029344adeca 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/delete_jobs.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/delete_jobs.js @@ -4,9 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ +import { i18n } from '@kbn/i18n'; import { toastNotifications } from 'ui/notify'; -import { deleteJobs as sendDeleteJobsRequest, createNoticeableDelay } from '../../services'; +import { + deleteJobs as sendDeleteJobsRequest, + createNoticeableDelay, + showApiError, +} from '../../services'; import { getDetailPanelJob } from '../selectors'; import { @@ -30,13 +35,21 @@ export const deleteJobs = (jobIds) => async (dispatch, getState) => { type: UPDATE_JOB_FAILURE, }); - return toastNotifications.addDanger(error.data.message); + return showApiError(error, i18n.translate('xpack.rollupJobs.api.errors.deleteJobsErrorLocation.text', { + defaultMessage: 'Rollup Job Wizard delete jobs', + })); } if (jobIds.length === 1) { - toastNotifications.addSuccess(`Rollup job '${jobIds[0]}' was deleted`); + toastNotifications.addSuccess(i18n.translate('xpack.rollupJobs.api.success.deleteJobsSuccessSingleToast.title', { + defaultMessage: 'Rollup job \'{jobId}\' was deleted', + values: { jobId: jobIds[0] }, + })); } else { - toastNotifications.addSuccess(`${jobIds.length} rollup jobs were deleted`); + toastNotifications.addSuccess(i18n.translate('xpack.rollupJobs.api.success.deleteJobsSuccessMultipleToast.title', { + defaultMessage: '{count} rollup jobs were deleted', + values: { count: jobIds.length }, + })); } // If we've just deleted a job we were looking at, we need to close the panel. diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/load_jobs.js b/x-pack/plugins/rollup/public/crud_app/store/actions/load_jobs.js index 65891ebff33b0..5c90703971c6b 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/load_jobs.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/load_jobs.js @@ -4,8 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ -import { toastNotifications } from 'ui/notify'; -import { loadJobs as sendLoadJobsRequest, deserializeJobs } from '../../services'; +import { i18n } from '@kbn/i18n'; + +import { + loadJobs as sendLoadJobsRequest, + deserializeJobs, + showApiError, +} from '../../services'; import { LOAD_JOBS_START, LOAD_JOBS_SUCCESS, @@ -26,7 +31,9 @@ export const loadJobs = () => async (dispatch) => { payload: { error } }); - return toastNotifications.addDanger(error.data.message); + return showApiError(error, i18n.translate('xpack.rollupJobs.api.errors.loadJobsErrorLocation.text', { + defaultMessage: 'Rollup Job Wizard load jobs', + })); } dispatch({ diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/refresh_jobs.js b/x-pack/plugins/rollup/public/crud_app/store/actions/refresh_jobs.js index 3e74c2dd47627..89ad97f222776 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/refresh_jobs.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/refresh_jobs.js @@ -4,8 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ -import { toastNotifications } from 'ui/notify'; -import { loadJobs as sendLoadJobsRequest, deserializeJobs } from '../../services'; +import { i18n } from '@kbn/i18n'; + +import { + loadJobs as sendLoadJobsRequest, + deserializeJobs, + showApiWarning, +} from '../../services'; import { REFRESH_JOBS_SUCCESS, } from '../action_types'; @@ -15,7 +20,9 @@ export const refreshJobs = () => async (dispatch) => { try { jobs = await sendLoadJobsRequest(); } catch (error) { - return toastNotifications.addWarning(error.data.message); + return showApiWarning(error, i18n.translate('xpack.rollupJobs.api.errors.refreshJobsErrorLocation.text', { + defaultMessage: 'Rollup Job Wizard refresh jobs', + })); } dispatch({ From 4ebf0b18dfda99a1da5a8eb34963b7d956b2814f Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 2 Nov 2018 17:03:11 -0700 Subject: [PATCH 2/8] Surface error in Job List when the jobs can't be loaded. --- .../crud_app/sections/job_list/job_list.js | 42 ++++++++++++++++++- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/rollup/public/crud_app/sections/job_list/job_list.js b/x-pack/plugins/rollup/public/crud_app/sections/job_list/job_list.js index d9b4ce1bf5ba7..c9d963f732544 100644 --- a/x-pack/plugins/rollup/public/crud_app/sections/job_list/job_list.js +++ b/x-pack/plugins/rollup/public/crud_app/sections/job_list/job_list.js @@ -127,6 +127,38 @@ export class JobListUi extends Component { ); } + renderError(error) { + // We can safely depend upon the shape of this error coming from Angular $http, because we + // handle unexpected error shapes in the API action. + const { + statusCode, + error: errorString, + } = error.data; + + const { intl } = this.props; + const title = intl.formatMessage({ + id: 'xpack.rollupJobs.jobList.errorTitle', + defaultMessage: 'Error loading rollup jobs', + }); + return ( + + {this.getHeaderSection()} + + + + + + ); + } + renderEmpty() { return ( Date: Fri, 2 Nov 2018 17:08:55 -0700 Subject: [PATCH 3/8] Remove unnecessary comment. --- .../plugins/rollup/public/crud_app/sections/job_list/job_list.js | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/rollup/public/crud_app/sections/job_list/job_list.js b/x-pack/plugins/rollup/public/crud_app/sections/job_list/job_list.js index c9d963f732544..7decb0858e1e4 100644 --- a/x-pack/plugins/rollup/public/crud_app/sections/job_list/job_list.js +++ b/x-pack/plugins/rollup/public/crud_app/sections/job_list/job_list.js @@ -261,7 +261,6 @@ export class JobListUi extends Component { if (jobLoadError.status === 403) { content = this.renderNoPermission(); } else { - // There was an eror content = this.renderError(jobLoadError); } } else if (!isLoading && !jobs.length) { From d0fb9a1dcd35c27bfbb6bd79796705782fd9a0e9 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 2 Nov 2018 22:10:11 -0700 Subject: [PATCH 4/8] Localize index pattern validation fatal error location. --- .../rollup/public/crud_app/sections/job_create/job_create.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/rollup/public/crud_app/sections/job_create/job_create.js b/x-pack/plugins/rollup/public/crud_app/sections/job_create/job_create.js index a2ced6da423dc..869a9d88280ca 100644 --- a/x-pack/plugins/rollup/public/crud_app/sections/job_create/job_create.js +++ b/x-pack/plugins/rollup/public/crud_app/sections/job_create/job_create.js @@ -267,7 +267,9 @@ export class JobCreateUi extends Component { // This error isn't an HTTP error, so let the fatal error screen tell the user something // unexpected happened. - fatalError(error, 'Rollup Job Wizard index pattern validation'); + fatalError(error, i18n.translate('xpack.rollupJobs.create.errors.indexPatternValidationFatalErrorLocation', { + defaultMessage: 'Rollup Job Wizard index pattern validation', + })); }); }, 300); From 803e8fbe11bd343a19233432d02adbc6c54c96fb Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 5 Nov 2018 09:41:23 -0800 Subject: [PATCH 5/8] Address i18n feedback. --- .../public/crud_app/sections/job_create/job_create.js | 2 +- .../rollup/public/crud_app/sections/job_list/job_list.js | 8 ++------ .../plugins/rollup/public/crud_app/services/api_errors.js | 8 ++++---- .../public/crud_app/store/actions/change_job_status.js | 4 ++-- .../rollup/public/crud_app/store/actions/create_job.js | 8 ++++---- .../rollup/public/crud_app/store/actions/delete_jobs.js | 8 ++++---- .../rollup/public/crud_app/store/actions/load_jobs.js | 2 +- .../rollup/public/crud_app/store/actions/refresh_jobs.js | 2 +- 8 files changed, 19 insertions(+), 23 deletions(-) diff --git a/x-pack/plugins/rollup/public/crud_app/sections/job_create/job_create.js b/x-pack/plugins/rollup/public/crud_app/sections/job_create/job_create.js index 869a9d88280ca..610dcc1348231 100644 --- a/x-pack/plugins/rollup/public/crud_app/sections/job_create/job_create.js +++ b/x-pack/plugins/rollup/public/crud_app/sections/job_create/job_create.js @@ -267,7 +267,7 @@ export class JobCreateUi extends Component { // This error isn't an HTTP error, so let the fatal error screen tell the user something // unexpected happened. - fatalError(error, i18n.translate('xpack.rollupJobs.create.errors.indexPatternValidationFatalErrorLocation', { + fatalError(error, i18n.translate('xpack.rollupJobs.create.errors.indexPatternValidationFatalErrorTitle', { defaultMessage: 'Rollup Job Wizard index pattern validation', })); }); diff --git a/x-pack/plugins/rollup/public/crud_app/sections/job_list/job_list.js b/x-pack/plugins/rollup/public/crud_app/sections/job_list/job_list.js index 7decb0858e1e4..bde1b826b7590 100644 --- a/x-pack/plugins/rollup/public/crud_app/sections/job_list/job_list.js +++ b/x-pack/plugins/rollup/public/crud_app/sections/job_list/job_list.js @@ -137,7 +137,7 @@ export class JobListUi extends Component { const { intl } = this.props; const title = intl.formatMessage({ - id: 'xpack.rollupJobs.jobList.errorTitle', + id: 'xpack.rollupJobs.jobList.loadingErrorTitle', defaultMessage: 'Error loading rollup jobs', }); return ( @@ -149,11 +149,7 @@ export class JobListUi extends Component { color="danger" iconType="alert" > - + {statusCode} {errorString} ); diff --git a/x-pack/plugins/rollup/public/crud_app/services/api_errors.js b/x-pack/plugins/rollup/public/crud_app/services/api_errors.js index 372b0a7228ebd..58cc2c2a3598c 100644 --- a/x-pack/plugins/rollup/public/crud_app/services/api_errors.js +++ b/x-pack/plugins/rollup/public/crud_app/services/api_errors.js @@ -17,7 +17,7 @@ function createToast(error) { } } -export function showApiWarning(error, location) { +export function showApiWarning(error, title) { const toast = createToast(error); if (toast) { @@ -26,10 +26,10 @@ export function showApiWarning(error, location) { // This error isn't an HTTP error, so let the fatal error screen tell the user something // unexpected happened. - return fatalError(error, location); + return fatalError(error, title); } -export function showApiError(error, location) { +export function showApiError(error, title) { const toast = createToast(error); if (toast) { @@ -38,5 +38,5 @@ export function showApiError(error, location) { // This error isn't an HTTP error, so let the fatal error screen tell the user something // unexpected happened. - return fatalError(error, location); + return fatalError(error, title); } diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/change_job_status.js b/x-pack/plugins/rollup/public/crud_app/store/actions/change_job_status.js index 0f1f6f8b0be04..5144fbbd128bf 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/change_job_status.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/change_job_status.js @@ -33,7 +33,7 @@ export const startJobs = (jobIds) => async (dispatch) => { type: UPDATE_JOB_FAILURE, }); - return showApiError(error, i18n.translate('xpack.rollupJobs.api.errors.startJobsErrorLocation.text', { + return showApiError(error, i18n.translate('xpack.rollupJobs.startJobsAction.fatalErrorTitle', { defaultMessage: 'Rollup Job Wizard start jobs', })); } @@ -57,7 +57,7 @@ export const stopJobs = (jobIds) => async (dispatch) => { type: UPDATE_JOB_FAILURE, }); - return showApiError(error, i18n.translate('xpack.rollupJobs.api.errors.stopJobsErrorLocation.text', { + return showApiError(error, i18n.translate('xpack.rollupJobs.stopJobsAction.fatalErrorTitle', { defaultMessage: 'Rollup Job Wizard stop jobs', })); } diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/create_job.js b/x-pack/plugins/rollup/public/crud_app/store/actions/create_job.js index ae20d4a5680b0..547c833792f8a 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/create_job.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/create_job.js @@ -47,8 +47,8 @@ export const createJob = (jobConfig) => async (dispatch) => { type: CREATE_JOB_FAILURE, payload: { error: { - message: i18n.translate('xpack.rollupJobs.api.errors.createJobAlreadyExistsError.text', { - defaultMessage: 'A job with ID \'{jobConfigId}\' already exists.', + message: i18n.translate('xpack.rollupJobs.createAction.jobIdAlreadyExistsErrorMessage', { + defaultMessage: `A job with ID '{jobConfigId}' already exists.`, values: { jobConfigId: jobConfig.id }, }), }, @@ -60,7 +60,7 @@ export const createJob = (jobConfig) => async (dispatch) => { type: CREATE_JOB_FAILURE, payload: { error: { - message: i18n.translate('xpack.rollupJobs.api.errors.createJobDefaultError.text', { + message: i18n.translate('xpack.rollupJobs.createAction.failedDefaultErrorMessage', { defaultMessage: 'Request failed with a {statusCode} error. {message}', values: { statusCode, message: data.message }, }), @@ -73,7 +73,7 @@ export const createJob = (jobConfig) => async (dispatch) => { // This error isn't an HTTP error, so let the fatal error screen tell the user something // unexpected happened. - return fatalError(error, i18n.translate('xpack.rollupJobs.api.errors.createJobErrorLocation.text', { + return fatalError(error, i18n.translate('xpack.rollupJobs.createAction.fatalErrorTitle', { defaultMessage: 'Rollup Job Wizard create job', })); } diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/delete_jobs.js b/x-pack/plugins/rollup/public/crud_app/store/actions/delete_jobs.js index df029344adeca..2aaa8669f0c06 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/delete_jobs.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/delete_jobs.js @@ -35,18 +35,18 @@ export const deleteJobs = (jobIds) => async (dispatch, getState) => { type: UPDATE_JOB_FAILURE, }); - return showApiError(error, i18n.translate('xpack.rollupJobs.api.errors.deleteJobsErrorLocation.text', { + return showApiError(error, i18n.translate('xpack.rollupJobs.deleteAction.fatalErrorTitle', { defaultMessage: 'Rollup Job Wizard delete jobs', })); } if (jobIds.length === 1) { - toastNotifications.addSuccess(i18n.translate('xpack.rollupJobs.api.success.deleteJobsSuccessSingleToast.title', { - defaultMessage: 'Rollup job \'{jobId}\' was deleted', + toastNotifications.addSuccess(i18n.translate('xpack.rollupJobs.deleteAction.successSingleNotificationTitle', { + defaultMessage: `Rollup job '{jobId}' was deleted`, values: { jobId: jobIds[0] }, })); } else { - toastNotifications.addSuccess(i18n.translate('xpack.rollupJobs.api.success.deleteJobsSuccessMultipleToast.title', { + toastNotifications.addSuccess(i18n.translate('xpack.rollupJobs.deleteAction.successMultipleNotificationTitle', { defaultMessage: '{count} rollup jobs were deleted', values: { count: jobIds.length }, })); diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/load_jobs.js b/x-pack/plugins/rollup/public/crud_app/store/actions/load_jobs.js index 5c90703971c6b..5135484365541 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/load_jobs.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/load_jobs.js @@ -31,7 +31,7 @@ export const loadJobs = () => async (dispatch) => { payload: { error } }); - return showApiError(error, i18n.translate('xpack.rollupJobs.api.errors.loadJobsErrorLocation.text', { + return showApiError(error, i18n.translate('xpack.rollupJobs.loadAction.fatalErrorTitle', { defaultMessage: 'Rollup Job Wizard load jobs', })); } diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/refresh_jobs.js b/x-pack/plugins/rollup/public/crud_app/store/actions/refresh_jobs.js index 89ad97f222776..648986e343fd8 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/refresh_jobs.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/refresh_jobs.js @@ -20,7 +20,7 @@ export const refreshJobs = () => async (dispatch) => { try { jobs = await sendLoadJobsRequest(); } catch (error) { - return showApiWarning(error, i18n.translate('xpack.rollupJobs.api.errors.refreshJobsErrorLocation.text', { + return showApiWarning(error, i18n.translate('xpack.rollupJobs.refreshAction.fatalErrorTitle', { defaultMessage: 'Rollup Job Wizard refresh jobs', })); } From afe1212f2d719acbd8ea4fa92dfdf709b4aeb3a2 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 5 Nov 2018 11:19:03 -0800 Subject: [PATCH 6/8] Don't return fatalError. --- x-pack/plugins/rollup/public/crud_app/services/api_errors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/rollup/public/crud_app/services/api_errors.js b/x-pack/plugins/rollup/public/crud_app/services/api_errors.js index 58cc2c2a3598c..6762fd45fd00e 100644 --- a/x-pack/plugins/rollup/public/crud_app/services/api_errors.js +++ b/x-pack/plugins/rollup/public/crud_app/services/api_errors.js @@ -38,5 +38,5 @@ export function showApiError(error, title) { // This error isn't an HTTP error, so let the fatal error screen tell the user something // unexpected happened. - return fatalError(error, title); + fatalError(error, title); } From 39770a5e2feac5aca815a0dc559357118014a048 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 5 Nov 2018 11:38:11 -0800 Subject: [PATCH 7/8] Identify origin of toasts in their titles. --- .../public/crud_app/services/api_errors.js | 18 +++++++++--------- .../store/actions/change_job_status.js | 8 ++++---- .../crud_app/store/actions/create_job.js | 4 ++-- .../crud_app/store/actions/delete_jobs.js | 4 ++-- .../public/crud_app/store/actions/load_jobs.js | 4 ++-- .../crud_app/store/actions/refresh_jobs.js | 4 ++-- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/x-pack/plugins/rollup/public/crud_app/services/api_errors.js b/x-pack/plugins/rollup/public/crud_app/services/api_errors.js index 6762fd45fd00e..a0e6f51c69a66 100644 --- a/x-pack/plugins/rollup/public/crud_app/services/api_errors.js +++ b/x-pack/plugins/rollup/public/crud_app/services/api_errors.js @@ -6,19 +6,19 @@ import { fatalError, toastNotifications } from 'ui/notify'; -function createToast(error) { +function createToast(error, errorTitle) { // Expect an error in the shape provided by Angular's $http service. if (error && error.data) { const { error: errorString, statusCode, message } = error.data; return { - title: `${statusCode}: ${errorString}`, - text: message, + title: errorTitle, + text: `${statusCode}: ${errorString}. ${message}`, }; } } -export function showApiWarning(error, title) { - const toast = createToast(error); +export function showApiWarning(error, errorTitle) { + const toast = createToast(error, errorTitle); if (toast) { return toastNotifications.addWarning(toast); @@ -26,11 +26,11 @@ export function showApiWarning(error, title) { // This error isn't an HTTP error, so let the fatal error screen tell the user something // unexpected happened. - return fatalError(error, title); + return fatalError(error, errorTitle); } -export function showApiError(error, title) { - const toast = createToast(error); +export function showApiError(error, errorTitle) { + const toast = createToast(error, errorTitle); if (toast) { return toastNotifications.addDanger(toast); @@ -38,5 +38,5 @@ export function showApiError(error, title) { // This error isn't an HTTP error, so let the fatal error screen tell the user something // unexpected happened. - fatalError(error, title); + fatalError(error, errorTitle); } diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/change_job_status.js b/x-pack/plugins/rollup/public/crud_app/store/actions/change_job_status.js index 5144fbbd128bf..d95e40acf5322 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/change_job_status.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/change_job_status.js @@ -33,8 +33,8 @@ export const startJobs = (jobIds) => async (dispatch) => { type: UPDATE_JOB_FAILURE, }); - return showApiError(error, i18n.translate('xpack.rollupJobs.startJobsAction.fatalErrorTitle', { - defaultMessage: 'Rollup Job Wizard start jobs', + return showApiError(error, i18n.translate('xpack.rollupJobs.startJobsAction.errorTitle', { + defaultMessage: 'Error starting rollup jobs', })); } @@ -57,8 +57,8 @@ export const stopJobs = (jobIds) => async (dispatch) => { type: UPDATE_JOB_FAILURE, }); - return showApiError(error, i18n.translate('xpack.rollupJobs.stopJobsAction.fatalErrorTitle', { - defaultMessage: 'Rollup Job Wizard stop jobs', + return showApiError(error, i18n.translate('xpack.rollupJobs.stopJobsAction.errorTitle', { + defaultMessage: 'Error stopping rollup jobs', })); } diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/create_job.js b/x-pack/plugins/rollup/public/crud_app/store/actions/create_job.js index 547c833792f8a..42ad9be4041b4 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/create_job.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/create_job.js @@ -73,8 +73,8 @@ export const createJob = (jobConfig) => async (dispatch) => { // This error isn't an HTTP error, so let the fatal error screen tell the user something // unexpected happened. - return fatalError(error, i18n.translate('xpack.rollupJobs.createAction.fatalErrorTitle', { - defaultMessage: 'Rollup Job Wizard create job', + return fatalError(error, i18n.translate('xpack.rollupJobs.createAction.errorTitle', { + defaultMessage: 'Error creating rollup job', })); } diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/delete_jobs.js b/x-pack/plugins/rollup/public/crud_app/store/actions/delete_jobs.js index 2aaa8669f0c06..866f79b7f423c 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/delete_jobs.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/delete_jobs.js @@ -35,8 +35,8 @@ export const deleteJobs = (jobIds) => async (dispatch, getState) => { type: UPDATE_JOB_FAILURE, }); - return showApiError(error, i18n.translate('xpack.rollupJobs.deleteAction.fatalErrorTitle', { - defaultMessage: 'Rollup Job Wizard delete jobs', + return showApiError(error, i18n.translate('xpack.rollupJobs.deleteAction.errorTitle', { + defaultMessage: 'Error deleting rollup jobs', })); } diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/load_jobs.js b/x-pack/plugins/rollup/public/crud_app/store/actions/load_jobs.js index 5135484365541..681b2d3e2391e 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/load_jobs.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/load_jobs.js @@ -31,8 +31,8 @@ export const loadJobs = () => async (dispatch) => { payload: { error } }); - return showApiError(error, i18n.translate('xpack.rollupJobs.loadAction.fatalErrorTitle', { - defaultMessage: 'Rollup Job Wizard load jobs', + return showApiError(error, i18n.translate('xpack.rollupJobs.loadAction.errorTitle', { + defaultMessage: 'Error loading rollup jobs', })); } diff --git a/x-pack/plugins/rollup/public/crud_app/store/actions/refresh_jobs.js b/x-pack/plugins/rollup/public/crud_app/store/actions/refresh_jobs.js index 648986e343fd8..2317f34dfccdf 100644 --- a/x-pack/plugins/rollup/public/crud_app/store/actions/refresh_jobs.js +++ b/x-pack/plugins/rollup/public/crud_app/store/actions/refresh_jobs.js @@ -20,8 +20,8 @@ export const refreshJobs = () => async (dispatch) => { try { jobs = await sendLoadJobsRequest(); } catch (error) { - return showApiWarning(error, i18n.translate('xpack.rollupJobs.refreshAction.fatalErrorTitle', { - defaultMessage: 'Rollup Job Wizard refresh jobs', + return showApiWarning(error, i18n.translate('xpack.rollupJobs.refreshAction.errorTitle', { + defaultMessage: 'Error refreshing rollup jobs', })); } From 2f759fb5947e15f40ca48584119e41f8a640b8fe Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 5 Nov 2018 12:11:26 -0800 Subject: [PATCH 8/8] Rename createToast to createToastConfig. --- .../rollup/public/crud_app/services/api_errors.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/rollup/public/crud_app/services/api_errors.js b/x-pack/plugins/rollup/public/crud_app/services/api_errors.js index a0e6f51c69a66..bacaf13405898 100644 --- a/x-pack/plugins/rollup/public/crud_app/services/api_errors.js +++ b/x-pack/plugins/rollup/public/crud_app/services/api_errors.js @@ -6,7 +6,7 @@ import { fatalError, toastNotifications } from 'ui/notify'; -function createToast(error, errorTitle) { +function createToastConfig(error, errorTitle) { // Expect an error in the shape provided by Angular's $http service. if (error && error.data) { const { error: errorString, statusCode, message } = error.data; @@ -18,10 +18,10 @@ function createToast(error, errorTitle) { } export function showApiWarning(error, errorTitle) { - const toast = createToast(error, errorTitle); + const toastConfig = createToastConfig(error, errorTitle); - if (toast) { - return toastNotifications.addWarning(toast); + if (toastConfig) { + return toastNotifications.addWarning(toastConfig); } // This error isn't an HTTP error, so let the fatal error screen tell the user something @@ -30,10 +30,10 @@ export function showApiWarning(error, errorTitle) { } export function showApiError(error, errorTitle) { - const toast = createToast(error, errorTitle); + const toastConfig = createToastConfig(error, errorTitle); - if (toast) { - return toastNotifications.addDanger(toast); + if (toastConfig) { + return toastNotifications.addDanger(toastConfig); } // This error isn't an HTTP error, so let the fatal error screen tell the user something