Compare commits

..
Author SHA1 Message Date
Elizabeth ThompsonandClaude Opus 4.8 893925f0eb test(tags): make TemplateError regression test hermetic, no DB dependency
The regression test let the real security_manager.raise_for_access(query=...)
run, which opens a live DB connection to introspect table-level perms before
reaching the Jinja parse. That only passed locally because a DB was up; the
unit_tests CI sandbox has no Postgres, so it failed there.

Mock raise_for_access directly to raise TemplateError instead. This still
proves what matters: except (SupersetSecurityException, TemplateError) in
create.py catches it and surfaces as TagInvalidError rather than escaping.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-08-22 17:16:37 +00:00
Elizabeth ThompsonandClaude Opus 4.8 698181e93d fix(tags): catch TemplateError when validating access for tagged SQL Lab queries
When tagging a saved SQL Lab query, CreateCustomTagCommand._validate_object_access
calls security_manager.raise_for_access(query=...). For a user relying on
per-table/dataset permissions (no blanket database access), that path parses the
query's Jinja-templated SQL via process_jinja_sql(), which can raise a raw
jinja2 TemplateError (e.g. TemplateSyntaxError on malformed Jinja). The narrow
`except SupersetSecurityException:` let it escape as an unhandled 500. Widen the
except to also catch TemplateError so it surfaces as a validation error.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-08-22 16:46:50 +00:00
Joe LiandClaude Sonnet 5 f2610e9dca fix(sqllab): default PostgreSQL port to 5432 in the dynamic connection form (#43351)
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-21 15:59:33 -07:00
abdullah revehaandAbdullah Sahin 09bd8c98de feat(chart): enable cross-filter on temporal x-axis (bar/label click) (#42792)
Co-authored-by: Abdullah Sahin <you@example.comclear>
2026-08-21 15:45:06 -07:00
Aurimas Navardauskas 5812c0eab5 fix(mcp): fail closed when MCP_AUTH_FACTORY raises (#43329) 2026-08-21 15:43:51 -07:00
Mike BridgeandClaude Fable 5 5f3e34908c fix(charts): surface blocking alerts/reports when archiving a chart (#43402)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-21 19:33:32 -03:00
Evan RusackasandSuperset Dev 9781254363 fix(export): escape formula-triggering values consistently and bound post-processing inputs (#43396)
Co-authored-by: Superset Dev <dev@superset.apache.org>
2026-08-21 12:37:33 -07:00
65893113fe fix(import): tighten ownership and validation checks across asset importers (#43393)
Co-authored-by: Superset Dev <dev@superset.apache.org>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-21 12:35:33 -07:00
04017f3956 fix(sqllab): re-validate access against rendered SQL and tighten cache/permalink scoping (#43394)
Co-authored-by: Superset Dev <dev@superset.apache.org>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-21 12:35:06 -07:00
2b6044b624 fix: tighten SSRF validation, executor resolution, and cache scoping across reports/thumbnails (#43395)
Co-authored-by: Superset Dev <dev@superset.apache.org>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-21 12:34:37 -07:00
Evan RusackasandSuperset Dev aee98b527d fix(charts): escape untrusted strings before rendering into chart tooltips and popups (#43397)
Co-authored-by: Superset Dev <dev@superset.apache.org>
2026-08-21 12:31:31 -07:00
edda9d1bbb fix(frontend): tighten SQL Lab autorun scoping and HTML-rendering defaults (#43398)
Co-authored-by: Superset Dev <dev@superset.apache.org>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-21 12:31:08 -07:00
Joe LiandClaude Sonnet 5 03eac279e5 fix(explore): exclude permalink_key from chart URL params (#43354)
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-21 11:43:18 -07:00
yousophandClaude Opus 4.8 fd7095df9a fix(explore): align viz type gallery thumbnails and Featured tag (#43373)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-08-21 10:40:37 -07:00
d422f5b4b6 fix: last date label hidden on time series x-axis (#39899) (#42299)
Co-authored-by: Prathamesh Hukkeri <prathamesh04@users.noreply.github.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Evan Rusackas <evan@preset.io>
2026-08-21 09:59:06 -07:00
Alexandru Soare a05a099987 fix(embedded): block custom SQL injection in guest user chart payloads (#43111) 2026-08-21 17:05:40 +03:00
Evan RusackasandClaude Code 05842a6350 feat(metrics): add MEDIAN/STDDEV_SAMP/VAR_SAMP as system-wide aggregates (#42895)
Co-authored-by: Claude Code <noreply@anthropic.com>
2026-08-21 04:15:17 -07:00
Evan RusackasandClaude Opus 4.8 e45dd2d3f1 ci: pull CI service images from GHCR mirror (fork-safe) [depends on #40880] (#40882)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-08-21 04:11:37 -07:00
Mehmet Salih Yavuz 65910abf21 fix(sqllab): disable Save dataset until the query runs successfully (#43330) 2026-08-21 12:56:42 +03:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> 6e22050b50 chore(deps): bump the storybook group in /docs with 2 updates (#43377)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-21 01:10:41 -07:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> 132340b652 chore(deps): bump astral-sh/setup-uv from 10.0.0 to 10.0.1 (#43378)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-21 01:10:37 -07:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> 14408eb3db chore(deps-dev): bump the storybook group in /superset-frontend with 5 updates (#43379)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-21 01:10:33 -07:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> 87743ef3f8 chore(deps): bump dompurify from 3.4.12 to 3.4.13 in /superset-frontend (#43380)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-21 01:10:29 -07:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> fb53761ec8 chore(deps-dev): bump baseline-browser-mapping from 2.11.13 to 2.11.14 in /superset-frontend (#43381)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-21 01:10:25 -07:00
Amin Ghadersohi f7d505e1fd fix(listview): stop card clicks creating a duplicate history entry (#43310) 2026-08-20 23:23:46 -04:00
22396d504a fix(ux): use title case for button labels (#40048)
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Evan Rusackas <evan@preset.io>
2026-08-20 20:14:33 -07:00
Gaston LaterzaandClaude Opus 5 27ea5de44a fix(i18n-es): correct case/number collapse in the semantic-layer labels (#43311)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-20 20:03:57 -07:00
Joe LiandClaude Sonnet 5 18fc2c6228 fix(sqllab): stop copying a permalink when opening a saved query (#43147)
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-20 15:44:20 -07:00
Amin Ghadersohi 18a36d04c7 fix(mcp): preserve user-authored result values (#43202) 2026-08-20 18:29:39 -04:00
ʈᵃᵢ 01b1d58ac9 fix(plugin-chart-echarts): restore tooltips for metrics labelled like… (#43369) 2026-08-20 14:58:11 -07:00
Joe Li 7441ce90ae fix(charts): align grain-less time comparisons safely (#43315) 2026-08-20 14:55:01 -07:00
Amin Ghadersohi 42ba2a4433 fix(reports): humanize day-of-month + day-of-week crontabs as OR (#43307) 2026-08-20 16:46:08 -04:00
madhushreeagandmadhushree agarwal 271564cb0d feat(config): add EXTRA_PANDAS_POSTPROCESSING_OPS extension point (#43337)
Co-authored-by: madhushree agarwal <madhushree_agarwal@apple.com>
2026-08-20 13:15:41 -07:00
JUST.in DO ITandClaude Sonnet 5 bcfb4346f6 fix(mcp): honor use_cache and cache_timeout in get_chart_data (#43349)
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-20 13:01:41 -07:00
Joe Li b5fe00b407 test(frontend): shrink flaky/misplaced recently-archived e2e coverage to Jest unit tests (#43264) 2026-08-20 11:48:02 -07:00
Parman MohammadalizadehandEvan Rusackas 148ffaff50 fix(plugin-chart-echarts): omit stacked value labels on zero-height segments (#42756)
Co-authored-by: Evan Rusackas <evan@preset.io>
2026-08-20 11:04:19 -07:00
Ankit 83c6ea4e03 fix(explore): show the empty state when Samples returns no result payload (#43115) 2026-08-20 11:00:54 -07:00
Hans Yu 1ca5e61f54 chore: Unset future flag in engines and sessions. (#43273) 2026-08-20 10:58:37 -07:00
rlei 1ef12580a7 fix(chart): ignore chart actions for a chart no longer in state (#43228) 2026-08-20 10:57:21 -07:00
c0884c0f0c fix(explore): keep x-axis label when overriding Time Column with time comparison (#42875)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Joe Li <joe@preset.io>
2026-08-20 10:52:22 -07:00
Sepuri Sai KrishnaandClaude Opus 5 2ff79bd495 fix(github): point the issue templates at labels that exist (#43357)
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-20 10:13:36 -07:00
453 changed files with 12346 additions and 3703 deletions
+1 -1
View File
@@ -1,6 +1,6 @@
name: Bug report
description: Report a bug to improve Superset's stability
labels: ["bug"]
labels: ["#bug"]
body:
- type: markdown
attributes:
+1 -1
View File
@@ -1,7 +1,7 @@
---
name: Cosmetic Issue
about: Describe a cosmetic issue with CSS, positioning, layout, labeling, or similar
labels: "cosmetic-issue"
labels: "#bug:cosmetic"
---
## Screenshot
+1 -1
View File
@@ -48,7 +48,7 @@ jobs:
python-version: "3.11"
- name: Install uv
uses: astral-sh/setup-uv@ae62891fec2bb8e7d6c99fc78c9fec3a63790f8d # v10.0.0
uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
with:
python-version: "3.11"
enable-cache: true
+4 -4
View File
@@ -78,14 +78,14 @@ jobs:
USE_DASHBOARD: ${{ github.event.inputs.use_dashboard == 'true' || 'false' }}
services:
postgres:
image: postgres:17-alpine
image: ghcr.io/apache/superset/ci/postgres:17-alpine
env:
POSTGRES_USER: superset
POSTGRES_PASSWORD: superset
ports:
- 15432:5432
redis:
image: redis:7-alpine
image: ghcr.io/apache/superset/ci/redis:7-alpine
ports:
- 16379:6379
steps:
@@ -186,14 +186,14 @@ jobs:
GITHUB_TOKEN: ${{ github.token }}
services:
postgres:
image: postgres:17-alpine
image: ghcr.io/apache/superset/ci/postgres:17-alpine
env:
POSTGRES_USER: superset
POSTGRES_PASSWORD: superset
ports:
- 15432:5432
redis:
image: redis:7-alpine
image: ghcr.io/apache/superset/ci/redis:7-alpine
ports:
- 16379:6379
steps:
@@ -53,9 +53,7 @@ jobs:
mysql+mysqldb://superset:superset@127.0.0.1:13306/superset?charset=utf8mb4&binary_prefix=true
services:
mysql:
image: mysql:8.0
# Authenticated pulls use our higher Docker Hub rate limit. Empty on
# fork PRs (secrets unavailable) -> runner falls back to anonymous.
image: ghcr.io/apache/superset/ci/mysql:8.0
env:
MYSQL_ROOT_PASSWORD: root
ports:
@@ -66,7 +64,7 @@ jobs:
--health-timeout=5s
--health-retries=5
redis:
image: redis:7-alpine
image: ghcr.io/apache/superset/ci/redis:7-alpine
options: --entrypoint redis-server
ports:
- 16379:6379
@@ -143,7 +141,7 @@ jobs:
SUPERSET__SQLALCHEMY_DATABASE_URI: postgresql+psycopg2://superset:superset@127.0.0.1:15432/superset
services:
postgres:
image: postgres:17-alpine
image: ghcr.io/apache/superset/ci/postgres:17-alpine
env:
POSTGRES_USER: superset
POSTGRES_PASSWORD: superset
@@ -152,7 +150,7 @@ jobs:
# GitHub action runner's default installations
- 15432:5432
redis:
image: redis:7-alpine
image: ghcr.io/apache/superset/ci/redis:7-alpine
ports:
- 16379:6379
steps:
@@ -202,7 +200,7 @@ jobs:
sqlite:///${{ github.workspace }}/.temp/examples.db?check_same_thread=true
services:
redis:
image: redis:7-alpine
image: ghcr.io/apache/superset/ci/redis:7-alpine
ports:
- 16379:6379
steps:
@@ -52,7 +52,7 @@ jobs:
SUPERSET__SQLALCHEMY_EXAMPLES_URI: presto://localhost:15433/memory/default
services:
postgres:
image: postgres:17-alpine
image: ghcr.io/apache/superset/ci/postgres:17-alpine
env:
POSTGRES_USER: superset
POSTGRES_PASSWORD: superset
@@ -61,7 +61,7 @@ jobs:
# GitHub action runner's default installations
- 15432:5432
presto:
image: starburstdata/presto:350-e.6
image: ghcr.io/apache/superset/ci/presto:350-e.6
env:
POSTGRES_USER: superset
POSTGRES_PASSWORD: superset
@@ -70,7 +70,7 @@ jobs:
# GitHub action runner's default installations
- 15433:8080
redis:
image: redis:7-alpine
image: ghcr.io/apache/superset/ci/redis:7-alpine
ports:
- 16379:6379
steps:
@@ -116,7 +116,7 @@ jobs:
UPLOAD_FOLDER: /tmp/.superset/uploads/
services:
postgres:
image: postgres:17-alpine
image: ghcr.io/apache/superset/ci/postgres:17-alpine
env:
POSTGRES_USER: superset
POSTGRES_PASSWORD: superset
@@ -125,7 +125,7 @@ jobs:
# GitHub action runner's default installations
- 15432:5432
redis:
image: redis:7-alpine
image: ghcr.io/apache/superset/ci/redis:7-alpine
ports:
- 16379:6379
steps:
@@ -75,13 +75,18 @@ jobs:
SUPERSET_SECRET_KEY: not-a-secret
run: |
pytest --durations-min=0.5 --cov-report= --cov=superset ./tests/common ./tests/unit_tests --cache-clear --maxfail=50 --junit-xml=test-results/junit-unit.xml
# COVERAGE_FILE keeps these scoped gates off the default .coverage that
# the step above wrote. pytest-cov starts a fresh data file per run, so
# without it the last gate replaces the full-suite data and the report
# uploaded below contains only that gate's subtree -- every other file
# then reads as uncovered, and patch coverage on an unrelated PR is 0%.
- name: Python 100% coverage unit tests
env:
SUPERSET_TESTENV: true
SUPERSET_SECRET_KEY: not-a-secret
run: |
pytest --durations-min=0.5 --cov=superset/sql/ ./tests/unit_tests/sql/ --cache-clear --cov-fail-under=100 --junit-xml=test-results/junit-sql-coverage.xml
pytest --durations-min=0.5 --cov=superset/semantic_layers/ ./tests/unit_tests/semantic_layers/ --cache-clear --cov-fail-under=100 --junit-xml=test-results/junit-semantic-layers-coverage.xml
COVERAGE_FILE="${RUNNER_TEMP}/.coverage.sql" pytest --durations-min=0.5 --cov=superset/sql/ ./tests/unit_tests/sql/ --cache-clear --cov-fail-under=100 --junit-xml=test-results/junit-sql-coverage.xml
COVERAGE_FILE="${RUNNER_TEMP}/.coverage.semantic_layers" pytest --durations-min=0.5 --cov=superset/semantic_layers/ ./tests/unit_tests/semantic_layers/ --cache-clear --cov-fail-under=100 --junit-xml=test-results/junit-semantic-layers-coverage.xml
- name: Upload code coverage
uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0
with:
+40
View File
@@ -26,6 +26,29 @@ assists people when migrating to a new version.
- `SAMPLES_ROW_LIMIT` is now the default for `/datasource/samples` requests without a valid explicit `per_page`, rather than a hard per-request ceiling; explicit limits are honored up to the existing global row-limit ceiling, matching `/chart/data` SAMPLES requests.
### MCP tool results preserve stored string values
Structured MCP tool results no longer add `<UNTRUSTED-CONTENT>` wrappers or
rewrite delimiter-looking text inside string fields. Tool-result content remains
user-controlled data, but clients must convey that trust boundary outside domain
values instead of recognizing or removing marker strings.
Clients that handled the former delimiter convention should stop stripping marker
text: the same text can be legitimate stored content. Response models and content
types are unchanged, and no metadata-database migration is required. Automated
read-modify-write workflows should be paused or pinned away from older instances
until every serving instance is upgraded; a mixed-version response has no reliable
signal that tells a client whether its text is decorated. Redis-backed MCP response
caches use a new internal namespace after the upgrade, so upgraded instances do not
reuse older cached results.
Values that a client already wrote back with presentation wrappers cannot be
distinguished safely from intentional content. Operators should review possible
`<UNTRUSTED-CONTENT>` / `</UNTRUSTED-CONTENT>` wrappers and
`[ESCAPED-UNTRUSTED-CONTENT-OPEN]` /
`[ESCAPED-UNTRUSTED-CONTENT-CLOSE]` substitutions rather than applying an automatic
marker-removal migration.
### OAuth2 database callback metrics include their outcome
The unqualified `DatabaseRestApi.oauth2` StatsD counter has been replaced with
@@ -102,6 +125,23 @@ dialect; each package's constraint in `pyproject.toml` documents why.
No application-level configuration changes are required for deployments
that don't touch SQLAlchemy directly.
### New metric aggregates: MEDIAN, Sample Standard Deviation, Sample Variance
`MEDIAN`, `STDDEV_SAMP`, and `VAR_SAMP` are now available anywhere a metric
aggregate is chosen (every chart type, SQL Lab, MCP), not only in Pivot
Table's controls. Support is opt-in per database engine *spec class*,
verified against a live instance before being enabled: Postgres, MySQL
(`STDDEV_SAMP`/`VAR_SAMP` only, no `MEDIAN`), DuckDB, and Redshift (inherits
Postgres's support, not yet separately verified) ship enabled in this
release. Engine specs that subclass one of those (e.g. MariaDB, Aurora
MySQL/Postgres, TimescaleDB) inherit the same support, on the same
not-yet-independently-verified basis. Picking one of these aggregates on a
database that has not opted in returns a clear "not supported on this
database" error rather than a failed query. See
`docs/sip/median-stddev-variance-aggregates.md` for the full design
rationale, including why this is safe to add without reintroducing the
totals/subtotals correctness bug fixed by #41184 (SIP-216).
### Soft delete is on by default, and purging is live
`SOFT_DELETE` now ships **on** (`DEFAULT_FEATURE_FLAGS`), so deleting a
+34 -2
View File
@@ -576,7 +576,7 @@ MCP_CACHE_CONFIG = {
| Key | Default | Description |
| -------------------- | --------- | ----------------------------------------------------------- |
| `enabled` | `False` | Enable response caching |
| `CACHE_KEY_PREFIX` | `None` | Optional prefix for cache keys (useful for shared Redis) |
| `CACHE_KEY_PREFIX` | `None` | Base prefix for shared Redis; Superset appends an internal response-contract namespace |
| `list_tools_ttl` | `300` | Cache TTL in seconds for `tools/list` |
| `list_resources_ttl` | `300` | Cache TTL for `resources/list` |
| `list_prompts_ttl` | `300` | Cache TTL for `prompts/list` |
@@ -718,6 +718,34 @@ Every MCP request passes through a middleware stack before reaching the tool fun
Additional middleware classes (`RateLimitMiddleware`, `FieldPermissionsMiddleware`, `PrivateToolMiddleware`) are implemented in `superset/mcp_service/middleware.py` but are not added to the default pipeline. They are available for operators who want to layer them in via a custom startup path.
### Tool Result Value Contract
Structured tool results preserve Superset domain values exactly. In particular,
string fields are not wrapped in trust delimiters, and text that resembles a
delimiter is returned as literal application data. This lets clients safely use a
read result as the basis for an update without persisting presentation markup.
All tool-result content should still be treated as user-controlled data with no
instruction authority. MCP clients should communicate that trust boundary through
their model instructions or presentation layer, outside the returned field values;
fixed or generated marker strings inside a value are ambiguous and must not be used
as a trust signal.
For compatibility, clients that supported the former
`<UNTRUSTED-CONTENT>` convention should stop recognizing or stripping those strings.
The response schemas and content types have not changed. Because marker-looking text
can be legitimate application data, a client cannot reliably distinguish a legacy
decorated response from a clean one. Pause automated read-modify-write workflows, or
route them only to upgraded instances, until every serving instance is upgraded.
Redis-backed MCP response caches include an internal response-contract namespace, so
an upgraded instance does not reuse responses cached by an older release. Older
instances can still return legacy values while they remain in service. After the
upgrade, review previously written values for wrapper text and both
`[ESCAPED-UNTRUSTED-CONTENT-OPEN]` and
`[ESCAPED-UNTRUSTED-CONTENT-CLOSE]`; do not remove these strings automatically,
because they may be intentional content.
### Error Sanitization
The `GlobalErrorHandlerMiddleware` automatically redacts sensitive information from all error messages before they reach the LLM client. The following are replaced with generic messages:
@@ -752,7 +780,11 @@ For a 3-pod Kubernetes deployment with the defaults above, expect up to 3 × (5
Enable response caching for read-heavy workloads (dashboards/datasets that don't change frequently). With the in-memory backend (default when `MCP_STORE_CONFIG` is disabled), caching is per-process. Use Redis-backed caching for consistent cache hits across multiple pods:
```python
MCP_CACHE_CONFIG = {"enabled": True, "call_tool_ttl": 3600}
MCP_CACHE_CONFIG = {
"enabled": True,
"CACHE_KEY_PREFIX": "mcp_cache_",
"call_tool_ttl": 3600,
}
MCP_STORE_CONFIG = {"enabled": True, "CACHE_REDIS_URL": "redis://redis:6379/0"}
```
@@ -11,6 +11,11 @@ When soft-delete is enabled, deleting a chart, dashboard, or dataset archives it
instead of removing it permanently. The **Recently Archived** view lets owners
and admins find archived objects and restore them.
A chart used by an alert or report cannot be archived while that dependency
exists. In the chart list view, the archive confirmation lists the alerts and
reports that use the chart; a blocked attempt names them and asks you to
detach or delete them first.
:::note
This view is gated by the `SOFT_DELETE` feature flag. When the flag is off the
+2 -2
View File
@@ -58,7 +58,7 @@
"@fontsource/inter": "^5.3.0",
"@mdx-js/react": "^3.1.1",
"@saucelabs/theme-github-codeblock": "^0.3.0",
"@storybook/addon-docs": "^10.5.7",
"@storybook/addon-docs": "^10.5.8",
"@superset-ui/core": "^0.20.4",
"@swc/core": "^1.15.47",
"antd": "^6.6.0",
@@ -77,7 +77,7 @@
"react-table": "^7.8.0",
"remark-import-partial": "^0.0.2",
"reselect": "^5.2.0",
"storybook": "^10.5.7",
"storybook": "^10.5.8",
"swagger-ui-react": "^5.32.13",
"swc-loader": "^0.2.7",
"tinycolor2": "^1.4.2",
@@ -0,0 +1,236 @@
<!--
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.
-->
# SIP: System-wide MEDIAN, Sample Standard Deviation, and Sample Variance metric aggregates
## [DRAFT — proposal for discussion]
This document now has an accompanying implementation in this PR, for the
proposed mechanism plus a first, empirically-verified engine set (Postgres,
MySQL, DuckDB, Redshift by inheritance). It is intended to seed discussion on
whether this is the right shape and scope before it goes up for a formal SIP
vote, not to pre-empt that discussion, the code exists so reviewers have a
concrete design to react to rather than a description of one.
## Motivation
Before #41184 (SIP-216, the non-additive-totals fix), the Pivot Table chart
exposed an "Aggregation function" control with 18 choices, including
`Median`, `Sample Standard Deviation`, `Sample Variance`, `First`, `Last`,
`Count Unique Values`, and `List Unique Values`. #41184 deleted that control
wholesale, and deliberately so: it re-aggregated already-aggregated cell
values to compute totals/subtotals, which is exactly the class of bug
SIP-216 fixed (summing per-group averages, averaging per-group medians, etc.
produces silently wrong totals). #42761 subsequently restored the one piece
of that control's functionality that was cleanly separable from the
correctness bug, the "show as % of row/column/total" display option,
redesigned as a decoupled, post-hoc-only `showValuesAs` control.
A user has since noticed that several of the other pre-#41184 options never
came back. Checking today's metric aggregate list (`AVG, COUNT,
COUNT_DISTINCT, MAX, MIN, SUM`, see
`superset-frontend/packages/superset-ui-core/src/query/types/Metric.ts`),
most of these have a reasonable equivalent already: `Count Unique Values`
maps to `COUNT_DISTINCT`; `Count`/`Average`/`Max`/`Min` are already standard
aggregates; the two "fraction of" variants are already covered by
`showValuesAs`. But `Median`, `Sample Standard Deviation`, and `Sample
Variance` have no equivalent today anywhere in Superset, not just in Pivot
Table, in any chart type, since the aggregate list is shared across the
whole app.
This is a real, currently-live gap, not a hypothetical one:
`superset/mcp_service/chart/chart_utils.py`, `schemas.py`, and
`prompts/create_chart_guided.py` already treat `STDDEV`, `VAR`, `MEDIAN`,
and `PERCENTILE` as valid aggregate values in their own validation and
documentation, but those values are never recognized by
`superset/connectors/sqla/models.py`'s `sqla_aggregations` dict (the actual
mapping from aggregate name to SQL), so an AI agent using the MCP tool to
build a chart with `"aggregate": "STDDEV"` today creates a chart that
**errors at query time** with "Adhoc metric aggregate is invalid." This SIP
proposes closing that gap for real, at the source, rather than patching
around it in MCP.
## Proposed change
Add `MEDIAN`, `STDDEV_SAMP`, and `VAR_SAMP` as first-class, system-wide
metric aggregates, available anywhere a metric aggregate is chosen (every
chart type, SQL Lab metric picker, MCP), not as a Pivot-Table-specific
control.
**Why this is safe with respect to SIP-216, and needs no Pivot-Table-specific
code at all:** Pivot Table's non-additive-totals machinery
(`superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/utilities.ts`)
already classifies any metric aggregate not in `ADDITIVE_AGGREGATES = {SUM,
COUNT, MIN, MAX}` as non-additive, which routes totals/subtotals through the
correct DB-`GROUPING SETS`-rollup path rather than client-side
re-aggregation (`AVG` and `COUNT_DISTINCT` already go through this path
today). `MEDIAN`/`STDDEV_SAMP`/`VAR_SAMP` fall into that bucket
automatically, with zero changes needed to the additivity logic. So once
these are valid, buildable SQL aggregates, Pivot Table (and every other
chart) gets correct behavior for free. This is the version of "restore the
control" that does not reopen the bug that was just fixed.
**Where the actual change needs to land, and what this PR does:**
1. **Done.** `superset-frontend/packages/superset-ui-core/src/query/types/Metric.ts`,
extended the `Aggregate` type.
2. **Done.** `superset-frontend/src/explore/constants.ts`, added to `AGGREGATES`
(drives `AGGREGATES_OPTIONS`, the dropdown in `AdhocMetricEditPopover`).
3. **Done**, but not consolidated. `superset/connectors/sqla/models.py`
(`sqla_aggregations`) and `superset/models/helpers.py`
(`ExploreMixin.sqla_aggregations`) are both wired to consult the new
`BaseEngineSpec.get_extended_aggregation_func`, in addition to their
existing 6-aggregate dict, so neither's original, already-tested behavior
changed. They remain two separate dicts, consolidating them into one
source of truth is left as a follow-up (see Open questions).
4. **Done**, and it surfaced a second, smaller bug on top of the one this SIP
opened with: MCP's own aggregate names (`STDDEV`, `VAR`) never matched any
real Superset aggregate, before or after this PR, they were always going
to error regardless of what this SIP does. `superset/mcp_service/chart/*`
now accepts the old shorthand as an alias, normalized to the real,
unambiguous names (`STDDEV_SAMP`, `VAR_SAMP`) this PR ships, and the guided
prompt text points at the correct names going forward. `MEDIAN`/
`PERCENTILE` were already spelled correctly in MCP; `PERCENTILE` remains
unimplemented (it needs a parameter this schema has no field for) and is
unchanged by this PR, out of scope here.
**The part that needs real engineering care, this must not be a blind
`sa.func.MEDIAN` / `sa.func.STDDEV_SAMP` / `sa.func.VAR_SAMP`:**
`sqla_aggregations` today is a flat, engine-unaware dict (`sa.func.AVG`,
etc., SQLAlchemy emits whatever function name it is given, with zero
validation that the target dialect actually has it). Superset already has
precedent for exactly this class of per-engine capability difference:
`BaseEngineSpec.supports_grouping_sets` and `_time_grain_expressions`, both
introduced by #41184 itself. This SIP proposes the same shape, a new
per-engine-overridable mechanism (for example
`BaseEngineSpec.get_aggregate_sql(aggregate, column)` with a sensible
default, overridden per engine spec where the default does not hold),
rather than a single hardcoded dict.
Verified findings so far (via `sqlglot.transpile`, cross-checked against
known engine docs; **not** exhaustively tested against live databases, that
is necessary follow-up work this SIP alone cannot complete):
| Engine | `MEDIAN(x)` | `STDDEV_SAMP(x)` | `VAR_SAMP(x)` |
|---|---|---|---|
| Postgres | `PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY x)` | native | native |
| MySQL | no native equivalent, needs explicit "unsupported" handling, not a blind emit | native | MySQL's `VARIANCE()` is an alias for `VAR_POP` (population), not `VAR_SAMP` (sample); a naive dialect-name substitution would silently compute the wrong statistic and needs an explicit, verified expression instead |
| SQLite | only if the specific build was compiled with the (non-default) `SQLITE_ENABLE_PERCENTILE` extension (added in SQLite 3.43, 2023), cannot be assumed available | not available in core SQLite | not available in core SQLite |
| BigQuery / Snowflake / DuckDB / Redshift / Oracle / T-SQL / Databricks / Spark | native `MEDIAN(x)` | native | native on BigQuery/Snowflake/Databricks/Spark, where `VARIANCE` is correctly sample variance; T-SQL has no function named `VARIANCE` at all and needs `VAR(x)` instead |
| Trino / Presto / Hive | `PERCENTILE_CONT` / `approx_percentile` (dialect- and exactness-dependent) | native | `variance` is correctly sample variance per Trino/Presto docs |
This table is deliberately not exhaustive, Superset has roughly 75
`db_engine_specs` files. The proposed default (`BaseEngineSpec`) should be
the safe choice (mark unsupported, surface a clear user-facing error) rather
than an optimistic one, with individual engine specs opting in once
verified. Ship for the handful of engines above first, extend
opportunistically.
**`Count Unique Values`, `First`, `Last`, `List Unique Values`, explicitly
out of scope for this SIP:**
- `Count Unique Values` needs no work, it is already `COUNT_DISTINCT`.
- `First`/`Last` have no well-defined, unambiguous meaning as a plain
`GROUP BY` aggregate without an explicit ordering; most engines only
support this via window functions (`FIRST_VALUE`/`LAST_VALUE` `OVER
(ORDER BY ...)`) or do not support it as a simple aggregate at all
(Postgres has neither built in). Restoring this properly would mean
designing an "order by" sub-control on the metric, a real, separate
feature, not a one-line aggregate addition. Proposed as a follow-up SIP if
there is demand.
- `List Unique Values` maps to the `STRING_AGG`/`GROUP_CONCAT`/`LISTAGG`/
`ARRAY_AGG(DISTINCT ...)` family, real dialect differences, plus an open
UX question (unbounded cell content for high-cardinality columns).
Proposed as a follow-up SIP.
## New or changed public interfaces
- New `Aggregate` values (`MEDIAN`, `STDDEV_SAMP`, `VAR_SAMP`) selectable
anywhere the standard metric control appears, every chart type, not just
Pivot Table.
- New `BaseEngineSpec` extensibility point for per-engine aggregate SQL
generation (exact shape TBD in implementation, likely mirrors
`_time_grain_expressions`).
- No REST API surface changes beyond the existing metric aggregate field
accepting new values.
## Migration plan and compatibility
No new tables/columns needed for the aggregate addition itself.
Restoring prior chart settings, the way #42761 restored `show_values_as` for
charts that had it before #41184, is murkier here than it was for that PR
and needs its own design pass: the old `aggregate_function` was a single
Pivot-Table-level setting applied uniformly to every metric on the chart,
not a per-metric property. A chart that had `aggregate_function: Median`
before #41184, with a metric of `SUM(sales)`, was already silently wrong
under the old architecture (that is the bug that was fixed); mechanically
rewriting its metric to `MEDIAN(sales)` on upgrade would change what the
chart's leaf cells display, not just its totals, which may not match user
intent. This SIP proposes a best-effort, flagged-for-review migration
(surface affected charts to admins rather than silently rewriting them)
rather than a fully automatic one-to-one restoration.
## Rejected alternatives
- **Restoring the old `aggregateFunction` Pivot-Table control as-is.**
Rejected: this is the literal mechanism SIP-216 removed because it
reintroduces incorrect totals for non-additive metrics. Any fix has to go
through the metric's own aggregate, not a separate pivot-level override.
- **Routing all metric SQL generation through `sqlglot` expression-building
instead of SQLAlchemy's `sa.func`.** More architecturally thorough (would
give correct dialect syntax for free across more of the roughly 75 engine
specs), but a much larger, more invasive change to a hot path used by
every chart query. Noted as a possible future direction, not this SIP's
scope; this SIP proposes the smaller, `supports_grouping_sets`-shaped
extensibility point instead.
## Open questions
- **Resolved for this PR, worth confirming as the community's preferred
shape:** implemented as `BaseEngineSpec._extended_aggregations` (a
`{aggregate_name: sqla_column -> sqla_column}` dict) plus a
`get_extended_aggregation_func` accessor, set on the concrete or shared
base engine spec class per engine (e.g. on `PostgresBaseEngineSpec` so
Redshift inherits it, but *not* on `PrestoBaseEngineSpec` so Hive/Spark/
Databricks don't silently inherit unverified behavior, mirroring how
`supports_grouping_sets` is opted into per-concrete-engine there today).
Did not route through the `superset/sql/dialects/` sqlglot-based layer;
that layer is for SQL Lab parsing, wiring it into chart-metric query
building felt like a separate, larger change from this SIP's scope.
- **Still open, not addressed in this PR:** how aggressively should
`MEDIAN` degrade on engines without a native or exact equivalent?
Trino/Presto/Hive were left unimplemented (unsupported) specifically to
avoid silently answering this with an approximate function
(`approx_percentile`) that changes the semantics of what a user asked
for. If someone wants `MEDIAN` on those engines, this needs a real
decision: require explicit opt-in, show a UI warning, or keep it
disallowed.
- **Resolved for this PR:** left the two `sqla_aggregations` dicts
(`connectors/sqla/models.py` and `models/helpers.py`) unconsolidated,
both now separately wired to the same new `get_extended_aggregation_func`
hook. Consolidating them into one source of truth is real but unrelated
cleanup, not bundled here to keep the diff reviewable.
- **New, from implementation:** only Postgres, MySQL (partial), DuckDB, and
Redshift (by inheritance, unverified) ship enabled. BigQuery, Snowflake,
Trino, Presto, Hive, Spark, Databricks, Oracle, and T-SQL all have
documented (not live-verified) support per the table above but are not
yet wired up, each needs the same live-instance verification treatment
before being enabled, this PR intentionally didn't guess.
+18 -18
View File
@@ -4122,23 +4122,23 @@
resolved "https://registry.yarnpkg.com/@standard-schema/utils/-/utils-0.3.0.tgz#3d5e608f16c2390c10528e98e59aef6bf73cae7b"
integrity sha512-e7Mew686owMaPJVNNLs55PUvgz371nKgwsc4vxE49zsODpJEnxgxRo2y/OKrqueavXgZNMDVj3DdHFlaSAeU8g==
"@storybook/addon-docs@^10.5.7":
version "10.5.7"
resolved "https://registry.yarnpkg.com/@storybook/addon-docs/-/addon-docs-10.5.7.tgz#6d599c94fc871c248ce06a5c081f57655c83f40a"
integrity sha512-KNARJfjICaizinsR3INMEiipZm1ObYo+xw+E26gteu50Bcy2dIZUtk5uHY5XdtardU3AXX6yRXoBZ2HCY3lbHA==
"@storybook/addon-docs@^10.5.8":
version "10.5.8"
resolved "https://registry.yarnpkg.com/@storybook/addon-docs/-/addon-docs-10.5.8.tgz#767c10c7a4cc1b625b93f869b2a2b09fc8514f2e"
integrity sha512-NlHiMKW/UvW/uL8HXFDCEVwoH3qZeGYZ/qlWax4d7H471b/T54MBq2KcB4ZrdA785FfIH3numAJdBb5jwn00Mg==
dependencies:
"@mdx-js/react" "^3.0.0"
"@storybook/csf-plugin" "10.5.7"
"@storybook/csf-plugin" "10.5.8"
"@storybook/icons" "^2.0.2"
"@storybook/react-dom-shim" "10.5.7"
"@storybook/react-dom-shim" "10.5.8"
react "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0"
react-dom "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0"
ts-dedent "^2.0.0"
"@storybook/csf-plugin@10.5.7":
version "10.5.7"
resolved "https://registry.yarnpkg.com/@storybook/csf-plugin/-/csf-plugin-10.5.7.tgz#bc73f164d1b5f8e2931b2774f4b389a06453cf6e"
integrity sha512-IaX8FlM0H36HNFhJ2+4L9bCldqfvHGqcLg841SJNyK/DhfMlM7JsvY/GDH2ZFuWrUf8FSOx96GRRnHq6XfRKag==
"@storybook/csf-plugin@10.5.8":
version "10.5.8"
resolved "https://registry.yarnpkg.com/@storybook/csf-plugin/-/csf-plugin-10.5.8.tgz#c626c5bfe55d0e279b2457e5cf150a788d1fc637"
integrity sha512-/FHiMyOWWEXfwK/lM0WxmkP9GLzbSJJuzGtfeuNWSOVDnvAMbjavitxfHb5wSbWKIQo0XYC1EJ2Y7x91XNYP4w==
dependencies:
unplugin "^2.3.5"
@@ -4152,10 +4152,10 @@
resolved "https://registry.yarnpkg.com/@storybook/icons/-/icons-2.1.0.tgz#edfc2450a39c5e780f28c6cbc49acd7bff59b41a"
integrity sha512-Fxh9vYpX9bQqFeHRiY8h2ApeRGDzRSMLwJwNZ/AIRqnyOKHxRKL+yFe+ctEkVJmuptRE9u1Hrn8ZZNHyfDKKNg==
"@storybook/react-dom-shim@10.5.7":
version "10.5.7"
resolved "https://registry.yarnpkg.com/@storybook/react-dom-shim/-/react-dom-shim-10.5.7.tgz#9a5aa0e0f89c09e71c6cbfc6bb1abeb537e5aabf"
integrity sha512-lxOkyh+wu/MiBXvYQHjZfD+DRKOa4bHBzbuGuiHXnHXmdOcTRdcrQTsoeN2FPtfugmmOG66cZUEgDwNX+k5eRA==
"@storybook/react-dom-shim@10.5.8":
version "10.5.8"
resolved "https://registry.yarnpkg.com/@storybook/react-dom-shim/-/react-dom-shim-10.5.8.tgz#40cc3e32af424baa2e4109a325dae2ede29999e2"
integrity sha512-N8D13/Xny+V3kfe1KBgsAHS0nKWXLLdgOOXS9poKdYzVwVCN+CGEGBxWX0zMMtdCptqa6/57em9coPlZMoO+bg==
"@superset-ui/core@^0.20.4":
version "0.20.4"
@@ -14783,10 +14783,10 @@ stop-iteration-iterator@^1.1.0:
es-errors "^1.3.0"
internal-slot "^1.1.0"
storybook@^10.5.7:
version "10.5.7"
resolved "https://registry.yarnpkg.com/storybook/-/storybook-10.5.7.tgz#adfc465e51f337291c095278c23f1b8024ef2da7"
integrity sha512-oiKvWIwIoOhFP1i6dASYyMXwPHKEtVZMshqSB7EvIVYjWRh0l9H7gHEt1z4Gh2rLGFMekWdsm4s94rvwpR7gkg==
storybook@^10.5.8:
version "10.5.8"
resolved "https://registry.yarnpkg.com/storybook/-/storybook-10.5.8.tgz#d5f051983e6232c0a73ea02149a72c7bafb43275"
integrity sha512-rR4oFMSiWBSqI0lvsJPtcQUPj8+hzj3TkLu+Mw61Wo6YxPSb5FsLSHai0jZnuaIdKIlmu25KCfwlSQl4e1uvnA==
dependencies:
"@storybook/global" "^5.0.0"
"@storybook/icons" "^2.0.2"
+1 -1
View File
@@ -93,7 +93,7 @@ def find_models(module: ModuleType) -> list[type[Model]]: # noqa: C901
# where the current model is out-of-sync with the existing table after a
# downgrade
sqlalchemy_uri = current_app.config["SQLALCHEMY_DATABASE_URI"]
engine = create_engine(sqlalchemy_uri, future=True)
engine = create_engine(sqlalchemy_uri)
Base = automap_base() # noqa: N806
Base.prepare(engine, reflect=True)
seen = set()
+77 -68
View File
@@ -180,9 +180,9 @@
"@istanbuljs/nyc-config-typescript": "^1.0.1",
"@playwright/test": "^1.62.1",
"@pmmmwh/react-refresh-webpack-plugin": "^0.6.2",
"@storybook/addon-docs": "10.5.7",
"@storybook/addon-links": "10.5.7",
"@storybook/react-webpack5": "10.5.7",
"@storybook/addon-docs": "10.5.8",
"@storybook/addon-links": "10.5.8",
"@storybook/react-webpack5": "10.5.8",
"@storybook/test-runner": "0.24.4",
"@svgr/webpack": "^8.1.0",
"@swc/core": "^1.15.47",
@@ -218,7 +218,7 @@
"babel-loader": "^10.1.1",
"babel-plugin-dynamic-import-node": "^2.3.3",
"babel-plugin-jsx-remove-data-test-id": "^3.0.0",
"baseline-browser-mapping": "^2.11.13",
"baseline-browser-mapping": "^2.11.14",
"cheerio": "1.2.0",
"concurrently": "^10.0.4",
"copy-webpack-plugin": "^14.0.0",
@@ -235,7 +235,7 @@
"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-storybook": "10.5.7",
"eslint-plugin-storybook": "10.5.8",
"eslint-plugin-testing-library": "^7.16.2",
"eslint-plugin-theme-colors": "file:eslint-rules/eslint-plugin-theme-colors",
"fetch-mock": "^12.6.0",
@@ -266,7 +266,7 @@
"source-map": "^0.8.0",
"source-map-support": "^0.5.21",
"speed-measure-webpack-plugin": "^1.6.0",
"storybook": "10.5.7",
"storybook": "10.5.8",
"style-loader": "^4.0.0",
"stylelint": "^17.14.1",
"swc-loader": "^0.2.7",
@@ -10765,16 +10765,16 @@
"license": "MIT"
},
"node_modules/@storybook/addon-docs": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/@storybook/addon-docs/-/addon-docs-10.5.7.tgz",
"integrity": "sha512-KNARJfjICaizinsR3INMEiipZm1ObYo+xw+E26gteu50Bcy2dIZUtk5uHY5XdtardU3AXX6yRXoBZ2HCY3lbHA==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/@storybook/addon-docs/-/addon-docs-10.5.8.tgz",
"integrity": "sha512-NlHiMKW/UvW/uL8HXFDCEVwoH3qZeGYZ/qlWax4d7H471b/T54MBq2KcB4ZrdA785FfIH3numAJdBb5jwn00Mg==",
"dev": true,
"license": "MIT",
"dependencies": {
"@mdx-js/react": "^3.0.0",
"@storybook/csf-plugin": "10.5.7",
"@storybook/csf-plugin": "10.5.8",
"@storybook/icons": "^2.0.2",
"@storybook/react-dom-shim": "10.5.7",
"@storybook/react-dom-shim": "10.5.8",
"react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"ts-dedent": "^2.0.0"
@@ -10785,7 +10785,7 @@
},
"peerDependencies": {
"@types/react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"storybook": "10.5.7"
"storybook": "10.5.8"
},
"peerDependenciesMeta": {
"@types/react": {
@@ -10794,9 +10794,9 @@
}
},
"node_modules/@storybook/addon-docs/node_modules/@storybook/csf-plugin": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/@storybook/csf-plugin/-/csf-plugin-10.5.7.tgz",
"integrity": "sha512-IaX8FlM0H36HNFhJ2+4L9bCldqfvHGqcLg841SJNyK/DhfMlM7JsvY/GDH2ZFuWrUf8FSOx96GRRnHq6XfRKag==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/@storybook/csf-plugin/-/csf-plugin-10.5.8.tgz",
"integrity": "sha512-/FHiMyOWWEXfwK/lM0WxmkP9GLzbSJJuzGtfeuNWSOVDnvAMbjavitxfHb5wSbWKIQo0XYC1EJ2Y7x91XNYP4w==",
"dev": true,
"license": "MIT",
"dependencies": {
@@ -10809,7 +10809,7 @@
"peerDependencies": {
"esbuild": "*",
"rollup": "*",
"storybook": "10.5.7",
"storybook": "10.5.8",
"vite": "*",
"webpack": "*"
},
@@ -10829,9 +10829,9 @@
}
},
"node_modules/@storybook/addon-docs/node_modules/@storybook/react-dom-shim": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/@storybook/react-dom-shim/-/react-dom-shim-10.5.7.tgz",
"integrity": "sha512-lxOkyh+wu/MiBXvYQHjZfD+DRKOa4bHBzbuGuiHXnHXmdOcTRdcrQTsoeN2FPtfugmmOG66cZUEgDwNX+k5eRA==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/@storybook/react-dom-shim/-/react-dom-shim-10.5.8.tgz",
"integrity": "sha512-N8D13/Xny+V3kfe1KBgsAHS0nKWXLLdgOOXS9poKdYzVwVCN+CGEGBxWX0zMMtdCptqa6/57em9coPlZMoO+bg==",
"dev": true,
"license": "MIT",
"funding": {
@@ -10843,7 +10843,7 @@
"@types/react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"storybook": "10.5.7"
"storybook": "10.5.8"
},
"peerDependenciesMeta": {
"@types/react": {
@@ -10855,9 +10855,9 @@
}
},
"node_modules/@storybook/addon-links": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/@storybook/addon-links/-/addon-links-10.5.7.tgz",
"integrity": "sha512-17PxEOocLhAEaPeQ4q+8yul/LF9YEIePS1arknCAS7U1pQXTe0uj+R0pB6uPLVflM5gECQMiP4WzIj4tEiL6+A==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/@storybook/addon-links/-/addon-links-10.5.8.tgz",
"integrity": "sha512-mpWw4alBJVGqgVh897LZ2keN/xnMHcH93wKJG+oGg4+cdEUA+06hCs5T4k+AS5Aa+EZ6LvdOoi2VPHssyQlCCA==",
"dev": true,
"license": "MIT",
"dependencies": {
@@ -10870,7 +10870,7 @@
"peerDependencies": {
"@types/react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"storybook": "10.5.7"
"storybook": "10.5.8"
},
"peerDependenciesMeta": {
"@types/react": {
@@ -10964,15 +10964,15 @@
}
},
"node_modules/@storybook/react-webpack5": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/@storybook/react-webpack5/-/react-webpack5-10.5.7.tgz",
"integrity": "sha512-vvl07oXp2qfmHJHZ77Aw1F3LFOo7XubOta+lC8UmlEw3rDDJhQxJN3erJJVHavNhdA2jBTK6VUXQKdqQh7X7nQ==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/@storybook/react-webpack5/-/react-webpack5-10.5.8.tgz",
"integrity": "sha512-HkPi42WaoNSHC0DAERsJEF7Vhnluzsp/aiuhnH65GGYG5TmdLL9G8KDiYvXHGDCyb4RfoAPYrtzxaLMFfPPFvQ==",
"dev": true,
"license": "MIT",
"dependencies": {
"@storybook/builder-webpack5": "10.5.7",
"@storybook/preset-react-webpack": "10.5.7",
"@storybook/react": "10.5.7"
"@storybook/builder-webpack5": "10.5.8",
"@storybook/preset-react-webpack": "10.5.8",
"@storybook/react": "10.5.8"
},
"funding": {
"type": "opencollective",
@@ -10981,7 +10981,7 @@
"peerDependencies": {
"react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"storybook": "10.5.7",
"storybook": "10.5.8",
"typescript": ">= 4.9.x"
},
"peerDependenciesMeta": {
@@ -10991,13 +10991,13 @@
}
},
"node_modules/@storybook/react-webpack5/node_modules/@storybook/builder-webpack5": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/@storybook/builder-webpack5/-/builder-webpack5-10.5.7.tgz",
"integrity": "sha512-4n4c60LihFivZnjAcXGO5+XbgZthoUtKb/nPKVgypj3MpEetzjq6XR83A4UNnRsXYmjqfn6bsDWNgEJ/RvQg5A==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/@storybook/builder-webpack5/-/builder-webpack5-10.5.8.tgz",
"integrity": "sha512-ke5x27gtWQ4gpXCLWxdGkr8ZlJwBykV/KjbBTAlC04dmS9OkI9MBzGj+TteUlgrcaN7LwoNTR5zRmxKStOZYzQ==",
"dev": true,
"license": "MIT",
"dependencies": {
"@storybook/core-webpack": "10.5.7",
"@storybook/core-webpack": "10.5.8",
"case-sensitive-paths-webpack-plugin": "^2.4.0",
"cjs-module-lexer": "^1.2.3",
"css-loader": "^7.1.2",
@@ -11019,7 +11019,7 @@
"url": "https://opencollective.com/storybook"
},
"peerDependencies": {
"storybook": "10.5.7"
"storybook": "10.5.8"
},
"peerDependenciesMeta": {
"typescript": {
@@ -11028,9 +11028,9 @@
}
},
"node_modules/@storybook/react-webpack5/node_modules/@storybook/builder-webpack5/node_modules/@storybook/core-webpack": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/@storybook/core-webpack/-/core-webpack-10.5.7.tgz",
"integrity": "sha512-0dtDw/FNPREoeCHX2RgZz0OecxaAGol1R7bCobFevArxyFIPJisTfjDMUFHKr+3B7BilTd3vnatl7Nlvgs0EiA==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/@storybook/core-webpack/-/core-webpack-10.5.8.tgz",
"integrity": "sha512-HccINB0UbTtnyJtKpaX+C35BRTSnAwnreIMwwI+LpeUd4x9mQg0G9orB7lfBBZwd5LQf8YhM2Vkjiawzo41GLg==",
"dev": true,
"license": "MIT",
"dependencies": {
@@ -11041,17 +11041,17 @@
"url": "https://opencollective.com/storybook"
},
"peerDependencies": {
"storybook": "10.5.7"
"storybook": "10.5.8"
}
},
"node_modules/@storybook/react-webpack5/node_modules/@storybook/preset-react-webpack": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/@storybook/preset-react-webpack/-/preset-react-webpack-10.5.7.tgz",
"integrity": "sha512-xwNRcoVlIDx1/YYCFBAxfh/91vFiOgrVI+0Ir4u9eO87SH2leehRnJh619QEOrlQEU5px487y2BmL2ZVtmTpYA==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/@storybook/preset-react-webpack/-/preset-react-webpack-10.5.8.tgz",
"integrity": "sha512-0JjgVoX5t9Wb+gwddYHx/Ej7KFqwd65lpHXEhBoT4pFWRqVI0pvfHu42M+DRGjsgOye3uE+3pH4yHR3+0/fCHA==",
"dev": true,
"license": "MIT",
"dependencies": {
"@storybook/core-webpack": "10.5.7",
"@storybook/core-webpack": "10.5.8",
"@storybook/react-docgen-typescript-plugin": "1.0.6--canary.9.0c3f3b7.0",
"@types/semver": "^7.7.1",
"magic-string": "^0.30.5",
@@ -11068,7 +11068,7 @@
"peerDependencies": {
"react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"storybook": "10.5.7"
"storybook": "10.5.8"
},
"peerDependenciesMeta": {
"typescript": {
@@ -11077,9 +11077,9 @@
}
},
"node_modules/@storybook/react-webpack5/node_modules/@storybook/preset-react-webpack/node_modules/@storybook/core-webpack": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/@storybook/core-webpack/-/core-webpack-10.5.7.tgz",
"integrity": "sha512-0dtDw/FNPREoeCHX2RgZz0OecxaAGol1R7bCobFevArxyFIPJisTfjDMUFHKr+3B7BilTd3vnatl7Nlvgs0EiA==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/@storybook/core-webpack/-/core-webpack-10.5.8.tgz",
"integrity": "sha512-HccINB0UbTtnyJtKpaX+C35BRTSnAwnreIMwwI+LpeUd4x9mQg0G9orB7lfBBZwd5LQf8YhM2Vkjiawzo41GLg==",
"dev": true,
"license": "MIT",
"dependencies": {
@@ -11090,18 +11090,18 @@
"url": "https://opencollective.com/storybook"
},
"peerDependencies": {
"storybook": "10.5.7"
"storybook": "10.5.8"
}
},
"node_modules/@storybook/react-webpack5/node_modules/@storybook/react": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/@storybook/react/-/react-10.5.7.tgz",
"integrity": "sha512-uFvty2MMdFXzW5PcQe1JqDAZkz6cQq7q/9G/cbGVnBEvP6zsOVeL+bmrQ0/WBlFQN0Ko9+ZoCTvaQ9s65zBa5g==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/@storybook/react/-/react-10.5.8.tgz",
"integrity": "sha512-6qqkmqX6imtL+0Z9Uan2tIfYivOI0FiVmWr0zpqqQR15AkJ18JfNcNTQoyjeAlCO0Kei56SWqnu2qLq52TYplg==",
"dev": true,
"license": "MIT",
"dependencies": {
"@storybook/global": "^5.0.0",
"@storybook/react-dom-shim": "10.5.7",
"@storybook/react-dom-shim": "10.5.8",
"react-docgen": "^8.0.2",
"react-docgen-typescript": "^2.2.2"
},
@@ -11114,7 +11114,7 @@
"@types/react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"storybook": "10.5.7",
"storybook": "10.5.8",
"typescript": ">= 4.9.x"
},
"peerDependenciesMeta": {
@@ -11130,9 +11130,9 @@
}
},
"node_modules/@storybook/react-webpack5/node_modules/@storybook/react/node_modules/@storybook/react-dom-shim": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/@storybook/react-dom-shim/-/react-dom-shim-10.5.7.tgz",
"integrity": "sha512-lxOkyh+wu/MiBXvYQHjZfD+DRKOa4bHBzbuGuiHXnHXmdOcTRdcrQTsoeN2FPtfugmmOG66cZUEgDwNX+k5eRA==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/@storybook/react-dom-shim/-/react-dom-shim-10.5.8.tgz",
"integrity": "sha512-N8D13/Xny+V3kfe1KBgsAHS0nKWXLLdgOOXS9poKdYzVwVCN+CGEGBxWX0zMMtdCptqa6/57em9coPlZMoO+bg==",
"dev": true,
"license": "MIT",
"funding": {
@@ -11144,7 +11144,7 @@
"@types/react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0",
"storybook": "10.5.7"
"storybook": "10.5.8"
},
"peerDependenciesMeta": {
"@types/react": {
@@ -15697,9 +15697,9 @@
"license": "MIT"
},
"node_modules/baseline-browser-mapping": {
"version": "2.11.13",
"resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.11.13.tgz",
"integrity": "sha512-k9HNuUVMlqVjQ9UHzfPjIqiDbWw7WqT1AoT7GL8VwvF3r0ZfArtgiSPAlmupyNquNgOJHTuH4CKYf8ttMTWBTQ==",
"version": "2.11.14",
"resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.11.14.tgz",
"integrity": "sha512-JyJ954WzuIR8/FFzX0o5krdSTrBAkcCSRfWSleRsIHSWV+cZe2FI1PKggVkFke1hBldRs+LRxUczzE9iPmgZww==",
"dev": true,
"license": "Apache-2.0",
"bin": {
@@ -20219,9 +20219,9 @@
}
},
"node_modules/eslint-plugin-storybook": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/eslint-plugin-storybook/-/eslint-plugin-storybook-10.5.7.tgz",
"integrity": "sha512-mLpamG1Rsica2jYbUzIZOEuy7Fm1IMtVLMvvxGTpjTVKUMxTXJsANx3MBpH2VSbGQB8Yzlt5399WL/O07K97Ig==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/eslint-plugin-storybook/-/eslint-plugin-storybook-10.5.8.tgz",
"integrity": "sha512-bf9W5nZyWdIaCUZf4aEZnEeD1mn+csNYX8dYUQjAo6L7/DkSLtr65R4zFZ1xeS4m6dOXO6UtUySesCSw4e8w1g==",
"dev": true,
"license": "MIT",
"dependencies": {
@@ -20230,7 +20230,7 @@
},
"peerDependencies": {
"eslint": ">=8",
"storybook": "10.5.7"
"storybook": "10.5.8"
}
},
"node_modules/eslint-plugin-testing-library": {
@@ -37711,9 +37711,9 @@
}
},
"node_modules/storybook": {
"version": "10.5.7",
"resolved": "https://registry.npmjs.org/storybook/-/storybook-10.5.7.tgz",
"integrity": "sha512-oiKvWIwIoOhFP1i6dASYyMXwPHKEtVZMshqSB7EvIVYjWRh0l9H7gHEt1z4Gh2rLGFMekWdsm4s94rvwpR7gkg==",
"version": "10.5.8",
"resolved": "https://registry.npmjs.org/storybook/-/storybook-10.5.8.tgz",
"integrity": "sha512-rR4oFMSiWBSqI0lvsJPtcQUPj8+hzj3TkLu+Mw61Wo6YxPSb5FsLSHai0jZnuaIdKIlmu25KCfwlSQl4e1uvnA==",
"dev": true,
"license": "MIT",
"dependencies": {
@@ -43073,6 +43073,15 @@
"node": ">=12"
}
},
"packages/superset-ui-core/node_modules/dompurify": {
"version": "3.4.13",
"resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.4.13.tgz",
"integrity": "sha512-2vmYIoqjze2d+kakP8S/nS5shfsl587kzwEjcGlTdiksUVgFHnFCsLYDVj/JNqJVOQZGSYBTmuycv0PodwmnMQ==",
"license": "(MPL-2.0 OR Apache-2.0)",
"optionalDependencies": {
"@types/trusted-types": "^2.0.7"
}
},
"packages/superset-ui-core/node_modules/react-ace": {
"version": "14.0.1",
"resolved": "https://registry.npmjs.org/react-ace/-/react-ace-14.0.1.tgz",
+6 -6
View File
@@ -257,9 +257,9 @@
"@istanbuljs/nyc-config-typescript": "^1.0.1",
"@playwright/test": "^1.62.1",
"@pmmmwh/react-refresh-webpack-plugin": "^0.6.2",
"@storybook/addon-docs": "10.5.7",
"@storybook/addon-links": "10.5.7",
"@storybook/react-webpack5": "10.5.7",
"@storybook/addon-docs": "10.5.8",
"@storybook/addon-links": "10.5.8",
"@storybook/react-webpack5": "10.5.8",
"@storybook/test-runner": "0.24.4",
"@svgr/webpack": "^8.1.0",
"@swc/core": "^1.15.47",
@@ -295,7 +295,7 @@
"babel-loader": "^10.1.1",
"babel-plugin-dynamic-import-node": "^2.3.3",
"babel-plugin-jsx-remove-data-test-id": "^3.0.0",
"baseline-browser-mapping": "^2.11.13",
"baseline-browser-mapping": "^2.11.14",
"cheerio": "1.2.0",
"concurrently": "^10.0.4",
"copy-webpack-plugin": "^14.0.0",
@@ -312,7 +312,7 @@
"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-storybook": "10.5.7",
"eslint-plugin-storybook": "10.5.8",
"eslint-plugin-testing-library": "^7.16.2",
"eslint-plugin-theme-colors": "file:eslint-rules/eslint-plugin-theme-colors",
"fetch-mock": "^12.6.0",
@@ -343,7 +343,7 @@
"source-map": "^0.8.0",
"source-map-support": "^0.5.21",
"speed-measure-webpack-plugin": "^1.6.0",
"storybook": "10.5.7",
"storybook": "10.5.8",
"style-loader": "^4.0.0",
"stylelint": "^17.14.1",
"swc-loader": "^0.2.7",
@@ -51,6 +51,20 @@ export interface MetricOptionProps {
shouldShowTooltip?: boolean;
}
/**
* `url` is an arbitrary caller-supplied string rendered as an href. Only
* http(s) and relative URLs become links; other schemes degrade to plain
* text.
*/
function isSafeHref(url: string): boolean {
try {
const { protocol } = new URL(url, window.location.origin);
return protocol === 'http:' || protocol === 'https:';
} catch {
return false;
}
}
export function MetricOption({
metric,
labelRef,
@@ -70,7 +84,7 @@ export function MetricOption({
`}
ref={labelRef}
>
{url ? (
{url && isSafeHref(url) ? (
<Typography.Link
href={url}
target={openInNewWindow ? '_blank' : ''}
@@ -112,3 +112,26 @@ test('shows a Tooltip for the verbose metric name', () => {
const { getByTestId } = setup();
expect(getByTestId('mock-tooltip')).toBeInTheDocument();
});
test('does not render javascript: URLs as links', () => {
// Regression test: the url prop can be creator-authored and must
// never become a script-bearing href for other viewers.
const { queryByRole, getByText } = setup({
url: 'javascript:alert(document.domain)', // eslint-disable-line no-script-url
});
expect(queryByRole('link')).not.toBeInTheDocument();
expect(getByText(defaultProps.metric.verbose_name)).toBeInTheDocument();
});
test('does not render data: URLs as links', () => {
const { queryByRole } = setup({
url: 'data:text/html,<script>alert(1)</script>',
});
expect(queryByRole('link')).not.toBeInTheDocument();
});
test('renders relative URLs as links', () => {
const { getByRole } = setup({
url: '/superset/dashboard/1/',
});
expect(
getByRole('link', { name: defaultProps.metric.verbose_name }),
).toHaveAttribute('href', '/superset/dashboard/1/');
});
@@ -25,8 +25,11 @@ export type Aggregate =
| 'COUNT'
| 'COUNT_DISTINCT'
| 'MAX'
| 'MEDIAN'
| 'MIN'
| 'SUM';
| 'STDDEV_SAMP'
| 'SUM'
| 'VAR_SAMP';
export interface AdhocMetricBase {
hasCustomLabel?: boolean;
@@ -49,5 +49,6 @@ export { default as finestTemporalGrainFormatter } from './formatters/finestTemp
export { default as normalizeTimestamp } from './utils/normalizeTimestamp';
export { default as denormalizeTimestamp } from './utils/denormalizeTimestamp';
export { default as createTimeRangeFromGranularity } from './utils/createTimeRangeFromGranularity';
export * from './types';
@@ -123,6 +123,25 @@ describe('isProbablyHTML', () => {
expect(isProbablyHTML('<canvas></canvas>')).toBe(true);
expect(isProbablyHTML('<iframe src="page.html"></iframe>')).toBe(true);
});
test('should return true for script-capable and foreign-content tags', () => {
expect(isProbablyHTML('<svg onload="alert(1)"></svg>')).toBe(true);
expect(isProbablyHTML('<math><mi>x</mi></math>')).toBe(true);
expect(
isProbablyHTML('<details open ontoggle="alert(1)">x</details>'),
).toBe(true);
expect(isProbablyHTML('<summary>x</summary>')).toBe(true);
expect(isProbablyHTML('<object data="x"></object>')).toBe(true);
expect(isProbablyHTML('<embed src="x">')).toBe(true);
expect(isProbablyHTML('<marquee>x</marquee>')).toBe(true);
expect(isProbablyHTML('<template>x</template>')).toBe(true);
expect(isProbablyHTML('<dialog open>x</dialog>')).toBe(true);
});
test('should return true for elements that parse into document.head', () => {
expect(isProbablyHTML('<style>body { display: none; }</style>')).toBe(true);
expect(isProbablyHTML('<title>injected</title>')).toBe(true);
});
});
describe('sanitizeHtmlIfNeeded', () => {
@@ -137,6 +156,24 @@ describe('sanitizeHtmlIfNeeded', () => {
const sanitizedString = sanitizeHtmlIfNeeded(plainText);
expect(sanitizedString).toEqual(plainText);
});
test('should sanitize svg/details/style payloads instead of passing them through', () => {
const svgPayload = '<svg onload="alert(document.cookie)"></svg>';
const sanitizedSvg = sanitizeHtmlIfNeeded(svgPayload);
expect(sanitizedSvg).not.toContain('<svg');
expect(sanitizedSvg).not.toContain('onload');
// `details` (with its `open` attribute) is in js-xss's default
// whitelist, so the tag itself survives sanitization; the fix is that
// the payload is now routed through FilterXSS at all, which strips the
// non-whitelisted `ontoggle` handler instead of returning it verbatim.
const detailsPayload = '<details open ontoggle="alert(1)">x</details>';
const sanitizedDetails = sanitizeHtmlIfNeeded(detailsPayload);
expect(sanitizedDetails).toEqual('<details open>x</details>');
const stylePayload = '<style>body { display: none; }</style>';
expect(sanitizeHtmlIfNeeded(stylePayload)).not.toContain('<style');
});
});
describe('safeHtmlSpan', () => {
@@ -154,6 +154,20 @@ const KNOWN_HTML_TAGS = new Set([
'html',
'head',
'body',
// Script-capable elements and foreign-content roots (SVG/MathML). These
// must be classified as HTML so that downstream sanitization is applied;
// omitting them makes the heuristic fail open — payloads such as
// `<svg onload=...>` or `<details open ontoggle=...>` would be classified
// "not HTML" and returned verbatim by sanitizeHtmlIfNeeded.
'svg',
'math',
'details',
'summary',
'object',
'embed',
'marquee',
'template',
'dialog',
]);
const HTML_TAG_PATTERN = new RegExp(
@@ -183,10 +197,15 @@ export function isProbablyHTML(text: string) {
const parser = new DOMParser();
const doc = parser.parseFromString(cleanedStr, 'text/html');
// Check if parsing created actual HTML elements (not just text nodes)
const elements = Array.from(doc.body.childNodes).filter(
node => node.nodeType === 1,
) as Element[];
// Check if parsing created actual HTML elements (not just text nodes).
// Some elements (e.g. <style>, <title>, <meta>, <link>) parse into
// document.head rather than document.body, so both must be inspected —
// otherwise a bare <style> payload is classified "not HTML" and skips
// sanitization.
const elements = [
...Array.from(doc.head.childNodes),
...Array.from(doc.body.childNodes),
].filter(node => node.nodeType === 1) as Element[];
// If no elements were created, it's not HTML
if (elements.length === 0) {
@@ -22,7 +22,7 @@ import { Modal } from '../core/Modal';
/**
* Confirm Dialog component for Ant Design Modal.confirm dialogs.
* These are the "OK" / "Cancel" confirmation dialogs used throughout Superset.
* These are the "Confirm" / "Cancel" confirmation dialogs used throughout Superset.
* Uses getByRole with name to target specific confirm dialogs when multiple are open.
*/
export class ConfirmDialog extends Modal {
@@ -43,7 +43,7 @@ export class ConfirmDialog extends Modal {
}
/**
* Clicks the OK button to confirm.
* Clicks the Confirm button to confirm.
* @param options.timeout - If provided, silently returns if dialog doesn't appear
* within timeout. If not provided, waits indefinitely (strict mode).
*/
@@ -53,7 +53,7 @@ export class ConfirmDialog extends Modal {
state: 'visible',
timeout: options?.timeout,
});
await this.clickFooterButton('OK');
await this.clickFooterButton('Confirm');
await this.waitForHidden();
} catch (error) {
// Only swallow TimeoutError when timeout was explicitly provided
@@ -1,58 +0,0 @@
/**
* 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.
*/
/**
* With SOFT_DELETE enabled the delete-confirmation modal becomes recoverable:
* it explains the object is moved to the archive (and for how long), and drops
* the "type DELETE to confirm" friction. Non-destructive the modal is opened
* and dismissed without deleting anything.
*/
import { test, expect } from '@playwright/test';
import { skipUnlessFeatureEnabled } from '../../helpers/featureFlags';
test.beforeEach(async ({ page }) => {
await skipUnlessFeatureEnabled(page, 'SOFT_DELETE');
});
test('chart delete confirmation reflects soft-delete (archive) semantics', async ({
page,
}) => {
await page.goto('chart/list/');
await page.locator('[data-test="chart-row-delete"]').first().waitFor();
await page.locator('[data-test="chart-row-delete"]').first().click();
// The action reads as "Archive", not "Delete". Scope to the dialog: with
// the flag on, every list row's delete action is also named "Archive", so
// an unscoped button query is a strict-mode violation (25 rows + modal).
const dialog = page.getByRole('dialog');
await expect(dialog.getByText(/^Archive .+\?$/)).toBeVisible();
await expect(dialog.getByRole('button', { name: 'Archive' })).toBeVisible();
// Recoverable copy instead of "Are you sure … permanently".
await expect(page.getByText(/moved to Recently Archived/i)).toBeVisible();
await expect(
page.getByText(/recover it there within \d+ days/i),
).toBeVisible();
// No "type DELETE to confirm" input in recoverable mode.
await expect(page.getByTestId('delete-modal-input')).toHaveCount(0);
// Dismiss without deleting.
await page.getByTestId('close-modal-btn').click();
});
@@ -29,7 +29,7 @@
* restore it and asserts via the API that it is live again.
*/
import { test, expect, Page } from '@playwright/test';
import { apiGet, apiPost } from '../../helpers/api/requests';
import { apiGet } from '../../helpers/api/requests';
import { extractIdFromResponse } from '../../helpers/api/assertions';
import {
apiPostChart,
@@ -188,58 +188,3 @@ test('permanently deletes an archived item from the view', async ({ page }) => {
await TYPES[0].softDelete(page, id).catch(() => {});
}
});
test('shows an empty message and no rows when the search matches nothing', async ({
page,
}) => {
await page.goto('archived/');
await expect(page.getByTestId('archived-list-view')).toBeVisible();
const search = page.getByPlaceholder(/type a value/i);
await search.click();
await search.fill(`e2e_nonexistent_${Date.now()}`);
await search.press('Enter');
await expect(
page.getByText('No results match your filter criteria'),
).toBeVisible();
await expect(page.getByTestId('archived-row-restore')).toHaveCount(0);
});
test('restoring an already-restored row surfaces an error without crashing', async ({
page,
}) => {
const name = `e2e_stale_${Date.now()}`;
const id = await TYPES[0].create(page, name);
// Capture the uuid before soft-delete (a soft-deleted GET returns 404).
const { uuid } = (await (await apiGetDashboard(page, id)).json()).result;
try {
expect((await apiDeleteDashboard(page, id)).ok()).toBeTruthy();
await openArchive(page, 'Dashboard', name);
await expect(page.getByText(name, { exact: false })).toBeVisible();
// Simulate another actor restoring the object out from under this view.
const restored = await apiPost(
page,
`api/v1/dashboard/${uuid}/restore`,
{},
);
expect(restored.ok()).toBeTruthy();
// Clicking the now-stale row's Restore yields a 404 → danger toast, no crash.
await page
.getByRole('row')
.filter({ hasText: name })
.getByTestId('archived-row-restore')
.click();
await expect(
page.getByText(`Failed to restore ${name}`, { exact: false }),
).toBeVisible({ timeout: 15000 });
// The page is still functional (the list view did not crash).
await expect(page.getByTestId('archived-list-view')).toBeVisible();
} finally {
// Re-archive the (possibly) restored dashboard, whatever happened above.
await apiDeleteDashboard(page, id).catch(() => {});
}
});
@@ -27,11 +27,21 @@ const StyledTotalCell = styled.div`
`}
`;
// `align` originates from the chart's stored column_config
// (col.config.horizontalAlign), which can be set to an arbitrary string via
// a direct chart-params API write. Emotion compiles interpolated strings as
// CSS source, so the value must be clamped to a closed set of keywords
// before it reaches the stylesheet — never interpolated raw.
const ALLOWED_ALIGN_VALUES = new Set(['left', 'right', 'center']);
const safeAlign = (align?: string) =>
align && ALLOWED_ALIGN_VALUES.has(align) ? align : 'left';
const CellContainer = styled.div<{ backgroundColor?: string; align?: string }>`
display: flex;
background-color: ${({ backgroundColor }) =>
backgroundColor || 'transparent'};
justify-content: ${({ align }) => align || 'left'};
justify-content: ${({ align }) => safeAlign(align)};
`;
const ArrowContainer = styled.div<{ arrowColor?: string }>`
@@ -0,0 +1,59 @@
/**
* 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 '@testing-library/jest-dom';
import { render } from '@superset-ui/core/spec';
import { NumericCellRenderer } from '../src/renderers/NumericCellRenderer';
const renderCell = (horizontalAlign?: string) => {
const params = {
value: 42,
valueFormatted: '42',
node: { rowPinned: undefined, rowIndex: 0 },
hasBasicColorFormatters: false,
basicColorFormatters: [],
col: {
isNumeric: true,
config: horizontalAlign ? { horizontalAlign } : {},
},
valueRange: undefined,
alignPositiveNegative: false,
colorPositiveNegative: false,
} as unknown as Parameters<typeof NumericCellRenderer>[0];
return render(<NumericCellRenderer {...params} />);
};
const collectInjectedCss = () =>
Array.from(document.querySelectorAll('style'))
.map(style => style.textContent ?? '')
.join('\n');
test('applies an allowed horizontalAlign value from column config', () => {
const { container } = renderCell('center');
expect(container.firstChild).toHaveStyle({ justifyContent: 'center' });
});
test('does not compile a malicious horizontalAlign into the stylesheet', () => {
const payload =
'right;} & { position:fixed; top:0; left:0; width:100vw; height:100vh; z-index:99999; background:#fff url(https://attacker.example/beacon) }';
const { container } = renderCell(payload);
const css = collectInjectedCss();
expect(css).not.toContain('position:fixed');
expect(css).not.toContain('attacker.example');
expect(container.firstChild).toHaveStyle({ justifyContent: 'left' });
});
@@ -38,3 +38,11 @@ export const convertUTCTimestampToLocal = (utcTimestamp: number): number => {
const offsetMs = date.getTimezoneOffset() * 60 * 1000;
return utcTimestamp + offsetMs;
};
// Escapes HTML special characters before formatter output reaches an
// innerHTML sink. Mirrors plugin-chart-country-map's escapeHtml.
export const escapeHtml = (text: unknown): string => {
const div = document.createElement('div');
div.textContent = String(text);
return div.innerHTML;
};
@@ -13,6 +13,7 @@ import d3tip from 'd3-tip';
import { t } from '@apache-superset/core/translation';
import { getContrastingColor } from '@superset-ui/core';
import { CALENDAR_TOOLTIP_CLASS } from '../tooltip';
import { escapeHtml } from '../utils';
var d3 = typeof require === 'function' ? require('d3') : window.d3;
@@ -22,14 +23,16 @@ var CalHeatMap = function () {
'use strict';
var self = this;
// d3-tip assigns the .html() return value to the tip node via
// innerHTML, so formatter output is HTML-escaped first.
self.tip = d3tip()
.attr('class', `d3-tip ${CALENDAR_TOOLTIP_CLASS}`)
.direction('n')
.offset([-5, 0])
.html(
d => `
${self.options.timeFormatter(d.t)}: <strong>${self.options.valueFormatter(
d.v,
${escapeHtml(self.options.timeFormatter(d.t))}: <strong>${escapeHtml(
self.options.valueFormatter(d.v),
)}</strong>
`,
);
@@ -37,7 +40,7 @@ var CalHeatMap = function () {
.attr('class', `d3-tip ${CALENDAR_TOOLTIP_CLASS}`)
.direction('n')
.offset([-5, 0])
.html(d => self.options.valueFormatter(d));
.html(d => escapeHtml(self.options.valueFormatter(d)));
this.allowedDataType = ['json', 'csv', 'tsv', 'txt'];
@@ -25,8 +25,12 @@ type FunctionalDateFormat = (date: Date) => string;
interface CalHeatMapInstance {
options: {
dateFormatter: DateFormatter | null;
timeFormatter: (t: number) => string;
valueFormatter: (v: number) => string;
};
formatDate(date: Date, format: string | FunctionalDateFormat): string;
tip: { html(): (d: { t: number; v: number }) => string };
legendTip: { html(): (d: number) => string };
}
const CalHeatMap = CalHeatMapImport as unknown as new () => CalHeatMapInstance;
@@ -59,3 +63,29 @@ test('CalHeatMap keeps the D3 formatter fallback', () => {
expect(calendar.formatDate(date, '%B')).toBe('January');
});
test('cell tooltip HTML escapes creator-controlled formatter output', () => {
// Regression test: the tip's .html() callback is assigned to the
// tooltip node via innerHTML (d3-tip), so formatter output must be
// escaped before it's returned.
const calendar = new CalHeatMap();
calendar.options.timeFormatter = () => '<img src=x onerror=alert(1)>';
calendar.options.valueFormatter = () => '<svg onload=alert(2)>';
const html = calendar.tip.html()({ t: 0, v: 1 });
expect(html).not.toContain('<img');
expect(html).not.toContain('<svg');
expect(html).toContain('&lt;img');
expect(html).toContain('&lt;svg');
});
test('legend tooltip HTML escapes creator-controlled formatter output', () => {
const calendar = new CalHeatMap();
calendar.options.valueFormatter = () => '<img src=x onerror=alert(1)>';
const html = calendar.legendTip.html()(1);
expect(html).not.toContain('<img');
expect(html).toContain('&lt;img');
});
@@ -17,7 +17,11 @@
* under the License.
*/
import { getFormattedUTCTime, convertUTCTimestampToLocal } from '../src/utils';
import {
getFormattedUTCTime,
convertUTCTimestampToLocal,
escapeHtml,
} from '../src/utils';
test('getFormattedUTCTime formats local timestamp for display as UTC date', () => {
const utcTimestamp = 1420070400000; // 2015-01-01 00:00:00 UTC
@@ -87,3 +91,22 @@ test('convertUTCTimestampToLocal and getFormattedUTCTime work together to displa
const formattedTime = getFormattedUTCTime(localTimestamp, '%Y-%m-%d');
expect(formattedTime).toContain('2024-01-01');
});
test('escapeHtml neutralizes markup smuggled through a time format string', () => {
// Regression test: d3-time-format passes non-% characters through
// verbatim, so escaping must happen before the innerHTML sink.
const formatted = getFormattedUTCTime(
1704067200000,
'%Y <img src=x onerror=alert(1)>',
);
const escaped = escapeHtml(formatted);
expect(formatted).toContain('<img');
expect(escaped).not.toContain('<img');
expect(escaped).toContain('&lt;img');
});
test('escapeHtml stringifies non-string formatter output safely', () => {
expect(escapeHtml(1234)).toEqual('1234');
expect(escapeHtml('a & b < c')).toEqual('a &amp; b &lt; c');
});
@@ -33,6 +33,27 @@ import { WmsLayerConf, WfsLayerConf, LayerConf, XyzLayerConf } from '../types';
import { isWfsLayerConf, isWmsLayerConf, isXyzLayerConf } from '../typeguards';
import { isVersionBelow } from './serviceUtil';
/**
* Escape HTML special characters in a layer attribution string.
*
* OpenLayers' Attribution control renders attribution strings via innerHTML,
* and the attribution here comes from creator-supplied chart form data, so it
* must be treated as untrusted text rather than markup to prevent stored XSS.
*
* @param attribution The attribution string from the layer configuration
*
* @returns The attribution with HTML special characters escaped
*/
export const escapeAttribution = (attribution?: string): string | undefined =>
attribution === undefined
? undefined
: attribution
.replace(/&/g, '&amp;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
.replace(/"/g, '&quot;')
.replace(/'/g, '&#039;');
/**
* Create a WMS layer.
*
@@ -49,7 +70,7 @@ export const createWmsLayer = (wmsLayerConf: WmsLayerConf) => {
LAYERS: layersParam,
VERSION: version,
},
attributions: attribution,
attributions: escapeAttribution(attribution),
}),
});
};
@@ -66,7 +87,7 @@ export const createXyzLayer = (xyzLayerConf: XyzLayerConf) => {
return new TileLayer({
source: new XyzSource({
url,
attributions: attribution,
attributions: escapeAttribution(attribution),
}),
});
};
@@ -90,7 +111,7 @@ export const createWfsLayer = async (wfsLayerConf: WfsLayerConf) => {
const wfsSource = new VectorSource({
format: new GeoJSON(),
attributions: attribution,
attributions: escapeAttribution(attribution),
url: extent => {
const requestUrl = new URL(url);
const params = requestUrl.searchParams;
@@ -17,20 +17,65 @@
* under the License.
*/
import { WfsLayerConf } from '../../src/types';
import { WfsLayerConf, WmsLayerConf, XyzLayerConf } from '../../src/types';
import {
createLayer,
createWfsLayer,
createWmsLayer,
createXyzLayer,
escapeAttribution,
} from '../../src/util/layerUtil';
describe('layerUtil', () => {
describe('escapeAttribution', () => {
test('escapes HTML markup in attribution strings', () => {
expect(escapeAttribution('(c) OSM <img src=x onerror=alert(1)>')).toBe(
'(c) OSM &lt;img src=x onerror=alert(1)&gt;',
);
expect(escapeAttribution('a & "b" \'c\'')).toBe(
'a &amp; &quot;b&quot; &#039;c&#039;',
);
expect(escapeAttribution(undefined)).toBeUndefined();
});
});
describe('createWmsLayer', () => {
test('exists', () => {
// function is trivial
expect(createWmsLayer).toBeDefined();
});
test('escapes HTML in the layer attribution', () => {
const wmsLayerConf: WmsLayerConf = {
title: 'wms',
type: 'WMS',
url: 'https://ows-demo.terrestris.de/geoserver/osm/wms',
version: '1.3.0',
layersParam: 'osm:osm-fuel',
attribution: '(c) OSM <img src=x onerror=alert(1)>',
};
const layer = createWmsLayer(wmsLayerConf);
const attributions = layer.getSource()?.getAttributions();
expect(attributions?.(undefined as never)).toEqual([
'(c) OSM &lt;img src=x onerror=alert(1)&gt;',
]);
});
});
describe('createXyzLayer', () => {
test('escapes HTML in the layer attribution', () => {
const xyzLayerConf: XyzLayerConf = {
title: 'osm',
type: 'XYZ',
url: 'https://tile.openstreetmap.org/{z}/{x}/{y}.png',
attribution: '(c) OSM <img src=x onerror=alert(1)>',
};
const layer = createXyzLayer(xyzLayerConf);
const attributions = layer.getSource()?.getAttributions();
expect(attributions?.(undefined as never)).toEqual([
'(c) OSM &lt;img src=x onerror=alert(1)&gt;',
]);
});
});
describe('createWfsLayer', () => {
@@ -80,6 +80,7 @@ import {
getAnnotationData,
} from '../utils/annotation';
import {
collapseForecastKeys,
extractForecastSeriesContext,
extractForecastValuesFromTooltipParams,
formatForecastTooltipSeries,
@@ -769,7 +770,9 @@ export default function transformProps(
nameGap: xAxisTitleMarginPx,
nameLocation: 'middle',
axisLabel: {
hideOverlap: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
hideOverlap: showMaxLabel
? false
: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
formatter: deduplicatedFormatter,
rotate: xAxisLabelRotation,
interval: xAxisLabelInterval,
@@ -861,12 +864,14 @@ export default function transformProps(
: params.value[0];
const forecastValue: any[] = richTooltip ? params : [params];
const sortedKeys = extractTooltipKeys(
forecastValue,
// horizontal mode is not supported in mixed series chart
1,
richTooltip,
tooltipSortByMetric,
const sortedKeys = collapseForecastKeys(
extractTooltipKeys(
forecastValue,
// horizontal mode is not supported in mixed series chart
1,
richTooltip,
tooltipSortByMetric,
),
);
const rows: string[][] = [];
@@ -18,6 +18,7 @@
*/
import { t } from '@apache-superset/core/translation';
import { NumberFormatter } from '@superset-ui/core';
import { sanitizeHtml } from '../utils/series';
/*
function for finding the max metric values among all series data for Radar Chart
@@ -63,7 +64,7 @@ export const renderNormalizedTooltip = (
const { color, name = '', value: values } = params;
const seriesName = name || 'series0';
const colorDot = `<span style="display:inline-block;margin-right:5px;border-radius:50%;width:5px;height:5px;background-color:${color}"></span>`;
const colorDot = `<span style="display:inline-block;margin-right:5px;border-radius:50%;width:5px;height:5px;background-color:${sanitizeHtml(color)}"></span>`;
// Get metric values with denormalization if needed
const metricValues: TooltipMetricValue[] = metrics.map((metric, index) => {
@@ -85,19 +86,26 @@ export const renderNormalizedTooltip = (
};
});
// Tooltip is rendered via innerHTML (ECharts default renderMode
// 'html'), so seriesName/metric/value/color are HTML-escaped, matching
// the treatment every other echarts tooltip path applies.
const tooltipRows = metricValues
.map(
({ metric, value }) => `
<div style="display:flex;">
<div>${colorDot}${metric}:</div>
<div style="font-weight:bold;margin-left:auto;">${value}</div>
<div>${colorDot}${sanitizeHtml(metric)}:</div>
<div style="font-weight:bold;margin-left:auto;">${sanitizeHtml(
String(value),
)}</div>
</div>
`,
)
.join('');
return `
<div style="font-weight:bold;margin-bottom:5px;">${seriesName}</div>
<div style="font-weight:bold;margin-bottom:5px;">${sanitizeHtml(
seriesName,
)}</div>
${tooltipRows}
`;
};
@@ -21,8 +21,14 @@ import {
waitFor,
cleanup,
} from '../../../../spec/helpers/testing-library';
import { AxisType, TimeGranularity } from '@superset-ui/core';
import {
AxisType,
createTimeRangeFromGranularity,
DTTM_ALIAS,
TimeGranularity,
} from '@superset-ui/core';
import { GenericDataType } from '@apache-superset/core/common';
import { logging } from '@apache-superset/core/utils';
import type { EChartsCoreOption } from 'echarts/core';
import type { ECElementEvent } from 'echarts/types/src/util/types';
import type { ReactNode } from 'react';
@@ -93,12 +99,17 @@ afterAll(() => {
});
afterEach(() => {
jest.useRealTimers();
cleanup();
mockEchart.mockReset();
(globalThis as { ResizeObserver?: typeof ResizeObserver }).ResizeObserver =
originalResizeObserver;
});
const advanceClickTimer = () => {
jest.advanceTimersByTime(300);
};
const defaultFormData: EchartsTimeseriesFormData & {
vizType: string;
dateFormat: string;
@@ -314,7 +325,8 @@ test('falls back to window resize listener when ResizeObserver is unavailable',
});
// Test for issue #25334: Bar chart cross-filter without dimensions
test('emits cross-filter on X-axis value when no dimensions and categorical X-axis', async () => {
test('emits cross-filter on X-axis value when no dimensions and categorical X-axis', () => {
jest.useFakeTimers();
const setDataMaskMock = jest.fn();
const propsWithCategoricalXAxis: TimeseriesChartTransformedProps = {
@@ -348,13 +360,7 @@ test('emits cross-filter on X-axis value when no dimensions and categorical X-ax
dataIndex: 0,
});
// Wait for the timer (TIMER_DURATION = 300ms)
await waitFor(
() => {
expect(setDataMaskMock).toHaveBeenCalled();
},
{ timeout: 500 },
);
advanceClickTimer();
// Verify the cross-filter uses the X-axis column and value, not the metric
const dataMaskCall = setDataMaskMock.mock.calls[0][0];
@@ -368,7 +374,8 @@ test('emits cross-filter on X-axis value when no dimensions and categorical X-ax
}
});
test('emits cross-filter on category value for horizontal bar clicks', async () => {
test('emits cross-filter on category value for horizontal bar clicks', () => {
jest.useFakeTimers();
const setDataMaskMock = jest.fn();
render(
@@ -397,12 +404,7 @@ test('emits cross-filter on category value for horizontal bar clicks', async ()
dataIndex: 0,
});
await waitFor(
() => {
expect(setDataMaskMock).toHaveBeenCalled();
},
{ timeout: 500 },
);
advanceClickTimer();
expect(setDataMaskMock.mock.calls[0][0].extraFormData.filters).toEqual([
{
@@ -424,9 +426,7 @@ test('uses rendered categorical axis for query event handlers', () => {
/>,
);
expect(getLatestEchartProps().queryEventHandlers?.[0].query).toBe(
'xAxis.category',
);
expect(getLatestEchartProps().queryEventHandlers?.[0].query).toBe('xAxis');
cleanup();
mockEchart.mockReset();
@@ -445,9 +445,7 @@ test('uses rendered categorical axis for query event handlers', () => {
/>,
);
expect(getLatestEchartProps().queryEventHandlers?.[0].query).toBe(
'yAxis.category',
);
expect(getLatestEchartProps().queryEventHandlers?.[0].query).toBe('yAxis');
});
test('emits cross-filter from horizontal categorical axis label clicks', () => {
@@ -473,8 +471,9 @@ test('emits cross-filter from horizontal categorical axis label clicks', () => {
getLatestEchartProps().queryEventHandlers?.[0].handler;
expect(labelClickHandler).toBeDefined();
labelClickHandler?.({
targetType: 'axisLabel',
value: 'Product A',
} as ECElementEvent);
} as unknown as ECElementEvent);
expect(setDataMaskMock.mock.calls[0][0].extraFormData.filters).toEqual([
{
@@ -485,7 +484,8 @@ test('emits cross-filter from horizontal categorical axis label clicks', () => {
]);
});
test('does not emit duplicate cross-filter for generic axis label clicks', async () => {
test('does not emit duplicate cross-filter for generic axis label clicks', () => {
jest.useFakeTimers();
const setDataMaskMock = jest.fn();
render(
@@ -507,11 +507,28 @@ test('does not emit duplicate cross-filter for generic axis label clicks', async
name: 'Product A',
});
await new Promise(resolve => setTimeout(resolve, 400));
jest.advanceTimersByTime(400);
expect(setDataMaskMock).not.toHaveBeenCalled();
});
test('does not emit cross-filter when no dimensions and time-based X-axis', async () => {
test('keeps temporal range exclusive ends on whole-second boundaries', () => {
const clickedTimestamp = new Date(Date.UTC(2021, 0, 15, 12, 34, 56, 789));
[TimeGranularity.DAY, TimeGranularity.MONTH, TimeGranularity.YEAR].forEach(
grain => {
const [, inclusiveEnd] = createTimeRangeFromGranularity(
clickedTimestamp,
grain,
false,
);
const exclusiveEnd = new Date(inclusiveEnd.getTime() + 1);
expect(exclusiveEnd.getUTCMilliseconds()).toBe(0);
},
);
});
test('emits TEMPORAL_RANGE cross-filter from time axis label click on day bucket', () => {
const setDataMaskMock = jest.fn();
const propsWithTimeXAxis: TimeseriesChartTransformedProps = {
@@ -519,39 +536,610 @@ test('does not emit cross-filter when no dimensions and time-based X-axis', asyn
emitCrossFilters: true,
setDataMask: setDataMaskMock,
groupby: [], // No dimensions
resolvedTimeGrain: TimeGranularity.DAY,
formData: {
...defaultFormData,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.DAY,
},
xAxis: {
label: '__timestamp',
type: AxisType.Time, // Time-based X-axis (not categorical)
label: DTTM_ALIAS,
type: AxisType.Time,
},
};
render(<EchartsTimeseries {...propsWithTimeXAxis} />);
const lastCall = mockEchart.mock.calls.at(-1);
expect(lastCall).toBeDefined();
const [props] = lastCall as [EchartsProps];
const labelClickHandler = getLatestEchartProps().queryEventHandlers?.find(
({ query }) => query === 'xAxis',
)?.handler;
expect(labelClickHandler).toBeDefined();
labelClickHandler?.({
targetType: 'axisLabel',
value: '2021-01-01',
} as unknown as ECElementEvent);
// Simulate a click event
const clickHandler = props.eventHandlers?.click;
if (clickHandler) {
clickHandler({
componentType: 'series',
seriesName: 'Sales',
data: [1609459200000, 100], // Timestamp
name: '2021-01-01',
dataIndex: 0,
});
expect(setDataMaskMock.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-01-02T00:00:00',
},
]);
});
// Wait a bit and verify setDataMask was NOT called
await new Promise(resolve => setTimeout(resolve, 400));
expect(setDataMaskMock).not.toHaveBeenCalled();
}
test('emits TEMPORAL_RANGE cross-filter from time axis label click on month bucket', () => {
const setDataMaskMock = jest.fn();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={[]}
resolvedTimeGrain={TimeGranularity.MONTH}
formData={{
...defaultFormData,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const labelClickHandler = getLatestEchartProps().queryEventHandlers?.find(
({ query }) => query === 'xAxis',
)?.handler;
expect(labelClickHandler).toBeDefined();
labelClickHandler?.({
targetType: 'axisLabel',
value: '2021-01-01',
} as unknown as ECElementEvent);
expect(setDataMaskMock.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-02-01T00:00:00',
},
]);
});
test('emits TEMPORAL_RANGE cross-filter from time axis label click on year bucket', () => {
const setDataMaskMock = jest.fn();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={[]}
resolvedTimeGrain={TimeGranularity.YEAR}
formData={{
...defaultFormData,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.YEAR,
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const labelClickHandler = getLatestEchartProps().queryEventHandlers?.find(
({ query }) => query === 'xAxis',
)?.handler;
expect(labelClickHandler).toBeDefined();
labelClickHandler?.({
targetType: 'axisLabel',
value: '2021-01-01',
} as unknown as ECElementEvent);
expect(setDataMaskMock.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2022-01-01T00:00:00',
},
]);
});
test('emits upper-exclusive TEMPORAL_RANGE from time point click on month bucket', () => {
jest.useFakeTimers();
const setDataMaskMock = jest.fn();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={[]}
resolvedTimeGrain={TimeGranularity.MONTH}
formData={{
...defaultFormData,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const clickHandler = getLatestEchartProps().eventHandlers?.click;
expect(clickHandler).toBeDefined();
clickHandler?.({
componentType: 'series',
seriesName: 'Sales',
data: [Date.UTC(2021, 0, 1), 100],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMaskMock.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-02-01T00:00:00',
},
]);
});
test('emits TEMPORAL_RANGE from string-typed time point click value', () => {
jest.useFakeTimers();
const setDataMaskMock = jest.fn();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={[]}
resolvedTimeGrain={TimeGranularity.MONTH}
formData={{
...defaultFormData,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const clickHandler = getLatestEchartProps().eventHandlers?.click;
expect(clickHandler).toBeDefined();
clickHandler?.({
componentType: 'series',
seriesName: 'Sales',
data: ['2021-01-01T00:00:00Z', 100],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMaskMock.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-02-01T00:00:00',
},
]);
});
test('uses resolved time grain for temporal point-click cross-filter', () => {
jest.useFakeTimers();
const setDataMaskMock = jest.fn();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={[]}
resolvedTimeGrain={TimeGranularity.MONTH}
formData={{
...defaultFormData,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.DAY,
extraFormData: {
time_grain_sqla: TimeGranularity.MONTH,
},
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const clickHandler = getLatestEchartProps().eventHandlers?.click;
expect(clickHandler).toBeDefined();
clickHandler?.({
componentType: 'series',
seriesName: 'Sales',
data: [Date.UTC(2021, 0, 1), 100],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMaskMock.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-02-01T00:00:00',
},
]);
});
test('emits TEMPORAL_RANGE from horizontal time point click using timestamp, not metric', () => {
jest.useFakeTimers();
const setDataMaskMock = jest.fn();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={[]}
resolvedTimeGrain={TimeGranularity.MONTH}
formData={{
...defaultFormData,
orientation: OrientationType.Horizontal,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const clickHandler = getLatestEchartProps().eventHandlers?.click;
expect(clickHandler).toBeDefined();
clickHandler?.({
componentType: 'series',
seriesName: 'Sales',
data: [129, Date.UTC(2021, 0, 1)],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMaskMock.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-02-01T00:00:00',
},
]);
});
test('emits TEMPORAL_RANGE cross-filter from horizontal time axis label click', () => {
const setDataMaskMock = jest.fn();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={[]}
resolvedTimeGrain={TimeGranularity.MONTH}
formData={{
...defaultFormData,
orientation: OrientationType.Horizontal,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const labelClickHandler = getLatestEchartProps().queryEventHandlers?.find(
({ query }) => query === 'yAxis',
)?.handler;
expect(labelClickHandler).toBeDefined();
labelClickHandler?.({
targetType: 'axisLabel',
value: '2021-01-01',
} as unknown as ECElementEvent);
expect(setDataMaskMock.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-02-01T00:00:00',
},
]);
});
test('warns and skips temporal axis label cross-filter when label value cannot be parsed', () => {
const setDataMaskMock = jest.fn();
const warn = jest.spyOn(logging, 'warn').mockImplementation();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={[]}
formData={{
...defaultFormData,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const labelClickHandler = getLatestEchartProps().queryEventHandlers?.find(
({ query }) => query === 'xAxis',
)?.handler;
expect(labelClickHandler).toBeDefined();
labelClickHandler?.({
targetType: 'axisLabel',
value: 'not-a-date',
} as unknown as ECElementEvent);
expect(setDataMaskMock).not.toHaveBeenCalled();
expect(warn).toHaveBeenCalledWith(
'Unable to parse time axis value for cross-filtering',
'not-a-date',
);
warn.mockRestore();
});
test('logs and skips temporal point-click cross-filter when string value cannot be parsed', () => {
jest.useFakeTimers();
const setDataMaskMock = jest.fn();
const warn = jest.spyOn(logging, 'warn').mockImplementation();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={[]}
resolvedTimeGrain={TimeGranularity.MONTH}
formData={{
...defaultFormData,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const clickHandler = getLatestEchartProps().eventHandlers?.click;
expect(clickHandler).toBeDefined();
clickHandler?.({
componentType: 'series',
seriesName: 'Sales',
data: ['not-a-date', 100],
name: 'not-a-date',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMaskMock).not.toHaveBeenCalled();
expect(warn).toHaveBeenCalledWith(
'Unable to parse time axis value for cross-filtering',
'not-a-date',
);
warn.mockRestore();
});
test('emits empty temporal X-axis data mask when filter grain is missing', () => {
jest.useFakeTimers();
const setDataMaskMock = jest.fn();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={[]}
formData={{
...defaultFormData,
granularitySqla: 'ds',
timeGrainSqla: undefined,
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const clickHandler = getLatestEchartProps().eventHandlers?.click;
expect(clickHandler).toBeDefined();
clickHandler?.({
componentType: 'series',
seriesName: 'Sales',
data: [Date.UTC(2021, 0, 1), 100],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMaskMock.mock.calls[0][0]).toEqual({
extraFormData: {
filters: [],
},
filterState: {
label: undefined,
value: null,
selectedValues: null,
},
});
});
test('clears temporal X-axis cross-filter when clicking selected bucket again', () => {
jest.useFakeTimers();
const setDataMaskMock = jest.fn();
const selectedRange = '2021-01-01T00:00:00 : 2021-02-01T00:00:00';
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={[]}
selectedValues={{ 0: selectedRange }}
resolvedTimeGrain={TimeGranularity.MONTH}
formData={{
...defaultFormData,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const clickHandler = getLatestEchartProps().eventHandlers?.click;
expect(clickHandler).toBeDefined();
clickHandler?.({
componentType: 'series',
seriesName: 'Sales',
data: [Date.UTC(2021, 0, 1), 100],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMaskMock.mock.calls[0][0]).toEqual({
extraFormData: {
filters: [],
},
filterState: {
label: undefined,
value: null,
selectedValues: null,
},
});
});
test('does not emit temporal X-axis label cross-filter when dimensions are set', () => {
const setDataMaskMock = jest.fn();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={['country']}
formData={{
...defaultFormData,
groupby: ['country'],
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const labelClickHandler = getLatestEchartProps().queryEventHandlers?.find(
({ query }) => query === 'xAxis',
)?.handler;
expect(labelClickHandler).toBeDefined();
labelClickHandler?.({
targetType: 'axisLabel',
value: '2021-01-01',
} as unknown as ECElementEvent);
expect(setDataMaskMock).not.toHaveBeenCalled();
});
test('does not emit temporal X-axis cross-filter when dimensions are set', () => {
jest.useFakeTimers();
const setDataMaskMock = jest.fn();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
setDataMask={setDataMaskMock}
groupby={['country']}
labelMap={{
Sales: ['US'],
}}
formData={{
...defaultFormData,
groupby: ['country'],
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const clickHandler = getLatestEchartProps().eventHandlers?.click;
expect(clickHandler).toBeDefined();
clickHandler?.({
componentType: 'series',
seriesName: 'Sales',
data: [Date.UTC(2021, 0, 1), 100],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMaskMock.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'country',
op: 'IN',
val: ['US'],
},
]);
expect(
setDataMaskMock.mock.calls[0][0].extraFormData.filters.some(
(filter: { op: string }) => filter.op === 'TEMPORAL_RANGE',
),
).toBe(false);
});
// Test for issue #41102: horizontal bar cross-filter must use the category
// value, not the metric. For horizontal bars the data tuple is value-first
// (e.g. [100, 'Product A']), so relying on data[0] emitted the metric value.
test('emits cross-filter on the category value for a horizontal categorical bar', async () => {
test('emits cross-filter on the category value for a horizontal categorical bar', () => {
jest.useFakeTimers();
const setDataMaskMock = jest.fn();
const propsWithHorizontalXAxis: TimeseriesChartTransformedProps = {
@@ -585,12 +1173,7 @@ test('emits cross-filter on the category value for a horizontal categorical bar'
dataIndex: 0,
});
await waitFor(
() => {
expect(setDataMaskMock).toHaveBeenCalled();
},
{ timeout: 500 },
);
advanceClickTimer();
// Must filter on the category ('Product A'), not the metric value (100)
const dataMaskCall = setDataMaskMock.mock.calls[0][0];
@@ -604,6 +1187,55 @@ test('emits cross-filter on the category value for a horizontal categorical bar'
}
});
test('context menu cross-filter is available for a temporal bar point', async () => {
const onContextMenuMock = jest.fn();
render(
<EchartsTimeseries
{...defaultProps}
emitCrossFilters
onContextMenu={onContextMenuMock}
groupby={[]}
resolvedTimeGrain={TimeGranularity.MONTH}
formData={{
...defaultFormData,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.DAY,
extraFormData: {
time_grain_sqla: TimeGranularity.MONTH,
},
}}
xAxis={{
label: DTTM_ALIAS,
type: AxisType.Time,
}}
/>,
);
const contextMenuHandler = getLatestEchartProps().eventHandlers?.contextmenu;
expect(contextMenuHandler).toBeDefined();
await contextMenuHandler?.({
componentType: 'series',
seriesName: 'Sales',
data: [Date.UTC(2021, 0, 1), 100],
name: '2021-01-01',
event: { stop: jest.fn(), event: { clientX: 10, clientY: 20 } },
});
await waitFor(() => {
expect(onContextMenuMock).toHaveBeenCalled();
});
const { crossFilter } = onContextMenuMock.mock.calls[0][2];
expect(crossFilter.dataMask.extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-02-01T00:00:00',
},
]);
});
// Test for issue #41102: the context-menu ("Add cross-filter") path must also
// use the category value, not the metric, for a horizontal categorical bar.
test('context menu cross-filter uses the category value for a horizontal categorical bar', async () => {
@@ -21,14 +21,17 @@ import {
DTTM_ALIAS,
BinaryQueryObjectFilterClause,
AxisType,
type TimeGranularity,
getTimeFormatter,
getColumnLabel,
getNumberFormatter,
LegendState,
ensureIsArray,
createTimeRangeFromGranularity,
} from '@superset-ui/core';
import { useTheme } from '@apache-superset/core/theme';
import { GenericDataType } from '@apache-superset/core/common';
import { logging } from '@apache-superset/core/utils';
import type {
ECElementEvent,
ViewRootGroup,
@@ -48,6 +51,29 @@ import { getTemporalXAxisDrillByFilter } from '../utils/xAxisDrillByFilter';
import { ExtraControls } from '../components/ExtraControls';
const TIMER_DURATION = 300;
const getTimestampFromTimeAxisValue = (value: string | number) => {
if (typeof value === 'number') {
return Number.isFinite(value) ? value : undefined;
}
const timestamp = Date.parse(value);
if (Number.isNaN(timestamp)) {
logging.warn('Unable to parse time axis value for cross-filtering', value);
}
return Number.isNaN(timestamp) ? undefined : timestamp;
};
// Day, month, and year ranges end at 23:59:59.999, so adding 1ms lands on a
// whole-second next bucket boundary. The formatter intentionally emits seconds.
const formatDateTime = (date: Date) =>
`${[
date.getUTCFullYear(),
String(date.getUTCMonth() + 1).padStart(2, '0'),
String(date.getUTCDate()).padStart(2, '0'),
].join('-')}T${[
String(date.getUTCHours()).padStart(2, '0'),
String(date.getUTCMinutes()).padStart(2, '0'),
String(date.getUTCSeconds()).padStart(2, '0'),
].join(':')}`;
// Percent-change draggable baseline handle geometry, in pixels.
const BASELINE_HANDLE_WIDTH = 8;
@@ -71,6 +97,7 @@ export default function EchartsTimeseries({
onFocusedSeries,
xValueFormatter,
xAxis,
resolvedTimeGrain,
refs,
emitCrossFilters,
coltypeMapping,
@@ -363,6 +390,65 @@ export default function EchartsTimeseries({
[selectedValues, xAxis.label],
);
const getTimeAxisCrossFilterDataMask = useCallback(
(clickedTimestamp: number) => {
const filterColumn =
xAxis.label === DTTM_ALIAS ? formData.granularitySqla : xAxis.label;
const grain = resolvedTimeGrain as TimeGranularity | undefined;
if (!filterColumn || !grain) {
return {
dataMask: {
extraFormData: {
filters: [],
},
filterState: {
label: undefined,
value: null,
selectedValues: null,
},
},
isCurrentValueSelected: false,
};
}
const [start, inclusiveEnd] = createTimeRangeFromGranularity(
new Date(clickedTimestamp),
grain,
false,
);
const exclusiveEnd = new Date(inclusiveEnd.getTime() + 1);
const timeRange = `${formatDateTime(start)} : ${formatDateTime(exclusiveEnd)}`;
const selected: string[] = Object.values(selectedValues);
const isCurrentValueSelected = selected.includes(timeRange);
const values = isCurrentValueSelected ? [] : [timeRange];
return {
dataMask: {
extraFormData: {
filters:
values.length === 0
? []
: [
{
col: filterColumn,
op: 'TEMPORAL_RANGE' as const,
val: timeRange,
},
],
},
filterState: {
label: values.length ? values : undefined,
value: values.length ? values : null,
selectedValues: values.length ? values : null,
},
},
isCurrentValueSelected,
};
},
[formData.granularitySqla, resolvedTimeGrain, selectedValues, xAxis.label],
);
const handleChange = useCallback(
(value: string) => {
if (!emitCrossFilters) {
@@ -384,15 +470,26 @@ export default function EchartsTimeseries({
[emitCrossFilters, setDataMask, getXAxisCrossFilterDataMask],
);
const handleTimeAxisChange = useCallback(
(clickedTimestamp: number) => {
if (!emitCrossFilters) {
return;
}
setDataMask(getTimeAxisCrossFilterDataMask(clickedTimestamp).dataMask);
},
[emitCrossFilters, setDataMask, getTimeAxisCrossFilterDataMask],
);
// Determine if X-axis can be used for cross-filtering (categorical axis without dimensions)
const canCrossFilterByXAxis =
!hasDimensions && xAxis.type === AxisType.Category;
const categoryAxisValueIndex =
!hasDimensions &&
(xAxis.type === AxisType.Category || xAxis.type === AxisType.Time);
const xAxisValueIndex =
formData.orientation === OrientationType.Horizontal ? 1 : 0;
const getCategoryAxisValue = useCallback(
const getXAxisValue = useCallback(
(data: unknown, name: unknown) => {
if (Array.isArray(data)) {
const categoryAxisValue = data[categoryAxisValueIndex];
const categoryAxisValue = data[xAxisValueIndex];
if (
typeof categoryAxisValue === 'string' ||
typeof categoryAxisValue === 'number'
@@ -405,7 +502,7 @@ export default function EchartsTimeseries({
}
return undefined;
},
[categoryAxisValueIndex],
[xAxisValueIndex],
);
const eventHandlers: EventHandlers = {
@@ -423,15 +520,28 @@ export default function EchartsTimeseries({
// Cross-filter by dimension (original behavior)
const { seriesName: name } = props;
handleChange(name);
} else if (canCrossFilterByXAxis && props.componentType === 'series') {
} else if (
canCrossFilterByXAxis &&
xAxis.type === AxisType.Category &&
props.componentType === 'series'
) {
// Cross-filter by X-axis value when no dimensions (issue #25334)
const categoryAxisValue = getCategoryAxisValue(
props.data,
props.name,
);
const categoryAxisValue = getXAxisValue(props.data, props.name);
if (categoryAxisValue !== undefined) {
handleXAxisChange(categoryAxisValue);
}
} else if (
canCrossFilterByXAxis &&
xAxis.type === AxisType.Time &&
props.componentType === 'series'
) {
const timeAxisValue = getXAxisValue(props.data, props.name);
if (timeAxisValue !== undefined) {
const timestamp = getTimestampFromTimeAxisValue(timeAxisValue);
if (timestamp !== undefined) {
handleTimeAxisChange(timestamp);
}
}
}
}, TIMER_DURATION);
},
@@ -466,17 +576,20 @@ export default function EchartsTimeseries({
];
const groupBy = ensureIsArray(formData.groupby);
if (data && xAxis.type === AxisType.Time) {
drillToDetailFilters.push({
col:
// if the xAxis is '__timestamp', granularity_sqla will be the column of filter
xAxis.label === DTTM_ALIAS
? formData.granularitySqla
: xAxis.label,
grain: formData.timeGrainSqla,
op: '==',
val: data[0],
formattedVal: xValueFormatter(data[0]),
});
const timeAxisValue = getXAxisValue(data, eventParams.name);
if (timeAxisValue !== undefined) {
drillToDetailFilters.push({
col:
// if the xAxis is '__timestamp', granularity_sqla will be the column of filter
xAxis.label === DTTM_ALIAS
? formData.granularitySqla
: xAxis.label,
grain: resolvedTimeGrain,
op: '==',
val: timeAxisValue,
formattedVal: xValueFormatter(timeAxisValue),
});
}
}
[
...(xAxis.type === AxisType.Category && data ? [xAxis.label] : []),
@@ -517,9 +630,7 @@ export default function EchartsTimeseries({
xAxis.label === DTTM_ALIAS ? formData.granularitySqla : xAxis.label;
if (data && xAxis.type === AxisType.Time && xAxisCol) {
// For horizontal orientation the [x, value] pair is swapped
const xValue = Array.isArray(data)
? data[categoryAxisValueIndex]
: data;
const xValue = Array.isArray(data) ? data[xAxisValueIndex] : data;
const xAxisFilter = getTemporalXAxisDrillByFilter(
xAxisCol,
xValue,
@@ -530,10 +641,7 @@ export default function EchartsTimeseries({
xAxisFilters.push(xAxisFilter);
}
} else if (xAxis.type === AxisType.Category && xAxisCol) {
const categoryAxisValue = getCategoryAxisValue(
data,
eventParams.name,
);
const categoryAxisValue = getXAxisValue(data, eventParams.name);
if (categoryAxisValue !== undefined) {
// A category axis can still sit on a temporal column when the
// axis is forced categorical; filter by time bucket in that case
@@ -564,15 +672,25 @@ export default function EchartsTimeseries({
crossFilter = getCrossFilterDataMask(seriesName);
} else if (
canCrossFilterByXAxis &&
xAxis.type === AxisType.Category &&
eventParams.componentType === 'series'
) {
const categoryAxisValue = getCategoryAxisValue(
data,
eventParams.name,
);
const categoryAxisValue = getXAxisValue(data, eventParams.name);
if (categoryAxisValue !== undefined) {
crossFilter = getXAxisCrossFilterDataMask(categoryAxisValue);
}
} else if (
canCrossFilterByXAxis &&
xAxis.type === AxisType.Time &&
eventParams.componentType === 'series'
) {
const timeAxisValue = getXAxisValue(data, eventParams.name);
if (timeAxisValue !== undefined) {
const timestamp = getTimestampFromTimeAxisValue(timeAxisValue);
if (timestamp !== undefined) {
crossFilter = getTimeAxisCrossFilterDataMask(timestamp);
}
}
}
onContextMenu(pointerEvent.clientX, pointerEvent.clientY, {
@@ -593,26 +711,39 @@ export default function EchartsTimeseries({
const { value } = event;
if (
canCrossFilterByXAxis &&
event.targetType === 'axisLabel' &&
(typeof value === 'string' || typeof value === 'number')
) {
handleXAxisChange(value);
if (xAxis.type === AxisType.Time) {
const timestamp = getTimestampFromTimeAxisValue(value);
if (timestamp !== undefined) {
handleTimeAxisChange(timestamp);
}
} else {
handleXAxisChange(value);
}
}
},
[canCrossFilterByXAxis, handleXAxisChange],
[
canCrossFilterByXAxis,
handleTimeAxisChange,
handleXAxisChange,
xAxis.type,
],
);
const categoryAxis =
const renderedXAxis =
formData.orientation === OrientationType.Horizontal ? 'yAxis' : 'xAxis';
const queryEventHandlers = useMemo(
() => [
{
name: 'click',
query: `${categoryAxis}.category`,
query: renderedXAxis,
handler: handleXAxisLabelClick,
},
],
[categoryAxis, handleXAxisLabelClick],
[renderedXAxis, handleXAxisLabelClick],
);
const zrEventHandlers: EventHandlers = {
@@ -95,6 +95,7 @@ import {
getAnnotationData,
} from '../utils/annotation';
import {
collapseForecastKeys,
extractForecastSeriesContext,
extractForecastSeriesContexts,
extractForecastValuesFromTooltipParams,
@@ -1247,7 +1248,7 @@ export default function transformProps(
name: xAxisTitle,
nameGap: convertInteger(xAxisTitleMargin),
nameLocation: 'middle',
...(xAxisType === AxisType.Category &&
...((xAxisType === AxisType.Category || xAxisType === AxisType.Time) &&
groupBy.length === 0 && {
triggerEvent: true,
}),
@@ -1255,10 +1256,12 @@ export default function transformProps(
// When rotation is applied on time axes, hideOverlap can
// aggressively hide the last label. Rotated labels already
// have less overlap, so disabling hideOverlap is safe.
// At 0° rotation, keep hideOverlap to prevent long labels
// from overlapping each other, with showMaxLabel to ensure
// the last data point label stays visible (#37181).
hideOverlap: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
// At 0° rotation, also disable hideOverlap when showMaxLabel
// is active so the forced boundary label is never suppressed
// by ECharts' overlap detection (#39899).
hideOverlap: showMaxLabel
? false
: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
formatter: deduplicatedFormatter,
rotate: xAxisLabelRotation,
interval: xAxisLabelInterval,
@@ -1392,11 +1395,13 @@ export default function transformProps(
const forecastValue: CallbackDataParams[] = richTooltip
? params
: [params];
const sortedKeys = extractTooltipKeys(
forecastValue,
yIndex,
richTooltip,
tooltipSortByMetric,
const sortedKeys = collapseForecastKeys(
extractTooltipKeys(
forecastValue,
yIndex,
richTooltip,
tooltipSortByMetric,
),
);
const filteredForecastValue = forecastValue.filter(
(item: CallbackDataParams) =>
@@ -1595,6 +1600,7 @@ export default function transformProps(
label: xAxisLabel,
type: xAxisType,
},
resolvedTimeGrain,
refs,
coltypeMapping: dataTypes,
onLegendScroll,
@@ -467,6 +467,14 @@ export function transformSeries(
return formatter(numericValue);
}
if (!onlyTotal) {
// A stacked segment with no height begins and ends at the same
// coordinate as the top of the segment beneath it, so its label is
// drawn over that segment's label. Zero and null have no height, so
// they carry no label. The rich tooltip omits zero observations from
// a stacked series for the same reason.
if (stack && !numericValue) {
return '';
}
if (
numericValue >=
(thresholdValues[dataIndex] || Number.MIN_SAFE_INTEGER)
@@ -122,5 +122,6 @@ export type TimeseriesChartTransformedProps =
label: string;
type: AxisType;
};
resolvedTimeGrain?: TimeGranularity;
onFocusedSeries: (series: string | null) => void;
};
@@ -28,6 +28,7 @@
*/
import { z } from 'zod';
import { sanitizeHtml } from '@superset-ui/core';
// =============================================================================
// Common Schemas
@@ -57,6 +58,33 @@ const fontStyleSchema = z.enum(['normal', 'italic', 'oblique']);
/** Symbol type */
const symbolTypeSchema = z.string();
/**
* With the ECharts default renderMode 'html', a string tooltip formatter is
* assigned to the tooltip DOM element via innerHTML. ECharts formatter
* strings commonly rely on inline markup (e.g. '{b}<br/>{c}') for layout, so
* rejecting every '<' would break that supported usage; instead the value is
* run through the same allowlist sanitizer used for other tooltip HTML,
* which keeps presentational tags and strips anything else.
*/
const sanitizedFormatterSchema = z
.string()
.transform(value => sanitizeHtml(value));
/**
* ECharts navigates to title.link/sublink on click, so restrict them to
* http(s) and same-origin relative paths.
*/
const safeLinkSchema = z
.string()
.refine(
value =>
/^https?:\/\//i.test(value) ||
(value.startsWith('/') && !value.startsWith('//')),
{
message: 'Only http(s) or same-origin relative URLs are allowed',
},
);
// =============================================================================
// Text Style Schema
// =============================================================================
@@ -168,11 +196,11 @@ export const titleSchema = z.object({
id: z.string().optional(),
show: z.boolean().optional(),
text: z.string().optional(),
link: z.string().optional(),
link: safeLinkSchema.optional(),
target: z.enum(['self', 'blank']).optional(),
textStyle: textStyleSchema.optional(),
subtext: z.string().optional(),
sublink: z.string().optional(),
sublink: safeLinkSchema.optional(),
subtarget: z.enum(['self', 'blank']).optional(),
subtextStyle: textStyleSchema.optional(),
textAlign: z.enum(['left', 'center', 'right']).optional(),
@@ -386,7 +414,9 @@ export const tooltipSchema = z.object({
z.array(z.union([z.number(), z.string()])),
])
.optional(),
formatter: z.string().optional(), // Only string formatters
// Only string formatters: a string tooltip formatter is rendered via
// innerHTML (default renderMode 'html'), so it is sanitized above.
formatter: sanitizedFormatterSchema.optional(),
padding: z.union([z.number(), z.array(z.number())]).optional(),
backgroundColor: colorSchema.optional(),
borderColor: colorSchema.optional(),
@@ -397,7 +427,9 @@ export const tooltipSchema = z.object({
shadowOffsetX: z.number().optional(),
shadowOffsetY: z.number().optional(),
textStyle: textStyleSchema.optional(),
extraCssText: z.string().optional(),
// `extraCssText` is intentionally not accepted; unknown keys are
// stripped by the schema, so configs that still carry it keep working
// minus the raw CSS.
order: z
.enum(['seriesAsc', 'seriesDesc', 'valueAsc', 'valueDesc'])
.optional(),
@@ -575,6 +607,9 @@ export const seriesSchema = z.object({
polarIndex: z.number().optional(),
geoIndex: z.number().optional(),
calendarIndex: z.number().optional(),
// Per-series `tooltip` is intentionally not admitted; the schema
// strips unknown keys. If per-series tooltips are ever admitted, reuse
// tooltipSchema so the formatter sanitization applies.
label: labelSchema.optional(),
labelLine: z
.object({
@@ -60,6 +60,21 @@ export const extractForecastSeriesContexts = (
{} as { [key: string]: ForecastSeriesEnum[] },
);
/**
* Collapses raw ECharts series ids onto the names used to key tooltip rows.
*
* Tooltip values are grouped by forecast-stripped name, so any ordering derived
* from the raw series ids has to be expressed in the same terms before it can be
* matched against them. This matters beyond real Prophet output: a metric simply
* labelled `ci__yhat_lower` collapses to `ci` exactly like a forecast bound
* does, and a chart whose every series carries such a suffix has no id that
* survives the comparison untouched.
*/
export const collapseForecastKeys = (seriesIds: string[]): string[] =>
Array.from(
new Set(seriesIds.map(id => extractForecastSeriesContext(id).name)),
);
export const extractForecastValuesFromTooltipParams = (
params: any[],
isHorizontal = false,
@@ -523,3 +523,73 @@ test('EChartOptionsParseError contains validation error details', () => {
);
}
});
// =============================================================================
// Creator-authored options must not reach the tooltip's innerHTML/
// navigation sinks unsanitized.
// =============================================================================
test('sanitizes tooltip string formatters instead of rejecting all markup', () => {
const input = `{ tooltip: { formatter: '<img src=x onerror=alert(1)>' } }`;
const result = parseEChartOptions(input);
expect(result.success).toBe(true);
expect(result.data?.tooltip).toEqual({ formatter: '<img src>' });
});
test('keeps presentational tags in tooltip string formatters', () => {
const input = `{ tooltip: { formatter: '{b}<br/>{c}' } }`;
const result = parseEChartOptions(input);
expect(result.success).toBe(true);
expect(result.data?.tooltip).toEqual({ formatter: '{b}<br />{c}' });
});
test('strips per-series tooltip config so its formatter never reaches the merge', () => {
const result = parseEChartOptions(
`{ series: [{ type: 'line', tooltip: { formatter: '<b onpointerover=alert(1)>x</b>' } }] }`,
);
expect(result.success).toBe(true);
expect(result.data).toEqual({ series: [{ type: 'line' }] });
});
test('accepts markup-free tooltip placeholder formatters', () => {
const input = `{ tooltip: { formatter: '{b}: {c}' } }`;
const result = parseEChartOptions(input);
expect(result.success).toBe(true);
expect(result.data).toEqual({ tooltip: { formatter: '{b}: {c}' } });
});
test('rejects javascript: URLs in title link and sublink', () => {
expect(() =>
parseEChartOptions(`{ title: { link: 'javascript:alert(1)' } }`),
).toThrow(EChartOptionsParseError);
expect(() =>
parseEChartOptions(`{ title: { sublink: 'javascript:alert(1)' } }`),
).toThrow(EChartOptionsParseError);
expect(() =>
parseEChartOptions(`{ title: { link: '//evil.example/x' } }`),
).toThrow(EChartOptionsParseError);
});
test('accepts http(s) and same-origin relative title links', () => {
const result = parseEChartOptions(
`{ title: { link: 'https://superset.apache.org', sublink: '/dashboard/1/' } }`,
);
expect(result.success).toBe(true);
expect(result.data).toEqual({
title: { link: 'https://superset.apache.org', sublink: '/dashboard/1/' },
});
});
test('strips tooltip extraCssText instead of passing raw CSS through', () => {
const result = parseEChartOptions(
`{ tooltip: { show: true, extraCssText: 'background:url(//evil.example/x)' } }`,
);
expect(result.success).toBe(true);
expect(result.data).toEqual({ tooltip: { show: true } });
});
@@ -1165,6 +1165,110 @@ test('x-axis dedup keeps the forced min label when the endpoints format identica
expect(formatter(min)).toBe('May');
});
test('#39899 - x-axis dates do not overlap and last label stays visible at 0° rotation (mixed)', () => {
// When showMaxLabel is active on a time axis with 0° rotation,
// hideOverlap must be off so ECharts cannot suppress the forced
// max label (the end-of-axis date).
const chartProps = createEchartsTimeseriesTestChartProps<
EchartsMixedTimeseriesFormData,
EchartsMixedTimeseriesProps
>({
...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
defaultQueriesData: [
createTestQueryData(
[
{
__timestamp: Date.UTC(2026, 0, 1),
sum__num: 100,
},
{
__timestamp: Date.UTC(2026, 6, 1),
sum__num: 200,
},
],
{
colnames: ['__timestamp', 'sum__num'],
coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
},
),
createTestQueryData(
[
{
__timestamp: Date.UTC(2026, 0, 1),
sum__num: 100,
},
{
__timestamp: Date.UTC(2026, 6, 1),
sum__num: 200,
},
],
{
colnames: ['__timestamp', 'sum__num'],
coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
},
),
],
formData: {
...formData,
x_axis: '__timestamp',
metrics: ['sum__num'],
metricsB: ['sum__num'],
groupby: [],
groupbyB: [],
xAxisLabelRotation: 0,
// showMaxLabel (and therefore hideOverlap: false) only activates when
// a time grain resolves, so this needs one set to actually exercise
// the #39899 fix rather than silently no-op.
timeGrainSqla: TimeGranularity.MONTH,
},
queriesData: [
createTestQueryData(
[
{
__timestamp: Date.UTC(2026, 0, 1),
sum__num: 100,
},
{
__timestamp: Date.UTC(2026, 6, 1),
sum__num: 200,
},
],
{
colnames: ['__timestamp', 'sum__num'],
coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
},
),
createTestQueryData(
[
{
__timestamp: Date.UTC(2026, 0, 1),
sum__num: 100,
},
{
__timestamp: Date.UTC(2026, 6, 1),
sum__num: 200,
},
],
{
colnames: ['__timestamp', 'sum__num'],
coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
},
),
],
});
const { echartOptions } = transformProps(chartProps);
const { axisLabel } = echartOptions.xAxis as Record<string, any>;
expect(axisLabel.showMaxLabel).toBe(true);
expect(axisLabel.alignMaxLabel).toBe('right');
expect(axisLabel.hideOverlap).toBe(false);
});
test('regression #37921: multi-metric Query A with groupby does not duplicate first metric in series names', () => {
// Regression test for https://github.com/apache/superset/issues/37921
// ("Residual" follow-up to #37055).
@@ -70,4 +70,42 @@ describe('renderNormalizedTooltip', () => {
expect(tooltip).toContain('N/A');
expect(tooltip).not.toContain('NaN');
});
test('should HTML-escape series names from query data', () => {
// Regression test: the tooltip is rendered via innerHTML, so markup
// in query-result values must not become live DOM.
const tooltip = renderNormalizedTooltip(
{ ...params, name: '<img src=x onerror=alert(1)>' },
metrics,
mockGetDenormalizedValue,
metricsWithCustomBounds,
);
expect(tooltip).not.toContain('<img');
expect(tooltip).toContain('&lt;img');
});
test('should HTML-escape metric labels', () => {
const tooltip = renderNormalizedTooltip(
params,
['<svg onload=alert(1)>', 'metric2'],
mockGetDenormalizedValue,
metricsWithCustomBounds,
);
expect(tooltip).not.toContain('<svg');
expect(tooltip).toContain('&lt;svg');
});
test('should HTML-escape the series color used for the tooltip color dot', () => {
// Regression test: `color` is interpolated into a style attribute
// unquoted, so an unescaped quote could break out of the attribute
// and inject markup.
const tooltip = renderNormalizedTooltip(
{ ...params, color: 'red" onmouseover="alert(1)' },
metrics,
mockGetDenormalizedValue,
metricsWithCustomBounds,
);
expect(tooltip).not.toContain('" onmouseover="alert(1)"');
expect(tooltip).toContain('&quot; onmouseover=&quot;alert(1)');
});
});
@@ -16,11 +16,17 @@
* specific language governing permissions and limitations
* under the License.
*/
import { render } from '@testing-library/react';
import { AxisType } from '@superset-ui/core';
import { render, waitFor } from '@testing-library/react';
import { AxisType, DTTM_ALIAS, TimeGranularity } from '@superset-ui/core';
import { supersetTheme, ThemeProvider } from '@apache-superset/core/theme';
import { logging } from '@apache-superset/core/utils';
import type { ECElementEvent } from 'echarts/types/src/util/types';
import EchartsTimeseries from '../../src/Timeseries/EchartsTimeseries';
import { TimeseriesChartTransformedProps } from '../../src/Timeseries/types';
import {
OrientationType,
TimeseriesChartTransformedProps,
} from '../../src/Timeseries/types';
import type { EchartsProps } from '../../src/types';
// Percent-change draggable baseline: this is the one piece of the ECharts
// rebuilds with zero prior test coverage despite six separate production
@@ -40,12 +46,14 @@ let mockChart: {
convertFromPixel: jest.Mock;
getModel: jest.Mock;
};
const mockEchart = jest.fn();
jest.mock('../../src/components/Echart', () => {
const { forwardRef, useImperativeHandle } = jest.requireActual('react');
return {
__esModule: true,
default: forwardRef((_props: unknown, ref: unknown) => {
default: forwardRef((props: unknown, ref: unknown) => {
mockEchart(props);
useImperativeHandle(ref, () => ({
getEchartInstance: () => mockChart,
}));
@@ -115,6 +123,17 @@ function renderTimeseries(
);
}
function getLatestEchartProps() {
const lastCall = mockEchart.mock.calls.at(-1);
expect(lastCall).toBeDefined();
const [props] = lastCall as [EchartsProps];
return props;
}
function advanceClickTimer() {
jest.advanceTimersByTime(300);
}
// Pulls the graphic descriptor for the draggable baseline handle out of the
// most recent setOption call, mirroring how ECharts itself would read it.
function getBaselineGraphic() {
@@ -126,6 +145,7 @@ function getBaselineGraphic() {
beforeEach(() => {
jest.clearAllMocks();
mockEchart.mockReset();
setupChartMock();
jest.spyOn(window, 'requestAnimationFrame').mockImplementation(cb => {
cb(0);
@@ -134,6 +154,7 @@ beforeEach(() => {
});
afterEach(() => {
jest.useRealTimers();
jest.restoreAllMocks();
});
@@ -245,3 +266,400 @@ test('does not touch the chart instance when rebase is disabled', () => {
expect(mockChart.setOption).not.toHaveBeenCalled();
});
test('emits TEMPORAL_RANGE cross-filter from time axis label click on day bucket', () => {
const setDataMask = jest.fn();
renderTimeseries({
emitCrossFilters: true,
setDataMask,
groupby: [],
resolvedTimeGrain: TimeGranularity.DAY,
formData: {
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.DAY,
vizType: 'echarts_timeseries_line',
} as any,
xAxis: {
label: DTTM_ALIAS,
type: AxisType.Time,
},
});
const labelClickHandler = getLatestEchartProps().queryEventHandlers?.find(
({ query }) => query === 'xAxis',
)?.handler;
expect(labelClickHandler).toBeDefined();
labelClickHandler?.({
targetType: 'axisLabel',
value: '2021-01-01',
} as unknown as ECElementEvent);
expect(setDataMask.mock.calls[0][0]).toEqual({
extraFormData: {
filters: [
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-01-02T00:00:00',
},
],
},
filterState: {
label: ['2021-01-01T00:00:00 : 2021-01-02T00:00:00'],
value: ['2021-01-01T00:00:00 : 2021-01-02T00:00:00'],
selectedValues: ['2021-01-01T00:00:00 : 2021-01-02T00:00:00'],
},
});
});
test('emits upper-exclusive TEMPORAL_RANGE from time point click on month bucket', () => {
jest.useFakeTimers();
const setDataMask = jest.fn();
renderTimeseries({
emitCrossFilters: true,
setDataMask,
groupby: [],
resolvedTimeGrain: TimeGranularity.MONTH,
formData: {
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
vizType: 'echarts_timeseries_line',
} as any,
xAxis: {
label: DTTM_ALIAS,
type: AxisType.Time,
},
});
getLatestEchartProps().eventHandlers?.click?.({
componentType: 'series',
seriesName: 'Sales',
data: [Date.UTC(2021, 0, 1), 100],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMask.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-02-01T00:00:00',
},
]);
});
test('uses resolved time grain for temporal point-click cross-filter', () => {
jest.useFakeTimers();
const setDataMask = jest.fn();
renderTimeseries({
emitCrossFilters: true,
setDataMask,
groupby: [],
resolvedTimeGrain: TimeGranularity.MONTH,
formData: {
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.DAY,
extraFormData: {
time_grain_sqla: TimeGranularity.MONTH,
},
vizType: 'echarts_timeseries_line',
} as any,
xAxis: {
label: DTTM_ALIAS,
type: AxisType.Time,
},
});
getLatestEchartProps().eventHandlers?.click?.({
componentType: 'series',
seriesName: 'Sales',
data: [Date.UTC(2021, 0, 1), 100],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMask.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-02-01T00:00:00',
},
]);
});
test('emits TEMPORAL_RANGE from string-typed time point click value', () => {
jest.useFakeTimers();
const setDataMask = jest.fn();
renderTimeseries({
emitCrossFilters: true,
setDataMask,
groupby: [],
resolvedTimeGrain: TimeGranularity.MONTH,
formData: {
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
vizType: 'echarts_timeseries_line',
} as any,
xAxis: {
label: DTTM_ALIAS,
type: AxisType.Time,
},
});
getLatestEchartProps().eventHandlers?.click?.({
componentType: 'series',
seriesName: 'Sales',
data: ['2021-01-01T00:00:00Z', 100],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMask.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-02-01T00:00:00',
},
]);
});
test('emits TEMPORAL_RANGE from horizontal time point click using timestamp, not metric', () => {
jest.useFakeTimers();
const setDataMask = jest.fn();
renderTimeseries({
emitCrossFilters: true,
setDataMask,
groupby: [],
resolvedTimeGrain: TimeGranularity.MONTH,
formData: {
orientation: OrientationType.Horizontal,
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
vizType: 'echarts_timeseries_line',
} as any,
xAxis: {
label: DTTM_ALIAS,
type: AxisType.Time,
},
});
getLatestEchartProps().eventHandlers?.click?.({
componentType: 'series',
seriesName: 'Sales',
data: [129, Date.UTC(2021, 0, 1)],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMask.mock.calls[0][0].extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-02-01T00:00:00',
},
]);
});
test('clears temporal X-axis cross-filter when clicking selected bucket again', () => {
jest.useFakeTimers();
const setDataMask = jest.fn();
const selectedRange = '2021-01-01T00:00:00 : 2021-02-01T00:00:00';
renderTimeseries({
emitCrossFilters: true,
setDataMask,
groupby: [],
selectedValues: { 0: selectedRange },
resolvedTimeGrain: TimeGranularity.MONTH,
formData: {
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
vizType: 'echarts_timeseries_line',
} as any,
xAxis: {
label: DTTM_ALIAS,
type: AxisType.Time,
},
});
getLatestEchartProps().eventHandlers?.click?.({
componentType: 'series',
seriesName: 'Sales',
data: [Date.UTC(2021, 0, 1), 100],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMask.mock.calls[0][0]).toEqual({
extraFormData: {
filters: [],
},
filterState: {
label: undefined,
value: null,
selectedValues: null,
},
});
});
test('emits empty temporal X-axis data mask when filter grain is missing', () => {
jest.useFakeTimers();
const setDataMask = jest.fn();
renderTimeseries({
emitCrossFilters: true,
setDataMask,
groupby: [],
formData: {
granularitySqla: 'ds',
timeGrainSqla: undefined,
vizType: 'echarts_timeseries_line',
} as any,
xAxis: {
label: DTTM_ALIAS,
type: AxisType.Time,
},
});
getLatestEchartProps().eventHandlers?.click?.({
componentType: 'series',
seriesName: 'Sales',
data: [Date.UTC(2021, 0, 1), 100],
name: '2021-01-01',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMask.mock.calls[0][0]).toEqual({
extraFormData: {
filters: [],
},
filterState: {
label: undefined,
value: null,
selectedValues: null,
},
});
});
test('warns and skips temporal cross-filter when string value cannot be parsed', () => {
jest.useFakeTimers();
const setDataMask = jest.fn();
const warn = jest.spyOn(logging, 'warn').mockImplementation();
renderTimeseries({
emitCrossFilters: true,
setDataMask,
groupby: [],
resolvedTimeGrain: TimeGranularity.MONTH,
formData: {
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
vizType: 'echarts_timeseries_line',
} as any,
xAxis: {
label: DTTM_ALIAS,
type: AxisType.Time,
},
});
getLatestEchartProps().eventHandlers?.click?.({
componentType: 'series',
seriesName: 'Sales',
data: ['not-a-date', 100],
name: 'not-a-date',
dataIndex: 0,
});
advanceClickTimer();
expect(setDataMask).not.toHaveBeenCalled();
expect(warn).toHaveBeenCalledWith(
'Unable to parse time axis value for cross-filtering',
'not-a-date',
);
});
test('does not emit temporal X-axis label cross-filter when dimensions are set', () => {
const setDataMask = jest.fn();
renderTimeseries({
emitCrossFilters: true,
setDataMask,
groupby: ['country'],
formData: {
groupby: ['country'],
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.MONTH,
vizType: 'echarts_timeseries_line',
} as any,
xAxis: {
label: DTTM_ALIAS,
type: AxisType.Time,
},
});
const labelClickHandler = getLatestEchartProps().queryEventHandlers?.find(
({ query }) => query === 'xAxis',
)?.handler;
expect(labelClickHandler).toBeDefined();
labelClickHandler?.({
targetType: 'axisLabel',
value: '2021-01-01',
} as unknown as ECElementEvent);
expect(setDataMask).not.toHaveBeenCalled();
});
test('context menu cross-filter is available for a temporal bar point', async () => {
const onContextMenu = jest.fn();
renderTimeseries({
emitCrossFilters: true,
onContextMenu,
groupby: [],
resolvedTimeGrain: TimeGranularity.MONTH,
formData: {
granularitySqla: 'ds',
timeGrainSqla: TimeGranularity.DAY,
extraFormData: {
time_grain_sqla: TimeGranularity.MONTH,
},
vizType: 'echarts_timeseries_line',
} as any,
xAxis: {
label: DTTM_ALIAS,
type: AxisType.Time,
},
});
await getLatestEchartProps().eventHandlers?.contextmenu?.({
componentType: 'series',
seriesName: 'Sales',
data: [Date.UTC(2021, 0, 1), 100],
name: '2021-01-01',
event: { stop: jest.fn(), event: { clientX: 10, clientY: 20 } },
});
await waitFor(() => {
expect(onContextMenu).toHaveBeenCalled();
});
const { crossFilter } = onContextMenu.mock.calls[0][2];
expect(crossFilter.dataMask.extraFormData.filters).toEqual([
{
col: 'ds',
op: 'TEMPORAL_RANGE',
val: '2021-01-01T00:00:00 : 2021-02-01T00:00:00',
},
]);
});
@@ -2083,6 +2083,39 @@ test('xAxisForceCategorical forces Category axis regardless of Numeric coltype',
expect(xAxis.triggerEvent).toBe(true);
});
test('temporal x-axis enables trigger events when no dimensions are set', () => {
const ts1 = 1745784000000;
const ts2 = 1745870400000;
const chartProps = createTestChartProps({
formData: {
metrics: ['metric'],
granularity_sqla: 'ds',
x_axis: '__timestamp',
},
queriesData: [
createTestQueryData(
[
{ __timestamp: ts1, metric: 10 },
{ __timestamp: ts2, metric: 20 },
],
{
colnames: ['__timestamp', 'metric'],
coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
},
),
],
});
const { echartOptions } = transformProps(chartProps);
const xAxis = echartOptions.xAxis as {
triggerEvent?: boolean;
type: string;
};
expect(xAxis.type).toBe(AxisType.Time);
expect(xAxis.triggerEvent).toBe(true);
});
test('temporal x coltype forced categorical yields a Category axis with date labels', () => {
// Issue #28204: with a temporal x-axis (e.g. weekly grain) the default Time
// scale places ticks at "nice" intervals that don't line up with the buckets.
@@ -2330,6 +2363,7 @@ test('tooltip time grain wiring: dashboard-level extraFormData time grain overri
});
const transformedProps = transformProps(chartProps);
expect(transformedProps.resolvedTimeGrain).toBe(TimeGranularity.MONTH);
const tooltipFormatter = (
transformedProps.echartOptions as unknown as TooltipFormatterOptions
).tooltip.formatter;
@@ -2363,6 +2397,7 @@ test('tooltip time grain wiring: chart-level time grain drives the tooltip when
});
const transformedProps = transformProps(chartProps);
expect(transformedProps.resolvedTimeGrain).toBe(TimeGranularity.YEAR);
const tooltipFormatter = (
transformedProps.echartOptions as unknown as TooltipFormatterOptions
).tooltip.formatter;
@@ -2529,3 +2564,64 @@ describe('EchartsTimeseries tooltip truncation', () => {
expect(buildTooltip(undefined, longCategory)).toContain(longCategory);
});
});
describe('tooltip for metrics whose labels end in forecast suffixes', () => {
const marker = '<span style="background-color:#1f77b4;"></span>';
const seriesIds = ['ci__yhat', 'ci__yhat_lower', 'ci__yhat_upper'];
const values = [1.5, 0.5, 2.0];
// Metrics can be labelled `ci__yhat*` with no forecast enabled and no plain
// observation series. Every series then collapses onto the same
// forecast-stripped tooltip key, so no raw series id matches itself.
const buildTooltip = (tooltipSortByMetric = false) => {
const chartProps = createTestChartProps({
formData: {
x_axis: 'dt',
metrics: seriesIds,
groupby: [],
richTooltip: true,
tooltipSortByMetric,
} as Partial<EchartsTimeseriesFormData>,
queriesData: [
createTestQueryData([
{
dt: 599616000000,
ci__yhat: 1.5,
ci__yhat_lower: 0.5,
ci__yhat_upper: 2.5,
},
]),
],
});
const tooltipFormatter = (transformProps(chartProps).echartOptions as any)
.tooltip.formatter;
return tooltipFormatter(
seriesIds.map((id, i) => ({
seriesId: id,
seriesName: id,
value: [599616000000, values[i]],
data: [599616000000, values[i]],
marker,
})),
);
};
test('renders the collapsed series rather than falling back to "No data"', () => {
const html = buildTooltip();
expect(html).not.toContain('No data');
expect(html).toContain('>ci<');
expect(html).toContain('ŷ = 1.5 (0.5, 2.5)');
});
test('renders a single row rather than one per forecast suffix', () => {
const html = buildTooltip();
expect(html.match(/<tr/g)).toHaveLength(1);
expect(html).toContain('>ci<');
});
test('still renders the row when the tooltip is sorted by metric', () => {
const html = buildTooltip(true);
expect(html).not.toContain('No data');
expect(html).toContain('>ci<');
});
});
@@ -20,13 +20,14 @@ import {
CategoricalColorScale,
ChartProps,
TimeGranularity,
getNumberFormatter,
} from '@superset-ui/core';
import { GenericDataType } from '@apache-superset/core/common';
import { supersetTheme } from '@apache-superset/core/theme';
import type { SeriesOption } from 'echarts';
import type { ScatterSeriesOption } from 'echarts/charts';
import { EchartsTimeseriesSeriesType } from '../../src';
import { TIMESERIES_CONSTANTS } from '../../src/constants';
import { StackControlsValue, TIMESERIES_CONSTANTS } from '../../src/constants';
import {
LegendOrientation,
EchartsTimeseriesChartProps,
@@ -340,15 +341,15 @@ test('should configure time axis labels to show max label for last month visibil
);
});
test('x-axis dates do not overlap and last label stays visible at 0° rotation', () => {
test('#39899 - x-axis dates do not overlap and last label stays visible at 0° rotation', () => {
const result = transformProps(buildTimeseriesChartProps());
const { axisLabel } = result.echartOptions.xAxis as Record<string, any>;
expect(axisLabel.hideOverlap).toBe(true);
// showMaxLabel forces the last data point label to render even
// when hideOverlap is active, preventing the #37181 regression.
// showMaxLabel forces the last data point label to render
expect(axisLabel.showMaxLabel).toBe(true);
expect(axisLabel.alignMaxLabel).toBe('right');
// hideOverlap must be OFF so ECharts cannot suppress the forced max label
expect(axisLabel.hideOverlap).toBe(false);
});
test('last x-axis date is visible and not cut off when rotated -45°', () => {
@@ -566,3 +567,70 @@ test('getPadding should handle Left position with zero margin correctly', () =>
getChartPaddingSpy.mockRestore();
}
});
/**
* #42702: a stacked segment with no height starts and ends at the same
* coordinate as the top of the segment beneath it, so a value label on it is
* drawn over that segment's label. `percentage_threshold` does not filter these
* out: it defaults to 0, and `thresholdValues[dataIndex] || MIN_SAFE_INTEGER`
* turns a 0 threshold into "no filtering", which is intentional.
*/
const stackedLabel = (
numericValue: number | null,
opts: Record<string, unknown> = {},
) => {
const series = transformSeries(
{ id: 'B', name: 'B', data: [[1, numericValue]] } as SeriesOption,
mockColorScale,
'B',
{
seriesType: EchartsTimeseriesSeriesType.Bar,
stack: StackControlsValue.Stack,
showValue: true,
onlyTotal: false,
formatter: getNumberFormatter(),
thresholdValues: [0],
...opts,
},
) as SeriesOption & {
label: { formatter: (params: unknown) => string };
};
return series.label.formatter({
value: [1, numericValue],
dataIndex: 0,
seriesIndex: 1,
seriesName: 'B',
});
};
test('stacked value labels are omitted for a zero-height segment', () => {
expect(stackedLabel(0)).toBe('');
expect(stackedLabel(null)).toBe('');
});
test('stacked value labels are kept for segments that have height', () => {
expect(stackedLabel(32)).toBe('32');
expect(stackedLabel(-5)).toBe('-5');
});
test('a zero value keeps its label when the series is not stacked', () => {
// Without a stack the label sits on the bar itself, so there is nothing for
// it to collide with.
expect(stackedLabel(0, { stack: undefined })).toBe('0');
});
test('percentage_threshold still filters values below the threshold', () => {
// 10% of a 100 total. The zero-height guard must not swallow this rule.
expect(stackedLabel(5, { thresholdValues: [10] })).toBe('');
expect(stackedLabel(50, { thresholdValues: [10] })).toBe('50');
});
test('only-total labels are unaffected by the zero-height guard', () => {
expect(
stackedLabel(0, {
onlyTotal: true,
showValueIndexes: [1],
totalStackedValues: [32],
}),
).toBe('32');
});
@@ -23,6 +23,7 @@ import {
} from '@superset-ui/core';
import { SeriesOption } from 'echarts';
import {
collapseForecastKeys,
extractForecastSeriesContext,
extractForecastValuesFromTooltipParams,
formatForecastTooltipSeries,
@@ -464,3 +465,35 @@ describe('formatForecastTooltipSeries truncation', () => {
expect(cell).toBe(`${marker}cpu`);
});
});
describe('collapseForecastKeys', () => {
test('leaves plain observation series untouched and in order', () => {
expect(collapseForecastKeys(['foo', 'bar'])).toEqual(['foo', 'bar']);
});
test('folds a forecast bundle down to a single key', () => {
expect(
collapseForecastKeys([
'foo',
'foo__yhat',
'foo__yhat_lower',
'foo__yhat_upper',
]),
).toEqual(['foo']);
});
test('keeps a key for metrics whose labels are entirely forecast suffixes', () => {
// Charts can carry metrics literally labelled `ci__yhat*` with no plain
// observation series. Callers match these against forecast-stripped keys,
// so an uncollapsed id here would match nothing and drop every row.
expect(
collapseForecastKeys(['ci__yhat', 'ci__yhat_lower', 'ci__yhat_upper']),
).toEqual(['ci']);
});
test('preserves the incoming order of distinct series', () => {
expect(
collapseForecastKeys(['b__yhat_lower', 'a__yhat', 'b__yhat']),
).toEqual(['b', 'a']);
});
});
@@ -290,6 +290,25 @@ test('isAdditiveMetric: non-additive aggregates, SQL, and saved metrics are not
expect(isAdditiveMetric('count')).toBe(false);
});
test('isAdditiveMetric: MEDIAN/STDDEV_SAMP/VAR_SAMP are non-additive, with no dedicated code needed', () => {
// Regression guard: MEDIAN/STDDEV_SAMP/VAR_SAMP are new system-wide metric
// aggregates (not pivot-table-specific). They must fall outside
// ADDITIVE_AGGREGATES so totals/subtotals route through the correct
// DB-rollup path automatically, same as AVG/COUNT_DISTINCT already do --
// averaging per-group medians (or variances) is exactly the class of bug
// SIP-216 fixed for AVG, and would be equally wrong here.
(['MEDIAN', 'STDDEV_SAMP', 'VAR_SAMP'] as const).forEach(aggregate => {
expect(
isAdditiveMetric({
expressionType: 'SIMPLE',
aggregate,
column: { column_name: 'num' },
label: `${aggregate.toLowerCase()}_num`,
} as QueryFormMetric),
).toBe(false);
});
});
test('allMetricsAdditive: all additive vs any non-additive vs empty', () => {
const sum = {
expressionType: 'SIMPLE',
@@ -80,6 +80,19 @@ interface DatamapSource {
country?: string;
}
/**
* Escape HTML special characters to prevent XSS attacks. Popup templates are
* assigned to the hover element via innerHTML by the datamaps library, and
* formatter output can echo a creator-controlled format string verbatim
* (see createD3NumberFormatter's invalid-format fallback), so both the name
* and the formatted value must be treated as untrusted text.
*/
function escapeHtml(text: string): string {
const div = document.createElement('div');
div.textContent = text;
return div.innerHTML;
}
const propTypes = {
data: PropTypes.arrayOf(
PropTypes.shape({
@@ -279,9 +292,9 @@ function WorldMap(element: HTMLElement, props: WorldMapProps): void {
highlightBorderWidth: 1,
popupTemplate: (geo, d) =>
d &&
`<div class="hoverinfo"><strong>${d.name}</strong><br>${formatter(
d.m1,
)}</div>`,
`<div class="hoverinfo"><strong>${escapeHtml(
d.name,
)}</strong><br>${escapeHtml(String(formatter(d.m1)))}</div>`,
},
bubblesConfig: {
borderWidth: 1,
@@ -290,9 +303,9 @@ function WorldMap(element: HTMLElement, props: WorldMapProps): void {
popupOnHover: !inContextMenu,
radius: null,
popupTemplate: (geo, d) =>
`<div class="hoverinfo"><strong>${d.name}</strong><br>${formatter(
d.m2,
)}</div>`,
`<div class="hoverinfo"><strong>${escapeHtml(
d.name,
)}</strong><br>${escapeHtml(String(formatter(d.m2)))}</div>`,
fillOpacity: 0.5,
animate: true,
highlightOnHover: !inContextMenu,
@@ -180,6 +180,33 @@ test('disables Datamaps highlightOnHover while the context menu is open', () =>
expect(geographyConfig?.highlightOnHover).toBe(false);
});
test('escapes markup in hover popup templates', () => {
// Regression test for stored XSS via the number-formatter fallback: an
// invalid Y Axis Format string is echoed verbatim by the formatter
// (createD3NumberFormatter's catch branch), so the popup templates must
// HTML-escape formatter output before datamaps assigns it via innerHTML.
const maliciousFormatter = getNumberFormatter('<img src=x onerror=alert(1)>');
WorldMap(container, { ...baseProps, formatter: maliciousFormatter });
const geographyConfig = lastDatamapConfig?.geographyConfig as {
popupTemplate: (geo: unknown, d: unknown) => string;
};
const bubblesConfig = lastDatamapConfig?.bubblesConfig as {
popupTemplate: (geo: unknown, d: unknown) => string;
};
const entry = { name: '<b>United States</b>', m1: 100, m2: 200 };
const geoPopup = geographyConfig.popupTemplate({}, entry);
const bubblePopup = bubblesConfig.popupTemplate({}, entry);
[geoPopup, bubblePopup].forEach(popup => {
expect(popup).not.toContain('<img');
expect(popup).not.toContain('<b>');
expect(popup).toContain('&lt;img src=x onerror=alert(1)&gt;');
expect(popup).toContain('&lt;b&gt;United States&lt;/b&gt;');
});
});
test('does not throw error when onContextMenu is undefined', () => {
const propsWithoutContextMenu = {
...baseProps,
@@ -1524,7 +1524,9 @@ export function popPermalink(key: string): SqlLabThunkAction<Promise<unknown>> {
dbId: json.dbId ? parseInt(json.dbId, 10) : undefined,
catalog: json.catalog ?? null,
schema: json.schema ?? undefined,
autorun: json.autorun ? json.autorun : false,
// The recipient must review the prefilled query and press
// Run; a permalink payload never auto-runs.
autorun: false,
sql: json.sql ? json.sql : 'SELECT ...',
templateParams: json.templateParams,
}),
@@ -1548,7 +1550,9 @@ export function popStoredQuery(
dbId: json.dbId ? parseInt(json.dbId, 10) : undefined,
catalog: json.catalog ?? null,
schema: json.schema ?? undefined,
autorun: json.autorun ? json.autorun : false,
// Same rule as popPermalink above — stored payloads never
// auto-run.
autorun: false,
sql: json.sql ? json.sql : 'SELECT ...',
templateParams: json.templateParams,
}),
@@ -1627,7 +1631,9 @@ export function popDatasourceQuery(
name: `${QUERY_TEXT} ${json.result.name}`,
dbId: json.result.database.id,
schema: json.result.schema,
autorun: sql !== undefined,
// `sql` here can come straight from the URL, so its mere
// presence must never imply auto-execution.
autorun: false,
sql: sql || json.result.select_star,
}),
),
@@ -390,7 +390,7 @@ const ResultSet = ({
// provides.
redirect(getExportCsvUrl(query.id));
},
confirmText: t('OK'),
confirmText: t('Confirm'),
cancelText: t('Close'),
});
}
@@ -719,7 +719,7 @@ const ResultSet = ({
if (data && data.length > 0) {
const allowHTML = getItem(
LocalStorageKeys.SqllabIsRenderHtmlEnabled,
true,
false,
);
const tableProps = {
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { render, screen } from 'spec/helpers/testing-library';
import { render, screen, userEvent } from 'spec/helpers/testing-library';
import SaveDatasetActionButton from 'src/SqlLab/components/SaveDatasetActionButton';
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
@@ -27,6 +27,7 @@ describe('SaveDatasetActionButton', () => {
<SaveDatasetActionButton
setShowSave={() => true}
onSaveAsExplore={onSaveAsExplore}
canSaveDataset
/>,
);
@@ -41,4 +42,27 @@ describe('SaveDatasetActionButton', () => {
expect(saveBtn).toBeVisible();
expect(saveDatasetBtn).toBeVisible();
});
test('disables the save dataset button when the query did not run successfully', async () => {
render(
<SaveDatasetActionButton
setShowSave={() => true}
onSaveAsExplore={jest.fn()}
canSaveDataset={false}
/>,
);
const saveDatasetBtn = screen.getByRole('button', {
name: /save dataset/i,
});
expect(saveDatasetBtn).toBeDisabled();
// the disabled button is wrapped in a span so the tooltip still triggers
userEvent.hover(saveDatasetBtn.parentElement as HTMLElement);
expect(
await screen.findByRole('tooltip', {
name: 'You must run the query successfully first',
}),
).toBeInTheDocument();
});
});
@@ -23,11 +23,13 @@ import { Button } from '@superset-ui/core/components';
interface SaveDatasetActionButtonProps {
setShowSave: (arg0: boolean) => void;
onSaveAsExplore?: () => void;
canSaveDataset: boolean;
}
const SaveDatasetActionButton = ({
setShowSave,
onSaveAsExplore,
canSaveDataset,
}: SaveDatasetActionButtonProps) => (
<>
<Button
@@ -43,8 +45,13 @@ const SaveDatasetActionButton = ({
color="default"
variant="text"
onClick={() => onSaveAsExplore?.()}
disabled={!canSaveDataset}
icon={<Icons.TableOutlined />}
tooltip={t('Save or Overwrite Dataset')}
tooltip={
canSaveDataset
? t('Save or Overwrite Dataset')
: t('You must run the query successfully first')
}
aria-label={t('Save dataset')}
/>
)}
@@ -35,6 +35,7 @@ const mockedProps = {
onSave: () => {},
saveQueryWarning: null,
columns: [],
canSaveDataset: true,
};
const mockState = {
@@ -52,6 +52,7 @@ interface SaveQueryProps {
onUpdate: (arg0: QueryPayload, id: string) => void;
saveQueryWarning: string | null;
database: Partial<DatabaseObject> | undefined;
canSaveDataset: boolean;
}
export type QueryPayload = {
@@ -81,6 +82,7 @@ const SaveQuery = ({
saveQueryWarning,
database,
columns,
canSaveDataset,
}: SaveQueryProps) => {
const queryEditor = useQueryEditor(queryEditorId, [
'autorun',
@@ -207,6 +209,7 @@ const SaveQuery = ({
<SaveDatasetActionButton
setShowSave={setShowSave}
onSaveAsExplore={canExploreDatabase ? onSaveAsExplore : undefined}
canSaveDataset={canSaveDataset}
/>
)}
<SaveDatasetModal
@@ -21,6 +21,7 @@ import {
isFeatureEnabled,
getExtensionsRegistry,
FeatureFlag,
QueryState,
} from '@superset-ui/core';
import {
act,
@@ -334,6 +335,47 @@ describe('SqlEditor', () => {
expect(await findByText('10 000')).toBeInTheDocument();
});
const setupWithLatestQuery = (overrides: Partial<typeof latestQuery>) =>
setup(
mockedProps,
createStore({
...mockInitialState,
sqlLab: {
...mockInitialState.sqlLab,
queries: {
[latestQuery.id]: { ...latestQuery, ...overrides },
},
databases: {
1991: {
...mockInitialState.sqlLab.databases[1991],
allows_virtual_table_explore: true,
},
},
},
}),
);
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();
});
test('disables the save dataset button when the latest query failed', async () => {
const { findByRole } = setupWithLatestQuery({
state: QueryState.Failed,
results: undefined,
});
expect(await findByRole('button', { name: 'Save dataset' })).toBeDisabled();
});
test('disables the save dataset button when the results are not loaded', async () => {
const { findByRole } = setupWithLatestQuery({
state: QueryState.Success,
results: undefined,
});
expect(await findByRole('button', { name: 'Save dataset' })).toBeDisabled();
});
test('renders an Extension if provided', async () => {
const extensionsRegistry = getExtensionsRegistry();
@@ -40,6 +40,7 @@ import {
getExtensionsRegistry,
QueryResponse,
Query,
QueryState,
} from '@superset-ui/core';
import { Alert } from '@apache-superset/core/components';
import { css, styled, useTheme } from '@apache-superset/core/theme';
@@ -283,7 +284,7 @@ const SqlEditor: FC<Props> = ({
getItem(LocalStorageKeys.SqllabIsAutocompleteEnabled, true),
);
const [renderHTMLEnabled, setRenderHTMLEnabled] = useState(
getItem(LocalStorageKeys.SqllabIsRenderHtmlEnabled, true),
getItem(LocalStorageKeys.SqllabIsRenderHtmlEnabled, false),
);
const [showCreateAsModal, setShowCreateAsModal] = useState(false);
const [createAs, setCreateAs] = useState('');
@@ -295,6 +296,9 @@ const SqlEditor: FC<Props> = ({
const SqlFormExtension = extensionsRegistry.get('sqleditor.extension.form');
const successful = latestQuery?.state === QueryState.Success;
const resultColumns = latestQuery?.results?.columns || [];
const startQuery = useCallback(
(
ctasArg = false,
@@ -712,7 +716,6 @@ const SqlEditor: FC<Props> = ({
const getSecondaryMenuItems = () => {
const qe = queryEditor;
const successful = latestQuery?.state === 'success';
const scheduleToolTip = successful
? t('Schedule the query periodically')
: t('You must run the query successfully first');
@@ -858,13 +861,14 @@ const SqlEditor: FC<Props> = ({
)}
<SaveQuery
queryEditorId={queryEditor.id}
columns={latestQuery?.results?.columns || []}
columns={resultColumns}
onSave={onSaveQuery}
onUpdate={(query, remoteId) =>
dispatch(updateSavedQuery(query, remoteId))
}
saveQueryWarning={saveQueryWarning}
database={database}
canSaveDataset={successful && resultColumns.length > 0}
/>
<ShareSqlLabQuery queryEditorId={queryEditor.id} />
</>
@@ -223,6 +223,11 @@ export default function chartReducer(
}
if (action.type in actionHandlers) {
// ADD_CHART creates the entry, so it runs without prior state; every other
// handler reads state that is absent once the chart has been removed
if (action.type !== actions.ADD_CHART && !charts[action.key]) {
return charts;
}
return {
...charts,
[action.key]: actionHandlers[action.type](charts[action.key]),
@@ -91,4 +91,20 @@ describe('chart reducers', () => {
expect(newState[chartKey].chartUpdateEndTime).toBeGreaterThan(0);
expect(newState[chartKey].chartStatus).toEqual('failed');
});
test('ignores an action for a chart that is no longer in state', () => {
const action = actions.chartUpdateStopped(999, new AbortController());
expect(() => chartReducer(charts, action)).not.toThrow();
expect(chartReducer(charts, action)).toEqual(charts);
});
test('still adds a chart that is not yet in state', () => {
const newChartKey = 2;
const newState = chartReducer(
charts,
actions.addChart({ ...chart, id: newChartKey }, newChartKey),
);
expect(newState[newChartKey].id).toEqual(newChartKey);
expect(newState[chartKey]).toEqual(testChart);
});
});
@@ -120,7 +120,7 @@ describe('DatasourceModal', () => {
});
const saveButton = screen.getByTestId('datasource-modal-save');
fireEvent.click(saveButton);
const okButton = await screen.findByRole('button', { name: 'OK' });
const okButton = await screen.findByRole('button', { name: 'Confirm' });
fireEvent.click(okButton);
await waitFor(() => {
expect(onDatasourceSave).toHaveBeenCalled();
@@ -142,7 +142,7 @@ describe('DatasourceModal', () => {
const saveButton = screen.getByTestId('datasource-modal-save');
fireEvent.click(saveButton);
const okButton = await screen.findByRole('button', { name: 'OK' });
const okButton = await screen.findByRole('button', { name: 'Confirm' });
fireEvent.click(okButton);
const errorElements = await screen.findAllByText('Error saving dataset');
@@ -230,7 +230,7 @@ describe('DatasourceModal', () => {
expect(checkbox).toBeChecked();
// Click OK to submit
const okButton = screen.getByRole('button', { name: 'OK' });
const okButton = screen.getByRole('button', { name: 'Confirm' });
fireEvent.click(okButton);
// Verify the PUT request was made with override_columns=true
@@ -297,7 +297,7 @@ describe('DatasourceModal', () => {
expect(checkbox).not.toBeChecked();
// Click OK to submit
const okButton = screen.getByRole('button', { name: 'OK' });
const okButton = screen.getByRole('button', { name: 'Confirm' });
fireEvent.click(okButton);
// Verify the PUT request was made with override_columns=false
@@ -395,7 +395,7 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
show={confirmModalOpen}
onHide={handleConfirmModalClose}
onHandledPrimaryAction={handleConfirmSave}
primaryButtonName={t('OK')}
primaryButtonName={t('Confirm')}
primaryButtonLoading={isSaving}
>
{getSaveDialog()}
@@ -1993,7 +1993,6 @@ function DatasourceEditor({
col => col.column_name,
)}
height={300}
allowHTML
/>
</>
)}
@@ -63,7 +63,7 @@ export const FilterableTable = ({
height,
filterText = '',
expandedColumns = [],
allowHTML = true,
allowHTML = false,
striped,
themeOverrides,
}: FilterableTableProps) => {
@@ -64,6 +64,35 @@ test('should render cellData value for default cell data', () => {
expect(container).toHaveTextContent('regular_text');
});
test('should render HTML cell data as inert text by default', () => {
const { container } = render(
<>
{renderResultCell({
cellData: '<img src="https://attacker.example/beacon.gif" />link',
columnKey: 'a',
})}
</>,
);
expect(container.querySelector('img')).not.toBeInTheDocument();
expect(container).toHaveTextContent(
'<img src="https://attacker.example/beacon.gif" />link',
);
});
test('should render sanitized HTML only when allowHTML is explicitly enabled', () => {
const { container } = render(
<>
{renderResultCell({
cellData: '<b>bold</b>',
columnKey: 'a',
allowHTML: true,
})}
</>,
);
expect(container.querySelector('b')).toBeInTheDocument();
expect(container).toHaveTextContent('bold');
});
test('should transform cell data by getCellContent for the regular text', () => {
const { container } = render(
<>
@@ -32,11 +32,15 @@ type Params = CellParams & {
getCellContent?: (args: CellParams) => string;
};
// Result cells carry untrusted warehouse data, so HTML rendering is opt-in:
// even sanitized markup keeps active capabilities (img/video fetch beacons,
// phishing anchors), which must not activate by default for data the viewer
// did not author.
export const renderResultCell = ({
cellData,
getCellContent,
columnKey,
allowHTML = true,
allowHTML = false,
}: Params) => {
const cellNode =
getCellContent?.({ cellData, columnKey }) ?? String(cellData);
+31
View File
@@ -0,0 +1,31 @@
/**
* 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 {
URL_PARAMS,
RESERVED_CHART_URL_PARAMS,
RESERVED_DASHBOARD_URL_PARAMS,
} from 'src/constants';
test('permalinkKey is reserved on both the chart and dashboard URL param lists', () => {
// Dashboard and explore permalinks resolve against different backend
// KV resources/salts, so a key from one must never leak into the other's
// URL via the reserved-params passthrough logic.
expect(RESERVED_DASHBOARD_URL_PARAMS).toContain(URL_PARAMS.permalinkKey.name);
expect(RESERVED_CHART_URL_PARAMS).toContain(URL_PARAMS.permalinkKey.name);
});
+1
View File
@@ -123,6 +123,7 @@ export const RESERVED_CHART_URL_PARAMS: string[] = [
URL_PARAMS.datasourceId.name,
URL_PARAMS.datasourceType.name,
URL_PARAMS.datasetId.name,
URL_PARAMS.permalinkKey.name,
URL_PARAMS.versionHistory.name,
];
export const RESERVED_DASHBOARD_URL_PARAMS: string[] = [
@@ -105,10 +105,17 @@ export const SamplesPane = ({
1,
)
.then(response => {
setData(ensureIsArray(response.data));
setColnames(ensureIsArray(response.colnames));
setColtypes(ensureIsArray(response.coltypes));
setRowCount(response.rowcount);
// A 200 that carries no `result` payload resolves to undefined here.
// Read through it so the pane falls back to its empty state instead
// of throwing a TypeError that surfaces as an internal error message.
const rows = ensureIsArray(response?.data);
setData(rows);
setColnames(ensureIsArray(response?.colnames));
setColtypes(ensureIsArray(response?.coltypes));
// Fall back to the rows actually returned rather than to zero: the
// controls only render when there are rows, and a hardcoded 0 would
// label a populated table as "0 rows".
setRowCount(response?.rowcount ?? rows.length);
setResponseError('');
cache.set(queryFormData, true);
if (queryForce) {
@@ -19,7 +19,6 @@
import { useMemo, useCallback, useRef, useState } from 'react';
import {
getTimeFormatter,
safeHtmlSpan,
TimeFormats,
getMetricLabel,
QueryFormMetric,
@@ -92,8 +91,11 @@ export function useGridColumns(
) {
return timeFormatter(value);
}
// Render string cells as plain text: this grid shows raw
// query results (untrusted warehouse data) to any viewer,
// so HTML must stay inert here even after sanitization.
if (typeof value === 'string') {
return safeHtmlSpan(value);
return value;
}
return String(value);
},
@@ -60,6 +60,27 @@ describe('SamplesPane', () => {
400,
);
// A 200 response that carries no `result` payload, as reported in #36840.
fetchMock.post(
'end:/datasource/samples?force=false&datasource_type=table&datasource_id=37&per_page=100&page=1',
{},
);
// A 200 whose result carries rows but omits `rowcount`.
fetchMock.post(
'end:/datasource/samples?force=false&datasource_type=table&datasource_id=38&per_page=100&page=1',
{
result: {
data: [
{ __timestamp: 1230768000000, genre: 'Action' },
{ __timestamp: 1230768000010, genre: 'Horror' },
],
colnames: ['__timestamp', 'genre'],
coltypes: [2, 1],
},
},
);
const setForceQuery = jest.fn();
afterAll(() => {
@@ -114,4 +135,29 @@ describe('SamplesPane', () => {
expect(queryByText('Action')).toBeVisible();
expect(queryByText('Horror')).toBeVisible();
});
test('renders the empty state when the response carries no result payload', async () => {
const props = createSamplesPaneProps({ datasourceId: 37 });
const { findByText, queryByRole } = render(<SamplesPane {...props} />, {
useRedux: true,
});
expect(
await findByText('No samples were returned for this dataset'),
).toBeVisible();
// The pane should not leak an internal TypeError through the error alert.
expect(queryByRole('alert')).not.toBeInTheDocument();
});
test('counts the returned rows when the response omits rowcount', async () => {
const props = createSamplesPaneProps({ datasourceId: 38 });
const { findByText, queryByText } = render(<SamplesPane {...props} />, {
useRedux: true,
});
expect(await findByText('Action')).toBeVisible();
// Falling back to 0 here would label a populated table as "0 rows".
expect(queryByText('0 rows')).not.toBeInTheDocument();
expect(queryByText('2 rows')).toBeVisible();
});
});
@@ -150,7 +150,7 @@ const waitForRender = (props?: any) =>
test('renders with default props', async () => {
await waitForRender();
expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled();
expect(screen.getByRole('button', { name: 'OK' })).toBeDisabled();
expect(screen.getByRole('button', { name: 'Confirm' })).toBeDisabled();
expect(screen.getByRole('button', { name: 'Cancel' })).toBeEnabled();
});
@@ -188,7 +188,7 @@ test('enables apply and ok buttons', async () => {
await waitFor(() => {
expect(screen.getByRole('button', { name: 'Apply' })).toBeEnabled();
expect(screen.getByRole('button', { name: 'OK' })).toBeEnabled();
expect(screen.getByRole('button', { name: 'Confirm' })).toBeEnabled();
});
});
@@ -203,7 +203,7 @@ test('triggers addAnnotationLayer and close when ok button is clicked', async ()
const addAnnotationLayer = jest.fn();
const close = jest.fn();
await waitForRender({ name: 'Test', value: '2x', addAnnotationLayer, close });
userEvent.click(screen.getByRole('button', { name: 'OK' }));
userEvent.click(screen.getByRole('button', { name: 'Confirm' }));
expect(addAnnotationLayer).toHaveBeenCalled();
expect(close).toHaveBeenCalled();
});
@@ -724,7 +724,7 @@ test('Disable apply button if formula is incorrect', async () => {
const formulaInput = screen.getByRole('textbox', { name: 'Formula' });
const applyButton = screen.getByRole('button', { name: 'Apply' });
const okButton = screen.getByRole('button', { name: 'OK' });
const okButton = screen.getByRole('button', { name: 'Confirm' });
userEvent.type(formulaInput, 'x+1');
expect(formulaInput).toHaveValue('x+1');
@@ -1303,7 +1303,7 @@ function AnnotationLayer({
disabled={!isValid}
onClick={submitAnnotation}
>
{t('OK')}
{t('Confirm')}
</Button>
</div>
</div>
@@ -112,6 +112,31 @@ describe('ColorBreakpointOption', () => {
expect(colorPreview).toBeInTheDocument();
});
test('should coerce non-numeric color channels instead of injecting CSS', async () => {
// Stored form_data is opaque JSON, so a saved chart can carry a string
// channel crafted to break out of the styled-component declaration.
const maliciousBreakpoint = {
id: 3,
color: {
r: '0,0,0,1); } body { display: none } .x { background: rgba(0' as unknown as number,
g: 0,
b: 0,
a: 1,
},
minValue: 0,
maxValue: 100,
};
renderComponent({ breakpoint: maliciousBreakpoint });
const colorPreview = await screen.findByTestId('color-preview');
expect(colorPreview).toBeInTheDocument();
// The malicious string is coerced to a safe numeric channel (0), so the
// rendered rule is a plain rgba() value with no injected CSS.
expect(colorPreview).toHaveStyle({ background: 'rgba(0, 0, 0, 1)' });
expect(document.body).toBeVisible();
});
test('should handle decimal values', async () => {
const decimalBreakpoint: ColorBreakpointType = {
id: 2,
@@ -31,6 +31,19 @@ const BreakpointColorPreview = styled.div`
margin-right: ${({ theme }) => theme.sizeUnit}px;
`;
// Chart params are stored server-side as opaque JSON, so breakpoint color
// channels can arrive as arbitrary strings despite the compile-time numeric
// type. The formatted color is interpolated into a styled-component template
// (a stylesheet, not a per-property style assignment), so each channel must
// be coerced to a plain number to keep attacker-controlled strings from
// injecting CSS rules.
const toRgbChannel = (channel: unknown): number => {
const value = Number(channel);
return Number.isFinite(value)
? Math.min(255, Math.max(0, Math.round(value)))
: 0;
};
const ColorBreakpointOption = ({
breakpoint,
colorBreakpoints,
@@ -41,7 +54,9 @@ const ColorBreakpointOption = ({
const { color, minValue, maxValue } = breakpoint;
const formattedColor = color
? `rgba(${color.r}, ${color.g}, ${color.b}, 1)`
? `rgba(${toRgbChannel(color.r)}, ${toRgbChannel(color.g)}, ${toRgbChannel(
color.b,
)}, 1)`
: '';
return (
@@ -29,9 +29,7 @@ import { ControlFormItemComponents } from './ControlForm';
* Column formatting configs.
*/
export type ColumnConfig = {
[
key in SharedColumnConfigProp
]?: (typeof SHARED_COLUMN_CONFIG_PROPS)[key]['value'];
[key in SharedColumnConfigProp]?: (typeof SHARED_COLUMN_CONFIG_PROPS)[key]['value'];
} & Record<string, StrictJsonValue>;
/**
@@ -187,7 +187,7 @@ async function openAndSaveChanges(
await userEvent.click(screen.getByTestId('datasource-menu-trigger'));
await userEvent.click(await screen.findByTestId('edit-dataset'));
await userEvent.click(await screen.findByTestId('datasource-modal-save'));
await userEvent.click(await screen.findByText('OK'));
await userEvent.click(await screen.findByText('Confirm'));
}
test('Should render', async () => {
@@ -714,10 +714,10 @@ test('should handle metric save confirmation modal', async () => {
await userEvent.click(await screen.findByTestId('datasource-modal-save'));
// Verify confirmation modal appears
expect(await screen.findByText('OK')).toBeInTheDocument();
expect(await screen.findByText('Confirm')).toBeInTheDocument();
// Confirm save
await userEvent.click(screen.getByText('OK'));
await userEvent.click(screen.getByText('Confirm'));
await waitFor(() => {
expect(props.onDatasourceSave).toHaveBeenCalled();
@@ -22,11 +22,10 @@ import FixedOrMetricControl from '.';
jest.mock(
'@superset-ui/core/components/Icons/AsyncIcon',
() =>
({ fileName }: { fileName: string }) =>
(
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- mirrors AsyncIcon's real span+role="img" shape
<span role="img" aria-label={fileName.replace('_', '-')} />
),
({ fileName }: { fileName: string }) => (
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- mirrors AsyncIcon's real span+role="img" shape
<span role="img" aria-label={fileName.replace('_', '-')} />
),
);
const createProps = () => ({
@@ -191,6 +191,29 @@ describe('AdhocMetric', () => {
expect(adhocMetric2.inferSqlExpressionAggregate()).toBeNull();
});
test('can infer the new extended aggregates (STDDEV_SAMP/VAR_SAMP/MEDIAN) from sql expressions', () => {
const stddevSamp = new AdhocMetric({
expressionType: EXPRESSION_TYPES.SQL,
sqlExpression: 'STDDEV_SAMP(my_column)',
});
expect(stddevSamp.inferSqlExpressionColumn()).toBe('my_column');
expect(stddevSamp.inferSqlExpressionAggregate()).toBe('STDDEV_SAMP');
const varSamp = new AdhocMetric({
expressionType: EXPRESSION_TYPES.SQL,
sqlExpression: 'VAR_SAMP(my_column)',
});
expect(varSamp.inferSqlExpressionColumn()).toBe('my_column');
expect(varSamp.inferSqlExpressionAggregate()).toBe('VAR_SAMP');
const median = new AdhocMetric({
expressionType: EXPRESSION_TYPES.SQL,
sqlExpression: 'MEDIAN(my_column)',
});
expect(median.inferSqlExpressionColumn()).toBe('my_column');
expect(median.inferSqlExpressionAggregate()).toBe('MEDIAN');
});
test('will infer columns and aggregates when converting to a simple expression', () => {
const adhocMetric = new AdhocMetric({
expressionType: EXPRESSION_TYPES.SQL,
@@ -248,4 +271,20 @@ describe('AdhocMetric', () => {
).toBe('COUNT_DISTINCT');
expect(emptyColumnName.getDefaultLabel()).toBe('COUNT_DISTINCT');
});
test('should prefill a portable MEDIAN expression for the Custom SQL tab, but keep the raw label', () => {
const median = new AdhocMetric({
column: valueColumn,
aggregate: AGGREGATES.MEDIAN,
hasCustomLabel: false,
});
// MEDIAN(column) isn't valid SQL on every engine this PR verifies it
// for (e.g. PostgreSQL has no MEDIAN function), so the editable Custom
// SQL tab is prefilled with the portable, standards-based spelling.
expect(median.translateToSql({ transformCountDistinct: true })).toBe(
'PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY value)',
);
// The display label stays the concise, human-readable form.
expect(median.getDefaultLabel()).toBe('MEDIAN(value)');
});
});
@@ -153,6 +153,19 @@ export default class AdhocMetric {
) {
return `COUNT(DISTINCT ${column.slice(1, -1)})`;
}
// MEDIAN(column) isn't a real function on every engine this PR
// verifies it for -- PostgreSQL/Redshift compile it to
// PERCENTILE_CONT(0.5) WITHIN GROUP instead. `transformCountDistinct`
// signals this call is prefilling the *editable, executable* Custom
// SQL tab (not just a display label), so use the portable,
// standards-based spelling there instead of the raw aggregate name.
if (
params.transformCountDistinct &&
aggregate === AGGREGATES.MEDIAN &&
/^\(.*\)$/.test(column)
) {
return `PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY ${column.slice(1, -1)})`;
}
return aggregate + column;
}
if (this.expressionType === EXPRESSION_TYPES.SQL) {
@@ -36,6 +36,7 @@ import {
import sqlKeywords from 'src/SqlLab/utils/sqlKeywords';
import { noOp } from 'src/utils/common';
import {
AGGREGATES_LABELS,
AGGREGATES_OPTIONS,
POPOVER_INITIAL_HEIGHT,
POPOVER_INITIAL_WIDTH,
@@ -548,7 +549,7 @@ function AdhocMetricEditPopover({
<Select
options={AGGREGATES_OPTIONS.map(option => ({
value: option,
label: option,
label: AGGREGATES_LABELS[option] ?? option,
key: option,
}))}
{...aggregateSelectProps}
@@ -278,6 +278,24 @@ describe('VizTypeControl', () => {
).not.toBeInTheDocument();
});
test('Thumbnail labels expose the full chart name via a title tooltip', async () => {
// Labels are clamped to a fixed two-line block so every tile is the same
// height; the full (possibly truncated) name must stay discoverable through
// the title attribute.
await waitForRenderWrapper();
userEvent.click(screen.getByRole('tab', { name: 'All charts' }));
const visualizations = screen.getByTestId(getTestId('viz-row'));
const labels = await within(visualizations).findAllByTestId(
getTestId('viztype-label'),
);
expect(labels.length).toBeGreaterThan(0);
labels.forEach(label => {
expect(label).toHaveAttribute('title', label.textContent ?? '');
});
});
test('Submit on viz type double-click', async () => {
await waitForRenderWrapper();
userEvent.click(screen.getByRole('tab', { name: 'All charts' }));
@@ -214,6 +214,9 @@ const IconsPane = styled.div`
justify-content: space-evenly;
grid-gap: ${({ theme }) => theme.sizeUnit * 2}px;
justify-items: center;
/* top-align every tile so a longer chart name never pushes the thumbnails
of the other tiles in the same row upward */
align-items: start;
/* for some reason this padding doesn't seem to apply at the bottom of the container. Why is a mystery. */
padding: ${({ theme }) => theme.sizeUnit * 2}px;
`;
@@ -274,7 +277,6 @@ const thumbnailContainerCss = (theme: SupersetTheme) => css`
font: inherit;
cursor: pointer;
width: ${theme.sizeUnit * THUMBNAIL_GRID_UNITS}px;
position: relative;
outline: none; /* Remove focus outline to show only selected state */
img {
@@ -297,6 +299,16 @@ const thumbnailContainerCss = (theme: SupersetTheme) => css`
.viztype-label {
margin-top: ${theme.sizeUnit * 2}px;
text-align: center;
/* reserve a fixed two-line block so every tile is the same height,
regardless of how long the chart name is. Longer names are clamped
with an ellipsis; the full name stays available via the title tooltip. */
line-height: ${theme.sizeUnit * 4}px;
height: ${theme.sizeUnit * 8}px;
display: -webkit-box;
-webkit-line-clamp: 2;
-webkit-box-orient: vertical;
overflow: hidden;
word-break: break-word;
}
`;
@@ -320,10 +332,19 @@ const HighlightLabel = styled.div`
`}
`;
// Wraps the thumbnail image so the "Featured" badge can be anchored to the
// image itself rather than to the whole tile (whose height varies with the
// chart-name length). line-height: 0 removes the inline-image descender gap.
const ThumbnailImageWrapper = styled.div`
position: relative;
width: ${({ theme }) => theme.sizeUnit * THUMBNAIL_GRID_UNITS}px;
line-height: 0;
`;
const ThumbnailLabelWrapper = styled.div`
position: absolute;
right: ${({ theme }) => theme.sizeUnit}px;
top: ${({ theme }) => theme.sizeUnit * 19}px;
top: ${({ theme }) => theme.sizeUnit}px;
`;
const TitleLabelWrapper = styled.div`
@@ -367,27 +388,32 @@ const Thumbnail: FC<ThumbnailProps> = ({
onFocus={handleFocus}
data-test="viztype-selector-container"
>
<img
alt={type.name}
width="100%"
className={`viztype-selector ${isSelected ? 'selected' : ''}`}
src={
isDarkMode && type.thumbnailDark ? type.thumbnailDark : type.thumbnail
}
/>
<ThumbnailImageWrapper>
<img
alt={type.name}
width="100%"
className={`viztype-selector ${isSelected ? 'selected' : ''}`}
src={
isDarkMode && type.thumbnailDark
? type.thumbnailDark
: type.thumbnail
}
/>
{type.label && (
<ThumbnailLabelWrapper>
<HighlightLabel>
<div>{t(type.label)}</div>
</HighlightLabel>
</ThumbnailLabelWrapper>
)}
</ThumbnailImageWrapper>
<div
className="viztype-label"
data-test={`${VIZ_TYPE_CONTROL_TEST_ID}__viztype-label`}
title={type.name}
>
{type.name}
</div>
{type.label && (
<ThumbnailLabelWrapper>
<HighlightLabel>
<div>{t(type.label)}</div>
</HighlightLabel>
</ThumbnailLabelWrapper>
)}
</button>
);
};
@@ -78,6 +78,37 @@ import { useExploreDataExport } from './useExploreDataExport';
export const SEARCH_THRESHOLD = 10;
/**
* Escape a single CSV cell value.
*
* Mirrors the server-side chokepoint (superset/utils/csv.py escape_value):
* values starting with a spreadsheet formula prefix (=, +, -, @, |, %, or a
* leading tab/carriage return, optionally behind leading whitespace) are
* neutralized with a leading single quote so exported cells cannot execute
* as formulas when opened in Excel/LibreOffice/Google Sheets. Plain negative
* numbers are left untouched. RFC-4180 quoting is applied afterwards.
*/
export const escapeCsvValue = (v: unknown): string => {
if (v === null || v === undefined) return '';
let s = String(v);
if (s.length > 0) {
const stripped = s.replace(/^\s+/, '');
const startsLikeFormula =
s[0] === '\t' ||
s[0] === '\r' ||
(stripped.length > 0 && '-@+|=%'.includes(stripped[0]));
const isNegativeNumber = s.length > 1 && /^-[0-9.]+$/.test(s);
if (startsLikeFormula && !isNegativeNumber) {
// Escape pipe to be extra safe (DDE payloads), then prefix with a
// single quote to prevent formula evaluation. Existing backslashes
// must be escaped first so the resulting `\|`/`\\` sequences are
// unambiguous to a downstream unescaper.
s = `'${s.replace(/\\/g, '\\\\').replace(/\|/g, '\\|')}`;
}
}
return /[",\r\n]/.test(s) ? `"${s.replace(/"/g, '""')}"` : s;
};
const MENU_KEYS = {
EDIT_PROPERTIES: 'edit_properties',
DASHBOARDS_ADDED_TO: 'dashboards_added_to',
@@ -488,15 +519,11 @@ export const useExploreAdditionalActionsMenu = (
filename: string,
) => {
if (!rows?.length || !columns?.length) return;
const esc = (v: unknown): string => {
if (v === null || v === undefined) return '';
const s = String(v);
const wrapped = /[",\n]/.test(s) ? `"${s.replace(/"/g, '""')}"` : s;
return wrapped;
};
const header = columns.map(c => esc(c.label ?? c.key ?? '')).join(',');
const header = columns
.map(c => escapeCsvValue(c.label ?? c.key ?? ''))
.join(',');
const body = rows
.map(r => columns.map(c => esc(r[c.key])).join(','))
.map(r => columns.map(c => escapeCsvValue(r[c.key])).join(','))
.join('\n');
const csv = `${header}\n${body}`;
const blob = new Blob([csv], { type: 'text/csv;charset=utf-8;' });
@@ -25,6 +25,7 @@ import downloadAsPdf from 'src/utils/downloadAsPdf';
import {
useExploreAdditionalActionsMenu,
getExportScreenshotMenuItems,
escapeCsvValue,
} from './index';
import * as exploreUtils from 'src/explore/exploreUtils';
import { Slice } from 'src/types/Chart';
@@ -180,6 +181,45 @@ test('hides Edit chart properties from a chart editor lacking chart write permis
expect(screen.queryByText('Edit chart properties')).not.toBeInTheDocument();
});
test('escapeCsvValue neutralizes spreadsheet formula prefixes', () => {
// Mirrors superset/utils/csv.py escape_value so the client-built
// "Current View" CSV cannot ship live formulas (CSV injection).
expect(escapeCsvValue('=HYPERLINK("https://attacker.example")')).toBe(
`"'=HYPERLINK(""https://attacker.example"")"`,
);
expect(escapeCsvValue('@SUM(1+1)')).toBe(`'@SUM(1+1)`);
expect(escapeCsvValue('+cmd')).toBe(`'+cmd`);
expect(escapeCsvValue('%x')).toBe(`'%x`);
expect(escapeCsvValue('\t=1+1')).toBe(`'\t=1+1`);
expect(escapeCsvValue(' =1+1')).toBe(`' =1+1`);
expect(escapeCsvValue('=cmd|calc')).toBe(`'=cmd\\|calc`);
});
test('escapeCsvValue escapes pre-existing backslashes before escaping pipes', () => {
// A literal backslash sitting next to a pipe must not be left as-is: if it
// were, the escaped output (`\|`) would be indistinguishable from an
// escaped pipe, so a downstream unescaper couldn't recover the original
// value. Escaping backslashes first keeps the two cases unambiguous.
expect(escapeCsvValue('=cmd\\|calc')).toBe(`'=cmd\\\\\\|calc`);
});
test('escapeCsvValue RFC-4180-quotes a value containing a bare carriage return', () => {
// A raw \r inside a cell can be read as a record separator by some CSV
// consumers, so it must trigger outer quoting the same way \n does, even
// when it also triggered the formula-prefix guard above.
expect(escapeCsvValue('\r=1+1')).toBe(`"'\r=1+1"`);
});
test('escapeCsvValue keeps ordinary values intact', () => {
expect(escapeCsvValue('regular text')).toBe('regular text');
expect(escapeCsvValue('-12.5')).toBe('-12.5');
expect(escapeCsvValue(42)).toBe('42');
expect(escapeCsvValue(null)).toBe('');
expect(escapeCsvValue(undefined)).toBe('');
expect(escapeCsvValue('a,b')).toBe(`"a,b"`);
expect(escapeCsvValue('say "hi"')).toBe(`"say ""hi"""`);
});
test('shows 413 error toast when exportCSV fails with 413', async () => {
mockExportChart.mockRejectedValue({ status: 413 });
+12 -1
View File
@@ -23,11 +23,22 @@ export const AGGREGATES = {
COUNT: 'COUNT',
COUNT_DISTINCT: 'COUNT_DISTINCT',
MAX: 'MAX',
MEDIAN: 'MEDIAN',
MIN: 'MIN',
STDDEV_SAMP: 'STDDEV_SAMP',
SUM: 'SUM',
VAR_SAMP: 'VAR_SAMP',
};
export const AGGREGATES_OPTIONS = Object.values(AGGREGATES);
// Human-readable labels for aggregates whose raw enum value isn't
// self-explanatory in the UI. Aggregates absent here (AVG, COUNT, MAX,
// MEDIAN, MIN, SUM, ...) are already clear as their raw value.
export const AGGREGATES_LABELS: Record<string, string> = {
STDDEV_SAMP: t('Sample Standard Deviation'),
VAR_SAMP: t('Sample Variance'),
};
export enum Operators {
Equals = 'EQUALS',
NotEquals = 'NOT_EQUALS',
@@ -176,7 +187,7 @@ export const DISABLE_INPUT_OPERATORS = [
export const sqlaAutoGeneratedMetricNameRegex =
/^(sum|min|max|avg|count|count_distinct)__.*$/i;
export const sqlaAutoGeneratedMetricRegex =
/^(LONG|DOUBLE|FLOAT)?(SUM|AVG|MAX|MIN|COUNT)\([A-Z0-9_."]*\)$/i;
/^(LONG|DOUBLE|FLOAT)?(SUM|AVG|MAX|MIN|COUNT|MEDIAN|STDDEV_SAMP|VAR_SAMP)\([A-Z0-9_."]*\)$/i;
export const TIME_FILTER_LABELS = {
time_range: t('Time range'),
@@ -0,0 +1,108 @@
/**
* 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 { createMemoryHistory, type Update } from 'history';
import { Router } from 'react-router-dom';
import { isFeatureEnabled } from '@superset-ui/core';
import { render, screen, fireEvent } from 'spec/helpers/testing-library';
import type Chart from 'src/types/Chart';
import ChartCard from './ChartCard';
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
isFeatureEnabled: jest.fn(),
}));
const mockChart = {
id: 1,
slice_name: 'Sample Chart',
url: '/explore/?slice_id=1',
changed_on_delta_humanized: '2 days ago',
datasource_name_text: 'Sample dataset',
thumbnail_url: '/thumbnail.png',
} as Chart;
const renderCard = (history: ReturnType<typeof createMemoryHistory>) =>
render(
<Router history={history}>
<ChartCard
chart={mockChart}
hasPerm={() => true}
openChartEditModal={jest.fn()}
bulkSelectEnabled={false}
addDangerToast={jest.fn()}
addSuccessToast={jest.fn()}
refreshData={jest.fn()}
saveFavoriteStatus={jest.fn()}
favoriteStatus={false}
showThumbnails
handleBulkChartExport={jest.fn()}
/>
</Router>,
);
const recordNavigations = (
history: ReturnType<typeof createMemoryHistory>,
): string[] => {
const navigations: string[] = [];
history.listen(({ action, location }: Update) =>
navigations.push(`${action} ${location.pathname}${location.search}`),
);
return navigations;
};
beforeEach(() => {
(isFeatureEnabled as jest.Mock).mockReturnValue(true);
});
afterEach(() => {
(isFeatureEnabled as jest.Mock).mockReset();
});
test('renders the chart title', () => {
renderCard(createMemoryHistory());
expect(screen.getByText('Sample Chart')).toBeInTheDocument();
});
test('clicking the thumbnail navigates to the chart exactly once', () => {
// The cover is a router link and the whole card is clickable, so a click on
// the cover used to be handled twice and pushed two identical entries. That
// left the Back button popping the duplicate instead of returning the user to
// the page they came from.
const history = createMemoryHistory({
initialEntries: ['/superset/welcome/'],
});
renderCard(history);
const navigations = recordNavigations(history);
fireEvent.click(screen.getByRole('link'));
expect(navigations).toEqual(['PUSH /explore/?slice_id=1']);
});
test('clicking the card outside the thumbnail navigates to the chart', () => {
const history = createMemoryHistory({
initialEntries: ['/superset/welcome/'],
});
renderCard(history);
const navigations = recordNavigations(history);
fireEvent.click(screen.getByText('Sample Chart'));
expect(navigations).toEqual(['PUSH /explore/?slice_id=1']);
});
@@ -32,7 +32,11 @@ import {
import Chart from 'src/types/Chart';
import { SubjectPile } from 'src/features/subjects/SubjectPile';
import { KebabMenuButton } from 'src/components';
import { handleChartDelete, CardStyles } from 'src/views/CRUD/utils';
import {
handleChartDelete,
CardStyles,
isNavigationHandledByLink,
} from 'src/views/CRUD/utils';
import { assetUrl } from 'src/utils/assetUrl';
import type { ListViewFetchDataConfig as FetchDataConfig } from 'src/components';
import { TableTab } from 'src/views/CRUD/types';
@@ -208,8 +212,12 @@ export default function ChartCard({
return (
<CardStyles
onClick={() => {
if (!bulkSelectEnabled && chart.url) {
onClick={event => {
if (
!bulkSelectEnabled &&
chart.url &&
!isNavigationHandledByLink(event)
) {
history.push(chart.url);
}
}}
@@ -17,10 +17,16 @@
* under the License.
*/
import { MemoryRouter } from 'react-router-dom';
import { createMemoryHistory, type Update } from 'history';
import { MemoryRouter, Router } from 'react-router-dom';
import { isFeatureEnabled } from '@superset-ui/core';
import { render, screen } from 'spec/helpers/testing-library';
import {
render,
screen,
fireEvent,
within,
} from 'spec/helpers/testing-library';
import { SubjectType } from 'src/types/Subject';
import DashboardCard from './DashboardCard';
@@ -63,6 +69,10 @@ afterAll(() => {
mockedIsFeatureEnabled.mockClear();
});
afterEach(() => {
jest.restoreAllMocks();
});
beforeEach(() => {
render(
<MemoryRouter>
@@ -101,6 +111,43 @@ test('Renders the modified date', () => {
expect(modifiedDateElement).toBeInTheDocument();
});
test('clicking the thumbnail navigates to the dashboard exactly once', () => {
// The cover is a router link and the whole card is clickable, so a click on
// the cover used to be handled twice and pushed two identical entries, which
// left the Back button popping the duplicate rather than returning the user
// to the page they came from.
jest.spyOn(global, 'fetch').mockResolvedValue({
blob: () => Promise.resolve(new Blob([''], { type: 'image/png' })),
} as Response);
const history = createMemoryHistory({
initialEntries: ['/superset/welcome/'],
});
const { container } = render(
<Router history={history}>
<DashboardCard
dashboard={mockDashboard}
hasPerm={mockHasPerm}
bulkSelectEnabled={false}
loading={false}
showThumbnails
openDashboardEditModal={mockOpenDashboardEditModal}
saveFavoriteStatus={mockSaveFavoriteStatus}
favoriteStatus={false}
handleBulkDashboardExport={mockHandleBulkDashboardExport}
onDelete={mockOnDelete}
/>
</Router>,
);
const navigations: string[] = [];
history.listen(({ action, location }: Update) =>
navigations.push(`${action} ${location.pathname}`),
);
fireEvent.click(within(container).getByRole('link'));
expect(navigations).toEqual(['PUSH /dashboard/1']);
});
describe('thumbnail URL construction', () => {
let fetchSpy: jest.SpyInstance;
@@ -20,7 +20,7 @@ import { Link, useHistory } from 'react-router-dom';
import { t } from '@apache-superset/core/translation';
import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core';
import { css } from '@apache-superset/core/theme';
import { CardStyles } from 'src/views/CRUD/utils';
import { CardStyles, isNavigationHandledByLink } from 'src/views/CRUD/utils';
import {
FaveStar,
Icons,
@@ -169,8 +169,8 @@ function DashboardCard({
return (
<CardStyles
onClick={() => {
if (!bulkSelectEnabled) {
onClick={event => {
if (!bulkSelectEnabled && !isNavigationHandledByLink(event)) {
history.push(dashboard.url);
}
}}
@@ -136,6 +136,7 @@ describe('DatabaseModal', () => {
format: 'int32',
maximum: 65536,
minimum: 0,
nullable: true,
type: 'integer',
},
query: {
@@ -153,7 +154,7 @@ describe('DatabaseModal', () => {
type: 'string',
},
},
required: ['database', 'host', 'port', 'username'],
required: ['database', 'host', 'username'],
type: 'object',
},
preferred: true,
@@ -38,11 +38,10 @@ import {
jest.mock(
'@superset-ui/core/components/Icons/AsyncIcon',
() =>
({ fileName }: { fileName: string }) =>
(
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- mirrors AsyncIcon's real span+role="img" shape
<span role="img" aria-label={fileName.replace('_', '-')} />
),
({ fileName }: { fileName: string }) => (
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- mirrors AsyncIcon's real span+role="img" shape
<span role="img" aria-label={fileName.replace('_', '-')} />
),
);
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
@@ -29,11 +29,10 @@ import DatasetPanelWrapper from 'src/features/datasets/AddDataset/DatasetPanel';
jest.mock(
'@superset-ui/core/components/Icons/AsyncIcon',
() =>
({ fileName }: { fileName: string }) =>
(
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- mirrors AsyncIcon's real span+role="img" shape
<span role="img" aria-label={fileName.replace('_', '-')} />
),
({ fileName }: { fileName: string }) => (
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- mirrors AsyncIcon's real span+role="img" shape
<span role="img" aria-label={fileName.replace('_', '-')} />
),
);
const errorMessageRegistry = getErrorMessageComponentRegistry();
@@ -141,6 +141,7 @@ const renderArchivedList = (withStore = store) =>
beforeEach(() => {
fetchMock.removeRoutes();
fetchMock.clearHistory();
mockAddDangerToast.mockClear();
});
test('renders archived rows with Name and Type columns', async () => {
@@ -204,6 +205,31 @@ test('restore failure surfaces an error and leaves the row in place', async () =
expect(screen.getByText('Deleted Chart One')).toBeInTheDocument();
});
test('restoring an already-restored row (404) surfaces an error without crashing', async () => {
// Simulates another actor having restored the object out from under this
// view: the server answers 404 to the now-stale row's restore request.
mockRoutes(404);
renderArchivedList();
await screen.findByTestId('archived-list-view');
const restoreButtons = await screen.findAllByTestId('archived-row-restore');
fireEvent.click(restoreButtons[0]);
await waitFor(() => {
expect(fetchMock.callHistory.calls(/chart\/uuid-1\/restore/)).toHaveLength(
1,
);
});
await waitFor(() => {
expect(mockAddDangerToast).toHaveBeenCalledWith(
expect.stringContaining('Failed to restore Deleted Chart One'),
);
});
expect(mockAddDangerToast).toHaveBeenCalledTimes(1);
// The page is still functional -- the list view did not crash.
expect(screen.getByTestId('archived-list-view')).toBeInTheDocument();
});
test('row actions are keyboard-operable (Enter restores)', async () => {
mockRoutes();
renderArchivedList();
@@ -273,6 +299,45 @@ test('name search refetches with a contains filter on the name field', async ()
});
});
test('a search that matches nothing shows the empty-state and no restore actions', async () => {
// The initial load returns real rows; only the search-triggered request
// answers empty. If the list were empty from the start, this test could
// pass even if the search never fired a request at all -- so the request
// itself is asserted below before trusting the rendered empty state.
fetchMock.get(infoEndpoint, { permissions: ['can_read', 'can_write'] });
fetchMock.getOnce(listEndpoint, {
result: mockCharts,
count: mockCharts.length,
});
fetchMock.get(listEndpoint, { result: [], count: 0 });
renderArchivedList();
await screen.findByText('Deleted Chart One');
const searchInput = screen.getByPlaceholderText(/type a value/i);
fireEvent.change(searchInput, { target: { value: 'e2e_nonexistent' } });
fireEvent.keyDown(searchInput, { key: 'Enter', keyCode: 13 });
await waitFor(() => {
const hit = fetchMock.callHistory
.calls(/chart\/\?q/)
.find(call =>
call.url.includes(
'(col:slice_name,opr:chart_all_text,value:e2e_nonexistent)',
),
);
expect(hit).toBeTruthy();
});
// ListView renders this hardcoded copy whenever a filter is active and the
// result set is empty, overriding the page's own `emptyState` prop
// entirely (see ListView.tsx) -- so this is the actual rendered text, not
// the page's "No archived items" default.
expect(
await screen.findByText('No results match your filter criteria'),
).toBeInTheDocument();
expect(screen.queryAllByTestId('archived-row-restore')).toHaveLength(0);
});
test('switching Type fetches the newly selected resource with its deleted-state filter', async () => {
mockRoutes();
renderArchivedList();
@@ -239,6 +239,40 @@ describe('ChartList', () => {
screen.getByRole('button', { name: 'Bulk select' }),
).toBeInTheDocument();
});
test('archive (soft-delete) confirmation reflects recoverable semantics, not delete', async () => {
// With SOFT_DELETE on, the same delete affordance becomes reversible: the
// dialog reads "Archive", not "Delete", and drops the "type DELETE to
// confirm" gate -- that friction is reserved for the permanent purge in
// the Recently Archived view, not this one.
(
isFeatureEnabled as jest.MockedFunction<typeof isFeatureEnabled>
).mockImplementation((feature: string) => feature === 'SOFT_DELETE');
// isUserEditorOrAdmin requires `username` + `permissions` to recognize an
// Admin role (see src/types/bootstrapTypes.ts's isUserWithPermissionsAndRoles);
// mockUser lacks both, so row actions would otherwise render disabled.
const adminUser = { ...mockUser, username: 'admin', permissions: {} };
renderChartList(adminUser);
await screen.findByTestId('chart-list-view');
const deleteButtons = await screen.findAllByTestId('chart-row-delete');
fireEvent.click(deleteButtons[0]);
const dialog = await screen.findByRole('dialog');
expect(
within(dialog).getByText(`Archive ${mockCharts[0].slice_name}?`),
).toBeInTheDocument();
expect(
within(dialog).getByRole('button', { name: 'Archive' }),
).toBeInTheDocument();
expect(
within(dialog).getByText(/moved to Recently Archived/i),
).toBeInTheDocument();
expect(within(dialog).getByText(/recover it there/i)).toBeInTheDocument();
expect(screen.queryByTestId('delete-modal-input')).not.toBeInTheDocument();
});
});
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
@@ -349,3 +383,223 @@ describe('ChartList - Global Filter Interactions', () => {
expect((searchInput as HTMLInputElement).value).toBe('');
});
});
// The blocking-alerts/reports pre-flight in the Archive modal (sc-117151).
// Each test registers its report-API route BEFORE setupMocks so it takes
// precedence over the catch-all route.
const adminChartUser = { ...mockUser, username: 'admin', permissions: {} };
const openFirstDeleteModal = async () => {
// ALERT_REPORTS must be on for the pre-flight to fire at all — with it off
// the modal opens synchronously with no dependency fetch (see the flag-off
// test below).
(
isFeatureEnabled as jest.MockedFunction<typeof isFeatureEnabled>
).mockImplementation(
(feature: string) =>
feature === 'SOFT_DELETE' || feature === 'ALERT_REPORTS',
);
renderChartList(adminChartUser);
await screen.findByTestId('chart-list-view');
const deleteButtons = await screen.findAllByTestId('chart-row-delete');
fireEvent.click(deleteButtons[0]);
return screen.findByRole('dialog');
};
test('archive modal lists the blocking alerts and reports with their types', async () => {
fetchMock.removeRoutes();
fetchMock.get('glob:*/api/v1/report/*', {
count: 2,
result: [
{ id: 1, name: 'TC-081 rerun report', type: 'Report' },
{ id: 2, name: 'Threshold alert', type: 'Alert' },
],
});
setupMocks();
try {
const dialog = await openFirstDeleteModal();
expect(
within(dialog).getByText('Associated alerts and reports'),
).toBeInTheDocument();
expect(within(dialog).getByText('TC-081 rerun report')).toBeInTheDocument();
expect(within(dialog).getByText('Threshold alert')).toBeInTheDocument();
expect(within(dialog).getByText('Report')).toBeInTheDocument();
expect(within(dialog).getByText('Alert')).toBeInTheDocument();
// Advisory only: the Archive button stays enabled.
expect(
within(dialog).getByRole('button', { name: 'Archive' }),
).toBeEnabled();
} finally {
fetchMock.clearHistory();
(
isFeatureEnabled as jest.MockedFunction<typeof isFeatureEnabled>
).mockReset();
}
});
test('archive modal is unchanged when the chart has no alerts or reports', async () => {
fetchMock.removeRoutes();
fetchMock.get('glob:*/api/v1/report/*', { count: 0, result: [] });
setupMocks();
try {
const dialog = await openFirstDeleteModal();
expect(
within(dialog).getByText(/moved to Recently Archived/i),
).toBeInTheDocument();
expect(
within(dialog).queryByText('Associated alerts and reports'),
).not.toBeInTheDocument();
} finally {
fetchMock.clearHistory();
(
isFeatureEnabled as jest.MockedFunction<typeof isFeatureEnabled>
).mockReset();
}
});
test('archive modal opens unchanged and confirm still deletes when the report API 404s', async () => {
fetchMock.removeRoutes();
fetchMock.get('glob:*/api/v1/report/*', 404);
fetchMock.delete(`glob:*/api/v1/chart/${mockCharts[0].id}`, {});
setupMocks();
try {
const dialog = await openFirstDeleteModal();
expect(
within(dialog).queryByText('Associated alerts and reports'),
).not.toBeInTheDocument();
fireEvent.click(within(dialog).getByRole('button', { name: 'Archive' }));
await waitFor(() =>
expect(
fetchMock.callHistory.calls(`glob:*/api/v1/chart/${mockCharts[0].id}`),
).toHaveLength(1),
);
} finally {
fetchMock.clearHistory();
(
isFeatureEnabled as jest.MockedFunction<typeof isFeatureEnabled>
).mockReset();
}
});
test('archive modal caps the list at ten and reports the overflow count', async () => {
fetchMock.removeRoutes();
fetchMock.get('glob:*/api/v1/report/*', {
count: 12,
result: Array.from({ length: 10 }, (_, i) => ({
id: i + 1,
name: `Blocking report ${i + 1}`,
type: 'Report',
})),
});
setupMocks();
try {
const dialog = await openFirstDeleteModal();
expect(within(dialog).getByText('Blocking report 10')).toBeInTheDocument();
expect(within(dialog).getByText('... and 2 more')).toBeInTheDocument();
} finally {
fetchMock.clearHistory();
(
isFeatureEnabled as jest.MockedFunction<typeof isFeatureEnabled>
).mockReset();
}
});
test('archive modal refetches on every open so the list stays fresh', async () => {
fetchMock.removeRoutes();
fetchMock.get(
'glob:*/api/v1/report/*',
{
count: 1,
result: [{ id: 1, name: 'Detach me first', type: 'Report' }],
},
{ name: 'blocking-reports' },
);
setupMocks();
try {
const dialog = await openFirstDeleteModal();
expect(within(dialog).getByText('Detach me first')).toBeInTheDocument();
fireEvent.click(within(dialog).getByRole('button', { name: 'Cancel' }));
await waitFor(() =>
expect(screen.queryByRole('dialog')).not.toBeInTheDocument(),
);
// The user detaches the report; the next open must show the new truth.
fetchMock.removeRoute('blocking-reports');
fetchMock.get(
'glob:*/api/v1/report/*',
{ count: 0, result: [] },
{ name: 'blocking-reports-empty' },
);
const deleteButtons = await screen.findAllByTestId('chart-row-delete');
fireEvent.click(deleteButtons[0]);
const reopened = await screen.findByRole('dialog');
expect(
within(reopened).queryByText('Detach me first'),
).not.toBeInTheDocument();
expect(
within(reopened).queryByText('Associated alerts and reports'),
).not.toBeInTheDocument();
} finally {
fetchMock.clearHistory();
(
isFeatureEnabled as jest.MockedFunction<typeof isFeatureEnabled>
).mockReset();
}
});
test('archive modal opens without any report fetch when ALERT_REPORTS is off', async () => {
fetchMock.removeRoutes();
fetchMock.get(
'glob:*/api/v1/report/*',
{
count: 1,
result: [{ id: 1, name: 'Should not appear', type: 'Report' }],
},
{ name: 'reports-should-not-be-called' },
);
setupMocks();
(
isFeatureEnabled as jest.MockedFunction<typeof isFeatureEnabled>
).mockImplementation((feature: string) => feature === 'SOFT_DELETE');
try {
renderChartList(adminChartUser);
await screen.findByTestId('chart-list-view');
const deleteButtons = await screen.findAllByTestId('chart-row-delete');
fireEvent.click(deleteButtons[0]);
const dialog = await screen.findByRole('dialog');
expect(
within(dialog).queryByText('Associated alerts and reports'),
).not.toBeInTheDocument();
expect(
fetchMock.callHistory.calls('reports-should-not-be-called'),
).toHaveLength(0);
} finally {
fetchMock.clearHistory();
(
isFeatureEnabled as jest.MockedFunction<typeof isFeatureEnabled>
).mockReset();
}
});
test('delete confirmation keeps the type-DELETE gate when SOFT_DELETE is off', async () => {
fetchMock.removeRoutes();
fetchMock.get('glob:*/api/v1/report/*', { count: 0, result: [] });
setupMocks();
(
isFeatureEnabled as jest.MockedFunction<typeof isFeatureEnabled>
).mockImplementation((feature: string) => feature === 'ALERT_REPORTS');
try {
renderChartList(adminChartUser);
await screen.findByTestId('chart-list-view');
const deleteButtons = await screen.findAllByTestId('chart-row-delete');
fireEvent.click(deleteButtons[0]);
const dialog = await screen.findByRole('dialog');
expect(within(dialog).getByText('Please confirm')).toBeInTheDocument();
expect(screen.getByTestId('delete-modal-input')).toBeInTheDocument();
} finally {
fetchMock.clearHistory();
(
isFeatureEnabled as jest.MockedFunction<typeof isFeatureEnabled>
).mockReset();
}
});
+170 -48
View File
@@ -17,7 +17,7 @@
* under the License.
*/
import { SupersetTheme, css, styled } from '@apache-superset/core/theme';
import { t } from '@apache-superset/core/translation';
import { t, tn } from '@apache-superset/core/translation';
import {
isFeatureEnabled,
FeatureFlag,
@@ -26,7 +26,7 @@ import {
SupersetClient,
isMatrixifyEnabled,
} from '@superset-ui/core';
import { useState, useMemo, useCallback } from 'react';
import { useState, useMemo, useCallback, useRef } from 'react';
import rison from 'rison';
import { uniqBy } from 'lodash-es';
import { useSelector } from 'react-redux';
@@ -53,6 +53,8 @@ import {
ActionButton,
ConfirmStatusChange,
CertifiedBadge,
DeleteModal,
List,
Tooltip,
FaveStar,
InfoTooltip,
@@ -111,6 +113,77 @@ const FlexRowContainer = styled.div`
`;
const PAGE_SIZE = 25;
// How many blocking alerts/reports the archive modal previews before the
// "... and N more" overflow line (dataset-modal parity).
const BLOCKING_REPORTS_PREVIEW_SIZE = 10;
interface BlockingReport {
id: number;
name: string;
type: 'Alert' | 'Report';
}
interface ChartDeleteState {
chart: Chart;
blockingReports: BlockingReport[];
blockingReportsCount: number;
}
function ChartArchiveDescription({
chart,
blockingReports,
blockingReportsCount,
softDelete,
}: ChartDeleteState & { softDelete: boolean }) {
const overflowCount = blockingReportsCount - blockingReports.length;
return (
<>
{softDelete ? (
<p>{archiveConfirmDescription(t('chart'))}</p>
) : (
<p>
{t('Are you sure you want to delete')} <b>{chart.slice_name}</b>?
</p>
)}
{blockingReports.length > 0 && (
<>
<h4>{t('Associated alerts and reports')}</h4>
<p>
{t(
'Archiving or deleting this chart will be blocked while the following alerts or reports use it. Detach or delete them first.',
)}
</p>
<List
split={false}
size="small"
dataSource={blockingReports}
renderItem={(report: BlockingReport) => (
<List.Item key={report.id} compact>
<List.Item.Meta
avatar={<span aria-hidden="true"></span>}
title={report.name}
description={
report.type === 'Alert' ? t('Alert') : t('Report')
}
/>
</List.Item>
)}
/>
{overflowCount > 0 && (
<p>
{tn(
'... and %s more',
'... and %s more',
overflowCount,
overflowCount,
)}
</p>
)}
</>
)}
</>
);
}
const PASSWORDS_NEEDED_MESSAGE = t(
'The passwords for the databases below are needed in order to ' +
'import them together with the charts. Please note that the ' +
@@ -211,6 +284,11 @@ function ChartList(props: ChartListProps) {
} = useChartEditModal(setCharts, charts);
const [importingChart, showImportModal] = useState<boolean>(false);
const [chartCurrentlyDeleting, setChartCurrentlyDeleting] =
useState<ChartDeleteState | null>(null);
// Monotonic token: a late pre-flight response for an earlier click must not
// swap the modal to a different chart (last-response-wins race).
const deleteModalRequestRef = useRef(0);
const [passwordFields, setPasswordFields] = useState<string[]>([]);
const [preparingExport, setPreparingExport] = useState<boolean>(false);
const [sshTunnelPasswordFields, setSSHTunnelPasswordFields] = useState<
@@ -274,6 +352,51 @@ function ChartList(props: ChartListProps) {
[addDangerToast],
);
const openChartDeleteModal = useCallback((chart: Chart) => {
deleteModalRequestRef.current += 1;
const requestToken = deleteModalRequestRef.current;
if (!isFeatureEnabled(FeatureFlag.AlertReports)) {
// The whole report API 404s when ALERT_REPORTS is off, while the delete
// guard still fires server-side. Skip the doomed request and open the
// unchanged modal immediately.
setChartCurrentlyDeleting({
chart,
blockingReports: [],
blockingReportsCount: 0,
});
return;
}
const queryParams = rison.encode({
filters: [{ col: 'chart_id', opr: 'eq', value: chart.id }],
columns: ['id', 'name', 'type'],
order_column: 'name',
order_direction: 'asc',
page_size: BLOCKING_REPORTS_PREVIEW_SIZE,
});
SupersetClient.get({ endpoint: `/api/v1/report/?q=${queryParams}` })
.then(({ json = {} }) => {
if (requestToken !== deleteModalRequestRef.current) return;
const blockingReports: BlockingReport[] = json.result ?? [];
setChartCurrentlyDeleting({
chart,
blockingReports,
blockingReportsCount: json.count ?? blockingReports.length,
});
})
.catch(() => {
if (requestToken !== deleteModalRequestRef.current) return;
// The report API can be visibility-filtered below what the delete
// guard sees, or fail outright. The list is advisory only, so every
// failure opens the unchanged modal rather than blocking the action;
// the confirm-time guard stays authoritative.
setChartCurrentlyDeleting({
chart,
blockingReports: [],
blockingReportsCount: 0,
});
});
}, []);
function handleBulkChartDelete(chartsToDelete: Chart[]) {
SupersetClient.delete({
endpoint: `/api/v1/chart/?q=${rison.encode(
@@ -528,13 +651,6 @@ function ChartList(props: ChartListProps) {
{
Cell: ({ row: { original } }: CellProps<Chart>) => {
const allowEdit = isUserEditorOrAdmin(user, original.editors);
const handleDelete = () =>
handleChartDelete(
original,
addSuccessToast,
addDangerToast,
refreshData,
);
const openEditModal = () => openChartEditModal(original);
const handleExport = () => handleBulkChartExport([original]);
if (!canEdit && !canDelete && !canExport) {
@@ -573,43 +689,21 @@ function ChartList(props: ChartListProps) {
/>
)}
{canDelete && (
<ConfirmStatusChange
recoverable={softDelete}
title={
softDelete
? t('Archive %(name)s?', { name: original.slice_name })
: t('Please confirm')
<ActionButton
label={deleteActionLabel()}
tooltip={
allowEdit
? deleteActionLabel()
: t(
'You must be a chart editor in order to delete. Please reach out to a chart editor to request modifications or edit access.',
)
}
description={
softDelete ? (
archiveConfirmDescription(t('chart'))
) : (
<>
{t('Are you sure you want to delete')}{' '}
<b>{original.slice_name}</b>?
</>
)
}
onConfirm={handleDelete}
>
{confirmDelete => (
<ActionButton
label={deleteActionLabel()}
tooltip={
allowEdit
? deleteActionLabel()
: t(
'You must be a chart editor in order to delete. Please reach out to a chart editor to request modifications or edit access.',
)
}
placement="bottom"
icon={<Icons.DeleteOutlined iconSize="l" />}
dataTest="chart-row-delete"
disabled={!allowEdit}
onClick={confirmDelete}
/>
)}
</ConfirmStatusChange>
placement="bottom"
icon={<Icons.DeleteOutlined iconSize="l" />}
dataTest="chart-row-delete"
disabled={!allowEdit}
onClick={() => openChartDeleteModal(original)}
/>
)}
</Actions>
);
@@ -634,11 +728,9 @@ function ChartList(props: ChartListProps) {
canExport,
saveFavoriteStatus,
favoriteStatus,
refreshData,
addSuccessToast,
addDangerToast,
handleBulkChartExport,
openChartEditModal,
openChartDeleteModal,
],
);
@@ -933,6 +1025,36 @@ function ChartList(props: ChartListProps) {
slice={sliceCurrentlyEditing}
/>
)}
{chartCurrentlyDeleting && (
<DeleteModal
recoverable={softDelete}
title={
softDelete
? t('Archive %(name)s?', {
name: chartCurrentlyDeleting.chart.slice_name,
})
: t('Please confirm')
}
name={chartCurrentlyDeleting.chart.slice_name}
open
description={
<ChartArchiveDescription
{...chartCurrentlyDeleting}
softDelete={softDelete}
/>
}
onConfirm={() => {
handleChartDelete(
chartCurrentlyDeleting.chart,
addSuccessToast,
addDangerToast,
refreshData,
);
setChartCurrentlyDeleting(null);
}}
onHide={() => setChartCurrentlyDeleting(null)}
/>
)}
<ConfirmStatusChange
recoverable={softDelete}
title={softDelete ? t('Archive selected charts?') : t('Please confirm')}
@@ -17,7 +17,7 @@
* under the License.
*/
import thunk from 'redux-thunk';
import configureStore from 'redux-mock-store';
import configureStore, { MockStoreEnhanced } from 'redux-mock-store';
import fetchMock from 'fetch-mock';
import {
render,
@@ -29,6 +29,7 @@ import { MemoryRouter, useLocation } from 'react-router-dom';
import { QueryParamProvider } from 'use-query-params';
import { ReactRouter5Adapter } from 'use-query-params/adapters/react-router-5';
import * as getBootstrapData from 'src/utils/getBootstrapData';
import { ADD_TOAST } from 'src/components/MessageToasts/actions';
import SavedQueryList from '.';
// Renders the current router pathname+search so tests can assert navigation.
@@ -92,8 +93,15 @@ fetchMock.post(permalinkEndpoint, {
fetchMock.delete(queryEndpoint, {}, { name: queryEndpoint });
const renderList = (props = {}, storeOverrides = {}) =>
render(
const renderList = (props = {}, storeOverrides = {}) => {
const store = configureStore([thunk])({
user: {
...mockUser,
roles: { Admin: [['can_write', 'SavedQuery']] },
},
...storeOverrides,
});
const utils = render(
<MemoryRouter>
<QueryParamProvider adapter={ReactRouter5Adapter}>
<SavedQueryList user={mockUser} {...props} />
@@ -102,15 +110,19 @@ const renderList = (props = {}, storeOverrides = {}) =>
</MemoryRouter>,
{
useRedux: true,
store: configureStore([thunk])({
user: {
...mockUser,
roles: { Admin: [['can_write', 'SavedQuery']] },
},
...storeOverrides,
}),
store,
},
);
return { ...utils, store };
};
// Finds any dispatched toast action whose text matches, regardless of
// toast type -- the regression this guards against could resurface the
// copy confirmation as any toast variant, not just a success toast.
const findToastAction = (store: MockStoreEnhanced<unknown>, text: string) =>
store
.getActions()
.find(action => action.type === ADD_TOAST && action.payload?.text === text);
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
describe('SavedQueryList', () => {
@@ -287,4 +299,113 @@ describe('SavedQueryList', () => {
applicationRootSpy.mockRestore();
}
});
test('opens a saved query in SQL Lab without copying a link', async () => {
// A prior test in this suite permanently swaps this route to read-only
// permissions, which would hide the edit action this test depends on.
fetchMock.removeRoute(queriesInfoEndpoint);
fetchMock.get(
queriesInfoEndpoint,
{ permissions: ['can_write', 'can_read', 'can_export'] },
{ name: queriesInfoEndpoint },
);
const clipboardCallback = jest.fn();
const originalClipboard = { ...global.navigator.clipboard };
// @ts-expect-error -- overriding a read-only browser API for the test
global.navigator.clipboard = {
write: clipboardCallback,
writeText: clipboardCallback,
};
try {
const { store } = renderList();
await screen.findByTestId('saved_query-list-view');
const editButtons = await screen.findAllByTestId('edit-action');
fireEvent.click(editButtons[0]);
await waitFor(() => {
const location = screen.getByTestId('location-display').textContent;
expect(location).toMatch(/^\/sqllab\?savedQueryId=\d+$/);
});
expect(clipboardCallback).not.toHaveBeenCalled();
expect(findToastAction(store, 'Link Copied!')).toBeUndefined();
} finally {
// @ts-expect-error -- restoring the read-only browser API after the test
global.navigator.clipboard = originalClipboard;
}
});
test('opens a saved query from the preview modal without copying a link', async () => {
const savedQueryDetailEndpoint = /\/api\/v1\/saved_query\/\d+$/;
fetchMock.get(
savedQueryDetailEndpoint,
{ result: mockQueries[0] },
{ name: 'saved-query-detail' },
);
const clipboardCallback = jest.fn();
const originalClipboard = { ...global.navigator.clipboard };
// @ts-expect-error -- overriding a read-only browser API for the test
global.navigator.clipboard = {
write: clipboardCallback,
writeText: clipboardCallback,
};
try {
const { store } = renderList();
await screen.findByTestId('saved_query-list-view');
const previewButtons = await screen.findAllByTestId('preview-action');
fireEvent.click(previewButtons[0]);
const openInSqlLabButton = await screen.findByTestId('open-in-sql-lab');
fireEvent.click(openInSqlLabButton);
await waitFor(() => {
const location = screen.getByTestId('location-display').textContent;
expect(location).toMatch(/^\/sqllab\?savedQueryId=\d+$/);
});
expect(clipboardCallback).not.toHaveBeenCalled();
expect(findToastAction(store, 'Link Copied!')).toBeUndefined();
} finally {
// @ts-expect-error -- restoring the read-only browser API after the test
global.navigator.clipboard = originalClipboard;
fetchMock.removeRoute('saved-query-detail');
}
});
test('copies a permalink to the clipboard when using the copy action', async () => {
const clipboardCallback = jest.fn();
const originalClipboard = { ...global.navigator.clipboard };
// @ts-expect-error -- overriding a read-only browser API for the test
global.navigator.clipboard = {
write: clipboardCallback,
writeText: clipboardCallback,
};
try {
const { store } = renderList();
await screen.findByTestId('saved_query-list-view');
const copyButtons = await screen.findAllByTestId('copy-action');
fireEvent.click(copyButtons[0]);
await waitFor(() => {
expect(clipboardCallback).toHaveBeenCalledWith(
'http://localhost/permalink',
);
});
await waitFor(() => {
expect(findToastAction(store, 'Link Copied!')).toBeDefined();
});
} finally {
// @ts-expect-error -- restoring the read-only browser API after the test
global.navigator.clipboard = originalClipboard;
}
});
});
@@ -61,12 +61,11 @@ import { QueryObjectColumns, SavedQueryObject } from 'src/views/CRUD/types';
import { TagTypeEnum } from 'src/components/Tag/TagType';
import { loadTags } from 'src/components/Tag/utils';
import { Icons } from '@superset-ui/core/components/Icons';
import copyTextToClipboard from 'src/utils/copy';
import type User from 'src/types/User';
import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import SavedQueryPreviewModal from 'src/features/queries/SavedQueryPreviewModal';
import { findPermission } from 'src/utils/findPermission';
import { getShareableUrl, openInNewTab } from 'src/utils/navigationUtils';
import { openInNewTab } from 'src/utils/navigationUtils';
const PAGE_SIZE = 25;
const PASSWORDS_NEEDED_MESSAGE = t(
@@ -245,13 +244,6 @@ function SavedQueryList({
// Action methods
const openInSqlLab = (id: number, openInNewWindow: boolean) => {
const path = `/sqllab?savedQueryId=${id}`;
copyTextToClipboard(() => Promise.resolve(getShareableUrl(path)))
.then(() => {
addSuccessToast(t('Link Copied!'));
})
.catch(() => {
addDangerToast(t('Sorry, your browser does not support copying.'));
});
if (openInNewWindow) {
openInNewTab(path);
} else {
@@ -263,6 +255,7 @@ function SavedQueryList({
const copyQueryLink = useCallback(
async (savedQuery: SavedQueryObject) => {
let permalink: string;
try {
const payload = {
dbId: savedQuery.db_id,
@@ -280,12 +273,19 @@ function SavedQueryList({
body: JSON.stringify(payload),
});
const { url: permalink } = response.json;
({ url: permalink } = response.json);
} catch (error) {
addDangerToast(t('There was an error generating the permalink.'));
return;
}
try {
await navigator.clipboard.writeText(permalink);
addSuccessToast(t('Link Copied!'));
} catch (error) {
addDangerToast(t('There was an error generating the permalink.'));
addDangerToast(
t('The link was generated but could not be copied: %s', permalink),
);
}
},
[addDangerToast, addSuccessToast],

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