From a374ed746e301f7071eff3bf933ffe30f9fb0a54 Mon Sep 17 00:00:00 2001 From: Jatin Kathuria Date: Fri, 29 Nov 2024 16:14:27 +0100 Subject: [PATCH 1/2] [Security Solution] Adds callback `onUpdatePageIndex` to get current `pageIndex` in Unified Data table (#201240) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Handles resolution for - Notes fetching data for all Timeline Records which leads to performance issues. - https://github.com/elastic/kibana/issues/201330 ## Issue - Notes fetching data for all Timeline Records Currently, there was no way for consumer of `UnifiedDataGrid` to get the current `pageIndex`. Security Solution needs to get the current `pageIndex` so the items on the current page can be calculated. @elastic/kibana-data-discovery , please let us know if you have any opinion here. This results in notes being fetched for all Timeline Records which means minimum of 500 records and if user has queries 5000 records ( for example ), a request will be made to query notes for all those 5000 notes which leads to performance issue and sometimes error as shown below: ![image](https://github.com/user-attachments/assets/6fcfe05d-340c-4dcb-a273-5af53ed12945) ## 👨‍💻 Changes This adds attribute `pageIndex` to timeline state. ```javascript { "pageIndex": number } ``` `pageIndex` helps with getting the events for that particular page. ## 🟡 Caveat - Currently this `pageIndex` is shared between Query and EQL tabs which can lead to wonky behavior at time. - Additionally, as of now table maintains its own page index and consumer component cannot effect the `pageIndex` of the UnifiedDataGrid. (cherry picked from commit de9d5465df5900936991d79306cb2cbbe63f4623) # Conflicts: # x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx # x-pack/plugins/security_solution/public/timelines/containers/index.test.tsx --- .../src/components/data_table.test.tsx | 60 ++++ .../src/components/data_table.tsx | 59 ++-- .../common/types/timeline/store.ts | 2 +- .../body/unified_timeline_body.test.tsx | 42 +-- .../timeline/body/unified_timeline_body.tsx | 28 +- .../timelines/components/timeline/events.ts | 2 +- .../timeline/tabs/eql/index.test.tsx | 266 +++++++++++++++--- .../components/timeline/tabs/eql/index.tsx | 68 +++-- .../components/timeline/tabs/pinned/index.tsx | 37 ++- .../timeline/tabs/query/index.test.tsx | 128 +++++++++ .../components/timeline/tabs/query/index.tsx | 36 ++- .../data_table/index.test.tsx | 2 +- .../unified_components/data_table/index.tsx | 13 +- .../unified_components/index.test.tsx | 3 +- .../timeline/unified_components/index.tsx | 13 +- .../timelines/containers/index.test.tsx | 162 ++++++----- .../public/timelines/containers/index.tsx | 69 ++++- 17 files changed, 738 insertions(+), 252 deletions(-) diff --git a/packages/kbn-unified-data-table/src/components/data_table.test.tsx b/packages/kbn-unified-data-table/src/components/data_table.test.tsx index f440c2845adaa..3ee4e5a9e7a13 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.test.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.test.tsx @@ -1399,4 +1399,64 @@ describe('UnifiedDataTable', () => { EXTENDED_JEST_TIMEOUT ); }); + + describe('pagination', () => { + const onChangePageMock = jest.fn(); + beforeEach(() => { + jest.clearAllMocks(); + }); + test('should effect pageIndex change', async () => { + const component = await getComponent({ + ...getProps(), + onUpdatePageIndex: onChangePageMock, + rowsPerPageState: 1, + rowsPerPageOptions: [1, 5], + }); + + expect(findTestSubject(component, 'pagination-button-1').exists()).toBeTruthy(); + onChangePageMock.mockClear(); + findTestSubject(component, 'pagination-button-1').simulate('click'); + expect(onChangePageMock).toHaveBeenNthCalledWith(1, 1); + }); + + test('should effect pageIndex change when itemsPerPage has been changed', async () => { + /* + * Use Case: + * + * Let's say we have 4 pages and we are on page 1 with 1 item per page. + * Now if we change items per page to 4, it should automatically change the pageIndex to 0. + * + * */ + const component = await getComponent({ + ...getProps(), + onUpdatePageIndex: onChangePageMock, + rowsPerPageState: 1, + rowsPerPageOptions: [1, 4], + }); + + expect(findTestSubject(component, 'pagination-button-4').exists()).toBeTruthy(); + onChangePageMock.mockClear(); + // go to last page + findTestSubject(component, 'pagination-button-4').simulate('click'); + expect(onChangePageMock).toHaveBeenNthCalledWith(1, 4); + onChangePageMock.mockClear(); + + // Change items per Page so that pageIndex autoamtically changes. + expect(findTestSubject(component, 'tablePaginationPopoverButton').text()).toBe( + 'Rows per page: 1' + ); + findTestSubject(component, 'tablePaginationPopoverButton').simulate('click'); + component.setProps({ + rowsPerPageState: 5, + }); + + await waitFor(() => { + expect(findTestSubject(component, 'tablePaginationPopoverButton').text()).toBe( + 'Rows per page: 5' + ); + }); + + expect(onChangePageMock).toHaveBeenNthCalledWith(1, 0); + }); + }); }); diff --git a/packages/kbn-unified-data-table/src/components/data_table.tsx b/packages/kbn-unified-data-table/src/components/data_table.tsx index a22ee8317be2f..7d57188dd1010 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.tsx @@ -261,6 +261,12 @@ export interface UnifiedDataTableProps { * Update rows per page state */ onUpdateRowsPerPage?: (rowsPerPage: number) => void; + /** + * + * this callback is triggered when user navigates to a different page + * + */ + onUpdatePageIndex?: (pageIndex: number) => void; /** * Configuration option to limit sample size slider */ @@ -493,6 +499,7 @@ export const UnifiedDataTable = ({ getRowIndicator, dataGridDensityState, onUpdateDataGridDensity, + onUpdatePageIndex, }: UnifiedDataTableProps) => { const { fieldFormats, toastNotifications, dataViewFieldEditor, uiSettings, storage, data } = services; @@ -519,6 +526,8 @@ export const UnifiedDataTable = ({ docIdsInSelectionOrder, } = selectedDocsState; + const [currentPageIndex, setCurrentPageIndex] = useState(0); + useEffect(() => { if (!hasSelectedDocs && isFilterActive) { setIsFilterActive(false); @@ -596,50 +605,56 @@ export const UnifiedDataTable = ({ typeof rowsPerPageState === 'number' && rowsPerPageState > 0 ? rowsPerPageState : DEFAULT_ROWS_PER_PAGE; - const [pagination, setPagination] = useState({ - pageIndex: 0, - pageSize: currentPageSize, - }); + const rowCount = useMemo(() => (displayedRows ? displayedRows.length : 0), [displayedRows]); const pageCount = useMemo( - () => Math.ceil(rowCount / pagination.pageSize), - [rowCount, pagination] + () => Math.ceil(rowCount / currentPageSize), + [rowCount, currentPageSize] ); + useEffect(() => { + /** + * Syncs any changes in pageIndex because of changes in pageCount + * to the consumer. + * + */ + setCurrentPageIndex((previousPageIndex: number) => { + const calculatedPageIndex = previousPageIndex > pageCount - 1 ? 0 : previousPageIndex; + if (calculatedPageIndex !== previousPageIndex) { + onUpdatePageIndex?.(calculatedPageIndex); + } + return calculatedPageIndex; + }); + }, [onUpdatePageIndex, pageCount]); + const paginationObj = useMemo(() => { const onChangeItemsPerPage = (pageSize: number) => { onUpdateRowsPerPage?.(pageSize); }; - const onChangePage = (pageIndex: number) => - setPagination((paginationData) => ({ ...paginationData, pageIndex })); + const onChangePage = (newPageIndex: number) => { + setCurrentPageIndex(newPageIndex); + onUpdatePageIndex?.(newPageIndex); + }; return isPaginationEnabled ? { onChangeItemsPerPage, onChangePage, - pageIndex: pagination.pageIndex > pageCount - 1 ? 0 : pagination.pageIndex, - pageSize: pagination.pageSize, - pageSizeOptions: rowsPerPageOptions ?? getRowsPerPageOptions(pagination.pageSize), + pageIndex: currentPageIndex, + pageSize: currentPageSize, + pageSizeOptions: rowsPerPageOptions ?? getRowsPerPageOptions(currentPageSize), } : undefined; }, [ isPaginationEnabled, - pagination.pageIndex, - pagination.pageSize, - pageCount, rowsPerPageOptions, onUpdateRowsPerPage, + currentPageSize, + currentPageIndex, + onUpdatePageIndex, ]); - useEffect(() => { - setPagination((paginationData) => - paginationData.pageSize === currentPageSize - ? paginationData - : { ...paginationData, pageSize: currentPageSize } - ); - }, [currentPageSize, setPagination]); - const unifiedDataTableContextValue = useMemo( () => ({ expanded: expandedDoc, diff --git a/x-pack/plugins/security_solution/common/types/timeline/store.ts b/x-pack/plugins/security_solution/common/types/timeline/store.ts index c65705e0c9a74..834949d2ed591 100644 --- a/x-pack/plugins/security_solution/common/types/timeline/store.ts +++ b/x-pack/plugins/security_solution/common/types/timeline/store.ts @@ -73,7 +73,7 @@ export type OnColumnRemoved = (columnId: ColumnId) => void; export type OnColumnResized = ({ columnId, delta }: { columnId: ColumnId; delta: number }) => void; /** Invoked when a user clicks to load more item */ -export type OnChangePage = (nextPage: number) => void; +export type OnFetchMoreRecords = (nextPage: number) => void; /** Invoked when a user checks/un-checks a row */ export type OnRowSelected = ({ diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/unified_timeline_body.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/unified_timeline_body.test.tsx index 41cdec6d6d4bb..77f26075581e0 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/unified_timeline_body.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/unified_timeline_body.test.tsx @@ -12,7 +12,7 @@ import { UnifiedTimeline } from '../unified_components'; import { defaultUdtHeaders } from './column_headers/default_headers'; import type { UnifiedTimelineBodyProps } from './unified_timeline_body'; import { UnifiedTimelineBody } from './unified_timeline_body'; -import { render, screen } from '@testing-library/react'; +import { render } from '@testing-library/react'; import { defaultHeaders, mockTimelineData, TestProviders } from '../../../../common/mock'; jest.mock('../unified_components', () => { @@ -32,17 +32,14 @@ const defaultProps: UnifiedTimelineBodyProps = { isTextBasedQuery: false, itemsPerPage: 25, itemsPerPageOptions: [10, 25, 50], - onChangePage: jest.fn(), + onFetchMoreRecords: jest.fn(), refetch: jest.fn(), rowRenderers: [], sort: [], timelineId: 'timeline-1', totalCount: 0, updatedAt: 0, - pageInfo: { - activePage: 0, - querySize: 0, - }, + onUpdatePageIndex: jest.fn(), }; const renderTestComponents = (props?: UnifiedTimelineBodyProps) => { @@ -57,39 +54,6 @@ describe('UnifiedTimelineBody', () => { beforeEach(() => { (UnifiedTimeline as unknown as jest.Mock).mockImplementation(MockUnifiedTimelineComponent); }); - it('should pass correct page rows', () => { - const { rerender } = renderTestComponents(); - - expect(screen.getByTestId('unifiedTimelineBody')).toBeVisible(); - expect(MockUnifiedTimelineComponent).toHaveBeenCalledTimes(2); - - expect(MockUnifiedTimelineComponent).toHaveBeenLastCalledWith( - expect.objectContaining({ - events: mockEventsData.flat(), - }), - {} - ); - - const newEventsData = structuredClone([mockEventsData[0]]); - - const newProps = { - ...defaultProps, - pageInfo: { - activePage: 1, - querySize: 0, - }, - events: newEventsData, - }; - - MockUnifiedTimelineComponent.mockClear(); - rerender(); - expect(MockUnifiedTimelineComponent).toHaveBeenLastCalledWith( - expect.objectContaining({ - events: [...mockEventsData, ...newEventsData].flat(), - }), - {} - ); - }); it('should pass default columns when empty column list is supplied', () => { const newProps = { ...defaultProps, columns: [] }; diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/unified_timeline_body.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/unified_timeline_body.tsx index 95feab8543617..b705717fc437f 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/unified_timeline_body.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/unified_timeline_body.tsx @@ -6,23 +6,20 @@ */ import type { ComponentProps, ReactElement } from 'react'; -import React, { useEffect, useState, useMemo } from 'react'; +import React, { useMemo } from 'react'; import { RootDragDropProvider } from '@kbn/dom-drag-drop'; import { StyledTableFlexGroup, StyledUnifiedTableFlexItem } from '../unified_components/styles'; import { UnifiedTimeline } from '../unified_components'; import { defaultUdtHeaders } from './column_headers/default_headers'; -import type { PaginationInputPaginated, TimelineItem } from '../../../../../common/search_strategy'; export interface UnifiedTimelineBodyProps extends ComponentProps { header: ReactElement; - pageInfo: Pick; } export const UnifiedTimelineBody = (props: UnifiedTimelineBodyProps) => { const { header, isSortEnabled, - pageInfo, columns, rowRenderers, timelineId, @@ -33,28 +30,14 @@ export const UnifiedTimelineBody = (props: UnifiedTimelineBodyProps) => { refetch, dataLoadingState, totalCount, - onChangePage, + onFetchMoreRecords, activeTab, updatedAt, trailingControlColumns, leadingControlColumns, + onUpdatePageIndex, } = props; - const [pageRows, setPageRows] = useState([]); - - const rows = useMemo(() => pageRows.flat(), [pageRows]); - - useEffect(() => { - setPageRows((currentPageRows) => { - if (pageInfo.activePage !== 0 && currentPageRows[pageInfo.activePage]?.length) { - return currentPageRows; - } - const newPageRows = pageInfo.activePage === 0 ? [] : [...currentPageRows]; - newPageRows[pageInfo.activePage] = events; - return newPageRows; - }); - }, [events, pageInfo.activePage]); - const columnsHeader = useMemo(() => columns ?? defaultUdtHeaders, [columns]); return ( @@ -73,16 +56,17 @@ export const UnifiedTimelineBody = (props: UnifiedTimelineBodyProps) => { itemsPerPage={itemsPerPage} itemsPerPageOptions={itemsPerPageOptions} sort={sort} - events={rows} + events={events} refetch={refetch} dataLoadingState={dataLoadingState} totalCount={totalCount} - onChangePage={onChangePage} + onFetchMoreRecords={onFetchMoreRecords} activeTab={activeTab} updatedAt={updatedAt} isTextBasedQuery={false} trailingControlColumns={trailingControlColumns} leadingControlColumns={leadingControlColumns} + onUpdatePageIndex={onUpdatePageIndex} /> diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/events.ts b/x-pack/plugins/security_solution/public/timelines/components/timeline/events.ts index 14046b853a235..c48e38ba69a21 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/events.ts +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/events.ts @@ -13,7 +13,7 @@ export type { OnColumnsSorted, OnColumnRemoved, OnColumnResized, - OnChangePage, + OnFetchMoreRecords as OnChangePage, OnPinEvent, OnRowSelected, OnSelectAll, diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.test.tsx index 23fb44d04910f..a71f99715131e 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.test.tsx @@ -5,18 +5,16 @@ * 2.0. */ -import React from 'react'; +import type { ComponentProps } from 'react'; +import React, { useEffect } from 'react'; import useResizeObserver from 'use-resize-observer/polyfilled'; -import type { Dispatch } from 'redux'; -import { defaultRowRenderers } from '../../body/renderers'; -import { DefaultCellRenderer } from '../../cell_rendering/default_cell_renderer'; -import { defaultHeaders, mockTimelineData } from '../../../../../common/mock'; +import { createMockStore, mockGlobalState, mockTimelineData } from '../../../../../common/mock'; import { TestProviders } from '../../../../../common/mock/test_providers'; import type { Props as EqlTabContentComponentProps } from '.'; -import { EqlTabContentComponent } from '.'; -import { TimelineId, TimelineTabs } from '../../../../../../common/types/timeline'; +import EqlTabContentComponent from '.'; +import { TimelineId } from '../../../../../../common/types/timeline'; import { useTimelineEvents } from '../../../../containers'; import { useTimelineEventsDetails } from '../../../../containers/details'; import { useSourcererDataView } from '../../../../../sourcerer/containers'; @@ -24,7 +22,15 @@ import { mockSourcererScope } from '../../../../../sourcerer/containers/mocks'; import { useIsExperimentalFeatureEnabled } from '../../../../../common/hooks/use_experimental_features'; import type { ExperimentalFeatures } from '../../../../../../common'; import { allowedExperimentalValues } from '../../../../../../common'; -import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import * as notesApi from '../../../../../notes/api/api'; +import { timelineActions } from '../../../../store'; +import { DefaultCellRenderer } from '../../cell_rendering/default_cell_renderer'; +import { defaultRowRenderers } from '../../body/renderers'; +import { useDispatch } from 'react-redux'; +import { TimelineTabs } from '@kbn/securitysolution-data-table'; + +const SPECIAL_TEST_TIMEOUT = 30000; jest.mock('../../../../containers', () => ({ useTimelineEvents: jest.fn(), @@ -50,10 +56,43 @@ mockUseResizeObserver.mockImplementation(() => ({})); jest.mock('../../../../../common/lib/kibana'); +let useTimelineEventsMock = jest.fn(); + +const loadPageMock = jest.fn(); + +const mockState = { + ...structuredClone(mockGlobalState), +}; +mockState.timeline.timelineById[TimelineId.test].activeTab = TimelineTabs.eql; + +const TestComponent = (props: Partial>) => { + const testComponentDefaultProps: ComponentProps = { + timelineId: TimelineId.test, + renderCellValue: DefaultCellRenderer, + rowRenderers: defaultRowRenderers, + }; + + const dispatch = useDispatch(); + + useEffect(() => { + // Unified field list can be a culprit for long load times, so we wait for the timeline to be interacted with to load + dispatch(timelineActions.showTimeline({ id: TimelineId.test, show: true })); + + // populating timeline so that it is not blank + dispatch( + timelineActions.updateEqlOptions({ + id: TimelineId.test, + field: 'query', + value: 'any where true', + }) + ); + }, [dispatch]); + + return ; +}; + describe('EQL Tab', () => { - let props = {} as EqlTabContentComponentProps; - const startDate = '2018-03-23T18:49:23.132Z'; - const endDate = '2018-03-24T03:33:52.253Z'; + const props = {} as EqlTabContentComponentProps; beforeAll(() => { // https://github.com/atlassian/react-beautiful-dnd/blob/4721a518356f72f1dac45b5fd4ee9d466aa2996b/docs/guides/setup-problem-detection-and-error-recovery.md#disable-logging @@ -65,7 +104,7 @@ describe('EQL Tab', () => { }); beforeEach(() => { - (useTimelineEvents as jest.Mock).mockReturnValue([ + useTimelineEventsMock = jest.fn(() => [ false, { events: mockTimelineData.slice(0, 1), @@ -75,6 +114,7 @@ describe('EQL Tab', () => { }, }, ]); + (useTimelineEvents as jest.Mock).mockImplementation(useTimelineEventsMock); (useTimelineEventsDetails as jest.Mock).mockReturnValue([false, {}]); (useSourcererDataView as jest.Mock).mockReturnValue(mockSourcererScope); @@ -85,30 +125,23 @@ describe('EQL Tab', () => { } ); - props = { - dispatch: {} as Dispatch, - activeTab: TimelineTabs.eql, - columns: defaultHeaders, - end: endDate, - eqlOptions: {}, - isLive: false, - itemsPerPage: 5, - itemsPerPageOptions: [5, 10, 20], - renderCellValue: DefaultCellRenderer, - rowRenderers: defaultRowRenderers, - start: startDate, - timelineId: TimelineId.test, - timerangeKind: 'absolute', - pinnedEventIds: {}, - eventIdToNoteIds: {}, - }; + HTMLElement.prototype.getBoundingClientRect = jest.fn(() => { + return { + width: 1000, + height: 1000, + x: 0, + y: 0, + } as DOMRect; + }); }); describe('rendering', () => { + const fetchNotesMock = jest.spyOn(notesApi, 'fetchNotesByDocumentIds'); test('should render the timeline table', async () => { + fetchNotesMock.mockImplementation(jest.fn()); render( - - + + ); @@ -117,8 +150,8 @@ describe('EQL Tab', () => { test('it renders the timeline column headers', async () => { render( - - + + ); @@ -138,12 +171,175 @@ describe('EQL Tab', () => { ]); render( - - + + ); expect(await screen.findByText('No results found')).toBeVisible(); }); + + describe('pagination', () => { + beforeEach(() => { + // pagination tests need more than 1 record so here + // we return 5 records instead of just 1. + useTimelineEventsMock = jest.fn(() => [ + false, + { + events: structuredClone(mockTimelineData.slice(0, 5)), + pageInfo: { + activePage: 0, + totalPages: 5, + }, + refreshedAt: Date.now(), + /* + * `totalCount` could be any number w.r.t this test + * and actually means total hits on elastic search + * and not the fecthed number of records. + * + * This helps in testing `sampleSize` and `loadMore` + */ + totalCount: 50, + loadPage: loadPageMock, + }, + ]); + + (useTimelineEvents as jest.Mock).mockImplementation(useTimelineEventsMock); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it( + 'should load notes for current page only', + async () => { + const mockStateWithNoteInTimeline = { + ...mockGlobalState, + timeline: { + ...mockGlobalState.timeline, + timelineById: { + [TimelineId.test]: { + ...mockGlobalState.timeline.timelineById[TimelineId.test], + /* 1 record for each page */ + activeTab: TimelineTabs.eql, + itemsPerPage: 1, + itemsPerPageOptions: [1, 2, 3, 4, 5], + savedObjectId: 'timeline-1', // match timelineId in mocked notes data + pinnedEventIds: { '1': true }, + }, + }, + }, + }; + + render( + + + + ); + + expect(await screen.findByTestId('discoverDocTable')).toBeVisible(); + + expect(screen.getByTestId('pagination-button-previous')).toBeVisible(); + + expect(screen.getByTestId('pagination-button-0')).toHaveAttribute('aria-current', 'true'); + expect(fetchNotesMock).toHaveBeenCalledWith(['1']); + + // Page : 2 + + fetchNotesMock.mockClear(); + expect(screen.getByTestId('pagination-button-1')).toBeVisible(); + + fireEvent.click(screen.getByTestId('pagination-button-1')); + + await waitFor(() => { + expect(screen.getByTestId('pagination-button-1')).toHaveAttribute( + 'aria-current', + 'true' + ); + + expect(fetchNotesMock).toHaveBeenNthCalledWith(1, [mockTimelineData[1]._id]); + }); + + // Page : 3 + + fetchNotesMock.mockClear(); + expect(screen.getByTestId('pagination-button-2')).toBeVisible(); + fireEvent.click(screen.getByTestId('pagination-button-2')); + + await waitFor(() => { + expect(screen.getByTestId('pagination-button-2')).toHaveAttribute( + 'aria-current', + 'true' + ); + + expect(fetchNotesMock).toHaveBeenNthCalledWith(1, [mockTimelineData[2]._id]); + }); + }, + SPECIAL_TEST_TIMEOUT + ); + + it( + 'should load notes for correct page size', + async () => { + const mockStateWithNoteInTimeline = { + ...mockGlobalState, + timeline: { + ...mockGlobalState.timeline, + timelineById: { + [TimelineId.test]: { + ...mockGlobalState.timeline.timelineById[TimelineId.test], + /* 1 record for each page */ + itemsPerPage: 1, + pageIndex: 0, + itemsPerPageOptions: [1, 2, 3, 4, 5], + savedObjectId: 'timeline-1', // match timelineId in mocked notes data + pinnedEventIds: { '1': true }, + }, + }, + }, + }; + + render( + + + + ); + + expect(await screen.findByTestId('discoverDocTable')).toBeVisible(); + + expect(screen.getByTestId('pagination-button-previous')).toBeVisible(); + + expect(screen.getByTestId('pagination-button-0')).toHaveAttribute('aria-current', 'true'); + expect(screen.getByTestId('tablePaginationPopoverButton')).toHaveTextContent( + 'Rows per page: 1' + ); + fireEvent.click(screen.getByTestId('tablePaginationPopoverButton')); + + await waitFor(() => { + expect(screen.getByTestId('tablePagination-2-rows')).toBeVisible(); + }); + + fetchNotesMock.mockClear(); + fireEvent.click(screen.getByTestId('tablePagination-2-rows')); + + await waitFor(() => { + expect(fetchNotesMock).toHaveBeenNthCalledWith(1, [ + mockTimelineData[0]._id, + mockTimelineData[1]._id, + ]); + }); + }, + SPECIAL_TEST_TIMEOUT + ); + }); }); }); diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx index bcf7a0de07ee5..0b2de48e89693 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx @@ -7,7 +7,7 @@ import { EuiFlexGroup } from '@elastic/eui'; import { isEmpty } from 'lodash/fp'; -import React, { useCallback, useMemo } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import type { ConnectedProps } from 'react-redux'; import { connect } from 'react-redux'; import deepEqual from 'fast-deep-equal'; @@ -17,6 +17,7 @@ import type { EuiDataGridControlColumn } from '@elastic/eui'; import { DataLoadingState } from '@kbn/unified-data-table'; import { useExpandableFlyoutApi } from '@kbn/expandable-flyout'; import type { RunTimeMappings } from '@kbn/timelines-plugin/common/search_strategy'; +import { useFetchNotes } from '../../../../../notes/hooks/use_fetch_notes'; import { InputsModelId } from '../../../../../common/store/inputs/constants'; import { useKibana } from '../../../../../common/lib/kibana'; import { @@ -65,6 +66,13 @@ export const EqlTabContentComponent: React.FC = ({ pinnedEventIds, eventIdToNoteIds, }) => { + /* + * Needs to be maintained for each table in each tab independently + * and consequently it cannot be the part of common redux state + * of the timeline. + * + */ + const [pageIndex, setPageIndex] = useState(0); const { telemetry } = useKibana().services; const { query: eqlQuery = '', ...restEqlOption } = eqlOptions; const { portalNode: eqlEventsCountPortalNode } = useEqlEventsCountPortal(); @@ -97,24 +105,42 @@ export const EqlTabContentComponent: React.FC = ({ [end, isBlankTimeline, loadingSourcerer, start] ); - const [ - dataLoadingState, - { events, inspect, totalCount, pageInfo, loadPage, refreshedAt, refetch }, - ] = useTimelineEvents({ - dataViewId, - endDate: end, - eqlOptions: restEqlOption, - fields: timelineQueryFieldsFromColumns, - filterQuery: eqlQuery ?? '', - id: timelineId, - indexNames: selectedPatterns, - language: 'eql', - limit: sampleSize, - runtimeMappings: sourcererDataView?.runtimeFieldMap as RunTimeMappings, - skip: !canQueryTimeline(), - startDate: start, - timerangeKind, - }); + const [dataLoadingState, { events, inspect, totalCount, loadPage, refreshedAt, refetch }] = + useTimelineEvents({ + dataViewId, + endDate: end, + eqlOptions: restEqlOption, + fields: timelineQueryFieldsFromColumns, + filterQuery: eqlQuery ?? '', + id: timelineId, + indexNames: selectedPatterns, + language: 'eql', + limit: sampleSize, + runtimeMappings: sourcererDataView.runtimeFieldMap as RunTimeMappings, + skip: !canQueryTimeline(), + startDate: start, + timerangeKind, + }); + + const { onLoad: loadNotesOnEventsLoad } = useFetchNotes(); + + useEffect(() => { + // This useEffect loads the notes only for the events on the current + // page. + const eventsOnCurrentPage = events.slice( + itemsPerPage * pageIndex, + itemsPerPage * (pageIndex + 1) + ); + + loadNotesOnEventsLoad(eventsOnCurrentPage); + }, [events, pageIndex, itemsPerPage, loadNotesOnEventsLoad]); + + /** + * + * Triggers on Datagrid page change + * + */ + const onUpdatePageIndex = useCallback((newPageIndex: number) => setPageIndex(newPageIndex), []); const { openFlyout } = useExpandableFlyoutApi(); const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled( @@ -263,12 +289,12 @@ export const EqlTabContentComponent: React.FC = ({ refetch={refetch} dataLoadingState={dataLoadingState} totalCount={isBlankTimeline ? 0 : totalCount} - onChangePage={loadPage} + onFetchMoreRecords={loadPage} activeTab={activeTab} updatedAt={refreshedAt} isTextBasedQuery={false} - pageInfo={pageInfo} leadingControlColumns={leadingControlColumns as EuiDataGridControlColumn[]} + onUpdatePageIndex={onUpdatePageIndex} /> diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/pinned/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/pinned/index.tsx index fe2816141fee5..3e3b8f6c564c6 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/pinned/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/pinned/index.tsx @@ -6,13 +6,14 @@ */ import { isEmpty } from 'lodash/fp'; -import React, { useMemo, useCallback, memo } from 'react'; +import React, { useMemo, useCallback, memo, useState, useEffect } from 'react'; import type { ConnectedProps } from 'react-redux'; import { connect } from 'react-redux'; import deepEqual from 'fast-deep-equal'; import type { EuiDataGridControlColumn } from '@elastic/eui'; import { useExpandableFlyoutApi } from '@kbn/expandable-flyout'; import type { RunTimeMappings } from '@kbn/timelines-plugin/common/search_strategy'; +import { useFetchNotes } from '../../../../../notes/hooks/use_fetch_notes'; import { DocumentDetailsLeftPanelKey, DocumentDetailsRightPanelKey, @@ -68,6 +69,14 @@ export const PinnedTabContentComponent: React.FC = ({ sort, eventIdToNoteIds, }) => { + /* + * Needs to be maintained for each table in each tab independently + * and consequently it cannot be the part of common redux state + * of the timeline. + * + */ + const [pageIndex, setPageIndex] = useState(0); + const { telemetry } = useKibana().services; const { dataViewId, sourcererDataView, selectedPatterns } = useSourcererDataView( SourcererScopeName.timeline @@ -130,7 +139,7 @@ export const PinnedTabContentComponent: React.FC = ({ ); const { augmentedColumnHeaders } = useTimelineColumns(columns); - const [queryLoadingState, { events, totalCount, pageInfo, loadPage, refreshedAt, refetch }] = + const [queryLoadingState, { events, totalCount, loadPage, refreshedAt, refetch }] = useTimelineEvents({ endDate: '', id: `pinned-${timelineId}`, @@ -146,6 +155,26 @@ export const PinnedTabContentComponent: React.FC = ({ timerangeKind: undefined, }); + const { onLoad: loadNotesOnEventsLoad } = useFetchNotes(); + + useEffect(() => { + // This useEffect loads the notes only for the events on the current + // page. + const eventsOnCurrentPage = events.slice( + itemsPerPage * pageIndex, + itemsPerPage * (pageIndex + 1) + ); + + loadNotesOnEventsLoad(eventsOnCurrentPage); + }, [events, pageIndex, itemsPerPage, loadNotesOnEventsLoad]); + + /** + * + * Triggers on Datagrid page change + * + */ + const onUpdatePageIndex = useCallback((newPageIndex: number) => setPageIndex(newPageIndex), []); + const { openFlyout } = useExpandableFlyoutApi(); const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled( 'securitySolutionNotesDisabled' @@ -257,13 +286,13 @@ export const PinnedTabContentComponent: React.FC = ({ refetch={refetch} dataLoadingState={queryLoadingState} totalCount={totalCount} - onChangePage={loadPage} + onFetchMoreRecords={loadPage} activeTab={TimelineTabs.pinned} updatedAt={refreshedAt} isTextBasedQuery={false} - pageInfo={pageInfo} leadingControlColumns={leadingControlColumns as EuiDataGridControlColumn[]} trailingControlColumns={rowDetailColumn} + onUpdatePageIndex={onUpdatePageIndex} /> ); diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/index.test.tsx index f0a2c06bbffb4..cfd8f86af9dac 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/index.test.tsx @@ -41,6 +41,7 @@ import { useExpandableFlyoutApi } from '@kbn/expandable-flyout'; import { createExpandableFlyoutApiMock } from '../../../../../common/mock/expandable_flyout'; import { OPEN_FLYOUT_BUTTON_TEST_ID } from '../../../../../notes/components/test_ids'; import { userEvent } from '@testing-library/user-event'; +import * as notesApi from '../../../../../notes/api/api'; jest.mock('../../../../../common/components/user_privileges'); @@ -154,7 +155,9 @@ const { storage: storageMock } = createSecuritySolutionStorageMock(); let useTimelineEventsMock = jest.fn(); describe('query tab with unified timeline', () => { + const fetchNotesMock = jest.spyOn(notesApi, 'fetchNotesByDocumentIds'); beforeAll(() => { + fetchNotesMock.mockImplementation(jest.fn()); jest.mocked(useExpandableFlyoutApi).mockImplementation(() => ({ ...createExpandableFlyoutApiMock(), openFlyout: mockOpenFlyout, @@ -176,6 +179,7 @@ describe('query tab with unified timeline', () => { afterEach(() => { jest.clearAllMocks(); storageMock.clear(); + fetchNotesMock.mockClear(); cleanup(); localStorage.clear(); }); @@ -424,6 +428,130 @@ describe('query tab with unified timeline', () => { }, SPECIAL_TEST_TIMEOUT ); + + it( + 'should load notes for current page only', + async () => { + const mockStateWithNoteInTimeline = { + ...mockGlobalState, + timeline: { + ...mockGlobalState.timeline, + timelineById: { + [TimelineId.test]: { + ...mockGlobalState.timeline.timelineById[TimelineId.test], + /* 1 record for each page */ + itemsPerPage: 1, + pageIndex: 0, + itemsPerPageOptions: [1, 2, 3, 4, 5], + savedObjectId: 'timeline-1', // match timelineId in mocked notes data + pinnedEventIds: { '1': true }, + }, + }, + }, + }; + + render( + + + + ); + + expect(await screen.findByTestId('discoverDocTable')).toBeVisible(); + + expect(screen.getByTestId('pagination-button-previous')).toBeVisible(); + + expect(screen.getByTestId('pagination-button-0')).toHaveAttribute('aria-current', 'true'); + expect(fetchNotesMock).toHaveBeenCalledWith(['1']); + + // Page : 2 + + fetchNotesMock.mockClear(); + expect(screen.getByTestId('pagination-button-1')).toBeVisible(); + + fireEvent.click(screen.getByTestId('pagination-button-1')); + + await waitFor(() => { + expect(screen.getByTestId('pagination-button-1')).toHaveAttribute('aria-current', 'true'); + + expect(fetchNotesMock).toHaveBeenNthCalledWith(1, [mockTimelineData[1]._id]); + }); + + // Page : 3 + + fetchNotesMock.mockClear(); + expect(screen.getByTestId('pagination-button-2')).toBeVisible(); + fireEvent.click(screen.getByTestId('pagination-button-2')); + + await waitFor(() => { + expect(screen.getByTestId('pagination-button-2')).toHaveAttribute('aria-current', 'true'); + + expect(fetchNotesMock).toHaveBeenNthCalledWith(1, [mockTimelineData[2]._id]); + }); + }, + SPECIAL_TEST_TIMEOUT + ); + + it( + 'should load notes for correct page size', + async () => { + const mockStateWithNoteInTimeline = { + ...mockGlobalState, + timeline: { + ...mockGlobalState.timeline, + timelineById: { + [TimelineId.test]: { + ...mockGlobalState.timeline.timelineById[TimelineId.test], + /* 1 record for each page */ + itemsPerPage: 1, + pageIndex: 0, + itemsPerPageOptions: [1, 2, 3, 4, 5], + savedObjectId: 'timeline-1', // match timelineId in mocked notes data + pinnedEventIds: { '1': true }, + }, + }, + }, + }; + + render( + + + + ); + + expect(await screen.findByTestId('discoverDocTable')).toBeVisible(); + + expect(screen.getByTestId('pagination-button-previous')).toBeVisible(); + + expect(screen.getByTestId('pagination-button-0')).toHaveAttribute('aria-current', 'true'); + expect(screen.getByTestId('tablePaginationPopoverButton')).toHaveTextContent( + 'Rows per page: 1' + ); + fireEvent.click(screen.getByTestId('tablePaginationPopoverButton')); + + await waitFor(() => { + expect(screen.getByTestId('tablePagination-2-rows')).toBeVisible(); + }); + + fetchNotesMock.mockClear(); + fireEvent.click(screen.getByTestId('tablePagination-2-rows')); + + await waitFor(() => { + expect(fetchNotesMock).toHaveBeenNthCalledWith(1, [ + mockTimelineData[0]._id, + mockTimelineData[1]._id, + ]); + }); + }, + SPECIAL_TEST_TIMEOUT + ); }); describe('columns', () => { diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/index.tsx index eaf994cc5ab3d..d15cf2b5539c3 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/index.tsx @@ -6,7 +6,7 @@ */ import { isEmpty } from 'lodash/fp'; -import React, { useMemo, useEffect, useCallback } from 'react'; +import React, { useMemo, useEffect, useCallback, useState } from 'react'; import type { ConnectedProps } from 'react-redux'; import { connect, useDispatch } from 'react-redux'; import deepEqual from 'fast-deep-equal'; @@ -15,6 +15,7 @@ import { getEsQueryConfig } from '@kbn/data-plugin/common'; import { DataLoadingState } from '@kbn/unified-data-table'; import { useExpandableFlyoutApi } from '@kbn/expandable-flyout'; import type { RunTimeMappings } from '@kbn/timelines-plugin/common/search_strategy'; +import { useFetchNotes } from '../../../../../notes/hooks/use_fetch_notes'; import { DocumentDetailsLeftPanelKey, DocumentDetailsRightPanelKey, @@ -93,6 +94,13 @@ export const QueryTabContentComponent: React.FC = ({ selectedPatterns, sourcererDataView, } = useSourcererDataView(SourcererScopeName.timeline); + /* + * `pageIndex` needs to be maintained for each table in each tab independently + * and consequently it cannot be the part of common redux state + * of the timeline. + * + */ + const [pageIndex, setPageIndex] = useState(0); const { uiSettings, telemetry, timelineDataService } = useKibana().services; const { @@ -168,7 +176,7 @@ export const QueryTabContentComponent: React.FC = ({ const [ dataLoadingState, - { events, inspect, totalCount, pageInfo, loadPage, refreshedAt, refetch }, + { events, inspect, totalCount, loadPage: loadNextEventBatch, refreshedAt, refetch }, ] = useTimelineEvents({ dataViewId, endDate: end, @@ -185,6 +193,26 @@ export const QueryTabContentComponent: React.FC = ({ timerangeKind, }); + const { onLoad: loadNotesOnEventsLoad } = useFetchNotes(); + + useEffect(() => { + // This useEffect loads the notes only for the events on the current + // page. + const eventsOnCurrentPage = events.slice( + itemsPerPage * pageIndex, + itemsPerPage * (pageIndex + 1) + ); + + loadNotesOnEventsLoad(eventsOnCurrentPage); + }, [events, pageIndex, itemsPerPage, loadNotesOnEventsLoad]); + + /** + * + * Triggers on Datagrid page change + * + */ + const onUpdatePageIndex = useCallback((newPageIndex: number) => setPageIndex(newPageIndex), []); + const { openFlyout } = useExpandableFlyoutApi(); const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled( 'securitySolutionNotesDisabled' @@ -356,11 +384,11 @@ export const QueryTabContentComponent: React.FC = ({ dataLoadingState={dataLoadingState} totalCount={isBlankTimeline ? 0 : totalCount} leadingControlColumns={leadingControlColumns as EuiDataGridControlColumn[]} - onChangePage={loadPage} + onFetchMoreRecords={loadNextEventBatch} activeTab={activeTab} updatedAt={refreshedAt} isTextBasedQuery={false} - pageInfo={pageInfo} + onUpdatePageIndex={onUpdatePageIndex} /> ); diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.test.tsx index 649817d5f8ef2..3f24fc8df4aa9 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.test.tsx @@ -72,7 +72,7 @@ const TestComponent = (props: TestComponentProps) => { refetch={refetchMock} dataLoadingState={DataLoadingState.loaded} totalCount={mockTimelineData.length} - onChangePage={onChangePageMock} + onFetchMoreRecords={onChangePageMock} updatedAt={Date.now()} onSetColumns={jest.fn()} onFilter={jest.fn()} diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.tsx index fa5b83f23576a..99e00547f1c33 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.tsx @@ -30,7 +30,7 @@ import type { TimelineItem } from '../../../../../../common/search_strategy'; import { useKibana } from '../../../../../common/lib/kibana'; import type { ColumnHeaderOptions, - OnChangePage, + OnFetchMoreRecords, RowRenderer, TimelineTabs, } from '../../../../../../common/types/timeline'; @@ -64,7 +64,7 @@ type CommonDataTableProps = { refetch: inputsModel.Refetch; onFieldEdited: () => void; totalCount: number; - onChangePage: OnChangePage; + onFetchMoreRecords: OnFetchMoreRecords; activeTab: TimelineTabs; dataLoadingState: DataLoadingState; updatedAt: number; @@ -79,6 +79,7 @@ type CommonDataTableProps = { | 'renderCustomGridBody' | 'trailingControlColumns' | 'isSortEnabled' + | 'onUpdatePageIndex' >; interface DataTableProps extends CommonDataTableProps { @@ -102,13 +103,14 @@ export const TimelineDataTableComponent: React.FC = memo( refetch, dataLoadingState, totalCount, - onChangePage, + onFetchMoreRecords, updatedAt, isTextBasedQuery = false, onSetColumns, onSort, onFilter, leadingControlColumns, + onUpdatePageIndex, }) { const dispatch = useDispatch(); @@ -235,9 +237,9 @@ export const TimelineDataTableComponent: React.FC = memo( ); const handleFetchMoreRecords = useCallback(() => { - onChangePage(fetchedPage + 1); + onFetchMoreRecords(fetchedPage + 1); setFechedPage(fetchedPage + 1); - }, [fetchedPage, onChangePage]); + }, [fetchedPage, onFetchMoreRecords]); const additionalControls = useMemo( () => , @@ -424,6 +426,7 @@ export const TimelineDataTableComponent: React.FC = memo( renderCustomGridBody={finalRenderCustomBodyCallback} trailingControlColumns={finalTrailControlColumns} externalControlColumns={leadingControlColumns} + onUpdatePageIndex={onUpdatePageIndex} /> diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/index.test.tsx index c660893ba379e..7d9bde02259a4 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/index.test.tsx @@ -107,10 +107,11 @@ const TestComponent = ( events: localMockedTimelineData, refetch: jest.fn(), totalCount: localMockedTimelineData.length, - onChangePage: jest.fn(), + onFetchMoreRecords: jest.fn(), dataLoadingState: DataLoadingState.loaded, updatedAt: Date.now(), isTextBasedQuery: false, + onUpdatePageIndex: jest.fn(), }; const dispatch = useDispatch(); diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/index.tsx index 112886f93ca32..d350b4b530808 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/index.tsx @@ -12,7 +12,7 @@ import { useDispatch } from 'react-redux'; import { generateFilters } from '@kbn/data-plugin/public'; import type { DataView, DataViewField } from '@kbn/data-plugin/common'; import type { SortOrder } from '@kbn/saved-search-plugin/public'; -import type { DataLoadingState } from '@kbn/unified-data-table'; +import type { DataLoadingState, UnifiedDataTableProps } from '@kbn/unified-data-table'; import { useColumns } from '@kbn/unified-data-table'; import { popularizeField } from '@kbn/unified-data-table/src/utils/popularize_field'; import type { DropType } from '@kbn/dom-drag-drop'; @@ -33,7 +33,7 @@ import type { TimelineItem } from '../../../../../common/search_strategy'; import { useKibana } from '../../../../common/lib/kibana'; import type { ColumnHeaderOptions, - OnChangePage, + OnFetchMoreRecords, RowRenderer, SortColumnTimeline, TimelineTabs, @@ -106,7 +106,7 @@ interface Props { events: TimelineItem[]; refetch: inputsModel.Refetch; totalCount: number; - onChangePage: OnChangePage; + onFetchMoreRecords: OnFetchMoreRecords; activeTab: TimelineTabs; dataLoadingState: DataLoadingState; updatedAt: number; @@ -114,6 +114,7 @@ interface Props { dataView: DataView; trailingControlColumns?: EuiDataGridProps['trailingControlColumns']; leadingControlColumns?: EuiDataGridProps['leadingControlColumns']; + onUpdatePageIndex?: UnifiedDataTableProps['onUpdatePageIndex']; } const UnifiedTimelineComponent: React.FC = ({ @@ -129,12 +130,13 @@ const UnifiedTimelineComponent: React.FC = ({ refetch, dataLoadingState, totalCount, - onChangePage, + onFetchMoreRecords, updatedAt, isTextBasedQuery, dataView, trailingControlColumns, leadingControlColumns, + onUpdatePageIndex, }) => { const dispatch = useDispatch(); const unifiedFieldListContainerRef = useRef(null); @@ -435,13 +437,14 @@ const UnifiedTimelineComponent: React.FC = ({ onFieldEdited={onFieldEdited} dataLoadingState={dataLoadingState} totalCount={totalCount} - onChangePage={onChangePage} + onFetchMoreRecords={onFetchMoreRecords} activeTab={activeTab} updatedAt={updatedAt} isTextBasedQuery={isTextBasedQuery} onFilter={onAddFilter as DocViewFilterFn} trailingControlColumns={trailingControlColumns} leadingControlColumns={leadingControlColumns} + onUpdatePageIndex={onUpdatePageIndex} /> diff --git a/x-pack/plugins/security_solution/public/timelines/containers/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/containers/index.test.tsx index 68846de0fed37..ba0151ff77b59 100644 --- a/x-pack/plugins/security_solution/public/timelines/containers/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/containers/index.test.tsx @@ -15,6 +15,7 @@ import { useIsExperimentalFeatureEnabled } from '../../common/hooks/use_experime import { mockTimelineData } from '../../common/mock'; import { useRouteSpy } from '../../common/utils/route/use_route_spy'; import { useFetchNotes } from '../../notes/hooks/use_fetch_notes'; +import { waitFor } from '@testing-library/dom'; const mockDispatch = jest.fn(); jest.mock('react-redux', () => { @@ -30,7 +31,7 @@ jest.mock('../../notes/hooks/use_fetch_notes'); const onLoadMock = jest.fn(); const useFetchNotesMock = useFetchNotes as jest.Mock; -const mockEvents = mockTimelineData.filter((i, index) => index <= 11); +const mockEvents = mockTimelineData.slice(0, 10); const mockSearch = jest.fn(); @@ -60,22 +61,28 @@ jest.mock('../../common/lib/kibana', () => ({ mockSearch(); return { subscribe: jest.fn().mockImplementation(({ next }) => { - next({ - isRunning: false, - isPartial: false, - inspect: { - dsl: [], - response: [], - }, - edges: mockEvents.map((item) => ({ node: item })), - pageInfo: { - activePage: 0, - totalPages: 10, - }, - rawResponse: {}, - totalCount: mockTimelineData.length, - }); - return { unsubscribe: jest.fn() }; + const timeoutHandler = setTimeout(() => { + next({ + isRunning: false, + isPartial: false, + inspect: { + dsl: [], + response: [], + }, + edges: mockEvents.map((item) => ({ node: item })), + pageInfo: { + activePage: args.pagination.activePage, + totalPages: 10, + }, + rawResponse: {}, + totalCount: mockTimelineData.length, + }); + }, 50); + return { + unsubscribe: jest.fn(() => { + clearTimeout(timeoutHandler); + }), + }; }), }; }), @@ -122,12 +129,12 @@ describe('useTimelineEvents', () => { const endDate: string = '3000-01-01T00:00:00.000Z'; const props: UseTimelineEventsProps = { dataViewId: 'data-view-id', - endDate: '', + endDate, id: TimelineId.active, indexNames: ['filebeat-*'], fields: ['@timestamp', 'event.kind'], filterQuery: '', - startDate: '', + startDate, limit: 25, runtimeMappings: {}, sort: initSortDefault, @@ -167,7 +174,7 @@ describe('useTimelineEvents', () => { UseTimelineEventsProps, [DataLoadingState, TimelineArgs] >((args) => useTimelineEvents(args), { - initialProps: { ...props }, + initialProps: { ...props, startDate: '', endDate: '' }, }); // useEffect on params request @@ -176,20 +183,22 @@ describe('useTimelineEvents', () => { // useEffect on params request await waitForNextUpdate(); - expect(mockSearch).toHaveBeenCalledTimes(2); - expect(result.current).toEqual([ - DataLoadingState.loaded, - { - events: mockEvents, - id: TimelineId.active, - inspect: result.current[1].inspect, - loadPage: result.current[1].loadPage, - pageInfo: result.current[1].pageInfo, - refetch: result.current[1].refetch, - totalCount: 32, - refreshedAt: result.current[1].refreshedAt, - }, - ]); + await waitFor(() => { + expect(mockSearch).toHaveBeenCalledTimes(2); + expect(result.current).toEqual([ + DataLoadingState.loaded, + { + events: mockEvents, + id: TimelineId.active, + inspect: result.current[1].inspect, + loadPage: result.current[1].loadPage, + pageInfo: result.current[1].pageInfo, + refetch: result.current[1].refetch, + totalCount: 32, + refreshedAt: result.current[1].refreshedAt, + }, + ]); + }); }); }); @@ -199,7 +208,7 @@ describe('useTimelineEvents', () => { UseTimelineEventsProps, [DataLoadingState, TimelineArgs] >((args) => useTimelineEvents(args), { - initialProps: { ...props }, + initialProps: { ...props, startDate: '', endDate: '' }, }); // useEffect on params request @@ -218,21 +227,23 @@ describe('useTimelineEvents', () => { }, ]); - expect(mockSearch).toHaveBeenCalledTimes(2); + await waitFor(() => { + expect(mockSearch).toHaveBeenCalledTimes(2); - expect(result.current).toEqual([ - DataLoadingState.loaded, - { - events: mockEvents, - id: TimelineId.active, - inspect: result.current[1].inspect, - loadPage: result.current[1].loadPage, - pageInfo: result.current[1].pageInfo, - refetch: result.current[1].refetch, - totalCount: 32, - refreshedAt: result.current[1].refreshedAt, - }, - ]); + expect(result.current).toEqual([ + DataLoadingState.loaded, + { + events: mockEvents, + id: TimelineId.active, + inspect: result.current[1].inspect, + loadPage: result.current[1].loadPage, + pageInfo: result.current[1].pageInfo, + refetch: result.current[1].refetch, + totalCount: 32, + refreshedAt: result.current[1].refreshedAt, + }, + ]); + }); }); }); @@ -285,7 +296,7 @@ describe('useTimelineEvents', () => { UseTimelineEventsProps, [DataLoadingState, TimelineArgs] >((args) => useTimelineEvents(args), { - initialProps: { ...props }, + initialProps: { ...props, startDate: '', endDate: '' }, }); // useEffect on params request @@ -304,9 +315,9 @@ describe('useTimelineEvents', () => { fields: ['@timestamp', 'event.kind', 'event.category'], }); - await waitForNextUpdate(); - - expect(mockSearch).toHaveBeenCalledTimes(1); + await waitFor(() => { + expect(mockSearch).toHaveBeenCalledTimes(1); + }); }); }); @@ -316,7 +327,7 @@ describe('useTimelineEvents', () => { UseTimelineEventsProps, [DataLoadingState, TimelineArgs] >((args) => useTimelineEvents(args), { - initialProps: { ...props }, + initialProps: { ...props, startDate: '', endDate: '' }, }); // useEffect on params request @@ -330,19 +341,17 @@ describe('useTimelineEvents', () => { rerender({ ...props, startDate, endDate, fields: ['@timestamp'] }); - // since there is no new update in useEffect, it should throw an timeout error - await expect(waitForNextUpdate()).rejects.toThrowError(); - expect(mockSearch).toHaveBeenCalledTimes(0); }); }); + test('should not query again when a removed field is added back', async () => { await act(async () => { const { waitForNextUpdate, rerender } = renderHook< UseTimelineEventsProps, [DataLoadingState, TimelineArgs] >((args) => useTimelineEvents(args), { - initialProps: { ...props }, + initialProps: { ...props, startDate: '', endDate: '' }, }); // useEffect on params request @@ -357,35 +366,32 @@ describe('useTimelineEvents', () => { // remove `event.kind` from default fields rerender({ ...props, startDate, endDate, fields: ['@timestamp'] }); - // since there is no new update in useEffect, it should throw an timeout error - await expect(waitForNextUpdate()).rejects.toThrowError(); - expect(mockSearch).toHaveBeenCalledTimes(0); // request default Fields rerender({ ...props, startDate, endDate }); - // since there is no new update in useEffect, it should throw an timeout error - await expect(waitForNextUpdate()).rejects.toThrowError(); - expect(mockSearch).toHaveBeenCalledTimes(0); }); }); - describe('Fetch Notes', () => { - test('should call onLoad for notes when events are fetched', async () => { - await act(async () => { - const { waitFor } = renderHook( - (args) => useTimelineEvents(args), - { - initialProps: { ...props }, - } - ); - - await waitFor(() => { - expect(mockSearch).toHaveBeenCalledTimes(1); - expect(onLoadMock).toHaveBeenNthCalledWith(1, expect.objectContaining(mockEvents)); - }); + test('should return the combined list of events for all the pages when multiple pages are queried', async () => { + await act(async () => { + const { result } = renderHook((args) => useTimelineEvents(args), { + initialProps: { ...props }, + }); + await waitFor(() => { + expect(result.current[1].events).toHaveLength(10); + }); + + result.current[1].loadPage(1); + + await waitFor(() => { + expect(result.current[0]).toEqual(DataLoadingState.loadingMore); + }); + + await waitFor(() => { + expect(result.current[1].events).toHaveLength(20); }); }); }); diff --git a/x-pack/plugins/security_solution/public/timelines/containers/index.tsx b/x-pack/plugins/security_solution/public/timelines/containers/index.tsx index 0301f0123c30f..baaed281c7393 100644 --- a/x-pack/plugins/security_solution/public/timelines/containers/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/containers/index.tsx @@ -7,7 +7,7 @@ import deepEqual from 'fast-deep-equal'; import { isEmpty, noop } from 'lodash/fp'; -import { useCallback, useEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useRef, useState, useMemo } from 'react'; import { useDispatch } from 'react-redux'; import { Subscription } from 'rxjs'; @@ -46,12 +46,21 @@ import type { } from '../../../common/search_strategy/timeline/events/eql'; import { useTrackHttpRequest } from '../../common/lib/apm/use_track_http_request'; import { APP_UI_ID } from '../../../common/constants'; -import { useFetchNotes } from '../../notes/hooks/use_fetch_notes'; export interface TimelineArgs { events: TimelineItem[]; id: string; inspect: InspectResponse; + + /** + * `loadPage` loads the next page/batch of records. + * This is different from the data grid pages. Data grid pagination is only + * client side and changing data grid pages does not impact this function. + * + * When user manually requests next batch of records, then a next batch is fetched + * irrespective of where user is in Data grid pagination. + * + */ loadPage: LoadPage; pageInfo: Pick; refetch: inputsModel.Refetch; @@ -174,6 +183,15 @@ export const useTimelineEventsHandler = ({ } }, [dispatch, id]); + /** + * `wrappedLoadPage` loads the next page/batch of records. + * This is different from the data grid pages. Data grid pagination is only + * client side and changing data grid pages does not impact this function. + * + * When user manually requests next batch of records, then a next batch is fetched + * irrespective of where user is in Data grid pagination. + * + */ const wrappedLoadPage = useCallback( (newActivePage: number) => { clearSignalsState(); @@ -186,6 +204,12 @@ export const useTimelineEventsHandler = ({ [clearSignalsState, id] ); + useEffect(() => { + return () => { + searchSubscription$.current?.unsubscribe(); + }; + }, []); + const refetchGrid = useCallback(() => { if (refetch.current != null) { refetch.current(); @@ -240,10 +264,12 @@ export const useTimelineEventsHandler = ({ next: (response) => { if (!isRunningResponse(response)) { endTracking('success'); + setLoading(DataLoadingState.loaded); setTimelineResponse((prevResponse) => { const newTimelineResponse = { ...prevResponse, + /**/ events: getTimelineEvents(response.edges), inspect: getInspectResponse(response, prevResponse.inspect), pageInfo: response.pageInfo, @@ -269,6 +295,7 @@ export const useTimelineEventsHandler = ({ }, error: (msg) => { endTracking(abortCtrl.current.signal.aborted ? 'aborted' : 'error'); + setLoading(DataLoadingState.loaded); data.search.showError(msg); searchSubscription$.current.unsubscribe(); @@ -483,8 +510,8 @@ export const useTimelineEvents = ({ sort = initSortDefault, skip = false, timerangeKind, - fetchNotes = true, }: UseTimelineEventsProps): [DataLoadingState, TimelineArgs] => { + const [eventsPerPage, setEventsPerPage] = useState([[]]); const [dataLoadingState, timelineResponse, timelineSearchHandler] = useTimelineEventsHandler({ dataViewId, endDate, @@ -501,19 +528,35 @@ export const useTimelineEvents = ({ skip, timerangeKind, }); - const { onLoad } = useFetchNotes(); - const onTimelineSearchComplete: OnNextResponseHandler = useCallback( - (response) => { - if (fetchNotes) onLoad(response.events); - }, - [fetchNotes, onLoad] - ); + useEffect(() => { + /* + * `timelineSearchHandler` only returns the events for the current page. + * This effect is responsible for storing the events for each page so that + * the combined list of events can be supplied to DataGrid. + * + * */ + setEventsPerPage((prev) => { + const result = [...prev]; + result[timelineResponse.pageInfo.activePage] = timelineResponse.events; + return result; + }); + }, [timelineResponse.events, timelineResponse.pageInfo.activePage]); useEffect(() => { if (!timelineSearchHandler) return; - timelineSearchHandler(onTimelineSearchComplete); - }, [timelineSearchHandler, onTimelineSearchComplete]); + timelineSearchHandler(); + }, [timelineSearchHandler]); + + const combinedEvents = useMemo(() => eventsPerPage.flat(), [eventsPerPage]); + + const combinedResponse = useMemo( + () => ({ + ...timelineResponse, + events: combinedEvents, + }), + [timelineResponse, combinedEvents] + ); - return [dataLoadingState, timelineResponse]; + return [dataLoadingState, combinedResponse]; }; From 52fa1c9f57aa650b26858f9732df0fca15b387d3 Mon Sep 17 00:00:00 2001 From: Jatin Kathuria Date: Fri, 29 Nov 2024 17:39:16 +0100 Subject: [PATCH 2/2] fix: types --- .../public/timelines/components/timeline/tabs/eql/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx index 0b2de48e89693..874acc77ed1b7 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx @@ -116,7 +116,7 @@ export const EqlTabContentComponent: React.FC = ({ indexNames: selectedPatterns, language: 'eql', limit: sampleSize, - runtimeMappings: sourcererDataView.runtimeFieldMap as RunTimeMappings, + runtimeMappings: sourcererDataView?.runtimeFieldMap as RunTimeMappings, skip: !canQueryTimeline(), startDate: start, timerangeKind,