Compare commits

...
Author SHA1 Message Date
Elizabeth Thompson 1cadf39ce8 test(sql): cover SqlglotError fallback branch in parse_predicate
The regression test only exercised the ParseError branch, leaving the
generic sqlglot.errors.SqlglotError fallback in
SQLStatement.parse_predicate uncovered and dropping line coverage below
the 100% gate. Add a test that mocks sqlglot.parse_one to raise a bare
SqlglotError and asserts it is converted to a SupersetParseError.
2026-08-28 22:14:52 +00:00
Elizabeth Thompson 876b8641e2 fix(sql): catch sqlglot ParseError when parsing RLS predicates
SQLStatement.parse_predicate called sqlglot.parse_one unguarded, so a
syntactically invalid RLS predicate raised a raw sqlglot ParseError.
Reachable via apply_rls (e.g. POST /api/v1/sqllab/estimate with
RLS_IN_SQLLAB enabled), this surfaced as an opaque 500 instead of a
typed 422.

Wrap the call to convert ParseError/SqlglotError into SupersetParseError,
mirroring the existing idiom in SQLStatement._parse.
2026-08-28 16:49:17 +00:00
rlei fc26991cd4 feat(plugin-chart-echarts): add gridline and axis tick controls (#43428) 2026-08-28 09:44:39 -07:00
shauryaandShaurya a5c68c8df9 fix(number-format): handle sub-byte values and unit rollover in memory formatter (#43549)
Co-authored-by: Shaurya <19599684+no-hup@users.noreply.github.com>
2026-08-28 09:44:19 -07:00
Đỗ Trọng Hải 53e76afd70 feat(ci): enforce min release age for npm dep installation (#43164)
Signed-off-by: hainenber <dotronghai96@gmail.com>
2026-08-28 23:42:41 +07:00
shauryaandShaurya d997d363e3 docs: update frontend Node/npm prerequisites to match engines (#43546)
Co-authored-by: Shaurya <19599684+no-hup@users.noreply.github.com>
2026-08-28 23:16:34 +07:00
Lalith Kothuru 9a6f6ee0c0 docs: fix docstring parameter names that do not match signatures (#43630) 2026-08-28 22:57:43 +07:00
Joe Li 94dd3d049c fix(ci): repair scheduled pre-commit drift (#43603) 2026-08-28 22:56:18 +07:00
b3f718da62 fix(explore): keep certification badges after saving or swapping a dataset (#43319)
Co-authored-by: rusackas <evan@rusackas.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-08-28 08:47:36 -07:00
Mehmet Salih YavuzandEnzo Martellucci abf338d611 feat(filters): search filter values server-side in Explore (#43518)
Co-authored-by: Enzo Martellucci <52219496+EnxDev@users.noreply.github.com>
2026-08-28 16:46:09 +03:00
Mehmet Salih Yavuz e518b21994 fix(dashboard): keep the chart menu usable in fullscreen on production builds (#43555) 2026-08-28 15:47:48 +03:00
Mehmet Salih Yavuz e18f27e1ce fix(native-filters): allow clearing an optional "select first value" filter (#43405) 2026-08-28 15:46:46 +03:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> 9bf457dea6 chore(deps): bump github/codeql-action/analyze from 4.37.7 to 4.37.8 (#43643)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-28 15:26:18 +03:00
Enzo Martellucci cb7b790733 fix(dataset): surface real validation message in save-error dialog (#43459) 2026-08-28 13:35:09 +02:00
Alexandru Soare 8bec85158c feat(modals): add renderExtraFields prop to chart and dashboard properties modals (#43622) 2026-08-28 13:18:51 +03:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> a30e4a4350 chore(deps): bump github/codeql-action/init from 4.37.7 to 4.37.8 (#43644)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-28 02:39:11 -07:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> 933dbbc2a2 chore(deps-dev): bump eslint-plugin-react-you-might-not-need-an-effect from 1.0.1 to 1.0.2 in /superset-frontend (#43645)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-28 02:39:06 -07:00
Enzo Martellucci a59b96c4f5 fix(explore): keep the row count label visible when the row limit is reached (#43296) 2026-08-28 10:51:27 +02:00
MafiandMatt Fitzgerald 12cd259c55 fix(dataset): preserve validation error messages (#43631)
Co-authored-by: Matt Fitzgerald <matt.fitzgerald@preset.io>
2026-08-28 01:01:17 -04:00
Chandan P 3ddc3b1d56 fix(cache): fall back to default timeout when cache_timeout is None (#43149) 2026-08-27 21:50:53 -07:00
Chandan P 98ec6018df fix(embedded): honor "can view query" permission for guest users (#43151) 2026-08-27 21:49:52 -07:00
dependabot[bot]dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>Joe Li
2ebd415b8a chore(deps-dev): bump mcp from 1.29.1 to 2.0.0 (#43611)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joe Li <joe@preset.io>
2026-08-27 21:49:50 -07:00
81b3e85522 fix(echarts): guard cross-filter when labelMap entry is missing (#42559)
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: rusackas <evan@rusackas.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-08-27 21:49:21 -07:00
Viktor Högberg fd64efd72d fix(uploads): set catalog on datasets created by file upload (#43301) 2026-08-27 21:46:51 -07:00
bucketbase26andJoe Li e39bfb255b fix(explore): prevent duplicate Date Range tooltips (#43425)
Co-authored-by: Joe Li <joe@preset.io>
2026-08-27 21:02:57 -07:00
b7301ac88a fix: Dashboard export with charts from multiple databases (#37120)
Co-authored-by: rusackas <evan@rusackas.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-08-27 20:27:54 -07:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> fa59b44cfe chore(deps): bump mapbox-gl from 3.28.1 to 3.29.0 in /superset-frontend (#43619)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-27 19:58:55 -07:00
104 changed files with 2616 additions and 330 deletions
+2 -2
View File
@@ -67,7 +67,7 @@ jobs:
# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7
uses: github/codeql-action/init@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4.37.8
with:
languages: ${{ matrix.language }}
# If you wish to specify custom queries, you can do so here or in a config file.
@@ -78,6 +78,6 @@ jobs:
# queries: security-extended,security-and-quality
- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7
uses: github/codeql-action/analyze@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4.37.8
with:
category: "/language:${{matrix.language}}"
+1
View File
@@ -10,6 +10,7 @@
.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 22 (LTS)
- `npm`: Version 10
- `Node.js`: Version 24 (see `superset-frontend/.nvmrc` for the exact version)
- `npm`: Version 11
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 --lts
nvm use --lts
nvm install
nvm use
```
Or if you use the default macOS starting with Catalina shell `zsh`, try:
+1 -1
View File
@@ -196,7 +196,7 @@ excel = ["xlrd>=2.0.2, <2.1"]
excel-export = ["boto3"]
fastmcp = [
"fastmcp>=3.4.7,<4.0",
"mcp>=1.29.1,<2.0",
"mcp>=1.29.1,<3.0",
# tiktoken backs the response-size-guard token estimator. Without
# it, the middleware falls back to a coarser character-based
# heuristic that under-counts JSON-heavy MCP responses.
@@ -21,6 +21,7 @@ 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
@@ -0,0 +1 @@
../superset-frontend/.npmrc
+1
View File
@@ -0,0 +1 @@
min-release-age=3
+9 -9
View File
@@ -108,7 +108,7 @@
"json-stringify-pretty-compact": "^4.0.0",
"lodash": "^4.18.1",
"lodash-es": "^4.18.1",
"mapbox-gl": "^3.28.1",
"mapbox-gl": "^3.29.0",
"markdown-to-jsx": "^9.10.2",
"match-sorter": "^8.3.0",
"memoize-one": "^6.0.0",
@@ -234,7 +234,7 @@
"eslint-plugin-lodash": "^8.0.0",
"eslint-plugin-no-only-tests": "^3.4.0",
"eslint-plugin-react-prefer-function-component": "^5.0.0",
"eslint-plugin-react-you-might-not-need-an-effect": "^1.0.1",
"eslint-plugin-react-you-might-not-need-an-effect": "^1.0.2",
"eslint-plugin-storybook": "10.5.10",
"eslint-plugin-testing-library": "^7.16.2",
"eslint-plugin-theme-colors": "file:eslint-rules/eslint-plugin-theme-colors",
@@ -20208,9 +20208,9 @@
"license": "MIT"
},
"node_modules/eslint-plugin-react-you-might-not-need-an-effect": {
"version": "1.0.1",
"resolved": "https://registry.npmjs.org/eslint-plugin-react-you-might-not-need-an-effect/-/eslint-plugin-react-you-might-not-need-an-effect-1.0.1.tgz",
"integrity": "sha512-oOhQTYhor88Xp8RVytq25tvBfiAjU0r9SCDC51Qop+3Wg5BR1xGMAkM+/dV4MZbcMhdaU1L9bkv6LC95JmiTig==",
"version": "1.0.2",
"resolved": "https://registry.npmjs.org/eslint-plugin-react-you-might-not-need-an-effect/-/eslint-plugin-react-you-might-not-need-an-effect-1.0.2.tgz",
"integrity": "sha512-HgYol2zhH3KbnW9Q4FY/FcIINbsYVL6rwQESChqBC2s9SLWtw1wg05+J7SCrr3BfQpNY2ReYhf8xjpd2JhKJOQ==",
"dev": true,
"license": "MIT",
"dependencies": {
@@ -28488,9 +28488,9 @@
}
},
"node_modules/mapbox-gl": {
"version": "3.28.1",
"resolved": "https://registry.npmjs.org/mapbox-gl/-/mapbox-gl-3.28.1.tgz",
"integrity": "sha512-f8bCHFzZ51bKig7rnD7e08aoFLOV3MNFZduspZ4lgOgiaNVp9sw4NSWcgo3IWTyekYKKXjiICD2BP2o4DiYfxw==",
"version": "3.29.0",
"resolved": "https://registry.npmjs.org/mapbox-gl/-/mapbox-gl-3.29.0.tgz",
"integrity": "sha512-Fnh1WLsZMfihwRZY5scp456iQuZo9G97tTpb26bf/Ejsi/L7O+4dE9+I03VoOor2ul3DEOp6F2P3273omyVsNw==",
"license": "SEE LICENSE IN LICENSE.txt",
"workspaces": [
"src/style-spec",
@@ -43490,7 +43490,7 @@
"license": "Apache-2.0",
"dependencies": {
"@math.gl/web-mercator": "^4.1.0",
"mapbox-gl": "^3.28.1",
"mapbox-gl": "^3.29.0",
"maplibre-gl": "^5.24.0",
"react-map-gl": "^8.1.2",
"supercluster": "^9.0.0"
+3 -3
View File
@@ -185,7 +185,7 @@
"json-stringify-pretty-compact": "^4.0.0",
"lodash": "^4.18.1",
"lodash-es": "^4.18.1",
"mapbox-gl": "^3.28.1",
"mapbox-gl": "^3.29.0",
"markdown-to-jsx": "^9.10.2",
"match-sorter": "^8.3.0",
"memoize-one": "^6.0.0",
@@ -311,7 +311,7 @@
"eslint-plugin-lodash": "^8.0.0",
"eslint-plugin-no-only-tests": "^3.4.0",
"eslint-plugin-react-prefer-function-component": "^5.0.0",
"eslint-plugin-react-you-might-not-need-an-effect": "^1.0.1",
"eslint-plugin-react-you-might-not-need-an-effect": "^1.0.2",
"eslint-plugin-storybook": "10.5.10",
"eslint-plugin-testing-library": "^7.16.2",
"eslint-plugin-theme-colors": "file:eslint-rules/eslint-plugin-theme-colors",
@@ -392,7 +392,7 @@
"@luma.gl/shadertools": "~9.2.5",
"@luma.gl/webgl": "~9.2.5",
"core-js": "^3.38.1",
"dompurify": "^3.4.11",
"dompurify": "^3.4.13",
"esbuild": "^0.28.1",
"eslint-plugin-import": {
"eslint": "$eslint"
@@ -65,6 +65,7 @@ export type AntdExposedProps = Pick<
| 'onOpenChange'
| 'optionRender'
| 'placeholder'
| 'prefix'
| 'showArrow'
| 'showSearch'
| 'tokenSeparators'
@@ -38,11 +38,21 @@ function formatMemory(
: ['B', 'kB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB', 'RB', 'QB'];
const base = binary ? 1024 : 1000;
const i = Math.min(
suffixes.length - 1,
Math.floor(Math.log(absValue) / Math.log(base)),
let i = Math.max(
0,
Math.min(
suffixes.length - 1,
Math.floor(Math.log(absValue) / Math.log(base)),
),
);
formatted = `${sign}${parseFloat((absValue / Math.pow(base, i)).toFixed(decimals))}${suffixes[i]}`;
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]}`;
}
if (transfer) {
@@ -119,6 +119,20 @@ export function retrieveErrorMessage(
return statusError || parseStringResponse(str);
}
function getFirstValidationError(message: JsonObject): string | undefined {
const [firstError] = Object.values(message);
if (typeof firstError === 'string') {
return firstError;
}
if (Array.isArray(firstError)) {
return firstError.find((item): item is string => typeof item === 'string');
}
return undefined;
}
export function parseErrorJson(responseJson: JsonObject): ClientErrorObject {
let error = { ...responseJson };
// Backwards compatibility for old error renderers with the new error object
@@ -126,13 +140,12 @@ export function parseErrorJson(responseJson: JsonObject): ClientErrorObject {
error.error = error.description = error.errors[0].message;
error.link = error.errors[0]?.extra?.link;
}
// Marshmallow field validation returns the error message in the format
// of { message: { field1: [msg1, msg2], field2: [msg], } }
// Marshmallow field validation returns arrays for string messages, but
// serializes lazy translation messages as strings instead.
if (!error.error && error.message) {
if (typeof error.message === 'object') {
error.error =
Object.values(error.message as Record<string, string[]>)[0]?.[0] ||
t('Invalid input');
getFirstValidationError(error.message) || t('Invalid input');
}
if (typeof error.message === 'string') {
if (checkForHtml(error.message)) {
@@ -60,6 +60,31 @@ 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');
@@ -244,6 +244,24 @@ test('parseErrorJson with message', () => {
});
});
test('parseErrorJson preserves string-valued validation messages', () => {
const calculatedColumnError =
'Custom SQL fields cannot be parsed as a single SQL statement.';
expect(
parseErrorJson({
message: {
'columns.0.expression': calculatedColumnError,
},
}),
).toEqual({
message: {
'columns.0.expression': calculatedColumnError,
},
error: calculatedColumnError,
});
});
test('parseErrorJson with HTML message', () => {
expect(
parseErrorJson({
@@ -38,6 +38,8 @@ import { EchartsTimeseriesSeriesType } from '../Timeseries/types';
import {
legendSection,
minorTicks,
axisTicks,
gridlines,
richTooltipSection,
truncateXAxis,
xAxisBounds,
@@ -391,6 +393,8 @@ const config: ControlPanelConfig = {
...createCustomizeSection(t('Query B'), 'B'),
['zoomable'],
[minorTicks],
[axisTicks],
[gridlines],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
['x_axis_time_format'],
@@ -184,6 +184,8 @@ export default function transformProps(
opacityB,
minorSplitLine,
minorTicks,
gridlines,
axisTicks,
seriesType,
seriesTypeB,
showLegend,
@@ -788,6 +790,8 @@ export default function transformProps(
}),
},
minorTick: { show: minorTicks },
axisTick: { show: axisTicks ? 'auto' : false },
...(gridlines ? {} : { splitLine: { show: false } }),
minInterval:
xAxisType === AxisType.Time && resolvedTimeGrain && !forceMaxInterval
? (TIMEGRAIN_TO_TIMESTAMP[
@@ -818,6 +822,8 @@ 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(
@@ -840,6 +846,7 @@ export default function transformProps(
min: minSecondary,
max: maxSecondary,
minorTick: { show: minorTicks },
axisTick: { show: axisTicks ? 'auto' : false },
splitLine: { show: false },
minorSplitLine: { show: minorSplitLine },
axisLabel: {
@@ -48,6 +48,8 @@ export type EchartsMixedTimeseriesFormData = QueryFormData & {
// shared properties
minorSplitLine: boolean;
minorTicks: boolean;
gridlines: boolean;
axisTicks: boolean;
logAxis: boolean;
logAxisSecondary: boolean;
yAxisFormat?: string;
@@ -113,6 +115,8 @@ 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,6 +44,8 @@ import {
truncateXAxis,
xAxisBounds,
minorTicks,
axisTicks,
gridlines,
forceMaxInterval,
} from '../../controls';
import { AreaChartStackControlOptions } from '../../constants';
@@ -174,6 +176,8 @@ const config: ControlPanelConfig = {
},
],
[minorTicks],
[axisTicks],
[gridlines],
['zoomable'],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
@@ -133,6 +133,8 @@ const defaultFormData: EchartsTimeseriesFormData & {
metrics: [],
minorSplitLine: false,
minorTicks: false,
gridlines: true,
axisTicks: true,
opacity: 1,
orderDesc: false,
rowLimit: 0,
@@ -40,6 +40,8 @@ import {
import {
legendSection,
minorTicks,
axisTicks,
gridlines,
richTooltipSection,
seriesOrderSection,
showValueSectionWithoutStream,
@@ -388,6 +390,8 @@ const config: ControlPanelConfig = {
},
],
[minorTicks],
[axisTicks],
[gridlines],
['zoomable'],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
@@ -37,6 +37,8 @@ import {
import {
legendSection,
minorTicks,
axisTicks,
gridlines,
richTooltipSection,
seriesOrderSection,
showValueSection,
@@ -156,6 +158,8 @@ const config: ControlPanelConfig = {
],
['zoomable'],
[minorTicks],
[axisTicks],
[gridlines],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
[
@@ -42,6 +42,8 @@ import {
import {
legendSection,
minorTicks,
axisTicks,
gridlines,
richTooltipSection,
seriesOrderSection,
showValueSection,
@@ -480,6 +482,8 @@ const config: ControlPanelConfig = {
],
['zoomable'],
[minorTicks],
[axisTicks],
[gridlines],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
...createAxisControl('x'),
@@ -37,6 +37,8 @@ import {
import {
legendSection,
minorTicks,
axisTicks,
gridlines,
richTooltipSection,
seriesOrderSection,
showValueSectionWithoutStack,
@@ -105,6 +107,8 @@ const config: ControlPanelConfig = {
],
['zoomable'],
[minorTicks],
[axisTicks],
[gridlines],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
[
@@ -35,6 +35,8 @@ import { DEFAULT_FORM_DATA, TIME_SERIES_DESCRIPTION_TEXT } from '../constants';
import {
legendSection,
minorTicks,
axisTicks,
gridlines,
richTooltipSection,
seriesOrderSection,
showValueSection,
@@ -157,6 +159,8 @@ const config: ControlPanelConfig = {
],
['zoomable'],
[minorTicks],
[axisTicks],
[gridlines],
...legendSection,
[<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
[
@@ -67,6 +67,8 @@ export const DEFAULT_FORM_DATA: EchartsTimeseriesFormData = {
maxMarkerSize: 30,
minMarkerSize: 5,
minorSplitLine: false,
gridlines: true,
axisTicks: true,
opacity: 0.2,
orderDesc: true,
rowLimit: 10000,
@@ -283,6 +283,8 @@ export default function transformProps(
metrics,
minorSplitLine,
minorTicks,
gridlines,
axisTicks,
onlyTotal,
opacity,
orientation,
@@ -1280,6 +1282,8 @@ export default function transformProps(
}),
},
minorTick: { show: minorTicks },
axisTick: { show: axisTicks ? 'auto' : false },
...(gridlines ? {} : { splitLine: { show: false } }),
minInterval:
xAxisType === AxisType.Time && resolvedTimeGrain && !forceMaxInterval
? (TIMEGRAIN_TO_TIMESTAMP[
@@ -1324,7 +1328,7 @@ export default function transformProps(
max: yAxisMax,
minorTick: { show: isSmallChart ? false : minorTicks },
minorSplitLine: { show: isSmallChart ? false : minorSplitLine },
splitLine: { show: !isSmallChart },
splitLine: { show: isSmallChart ? false : gridlines },
axisLabel: {
show: !isMicroChart,
showMinLabel: !isMicroChart,
@@ -1338,7 +1342,7 @@ export default function transformProps(
yAxisFormat,
),
},
axisTick: { show: !isSmallChart },
axisTick: { show: isSmallChart ? false : axisTicks },
scale: truncateYAxis,
name: isSmallChart ? undefined : yAxisTitle,
nameGap: convertInteger(yAxisTitleMargin),
@@ -73,6 +73,8 @@ export type EchartsTimeseriesFormData = QueryFormData & {
metrics: QueryFormMetric[];
minorSplitLine: boolean;
minorTicks: boolean;
gridlines: boolean;
axisTicks: boolean;
opacity: number;
orderDesc: boolean;
rowLimit: number;
@@ -495,6 +495,28 @@ 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: {
@@ -47,6 +47,9 @@ const getCrossFilterDataMask =
) =>
(value: string) => {
const selected = Object.values(selectedValues);
if (!labelMap[value] && !selected.includes(value)) {
return undefined;
}
let values: string[];
if (selected.includes(value)) {
values = selected.filter(v => v !== value);
@@ -116,6 +116,8 @@ const formData: EchartsMixedTimeseriesFormData = {
markerSizeB: 0,
minorSplitLine: false,
minorTicks: false,
gridlines: true,
axisTicks: true,
opacity: 0,
opacityB: 0,
orderDesc: false,
@@ -1509,3 +1511,56 @@ describe('EchartsMixedTimeseries tooltip truncation', () => {
expect(html).not.toContain(longSeriesName);
});
});
function transformWithChrome(
overrides: Partial<EchartsMixedTimeseriesFormData>,
) {
const chartProps = createEchartsTimeseriesTestChartProps<
EchartsMixedTimeseriesFormData,
EchartsMixedTimeseriesProps
>({
...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
defaultQueriesData: queriesData,
formData: { ...formData, ...overrides },
queriesData,
});
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({});
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');
});
test('hides the gridlines on the primary axis', () => {
const { xAxis, yAxis } = transformWithChrome({ gridlines: false });
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);
});
test('never turns the secondary axis gridlines on', () => {
const { xAxis, yAxis } = transformWithChrome({ gridlines: true });
expect(yAxis[0].splitLine.show).toBe(true);
expect(yAxis[1].splitLine.show).toBe(false);
expect(xAxis.splitLine).toBeUndefined();
});
test('hides the ticks on the x axis and both y axes', () => {
const { xAxis, yAxis } = transformWithChrome({ axisTicks: false });
expect(xAxis.axisTick.show).toBe(false);
expect(yAxis[0].axisTick.show).toBe(false);
expect(yAxis[1].axisTick.show).toBe(false);
});
@@ -2766,3 +2766,76 @@ 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);
});
@@ -180,3 +180,46 @@ test('cross-filter does nothing when emitCrossFilters is false', () => {
expect(setDataMask).not.toHaveBeenCalled();
});
test('cross-filter does nothing when name is missing from labelMap', () => {
const setDataMask = jest.fn();
const props = buildProps({
groupby: ['topics'],
labelMap: {
cancellations: ['cancellations'],
},
selectedValues: {},
setDataMask,
});
const handlers = allEventHandlers(props);
// e.g. Pie "Other" category is not present in labelMap
handlers.click({ name: 'Other' });
expect(setDataMask).not.toHaveBeenCalled();
});
test('cross-filter still deselects a previously selected value that is missing from labelMap', () => {
const setDataMask = jest.fn();
const props = buildProps({
groupby: ['topics'],
labelMap: {
cancellations: ['cancellations'],
},
// "Other" was selected before it dropped out of labelMap (e.g. a stale
// cross-filter from an earlier render or dashboard state).
selectedValues: { 0: 'Other' },
setDataMask,
});
const handlers = allEventHandlers(props);
handlers.click({ name: 'Other' });
expect(setDataMask).toHaveBeenCalledWith(
expect.objectContaining({
extraFormData: {
filters: [],
},
}),
);
});
@@ -30,7 +30,7 @@
},
"dependencies": {
"@math.gl/web-mercator": "^4.1.0",
"mapbox-gl": "^3.28.1",
"mapbox-gl": "^3.29.0",
"maplibre-gl": "^5.24.0",
"react-map-gl": "^8.1.2",
"supercluster": "^9.0.0"
@@ -355,25 +355,31 @@ describe('SqlEditor', () => {
}),
);
// findByRole('button', { name }) walks every stylesheet rule via nwsapi to
// compute the accessible name, which can crash on an unrelated antd Tabs
// "more" button style; findByLabelText matches the same aria-label without
// that traversal.
test('enables the save dataset button when the latest query succeeded', async () => {
const { findByRole } = setupWithLatestQuery({ state: QueryState.Success });
expect(await findByRole('button', { name: 'Save dataset' })).toBeEnabled();
const { findByLabelText } = setupWithLatestQuery({
state: QueryState.Success,
});
expect(await findByLabelText('Save dataset')).toBeEnabled();
});
test('disables the save dataset button when the latest query failed', async () => {
const { findByRole } = setupWithLatestQuery({
const { findByLabelText } = setupWithLatestQuery({
state: QueryState.Failed,
results: undefined,
});
expect(await findByRole('button', { name: 'Save dataset' })).toBeDisabled();
expect(await findByLabelText('Save dataset')).toBeDisabled();
});
test('disables the save dataset button when the results are not loaded', async () => {
const { findByRole } = setupWithLatestQuery({
const { findByLabelText } = setupWithLatestQuery({
state: QueryState.Success,
results: undefined,
});
expect(await findByRole('button', { name: 'Save dataset' })).toBeDisabled();
expect(await findByLabelText('Save dataset')).toBeDisabled();
});
test('renders an Extension if provided', async () => {
@@ -53,6 +53,22 @@ test('RowCountLabel renders limit with danger and tooltip', async () => {
expect(tooltip).toHaveTextContent('The row limit');
});
test('RowCountLabel uses a caller-provided limitReachedMessage instead of the default', async () => {
render(
<RowCountLabel
rowcount={100}
limit={100}
limitReachedMessage="Custom limit message"
/>,
);
const expectedText = '100 rows';
expect(screen.getByText(expectedText)).toBeInTheDocument();
userEvent.hover(screen.getByText(expectedText));
const tooltip = await screen.findByRole('tooltip');
expect(tooltip).toHaveTextContent('Custom limit message');
expect(tooltip).not.toHaveTextContent('The row limit set for the chart');
});
test('RowCountLabel renders loading', () => {
render(<RowCountLabel loading />);
const expectedText = 'Loading...';
@@ -26,6 +26,9 @@ type RowCountLabelProps = {
limit?: number;
loading?: boolean;
label?: JSX.Element;
// Overrides the default "chart" wording for panes (e.g. samples) where the
// limit reached isn't the chart's own row_limit.
limitReachedMessage?: React.ReactNode;
};
const limitReachedMsg = t(
@@ -33,7 +36,13 @@ const limitReachedMsg = t(
);
export default function RowCountLabel(props: RowCountLabelProps) {
const { rowcount = 0, limit = null, loading, label } = props;
const {
rowcount = 0,
limit = null,
loading,
label,
limitReachedMessage,
} = props;
const limitReached = limit && rowcount >= limit;
const type =
limitReached || (rowcount === 0 && !loading) ? 'error' : 'default';
@@ -50,7 +59,10 @@ export default function RowCountLabel(props: RowCountLabelProps) {
</Label>
);
return limitReached ? (
<Tooltip id="tt-rowcount-tooltip" title={<span>{limitReachedMsg}</span>}>
<Tooltip
id="tt-rowcount-tooltip"
title={<span>{limitReachedMessage ?? limitReachedMsg}</span>}
>
{label || labelText}
</Tooltip>
) : (
@@ -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));
@@ -117,7 +117,7 @@ const StyledDiv = styled.div`
${
isMobileConsumptionEnabled()
? `@media (max-width: ${theme.screenSMMax}px) {
[data-test='slice-header'] .header-title {
.slice-header .header-title {
-webkit-line-clamp: unset;
display: block;
white-space: normal;
@@ -79,6 +79,15 @@ type PropertiesModalProps = {
addSuccessToast: (message: string) => void;
addDangerToast: (message: string) => void;
onlyApply?: boolean;
renderExtraFields?: (context: {
assetId: number;
assetType: 'dashboard';
accessorCount: number;
}) => {
content: React.ReactNode;
saveDisabled?: boolean;
saveTooltip?: string;
};
};
type DashboardInfo = {
@@ -107,6 +116,7 @@ const PropertiesModal = ({
onlyApply = false,
onSubmit = () => {},
show = false,
renderExtraFields,
}: PropertiesModalProps) => {
const dispatch = useDispatch();
const [form] = Form.useForm();
@@ -123,6 +133,17 @@ const PropertiesModal = ({
});
const [editors, setEditors] = useState<Subject[]>([]);
const [viewers, setViewers] = useState<Subject[]>([]);
const extraFields = useMemo(
() =>
renderExtraFields?.({
assetId: dashboardId,
assetType: 'dashboard',
accessorCount: editors.length + viewers.length,
}),
[renderExtraFields, dashboardId, editors.length, viewers.length],
);
const saveLabel = onlyApply ? t('Apply') : t('Save');
const [tags, setTags] = useState<TagType[]>([]);
const [customCss, setCustomCss] = useState('');
@@ -698,15 +719,21 @@ const PropertiesModal = ({
}}
title={t('Dashboard properties')}
isEditMode
saveDisabled={dashboardInfo?.isManagedExternally || hasErrors}
saveDisabled={
dashboardInfo?.isManagedExternally ||
hasErrors ||
extraFields?.saveDisabled
}
saveLoading={isApplying}
contentLoading={isLoading}
errorTooltip={
dashboardInfo?.isManagedExternally
? t(
"This dashboard is managed externally, and can't be edited in Superset",
)
: errorTooltip
extraFields?.saveDisabled && extraFields?.saveTooltip
? extraFields.saveTooltip
: dashboardInfo?.isManagedExternally
? t(
"This dashboard is managed externally, and can't be edited in Superset",
)
: errorTooltip
}
saveText={saveLabel}
wrapProps={{ 'data-test': 'properties-edit-modal' }}
@@ -769,6 +796,7 @@ const PropertiesModal = ({
onChangeViewers={handleOnChangeViewers}
onChangeTags={handleChangeTags}
onClearTags={handleClearTags}
renderExtraFields={extraFields}
/>
),
},
@@ -38,6 +38,11 @@ interface AccessSectionProps {
onChangeViewers: (viewers: SubjectPickerValue[]) => void;
onChangeTags: (tags: { label: string; value: number }[]) => void;
onClearTags: () => void;
renderExtraFields?: {
content: React.ReactNode;
saveDisabled?: boolean;
saveTooltip?: string;
};
}
const AccessSection = ({
@@ -49,6 +54,7 @@ const AccessSection = ({
onChangeViewers,
onChangeTags,
onClearTags,
renderExtraFields,
}: AccessSectionProps) => {
const enableViewers = isFeatureEnabled(FeatureFlag.EnableViewers);
@@ -134,6 +140,7 @@ const AccessSection = ({
/>
</ModalFormField>
)}
{renderExtraFields?.content}
</>
);
};
@@ -208,6 +208,18 @@ test('Should render', () => {
expect(screen.getByTestId('slice-header')).toBeInTheDocument();
});
test('Should expose a class hook, not just data-test, for fullscreen styling', () => {
const props = createProps();
render(<SliceHeader {...props} />, {
useRedux: true,
useRouter: true,
initialState,
});
// The production build strips data-test attributes, so CSS that targets the
// header must hang off a class instead.
expect(screen.getByTestId('slice-header')).toHaveClass('slice-header');
});
test('Should render - default props', () => {
const props = createProps();
@@ -270,7 +270,11 @@ const SliceHeader = forwardRef<HTMLDivElement, SliceHeaderProps>(
);
return (
<ChartHeaderStyles data-test="slice-header" ref={ref}>
<ChartHeaderStyles
className="slice-header"
data-test="slice-header"
ref={ref}
>
<div className="header-title" ref={headerRef}>
<Tooltip title={headerTooltip}>
{/* this div ensures the hover event triggers correctly and prevents flickering */}
@@ -19,20 +19,13 @@
import { css, SupersetTheme } from '@apache-superset/core/theme';
export const fullscreenStyles = (theme: SupersetTheme) => css`
[data-test='dashboard-component-chart-holder']:fullscreen {
.dashboard-component-chart-holder:fullscreen {
background-color: ${theme.colorBgBase};
width: 100vw;
height: 100vh;
box-sizing: border-box;
display: flex;
flex-direction: column;
padding: ${theme.sizeUnit * 4}px;
overflow: visible;
position: relative;
pointer-events: auto;
z-index: ${theme.zIndexPopupBase};
opacity: 1;
visibility: visible;
/* Ensure children take up available space */
.dashboard-chart,
@@ -58,13 +51,8 @@ export const fullscreenStyles = (theme: SupersetTheme) => css`
}
}
/* Interaction and Header fixes */
[data-test='dashboard-component-chart-holder']:fullscreen * {
pointer-events: auto;
}
[data-test='dashboard-component-chart-holder']:fullscreen
[data-test='slice-header'] {
/* Keep the header above the chart it shares the fullscreen layer with */
.dashboard-component-chart-holder:fullscreen .slice-header {
z-index: ${theme.zIndexPopupBase};
position: relative;
}
@@ -337,13 +337,17 @@ const ChartHolder = ({
)}
>
<AntdThemeProvider
getPopupContainer={(triggerNode: HTMLElement) =>
document.fullscreenElement
? (triggerNode?.closest?.(
'[data-test="dashboard-component-chart-holder"]',
) as HTMLElement) || document.body
: document.body
}
getPopupContainer={(triggerNode?: HTMLElement) => {
// Only the fullscreen element's subtree is painted, so popups
// have to be portaled into it rather than to document.body.
// Resolve it directly instead of matching a selector: the
// production build strips data-test attributes.
const fullscreenElement =
document.fullscreenElement as HTMLElement | null;
return triggerNode && fullscreenElement?.contains(triggerNode)
? fullscreenElement
: document.body;
}}
>
{!editMode && (
<AnchorLink
@@ -97,3 +97,29 @@ test('does not render DeckglLayerVisibilityTooltip for standard filter type', ()
screen.queryByTestId('deckgl-layer-visibility-tooltip-icon'),
).not.toBeInTheDocument();
});
test('does not mark a defaultToFirstItem-only filter as required', () => {
render(
<FilterControl
filter={{
...nativeFilter,
controlValues: { defaultToFirstItem: true },
}}
onFilterSelectionChange={jest.fn()}
/>,
);
expect(screen.queryByText('*')).not.toBeInTheDocument();
});
test('marks an enableEmptyFilter filter as required', () => {
render(
<FilterControl
filter={{
...nativeFilter,
controlValues: { enableEmptyFilter: true },
}}
onFilterSelectionChange={jest.fn()}
/>,
);
expect(screen.getByText('*')).toBeInTheDocument();
});
@@ -65,9 +65,7 @@ const FilterControl = ({
isFilterInScope(filter) &&
checkIsMissingRequiredValue(filter, filter.dataMask?.filterState);
const validateStatus = isMissingRequiredValue ? 'error' : undefined;
const isRequired =
!!filter.controlValues?.enableEmptyFilter ||
!!filter.controlValues?.defaultToFirstItem;
const isRequired = !!filter.controlValues?.enableEmptyFilter;
const inverseSelection = !!filter.controlValues?.inverseSelection;
const {
@@ -211,6 +211,16 @@ test('checkIsMissingRequiredValue returns false for non-required filter with und
expect(checkIsMissingRequiredValue(filter, filterState)).toBe(false);
});
test('checkIsMissingRequiredValue returns false when only defaultToFirstItem is set', () => {
const filter = createFilter('test-filter', {
enableEmptyFilter: false,
controlValues: { defaultToFirstItem: true },
});
expect(checkIsMissingRequiredValue(filter, { value: null })).toBe(false);
expect(checkIsMissingRequiredValue(filter, { value: undefined })).toBe(false);
});
test('checkIsMissingRequiredValue returns falsy for filter without controlValues', () => {
const filter = { id: 'test-filter' } as Filter;
const filterState: FilterState = { value: undefined };
@@ -299,6 +309,48 @@ test('checkIsApplyDisabled returns true when required filter is missing value in
);
});
test('checkIsApplyDisabled enables Apply after clearing a cascading defaultToFirstItem child', () => {
// Regression: a child filter that is dependent on a parent and configured with
// "Select first filter value by default" but NOT "Filter value is required"
// must stay clearable — clearing it may not disable Apply.
const parent = createFilter('parent', {
enableEmptyFilter: true,
controlValues: { defaultToFirstItem: true },
});
const child = createFilter('child', {
enableEmptyFilter: false,
controlValues: { defaultToFirstItem: true },
});
const dataMaskSelected: DataMaskStateWithId = {
parent: {
id: 'parent',
filterState: { value: ['USA'] },
extraFormData: createExtraFormDataWithFilter('country', ['USA']),
},
child: {
id: 'child',
filterState: { value: null },
extraFormData: {},
},
};
const dataMaskApplied: DataMaskStateWithId = {
parent: {
id: 'parent',
filterState: { value: ['USA'] },
extraFormData: createExtraFormDataWithFilter('country', ['USA']),
},
child: {
id: 'child',
filterState: { value: ['CA'] },
extraFormData: createExtraFormDataWithFilter('state', ['CA']),
},
};
expect(
checkIsApplyDisabled(dataMaskSelected, dataMaskApplied, [parent, child]),
).toBe(false);
});
test('checkIsApplyDisabled enables Apply when Selected has a filter value not yet in Applied', () => {
// Regression: when a required filter's default isn't applied (Applied missing
// the entry) and the user types a value, Selected gains an entry Applied
@@ -48,9 +48,10 @@ export const checkIsMissingRequiredValue = (
filter: FilterElement,
filterState?: FilterState,
) => {
const isRequired =
!!filter.controlValues?.enableEmptyFilter ||
!!filter.controlValues?.defaultToFirstItem;
// Only `enableEmptyFilter` ("Filter value is required") makes a value
// mandatory. `defaultToFirstItem` merely seeds an initial selection, so a
// filter cleared by the user must stay clearable, with Apply enabled.
const isRequired = !!filter.controlValues?.enableEmptyFilter;
if (!isRequired) return false;
+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;
@@ -16,7 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import {
render,
screen,
waitFor,
userEvent,
fireEvent,
} from 'spec/helpers/testing-library';
import Control, { ControlProps } from 'src/explore/components/Control';
const defaultProps: ControlProps = {
@@ -77,3 +83,72 @@ test('call setControlValue if isVisible is false', async () => {
expect(defaultProps.actions.setControlValue).toHaveBeenCalled(),
);
});
test('shows the description icon while the control is hovered', async () => {
render(
setup({
label: 'My checkbox',
description: 'Help text',
}),
);
expect(
screen.queryByRole('button', { name: 'Show info tooltip' }),
).not.toBeInTheDocument();
await userEvent.hover(screen.getByTestId('checkbox'));
expect(
screen.getByRole('button', { name: 'Show info tooltip' }),
).toBeInTheDocument();
await userEvent.unhover(screen.getByTestId('checkbox'));
expect(
screen.queryByRole('button', { name: 'Show info tooltip' }),
).not.toBeInTheDocument();
});
test('shows the description icon while the control has keyboard focus', () => {
render(
setup({
label: 'My checkbox',
description: 'Help text',
}),
);
expect(
screen.queryByRole('button', { name: 'Show info tooltip' }),
).not.toBeInTheDocument();
fireEvent.focus(screen.getByRole('checkbox'));
const infoIcon = screen.getByRole('button', { name: 'Show info tooltip' });
expect(infoIcon).toBeInTheDocument();
fireEvent.blur(screen.getByRole('checkbox'), { relatedTarget: infoIcon });
expect(
screen.getByRole('button', { name: 'Show info tooltip' }),
).toBeInTheDocument();
fireEvent.blur(infoIcon, { relatedTarget: document.body });
expect(
screen.queryByRole('button', { name: 'Show info tooltip' }),
).not.toBeInTheDocument();
});
test('keeps the description icon visible when the pointer leaves a focused control', () => {
render(
setup({
label: 'My checkbox',
description: 'Help text',
}),
);
fireEvent.focus(screen.getByRole('checkbox'));
expect(
screen.getByRole('button', { name: 'Show info tooltip' }),
).toBeInTheDocument();
fireEvent.mouseLeave(screen.getByTestId('checkbox'));
expect(
screen.getByRole('button', { name: 'Show info tooltip' }),
).toBeInTheDocument();
});
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { ReactNode, useCallback, useState, useEffect } from 'react';
import { ReactNode, useCallback, useState, useEffect, FocusEvent } from 'react';
import { isEqual } from 'lodash-es';
import {
ControlType,
@@ -70,7 +70,18 @@ export default function Control(props: ControlProps) {
} = props;
const [hovered, setHovered] = useState(false);
const [focused, setFocused] = useState(false);
const wasVisible = usePrevious(isVisible);
const handleBlur = (event: FocusEvent<HTMLDivElement>) => {
if (
!(event.relatedTarget instanceof Node) ||
!event.currentTarget.contains(event.relatedTarget)
) {
setFocused(false);
}
};
const onChange = useCallback(
(value: any, errors: any[]) => setControlValue(name, value, errors),
[name, setControlValue],
@@ -119,9 +130,15 @@ export default function Control(props: ControlProps) {
style={hidden ? { display: 'none' } : undefined}
onMouseEnter={() => setHovered(true)}
onMouseLeave={() => setHovered(false)}
onFocus={() => setFocused(true)}
onBlur={handleBlur}
>
<ErrorBoundary>
<ControlComponent onChange={onChange} hovered={hovered} {...props} />
<ControlComponent
onChange={onChange}
hovered={hovered || focused}
{...props}
/>
</ErrorBoundary>
</StyledControl>
);
@@ -0,0 +1,112 @@
/**
* 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 {
render,
screen,
userEvent,
fireEvent,
} from 'spec/helpers/testing-library';
import ControlHeader from './ControlHeader';
const description = 'This control filters the whole chart.';
test('does not render the description icon until the control is hovered', () => {
const { rerender } = render(
<ControlHeader
name="time_range"
label="Date Range"
description={description}
/>,
);
expect(
screen.queryByRole('button', { name: 'Show info tooltip' }),
).not.toBeInTheDocument();
rerender(
<ControlHeader
name="time_range"
label="Date Range"
description={description}
hovered
/>,
);
expect(
screen.getByRole('button', { name: 'Show info tooltip' }),
).toBeInTheDocument();
});
test('notifies onDescriptionHoverChange when the info icon is hovered', async () => {
const onDescriptionHoverChange = jest.fn();
render(
<ControlHeader
name="time_range"
label="Date Range"
description={description}
hovered
onDescriptionHoverChange={onDescriptionHoverChange}
/>,
);
const infoIcon = screen.getByRole('button', { name: 'Show info tooltip' });
await userEvent.hover(infoIcon);
expect(onDescriptionHoverChange).toHaveBeenCalledWith(true);
await userEvent.unhover(infoIcon);
expect(onDescriptionHoverChange).toHaveBeenCalledWith(false);
});
test('notifies onDescriptionHoverChange when the info icon is focused', () => {
const onDescriptionHoverChange = jest.fn();
render(
<ControlHeader
name="time_range"
label="Date Range"
description={description}
hovered
onDescriptionHoverChange={onDescriptionHoverChange}
/>,
);
const infoIcon = screen.getByRole('button', { name: 'Show info tooltip' });
fireEvent.focus(infoIcon);
expect(onDescriptionHoverChange).toHaveBeenCalledWith(true);
fireEvent.blur(infoIcon);
expect(onDescriptionHoverChange).toHaveBeenCalledWith(false);
});
test('activates tooltipOnClick from the keyboard', () => {
const tooltipOnClick = jest.fn();
render(
<ControlHeader
name="time_range"
label="Date Range"
description={description}
hovered
tooltipOnClick={tooltipOnClick}
/>,
);
fireEvent.keyDown(screen.getByRole('button', { name: 'Show info tooltip' }), {
key: 'Enter',
});
expect(tooltipOnClick).toHaveBeenCalledTimes(1);
});
@@ -38,6 +38,7 @@ export type ControlHeaderProps = {
tooltipOnClick?: () => void;
warning?: string;
danger?: string;
onDescriptionHoverChange?: (hovered: boolean) => void;
// Allow extra props from control spread patterns (e.g. {...this.props})
[key: string]: unknown;
};
@@ -71,6 +72,7 @@ const ControlHeader: FC<ControlHeaderProps> = ({
tooltipOnClick = () => {},
warning,
danger,
onDescriptionHoverChange,
}) => {
const theme = useTheme();
@@ -89,24 +91,44 @@ const ControlHeader: FC<ControlHeaderProps> = ({
position: absolute;
top: 50%;
right: 0;
z-index: 1;
padding-left: ${theme.sizeUnit}px;
transform: translate(100%, -50%);
white-space: nowrap;
pointer-events: auto;
`}
>
{description && (
<span>
<>
<Tooltip
id="description-tooltip"
title={description}
placement="top"
mouseLeaveDelay={0}
trigger={['hover', 'focus']}
>
<Icons.InfoCircleOutlined
css={iconStyles}
{/* Same role="button" pattern as the label text: a real <button>
is not valid inside FormLabel's <label>. */}
<span
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role
role="button"
tabIndex={0}
data-test={`${name}-description-icon`}
aria-label={t('Show info tooltip')}
onMouseEnter={() => onDescriptionHoverChange?.(true)}
onMouseLeave={() => onDescriptionHoverChange?.(false)}
onFocus={() => onDescriptionHoverChange?.(true)}
onBlur={() => onDescriptionHoverChange?.(false)}
onClick={tooltipOnClick}
/>
onKeyDown={handleKeyboardActivation(tooltipOnClick)}
css={css`
cursor: pointer;
`}
>
<Icons.InfoCircleOutlined css={iconStyles} />
</span>
</Tooltip>{' '}
</span>
</>
)}
{renderTrigger && (
<span>
@@ -68,6 +68,8 @@ export const TableControls = ({
canDownload,
rowLimit,
rowLimitOptions,
effectiveRowLimit,
limitReachedMessage,
onRowLimitChange,
onDownloadCSV,
onDownloadXLSX,
@@ -111,14 +113,19 @@ export const TableControls = ({
value={rowLimit}
onChange={onRowLimitChange}
options={rowLimitOptions ?? []}
// Labelled as the applied limit to avoid a second row count next to RowCountLabel.
prefix={t('Limit')}
css={css`
min-width: 110px;
min-width: 160px;
`}
/>
)}
{(!onRowLimitChange || rowcount < (rowLimit ?? Infinity)) && (
<RowCountLabel rowcount={rowcount} loading={isLoading} />
)}
<RowCountLabel
rowcount={rowcount}
limit={effectiveRowLimit ?? rowLimit}
limitReachedMessage={limitReachedMessage}
loading={isLoading}
/>
{canDownload && onDownloadCSV && onDownloadXLSX && (
<DownloadDropdown
onDownloadCSV={onDownloadCSV}
@@ -136,6 +136,12 @@ export const SamplesPane = ({
const columns = useGridColumns(colnames, coltypes, data);
const keywordFilter = useKeywordFilter(filterText);
// Samples aren't capped by a chart's row_limit, just this pane's own
// page-size selector, so RowCountLabel's default "chart" wording is wrong here.
const limitReachedMessage = t(
'The sample row limit was reached. This %s may contain more rows.',
datasetLabelLower(),
);
const handleInputChange = useCallback(
(input: string) => setFilterText(input),
@@ -161,6 +167,7 @@ export const SamplesPane = ({
canDownload={canDownload}
rowLimit={rowLimit}
rowLimitOptions={ROW_LIMIT_OPTIONS}
limitReachedMessage={limitReachedMessage}
onRowLimitChange={handleRowLimitChange}
/>
<ErrorAlertWrapper>
@@ -197,6 +204,7 @@ export const SamplesPane = ({
canDownload={canDownload}
rowLimit={rowLimit}
rowLimitOptions={ROW_LIMIT_OPTIONS}
limitReachedMessage={limitReachedMessage}
onRowLimitChange={handleRowLimitChange}
/>
<GridContainer>
@@ -56,6 +56,8 @@ export const SingleQueryResultPane = ({
columnDisplayNames,
rowLimit,
rowLimitOptions,
effectiveRowLimit,
limitReachedMessage,
onRowLimitChange,
onDownloadCSV,
onDownloadXLSX,
@@ -86,6 +88,8 @@ export const SingleQueryResultPane = ({
canDownload={canDownload}
rowLimit={rowLimit}
rowLimitOptions={rowLimitOptions}
effectiveRowLimit={effectiveRowLimit}
limitReachedMessage={limitReachedMessage}
onRowLimitChange={onRowLimitChange}
onDownloadCSV={onDownloadCSV}
onDownloadXLSX={onDownloadXLSX}
@@ -84,6 +84,17 @@ export const useResultsPane = ({
// Never exceed the chart's own row_limit
const effectiveRowLimit = Math.min(rowLimit, chartRowLimit);
// When this pane's own row-limit selector is stricter than the chart's
// row_limit, it - not the chart - is what caps the result, so
// RowCountLabel's default "chart" wording would be misleading (the chart's
// configured row_limit was never actually reached).
const limitReachedMessage =
rowLimit < chartRowLimit
? t(
'The row limit selected for this pane was reached. There may be more matching rows.',
)
: undefined;
const cappedFormData = useMemo(
() => ({ ...queryFormData, row_limit: effectiveRowLimit }),
[queryFormData, effectiveRowLimit],
@@ -236,6 +247,8 @@ export const useResultsPane = ({
columnDisplayNames={columnDisplayNames}
rowLimit={rowLimit}
rowLimitOptions={ROW_LIMIT_OPTIONS}
effectiveRowLimit={effectiveRowLimit}
limitReachedMessage={limitReachedMessage}
onRowLimitChange={handleRowLimitChange}
/>
</StyledDiv>
@@ -0,0 +1,94 @@
/**
* 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 {
act,
render,
screen,
sleep,
userEvent,
} from 'spec/helpers/testing-library';
import { GenericDataType } from '@apache-superset/core/common';
import {
TableControls,
ROW_LIMIT_OPTIONS,
} from '../components/DataTableControls';
import { TableControlsProps } from '../types';
const setup = (overrides: Partial<TableControlsProps> = {}) =>
render(
<TableControls
data={[]}
columnNames={['name']}
columnTypes={[GenericDataType.String]}
rowcount={0}
onInputChange={jest.fn()}
isLoading={false}
canDownload
rowLimit={100}
rowLimitOptions={ROW_LIMIT_OPTIONS}
onRowLimitChange={jest.fn()}
{...overrides}
/>,
{ useRedux: true },
);
test('shows the row count when the result fills the selected row limit', () => {
setup({ rowcount: 100, rowLimit: 100 });
expect(screen.getByTestId('row-count-label')).toHaveTextContent('100 rows');
});
test('warns that the row limit was reached when the result fills it', async () => {
setup({ rowcount: 100, rowLimit: 100 });
userEvent.hover(screen.getByTestId('row-count-label'));
expect(await screen.findByRole('tooltip')).toHaveTextContent(
'The row limit set for the chart was reached',
);
});
test('does not warn when the result is smaller than the selected row limit', async () => {
setup({ rowcount: 42, rowLimit: 100 });
expect(screen.getByTestId('row-count-label')).toHaveTextContent('42 rows');
userEvent.hover(screen.getByTestId('row-count-label'));
// Wait past antd's 0.1s mouseEnterDelay so a regression that made the
// tooltip appear would be caught here instead of racing the delay.
await act(() => sleep(150));
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument();
});
test("warns when the chart's own row limit truncates below the selected row limit", async () => {
setup({ rowcount: 250, rowLimit: 1000, effectiveRowLimit: 250 });
userEvent.hover(screen.getByTestId('row-count-label'));
expect(await screen.findByRole('tooltip')).toHaveTextContent(
'The row limit set for the chart was reached',
);
});
test('labels the row limit selector so it is not read as a second row count', () => {
setup({ rowcount: 100, rowLimit: 100 });
expect(screen.getByText('Limit')).toBeInTheDocument();
});
@@ -16,7 +16,12 @@
* specific language governing permissions and limitations
* under the License.
*/
import { screen, render, waitFor } from 'spec/helpers/testing-library';
import {
screen,
render,
waitFor,
userEvent,
} from 'spec/helpers/testing-library';
import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact';
import { getChartDataRequest } from 'src/components/Chart/chartAction';
import { ResultsPaneOnDashboard } from '../components';
@@ -157,6 +162,42 @@ describe('useResultsPane query data reuse', () => {
expect(screen.queryByText('Sci-Fi')).not.toBeInTheDocument();
expect(screen.getByText('2 rows')).toBeVisible();
expect(mockedGetChartDataRequest).not.toHaveBeenCalled();
userEvent.hover(screen.getByText('2 rows'));
expect(await screen.findByRole('tooltip')).toHaveTextContent(
'The row limit set for the chart was reached',
);
});
test("warns about this pane's own row limit, not the chart's, when the pane's selector is what caps the result", async () => {
// chart row_limit (2000) is well above this pane's default 1000-row
// selector, so the selector - not the chart - is what truncates here.
const props = createResultsPaneOnDashboardProps({
sliceId: 208,
rowLimit: 2000,
queriesResponse: [
{
colnames: ['genre'],
coltypes: [1],
data: Array.from({ length: 1500 }, (_, i) => ({
genre: `genre-${i}`,
})),
rowcount: 1500,
},
],
});
render(<ResultsPaneOnDashboard {...props} />, { useRedux: true });
const rowCountLabel = await screen.findByTestId('row-count-label');
expect(rowCountLabel).toHaveTextContent('1k rows');
userEvent.hover(rowCountLabel);
const tooltip = await screen.findByRole('tooltip');
expect(tooltip).toHaveTextContent(
'The row limit selected for this pane was reached',
);
expect(tooltip).not.toHaveTextContent('for the chart');
});
test('renders an empty (0 rows) result from reused data without an API call', async () => {
@@ -84,6 +84,12 @@ export interface TableControlsProps extends DrillControlsProps {
canDownload: boolean;
rowLimit?: number;
rowLimitOptions?: { value: number; label: string }[];
// Effective result limit, capped by the chart's row limit.
// Defaults to `rowLimit` and controls the "row limit reached" warning.
effectiveRowLimit?: number;
// Overrides RowCountLabel's default "chart" wording for panes (e.g.
// samples) where the limit reached isn't the chart's own row_limit.
limitReachedMessage?: React.ReactNode;
onRowLimitChange?: (limit: number) => void;
}
@@ -104,5 +110,9 @@ export interface SingleQueryResultPaneProp
columnDisplayNames?: Record<string, string>;
rowLimit?: number;
rowLimitOptions?: { value: number; label: string }[];
effectiveRowLimit?: number;
// Overrides RowCountLabel's default "chart" wording when the pane's own
// row-limit selector, not the chart's row_limit, is what capped the result.
limitReachedMessage?: React.ReactNode;
onRowLimitChange?: (limit: number) => void;
}
@@ -16,7 +16,14 @@
* specific language governing permissions and limitations
* under the License.
*/
import { ChangeEvent, useMemo, useState, useCallback, useEffect } from 'react';
import {
type ReactNode,
ChangeEvent,
useMemo,
useState,
useCallback,
useEffect,
} from 'react';
import {
Input,
@@ -56,6 +63,12 @@ export type PropertiesModalProps = {
permissionsError?: string;
addSuccessToast: (msg: string) => void;
addDangerToast: (msg: string) => void;
/** Optional render prop for injecting extra fields (e.g. folder selector). */
renderExtraFields?: (context: {
assetId: number;
assetType: 'chart';
accessorCount: number;
}) => { content: ReactNode; saveDisabled?: boolean; saveTooltip?: string };
};
function PropertiesModal({
@@ -65,6 +78,7 @@ function PropertiesModal({
show,
addSuccessToast,
addDangerToast,
renderExtraFields,
}: PropertiesModalProps) {
const [submitting, setSubmitting] = useState(false);
// values of form inputs
@@ -87,6 +101,25 @@ function PropertiesModal({
>(null);
const [tags, setTags] = useState<TagType[]>([]);
const chartId = slice.slice_id;
const extraFields = useMemo(
() =>
chartId
? renderExtraFields?.({
assetId: chartId,
assetType: 'chart',
accessorCount:
(selectedEditors?.length ?? 0) + (selectedViewers?.length ?? 0),
})
: undefined,
[
chartId,
renderExtraFields,
selectedEditors?.length,
selectedViewers?.length,
],
);
// Validation setup
const modalSections = useMemo(
() => [
@@ -281,14 +314,20 @@ function PropertiesModal({
title={t('Chart properties')}
isEditMode
saveDisabled={
submitting || !name || slice.is_managed_externally || hasErrors
submitting ||
!name ||
slice.is_managed_externally ||
hasErrors ||
extraFields?.saveDisabled
}
errorTooltip={
slice.is_managed_externally
? t(
"This chart is managed externally, and can't be edited in Superset",
)
: errorTooltip
extraFields?.saveDisabled && extraFields?.saveTooltip
? extraFields.saveTooltip
: slice.is_managed_externally
? t(
"This chart is managed externally, and can't be edited in Superset",
)
: errorTooltip
}
wrapProps={{ 'data-test': 'properties-edit-modal' }}
>
@@ -395,6 +434,7 @@ function PropertiesModal({
/>
</ModalFormField>
)}
{extraFields?.content}
</>
),
},
@@ -29,7 +29,9 @@ 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>;
/**
@@ -147,6 +147,7 @@ export default function DateFilterLabel(props: DateFilterControlProps) {
onOpenPopover = noOp,
onClosePopover = noOp,
isOverflowingFilterBar = false,
hovered: isControlHovered = false,
} = props;
const defaultTimeFilter = useDefaultTimeFilter();
@@ -161,9 +162,16 @@ export default function DateFilterLabel(props: DateFilterControlProps) {
const [validTimeRange, setValidTimeRange] = useState<boolean>(false);
const [evalResponse, setEvalResponse] = useState<string>(value);
const [tooltipTitle, setTooltipTitle] = useState<ReactNode | null>(t(value));
const [isDescriptionHovered, setIsDescriptionHovered] = useState(false);
const theme = useTheme();
const [labelRef, labelIsTruncated] = useCSSTextTruncation<HTMLSpanElement>();
useEffect(() => {
if (!isControlHovered) {
setIsDescriptionHovered(false);
}
}, [isControlHovered]);
useEffect(() => {
if (value === NO_TIME_RANGE) {
setActualTimeRange(NO_TIME_RANGE);
@@ -368,7 +376,12 @@ export default function DateFilterLabel(props: DateFilterControlProps) {
}
overlayClassName="time-range-popover"
>
<Tooltip placement="top" title={tooltipTitle}>
<Tooltip
placement="top"
title={isDescriptionHovered ? null : tooltipTitle}
mouseLeaveDelay={0}
overlayStyle={{ pointerEvents: 'none' }}
>
{/* Wrap in a span so the Popover gets a stable DOM ref target;
DateLabel forwards its ref to an inner span used for measuring
text truncation, which would otherwise become the popover's
@@ -390,7 +403,10 @@ export default function DateFilterLabel(props: DateFilterControlProps) {
return (
<>
<ControlHeader {...props} />
<ControlHeader
{...props}
onDescriptionHoverChange={setIsDescriptionHovered}
/>
{popoverContent}
</>
);
@@ -18,16 +18,35 @@
*/
import thunk from 'redux-thunk';
import { Provider } from 'react-redux';
import configureStore from 'redux-mock-store';
import configureMockStore from 'redux-mock-store';
import { render, screen, userEvent } from 'spec/helpers/testing-library';
import {
render,
screen,
userEvent,
waitFor,
fireEvent,
} from 'spec/helpers/testing-library';
import { NO_TIME_RANGE } from '@superset-ui/core';
import { NO_TIME_RANGE, fetchTimeRange } from '@superset-ui/core';
import DateFilterLabel from '..';
import { DateFilterControlProps } from '../types';
import { DateFilterTestKey } from '../utils';
const mockStore = configureStore([thunk]);
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
fetchTimeRange: jest.fn(),
}));
const mockedFetchTimeRange = fetchTimeRange as jest.MockedFunction<
typeof fetchTimeRange
>;
const FIELD_TOOLTIP = '2024-01-01 ≤ col < 2024-01-08';
const DESCRIPTION_TOOLTIP =
'This control filters the whole chart based on the selected time range.';
const mockStore = configureMockStore([thunk]);
const defaultProps = {
onChange: jest.fn(),
@@ -35,6 +54,11 @@ const defaultProps = {
onOpenPopover: jest.fn(),
};
beforeEach(() => {
mockedFetchTimeRange.mockReset();
mockedFetchTimeRange.mockResolvedValue({ value: FIELD_TOOLTIP });
});
function setup(
props: Omit<DateFilterControlProps, 'name'> = defaultProps,
store: any = mockStore({}),
@@ -136,3 +160,61 @@ test('DateFilter should properly handle isOverflowingFilterBar prop changes', ()
expect(popoverAfterRerender?.parentElement).toBe(trigger.parentElement);
expect(popoverAfterRerender?.parentElement).not.toBe(document.body);
});
test('hovering the description icon does not show the date range tooltip', async () => {
const tooltipOnClick = jest.fn();
render(
setup({
...defaultProps,
value: 'Last week',
label: 'Date Range',
description: DESCRIPTION_TOOLTIP,
hovered: true,
tooltipOnClick,
}),
);
await waitFor(() => {
expect(screen.getByText('Last week')).toBeInTheDocument();
});
await userEvent.hover(screen.getByText('Last week'));
expect(await screen.findByRole('tooltip')).toHaveTextContent(FIELD_TOOLTIP);
const descriptionIcon = screen.getByRole('button', {
name: 'Show info tooltip',
});
fireEvent.focus(descriptionIcon);
await waitFor(() => {
expect(screen.getByRole('tooltip')).toHaveTextContent(DESCRIPTION_TOOLTIP);
expect(screen.getByRole('tooltip')).not.toHaveTextContent(FIELD_TOOLTIP);
expect(screen.getAllByRole('tooltip')).toHaveLength(1);
});
fireEvent.blur(descriptionIcon);
await waitFor(() => {
expect(screen.getByRole('tooltip')).toHaveTextContent(FIELD_TOOLTIP);
expect(screen.getAllByRole('tooltip')).toHaveLength(1);
});
await userEvent.unhover(screen.getByText('Last week'));
await waitFor(() => {
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument();
});
await userEvent.hover(descriptionIcon);
const tooltip = await screen.findByRole('tooltip');
expect(tooltip).toHaveTextContent(DESCRIPTION_TOOLTIP);
expect(tooltip).not.toHaveTextContent(FIELD_TOOLTIP);
expect(screen.getAllByRole('tooltip')).toHaveLength(1);
await userEvent.unhover(descriptionIcon);
await waitFor(() => {
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument();
});
fireEvent.keyDown(descriptionIcon, { key: 'Enter' });
expect(tooltipOnClick).toHaveBeenCalled();
});
@@ -16,6 +16,8 @@
* specific language governing permissions and limitations
* under the License.
*/
import { ReactNode } from 'react';
export type SelectOptionType = {
value: string;
label: string;
@@ -113,4 +115,8 @@ export interface DateFilterControlProps {
onOpenPopover?: () => void;
onClosePopover?: () => void;
isOverflowingFilterBar?: boolean;
hovered?: boolean;
description?: ReactNode;
label?: ReactNode;
tooltipOnClick?: () => void;
}
@@ -951,3 +951,167 @@ 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,13 +16,25 @@
* specific language governing permissions and limitations
* under the License.
*/
import { FC, ChangeEvent, useEffect, useState, useRef } from 'react';
import {
FC,
ChangeEvent,
useCallback,
useEffect,
useMemo,
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';
@@ -57,7 +69,7 @@ import { useDatePickerInAdhocFilter } from '../utils';
import { useDefaultTimeFilter } from '../../DateFilterControl/utils';
import { Clauses, ExpressionTypes } from '../types';
const SelectWithLabel = styled(Select)<{ labelText: string }>`
const SelectWithLabel = styled(AsyncSelect)<{ labelText: string }>`
.ant-select-content::after {
content: ${({ labelText }) => labelText || '\\A0'};
display: inline-block;
@@ -67,6 +79,30 @@ const SelectWithLabel = styled(Select)<{ 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;
@@ -347,11 +383,9 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
} = useSimpleTabFilterProps(props);
const [comparator, setComparator] = useState(props.adhocFilter.comparator);
const comparatorInputRef = useRef<InputRef | null>(null);
const [suggestions, setSuggestions] = useState<
Record<'label' | 'value', any>[]
>([]);
const [loadingComparatorSuggestions, setLoadingComparatorSuggestions] =
useState<boolean>(false);
const comparatorSelectRef = useRef<AsyncSelectRef>(null);
const [loadedOptionCount, setLoadedOptionCount] = useState(0);
const [optionsTruncated, setOptionsTruncated] = useState(false);
const [hasFocusedComparator, setHasFocusedComparator] =
useState<boolean>(false);
@@ -387,18 +421,8 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
/>
);
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 createSuggestionsPlaceholder = () =>
loadedOptionCount ? t('%s option(s)', loadedOptionCount) : '';
const handleSubjectChange = (subject: string) => {
setComparator(undefined);
@@ -455,21 +479,63 @@ 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)) ||
suggestions.length > 0;
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],
);
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),
loading: loadingComparatorSuggestions,
value: comparator as SelectValue,
onChange: onComparatorChange,
value: comparatorSelectValue as SelectValue,
onChange: handleComparatorChange,
notFoundContent: t('Type a value here'),
placeholder: createSuggestionsPlaceholder(),
};
@@ -495,76 +561,89 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
onChange: onDatePickerChange,
});
useEffect(() => {
const refreshComparatorSuggestions = () => {
const { datasource } = props;
const col = props.adhocFilter.subject;
const having = props.adhocFilter.clause === Clauses.Having;
// 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;
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);
});
// 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 };
}
};
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,
]);
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]);
useEffect(() => {
if (isFeatureEnabled(FeatureFlag.EnableAdvancedDataTypes)) {
@@ -670,11 +749,12 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
}
>
<SelectWithLabel
ref={comparatorSelectRef}
css={css`
margin-top: ${theme.marginXS}px;
`}
labelText={labelText}
options={suggestions}
options={loadComparatorOptions}
{...comparatorSelectProps}
/>
</Tooltip>
@@ -16,10 +16,12 @@
* specific language governing permissions and limitations
* under the License.
*/
import { useCallback, useState } from 'react';
import {
AppSection,
Behavior,
ChartProps,
type DataMask,
type FilterState,
} from '@superset-ui/core';
import { supersetTheme } from '@apache-superset/core/theme';
@@ -877,10 +879,41 @@ describe('SelectFilterPlugin', () => {
expect(await screen.findByTitle('brand-new')).toBeInTheDocument();
});
test('does not show create option when searchAllOptions is true', () => {
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.
getWrapper({ creatable: true, searchAllOptions: true });
userEvent.type(screen.getByRole('combobox'), 'brand-new');
expect(screen.queryByTitle('brand-new')).not.toBeInTheDocument();
expect(await screen.findByTitle('brand-new')).toBeInTheDocument();
});
});
@@ -1393,6 +1426,153 @@ test('preserves dependent filter value restored from URL when it exists in data'
});
});
test('keeps a dependent filter empty after the user clears it', async () => {
// Regression: a dependent filter with "Select first filter value by default"
// used to re-apply the first option as soon as the cleared value round-tripped
// through the filter bar, making it impossible to clear.
jest.useRealTimers();
const setDataMaskMock = jest.fn();
const testProps = {
...selectMultipleProps,
formData: {
...selectMultipleProps.formData,
multiSelect: false,
enableEmptyFilter: false,
defaultToFirstItem: true,
// Non-empty extraFormData is what marks this filter as dependent
extraFormData: {
filters: [{ col: 'region', op: 'IN', val: ['North America'] }],
},
},
};
// The filter bar feeds every dispatched dataMask back into the plugin as the
// controlled `filterState` prop; the harness reproduces that round-trip.
const ControlledSelectFilter = () => {
const [filterState, setFilterState] = useState<FilterState>({
value: ['boy'],
});
const handleDataMask = useCallback((dataMask: DataMask) => {
setDataMaskMock(dataMask);
setFilterState(prev => ({ ...prev, ...dataMask.filterState }));
}, []);
return (
// @ts-expect-error
<SelectFilterPlugin
// @ts-expect-error
{...transformProps({ ...testProps, filterState })}
setDataMask={handleDataMask}
showOverflow={false}
/>
);
};
render(<ControlledSelectFilter />, {
useRedux: true,
initialState: {
nativeFilters: {
filters: {
'test-filter': {
name: 'Test Filter',
},
},
},
dataMask: {
'test-filter': {
extraFormData: {},
filterState: { value: ['boy'] },
},
},
},
});
userEvent.click(
screen.getByRole('img', {
name: /close-circle/i,
hidden: true,
}),
);
await waitFor(() =>
expect(setDataMaskMock).toHaveBeenLastCalledWith(
expect.objectContaining({
extraFormData: {},
filterState: expect.objectContaining({ value: null }),
}),
),
);
// Let the re-validation effects settle: the value must not come back
await act(async () => {
await Promise.resolve();
});
expect(setDataMaskMock).toHaveBeenLastCalledWith(
expect.objectContaining({
filterState: expect.objectContaining({ value: null }),
}),
);
expect(screen.queryByTitle('boy')).not.toBeInTheDocument();
});
test('keeps a dependent filter empty when it mounts with a cleared value', async () => {
// Regression: after a reload the cleared state comes back as `value: null` on
// a fresh component, so the in-memory "user cleared this" ref is gone. The
// first item must still not be re-applied.
const setDataMaskMock = jest.fn();
const testProps = {
...selectMultipleProps,
formData: {
...selectMultipleProps.formData,
multiSelect: false,
enableEmptyFilter: false,
defaultToFirstItem: true,
extraFormData: {
filters: [{ col: 'region', op: 'IN', val: ['North America'] }],
},
},
filterState: { value: null },
};
render(
// @ts-expect-error
<SelectFilterPlugin
// @ts-expect-error
{...transformProps(testProps)}
setDataMask={setDataMaskMock}
showOverflow={false}
/>,
{
useRedux: true,
initialState: {
nativeFilters: {
filters: {
'test-filter': {
name: 'Test Filter',
},
},
},
dataMask: {
'test-filter': {
extraFormData: {},
filterState: { value: null },
},
},
},
},
);
// Let the re-validation effect run before asserting it did nothing
await act(async () => {
await Promise.resolve();
});
expect(setDataMaskMock).toHaveBeenCalled();
expect(setDataMaskMock).not.toHaveBeenCalledWith(
expect.objectContaining({
filterState: expect.objectContaining({ value: ['boy'] }),
}),
);
});
test('resets dependent filter to first item when value does not exist in data', async () => {
const setDataMaskMock = jest.fn();
const testProps = {
@@ -157,7 +157,6 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
const [col] = groupby;
const [initialColtypeMap] = useState(coltypeMap);
const [search, setSearch] = useState('');
const prevDataRef = useRef(data);
const userClearedRef = useRef(false);
const [dataMask, dispatchDataMask] = useImmerReducer(reducer, {
extraFormData: {},
@@ -272,7 +271,10 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
type: 'ownState',
ownState: {
coltypeMap: initialColtypeMap,
search,
// 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(),
},
});
}
@@ -282,8 +284,10 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
const handleBlur = useCallback(() => {
unsetFocusedFilter();
onSearch('');
}, [onSearch, unsetFocusedFilter]);
if (search) {
onSearch('');
}
}, [onSearch, search, unsetFocusedFilter]);
const handleChange = useCallback(
(value?: SelectValue | number | string) => {
@@ -305,6 +309,25 @@ 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 (
@@ -337,7 +360,6 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
const unquotedSearch = stripSurroundingQuotes(search);
if (
unquotedSearch &&
!searchAllOptions &&
creatable !== false &&
!hasOption(unquotedSearch, uniqueOptions, true)
) {
@@ -347,7 +369,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
];
}
return uniqueOptions;
}, [search, uniqueOptions, creatable, searchAllOptions]);
}, [search, uniqueOptions, creatable]);
const sortComparator = useCallback(
(a: LabeledValue, b: LabeledValue) => {
@@ -430,26 +452,6 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
clearAllTrigger,
]);
useEffect(() => {
const prev = prevDataRef.current;
const curr = data;
const hasDataChanged =
prev?.length !== curr?.length ||
prev?.some((row, i) => {
const prevVal = row[col];
const currVal = curr[i][col];
return typeof prevVal === 'bigint' || typeof currVal === 'bigint'
? prevVal?.toString() !== currVal?.toString()
: prevVal !== currVal;
});
// If data actually changed (e.g., due to parent filter), reset flag
if (hasDataChanged) {
prevDataRef.current = data;
}
}, [data, col]);
useEffect(() => {
if (
filterState.value?.every((value?: any) =>
@@ -462,13 +464,17 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
? (groupby.map(col => data[0][col]) as string[])
: null;
// Skip default value update when clearAllTrigger is active
// Skip default value update when clearAllTrigger is active.
// `null` is a persisted "user cleared this" state, as opposed to
// `undefined` for "never set", so it must not be re-defaulted either —
// `userClearedRef` alone would not survive a reload.
if (
!clearAllTrigger &&
defaultToFirstItem &&
!userClearedRef.current &&
Object.keys(formData?.extraFormData || {}).length &&
filterState.value !== undefined &&
filterState.value !== null &&
firstItem !== null &&
filterState.value !== firstItem
) {
@@ -634,7 +640,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
name={formData.nativeFilterId}
allowClear
autoClearSearchValue
allowNewOptions={!searchAllOptions && creatable !== false}
allowNewOptions={creatable !== false}
allowNewOptionsOnPaste={multiSelect && searchAllOptions}
allowSelectAll={!searchAllOptions}
value={multiSelect ? filterState.value || [] : filterState.value}
@@ -643,6 +649,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
showSearch={showSearch}
mode={multiSelect ? 'multiple' : 'single'}
placeholder={placeholderText}
helperText={helperText}
onClear={() => onSearch('')}
onSearch={onSearch}
onBlur={handleBlur}
@@ -117,6 +117,38 @@ 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,6 +54,13 @@ 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,
@@ -61,7 +68,7 @@ const buildQuery: BuildQuery<PluginFilterSelectQueryFormData> = (
metrics: sortMetric ? [sortMetric] : [],
filters: filters.concat(extraFilters),
orderby:
sortMetric || sortAscending !== undefined
!skipOrderBy && (sortMetric || sortAscending !== undefined)
? sortColumns.map(column => [column, !!sortAscending])
: [],
},
+1
View File
@@ -0,0 +1 @@
../superset-frontend/.npmrc
+10 -1
View File
@@ -585,8 +585,17 @@ class ChartDataRestApi(ChartRestApi):
query["timing"] = query_result.timing.as_public_dict()
if security_manager.is_guest_user():
# Guests may see the generated SQL only when the role attached to
# their guest token has been granted "can view query on Dashboard",
# mirroring the permission the frontend uses to expose the
# "View query" action. Stacktraces and driver errors stay redacted
# regardless, as those leak details of the deployment itself.
can_view_query = security_manager.can_access(
"can_view_query", "Dashboard"
)
for query in queries:
query.pop("query", None)
if not can_view_query:
query.pop("query", None)
query.pop("stacktrace", None)
if query.get("error"):
query["error"] = sanitize_error_message(query["error"])
+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 id: Tuple of chart IDs to migrate
:param ids: 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))
+11 -4
View File
@@ -91,8 +91,10 @@ class ExportChartsCommand(ExportModelsCommand):
def enable_tag_export(cls) -> None:
cls._include_tags = True
def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
yield from super().run()
def run(
self, seen: set[str] | None = None
) -> Iterator[tuple[str, Callable[[], str]]]:
yield from super().run(seen=seen)
# Tags are exported once for all requested charts (rather than per
# chart in `_export`) so a multi-chart export doesn't lose tags to
@@ -108,12 +110,17 @@ class ExportChartsCommand(ExportModelsCommand):
@staticmethod
def _export(
model: Slice, export_related: bool = True
model: Slice, export_related: bool = True, seen: set[str] | None = None
) -> Iterator[tuple[str, Callable[[], str]]]:
# Initialize seen set if not provided
if seen is None:
seen = set()
yield (
ExportChartsCommand._file_name(model),
lambda: ExportChartsCommand._file_content(model),
)
if model.table and export_related:
yield from ExportDatasetsCommand([model.table.id]).run()
# Pass the shared seen set to the dataset export command
yield from ExportDatasetsCommand([model.table.id]).run(seen=seen)
+20 -6
View File
@@ -383,8 +383,12 @@ class ExportDashboardsCommand(ExportModelsCommand):
@staticmethod
# ruff: noqa: C901
def _export(
model: Dashboard, export_related: bool = True
model: Dashboard, export_related: bool = True, seen: set[str] | None = None
) -> Iterator[tuple[str, Callable[[], str]]]:
# Initialize seen set if not provided
if seen is None:
seen = set()
yield (
ExportDashboardsCommand._file_name(model),
lambda: ExportDashboardsCommand._file_content(model),
@@ -395,8 +399,11 @@ class ExportDashboardsCommand(ExportModelsCommand):
dashboard_ids = model.id
command = ExportChartsCommand(chart_ids)
command.disable_tag_export()
yield from command.run()
command.enable_tag_export()
try:
# Pass the shared seen set to the chart export command
yield from command.run(seen=seen)
finally:
command.enable_tag_export()
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
yield from ExportTagsCommand(
dashboard_ids=dashboard_ids, chart_ids=chart_ids
@@ -406,7 +413,8 @@ class ExportDashboardsCommand(ExportModelsCommand):
if model.theme:
from superset.commands.theme.export import ExportThemesCommand
yield from ExportThemesCommand([model.theme.id]).run()
# Pass the shared seen set to the theme export command
yield from ExportThemesCommand([model.theme.id]).run(seen=seen)
payload = model.export_to_dict(
recursive=False,
@@ -435,7 +443,10 @@ class ExportDashboardsCommand(ExportModelsCommand):
if dataset_id is not None:
dataset = DatasetDAO.find_by_id(dataset_id)
if dataset:
yield from ExportDatasetsCommand([dataset_id]).run()
# Pass the shared seen set to the dataset export command
yield from ExportDatasetsCommand([dataset_id]).run(
seen=seen
)
# Export datasets referenced by display controls
for customization in (
@@ -446,4 +457,7 @@ class ExportDashboardsCommand(ExportModelsCommand):
if dataset_id is not None:
dataset = DatasetDAO.find_by_id(dataset_id)
if dataset:
yield from ExportDatasetsCommand([dataset_id]).run()
# Pass the shared seen set to the dataset export command
yield from ExportDatasetsCommand([dataset_id]).run(
seen=seen
)
+5 -1
View File
@@ -113,8 +113,12 @@ class ExportDatabasesCommand(ExportModelsCommand):
@staticmethod
def _export(
model: Database, export_related: bool = True
model: Database, export_related: bool = True, seen: set[str] | None = None
) -> Iterator[tuple[str, Callable[[], str]]]:
# Initialize seen set if not provided
if seen is None:
seen = set()
yield (
ExportDatabasesCommand._file_name(model),
lambda: ExportDatabasesCommand._file_content(model),
@@ -219,6 +219,8 @@ class UploadCommand(BaseCommand):
database_id=self._model_id,
editors=editors,
schema=self._schema,
# Ensure catalog is set
catalog=self._model.get_default_catalog(),
)
db.session.add(sqla_table)
+37 -24
View File
@@ -89,8 +89,12 @@ class ExportDatasetsCommand(ExportModelsCommand):
@staticmethod
def _export(
model: SqlaTable, export_related: bool = True
model: SqlaTable, export_related: bool = True, seen: set[str] | None = None
) -> Iterator[tuple[str, Callable[[], str]]]:
# Initialize seen set if not provided
if seen is None:
seen = set()
yield (
ExportDatasetsCommand._file_name(model),
lambda: ExportDatasetsCommand._file_content(model),
@@ -103,32 +107,41 @@ class ExportDatasetsCommand(ExportModelsCommand):
)
file_path = f"databases/{db_file_name}.yaml"
payload = model.database.export_to_dict(
recursive=False,
include_parent_ref=False,
include_defaults=True,
export_uuids=True,
)
# TODO (betodealmeida): move this logic to export_to_dict once this
# becomes the default export endpoint
if payload.get("extra"):
try:
payload["extra"] = json.loads(payload["extra"])
except json.JSONDecodeError:
logger.info("Unable to decode `extra` field: %s", payload["extra"])
if ssh_tunnel := model.database.ssh_tunnel:
ssh_tunnel_payload = ssh_tunnel.export_to_dict(
# Only yield the database file if not already seen. This is
# critical to fix the issue where databases were being
# duplicated and potentially overwritten when charts from
# different databases were exported.
if file_path not in seen:
payload = model.database.export_to_dict(
recursive=False,
include_parent_ref=False,
include_defaults=True,
export_uuids=False,
export_uuids=True,
)
payload["ssh_tunnel"] = mask_password_info(ssh_tunnel_payload)
# TODO (betodealmeida): move this logic to export_to_dict once this
# becomes the default export endpoint
if payload.get("extra"):
try:
payload["extra"] = json.loads(payload["extra"])
except json.JSONDecodeError:
logger.info(
"Unable to decode `extra` field: %s", payload["extra"]
)
payload["version"] = EXPORT_VERSION
if ssh_tunnel := model.database.ssh_tunnel:
ssh_tunnel_payload = ssh_tunnel.export_to_dict(
recursive=False,
include_parent_ref=False,
include_defaults=True,
export_uuids=False,
)
payload["ssh_tunnel"] = mask_password_info(ssh_tunnel_payload)
yield (
file_path,
lambda: yaml.safe_dump(payload, sort_keys=False, allow_unicode=True),
)
payload["version"] = EXPORT_VERSION
yield (
file_path,
lambda: yaml.safe_dump(
payload, sort_keys=False, allow_unicode=True
),
)
+2 -2
View File
@@ -384,7 +384,7 @@ class UpdateDatasetCommand(UpdateMixin, BaseCommand):
)
exceptions.append(
ValidationError(
message,
[message],
field_name=f"{label}.{idx}.expression",
)
)
@@ -412,7 +412,7 @@ class UpdateDatasetCommand(UpdateMixin, BaseCommand):
)
exceptions.append(
ValidationError(
message,
[message],
field_name="fetch_values_predicate",
)
)
+29 -11
View File
@@ -47,27 +47,45 @@ class ExportModelsCommand(BaseCommand):
@staticmethod
def _file_content(model: Model) -> str:
raise NotImplementedError("Subclasses MUST implement _export")
raise NotImplementedError("Subclasses MUST implement _file_content")
@staticmethod
def _export(
model: Model, export_related: bool = True
model: Model, export_related: bool = True, seen: set[str] | None = None
) -> Iterator[tuple[str, Callable[[], str]]]:
raise NotImplementedError("Subclasses MUST implement _export")
def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
def run(
self, seen: set[str] | None = None
) -> Iterator[tuple[str, Callable[[], str]]]:
self.validate()
metadata = {
"version": EXPORT_VERSION,
"type": self.dao.model_cls.__name__, # type: ignore
"timestamp": datetime.now(tz=timezone.utc).isoformat(),
}
yield METADATA_FILE_NAME, lambda: yaml.safe_dump(metadata, sort_keys=False)
# Use provided seen set or create new one
if seen is None:
seen = set()
should_add_metadata = True
else:
# If seen set is provided, we're being called from another command
should_add_metadata = False
# Only add metadata if this is the root command
if should_add_metadata:
metadata = {
"version": EXPORT_VERSION,
"type": self.dao.model_cls.__name__, # type: ignore
"timestamp": datetime.now(tz=timezone.utc).isoformat(),
}
if METADATA_FILE_NAME not in seen:
yield (
METADATA_FILE_NAME,
lambda: yaml.safe_dump(metadata, sort_keys=False),
)
seen.add(METADATA_FILE_NAME)
seen = {METADATA_FILE_NAME}
for model in self._models:
for file_name, file_content in self._export(model, self.export_related):
for file_name, file_content in self._export(
model, self.export_related, seen
):
if file_name not in seen:
yield file_name, file_content
seen.add(file_name)
+25 -17
View File
@@ -67,8 +67,12 @@ class ExportSavedQueriesCommand(ExportModelsCommand):
@staticmethod
def _export(
model: SavedQuery, export_related: bool = True
model: SavedQuery, export_related: bool = True, seen: set[str] | None = None
) -> Iterator[tuple[str, Callable[[], str]]]:
# Initialize seen set if not provided
if seen is None:
seen = set()
yield (
ExportSavedQueriesCommand._file_name(model),
lambda: ExportSavedQueriesCommand._file_content(model),
@@ -79,21 +83,25 @@ class ExportSavedQueriesCommand(ExportModelsCommand):
database_slug = secure_filename(model.database.database_name)
file_name = f"databases/{database_slug}.yaml"
payload = model.database.export_to_dict(
recursive=False,
include_parent_ref=False,
include_defaults=True,
export_uuids=True,
)
# TODO (betodealmeida): move this logic to export_to_dict once this
# becomes the default export endpoint
if "extra" in payload:
try:
payload["extra"] = json.loads(payload["extra"])
except json.JSONDecodeError:
logger.info("Unable to decode `extra` field: %s", payload["extra"])
# Only yield if not already seen (similar to dataset export)
if file_name not in seen:
payload = model.database.export_to_dict(
recursive=False,
include_parent_ref=False,
include_defaults=True,
export_uuids=True,
)
# TODO (betodealmeida): move this logic to export_to_dict once this
# becomes the default export endpoint
if "extra" in payload:
try:
payload["extra"] = json.loads(payload["extra"])
except (TypeError, json.JSONDecodeError):
logger.info(
"Unable to decode `extra` field: %s", payload["extra"]
)
payload["version"] = EXPORT_VERSION
payload["version"] = EXPORT_VERSION
file_content = yaml.safe_dump(payload, sort_keys=False)
yield file_name, lambda: file_content
file_content = yaml.safe_dump(payload, sort_keys=False)
yield file_name, lambda: file_content
+3 -1
View File
@@ -46,7 +46,9 @@ class ExportTagsCommand(ExportModelsCommand):
self.dashboard_ids = dashboard_ids
self.chart_ids = chart_ids
def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
def run(
self, seen: set[str] | None = None
) -> Iterator[tuple[str, Callable[[], str]]]:
if not feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
return
+5 -1
View File
@@ -67,8 +67,12 @@ class ExportThemesCommand(ExportModelsCommand):
@staticmethod
def _export(
model: Theme, export_related: bool = True
model: Theme, export_related: bool = True, seen: set[str] | None = None
) -> Iterator[tuple[str, Callable[[], str]]]:
# Initialize seen set if not provided (for consistency)
if seen is None:
seen = set()
yield (
ExportThemesCommand._file_name(model),
lambda: ExportThemesCommand._file_content(model),
+1
View File
@@ -964,6 +964,7 @@ 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,6 +278,18 @@ 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",
+23 -2
View File
@@ -41,6 +41,9 @@ 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
@@ -87,6 +90,14 @@ 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
@@ -136,6 +147,10 @@ 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
@@ -169,6 +184,7 @@ 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", "")),
},
@@ -184,7 +200,7 @@ class DatasourceRestApi(BaseSupersetApi):
logger.debug(
"column-values cache HIT: uid=%s col=%s", datasource.uid, column_name
)
response = self.response(200, result=cached)
response = self.response(200, result=cached, limit=row_limit)
response.headers["X-Cache-Status"] = "HIT"
return response
@@ -194,6 +210,7 @@ class DatasourceRestApi(BaseSupersetApi):
limit=row_limit,
denormalize_column=denormalize_column,
array_elements=array_elements,
search=search,
)
except KeyError:
return self.response(
@@ -225,11 +242,15 @@ 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)
response = self.response(200, result=payload, limit=row_limit)
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.
: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
: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
"""
bind = op.get_bind()
+55 -4
View File
@@ -198,6 +198,41 @@ 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:
@@ -1611,16 +1646,28 @@ 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]:
try:
return json.loads(self.extra)
except (TypeError, json.JSONDecodeError):
return {}
# 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
@property
def is_certified(self) -> bool:
@@ -4010,6 +4057,7 @@ 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
@@ -4047,6 +4095,9 @@ 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 = {
column_formats: dict[str, str | None] = {
metric.name: metric.d3format for metric in metrics if metric.d3format
}
+19 -1
View File
@@ -1539,7 +1539,25 @@ class SQLStatement(BaseSQLStatement[exp.Expression]):
:return: The parsed predicate.
"""
_check_script_length(predicate, self.engine)
return sqlglot.parse_one(predicate, dialect=self._dialect)
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
def apply_rls(
self,
+11 -3
View File
@@ -162,13 +162,21 @@ def memoized_func(key: str, cache: Cache = cache_manager.cache) -> Callable[...,
def wrapped_f(*args: Any, **kwargs: Any) -> Any:
should_cache = kwargs.pop("cache", True)
force = kwargs.pop("force", False)
cache_timeout = kwargs.pop(
"cache_timeout", app.config["CACHE_DEFAULT_TIMEOUT"]
)
# always popped, even when caching is skipped, so it is never forwarded
# to the decorated function as an unexpected keyword argument.
cache_timeout = kwargs.pop("cache_timeout", None)
if not should_cache:
return f(*args, **kwargs)
# callers may explicitly pass ``cache_timeout=None`` (eg, when a database
# has no custom metadata cache timeout configured), which should fall back
# to the default timeout rather than be forwarded to the cache backend.
# the config lookup happens here so the uncached path stays independent
# of the Flask app config.
if cache_timeout is None:
cache_timeout = app.config["CACHE_DEFAULT_TIMEOUT"]
# format the key using args/kwargs passed to the decorated function
signature = inspect.signature(f)
bound_args = signature.bind(*args, **kwargs)
+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: The cache to keep the thumbnail payload
:param cache_key: The cache key to store the thumbnail payload under
: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
@@ -34,6 +34,7 @@ import pytest
from flask import g, Response
from flask.ctx import AppContext
from superset import security_manager
from superset.charts.data.api import ChartDataRestApi
from superset.commands.chart.data.get_data_command import ChartDataCommand
from superset.common.chart_data import ChartDataResultFormat, ChartDataResultType
@@ -98,6 +99,25 @@ INCOMPATIBLE_ADHOC_COLUMN_FIXTURE: AdhocColumn = {
}
def _override_view_query_permission(granted: bool) -> Any:
"""
Answer ("can_view_query", "Dashboard") with ``granted`` and let every other
permission check fall through to the real security manager, so the rest of
the request keeps its normal access rules.
"""
real_can_access = security_manager.can_access
def can_access(permission_name: str, view_name: str) -> bool:
if (permission_name, view_name) == ("can_view_query", "Dashboard"):
return granted
return real_can_access(permission_name, view_name)
return mock.patch(
"superset.charts.data.api.security_manager.can_access",
side_effect=can_access,
)
def _query_timing() -> QueryTiming:
return QueryTiming(
query_planning_ns=0,
@@ -1572,19 +1592,40 @@ class TestGetChartDataApi(BaseTestChartDataApi):
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_chart_data_as_guest_user(self, is_guest_user, has_guest_access):
"""
Chart data API: Test response does not inlcude the SQL query for embedded
users.
Chart data API: Test response does not include the SQL query for embedded
users whose role lacks "can view query on Dashboard".
"""
g.user.rls = []
is_guest_user.return_value = True
has_guest_access.return_value = True
rv = self.client.post(CHART_DATA_URI, json=self.query_context_payload)
with _override_view_query_permission(granted=False):
rv = self.client.post(CHART_DATA_URI, json=self.query_context_payload)
data = json.loads(rv.data.decode("utf-8"))
result = data["result"]
excluded_key = "query"
assert all([excluded_key not in query for query in result]) # noqa: C419
@mock.patch("superset.security.manager.SupersetSecurityManager.has_guest_access")
@mock.patch("superset.security.manager.SupersetSecurityManager.is_guest_user")
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_chart_data_as_guest_user_allowed_to_view_query(
self, is_guest_user, has_guest_access
):
"""
Chart data API: Test response includes the SQL query for embedded users
whose role carries "can view query on Dashboard".
"""
g.user.rls = []
is_guest_user.return_value = True
has_guest_access.return_value = True
with _override_view_query_permission(granted=True):
rv = self.client.post(CHART_DATA_URI, json=self.query_context_payload)
data = json.loads(rv.data.decode("utf-8"))
result = data["result"]
assert all("query" in query for query in result)
def test_chart_data_table_chart_with_time_grain_filter(self):
"""
Chart data API: Test that a table chart that's not using a temporal column can
@@ -533,6 +533,110 @@ class TestExportDashboardsCommand(SupersetTestCase):
{"dashboard_title": "World Bank's Data"},
)
@pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
@patch("superset.security.manager.g")
@patch("superset.views.base.g")
def test_export_dashboard_cross_database_charts(self, mock_g1, mock_g2):
"""
Test that dashboards with charts from multiple databases export correctly.
This reproduces issue #37113 where charts from different databases were missing.
"""
mock_g1.user = security_manager.find_user("admin")
mock_g2.user = security_manager.find_user("admin")
# Create a second database for testing
second_db = Database(database_name="test_db_2", sqlalchemy_uri="sqlite://")
db.session.add(second_db)
# Create a dataset in the second database
second_dataset = SqlaTable(
table_name="second_dataset",
database=second_db,
database_id=second_db.id,
columns=[],
)
db.session.add(second_dataset)
# Flush so `second_dataset.id` is populated before it's read below;
# otherwise the chart would be constructed with `datasource_id=None`
# and never actually link back to this dataset.
db.session.flush()
# Create a chart using the second database's dataset
chart_from_second_db = Slice(
slice_name="Chart from Second Database",
datasource_type="table",
datasource_id=second_dataset.id,
datasource_name=second_dataset.table_name,
viz_type="bar",
params=json.dumps({"viz_type": "bar"}),
)
db.session.add(chart_from_second_db)
# Get the example dashboard and add the new chart
example_dashboard = (
db.session.query(Dashboard).filter_by(slug="world_health").one()
)
# Store original charts count
original_charts_count = len(example_dashboard.slices)
# Add the new chart from different database to the dashboard
example_dashboard.slices.append(chart_from_second_db)
db.session.commit()
try:
# Export the dashboard
command = ExportDashboardsCommand([example_dashboard.id])
contents = dict(command.run())
# Verify all databases are exported
db_files = [key for key in contents.keys() if key.startswith("databases/")]
assert len(db_files) >= 2, (
f"Expected at least 2 database files, got {db_files}"
)
# Verify the second database is included
assert "databases/test_db_2.yaml" in contents.keys(), (
f"Second database not found in export. Keys: {list(contents.keys())}"
)
# Verify all charts are exported (original + new one)
chart_files = [key for key in contents.keys() if key.startswith("charts/")]
assert len(chart_files) == original_charts_count + 1, (
f"Expected {original_charts_count + 1} charts, got {len(chart_files)}"
)
# Verify the new chart from second database is included
chart_from_second_db_file = None
for key in chart_files:
if f"Chart_from_Second_Database_{chart_from_second_db.id}" in key:
chart_from_second_db_file = key
break
assert chart_from_second_db_file is not None, (
f"Chart from second database not found in export. "
f"Chart files: {chart_files}"
)
# Verify the dataset from second database is included
dataset_files = [
key for key in contents.keys() if key.startswith("datasets/")
]
second_dataset_file = (
f"datasets/test_db_2/second_dataset_{second_dataset.id}.yaml"
)
assert second_dataset_file in contents.keys(), (
f"Second dataset not found. Dataset files: {dataset_files}"
)
finally:
# Clean up, even if an assertion above failed, so a failing run
# doesn't leave extra Database/Slice/SqlaTable rows for later tests.
example_dashboard.slices.remove(chart_from_second_db)
db.session.delete(chart_from_second_db)
db.session.delete(second_dataset)
db.session.delete(second_db)
db.session.commit()
class TestImportDashboardsCommand(SupersetTestCase):
def test_import_v0_dashboard_cli_export(self):
@@ -146,6 +146,36 @@ def test_csv_upload_dataset():
assert user_is_editor(security_manager.find_user("admin"), dataset)
@only_postgresql
@pytest.mark.usefixtures("setup_csv_upload_with_context_schema")
def test_csv_upload_dataset_catalog():
admin_user = security_manager.find_user(username="admin")
upload_database = get_upload_db()
with override_user(admin_user):
UploadCommand(
upload_database.id,
CSV_UPLOAD_TABLE_W_SCHEMA,
create_csv_file(CSV_FILE_1),
"public",
CSVReader({}),
).run()
dataset = (
db.session.query(SqlaTable)
.filter_by(
database_id=upload_database.id,
table_name=CSV_UPLOAD_TABLE_W_SCHEMA,
)
.one()
)
catalog = upload_database.get_default_catalog()
assert dataset.catalog == catalog
assert dataset.schema_perm == (
f"[{upload_database.database_name}].[{catalog}].[public]"
)
@pytest.mark.usefixtures("setup_csv_upload_with_context")
def test_csv_upload_with_index():
admin_user = security_manager.find_user(username="admin")
+26 -18
View File
@@ -20,6 +20,7 @@ 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
@@ -72,6 +73,27 @@ 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")
@@ -1316,17 +1338,10 @@ class TestDatasetApi(SupersetTestCase):
rv = self.get_assert_metric(uri, "get")
data = json.loads(rv.data.decode("utf-8"))
for column in data["result"]["columns"]:
column.pop("changed_on", None)
column.pop("created_on", None)
column.pop("type_generic", None)
strip_read_only_fields(data["result"]["columns"])
data["result"]["columns"].append(new_column_data)
for metric in data["result"]["metrics"]:
metric.pop("changed_on", None)
metric.pop("created_on", None)
metric.pop("type_generic", None)
strip_read_only_fields(data["result"]["metrics"])
data["result"]["metrics"].append(new_metric_data)
with freeze_time() as frozen:
@@ -1404,11 +1419,7 @@ class TestDatasetApi(SupersetTestCase):
rv = self.get_assert_metric(uri, "get")
data = json.loads(rv.data.decode("utf-8"))
for column in data["result"]["columns"]:
column.pop("changed_on", None)
column.pop("created_on", None)
column.pop("type_generic", None)
strip_read_only_fields(data["result"]["columns"])
data["result"]["columns"].append(new_column_data)
rv = self.client.put(uri, json={"columns": data["result"]["columns"]})
@@ -1443,10 +1454,7 @@ 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"]
for column in resp_columns:
column.pop("changed_on", None)
column.pop("created_on", None)
column.pop("type_generic", None)
strip_read_only_fields(resp_columns)
resp_columns[0]["groupby"] = False
resp_columns[0]["filterable"] = False
@@ -155,6 +155,7 @@ class TestDatasourceApi(SupersetTestCase):
limit=10000,
denormalize_column=False,
array_elements=False,
search=None,
)
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@@ -170,6 +171,79 @@ 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):
@@ -191,6 +265,7 @@ class TestDatasourceApi(SupersetTestCase):
limit=10000,
denormalize_column=True,
array_elements=False,
search=None,
)
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@@ -328,6 +328,10 @@ def test_send_chart_response_strips_guest_query_after_timing_projection(
"superset.charts.data.api.security_manager.is_guest_user",
return_value=True,
),
patch(
"superset.charts.data.api.security_manager.can_access",
return_value=False,
),
):
response = api._send_chart_response(result)
finally:
@@ -339,6 +343,35 @@ def test_send_chart_response_strips_guest_query_after_timing_projection(
assert "query" in query_payload
def test_send_chart_response_keeps_guest_query_when_permitted(
app: SupersetApp,
) -> None:
"""
A guest whose role carries "can view query on Dashboard" must receive the
generated SQL, otherwise "View query" is empty on embedded dashboards.
"""
query_payload = {"data": [{"col1": 1}], "query": "SELECT 1"}
result = _json_execution_result(query_payload)
api = ChartDataRestApi()
with (
app.test_request_context("/api/v1/chart/data"),
patch(
"superset.charts.data.api.security_manager.is_guest_user",
return_value=True,
),
patch(
"superset.charts.data.api.security_manager.can_access",
return_value=True,
) as can_access,
):
response = api._send_chart_response(result)
query = json.loads(response.get_data(as_text=True))["result"][0]
assert query["query"] == "SELECT 1"
can_access.assert_called_once_with("can_view_query", "Dashboard")
def test_send_chart_response_redacts_guest_query_error(app: SupersetApp) -> None:
result = _json_execution_result(
{
@@ -356,6 +389,10 @@ def test_send_chart_response_redacts_guest_query_error(app: SupersetApp) -> None
"superset.charts.data.api.security_manager.is_guest_user",
return_value=True,
),
patch(
"superset.charts.data.api.security_manager.can_access",
return_value=False,
),
):
response = api._send_chart_response(result)
@@ -364,6 +401,42 @@ def test_send_chart_response_redacts_guest_query_error(app: SupersetApp) -> None
assert "stacktrace" not in query
def test_send_chart_response_still_redacts_guest_errors_when_query_permitted(
app: SupersetApp,
) -> None:
"""
"can view query on Dashboard" only unlocks the generated SQL; stacktraces
and driver errors describe the deployment and stay redacted for guests.
"""
result = _json_execution_result(
{
"error": "Table mydb.myschema.mytable was not found",
"stacktrace": "Traceback ...",
"query": "SELECT 1",
},
result_type=ChartDataResultType.QUERY,
)
api = ChartDataRestApi()
with (
app.test_request_context("/api/v1/chart/data"),
patch(
"superset.charts.data.api.security_manager.is_guest_user",
return_value=True,
),
patch(
"superset.charts.data.api.security_manager.can_access",
return_value=True,
),
):
response = api._send_chart_response(result)
query = json.loads(response.get_data(as_text=True))["result"][0]
assert query["query"] == "SELECT 1"
assert query["error"] == str(GENERIC_ERROR_MESSAGE)
assert "stacktrace" not in query
def test_get_data_response_redacts_guest_query_failure(app: SupersetApp) -> None:
command = MagicMock()
command.execute.side_effect = ChartDataQueryFailedError(
@@ -473,6 +473,10 @@ def test_update_dataset_rejects_malicious_expression(
f"Expected a field-level ValidationError on '{field}'. Got: "
f"{[(type(e).__name__, getattr(e, 'field_name', None), str(e)) for e in excinfo.value._exceptions]}" # noqa: E501
)
# `messages` must stay a list even though the underlying message is a
# `LazyString`, which marshmallow does not treat as `str`/`bytes` and so
# would otherwise store bare instead of wrapping it.
assert isinstance(expression_errors[0].messages, list)
def test_update_dataset_accepts_benign_expression(mocker: MockerFixture) -> None:

Some files were not shown because too many files have changed in this diff Show More