Skip to content

Commit

Permalink
[Security Solution][Detections] EQL UX Improvements (#80230) (#80376)
Browse files Browse the repository at this point in the history
* Add loading spinner to EQL Query Bar

This gives the user some indication that the validation is being
performed in the background, and that their query's validity is
currently undefined.

* Preserve whitespace on display of rule queries

This is useful for all rules, but particularly for EQL rules, where
sequences will be multiline.

* Fix tests around query description

This content is now wrapped in a whitespace-preserving element, so these
need to be updated ever so slightly.

* Update cypress assertions around rule queries

* Remove extra space from assertion
  * This extra space was removed in the presentation component. I didn't
    find an explicit reason why it was there in the first place.
* Use have.value when asserting on form inputs
  • Loading branch information
rylnd authored Oct 13, 2020
1 parent 13fe639 commit b934016
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe('Custom detection rules creation', () => {

// expect define step to repopulate
cy.get(DEFINE_EDIT_BUTTON).click();
cy.get(CUSTOM_QUERY_INPUT).should('have.text', newRule.customQuery);
cy.get(CUSTOM_QUERY_INPUT).should('have.value', newRule.customQuery);
cy.get(DEFINE_CONTINUE_BUTTON).should('exist').click({ force: true });
cy.get(DEFINE_CONTINUE_BUTTON).should('not.exist');

Expand Down Expand Up @@ -182,7 +182,7 @@ describe('Custom detection rules creation', () => {
cy.get(ABOUT_INVESTIGATION_NOTES).should('have.text', INVESTIGATION_NOTES_MARKDOWN);
cy.get(DEFINITION_DETAILS).within(() => {
getDetails(INDEX_PATTERNS_DETAILS).should('have.text', indexPatterns.join(''));
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', `${newRule.customQuery} `);
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', newRule.customQuery);
getDetails(RULE_TYPE_DETAILS).should('have.text', 'Query');
getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
});
Expand Down Expand Up @@ -293,7 +293,7 @@ describe('Custom detection rules deletion and edition', () => {
waitForKibana();

// expect define step to populate
cy.get(CUSTOM_QUERY_INPUT).should('have.text', existingRule.customQuery);
cy.get(CUSTOM_QUERY_INPUT).should('have.value', existingRule.customQuery);
if (existingRule.index && existingRule.index.length > 0) {
cy.get(DEFINE_INDEX_INPUT).should('have.text', existingRule.index.join(''));
}
Expand Down Expand Up @@ -344,7 +344,7 @@ describe('Custom detection rules deletion and edition', () => {
'have.text',
expectedEditedIndexPatterns.join('')
);
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', `${editedRule.customQuery} `);
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', editedRule.customQuery);
getDetails(RULE_TYPE_DETAILS).should('have.text', 'Query');
getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ describe('Detection rules, EQL', () => {
cy.get(ABOUT_INVESTIGATION_NOTES).should('have.text', INVESTIGATION_NOTES_MARKDOWN);
cy.get(DEFINITION_DETAILS).within(() => {
getDetails(INDEX_PATTERNS_DETAILS).should('have.text', indexPatterns.join(''));
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', `${eqlRule.customQuery} `);
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', eqlRule.customQuery);
getDetails(RULE_TYPE_DETAILS).should('have.text', 'Event Correlation');
getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ describe('Detection rules, override', () => {
cy.get(ABOUT_INVESTIGATION_NOTES).should('have.text', INVESTIGATION_NOTES_MARKDOWN);
cy.get(DEFINITION_DETAILS).within(() => {
getDetails(INDEX_PATTERNS_DETAILS).should('have.text', indexPatterns.join(''));
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', `${newOverrideRule.customQuery} `);
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', newOverrideRule.customQuery);
getDetails(RULE_TYPE_DETAILS).should('have.text', 'Query');
getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ describe('Detection rules, threshold', () => {
cy.get(ABOUT_INVESTIGATION_NOTES).should('have.text', INVESTIGATION_NOTES_MARKDOWN);
cy.get(DEFINITION_DETAILS).within(() => {
getDetails(INDEX_PATTERNS_DETAILS).should('have.text', indexPatterns.join(''));
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', `${newThresholdRule.customQuery} `);
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', newThresholdRule.customQuery);
getDetails(RULE_TYPE_DETAILS).should('have.text', 'Threshold');
getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
getDetails(THRESHOLD_DETAILS).should(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export const fillDefineCustomRuleWithImportedQueryAndContinue = (
) => {
cy.get(IMPORT_QUERY_FROM_SAVED_TIMELINE_LINK).click();
cy.get(TIMELINE(rule.timelineId)).click();
cy.get(CUSTOM_QUERY_INPUT).should('have.text', rule.customQuery);
cy.get(CUSTOM_QUERY_INPUT).should('have.value', rule.customQuery);
cy.get(DEFINE_CONTINUE_BUTTON).should('exist').click({ force: true });

cy.get(CUSTOM_QUERY_INPUT).should('not.exist');
Expand All @@ -208,7 +208,7 @@ export const fillDefineThresholdRuleAndContinue = (rule: ThresholdRule) => {
const threshold = 1;

cy.get(CUSTOM_QUERY_INPUT).type(rule.customQuery);
cy.get(CUSTOM_QUERY_INPUT).invoke('text').should('eq', rule.customQuery);
cy.get(CUSTOM_QUERY_INPUT).should('have.value', rule.customQuery);
cy.get(THRESHOLD_INPUT_AREA)
.find(INPUT)
.then((inputs) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,11 @@ describe('helpers', () => {
query: mockQueryBarWithQuery.query,
savedId: mockQueryBarWithQuery.saved_id,
});
expect(result[0].title).toEqual(<>{i18n.QUERY_LABEL} </>);
expect(result[0].description).toEqual(<>{mockQueryBarWithQuery.query} </>);

expect(result[0].title).toEqual(<>{i18n.QUERY_LABEL}</>);
expect(shallow(result[0].description as React.ReactElement).text()).toEqual(
mockQueryBarWithQuery.query
);
});

test('returns expected array of ListItems when "savedId" exists', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ const EuiBadgeWrap = (styled(EuiBadge)`
}
` as unknown) as typeof EuiBadge;

const Query = styled.div`
white-space: pre-wrap;
`;

export const buildQueryBarDescription = ({
field,
filters,
Expand Down Expand Up @@ -92,8 +96,8 @@ export const buildQueryBarDescription = ({
items = [
...items,
{
title: <>{queryLabel ?? i18n.QUERY_LABEL} </>,
description: <>{query} </>,
title: <>{queryLabel ?? i18n.QUERY_LABEL}</>,
description: <Query>{query}</Query>,
},
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,10 @@ describe('description_step', () => {
mockFilterManager
);

expect(result[0].title).toEqual(<>{i18n.QUERY_LABEL} </>);
expect(result[0].description).toEqual(<>{mockQueryBar.queryBar.query.query} </>);
expect(result[0].title).toEqual(<>{i18n.QUERY_LABEL}</>);
expect(shallow(result[0].description as React.ReactElement).text()).toEqual(
mockQueryBar.queryBar.query.query
);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export interface EqlQueryBarProps {
export const EqlQueryBar: FC<EqlQueryBarProps> = ({ dataTestSubj, field, idAria }) => {
const { addError } = useAppToasts();
const [errorMessages, setErrorMessages] = useState<string[]>([]);
const { setValue } = field;
const { isValidating, setValue } = field;
const { isValid, message, messages, error } = getValidationResults(field);
const fieldValue = field.value.query.query as string;

Expand Down Expand Up @@ -81,7 +81,7 @@ export const EqlQueryBar: FC<EqlQueryBarProps> = ({ dataTestSubj, field, idAria
value={fieldValue}
onChange={handleChange}
/>
<EqlQueryBarFooter errors={errorMessages} />
<EqlQueryBarFooter errors={errorMessages} isLoading={isValidating} />
</>
</EuiFormRow>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,39 @@

import React, { FC } from 'react';
import styled from 'styled-components';
import { EuiFlexGroup, EuiFlexItem, EuiPanel } from '@elastic/eui';
import { EuiFlexGroup, EuiFlexItem, EuiLoadingSpinner, EuiPanel } from '@elastic/eui';

import * as i18n from './translations';
import { ErrorsPopover } from './errors_popover';
import { EqlOverviewLink } from './eql_overview_link';

export interface Props {
errors: string[];
isLoading?: boolean;
}

const Container = styled(EuiPanel)`
border-radius: 0;
background: ${({ theme }) => theme.eui.euiPageBackgroundColor};
padding: ${({ theme }) => theme.eui.euiSizeXS};
padding: ${({ theme }) => theme.eui.euiSizeXS} ${({ theme }) => theme.eui.euiSizeS};
`;

const FlexGroup = styled(EuiFlexGroup)`
min-height: ${({ theme }) => theme.eui.euiSizeXL};
`;

export const EqlQueryBarFooter: FC<Props> = ({ errors }) => (
const Spinner = styled(EuiLoadingSpinner)`
margin: 0 ${({ theme }) => theme.eui.euiSizeS};
`;

export const EqlQueryBarFooter: FC<Props> = ({ errors, isLoading }) => (
<Container>
<FlexGroup alignItems="center" justifyContent="spaceBetween" gutterSize="none">
<EuiFlexItem>
{errors.length > 0 && (
<ErrorsPopover ariaLabel={i18n.EQL_VALIDATION_ERROR_POPOVER_LABEL} errors={errors} />
)}
{isLoading && <Spinner data-test-subj="eql-validation-loading" size="m" />}
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EqlOverviewLink />
Expand Down

0 comments on commit b934016

Please sign in to comment.