Add error handling when fetching configuration fails

elastic · Mar 23, 2020 · d83d7a1 · d83d7a1
1 parent c585a15
commit d83d7a1
Show file tree

Hide file tree

Showing 11 changed files with 46 additions and 13 deletions.
diff --git a/...app/Settings/AgentConfigurations/AgentConfigurationCreateEdit/ServicePage/ServicePage.tsx b/...app/Settings/AgentConfigurations/AgentConfigurationCreateEdit/ServicePage/ServicePage.tsx
@@ -20,7 +20,7 @@ import { AgentConfigurationIntake } from '../../../../../../../../../../plugins/
 import {
   omitAllOption,
   getOptionLabel
-} from '../../../../../../../../../../plugins/apm/common/agent_configuration/constants';
+} from '../../../../../../../../../../plugins/apm/common/agent_configuration/all_option';
 import { useFetcher, FETCH_STATUS } from '../../../../../../hooks/useFetcher';
 import { FormRowSelect } from './FormRowSelect';
 import { APMLink } from '../../../../../shared/Links/apm/APMLink';

diff --git a/...p/Settings/AgentConfigurations/AgentConfigurationCreateEdit/SettingsPage/SettingsPage.tsx b/...p/Settings/AgentConfigurations/AgentConfigurationCreateEdit/SettingsPage/SettingsPage.tsx
@@ -21,6 +21,8 @@ import {
 import React, { useState, useMemo } from 'react';
 import { i18n } from '@kbn/i18n';
 import { EuiButtonEmpty } from '@elastic/eui';
+import { EuiCallOut } from '@elastic/eui';
+import { FETCH_STATUS } from '../../../../../../hooks/useFetcher';
 import { AgentName } from '../../../../../../../../../../plugins/apm/typings/es_schemas/ui/fields/agent';
 import { history } from '../../../../../../utils/history';
 import { AgentConfigurationIntake } from '../../../../../../../../../../plugins/apm/common/agent_configuration/configuration_types';
@@ -33,7 +35,7 @@ import { saveConfig } from './saveConfig';
 import { useApmPluginContext } from '../../../../../../hooks/useApmPluginContext';
 import { useUiTracker } from '../../../../../../../../../../plugins/observability/public';
 import { SettingFormRow } from './SettingFormRow';
-import { getOptionLabel } from '../../../../../../../../../../plugins/apm/common/agent_configuration/constants';
+import { getOptionLabel } from '../../../../../../../../../../plugins/apm/common/agent_configuration/all_option';
 
 function removeEmpty<T>(obj: T): T {
   return Object.fromEntries(
@@ -42,15 +44,15 @@ function removeEmpty<T>(obj: T): T {
 }
 
 export function SettingsPage({
-  isLoading,
+  status,
   unsavedChanges,
   newConfig,
   setNewConfig,
   resetSettings,
   isEditMode,
   onClickEdit
 }: {
-  isLoading: boolean;
+  status?: FETCH_STATUS;
   unsavedChanges: Record<string, string>;
   newConfig: AgentConfigurationIntake;
   setNewConfig: React.Dispatch<React.SetStateAction<AgentConfigurationIntake>>;
@@ -63,6 +65,7 @@ export function SettingsPage({
   const { toasts } = useApmPluginContext().core.notifications;
   const [isSaving, setIsSaving] = useState(false);
   const unsavedChangesCount = Object.keys(unsavedChanges).length;
+  const isLoading = status === FETCH_STATUS.LOADING;
 
   const isFormValid = useMemo(() => {
     return (
@@ -96,6 +99,26 @@ export function SettingsPage({
     });
   };
 
+  if (status === FETCH_STATUS.FAILURE) {
+    return (
+      <EuiCallOut
+        title={i18n.translate(
+          'xpack.apm.agentConfig.settingsPage.notFound.title',
+          { defaultMessage: 'Sorry, there was an error' }
+        )}
+        color="danger"
+        iconType="alert"
+      >
+        <p>
+          {i18n.translate(
+            'xpack.apm.agentConfig.settingsPage.notFound.message',
+            { defaultMessage: 'The requested configuration does not exist' }
+          )}
+        </p>
+      </EuiCallOut>
+    );
+  }
+
   return (
     <>
       <EuiForm>

diff --git a/.../app/Settings/AgentConfigurations/AgentConfigurationCreateEdit/SettingsPage/saveConfig.ts b/.../app/Settings/AgentConfigurations/AgentConfigurationCreateEdit/SettingsPage/saveConfig.ts
@@ -10,7 +10,7 @@ import { AgentConfigurationIntake } from '../../../../../../../../../../plugins/
 import {
   getOptionLabel,
   omitAllOption
-} from '../../../../../../../../../../plugins/apm/common/agent_configuration/constants';
+} from '../../../../../../../../../../plugins/apm/common/agent_configuration/all_option';
 import { callApmApi } from '../../../../../../services/rest/createCallApmApi';
 
 export async function saveConfig({

diff --git a/...public/components/app/Settings/AgentConfigurations/AgentConfigurationCreateEdit/index.tsx b/...public/components/app/Settings/AgentConfigurations/AgentConfigurationCreateEdit/index.tsx
@@ -4,10 +4,11 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import { isEmpty } from 'lodash';
 import { EuiTitle, EuiText, EuiSpacer } from '@elastic/eui';
 import React, { useState, useEffect, useCallback } from 'react';
 import { i18n } from '@kbn/i18n';
-import { FetcherResult, FETCH_STATUS } from '../../../../../hooks/useFetcher';
+import { FetcherResult } from '../../../../../hooks/useFetcher';
 import { history } from '../../../../../utils/history';
 import {
   AgentConfigurationIntake,
@@ -90,6 +91,15 @@ export function AgentConfigurationCreateEdit({
     if (pageStep === 'choose-service-step' && isEditMode) {
       setPage('choose-settings-step');
     }
+
+    // the user skipped the first step (select service)
+    if (
+      pageStep === 'choose-settings-step' &&
+      !isEditMode &&
+      isEmpty(newConfig.service)
+    ) {
+      setPage('choose-service-step');
+    }
   }, [isEditMode, newConfig, pageStep]);
 
   const unsavedChanges = getUnsavedChanges({ newConfig, existingConfig });
@@ -128,7 +138,7 @@ export function AgentConfigurationCreateEdit({
 
       {pageStep === 'choose-settings-step' && (
         <SettingsPage
-          isLoading={existingConfigResult?.status === FETCH_STATUS.LOADING}
+          status={existingConfigResult?.status}
           unsavedChanges={unsavedChanges}
           onClickEdit={() => setPage('choose-service-step')}
           newConfig={newConfig}

diff --git a/...lugins/apm/public/components/app/Settings/AgentConfigurations/List/ConfirmDeleteModal.tsx b/...lugins/apm/public/components/app/Settings/AgentConfigurations/List/ConfirmDeleteModal.tsx
@@ -10,7 +10,7 @@ import { NotificationsStart } from 'kibana/public';
 import { i18n } from '@kbn/i18n';
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
 import { AgentConfigurationListAPIResponse } from '../../../../../../../../../plugins/apm/server/lib/settings/agent_configuration/list_configurations';
-import { getOptionLabel } from '../../../../../../../../../plugins/apm/common/agent_configuration/constants';
+import { getOptionLabel } from '../../../../../../../../../plugins/apm/common/agent_configuration/all_option';
 import { callApmApi } from '../../../../../services/rest/createCallApmApi';
 import { useApmPluginContext } from '../../../../../hooks/useApmPluginContext';
 

diff --git a/x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/List/index.tsx b/x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/List/index.tsx
@@ -23,7 +23,7 @@ import { LoadingStatePrompt } from '../../../../shared/LoadingStatePrompt';
 import { AgentConfigurationListAPIResponse } from '../../../../../../../../../plugins/apm/server/lib/settings/agent_configuration/list_configurations';
 import { TimestampTooltip } from '../../../../shared/TimestampTooltip';
 import { px, units } from '../../../../../style/variables';
-import { getOptionLabel } from '../../../../../../../../../plugins/apm/common/agent_configuration/constants';
+import { getOptionLabel } from '../../../../../../../../../plugins/apm/common/agent_configuration/all_option';
 import {
   createAgentConfigurationHref,
   editAgentConfigurationHref

diff --git a/...m/common/agent_configuration/constants.ts → .../common/agent_configuration/all_option.ts b/...m/common/agent_configuration/constants.ts → .../common/agent_configuration/all_option.ts
diff --git a/...gins/apm/server/lib/settings/agent_configuration/get_environments/get_all_environments.ts b/...gins/apm/server/lib/settings/agent_configuration/get_environments/get_all_environments.ts
@@ -10,7 +10,7 @@ import {
   SERVICE_NAME,
   SERVICE_ENVIRONMENT
 } from '../../../../../common/elasticsearch_fieldnames';
-import { ALL_OPTION_VALUE } from '../../../../../common/agent_configuration/constants';
+import { ALL_OPTION_VALUE } from '../../../../../common/agent_configuration/all_option';
 
 export async function getAllEnvironments({
   serviceName,

diff --git a/...ib/settings/agent_configuration/get_environments/get_existing_environments_for_service.ts b/...ib/settings/agent_configuration/get_environments/get_existing_environments_for_service.ts
@@ -9,7 +9,7 @@ import {
   SERVICE_NAME,
   SERVICE_ENVIRONMENT
 } from '../../../../../common/elasticsearch_fieldnames';
-import { ALL_OPTION_VALUE } from '../../../../../common/agent_configuration/constants';
+import { ALL_OPTION_VALUE } from '../../../../../common/agent_configuration/all_option';
 
 export async function getExistingEnvironmentsForService({
   serviceName,

diff --git a/x-pack/plugins/apm/server/lib/settings/agent_configuration/get_service_names.ts b/x-pack/plugins/apm/server/lib/settings/agent_configuration/get_service_names.ts
@@ -10,7 +10,7 @@ import {
   PROCESSOR_EVENT,
   SERVICE_NAME
 } from '../../../../common/elasticsearch_fieldnames';
-import { ALL_OPTION_VALUE } from '../../../../common/agent_configuration/constants';
+import { ALL_OPTION_VALUE } from '../../../../common/agent_configuration/all_option';
 
 export type AgentConfigurationServicesAPIResponse = PromiseReturnType<
   typeof getServiceNames

diff --git a/x-pack/test/api_integration/apis/apm/agent_configuration.ts b/x-pack/test/api_integration/apis/apm/agent_configuration.ts
@@ -5,7 +5,7 @@
  */
 
 import expect from '@kbn/expect';
-import { AgentConfigurationIntake } from '../../../../plugins/apm/common/runtime_types/agent_configuration/configuration_types';
+import { AgentConfigurationIntake } from '../../../../plugins/apm/common/agent_configuration/configuration_types';
 import { FtrProviderContext } from '../../ftr_provider_context';
 
 export default function agentConfigurationTests({ getService }: FtrProviderContext) {