[Fleet] Fix Fleet settings and HostInput error handling (#109418) (#109636)

* fix: HostInput error placement onChange

* fix: check for duplicates in hosts

* fix: validate all sections

* fix: error swapping

* fix: yaml placeholder position and styling

* test: add unit tests for delete behaviour

* tidy: use generic for reduce typing

Co-authored-by: Mark Hopkin <mark.hopkin@elastic.co>

x-pack/plugins/fleet/public/components/settings_flyout/hosts_input.test.tsx

@@ -12,13 +12,17 @@ import { createFleetTestRendererMock } from '../../mock';
 
 import { HostsInput } from './hosts_input';
 
-function renderInput(value = ['http://host1.com']) {
+function renderInput(
+  value = ['http://host1.com'],
+  errors: Array<{ message: string; index?: number }> = [],
+  mockOnChange: (...args: any[]) => void = jest.fn()
+) {
   const renderer = createFleetTestRendererMock();
-  const mockOnChange = jest.fn();
 
   const utils = renderer.render(
     <HostsInput
       value={value}
+      errors={errors}
       label="HOST LABEL"
       helpText="HELP TEXT"
       id="ID"
@@ -75,3 +79,65 @@ test('it should render an input if there is not hosts', async () => {
   fireEvent.change(inputEl, { target: { value: 'http://newhost.com' } });
   expect(mockOnChange).toHaveBeenCalledWith(['http://newhost.com']);
 });
+
+test('Should display single indexed error message', async () => {
+  const { utils } = renderInput(['bad host'], [{ message: 'Invalid URL', index: 0 }]);
+  const inputEl = await utils.findByText('Invalid URL');
+  expect(inputEl).toBeDefined();
+});
+
+test('Should display errors in order', async () => {
+  const { utils } = renderInput(
+    ['bad host 1', 'bad host 2', 'bad host 3'],
+    [
+      { message: 'Error 1', index: 0 },
+      { message: 'Error 2', index: 1 },
+      { message: 'Error 3', index: 2 },
+    ]
+  );
+  await act(async () => {
+    const errors = await utils.queryAllByText(/Error [1-3]/);
+    expect(errors[0]).toHaveTextContent('Error 1');
+    expect(errors[1]).toHaveTextContent('Error 2');
+    expect(errors[2]).toHaveTextContent('Error 3');
+  });
+});
+
+test('Should remove error when item deleted', async () => {
+  const mockOnChange = jest.fn();
+  const errors = [
+    { message: 'Error 1', index: 0 },
+    { message: 'Error 2', index: 1 },
+    { message: 'Error 3', index: 2 },
+  ];
+
+  const { utils } = renderInput(['bad host 1', 'bad host 2', 'bad host 3'], errors, mockOnChange);
+
+  mockOnChange.mockImplementation((newValue) => {
+    utils.rerender(
+      <HostsInput
+        value={newValue}
+        errors={errors}
+        label="HOST LABEL"
+        helpText="HELP TEXT"
+        id="ID"
+        onChange={mockOnChange}
+      />
+    );
+  });
+
+  await act(async () => {
+    const deleteRowButtons = await utils.container.querySelectorAll('[aria-label="Delete host"]');
+    if (deleteRowButtons.length !== 3) {
+      throw new Error('Delete host buttons not found');
+    }
+
+    fireEvent.click(deleteRowButtons[1]);
+    expect(mockOnChange).toHaveBeenCalled();
+
+    const renderedErrors = await utils.queryAllByText(/Error [1-3]/);
+    expect(renderedErrors).toHaveLength(2);
+    expect(renderedErrors[0]).toHaveTextContent('Error 1');
+    expect(renderedErrors[1]).toHaveTextContent('Error 3');
+  });
+});

x-pack/plugins/fleet/public/components/settings_flyout/hosts_input.tsx

@@ -158,38 +158,11 @@ export const HostsInput: FunctionComponent<Props> = ({
     [value, onChange]
   );
 
-  const onDelete = useCallback(
-    (idx: number) => {
-      onChange([...value.slice(0, idx), ...value.slice(idx + 1)]);
-    },
-    [value, onChange]
-  );
-
-  const addRowHandler = useCallback(() => {
-    setAutoFocus(true);
-    onChange([...value, '']);
-  }, [value, onChange]);
-
-  const onDragEndHandler = useCallback(
-    ({ source, destination }) => {
-      if (source && destination) {
-        const items = euiDragDropReorder(value, source.index, destination.index);
-
-        onChange(items);
-      }
-    },
-    [value, onChange]
-  );
-
-  const globalErrors = useMemo(() => {
-    return errors && errors.filter((err) => err.index === undefined).map(({ message }) => message);
-  }, [errors]);
-
   const indexedErrors = useMemo(() => {
     if (!errors) {
       return [];
     }
-    return errors.reduce((acc, err) => {
+    return errors.reduce<string[][]>((acc, err) => {
       if (err.index === undefined) {
         return acc;
       }
@@ -201,7 +174,37 @@ export const HostsInput: FunctionComponent<Props> = ({
       acc[err.index].push(err.message);
 
       return acc;
-    }, [] as string[][]);
+    }, []);
+  }, [errors]);
+
+  const onDelete = useCallback(
+    (idx: number) => {
+      indexedErrors.splice(idx, 1);
+      onChange([...value.slice(0, idx), ...value.slice(idx + 1)]);
+    },
+    [value, onChange, indexedErrors]
+  );
+
+  const addRowHandler = useCallback(() => {
+    setAutoFocus(true);
+    onChange([...value, '']);
+  }, [value, onChange]);
+
+  const onDragEndHandler = useCallback(
+    ({ source, destination }) => {
+      if (source && destination) {
+        const items = euiDragDropReorder(value, source.index, destination.index);
+        const sourceErrors = indexedErrors[source.index];
+        indexedErrors.splice(source.index, 1);
+        indexedErrors.splice(destination.index, 0, sourceErrors);
+        onChange(items);
+      }
+    },
+    [value, onChange, indexedErrors]
+  );
+
+  const globalErrors = useMemo(() => {
+    return errors && errors.filter((err) => err.index === undefined).map(({ message }) => message);
   }, [errors]);
 
   const isSortable = rows.length > 1;

x-pack/plugins/fleet/public/components/settings_flyout/index.tsx

@@ -58,9 +58,11 @@ const CodeEditorPlaceholder = styled(EuiTextColor).attrs((props) => ({
 }))`
   position: absolute;
   top: 0;
-  right: 0;
+  left: 0;
   // Matches monaco editor
   font-family: Menlo, Monaco, 'Courier New', monospace;
+  font-size: 12px;
+  line-height: 21px;
   pointer-events: none;
 `;
 
@@ -102,6 +104,7 @@ function useSettingsForm(outputId: string | undefined, onSuccess: () => void) {
     }
 
     const res: Array<{ message: string; index: number }> = [];
+    const hostIndexes: { [key: string]: number[] } = {};
     value.forEach((val, idx) => {
       if (!val.match(URL_REGEX)) {
         res.push({
@@ -111,7 +114,23 @@ function useSettingsForm(outputId: string | undefined, onSuccess: () => void) {
           index: idx,
         });
       }
+      const curIndexes = hostIndexes[val] || [];
+      hostIndexes[val] = [...curIndexes, idx];
     });
+
+    Object.values(hostIndexes)
+      .filter(({ length }) => length > 1)
+      .forEach((indexes) => {
+        indexes.forEach((index) =>
+          res.push({
+            message: i18n.translate('xpack.fleet.settings.fleetServerHostsDuplicateError', {
+              defaultMessage: 'Duplicate URL',
+            }),
+            index,
+          })
+        );
+      });
+
     if (res.length) {
       return res;
     }
@@ -132,6 +151,7 @@ function useSettingsForm(outputId: string | undefined, onSuccess: () => void) {
 
   const elasticsearchUrlInput = useComboInput('esHostsComboxBox', [], (value) => {
     const res: Array<{ message: string; index: number }> = [];
+    const urlIndexes: { [key: string]: number[] } = {};
     value.forEach((val, idx) => {
       if (!val.match(URL_REGEX)) {
         res.push({
@@ -141,7 +161,23 @@ function useSettingsForm(outputId: string | undefined, onSuccess: () => void) {
           index: idx,
         });
       }
+      const curIndexes = urlIndexes[val] || [];
+      urlIndexes[val] = [...curIndexes, idx];
     });
+
+    Object.values(urlIndexes)
+      .filter(({ length }) => length > 1)
+      .forEach((indexes) => {
+        indexes.forEach((index) =>
+          res.push({
+            message: i18n.translate('xpack.fleet.settings.elasticHostDuplicateError', {
+              defaultMessage: 'Duplicate URL',
+            }),
+            index,
+          })
+        );
+      });
+
     if (res.length) {
       return res;
     }
@@ -162,11 +198,11 @@ function useSettingsForm(outputId: string | undefined, onSuccess: () => void) {
   });
 
   const validate = useCallback(() => {
-    if (
-      !fleetServerHostsInput.validate() ||
-      !elasticsearchUrlInput.validate() ||
-      !additionalYamlConfigInput.validate()
-    ) {
+    const fleetServerHostsValid = fleetServerHostsInput.validate();
+    const elasticsearchUrlsValid = elasticsearchUrlInput.validate();
+    const additionalYamlConfigValid = additionalYamlConfigInput.validate();
+
+    if (!fleetServerHostsValid || !elasticsearchUrlsValid || !additionalYamlConfigValid) {
       return false;
     }