mirror of
https://github.com/apache/superset.git
synced 2026-04-28 04:25:07 +00:00
Compare commits
8 Commits
fix-mcp-pe
...
fix-interm
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
31dcc1d954 | ||
|
|
eea7402e4e | ||
|
|
813d184ea2 | ||
|
|
13553a7205 | ||
|
|
ecce53e12f | ||
|
|
bd35faa8a0 | ||
|
|
69879986e6 | ||
|
|
41fd703a8c |
@@ -16,7 +16,13 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { FunctionComponent, useState, useEffect, useCallback } from 'react';
|
||||
import {
|
||||
FunctionComponent,
|
||||
useState,
|
||||
useEffect,
|
||||
useCallback,
|
||||
useRef,
|
||||
} from 'react';
|
||||
import { useSelector } from 'react-redux';
|
||||
import {
|
||||
styled,
|
||||
@@ -108,6 +114,14 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
|
||||
const [isSaving, setIsSaving] = useState(false);
|
||||
const [isEditing, setIsEditing] = useState<boolean>(false);
|
||||
const [modal, contextHolder] = Modal.useModal();
|
||||
const isMountedRef = useRef(true);
|
||||
|
||||
useEffect(() => {
|
||||
isMountedRef.current = true;
|
||||
return () => {
|
||||
isMountedRef.current = false;
|
||||
};
|
||||
}, []);
|
||||
const buildPayload = (datasource: Record<string, any>) => {
|
||||
const payload: Record<string, any> = {
|
||||
table_name: datasource.table_name,
|
||||
@@ -185,6 +199,9 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
|
||||
};
|
||||
const onConfirmSave = async () => {
|
||||
// Pull out extra fields into the extra object
|
||||
if (!isMountedRef.current) {
|
||||
return;
|
||||
}
|
||||
setIsSaving(true);
|
||||
try {
|
||||
await SupersetClient.put({
|
||||
@@ -195,6 +212,10 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
|
||||
const { json } = await SupersetClient.get({
|
||||
endpoint: `/api/v1/dataset/${currentDatasource?.id}`,
|
||||
});
|
||||
if (!isMountedRef.current) {
|
||||
return;
|
||||
}
|
||||
setIsSaving(false);
|
||||
addSuccessToast(t('The dataset has been saved'));
|
||||
// eslint-disable-next-line no-param-reassign
|
||||
json.result.type = 'table';
|
||||
@@ -204,8 +225,11 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
|
||||
});
|
||||
onHide();
|
||||
} catch (response) {
|
||||
setIsSaving(false);
|
||||
const error = await getClientErrorObject(response);
|
||||
if (!isMountedRef.current) {
|
||||
return;
|
||||
}
|
||||
setIsSaving(false);
|
||||
let errorResponse: SupersetError | undefined;
|
||||
let errorText: string | undefined;
|
||||
// sip-40 error response
|
||||
|
||||
@@ -622,6 +622,7 @@ const ResultTable =
|
||||
class DatasourceEditor extends PureComponent {
|
||||
constructor(props) {
|
||||
super(props);
|
||||
this.isComponentMounted = false;
|
||||
this.state = {
|
||||
datasource: {
|
||||
...props.datasource,
|
||||
@@ -760,11 +761,17 @@ class DatasourceEditor extends PureComponent {
|
||||
|
||||
try {
|
||||
const response = await this.props.formatQuery(datasource.sql);
|
||||
if (!this.isComponentMounted) {
|
||||
return;
|
||||
}
|
||||
this.onDatasourcePropChange('sql', response.json.result);
|
||||
this.props.addSuccessToast(t('SQL was formatted'));
|
||||
} catch (error) {
|
||||
const { error: clientError, statusText } =
|
||||
await getClientErrorObject(error);
|
||||
if (!this.isComponentMounted) {
|
||||
return;
|
||||
}
|
||||
this.props.addDangerToast(
|
||||
clientError ||
|
||||
statusText ||
|
||||
@@ -808,11 +815,17 @@ class DatasourceEditor extends PureComponent {
|
||||
body: JSON.stringify({ sql: datasource.sql }),
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
if (!this.isComponentMounted) {
|
||||
return;
|
||||
}
|
||||
this.onDatasourcePropChange('sql', response.json.result);
|
||||
this.props.addSuccessToast(t('SQL was formatted'));
|
||||
} catch (error) {
|
||||
const { error: clientError, statusText } =
|
||||
await getClientErrorObject(error);
|
||||
if (!this.isComponentMounted) {
|
||||
return;
|
||||
}
|
||||
this.props.addDangerToast(
|
||||
clientError ||
|
||||
statusText ||
|
||||
@@ -823,28 +836,41 @@ class DatasourceEditor extends PureComponent {
|
||||
|
||||
async syncMetadata() {
|
||||
const { datasource } = this.state;
|
||||
if (!this.isComponentMounted) {
|
||||
return;
|
||||
}
|
||||
this.setState({ metadataLoading: true });
|
||||
try {
|
||||
const newCols = await fetchSyncedColumns(datasource);
|
||||
if (!this.isComponentMounted) {
|
||||
return;
|
||||
}
|
||||
const columnChanges = updateColumns(
|
||||
datasource.columns,
|
||||
newCols,
|
||||
this.props.addSuccessToast,
|
||||
);
|
||||
if (!this.isComponentMounted) {
|
||||
return;
|
||||
}
|
||||
this.setColumns({
|
||||
databaseColumns: columnChanges.finalColumns.filter(
|
||||
col => !col.expression, // remove calculated columns
|
||||
),
|
||||
});
|
||||
this.props.addSuccessToast(t('Metadata has been synced'));
|
||||
this.setState({ metadataLoading: false });
|
||||
if (this.isComponentMounted) {
|
||||
this.props.addSuccessToast(t('Metadata has been synced'));
|
||||
this.setState({ metadataLoading: false });
|
||||
}
|
||||
} catch (error) {
|
||||
const { error: clientError, statusText } =
|
||||
await getClientErrorObject(error);
|
||||
this.props.addDangerToast(
|
||||
clientError || statusText || t('An error has occurred'),
|
||||
);
|
||||
this.setState({ metadataLoading: false });
|
||||
if (this.isComponentMounted) {
|
||||
this.props.addDangerToast(
|
||||
clientError || statusText || t('An error has occurred'),
|
||||
);
|
||||
this.setState({ metadataLoading: false });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -901,10 +927,12 @@ class DatasourceEditor extends PureComponent {
|
||||
id: ids[index],
|
||||
}));
|
||||
|
||||
this.setState({
|
||||
usageCharts: chartsWithIds,
|
||||
usageChartsCount: json?.count || 0,
|
||||
});
|
||||
if (this.isComponentMounted) {
|
||||
this.setState({
|
||||
usageCharts: chartsWithIds,
|
||||
usageChartsCount: json?.count || 0,
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
charts: chartsWithIds,
|
||||
@@ -914,15 +942,17 @@ class DatasourceEditor extends PureComponent {
|
||||
} catch (error) {
|
||||
const { error: clientError, statusText } =
|
||||
await getClientErrorObject(error);
|
||||
this.props.addDangerToast(
|
||||
clientError ||
|
||||
statusText ||
|
||||
t('An error occurred while fetching usage data'),
|
||||
);
|
||||
this.setState({
|
||||
usageCharts: [],
|
||||
usageChartsCount: 0,
|
||||
});
|
||||
if (this.isComponentMounted) {
|
||||
this.props.addDangerToast(
|
||||
clientError ||
|
||||
statusText ||
|
||||
t('An error occurred while fetching usage data'),
|
||||
);
|
||||
this.setState({
|
||||
usageCharts: [],
|
||||
usageChartsCount: 0,
|
||||
});
|
||||
}
|
||||
return {
|
||||
charts: [],
|
||||
count: 0,
|
||||
@@ -1883,6 +1913,7 @@ class DatasourceEditor extends PureComponent {
|
||||
}
|
||||
|
||||
componentDidMount() {
|
||||
this.isComponentMounted = true;
|
||||
Mousetrap.bind('ctrl+shift+f', e => {
|
||||
e.preventDefault();
|
||||
if (this.state.isEditMode) {
|
||||
@@ -1894,6 +1925,7 @@ class DatasourceEditor extends PureComponent {
|
||||
}
|
||||
|
||||
componentWillUnmount() {
|
||||
this.isComponentMounted = false;
|
||||
Mousetrap.unbind('ctrl+shift+f');
|
||||
this.props.resetQuery();
|
||||
}
|
||||
|
||||
@@ -17,14 +17,149 @@
|
||||
* under the License.
|
||||
*/
|
||||
import {
|
||||
act,
|
||||
createWrapper,
|
||||
render,
|
||||
screen,
|
||||
waitFor,
|
||||
} from 'spec/helpers/testing-library';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import fetchMock from 'fetch-mock';
|
||||
import {
|
||||
TableProps,
|
||||
OnChangeFunction,
|
||||
} from '@superset-ui/core/components/Table';
|
||||
import DatasetUsageTab from '.';
|
||||
|
||||
interface Chart {
|
||||
id?: number;
|
||||
slice_name: string;
|
||||
url: string;
|
||||
certified_by?: string;
|
||||
certification_details?: string;
|
||||
description?: string;
|
||||
owners: Array<{
|
||||
first_name: string;
|
||||
last_name: string;
|
||||
id: number;
|
||||
}>;
|
||||
changed_on_delta_humanized: string;
|
||||
changed_on?: string;
|
||||
changed_by: {
|
||||
first_name: string;
|
||||
last_name: string;
|
||||
id: number;
|
||||
} | null;
|
||||
dashboards: Array<{
|
||||
id: number;
|
||||
dashboard_title: string;
|
||||
url: string;
|
||||
}>;
|
||||
}
|
||||
|
||||
jest.mock('@superset-ui/core/components/Table', () => {
|
||||
const actual = jest.requireActual('@superset-ui/core/components/Table');
|
||||
const ActualTable = actual.default;
|
||||
|
||||
let latestPaginationHandler:
|
||||
| ((page: number, pageSize?: number) => void)
|
||||
| null = null;
|
||||
let latestPaginationPageSize: number | undefined;
|
||||
let latestPaginationCurrent: number | undefined;
|
||||
let latestPaginationTotal: number | undefined;
|
||||
let latestOnChangeHandler: OnChangeFunction<Chart> | null = null;
|
||||
|
||||
const MockTable = <RecordType extends object>({
|
||||
pagination,
|
||||
onChange,
|
||||
recordCount,
|
||||
defaultPageSize = 15,
|
||||
usePagination = true,
|
||||
...rest
|
||||
}: TableProps<RecordType>) => {
|
||||
// Simulate the Table wrapper's pagination merging logic
|
||||
let mergedPagination = pagination;
|
||||
|
||||
if (usePagination) {
|
||||
// Build default pagination settings like the wrapper does
|
||||
const paginationDefaults: {
|
||||
hideOnSinglePage: boolean;
|
||||
pageSize: number;
|
||||
total?: number;
|
||||
} = {
|
||||
hideOnSinglePage: true,
|
||||
pageSize: defaultPageSize,
|
||||
};
|
||||
|
||||
// Add total from recordCount if provided
|
||||
if (recordCount) {
|
||||
paginationDefaults.total = recordCount;
|
||||
}
|
||||
|
||||
// Merge with pagination overrides
|
||||
if (pagination && typeof pagination !== 'boolean') {
|
||||
mergedPagination = { ...paginationDefaults, ...pagination };
|
||||
} else if (!pagination) {
|
||||
mergedPagination = paginationDefaults;
|
||||
}
|
||||
}
|
||||
|
||||
// Capture the merged pagination for assertions
|
||||
if (mergedPagination && typeof mergedPagination !== 'boolean') {
|
||||
latestPaginationPageSize = mergedPagination.pageSize;
|
||||
latestPaginationCurrent = mergedPagination.current;
|
||||
latestPaginationTotal = mergedPagination.total;
|
||||
latestPaginationHandler = mergedPagination.onChange || null;
|
||||
}
|
||||
|
||||
// Cast to Chart type since we know DatasetUsageTab uses Chart
|
||||
latestOnChangeHandler =
|
||||
(onChange as OnChangeFunction<Chart> | undefined) || null;
|
||||
|
||||
return (
|
||||
<ActualTable
|
||||
pagination={mergedPagination}
|
||||
onChange={onChange}
|
||||
recordCount={recordCount}
|
||||
defaultPageSize={defaultPageSize}
|
||||
usePagination={usePagination}
|
||||
{...rest}
|
||||
/>
|
||||
);
|
||||
};
|
||||
|
||||
return {
|
||||
__esModule: true,
|
||||
...actual,
|
||||
default: MockTable,
|
||||
__getLatestPaginationHandler: () => latestPaginationHandler,
|
||||
__getLatestPaginationPageSize: () => latestPaginationPageSize,
|
||||
__getLatestPaginationCurrent: () => latestPaginationCurrent,
|
||||
__getLatestPaginationTotal: () => latestPaginationTotal,
|
||||
__getLatestOnChangeHandler: () => latestOnChangeHandler,
|
||||
__resetPaginationHandler: () => {
|
||||
latestPaginationHandler = null;
|
||||
latestPaginationPageSize = undefined;
|
||||
latestPaginationCurrent = undefined;
|
||||
latestPaginationTotal = undefined;
|
||||
latestOnChangeHandler = null;
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
const tableMock = jest.requireMock('@superset-ui/core/components/Table') as {
|
||||
__getLatestPaginationHandler: () =>
|
||||
| ((page: number, pageSize?: number) => void)
|
||||
| null;
|
||||
__getLatestPaginationPageSize: () => number | undefined;
|
||||
__getLatestPaginationCurrent: () => number | undefined;
|
||||
__getLatestPaginationTotal: () => number | undefined;
|
||||
__resetPaginationHandler: () => void;
|
||||
__getLatestOnChangeHandler: () =>
|
||||
| ((pagination: { current: number; pageSize?: number }) => void)
|
||||
| null;
|
||||
};
|
||||
|
||||
const mockChartsResponse = {
|
||||
result: [
|
||||
{
|
||||
@@ -83,6 +218,9 @@ const mockChartsResponse = {
|
||||
ids: [1, 2],
|
||||
};
|
||||
|
||||
// Store original scrollTo to restore after tests
|
||||
let originalScrollTo: typeof Element.prototype.scrollTo;
|
||||
|
||||
const setupTest = (props = {}) => {
|
||||
const mockOnFetchCharts = jest.fn(() =>
|
||||
Promise.resolve({
|
||||
@@ -101,21 +239,48 @@ const setupTest = (props = {}) => {
|
||||
...props,
|
||||
};
|
||||
|
||||
return render(<DatasetUsageTab {...defaultProps} />, {
|
||||
const result = render(<DatasetUsageTab {...defaultProps} />, {
|
||||
wrapper: createWrapper({
|
||||
useRedux: true,
|
||||
useRouter: true,
|
||||
}),
|
||||
});
|
||||
|
||||
return { ...result, mockOnFetchCharts };
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
fetchMock.reset();
|
||||
jest.clearAllMocks();
|
||||
|
||||
// Save original scrollTo and mock it for JSDOM compatibility
|
||||
originalScrollTo = Element.prototype.scrollTo;
|
||||
Object.defineProperty(Element.prototype, 'scrollTo', {
|
||||
value: jest.fn(),
|
||||
configurable: true,
|
||||
writable: true,
|
||||
});
|
||||
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
tableMock.__resetPaginationHandler();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fetchMock.restore();
|
||||
|
||||
// Restore real timers in case a test with fakeTimers failed mid-test
|
||||
jest.useRealTimers();
|
||||
|
||||
// Restore original scrollTo to avoid test pollution
|
||||
if (originalScrollTo) {
|
||||
Object.defineProperty(Element.prototype, 'scrollTo', {
|
||||
value: originalScrollTo,
|
||||
configurable: true,
|
||||
writable: true,
|
||||
});
|
||||
} else {
|
||||
delete (Element.prototype as any).scrollTo;
|
||||
}
|
||||
});
|
||||
|
||||
test('renders empty state when no charts provided', () => {
|
||||
@@ -212,3 +377,363 @@ test('enables sorting for Chart and Last modified columns', async () => {
|
||||
expect(dashboardHeader).not.toHaveClass('ant-table-column-has-sorters');
|
||||
});
|
||||
});
|
||||
|
||||
test('clears scroll timeout on unmount to prevent async updates', async () => {
|
||||
jest.useFakeTimers();
|
||||
|
||||
const scrollToMock = Element.prototype.scrollTo as jest.Mock;
|
||||
scrollToMock.mockClear();
|
||||
|
||||
const manyCharts = Array.from({ length: 51 }, (_, i) => ({
|
||||
id: i + 1,
|
||||
slice_name: `Test Chart ${i + 1}`,
|
||||
url: `/explore/${i + 1}/`,
|
||||
owners: [],
|
||||
changed_on_delta_humanized: '1 day ago',
|
||||
changed_by: null,
|
||||
dashboards: [],
|
||||
}));
|
||||
|
||||
const { unmount, mockOnFetchCharts } = setupTest({
|
||||
charts: manyCharts.slice(0, 25),
|
||||
totalCount: 51,
|
||||
});
|
||||
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
const onChangeHandler = tableMock.__getLatestOnChangeHandler();
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
const pageSize = tableMock.__getLatestPaginationPageSize();
|
||||
expect(onChangeHandler).toBeDefined();
|
||||
|
||||
onChangeHandler?.({ current: 2, pageSize } as {
|
||||
current: number;
|
||||
pageSize?: number;
|
||||
});
|
||||
|
||||
await waitFor(() =>
|
||||
expect(mockOnFetchCharts).toHaveBeenCalledWith(
|
||||
2,
|
||||
25,
|
||||
expect.any(String),
|
||||
expect.any(String),
|
||||
),
|
||||
);
|
||||
|
||||
unmount();
|
||||
|
||||
act(() => {
|
||||
jest.runAllTimers();
|
||||
});
|
||||
|
||||
expect(scrollToMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('clears pending scroll timeout on rapid pagination clicks', async () => {
|
||||
jest.useFakeTimers();
|
||||
|
||||
const scrollToMock = Element.prototype.scrollTo as jest.Mock;
|
||||
scrollToMock.mockClear();
|
||||
|
||||
const manyCharts = Array.from({ length: 60 }, (_, i) => ({
|
||||
id: i + 1,
|
||||
slice_name: `Test Chart ${i + 1}`,
|
||||
url: `/explore/${i + 1}/`,
|
||||
owners: [],
|
||||
changed_on_delta_humanized: '1 day ago',
|
||||
changed_by: null,
|
||||
dashboards: [],
|
||||
}));
|
||||
|
||||
const { mockOnFetchCharts } = setupTest({
|
||||
charts: manyCharts.slice(0, 25),
|
||||
totalCount: 60,
|
||||
});
|
||||
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
const firstOnChange = tableMock.__getLatestOnChangeHandler();
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
const pageSize = tableMock.__getLatestPaginationPageSize();
|
||||
expect(firstOnChange).toBeDefined();
|
||||
|
||||
firstOnChange?.({ current: 2, pageSize } as {
|
||||
current: number;
|
||||
pageSize?: number;
|
||||
});
|
||||
await waitFor(() =>
|
||||
expect(mockOnFetchCharts).toHaveBeenCalledWith(
|
||||
2,
|
||||
25,
|
||||
expect.any(String),
|
||||
expect.any(String),
|
||||
),
|
||||
);
|
||||
|
||||
act(() => {
|
||||
jest.advanceTimersByTime(50);
|
||||
});
|
||||
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
const secondOnChange = tableMock.__getLatestOnChangeHandler();
|
||||
expect(secondOnChange).toBeDefined();
|
||||
|
||||
secondOnChange?.({ current: 3, pageSize } as {
|
||||
current: number;
|
||||
pageSize?: number;
|
||||
});
|
||||
await waitFor(() =>
|
||||
expect(mockOnFetchCharts).toHaveBeenCalledWith(
|
||||
3,
|
||||
25,
|
||||
expect.any(String),
|
||||
expect.any(String),
|
||||
),
|
||||
);
|
||||
|
||||
act(() => {
|
||||
jest.runAllTimers();
|
||||
});
|
||||
|
||||
expect(scrollToMock).toHaveBeenCalledTimes(1);
|
||||
expect(scrollToMock).toHaveBeenCalledWith({ top: 0, behavior: 'smooth' });
|
||||
});
|
||||
|
||||
test('scrolls table to top after pagination with delay', async () => {
|
||||
jest.useFakeTimers();
|
||||
|
||||
const scrollToMock = Element.prototype.scrollTo as jest.Mock;
|
||||
scrollToMock.mockClear();
|
||||
|
||||
const manyCharts = Array.from({ length: 30 }, (_, i) => ({
|
||||
id: i + 1,
|
||||
slice_name: `Test Chart ${i + 1}`,
|
||||
url: `/explore/${i + 1}/`,
|
||||
owners: [],
|
||||
changed_on_delta_humanized: '1 day ago',
|
||||
changed_by: null,
|
||||
dashboards: [],
|
||||
}));
|
||||
|
||||
const { mockOnFetchCharts } = setupTest({
|
||||
charts: manyCharts.slice(0, 25),
|
||||
totalCount: 30,
|
||||
});
|
||||
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
const onChangeHandler = tableMock.__getLatestOnChangeHandler();
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
const pageSize = tableMock.__getLatestPaginationPageSize();
|
||||
expect(onChangeHandler).toBeDefined();
|
||||
|
||||
onChangeHandler?.({ current: 2, pageSize } as {
|
||||
current: number;
|
||||
pageSize?: number;
|
||||
});
|
||||
|
||||
await waitFor(() =>
|
||||
expect(mockOnFetchCharts).toHaveBeenCalledWith(
|
||||
2,
|
||||
25,
|
||||
expect.any(String),
|
||||
expect.any(String),
|
||||
),
|
||||
);
|
||||
|
||||
expect(scrollToMock).not.toHaveBeenCalled();
|
||||
|
||||
act(() => {
|
||||
jest.advanceTimersByTime(100);
|
||||
});
|
||||
|
||||
expect(scrollToMock).toHaveBeenCalledWith({
|
||||
top: 0,
|
||||
behavior: 'smooth',
|
||||
});
|
||||
});
|
||||
|
||||
test('displays correct pagination and navigates between pages', async () => {
|
||||
const manyCharts = Array.from({ length: 30 }, (_, i) => ({
|
||||
id: i + 1,
|
||||
slice_name: `Test Chart ${i + 1}`,
|
||||
url: `/explore/${i + 1}/`,
|
||||
owners: [],
|
||||
changed_on_delta_humanized: '1 day ago',
|
||||
changed_by: null,
|
||||
dashboards: [],
|
||||
}));
|
||||
|
||||
const { mockOnFetchCharts } = setupTest({
|
||||
charts: manyCharts.slice(0, 25),
|
||||
totalCount: 30,
|
||||
});
|
||||
|
||||
// Page 1: Verify UI shows page 1 is active (using Ant Design active class)
|
||||
await waitFor(() => {
|
||||
const page1Item = screen.getByRole('listitem', { name: '1' });
|
||||
expect(page1Item).toHaveClass('ant-pagination-item-active');
|
||||
});
|
||||
|
||||
// Soft assertion: verify component passes correct props to Table
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationCurrent()).toBe(1);
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationTotal()).toBe(30);
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationPageSize()).toBe(25);
|
||||
|
||||
// User clicks page 2 (using userEvent for realistic interaction)
|
||||
const page2Item = screen.getByRole('listitem', { name: '2' });
|
||||
await userEvent.click(page2Item);
|
||||
|
||||
// Data fetch called for page 2
|
||||
await waitFor(() =>
|
||||
expect(mockOnFetchCharts).toHaveBeenCalledWith(
|
||||
2,
|
||||
25,
|
||||
expect.any(String),
|
||||
expect.any(String),
|
||||
),
|
||||
);
|
||||
|
||||
// Verify UI reflects page 2 is now active (using Ant Design active class)
|
||||
await waitFor(() => {
|
||||
const page2Item = screen.getByRole('listitem', { name: '2' });
|
||||
expect(page2Item).toHaveClass('ant-pagination-item-active');
|
||||
});
|
||||
|
||||
// Soft assertion: Table still receives correct total (catches the bug!)
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationCurrent()).toBe(2);
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationTotal()).toBe(30);
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationPageSize()).toBe(25);
|
||||
});
|
||||
|
||||
test('maintains pagination state across multiple page changes', async () => {
|
||||
const manyCharts = Array.from({ length: 60 }, (_, i) => ({
|
||||
id: i + 1,
|
||||
slice_name: `Test Chart ${i + 1}`,
|
||||
url: `/explore/${i + 1}/`,
|
||||
owners: [],
|
||||
changed_on_delta_humanized: '1 day ago',
|
||||
changed_by: null,
|
||||
dashboards: [],
|
||||
}));
|
||||
|
||||
const { mockOnFetchCharts } = setupTest({
|
||||
charts: manyCharts.slice(0, 25),
|
||||
totalCount: 60,
|
||||
});
|
||||
|
||||
// Verify initial state (page 1) using Ant Design active class
|
||||
await waitFor(() => {
|
||||
const page1Item = screen.getByRole('listitem', { name: '1' });
|
||||
expect(page1Item).toHaveClass('ant-pagination-item-active');
|
||||
});
|
||||
|
||||
// Soft assertion: initial props are correct
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationTotal()).toBe(60);
|
||||
|
||||
// User navigates to page 2
|
||||
await userEvent.click(screen.getByRole('listitem', { name: '2' }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockOnFetchCharts).toHaveBeenCalledWith(
|
||||
2,
|
||||
25,
|
||||
expect.any(String),
|
||||
expect.any(String),
|
||||
);
|
||||
const page2Item = screen.getByRole('listitem', { name: '2' });
|
||||
expect(page2Item).toHaveClass('ant-pagination-item-active');
|
||||
});
|
||||
|
||||
// Soft assertion: props update correctly after navigation
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationCurrent()).toBe(2);
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationTotal()).toBe(60);
|
||||
|
||||
// User navigates to page 3
|
||||
await userEvent.click(screen.getByRole('listitem', { name: '3' }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockOnFetchCharts).toHaveBeenCalledWith(
|
||||
3,
|
||||
25,
|
||||
expect.any(String),
|
||||
expect.any(String),
|
||||
);
|
||||
const page3Item = screen.getByRole('listitem', { name: '3' });
|
||||
expect(page3Item).toHaveClass('ant-pagination-item-active');
|
||||
});
|
||||
|
||||
// Soft assertion: total remains consistent across all navigations
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationCurrent()).toBe(3);
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationTotal()).toBe(60);
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationPageSize()).toBe(25);
|
||||
});
|
||||
|
||||
test('handles total count below one page (no pagination needed)', async () => {
|
||||
const fewCharts = Array.from({ length: 5 }, (_, i) => ({
|
||||
id: i + 1,
|
||||
slice_name: `Test Chart ${i + 1}`,
|
||||
url: `/explore/${i + 1}/`,
|
||||
owners: [],
|
||||
changed_on_delta_humanized: '1 day ago',
|
||||
changed_by: null,
|
||||
dashboards: [],
|
||||
}));
|
||||
|
||||
setupTest({
|
||||
charts: fewCharts,
|
||||
totalCount: 5,
|
||||
});
|
||||
|
||||
// Pagination should not show page 2 item since only 1 page exists
|
||||
await waitFor(() => {
|
||||
expect(
|
||||
screen.queryByRole('listitem', { name: '2' }),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
// Table receives correct props
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationTotal()).toBe(5);
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationCurrent()).toBe(1);
|
||||
});
|
||||
|
||||
test('handles server returning fewer rows than page size', async () => {
|
||||
const partialPage = Array.from({ length: 12 }, (_, i) => ({
|
||||
id: i + 1,
|
||||
slice_name: `Test Chart ${i + 1}`,
|
||||
url: `/explore/${i + 1}/`,
|
||||
owners: [],
|
||||
changed_on_delta_humanized: '1 day ago',
|
||||
changed_by: null,
|
||||
dashboards: [],
|
||||
}));
|
||||
|
||||
setupTest({
|
||||
charts: partialPage,
|
||||
totalCount: 12,
|
||||
});
|
||||
|
||||
// Only page 1 should exist
|
||||
await waitFor(() => {
|
||||
expect(
|
||||
screen.queryByRole('listitem', { name: '2' }),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
// Table receives correct total (not data.length!)
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationTotal()).toBe(12);
|
||||
// eslint-disable-next-line no-underscore-dangle
|
||||
expect(tableMock.__getLatestPaginationPageSize()).toBe(25);
|
||||
});
|
||||
|
||||
@@ -99,6 +99,8 @@ const DatasetUsageTab = ({
|
||||
}: DatasetUsageTabProps) => {
|
||||
const addDangerToastRef = useRef(addDangerToast);
|
||||
const tableContainerRef = useRef<HTMLDivElement>(null);
|
||||
const scrollTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
const isMountedRef = useRef(true);
|
||||
|
||||
const [loading, setLoading] = useState(false);
|
||||
const [currentPage, setCurrentPage] = useState(1);
|
||||
@@ -115,14 +117,19 @@ const DatasetUsageTab = ({
|
||||
|
||||
try {
|
||||
await onFetchCharts(page, PAGE_SIZE, column, direction);
|
||||
setCurrentPage(page);
|
||||
setSortColumn(column);
|
||||
setSortDirection(direction);
|
||||
// Only update state if component is still mounted
|
||||
if (isMountedRef.current) {
|
||||
setCurrentPage(page);
|
||||
setSortColumn(column);
|
||||
setSortDirection(direction);
|
||||
}
|
||||
} catch (error) {
|
||||
if (addDangerToastRef.current)
|
||||
if (isMountedRef.current && addDangerToastRef.current)
|
||||
addDangerToastRef.current(t('Error fetching charts'));
|
||||
} finally {
|
||||
setLoading(false);
|
||||
if (isMountedRef.current) {
|
||||
setLoading(false);
|
||||
}
|
||||
}
|
||||
},
|
||||
[datasourceId, onFetchCharts, sortColumn, sortDirection],
|
||||
@@ -132,11 +139,31 @@ const DatasetUsageTab = ({
|
||||
addDangerToastRef.current = addDangerToast;
|
||||
}, [addDangerToast]);
|
||||
|
||||
// Cleanup on unmount
|
||||
useEffect(() => {
|
||||
// Set mounted flag to true on mount (important for Strict Mode double-mount)
|
||||
isMountedRef.current = true;
|
||||
|
||||
return () => {
|
||||
// Mark component as unmounted to prevent state updates
|
||||
isMountedRef.current = false;
|
||||
// Clear pending scroll timeout
|
||||
if (scrollTimeoutRef.current) {
|
||||
clearTimeout(scrollTimeoutRef.current);
|
||||
}
|
||||
};
|
||||
}, []);
|
||||
|
||||
const handlePageChange = useCallback(
|
||||
(page: number) => {
|
||||
handleFetchCharts(page);
|
||||
|
||||
setTimeout(() => {
|
||||
// Clear any pending scroll timeout
|
||||
if (scrollTimeoutRef.current) {
|
||||
clearTimeout(scrollTimeoutRef.current);
|
||||
}
|
||||
|
||||
scrollTimeoutRef.current = setTimeout(() => {
|
||||
const tableBody =
|
||||
tableContainerRef.current?.querySelector('.ant-table-body');
|
||||
if (tableBody) {
|
||||
@@ -145,6 +172,7 @@ const DatasetUsageTab = ({
|
||||
behavior: 'smooth',
|
||||
});
|
||||
}
|
||||
scrollTimeoutRef.current = null;
|
||||
}, 100);
|
||||
},
|
||||
[handleFetchCharts],
|
||||
@@ -261,14 +289,9 @@ const DatasetUsageTab = ({
|
||||
sticky
|
||||
columns={columns}
|
||||
data={charts}
|
||||
pagination={{
|
||||
current: currentPage,
|
||||
total: totalCount,
|
||||
pageSize: PAGE_SIZE,
|
||||
onChange: handlePageChange,
|
||||
showSizeChanger: false,
|
||||
size: 'default',
|
||||
}}
|
||||
recordCount={totalCount}
|
||||
usePagination
|
||||
defaultPageSize={PAGE_SIZE}
|
||||
loading={loading}
|
||||
size={TableSize.Middle}
|
||||
rowKey={(record: Chart) =>
|
||||
@@ -276,6 +299,12 @@ const DatasetUsageTab = ({
|
||||
}
|
||||
tableLayout="fixed"
|
||||
scroll={{ y: 293, x: '100%' }}
|
||||
pagination={{
|
||||
current: currentPage,
|
||||
total: totalCount,
|
||||
pageSize: PAGE_SIZE,
|
||||
hideOnSinglePage: false,
|
||||
}}
|
||||
css={css`
|
||||
.ant-table-pagination.ant-pagination {
|
||||
margin-bottom: 0;
|
||||
|
||||
Reference in New Issue
Block a user