fix(explore): replace TableView with virtualized GridTable, add row limit controls, restore sample filters (#39212)

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Maxime Beauchemin
2026-04-13 08:19:49 -07:00
committed by GitHub
parent de40b58e10
commit fa1f12a0b5
16 changed files with 371 additions and 252 deletions

View File

@@ -29,9 +29,12 @@ import {
} from 'spec/helpers/testing-library';
import chartQueries, { sliceId } from 'spec/fixtures/mockChartQueries';
import mockState from 'spec/fixtures/mockState';
import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact';
import { DashboardPageIdContext } from 'src/dashboard/containers/DashboardPage';
import DrillByModal, { DrillByModalProps } from './DrillByModal';
setupAGGridModules();
// Mock the isEmbedded function
jest.mock('src/dashboard/util/isEmbedded', () => ({
isEmbedded: jest.fn(() => false),
@@ -406,16 +409,9 @@ describe('Table view with pagination', () => {
await waitFor(() => {
expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument();
});
// Check that pagination is rendered (there's also a breadcrumb list)
const lists = screen.getAllByRole('list');
const paginationList = lists.find(list =>
list.className?.includes('pagination'),
);
expect(paginationList).toBeInTheDocument();
});
test('should handle pagination in table view', async () => {
test('should render data in table view', async () => {
await renderModal({
column: { column_name: 'state', verbose_name: null },
drillByConfig: {
@@ -432,19 +428,9 @@ describe('Table view with pagination', () => {
expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument();
});
// Check that first page data is shown
expect(screen.getByText('State0')).toBeInTheDocument();
// Check pagination controls exist
const nextPageButton = screen.getByTitle('Next Page');
expect(nextPageButton).toBeInTheDocument();
// Click next page
userEvent.click(nextPageButton);
// Verify page changed (State0 should not be visible on page 2)
// Check that data is rendered in the grid
await waitFor(() => {
expect(screen.queryByText('State0')).not.toBeInTheDocument();
expect(screen.getByText('State0')).toBeInTheDocument();
});
});
@@ -542,11 +528,12 @@ describe('Table view with pagination', () => {
expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument();
});
// Should show empty state
expect(screen.getByText('No data')).toBeInTheDocument();
// ag-grid shows its own empty overlay when there are no rows
const tableContainer = screen.getByTestId('drill-by-results-table');
expect(tableContainer).toBeInTheDocument();
});
test('should handle sorting in table view', async () => {
test('should render grid in table view', async () => {
await renderModal({
column: { column_name: 'state', verbose_name: null },
drillByConfig: {
@@ -563,16 +550,7 @@ describe('Table view with pagination', () => {
expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument();
});
// Find sortable column header
const sortableHeaders = screen.getAllByTestId('sort-header');
expect(sortableHeaders.length).toBeGreaterThan(0);
// Click to sort
userEvent.click(sortableHeaders[0]);
// Table should still be rendered without crashes
await waitFor(() => {
expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument();
});
expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument();
});
});

View File

@@ -25,25 +25,12 @@ import {
within,
waitFor,
} from 'spec/helpers/testing-library';
import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact';
import { useResultsTableView } from './useResultsTableView';
const capturedProps: any[] = [];
jest.mock(
'src/explore/components/DataTablesPane/components/SingleQueryResultPane',
() => {
const actual = jest.requireActual(
'src/explore/components/DataTablesPane/components/SingleQueryResultPane',
);
return {
...actual,
SingleQueryResultPane: (props: any) => {
capturedProps.push(props);
return actual.SingleQueryResultPane(props);
},
};
},
);
beforeAll(() => {
setupAGGridModules();
});
const MOCK_CHART_DATA_RESULT = [
{
@@ -92,9 +79,9 @@ test('Displays results table for 1 query', () => {
);
render(result.current, { useRedux: true });
expect(screen.queryByRole('tablist')).not.toBeInTheDocument();
expect(screen.getByRole('table')).toBeInTheDocument();
expect(screen.getAllByTestId('sort-header')).toHaveLength(2);
expect(screen.getAllByTestId('table-row')).toHaveLength(4);
expect(screen.getByText('name')).toBeInTheDocument();
expect(screen.getByText('sum__num')).toBeInTheDocument();
expect(screen.getByText('Michael')).toBeInTheDocument();
});
test('Displays results for 2 queries', async () => {
@@ -102,60 +89,18 @@ test('Displays results for 2 queries', async () => {
useResultsTableView(MOCK_CHART_DATA_RESULT, '1__table', true),
);
render(result.current, { useRedux: true });
const getActiveTabElement = () =>
document.querySelector('.ant-tabs-tabpane-active') as HTMLElement;
const tablistElement = screen.getByRole('tablist');
expect(tablistElement).toBeInTheDocument();
expect(within(tablistElement).getByText('Results 1')).toBeInTheDocument();
expect(within(tablistElement).getByText('Results 2')).toBeInTheDocument();
expect(within(getActiveTabElement()).getByRole('table')).toBeInTheDocument();
expect(
within(getActiveTabElement()).getAllByTestId('sort-header'),
).toHaveLength(2);
expect(
within(getActiveTabElement()).getAllByTestId('table-row'),
).toHaveLength(4);
expect(screen.getByText('Michael')).toBeInTheDocument();
userEvent.click(screen.getByText('Results 2'));
await waitFor(() => {
expect(
within(getActiveTabElement()).getAllByTestId('sort-header'),
).toHaveLength(3);
});
expect(
within(getActiveTabElement()).getAllByTestId('table-row'),
).toHaveLength(2);
});
test('passes isPaginationSticky={false} to SingleQueryResultPane for single query', () => {
capturedProps.length = 0;
const { result } = renderHook(() =>
useResultsTableView(MOCK_CHART_DATA_RESULT.slice(0, 1), '1__table', true),
);
render(result.current, { useRedux: true });
expect(capturedProps.length).toBeGreaterThan(0);
capturedProps.forEach(props => {
expect(props).toMatchObject({
isPaginationSticky: false,
});
});
});
test('passes isPaginationSticky={false} to SingleQueryResultPane for multiple queries', () => {
capturedProps.length = 0;
const { result } = renderHook(() =>
useResultsTableView(MOCK_CHART_DATA_RESULT, '1__table', true),
);
render(result.current, { useRedux: true });
expect(capturedProps.length).toBeGreaterThanOrEqual(2);
capturedProps.forEach(props => {
expect(props).toMatchObject({
isPaginationSticky: false,
});
expect(screen.getByText('gender')).toBeInTheDocument();
});
expect(screen.getByText('boy')).toBeInTheDocument();
});

View File

@@ -22,13 +22,12 @@ import { t } from '@apache-superset/core/translation';
import { SingleQueryResultPane } from 'src/explore/components/DataTablesPane/components/SingleQueryResultPane';
import Tabs from '@superset-ui/core/components/Tabs';
const DATA_SIZE = 15;
const PaginationContainer = styled.div`
${({ theme }) => css`
& .pagination-container {
bottom: ${-theme.sizeUnit * 4}px;
}
const ResultContainer = styled.div`
${() => css`
display: flex;
flex-direction: column;
flex: 1;
min-height: 0;
`}
`;
@@ -42,19 +41,17 @@ export const useResultsTableView = (
}
if (chartDataResult.length === 1) {
return (
<PaginationContainer data-test="drill-by-results-table">
<ResultContainer data-test="drill-by-results-table">
<SingleQueryResultPane
colnames={chartDataResult[0].colnames}
coltypes={chartDataResult[0].coltypes}
rowcount={chartDataResult[0].sql_rowcount}
data={chartDataResult[0].data}
dataSize={DATA_SIZE}
datasourceId={datasourceId}
isVisible
canDownload={canDownload}
isPaginationSticky={false}
/>
</PaginationContainer>
</ResultContainer>
);
}
return (
@@ -64,19 +61,17 @@ export const useResultsTableView = (
key: `result-tab-${index}`,
label: t('Results %s', index + 1),
children: (
<PaginationContainer>
<ResultContainer>
<SingleQueryResultPane
colnames={res.colnames}
coltypes={res.coltypes}
data={res.data}
rowcount={res.sql_rowcount}
dataSize={DATA_SIZE}
datasourceId={datasourceId}
isVisible
canDownload={canDownload}
isPaginationSticky={false}
/>
</PaginationContainer>
</ResultContainer>
),
}))}
/>

View File

@@ -206,6 +206,7 @@ export const DataTablesPane = ({
<StyledDiv>
<SamplesPane
datasource={datasource}
queryFormData={queryFormData}
queryForce={queryForce}
isRequest={isRequest.samples}
setForceQuery={setForceQuery}

View File

@@ -20,6 +20,7 @@ import { styled, css } from '@apache-superset/core/theme';
import { GenericDataType } from '@apache-superset/core/common';
import { useMemo } from 'react';
import { zip } from 'lodash';
import { Select } from 'antd';
import {
CopyToClipboardButton,
FilterInput,
@@ -29,10 +30,19 @@ import { getTimeColumns } from 'src/explore/components/DataTableControl/utils';
import RowCountLabel from 'src/components/RowCountLabel';
import { TableControlsProps } from '../types';
export const ROW_LIMIT_OPTIONS = [
{ value: 100, label: '100 rows' },
{ value: 500, label: '500 rows' },
{ value: 1000, label: '1k rows' },
{ value: 5000, label: '5k rows' },
{ value: 10000, label: '10k rows' },
];
export const TableControlsWrapper = styled.div`
${({ theme }) => `
display: flex;
align-items: center;
padding-top: ${theme.sizeUnit * 2}px;
padding-bottom: ${theme.sizeUnit * 2}px;
justify-content: space-between;
@@ -51,6 +61,9 @@ export const TableControls = ({
rowcount,
isLoading,
canDownload,
rowLimit,
rowLimitOptions,
onRowLimitChange,
}: TableControlsProps) => {
const originalTimeColumns = getTimeColumns(datasourceId);
const formattedTimeColumns = zip<string, GenericDataType>(
@@ -76,9 +89,23 @@ export const TableControls = ({
css={css`
display: flex;
align-items: center;
gap: 8px;
`}
>
<RowCountLabel rowcount={rowcount} loading={isLoading} />
{onRowLimitChange && (
<Select
value={rowLimit}
onChange={onRowLimitChange}
options={rowLimitOptions}
size="small"
css={css`
min-width: 110px;
`}
/>
)}
{(!onRowLimitChange || rowcount < (rowLimit ?? Infinity)) && (
<RowCountLabel rowcount={rowcount} loading={isLoading} />
)}
{canDownload && (
<CopyToClipboardButton data={formattedData} columns={columnNames} />
)}

View File

@@ -20,64 +20,96 @@ import { useState, useEffect, useMemo, useCallback } from 'react';
import { t } from '@apache-superset/core/translation';
import { ensureIsArray } from '@superset-ui/core';
import { styled } from '@apache-superset/core/theme';
import {
TableView,
TableSize,
EmptyState,
Loading,
EmptyWrapperType,
} from '@superset-ui/core/components';
import { EmptyState, Loading } from '@superset-ui/core/components';
import { GenericDataType } from '@apache-superset/core/common';
import {
useFilteredTableData,
useTableColumns,
} from 'src/explore/components/DataTableControl';
import { GridTable } from 'src/components/GridTable';
import { GridSize } from 'src/components/GridTable/constants';
import { getDatasourceSamples } from 'src/components/Chart/chartAction';
import { TableControls } from './DataTableControls';
import { getDrillPayload } from 'src/components/Chart/DrillDetail/utils';
import {
useGridColumns,
useKeywordFilter,
useGridHeight,
} from './useGridResultTable';
import { TableControls, ROW_LIMIT_OPTIONS } from './DataTableControls';
import { SamplesPaneProps } from '../types';
const Error = styled.pre`
margin-top: ${({ theme }) => `${theme.sizeUnit * 4}px`};
`;
const cache = new WeakSet();
const GridContainer = styled.div`
flex: 1;
min-height: 0;
position: relative;
`;
const GridSizer = styled.div`
position: absolute;
inset: 0;
`;
const cache = new WeakMap();
const DEFAULT_ROW_LIMIT = 100;
export const SamplesPane = ({
isRequest,
datasource,
queryFormData,
queryForce,
setForceQuery,
dataSize = 50,
isVisible,
canDownload,
}: SamplesPaneProps) => {
const [filterText, setFilterText] = useState('');
const [rowLimit, setRowLimit] = useState(DEFAULT_ROW_LIMIT);
const [data, setData] = useState<Record<string, any>[][]>([]);
const [colnames, setColnames] = useState<string[]>([]);
const [coltypes, setColtypes] = useState<GenericDataType[]>([]);
const [isLoading, setIsLoading] = useState<boolean>(false);
const [rowcount, setRowCount] = useState<number>(0);
const [responseError, setResponseError] = useState<string>('');
const { gridHeight, measuredRef } = useGridHeight();
const datasourceId = useMemo(
() => `${datasource.id}__${datasource.type}`,
[datasource],
);
const handleRowLimitChange = useCallback(
(limit: number) => {
setRowLimit(limit);
cache.delete(queryFormData);
},
[queryFormData],
);
useEffect(() => {
if (isRequest && queryForce) {
cache.delete(datasource);
cache.delete(queryFormData);
}
if (isRequest && !cache.has(datasource)) {
if (isRequest && !cache.has(queryFormData)) {
setIsLoading(true);
getDatasourceSamples(datasource.type, datasource.id, queryForce, {})
const payload =
getDrillPayload(
queryFormData as Parameters<typeof getDrillPayload>[0],
) ?? {};
getDatasourceSamples(
datasource.type,
datasource.id,
queryForce,
payload,
rowLimit,
1,
)
.then(response => {
setData(ensureIsArray(response.data));
setColnames(ensureIsArray(response.colnames));
setColtypes(ensureIsArray(response.coltypes));
setRowCount(response.rowcount);
setResponseError('');
cache.add(datasource);
cache.set(queryFormData, true);
if (queryForce) {
setForceQuery?.(false);
}
@@ -92,20 +124,10 @@ export const SamplesPane = ({
setIsLoading(false);
});
}
}, [datasource, isRequest, queryForce]);
}, [datasource, queryFormData, isRequest, queryForce, rowLimit]);
// this is to preserve the order of the columns, even if there are integer values,
// while also only grabbing the first column's keys
const columns = useTableColumns(
colnames,
coltypes,
data,
datasourceId,
isVisible,
{}, // moreConfig
true, // allowHTML
);
const filteredData = useFilteredTableData(filterText, data);
const columns = useGridColumns(colnames, coltypes, data);
const keywordFilter = useKeywordFilter(filterText);
const handleInputChange = useCallback(
(input: string) => setFilterText(input),
@@ -120,7 +142,7 @@ export const SamplesPane = ({
return (
<>
<TableControls
data={filteredData}
data={data}
columnNames={colnames}
columnTypes={coltypes}
rowcount={rowcount}
@@ -128,6 +150,9 @@ export const SamplesPane = ({
onInputChange={handleInputChange}
isLoading={isLoading}
canDownload={canDownload}
rowLimit={rowLimit}
rowLimitOptions={ROW_LIMIT_OPTIONS}
onRowLimitChange={handleRowLimitChange}
/>
<Error>{responseError}</Error>
</>
@@ -142,7 +167,7 @@ export const SamplesPane = ({
return (
<>
<TableControls
data={filteredData}
data={data}
columnNames={colnames}
columnTypes={coltypes}
rowcount={rowcount}
@@ -150,19 +175,22 @@ export const SamplesPane = ({
onInputChange={handleInputChange}
isLoading={isLoading}
canDownload={canDownload}
rowLimit={rowLimit}
rowLimitOptions={ROW_LIMIT_OPTIONS}
onRowLimitChange={handleRowLimitChange}
/>
<TableView
columns={columns}
data={filteredData}
pageSize={dataSize}
noDataText={t('No results')}
emptyWrapperType={EmptyWrapperType.Small}
className="table-condensed"
isPaginationSticky
showRowCount={false}
size={TableSize.Small}
small
/>
<GridContainer>
<GridSizer ref={measuredRef}>
<GridTable
data={data}
columns={columns}
height={gridHeight}
size={GridSize.Small}
externalFilter={keywordFilter}
showRowNumber
/>
</GridSizer>
</GridContainer>
</>
);
};

View File

@@ -17,46 +17,52 @@
* under the License.
*/
import { useState, useCallback } from 'react';
import { t } from '@apache-superset/core/translation';
import { styled } from '@apache-superset/core/theme';
import { GridTable } from 'src/components/GridTable';
import { GridSize } from 'src/components/GridTable/constants';
import {
TableView,
TableSize,
EmptyWrapperType,
} from '@superset-ui/core/components';
import {
useFilteredTableData,
useTableColumns,
} from 'src/explore/components/DataTableControl';
useGridColumns,
useKeywordFilter,
useGridHeight,
} from './useGridResultTable';
import { TableControls } from './DataTableControls';
import { SingleQueryResultPaneProp } from '../types';
const ResultPaneContainer = styled.div`
display: flex;
flex-direction: column;
flex: 1;
min-height: 0;
`;
const GridContainer = styled.div`
flex: 1;
min-height: 0;
position: relative;
`;
const GridSizer = styled.div`
position: absolute;
inset: 0;
`;
export const SingleQueryResultPane = ({
data,
colnames,
coltypes,
rowcount,
datasourceId,
dataSize = 50,
isVisible,
canDownload,
columnDisplayNames,
isPaginationSticky = true,
rowLimit,
rowLimitOptions,
onRowLimitChange,
}: SingleQueryResultPaneProp) => {
const [filterText, setFilterText] = useState('');
const { gridHeight, measuredRef } = useGridHeight();
// this is to preserve the order of the columns, even if there are integer values,
// while also only grabbing the first column's keys
const columns = useTableColumns(
colnames,
coltypes,
data,
datasourceId,
isVisible,
{}, // moreConfig
true, // allowHTML
columnDisplayNames,
);
const filteredData = useFilteredTableData(filterText, data);
const columns = useGridColumns(colnames, coltypes, data, columnDisplayNames);
const keywordFilter = useKeywordFilter(filterText);
const handleInputChange = useCallback(
(input: string) => setFilterText(input),
@@ -64,9 +70,9 @@ export const SingleQueryResultPane = ({
);
return (
<>
<ResultPaneContainer>
<TableControls
data={filteredData}
data={data}
columnNames={colnames}
columnTypes={coltypes}
rowcount={rowcount}
@@ -74,19 +80,22 @@ export const SingleQueryResultPane = ({
onInputChange={handleInputChange}
isLoading={false}
canDownload={canDownload}
rowLimit={rowLimit}
rowLimitOptions={rowLimitOptions}
onRowLimitChange={onRowLimitChange}
/>
<TableView
columns={columns}
size={TableSize.Small}
data={filteredData}
pageSize={dataSize}
noDataText={t('No results')}
emptyWrapperType={EmptyWrapperType.Small}
className="table-condensed"
isPaginationSticky={isPaginationSticky}
showRowCount={false}
small
/>
</>
<GridContainer>
<GridSizer ref={measuredRef}>
<GridTable
data={data}
columns={columns}
height={gridHeight}
size={GridSize.Small}
externalFilter={keywordFilter}
showRowNumber
/>
</GridSizer>
</GridContainer>
</ResultPaneContainer>
);
};

View File

@@ -0,0 +1,123 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you 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 { useMemo, useCallback, useRef, useState } from 'react';
import { getTimeFormatter, safeHtmlSpan, TimeFormats } from '@superset-ui/core';
import { Constants } from '@superset-ui/core/components';
import { GenericDataType } from '@apache-superset/core/common';
import type { IRowNode } from 'ag-grid-community';
const timeFormatter = getTimeFormatter(TimeFormats.DATABASE_DATETIME);
export function useGridColumns(
colnames: string[] | undefined,
coltypes: GenericDataType[] | undefined,
data: Record<string, any>[] | undefined,
columnDisplayNames?: Record<string, string>,
) {
return useMemo(
() =>
colnames && data?.length
? colnames
.filter((column: string) => Object.keys(data[0]).includes(column))
.map((key, index) => {
const colType = coltypes?.[index];
const headerLabel = columnDisplayNames?.[key] ?? key;
return {
label: key,
headerName: headerLabel,
render: ({ value }: { value: unknown }) => {
if (value === true) {
return Constants.BOOL_TRUE_DISPLAY;
}
if (value === false) {
return Constants.BOOL_FALSE_DISPLAY;
}
if (value === null) {
return (
<span style={{ color: 'var(--ant-color-text-tertiary)' }}>
{Constants.NULL_DISPLAY}
</span>
);
}
if (
colType === GenericDataType.Temporal &&
typeof value === 'number'
) {
return timeFormatter(value);
}
if (typeof value === 'string') {
return safeHtmlSpan(value);
}
return String(value);
},
};
})
: [],
[colnames, data, coltypes, columnDisplayNames],
);
}
export function useKeywordFilter(filterText: string) {
return useCallback(
(node: IRowNode) => {
if (filterText && node.data) {
const lowerFilter = filterText.toLowerCase();
return Object.values(node.data).some(
(value: unknown) =>
value != null && String(value).toLowerCase().includes(lowerFilter),
);
}
return true;
},
[filterText],
);
}
/**
* Measures the height of an absolutely-positioned inner element that fills
* its relative-positioned parent. Uses a callback ref so the ResizeObserver
* is created when the element mounts (which may be after initial render if
* the component conditionally renders a loading state first).
*/
export function useGridHeight(fallbackHeight = 400) {
const [gridHeight, setGridHeight] = useState(fallbackHeight);
const observerRef = useRef<ResizeObserver | null>(null);
const measuredRef = useCallback((el: HTMLDivElement | null) => {
if (observerRef.current) {
observerRef.current.disconnect();
observerRef.current = null;
}
if (!el) return;
const observer = new ResizeObserver(entries => {
const entry = entries[0];
if (entry) {
const h = Math.floor(entry.contentRect.height);
if (h > 0) {
setGridHeight(prev => (prev !== h ? h : prev));
}
}
});
observer.observe(el);
observerRef.current = observer;
}, []);
return { gridHeight, measuredRef };
}

View File

@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { useState, useEffect, ReactElement, useCallback } from 'react';
import { useState, useEffect, useMemo, ReactElement, useCallback } from 'react';
import { t } from '@apache-superset/core/translation';
import {
@@ -29,7 +29,7 @@ import { EmptyState, Loading } from '@superset-ui/core/components';
import { getChartDataRequest } from 'src/components/Chart/chartAction';
import { ResultsPaneProps, QueryResultInterface } from '../types';
import { SingleQueryResultPane } from './SingleQueryResultPane';
import { TableControls } from './DataTableControls';
import { TableControls, ROW_LIMIT_OPTIONS } from './DataTableControls';
const Error = styled.pre`
margin-top: ${({ theme }) => `${theme.sizeUnit * 4}px`};
@@ -53,7 +53,6 @@ export const useResultsPane = ({
errorMessage,
setForceQuery,
isVisible,
dataSize = 50,
canDownload,
columnDisplayNames,
}: ResultsPaneProps): ReactElement[] => {
@@ -61,6 +60,8 @@ export const useResultsPane = ({
queryFormData?.viz_type || queryFormData?.vizType,
);
const chartRowLimit = Number(queryFormData?.row_limit) || 10000;
const [rowLimit, setRowLimit] = useState(1000);
const [resultResp, setResultResp] = useState<QueryResultInterface[]>([]);
const [isLoading, setIsLoading] = useState<boolean>(true);
const [responseError, setResponseError] = useState<string>('');
@@ -69,12 +70,28 @@ export const useResultsPane = ({
const noOpInputChange = useCallback(() => {}, []);
// Never exceed the chart's own row_limit
const effectiveRowLimit = Math.min(rowLimit, chartRowLimit);
const cappedFormData = useMemo(
() => ({ ...queryFormData, row_limit: effectiveRowLimit }),
[queryFormData, effectiveRowLimit],
);
const handleRowLimitChange = useCallback(
(limit: number) => {
setRowLimit(limit);
cache.delete(cappedFormData);
},
[cappedFormData],
);
useEffect(() => {
// it's an invalid formData when gets a errorMessage
if (errorMessage) return;
if (isRequest && cache.has(queryFormData)) {
if (isRequest && cache.has(cappedFormData)) {
setResultResp(
ensureIsArray(cache.get(queryFormData)) as QueryResultInterface[],
ensureIsArray(cache.get(cappedFormData)) as QueryResultInterface[],
);
setResponseError('');
if (queryForce) {
@@ -82,10 +99,10 @@ export const useResultsPane = ({
}
setIsLoading(false);
}
if (isRequest && !cache.has(queryFormData)) {
if (isRequest && !cache.has(cappedFormData)) {
setIsLoading(true);
getChartDataRequest({
formData: queryFormData,
formData: cappedFormData,
force: queryForce,
resultFormat: 'json',
resultType: 'results',
@@ -94,7 +111,7 @@ export const useResultsPane = ({
.then(({ json }) => {
setResultResp(ensureIsArray(json.result) as QueryResultInterface[]);
setResponseError('');
cache.set(queryFormData, json.result);
cache.set(cappedFormData, json.result);
if (queryForce) {
setForceQuery?.(false);
}
@@ -108,7 +125,7 @@ export const useResultsPane = ({
setIsLoading(false);
});
}
}, [queryFormData, isRequest]);
}, [cappedFormData, isRequest]);
useEffect(() => {
if (errorMessage) {
@@ -163,11 +180,13 @@ export const useResultsPane = ({
colnames={result.colnames}
coltypes={result.coltypes}
rowcount={result.rowcount}
dataSize={dataSize}
datasourceId={queryFormData.datasource}
isVisible={isVisible}
canDownload={canDownload}
columnDisplayNames={columnDisplayNames}
rowLimit={rowLimit}
rowLimitOptions={ROW_LIMIT_OPTIONS}
onRowLimitChange={handleRowLimitChange}
/>
</StyledDiv>
));

View File

@@ -19,16 +19,16 @@
import fetchMock from 'fetch-mock';
import { FeatureFlag } from '@superset-ui/core';
import * as copyUtils from 'src/utils/copy';
import {
render,
screen,
userEvent,
waitForElementToBeRemoved,
} from 'spec/helpers/testing-library';
import { render, screen, userEvent } from 'spec/helpers/testing-library';
import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact';
import { setItem, LocalStorageKeys } from 'src/utils/localStorageHelpers';
import { DataTablesPane } from '..';
import { createDataTablesPaneProps } from './fixture';
beforeAll(() => {
setupAGGridModules();
});
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
describe('DataTablesPane', () => {
// Collapsed/expanded state depends on local storage
@@ -175,12 +175,6 @@ describe('DataTablesPane', () => {
expect(screen.getByText('Action')).toBeVisible();
expect(screen.getByText('Horror')).toBeVisible();
userEvent.type(screen.getByPlaceholderText('Search'), 'hor');
await waitForElementToBeRemoved(() => screen.queryByText('Action'));
expect(screen.getByText('Horror')).toBeVisible();
expect(screen.queryByText('Action')).not.toBeInTheDocument();
fetchMock.clearHistory().removeRoutes();
});

View File

@@ -20,14 +20,18 @@ import fetchMock from 'fetch-mock';
import {
screen,
render,
userEvent,
waitForElementToBeRemoved,
waitFor,
} from 'spec/helpers/testing-library';
import { ChartMetadata, ChartPlugin, VizType } from '@superset-ui/core';
import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact';
import { ResultsPaneOnDashboard } from '../components';
import { createResultsPaneOnDashboardProps } from './fixture';
beforeAll(() => {
setupAGGridModules();
});
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
describe('ResultsPaneOnDashboard', () => {
// render and render errorMessage
@@ -126,12 +130,12 @@ describe('ResultsPaneOnDashboard', () => {
expect(await findByText('Bad request')).toBeVisible();
});
test('force query, render and search', async () => {
test('force query, render', async () => {
const props = createResultsPaneOnDashboardProps({
sliceId: 144,
queryForce: true,
});
const { queryByText, getByPlaceholderText } = render(
const { queryByText } = render(
<ResultsPaneOnDashboard {...props} setForceQuery={setForceQuery} />,
{
useRedux: true,
@@ -144,11 +148,6 @@ describe('ResultsPaneOnDashboard', () => {
expect(queryByText('2 rows')).toBeVisible();
expect(queryByText('Action')).toBeVisible();
expect(queryByText('Horror')).toBeVisible();
userEvent.type(getByPlaceholderText('Search'), 'hor');
await waitForElementToBeRemoved(() => queryByText('Action'));
expect(queryByText('Horror')).toBeVisible();
expect(queryByText('Action')).not.toBeInTheDocument();
});
test('multiple results pane', async () => {

View File

@@ -17,19 +17,19 @@
* under the License.
*/
import fetchMock from 'fetch-mock';
import {
render,
userEvent,
waitForElementToBeRemoved,
waitFor,
} from 'spec/helpers/testing-library';
import { render, waitFor } from 'spec/helpers/testing-library';
import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact';
import { SamplesPane } from '../components';
import { createSamplesPaneProps } from './fixture';
beforeAll(() => {
setupAGGridModules();
});
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
describe('SamplesPane', () => {
fetchMock.post(
'end:/datasource/samples?force=false&datasource_type=table&datasource_id=34',
'end:/datasource/samples?force=false&datasource_type=table&datasource_id=34&per_page=100&page=1',
{
result: {
data: [],
@@ -40,7 +40,7 @@ describe('SamplesPane', () => {
);
fetchMock.post(
'end:/datasource/samples?force=true&datasource_type=table&datasource_id=35',
'end:/datasource/samples?force=true&datasource_type=table&datasource_id=35&per_page=100&page=1',
{
result: {
data: [
@@ -56,7 +56,7 @@ describe('SamplesPane', () => {
);
fetchMock.post(
'end:/datasource/samples?force=false&datasource_type=table&datasource_id=36',
'end:/datasource/samples?force=false&datasource_type=table&datasource_id=36&per_page=100&page=1',
400,
);
@@ -91,12 +91,12 @@ describe('SamplesPane', () => {
expect(await findByText('Error: Bad request')).toBeVisible();
});
test('force query, render and search', async () => {
test('force query, render', async () => {
const props = createSamplesPaneProps({
datasourceId: 35,
queryForce: true,
});
const { queryByText, getByPlaceholderText } = render(
const { queryByText } = render(
<SamplesPane {...props} setForceQuery={setForceQuery} />,
{
useRedux: true,
@@ -109,10 +109,5 @@ describe('SamplesPane', () => {
expect(queryByText('2 rows')).toBeVisible();
expect(queryByText('Action')).toBeVisible();
expect(queryByText('Horror')).toBeVisible();
userEvent.type(getByPlaceholderText('Search'), 'hor');
await waitForElementToBeRemoved(() => queryByText('Action'));
expect(queryByText('Horror')).toBeVisible();
expect(queryByText('Action')).not.toBeInTheDocument();
});
});

View File

@@ -90,6 +90,10 @@ export const createSamplesPaneProps = ({
({
isRequest,
datasource: { ...datasource, id: datasourceId },
queryFormData: {
...queryFormData,
datasource: `${datasourceId}__table`,
},
queryForce,
isVisible: true,
setForceQuery: jest.fn(),

View File

@@ -56,10 +56,9 @@ export interface ResultsPaneProps {
export interface SamplesPaneProps {
isRequest: boolean;
datasource: Datasource;
queryFormData: LatestQueryFormData;
queryForce: boolean;
setForceQuery?: SetForceQueryAction;
dataSize?: number;
// reload OriginalFormattedTimeColumns from localStorage when isVisible is true
isVisible: boolean;
canDownload: boolean;
}
@@ -74,6 +73,9 @@ export interface TableControlsProps {
isLoading: boolean;
rowcount: number;
canDownload: boolean;
rowLimit?: number;
rowLimitOptions?: { value: number; label: string }[];
onRowLimitChange?: (limit: number) => void;
}
export interface QueryResultInterface {
@@ -86,11 +88,11 @@ export interface QueryResultInterface {
export interface SingleQueryResultPaneProp extends QueryResultInterface {
// {datasource.id}__{datasource.type}, eg: 1__table
datasourceId?: string;
dataSize?: number;
// reload OriginalFormattedTimeColumns from localStorage when isVisible is true
isVisible: boolean;
canDownload: boolean;
// Optional map of column/metric name -> verbose label
columnDisplayNames?: Record<string, string>;
isPaginationSticky?: boolean;
rowLimit?: number;
rowLimitOptions?: { value: number; label: string }[];
onRowLimitChange?: (limit: number) => void;
}

View File

@@ -100,7 +100,7 @@ class SamplesRequestSchema(Schema):
force = fields.Boolean(load_default=False)
page = fields.Integer(load_default=1)
per_page = fields.Integer(
validate=validate.Range(min=1, max=1000),
validate=validate.Range(min=1, max=10000),
load_default=None,
)
dashboard_id = fields.Integer(required=False, allow_none=True, load_default=None)

View File

@@ -807,7 +807,7 @@ def test_get_samples_pagination(test_client, login_as_admin, virtual_dataset):
assert rv.json["result"]["total_count"] == 10
# 2. incorrect per_page
per_pages = (current_app.config["SAMPLES_ROW_LIMIT"] + 1, 0, "xx")
per_pages = (10001, 0, "xx")
for per_page in per_pages:
uri = f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=table&per_page={per_page}" # noqa: E501
rv = test_client.post(uri, json={})