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.

Sign up for GitHub

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] add pagination to resource findings table #131422

Merged
merged 8 commits into from
May 16, 2022
Merged

[Cloud Posture] add pagination to resource findings table #131422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7da71cf
add pagination
orouz May 3, 2022
ca39595
clean
orouz May 3, 2022
262d326
fix i18n
orouz May 3, 2022
c4749dc
fix pagination
orouz May 4, 2022
3e5ccd7
rename
orouz May 4, 2022
1615bf0
fixes
orouz May 12, 2022
7c737b1
clean
orouz May 15, 2022
7360740
use old pagination counter
orouz May 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion x-pack/plugins/cloud_security_posture/public/common/hooks/use_url_query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export const useUrlQuery = <T extends object>(getDefaultQuery: () => T) => {

// Set initial query
useEffect(() => {
// TODO: condition should be if decoding failed
if (search) return;

replace({ search: encodeQuery(getDefaultQuery() as RisonObject) });
Expand Down
4 changes: 2 additions & 2 deletions ...security_posture/public/pages/findings/latest_findings/latest_findings_container.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { useLocation } from 'react-router-dom';
import { RisonObject } from 'rison-node';
import { buildEsQuery } from '@kbn/es-query';
import { getFindingsCountAggQuery } from '../use_findings_count';
import { getPaginationQuery } from '../utils';

jest.mock('../../../common/api/use_latest_findings_data_view');
jest.mock('../../../common/api/use_cis_kubernetes_integration');
Expand Down Expand Up @@ -69,9 +70,8 @@ describe('<LatestFindingsContainer />', () => {
expect(dataMock.search.search).toHaveBeenNthCalledWith(2, {
params: getFindingsQuery({
...baseQuery,
...getPaginationQuery(query),
sort: query.sort,
size: query.size,
from: query.from,
}),
});
});
Expand Down
50 changes: 30 additions & 20 deletions ...loud_security_posture/public/pages/findings/latest_findings/latest_findings_container.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import React, { useMemo } from 'react';
import { EuiSpacer } from '@elastic/eui';
import type { DataView } from '@kbn/data-plugin/common';
import { SortDirection } from '@kbn/data-plugin/common';
import { FormattedMessage } from '@kbn/i18n-react';
import { FindingsTable } from './latest_findings_table';
import { FindingsSearchBar } from '../layout/findings_search_bar';
Expand All @@ -18,7 +17,7 @@ import type { FindingsGroupByNoneQuery } from './use_latest_findings';
import type { FindingsBaseURLQuery } from '../types';
import { useFindingsCounter } from '../use_findings_count';
import { FindingsDistributionBar } from '../layout/findings_distribution_bar';
import { getBaseQuery } from '../utils';
import { getBaseQuery, getPaginationQuery, getPaginationTableParams } from '../utils';
import { PageWrapper, PageTitle, PageTitleText } from '../layout/findings_layout';
import { FindingsGroupBySelector } from '../layout/findings_group_by_selector';
import { useCspBreadcrumbs } from '../../../common/navigation/use_csp_breadcrumbs';
Expand All @@ -27,14 +26,15 @@ import { findingsNavigation } from '../../../common/navigation/constants';
export const getDefaultQuery = (): FindingsBaseURLQuery & FindingsGroupByNoneQuery => ({
query: { language: 'kuery', query: '' },
filters: [],
sort: [{ ['@timestamp']: SortDirection.desc }],
from: 0,
size: 10,
sort: { field: '@timestamp', direction: 'desc' },
pageIndex: 0,
pageSize: 10,
});

export const LatestFindingsContainer = ({ dataView }: { dataView: DataView }) => {
useCspBreadcrumbs([findingsNavigation.findings_default]);
const { urlQuery, setUrlQuery } = useUrlQuery(getDefaultQuery);

const baseEsQuery = useMemo(
() => getBaseQuery({ dataView, filters: urlQuery.filters, query: urlQuery.query }),
[dataView, urlQuery.filters, urlQuery.query]
Expand All @@ -43,8 +43,7 @@ export const LatestFindingsContainer = ({ dataView }: { dataView: DataView }) =>
const findingsCount = useFindingsCounter(baseEsQuery);
const findingsGroupByNone = useLatestFindings({
...baseEsQuery,
size: urlQuery.size,
from: urlQuery.from,
...getPaginationQuery({ pageIndex: urlQuery.pageIndex, pageSize: urlQuery.pageSize }),
sort: urlQuery.sort,
});

Expand All @@ -55,33 +54,44 @@ export const LatestFindingsContainer = ({ dataView }: { dataView: DataView }) =>
setQuery={setUrlQuery}
query={urlQuery.query}
filters={urlQuery.filters}
loading={findingsGroupByNone.isLoading}
loading={findingsGroupByNone.isFetching}
/>
<PageWrapper>
<PageTitle>
<PageTitleText
title={
<FormattedMessage id="xpack.csp.findings.findingsTitle" defaultMessage="Findings" />
}
/>
</PageTitle>
<LatestFindingsPageTitle />
<FindingsGroupBySelector type="default" />
<FindingsDistributionBar
total={findingsGroupByNone.data?.total || 0}
passed={findingsCount.data?.passed || 0}
failed={findingsCount.data?.failed || 0}
pageStart={urlQuery.from + 1} // API index is 0, but UI is 1
pageEnd={urlQuery.from + urlQuery.size}
pageStart={urlQuery.pageIndex * urlQuery.pageSize + 1} // API index is 0, but UI is 1
pageEnd={urlQuery.pageIndex * urlQuery.pageSize + urlQuery.pageSize}
orouz marked this conversation as resolved.
Show resolved Hide resolved
/>
<EuiSpacer />
<FindingsTable
{...urlQuery}
setQuery={setUrlQuery}
data={findingsGroupByNone.data}
error={findingsGroupByNone.error}
loading={findingsGroupByNone.isLoading}
loading={findingsGroupByNone.isFetching}
pagination={getPaginationTableParams({
pageSize: urlQuery.pageSize,
pageIndex: urlQuery.pageIndex,
totalItemCount: findingsGroupByNone.data?.total || 0,
})}
sorting={{
sort: { field: urlQuery.sort.field, direction: urlQuery.sort.direction },
}}
setTableOptions={({ page, sort }) =>
setUrlQuery({ pageIndex: page.index, pageSize: page.size, sort })
}
/>
</PageWrapper>
</div>
);
};

const LatestFindingsPageTitle = () => (
<PageTitle>
<PageTitleText
title={<FormattedMessage id="xpack.csp.findings.findingsTitle" defaultMessage="Findings" />}
/>
</PageTitle>
);
Comment on lines +91 to +97
Copy link
Contributor

Choose a reason for hiding this comment

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

nit but I'm not sure why this static component needs to be separated. I would just leave it inside the page component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably subjective, but IMO it makes the root component read nicer as we don't look at long lines that aren't really interesting. LatestFindingsPageTitle tells me all i need to know without looking how it's done

14 changes: 6 additions & 8 deletions ...oud_security_posture/public/pages/findings/latest_findings/latest_findings_table.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,9 @@ describe('<FindingsTable />', () => {
loading: false,
data: { page: [], total: 0 },
error: null,
sort: [],
from: 1,
size: 10,
setQuery: jest.fn,
sorting: { sort: { field: '@timestamp', direction: 'desc' } },
pagination: { pageSize: 10, pageIndex: 1, totalItemCount: 0 },
setTableOptions: jest.fn(),
};

render(
Expand All @@ -93,10 +92,9 @@ describe('<FindingsTable />', () => {
loading: false,
data: { page: data, total: 10 },
error: null,
sort: [],
from: 0,
size: 10,
setQuery: jest.fn,
sorting: { sort: { field: '@timestamp', direction: 'desc' } },
pagination: { pageSize: 10, pageIndex: 1, totalItemCount: 0 },
setTableOptions: jest.fn(),
};

render(
Expand Down
82 changes: 16 additions & 66 deletions ...ns/cloud_security_posture/public/pages/findings/latest_findings/latest_findings_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,41 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import React, { useCallback, useMemo, useState } from 'react';
import React, { useMemo, useState } from 'react';
import {
type Criteria,
EuiEmptyPrompt,
EuiBasicTable,
EuiBasicTableProps,
EuiBasicTableColumn,
type Pagination,
type EuiBasicTableProps,
type CriteriaWithPagination,
type EuiTableActionsColumnType,
} from '@elastic/eui';
import { SortDirection } from '@kbn/data-plugin/common';
import { EuiTableActionsColumnType } from '@elastic/eui/src/components/basic_table/table_types';
import { extractErrorMessage } from '../../../../common/utils/helpers';
import * as TEST_SUBJECTS from '../test_subjects';
import * as TEXT from '../translations';
import type { CspFinding } from '../types';
import type { FindingsGroupByNoneQuery, CspFindingsResult } from './use_latest_findings';
import type { CspFindingsResult } from './use_latest_findings';
import { FindingsRuleFlyout } from '../findings_flyout/findings_flyout';
import { getExpandColumn, getFindingsColumns } from '../layout/findings_layout';

interface BaseFindingsTableProps extends FindingsGroupByNoneQuery {
setQuery(query: Partial<FindingsGroupByNoneQuery>): void;
type TableProps = Required<EuiBasicTableProps<CspFinding>>;

interface BaseFindingsTableProps {
pagination: Pagination;
sorting: TableProps['sorting'];
setTableOptions(options: CriteriaWithPagination<CspFinding>): void;
}

type FindingsTableProps = CspFindingsResult & BaseFindingsTableProps;

const FindingsTableComponent = ({
setQuery,
from,
size,
sort = [],
error,
data,
loading,
pagination,
sorting,
setTableOptions,
}: FindingsTableProps) => {
const [selectedFinding, setSelectedFinding] = useState<CspFinding>();

Expand All @@ -47,25 +50,6 @@ const FindingsTableComponent = ({
[]
);

const pagination = useMemo(
() =>
getEuiPaginationFromEsSearchSource({
from,
size,
total: data?.total,
}),
[from, size, data]
);

const sorting = useMemo(() => getEuiSortFromEsSearchSource(sort), [sort]);

const onTableChange = useCallback(
(params: Criteria<CspFinding>) => {
setQuery(getEsSearchQueryFromEuiTableParams(params));
},
[setQuery]
);

// Show "zero state"
if (!loading && !data?.page.length)
// TODO: use our own logo
Expand All @@ -87,7 +71,7 @@ const FindingsTableComponent = ({
columns={columns}
pagination={pagination}
sorting={sorting}
onChange={onTableChange}
onChange={setTableOptions}
hasActions
/>
{selectedFinding && (
Expand All @@ -100,38 +84,4 @@ const FindingsTableComponent = ({
);
};

const getEuiPaginationFromEsSearchSource = ({
from: pageIndex,
size: pageSize,
total,
}: Pick<FindingsTableProps, 'from' | 'size'> & {
total: number | undefined;
}): EuiBasicTableProps<CspFinding>['pagination'] => ({
pageSize,
pageIndex: Math.ceil(pageIndex / pageSize),
totalItemCount: total || 0,
pageSizeOptions: [10, 25, 100],
showPerPageOptions: true,
});

const getEuiSortFromEsSearchSource = (
sort: FindingsGroupByNoneQuery['sort']
): EuiBasicTableProps<CspFinding>['sorting'] => {
if (!sort.length) return;

const entry = Object.entries(sort[0])?.[0];
if (!entry) return;

const [field, direction] = entry;
return { sort: { field: field as keyof CspFinding, direction: direction as SortDirection } };
};

const getEsSearchQueryFromEuiTableParams = ({
page,
sort,
}: Criteria<CspFinding>): Partial<FindingsGroupByNoneQuery> => ({
...(!!page && { from: page.index * page.size, size: page.size }),
sort: sort ? [{ [sort.field]: SortDirection[sort.direction] }] : undefined,
});

export const FindingsTable = React.memo(FindingsTableComponent);
36 changes: 13 additions & 23 deletions ...ugins/cloud_security_posture/public/pages/findings/latest_findings/use_latest_findings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,28 @@
import { useQuery } from 'react-query';
import { number } from 'io-ts';
import { lastValueFrom } from 'rxjs';
import type { EsQuerySortValue, IEsSearchResponse } from '@kbn/data-plugin/common';
import type { IEsSearchResponse } from '@kbn/data-plugin/common';
import type { CoreStart } from '@kbn/core/public';
import type { Criteria, Pagination } from '@elastic/eui';
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { extractErrorMessage } from '../../../../common/utils/helpers';
import * as TEXT from '../translations';
import type { CspFinding, FindingsQueryResult } from '../types';
import { useKibana } from '../../../common/hooks/use_kibana';
import type { FindingsBaseEsQuery } from '../types';

interface UseFindingsOptions extends FindingsBaseEsQuery, FindingsGroupByNoneQuery {}

export interface FindingsGroupByNoneQuery {
interface UseFindingsOptions extends FindingsBaseEsQuery {
from: NonNullable<estypes.SearchRequest['from']>;
size: NonNullable<estypes.SearchRequest['size']>;
sort: EsQuerySortValue[];
sort: Sort;
}

type Sort = NonNullable<Criteria<CspFinding>['sort']>;

export interface FindingsGroupByNoneQuery {
pageIndex: Pagination['pageIndex'];
pageSize: Pagination['pageSize'];
sort: Sort;
}

interface CspFindingsData {
Expand All @@ -37,23 +44,6 @@ const FIELDS_WITHOUT_KEYWORD_MAPPING = new Set(['@timestamp']);
const getSortKey = (key: string): string =>
FIELDS_WITHOUT_KEYWORD_MAPPING.has(key) ? key : `${key}.keyword`;

/**
* @description utility to transform a column header key to its field mapping for sorting
* @example Adds '.keyword' to every property we sort on except values of `FIELDS_WITHOUT_KEYWORD_MAPPING`
* @todo find alternative
* @note we choose the keyword 'keyword' in the field mapping
*/
const mapEsQuerySortKey = (sort: readonly EsQuerySortValue[]): EsQuerySortValue[] =>
sort.slice().reduce<EsQuerySortValue[]>((acc, cur) => {
const entry = Object.entries(cur)[0];
if (!entry) return acc;

const [k, v] = entry;
acc.push({ [getSortKey(k)]: v });

return acc;
}, []);

export const showErrorToast = (
toasts: CoreStart['notifications']['toasts'],
error: unknown
Expand All @@ -67,7 +57,7 @@ export const getFindingsQuery = ({ index, query, size, from, sort }: UseFindings
query,
size,
from,
sort: mapEsQuerySortKey(sort),
sort: [{ [getSortKey(sort.field)]: sort.direction }],
});

export const useLatestFindings = ({ index, query, sort, from, size }: UseFindingsOptions) => {
Expand Down
Loading