Skip to content

Commit

Permalink
[Security Solution] Fix rule filters on the Rule Details page (elasti…
Browse files Browse the repository at this point in the history
…c#177081)

**Fixes: elastic#141458
**Fixes: elastic#176866

## Summary

Fixes the bugs above by changing the `Filters` component:

- from using lower-level components like `FilterBadgeGroup` and custom
rendering
- to using a higher-level `FilterItems` component that's used inside a
larger component `QueryBar` (see the first screenshot below) on the Rule
Creation / Editing pages

Note that for some reason the end result still does not fully equal to
how filters look on the Rule Creation / Editing pages, where there are
fewer warnings. It's hard to say which rendering is the right one.

## Screenshots

**How filters look on the Rule Creation / Editing pages:**

<img width="989" alt="Screenshot 2024-02-15 at 21 25 00"
src="https://github.com/elastic/kibana/assets/7359339/01ca468f-be99-469a-8d75-ee5aa1a31fb0">

**Rule Details page BEFORE the fix:**

<img width="1792" alt="Screenshot 2024-02-15 at 21 23 46"
src="https://github.com/elastic/kibana/assets/7359339/d0e2aa6e-3050-4327-8025-f37125498fd6">
<img width="1792" alt="Screenshot 2024-02-15 at 21 24 02"
src="https://github.com/elastic/kibana/assets/7359339/a89302b2-f991-4547-bdac-c0f5a594a881">
<img width="1792" alt="Screenshot 2024-02-15 at 21 24 18"
src="https://github.com/elastic/kibana/assets/7359339/49c16b02-8d82-4f93-932f-3846880a0457">

**Rule Details page AFTER the fix 1 (filters use non-existing fields and
show warnings):**

<img width="1790" alt="Screenshot 2024-02-15 at 21 28 46"
src="https://github.com/elastic/kibana/assets/7359339/e229b4ff-6ee7-4444-b5c1-deb00d2b9b39">

**Rule Details page AFTER the fix 2 (filters use existing fields and
look normal):**

<img width="1792" alt="Screenshot 2024-02-15 at 21 37 45"
src="https://github.com/elastic/kibana/assets/7359339/b10905e7-803d-4404-aa02-8692ff964891">

### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 2672662)
  • Loading branch information
banderror committed Feb 19, 2024
1 parent dca2f55 commit e92252d
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 69 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* 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 { useEuiTheme } from '@elastic/eui';
import { css } from '@emotion/css';
import { useMemo } from 'react';

export const useFiltersStyles = () => {
return useMemo(
() => ({
flexGroup: css`
max-width: 600px;
`,
}),
[]
);
};

export const useQueryStyles = () => {
return useMemo(
() => ({
content: css`
white-space: pre-wrap;
`,
}),
[]
);
};

export const useRequiredFieldsStyles = () => {
const { euiTheme } = useEuiTheme();
return useMemo(
() => ({
fieldTypeText: css({
fontFamily: euiTheme.font.familyCode,
display: 'inline',
}),
}),
[euiTheme.font.familyCode]
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,13 @@

import React from 'react';
import { isEmpty } from 'lodash/fp';
import styled from 'styled-components';
import {
EuiDescriptionList,
EuiText,
EuiFlexGrid,
EuiFlexItem,
EuiFlexGroup,
EuiLoadingSpinner,
EuiBadge,
} from '@elastic/eui';
import type { EuiDescriptionListProps } from '@elastic/eui';
import type {
Expand All @@ -25,9 +23,10 @@ import type {
import type { Filter } from '@kbn/es-query';
import type { SavedQuery } from '@kbn/data-plugin/public';
import { mapAndFlattenFilters } from '@kbn/data-plugin/public';
import type { DataView } from '@kbn/data-views-plugin/public';
import { FieldIcon } from '@kbn/react-field';
import { castEsToKbnFieldTypeName } from '@kbn/field-types';
import { FilterBadgeGroup } from '@kbn/unified-search-plugin/public';
import { FilterItems } from '@kbn/unified-search-plugin/public';
import type {
AlertSuppressionMissingFieldsStrategy,
RequiredFieldArray,
Expand Down Expand Up @@ -55,6 +54,11 @@ import { BadgeList } from './badge_list';
import { DEFAULT_DESCRIPTION_LIST_COLUMN_WIDTHS } from './constants';
import * as i18n from './translations';
import { useAlertSuppression } from '../../logic/use_alert_suppression';
import {
useFiltersStyles,
useQueryStyles,
useRequiredFieldsStyles,
} from './rule_definition_section.styles';

interface SavedQueryNameProps {
savedQueryName: string;
Expand All @@ -66,12 +70,6 @@ const SavedQueryName = ({ savedQueryName }: SavedQueryNameProps) => (
</EuiText>
);

const EuiBadgeWrap = styled(EuiBadge)`
.euiBadge__text {
white-space: pre-wrap !important;
}
`;

interface FiltersProps {
filters: Filter[];
dataViewId?: string;
Expand All @@ -80,51 +78,42 @@ interface FiltersProps {
}

const Filters = ({ filters, dataViewId, index, 'data-test-subj': dataTestSubj }: FiltersProps) => {
const flattenedFilters = mapAndFlattenFilters(filters);

const { indexPattern } = useRuleIndexPattern({
dataSourceType: dataViewId ? DataSourceType.DataView : DataSourceType.IndexPatterns,
index: index ?? [],
dataViewId,
});

const flattenedFilters = mapAndFlattenFilters(filters);
const styles = useFiltersStyles();

return (
<EuiFlexGroup wrap responsive={false} gutterSize="xs" data-test-subj={dataTestSubj}>
{flattenedFilters.map((filter, idx) => {
const displayContent = filter.meta.alias ? (
filter.meta.alias
) : (
<FilterBadgeGroup filters={[filter]} dataViews={[indexPattern]} />
);
return (
<EuiFlexItem
grow={false}
key={`filter-${idx}`}
css={{ width: '100%' }}
data-test-subj={`filterItem-${filter.meta.key}`}
>
<EuiBadgeWrap color="hollow">
{indexPattern != null ? displayContent : <EuiLoadingSpinner size="m" />}
</EuiBadgeWrap>
</EuiFlexItem>
);
})}
<EuiFlexGroup
data-test-subj={dataTestSubj}
className={styles.flexGroup}
wrap
responsive={false}
gutterSize="xs"
>
<FilterItems filters={flattenedFilters} indexPatterns={[indexPattern as DataView]} readOnly />
</EuiFlexGroup>
);
};

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

interface QueryProps {
query: string;
'data-test-subj'?: string;
}

const Query = ({ query, 'data-test-subj': dataTestSubj = 'query' }: QueryProps) => (
<QueryContent data-test-subj={dataTestSubj}>{query}</QueryContent>
);
const Query = ({ query, 'data-test-subj': dataTestSubj = 'query' }: QueryProps) => {
const styles = useQueryStyles();
return (
<div data-test-subj={dataTestSubj} className={styles.content}>
{query}
</div>
);
};

interface IndexProps {
index: string[];
Expand Down Expand Up @@ -260,42 +249,40 @@ const RuleType = ({ type }: RuleTypeProps) => (
<EuiText size="s">{getRuleTypeDescription(type)}</EuiText>
);

const StyledFieldTypeText = styled(EuiText)`
font-size: ${({ theme }) => theme.eui.euiFontSizeXS};
font-family: ${({ theme }) => theme.eui.euiCodeFontFamily};
display: inline;
`;

interface RequiredFieldsProps {
requiredFields: RequiredFieldArray;
}

const RequiredFields = ({ requiredFields }: RequiredFieldsProps) => (
<EuiFlexGrid gutterSize={'s'} data-test-subj="requiredFieldsPropertyValue">
{requiredFields.map((rF, index) => (
<EuiFlexItem grow={false} key={rF.name}>
<EuiFlexGroup alignItems="center" gutterSize={'xs'}>
<EuiFlexItem grow={false}>
<FieldIcon
data-test-subj="field-type-icon"
type={castEsToKbnFieldTypeName(rF.type)}
label={rF.type}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<StyledFieldTypeText
grow={false}
size={'s'}
data-test-subj="requiredFieldsPropertyValueItem"
>
{` ${rF.name}${index + 1 !== requiredFields.length ? ', ' : ''}`}
</StyledFieldTypeText>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
))}
</EuiFlexGrid>
);
const RequiredFields = ({ requiredFields }: RequiredFieldsProps) => {
const styles = useRequiredFieldsStyles();
return (
<EuiFlexGrid data-test-subj="requiredFieldsPropertyValue" gutterSize={'s'}>
{requiredFields.map((rF, index) => (
<EuiFlexItem grow={false} key={rF.name}>
<EuiFlexGroup alignItems="center" gutterSize={'xs'}>
<EuiFlexItem grow={false}>
<FieldIcon
data-test-subj="field-type-icon"
type={castEsToKbnFieldTypeName(rF.type)}
label={rF.type}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiText
data-test-subj="requiredFieldsPropertyValueItem"
className={styles.fieldTypeText}
grow={false}
size="xs"
>
{` ${rF.name}${index + 1 !== requiredFields.length ? ', ' : ''}`}
</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
))}
</EuiFlexGrid>
);
};

interface TimelineTitleProps {
timelineTitle: string;
Expand Down

0 comments on commit e92252d

Please sign in to comment.