Skip to content

Commit

Permalink
[Security Solution] Remove feature flag entityAlertPreviewDisabled (e…
Browse files Browse the repository at this point in the history 
…lastic#203171)

## Summary

This PR removes the feature flag `entityAlertPreviewDisabled` and
cleaned up related unit tests.


### 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 @benakansara
christineweng authored and benakansara committed Jan 2, 2025
1 parent c5e5ce7 commit 89be520
Show file tree
Hide file tree
Showing 19 changed files with 124 additions and 425 deletions.
5 changes: 0 additions & 5 deletions x-pack/solutions/security/plugins/security_solution/common/experimental_features.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,6 @@ export const allowedExperimentalValues = Object.freeze({
*/
securitySolutionNotesDisabled: false,

/**
* Disables entity and alert previews
*/
entityAlertPreviewDisabled: false,

/**
* Enables the Assistant Model Evaluation advanced setting and API endpoint, introduced in `8.11.0`.
*/
Expand Down
31 changes: 1 addition & 30 deletions ...c/entity_analytics/components/entity_details_flyout/tabs/risk_inputs/risk_inputs.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,6 @@ const riskScore = {
},
};

const mockUseIsExperimentalFeatureEnabled = jest.fn().mockReturnValue(false);

jest.mock('../../../../../common/hooks/use_experimental_features', () => ({
useIsExperimentalFeatureEnabled: () => mockUseIsExperimentalFeatureEnabled(),
}));

const riskScoreWithAssetCriticalityContribution = (contribution: number) => {
const score = JSON.parse(JSON.stringify(riskScore));
score.user.risk.category_2_score = contribution;
Expand Down Expand Up @@ -129,8 +123,7 @@ describe('RiskInputsTab', () => {
expect(queryByTestId('risk-input-contexts-title')).toBeInTheDocument();
});

it('it renders alert preview button when feature flag is enable', () => {
mockUseIsExperimentalFeatureEnabled.mockReturnValue(false);
it('it renders alert preview button', () => {
mockUseRiskScore.mockReturnValue({
loading: false,
error: false,
Expand All @@ -151,28 +144,6 @@ describe('RiskInputsTab', () => {
expect(getByTestId(EXPAND_ALERT_TEST_ID)).toBeInTheDocument();
});

it('it does not render alert preview button when feature flag is disable', () => {
mockUseIsExperimentalFeatureEnabled.mockReturnValue(true);
mockUseRiskScore.mockReturnValue({
loading: false,
error: false,
data: [riskScore],
});
mockUseRiskContributingAlerts.mockReturnValue({
loading: false,
error: false,
data: [alertInputDataMock],
});

const { queryByTestId } = render(
<TestProviders>
<RiskInputsTab entityType={RiskScoreEntity.user} entityName="elastic" scopeId={'scopeId'} />
</TestProviders>
);

expect(queryByTestId(EXPAND_ALERT_TEST_ID)).not.toBeInTheDocument();
});

it('Displays 0.00 for the asset criticality contribution if the contribution value is less than -0.01', () => {
mockUseUiSetting.mockReturnValue([true]);

Expand Down
31 changes: 12 additions & 19 deletions ...ic/entity_analytics/components/entity_details_flyout/tabs/risk_inputs/risk_inputs_tab.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { ALERT_RULE_NAME } from '@kbn/rule-data-utils';

import { get } from 'lodash/fp';
import { AlertPreviewButton } from '../../../../../flyout/shared/components/alert_preview_button';
import { useIsExperimentalFeatureEnabled } from '../../../../../common/hooks/use_experimental_features';
import { useGlobalTime } from '../../../../../common/containers/use_global_time';
import { useQueryInspector } from '../../../../../common/components/page/manage_query';
import { formatRiskScore } from '../../../../common';
Expand Down Expand Up @@ -98,26 +97,20 @@ export const RiskInputsTab = ({ entityType, entityName, scopeId }: RiskInputsTab
}),
[]
);
const isPreviewEnabled = !useIsExperimentalFeatureEnabled('entityAlertPreviewDisabled');

const inputColumns: Array<EuiBasicTableColumn<InputAlert>> = useMemo(
() => [
...(isPreviewEnabled
? [
{
render: (data: InputAlert) => (
<AlertPreviewButton
id={data._id}
indexName={data.input.index}
scopeId={scopeId}
data-test-subj={EXPAND_ALERT_TEST_ID}
/>
),
width: '5%',
},
]
: []),

{
render: (data: InputAlert) => (
<AlertPreviewButton
id={data._id}
indexName={data.input.index}
scopeId={scopeId}
data-test-subj={EXPAND_ALERT_TEST_ID}
/>
),
width: '5%',
},
{
name: (
<FormattedMessage
Expand Down Expand Up @@ -172,7 +165,7 @@ export const RiskInputsTab = ({ entityType, entityName, scopeId }: RiskInputsTab
render: formatContribution,
},
],
[isPreviewEnabled, scopeId]
[scopeId]
);

if (riskScoreError) {
Expand Down
19 changes: 6 additions & 13 deletions ...public/flyout/document_details/left/components/correlations_details_alerts_table.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { TestProviders } from '../../../../common/mock';
import { useExpandableFlyoutApi } from '@kbn/expandable-flyout';
import { CorrelationsDetailsAlertsTable } from './correlations_details_alerts_table';
import { usePaginatedAlerts } from '../hooks/use_paginated_alerts';
import { useIsExperimentalFeatureEnabled } from '../../../../common/hooks/use_experimental_features';
import { mockFlyoutApi } from '../../shared/mocks/mock_flyout_context';
import { mockContextValue } from '../../shared/mocks/mock_context';
import { DocumentDetailsPreviewPanelKey } from '../../shared/constants/panel_keys';
Expand All @@ -20,8 +19,6 @@ import { DocumentDetailsContext } from '../../shared/context';
import { RulePreviewPanelKey, RULE_PREVIEW_BANNER } from '../../../rule_details/right';

jest.mock('../hooks/use_paginated_alerts');
jest.mock('../../../../common/hooks/use_experimental_features');
const mockUseIsExperimentalFeatureEnabled = useIsExperimentalFeatureEnabled as jest.Mock;

jest.mock('@kbn/expandable-flyout');

Expand All @@ -47,7 +44,6 @@ const renderCorrelationsTable = (panelContext: DocumentDetailsContext) =>
describe('CorrelationsDetailsAlertsTable', () => {
beforeEach(() => {
jest.mocked(useExpandableFlyoutApi).mockReturnValue(mockFlyoutApi);
mockUseIsExperimentalFeatureEnabled.mockReturnValue(true);
jest.mocked(usePaginatedAlerts).mockReturnValue({
setPagination: jest.fn(),
setSorting: jest.fn(),
Expand Down Expand Up @@ -88,25 +84,24 @@ describe('CorrelationsDetailsAlertsTable', () => {
});

it('renders EuiBasicTable with correct props', () => {
const { getByTestId, queryByTestId, queryAllByRole } =
const { getByTestId, getAllByTestId, queryAllByRole } =
renderCorrelationsTable(mockContextValue);

expect(getByTestId(`${TEST_ID}InvestigateInTimeline`)).toBeInTheDocument();
expect(getByTestId(`${TEST_ID}Table`)).toBeInTheDocument();
expect(queryByTestId(`${TEST_ID}AlertPreviewButton`)).not.toBeInTheDocument();
expect(getAllByTestId(`${TEST_ID}AlertPreviewButton`)).toHaveLength(2);

expect(jest.mocked(usePaginatedAlerts)).toHaveBeenCalled();

expect(queryAllByRole('columnheader').length).toBe(4);
expect(queryAllByRole('columnheader').length).toBe(5);
expect(queryAllByRole('row').length).toBe(3); // 1 header row and 2 data rows
expect(queryAllByRole('row')[1].textContent).toContain('Jan 1, 2022 @ 00:00:00.000');
expect(queryAllByRole('row')[1].textContent).toContain('Reason1');
expect(queryAllByRole('row')[1].textContent).toContain('Rule1');
expect(queryAllByRole('row')[1].textContent).toContain('Severity1');
});

it('renders open preview button when feature flag is on', () => {
mockUseIsExperimentalFeatureEnabled.mockReturnValue(false);
it('renders open preview button', () => {
const { getByTestId, getAllByTestId } = renderCorrelationsTable({
...mockContextValue,
isPreviewMode: true,
Expand All @@ -128,8 +123,7 @@ describe('CorrelationsDetailsAlertsTable', () => {
});
});

it('opens rule preview when feature flag is on and isPreview is false', () => {
mockUseIsExperimentalFeatureEnabled.mockReturnValue(false);
it('opens rule preview when isPreview is false', () => {
const { getAllByTestId } = renderCorrelationsTable(mockContextValue);

expect(getAllByTestId(`${TEST_ID}RulePreview`).length).toBe(2);
Expand All @@ -145,8 +139,7 @@ describe('CorrelationsDetailsAlertsTable', () => {
});
});

it('does not render preview link when feature flag is on and isPreview is true', () => {
mockUseIsExperimentalFeatureEnabled.mockReturnValue(false);
it('does not render preview link when isPreview is true', () => {
const { queryByTestId } = renderCorrelationsTable({ ...mockContextValue, isPreview: true });
expect(queryByTestId(`${TEST_ID}RulePreview`)).not.toBeInTheDocument();
});
Expand Down
53 changes: 21 additions & 32 deletions ...tion/public/flyout/document_details/left/components/correlations_details_alerts_table.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { isRight } from 'fp-ts/lib/Either';
import { ALERT_REASON, ALERT_RULE_NAME } from '@kbn/rule-data-utils';
import { FormattedMessage } from '@kbn/i18n-react';
import { i18n } from '@kbn/i18n';
import { useIsExperimentalFeatureEnabled } from '../../../../common/hooks/use_experimental_features';
import { CellTooltipWrapper } from '../../shared/components/cell_tooltip_wrapper';
import type { DataProvider } from '../../../../../common/types';
import { SeverityBadge } from '../../../../common/components/severity_badge';
Expand Down Expand Up @@ -82,8 +81,6 @@ export const CorrelationsDetailsAlertsTable: FC<CorrelationsDetailsAlertsTablePr
sorting,
error,
} = usePaginatedAlerts(alertIds || []);
const isPreviewEnabled = !useIsExperimentalFeatureEnabled('entityAlertPreviewDisabled');

const { isPreview } = useDocumentDetailsContext();

const onTableChange = useCallback(
Expand Down Expand Up @@ -129,21 +126,17 @@ export const CorrelationsDetailsAlertsTable: FC<CorrelationsDetailsAlertsTablePr

const columns = useMemo(
() => [
...(isPreviewEnabled
? [
{
render: (row: Record<string, unknown>) => (
<AlertPreviewButton
id={row.id as string}
indexName={row.index as string}
data-test-subj={`${dataTestSubj}AlertPreviewButton`}
scopeId={scopeId}
/>
),
width: '5%',
},
]
: []),
{
render: (row: Record<string, unknown>) => (
<AlertPreviewButton
id={row.id as string}
indexName={row.index as string}
data-test-subj={`${dataTestSubj}AlertPreviewButton`}
scopeId={scopeId}
/>
),
width: '5%',
},
{
field: '@timestamp',
name: (
Expand Down Expand Up @@ -176,20 +169,16 @@ export const CorrelationsDetailsAlertsTable: FC<CorrelationsDetailsAlertsTablePr
const ruleId = row['kibana.alert.rule.uuid'] as string;
return (
<CellTooltipWrapper tooltip={ruleName}>
{isPreviewEnabled ? (
<PreviewLink
field={ALERT_RULE_NAME}
value={ruleName}
scopeId={scopeId}
ruleId={ruleId}
isPreview={isPreview}
data-test-subj={`${dataTestSubj}RulePreview`}
>
<span>{ruleName}</span>
</PreviewLink>
) : (
<PreviewLink
field={ALERT_RULE_NAME}
value={ruleName}
scopeId={scopeId}
ruleId={ruleId}
isPreview={isPreview}
data-test-subj={`${dataTestSubj}RulePreview`}
>
<span>{ruleName}</span>
)}
</PreviewLink>
</CellTooltipWrapper>
);
},
Expand Down Expand Up @@ -229,7 +218,7 @@ export const CorrelationsDetailsAlertsTable: FC<CorrelationsDetailsAlertsTablePr
},
},
],
[isPreviewEnabled, scopeId, dataTestSubj, isPreview]
[scopeId, dataTestSubj, isPreview]
);

return (
Expand Down
19 changes: 6 additions & 13 deletions ...ns/security_solution/public/flyout/document_details/left/components/host_details.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { useMlCapabilities } from '../../../../common/components/ml/hooks/use_ml
import { mockAnomalies } from '../../../../common/components/ml/mock';
import { useHostDetails } from '../../../../explore/hosts/containers/hosts/details';
import { useHostRelatedUsers } from '../../../../common/containers/related_entities/related_users';
import { useIsExperimentalFeatureEnabled } from '../../../../common/hooks/use_experimental_features';
import { RiskSeverity } from '../../../../../common/search_strategy';
import {
HOST_DETAILS_TEST_ID,
Expand Down Expand Up @@ -46,9 +45,6 @@ jest.mock('@kbn/expandable-flyout');
jest.mock('@kbn/cloud-security-posture/src/hooks/use_misconfiguration_preview');
jest.mock('@kbn/cloud-security-posture/src/hooks/use_vulnerabilities_preview');

jest.mock('../../../../common/hooks/use_experimental_features');
const mockUseIsExperimentalFeatureEnabled = useIsExperimentalFeatureEnabled as jest.Mock;

jest.mock('react-router-dom', () => {
const actual = jest.requireActual('react-router-dom');
return { ...actual, useLocation: jest.fn().mockReturnValue({ pathname: '' }) };
Expand Down Expand Up @@ -180,20 +176,18 @@ describe('<HostDetails />', () => {
mockUseHostDetails.mockReturnValue(mockHostDetailsResponse);
mockUseRiskScore.mockReturnValue(mockRiskScoreResponse);
mockUseHostsRelatedUsers.mockReturnValue(mockRelatedUsersResponse);
mockUseIsExperimentalFeatureEnabled.mockReturnValue(true);
(useMisconfigurationPreview as jest.Mock).mockReturnValue({});
(useVulnerabilitiesPreview as jest.Mock).mockReturnValue({});
(useAlertsByStatus as jest.Mock).mockReturnValue({ isLoading: false, items: {} });
});

it('should render host details correctly', () => {
const { getByTestId, queryByTestId } = renderHostDetails(mockContextValue);
const { getByTestId } = renderHostDetails(mockContextValue);
expect(getByTestId(EXPANDABLE_PANEL_CONTENT_TEST_ID(HOST_DETAILS_TEST_ID))).toBeInTheDocument();
expect(queryByTestId(HOST_DETAILS_LINK_TEST_ID)).not.toBeInTheDocument();
expect(getByTestId(HOST_DETAILS_LINK_TEST_ID)).toBeInTheDocument();
});

it('should render host name as clicable link when preview is not disabled', () => {
mockUseIsExperimentalFeatureEnabled.mockReturnValue(false);
it('should render host name as clicable link', () => {
const { getByTestId } = renderHostDetails(mockContextValue);
expect(getByTestId(HOST_DETAILS_LINK_TEST_ID)).toBeInTheDocument();

Expand Down Expand Up @@ -242,15 +236,15 @@ describe('<HostDetails />', () => {

describe('Related users', () => {
it('should render the related user table with correct dates and indices', () => {
const { getByTestId, queryByTestId } = renderHostDetails(mockContextValue);
const { getByTestId } = renderHostDetails(mockContextValue);
expect(mockUseHostsRelatedUsers).toBeCalledWith({
from: timestamp,
hostName: 'test host',
indexNames: ['index'],
skip: false,
});
expect(getByTestId(HOST_DETAILS_RELATED_USERS_TABLE_TEST_ID)).toBeInTheDocument();
expect(queryByTestId(HOST_DETAILS_RELATED_USERS_LINK_TEST_ID)).not.toBeInTheDocument();
expect(getByTestId(HOST_DETAILS_RELATED_USERS_LINK_TEST_ID)).toBeInTheDocument();
});

it('should render user risk score column when license and capabilities are valid', () => {
Expand Down Expand Up @@ -296,8 +290,7 @@ describe('<HostDetails />', () => {
);
});

it('should render user name as clicable link when preview is not disabled', () => {
mockUseIsExperimentalFeatureEnabled.mockReturnValue(false);
it('should render user name as clicable link', () => {
const { getAllByTestId } = renderHostDetails(mockContextValue);
expect(getAllByTestId(HOST_DETAILS_RELATED_USERS_LINK_TEST_ID).length).toBe(1);

Expand Down
Loading

0 comments on commit 89be520

Please sign in to comment.