Skip to content

Commit

Permalink
[Synthetics] Monitor Add/Edit - Fix form error states (#156626)
Browse files Browse the repository at this point in the history
- Connects form validation states with react-hoom-form's state (which
was the reason of the reported bug that the error state was only
reflected in euiForm and not the react-hook-form)
- Changes `reValidateMode` from `onChange` to `onSubmit` and manually
invalidates the form when fields change so that dependent validations
can be executed reactively e.g. fixes the following state:
<img width="703" alt="Screenshot 2023-05-04 at 00 10 03"
src="https://user-images.githubusercontent.com/2748376/236061192-a9417915-1eca-4cff-b871-b680955ec15a.png">
- Extracts validation and dependencies logic into a hook to simplify
state management
- Fixes the following error where "Response body max bytes" field appear
despite "Index response body" flag being unchecked.
<img width="1205" alt="Screenshot 2023-05-05 at 19 35 27"
src="https://user-images.githubusercontent.com/2748376/236527581-ff62550a-2679-431e-8571-da02a89c0a32.png">
- Fixes a case where updated values from for nested fields (such as
"Response body contains JSON") weren't being passed to monitor inspect
component ("Inspect configuration" flyout on monitor Add/Edit).
  • Loading branch information
awahab07 authored Jul 24, 2023
1 parent d2ac703 commit e3cc4a9
Show file tree
Hide file tree
Showing 22 changed files with 990 additions and 328 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,23 @@ export function syntheticsAppPageProvider({ page, kibanaUrl }: { page: Page; kib
}
},

async selectFrequencyAddEdit({
value,
unit,
}: {
value: number;
unit: 'minute' | 'minutes' | 'hours';
}) {
await page.click(this.byTestId('syntheticsMonitorConfigSchedule'));

const optionLocator = page.locator(`text=Every ${value} ${unit}`);
await optionLocator.evaluate((element: HTMLOptionElement) => {
if (element && element.parentElement) {
(element.parentElement as HTMLSelectElement).selectedIndex = element.index;
}
});
},

async fillFirstMonitorDetails({ url, locations }: { url: string; locations: string[] }) {
await this.fillByTestSubj('urls-input', url);
await page.click(this.byTestId('comboBoxInput'));
Expand Down
43 changes: 43 additions & 0 deletions x-pack/plugins/synthetics/e2e/synthetics/page_objects/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { expect, Page } from '@elastic/synthetics';

export async function isEuiFormFieldInValid(locator: ReturnType<Page['locator']>) {
const elementHandle = await locator.elementHandle();
expect(elementHandle).toBeTruthy();

const classAttribute = (await elementHandle!.asElement().getAttribute('class')) ?? '';
const isAriaInvalid = (await elementHandle!.asElement().getAttribute('aria-invalid')) ?? 'false';

return classAttribute.indexOf('-isInvalid') > -1 || isAriaInvalid === 'true';
}

export async function clearAndType(locator: ReturnType<Page['locator']>, value: string) {
await locator.fill('');
await locator.type(value);
}

export async function typeViaKeyboard(locator: ReturnType<Page['locator']>, value: string) {
await locator.click();
await locator.page().keyboard.type(value);
}

export async function blur(locator: ReturnType<Page['locator']>) {
await locator.evaluate((e) => {
e.blur();
});
}

export async function clickAndBlur(locator: ReturnType<Page['locator']>) {
await locator.click();
await blur(locator);
}

export async function assertShouldNotExist(locator: ReturnType<Page['locator']>) {
await locator.waitFor({ state: 'detached' });
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import React, { useState } from 'react';
import { useFormContext } from 'react-hook-form';
import { useKibana } from '@kbn/kibana-react-plugin/public';
import { enableInspectEsQueries } from '@kbn/observability-plugin/common';
import { useFetcher } from '@kbn/observability-shared-plugin/public';
Expand All @@ -28,10 +27,14 @@ import {
import { ClientPluginsStart } from '../../../../../plugin';
import { useSyntheticsSettingsContext } from '../../../contexts';
import { LoadingState } from '../../monitors_page/overview/overview/monitor_detail_flyout';
import { DataStream, SyntheticsMonitor } from '../../../../../../common/runtime_types';
import { DataStream, MonitorFields } from '../../../../../../common/runtime_types';
import { inspectMonitorAPI, MonitorInspectResponse } from '../../../state/monitor_management/api';

export const MonitorInspectWrapper = () => {
interface InspectorProps {
isValid: boolean;
monitorFields: MonitorFields;
}
export const MonitorInspectWrapper = (props: InspectorProps) => {
const {
services: { uiSettings },
} = useKibana<ClientPluginsStart>();
Expand All @@ -40,10 +43,10 @@ export const MonitorInspectWrapper = () => {

const isInspectorEnabled = uiSettings?.get<boolean>(enableInspectEsQueries);

return isDev || isInspectorEnabled ? <MonitorInspect /> : null;
return isDev || isInspectorEnabled ? <MonitorInspect {...props} /> : null;
};

const MonitorInspect = () => {
const MonitorInspect = ({ isValid, monitorFields }: InspectorProps) => {
const { isDev } = useSyntheticsSettingsContext();

const [hideParams, setHideParams] = useState(() => !isDev);
Expand All @@ -60,13 +63,11 @@ const MonitorInspect = () => {
setIsFlyoutVisible(() => !isFlyoutVisible);
};

const { getValues, formState } = useFormContext();

const { data, loading, error } = useFetcher(() => {
if (isInspecting) {
return inspectMonitorAPI({
hideParams,
monitor: getValues() as SyntheticsMonitor,
monitor: monitorFields,
});
}
}, [isInspecting, hideParams]);
Expand Down Expand Up @@ -121,9 +122,9 @@ const MonitorInspect = () => {
}
return (
<>
<EuiToolTip content={formState.isValid ? FORMATTED_CONFIG_DESCRIPTION : VALID_CONFIG_LABEL}>
<EuiToolTip content={isValid ? FORMATTED_CONFIG_DESCRIPTION : VALID_CONFIG_LABEL}>
<EuiButton
disabled={!formState.isValid}
disabled={!isValid}
data-test-subj="syntheticsMonitorInspectShowFlyoutExampleButton"
onClick={onButtonClick}
iconType="inspect"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
*/

import React from 'react';
import { Controller, FieldErrors, Control } from 'react-hook-form';
import { Controller, Control } from 'react-hook-form';
import { useSelector } from 'react-redux';

import { EuiComboBox, EuiFormRow } from '@elastic/eui';
import { EuiComboBox, EuiComboBoxProps, EuiFormRow } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { ServiceLocation } from '../../../../../../common/runtime_types';
import { formatLocation } from '../../../../../../common/utils/location_formatter';
Expand All @@ -19,28 +19,37 @@ import { SimpleFormData } from '../simple_monitor_form';
import { ConfigKey } from '../../../../../../common/constants/monitor_management';

export const ServiceLocationsField = ({
errors,
control,
onChange,
}: {
errors: FieldErrors;
control: Control<SimpleFormData, any>;
onChange: (locations: ServiceLocation[]) => void;
}) => {
const { locations } = useSelector(selectServiceLocationsState);

const fieldState = control.getFieldState(ConfigKey.LOCATIONS);
const showError = fieldState.isTouched || control._formState.isSubmitted;

return (
<EuiFormRow
fullWidth
label={LOCATIONS_LABEL}
helpText={!errors?.[ConfigKey.LOCATIONS] ? SELECT_ONE_OR_MORE_LOCATIONS_DETAILS : undefined}
isInvalid={!!errors?.[ConfigKey.LOCATIONS]}
error={SELECT_ONE_OR_MORE_LOCATIONS}
helpText={showError && fieldState.invalid ? undefined : SELECT_ONE_OR_MORE_LOCATIONS_DETAILS}
isInvalid={showError && fieldState.invalid}
error={showError ? SELECT_ONE_OR_MORE_LOCATIONS : undefined}
>
<Controller
name={ConfigKey.LOCATIONS}
control={control}
rules={{ required: true }}
rules={{
validate: {
notEmpty: (value: ServiceLocation[]) => {
return value?.length > 0 ? true : SELECT_ONE_OR_MORE_LOCATIONS;
},
},
}}
render={({ field }) => (
<EuiComboBox
<ComboBoxWithRef
fullWidth
aria-label={SELECT_ONE_OR_MORE_LOCATIONS}
options={locations.map((location) => ({
Expand All @@ -51,17 +60,37 @@ export const ServiceLocationsField = ({
isClearable={true}
data-test-subj="syntheticsServiceLocations"
{...field}
onChange={(selectedOptions) =>
field.onChange(selectedOptions.map((loc) => formatLocation(loc as ServiceLocation)))
}
isInvalid={!!errors?.[ConfigKey.LOCATIONS]}
onChange={async (selectedOptions) => {
const updatedLocations = selectedOptions.map((loc) =>
formatLocation(loc as ServiceLocation)
);
field.onChange(updatedLocations);
onChange(updatedLocations as ServiceLocation[]);
}}
/>
)}
/>
</EuiFormRow>
);
};

const ComboBoxWithRef = React.forwardRef<HTMLInputElement, EuiComboBoxProps<ServiceLocation>>(
(props, ref) => (
<EuiComboBox
{...props}
inputRef={(element) => {
if (ref) {
if (typeof ref === 'function') {
ref(element);
} else {
ref.current = element;
}
}
}}
/>
)
);

const SELECT_ONE_OR_MORE_LOCATIONS = i18n.translate(
'xpack.synthetics.monitorManagement.selectOneOrMoreLocations',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import React, { useState } from 'react';
import { i18n } from '@kbn/i18n';
import { useSimpleMonitor } from './use_simple_monitor';
import { ServiceLocationsField } from './form_fields/service_locations';
import { ConfigKey, ServiceLocations } from '../../../../../common/runtime_types';
import { ConfigKey, ServiceLocation, ServiceLocations } from '../../../../../common/runtime_types';
import { useCanEditSynthetics } from '../../../../hooks/use_capabilities';
import { useFormWrapped } from '../../../../hooks/use_form_wrapped';
import { NoPermissionsTooltip } from '../common/components/permissions';
Expand All @@ -33,10 +33,12 @@ export const SimpleMonitorForm = () => {
control,
register,
handleSubmit,
formState: { errors, isValid, isSubmitted },
formState: { isValid, isSubmitted },
getFieldState,
trigger,
} = useFormWrapped({
mode: 'onSubmit',
reValidateMode: 'onChange',
reValidateMode: 'onSubmit',
shouldFocusError: true,
defaultValues: { urls: '', locations: [] as ServiceLocations },
});
Expand All @@ -51,7 +53,8 @@ export const SimpleMonitorForm = () => {

const canEditSynthetics = useCanEditSynthetics();

const hasURLError = !!errors?.[ConfigKey.URLS];
const urlFieldState = getFieldState(ConfigKey.URLS);
const urlError = isSubmitted || urlFieldState.isTouched ? urlFieldState.error : undefined;

return (
<EuiForm
Expand All @@ -63,20 +66,29 @@ export const SimpleMonitorForm = () => {
<EuiFormRow
fullWidth
label={WEBSITE_URL_LABEL}
helpText={!hasURLError ? WEBSITE_URL_HELP_TEXT : ''}
isInvalid={!!errors?.[ConfigKey.URLS]}
error={hasURLError ? URL_REQUIRED_LABEL : undefined}
helpText={urlError ? undefined : WEBSITE_URL_HELP_TEXT}
isInvalid={!!urlError}
error={urlError?.message}
>
<EuiFieldText
fullWidth
{...register(ConfigKey.URLS, { required: true })}
isInvalid={!!errors?.[ConfigKey.URLS]}
{...register(ConfigKey.URLS, {
validate: {
notEmpty: (value: string) => (!Boolean(value.trim()) ? URL_REQUIRED_LABEL : true),
},
})}
isInvalid={!!urlError}
data-test-subj={`${ConfigKey.URLS}-input`}
tabIndex={0}
/>
</EuiFormRow>
<EuiSpacer />
<ServiceLocationsField errors={errors} control={control} />
<ServiceLocationsField
control={control}
onChange={async (_locations: ServiceLocation[]) => {
await trigger?.();
}}
/>
<EuiSpacer size="m" />
<EuiFlexGroup justifyContent="flexEnd">
<EuiFlexItem grow={false}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,14 @@
import React, { useMemo } from 'react';
import { i18n } from '@kbn/i18n';
import { EuiAccordion, EuiDescribedFormGroup, EuiPanel, EuiSpacer } from '@elastic/eui';
import { useFormContext, FieldError } from 'react-hook-form';
import { useFormContext } from 'react-hook-form';
import styled from 'styled-components';
import { FORM_CONFIG } from '../form/form_config';
import { Field } from '../form/field';
import { ConfigKey, FormMonitorType } from '../types';

export const AdvancedConfig = ({ readOnly }: { readOnly: boolean }) => {
const {
watch,
formState: { errors },
} = useFormContext();
const { watch } = useFormContext();
const [type]: [FormMonitorType] = watch([ConfigKey.FORM_MONITOR_TYPE]);

const formConfig = useMemo(() => {
Expand All @@ -36,7 +33,7 @@ export const AdvancedConfig = ({ readOnly }: { readOnly: boolean }) => {
<EuiSpacer />
{formConfig.advanced?.map((configGroup) => {
return (
<DescripedFormGroup
<DescribedFormGroup
description={configGroup.description}
title={<h4>{configGroup.title}</h4>}
fullWidth
Expand All @@ -46,23 +43,17 @@ export const AdvancedConfig = ({ readOnly }: { readOnly: boolean }) => {
style={{ flexWrap: 'wrap' }}
>
{configGroup.components.map((field) => {
return (
<Field
{...field}
key={field.fieldKey}
fieldError={errors[field.fieldKey] as FieldError}
/>
);
return <Field {...field} key={field.fieldKey} />;
})}
</DescripedFormGroup>
</DescribedFormGroup>
);
})}
</EuiAccordion>
</EuiPanel>
) : null;
};

const DescripedFormGroup = styled(EuiDescribedFormGroup)`
const DescribedFormGroup = styled(EuiDescribedFormGroup)`
> div.euiFlexGroup {
flex-wrap: wrap;
}
Expand Down
Loading

0 comments on commit e3cc4a9

Please sign in to comment.