Fix validation for entry fields in exception form (#151654)

## Change validation logic for entry exception field.

Close:
[https://github.com/elastic/kibana/issues/143051](https://github.com/elastic/kibana/issues/143051)

Previously we didn't keep a validation state per field which caused a
reset of validation if we still had invalid fields. Or we can have an
invalid state for the form, but we removed the invalid field. You can
see the videos on the ticket above.

## Solution:
Keep validation state per field, like:
```js 
{
   [entry.id]: true,
}
```
This state can keep old fields, which already were removed, this is why
we use the selector to get the actual amount of errors.



https://user-images.githubusercontent.com/7609147/220337447-95c1558c-aa85-43d1-87e8-76370aeaf141.mov

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Khristinin Nikita 2023-02-21 16:59:54 +01:00 committed by GitHub
parent 07d1adfb14
commit d93eaa0109
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 59 additions and 30 deletions

View file

@ -214,7 +214,10 @@ export const AutocompleteFieldMatchComponent: React.FC<AutocompleteFieldMatchPro
if (onError != null) onError(false);
handleSpacesWarning(selectedValue);
}, [selectedField, selectedValue, handleSpacesWarning, onError]);
// Looks like selectedField return new object every time when we for example add "and" entry
// that's why we need to check for name and type here
// Probably we should use some kind of memoization on parent components for entries
}, [selectedField?.name, selectedField?.type, selectedValue, handleSpacesWarning, onError]);
const defaultInput = useMemo((): JSX.Element => {
return (

View file

@ -913,7 +913,7 @@ describe('BuilderEntryItem', () => {
).onBlur();
});
expect(mockSetErrorExists).toHaveBeenCalledWith(true);
expect(mockSetErrorExists).toHaveBeenCalledWith({ '123': true });
});
test('it invokes "setErrorsExist" when invalid value inputted for field value input', async () => {
@ -960,7 +960,7 @@ describe('BuilderEntryItem', () => {
).onSearchChange('hellooo');
});
expect(mockSetErrorExists).toHaveBeenCalledWith(true);
expect(mockSetErrorExists).toHaveBeenCalledWith({ '123': true });
});
test('it invokes "setWarningsExist" when invalid value in field value input', async () => {

View file

@ -60,6 +60,7 @@ import { HttpStart } from '@kbn/core/public';
import { getEmptyValue } from '../../../common/empty_value';
import * as i18n from './translations';
import { EntryFieldError } from './reducer';
const FieldFlexItem = styled(EuiFlexItem)`
overflow: hidden;
@ -81,7 +82,7 @@ export interface EntryItemProps {
) => DataViewBase;
onChange: (arg: BuilderEntry, i: number) => void;
onlyShowListOperators?: boolean;
setErrorsExist: (arg: boolean) => void;
setErrorsExist: (arg: EntryFieldError) => void;
setWarningsExist: (arg: boolean) => void;
isDisabled?: boolean;
operatorsList?: OperatorOption[];
@ -110,9 +111,9 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
const handleError = useCallback(
(err: boolean): void => {
setErrorsExist(err);
setErrorsExist({ [entry.id]: err });
},
[setErrorsExist]
[setErrorsExist, entry.id]
);
const handleWarning = useCallback(
(warn: boolean): void => {

View file

@ -24,6 +24,7 @@ import { DataViewBase } from '@kbn/es-query';
import { BuilderAndBadgeComponent } from './and_badge';
import { BuilderEntryDeleteButtonComponent } from './entry_delete_button';
import { BuilderEntryItem } from './entry_renderer';
import { EntryFieldError } from './reducer';
const MyBeautifulLine = styled(EuiFlexItem)`
&:after {
@ -58,7 +59,7 @@ interface BuilderExceptionListItemProps {
) => DataViewBase;
onDeleteExceptionItem: (item: ExceptionsBuilderExceptionItem, index: number) => void;
onChangeExceptionItem: (item: ExceptionsBuilderExceptionItem, index: number) => void;
setErrorsExist: (arg: boolean) => void;
setErrorsExist: (arg: EntryFieldError) => void;
setWarningsExist: (arg: boolean) => void;
onlyShowListOperators?: boolean;
isDisabled?: boolean;

View file

@ -38,7 +38,8 @@ import { AndOrBadge } from '../and_or_badge';
import { BuilderExceptionListItemComponent } from './exception_item_renderer';
import { BuilderLogicButtons } from './logic_buttons';
import { State, exceptionsBuilderReducer } from './reducer';
import { getTotalErrorExist } from './selectors';
import { EntryFieldError, State, exceptionsBuilderReducer } from './reducer';
const MyInvisibleAndBadge = styled(EuiFlexItem)`
visibility: hidden;
@ -60,7 +61,7 @@ const initialState: State = {
disableAnd: false,
disableNested: false,
disableOr: false,
errorExists: 0,
errors: {},
exceptions: [],
exceptionsToDelete: [],
warningExists: 0,
@ -121,30 +122,30 @@ export const ExceptionBuilderComponent = ({
operatorsList,
allowCustomFieldOptions = false,
}: ExceptionBuilderProps): JSX.Element => {
const [
{
addNested,
andLogicIncluded,
disableAnd,
disableNested,
disableOr,
errorExists,
warningExists,
exceptions,
exceptionsToDelete,
},
dispatch,
] = useReducer(exceptionsBuilderReducer(), {
const [state, dispatch] = useReducer(exceptionsBuilderReducer(), {
...initialState,
disableAnd: isAndDisabled,
disableNested: isNestedDisabled,
disableOr: isOrDisabled,
});
const {
addNested,
andLogicIncluded,
disableAnd,
disableNested,
disableOr,
warningExists,
exceptions,
exceptionsToDelete,
} = state;
const errorExists = getTotalErrorExist(state);
const setErrorsExist = useCallback(
(hasErrors: boolean): void => {
(error: EntryFieldError): void => {
dispatch({
errorExists: hasErrors,
error,
type: 'setErrorsExist',
});
},

View file

@ -16,6 +16,8 @@ import {
export type ViewerModalName = 'addModal' | 'editModal' | null;
export type EntryFieldError = Record<string, boolean>;
export interface State {
disableAnd: boolean;
disableNested: boolean;
@ -24,7 +26,7 @@ export interface State {
addNested: boolean;
exceptions: ExceptionsBuilderExceptionItem[];
exceptionsToDelete: ExceptionListItemSchema[];
errorExists: number;
errors: EntryFieldError;
warningExists: number;
}
@ -56,7 +58,7 @@ export type Action =
}
| {
type: 'setErrorsExist';
errorExists: boolean;
error: EntryFieldError;
}
| {
type: 'setWarningsExist';
@ -125,12 +127,14 @@ export const exceptionsBuilderReducer =
};
}
case 'setErrorsExist': {
const { errorExists } = state;
const errTotal = action.errorExists ? errorExists + 1 : errorExists - 1;
const newErrorsState = {
...state.errors,
...action.error,
};
return {
...state,
errorExists: errTotal < 0 ? 0 : errTotal,
errors: newErrorsState,
};
}
case 'setWarningsExist': {

View file

@ -0,0 +1,19 @@
/*
* 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 type { State } from './reducer';
export const getTotalErrorExist = (state: State): number => {
const { exceptions, errors } = state;
const allEntryIds = exceptions
.map((exception) => exception.entries.map((entry) => entry.id))
.flat();
const errTotal = Object.keys(errors).filter(
(id) => allEntryIds.includes(id) && errors[id]
).length;
return errTotal;
};