Skip to content

Commit

Permalink
[Security Solution][Detections] Rule Form Fixes (#84169) (#84288)
Browse files Browse the repository at this point in the history
* Prevent error from being displayed when choosing action throttle

Addresses #83230.

This field was previously refactored to not require a callback prop;
simply updating the value via `field.setValue` is sufficient for our use
case.

This fix removes the errant code that assumed a callback prop, since
such a prop does not exist on the underlying component.

* Fix UI bug on ML Jobs popover

EUI links now add an SVG if they're an external link; our use of a div
was causing that to wrap. Since the div was only needed to change the
text size, a refactor makes this all work.

* Exercise editing of tags in E2E tests

These tests were recently skipped due to their improper teardown. Since
that's a broader issue across most of these tests, I'm reopening these
so we can get the coverage provided here for the time being.

* useFetchIndex defaults to isLoading: false

In the case where no index pattern is provided, the hook exits without
doing any work but does not update the loading state; this had the
downstream effect of disabling a form field that was waiting for this
hook to stop loading.

* Move situational action into ... the situation

We only need to clear existing tags in the case where we're editing the
rule (and it has tags); in all other cases, this method fails. This
fixes things by moving that conditional logic (clear the tags field)
into the test that needs it (editing custom rules).
  • Loading branch information
rylnd authored Nov 25, 2020
1 parent f93ffd2 commit ef6db6d
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
SCHEDULE_INTERVAL_AMOUNT_INPUT,
SCHEDULE_INTERVAL_UNITS_INPUT,
SEVERITY_DROPDOWN,
TAGS_CLEAR_BUTTON,
TAGS_FIELD,
} from '../screens/create_new_rule';
import {
Expand Down Expand Up @@ -215,8 +216,7 @@ describe('Custom detection rules creation', () => {
});
});

// FLAKY: https://github.com/elastic/kibana/issues/83772
describe.skip('Custom detection rules deletion and edition', () => {
describe('Custom detection rules deletion and edition', () => {
beforeEach(() => {
esArchiverLoad('custom_rules');
loginAndWaitForPageWithoutDateRange(DETECTIONS_URL);
Expand Down Expand Up @@ -328,6 +328,7 @@ describe.skip('Custom detection rules deletion and edition', () => {
cy.get(ACTIONS_THROTTLE_INPUT).invoke('val').should('eql', 'no_actions');

goToAboutStepTab();
cy.get(TAGS_CLEAR_BUTTON).click({ force: true });
fillAboutRule(editedRule);
saveEditedRule();

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/security_solution/cypress/objects/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,4 +265,5 @@ export const editedRule = {
...existingRule,
severity: 'Medium',
description: 'Edited Rule description',
tags: [...existingRule.tags, 'edited'],
};
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ export const TAGS_FIELD =
export const TAGS_INPUT =
'[data-test-subj="detectionEngineStepAboutRuleTags"] [data-test-subj="comboBoxSearchInput"]';

export const TAGS_CLEAR_BUTTON =
'[data-test-subj="detectionEngineStepAboutRuleTags"] [data-test-subj="comboBoxClearButton"]';

export const THRESHOLD_FIELD_SELECTION = '.euiFilterSelectItem';

export const THRESHOLD_INPUT_AREA = '[data-test-subj="thresholdInput"]';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,11 @@ const JobName = ({ id, description, basePath }: JobNameProps) => {

return (
<JobNameWrapper>
<EuiLink data-test-subj="jobs-table-link" href={jobUrl} target="_blank">
<EuiText size="s">{id}</EuiText>
</EuiLink>
<EuiText size="s">
<EuiLink data-test-subj="jobs-table-link" href={jobUrl} target="_blank">
{id}
</EuiLink>
</EuiText>
<EuiText color="subdued" size="xs">
{description.length > truncateThreshold
? `${description.substring(0, truncateThreshold)}...`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export const useFetchIndex = (
const { data, notifications } = useKibana().services;
const abortCtrl = useRef(new AbortController());
const previousIndexesName = useRef<string[]>([]);
const [isLoading, setLoading] = useState(true);
const [isLoading, setLoading] = useState(false);

const [state, setState] = useState<FetchIndexReturn>({
browserFields: DEFAULT_BROWSER_FIELDS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,13 @@ export const DEFAULT_THROTTLE_OPTION = THROTTLE_OPTIONS[0];
type ThrottleSelectField = typeof SelectField;

export const ThrottleSelectField: ThrottleSelectField = (props) => {
const { setValue } = props.field;
const onChange = useCallback(
(e) => {
const throttle = e.target.value;
props.field.setValue(throttle);
props.handleChange(throttle);
setValue(throttle);
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[props.field.setValue, props.handleChange]
[setValue]
);
const newEuiFieldProps = { ...props.euiFieldProps, onChange };
return <SelectField {...props} euiFieldProps={newEuiFieldProps} />;
Expand Down

0 comments on commit ef6db6d

Please sign in to comment.