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] [Quick wins] Enable edit DataView on the Misconfigurations Findings Table #173870

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,19 @@ export const useLatestFindingsDataView = (dataView: string) => {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

btw what use perfected kibana data views means (in the TODO for this hook)? not directly related to this PR, but I'd add more context to this todo, as I for instance wouldn't know what to do with it


if (dataView === LATEST_FINDINGS_INDEX_PATTERN) {
let shouldUpdate = false;
Object.entries(cloudSecurityFieldLabels).forEach(([field, label]) => {
if (
!dataViewObj.getFieldAttrs()[field]?.customLabel ||
dataViewObj.getFieldAttrs()[field]?.customLabel === field
) {
dataViewObj.setFieldCustomLabel(field, label);
shouldUpdate = true;
}
});
await dataViews.updateSavedObject(dataViewObj);
if (shouldUpdate) {
await dataViews.updateSavedObject(dataViewObj);
}
}

return dataViewObj;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ export interface CloudPostureTableResult {
getRowsFromPages: (data: Array<{ page: DataTableRecord[] }> | undefined) => DataTableRecord[];
}

/*
Hook for managing common table state and methods for Cloud Posture
*/
/**
* @deprecated will be replaced by useCloudPostureDataTable
*/
export const useCloudPostureTable = ({
defaultQuery = getDefaultQuery,
dataView,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export interface FindingsBaseURLQuery {

export interface FindingsBaseProps {
dataView: DataView;
dataViewRefetch?: () => void;
dataViewIsRefetching?: boolean;
}

export interface FindingsBaseESQueryConfig {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* 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 { render } from '@testing-library/react';
import React from 'react';
import { TestProvider } from '../../test/test_provider';
import { CloudSecurityDataTable, CloudSecurityDataTableProps } from './cloud_security_data_table';

const mockDataView = {
fields: {
getAll: () => [
{ id: 'field1', name: 'field1', customLabel: 'Label 1', visualizable: true },
{ id: 'field2', name: 'field2', customLabel: 'Label 2', visualizable: true },
],
getByName: (name: string) => ({ id: name }),
},
getFieldByName: (name: string) => ({ id: name }),
getFormatterForField: (name: string) => ({
convert: (value: string) => value,
}),
} as any;

const mockDefaultColumns = [{ id: 'field1' }, { id: 'field2' }];

const mockCloudPostureDataTable = {
setUrlQuery: jest.fn(),
columnsLocalStorageKey: 'test',
filters: [],
onSort: jest.fn(),
sort: [],
query: {},
queryError: undefined,
pageIndex: 0,
urlQuery: {},
setTableOptions: jest.fn(),
handleUpdateQuery: jest.fn(),
pageSize: 10,
setPageSize: jest.fn(),
onChangeItemsPerPage: jest.fn(),
onChangePage: jest.fn(),
onResetFilters: jest.fn(),
getRowsFromPages: jest.fn(),
} as any;

const renderDataTable = (props: Partial<CloudSecurityDataTableProps> = {}) => {
const defaultProps: CloudSecurityDataTableProps = {
dataView: mockDataView,
isLoading: false,
defaultColumns: mockDefaultColumns,
rows: [],
total: 0,
flyoutComponent: () => <></>,
cloudPostureDataTable: mockCloudPostureDataTable,
loadMore: jest.fn(),
title: 'Test Table',
};

return render(
<TestProvider>
<CloudSecurityDataTable {...defaultProps} {...props} />
</TestProvider>
);
};

describe('CloudSecurityDataTable', () => {
it('renders loading state', () => {
const { getByTestId } = renderDataTable({ isLoading: true });
expect(getByTestId('unifiedDataTableLoading')).toBeInTheDocument();
});

it('renders empty state when no rows are present', () => {
const { getByTestId } = renderDataTable();
expect(getByTestId('csp:empty-state')).toBeInTheDocument();
});

it('renders data table with rows', async () => {
const mockRows = [
{
id: '1',
raw: {
field1: 'Label 1',
field2: 'Label 2',
},
flattened: {
field1: 'Label 1',
field2: 'Label 2',
},
},
] as any;
const { getByTestId, getByText } = renderDataTable({
rows: mockRows,
total: mockRows.length,
});

expect(getByTestId('discoverDocTable')).toBeInTheDocument();
expect(getByText('Label 1')).toBeInTheDocument();
expect(getByText('Label 2')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { generateFilters } from '@kbn/data-plugin/public';
import { DocViewFilterFn } from '@kbn/unified-doc-viewer/types';
import useLocalStorage from 'react-use/lib/useLocalStorage';
import { useKibana } from '../../common/hooks/use_kibana';
import { CloudPostureTableResult } from '../../common/hooks/use_cloud_posture_table';
import { CloudPostureDataTableResult } from '../../common/hooks/use_cloud_posture_data_table';
import { EmptyState } from '../empty_state';
import { MAX_FINDINGS_TO_LOAD } from '../../common/constants';
import { useStyles } from './use_styles';
Expand All @@ -40,7 +40,7 @@ const useNewFieldsApi = true;
// Hide Checkbox, enable open details Flyout
const controlColumnIds = ['openDetails'];

interface CloudSecurityDataGridProps {
export interface CloudSecurityDataTableProps {
dataView: DataView;
isLoading: boolean;
defaultColumns: CloudSecurityDefaultColumn[];
Expand All @@ -52,10 +52,10 @@ interface CloudSecurityDataGridProps {
*/
flyoutComponent: (hit: DataTableRecord, onCloseFlyout: () => void) => JSX.Element;
/**
* This is the object that contains all the data and functions from the useCloudPostureTable hook.
* This is the object that contains all the data and functions from the useCloudPostureDataTable hook.
* This is also used to manage the table state from the parent component.
*/
cloudPostureTable: CloudPostureTableResult;
cloudPostureDataTable: CloudPostureDataTableResult;
title: string;
/**
* This is a function that returns a map of column ids to custom cell renderers.
Expand All @@ -78,6 +78,16 @@ interface CloudSecurityDataGridProps {
* Height override for the data grid.
*/
height?: number;
/**
* Callback Function when the DataView field is edited.
* Required to enable editing of the field in the data grid.
*/
dataViewRefetch?: () => void;
/**
* Flag to indicate if the data view is refetching.
* Required for smoothing re-rendering the DataTable columns.
*/
dataViewIsRefetching?: boolean;
}

export const CloudSecurityDataTable = ({
Expand All @@ -87,14 +97,16 @@ export const CloudSecurityDataTable = ({
rows,
total,
flyoutComponent,
cloudPostureTable,
cloudPostureDataTable,
loadMore,
title,
customCellRenderer,
groupSelectorComponent,
height,
dataViewRefetch,
dataViewIsRefetching,
...rest
}: CloudSecurityDataGridProps) => {
}: CloudSecurityDataTableProps) => {
const {
columnsLocalStorageKey,
pageSize,
Expand All @@ -104,7 +116,7 @@ export const CloudSecurityDataTable = ({
onResetFilters,
filters,
sort,
} = cloudPostureTable;
} = cloudPostureDataTable;

const [columns, setColumns] = useLocalStorage(
columnsLocalStorageKey,
Expand Down Expand Up @@ -237,6 +249,9 @@ export const CloudSecurityDataTable = ({
opacity: isLoading ? 1 : 0,
};

const loadingState =
isLoading || dataViewIsRefetching ? DataLoadingState.loading : DataLoadingState.loaded;

return (
<CellActionsProvider getTriggerCompatibleActions={uiActions.getTriggerCompatibleActions}>
<div
Expand All @@ -251,7 +266,7 @@ export const CloudSecurityDataTable = ({
columns={currentColumns}
expandedDoc={expandedDoc}
dataView={dataView}
loadingState={isLoading ? DataLoadingState.loading : DataLoadingState.loaded}
loadingState={loadingState}
onFilter={onAddFilter as DocViewFilterFn}
onResize={onResize}
onSetColumns={onSetColumns}
Expand All @@ -276,6 +291,7 @@ export const CloudSecurityDataTable = ({
gridStyleOverride={gridStyle}
rowLineHeightOverride="24px"
controlColumnIds={controlColumnIds}
onFieldEdited={dataViewRefetch}
/>
</div>
</CellActionsProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,13 @@ import { render } from '@testing-library/react';
import { expectIdsInDoc } from '../../test/utils';
import { PACKAGE_NOT_INSTALLED_TEST_SUBJECT } from '../../components/cloud_posture_page';
import { useLicenseManagementLocatorApi } from '../../common/api/use_license_management_locator_api';
import { useCloudPostureTable } from '../../common/hooks/use_cloud_posture_table';

jest.mock('../../common/api/use_latest_findings_data_view');
jest.mock('../../common/api/use_setup_status_api');
jest.mock('../../common/api/use_license_management_locator_api');
jest.mock('../../common/hooks/use_subscription_status');
jest.mock('../../common/navigation/use_navigate_to_cis_integration_policies');
jest.mock('../../common/navigation/use_csp_integration_link');
jest.mock('../../common/hooks/use_cloud_posture_table');

const chance = new Chance();

Expand All @@ -54,13 +52,6 @@ beforeEach(() => {
data: true,
})
);

(useCloudPostureTable as jest.Mock).mockImplementation(() => ({
getRowsFromPages: jest.fn(),
columnsLocalStorageKey: 'test',
filters: [],
sort: [],
}));
});

const renderFindingsPage = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ export const Configurations = () => {
path={findingsNavigation.findings_default.path}
render={() => (
<TrackApplicationView viewId={findingsNavigation.findings_default.id}>
<LatestFindingsContainer dataView={dataViewQuery.data!} />
<LatestFindingsContainer
dataView={dataViewQuery.data!}
Copy link
Contributor

Choose a reason for hiding this comment

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

Couple of general questions:

  • any downside in passing dataViewQuery to avoid having these three props being passed together all the time?
  • we don't use context that much in our code, wdyt about the context for the dataView to avoid this multi layer prop drill?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

any downside in passing dataViewQuery to avoid having these three props being passed together all the time?

IMO the downside is that we add a deep coupling with an object from the react-query third-party library which is an implementation detail, then we need to mock the library to be able to unit test it, and these tests won't be so reliable in case something break in future releases of the library.

we don't use context that much in our code, wdyt about the context for the dataView to avoid this multi layer prop drill?

that's a good suggestion, better than passing down a react-query object, I created a technical debt ticket to address that in a follow up PR

dataViewRefetch={dataViewQuery.refetch}
dataViewIsRefetching={dataViewQuery.isRefetching}
/>
</TrackApplicationView>
)}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ import { groupPanelRenderer, groupStatsRenderer } from './latest_findings_group_
import { FindingsDistributionBar } from '../layout/findings_distribution_bar';
import { ErrorCallout } from '../layout/error_callout';

export const LatestFindingsContainer = ({ dataView }: FindingsBaseProps) => {
export const LatestFindingsContainer = ({
dataView,
dataViewRefetch,
dataViewIsRefetching,
}: FindingsBaseProps) => {
const renderChildComponent = useCallback(
(groupFilters: Filter[]) => {
return (
Expand All @@ -27,10 +31,12 @@ export const LatestFindingsContainer = ({ dataView }: FindingsBaseProps) => {
nonPersistedFilters={groupFilters}
height={DEFAULT_TABLE_HEIGHT}
showDistributionBar={false}
dataViewRefetch={dataViewRefetch}
dataViewIsRefetching={dataViewIsRefetching}
/>
);
},
[dataView]
[dataView, dataViewIsRefetching, dataViewRefetch]
);

const {
Expand Down Expand Up @@ -94,7 +100,12 @@ export const LatestFindingsContainer = ({ dataView }: FindingsBaseProps) => {
return (
<>
<FindingsSearchBar dataView={dataView} setQuery={setUrlQuery} loading={isFetching} />
<LatestFindingsTable dataView={dataView} groupSelectorComponent={grouping.groupSelector} />
<LatestFindingsTable
dataView={dataView}
groupSelectorComponent={grouping.groupSelector}
dataViewRefetch={dataViewRefetch}
dataViewIsRefetching={dataViewIsRefetching}
/>
</>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ type LatestFindingsTableProps = FindingsBaseProps & {
height?: number;
showDistributionBar?: boolean;
nonPersistedFilters?: Filter[];
dataViewRefetch?: () => void;
dataViewIsRefetching?: boolean;
};

/**
Expand Down Expand Up @@ -87,9 +89,11 @@ export const LatestFindingsTable = ({
height,
showDistributionBar = true,
nonPersistedFilters,
dataViewRefetch,
dataViewIsRefetching,
}: LatestFindingsTableProps) => {
const {
cloudPostureTable,
cloudPostureDataTable,
rows,
error,
isFetching,
Expand Down Expand Up @@ -134,12 +138,14 @@ export const LatestFindingsTable = ({
rows={rows}
total={total}
flyoutComponent={flyoutComponent}
cloudPostureTable={cloudPostureTable}
cloudPostureDataTable={cloudPostureDataTable}
loadMore={fetchNextPage}
title={title}
customCellRenderer={customCellRenderer}
groupSelectorComponent={groupSelectorComponent}
height={height}
dataViewRefetch={dataViewRefetch}
dataViewIsRefetching={dataViewIsRefetching}
/>
</>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ export const useLatestFindingsTable = ({
nonPersistedFilters?: Filter[];
showDistributionBar?: boolean;
}) => {
const cloudPostureTable = useCloudPostureDataTable({
const cloudPostureDataTable = useCloudPostureDataTable({
dataView,
paginationLocalStorageKey: LOCAL_STORAGE_DATA_TABLE_PAGE_SIZE_KEY,
columnsLocalStorageKey,
defaultQuery: getDefaultQuery,
nonPersistedFilters,
});

const { query, sort, queryError, setUrlQuery, filters, getRowsFromPages } = cloudPostureTable;
const { query, sort, queryError, setUrlQuery, filters, getRowsFromPages } = cloudPostureDataTable;

const {
data,
Expand Down Expand Up @@ -72,7 +72,7 @@ export const useLatestFindingsTable = ({
const canShowDistributionBar = showDistributionBar && total > 0;

return {
cloudPostureTable,
cloudPostureDataTable,
rows,
error,
isFetching,
Expand Down