mirror of
https://github.com/open-metadata/OpenMetadata.git
synced 2025-11-01 19:18:05 +00:00
fix the table version page breaking if large column list (#22864)
* fix the table version page breaking if large column list * fix the table version page breaking in case of large column set by adding pagination support * reverted some unwanted code changes * minor fix * added playwright and minor code fixes * fix the deplay paint issue of table data due to passing data as key value to table, have use state to manage expanded infomration * fix the sonar and switch color (cherry picked from commit 6dea079aa5d02a4c11b938187c4fa16b7a28b314)
This commit is contained in:
parent
b94e77761e
commit
89ea5d030d
@ -21,6 +21,7 @@ import {
|
||||
removeTagsFromChildren,
|
||||
} from '../../utils/entity';
|
||||
import { sidebarClick } from '../../utils/sidebar';
|
||||
import { columnPaginationTable } from '../../utils/table';
|
||||
import { test } from '../fixtures/pages';
|
||||
|
||||
const table1 = new TableClass();
|
||||
@ -262,46 +263,7 @@ test.describe('Table & Data Model columns table pagination', () => {
|
||||
'2000'
|
||||
);
|
||||
|
||||
// 50 Row + 1 Header row
|
||||
expect(page.getByTestId('entity-table').getByRole('row')).toHaveCount(51);
|
||||
|
||||
expect(page.getByTestId('page-indicator')).toHaveText(`Page 1 of 40`);
|
||||
|
||||
await page.getByTestId('next').click();
|
||||
|
||||
await page.waitForSelector('[data-testid="loader"]', {
|
||||
state: 'detached',
|
||||
});
|
||||
|
||||
expect(page.getByTestId('page-indicator')).toHaveText(`Page 2 of 40`);
|
||||
|
||||
expect(page.getByTestId('entity-table').getByRole('row')).toHaveCount(51);
|
||||
|
||||
await page.getByTestId('previous').click();
|
||||
|
||||
expect(page.getByTestId('page-indicator')).toHaveText(`Page 1 of 40`);
|
||||
|
||||
// Change page size to 15
|
||||
await page.getByTestId('page-size-selection-dropdown').click();
|
||||
await page.getByRole('menuitem', { name: '15 / Page' }).click();
|
||||
|
||||
await page.waitForSelector('[data-testid="loader"]', {
|
||||
state: 'detached',
|
||||
});
|
||||
|
||||
// 15 Row + 1 Header row
|
||||
expect(page.getByTestId('entity-table').getByRole('row')).toHaveCount(16);
|
||||
|
||||
// Change page size to 25
|
||||
await page.getByTestId('page-size-selection-dropdown').click();
|
||||
await page.getByRole('menuitem', { name: '25 / Page' }).click();
|
||||
|
||||
await page.waitForSelector('[data-testid="loader"]', {
|
||||
state: 'detached',
|
||||
});
|
||||
|
||||
// 25 Row + 1 Header row
|
||||
expect(page.getByTestId('entity-table').getByRole('row')).toHaveCount(26);
|
||||
await columnPaginationTable(page);
|
||||
});
|
||||
|
||||
test('pagination for dashboard data model columns should work', async ({
|
||||
|
||||
@ -0,0 +1,54 @@
|
||||
/*
|
||||
* Copyright 2025 Collate.
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
import { expect, test } from '@playwright/test';
|
||||
import { redirectToHomePage } from '../../utils/common';
|
||||
import { columnPaginationTable } from '../../utils/table';
|
||||
|
||||
// use the admin user to login
|
||||
test.use({ storageState: 'playwright/.auth/admin.json' });
|
||||
|
||||
test.describe('Table Version Page', () => {
|
||||
test('Pagination and Search should works for columns', async ({ page }) => {
|
||||
await redirectToHomePage(page);
|
||||
await page.goto(
|
||||
'/table/sample_data.ecommerce_db.shopify.performance_test_table/versions/0.1'
|
||||
);
|
||||
|
||||
await page.waitForLoadState('networkidle');
|
||||
await page.waitForSelector('[data-testid="loader"]', {
|
||||
state: 'detached',
|
||||
});
|
||||
|
||||
await test.step('Pagination Should Work', async () => {
|
||||
await columnPaginationTable(page);
|
||||
});
|
||||
|
||||
await test.step('Search Should Work', async () => {
|
||||
const searchResponse = page.waitForResponse(
|
||||
'/api/v1/tables/name/sample_data.ecommerce_db.shopify.performance_test_table/columns/search?q=test_col_0250*'
|
||||
);
|
||||
await page.getByTestId('searchbar').fill('test_col_0250');
|
||||
await searchResponse;
|
||||
|
||||
await page.waitForSelector('[data-testid="loader"]', {
|
||||
state: 'detached',
|
||||
});
|
||||
|
||||
expect(page.getByTestId('entity-table').getByRole('row')).toHaveCount(2);
|
||||
|
||||
expect(
|
||||
page.getByTestId('entity-table').getByText('test_col_0250')
|
||||
).toBeVisible();
|
||||
});
|
||||
});
|
||||
});
|
||||
@ -0,0 +1,57 @@
|
||||
/*
|
||||
* Copyright 2025 Collate.
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
import { expect, Page } from '@playwright/test';
|
||||
|
||||
// Pagination is performed for "performance_test_table" Table Entity
|
||||
export const columnPaginationTable = async (page: Page) => {
|
||||
// 50 Row + 1 Header row
|
||||
expect(page.getByTestId('entity-table').getByRole('row')).toHaveCount(51);
|
||||
|
||||
expect(page.getByTestId('page-indicator')).toHaveText(`Page 1 of 40`);
|
||||
|
||||
await page.getByTestId('next').click();
|
||||
|
||||
await page.waitForSelector('[data-testid="loader"]', {
|
||||
state: 'detached',
|
||||
});
|
||||
|
||||
expect(page.getByTestId('page-indicator')).toHaveText(`Page 2 of 40`);
|
||||
|
||||
expect(page.getByTestId('entity-table').getByRole('row')).toHaveCount(51);
|
||||
|
||||
await page.getByTestId('previous').click();
|
||||
|
||||
expect(page.getByTestId('page-indicator')).toHaveText(`Page 1 of 40`);
|
||||
|
||||
// Change page size to 15
|
||||
await page.getByTestId('page-size-selection-dropdown').click();
|
||||
await page.getByRole('menuitem', { name: '15 / Page' }).click();
|
||||
|
||||
await page.waitForSelector('[data-testid="loader"]', {
|
||||
state: 'detached',
|
||||
});
|
||||
|
||||
// 15 Row + 1 Header row
|
||||
expect(page.getByTestId('entity-table').getByRole('row')).toHaveCount(16);
|
||||
|
||||
// Change page size to 25
|
||||
await page.getByTestId('page-size-selection-dropdown').click();
|
||||
await page.getByRole('menuitem', { name: '25 / Page' }).click();
|
||||
|
||||
await page.waitForSelector('[data-testid="loader"]', {
|
||||
state: 'detached',
|
||||
});
|
||||
|
||||
// 25 Row + 1 Header row
|
||||
expect(page.getByTestId('entity-table').getByRole('row')).toHaveCount(26);
|
||||
};
|
||||
@ -14,10 +14,11 @@
|
||||
import { Col, Row, Space, Tabs, TabsProps } from 'antd';
|
||||
import classNames from 'classnames';
|
||||
import { cloneDeep, toString } from 'lodash';
|
||||
import { useEffect, useMemo, useState } from 'react';
|
||||
import { useCallback, useEffect, useMemo, useState } from 'react';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
import { useNavigate } from 'react-router-dom';
|
||||
import { FQN_SEPARATOR_CHAR } from '../../../constants/char.constants';
|
||||
import { PAGE_SIZE_LARGE } from '../../../constants/constants';
|
||||
import { EntityField } from '../../../constants/Feeds.constants';
|
||||
import { EntityTabs, EntityType, FqnPart } from '../../../enums/entity.enum';
|
||||
import {
|
||||
@ -26,6 +27,12 @@ import {
|
||||
ColumnJoins,
|
||||
} from '../../../generated/entity/data/table';
|
||||
import { TagSource } from '../../../generated/type/tagLabel';
|
||||
import { usePaging } from '../../../hooks/paging/usePaging';
|
||||
import { useFqn } from '../../../hooks/useFqn';
|
||||
import {
|
||||
getTableColumnsByFQN,
|
||||
searchTableColumnsByFQN,
|
||||
} from '../../../rest/tableAPI';
|
||||
import { getPartialNameFromTableFQN } from '../../../utils/CommonUtils';
|
||||
import {
|
||||
getColumnsDataWithVersionChanges,
|
||||
@ -35,10 +42,12 @@ import {
|
||||
getEntityVersionTags,
|
||||
} from '../../../utils/EntityVersionUtils';
|
||||
import { getVersionPath } from '../../../utils/RouterUtils';
|
||||
import { pruneEmptyChildren } from '../../../utils/TableUtils';
|
||||
import { useRequiredParams } from '../../../utils/useRequiredParams';
|
||||
import { CustomPropertyTable } from '../../common/CustomPropertyTable/CustomPropertyTable';
|
||||
import DescriptionV1 from '../../common/EntityDescription/DescriptionV1';
|
||||
import Loader from '../../common/Loader/Loader';
|
||||
import { PagingHandlerParams } from '../../common/NextPrevious/NextPrevious.interface';
|
||||
import TabsLabel from '../../common/TabsLabel/TabsLabel.component';
|
||||
import { GenericProvider } from '../../Customization/GenericProvider/GenericProvider';
|
||||
import DataAssetsVersionHeader from '../../DataAssets/DataAssetsVersionHeader/DataAssetsVersionHeader';
|
||||
@ -66,10 +75,101 @@ const TableVersion: React.FC<TableVersionProp> = ({
|
||||
const { t } = useTranslation();
|
||||
const navigate = useNavigate();
|
||||
const { tab } = useRequiredParams<{ tab: EntityTabs }>();
|
||||
const {
|
||||
currentPage,
|
||||
pageSize,
|
||||
handlePageChange,
|
||||
handlePageSizeChange,
|
||||
showPagination,
|
||||
paging,
|
||||
handlePagingChange,
|
||||
} = usePaging(PAGE_SIZE_LARGE);
|
||||
const { fqn: tableFqn } = useFqn();
|
||||
const [searchText, setSearchText] = useState('');
|
||||
// Pagination state for columns
|
||||
const [tableColumns, setTableColumns] = useState<Column[]>([]);
|
||||
const [columnsLoading, setColumnsLoading] = useState(true); // Start with loading state
|
||||
const [changeDescription, setChangeDescription] = useState<ChangeDescription>(
|
||||
currentVersionData.changeDescription as ChangeDescription
|
||||
);
|
||||
|
||||
// Function to fetch paginated columns or search results
|
||||
const fetchPaginatedColumns = useCallback(
|
||||
async (page = 1, searchQuery?: string) => {
|
||||
if (!tableFqn) {
|
||||
return;
|
||||
}
|
||||
|
||||
setColumnsLoading(true);
|
||||
try {
|
||||
const offset = (page - 1) * pageSize;
|
||||
|
||||
// Use search API if there's a search query, otherwise use regular pagination
|
||||
const response = searchQuery
|
||||
? await searchTableColumnsByFQN(tableFqn, {
|
||||
q: searchQuery,
|
||||
limit: pageSize,
|
||||
offset: offset,
|
||||
fields: 'tags',
|
||||
})
|
||||
: await getTableColumnsByFQN(tableFqn, {
|
||||
limit: pageSize,
|
||||
offset: offset,
|
||||
fields: 'tags',
|
||||
});
|
||||
|
||||
setTableColumns(pruneEmptyChildren(response.data) || []);
|
||||
handlePagingChange(response.paging);
|
||||
} catch {
|
||||
// Set empty state if API fails
|
||||
setTableColumns([]);
|
||||
handlePagingChange({
|
||||
offset: 1,
|
||||
limit: pageSize,
|
||||
total: 0,
|
||||
});
|
||||
} finally {
|
||||
setColumnsLoading(false);
|
||||
}
|
||||
},
|
||||
[tableFqn, pageSize]
|
||||
);
|
||||
|
||||
const handleSearchAction = useCallback((searchValue: string) => {
|
||||
setSearchText(searchValue);
|
||||
}, []);
|
||||
|
||||
const handleColumnsPageChange = useCallback(
|
||||
({ currentPage }: PagingHandlerParams) => {
|
||||
fetchPaginatedColumns(currentPage, searchText);
|
||||
handlePageChange(currentPage);
|
||||
},
|
||||
[paging, fetchPaginatedColumns, searchText]
|
||||
);
|
||||
|
||||
const paginationProps = useMemo(
|
||||
() => ({
|
||||
currentPage,
|
||||
showPagination,
|
||||
isLoading: columnsLoading,
|
||||
isNumberBased: Boolean(searchText),
|
||||
pageSize,
|
||||
paging,
|
||||
pagingHandler: handleColumnsPageChange,
|
||||
onShowSizeChange: handlePageSizeChange,
|
||||
}),
|
||||
[
|
||||
currentPage,
|
||||
showPagination,
|
||||
columnsLoading,
|
||||
searchText,
|
||||
pageSize,
|
||||
paging,
|
||||
handleColumnsPageChange,
|
||||
handlePageSizeChange,
|
||||
]
|
||||
);
|
||||
|
||||
const entityFqn = useMemo(
|
||||
() => currentVersionData.fullyQualifiedName ?? '',
|
||||
[currentVersionData.fullyQualifiedName]
|
||||
@ -88,10 +188,10 @@ const TableVersion: React.FC<TableVersionProp> = ({
|
||||
);
|
||||
|
||||
const columns = useMemo(() => {
|
||||
const colList = cloneDeep(currentVersionData.columns);
|
||||
const colList = cloneDeep(tableColumns);
|
||||
|
||||
return getColumnsDataWithVersionChanges<Column>(changeDescription, colList);
|
||||
}, [currentVersionData, changeDescription]);
|
||||
}, [tableColumns, changeDescription]);
|
||||
|
||||
const handleTabChange = (activeKey: string) => {
|
||||
navigate(
|
||||
@ -170,7 +270,10 @@ const TableVersion: React.FC<TableVersionProp> = ({
|
||||
columns={columns}
|
||||
deletedColumnConstraintDiffs={deletedColumnConstraintDiffs}
|
||||
deletedTableConstraintDiffs={deletedTableConstraintDiffs}
|
||||
handelSearchCallback={handleSearchAction}
|
||||
isLoading={columnsLoading}
|
||||
joins={currentVersionData.joins as ColumnJoins[]}
|
||||
paginationProps={paginationProps}
|
||||
tableConstraints={currentVersionData.tableConstraints}
|
||||
/>
|
||||
</Col>
|
||||
@ -221,6 +324,9 @@ const TableVersion: React.FC<TableVersionProp> = ({
|
||||
},
|
||||
],
|
||||
[
|
||||
columnsLoading,
|
||||
handleSearchAction,
|
||||
paginationProps,
|
||||
description,
|
||||
entityFqn,
|
||||
columns,
|
||||
@ -233,6 +339,14 @@ const TableVersion: React.FC<TableVersionProp> = ({
|
||||
]
|
||||
);
|
||||
|
||||
// Fetch columns when search changes
|
||||
useEffect(() => {
|
||||
if (tableFqn && !isVersionLoading) {
|
||||
// Reset to first page when search changes
|
||||
fetchPaginatedColumns(1, searchText || undefined);
|
||||
}
|
||||
}, [isVersionLoading, tableFqn, searchText, fetchPaginatedColumns, pageSize]);
|
||||
|
||||
return (
|
||||
<>
|
||||
{isVersionLoading ? (
|
||||
|
||||
@ -56,6 +56,7 @@ jest.mock('react-router-dom', () => ({
|
||||
useParams: jest.fn().mockReturnValue({
|
||||
tab: 'tables',
|
||||
}),
|
||||
useLocation: jest.fn().mockImplementation(() => ({ pathname: 'pathname' })),
|
||||
}));
|
||||
|
||||
describe('TableVersion tests', () => {
|
||||
|
||||
@ -14,7 +14,7 @@
|
||||
import { Tooltip } from 'antd';
|
||||
import { ColumnsType } from 'antd/lib/table';
|
||||
import { isEmpty, isUndefined } from 'lodash';
|
||||
import { useCallback, useEffect, useMemo, useState } from 'react';
|
||||
import { Key, useCallback, useEffect, useMemo, useState } from 'react';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
import { NO_DATA_PLACEHOLDER } from '../../../constants/constants';
|
||||
import { TABLE_SCROLL_VALUE } from '../../../constants/Table.constants';
|
||||
@ -27,6 +27,7 @@ import {
|
||||
} from '../../../utils/EntityUtils';
|
||||
import { getFilterTags } from '../../../utils/TableTags/TableTags.utils';
|
||||
import {
|
||||
getAllRowKeysByKeyName,
|
||||
getTableExpandableConfig,
|
||||
makeData,
|
||||
prepareConstraintIcon,
|
||||
@ -41,19 +42,30 @@ import { VersionTableProps } from './VersionTable.interfaces';
|
||||
function VersionTable<T extends Column | SearchIndexField>({
|
||||
columnName,
|
||||
columns,
|
||||
isLoading,
|
||||
paginationProps,
|
||||
joins,
|
||||
tableConstraints,
|
||||
addedColumnConstraintDiffs,
|
||||
deletedColumnConstraintDiffs,
|
||||
addedTableConstraintDiffs,
|
||||
deletedTableConstraintDiffs,
|
||||
handelSearchCallback,
|
||||
}: Readonly<VersionTableProps<T>>) {
|
||||
const [searchedColumns, setSearchedColumns] = useState<Array<T>>([]);
|
||||
const { t } = useTranslation();
|
||||
|
||||
const [searchText, setSearchText] = useState('');
|
||||
const [expandedRowKeys, setExpandedRowKeys] = useState<string[]>([]);
|
||||
|
||||
const data = useMemo(() => makeData<T>(searchedColumns), [searchedColumns]);
|
||||
const data = useMemo(() => {
|
||||
if (!searchText) {
|
||||
return makeData<T>(columns);
|
||||
} else {
|
||||
const searchCols = searchInColumns<T>(columns, searchText);
|
||||
|
||||
return makeData<T>(searchCols);
|
||||
}
|
||||
}, [searchText, columns]);
|
||||
|
||||
const renderColumnName = useCallback(
|
||||
(name: T['name'], record: T) => {
|
||||
@ -129,7 +141,6 @@ function VersionTable<T extends Column | SearchIndexField>({
|
||||
);
|
||||
},
|
||||
[
|
||||
columns,
|
||||
tableConstraints,
|
||||
addedColumnConstraintDiffs,
|
||||
deletedColumnConstraintDiffs,
|
||||
@ -218,11 +229,12 @@ function VersionTable<T extends Column | SearchIndexField>({
|
||||
),
|
||||
},
|
||||
],
|
||||
[columnName, joins, data, renderColumnName]
|
||||
[columnName, joins, renderColumnName]
|
||||
);
|
||||
|
||||
const handleSearchAction = (searchValue: string) => {
|
||||
setSearchText(searchValue);
|
||||
handelSearchCallback?.(searchValue);
|
||||
};
|
||||
|
||||
const searchProps = useMemo(
|
||||
@ -235,26 +247,28 @@ function VersionTable<T extends Column | SearchIndexField>({
|
||||
[searchText, handleSearchAction]
|
||||
);
|
||||
|
||||
const handleExpandedRowsChange = useCallback((keys: readonly Key[]) => {
|
||||
setExpandedRowKeys(keys as string[]);
|
||||
}, []);
|
||||
|
||||
useEffect(() => {
|
||||
if (!searchText) {
|
||||
setSearchedColumns(columns);
|
||||
} else {
|
||||
const searchCols = searchInColumns<T>(columns, searchText);
|
||||
setSearchedColumns(searchCols);
|
||||
}
|
||||
}, [searchText, columns]);
|
||||
setExpandedRowKeys(getAllRowKeysByKeyName<T>(columns ?? [], 'name'));
|
||||
}, [columns]);
|
||||
|
||||
return (
|
||||
<Table
|
||||
columns={versionTableColumns}
|
||||
containerClassName="m-b-sm"
|
||||
customPaginationProps={paginationProps}
|
||||
data-testid="entity-table"
|
||||
dataSource={data}
|
||||
expandable={{
|
||||
...getTableExpandableConfig<T>(),
|
||||
defaultExpandAllRows: true,
|
||||
rowExpandable: (record) => !isEmpty(record.children),
|
||||
onExpandedRowsChange: handleExpandedRowsChange,
|
||||
expandedRowKeys: expandedRowKeys,
|
||||
}}
|
||||
key={`${String(data)}`} // Necessary for working of the default auto expand all rows functionality.
|
||||
loading={isLoading}
|
||||
locale={{
|
||||
emptyText: <FilterTablePlaceHolder />,
|
||||
}}
|
||||
|
||||
@ -18,6 +18,8 @@ import {
|
||||
FieldChange,
|
||||
TableConstraint,
|
||||
} from '../../../generated/entity/data/table';
|
||||
import { Paging } from '../../../generated/type/paging';
|
||||
import { PagingHandlerParams } from '../../common/NextPrevious/NextPrevious.interface';
|
||||
|
||||
export interface VersionTableProps<T extends Column | SearchIndexField> {
|
||||
columnName: string;
|
||||
@ -29,4 +31,16 @@ export interface VersionTableProps<T extends Column | SearchIndexField> {
|
||||
deletedTableConstraintDiffs?: FieldChange[];
|
||||
constraintUpdatedColumns?: string[];
|
||||
tableConstraints?: Array<TableConstraint>;
|
||||
isLoading?: boolean;
|
||||
paginationProps?: {
|
||||
currentPage: number;
|
||||
showPagination: boolean;
|
||||
isLoading: boolean;
|
||||
isNumberBased: boolean;
|
||||
pageSize: number;
|
||||
paging: Paging;
|
||||
pagingHandler: ({ currentPage }: PagingHandlerParams) => void;
|
||||
onShowSizeChange: (page: number) => void;
|
||||
};
|
||||
handelSearchCallback?: (searchValue: string) => void;
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user