fix(dashboard): row limit warning missing for non-table charts

Boolean short-circuit on `isTableChart && secondQueryResponse?.data?.[0]?.rowcount`
returned `false` for non-table viz types; `false != null` is `true`, so
`Number(false) = 0` became `sqlRowCount` and the warning never fired.
Also widens the table check to include AG Grid table so server-paginated
AG Grid charts read the rowcount sidecar query like the regular table.
This commit is contained in:
Mehmet Salih Yavuz
2026-05-06 18:47:27 +03:00
parent e667ceb6cf
commit 85b9802bbd
4 changed files with 132 additions and 7 deletions

View File

@@ -763,6 +763,123 @@ test('Should show row count warning for table chart with server pagination when
mockUseUiConfig.mockRestore();
});
test('Should show row count warning for non-table chart when row limit is reached', () => {
const props = createProps({
formData: {
...createProps().formData,
viz_type: VizType.Bar,
row_limit: 10,
},
slice: {
...createProps().slice,
form_data: {
...createProps().slice.form_data,
viz_type: VizType.Bar,
row_limit: 10,
},
viz_type: VizType.Bar,
},
});
const barChartState = {
...initialState,
charts: {
[props.slice.slice_id]: {
id: MOCKED_CHART_ID,
chartStatus: 'rendered',
queriesResponse: [
{
sql_rowcount: 10,
data: Array(10).fill({}),
},
],
},
},
};
const mockUseUiConfig = useUiConfig as jest.MockedFunction<
typeof useUiConfig
>;
mockUseUiConfig.mockReturnValue({
hideTitle: false,
hideTab: false,
hideNav: false,
hideChartControls: false,
emitDataMasks: false,
showRowLimitWarning: true,
});
render(<SliceHeader {...props} />, {
useRedux: true,
useRouter: true,
initialState: barChartState,
});
expect(screen.getByTestId('warning')).toBeInTheDocument();
mockUseUiConfig.mockRestore();
});
test('Should show row count warning for ag-grid table chart with server pagination when limit is reached', () => {
const props = createProps({
formData: {
...createProps().formData,
viz_type: VizType.TableAgGrid,
row_limit: 10,
server_pagination: true,
},
slice: {
...createProps().slice,
form_data: {
...createProps().slice.form_data,
viz_type: VizType.TableAgGrid,
row_limit: 10,
server_pagination: true,
},
viz_type: VizType.TableAgGrid,
},
});
const agGridWithPaginationState = {
...initialState,
charts: {
[props.slice.slice_id]: {
id: MOCKED_CHART_ID,
chartStatus: 'rendered',
queriesResponse: [
{
sql_rowcount: 10,
data: Array(10).fill({}),
},
{
data: [{ rowcount: 50 }],
},
],
},
},
};
const mockUseUiConfig = useUiConfig as jest.MockedFunction<
typeof useUiConfig
>;
mockUseUiConfig.mockReturnValue({
hideTitle: false,
hideTab: false,
hideNav: false,
hideChartControls: false,
emitDataMasks: false,
showRowLimitWarning: true,
});
render(<SliceHeader {...props} />, {
useRedux: true,
useRouter: true,
initialState: agGridWithPaginationState,
});
expect(screen.getByTestId('warning')).toBeInTheDocument();
mockUseUiConfig.mockRestore();
});
test('Should NOT show row count warning for table chart with server pagination when limit is NOT reached', () => {
const props = createProps({
formData: {

View File

@@ -26,7 +26,7 @@ import {
useState,
} from 'react';
import { t } from '@apache-superset/core/translation';
import { getExtensionsRegistry, QueryData } from '@superset-ui/core';
import { getExtensionsRegistry, QueryData, VizType } from '@superset-ui/core';
import {
css,
styled,
@@ -206,9 +206,12 @@ const SliceHeader = forwardRef<HTMLDivElement, SliceHeaderProps>(
const rowLimit = Number(formData.row_limit ?? 0);
const isTableChart = formData.viz_type === 'table';
const countFromSecondQuery =
isTableChart && secondQueryResponse?.data?.[0]?.rowcount;
const isTableChart =
formData.viz_type === VizType.Table ||
formData.viz_type === VizType.TableAgGrid;
const countFromSecondQuery = isTableChart
? secondQueryResponse?.data?.[0]?.rowcount
: undefined;
const sqlRowCount =
countFromSecondQuery != null

View File

@@ -27,6 +27,7 @@ import {
RefObject,
} from 'react';
import type { ChartCustomization, JsonObject } from '@superset-ui/core';
import { VizType } from '@superset-ui/core';
import { styled } from '@apache-superset/core/theme';
import { t } from '@apache-superset/core/translation';
import { debounce } from 'lodash';
@@ -495,7 +496,9 @@ const Chart = (props: ChartProps) => {
const resultType = isPivot ? 'post_processed' : 'full';
let actualRowCount: number | undefined;
const isTableViz = (formData as JsonObject)?.viz_type === 'table';
const vizType = (formData as JsonObject)?.viz_type;
const isTableViz =
vizType === VizType.Table || vizType === VizType.TableAgGrid;
if (
isTableViz &&

View File

@@ -17,7 +17,7 @@
* under the License.
*/
import { forwardRef, RefObject } from 'react';
import { QueryData } from '@superset-ui/core';
import { QueryData, VizType } from '@superset-ui/core';
import { css, SupersetTheme } from '@apache-superset/core/theme';
import {
CachedLabel,
@@ -68,7 +68,9 @@ export const ChartPills = forwardRef(
const firstQueryResponse = queriesResponse?.[0];
// For table charts with server pagination, check second query for total count
const isTableChart = formData?.viz_type === 'table';
const isTableChart =
formData?.viz_type === VizType.Table ||
formData?.viz_type === VizType.TableAgGrid;
const hasCountQuery = queriesResponse && queriesResponse.length > 1;
const countFromSecondQuery = hasCountQuery
? queriesResponse[1]?.data?.[0]?.rowcount