Compare commits

..
Author SHA1 Message Date
Enzo MartellucciandClaude Sonnet 5 6dad5c9390 fix(echarts): align pinned axis labels with capped ticks, extract shared helper
axisLabel.customValues carried the full weekly bucket list while
axisTick.customValues (which splitLine also follows) carried the
capTickMarks()-downsampled subset. hideOverlap could keep a label at an
index capTickMarks had dropped, leaving it with no tick or gridline
under it. Both now share one capped set via a new
getTemporalAxisTickConfig() helper in series.ts, which also de-dupes
the near-identical axis-fragment block between Timeseries and
MixedTimeseries.

Also keeps showMaxLabel active on pinned (weekly) axes instead of
skipping it: it only shields the boundary label's immediate neighbour,
but that's strictly better than the no protection pinned axes had
before, and matches the guarantee unpinned axes already get (#39899).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-28 15:52:51 +02:00
Enzo Martellucci 1c3aeecbd2 Merge branch 'master' into enxdev/fix/echarts 2026-08-28 15:45:02 +02:00
Enzo Martellucci 3b801354f3 Merge branch 'master' into enxdev/fix/echarts 2026-08-28 11:43:39 +02:00
Enzo MartellucciandClaude Sonnet 5 92085abc0d fix(echarts): cap pinned axis tick marks on long weekly ranges
axisLabel.hideOverlap thins displayed labels dynamically, but
axisTick.customValues had no such mechanism, so pinning it to every
bucket drew an unlabeled comb of tick marks (and matching gridlines)
on wide weekly-grain ranges.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-28 11:43:04 +02:00
Enzo Martellucci a5657e04e1 Merge branch 'master' into enxdev/fix/echarts 2026-08-28 11:38:36 +02:00
Enzo Martellucci e61295d4b1 Merge branch 'master' into enxdev/fix/echarts 2026-08-25 11:53:55 +02:00
Enzo Martellucci 1eba5c0b7c chore: address review comments 2026-08-25 11:53:35 +02:00
Enzo Martellucci 94d8a0bf77 Merge branch 'master' into enxdev/fix/echarts 2026-08-24 17:48:15 +02:00
Enzo Martellucci 5a1f6332bf Merge branch 'master' into enxdev/fix/echarts
# Conflicts:
#	superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
#	superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
#	superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
2026-08-24 17:00:52 +02:00
Enzo Martellucci 4d30f6fb65 fix(echarts): keep pinned weekly axis labels thinned and annotation-safe 2026-08-19 19:30:05 +02:00
Enzo Martellucci 296ab06d8d fix(echarts): place weekly time-axis ticks on the data buckets 2026-08-19 18:50:51 +02:00
51 changed files with 816 additions and 1116 deletions
-1
View File
@@ -10,7 +10,6 @@
.stylelintignore
.flake8
.nvmrc
.npmrc
.rat-excludes
.swcrc
.*log
@@ -493,8 +493,8 @@ Frontend assets (TypeScript, JavaScript, CSS, and images) must be compiled in or
First, be sure you are using the following versions of Node.js and npm:
- `Node.js`: Version 24 (see `superset-frontend/.nvmrc` for the exact version)
- `npm`: Version 11
- `Node.js`: Version 22 (LTS)
- `npm`: Version 10
We recommend using [nvm](https://github.com/nvm-sh/nvm) to manage your node environment:
@@ -507,8 +507,8 @@ export NVM_DIR="$HOME/.nvm"
[ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion" # This loads nvm bash_completion
cd superset-frontend
nvm install
nvm use
nvm install --lts
nvm use --lts
```
Or if you use the default macOS starting with Catalina shell `zsh`, try:
@@ -21,7 +21,6 @@ from abc import ABC, abstractmethod
from typing import Any, Generic, TypeVar
from pydantic import BaseModel
from superset_core.semantic_layers.view import SemanticView
ConfigT = TypeVar("ConfigT", bound=BaseModel)
-1
View File
@@ -1 +0,0 @@
../superset-frontend/.npmrc
-1
View File
@@ -1 +0,0 @@
min-release-age=3
+1 -1
View File
@@ -392,7 +392,7 @@
"@luma.gl/shadertools": "~9.2.5",
"@luma.gl/webgl": "~9.2.5",
"core-js": "^3.38.1",
"dompurify": "^3.4.13",
"dompurify": "^3.4.11",
"esbuild": "^0.28.1",
"eslint-plugin-import": {
"eslint": "$eslint"
@@ -38,21 +38,11 @@ function formatMemory(
: ['B', 'kB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB', 'RB', 'QB'];
const base = binary ? 1024 : 1000;
let i = Math.max(
0,
Math.min(
suffixes.length - 1,
Math.floor(Math.log(absValue) / Math.log(base)),
),
const i = Math.min(
suffixes.length - 1,
Math.floor(Math.log(absValue) / Math.log(base)),
);
let scaled = parseFloat((absValue / Math.pow(base, i)).toFixed(decimals));
if (scaled >= base && i < suffixes.length - 1) {
i += 1;
scaled = parseFloat((absValue / Math.pow(base, i)).toFixed(decimals));
}
formatted = `${sign}${scaled}${suffixes[i]}`;
formatted = `${sign}${parseFloat((absValue / Math.pow(base, i)).toFixed(decimals))}${suffixes[i]}`;
}
if (transfer) {
@@ -60,31 +60,6 @@ test('formats float bytes in human readable format with default options', () =>
expect(formatter(1200.666)).toBe('1.2kB');
});
test('formats values below one byte without dropping the unit', () => {
const formatter = createMemoryFormatter();
expect(formatter(0.5)).toBe('0.5B');
expect(formatter(0.004)).toBe('0B');
expect(formatter(-0.25)).toBe('-0.25B');
const binaryFormatter = createMemoryFormatter({ binary: true });
expect(binaryFormatter(0.5)).toBe('0.5B');
});
test('rolls over to the next unit when rounding reaches the base', () => {
const formatter = createMemoryFormatter();
expect(formatter(999999)).toBe('1MB');
expect(formatter(999995)).toBe('1MB');
expect(formatter(999994)).toBe('999.99kB');
expect(formatter(-999999)).toBe('-1MB');
const binaryFormatter = createMemoryFormatter({ binary: true });
expect(binaryFormatter(1024 * 1024 - 1)).toBe('1MiB');
// the largest unit has nothing to roll over into
const largest = createMemoryFormatter();
expect(largest(Math.pow(1000, 11))).toBe('1000QB');
});
test('formats bytes in human readable format with additional binary option', () => {
const formatter = createMemoryFormatter({ binary: true });
expect(formatter(0)).toBe('0B');
@@ -38,8 +38,6 @@ import { EchartsTimeseriesSeriesType } from '../Timeseries/types';
import {
legendSection,
minorTicks,
axisTicks,
gridlines,
richTooltipSection,
truncateXAxis,
xAxisBounds,
@@ -393,8 +391,6 @@ const config: ControlPanelConfig = {
...createCustomizeSection(t('Query B'), 'B'),
['zoomable'],
[minorTicks],
[axisTicks],
[gridlines],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
['x_axis_time_format'],
@@ -73,6 +73,8 @@ import {
getLegendProps,
getMinAndMaxFromBounds,
getOverMaxHiddenFormatter,
getTemporalAxisTickConfig,
getTemporalTickValues,
} from '../utils/series';
import { resolveLegendLayout } from '../utils/legendLayout';
import {
@@ -184,8 +186,6 @@ export default function transformProps(
opacityB,
minorSplitLine,
minorTicks,
gridlines,
axisTicks,
seriesType,
seriesTypeB,
showLegend,
@@ -764,6 +764,22 @@ export default function transformProps(
const { setDataMask = () => {}, onContextMenu } = hooks;
const alignTicks = yAxisIndex !== yAxisIndexB;
// Weekly grains: pin the ticks to the buckets. Both queries share the axis.
// Skipped when a timeseries annotation is shown: it widens the axis past the
// buckets and ECharts clips pinned ticks to the extent, leaving that span bare.
const hasTimeseriesAnnotation = annotationLayers.some(
(layer: AnnotationLayer) =>
layer.show && isTimeseriesAnnotationLayer(layer),
);
const temporalTickValues = hasTimeseriesAnnotation
? undefined
: getTemporalTickValues(
[...rebasedDataA, ...rebasedDataB],
xAxisLabel,
xAxisType,
resolvedTimeGrain,
);
const echartOptions: EChartsCoreOption = {
useUTC: true,
grid: {
@@ -775,23 +791,15 @@ export default function transformProps(
name: xAxisTitle,
nameGap: xAxisTitleMarginPx,
nameLocation: 'middle',
axisLabel: {
hideOverlap: showMaxLabel
? false
: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
formatter: deduplicatedFormatter,
rotate: xAxisLabelRotation,
interval: xAxisLabelInterval,
...(showMaxLabel && {
showMaxLabel: true,
alignMaxLabel: 'right',
showMinLabel: true,
alignMinLabel: 'left',
}),
},
...getTemporalAxisTickConfig(
temporalTickValues,
showMaxLabel,
xAxisType,
xAxisLabelRotation,
xAxisLabelInterval,
deduplicatedFormatter,
),
minorTick: { show: minorTicks },
axisTick: { show: axisTicks ? 'auto' : false },
...(gridlines ? {} : { splitLine: { show: false } }),
minInterval:
xAxisType === AxisType.Time && resolvedTimeGrain && !forceMaxInterval
? (TIMEGRAIN_TO_TIMESTAMP[
@@ -822,8 +830,6 @@ export default function transformProps(
min: yAxisMin,
max: yAxisMax,
minorTick: { show: minorTicks },
axisTick: { show: axisTicks ? 'auto' : false },
splitLine: { show: gridlines },
minorSplitLine: { show: minorSplitLine },
axisLabel: {
formatter: getYAxisFormatter(
@@ -846,7 +852,6 @@ export default function transformProps(
min: minSecondary,
max: maxSecondary,
minorTick: { show: minorTicks },
axisTick: { show: axisTicks ? 'auto' : false },
splitLine: { show: false },
minorSplitLine: { show: minorSplitLine },
axisLabel: {
@@ -48,8 +48,6 @@ export type EchartsMixedTimeseriesFormData = QueryFormData & {
// shared properties
minorSplitLine: boolean;
minorTicks: boolean;
gridlines: boolean;
axisTicks: boolean;
logAxis: boolean;
logAxisSecondary: boolean;
yAxisFormat?: string;
@@ -115,8 +113,6 @@ export const DEFAULT_FORM_DATA: EchartsMixedTimeseriesFormData = {
...DEFAULT_LEGEND_FORM_DATA,
annotationLayers: [],
minorSplitLine: TIMESERIES_DEFAULTS.minorSplitLine,
gridlines: TIMESERIES_DEFAULTS.gridlines,
axisTicks: TIMESERIES_DEFAULTS.axisTicks,
truncateYAxis: TIMESERIES_DEFAULTS.truncateYAxis,
truncateYAxisSecondary: TIMESERIES_DEFAULTS.truncateYAxis,
logAxis: TIMESERIES_DEFAULTS.logAxis,
@@ -44,8 +44,6 @@ import {
truncateXAxis,
xAxisBounds,
minorTicks,
axisTicks,
gridlines,
forceMaxInterval,
} from '../../controls';
import { AreaChartStackControlOptions } from '../../constants';
@@ -176,8 +174,6 @@ const config: ControlPanelConfig = {
},
],
[minorTicks],
[axisTicks],
[gridlines],
['zoomable'],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
@@ -133,8 +133,6 @@ const defaultFormData: EchartsTimeseriesFormData & {
metrics: [],
minorSplitLine: false,
minorTicks: false,
gridlines: true,
axisTicks: true,
opacity: 1,
orderDesc: false,
rowLimit: 0,
@@ -40,8 +40,6 @@ import {
import {
legendSection,
minorTicks,
axisTicks,
gridlines,
richTooltipSection,
seriesOrderSection,
showValueSectionWithoutStream,
@@ -390,8 +388,6 @@ const config: ControlPanelConfig = {
},
],
[minorTicks],
[axisTicks],
[gridlines],
['zoomable'],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
@@ -37,8 +37,6 @@ import {
import {
legendSection,
minorTicks,
axisTicks,
gridlines,
richTooltipSection,
seriesOrderSection,
showValueSection,
@@ -158,8 +156,6 @@ const config: ControlPanelConfig = {
],
['zoomable'],
[minorTicks],
[axisTicks],
[gridlines],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
[
@@ -42,8 +42,6 @@ import {
import {
legendSection,
minorTicks,
axisTicks,
gridlines,
richTooltipSection,
seriesOrderSection,
showValueSection,
@@ -482,8 +480,6 @@ const config: ControlPanelConfig = {
],
['zoomable'],
[minorTicks],
[axisTicks],
[gridlines],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
...createAxisControl('x'),
@@ -37,8 +37,6 @@ import {
import {
legendSection,
minorTicks,
axisTicks,
gridlines,
richTooltipSection,
seriesOrderSection,
showValueSectionWithoutStack,
@@ -107,8 +105,6 @@ const config: ControlPanelConfig = {
],
['zoomable'],
[minorTicks],
[axisTicks],
[gridlines],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
[
@@ -35,8 +35,6 @@ import { DEFAULT_FORM_DATA, TIME_SERIES_DESCRIPTION_TEXT } from '../constants';
import {
legendSection,
minorTicks,
axisTicks,
gridlines,
richTooltipSection,
seriesOrderSection,
showValueSection,
@@ -159,8 +157,6 @@ const config: ControlPanelConfig = {
],
['zoomable'],
[minorTicks],
[axisTicks],
[gridlines],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
[
@@ -67,8 +67,6 @@ export const DEFAULT_FORM_DATA: EchartsTimeseriesFormData = {
maxMarkerSize: 30,
minMarkerSize: 5,
minorSplitLine: false,
gridlines: true,
axisTicks: true,
opacity: 0.2,
orderDesc: true,
rowLimit: 10000,
@@ -88,6 +88,8 @@ import {
getHorizontalLegendAvailableWidth,
getLegendProps,
getMinAndMaxFromBounds,
getTemporalAxisTickConfig,
getTemporalTickValues,
} from '../utils/series';
import { resolveLegendLayout } from '../utils/legendLayout';
import {
@@ -283,8 +285,6 @@ export default function transformProps(
metrics,
minorSplitLine,
minorTicks,
gridlines,
axisTicks,
onlyTotal,
opacity,
orientation,
@@ -1247,6 +1247,23 @@ export default function transformProps(
})()
: xAxisFormatter;
// Weekly grains: pin the ticks to the buckets ECharts would otherwise miss.
// A timeseries annotation contributes its own timestamps and widens the axis
// past the buckets, and ECharts clips pinned ticks to the extent, so that
// span would render bare — leave those charts on ECharts' own ticks.
const hasTimeseriesAnnotation = annotationLayers.some(
(layer: AnnotationLayer) =>
layer.show && isTimeseriesAnnotationLayer(layer),
);
const temporalTickValues = hasTimeseriesAnnotation
? undefined
: getTemporalTickValues(
rebasedData,
xAxisLabel,
xAxisType,
resolvedTimeGrain,
);
let xAxis: any = {
type: xAxisType,
name: xAxisTitle,
@@ -1256,34 +1273,15 @@ export default function transformProps(
groupBy.length === 0 && {
triggerEvent: true,
}),
axisLabel: {
// When rotation is applied on time axes, hideOverlap can
// aggressively hide the last label. Rotated labels already
// have less overlap, so disabling hideOverlap is safe.
// At 0° rotation, also disable hideOverlap when showMaxLabel
// is active so the forced boundary label is never suppressed
// by ECharts' overlap detection (#39899).
hideOverlap: showMaxLabel
? false
: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
formatter: deduplicatedFormatter,
rotate: xAxisLabelRotation,
interval: xAxisLabelInterval,
// Force the boundary labels on non-rotated time axes so the first
// and last dates stay visible: hideOverlap can hide the last label,
// and a min date that falls between "nice" ticks otherwise renders
// no beginning label. Skipped when rotated to avoid phantom labels
// at the axis boundary.
...(showMaxLabel && {
showMaxLabel: true,
alignMaxLabel: 'right',
showMinLabel: true,
alignMinLabel: 'left',
}),
},
...getTemporalAxisTickConfig(
temporalTickValues,
showMaxLabel,
xAxisType,
xAxisLabelRotation,
xAxisLabelInterval,
deduplicatedFormatter,
),
minorTick: { show: minorTicks },
axisTick: { show: axisTicks ? 'auto' : false },
...(gridlines ? {} : { splitLine: { show: false } }),
minInterval:
xAxisType === AxisType.Time && resolvedTimeGrain && !forceMaxInterval
? (TIMEGRAIN_TO_TIMESTAMP[
@@ -1328,7 +1326,7 @@ export default function transformProps(
max: yAxisMax,
minorTick: { show: isSmallChart ? false : minorTicks },
minorSplitLine: { show: isSmallChart ? false : minorSplitLine },
splitLine: { show: isSmallChart ? false : gridlines },
splitLine: { show: !isSmallChart },
axisLabel: {
show: !isMicroChart,
showMinLabel: !isMicroChart,
@@ -1342,7 +1340,7 @@ export default function transformProps(
yAxisFormat,
),
},
axisTick: { show: isSmallChart ? false : axisTicks },
axisTick: { show: !isSmallChart },
scale: truncateYAxis,
name: isSmallChart ? undefined : yAxisTitle,
nameGap: convertInteger(yAxisTitleMargin),
@@ -73,8 +73,6 @@ export type EchartsTimeseriesFormData = QueryFormData & {
metrics: QueryFormMetric[];
minorSplitLine: boolean;
minorTicks: boolean;
gridlines: boolean;
axisTicks: boolean;
opacity: number;
orderDesc: boolean;
rowLimit: number;
@@ -89,6 +89,16 @@ export const StackControlOptionsWithoutStream: [
[StackControlsValue.Stack, t('Stack')],
];
// Grains ECharts' time axis cannot tick on; see getTemporalTickValues in
// utils/series.
export const WEEKLY_TIME_GRAINS: ReadonlySet<string> = new Set([
TimeGranularity.WEEK,
TimeGranularity.WEEK_STARTING_SUNDAY,
TimeGranularity.WEEK_STARTING_MONDAY,
TimeGranularity.WEEK_ENDING_SATURDAY,
TimeGranularity.WEEK_ENDING_SUNDAY,
]);
export const TIMEGRAIN_TO_TIMESTAMP = {
[TimeGranularity.HOUR]: 3600 * 1000,
[TimeGranularity.DAY]: 3600 * 1000 * 24,
@@ -495,28 +495,6 @@ export const minorTicks: ControlSetItem = {
},
};
export const axisTicks: ControlSetItem = {
name: 'axisTicks',
config: {
type: 'CheckboxControl',
label: t('Axis ticks'),
default: true,
renderTrigger: true,
description: t('Show the main ticks on axes.'),
},
};
export const gridlines: ControlSetItem = {
name: 'gridlines',
config: {
type: 'CheckboxControl',
label: t('Gridlines'),
default: true,
renderTrigger: true,
description: t('Draw split lines for the main value axis ticks.'),
},
};
export const forceCategorical: ControlSetItem = {
name: 'forceCategorical',
config: {
@@ -42,6 +42,7 @@ import {
NULL_STRING,
StackControlsValue,
TIMESERIES_CONSTANTS,
WEEKLY_TIME_GRAINS,
} from '../constants';
import {
EchartsTimeseriesSeriesType,
@@ -986,6 +987,134 @@ export function getAxisType(
return AxisType.Category;
}
// `new Date('2024-04-06')` parses as UTC, but ECharts' own date parser treats
// zone-less strings as local time — mismatch would offset the pinned tick.
const DATE_ONLY_RE = /^(\d{4})(?:-(\d{1,2})(?:-(\d{1,2}))?)?$/;
function parseTemporalString(value: string): number {
const dateOnly = DATE_ONLY_RE.exec(value);
if (dateOnly) {
const [, year, month, day] = dateOnly;
return new Date(
Number(year),
Number(month || 1) - 1,
Number(day || 1),
).getTime();
}
return new Date(value).getTime();
}
/**
* Bucket timestamps a temporal axis should tick on, or undefined to let ECharts
* choose.
*
* ECharts generates time ticks from a calendar ladder with no week unit, so for
* weekly data it steps days from the 1st of each month instead: labels drift
* across weekdays and snap to month starts (#17226). Coarser grains already land
* on their data and keep ECharts' calendar-nice labels.
*/
export function getTemporalTickValues(
data: DataRecord[],
xAxisLabel: string,
xAxisType: AxisType,
timeGrain?: string,
): number[] | undefined {
if (
xAxisType !== AxisType.Time ||
!timeGrain ||
!WEEKLY_TIME_GRAINS.has(timeGrain)
) {
return undefined;
}
const values = new Set<number>();
data.forEach(row => {
const value = row[xAxisLabel];
const timestamp =
// eslint-disable-next-line no-nested-ternary
value instanceof Date
? value.getTime()
: typeof value === 'string'
? parseTemporalString(value)
: Number(value ?? NaN);
if (Number.isFinite(timestamp)) {
values.add(timestamp);
}
});
return values.size ? [...values].sort((a, b) => a - b) : undefined;
}
// Unlike axisLabel, axisTick has no overlap-based thinning, so pinning it to
// every bucket combs a long weekly range. Downsample evenly, keeping ends.
const MAX_PINNED_AXIS_TICKS = 60;
export function capTickMarks(
values: number[],
maxTicks: number = MAX_PINNED_AXIS_TICKS,
): number[] {
if (values.length <= maxTicks) {
return values;
}
const step = Math.ceil(values.length / maxTicks);
const capped = values.filter((_, index) => index % step === 0);
const last = values[values.length - 1];
if (capped[capped.length - 1] !== last) {
capped.push(last);
}
return capped;
}
/**
* axisLabel/axisTick fragment for a temporal x-axis, shared by Timeseries and
* MixedTimeseries. When temporalTickValues pins the axis to weekly buckets,
* both axisLabel.customValues (what hideOverlap thins from) and
* axisTick.customValues (what splitLine/gridlines follow) use the same capped
* set, so a label that survives hideOverlap thinning always lands on a real
* tick and gridline rather than a capped-away bucket.
*/
export function getTemporalAxisTickConfig(
temporalTickValues: number[] | undefined,
showMaxLabel: boolean,
xAxisType: AxisType,
xAxisLabelRotation: number,
xAxisLabelInterval: number | string | undefined,
formatter: unknown,
): {
axisLabel: Record<string, unknown>;
axisTick?: { customValues: number[] };
} {
const cappedTickValues = temporalTickValues
? capTickMarks(temporalTickValues)
: undefined;
return {
axisLabel: {
// Pinned ticks label every bucket, which does crowd, so thinning
// always wins there.
hideOverlap:
!!temporalTickValues ||
(showMaxLabel
? false
: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0)),
formatter,
rotate: xAxisLabelRotation,
interval: xAxisLabelInterval,
// Force the boundary labels so the first and last dates stay visible:
// hideOverlap can hide the last label, and a min date that falls
// between "nice" ticks otherwise renders no beginning label. Applied
// for pinned axes too — showMaxLabel only shields its immediate
// neighbour, so a farther label on a crowded weekly axis can still be
// dropped, but that's strictly better than no shielding at all.
...(showMaxLabel && {
showMaxLabel: true,
alignMaxLabel: 'right',
showMinLabel: true,
alignMinLabel: 'left',
}),
...(cappedTickValues && { customValues: cappedTickValues }),
},
...(cappedTickValues && { axisTick: { customValues: cappedTickValues } }),
};
}
export function getOverMaxHiddenFormatter(
config: {
max?: number;
@@ -116,8 +116,6 @@ const formData: EchartsMixedTimeseriesFormData = {
markerSizeB: 0,
minorSplitLine: false,
minorTicks: false,
gridlines: true,
axisTicks: true,
opacity: 0,
opacityB: 0,
orderDesc: false,
@@ -1512,55 +1510,94 @@ describe('EchartsMixedTimeseries tooltip truncation', () => {
});
});
function transformWithChrome(
overrides: Partial<EchartsMixedTimeseriesFormData>,
) {
const chartProps = createEchartsTimeseriesTestChartProps<
EchartsMixedTimeseriesFormData,
EchartsMixedTimeseriesProps
>({
...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
defaultQueriesData: queriesData,
formData: { ...formData, ...overrides },
queriesData,
describe('weekly x-axis tick alignment', () => {
const WEEK_MS = 7 * 24 * 3600 * 1000;
const MONDAYS = Array.from(
{ length: 6 },
(_, i) => Date.UTC(2026, 3, 6) + i * WEEK_MS,
);
const weeklyLabelMap = { ds: ['ds'], sum__num: ['sum__num'] };
const weeklyQuery = (timestamps: number[]) =>
createTestQueryData(
timestamps.map((ds, i) => ({ ds, sum__num: 10 + i })),
{
label_map: weeklyLabelMap,
colnames: ['ds', 'sum__num'],
coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
},
);
const weeklyChartProps = (
queryA: number[],
queryB: number[],
overrides: Partial<EchartsMixedTimeseriesFormData> = {},
) =>
createEchartsTimeseriesTestChartProps<
EchartsMixedTimeseriesFormData,
EchartsMixedTimeseriesProps
>({
...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
defaultQueriesData: [weeklyQuery(queryA), weeklyQuery(queryB)],
formData: {
...formData,
groupby: [],
groupbyB: [],
timeGrainSqla: TimeGranularity.WEEK_STARTING_MONDAY,
...overrides,
},
queriesData: [weeklyQuery(queryA), weeklyQuery(queryB)],
});
test('pins ticks, labels and gridlines to the weekly buckets', () => {
const { xAxis } = transformProps(weeklyChartProps(MONDAYS, MONDAYS))
.echartOptions as any;
expect(xAxis.type).toBe(AxisType.Time);
expect(xAxis.axisLabel.customValues).toEqual(MONDAYS);
// Gridlines follow axisTick.customValues, so splitLine needs no own copy.
expect(xAxis.axisTick.customValues).toEqual(MONDAYS);
expect(xAxis.splitLine).toBeUndefined();
});
const { echartOptions } = transformProps(chartProps);
return {
xAxis: echartOptions.xAxis as any,
yAxis: echartOptions.yAxis as any[],
};
}
test('draws gridlines and axis ticks when both are enabled', () => {
const { xAxis, yAxis } = transformWithChrome({});
test('keeps label thinning on when the labels are rotated', () => {
const { xAxis } = transformProps(
weeklyChartProps(MONDAYS, MONDAYS, { xAxisLabelRotation: 45 }),
).echartOptions as any;
expect(yAxis[0].splitLine.show).toBe(true);
// Both axes keep ECharts' own default, which the Mixed chart never overrode.
expect(yAxis[0].axisTick.show).toBe('auto');
expect(xAxis.axisTick.show).toBe('auto');
});
expect(xAxis.axisLabel.customValues).toEqual(MONDAYS);
expect(xAxis.axisLabel.hideOverlap).toBe(true);
});
test('hides the gridlines on the primary axis', () => {
const { xAxis, yAxis } = transformWithChrome({ gridlines: false });
test('keeps the showMaxLabel override at 0° rotation on pinned axes', () => {
// hideOverlap stays on for pinned ticks (they label every bucket), but
// showMaxLabel still shields the boundary label's immediate neighbour
// so the last bucket isn't silently dropped (#39899).
const { xAxis } = transformProps(weeklyChartProps(MONDAYS, MONDAYS))
.echartOptions as any;
expect(yAxis[0].splitLine.show).toBe(false);
// The secondary axis never draws gridlines, so the two grids cannot double up.
expect(yAxis[1].splitLine.show).toBe(false);
expect(xAxis.splitLine.show).toBe(false);
});
expect(xAxis.axisLabel.showMaxLabel).toBe(true);
expect(xAxis.axisLabel.hideOverlap).toBe(true);
});
test('never turns the secondary axis gridlines on', () => {
const { xAxis, yAxis } = transformWithChrome({ gridlines: true });
test('covers buckets contributed by either query', () => {
// The two queries share one axis, so a bucket present in only one of them
// still needs a tick.
const { xAxis } = transformProps(
weeklyChartProps(MONDAYS.slice(0, 3), MONDAYS.slice(2)),
).echartOptions as any;
expect(yAxis[0].splitLine.show).toBe(true);
expect(yAxis[1].splitLine.show).toBe(false);
expect(xAxis.splitLine).toBeUndefined();
});
expect(xAxis.axisLabel.customValues).toEqual(MONDAYS);
});
test('hides the ticks on the x axis and both y axes', () => {
const { xAxis, yAxis } = transformWithChrome({ axisTicks: false });
test('leaves grains ECharts places correctly untouched', () => {
const { xAxis } = transformProps(
weeklyChartProps(MONDAYS, MONDAYS, {
timeGrainSqla: TimeGranularity.MONTH,
}),
).echartOptions as any;
expect(xAxis.axisTick.show).toBe(false);
expect(yAxis[0].axisTick.show).toBe(false);
expect(yAxis[1].axisTick.show).toBe(false);
expect(xAxis.axisLabel.customValues).toBeUndefined();
expect(xAxis.axisTick?.customValues).toBeUndefined();
});
});
@@ -17,6 +17,7 @@
* under the License.
*/
import {
AnnotationData,
AnnotationSourceType,
AnnotationStyle,
AnnotationType,
@@ -2706,6 +2707,275 @@ describe('EchartsTimeseries tooltip truncation', () => {
});
});
describe('weekly x-axis tick alignment', () => {
// 13 Monday-aligned weekly buckets, the shape produced by a dataset that is
// pre-aggregated to weeks.
const WEEK_MS = 7 * 24 * 3600 * 1000;
const MONDAYS = Array.from(
{ length: 13 },
(_, i) => Date.UTC(2026, 3, 6) + i * WEEK_MS,
);
const weeklyChartProps = (
formDataOverrides: Partial<EchartsTimeseriesFormData> = {},
annotationData?: AnnotationData,
) =>
createTestChartProps({
annotationData,
formData: {
granularity_sqla: 'ds',
timeGrainSqla: TimeGranularity.WEEK_STARTING_MONDAY,
xAxisTimeFormat: '%m-%d',
...formDataOverrides,
},
queriesData: [
createTestQueryData(
MONDAYS.map((__timestamp, i) => ({ __timestamp, sales: 100 + i })),
{
colnames: ['__timestamp', 'sales'],
coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
// transformProps reads annotations off the query, not chartProps.
...(annotationData && { annotation_data: annotationData }),
},
),
],
});
test('pins ticks, labels and gridlines to the weekly buckets', () => {
const { xAxis } = transformProps(weeklyChartProps()).echartOptions as any;
expect(xAxis.type).toBe(AxisType.Time);
expect(xAxis.axisLabel.customValues).toEqual(MONDAYS);
// Gridlines follow axisTick.customValues, so splitLine needs no own copy.
expect(xAxis.axisTick.customValues).toEqual(MONDAYS);
expect(xAxis.splitLine).toBeUndefined();
});
test('caps axisLabel.customValues to the same subset as axisTick, not the full bucket set', () => {
// hideOverlap thins whichever set axisLabel.customValues offers it. If
// that set were the full (uncapped) bucket list while axisTick/splitLine
// only kept a downsampled subset, a surviving label could land on a
// bucket with no tick or gridline under it.
const manyMondays = Array.from(
{ length: 261 },
(_, i) => Date.UTC(2021, 0, 4) + i * WEEK_MS,
);
const chartProps = createTestChartProps({
formData: {
granularity_sqla: 'ds',
timeGrainSqla: TimeGranularity.WEEK_STARTING_MONDAY,
xAxisTimeFormat: '%m-%d',
},
queriesData: [
createTestQueryData(
manyMondays.map((__timestamp, i) => ({
__timestamp,
sales: 100 + i,
})),
{
colnames: ['__timestamp', 'sales'],
coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
},
),
],
});
const { xAxis } = transformProps(chartProps).echartOptions as any;
expect(xAxis.axisTick.customValues.length).toBeLessThan(manyMondays.length);
expect(xAxis.axisLabel.customValues).toEqual(xAxis.axisTick.customValues);
});
test('keeps the showMaxLabel override at 0° rotation on pinned axes', () => {
// hideOverlap stays on for pinned ticks (they label every bucket), but
// showMaxLabel still shields the boundary label's immediate neighbour
// so the last bucket isn't silently dropped (#39899).
const { xAxis } = transformProps(weeklyChartProps()).echartOptions as any;
expect(xAxis.axisLabel.showMaxLabel).toBe(true);
expect(xAxis.axisLabel.hideOverlap).toBe(true);
});
test('pins ticks when the bucket column holds ISO date strings', () => {
// A dataset can arrive with __timestamp serialized as an ISO string
// rather than a Date/epoch-ms value.
const chartProps = createTestChartProps({
formData: {
granularity_sqla: 'ds',
timeGrainSqla: TimeGranularity.WEEK_STARTING_MONDAY,
},
queriesData: [
createTestQueryData(
MONDAYS.map((__timestamp, i) => ({
__timestamp: new Date(__timestamp).toISOString(),
sales: 100 + i,
})),
{
colnames: ['__timestamp', 'sales'],
coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
},
),
],
});
const { xAxis } = transformProps(chartProps).echartOptions as any;
expect(xAxis.axisLabel.customValues).toEqual(MONDAYS);
});
test('keeps label thinning on when the labels are rotated', () => {
// Rotation normally turns hideOverlap off, but pinned ticks put a label on
// every bucket, so without thinning a multi-year range draws hundreds.
const { xAxis } = transformProps(
weeklyChartProps({ xAxisLabelRotation: 45 }),
).echartOptions as any;
expect(xAxis.axisLabel.customValues).toEqual(MONDAYS);
expect(xAxis.axisLabel.hideOverlap).toBe(true);
});
test('leaves rotation thinning alone when the ticks are not pinned', () => {
const { xAxis } = transformProps(
weeklyChartProps({
timeGrainSqla: TimeGranularity.MONTH,
xAxisLabelRotation: 45,
}),
).echartOptions as any;
expect(xAxis.axisLabel.customValues).toBeUndefined();
expect(xAxis.axisLabel.hideOverlap).toBe(false);
});
const timeseriesLayer = (show: boolean) =>
({
name: 'my annotation',
annotationType: AnnotationType.Timeseries,
sourceType: AnnotationSourceType.Line,
style: AnnotationStyle.Solid,
show,
value: 1,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
}) as any;
// The annotation's own timestamps run a year past the last bucket.
const annotationRecords = {
'my annotation': {
records: [
{ ds: MONDAYS[0], y: 1 },
{ ds: MONDAYS[12] + 52 * WEEK_MS, y: 2 },
],
},
};
test('does not pin ticks when a timeseries annotation widens the axis', () => {
// A Time axis takes no min/max, so it stretches to cover the annotation
// while ECharts clips pinned ticks to the extent — that span would be bare.
const { xAxis } = transformProps(
weeklyChartProps(
{ annotationLayers: [timeseriesLayer(true)] },
annotationRecords,
),
).echartOptions as any;
expect(xAxis.axisLabel.customValues).toBeUndefined();
expect(xAxis.axisTick?.customValues).toBeUndefined();
});
test('still pins ticks for a hidden timeseries annotation', () => {
const { xAxis } = transformProps(
weeklyChartProps(
{ annotationLayers: [timeseriesLayer(false)] },
annotationRecords,
),
).echartOptions as any;
expect(xAxis.axisLabel.customValues).toEqual(MONDAYS);
});
test.each([
TimeGranularity.WEEK,
TimeGranularity.WEEK_STARTING_SUNDAY,
TimeGranularity.WEEK_STARTING_MONDAY,
TimeGranularity.WEEK_ENDING_SATURDAY,
TimeGranularity.WEEK_ENDING_SUNDAY,
])('applies to the %s grain', grain => {
const { xAxis } = transformProps(weeklyChartProps({ timeGrainSqla: grain }))
.echartOptions as any;
expect(xAxis.axisLabel.customValues).toEqual(MONDAYS);
});
test('a dashboard time-grain override drives the alignment', () => {
const { xAxis } = transformProps(
weeklyChartProps({
timeGrainSqla: TimeGranularity.DAY,
extraFormData: { time_grain_sqla: TimeGranularity.WEEK },
}),
).echartOptions as any;
expect(xAxis.axisLabel.customValues).toEqual(MONDAYS);
});
test('deduplicates and sorts the bucket timestamps', () => {
// A grouped query repeats each bucket once per series, and the rows are
// not necessarily ordered.
const chartProps = createTestChartProps({
formData: {
granularity_sqla: 'ds',
timeGrainSqla: TimeGranularity.WEEK,
groupby: ['region'],
},
queriesData: [
createTestQueryData(
[
{ __timestamp: MONDAYS[1], region: 'b', sales: 2 },
{ __timestamp: MONDAYS[0], region: 'a', sales: 1 },
{ __timestamp: MONDAYS[1], region: 'a', sales: 3 },
{ __timestamp: MONDAYS[0], region: 'b', sales: 4 },
],
{
colnames: ['__timestamp', 'region', 'sales'],
coltypes: [
GenericDataType.Temporal,
GenericDataType.String,
GenericDataType.Numeric,
],
},
),
],
});
const { xAxis } = transformProps(chartProps).echartOptions as any;
expect(xAxis.axisLabel.customValues).toEqual([MONDAYS[0], MONDAYS[1]]);
});
test('leaves grains ECharts places correctly untouched', () => {
(
[
TimeGranularity.DAY,
TimeGranularity.MONTH,
TimeGranularity.QUARTER,
TimeGranularity.YEAR,
undefined,
] as const
).forEach(grain => {
const { xAxis } = transformProps(
weeklyChartProps({ timeGrainSqla: grain }),
).echartOptions as any;
expect(xAxis.axisLabel.customValues).toBeUndefined();
expect(xAxis.axisTick?.customValues).toBeUndefined();
});
});
test('leaves a categorical x-axis untouched', () => {
const { xAxis } = transformProps(
weeklyChartProps({ xAxisForceCategorical: true }),
).echartOptions as any;
expect(xAxis.type).toBe(AxisType.Category);
expect(xAxis.axisLabel.customValues).toBeUndefined();
});
});
describe('tooltip for metrics whose labels end in forecast suffixes', () => {
const marker = '<span style="background-color:#1f77b4;"></span>';
const seriesIds = ['ci__yhat', 'ci__yhat_lower', 'ci__yhat_upper'];
@@ -2766,76 +3036,3 @@ describe('tooltip for metrics whose labels end in forecast suffixes', () => {
expect(html).toContain('>ci<');
});
});
test('shows gridlines and axis ticks by default', () => {
const { echartOptions } = transformProps(createTestChartProps({}));
const xAxis = echartOptions.xAxis as any;
const yAxis = echartOptions.yAxis as any;
expect(yAxis.splitLine.show).toBe(true);
expect(yAxis.axisTick.show).toBe(true);
// Left to ECharts, which draws no ticks on a banded category axis. Forcing
// true would add ticks the chart does not have today.
expect(xAxis.axisTick.show).toBe('auto');
});
test('hides gridlines without touching the minor split lines', () => {
const { echartOptions } = transformProps(
createTestChartProps({ formData: { gridlines: false } }),
);
const yAxis = echartOptions.yAxis as any;
expect(yAxis.splitLine.show).toBe(false);
expect(yAxis.minorSplitLine.show).toBe(DEFAULT_FORM_DATA.minorSplitLine);
expect(yAxis.axisTick.show).toBe(true);
});
test('leaves the category axis split lines alone until gridlines are turned off', () => {
const shown = transformProps(createTestChartProps({}));
// Writing show:true here would draw gridlines on axis types that default to
// none, so the key is only ever added to hide them.
expect((shown.echartOptions.xAxis as any).splitLine).toBeUndefined();
const hidden = transformProps(
createTestChartProps({ formData: { gridlines: false } }),
);
expect((hidden.echartOptions.xAxis as any).splitLine.show).toBe(false);
});
test('hides the ticks on both axes', () => {
const { echartOptions } = transformProps(
createTestChartProps({ formData: { axisTicks: false } }),
);
expect((echartOptions.yAxis as any).axisTick.show).toBe(false);
expect((echartOptions.xAxis as any).axisTick.show).toBe(false);
expect((echartOptions.yAxis as any).splitLine.show).toBe(true);
});
test('keeps gridlines and ticks off on a compact chart even when both are enabled', () => {
const { echartOptions } = transformProps(
createTestChartProps({
height: TIMESERIES_CONSTANTS.compactChartHeight - 1,
formData: { gridlines: true, axisTicks: true },
}),
);
const yAxis = echartOptions.yAxis as any;
expect(yAxis.splitLine.show).toBe(false);
expect(yAxis.axisTick.show).toBe(false);
});
test('applies gridlines to the value axis after a horizontal orientation swaps it', () => {
const { echartOptions } = transformProps(
createTestChartProps({
formData: {
orientation: OrientationType.Horizontal,
gridlines: false,
},
}),
);
// The transform swaps the axes for a horizontal chart, so the value axis —
// and the gridlines belonging to it — end up on xAxis.
expect((echartOptions.xAxis as any).splitLine.show).toBe(false);
});
@@ -22,6 +22,7 @@ import {
DataRecord,
getNumberFormatter,
getTimeFormatter,
TimeGranularity,
} from '@superset-ui/core';
import { supersetTheme as theme } from '@apache-superset/core/theme';
import { GenericDataType } from '@apache-superset/core/common';
@@ -40,6 +41,8 @@ import {
getLegendProps,
getOverMaxHiddenFormatter,
getMinAndMaxFromBounds,
capTickMarks,
getTemporalTickValues,
sanitizeHtml,
sortAndFilterSeries,
sortRows,
@@ -1705,6 +1708,129 @@ test('getAxisType does not coerce Numeric x-axis to Time regardless of values',
);
});
describe('getTemporalTickValues', () => {
const xAxisLabel = '__timestamp';
test('returns undefined for a non-time axis', () => {
const data: DataRecord[] = [{ [xAxisLabel]: 1712361600000 }];
expect(
getTemporalTickValues(
data,
xAxisLabel,
AxisType.Category,
TimeGranularity.WEEK,
),
).toBeUndefined();
});
test('returns undefined when there is no time grain', () => {
const data: DataRecord[] = [{ [xAxisLabel]: 1712361600000 }];
expect(
getTemporalTickValues(data, xAxisLabel, AxisType.Time, undefined),
).toBeUndefined();
});
test('returns undefined for a non-weekly time grain', () => {
const data: DataRecord[] = [{ [xAxisLabel]: 1712361600000 }];
expect(
getTemporalTickValues(
data,
xAxisLabel,
AxisType.Time,
TimeGranularity.MONTH,
),
).toBeUndefined();
});
test('returns sorted, de-duplicated bucket timestamps for numbers and Dates', () => {
const t0 = Date.UTC(2026, 3, 6);
const t1 = Date.UTC(2026, 3, 13);
const data: DataRecord[] = [
{ [xAxisLabel]: t1 },
{ [xAxisLabel]: new Date(t0) },
{ [xAxisLabel]: t0 }, // duplicate of the Date row above
];
expect(
getTemporalTickValues(
data,
xAxisLabel,
AxisType.Time,
TimeGranularity.WEEK,
),
).toEqual([t0, t1]);
});
test('parses a zoned ISO string as the instant it names', () => {
const data: DataRecord[] = [{ [xAxisLabel]: '2026-04-06T00:00:00.000Z' }];
expect(
getTemporalTickValues(
data,
xAxisLabel,
AxisType.Time,
TimeGranularity.WEEK,
),
).toEqual([Date.UTC(2026, 3, 6)]);
});
test('parses a zone-less datetime string as local time, matching ECharts', () => {
const data: DataRecord[] = [{ [xAxisLabel]: '2026-04-06T00:00:00' }];
expect(
getTemporalTickValues(
data,
xAxisLabel,
AxisType.Time,
TimeGranularity.WEEK,
),
).toEqual([new Date(2026, 3, 6, 0, 0, 0).getTime()]);
});
test('parses a bare date string as local midnight, matching ECharts rather than native Date', () => {
// `new Date('2026-04-06')` is UTC, but ECharts parses it as local time.
// jest.config.js fixes the test TZ to America/New_York, so they disagree.
const data: DataRecord[] = [{ [xAxisLabel]: '2026-04-06' }];
const localMidnight = new Date(2026, 3, 6).getTime();
expect(localMidnight).not.toEqual(new Date('2026-04-06').getTime());
expect(
getTemporalTickValues(
data,
xAxisLabel,
AxisType.Time,
TimeGranularity.WEEK,
),
).toEqual([localMidnight]);
});
test('drops unparseable or nullish values and returns undefined when none remain', () => {
const data: DataRecord[] = [
{ [xAxisLabel]: 'not-a-date' },
{ [xAxisLabel]: null },
];
expect(
getTemporalTickValues(
data,
xAxisLabel,
AxisType.Time,
TimeGranularity.WEEK,
),
).toBeUndefined();
});
});
describe('capTickMarks', () => {
test('returns values unchanged when within the cap', () => {
const values = [1, 2, 3];
expect(capTickMarks(values, 60)).toEqual(values);
});
test('downsamples evenly and always keeps the last value', () => {
const values = Array.from({ length: 261 }, (_, i) => i);
const capped = capTickMarks(values, 60);
expect(capped.length).toBeLessThanOrEqual(60);
expect(capped[0]).toEqual(0);
expect(capped[capped.length - 1]).toEqual(260);
});
});
test('getMinAndMaxFromBounds returns empty object when not truncating', () => {
expect(
getMinAndMaxFromBounds(
@@ -312,7 +312,7 @@ export function handleComponentDrop(dropResult: DropResult) {
source &&
!(
// ensure it has moved
destination.id === source.id && destination.index === source.index
(destination.id === source.id && destination.index === source.index)
)
) {
dispatch(moveComponent(dropResult));
+1 -1
View File
@@ -126,7 +126,7 @@ function fillNativeFilters(
!(
// Treat all-null arrays (range filters use [null, null] as their
// canonical cleared value) and empty arrays as "no value".
Array.isArray(loadedValue) && loadedValue.every(v => v === null)
(Array.isArray(loadedValue) && loadedValue.every(v => v === null))
);
const loadedHasExtraFormData =
!!loaded?.extraFormData && Object.keys(loaded.extraFormData).length > 0;
@@ -29,9 +29,7 @@ import { ControlFormItemComponents } from './ControlForm';
* Column formatting configs.
*/
export type ColumnConfig = {
[
key in SharedColumnConfigProp
]?: (typeof SHARED_COLUMN_CONFIG_PROPS)[key]['value'];
[key in SharedColumnConfigProp]?: (typeof SHARED_COLUMN_CONFIG_PROPS)[key]['value'];
} & Record<string, StrictJsonValue>;
/**
@@ -951,167 +951,3 @@ test('filters the subject select by column verbose_name as well as column_name',
expect(within(dropdown).getByText('total_count')).toBeInTheDocument();
expect(within(dropdown).queryByText('Full Name')).not.toBeInTheDocument();
});
const COLUMN_VALUES_ENDPOINT =
'glob:*/api/v1/datasource/*/column/value/values/*';
let columnValues: { result: unknown[]; limit: number } = {
result: [],
limit: 10000,
};
fetchMock.get(COLUMN_VALUES_ENDPOINT, () => columnValues);
const setupWithFilterValues = (result: unknown[], limit = 10000) => {
columnValues = { result, limit };
const onChange = jest.fn();
const validHandler = jest.fn();
const spy = jest.spyOn(redux, 'useSelector');
spy.mockReturnValue({});
const props = {
adhocFilter: new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: 'value',
operatorId: Operators.In,
operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.In].operation,
comparator: [],
clause: Clauses.Where,
}),
onChange,
options,
datasource: {
...TestDataset,
columns: [{ column_name: 'value', type: 'VARCHAR', id: 3 }],
filter_select: true,
},
partitionColumn: 'test',
validHandler,
};
render(
<AdhocFilterEditPopoverSimpleTabContent {...(props as unknown as Props)} />,
);
return props;
};
const openComparator = async () => {
const comparator = screen.getByRole('combobox', {
name: 'Comparator option',
});
userEvent.click(comparator);
return comparator;
};
test('loads comparator values from the server', async () => {
setupWithFilterValues(['alpha', 'beta']);
await openComparator();
expect(await screen.findByTitle('alpha')).toBeInTheDocument();
});
test('sends the typed text to the server rather than filtering the loaded page', async () => {
// The loaded page is bounded, so matching client-side cannot reach a value
// beyond the row limit. The search has to reach the database.
setupWithFilterValues(['alpha']);
const comparator = await openComparator();
userEvent.type(comparator, 'gamma');
await waitFor(
() => {
const searched = fetchMock.callHistory
.calls(COLUMN_VALUES_ENDPOINT)
.map(call => String(call.url));
expect(searched.some(url => url.includes('q=gamma'))).toBe(true);
},
{ timeout: 3000 },
);
});
test('lets a value the server did not return still be selected', async () => {
// Even with server-side search a match can fall outside the page; typing the
// exact value has to remain a way through.
setupWithFilterValues([]);
const comparator = await openComparator();
userEvent.type(comparator, 'not-in-the-page');
expect(await screen.findByTitle('not-in-the-page')).toBeInTheDocument();
});
test('does not query for values when the dataset disables them', async () => {
fetchMock.clearHistory();
setup({
adhocFilter: new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: 'value',
operatorId: Operators.In,
operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.In].operation,
comparator: [],
clause: Clauses.Where,
}),
});
await openComparator();
expect(fetchMock.callHistory.calls(COLUMN_VALUES_ENDPOINT)).toHaveLength(0);
});
test('stores the picked value, not the option object', async () => {
// AsyncSelect is labelInValue: taking its argument at face value puts
// {label, value} into the comparator, and the engine then fails to render it
// as a literal.
const props = setupWithFilterValues(['Michael']);
await openComparator();
userEvent.click(await screen.findByTitle('Michael'));
await waitFor(() => expect(props.onChange).toHaveBeenCalled());
const [filter] = props.onChange.mock.calls.at(-1);
expect(filter.comparator).toEqual(['Michael']);
});
test('can remove a value that was saved earlier', async () => {
// Reopening the popover restores the comparator from the saved filter, and
// the value is not in the freshly loaded page. Removing it has to still work.
columnValues = { result: [], limit: 10000 };
const onChange = jest.fn();
const validHandler = jest.fn();
jest.spyOn(redux, 'useSelector').mockReturnValue({});
render(
<AdhocFilterEditPopoverSimpleTabContent
{...({
adhocFilter: new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: 'value',
operatorId: Operators.In,
operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.In].operation,
comparator: ['Michael'],
clause: Clauses.Where,
}),
onChange,
options,
datasource: {
...TestDataset,
columns: [{ column_name: 'value', type: 'VARCHAR', id: 3 }],
filter_select: true,
},
partitionColumn: 'test',
validHandler,
} as unknown as Props)}
/>,
);
// Remove it the way a user does: the tag's own close control.
userEvent.click(await screen.findByLabelText('close'));
await waitFor(() => expect(onChange).toHaveBeenCalled());
const [filter] = onChange.mock.calls.at(-1);
expect(filter.comparator).toEqual([]);
});
test('says the list is partial when the server capped it', async () => {
setupWithFilterValues(['alpha', 'beta'], 2);
await openComparator();
expect(
await screen.findByText(/Only the first 2 values are listed/),
).toBeInTheDocument();
});
test('does not say the list is partial when it is complete', async () => {
setupWithFilterValues(['alpha', 'beta'], 10000);
await openComparator();
expect(await screen.findByTitle('alpha')).toBeInTheDocument();
expect(screen.queryByText(/Only the first/)).not.toBeInTheDocument();
});
@@ -16,25 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
import {
FC,
ChangeEvent,
useCallback,
useEffect,
useMemo,
useState,
useRef,
} from 'react';
import { FC, ChangeEvent, useEffect, useState, useRef } from 'react';
import {
AsyncSelect,
Input,
InputRef,
Select,
Tooltip,
type AsyncSelectRef,
type LabeledValue,
type SelectOptionsTypePage,
type SelectValue,
} from '@superset-ui/core/components';
import { t } from '@apache-superset/core/translation';
@@ -69,7 +57,7 @@ import { useDatePickerInAdhocFilter } from '../utils';
import { useDefaultTimeFilter } from '../../DateFilterControl/utils';
import { Clauses, ExpressionTypes } from '../types';
const SelectWithLabel = styled(AsyncSelect)<{ labelText: string }>`
const SelectWithLabel = styled(Select)<{ labelText: string }>`
.ant-select-content::after {
content: ${({ labelText }) => labelText || '\\A0'};
display: inline-block;
@@ -79,30 +67,6 @@ const SelectWithLabel = styled(AsyncSelect)<{ labelText: string }>`
}
`;
// The server answers with one bounded page, not an offset window: paging would
// need a stable ORDER BY, and ordering a high-cardinality column is the full
// scan this search exists to avoid. A page size no response can reach keeps
// AsyncSelect from asking for a second page.
const COMPARATOR_PAGE_SIZE = 1_000_000;
const toLabeledValue = (value: unknown): LabeledValue => ({
value: value as LabeledValue['value'],
label: optionLabel(value as null | number | boolean | string),
});
// The reverse of toLabeledValue: what AsyncSelect emits is labelled, and the
// comparator has to be the raw value or the engine cannot render it as a
// literal.
const unwrapComparator = (value: unknown): unknown => {
if (Array.isArray(value)) {
return value.map(unwrapComparator);
}
if (value !== null && typeof value === 'object' && 'value' in value) {
return (value as LabeledValue).value;
}
return value;
};
export interface SimpleExpressionType {
expressionType: keyof typeof ExpressionTypes;
column: ColumnMeta;
@@ -383,9 +347,11 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
} = useSimpleTabFilterProps(props);
const [comparator, setComparator] = useState(props.adhocFilter.comparator);
const comparatorInputRef = useRef<InputRef | null>(null);
const comparatorSelectRef = useRef<AsyncSelectRef>(null);
const [loadedOptionCount, setLoadedOptionCount] = useState(0);
const [optionsTruncated, setOptionsTruncated] = useState(false);
const [suggestions, setSuggestions] = useState<
Record<'label' | 'value', any>[]
>([]);
const [loadingComparatorSuggestions, setLoadingComparatorSuggestions] =
useState<boolean>(false);
const [hasFocusedComparator, setHasFocusedComparator] =
useState<boolean>(false);
@@ -421,8 +387,18 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
/>
);
const createSuggestionsPlaceholder = () =>
loadedOptionCount ? t('%s option(s)', loadedOptionCount) : '';
const getOptionsRemaining = () => {
// if select is multi/value is array, we show the options not selected
const valuesFromSuggestionsLength = Array.isArray(comparator)
? comparator.filter(v => suggestions.includes(v)).length
: 0;
return suggestions ? suggestions.length - valuesFromSuggestionsLength : 0;
};
const createSuggestionsPlaceholder = () => {
const optionsRemaining = getOptionsRemaining();
const placeholder = t('%s option(s)', optionsRemaining);
return optionsRemaining ? placeholder : '';
};
const handleSubjectChange = (subject: string) => {
setComparator(undefined);
@@ -479,63 +455,21 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
operatorId !== undefined &&
DISABLE_INPUT_OPERATORS.includes(operatorId as Operators);
const canSuggestComparatorValues = Boolean(
subjectString &&
props.datasource?.filter_select &&
props.adhocFilter.clause !== Clauses.Having,
);
const hasComparatorOptions =
(operatorId && MULTI_OPERATORS.has(operatorId as Operators)) ||
canSuggestComparatorValues;
// AsyncSelect is labelInValue, so the value it is given has to be labelled
// too. Handed a bare value it still renders, but `handleOnDeselect` then
// compares `element.value` against entries that have no `.value`, matches
// nothing, and the tag cannot be removed.
//
// Memoised because AsyncSelect resets its internal selection whenever the
// identity of `value` changes. A fresh array every render would wipe out
// each pick as soon as it was made.
const comparatorSelectValue = useMemo(
() =>
Array.isArray(comparator)
? comparator.map(toLabeledValue)
: isDefined(comparator) && comparator !== ''
? toLabeledValue(comparator)
: undefined,
[comparator],
);
const handleComparatorChange = useCallback(
(value: unknown) => {
onComparatorChange(unwrapComparator(value) as string);
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[props.adhocFilter, props.onChange],
);
suggestions.length > 0;
const comparatorSelectProps = {
allowClear: true,
allowNewOptions: true,
ariaLabel: t('Comparator option'),
pageSize: COMPARATOR_PAGE_SIZE,
// A capped list reads as the whole set unless it says otherwise, so an
// absent value looks like a value that does not exist. Only shown when the
// list is actually cut short.
helperText: optionsTruncated
? t(
'Only the first %s values are listed. Type to search all of them, ' +
'or enter a value that is not listed.',
loadedOptionCount,
)
: undefined,
mode:
operatorId && MULTI_OPERATORS.has(operatorId as Operators)
? ('multiple' as const)
: ('single' as const),
value: comparatorSelectValue as SelectValue,
onChange: handleComparatorChange,
loading: loadingComparatorSuggestions,
value: comparator as SelectValue,
onChange: onComparatorChange,
notFoundContent: t('Type a value here'),
placeholder: createSuggestionsPlaceholder(),
};
@@ -561,89 +495,76 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
onChange: onDatePickerChange,
});
// Element-level array operators (Contains any / Contains all) search inside
// the array, so suggest individual elements; whole-array operators (=, In, …)
// keep the default distinct-array suggestions.
const arrayElements =
props.adhocFilter.operatorId === Operators.ContainsAny ||
props.adhocFilter.operatorId === Operators.ContainsAll;
// AsyncSelect throws away every loaded option when the identity of its
// `options` callback changes, so this depends on plain values rather than on
// `props.datasource`, whose identity the parent does not guarantee.
const datasourceType = props.datasource?.type;
const datasourceId = props.datasource?.id;
const loadComparatorOptions = useCallback(
async (search: string): Promise<SelectOptionsTypePage> => {
const col = subjectString;
if (!col || !canSuggestComparatorValues) {
return { data: [], totalCount: 0 };
}
const params = new URLSearchParams();
if (arrayElements) {
params.set('array_elements', 'true');
}
if (search) {
params.set('q', search);
}
const query = params.toString();
try {
const { json } = await SupersetClient.get({
endpoint:
`/api/v1/datasource/${datasourceType}/${datasourceId}` +
`/column/${encodeURIComponent(col)}/values/${query ? `?${query}` : ''}`,
});
const data = json.result.map((suggestion: unknown) => {
// Complex column values arrive as JS arrays or objects: whole arrays
// for MULTI_VALUE columns (e.g. [5, 6, 7]) and Map/Tuple objects for
// nested-container columns (e.g. {"a": ["x","y"]}). A raw
// array/object is neither a valid single-select value (antd collapses
// an array to its first element) nor renderable as a React child (an
// object throws). Render it as its literal string, which is also
// exactly what the backend's parse_array_literal expects for the
// whole-array operators.
if (suggestion !== null && typeof suggestion === 'object') {
const literal = JSON.stringify(suggestion);
return { value: literal, label: literal };
}
return {
value: suggestion as null | number | boolean | string,
label: optionLabel(suggestion as null | number | boolean | string),
};
});
setLoadedOptionCount(data.length);
setOptionsTruncated(isDefined(json.limit) && data.length >= json.limit);
// The count has to exceed what was returned. AsyncSelect treats
// `loaded >= totalCount` as "that is every value", sets allValuesLoaded
// and from then on serves searches by filtering the loaded page
// client-side -- which is the behaviour this whole change exists to
// replace. Pagination is held off by COMPARATOR_PAGE_SIZE instead.
return { data, totalCount: data.length + 1 };
} catch {
setLoadedOptionCount(0);
setOptionsTruncated(false);
return { data: [], totalCount: 0 };
}
},
[
subjectString,
canSuggestComparatorValues,
datasourceType,
datasourceId,
arrayElements,
],
);
// Options are cached per search term inside AsyncSelect; a different column
// or a switch to element-level suggestions invalidates all of them.
useEffect(() => {
comparatorSelectRef.current?.clearCache();
}, [subjectString, arrayElements]);
const refreshComparatorSuggestions = () => {
const { datasource } = props;
const col = props.adhocFilter.subject;
const having = props.adhocFilter.clause === Clauses.Having;
if (col && datasource && datasource.filter_select && !having) {
const controller = new AbortController();
const { signal } = controller;
if (loadingComparatorSuggestions) {
controller.abort();
}
// Element-level array operators (Contains any / Contains all) search
// inside the array, so suggest individual elements; whole-array
// operators (=, In, …) keep the default distinct-array suggestions.
const { operatorId } = props.adhocFilter;
const arrayElements =
operatorId === Operators.ContainsAny ||
operatorId === Operators.ContainsAll;
setLoadingComparatorSuggestions(true);
SupersetClient.get({
signal,
endpoint: `/api/v1/datasource/${datasource.type}/${datasource.id}/column/${col}/values/${
arrayElements ? '?array_elements=true' : ''
}`,
})
.then(({ json }) => {
setSuggestions(
json.result.map((suggestion: unknown) => {
// Complex column values arrive as JS arrays or objects: whole
// arrays for MULTI_VALUE columns (e.g. [5, 6, 7]) and Map/Tuple
// objects for nested-container columns (e.g. {"a": ["x","y"]}).
// A raw array/object is neither a valid single-select value
// (antd collapses an array to its first element) nor renderable
// as a React child (an object throws). Render it as its literal
// string, which is also exactly what the backend's
// parse_array_literal expects for the whole-array operators.
if (suggestion !== null && typeof suggestion === 'object') {
const literal = JSON.stringify(suggestion);
return { value: literal, label: literal };
}
return {
value: suggestion as null | number | boolean | string,
label: optionLabel(
suggestion as null | number | boolean | string,
),
};
}),
);
setLoadingComparatorSuggestions(false);
})
.catch(() => {
setSuggestions([]);
setLoadingComparatorSuggestions(false);
});
}
};
if (!datePicker) {
refreshComparatorSuggestions();
}
// loadingComparatorSuggestions intentionally omitted - set inside effect, would cause infinite loop
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [
props.adhocFilter.subject,
props.adhocFilter.clause,
props.adhocFilter.operatorId,
props.datasource,
datePicker,
]);
useEffect(() => {
if (isFeatureEnabled(FeatureFlag.EnableAdvancedDataTypes)) {
@@ -749,12 +670,11 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
}
>
<SelectWithLabel
ref={comparatorSelectRef}
css={css`
margin-top: ${theme.marginXS}px;
`}
labelText={labelText}
options={loadComparatorOptions}
options={suggestions}
{...comparatorSelectProps}
/>
</Tooltip>
@@ -879,41 +879,10 @@ describe('SelectFilterPlugin', () => {
expect(await screen.findByTitle('brand-new')).toBeInTheDocument();
});
test('says the list is capped when it hits the row limit', async () => {
// 3 rows of data against a limit of 3: the user is looking at a page, not
// at every value the column has.
getWrapper({ rowLimit: 3 });
userEvent.click(screen.getAllByRole('combobox')[0]);
expect(
await screen.findByText(/Only the first 3 values are listed/),
).toBeInTheDocument();
});
test('offers the ways out that the filter actually supports', async () => {
getWrapper({ rowLimit: 3, creatable: true, searchAllOptions: true });
userEvent.click(screen.getAllByRole('combobox')[0]);
expect(
await screen.findByText(/Type to search all of them/),
).toBeInTheDocument();
expect(
screen.getByText(/You can enter a value that is not listed/),
).toBeInTheDocument();
});
test('says nothing when the whole column fits under the limit', async () => {
getWrapper();
userEvent.click(screen.getAllByRole('combobox')[0]);
expect(await screen.findByRole('combobox')).toBeInTheDocument();
expect(screen.queryByText(/Only the first/)).not.toBeInTheDocument();
});
test('shows create option when searchAllOptions is true', async () => {
// Server-side search returns a bounded page, so a value that exists in the
// data can still be missing from the dropdown. Suppressing the create
// option there leaves the user with no way to apply it at all.
test('does not show create option when searchAllOptions is true', () => {
getWrapper({ creatable: true, searchAllOptions: true });
userEvent.type(screen.getByRole('combobox'), 'brand-new');
expect(await screen.findByTitle('brand-new')).toBeInTheDocument();
expect(screen.queryByTitle('brand-new')).not.toBeInTheDocument();
});
});
@@ -271,10 +271,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
type: 'ownState',
ownState: {
coltypeMap: initialColtypeMap,
// The dropdown offers `stripSurroundingQuotes(search)` as the
// creatable option, so the server has to be asked for the same
// string or the two disagree about what was searched for.
search: stripSurroundingQuotes(search).trim(),
search,
},
});
}
@@ -284,10 +281,8 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
const handleBlur = useCallback(() => {
unsetFocusedFilter();
if (search) {
onSearch('');
}
}, [onSearch, search, unsetFocusedFilter]);
onSearch('');
}, [onSearch, unsetFocusedFilter]);
const handleChange = useCallback(
(value?: SelectValue | number | string) => {
@@ -309,25 +304,6 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
? t('No data')
: tn('%s option', '%s options', data.length, data.length);
// A capped list reads as the whole set, so a value sitting past the row
// limit looks like a value that does not exist. Each sentence is only added
// when it is actually true of this filter's configuration.
const rowLimit = Number(formData.rowLimit) || 0;
const helperText = useMemo(() => {
if (!rowLimit || data.length < rowLimit) {
return undefined;
}
return [
t('Only the first %s values are listed.', data.length),
searchAllOptions ? t('Type to search all of them.') : undefined,
creatable !== false
? t('You can enter a value that is not listed.')
: undefined,
]
.filter(Boolean)
.join(' ');
}, [creatable, data.length, rowLimit, searchAllOptions]);
const formItemExtra = useMemo(() => {
if (filterState.validateMessage) {
return (
@@ -360,6 +336,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
const unquotedSearch = stripSurroundingQuotes(search);
if (
unquotedSearch &&
!searchAllOptions &&
creatable !== false &&
!hasOption(unquotedSearch, uniqueOptions, true)
) {
@@ -369,7 +346,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
];
}
return uniqueOptions;
}, [search, uniqueOptions, creatable]);
}, [search, uniqueOptions, creatable, searchAllOptions]);
const sortComparator = useCallback(
(a: LabeledValue, b: LabeledValue) => {
@@ -640,7 +617,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
name={formData.nativeFilterId}
allowClear
autoClearSearchValue
allowNewOptions={creatable !== false}
allowNewOptions={!searchAllOptions && creatable !== false}
allowNewOptionsOnPaste={multiSelect && searchAllOptions}
allowSelectAll={!searchAllOptions}
value={multiSelect ? filterState.value || [] : filterState.value}
@@ -649,7 +626,6 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
showSearch={showSearch}
mode={multiSelect ? 'multiple' : 'single'}
placeholder={placeholderText}
helperText={helperText}
onClear={() => onSearch('')}
onSearch={onSearch}
onBlur={handleBlur}
@@ -117,38 +117,6 @@ describe('Select buildQuery', () => {
]);
});
test('should not sort by the searched column', () => {
// Ordering by a high-cardinality column makes the engine sort every match
// before applying the row limit; the dropdown re-sorts the page anyway.
const queryContext = buildQuery(
{ ...formData, sortAscending: true },
{
ownState: {
search: 'abc',
coltypeMap: { my_col: GenericDataType.String },
},
},
);
const [query] = queryContext.queries;
expect(query.orderby).toEqual([]);
});
test('should keep the sort metric while searching', () => {
// A sort metric decides which rows come back, so dropping it would change
// the result set rather than just its order.
const queryContext = buildQuery(
{ ...formData, sortMetric: 'my_metric', sortAscending: false },
{
ownState: {
search: 'abc',
coltypeMap: { my_col: GenericDataType.String },
},
},
);
const [query] = queryContext.queries;
expect(query.orderby).toEqual([['my_metric', false]]);
});
test('should add text search parameter for numeric to query filter', () => {
const queryContext = buildQuery(formData, {
ownState: {
@@ -54,13 +54,6 @@ const buildQuery: BuildQuery<PluginFilterSelectQueryFormData> = (
}
const sortColumns = sortMetric ? [sortMetric] : columns;
// Sorting by the searched column makes the engine scan and sort every
// match before applying the row limit, which is the dominant cost of
// search-as-you-type on a high-cardinality column. The dropdown re-sorts
// the returned page client-side, so the server sort buys nothing here. A
// sort metric is different: it selects *which* rows come back, so it has
// to stay.
const skipOrderBy = !!search && !sortMetric;
const query: QueryObject[] = [
{
...baseQueryObject,
@@ -68,7 +61,7 @@ const buildQuery: BuildQuery<PluginFilterSelectQueryFormData> = (
metrics: sortMetric ? [sortMetric] : [],
filters: filters.concat(extraFilters),
orderby:
!skipOrderBy && (sortMetric || sortAscending !== undefined)
sortMetric || sortAscending !== undefined
? sortColumns.map(column => [column, !!sortAscending])
: [],
},
-1
View File
@@ -1 +0,0 @@
../superset-frontend/.npmrc
+1 -1
View File
@@ -160,7 +160,7 @@ def migrate_by_id(ids: tuple[int, ...], is_downgrade: bool = False) -> None:
"""
Migrate a subset of charts by IDs.
:param ids: Tuple of chart IDs to migrate
:param id: Tuple of chart IDs to migrate
:param is_downgrade: Whether to downgrade the charts. Default is upgrade.
"""
slices = db.session.query(Slice).filter(Slice.id.in_(ids))
-1
View File
@@ -964,7 +964,6 @@ class AnnotationDatasource(BaseDatasource):
limit: int = 10000,
denormalize_column: bool = False,
array_elements: bool = False,
search: str | None = None,
) -> list[Any]:
raise NotImplementedError()
-12
View File
@@ -278,18 +278,6 @@ class DatasetRestApi(SoftDeleteApiMixin, BaseSupersetModelRestApi):
"database.backend",
"database.allow_multi_catalog",
"columns.advanced_data_type",
# Certification/warning metadata is stored serialized in the ``extra``
# column and surfaced through model properties. Exposing them keeps this
# payload consistent with the datasource serialization used by Explore,
# so clients hydrating from this endpoint don't lose the badges.
"columns.certification_details",
"columns.certified_by",
"columns.is_certified",
"columns.warning_markdown",
"metrics.certification_details",
"metrics.certified_by",
"metrics.is_certified",
"metrics.warning_markdown",
"is_managed_externally",
"uid",
"uuid",
+2 -23
View File
@@ -41,9 +41,6 @@ from superset.views.base_api import BaseSupersetApi, statsd_metrics
logger = logging.getLogger(__name__)
# Cache lifetime for search-filtered column values, in seconds.
SEARCH_CACHE_TIMEOUT = 60
class DatasourceRestApi(BaseSupersetApi):
allow_browser_login = True
@@ -90,14 +87,6 @@ class DatasourceRestApi(BaseSupersetApi):
type: string
name: column_name
description: The name of the column to get values for
- in: query
schema:
type: string
name: q
description: >-
Optional case-insensitive substring; only values containing it are
returned. Lets the client search the full column rather than the
truncated first page.
responses:
200:
description: A List of distinct values for the column
@@ -147,10 +136,6 @@ class DatasourceRestApi(BaseSupersetApi):
# Element-level operators (Contains any / Contains all) request the
# distinct array *elements* rather than distinct whole arrays.
array_elements = parse_boolean_string(request.args.get("array_elements"))
# Server-side search. Without it the client can only match against the
# bounded first page, so a value beyond ``FILTER_SELECT_ROW_LIMIT`` is
# unfindable on a high-cardinality column.
search = (request.args.get("q") or "").strip() or None
# Cache distinct column-value results so a dashboard with many filters
# backed by the same (often heavy) virtual dataset doesn't re-execute
@@ -184,7 +169,6 @@ class DatasourceRestApi(BaseSupersetApi):
"limit": row_limit,
"denorm": denormalize_column,
"elements": array_elements,
"q": search,
"rls": security_manager.get_rls_cache_key(datasource),
"changed_on": str(getattr(datasource, "changed_on", "")),
},
@@ -200,7 +184,7 @@ class DatasourceRestApi(BaseSupersetApi):
logger.debug(
"column-values cache HIT: uid=%s col=%s", datasource.uid, column_name
)
response = self.response(200, result=cached, limit=row_limit)
response = self.response(200, result=cached)
response.headers["X-Cache-Status"] = "HIT"
return response
@@ -210,7 +194,6 @@ class DatasourceRestApi(BaseSupersetApi):
limit=row_limit,
denormalize_column=denormalize_column,
array_elements=array_elements,
search=search,
)
except KeyError:
return self.response(
@@ -242,15 +225,11 @@ class DatasourceRestApi(BaseSupersetApi):
timeout = datasource.cache_timeout or app.config.get(
"CACHE_DEFAULT_TIMEOUT", 300
)
if search:
# Every distinct search term is its own key, so a few users typing
# would otherwise pin one entry per keystroke for the full timeout.
timeout = min(timeout, SEARCH_CACHE_TIMEOUT)
cache_manager.data_cache.set(cache_key, payload, timeout=timeout)
logger.debug(
"column-values cache MISS: uid=%s col=%s", datasource.uid, column_name
)
response = self.response(200, result=payload, limit=row_limit)
response = self.response(200, result=payload)
response.headers["X-Cache-Status"] = "MISS"
return response
+3 -3
View File
@@ -45,9 +45,9 @@ def redefine(
Redefine the foreign key constraint to include the ON DELETE and ON UPDATE
constructs for cascading purposes.
:param foreign_key: The foreign key constraint
:param on_delete: If set, emit ON DELETE <value> when issuing DDL operations
:param on_update: If set, emit ON UPDATE <value> when issuing DDL operations
:params foreign_key: The foreign key constraint
:param ondelete: If set, emit ON DELETE <value> when issuing DDL operations
:param onupdate: If set, emit ON UPDATE <value> when issuing DDL operations
"""
bind = op.get_bind()
+4 -55
View File
@@ -198,41 +198,6 @@ def get_effective_hours_offset(
R_SUFFIX = "__right_suffix"
# Escape character for LIKE patterns built from user-supplied search text.
# Deliberately not a backslash: dialects that escape backslashes when rendering
# string literals would emit a two-character ESCAPE clause, which is a syntax
# error on engines that honour standard-conforming strings.
LIKE_ESCAPE_CHAR = "!"
def escape_like_pattern(value: str) -> str:
"""
Neutralize LIKE wildcards in user-supplied search text.
Without this a user typing ``%`` or ``_`` would match every row, which is
both wrong and, on a large table, a scan the search was meant to avoid.
"""
return (
value.replace(LIKE_ESCAPE_CHAR, LIKE_ESCAPE_CHAR * 2)
.replace("%", f"{LIKE_ESCAPE_CHAR}%")
.replace("_", f"{LIKE_ESCAPE_CHAR}_")
)
def build_like_predicate(
expr: ColumnElement[Any],
search: str,
) -> ColumnElement[Any]:
"""
Build a case-insensitive containment predicate for ``expr``.
``lower(expr) LIKE lower('%term%')`` is used rather than ``ILIKE`` because
the latter is not portable across engines.
"""
pattern = f"%{escape_like_pattern(search)}%".lower()
return sa.func.lower(expr).like(pattern, escape=LIKE_ESCAPE_CHAR)
def _normalize_mssql_virtual_dataset_sql(
sql: str, parsed_script: SQLScript, engine: str
) -> str:
@@ -1646,28 +1611,16 @@ class ExtraJSONMixin:
return value
_EXTRA_DICT_CACHE_UNSET = object()
class CertificationMixin:
"""Mixin to add extra certification fields"""
extra = sa.Column(sa.Text, default="{}")
def get_extra_dict(self) -> dict[str, Any]:
# Cache the parsed ``extra`` payload on the instance, keyed by the raw
# string it was parsed from, so callers reading multiple
# certification/warning properties off the same object don't each
# trigger their own ``json.loads``. The cache is transient (not a
# mapped column) and self-invalidates whenever ``extra`` changes.
cache_raw = getattr(self, "_extra_dict_cache_raw", _EXTRA_DICT_CACHE_UNSET)
if cache_raw is _EXTRA_DICT_CACHE_UNSET or cache_raw != self.extra:
try:
self._extra_dict_cache = json.loads(self.extra)
except (TypeError, json.JSONDecodeError):
self._extra_dict_cache = {}
self._extra_dict_cache_raw = self.extra
return self._extra_dict_cache
try:
return json.loads(self.extra)
except (TypeError, json.JSONDecodeError):
return {}
@property
def is_certified(self) -> bool:
@@ -4057,7 +4010,6 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
limit: int = 10000,
denormalize_column: bool = False,
array_elements: bool = False,
search: str | None = None,
) -> list[Any]:
# denormalize column name before querying for values
# unless disabled in the dataset configuration
@@ -4095,9 +4047,6 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
.select_from(tbl)
.distinct()
)
if search:
qry = qry.where(build_like_predicate(value_expr, search))
if limit:
qry = qry.limit(limit)
+1 -1
View File
@@ -404,7 +404,7 @@ class SemanticView(AuditMixinNullable, Model):
for dimension in dimensions
},
}
column_formats: dict[str, str | None] = {
column_formats = {
metric.name: metric.d3format for metric in metrics if metric.d3format
}
+1 -19
View File
@@ -1539,25 +1539,7 @@ class SQLStatement(BaseSQLStatement[exp.Expression]):
:return: The parsed predicate.
"""
_check_script_length(predicate, self.engine)
try:
return sqlglot.parse_one(predicate, dialect=self._dialect)
except sqlglot.errors.ParseError as ex:
kwargs = (
{
"highlight": ex.errors[0]["highlight"],
"line": ex.errors[0]["line"],
"column": ex.errors[0]["col"],
}
if ex.errors
else {}
)
raise SupersetParseError(predicate, self.engine, **kwargs) from ex
except sqlglot.errors.SqlglotError as ex:
raise SupersetParseError(
predicate,
self.engine,
message="Unable to parse predicate",
) from ex
return sqlglot.parse_one(predicate, dialect=self._dialect)
def apply_rls(
self,
+1 -1
View File
@@ -333,7 +333,7 @@ class BaseScreenshot:
Computes the thumbnail and caches the result
:param user: If no user is given will use the current context
:param cache_key: The cache key to store the thumbnail payload under
:param cache: The cache to keep the thumbnail payload
:param window_size: The window size from which will process the thumb
:param thumb_size: The final thumbnail size
:param force: Will force the computation even if it's already cached
+18 -26
View File
@@ -20,7 +20,6 @@ import copy
import unittest
from datetime import timedelta
from io import BytesIO
from typing import Any
from unittest.mock import ANY, patch
from zipfile import is_zipfile, ZipFile
@@ -73,27 +72,6 @@ from tests.integration_tests.fixtures.importexport import (
dataset_ui_export,
)
# Fields the dataset ``show`` payload exposes but the ``PUT`` schema doesn't
# accept: audit timestamps plus attributes derived from the model (type
# affinity and the certification/warning metadata stored in ``extra``).
DATASET_READ_ONLY_ITEM_FIELDS = (
"changed_on",
"created_on",
"type_generic",
"certification_details",
"certified_by",
"is_certified",
"warning_markdown",
)
def strip_read_only_fields(items: list[dict[str, Any]]) -> list[dict[str, Any]]:
"""Drop read-only fields so a ``show`` payload can be fed back to ``PUT``."""
for item in items:
for field in DATASET_READ_ONLY_ITEM_FIELDS:
item.pop(field, None)
return items
class TestDatasetApi(SupersetTestCase):
fixture_tables_names = ("ab_permission", "ab_permission_view", "ab_view_menu")
@@ -1338,10 +1316,17 @@ class TestDatasetApi(SupersetTestCase):
rv = self.get_assert_metric(uri, "get")
data = json.loads(rv.data.decode("utf-8"))
strip_read_only_fields(data["result"]["columns"])
for column in data["result"]["columns"]:
column.pop("changed_on", None)
column.pop("created_on", None)
column.pop("type_generic", None)
data["result"]["columns"].append(new_column_data)
strip_read_only_fields(data["result"]["metrics"])
for metric in data["result"]["metrics"]:
metric.pop("changed_on", None)
metric.pop("created_on", None)
metric.pop("type_generic", None)
data["result"]["metrics"].append(new_metric_data)
with freeze_time() as frozen:
@@ -1419,7 +1404,11 @@ class TestDatasetApi(SupersetTestCase):
rv = self.get_assert_metric(uri, "get")
data = json.loads(rv.data.decode("utf-8"))
strip_read_only_fields(data["result"]["columns"])
for column in data["result"]["columns"]:
column.pop("changed_on", None)
column.pop("created_on", None)
column.pop("type_generic", None)
data["result"]["columns"].append(new_column_data)
rv = self.client.put(uri, json={"columns": data["result"]["columns"]})
@@ -1454,7 +1443,10 @@ class TestDatasetApi(SupersetTestCase):
# Get current cols and alter one
rv = self.get_assert_metric(uri, "get")
resp_columns = json.loads(rv.data.decode("utf-8"))["result"]["columns"]
strip_read_only_fields(resp_columns)
for column in resp_columns:
column.pop("changed_on", None)
column.pop("created_on", None)
column.pop("type_generic", None)
resp_columns[0]["groupby"] = False
resp_columns[0]["filterable"] = False
@@ -155,7 +155,6 @@ class TestDatasourceApi(SupersetTestCase):
limit=10000,
denormalize_column=False,
array_elements=False,
search=None,
)
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@@ -171,79 +170,6 @@ class TestDatasourceApi(SupersetTestCase):
)
assert values_for_column_mock.call_args.kwargs["array_elements"] is True
@pytest.mark.usefixtures("app_context", "virtual_dataset")
def test_get_column_values_search_filters_server_side(self):
"""``?q=`` narrows the values in the database rather than client-side,
which is what makes a value beyond the row limit reachable at all."""
self.login(ADMIN_USERNAME)
table = self.get_virtual_dataset()
rv = self.client.get(
f"api/v1/datasource/table/{table.id}/column/col2/values/?q=b"
)
assert rv.status_code == 200
assert json.loads(rv.data.decode("utf-8"))["result"] == ["b"]
@pytest.mark.usefixtures("app_context", "virtual_dataset")
def test_get_column_values_search_is_case_insensitive(self):
self.login(ADMIN_USERNAME)
table = self.get_virtual_dataset()
rv = self.client.get(
f"api/v1/datasource/table/{table.id}/column/col2/values/?q=B"
)
assert rv.status_code == 200
assert json.loads(rv.data.decode("utf-8"))["result"] == ["b"]
@pytest.mark.usefixtures("app_context", "virtual_dataset")
def test_get_column_values_search_escapes_wildcards(self):
"""A literal ``%`` must not be treated as "match everything"."""
self.login(ADMIN_USERNAME)
table = self.get_virtual_dataset()
rv = self.client.get(
f"api/v1/datasource/table/{table.id}/column/col2/values/?q=%25"
)
assert rv.status_code == 200
assert json.loads(rv.data.decode("utf-8"))["result"] == []
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@patch("superset.models.helpers.ExploreMixin.values_for_column")
def test_get_column_values_blank_search_is_ignored(self, values_for_column_mock):
"""Whitespace is not a search term; it must not narrow the list."""
values_for_column_mock.return_value = []
self.login(ADMIN_USERNAME)
table = self.get_virtual_dataset()
self.client.get(
f"api/v1/datasource/table/{table.id}/column/col2/values/?q=%20%20"
)
assert values_for_column_mock.call_args.kwargs["search"] is None
@pytest.mark.usefixtures("app_context", "virtual_dataset")
def test_get_column_values_returns_applied_limit(self):
"""The client needs the limit to tell a short list from a truncated
one, so it can say the list is partial instead of implying it is whole."""
self.login(ADMIN_USERNAME)
table = self.get_virtual_dataset()
rv = self.client.get(f"api/v1/datasource/table/{table.id}/column/col2/values/")
assert rv.status_code == 200
assert json.loads(rv.data.decode("utf-8"))["limit"] == 10000
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@patch("superset.models.helpers.ExploreMixin.values_for_column")
def test_get_column_values_cache_isolated_per_search(self, values_for_column_mock):
"""Search terms must partition the cache; sharing one entry would serve
the results of somebody else's search."""
cache_manager.data_cache.clear()
values_for_column_mock.return_value = ["x"]
self.login(ADMIN_USERNAME)
table = self.get_virtual_dataset()
url = f"api/v1/datasource/table/{table.id}/column/col2/values/"
self.client.get(url)
self.client.get(f"{url}?q=a")
self.client.get(f"{url}?q=b")
self.client.get(f"{url}?q=a")
assert values_for_column_mock.call_count == 3
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@patch("superset.db_engine_specs.base.BaseEngineSpec.denormalize_name")
def test_get_column_values_not_denormalize_column(self, denormalize_name_mock):
@@ -265,7 +191,6 @@ class TestDatasourceApi(SupersetTestCase):
limit=10000,
denormalize_column=True,
array_elements=False,
search=None,
)
@pytest.mark.usefixtures("app_context", "virtual_dataset")
-61
View File
@@ -21,7 +21,6 @@ from unittest.mock import MagicMock, patch
from sqlalchemy.orm.session import Session
from superset import db
from superset.utils import json
def test_put_invalid_dataset(
@@ -215,63 +214,3 @@ def test_handle_filters_args_returns_request_scoped_filters(
fresh_filters = api.datamodel.get_filters.return_value
assert fresh_filters.rest_add_filters.call_count == 2
assert fresh_filters.get_joined_filters.call_count == 2
def test_get_dataset_exposes_certification_metadata(
session: Session,
client: Any,
full_api_access: None,
) -> None:
"""
Dataset API: Test that the show payload exposes the certification and
warning metadata for both columns and metrics.
Regression test for #43279: Explore hydrates its datasource from this
endpoint after a dataset save or swap. Without these fields the certified
and warning badges disappeared until the page was reloaded, because the
Explore bootstrap payload serializes them but this endpoint did not.
"""
from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn
from superset.models.core import Database
SqlaTable.metadata.create_all(db.session.get_bind())
extra = json.dumps(
{
"certification": {
"certified_by": "Data Platform",
"details": "Reviewed quarterly",
},
"warning_markdown": "This is a **warning**",
}
)
database = Database(
database_name="my_db",
sqlalchemy_uri="sqlite://",
)
dataset = SqlaTable(
table_name="test_certification_table",
database=database,
columns=[
TableColumn(column_name="ds", type="TIMESTAMP", extra=extra),
TableColumn(
column_name="calculated",
type="INTEGER",
expression="1 + 1",
extra=extra,
),
],
metrics=[SqlMetric(metric_name="cnt", expression="COUNT(*)", extra=extra)],
)
db.session.add(dataset)
db.session.flush()
response = client.get(f"/api/v1/dataset/{dataset.id}")
assert response.status_code == 200
result = response.json["result"]
for item in [*result["columns"], *result["metrics"]]:
assert item["is_certified"] is True
assert item["certified_by"] == "Data Platform"
assert item["certification_details"] == "Reviewed quarterly"
assert item["warning_markdown"] == "This is a **warning**"
-80
View File
@@ -105,86 +105,6 @@ def test_values_for_column(database: Database) -> None:
assert table.values_for_column("a") == [1, None]
@pytest.mark.parametrize(
"raw,expected",
[
("plain", "plain"),
("50%", "50!%"),
("a_b", "a!_b"),
("wow!", "wow!!"),
("!%_", "!!!%!_"),
],
)
def test_escape_like_pattern(raw: str, expected: str) -> None:
"""Wildcards typed by a user are data, not pattern syntax."""
from superset.models.helpers import escape_like_pattern
assert escape_like_pattern(raw) == expected
def test_build_like_predicate_is_case_insensitive_and_escaped() -> None:
import sqlalchemy as sa
from superset.models.helpers import build_like_predicate
compiled = str(
build_like_predicate(sa.column("c"), "50%").compile(
dialect=sa.dialects.registry.load("postgresql")(),
compile_kwargs={"literal_binds": True},
)
).replace("%%", "%")
assert compiled == "lower(c) LIKE '%50!%%' ESCAPE '!'"
def test_values_for_column_search(database: Database) -> None:
"""``search`` narrows the distinct-value query in the database."""
import pandas as pd
from superset.connectors.sqla.models import SqlaTable, TableColumn
table = SqlaTable(
database=database,
schema=None,
table_name="t",
columns=[TableColumn(column_name="a")],
)
with patch(
"pandas.read_sql_query",
return_value=pd.DataFrame({"column_values": ["Alice"]}),
) as read_sql_query:
assert table.values_for_column("a", search="ali") == ["Alice"]
sql = str(read_sql_query.call_args.kwargs["sql"])
assert "LIKE" in sql
assert "'%ali%'" in sql
def test_values_for_column_without_search_has_no_predicate(
database: Database,
) -> None:
"""The unsearched list must stay a plain bounded DISTINCT scan."""
import pandas as pd
from superset.connectors.sqla.models import SqlaTable, TableColumn
table = SqlaTable(
database=database,
schema=None,
table_name="t",
columns=[TableColumn(column_name="a")],
)
with patch(
"pandas.read_sql_query",
return_value=pd.DataFrame({"column_values": ["Alice"]}),
) as read_sql_query:
table.values_for_column("a")
assert "LIKE" not in str(read_sql_query.call_args.kwargs["sql"])
def test_values_for_column_passes_catalog_and_schema(
mocker: MockerFixture,
session: Session,
-35
View File
@@ -5578,41 +5578,6 @@ def test_parse_predicate_length_check() -> None:
stmt.parse_predicate("x" * 101)
def test_parse_predicate_invalid_sql_raises_superset_parse_error() -> None:
"""
A syntactically invalid RLS predicate raises ``SupersetParseError``.
``parse_predicate`` is reachable via ``apply_rls`` for any RLS clause
configured on a queried table; an invalid clause must surface as the
typed 422 parse error rather than leaking a raw ``sqlglot`` exception.
"""
stmt = SQLStatement("SELECT 1", "postgresql")
with pytest.raises(SupersetParseError) as excinfo:
stmt.parse_predicate("a >")
assert excinfo.value.status == 422
def test_parse_predicate_sqlglot_error_raises_superset_parse_error(
mocker: MockerFixture,
) -> None:
"""
A non-``ParseError`` ``sqlglot`` failure also surfaces as a typed error.
``parse_predicate`` catches the generic ``SqlglotError`` base class as a
fallback so any sqlglot failure (e.g. tokenize errors) is converted into a
``SupersetParseError`` rather than leaking a raw sqlglot exception.
"""
# Build the statement before patching, since the constructor also parses.
stmt = SQLStatement("SELECT 1", "postgresql")
mocker.patch(
"sqlglot.parse_one",
side_effect=sqlglot.errors.SqlglotError("boom"),
)
with pytest.raises(SupersetParseError) as excinfo:
stmt.parse_predicate("a > 1")
assert excinfo.value.status == 422
@pytest.mark.usefixtures("_small_parse_cap")
def test_transpile_to_dialect_length_check() -> None:
"""