Skip to content

Commit

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

# Backport

This will backport the following commits from `main` to `8.x`:
- [[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","v9.0.0","Team:Defend
Workflows","backport:version","v8.17.0","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.x"],"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.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.16","label":"v8.16.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/199992","number":199992,"state":"MERGED","mergeCommit":{"sha":"4c5d5fa5a40014b0d8b09d0a9d07b7a7eec899a1","message":"[8.16]
[EDR Workflows] Error message &#x27;the value already exists&#x27; shown
for duplicate values with the &#x27;is one of&#x27; operator on
Blocklist tab (#196071) (#199992)\n\n# Backport\n\nThis will backport
the following commits from `main` to `8.16`:\n- [[EDR Workflows] Error
message &#x27;the value already exists&#x27;\nshown for duplicate values
with the &#x27;is one of&#x27; operator on\nBlocklist tab
(#196071)](https://github.com/elastic/kibana/pull/196071)\n\n<!---
Backport version: 9.4.3 -->\n\n### Questions ?\nPlease refer to the
[Backport
tool\ndocumentation](https://github.com/sqren/backport)\n\n<!--BACKPORT
[{\"author\":{\"name\":\"Konrad\nSzwarc\",\"email\":\"[email protected]\"},\"sourceCommit\":{\"committedDate\":\"2024-10-23T10:29:32Z\",\"message\":\"[EDR\nWorkflows]
Error message 'the value already exists' shown for duplicate\nvalues
with the 'is one of' operator on Blocklist tab
(#196071)\\n\\nThese\nchanges ensure that the warning displayed to the
user for\\r\\nduplicate\nvalues remains visible after onBlur. I’ve opted
to keep it as\\r\\na\nwarning rather than an error since entering a
duplicate value\ndoesn’t\\r\\nallow the user to “save” it as a selected
item in the combo\nbox—it\\r\\nremains an active string in the input
field. We don’t block\nform\\r\\nsubmission due to this warning; any
unselected value in the\ncombo box is\\r\\nstripped out. The red border
on the combo box is part of\nEUI’s behavior\\r\\nwhen attempting to
select a duplicate, and I don’t see\nan easy way to\\r\\nmodify this at
the\nmoment.\\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\nmissing\",\"v9.0.0\",\"Team:Defend\nWorkflows\",\"backport:version\",\"v8.16.1\"],\"title\":\"[EDR
Workflows] Error\nmessage 'the value already exists' shown for duplicate
values with the\n'is one of' operator on
Blocklist\ntab\",\"number\":196071,\"url\":\"https://github.com/elastic/kibana/pull/196071\",\"mergeCommit\":{\"message\":\"[EDR\nWorkflows]
Error message 'the value already exists' shown for duplicate\nvalues
with the 'is one of' operator on Blocklist tab
(#196071)\\n\\nThese\nchanges ensure that the warning displayed to the
user for\\r\\nduplicate\nvalues remains visible after onBlur. I’ve opted
to keep it as\\r\\na\nwarning rather than an error since entering a
duplicate value\ndoesn’t\\r\\nallow the user to “save” it as a selected
item in the combo\nbox—it\\r\\nremains an active string in the input
field. We don’t block\nform\\r\\nsubmission due to this warning; any
unselected value in the\ncombo box is\\r\\nstripped out. The red border
on the combo box is part of\nEUI’s behavior\\r\\nwhen attempting to
select a duplicate, and I don’t see\nan easy way to\\r\\nmodify this at
the\nmoment.\\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\nWorkflows]
Error message 'the value already exists' shown for duplicate\nvalues
with the 'is one of' operator on Blocklist tab
(#196071)\\n\\nThese\nchanges ensure that the warning displayed to the
user for\\r\\nduplicate\nvalues remains visible after onBlur. I’ve opted
to keep it as\\r\\na\nwarning rather than an error since entering a
duplicate value\ndoesn’t\\r\\nallow the user to “save” it as a selected
item in the combo\nbox—it\\r\\nremains an active string in the input
field. We don’t block\nform\\r\\nsubmission due to this warning; any
unselected value in the\ncombo box is\\r\\nstripped out. The red border
on the combo box is part of\nEUI’s behavior\\r\\nwhen attempting to
select a duplicate, and I don’t see\nan easy way to\\r\\nmodify this at
the\nmoment.\\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\"}]}]\nBACKPORT-->\n\nCo-authored-by:
Konrad Szwarc <[email protected]>"}}]}] BACKPORT-->

Co-authored-by: Konrad Szwarc <[email protected]>
  • Loading branch information
kibanamachine and szwarckonrad authored Nov 20, 2024
1 parent b476f7f commit 096a3cd
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 50 deletions.
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
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 096a3cd

Please sign in to comment.