Skip to content

Commit

Permalink
[8.16] [EDR Workflows] Error message 'the value already exists&#…
Browse files Browse the repository at this point in the history 
…x27; shown for duplicate values with the 'is one of' operator on Blocklist tab (#196071) (#199992)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[EDR Workflows] Error message 'the value already exists'
shown for duplicate values with the 'is one of' operator on
Blocklist tab (#196071)](#196071)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Konrad
Szwarc","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-23T10:29:32Z","message":"[EDR
Workflows] Error message 'the value already exists' shown for duplicate
values with the 'is one of' operator on Blocklist tab (#196071)\n\nThese
changes ensure that the warning displayed to the user for\r\nduplicate
values remains visible after onBlur. I’ve opted to keep it as\r\na
warning rather than an error since entering a duplicate value
doesn’t\r\nallow the user to “save” it as a selected item in the combo
box—it\r\nremains an active string in the input field. We don’t block
form\r\nsubmission due to this warning; any unselected value in the
combo box is\r\nstripped out. The red border on the combo box is part of
EUI’s behavior\r\nwhen attempting to select a duplicate, and I don’t see
an easy way to\r\nmodify this at the
moment.\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/84b6d8af-02a8-41f3-88dc-892ed408a098","sha":"99a19e762daf08376828032e593a9e88befe6d23","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport
missing","v9.0.0","Team:Defend
Workflows","backport:version","v8.16.1"],"title":"[EDR Workflows] Error
message 'the value already exists' shown for duplicate values with the
'is one of' operator on Blocklist
tab","number":196071,"url":"https://github.com/elastic/kibana/pull/196071","mergeCommit":{"message":"[EDR
Workflows] Error message 'the value already exists' shown for duplicate
values with the 'is one of' operator on Blocklist tab (#196071)\n\nThese
changes ensure that the warning displayed to the user for\r\nduplicate
values remains visible after onBlur. I’ve opted to keep it as\r\na
warning rather than an error since entering a duplicate value
doesn’t\r\nallow the user to “save” it as a selected item in the combo
box—it\r\nremains an active string in the input field. We don’t block
form\r\nsubmission due to this warning; any unselected value in the
combo box is\r\nstripped out. The red border on the combo box is part of
EUI’s behavior\r\nwhen attempting to select a duplicate, and I don’t see
an easy way to\r\nmodify this at the
moment.\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/84b6d8af-02a8-41f3-88dc-892ed408a098","sha":"99a19e762daf08376828032e593a9e88befe6d23"}},"sourceBranch":"main","suggestedTargetBranches":["8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196071","number":196071,"mergeCommit":{"message":"[EDR
Workflows] Error message 'the value already exists' shown for duplicate
values with the 'is one of' operator on Blocklist tab (#196071)\n\nThese
changes ensure that the warning displayed to the user for\r\nduplicate
values remains visible after onBlur. I’ve opted to keep it as\r\na
warning rather than an error since entering a duplicate value
doesn’t\r\nallow the user to “save” it as a selected item in the combo
box—it\r\nremains an active string in the input field. We don’t block
form\r\nsubmission due to this warning; any unselected value in the
combo box is\r\nstripped out. The red border on the combo box is part of
EUI’s behavior\r\nwhen attempting to select a duplicate, and I don’t see
an easy way to\r\nmodify this at the
moment.\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/84b6d8af-02a8-41f3-88dc-892ed408a098","sha":"99a19e762daf08376828032e593a9e88befe6d23"}},{"branch":"8.16","label":"v8.16.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Konrad Szwarc <[email protected]>
  • Loading branch information
@kibanamachine @szwarckonrad
kibanamachine and szwarckonrad authored Nov 13, 2024
1 parent ed68974 commit 4c5d5fa
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 50 deletions.
12 changes: 12 additions & 0 deletions ...curity_solution/public/management/pages/blocklist/view/components/blocklist_form.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,18 @@ describe('blocklist form', () => {
expect(screen.queryByText(ERRORS.INVALID_PATH)).toBeTruthy();
});

it('should prevail duplicate value warning on lost focus', async () => {
const item = createItem({
os_types: [OperatingSystem.WINDOWS],
entries: [createEntry('file.Ext.code_signature', ['valid', 'invalid'])],
});
render(createProps({ item }));
await user.type(screen.getByRole('combobox'), 'invalid{enter}');
expect(screen.queryByText(ERRORS.DUPLICATE_VALUE)).toBeTruthy();
await user.click(screen.getByTestId('blocklist-form-os-select'));
expect(screen.queryByText(ERRORS.DUPLICATE_VALUE)).toBeTruthy();
});

it('should warn if single duplicate value entry', async () => {
const hash = 'C3AB8FF13720E8AD9047DD39466B3C8974E592C2FA383D4A3960714CAEF0C4F2';
const item = createItem({
Expand Down
116 changes: 66 additions & 50 deletions ...ns/security_solution/public/management/pages/blocklist/view/components/blocklist_form.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,8 @@ function isValid(itemValidation: ItemValidation): boolean {
// eslint-disable-next-line react/display-name
export const BlockListForm = memo<ArtifactFormComponentProps>(
({ item, policies, policiesIsLoading, onChange, mode }) => {
const [visited, setVisited] = useState<{ name: boolean; value: boolean }>({
name: false,
value: false,
});
const [nameVisited, setNameVisited] = useState(false);
const [valueVisited, setValueVisited] = useState({ value: false }); // Use object to trigger re-render
const warningsRef = useRef<ItemValidation>({ name: {}, value: {} });
const errorsRef = useRef<ItemValidation>({ name: {}, value: {} });
const [selectedPolicies, setSelectedPolicies] = useState<PolicyData[]>([]);
Expand Down Expand Up @@ -269,57 +267,75 @@ export const BlockListForm = memo<ArtifactFormComponentProps>(
);
}, [displaySingleValueInput]);

const validateValues = useCallback((nextItem: ArtifactFormComponentProps['item']) => {
const os = ((nextItem.os_types ?? [])[0] as OperatingSystem) ?? OperatingSystem.WINDOWS;
const {
field = 'file.hash.*',
type = ListOperatorTypeEnum.MATCH_ANY,
value = [],
} = (nextItem.entries[0] ?? {}) as BlocklistEntry;

// value can be a string when isOperator is selected
const values = Array.isArray(value) ? value : [value].filter(Boolean);

const newValueWarnings: ItemValidationNodes = {};
const newNameErrors: ItemValidationNodes = {};
const newValueErrors: ItemValidationNodes = {};
// error if name empty
if (!nextItem.name.trim()) {
newNameErrors.NAME_REQUIRED = createValidationMessage(ERRORS.NAME_REQUIRED);
}
const validateValues = useCallback(
(nextItem: ArtifactFormComponentProps['item'], cleanState = false) => {
const os = ((nextItem.os_types ?? [])[0] as OperatingSystem) ?? OperatingSystem.WINDOWS;
const {
field = 'file.hash.*',
type = ListOperatorTypeEnum.MATCH_ANY,
value = [],
} = (nextItem.entries[0] ?? {}) as BlocklistEntry;

// value can be a string when isOperator is selected
const values = Array.isArray(value) ? value : [value].filter(Boolean);

const newValueWarnings: ItemValidationNodes = cleanState
? {}
: { ...warningsRef.current.value };
const newNameErrors: ItemValidationNodes = cleanState ? {} : { ...errorsRef.current.name };
const newValueErrors: ItemValidationNodes = cleanState
? {}
: { ...errorsRef.current.value };

// error if name empty
if (!nextItem.name.trim()) {
newNameErrors.NAME_REQUIRED = createValidationMessage(ERRORS.NAME_REQUIRED);
} else {
delete newNameErrors.NAME_REQUIRED;
}

// error if no values
if (!values.length) {
newValueErrors.VALUE_REQUIRED = createValidationMessage(ERRORS.VALUE_REQUIRED);
}
// error if no values
if (!values.length) {
newValueErrors.VALUE_REQUIRED = createValidationMessage(ERRORS.VALUE_REQUIRED);
} else {
delete newValueErrors.VALUE_REQUIRED;
}

// error if invalid hash
if (field === 'file.hash.*' && values.some((v) => !isValidHash(v))) {
newValueErrors.INVALID_HASH = createValidationMessage(ERRORS.INVALID_HASH);
}
// error if invalid hash
if (field === 'file.hash.*' && values.some((v) => !isValidHash(v))) {
newValueErrors.INVALID_HASH = createValidationMessage(ERRORS.INVALID_HASH);
} else {
delete newValueErrors.INVALID_HASH;
}

const isInvalidPath = values.some((v) => !isPathValid({ os, field, type, value: v }));
// warn if invalid path
if (field !== 'file.hash.*' && isInvalidPath) {
newValueWarnings.INVALID_PATH = createValidationMessage(ERRORS.INVALID_PATH);
}
// warn if duplicates
if (values.length !== uniq(values).length) {
newValueWarnings.DUPLICATE_VALUES = createValidationMessage(ERRORS.DUPLICATE_VALUES);
}
const isInvalidPath = values.some((v) => !isPathValid({ os, field, type, value: v }));
// warn if invalid path
if (field !== 'file.hash.*' && isInvalidPath) {
newValueWarnings.INVALID_PATH = createValidationMessage(ERRORS.INVALID_PATH);
} else {
delete newValueWarnings.INVALID_PATH;
}
// warn if duplicates
if (values.length !== uniq(values).length) {
newValueWarnings.DUPLICATE_VALUES = createValidationMessage(ERRORS.DUPLICATE_VALUES);
} else {
delete newValueWarnings.DUPLICATE_VALUES;
}

warningsRef.current = { ...warningsRef.current, value: newValueWarnings };
errorsRef.current = { name: newNameErrors, value: newValueErrors };
}, []);
warningsRef.current = { ...warningsRef.current, value: newValueWarnings };
errorsRef.current = { name: newNameErrors, value: newValueErrors };
},
[]
);

const handleOnNameBlur = useCallback(() => {
validateValues(item);
setVisited((prevVisited) => ({ ...prevVisited, name: true }));
setNameVisited(true);
}, [item, validateValues]);

const handleOnValueBlur = useCallback(() => {
validateValues(item);
setVisited((prevVisited) => ({ ...prevVisited, value: true }));
setValueVisited({ value: true });
}, [item, validateValues]);

const handleOnNameChange = useCallback(
Expand Down Expand Up @@ -463,7 +479,7 @@ export const BlockListForm = memo<ArtifactFormComponentProps>(
};

// trigger re-render without modifying item
setVisited((prevVisited) => ({ ...prevVisited }));
setValueVisited((prevState) => ({ ...prevState }));
},
[blocklistEntry]
);
Expand Down Expand Up @@ -495,7 +511,7 @@ export const BlockListForm = memo<ArtifactFormComponentProps>(
entries: [{ ...blocklistEntry, value }],
} as ArtifactFormComponentProps['item'];

validateValues(nextItem);
validateValues(nextItem, true);
onChange({
isValid: isValid(errorsRef.current),
item: nextItem,
Expand All @@ -518,7 +534,7 @@ export const BlockListForm = memo<ArtifactFormComponentProps>(
validateValues(nextItem as ArtifactFormComponentProps['item']);
nextItem.entries[0].value = uniq(nextItem.entries[0].value);

setVisited((prevVisited) => ({ ...prevVisited, value: true }));
setValueVisited({ value: true });
onChange({
isValid: isValid(errorsRef.current),
item: nextItem as ArtifactFormComponentProps['item'],
Expand Down Expand Up @@ -563,7 +579,7 @@ export const BlockListForm = memo<ArtifactFormComponentProps>(

<EuiFormRow
label={NAME_LABEL}
isInvalid={visited.name && !!Object.keys(errorsRef.current.name).length}
isInvalid={nameVisited && !!Object.keys(errorsRef.current.name).length}
error={Object.values(errorsRef.current.name)}
fullWidth
>
Expand All @@ -572,7 +588,7 @@ export const BlockListForm = memo<ArtifactFormComponentProps>(
value={item.name}
onChange={handleOnNameChange}
onBlur={handleOnNameBlur}
required={visited.name}
required={nameVisited}
maxLength={256}
data-test-subj={getTestId('name-input')}
fullWidth
Expand Down Expand Up @@ -651,7 +667,7 @@ export const BlockListForm = memo<ArtifactFormComponentProps>(
</EuiFormRow>
<EuiFormRow
label={valueLabel}
isInvalid={visited.value && !!Object.keys(errorsRef.current.value).length}
isInvalid={valueVisited.value && !!Object.keys(errorsRef.current.value).length}
helpText={Object.values(warningsRef.current.value)}
error={Object.values(errorsRef.current.value)}
fullWidth
Expand Down

0 comments on commit 4c5d5fa

Please sign in to comment.