Skip to content

Commit

Permalink
[Security Solution][Alert Details] Remove alert type charts feature f…
Browse files Browse the repository at this point in the history
…lag (elastic#189437)

## Summary

Removing an unused feature flag called `alertTypeEnabled`. The feature
was added in 8.7 (elastic#152872) but was
never enabled.

This PR removed unused components and renamed directory from
`alert_by_type_panel` to `alerts_by_rule_panel`.


### Checklist

- [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
  • Loading branch information
christineweng authored Jul 31, 2024
1 parent d9282a6 commit 5fd6a48
Show file tree
Hide file tree
Showing 26 changed files with 298 additions and 953 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,6 @@ export const allowedExperimentalValues = Object.freeze({
*/
alertsPageChartsEnabled: true,

/**
* Enables the alert type column in KPI visualizations on Alerts Page
*/
alertTypeEnabled: false,

/**
* Enables new notes
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* 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 { act, render } from '@testing-library/react';
import React from 'react';
import { TestProviders } from '../../../../common/mock';
import { AlertsByRule } from './alerts_by_rule';
import { parsedAlerts } from './mock_rule_data';

jest.mock('../../../../common/lib/kibana');

jest.mock('react-router-dom', () => {
const actual = jest.requireActual('react-router-dom');
return { ...actual, useLocation: jest.fn().mockReturnValue({ pathname: '' }) };
});

describe('Alert by rule chart', () => {
const defaultProps = {
data: [],
isLoading: false,
};

afterEach(() => {
jest.clearAllMocks();
});

test('renders table correctly without data', () => {
act(() => {
const { container } = render(
<TestProviders>
<AlertsByRule {...defaultProps} />
</TestProviders>
);
expect(
container.querySelector('[data-test-subj="alerts-by-rule-table"]')
).toBeInTheDocument();
expect(
container.querySelector('[data-test-subj="alerts-by-rule-table"] tbody')?.textContent
).toEqual('No items found');
});
});

test('renders table correctly with data', () => {
act(() => {
const { queryAllByRole } = render(
<TestProviders>
<AlertsByRule data={parsedAlerts} isLoading={false} />
</TestProviders>
);

parsedAlerts.forEach((_, i) => {
expect(queryAllByRole('row')[i + 1].textContent).toContain(parsedAlerts[i].rule);
expect(queryAllByRole('row')[i + 1].textContent).toContain(
parsedAlerts[i].value.toString()
);
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/*
* 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 type { EuiBasicTableColumn } from '@elastic/eui';
import { EuiInMemoryTable, EuiSpacer, EuiText } from '@elastic/eui';
import React from 'react';
import styled from 'styled-components';
import type { SortOrder } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { ALERT_RULE_NAME } from '@kbn/rule-data-utils';
import { TableId } from '@kbn/securitysolution-data-table';
import type { AlertsByRuleData } from './types';
import { FormattedCount } from '../../../../common/components/formatted_number';
import { DefaultDraggable } from '../../../../common/components/draggables';
import { ALERTS_HEADERS_RULE_NAME } from '../../alerts_table/translations';
import { COUNT_TABLE_TITLE } from '../alerts_count_panel/translations';

const Wrapper = styled.div`
margin-top: -${({ theme }) => theme.eui.euiSizeM};
`;
const TableWrapper = styled.div`
height: 178px;
`;

export interface AlertsByRuleProps {
data: AlertsByRuleData[];
isLoading: boolean;
}

const COLUMNS: Array<EuiBasicTableColumn<AlertsByRuleData>> = [
{
field: 'rule',
name: ALERTS_HEADERS_RULE_NAME,
'data-test-subj': 'alert-by-rule-table-rule-name',
truncateText: true,
render: (rule: string) => (
<EuiText size="xs" className="eui-textTruncate">
<DefaultDraggable
isDraggable={false}
field={ALERT_RULE_NAME}
hideTopN={true}
id={`alert-detection-draggable-${rule}`}
value={rule}
queryValue={rule}
tooltipContent={null}
truncate={true}
scopeId={TableId.alertsOnAlertsPage}
/>
</EuiText>
),
},
{
field: 'value',
name: COUNT_TABLE_TITLE,
dataType: 'number',
sortable: true,
'data-test-subj': 'alert-by-rule-table-count',
render: (count: number) => (
<EuiText grow={false} size="xs">
<FormattedCount count={count} />
</EuiText>
),
width: '22%',
},
];

const SORTING: { sort: { field: keyof AlertsByRuleData; direction: SortOrder } } = {
sort: {
field: 'value',
direction: 'desc',
},
};

const PAGINATION: {} = {
pageSize: 25,
showPerPageOptions: false,
};

export const AlertsByRule: React.FC<AlertsByRuleProps> = ({ data, isLoading }) => {
return (
<Wrapper data-test-subj="alerts-by-rule">
<EuiSpacer size="xs" />
<TableWrapper className="eui-yScroll">
<EuiInMemoryTable
data-test-subj="alerts-by-rule-table"
columns={COLUMNS}
items={data}
loading={isLoading}
sorting={SORTING}
pagination={PAGINATION}
/>
</TableWrapper>
</Wrapper>
);
};

AlertsByRule.displayName = 'AlertsByRule';
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* 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 { parseAlertsRuleData, getIsAlertsByRuleData, getIsAlertsByRuleAgg } from './helpers';
import * as mockRule from './mock_rule_data';
import type { AlertsByRuleAgg } from './types';
import type { AlertSearchResponse } from '../../../containers/detection_engine/alerts/types';

describe('parse alerts by rule data', () => {
test('parse alerts with data', () => {
const res = parseAlertsRuleData(
mockRule.mockAlertsData as AlertSearchResponse<{}, AlertsByRuleAgg>
);
expect(res).toEqual(mockRule.parsedAlerts);
});

test('parse alerts without data', () => {
const res = parseAlertsRuleData(
mockRule.mockAlertsEmptyData as AlertSearchResponse<{}, AlertsByRuleAgg>
);
expect(res).toEqual([]);
});
});

describe('get is alerts by rule data', () => {
test('should return true for rule data', () => {
expect(getIsAlertsByRuleData(mockRule.parsedAlerts)).toBe(true);
});

test('should return false for non rule data', () => {
expect(getIsAlertsByRuleData([{ key: 'low', value: 1 }])).toBe(false);
});

test('should return false for empty array', () => {
expect(getIsAlertsByRuleData([])).toBe(false);
});
});

describe('get is alerts by rule agg', () => {
test('return true for rule aggregation query', () => {
expect(getIsAlertsByRuleAgg(mockRule.mockAlertsData)).toBe(true);
});

test('return false for queries without alertByRule key', () => {
expect(getIsAlertsByRuleAgg({ ...mockRule.mockAlertsEmptyData, aggregations: {} })).toBe(false);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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 { has } from 'lodash';
import type { AlertsByRuleData, AlertsByRuleAgg } from './types';
import type { AlertSearchResponse } from '../../../containers/detection_engine/alerts/types';
import type { SummaryChartsData, SummaryChartsAgg } from '../alerts_summary_charts_panel/types';

export const parseAlertsRuleData = (
response: AlertSearchResponse<{}, AlertsByRuleAgg>
): AlertsByRuleData[] => {
const rulesBuckets = response?.aggregations?.alertsByRule?.buckets ?? [];

return rulesBuckets.length === 0
? []
: rulesBuckets.map((rule) => {
return {
rule: rule.key,
value: rule.doc_count,
};
});
};

export const getIsAlertsByRuleData = (data: SummaryChartsData[]): data is AlertsByRuleData[] => {
return data.length > 0 && data.every((x) => has(x, 'rule'));
};

export const getIsAlertsByRuleAgg = (
data: AlertSearchResponse<{}, SummaryChartsAgg>
): data is AlertSearchResponse<{}, AlertsByRuleAgg> => {
return has(data, 'aggregations.alertsByRule');
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { act, render } from '@testing-library/react';
import React from 'react';
import { TestProviders } from '../../../../common/mock';
import { AlertsByTypePanel } from '.';
import { AlertsByRulePanel } from '.';

jest.mock('../../../../common/lib/kibana');

Expand All @@ -16,7 +16,7 @@ jest.mock('react-router-dom', () => {
return { ...actual, useLocation: jest.fn().mockReturnValue({ pathname: '' }) };
});

describe('Alert by type panel', () => {
describe('Alert by rule panel', () => {
const defaultProps = {
signalIndexName: 'signalIndexName',
skip: false,
Expand All @@ -30,11 +30,11 @@ describe('Alert by type panel', () => {
await act(async () => {
const { container } = render(
<TestProviders>
<AlertsByTypePanel {...defaultProps} />
<AlertsByRulePanel {...defaultProps} />
</TestProviders>
);
expect(
container.querySelector('[data-test-subj="alerts-by-type-panel"]')
container.querySelector('[data-test-subj="alerts-by-rule-panel"]')
).toBeInTheDocument();
});
});
Expand All @@ -43,7 +43,7 @@ describe('Alert by type panel', () => {
await act(async () => {
const { container } = render(
<TestProviders>
<AlertsByTypePanel {...defaultProps} />
<AlertsByRulePanel {...defaultProps} />
</TestProviders>
);
expect(container.querySelector(`[data-test-subj="header-section"]`)).toBeInTheDocument();
Expand All @@ -54,21 +54,21 @@ describe('Alert by type panel', () => {
await act(async () => {
const { container } = render(
<TestProviders>
<AlertsByTypePanel {...defaultProps} />
<AlertsByRulePanel {...defaultProps} />
</TestProviders>
);
expect(container.querySelector('[data-test-subj="inspect-icon-button"]')).toBeInTheDocument();
});
});

test('renders alert by type chart', async () => {
test('renders alert by rule chart', async () => {
await act(async () => {
const { container } = render(
<TestProviders>
<AlertsByTypePanel {...defaultProps} />
<AlertsByRulePanel {...defaultProps} />
</TestProviders>
);
expect(container.querySelector('[data-test-subj="alerts-by-type"]')).toBeInTheDocument();
expect(container.querySelector('[data-test-subj="alerts-by-rule"]')).toBeInTheDocument();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,56 +9,51 @@ import { EuiPanel } from '@elastic/eui';
import React, { useMemo } from 'react';
import { v4 as uuid } from 'uuid';
import type { ChartsPanelProps } from '../alerts_summary_charts_panel/types';
import { AlertsByType } from './alerts_by_type';
import { AlertsByRule } from './alerts_by_rule';
import { HeaderSection } from '../../../../common/components/header_section';
import { InspectButtonContainer } from '../../../../common/components/inspect';
import { useSummaryChartData } from '../alerts_summary_charts_panel/use_summary_chart_data';
import {
alertTypeAggregations,
alertRuleAggregations,
} from '../alerts_summary_charts_panel/aggregations';
import { getIsAlertsTypeData } from './helpers';
import { alertRuleAggregations } from '../alerts_summary_charts_panel/aggregations';
import { getIsAlertsByRuleData } from './helpers';
import * as i18n from './translations';
import { useIsExperimentalFeatureEnabled } from '../../../../common/hooks/use_experimental_features';

const ALERTS_BY_TYPE_CHART_ID = 'alerts-summary-alert_by_type';

export const AlertsByTypePanel: React.FC<ChartsPanelProps> = ({
export const AlertsByRulePanel: React.FC<ChartsPanelProps> = ({
filters,
query,
signalIndexName,
runtimeMappings,
skip,
}) => {
const isAlertTypeEnabled = useIsExperimentalFeatureEnabled('alertTypeEnabled');
const uniqueQueryId = useMemo(() => `${ALERTS_BY_TYPE_CHART_ID}-${uuid()}`, []);

const { items, isLoading } = useSummaryChartData({
aggregations: isAlertTypeEnabled ? alertTypeAggregations : alertRuleAggregations,
aggregations: alertRuleAggregations,
filters,
query,
signalIndexName,
runtimeMappings,
skip,
uniqueQueryId,
});
const data = useMemo(() => (getIsAlertsTypeData(items) ? items : []), [items]);
const data = useMemo(() => (getIsAlertsByRuleData(items) ? items : []), [items]);

return (
<InspectButtonContainer>
<EuiPanel hasBorder hasShadow={false} data-test-subj="alerts-by-type-panel">
<EuiPanel hasBorder hasShadow={false} data-test-subj="alerts-by-rule-panel">
<HeaderSection
id={uniqueQueryId}
inspectTitle={isAlertTypeEnabled ? i18n.ALERTS_TYPE_TITLE : i18n.ALERTS_RULE_TITLE}
inspectTitle={i18n.ALERTS_RULE_TITLE}
outerDirection="row"
title={isAlertTypeEnabled ? i18n.ALERTS_TYPE_TITLE : i18n.ALERTS_RULE_TITLE}
title={i18n.ALERTS_RULE_TITLE}
titleSize="xs"
hideSubtitle
/>
<AlertsByType data={data} isLoading={isLoading} />
<AlertsByRule data={data} isLoading={isLoading} />
</EuiPanel>
</InspectButtonContainer>
);
};

AlertsByTypePanel.displayName = 'AlertsByTypePanel';
AlertsByRulePanel.displayName = 'AlertsByRulePanel';
Loading

0 comments on commit 5fd6a48

Please sign in to comment.