From 59f4dd09651af23e4ce5d68eaa86480f6ecf337b Mon Sep 17 00:00:00 2001
From: Elena Stoeva <59341489+ElenaStoeva@users.noreply.github.com>
Date: Thu, 1 Feb 2024 16:52:19 +0000
Subject: [PATCH] [APM] Add validations to general settings (#175874)

## Summary

In https://github.com/elastic/kibana/pull/174712, we integrated the new
settings field component into APM General settings. The new setting
field supports validation on the user input, based on the `schema` of
the given setting.

Since some of the APM General settings have `schema` with specified
restrictions, which were not enforced in the UI before, this PR adds
schema validation so that they are enforced.

Note that this validation adds some performance overhead as it sends a
request to the server (to the `internal/kibana/settings/validate`
endpoint) for every user input change. It's up to the team to decide
whether you think that is okay for the app and whether validation for
these settings is really beneficial to have.

**Settings with `schema` restrictions:**


[apmServiceGroupMaxNumberOfServices](https://github.com/elastic/kibana/blob/b4d93fc145c3c09eb1096c610b7cd736f19f6a3a/x-pack/plugins/observability/server/ui_settings.ts#L189)
(currently input value `0` is not validated but a fix is coming with
https://github.com/elastic/kibana/pull/175957):
<img width="1071" alt="Screenshot 2024-01-31 at 08 57 58"
src="https://github.com/elastic/kibana/assets/59341489/35d9a4fc-4641-4603-97f9-9fbc1a76a778">



[observability:apmAWSLambdaPriceFactor](https://github.com/elastic/kibana/blob/b4d93fc145c3c09eb1096c610b7cd736f19f6a3a/x-pack/plugins/observability/server/ui_settings.ts#L315):
<img width="1071" alt="Screenshot 2024-01-31 at 08 58 39"
src="https://github.com/elastic/kibana/assets/59341489/810948f5-134a-49ec-8d6e-b15ea7896624">



[observability:apmAWSLambdaRequestCostPerMillion](https://github.com/elastic/kibana/blob/b4d93fc145c3c09eb1096c610b7cd736f19f6a3a/x-pack/plugins/observability/server/ui_settings.ts#L326):
<img width="1071" alt="Screenshot 2024-01-31 at 08 59 36"
src="https://github.com/elastic/kibana/assets/59341489/269c0a3c-d257-43b7-bcf7-2e10dffdb0b3">



**Bottom bar when there are invalid changes:**
<img width="1084" alt="Screenshot 2024-01-31 at 09 04 57"
src="https://github.com/elastic/kibana/assets/59341489/1d6a44a2-2659-421b-84b4-44fc693a645b">
---
 .../app/settings/general_settings/index.tsx       | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

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
index 114d080fb3a34..5eae7486a9290 100644
--- 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
@@ -30,7 +30,6 @@ import {
   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 { useApmFeatureFlag } from '../../../../hooks/use_apm_feature_flag';
 import { ApmFeatureFlagName } from '../../../../../common/apm_feature_flags';
 import { useApmPluginContext } from '../../../../context/apm_plugin/use_apm_plugin_context';
@@ -66,7 +65,7 @@ function getApmSettingsKeys(isProfilingIntegrationEnabled: boolean) {
 
 export function GeneralSettings() {
   const trackApmEvent = useUiTracker({ app: 'apm' });
-  const { docLinks, notifications } = useApmPluginContext().core;
+  const { docLinks, notifications, settings } = useApmPluginContext().core;
   const isProfilingIntegrationEnabled = useApmFeatureFlag(
     ApmFeatureFlagName.ProfilingIntegrationAvailable
   );
@@ -101,11 +100,9 @@ export function GeneralSettings() {
     }
   }
 
-  // We don't validate the user input on these settings
-  const settingsValidationResponse: ValueValidation = {
-    successfulValidation: true,
-    valid: true,
-  };
+  const hasInvalidChanges = Object.values(unsavedChanges).some(
+    ({ isInvalid }) => isInvalid
+  );
 
   return (
     <>
@@ -118,7 +115,8 @@ export function GeneralSettings() {
               links: docLinks.links.management,
               showDanger: (message: string) =>
                 notifications.toasts.addDanger(message),
-              validateChange: async () => settingsValidationResponse,
+              validateChange: (key: string, value: any) =>
+                settings.client.validateValue(key, value),
             }}
           >
             <FieldRow
@@ -140,6 +138,7 @@ export function GeneralSettings() {
           })}
           unsavedChangesCount={Object.keys(unsavedChanges).length}
           appTestSubj="apm"
+          areChangesInvalid={hasInvalidChanges}
         />
       )}
     </>