From 94b95f76d02956f74f88a3c7a2590e19ca1fa841 Mon Sep 17 00:00:00 2001 From: Kishor Rathva Date: Thu, 14 Dec 2023 04:14:15 +0530 Subject: [PATCH 1/6] Added helper function to generate page options based on UI setting value. Signed-off-by: Kishor Rathva --- .../utils/page_size_options.test.ts | 40 +++++++++++++++++++ .../components/utils/page_size_options.ts | 27 +++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 src/plugins/discover/public/application/components/utils/page_size_options.test.ts create mode 100644 src/plugins/discover/public/application/components/utils/page_size_options.ts diff --git a/src/plugins/discover/public/application/components/utils/page_size_options.test.ts b/src/plugins/discover/public/application/components/utils/page_size_options.test.ts new file mode 100644 index 00000000000..14ccc760d55 --- /dev/null +++ b/src/plugins/discover/public/application/components/utils/page_size_options.test.ts @@ -0,0 +1,40 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { defaultPageOptions, generatePageSizeOptions } from './page_size_options'; + +describe('generatePageSizeOptions', () => { + it('generates default options and additional options based on sample size', () => { + const sampleSize = 1000; + + const pageSizeOptions = generatePageSizeOptions(sampleSize); + + // Expected result based on the provided sample size + const expectedOptions = [...defaultPageOptions, 500, 1000]; + + // Check if the generated options match the expected result + expect(pageSizeOptions).toEqual(expectedOptions); + }); + + it('handles edge case when sample size is less than maxSize', () => { + const sampleSize = 50; + + // Call the function + const pageSizeOptions = generatePageSizeOptions(sampleSize); + + // Check if the generated options match the expected result + expect(pageSizeOptions).toEqual(defaultPageOptions); + }); + + it('handles edge case when sample size is less than 0', () => { + const sampleSize = -10; + + // Call the function + const pageSizeOptions = generatePageSizeOptions(sampleSize); + + // Check if the generated options match the expected result + expect(pageSizeOptions).toEqual(defaultPageOptions); + }); +}); diff --git a/src/plugins/discover/public/application/components/utils/page_size_options.ts b/src/plugins/discover/public/application/components/utils/page_size_options.ts new file mode 100644 index 00000000000..f5d9b9e0cd0 --- /dev/null +++ b/src/plugins/discover/public/application/components/utils/page_size_options.ts @@ -0,0 +1,27 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { uniq } from 'lodash'; + +/** + * Generates an array of pagination options based on the provided `sampleSize`. + * The array includes default values (25, 50, 100) and additional options derived from the `sampleSize` setting. + * Ensures uniqueness and sorts the array in ascending order, representing available page size options for pagination. + * @param {number} sampleSize - The sample size used to determine additional pagination options. + * @returns {number[]} - An array of available page size options. + */ + +export const generatePageSizeOptions = (sampleSize: number): number[] => { + if (sampleSize && sampleSize > 0) { + const maxSize = 500; + const pageSizeFromSetting = [...Array(Math.floor(sampleSize / maxSize)).keys()].map( + (i) => (i + 1) * maxSize + ); + return uniq([...defaultPageOptions, ...pageSizeFromSetting]).sort((a, b) => a - b); + } + return defaultPageOptions; +}; + +export const defaultPageOptions = [25, 50, 100]; From 484e681ecdc9a5bb93b13db5a0d9690e7885d202 Mon Sep 17 00:00:00 2001 From: Kishor Rathva Date: Thu, 14 Dec 2023 04:16:17 +0530 Subject: [PATCH 2/6] Added dynamic pagination based on config value for data grid table Signed-off-by: Kishor Rathva --- .../components/data_grid/data_grid_table.tsx | 6 +++++- .../components/utils/use_pagination.test.ts | 15 +++++++++++---- .../components/utils/use_pagination.ts | 18 +++++++++++++++--- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx b/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx index 655eb087e84..4cd59c57484 100644 --- a/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx +++ b/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx @@ -16,6 +16,8 @@ import { DocViewFilterFn, OpenSearchSearchHit } from '../../doc_views/doc_views_ import { usePagination } from '../utils/use_pagination'; import { SortOrder } from '../../../saved_searches/types'; import { buildColumns } from '../../utils/columns'; +import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public'; +import { DiscoverServices } from '../../../build_services'; export interface DataGridTableProps { columns: string[]; @@ -52,9 +54,11 @@ export const DataGridTable = ({ isContextView = false, isLoading = false, }: DataGridTableProps) => { + const { services } = useOpenSearchDashboards(); + const [inspectedHit, setInspectedHit] = useState(); const rowCount = useMemo(() => (rows ? rows.length : 0), [rows]); - const pagination = usePagination(rowCount); + const pagination = usePagination({ rowCount, services }); let adjustedColumns = buildColumns(columns); // handle case where the user removes selected filed and leaves only time column diff --git a/src/plugins/discover/public/application/components/utils/use_pagination.test.ts b/src/plugins/discover/public/application/components/utils/use_pagination.test.ts index 6eda1f7b4ca..ac1d3617081 100644 --- a/src/plugins/discover/public/application/components/utils/use_pagination.test.ts +++ b/src/plugins/discover/public/application/components/utils/use_pagination.test.ts @@ -6,10 +6,17 @@ import { renderHook, act } from '@testing-library/react-hooks'; import { usePagination } from './use_pagination'; +import { uiSettingsServiceMock } from '../../../../../../core/public/mocks'; + describe('usePagination', () => { + // TODO: create mock for DiscoverServices + const services: any = { + uiSettings: uiSettingsServiceMock.createSetupContract(), + }; + it('should initialize correctly with visParams and nRow', () => { const nRow = 30; - const { result } = renderHook(() => usePagination(nRow)); + const { result } = renderHook(() => usePagination({ rowCount: nRow, services })); expect(result.current).toEqual({ pageIndex: 0, @@ -22,7 +29,7 @@ describe('usePagination', () => { it('should update pageSize correctly when calling onChangeItemsPerPage', () => { const nRow = 30; - const { result } = renderHook(() => usePagination(nRow)); + const { result } = renderHook(() => usePagination({ rowCount: nRow, services })); act(() => { result.current?.onChangeItemsPerPage(20); @@ -39,7 +46,7 @@ describe('usePagination', () => { it('should update pageIndex correctly when calling onChangePage', () => { const nRow = 30; - const { result } = renderHook(() => usePagination(nRow)); + const { result } = renderHook(() => usePagination({ rowCount: nRow, services })); act(() => { result.current?.onChangePage(1); @@ -56,7 +63,7 @@ describe('usePagination', () => { it('should correct pageIndex if it exceeds maximum page index after nRow or perPage change', () => { const nRow = 300; - const { result } = renderHook(() => usePagination(nRow)); + const { result } = renderHook(() => usePagination({ rowCount: nRow, services })); act(() => { result.current?.onChangePage(4); diff --git a/src/plugins/discover/public/application/components/utils/use_pagination.ts b/src/plugins/discover/public/application/components/utils/use_pagination.ts index 98363e57ed9..fc334cbcb48 100644 --- a/src/plugins/discover/public/application/components/utils/use_pagination.ts +++ b/src/plugins/discover/public/application/components/utils/use_pagination.ts @@ -4,13 +4,25 @@ */ import { useState, useMemo, useCallback } from 'react'; +import { DiscoverServices } from '../../../build_services'; +import { SAMPLE_SIZE_SETTING } from '../../../../common'; +import { generatePageSizeOptions } from './page_size_options'; +export interface Props { + services: DiscoverServices; + rowCount: number; +} + +export const usePagination = ({ rowCount, services }: Props) => { + const { uiSettings } = services; -export const usePagination = (rowCount: number) => { const [pagination, setPagination] = useState({ pageIndex: 0, pageSize: 100 }); const pageCount = useMemo(() => Math.ceil(rowCount / pagination.pageSize), [ rowCount, pagination, ]); + const sampleSize = uiSettings.get(SAMPLE_SIZE_SETTING); + + const pageSizeOptions = generatePageSizeOptions(sampleSize); const onChangeItemsPerPage = useCallback( (pageSize: number) => setPagination((p) => ({ ...p, pageSize })), @@ -31,9 +43,9 @@ export const usePagination = (rowCount: number) => { onChangePage, pageIndex: pagination.pageIndex > pageCount - 1 ? 0 : pagination.pageIndex, pageSize: pagination.pageSize, - pageSizeOptions: [25, 50, 100], // TODO: make this configurable + pageSizeOptions, } : undefined, - [pagination, onChangeItemsPerPage, onChangePage, pageCount] + [pagination, onChangeItemsPerPage, onChangePage, pageCount, pageSizeOptions] ); }; From aed8b0d96846f22bbbb42ea5cd44477013165d4b Mon Sep 17 00:00:00 2001 From: Kishor Rathva Date: Thu, 14 Dec 2023 04:37:37 +0530 Subject: [PATCH 3/6] Update changelog Signed-off-by: Kishor Rathva --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 606355e8458..1c5dd96524c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Workspace] Setup workspace skeleton and implement basic CRUD API ([#5075](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5075)) - [Decouple] Add new cross compatibility check core service which export functionality for plugins to verify if their OpenSearch plugin counterpart is installed on the cluster or has incompatible version to configure the plugin behavior([#4710](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4710)) - [Discover] Display inner properties in the left navigation bar [#5429](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5429) +- [Discover] Added customizable pagination options based on Discover UI settings [#5610](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5610) ### 🐛 Bug Fixes @@ -947,4 +948,4 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### 🔩 Tests -- Update caniuse to fix failed integration tests ([#2322](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2322)) +- Update caniuse to fix failed integration tests ([#2322](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2322)) \ No newline at end of file From f170f1e292192dbd330e2fc0fe259fec1cca4a9c Mon Sep 17 00:00:00 2001 From: kishor82 Date: Tue, 19 Dec 2023 11:22:00 +0530 Subject: [PATCH 4/6] Removed service object from the props. Signed-off-by: kishor82 --- .../application/components/utils/use_pagination.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/plugins/discover/public/application/components/utils/use_pagination.ts b/src/plugins/discover/public/application/components/utils/use_pagination.ts index fc334cbcb48..427a18990a2 100644 --- a/src/plugins/discover/public/application/components/utils/use_pagination.ts +++ b/src/plugins/discover/public/application/components/utils/use_pagination.ts @@ -4,25 +4,20 @@ */ import { useState, useMemo, useCallback } from 'react'; -import { DiscoverServices } from '../../../build_services'; -import { SAMPLE_SIZE_SETTING } from '../../../../common'; import { generatePageSizeOptions } from './page_size_options'; export interface Props { - services: DiscoverServices; + pageSizeLimit: number; rowCount: number; } -export const usePagination = ({ rowCount, services }: Props) => { - const { uiSettings } = services; - +export const usePagination = ({ rowCount, pageSizeLimit }: Props) => { const [pagination, setPagination] = useState({ pageIndex: 0, pageSize: 100 }); const pageCount = useMemo(() => Math.ceil(rowCount / pagination.pageSize), [ rowCount, pagination, ]); - const sampleSize = uiSettings.get(SAMPLE_SIZE_SETTING); - const pageSizeOptions = generatePageSizeOptions(sampleSize); + const pageSizeOptions = generatePageSizeOptions(pageSizeLimit); const onChangeItemsPerPage = useCallback( (pageSize: number) => setPagination((p) => ({ ...p, pageSize })), From ed3380775947236c8e608ca89cf38a981de1b565 Mon Sep 17 00:00:00 2001 From: kishor82 Date: Tue, 19 Dec 2023 11:22:49 +0530 Subject: [PATCH 5/6] Added suggested changes and updated tests. Signed-off-by: kishor82 --- .../components/data_grid/data_grid_table.tsx | 4 +++- .../components/utils/page_size_options.ts | 24 ++++++++++--------- .../components/utils/use_pagination.test.ts | 24 +++++++------------ 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx b/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx index 4cd59c57484..99e1c167453 100644 --- a/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx +++ b/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx @@ -18,6 +18,7 @@ import { SortOrder } from '../../../saved_searches/types'; import { buildColumns } from '../../utils/columns'; import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public'; import { DiscoverServices } from '../../../build_services'; +import { SAMPLE_SIZE_SETTING } from '../../../../common'; export interface DataGridTableProps { columns: string[]; @@ -58,7 +59,8 @@ export const DataGridTable = ({ const [inspectedHit, setInspectedHit] = useState(); const rowCount = useMemo(() => (rows ? rows.length : 0), [rows]); - const pagination = usePagination({ rowCount, services }); + const pageSizeLimit = services.uiSettings.get(SAMPLE_SIZE_SETTING); + const pagination = usePagination({ rowCount, pageSizeLimit }); let adjustedColumns = buildColumns(columns); // handle case where the user removes selected filed and leaves only time column diff --git a/src/plugins/discover/public/application/components/utils/page_size_options.ts b/src/plugins/discover/public/application/components/utils/page_size_options.ts index f5d9b9e0cd0..93e527f129c 100644 --- a/src/plugins/discover/public/application/components/utils/page_size_options.ts +++ b/src/plugins/discover/public/application/components/utils/page_size_options.ts @@ -3,23 +3,25 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { uniq } from 'lodash'; - /** - * Generates an array of pagination options based on the provided `sampleSize`. - * The array includes default values (25, 50, 100) and additional options derived from the `sampleSize` setting. + * Generates an array of pagination options based on the provided `pageSizeLimit`. + * The array includes default values (25, 50, 100) and additional options derived from the `pageSizeLimit` setting. * Ensures uniqueness and sorts the array in ascending order, representing available page size options for pagination. - * @param {number} sampleSize - The sample size used to determine additional pagination options. + * @param {number} pageSizeLimit - The sample size used to determine additional pagination options. * @returns {number[]} - An array of available page size options. */ -export const generatePageSizeOptions = (sampleSize: number): number[] => { - if (sampleSize && sampleSize > 0) { - const maxSize = 500; - const pageSizeFromSetting = [...Array(Math.floor(sampleSize / maxSize)).keys()].map( - (i) => (i + 1) * maxSize +export const generatePageSizeOptions = (pageSizeLimit: number): number[] => { + const isInDefaultRange = pageSizeLimit < defaultPageOptions[defaultPageOptions.length - 1]; + + if (pageSizeLimit && pageSizeLimit > 0 && !isInDefaultRange) { + const stepSize = 500; + const pageSizeFromSetting = [...Array(Math.ceil(pageSizeLimit / stepSize)).keys()].map( + (i) => (i + 1) * stepSize + ); + return Array.from(new Set([...defaultPageOptions, ...pageSizeFromSetting])).sort( + (a, b) => a - b ); - return uniq([...defaultPageOptions, ...pageSizeFromSetting]).sort((a, b) => a - b); } return defaultPageOptions; }; diff --git a/src/plugins/discover/public/application/components/utils/use_pagination.test.ts b/src/plugins/discover/public/application/components/utils/use_pagination.test.ts index ac1d3617081..0a6f7363d80 100644 --- a/src/plugins/discover/public/application/components/utils/use_pagination.test.ts +++ b/src/plugins/discover/public/application/components/utils/use_pagination.test.ts @@ -6,30 +6,24 @@ import { renderHook, act } from '@testing-library/react-hooks'; import { usePagination } from './use_pagination'; -import { uiSettingsServiceMock } from '../../../../../../core/public/mocks'; - describe('usePagination', () => { - // TODO: create mock for DiscoverServices - const services: any = { - uiSettings: uiSettingsServiceMock.createSetupContract(), - }; - + const pageSizeLimit = 500; it('should initialize correctly with visParams and nRow', () => { const nRow = 30; - const { result } = renderHook(() => usePagination({ rowCount: nRow, services })); + const { result } = renderHook(() => usePagination({ rowCount: nRow, pageSizeLimit })); expect(result.current).toEqual({ pageIndex: 0, pageSize: 100, onChangeItemsPerPage: expect.any(Function), onChangePage: expect.any(Function), - pageSizeOptions: [25, 50, 100], + pageSizeOptions: [25, 50, 100, 500], }); }); it('should update pageSize correctly when calling onChangeItemsPerPage', () => { const nRow = 30; - const { result } = renderHook(() => usePagination({ rowCount: nRow, services })); + const { result } = renderHook(() => usePagination({ rowCount: nRow, pageSizeLimit })); act(() => { result.current?.onChangeItemsPerPage(20); @@ -40,13 +34,13 @@ describe('usePagination', () => { pageSize: 20, onChangeItemsPerPage: expect.any(Function), onChangePage: expect.any(Function), - pageSizeOptions: [25, 50, 100], + pageSizeOptions: [25, 50, 100, 500], }); }); it('should update pageIndex correctly when calling onChangePage', () => { const nRow = 30; - const { result } = renderHook(() => usePagination({ rowCount: nRow, services })); + const { result } = renderHook(() => usePagination({ rowCount: nRow, pageSizeLimit })); act(() => { result.current?.onChangePage(1); @@ -57,13 +51,13 @@ describe('usePagination', () => { pageSize: 100, onChangeItemsPerPage: expect.any(Function), onChangePage: expect.any(Function), - pageSizeOptions: [25, 50, 100], + pageSizeOptions: [25, 50, 100, 500], }); }); it('should correct pageIndex if it exceeds maximum page index after nRow or perPage change', () => { const nRow = 300; - const { result } = renderHook(() => usePagination({ rowCount: nRow, services })); + const { result } = renderHook(() => usePagination({ rowCount: nRow, pageSizeLimit })); act(() => { result.current?.onChangePage(4); @@ -74,7 +68,7 @@ describe('usePagination', () => { pageSize: 100, onChangeItemsPerPage: expect.any(Function), onChangePage: expect.any(Function), - pageSizeOptions: [25, 50, 100], + pageSizeOptions: [25, 50, 100, 500], }); }); }); From 899516dc1d29f2d80fadcac93c6085c17574a961 Mon Sep 17 00:00:00 2001 From: kishor82 Date: Tue, 19 Dec 2023 17:49:29 +0530 Subject: [PATCH 6/6] fix functional test issue due to ui setting Signed-off-by: kishor82 --- .../public/application/components/data_grid/data_grid_table.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx b/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx index 99e1c167453..1bac2f0f406 100644 --- a/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx +++ b/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx @@ -59,7 +59,7 @@ export const DataGridTable = ({ const [inspectedHit, setInspectedHit] = useState(); const rowCount = useMemo(() => (rows ? rows.length : 0), [rows]); - const pageSizeLimit = services.uiSettings.get(SAMPLE_SIZE_SETTING); + const pageSizeLimit = services.uiSettings?.get(SAMPLE_SIZE_SETTING); const pagination = usePagination({ rowCount, pageSizeLimit }); let adjustedColumns = buildColumns(columns);