Skip to content

Commit

Permalink
[Security Solution] Fix empty fields and tab titles on Alerts page ch…
Browse files Browse the repository at this point in the history
…arts (elastic#152402)

This PR contains fixes/enhancements on charts section on Alerts Page:

1. Updated tab names

![image](https://user-images.githubusercontent.com/18648970/222000232-e8681a19-3986-4b7a-a7f1-e92b805ad965.png)

2. Updated inspect modal titles to match actual tab name (from
elastic#151842)

- `Counts` (used to be `Aggregations` on alerts page and `Table` in
inspect modal, they are both `Counts` now)

![image](https://user-images.githubusercontent.com/18648970/222000544-575b33ee-dddd-4e8b-b7f6-8bc2b2c67545.png)

3. Updated `querySkip` in `Trend`, `Counts`, and `Summary` as mentioned
on elastic#150382
- `querySkip` followed the same pattern of `toggleStatus` that each
chart keeps track of its own `querySkip` based on toggle status (skip
query if charts is collapsed). This is no longer necessary because
toggle is now managed at a higher level.

4. Fixed a bug that the top alerts chart was calculating percentages
based on available fields
- For instance, there are 100 alerts, 20 has `host.name="host-1"`, 30
has `host.name="host-2"`, the bars will show 40% and 60% for each, and
it adds up to 100%. This does not factor in the 50 alerts with
empty/null fields.
- This PR added an info button that shows the percentage of available
fields, as well as on-click action to add a filter to show alerts with
empty fields

![image](https://user-images.githubusercontent.com/18648970/222000280-456b435e-193c-45e2-b4a6-9b6cf4cfee08.png)

https://user-images.githubusercontent.com/18648970/222000307-764b1e90-ac88-40c7-9f26-a9372e8592a8.mov

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [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

(cherry picked from commit 012ec79)
  • Loading branch information
christineweng committed Mar 6, 2023
1 parent 3340aef commit 87e923a
Show file tree
Hide file tree
Showing 19 changed files with 290 additions and 143 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { EuiProgress } from '@elastic/eui';
import type { EuiComboBox } from '@elastic/eui';
import type { Action } from '@kbn/ui-actions-plugin/public';
import type { MappingRuntimeFields } from '@elastic/elasticsearch/lib/api/types';
import React, { memo, useMemo, useState, useEffect, useCallback } from 'react';
import React, { memo, useMemo, useEffect, useCallback } from 'react';
import { v4 as uuidv4 } from 'uuid';

import type { Filter, Query } from '@kbn/es-query';
Expand Down Expand Up @@ -108,28 +108,20 @@ export const AlertsCountPanel = memo<AlertsCountPanelProps>(
}, [query, filters]);

const { toggleStatus, setToggleStatus } = useQueryToggle(DETECTIONS_ALERTS_COUNT_ID);
const [querySkip, setQuerySkip] = useState(
isAlertsPageChartsEnabled ? !isExpanded : !toggleStatus
);
useEffect(() => {
if (isAlertsPageChartsEnabled) {
setQuerySkip(!isExpanded);
} else {
setQuerySkip(!toggleStatus);
}
}, [toggleStatus, isAlertsPageChartsEnabled, isExpanded]);

const toggleQuery = useCallback(
(newToggleStatus: boolean) => {
if (isAlertsPageChartsEnabled && setIsExpanded) {
setIsExpanded(newToggleStatus);
} else {
setToggleStatus(newToggleStatus);
}
// toggle on = skipQuery false
setQuerySkip(!newToggleStatus);
},
[setQuerySkip, setToggleStatus, setIsExpanded, isAlertsPageChartsEnabled]
[setToggleStatus, setIsExpanded, isAlertsPageChartsEnabled]
);

const querySkip = useMemo(
() => (isAlertsPageChartsEnabled ? !isExpanded : !toggleStatus),
[isAlertsPageChartsEnabled, isExpanded, toggleStatus]
);

const timerange = useMemo(() => ({ from, to }), [from, to]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,16 +181,6 @@ export const AlertsHistogramPanel = memo<AlertsHistogramPanelProps>(
}, [defaultStackByOption, onlyField]);

const { toggleStatus, setToggleStatus } = useQueryToggle(DETECTIONS_HISTOGRAM_ID);
const [querySkip, setQuerySkip] = useState(
isAlertsPageChartsEnabled ? !isExpanded : !toggleStatus
);
useEffect(() => {
if (isAlertsPageChartsEnabled && isExpanded !== undefined) {
setQuerySkip(!isExpanded);
} else {
setQuerySkip(!toggleStatus);
}
}, [toggleStatus, isAlertsPageChartsEnabled, isExpanded]);

const toggleQuery = useCallback(
(newToggleStatus: boolean) => {
Expand All @@ -199,10 +189,14 @@ export const AlertsHistogramPanel = memo<AlertsHistogramPanelProps>(
} else {
setToggleStatus(newToggleStatus);
}
// toggle on = skipQuery false
setQuerySkip(!newToggleStatus);
},
[setQuerySkip, setToggleStatus, setIsExpanded, isAlertsPageChartsEnabled]
[setToggleStatus, setIsExpanded, isAlertsPageChartsEnabled]
);

const querySkip = useMemo(
() =>
isAlertsPageChartsEnabled && setIsExpanded !== undefined ? !isExpanded : !toggleStatus,
[isAlertsPageChartsEnabled, setIsExpanded, isExpanded, toggleStatus]
);

const timerange = useMemo(() => ({ from, to }), [from, to]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,17 @@ describe('Alert by grouping', () => {
).not.toBeInTheDocument();

parsedAlerts.forEach((alert, i) => {
expect(
container.querySelector(`[data-test-subj="progress-bar-${alert.key}"]`)
).toBeInTheDocument();
expect(
container.querySelector(`[data-test-subj="progress-bar-${alert.key}"]`)?.textContent
).toContain(parsedAlerts[i].label);
expect(
container.querySelector(`[data-test-subj="progress-bar-${alert.key}"]`)?.textContent
).toContain(parsedAlerts[i].percentage.toString());
if (alert.key !== '-') {
expect(
container.querySelector(`[data-test-subj="progress-bar-${alert.key}"]`)
).toBeInTheDocument();
expect(
container.querySelector(`[data-test-subj="progress-bar-${alert.key}"]`)?.textContent
).toContain(parsedAlerts[i].label);
expect(
container.querySelector(`[data-test-subj="progress-bar-${alert.key}"]`)?.textContent
).toContain(parsedAlerts[i].percentage.toString());
}
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,91 +4,175 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { EuiProgress, EuiSpacer, EuiText, EuiHorizontalRule } from '@elastic/eui';
import React from 'react';
import {
EuiProgress,
EuiSpacer,
EuiText,
EuiHorizontalRule,
EuiPopoverTitle,
EuiLink,
EuiFlexGroup,
EuiFlexItem,
EuiPopover,
EuiButtonIcon,
} from '@elastic/eui';
import React, { useState } from 'react';
import styled from 'styled-components';
import type { AlertsProgressBarData, GroupBySelection } from './types';
import type { AddFilterProps } from '../common/types';
import { getNonEmptyPercent } from './helpers';
import { DefaultDraggable } from '../../../../common/components/draggables';
import * as i18n from './translations';

const ProgressWrapper = styled.div`
height: 160px;
`;

const StyledEuiText = styled(EuiText)`
const StyledEuiHorizontalRule = styled(EuiHorizontalRule)`
margin-top: 0;
margin-bottom: ${({ theme }) => theme.eui.euiSizeS};
`;

const StyledEuiFlexGroup = styled(EuiFlexGroup)`
margin-top: -${({ theme }) => theme.eui.euiSizeM};
`;

const StyledEuiProgress = styled(EuiProgress)`
margin-top: ${({ theme }) => theme.eui.euiSizeS};
margin-bottom: ${({ theme }) => theme.eui.euiSizeS};
`;

const DataStatsWrapper = styled.div`
width: 250px;
`;
export interface AlertsProcessBarProps {
data: AlertsProgressBarData[];
isLoading: boolean;
addFilter?: ({ field, value }: { field: string; value: string | number }) => void;
addFilter?: ({ field, value, negate }: AddFilterProps) => void;
groupBySelection: GroupBySelection;
}

export const AlertsProgressBar: React.FC<AlertsProcessBarProps> = ({
data,
isLoading,
addFilter,
groupBySelection,
}) => {
const [isPopoverOpen, setIsPopoverOpen] = useState(false);
const onButtonClick = () => setIsPopoverOpen(!isPopoverOpen);
const closePopover = () => setIsPopoverOpen(false);

const validPercent = getNonEmptyPercent(data);

const dataStatsButton = (
<EuiButtonIcon
color="text"
iconType="iInCircle"
aria-label="info"
size="xs"
onClick={onButtonClick}
/>
);

const dataStatsMessage = (
<DataStatsWrapper>
<EuiPopoverTitle>{i18n.DATA_STATISTICS_TITLE(validPercent.toString())}</EuiPopoverTitle>
<EuiText size="s">
{i18n.DATA_STATISTICS_MESSAGE(groupBySelection)}
<EuiLink
color="primary"
onClick={() => {
setIsPopoverOpen(false);
if (addFilter) {
addFilter({ field: groupBySelection, value: null, negate: true });
}
}}
>
{i18n.NON_EMPTY_FILTER(groupBySelection)}
</EuiLink>
</EuiText>
</DataStatsWrapper>
);

const labelWithHoverActions = (key: string) => {
return (
<DefaultDraggable
isDraggable={false}
field={groupBySelection}
hideTopN={true}
id={`top-alerts-${key}`}
value={key}
queryValue={key}
tooltipContent={null}
>
<EuiText size="xs" className="eui-textTruncate">
{key}
</EuiText>
</DefaultDraggable>
);
};

return (
<>
<StyledEuiText size="s" data-test-subj="alerts-progress-bar-title">
<h5>{groupBySelection}</h5>
</StyledEuiText>
<StyledEuiFlexGroup alignItems="center" gutterSize="xs">
<EuiFlexItem grow={false}>
<EuiText size="s" data-test-subj="alerts-progress-bar-title">
<h5>{groupBySelection}</h5>
</EuiText>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiPopover
button={dataStatsButton}
isOpen={isPopoverOpen}
closePopover={closePopover}
anchorPosition="rightCenter"
panelPaddingSize="s"
>
{dataStatsMessage}
</EuiPopover>
</EuiFlexItem>
</StyledEuiFlexGroup>
{isLoading ? (
<StyledEuiProgress size="xs" color="primary" />
) : (
<EuiHorizontalRule margin="xs" />
)}
{!isLoading && data.length === 0 ? (
<>
<EuiText size="s" textAlign="center" data-test-subj="empty-proress-bar">
{i18n.EMPTY_DATA_MESSAGE}
</EuiText>
<EuiHorizontalRule margin="xs" />
<StyledEuiHorizontalRule />
<ProgressWrapper data-test-subj="progress-bar" className="eui-yScroll">
{validPercent === 0 ? (
<>
<EuiText size="s" textAlign="center" data-test-subj="empty-proress-bar">
{i18n.EMPTY_DATA_MESSAGE}
</EuiText>
<EuiSpacer size="l" />
</>
) : (
<>
{data.map(
(item) =>
item.key !== '-' && (
<div key={`${item.key}`} data-test-subj={`progress-bar-${item.key}`}>
<EuiProgress
valueText={
<EuiText size="xs" color="default">
<strong>{`${item.percentage}%`}</strong>
</EuiText>
}
max={100}
color={`vis9`}
size="s"
value={item.percentage}
label={
item.key === 'Other' ? item.label : labelWithHoverActions(item.key)
}
/>
<EuiSpacer size="s" />
</div>
)
)}
</>
)}
<EuiSpacer size="s" />
</ProgressWrapper>
</>
) : (
<ProgressWrapper data-test-subj="progress-bar" className="eui-yScroll">
{data.map((item) => (
<div key={`${item.key}`} data-test-subj={`progress-bar-${item.key}`}>
<EuiProgress
valueText={
<EuiText size="xs" color="default">
<strong>{`${item.percentage}%`}</strong>
</EuiText>
}
max={100}
color={`vis9`}
size="s"
value={item.percentage}
label={
item.key === 'Other' ? (
item.label
) : (
<DefaultDraggable
isDraggable={false}
field={groupBySelection}
hideTopN={true}
id={`top-alerts-${item.key}`}
value={item.key}
queryValue={item.key}
tooltipContent={null}
>
<EuiText size="xs" className="eui-textTruncate">
{item.key}
</EuiText>
</DefaultDraggable>
)
}
/>
<EuiSpacer size="s" />
</div>
))}
</ProgressWrapper>
)}
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { parseAlertsGroupingData } from './helpers';
import { parseAlertsGroupingData, getNonEmptyPercent } from './helpers';
import * as mock from './mock_data';
import type { AlertsByGroupingAgg } from './types';
import type { AlertSearchResponse } from '../../../containers/detection_engine/alerts/types';
Expand All @@ -24,3 +24,11 @@ describe('parse progress bar data', () => {
expect(res).toEqual([]);
});
});

describe('test non-empty percentage', () => {
test('should return correct non-empty percentage', () => {
const expected = Math.round((620 / 630) * 100);
const res = getNonEmptyPercent(mock.parsedAlerts);
expect(res).toEqual(expected);
});
});

0 comments on commit 87e923a

Please sign in to comment.