[Infra] [APM] Use bottom bar action buttons in infra settings (#175024)

Closes #169210 ## Summary This PR adds bottom bar action buttons to the infra settings page to make it consistent with the APM settings page. The bottom bar appears after a change is made (and won't appear if the value is the same as it was before - same as APM). | APM | Infra Now | | ---- | ---------- | | ![image](https://github.com/elastic/kibana/assets/14139027/8400cc75-c3d3-40c6-adf7-040779598c7d) | ![image](https://github.com/elastic/kibana/assets/14139027/60f49a78-f733-43f4-a1c8-9b9c337b49f6) | The infra page validation is a bit different so I used the same logic as in advance settings - if the field is invalid disable the button and show a tooltip: <img width="1920" alt="image" src="https://github.com/elastic/kibana/assets/14139027/9e73e77f-f572-4dbe-a8cc-9aa7bf5ec370"> ## Testing - Open the infra settings page - Check if the bottom bar is displayed and the save/discard functionality works as expected - Check infra valdation - Verify that the APM settings page works as expected https://github.com/elastic/kibana/assets/14139027/6804e24b-2b33-456d-be8f-cb17cc044759
elastic · Jan 22, 2024 · 7a503e7 · 7a503e7
1 parent 89504a8
commit 7a503e7
Show file tree

Hide file tree

Showing 13 changed files with 440 additions and 235 deletions.
diff --git a/...ings/agent_configurations/agent_configuration_create_edit/settings_page/settings_page.tsx b/...ings/agent_configurations/agent_configuration_create_edit/settings_page/settings_page.tsx
@@ -17,7 +17,10 @@ import {
   EuiStat,
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
-import { useUiTracker } from '@kbn/observability-shared-plugin/public';
+import {
+  BottomBarActions,
+  useUiTracker,
+} from '@kbn/observability-shared-plugin/public';
 import React, { useMemo, useState } from 'react';
 import { useHistory } from 'react-router-dom';
 import { getOptionLabel } from '../../../../../../../common/agent_configuration/all_option';
@@ -30,7 +33,6 @@ import {
 import { AgentName } from '../../../../../../../typings/es_schemas/ui/fields/agent';
 import { useApmPluginContext } from '../../../../../../context/apm_plugin/use_apm_plugin_context';
 import { FETCH_STATUS } from '../../../../../../hooks/use_fetcher';
-import { BottomBarActions } from '../../../bottom_bar_actions';
 import { saveConfig } from './save_config';
 import { SettingFormRow } from './setting_form_row';
 
@@ -200,6 +202,7 @@ export function SettingsPage({
             { defaultMessage: 'Save configuration' }
           )}
           unsavedChangesCount={unsavedChangesCount}
+          appTestSubj="apm"
         />
       )}
     </>

diff --git a/x-pack/plugins/apm/public/components/app/settings/general_settings/index.tsx b/x-pack/plugins/apm/public/components/app/settings/general_settings/index.tsx
@@ -25,13 +25,13 @@ import {
 import { isEmpty } from 'lodash';
 import React from 'react';
 import {
+  BottomBarActions,
   useEditableSettings,
   useUiTracker,
 } from '@kbn/observability-shared-plugin/public';
 import { FieldRowProvider } from '@kbn/management-settings-components-field-row';
 import { ValueValidation } from '@kbn/core-ui-settings-browser/src/types';
 import { useApmPluginContext } from '../../../../context/apm_plugin/use_apm_plugin_context';
-import { BottomBarActions } from '../bottom_bar_actions';
 
 const LazyFieldRow = React.lazy(async () => ({
   default: (await import('@kbn/management-settings-components-field-row'))
@@ -126,6 +126,7 @@ export function GeneralSettings() {
             defaultMessage: 'Save changes',
           })}
           unsavedChangesCount={Object.keys(unsavedChanges).length}
+          appTestSubj="apm"
         />
       )}
     </>

diff --git a/x-pack/plugins/infra/public/pages/metrics/settings/indices_configuration_form_state.ts b/x-pack/plugins/infra/public/pages/metrics/settings/indices_configuration_form_state.ts
@@ -99,16 +99,34 @@ export const useIndicesConfigurationFormState = ({
 
   const isFormValid = useMemo(() => errors.length <= 0, [errors]);
 
-  const isFormDirty = useMemo(() => Object.keys(formStateChanges).length > 0, [formStateChanges]);
+  const getUnsavedChanges = ({
+    changedConfig,
+    existingConfig,
+  }: {
+    changedConfig: FormStateChanges;
+    existingConfig?: FormState;
+  }) => {
+    return Object.fromEntries(
+      Object.entries(changedConfig).filter(([key, value]) => {
+        const existingValue = existingConfig?.[key as keyof FormState];
+        // don't highlight changes that were added and removed
+        if (value === '' && existingValue == null) {
+          return false;
+        }
+
+        return existingValue !== value;
+      })
+    );
+  };
 
   return {
     errors,
     fieldProps,
     formState,
     formStateChanges,
-    isFormDirty,
     isFormValid,
     resetForm,
+    getUnsavedChanges,
   };
 };
 

diff --git a/x-pack/plugins/infra/public/pages/metrics/settings/source_configuration_form_state.tsx b/x-pack/plugins/infra/public/pages/metrics/settings/source_configuration_form_state.tsx
@@ -12,19 +12,21 @@ import { useIndicesConfigurationFormState } from './indices_configuration_form_s
 export const useSourceConfigurationFormState = (
   configuration?: MetricsSourceConfigurationProperties
 ) => {
+  const initialFormState = useMemo(
+    () =>
+      configuration
+        ? {
+            name: configuration.name,
+            description: configuration.description,
+            metricAlias: configuration.metricAlias,
+            anomalyThreshold: configuration.anomalyThreshold,
+          }
+        : undefined,
+    [configuration]
+  );
+
   const indicesConfigurationFormState = useIndicesConfigurationFormState({
-    initialFormState: useMemo(
-      () =>
-        configuration
-          ? {
-              name: configuration.name,
-              description: configuration.description,
-              metricAlias: configuration.metricAlias,
-              anomalyThreshold: configuration.anomalyThreshold,
-            }
-          : undefined,
-      [configuration]
-    ),
+    initialFormState,
   });
 
   const errors = useMemo(
@@ -36,11 +38,6 @@ export const useSourceConfigurationFormState = (
     indicesConfigurationFormState.resetForm();
   }, [indicesConfigurationFormState]);
 
-  const isFormDirty = useMemo(
-    () => indicesConfigurationFormState.isFormDirty,
-    [indicesConfigurationFormState.isFormDirty]
-  );
-
   const isFormValid = useMemo(
     () => indicesConfigurationFormState.isFormValid,
     [indicesConfigurationFormState.isFormValid]
@@ -66,13 +63,20 @@ export const useSourceConfigurationFormState = (
     [indicesConfigurationFormState.formStateChanges]
   );
 
+  const getUnsavedChanges = useCallback(() => {
+    return indicesConfigurationFormState.getUnsavedChanges({
+      changedConfig: formState,
+      existingConfig: initialFormState,
+    });
+  }, [formState, indicesConfigurationFormState, initialFormState]);
+
   return {
     errors,
     formState,
     formStateChanges,
-    isFormDirty,
     isFormValid,
     indicesConfigurationProps: indicesConfigurationFormState.fieldProps,
     resetForm,
+    getUnsavedChanges,
   };
 };
diff --git a/x-pack/plugins/infra/public/pages/metrics/settings/source_configuration_settings.tsx b/x-pack/plugins/infra/public/pages/metrics/settings/source_configuration_settings.tsx
@@ -5,18 +5,14 @@
  * 2.0.
  */
 
-import {
-  EuiButton,
-  EuiCallOut,
-  EuiFlexGroup,
-  EuiFlexItem,
-  EuiPanel,
-  EuiSpacer,
-} from '@elastic/eui';
+import { EuiCallOut, EuiFlexGroup, EuiFlexItem, EuiPanel, EuiSpacer } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
-import { FormattedMessage } from '@kbn/i18n-react';
 import React, { useCallback } from 'react';
-import { Prompt, useEditableSettings } from '@kbn/observability-shared-plugin/public';
+import {
+  BottomBarActions,
+  Prompt,
+  useEditableSettings,
+} from '@kbn/observability-shared-plugin/public';
 import {
   enableInfrastructureHostsView,
   enableInfrastructureProfilingIntegration,
@@ -59,11 +55,11 @@ export const SourceConfigurationSettings = ({
     indicesConfigurationProps,
     errors,
     resetForm,
-    isFormDirty,
     isFormValid,
     formState,
     formStateChanges,
-  } = useSourceConfigurationFormState(source && source.configuration);
+    getUnsavedChanges,
+  } = useSourceConfigurationFormState(source?.configuration);
   const infraUiSettings = useEditableSettings('infra_metrics', [
     enableInfrastructureHostsView,
     enableInfrastructureProfilingIntegration,
@@ -92,7 +88,12 @@ export const SourceConfigurationSettings = ({
     formState,
   ]);
 
-  const hasUnsavedChanges = isFormDirty || Object.keys(infraUiSettings.unsavedChanges).length > 0;
+  const unsavedChangesCount = Object.keys(getUnsavedChanges()).length;
+  const infraUiSettingsUnsavedChangesCount = Object.keys(infraUiSettings.unsavedChanges).length;
+  // Count changes from the feature section settings and general infra settings
+  const unsavedFormChangesCount = infraUiSettingsUnsavedChangesCount + unsavedChangesCount;
+
+  const isFormDirty = infraUiSettingsUnsavedChangesCount > 0 || unsavedChangesCount > 0;
 
   const isWriteable = shouldAllowEdit && (!Boolean(source) || source?.origin !== 'internal');
 
@@ -171,54 +172,18 @@ export const SourceConfigurationSettings = ({
       <EuiFlexGroup>
         {isWriteable && (
           <EuiFlexItem>
-            {isLoading || infraUiSettings.isSaving ? (
-              <EuiFlexGroup justifyContent="flexEnd">
-                <EuiFlexItem grow={false}>
-                  <EuiButton
-                    data-test-subj="infraSourceConfigurationSettingsLoadingButton"
-                    color="primary"
-                    isLoading
-                    fill
-                  >
-                    {i18n.translate('xpack.infra.sourceConfiguration.loadingButtonLabel', {
-                      defaultMessage: 'Loading',
-                    })}
-                  </EuiButton>
-                </EuiFlexItem>
-              </EuiFlexGroup>
-            ) : (
-              <>
-                <EuiFlexGroup justifyContent="flexEnd">
-                  <EuiFlexItem grow={false}>
-                    <EuiButton
-                      data-test-subj="discardSettingsButton"
-                      color="danger"
-                      iconType="cross"
-                      isDisabled={!hasUnsavedChanges}
-                      onClick={resetAllUnsavedChanges}
-                    >
-                      <FormattedMessage
-                        id="xpack.infra.sourceConfiguration.discardSettingsButtonLabel"
-                        defaultMessage="Discard"
-                      />
-                    </EuiButton>
-                  </EuiFlexItem>
-                  <EuiFlexItem grow={false}>
-                    <EuiButton
-                      data-test-subj="applySettingsButton"
-                      color="primary"
-                      isDisabled={!hasUnsavedChanges || !isFormValid}
-                      fill
-                      onClick={persistUpdates}
-                    >
-                      <FormattedMessage
-                        id="xpack.infra.sourceConfiguration.applySettingsButtonLabel"
-                        defaultMessage="Apply"
-                      />
-                    </EuiButton>
-                  </EuiFlexItem>
-                </EuiFlexGroup>
-              </>
+            {isFormDirty && (
+              <BottomBarActions
+                areChangesInvalid={!isFormValid}
+                isLoading={infraUiSettings.isSaving}
+                onDiscardChanges={resetAllUnsavedChanges}
+                onSave={persistUpdates}
+                saveLabel={i18n.translate('xpack.infra.sourceConfiguration.saveButton', {
+                  defaultMessage: 'Save changes',
+                })}
+                unsavedChangesCount={unsavedFormChangesCount}
+                appTestSubj="infra"
+              />
             )}
           </EuiFlexItem>
         )}

diff --git a/...app/settings/bottom_bar_actions/index.tsx → ...bottom_bar_actions/bottom_bar_actions.tsx b/...app/settings/bottom_bar_actions/index.tsx → ...bottom_bar_actions/bottom_bar_actions.tsx
@@ -12,6 +12,7 @@ import {
   EuiFlexItem,
   EuiHealth,
   EuiText,
+  EuiToolTip,
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import React from 'react';
@@ -22,15 +23,19 @@ interface Props {
   onDiscardChanges: () => void;
   onSave: () => void;
   saveLabel: string;
+  appTestSubj: string;
+  areChangesInvalid?: boolean;
 }
 
-export function BottomBarActions({
+export const BottomBarActions = ({
   isLoading,
   onDiscardChanges,
   onSave,
   unsavedChangesCount,
   saveLabel,
-}: Props) {
+  appTestSubj,
+  areChangesInvalid = false,
+}: Props) => {
   return (
     <EuiBottomBar paddingSize="s">
       <EuiFlexGroup justifyContent="spaceBetween" alignItems="center">
@@ -43,7 +48,7 @@ export function BottomBarActions({
         >
           <EuiHealth color="warning" />
           <EuiText>
-            {i18n.translate('xpack.apm.bottomBarActions.unsavedChanges', {
+            {i18n.translate('xpack.observabilityShared.bottomBarActions.unsavedChanges', {
               defaultMessage:
                 '{unsavedChangesCount, plural, =0{0 unsaved changes} one {1 unsaved change} other {# unsaved changes}} ',
               values: { unsavedChangesCount },
@@ -54,33 +59,40 @@ export function BottomBarActions({
           <EuiFlexGroup justifyContent="flexEnd">
             <EuiFlexItem grow={false}>
               <EuiButtonEmpty
-                data-test-subj="apmBottomBarActionsDiscardChangesButton"
+                data-test-subj={`${appTestSubj}BottomBarActionsDiscardChangesButton`}
                 color="text"
                 onClick={onDiscardChanges}
               >
-                {i18n.translate(
-                  'xpack.apm.bottomBarActions.discardChangesButton',
-                  {
-                    defaultMessage: 'Discard changes',
-                  }
-                )}
+                {i18n.translate('xpack.observabilityShared.bottomBarActions.discardChangesButton', {
+                  defaultMessage: 'Discard changes',
+                })}
               </EuiButtonEmpty>
             </EuiFlexItem>
             <EuiFlexItem grow={false}>
-              <EuiButton
-                data-test-subj="apmBottomBarActionsButton"
-                onClick={onSave}
-                fill
-                isLoading={isLoading}
-                color="success"
-                iconType="check"
+              <EuiToolTip
+                content={
+                  areChangesInvalid &&
+                  i18n.translate('xpack.observabilityShared.saveButtonTooltipWithInvalidChanges', {
+                    defaultMessage: 'Fix invalid settings before saving.',
+                  })
+                }
               >
-                {saveLabel}
-              </EuiButton>
+                <EuiButton
+                  disabled={areChangesInvalid}
+                  data-test-subj={`${appTestSubj}BottomBarActionsButton`}
+                  onClick={onSave}
+                  fill
+                  isLoading={isLoading}
+                  color="success"
+                  iconType="check"
+                >
+                  {saveLabel}
+                </EuiButton>
+              </EuiToolTip>
             </EuiFlexItem>
           </EuiFlexGroup>
         </EuiFlexItem>
       </EuiFlexGroup>
     </EuiBottomBar>
   );
-}
+};
diff --git a/x-pack/plugins/observability_shared/public/index.ts b/x-pack/plugins/observability_shared/public/index.ts
@@ -93,3 +93,4 @@ export {
 
 export { ProfilingEmptyState } from './components/profiling/profiling_empty_state';
 export { FeatureFeedbackButton } from './components/feature_feedback_button/feature_feedback_button';
+export { BottomBarActions } from './components/bottom_bar_actions/bottom_bar_actions';
Original file line number	Diff line number	Diff line change
Expand Up		@@ -93,3 +93,4 @@ export {

		export { ProfilingEmptyState } from './components/profiling/profiling_empty_state';
		export { FeatureFeedbackButton } from './components/feature_feedback_button/feature_feedback_button';
		export { BottomBarActions } from './components/bottom_bar_actions/bottom_bar_actions';