Fix #19068: While user in Pagination, Browser Back button takes them to start page (#19229)

* fix browser back btn issue related to pagination

* common function to handle cursor based data fetches

* Revert "common function to handle cursor based data fetches"

This reverts commit a585f3aaec49c966ae8c1aa5436d41c2456073c5.

* updated usePaging hook to store and retrieve cursor

* address PR comments

* address pr comments

* refactor handle paging cursor

* updated handlPageChange to setPage and cursorValues

* used useCustomLocation inplace of useLocation

* added Pagesize and refactor the history state object

* issue fix

* handle filter issue

* address pr comments

* fix failing unit test cases

* fix sonarqube issue

* address pr comment

* address pr comment

* added useTableFilters hook

* address pr comments

* update useTableFilter to handle multiple query params

* update useTableFilter

* fix failing unit test

* address pr comments

* address pr comments

* playwright test cases

* address pr comments

* issue fix

* issue fix

* revert generated file changes

* address pr comment for playwright test

* minor fix

* fix breadcrumb issue

* revert location changes

* address pr comment

* fix failing e2e test

---------

Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com>
This commit is contained in:
Shrushti Polekar 2025-01-29 17:06:19 +05:30 committed by GitHub
parent b3eac6b7c5
commit d1710c8125
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
20 changed files with 363 additions and 25 deletions

View File

@ -14,6 +14,7 @@ import test, { expect } from '@playwright/test';
import { SidebarItem } from '../../constant/sidebar';
import { TableClass } from '../../support/entity/TableClass';
import { createNewPage, redirectToHomePage } from '../../utils/common';
import { getFirstRowColumnLink } from '../../utils/entity';
import { sidebarClick } from '../../utils/sidebar';
// use the admin user to login
@ -92,4 +93,50 @@ test.describe('Table pagination sorting search scenarios ', () => {
await expect(page.getByRole('tab', { name: 'Schema' })).toContainText('4');
});
test('should persist current page', async ({ page }) => {
page.goto('/databaseSchema/sample_data.ecommerce_db.shopify');
await expect(page.getByTestId('databaseSchema-tables')).toBeVisible();
await page.getByTestId('next').click();
const initialPageIndicator = await page
.locator('[data-testid="page-indicator"]')
.textContent();
const linkInColumn = getFirstRowColumnLink(page);
await linkInColumn.click();
await page.goBack();
const pageIndicatorAfterBack = await page
.locator('[data-testid="page-indicator"]')
.textContent();
expect(pageIndicatorAfterBack).toBe(initialPageIndicator);
});
test('should persist page size', async ({ page }) => {
page.goto('/databaseSchema/sample_data.ecommerce_db.shopify');
await expect(page.getByTestId('databaseSchema-tables')).toBeVisible();
const pageSizeDropdown = page.getByTestId('page-size-selection-dropdown');
await pageSizeDropdown.click();
const dropdownOptions = page.locator('.ant-dropdown-menu');
await expect(dropdownOptions).toBeVisible();
const optionToSelect = dropdownOptions.locator('text="15 / Page"');
await optionToSelect.click();
const linkInColumn = getFirstRowColumnLink(page);
await linkInColumn.click();
await page.goBack();
await expect(pageSizeDropdown).toHaveText('15 / Page');
});
});

View File

@ -1367,3 +1367,12 @@ export const getTextFromHtmlString = (description?: string): string => {
return description.replace(/<[^>]*>/g, '').trim();
};
export const getFirstRowColumnLink = (page: Page) => {
const table = page.locator('[data-testid="databaseSchema-tables"]');
const firstRowFirstColumn = table.locator(
'tbody tr:first-child td:first-child'
);
return firstRowFirstColumn.locator('[data-testid="column-name"] a');
};

View File

@ -99,6 +99,19 @@ const mockSearchAPIResponse = {
},
},
};
const mockLocationPathname = '/mock-path';
jest.mock('react-router-dom', () => ({
useHistory: jest.fn().mockImplementation(() => ({
history: {
push: jest.fn(),
},
replace: jest.fn(),
})),
useLocation: jest.fn().mockImplementation(() => ({
pathname: mockLocationPathname,
})),
}));
describe('DataAssetAsyncSelectList', () => {
it('should render without crashing', async () => {

View File

@ -69,6 +69,19 @@ jest.mock('../../../../../rest/ingestionPipelineAPI', () => ({
})
),
}));
const mockLocationPathname = '/mock-path';
jest.mock('react-router-dom', () => ({
useHistory: jest.fn().mockImplementation(() => ({
history: {
push: jest.fn(),
},
replace: jest.fn(),
})),
useLocation: jest.fn().mockImplementation(() => ({
pathname: mockLocationPathname,
})),
}));
describe('IngestionPipelineList', () => {
it('should show loader until get status of airflow', () => {

View File

@ -120,7 +120,9 @@ const NextPrevious: FC<NextPreviousProps> = ({
onClick: () => onShowSizeChange(size),
})),
}}>
<Button onClick={(e) => e.preventDefault()}>
<Button
data-testid="page-size-selection-dropdown"
onClick={(e) => e.preventDefault()}>
{`${pageSize} / ${t('label.page')}`}
<DownOutlined />
</Button>

View File

@ -612,3 +612,7 @@ export const STATUS_LABEL = {
[Status.Stopped]: 'Stopped',
[Status.Success]: 'Success',
};
export const INITIAL_TABLE_FILTERS = {
showDeletedTables: false,
};

View File

@ -10,6 +10,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import qs from 'qs';
import {
Dispatch,
SetStateAction,
@ -17,49 +18,118 @@ import {
useMemo,
useState,
} from 'react';
import { useHistory } from 'react-router-dom';
import {
INITIAL_PAGING_VALUE,
PAGE_SIZE_BASE,
pagingObject,
} from '../../constants/constants';
import { CursorType } from '../../enums/pagination.enum';
import { Paging } from '../../generated/type/paging';
import useCustomLocation from '../useCustomLocation/useCustomLocation';
interface CursorState {
cursorType: CursorType | null;
cursorValue: string | null;
}
interface PagingHistoryStateData {
cursorData: CursorState;
currentPage: number | null;
pageSize: number | null;
}
export interface UsePagingInterface {
paging: Paging;
handlePagingChange: Dispatch<SetStateAction<Paging>>;
currentPage: number;
handlePageChange: Dispatch<SetStateAction<number>>;
handlePageChange: (
page: number | ((page: number) => number),
cursorData?: CursorState,
pageSize?: number
) => void;
pageSize: number;
handlePageSizeChange: (page: number) => void;
showPagination: boolean;
pagingCursor: PagingHistoryStateData;
}
export const usePaging = (
defaultPageSize = PAGE_SIZE_BASE
): UsePagingInterface => {
// Extract initial values from history state if present
const history = useHistory();
const location = useCustomLocation();
const historyState = location.state as any;
const initialPageSize = historyState?.pageSize ?? defaultPageSize;
const initialCurrentPage = historyState?.cursorData?.cursorType
? historyState.currentPage
: INITIAL_PAGING_VALUE;
const [paging, setPaging] = useState<Paging>(pagingObject);
const [currentPage, setCurrentPage] = useState<number>(INITIAL_PAGING_VALUE);
const [pageSize, setPageSize] = useState(defaultPageSize);
const [currentPage, setCurrentPage] = useState<number>(initialCurrentPage);
const [pageSize, setPageSize] = useState<number>(initialPageSize);
const pagingCursorHistoryState: PagingHistoryStateData = {
cursorData: historyState?.cursorData,
currentPage: historyState?.currentPage,
pageSize: historyState?.pageSize,
};
const handlePageSize = useCallback(
(page: number) => {
setPageSize(page);
setCurrentPage(INITIAL_PAGING_VALUE);
const searchParams = qs.stringify(
qs.parse(location.search, { ignoreQueryPrefix: true }),
{ addQueryPrefix: true }
);
// Update location state to persist pageSize for navigation
history.replace({
state: {
pageSize: page,
},
search: searchParams,
});
},
[setPageSize, setCurrentPage]
[setPageSize, setCurrentPage, history, location]
);
const paginationVisible = useMemo(() => {
return paging.total > pageSize || pageSize !== defaultPageSize;
}, [defaultPageSize, paging, pageSize]);
const handlePageChange = useCallback(
(
page: number | ((page: number) => number),
cursorData?: CursorState,
pageSize?: number
) => {
const searchParams = qs.stringify(
qs.parse(location.search, { ignoreQueryPrefix: true }),
{ addQueryPrefix: true }
);
setCurrentPage(page);
if (cursorData) {
history.replace({
state: {
cursorData,
currentPage: page,
pageSize,
},
search: searchParams,
});
}
},
[setCurrentPage, history, location]
);
return {
paging,
handlePagingChange: setPaging,
currentPage,
handlePageChange: setCurrentPage,
handlePageChange,
pageSize,
handlePageSizeChange: handlePageSize,
showPagination: paginationVisible,
pagingCursor: pagingCursorHistoryState,
};
};

View File

@ -0,0 +1,90 @@
/*
* 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 { isArray, isEmpty, isNil } from 'lodash';
import qs, { ParsedQs } from 'qs';
import { useMemo } from 'react';
import { useHistory } from 'react-router-dom';
import { useLocationSearch } from './LocationSearch/useLocationSearch';
import useCustomLocation from './useCustomLocation/useCustomLocation';
type FilterState = Record<string, string | boolean | string[] | null>;
export const useTableFilters = <T extends FilterState>(initialFilters: T) => {
const location = useCustomLocation();
const history = useHistory();
const searchQuery = useLocationSearch<ParsedQs>();
const parseFiltersFromUrl = (): T => {
const parsedFilters = { ...initialFilters };
for (const key of Object.keys(initialFilters)) {
const paramValue = searchQuery[key];
const initialValue = initialFilters[key as keyof T];
if (!isNil(paramValue)) {
if (typeof initialValue === 'boolean') {
parsedFilters[key as keyof T] = (paramValue === 'true') as T[keyof T];
} else if (isArray(initialValue)) {
parsedFilters[key as keyof T] =
typeof paramValue === 'string'
? (paramValue.split(',').map((val) => val.trim()) as T[keyof T])
: (paramValue as T[keyof T]);
} else {
parsedFilters[key as keyof T] = paramValue as T[keyof T];
}
}
}
return parsedFilters;
};
// Update URL with the filters applied for the table as query parameters.
const updateUrlWithFilters = (updatedFilters: FilterState) => {
const currentQueryParams = qs.parse(window.location.search, {
ignoreQueryPrefix: true,
});
// Merge the existing query params with the updated filters
const mergedQueryParams = {
...currentQueryParams,
...updatedFilters,
};
Object.keys(mergedQueryParams).forEach((key) => {
const value = mergedQueryParams[key];
if (isNil(value) || (isArray(value) && isEmpty(value))) {
delete mergedQueryParams[key];
} else if (isArray(value)) {
mergedQueryParams[key] = value.join(',');
}
});
history.replace({
search: qs.stringify(mergedQueryParams, {
addQueryPrefix: true,
}),
});
};
const filters = useMemo(
() => parseFiltersFromUrl(),
[location.search, initialFilters]
);
// Update multiple filters at a time
const setFilters = (newFilters: FilterState) => {
updateUrlWithFilters(newFilters);
};
return { filters, setFilters };
};

View File

@ -212,6 +212,15 @@ const mockPaging = {
total: 1,
};
const mockPagingCursor = {
cursorData: {
cursorType: null,
cursorValue: null,
},
currentPage: 1,
pageSize: 10,
};
const mockCurrentHandleIngestionListUpdate = jest.fn();
const mockCurrentHandleSearchChange = jest.fn();
const mockCurrentOnPageChange = jest.fn();
@ -240,5 +249,6 @@ export const ingestionProps: IngestionProps = {
pageSize: 10,
handlePageSizeChange: jest.fn(),
showPagination: true,
pagingCursor: mockPagingCursor,
},
};

View File

@ -15,6 +15,7 @@ import { AddIngestionButtonProps } from '../components/Settings/Services/Ingesti
import { IngestionListTableProps } from '../components/Settings/Services/Ingestion/IngestionListTable/IngestionListTable.interface';
import { PipelineActionsProps } from '../components/Settings/Services/Ingestion/IngestionListTable/PipelineActions/PipelineActions.interface';
import { PipelineActionsDropdownProps } from '../components/Settings/Services/Ingestion/IngestionListTable/PipelineActions/PipelineActionsDropdown.interface';
import { CursorType } from '../enums/pagination.enum';
import { ServiceCategory } from '../enums/service.enum';
import { DatabaseServiceType } from '../generated/entity/data/database';
import { ConfigType } from '../generated/entity/services/databaseService';
@ -146,6 +147,14 @@ export const mockESIngestionData: IngestionPipeline = {
deleted: false,
provider: ProviderType.User,
};
const mockPagingCursor = {
cursorData: {
cursorType: CursorType.AFTER,
cursorValue: 'mockCursorValue',
},
currentPage: 1,
pageSize: 10,
};
const mockPagingInfoObj = {
paging: { total: 10 },
handlePagingChange: jest.fn(),
@ -154,6 +163,7 @@ const mockPagingInfoObj = {
pageSize: 10,
handlePageSizeChange: jest.fn(),
showPagination: true,
pagingCursor: mockPagingCursor,
};
export const mockIngestionListTableProps: IngestionListTableProps = {

View File

@ -40,6 +40,7 @@ import {
getEntityDetailsPath,
getVersionPath,
INITIAL_PAGING_VALUE,
INITIAL_TABLE_FILTERS,
PAGE_SIZE,
ROUTES,
} from '../../constants/constants';
@ -65,6 +66,7 @@ import { Include } from '../../generated/type/include';
import { TagLabel } from '../../generated/type/tagLabel';
import { usePaging } from '../../hooks/paging/usePaging';
import { useFqn } from '../../hooks/useFqn';
import { useTableFilters } from '../../hooks/useTableFilters';
import { FeedCounts } from '../../interface/feed.interface';
import {
getDatabaseSchemaDetailsByFQN,
@ -100,13 +102,16 @@ const DatabaseSchemaPage: FunctionComponent = () => {
handlePagingChange,
currentPage,
handlePageChange,
pagingCursor,
} = pagingInfo;
const { filters: tableFilters, setFilters } = useTableFilters(
INITIAL_TABLE_FILTERS
);
const { tab: activeTab = EntityTabs.TABLE } =
useParams<{ tab: EntityTabs }>();
const { fqn: decodedDatabaseSchemaFQN } = useFqn();
const history = useHistory();
const [threadType, setThreadType] = useState<ThreadType>(
ThreadType.Conversation
);
@ -126,7 +131,6 @@ const DatabaseSchemaPage: FunctionComponent = () => {
const [threadLink, setThreadLink] = useState<string>('');
const [databaseSchemaPermission, setDatabaseSchemaPermission] =
useState<OperationPermission>(DEFAULT_ENTITY_PERMISSION);
const [showDeletedTables, setShowDeletedTables] = useState<boolean>(false);
const [storedProcedureCount, setStoredProcedureCount] = useState(0);
const [updateProfilerSetting, setUpdateProfilerSetting] =
@ -143,7 +147,7 @@ const DatabaseSchemaPage: FunctionComponent = () => {
);
const handleShowDeletedTables = (value: boolean) => {
setShowDeletedTables(value);
setFilters({ showDeletedTables: value });
handlePageChange(INITIAL_PAGING_VALUE);
};
@ -226,7 +230,11 @@ const DatabaseSchemaPage: FunctionComponent = () => {
const { description: schemaDescription = '' } = response;
setDatabaseSchema(response);
setDescription(schemaDescription);
setShowDeletedTables(response.deleted ?? false);
if (response.deleted) {
setFilters({
showDeletedTables: response.deleted,
});
}
} catch (err) {
// Error
if ((err as AxiosError)?.response?.status === ClientErrors.FORBIDDEN) {
@ -245,7 +253,9 @@ const DatabaseSchemaPage: FunctionComponent = () => {
...params,
databaseSchema: decodedDatabaseSchemaFQN,
limit: pageSize,
include: showDeletedTables ? Include.Deleted : Include.NonDeleted,
include: tableFilters.showDeletedTables
? Include.Deleted
: Include.NonDeleted,
});
setTableData(res.data);
handlePagingChange(res.paging);
@ -255,7 +265,7 @@ const DatabaseSchemaPage: FunctionComponent = () => {
setTableDataLoading(false);
}
},
[decodedDatabaseSchemaFQN, showDeletedTables, pageSize]
[decodedDatabaseSchemaFQN, tableFilters.showDeletedTables, pageSize]
);
const onDescriptionEdit = useCallback((): void => {
@ -422,6 +432,13 @@ const DatabaseSchemaPage: FunctionComponent = () => {
);
const handleToggleDelete = (version?: number) => {
history.replace({
state: {
cursorData: null,
pageSize: null,
currentPage: INITIAL_PAGING_VALUE,
},
});
setDatabaseSchema((prev) => {
if (!prev) {
return prev;
@ -461,10 +478,18 @@ const DatabaseSchemaPage: FunctionComponent = () => {
({ cursorType, currentPage }: PagingHandlerParams) => {
if (cursorType) {
getSchemaTables({ [cursorType]: paging[cursorType] });
handlePageChange(
currentPage,
{
cursorType: cursorType,
cursorValue: paging[cursorType]!,
},
pageSize
);
}
handlePageChange(currentPage);
},
[paging, getSchemaTables]
[paging, getSchemaTables, handlePageChange]
);
const versionHandler = useCallback(() => {
@ -521,10 +546,19 @@ const DatabaseSchemaPage: FunctionComponent = () => {
useEffect(() => {
if (viewDatabaseSchemaPermission && decodedDatabaseSchemaFQN) {
getSchemaTables({ limit: pageSize });
if (pagingCursor?.cursorData?.cursorType) {
// Fetch data if cursorType is present in state with cursor Value to handle browser back navigation
getSchemaTables({
[pagingCursor?.cursorData?.cursorType]:
pagingCursor?.cursorData?.cursorValue,
});
} else {
// Otherwise, just fetch the data without cursor value
getSchemaTables({ limit: pageSize });
}
}
}, [
showDeletedTables,
tableFilters.showDeletedTables,
decodedDatabaseSchemaFQN,
viewDatabaseSchemaPermission,
deleted,
@ -588,7 +622,7 @@ const DatabaseSchemaPage: FunctionComponent = () => {
description,
editDescriptionPermission,
isEdit,
showDeletedTables,
showDeletedTables: tableFilters.showDeletedTables,
tableDataLoading,
editCustomAttributePermission,
editTagsPermission,
@ -620,7 +654,7 @@ const DatabaseSchemaPage: FunctionComponent = () => {
description,
editDescriptionPermission,
isEdit,
showDeletedTables,
tableFilters.showDeletedTables,
tableDataLoading,
editCustomAttributePermission,
editTagsPermission,

View File

@ -186,11 +186,18 @@ const API_FIELDS = [
'dataProducts',
];
const mockLocationPathname =
'/databaseSchema/sample_data.ecommerce_db.shopify/table';
jest.mock('react-router-dom', () => ({
useHistory: jest.fn().mockImplementation(() => ({
history: {
push: jest.fn(),
},
replace: jest.fn(),
})),
useLocation: jest.fn().mockImplementation(() => ({
pathname: mockLocationPathname,
})),
useParams: jest.fn().mockImplementation(() => mockParams),
}));

View File

@ -151,12 +151,15 @@ jest.mock('../../utils/EntityVersionUtils', () => ({
}));
const mockPush = jest.fn();
const mockLocationPathname = '/mock-path';
jest.mock('react-router-dom', () => ({
useParams: jest.fn(() => MOCK_PARAMS),
useHistory: jest.fn(() => ({
push: mockPush,
})),
useLocation: jest.fn().mockImplementation(() => ({
pathname: mockLocationPathname,
})),
}));
describe('DatabaseSchemaVersionPage', () => {

View File

@ -24,7 +24,7 @@ import GlossaryPage from './GlossaryPage.component';
jest.mock('../../../hooks/useFqn', () => ({
useFqn: jest.fn().mockReturnValue({ fqn: 'Business Glossary' }),
}));
const mockLocationPathname = '/mock-path';
jest.mock('react-router-dom', () => ({
useHistory: () => ({
push: jest.fn(),
@ -33,6 +33,9 @@ jest.mock('react-router-dom', () => ({
useParams: jest.fn().mockReturnValue({
glossaryName: 'GlossaryName',
}),
useLocation: jest.fn().mockImplementation(() => ({
pathname: mockLocationPathname,
})),
}));
jest.mock('../../../components/MyData/LeftSidebar/LeftSidebar.component', () =>

View File

@ -52,6 +52,7 @@ const MOCK_DATA = [
},
];
const mockPush = jest.fn();
const mockLocationPathname = '/mock-path';
jest.mock('react-router-dom', () => ({
Link: jest
@ -64,6 +65,9 @@ jest.mock('react-router-dom', () => ({
useHistory: jest.fn().mockImplementation(() => ({
push: mockPush,
})),
useLocation: jest.fn().mockImplementation(() => ({
pathname: mockLocationPathname,
})),
}));
jest.mock('../../rest/alertsAPI', () => ({

View File

@ -50,7 +50,7 @@ const MOCK_DATA = [
},
];
const mockPush = jest.fn();
const mockLocationPathname = '/mock-path';
jest.mock('react-router-dom', () => ({
Link: jest
.fn()
@ -62,6 +62,9 @@ jest.mock('react-router-dom', () => ({
useHistory: jest.fn().mockImplementation(() => ({
push: mockPush,
})),
useLocation: jest.fn().mockImplementation(() => ({
pathname: mockLocationPathname,
})),
}));
jest.mock('../../rest/alertsAPI', () => ({

View File

@ -18,11 +18,14 @@ import { POLICY_LIST_WITH_PAGING } from '../../RolesPage/Roles.mock';
import PoliciesListPage from './PoliciesListPage';
const mockPush = jest.fn();
const mockLocationPathname = '/mock-path';
jest.mock('react-router-dom', () => ({
useHistory: jest.fn().mockImplementation(() => ({
push: mockPush,
})),
useLocation: jest.fn().mockImplementation(() => ({
pathname: mockLocationPathname,
})),
Link: jest.fn().mockImplementation(({ children, to, ...res }) => (
<a href={to} {...res}>
{children}

View File

@ -18,11 +18,14 @@ import { ROLES_LIST_WITH_PAGING } from '../Roles.mock';
import RolesListPage from './RolesListPage';
const mockPush = jest.fn();
const mockLocationPathname = '/mock-path';
jest.mock('react-router-dom', () => ({
useHistory: jest.fn().mockImplementation(() => ({
push: mockPush,
})),
useLocation: jest.fn().mockImplementation(() => ({
pathname: mockLocationPathname,
})),
Link: jest.fn().mockImplementation(({ children, to, ...res }) => (
<a href={to} {...res}>
{children}

View File

@ -44,11 +44,18 @@ jest.mock('../../components/common/Loader/Loader', () => {
});
// mock library imports
const mockLocationPathname = '/mock-path';
jest.mock('react-router-dom', () => ({
Link: jest
.fn()
.mockImplementation(({ children }) => <a href="#">{children}</a>),
useParams: jest.fn().mockImplementation(() => ({ fqn: 'something' })),
useHistory: jest.fn().mockImplementation(() => ({
push: jest.fn(),
})),
useLocation: jest.fn().mockImplementation(() => ({
pathname: mockLocationPathname,
})),
}));
jest.mock('../../utils/EntityUtils', () => ({

View File

@ -73,11 +73,14 @@ jest.mock('../../hooks/useApplicationStore', () => {
.mockImplementation(() => ({ isAuthDisabled: true })),
};
});
const mockLocationPathname = '/mock-path';
jest.mock('react-router-dom', () => {
return {
useHistory: jest.fn().mockImplementation(() => ({ push: jest.fn() })),
useParams: jest.fn().mockImplementation(() => ({ fqn: 'testSuiteFQN' })),
useLocation: jest.fn().mockImplementation(() => ({
pathname: mockLocationPathname,
})),
};
});
jest.mock('../../rest/testAPI', () => {