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

[Cloud Posture] Dashboard Redesign - data counter cards #144565

Merged
merged 17 commits into from
Nov 9, 2022
1 change: 1 addition & 0 deletions x-pack/plugins/cloud_security_posture/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface FindingsEvaluation {

export interface Stats extends FindingsEvaluation {
postureScore: Score;
resourcesEvaluated?: number;
}

export interface GroupedFindingsEvaluation extends FindingsEvaluation {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,14 @@ export const useNavigateFindings = () => {
});
};
};

export const useNavigateFindingsByResource = () => {
const history = useHistory();

return (query?: Query['query']) => {
history.push({
pathname: findingsNavigation.findings_by_resource.path,
...(query && { search: encodeQuery(getFindingsQuery(query)) }),
});
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,28 @@
* 2.0.
*/
import React from 'react';
import { FormattedNumber } from '@kbn/i18n-react';
import { EuiToolTip } from '@elastic/eui';

export const CompactFormattedNumber = ({ number }: { number: number }) => (
<FormattedNumber
value={number}
maximumFractionDigits={1}
notation="compact"
compactDisplay="short"
/>
);
export const CompactFormattedNumber = ({
number = 0,
abbreviateAbove = 999999,
}: {
number: number;
/** numbers higher than the value of this field will be abbreviated using compact notation and have a tooltip displaying the full value */
abbreviateAbove?: number;
}) => {
if (number <= abbreviateAbove) {
return <span>{number.toLocaleString()}</span>;
}

return (
<EuiToolTip content={number.toLocaleString()}>
<span>
{number.toLocaleString(undefined, {
notation: 'compact',
maximumFractionDigits: 1,
})}
</span>
</EuiToolTip>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* 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 React from 'react';
import { css } from '@emotion/react';
import { EuiCard, EuiIcon, EuiText, EuiTitle, useEuiTheme } from '@elastic/eui';
import type { EuiTextProps, EuiCardProps } from '@elastic/eui';

export type CspCounterCardProps = Pick<EuiCardProps, 'onClick' | 'id' | 'title' | 'description'> & {
descriptionColor?: EuiTextProps['color'];
};

export const CspCounterCard = (counter: CspCounterCardProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider de-structuring props in the arguments: ({ title, onClick, id, description, descriptionColor })

const { euiTheme } = useEuiTheme();

return (
<EuiCard
title={
<EuiTitle size="xxxs">
<h6>{counter.title}</h6>
</EuiTitle>
}
hasBorder
onClick={counter.onClick}
paddingSize="m"
textAlign="left"
layout="vertical"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the default and can be omitted as we don't really care about the layout since we don't use an icon, we only care about the text alignment

css={css`
position: relative;

:hover .euiIcon {
color: ${euiTheme.colors.primary};
transition: ${euiTheme.animation.normal};
}
`}
data-test-subj={counter.id}
>
<EuiText color={counter.descriptionColor}>
<EuiTitle size="xs">
<h3>{counter.description}</h3>
</EuiTitle>
</EuiText>
{counter.onClick && (
<EuiIcon
type="link"
css={css`
position: absolute;
top: ${euiTheme.size.m};
right: ${euiTheme.size.m};
`}
/>
)}
</EuiCard>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { NO_FINDINGS_STATUS_TEST_SUBJ } from '../../components/test_subjects';
import { useCISIntegrationPoliciesLink } from '../../common/navigation/use_navigate_to_cis_integration_policies';
import { useCISIntegrationLink } from '../../common/navigation/use_navigate_to_cis_integration';
import { expectIdsInDoc } from '../../test/utils';
import { ComplianceDashboardData } from '../../../common/types';

jest.mock('../../common/api/use_setup_status_api');
jest.mock('../../common/api/use_compliance_dashboard_data_api');
Expand All @@ -28,12 +29,13 @@ jest.mock('../../common/navigation/use_navigate_to_cis_integration_policies');
jest.mock('../../common/navigation/use_navigate_to_cis_integration');
const chance = new Chance();

const mockDashboardData = {
export const mockDashboardData: ComplianceDashboardData = {
stats: {
totalFailed: 17,
totalPassed: 155,
totalFindings: 172,
postureScore: 90.1,
resourcesEvaluated: 162,
},
groupedFindingsEvaluation: [
{
Expand Down Expand Up @@ -84,7 +86,8 @@ const mockDashboardData = {
meta: {
clusterId: '8f9c5b98-cc02-4827-8c82-316e2cc25870',
benchmarkName: 'CIS Kubernetes V1.20',
lastUpdate: 1653218903921,
lastUpdate: '2022-11-07T13:14:34.990Z',
benchmarkId: 'cis_k8s',
},
stats: {
totalFailed: 17,
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 React from 'react';
import { render } from '@testing-library/react';
import { expectIdsInDoc } from '../../../test/utils';
import { DASHBOARD_COUNTER_CARDS } from '../test_subjects';
import { CloudSummarySection } from './cloud_summary_section';
import { mockDashboardData } from '../compliance_dashboard.test';
import { TestProvider } from '../../../test/test_provider';
import { screen } from '@testing-library/react';

describe('<CloudSummarySection />', () => {
Copy link
Contributor

@ari-aviran ari-aviran Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd expect tests here to verify the rest of the functionality of the section (risks table, score chart, etc.) If you consider this out of scope please open a task

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i'll add those tests here on every new component task, its already listed in the tasks

const renderCloudSummarySection = (alterMockData = {}) => {
render(
<TestProvider>
<CloudSummarySection complianceData={{ ...mockDashboardData, ...alterMockData }} />
</TestProvider>
);
};

it('renders all counter cards', () => {
renderCloudSummarySection();

expectIdsInDoc({
be: [
DASHBOARD_COUNTER_CARDS.CLUSTERS_EVALUATED,
DASHBOARD_COUNTER_CARDS.RESOURCES_EVALUATED,
DASHBOARD_COUNTER_CARDS.FAILING_FINDINGS,
],
});
});

it('renders counters content according to mock', async () => {
renderCloudSummarySection();

expect(screen.getByTestId(DASHBOARD_COUNTER_CARDS.CLUSTERS_EVALUATED)).toHaveTextContent('1');
expect(screen.getByTestId(DASHBOARD_COUNTER_CARDS.RESOURCES_EVALUATED)).toHaveTextContent(
'162'
);
expect(screen.getByTestId(DASHBOARD_COUNTER_CARDS.FAILING_FINDINGS)).toHaveTextContent('17');
});

it('renders counters value in compact abbreviation if its above one million', () => {
renderCloudSummarySection({ stats: { resourcesEvaluated: 999999, totalFailed: 1000000 } });

expect(screen.getByTestId(DASHBOARD_COUNTER_CARDS.RESOURCES_EVALUATED)).toHaveTextContent(
'999,999'
);
expect(screen.getByTestId(DASHBOARD_COUNTER_CARDS.FAILING_FINDINGS)).toHaveTextContent('1M');
});

it('renders 0 as empty state', () => {
renderCloudSummarySection({ stats: { totalFailed: undefined } });

expect(screen.getByTestId(DASHBOARD_COUNTER_CARDS.FAILING_FINDINGS)).toHaveTextContent('0');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,22 @@
* 2.0.
*/

import React from 'react';
import React, { useMemo } from 'react';
import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { PartitionElementEvent } from '@elastic/charts';
import { i18n } from '@kbn/i18n';
import { FlexItemGrowSize } from '@elastic/eui/src/components/flex/flex_item';
import { DASHBOARD_COUNTER_CARDS } from '../test_subjects';
import { CspCounterCard, CspCounterCardProps } from '../../../components/csp_counter_card';
import { CompactFormattedNumber } from '../../../components/compact_formatted_number';
import { ChartPanel } from '../../../components/chart_panel';
import { CloudPostureScoreChart } from '../compliance_charts/cloud_posture_score_chart';
import type { ComplianceDashboardData, Evaluation } from '../../../../common/types';
import { RisksTable } from '../compliance_charts/risks_table';
import { useNavigateFindings } from '../../../common/hooks/use_navigate_findings';
import {
useNavigateFindings,
useNavigateFindingsByResource,
} from '../../../common/hooks/use_navigate_findings';
import { RULE_FAILED } from '../../../../common/constants';

const defaultHeight = 360;
Expand All @@ -36,6 +42,7 @@ export const CloudSummarySection = ({
complianceData: ComplianceDashboardData;
}) => {
const navToFindings = useNavigateFindings();
const navToFindingsByResource = useNavigateFindingsByResource();

const handleElementClick = (elements: PartitionElementEvent[]) => {
const [element] = elements;
Expand All @@ -56,9 +63,62 @@ export const CloudSummarySection = ({
navToFindings({ 'result.evaluation': RULE_FAILED });
};

const counters: CspCounterCardProps[] = useMemo(
Copy link
Contributor

@ari-aviran ari-aviran Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider just writing this in the declarative JSX instead of creating a variable and then using it:

<EuiFlexItem key={DASHBOARD_COUNTER_CARDS.CLUSTERS_EVALUATED}>
  <CspCounterCard
    counter={{
      id: DASHBOARD_COUNTER_CARDS.CLUSTERS_EVALUATED,
      title: <CompactFormattedNumber number={complianceData.clusters.length} />,
      description: i18n.translate(
        'xpack.csp.dashboard.summarySection.counterCard.clustersEvaluatedDescription',
        { defaultMessage: 'Clusters Evaluated' }
      ),
    }}
  />
</EuiFlexItem>
/* Rest of the cards */

I prefer it this way since you immediately "see" the layout when you look at the JSX, instead of jumping back and forth between counters definition and the JSX

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess its subjective, I prefer to have the JSX as clean as possible. "seeing" the layout is very important to me as well, I find it easier when things are clean and compact, I'm looking at it as a "list of counter cards" and keep the properties mess outside

() => [
{
id: DASHBOARD_COUNTER_CARDS.CLUSTERS_EVALUATED,
title: i18n.translate(
'xpack.csp.dashboard.summarySection.counterCard.clustersEvaluatedDescription',
{ defaultMessage: 'Clusters Evaluated' }
),
description: <CompactFormattedNumber number={complianceData.clusters.length} />,
},
{
id: DASHBOARD_COUNTER_CARDS.RESOURCES_EVALUATED,
title: i18n.translate(
'xpack.csp.dashboard.summarySection.counterCard.resourcesEvaluatedDescription',
{ defaultMessage: 'Resources Evaluated' }
),
description: (
<CompactFormattedNumber number={complianceData.stats.resourcesEvaluated || 0} />
),
onClick: () => {
navToFindingsByResource();
},
},
{
id: DASHBOARD_COUNTER_CARDS.FAILING_FINDINGS,
title: i18n.translate(
'xpack.csp.dashboard.summarySection.counterCard.failingFindingsDescription',
{ defaultMessage: 'Failing Findings' }
),
description: <CompactFormattedNumber number={complianceData.stats.totalFailed} />,
descriptionColor: complianceData.stats.totalFailed > 0 ? 'danger' : 'text',
onClick: () => {
navToFindings({ 'result.evaluation': RULE_FAILED });
},
},
],
[
complianceData.clusters.length,
complianceData.stats.resourcesEvaluated,
complianceData.stats.totalFailed,
navToFindings,
navToFindingsByResource,
]
);

return (
<EuiFlexGroup gutterSize="l" style={summarySectionWrapperStyle}>
<EuiFlexItem grow={dashboardColumnsGrow.first} />
<EuiFlexItem grow={dashboardColumnsGrow.first}>
<EuiFlexGroup direction="column">
{counters.map((counter) => (
<EuiFlexItem key={counter.id}>
<CspCounterCard {...counter} />
</EuiFlexItem>
))}
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem grow={dashboardColumnsGrow.second}>
<ChartPanel
title={i18n.translate('xpack.csp.dashboard.summarySection.cloudPostureScorePanelTitle', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,8 @@

export const MISSING_FINDINGS_NO_DATA_CONFIG = 'missing-findings-no-data-config';
export const DASHBOARD_CONTAINER = 'dashboard-container';
export const DASHBOARD_COUNTER_CARDS = {
CLUSTERS_EVALUATED: 'dashboard-counter-card-clusters-evaluated',
RESOURCES_EVALUATED: 'dashboard-counter-card-resources-evaluated',
FAILING_FINDINGS: 'dashboard-counter-card-failing-findings',
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { screen } from '@testing-library/react';

export const expectIdsInDoc = ({ be = [], notToBe = [] }: { be: string[]; notToBe: string[] }) => {
export const expectIdsInDoc = ({ be = [], notToBe = [] }: { be: string[]; notToBe?: string[] }) => {
be.forEach((testId) => {
expect(screen.getByTestId(testId)).toBeInTheDocument();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import {
} from './get_stats';

const standardQueryResult: FindingsEvaluationsQueryResult = {
resources_evaluated: {
value: 30,
},
failed_findings: {
doc_count: 30,
},
Expand All @@ -22,6 +25,9 @@ const standardQueryResult: FindingsEvaluationsQueryResult = {
};

const oneIsZeroQueryResult: FindingsEvaluationsQueryResult = {
resources_evaluated: {
value: 30,
},
failed_findings: {
doc_count: 0,
},
Expand All @@ -31,6 +37,9 @@ const oneIsZeroQueryResult: FindingsEvaluationsQueryResult = {
};

const bothAreZeroQueryResult: FindingsEvaluationsQueryResult = {
resources_evaluated: {
value: 30,
},
failed_findings: {
doc_count: 0,
},
Expand Down Expand Up @@ -66,6 +75,7 @@ describe('getStatsFromFindingsEvaluationsAggs', () => {
totalPassed: 11,
totalFindings: 41,
postureScore: 26.8,
resourcesEvaluated: 30,
});
});

Expand All @@ -76,6 +86,7 @@ describe('getStatsFromFindingsEvaluationsAggs', () => {
totalPassed: 11,
totalFindings: 11,
postureScore: 100.0,
resourcesEvaluated: 30,
});
});

Expand Down
Loading