From 35790b11685e80e33d97ceede71240fdc32b8f98 Mon Sep 17 00:00:00 2001 From: Or Ouziel Date: Wed, 29 Dec 2021 10:52:17 +0200 Subject: [PATCH] Findings polish iteration (#69) --- .../public/pages/findings/findings.test.tsx | 4 +-- .../pages/findings/findings_container.tsx | 6 ++-- .../public/pages/findings/findings_flyout.tsx | 4 +-- .../pages/findings/findings_search_bar.tsx | 4 --- .../public/pages/findings/findings_table.tsx | 32 ++++++++++--------- .../public/pages/findings/utils.tsx | 10 ++---- 6 files changed, 26 insertions(+), 34 deletions(-) diff --git a/x-pack/plugins/cloud_security_posture/public/pages/findings/findings.test.tsx b/x-pack/plugins/cloud_security_posture/public/pages/findings/findings.test.tsx index 78933d6c63423..af164764d4675 100644 --- a/x-pack/plugins/cloud_security_posture/public/pages/findings/findings.test.tsx +++ b/x-pack/plugins/cloud_security_posture/public/pages/findings/findings.test.tsx @@ -35,9 +35,9 @@ const FindingsComponentWithTestProvider = () => { ); }; -describe('Test findings page conditional rendering', () => { +describe('', () => { it("renders the error state component when 'kubebeat' DataView doesn't exists", async () => { - spy.mockImplementation(() => ({ status: 'success', data: undefined } as any)); + spy.mockImplementation(() => ({ status: 'success' } as UseQueryResult)); render(); diff --git a/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_container.tsx b/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_container.tsx index ced37e047f520..513713921d2e4 100644 --- a/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_container.tsx +++ b/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_container.tsx @@ -45,8 +45,8 @@ const getDefaultQuery = (): Required => ({ }, }); -// with https://github.com/microsoft/TypeScript/pull/46266 -// destructuring would make this more concise and won't leak props +// TODO(TS 4.6): destructure {status, error, data} to make this more concise without losing types +// see with https://github.com/microsoft/TypeScript/pull/46266 export const getFetchState = (v: T): FindingsFetchState => { switch (v.status) { case 'error': @@ -77,7 +77,7 @@ export const FindingsTableContainer = ({ dataView }: { dataView: DataView }) => // This sends a new search request to ES // it's called whenever we have a new searchState from the URL useEffect(() => { - mutation.mutate(void 0, { + mutation.mutate(undefined, { onError: (e) => { notifications?.toasts.addError(e instanceof Error ? e : new Error(), { title: 'Search failed', diff --git a/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_flyout.tsx b/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_flyout.tsx index e375b32e849eb..592fe6dac0ac8 100644 --- a/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_flyout.tsx +++ b/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_flyout.tsx @@ -42,7 +42,6 @@ interface FindingFlyoutProps { findings: CSPFinding; } -// TODO: fix scrollbar jumps export const FindingsRuleFlyout = ({ onClose, findings }: FindingFlyoutProps) => { const [tab, setTab] = useState('result'); return ( @@ -98,8 +97,9 @@ const FindingsTab = ({ tab, findings }: { findings: CSPFinding; tab: FindingsTab return ; case 'resource': return ; + default: + assertNever(tab); } - assertNever(tab); }; const getResourceCards = ({ resource }: CSPFinding): Card[] => [ diff --git a/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_search_bar.tsx b/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_search_bar.tsx index 48379e0d11484..ef3d1135fae16 100644 --- a/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_search_bar.tsx +++ b/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_search_bar.tsx @@ -24,10 +24,6 @@ interface BaseFindingsSearchBarProps { type FindingsSearchBarProps = FindingsFetchState & BaseFindingsSearchBarProps; -/** - * Temporary Search Bar using x-pack/plugins/data - * - */ export const FindingsSearchBar = ({ dataView, dateRange, diff --git a/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_table.tsx b/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_table.tsx index 93479f0cada7d..6c979e1e75d5c 100644 --- a/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_table.tsx +++ b/x-pack/plugins/cloud_security_posture/public/pages/findings/findings_table.tsx @@ -4,7 +4,7 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import React, { useMemo, useState } from 'react'; +import React, { useState, useMemo } from 'react'; import { Criteria, EuiLink, @@ -28,13 +28,9 @@ interface BaseFindingsTableProps { type FindingsTableProps = FindingsFetchState & BaseFindingsTableProps; -/** - * Temporary findings table - */ export const FindingsTable = ({ data, status, error, selectItem }: FindingsTableProps) => { const [pageIndex, setPageIndex] = useState(0); const [pageSize, setPageSize] = useState(25); - const columns = useMemo(getColumns, []); const getCellProps = (item: CSPFinding, column: EuiTableFieldDataColumnType) => ({ onClick: column.field === 'rule.name' ? () => selectItem(item) : undefined, @@ -48,6 +44,15 @@ export const FindingsTable = ({ data, status, error, selectItem }: FindingsTable setPageSize(size); }; + const page = useMemo( + () => + orderBy(data, ['@timestamp'], ['desc']).slice( + pageIndex * pageSize, + pageSize * pageIndex + pageSize + ), + [data, pageSize, pageIndex] + ); + // TODO: add empty/error/loading views if (!data) return null; @@ -60,9 +65,6 @@ export const FindingsTable = ({ data, status, error, selectItem }: FindingsTable hidePerPageOptions: false, }; - const sortedData = orderBy(data, ['@timestamp'], ['desc']); - const page = sortedData.slice(pageIndex * pageSize, pageSize * pageIndex + pageSize); - return ( {name}; -const RuleTags = (tags: string[]) => ( +const ruleNameRenderer = (name: string) => {name}; +const ruleTagsRenderer = (tags: string[]) => ( @@ -92,11 +94,11 @@ const RuleTags = (tags: string[]) => ( ); -const ResultEvaluation = (type: PropsOf['type']) => ( +const resultEvaluationRenderer = (type: PropsOf['type']) => ( ); -const getColumns = (): Array> => [ +const columns: Array> = [ { field: 'resource.filename', name: 'Resource', @@ -107,18 +109,18 @@ const getColumns = (): Array> => [ name: 'Rule Name', width: '50%', truncateText: true, - render: RuleName, + render: ruleNameRenderer, }, { field: 'result.evaluation', name: 'Evaluation', width: '80px', - render: ResultEvaluation, + render: resultEvaluationRenderer, }, { field: 'rule.tags', name: 'Tags', - render: RuleTags, + render: ruleTagsRenderer, }, { field: '@timestamp', diff --git a/x-pack/plugins/cloud_security_posture/public/pages/findings/utils.tsx b/x-pack/plugins/cloud_security_posture/public/pages/findings/utils.tsx index 8d16e595c5fe7..2bbb1864d7908 100644 --- a/x-pack/plugins/cloud_security_posture/public/pages/findings/utils.tsx +++ b/x-pack/plugins/cloud_security_posture/public/pages/findings/utils.tsx @@ -19,17 +19,14 @@ import type { CspPluginSetup } from '../../types'; import { CSP_KUBEBEAT_INDEX_PATTERN } from '../../../common/constants'; import { useKibana } from '../../../../../../src/plugins/kibana_react/public'; -// TODO: find similar/existing function -export const extractErrorMessage = (e: unknown) => +export const extractErrorMessage = (e: unknown): string => typeof e === 'string' ? e : (e as Error)?.message || 'Unknown Error'; -// TODO: find kibanas' equivalent fn export const isNonNullable = (v: T): v is NonNullable => v !== null && v !== undefined; /** - * Temp DataView Utility - * registers a kibana data view for kubebeat* index + * registers a kibana data view for kubebeat* index and fetches it * TODO: use perfected kibana data views */ @@ -55,9 +52,6 @@ export const useKubebeatDataView = () => { return useQuery(['kubebeat_dataview'], getKubebeatDataView); }; -/** - * Temp URL state utility - */ export const useSourceQueryParam = (getDefaultQuery: () => T) => { const history = useHistory(); const [state, set] = useState(getDefaultQuery());