Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Alert page charts new layout and chart collapse #150242

Merged
merged 8 commits into from
Feb 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const allowedExperimentalValues = Object.freeze({
/**
* Enables top charts on Alerts Page
*/
alertsPageChartsEnabled: false,
alertsPageChartsEnabled: true,

/**
* Keep DEPRECATED experimental flags that are documented to prevent failed upgrades.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ export const ALERTS_COUNT =
export const ALERTS_TREND_SIGNAL_RULE_NAME_PANEL =
'[data-test-subj="render-content-kibana.alert.rule.name"]';

export const CHART_SELECT = '[data-test-subj="chartSelect"]';

export const CLOSE_ALERT_BTN = '[data-test-subj="close-alert-status"]';

export const CLOSE_SELECTED_ALERTS_BTN = '[data-test-subj="close-alert-status"]';
Expand Down Expand Up @@ -92,7 +90,7 @@ export const RULE_NAME = '[data-test-subj^=formatted-field][data-test-subj$=rule

export const SELECTED_ALERTS = '[data-test-subj="selectedShowBulkActionsButton"]';

export const SELECT_TABLE = '[data-test-subj="table"]';
export const SELECT_AGGREGATION_CHART = '[data-test-subj="chart-select-table"]';

export const SEND_ALERT_TO_TIMELINE_BTN = '[data-test-subj="send-alert-to-timeline-button"]';

Expand Down
6 changes: 2 additions & 4 deletions x-pack/plugins/security_solution/cypress/tasks/alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import {
ADD_EXCEPTION_BTN,
ALERT_CHECKBOX,
CHART_SELECT,
CLOSE_ALERT_BTN,
CLOSE_SELECTED_ALERTS_BTN,
EXPAND_ALERT_BTN,
Expand All @@ -18,7 +17,7 @@ import {
MARK_ALERT_ACKNOWLEDGED_BTN,
OPEN_ALERT_BTN,
SEND_ALERT_TO_TIMELINE_BTN,
SELECT_TABLE,
SELECT_AGGREGATION_CHART,
TAKE_ACTION_POPOVER_BTN,
TIMELINE_CONTEXT_MENU_BTN,
CLOSE_FLYOUT,
Expand Down Expand Up @@ -257,8 +256,7 @@ export const openAlerts = () => {
};

export const selectCountTable = () => {
cy.get(CHART_SELECT).click({ force: true });
cy.get(SELECT_TABLE).click();
cy.get(SELECT_AGGREGATION_CHART).click({ force: true });
};

export const clearGroupByTopInput = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('AlertsTreemap', () => {
});

test('it renders the treemap', () => {
expect(screen.getByTestId('treemap').querySelector('.echChart')).toBeInTheDocument();
expect(screen.getByTestId('alerts-treemap').querySelector('.echChart')).toBeInTheDocument();
});

test('it renders the legend with the expected overflow-y style', () => {
Expand All @@ -71,7 +71,7 @@ describe('AlertsTreemap', () => {
});

test('it does NOT render the treemap', () => {
expect(screen.queryByTestId('treemap')).not.toBeInTheDocument();
expect(screen.queryByTestId('alerts-treemap')).not.toBeInTheDocument();
});

test('it does NOT render the legend', () => {
Expand Down Expand Up @@ -127,7 +127,7 @@ describe('AlertsTreemap', () => {
});

test('it renders the treemap', () => {
expect(screen.getByTestId('treemap').querySelector('.echChart')).toBeInTheDocument();
expect(screen.getByTestId('alerts-treemap').querySelector('.echChart')).toBeInTheDocument();
});

test('it does NOT render the "no data" message', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { NoData } from './no_data';
import { NO_DATA_REASON_LABEL } from './translations';
import type { AlertsTreeMapAggregation, FlattenedBucket, RawBucket } from './types';

export const DEFAULT_MIN_CHART_HEIGHT = 370; // px
export const DEFAULT_MIN_CHART_HEIGHT = 240; // px
const DEFAULT_LEGEND_WIDTH = 300; // px

export interface Props {
Expand Down Expand Up @@ -165,7 +165,7 @@ const AlertsTreemapComponent: React.FC<Props> = ({
}

return (
<div data-test-subj="treemap">
<div data-test-subj="alerts-treemap">
<EuiFlexGroup gutterSize="none">
<ChartFlexItem grow={true} $minChartHeight={minChartHeight}>
{stackByField1 != null && !isEmpty(stackByField1) && normalizedData.length === 0 ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ describe('AlertsTreemapPanel', () => {
</TestProviders>
);

await waitFor(() => expect(screen.getByTestId('chartSelect')).toBeInTheDocument());
await waitFor(() => expect(screen.getByTestId('chart-select-tabs')).toBeInTheDocument());
});

it('renders field selection when `isPanelExpanded` is true', async () => {
Expand Down Expand Up @@ -305,6 +305,6 @@ describe('AlertsTreemapPanel', () => {
</TestProviders>
);

await waitFor(() => expect(screen.getByTestId('treemap')).toBeInTheDocument());
await waitFor(() => expect(screen.getByTestId('alerts-treemap')).toBeInTheDocument());
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { has } from 'lodash';
import type { AlertType, AlertsByTypeAgg, AlertsTypeData } from './types';
import type { AlertSearchResponse } from '../../../containers/detection_engine/alerts/types';
import type { SummaryChartsData } from '../alerts_summary_charts_panel/types';
import type { SummaryChartsData, SummaryChartsAgg } from '../alerts_summary_charts_panel/types';

export const ALERT_TYPE_COLOR = {
Detection: '#D36086',
Expand Down Expand Up @@ -59,6 +59,12 @@ const getAggregateAlerts = (
return ret;
};

export const isAlertsTypeData = (data: SummaryChartsData[]): data is AlertsTypeData[] => {
export const getIsAlertsTypeData = (data: SummaryChartsData[]): data is AlertsTypeData[] => {
return data?.every((x) => has(x, 'type'));
};

export const getIsAlertsByTypeAgg = (
data: AlertSearchResponse<{}, SummaryChartsAgg>
): data is AlertSearchResponse<{}, AlertsByTypeAgg> => {
return has(data, 'aggregations.alertsByRule');
};
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ 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 } from '../alerts_summary_charts_panel/aggregations';
import { isAlertsTypeData } from './helpers';
import { getIsAlertsTypeData } from './helpers';
import * as i18n from './translations';

const ALERTS_BY_TYPE_CHART_ID = 'alerts-summary-alert_by_type';
Expand All @@ -37,7 +37,7 @@ export const AlertsByTypePanel: React.FC<ChartsPanelProps> = ({
skip,
uniqueQueryId,
});
const data = useMemo(() => (isAlertsTypeData(items) ? items : []), [items]);
const data = useMemo(() => (getIsAlertsTypeData(items) ? items : []), [items]);

return (
<InspectButtonContainer>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import React from 'react';
import { waitFor, act } from '@testing-library/react';
import { mount } from 'enzyme';

import { AlertsCountPanel } from '.';

import type { Status } from '../../../../../common/detection_engine/schemas/common';
Expand Down Expand Up @@ -61,25 +60,31 @@ jest.mock('../common/hooks', () => ({
useInspectButton: jest.fn(),
useStackByFields: jest.fn(),
}));
const mockUseIsExperimentalFeatureEnabled = useIsExperimentalFeatureEnabled as jest.Mock;
jest.mock('../../../../common/hooks/use_experimental_features');

const defaultProps = {
inspectTitle: TABLE,
signalIndexName: 'signalIndexName',
stackByField0: DEFAULT_STACK_BY_FIELD,
stackByField1: DEFAULT_STACK_BY_FIELD1,
setStackByField0: jest.fn(),
setStackByField1: jest.fn(),
isExpanded: true,
setIsExpanded: jest.fn(),
showBuildingBlockAlerts: false,
showOnlyThreatIndicatorAlerts: false,
signalIndexName: 'signalIndexName',
stackByField0: DEFAULT_STACK_BY_FIELD,
stackByField1: DEFAULT_STACK_BY_FIELD1,
status: 'open' as Status,
};
const mockUseQueryToggle = useQueryToggle as jest.Mock;
const mockSetToggle = jest.fn();
const mockUseQueryToggle = useQueryToggle as jest.Mock;

describe('AlertsCountPanel', () => {
beforeEach(() => {
jest.clearAllMocks();
mockUseQueryToggle.mockReturnValue({ toggleStatus: true, setToggleStatus: mockSetToggle });
mockUseIsExperimentalFeatureEnabled.mockReturnValueOnce(false); // for chartEmbeddablesEnabled flag
mockUseIsExperimentalFeatureEnabled.mockReturnValueOnce(false); // for alertsPageChartsEnabled flag
});

it('renders correctly', async () => {
Expand Down Expand Up @@ -177,6 +182,7 @@ describe('AlertsCountPanel', () => {
});
});
});

describe('toggleQuery', () => {
it('toggles', async () => {
await act(async () => {
Expand All @@ -189,7 +195,7 @@ describe('AlertsCountPanel', () => {
expect(mockSetToggle).toBeCalledWith(false);
});
});
it('toggleStatus=true, render', async () => {
it('alertsPageChartsEnabled is false and toggleStatus=true, render', async () => {
await act(async () => {
const wrapper = mount(
<TestProviders>
Expand All @@ -199,7 +205,7 @@ describe('AlertsCountPanel', () => {
expect(wrapper.find('[data-test-subj="alertsCountTable"]').exists()).toEqual(true);
});
});
it('toggleStatus=false, hide', async () => {
it('alertsPageChartsEnabled is false and toggleStatus=false, hide', async () => {
christineweng marked this conversation as resolved.
Show resolved Hide resolved
mockUseQueryToggle.mockReturnValue({ toggleStatus: false, setToggleStatus: mockSetToggle });
await act(async () => {
const wrapper = mount(
Expand All @@ -210,16 +216,41 @@ describe('AlertsCountPanel', () => {
expect(wrapper.find('[data-test-subj="alertsCountTable"]').exists()).toEqual(false);
});
});

it('alertsPageChartsEnabled is true and isExpanded=true, render', async () => {
mockUseIsExperimentalFeatureEnabled.mockReturnValueOnce(false); // for chartEmbeddablesEnabled flag
mockUseIsExperimentalFeatureEnabled.mockReturnValueOnce(true); // for alertsPageChartsEnabled flag
await act(async () => {
mockUseIsExperimentalFeatureEnabled('charts', true);
const wrapper = mount(
<TestProviders>
<AlertsCountPanel {...defaultProps} />
</TestProviders>
);
expect(wrapper.find('[data-test-subj="alertsCountTable"]').exists()).toEqual(true);
});
});
it('alertsPageChartsEnabled is true and isExpanded=false, hide', async () => {
mockUseIsExperimentalFeatureEnabled.mockReturnValueOnce(false); // for chartEmbeddablesEnabled flag
mockUseIsExperimentalFeatureEnabled.mockReturnValueOnce(true); // for alertsPageChartsEnabled flag
await act(async () => {
const wrapper = mount(
<TestProviders>
<AlertsCountPanel {...defaultProps} isExpanded={false} />
</TestProviders>
);
expect(wrapper.find('[data-test-subj="alertsCountTable"]').exists()).toEqual(false);
});
});
});
});

describe('when isChartEmbeddablesEnabled = true', () => {
beforeEach(() => {
jest.clearAllMocks();

mockUseQueryToggle.mockReturnValue({ toggleStatus: true, setToggleStatus: mockSetToggle });

(useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(true);
mockUseIsExperimentalFeatureEnabled.mockReturnValueOnce(true); // for chartEmbeddablesEnabled flag
mockUseIsExperimentalFeatureEnabled.mockReturnValueOnce(false); // for alertsPageChartsEnabled flag
});

it('renders LensEmbeddable', async () => {
Expand All @@ -229,7 +260,7 @@ describe('when isChartEmbeddablesEnabled = true', () => {
<AlertsCountPanel {...defaultProps} />
</TestProviders>
);
expect(wrapper.find('[data-test-subj="lens-embeddable"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="embeddable-count-table"]').exists()).toBeTruthy();
});
});

Expand Down