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
This commit is contained in:
Ashish Gupta 2025-08-25 23:38:20 +05:30 committed by GitHub
parent d054f386c7
commit 6dea079aa5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 274 additions and 58 deletions

View File

@ -21,6 +21,7 @@ import {
removeTagsFromChildren, removeTagsFromChildren,
} from '../../utils/entity'; } from '../../utils/entity';
import { sidebarClick } from '../../utils/sidebar'; import { sidebarClick } from '../../utils/sidebar';
import { columnPaginationTable } from '../../utils/table';
import { test } from '../fixtures/pages'; import { test } from '../fixtures/pages';
const table1 = new TableClass(); const table1 = new TableClass();
@ -262,46 +263,7 @@ test.describe('Table & Data Model columns table pagination', () => {
'2000' '2000'
); );
// 50 Row + 1 Header row await columnPaginationTable(page);
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);
}); });
test('pagination for dashboard data model columns should work', async ({ test('pagination for dashboard data model columns should work', async ({

View File

@ -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();
});
});
});

View File

@ -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);
};

View File

@ -14,10 +14,11 @@
import { Col, Row, Space, Tabs, TabsProps } from 'antd'; import { Col, Row, Space, Tabs, TabsProps } from 'antd';
import classNames from 'classnames'; import classNames from 'classnames';
import { cloneDeep, toString } from 'lodash'; import { cloneDeep, toString } from 'lodash';
import { useEffect, useMemo, useState } from 'react'; import { useCallback, useEffect, useMemo, useState } from 'react';
import { useTranslation } from 'react-i18next'; import { useTranslation } from 'react-i18next';
import { useNavigate } from 'react-router-dom'; import { useNavigate } from 'react-router-dom';
import { FQN_SEPARATOR_CHAR } from '../../../constants/char.constants'; import { FQN_SEPARATOR_CHAR } from '../../../constants/char.constants';
import { PAGE_SIZE_LARGE } from '../../../constants/constants';
import { EntityField } from '../../../constants/Feeds.constants'; import { EntityField } from '../../../constants/Feeds.constants';
import { EntityTabs, EntityType, FqnPart } from '../../../enums/entity.enum'; import { EntityTabs, EntityType, FqnPart } from '../../../enums/entity.enum';
import { import {
@ -26,6 +27,12 @@ import {
ColumnJoins, ColumnJoins,
} from '../../../generated/entity/data/table'; } from '../../../generated/entity/data/table';
import { TagSource } from '../../../generated/type/tagLabel'; 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 { getPartialNameFromTableFQN } from '../../../utils/CommonUtils';
import { import {
getColumnsDataWithVersionChanges, getColumnsDataWithVersionChanges,
@ -35,10 +42,12 @@ import {
getEntityVersionTags, getEntityVersionTags,
} from '../../../utils/EntityVersionUtils'; } from '../../../utils/EntityVersionUtils';
import { getVersionPath } from '../../../utils/RouterUtils'; import { getVersionPath } from '../../../utils/RouterUtils';
import { pruneEmptyChildren } from '../../../utils/TableUtils';
import { useRequiredParams } from '../../../utils/useRequiredParams'; import { useRequiredParams } from '../../../utils/useRequiredParams';
import { CustomPropertyTable } from '../../common/CustomPropertyTable/CustomPropertyTable'; import { CustomPropertyTable } from '../../common/CustomPropertyTable/CustomPropertyTable';
import DescriptionV1 from '../../common/EntityDescription/DescriptionV1'; import DescriptionV1 from '../../common/EntityDescription/DescriptionV1';
import Loader from '../../common/Loader/Loader'; import Loader from '../../common/Loader/Loader';
import { PagingHandlerParams } from '../../common/NextPrevious/NextPrevious.interface';
import TabsLabel from '../../common/TabsLabel/TabsLabel.component'; import TabsLabel from '../../common/TabsLabel/TabsLabel.component';
import { GenericProvider } from '../../Customization/GenericProvider/GenericProvider'; import { GenericProvider } from '../../Customization/GenericProvider/GenericProvider';
import DataAssetsVersionHeader from '../../DataAssets/DataAssetsVersionHeader/DataAssetsVersionHeader'; import DataAssetsVersionHeader from '../../DataAssets/DataAssetsVersionHeader/DataAssetsVersionHeader';
@ -66,10 +75,101 @@ const TableVersion: React.FC<TableVersionProp> = ({
const { t } = useTranslation(); const { t } = useTranslation();
const navigate = useNavigate(); const navigate = useNavigate();
const { tab } = useRequiredParams<{ tab: EntityTabs }>(); 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>( const [changeDescription, setChangeDescription] = useState<ChangeDescription>(
currentVersionData.changeDescription as 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( const entityFqn = useMemo(
() => currentVersionData.fullyQualifiedName ?? '', () => currentVersionData.fullyQualifiedName ?? '',
[currentVersionData.fullyQualifiedName] [currentVersionData.fullyQualifiedName]
@ -88,10 +188,10 @@ const TableVersion: React.FC<TableVersionProp> = ({
); );
const columns = useMemo(() => { const columns = useMemo(() => {
const colList = cloneDeep(currentVersionData.columns); const colList = cloneDeep(tableColumns);
return getColumnsDataWithVersionChanges<Column>(changeDescription, colList); return getColumnsDataWithVersionChanges<Column>(changeDescription, colList);
}, [currentVersionData, changeDescription]); }, [tableColumns, changeDescription]);
const handleTabChange = (activeKey: string) => { const handleTabChange = (activeKey: string) => {
navigate( navigate(
@ -170,7 +270,10 @@ const TableVersion: React.FC<TableVersionProp> = ({
columns={columns} columns={columns}
deletedColumnConstraintDiffs={deletedColumnConstraintDiffs} deletedColumnConstraintDiffs={deletedColumnConstraintDiffs}
deletedTableConstraintDiffs={deletedTableConstraintDiffs} deletedTableConstraintDiffs={deletedTableConstraintDiffs}
handelSearchCallback={handleSearchAction}
isLoading={columnsLoading}
joins={currentVersionData.joins as ColumnJoins[]} joins={currentVersionData.joins as ColumnJoins[]}
paginationProps={paginationProps}
tableConstraints={currentVersionData.tableConstraints} tableConstraints={currentVersionData.tableConstraints}
/> />
</Col> </Col>
@ -221,6 +324,9 @@ const TableVersion: React.FC<TableVersionProp> = ({
}, },
], ],
[ [
columnsLoading,
handleSearchAction,
paginationProps,
description, description,
entityFqn, entityFqn,
columns, 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 ( return (
<> <>
{isVersionLoading ? ( {isVersionLoading ? (

View File

@ -56,6 +56,7 @@ jest.mock('react-router-dom', () => ({
useParams: jest.fn().mockReturnValue({ useParams: jest.fn().mockReturnValue({
tab: 'tables', tab: 'tables',
}), }),
useLocation: jest.fn().mockImplementation(() => ({ pathname: 'pathname' })),
})); }));
describe('TableVersion tests', () => { describe('TableVersion tests', () => {

View File

@ -14,7 +14,7 @@
import { Tooltip } from 'antd'; import { Tooltip } from 'antd';
import { ColumnsType } from 'antd/lib/table'; import { ColumnsType } from 'antd/lib/table';
import { isEmpty, isUndefined } from 'lodash'; 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 { useTranslation } from 'react-i18next';
import { NO_DATA_PLACEHOLDER } from '../../../constants/constants'; import { NO_DATA_PLACEHOLDER } from '../../../constants/constants';
import { TABLE_SCROLL_VALUE } from '../../../constants/Table.constants'; import { TABLE_SCROLL_VALUE } from '../../../constants/Table.constants';
@ -27,6 +27,7 @@ import {
} from '../../../utils/EntityUtils'; } from '../../../utils/EntityUtils';
import { getFilterTags } from '../../../utils/TableTags/TableTags.utils'; import { getFilterTags } from '../../../utils/TableTags/TableTags.utils';
import { import {
getAllRowKeysByKeyName,
getTableExpandableConfig, getTableExpandableConfig,
makeData, makeData,
prepareConstraintIcon, prepareConstraintIcon,
@ -41,19 +42,30 @@ import { VersionTableProps } from './VersionTable.interfaces';
function VersionTable<T extends Column | SearchIndexField>({ function VersionTable<T extends Column | SearchIndexField>({
columnName, columnName,
columns, columns,
isLoading,
paginationProps,
joins, joins,
tableConstraints, tableConstraints,
addedColumnConstraintDiffs, addedColumnConstraintDiffs,
deletedColumnConstraintDiffs, deletedColumnConstraintDiffs,
addedTableConstraintDiffs, addedTableConstraintDiffs,
deletedTableConstraintDiffs, deletedTableConstraintDiffs,
handelSearchCallback,
}: Readonly<VersionTableProps<T>>) { }: Readonly<VersionTableProps<T>>) {
const [searchedColumns, setSearchedColumns] = useState<Array<T>>([]);
const { t } = useTranslation(); const { t } = useTranslation();
const [searchText, setSearchText] = useState(''); 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( const renderColumnName = useCallback(
(name: T['name'], record: T) => { (name: T['name'], record: T) => {
@ -129,7 +141,6 @@ function VersionTable<T extends Column | SearchIndexField>({
); );
}, },
[ [
columns,
tableConstraints, tableConstraints,
addedColumnConstraintDiffs, addedColumnConstraintDiffs,
deletedColumnConstraintDiffs, deletedColumnConstraintDiffs,
@ -218,11 +229,12 @@ function VersionTable<T extends Column | SearchIndexField>({
), ),
}, },
], ],
[columnName, joins, data, renderColumnName] [columnName, joins, renderColumnName]
); );
const handleSearchAction = (searchValue: string) => { const handleSearchAction = (searchValue: string) => {
setSearchText(searchValue); setSearchText(searchValue);
handelSearchCallback?.(searchValue);
}; };
const searchProps = useMemo( const searchProps = useMemo(
@ -235,26 +247,28 @@ function VersionTable<T extends Column | SearchIndexField>({
[searchText, handleSearchAction] [searchText, handleSearchAction]
); );
const handleExpandedRowsChange = useCallback((keys: readonly Key[]) => {
setExpandedRowKeys(keys as string[]);
}, []);
useEffect(() => { useEffect(() => {
if (!searchText) { setExpandedRowKeys(getAllRowKeysByKeyName<T>(columns ?? [], 'name'));
setSearchedColumns(columns); }, [columns]);
} else {
const searchCols = searchInColumns<T>(columns, searchText);
setSearchedColumns(searchCols);
}
}, [searchText, columns]);
return ( return (
<Table <Table
columns={versionTableColumns} columns={versionTableColumns}
containerClassName="m-b-sm" containerClassName="m-b-sm"
customPaginationProps={paginationProps}
data-testid="entity-table" data-testid="entity-table"
dataSource={data} dataSource={data}
expandable={{ expandable={{
...getTableExpandableConfig<T>(), ...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={{ locale={{
emptyText: <FilterTablePlaceHolder />, emptyText: <FilterTablePlaceHolder />,
}} }}

View File

@ -18,6 +18,8 @@ import {
FieldChange, FieldChange,
TableConstraint, TableConstraint,
} from '../../../generated/entity/data/table'; } 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> { export interface VersionTableProps<T extends Column | SearchIndexField> {
columnName: string; columnName: string;
@ -29,4 +31,16 @@ export interface VersionTableProps<T extends Column | SearchIndexField> {
deletedTableConstraintDiffs?: FieldChange[]; deletedTableConstraintDiffs?: FieldChange[];
constraintUpdatedColumns?: string[]; constraintUpdatedColumns?: string[];
tableConstraints?: Array<TableConstraint>; 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;
} }

View File

@ -12,7 +12,7 @@
*/ */
@import '../variables.less'; @import '../variables.less';
@switch-bg-color-primary: rgb(107 114 128 / 15%); @switch-bg-color-primary: rgba(107, 114, 128, 0.15);
@switch-border-color: rgba(107, 114, 128, 1); @switch-border-color: rgba(107, 114, 128, 1);
.ant-switch { .ant-switch {