Compare commits

...

25 Commits

Author SHA1 Message Date
Joe Li
cbbf7e4cd4 test(ci): stabilize master checks 2026-07-01 18:11:29 -07:00
Evan Rusackas
748060d35e feat(i18n): backfill Thai (th) translations (AI-generated, needs review) (#41641)
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 17:43:11 -07:00
Evan Rusackas
d57d69d3a6 feat(i18n): backfill Ukrainian (uk) translations (AI-generated, needs review) (#41645)
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 17:42:58 -07:00
Evan Rusackas
706ff94f0b feat(i18n): backfill Slovak (sk) translations (AI-generated, needs review) (#41640)
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 17:42:47 -07:00
Evan Rusackas
661d362580 feat(i18n): backfill new safe-link strings in de/lv/fi (AI-generated, needs review) (#41646)
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 17:42:38 -07:00
Evan Rusackas
6bdcb7a83a feat(i18n): backfill Czech (cs) translations (AI-generated, needs review) (#41647)
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 17:41:34 -07:00
Alejandro Solares
d1f7dd9c67 fix(deps): override fast-uri to 3.1.3 to fix CVE-2026-13676 (#41631)
Signed-off-by: Alejandro Solares <219859296+ASolarers-Rodriguez@users.noreply.github.com>
2026-07-01 17:28:45 -07:00
Evan Rusackas
c718f717cb feat(i18n): backfill Spanish (es) translations (AI-generated, needs review) (#41609)
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 17:25:13 -07:00
innovark
b3197c9b5e fix(table-chart): fix "Search by" control visibility and improve table controls layout (#36073)
Co-authored-by: SBIN2010 <Sbin2010@mail.ru>
2026-07-01 17:22:36 -07:00
Mike Bridge
af0a55a4f3 feat(dashboards): soft-delete and restore (#40128)
Co-authored-by: Mike Bridge <michael.bridge@ext.preset.io>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-07-01 17:20:44 -07:00
Evan Rusackas
8be255de40 chore(i18n): harden backfill_po — full language-name map + resilient batch translation (#41644)
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 16:28:19 -07:00
dependabot[bot]
f2e322c3c0 chore(deps-dev): bump sigstore from 4.1.0 to 4.1.1 in /superset-frontend (#41638)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-07-01 16:25:50 -07:00
Jean Massucatto
e58ce1cf39 fix(dashboard): pre-filter time grain for display controls (#40000)
Co-authored-by: Joe Li <joe@preset.io>
2026-07-01 16:10:52 -07:00
Mike Bridge
393adc4535 refactor(db): composite PK on M2M association tables (#39859)
Co-authored-by: Mike Bridge <michael.bridge@ext.preset.io>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-07-01 13:07:15 -07:00
Amin Ghadersohi
e0a3b1c10c fix(mcp): document select_columns valid fields and URL scheme for preview tools (#41595) 2026-07-01 15:58:12 -04:00
Evan Rusackas
6c57919647 chore(codeowners): update maintainer assignments (#41634)
Co-authored-by: Claude Code <noreply@anthropic.com>
2026-07-01 12:25:13 -07:00
Joe Li
bed1034c2f refactor(frontend): centralize subdirectory URL prefixing behind nav helpers (#39925)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Evan <evan@preset.io>
2026-07-01 11:20:13 -07:00
Mehmet Salih Yavuz
b7d5de8e52 fix(sqllab): truncate long tab names in the overflow ("...") dropdown (#41585) 2026-07-01 20:52:27 +03:00
Đỗ Trọng Hải
7d7c3ce723 feat(ci): install helm-docs directly instead of using whole brew setup (#41629) 2026-07-02 00:35:43 +07:00
Evan Rusackas
692f81d945 feat(i18n): backfill Finnish (fi) translations (AI-generated, needs review) (#41613)
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 10:27:28 -07:00
Evan Rusackas
eeacd9b6dd feat(i18n): backfill Latvian (lv) translations (AI-generated, needs review) (#41612)
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 10:27:07 -07:00
Evan Rusackas
b3c709b3d5 feat(i18n): backfill German (de) translations (AI-generated, needs review) (#41608)
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 10:24:20 -07:00
Nitish Agarwal
792d677634 fix(reports): respect CSV_EXPORT sep and decimal config in email reports (#38616) 2026-07-01 10:23:48 -07:00
Evan Rusackas
2d2a72b721 fix(sqllab): show non-ASCII text in array/JSON columns instead of \uXXXX escapes (#41533)
Co-authored-by: Vladislav Korenkov <73882772+Quatters@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 10:22:11 -07:00
Evan Rusackas
55b2da75f6 fix(echarts): allow forcing categorical x-axis for temporal columns (#41221)
Co-authored-by: Claude Code <noreply@anthropic.com>
2026-07-01 10:21:59 -07:00
269 changed files with 20994 additions and 4006 deletions

14
.github/CODEOWNERS vendored
View File

@@ -1,7 +1,5 @@
# Notify all committers of DB migration changes, per SIP-59
# https://github.com/apache/superset/issues/13351
/superset/migrations/ @mistercrunch @michael-s-molina @betodealmeida @eschutho @sadpandajoe
# Notify some committers of changes in the components
@@ -12,28 +10,30 @@
# Notify Helm Chart maintainers about changes in it
/helm/superset/ @craig-rueda @dpgaspar @villebro @nytai @michael-s-molina @mistercrunch @rusackas @Antonio-RiveroMartnez
/helm/superset/ @dpgaspar @villebro @nytai @michael-s-molina @mistercrunch @rusackas @Antonio-RiveroMartnez @hainenber
# Notify E2E test maintainers of changes
/superset-frontend/cypress-base/ @sadpandajoe @geido @eschutho @rusackas @betodealmeida @mistercrunch
/superset-frontend/playwright/ @sadpandajoe @geido @eschutho @rusackas @mistercrunch
/superset-frontend/cypress-base/ @sadpandajoe @geido @eschutho @rusackas @mistercrunch
# Notify PMC members of changes to GitHub Actions
/.github/ @villebro @geido @eschutho @rusackas @betodealmeida @nytai @mistercrunch @craig-rueda @kgabryje @dpgaspar @sadpandajoe @hainenber
/.github/ @villebro @geido @eschutho @rusackas @betodealmeida @nytai @mistercrunch @kgabryje @sha174n @dpgaspar @sadpandajoe @hainenber
# Notify PMC members of changes to CI-executed scripts (supply-chain risk:
# scripts/ files run directly in CI workflows and can execute arbitrary code)
/scripts/ @villebro @geido @eschutho @rusackas @betodealmeida @nytai @mistercrunch @craig-rueda @kgabryje @dpgaspar @sadpandajoe @hainenber
/scripts/ @villebro @geido @eschutho @rusackas @betodealmeida @nytai @mistercrunch @kgabryje @dpgaspar @sha174n @sadpandajoe @hainenber
# Notify PMC members of changes to required GitHub Actions
/.asf.yaml @villebro @geido @eschutho @rusackas @betodealmeida @nytai @mistercrunch @craig-rueda @kgabryje @dpgaspar @Antonio-RiveroMartnez
/.asf.yaml @villebro @geido @eschutho @rusackas @betodealmeida @nytai @mistercrunch @kgabryje @dpgaspar @sha174n @Antonio-RiveroMartnez
# Maps are a finicky contribution process we care about
**/*.geojson @villebro @rusackas
**/*.ipynb @villebro @rusackas
/superset-frontend/plugins/legacy-plugin-chart-country-map/ @villebro @rusackas
# Notify translation maintainers of changes to translations

View File

@@ -32,19 +32,18 @@ jobs:
with:
persist-credentials: false
submodules: recursive
- name: Setup Python
uses: ./.github/actions/setup-backend/
with:
python-version: ${{ matrix.python-version }}
- name: Enable brew and helm-docs
# Add brew to the path - see https://github.com/actions/runner-images/issues/6283
run: |
echo "/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin" >> $GITHUB_PATH
eval "$(/home/linuxbrew/.linuxbrew/bin/brew shellenv)"
echo "HOMEBREW_PREFIX=$HOMEBREW_PREFIX" >>"${GITHUB_ENV}"
echo "HOMEBREW_CELLAR=$HOMEBREW_CELLAR" >>"${GITHUB_ENV}"
echo "HOMEBREW_REPOSITORY=$HOMEBREW_REPOSITORY" >>"${GITHUB_ENV}"
brew install norwoodj/tap/helm-docs
- name: Setup Go
uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0
- name: Install helm-docs
run: go install github.com/norwoodj/helm-docs/cmd/helm-docs@v1.14.2
- name: Setup Node.js
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6
with:

View File

@@ -24,6 +24,24 @@ assists people when migrating to a new version.
## Next
- [39925](https://github.com/apache/superset/pull/39925): URL prefixing for `SUPERSET_APP_ROOT` subdirectory deployments is now handled automatically by helpers in `src/utils/navigationUtils` (`openInNewTab`, `redirect`, `getShareableUrl`, `<AppLink>`). Direct imports of `ensureAppRoot` / `makeUrl` from `src/utils/pathUtils` are forbidden outside `navigationUtils.ts` (enforced by a static-invariant test); contributors writing new code should use the focused helpers instead. No runtime behaviour change for existing callers — all 19 prior call sites have been migrated and four pre-existing double-prefix and missing-prefix bugs are fixed as part of the migration.
- [39925](https://github.com/apache/superset/pull/39925): `SupersetClient.getUrl()` now strips a single leading application-root segment from the supplied `endpoint` before building the request URL, so a caller that accidentally pre-prefixes its endpoint (for example by wrapping it with `ensureAppRoot` before passing it to the client) no longer produces a doubled `/superset/superset/...` URL under subdirectory deployment. The strip is **single-pass** — a genuine `/superset/superset/<slug>` route is preserved, not collapsed — and **silent** (no console warning); the static-invariant test remains the primary signal for pre-prefixing at the call site, and this runtime strip is a safety net beneath it. Code that intentionally targeted a literal `/<app_root>/<app_root>/...` endpoint through `getUrl` (a configuration that has no legitimate use under the prefixing model) would have its first redundant segment removed.
- **Breaking — `Superset` view class route prefix removed.** The `Superset` view in `superset/views/core.py` now declares `route_base = ""`, overriding Flask-AppBuilder's auto-derived `/superset` prefix. Routes that previously lived at `/superset/welcome/`, `/superset/dashboard/<id>/`, `/superset/dashboard/p/<key>/`, `/superset/explore/`, etc. now respond at `/welcome/`, `/dashboard/<id>/`, `/dashboard/p/<key>/`, `/explore/`, etc. Under subdirectory deployment (`SUPERSET_APP_ROOT=/superset`) the URLs are unchanged from end-user perspective — `AppRootMiddleware` re-applies the prefix via `SCRIPT_NAME`. Under root deployments, any external integration or bookmark that hard-codes `/superset/<endpoint>/` paths must be updated to drop the prefix. This fixes the doubled `/superset/superset/...` URLs that `url_for` emitted for these endpoints under subdirectory deployment and the related 404s on the routes themselves.
- **Breaking — Three sibling view classes route prefix removed.** Following the same rationale as the `Superset` class above, `ExplorePermalinkView` (`superset/views/explore.py`), `TagModelView`, and `TaggedObjectsModelView` (`superset/views/tags.py`, `superset/views/all_entities.py`) now mount at the application root rather than a hard-coded `/superset/...`. The user-visible URLs `/superset/explore/p/<key>/`, `/superset/tags/`, and `/superset/all_entities/` are unchanged under subdirectory deployment; under root deployments these views now serve `/explore/p/<key>/`, `/tags/`, and `/all_entities/`, so any external integration or bookmark must drop the `/superset/` prefix. `Dashboard.url` and `Dashboard.get_url` likewise return `/dashboard/<id>/` instead of the prior `/superset/dashboard/<id>/` literal so downstream consumers (DashboardList row hrefs, MCP service `dashboard_url`) emit a single, deployment-correct prefix.
- **Legacy `/superset/*` path support.** A new outermost WSGI middleware `LegacyPrefixRedirectMiddleware` (`superset/middleware/legacy_prefix_redirect.py`) 308-redirects every enumerated legacy `/superset/<canonical>` path to its post-`route_base=""` canonical location (e.g. `/superset/welcome/``/welcome/` under root; → `/superset/welcome/` under `SUPERSET_APP_ROOT=/superset`, because the canonical resolves through `AppRootMiddleware`). Bookmarks, email links, and external integrations survive the route-base collapse for one release cycle. POST against a GET-only canonical returns 410 Gone instead of 308 (308 would 405 on retry). The shim is removed at EOL `5.0.0`, matching the `@deprecated(eol_version="5.0.0")` gate on `Superset.explore` and `Superset.explore_json`.
- **PWA web app manifest served dynamically.** The PWA manifest is now served at `/pwa-manifest.json` (under `APPLICATION_ROOT`) by a new `PwaManifestView` (`superset/views/pwa_manifest.py`) instead of the static file at `/static/assets/pwa-manifest.json`. The legacy static source at `superset-frontend/src/pwa-manifest.json` has been removed (along with its `webpack.config.js` `CopyPlugin` rule). The new endpoint resolves `APPLICATION_ROOT` and `STATIC_ASSETS_PREFIX` at request time so PWA install works under subdirectory deployments and split static-prefix / app-root deployments (where `STATIC_ASSETS_PREFIX` points to a CDN host while the Superset backend stays under `APPLICATION_ROOT`). The `<link rel="manifest">` href in `superset/templates/superset/spa.html` was updated correspondingly (using a new `application_root_rstrip` template global). Operators with a forked `spa.html` should switch any manifest `<link>` to `{{ application_root_rstrip }}/pwa-manifest.json`.
- **Hard re-bookmark break — `/superset/sql/<database_id>/`.** SQL Lab moved to its own blueprint at `/sqllab/`. The legacy `/superset/sql/<id>/` shape changed to a query-string form (`/sqllab/?dbid=<id>`); no 1:1 path mapping exists, so `LegacyPrefixRedirectMiddleware` does **not** redirect this route — it passes through and surfaces a 404. Users with bookmarks to `/superset/sql/<id>/` must update them to `/sqllab/?dbid=<id>`.
- **`SqlaTable.sql_url` query-string format.** `SqlaTable.sql_url` now URL-encodes `table_name` and joins it as a query parameter rather than concatenating a second `?`. Previously, with `Database.sql_url` returning `/sqllab/?dbid=<id>`, the concatenation produced `/sqllab/?dbid=<id>?table_name=<raw>` — a malformed second `?` that broke the query parser. External code that parsed the legacy `<base>?table_name=<raw>` shape now sees properly percent-encoded values (e.g. `/``%2F`, ` ``+` or `%20`); decode with `urllib.parse.parse_qsl`.
- **New config flag `EMBEDDED_DISABLE_PERMALINK_ORIGIN_REWRITE` (default `False`).** Share/permalink URLs now substitute `window.location.origin` for the backend-supplied origin so a proxied or subdirectory-deployed Superset never hands the user an unreachable internal hostname. Operators whose reverse proxy correctly forwards `X-Forwarded-Host` *and* who want permalinks to carry the backend's literal origin can opt out by setting `EMBEDDED_DISABLE_PERMALINK_ORIGIN_REWRITE = True` in `superset_config.py`. Default `False` (rewrite is on); flipping the default would regress the dominant proxied/subdir deployment to an unreachable host.
### SQL Lab denies large-object and information_schema access by default
`DISALLOWED_SQL_FUNCTIONS` and `DISALLOWED_SQL_TABLES` now ship with additional default entries, so SQL Lab and chart-data queries that reference them are rejected where they were previously allowed:
@@ -271,6 +289,28 @@ Schedule the cutover in a quiet window. Runtime reads use only the single config
The migration is transactional (all-or-nothing) and idempotent — it can be safely re-run or resumed. Note that AES-GCM, unlike AES-CBC, does not support querying directly over encrypted columns; audit any code that filters on an encrypted column before switching. See the SIP at `docs/sip/authenticated-encryption-at-rest.md` for details.
### Soft delete and restore for dashboards
**Everything in this section applies only when the `SOFT_DELETE` feature flag is enabled. The flag defaults to `False`** (`@lifecycle: development`), so on a default deployment `DELETE /api/v1/dashboard/<id>` continues to **hard-delete permanently** — nothing is recoverable. Enable `SOFT_DELETE` to get the behavior described below.
**Flag-toggle caveat:** the soft-delete visibility filter is evaluated per query while the flag is on. If dashboards are soft-deleted during a flag-on window and the flag is later turned **off**, those rows reappear as live dashboards in all lists and lookups (including slug lookups — if a soft-deleted dashboard's slug was reused while the flag was on, both rows become visible with the same slug). The `POST /<uuid>/restore` endpoint and the `dashboard_deleted_state` list filter remain functional regardless of the flag, deliberately, so rows soft-deleted during a flag-on window stay discoverable and restorable after a rollback of the flag.
With the flag enabled: `DELETE /api/v1/dashboard/<id>` no longer hard-deletes the dashboard (the bulk-delete endpoint behaves the same way). The row is marked with a `deleted_at` timestamp and hidden from the dashboard API's list, detail, and lookup endpoints, which return 404 for soft-deleted dashboards. The embedded-dashboard iframe URL (`/embedded/<uuid>`) keeps rendering because it reads only `embedded.allowed_domains` and `embedded.dashboard_id` (the FK column) without dereferencing the parent dashboard; the frontend's subsequent dashboard-API fetch is what sees the 404 and surfaces "dashboard not found" to the user.
**New endpoint** — `POST /api/v1/dashboard/<uuid>/restore` clears `deleted_at` and returns the dashboard to active state. Requires `can_write on Dashboard` and ownership of the row (or admin). Soft-deleted dashboards can also be surfaced in the list endpoint via the new `dashboard_deleted_state` rison filter: `include` returns both live and soft-deleted rows, `only` returns just the soft-deleted ones. Any other value is ignored. For non-admin users, soft-deleted rows are limited to dashboards they own — the same audience that can restore them.
**Permissions migration:** existing role grants of `can_write on Dashboard` cover the new restore endpoint automatically; no role migration is required.
**Schema migration:** the migration adds a nullable `deleted_at` column and an index on it (`ix_dashboards_deleted_at`) to the `dashboards` table, and **replaces the full unique constraint on `slug`** with a partial unique index (`ix_dashboards_active_slug`) enforcing slug uniqueness only among active (non-soft-deleted) rows. The column add is instant. On Postgres the constraint swap briefly blocks reads and writes during `ALTER TABLE ... DROP CONSTRAINT` (acquires `ACCESS EXCLUSIVE`), then blocks writes only during `CREATE UNIQUE INDEX` (acquires `ShareLock`); reads pass through during the index build. Both windows are sub-second on a typical `dashboards` table. MySQL InnoDB builds the functional index online (no blocking).
**Rollback note:** the downgrade restores the original full unique constraint on `slug`. If the partial-index window allowed slug reuse (a soft-deleted row and an active row holding the same slug), `ALTER TABLE ... ADD CONSTRAINT idx_unique_slug UNIQUE (slug)` will abort with a unique-constraint violation. Before downgrading, hard-delete the soft-deleted duplicates (or rename one side) so each slug appears at most once across all rows. Rolling back the application code while leaving the new migration in place is also possible but exposes soft-deleted rows to the older code path; pair the rollback with a data decision (restore, hard-delete, or migrate-down).
The partial-index replacement is dialect-dependent: PostgreSQL uses a native `WHERE deleted_at IS NULL` partial index; MySQL 8.0.13+ uses a functional index over `(CASE WHEN deleted_at IS NULL THEN slug END)` (8.0.13 is the first release with functional key parts). **MySQL <8.0.13, MariaDB, and SQLite keep the original full unique constraint** (functional indexes / column-level UNIQUE recreation aren't supported cleanly — MariaDB is excluded even at 10.x because its `CASE`-expression index semantics differ), so on those backends a soft-deleted dashboard continues to reserve its slug for the lifetime of the row.
**Slug semantics:** on PostgreSQL and MySQL 8.0.13+, the slug of a soft-deleted dashboard is **free for reuse**. A new active dashboard can claim it immediately. Restoring a soft-deleted dashboard whose slug has since been claimed returns **422 with a clean error** (`DashboardSlugConflictError`) — rename one of the dashboards and retry; the restore is not silently rejected by a database-level constraint violation.
**Importer behavior:** importing a dashboard YAML whose UUID matches an existing **soft-deleted** dashboard is treated as an implicit restore-with-update — **and this happens even when `overwrite` is not set**. This is a deliberate asymmetry with active rows: an active dashboard imported without `overwrite=true` is returned unchanged (the import never mutates it), but a soft-deleted UUID match is restored *and* has the upload's contents applied regardless of the `overwrite` argument, on the reasoning that re-importing a deleted dashboard's exact UUID is an explicit request to bring it back. The restore preserves the original PK and all pre-deletion relationship rows (`dashboard_slices` junctions, role grants, owners, tags) — including role grants that were implicitly revoked by the deletion. Callers whose imports must never mutate existing state should treat bundles that may contain previously deleted UUIDs accordingly. The operation is permission-gated: it requires `can_write` and ownership of the deleted row (or admin) — non-owners get `ImportFailedError`, and callers without `can_write` get `ImportFailedError` instead of silently receiving the soft-deleted row.
### Granular Export Controls
A new feature flag `GRANULAR_EXPORT_CONTROLS` introduces three fine-grained permissions that replace the legacy `can_csv` permission:
@@ -560,6 +600,29 @@ See `superset/mcp_service/PRODUCTION.md` for deployment guides.
}
```
### Composite primary keys on many-to-many association tables
Eight M:N association tables move from a synthetic `id INTEGER PRIMARY KEY` to a composite `PRIMARY KEY (fk1, fk2)` on their two foreign-key columns. The surrogate `id` is dropped, and the redundant `UNIQUE (fk1, fk2)` on the two tables that carried one is removed (now subsumed by the PK).
| Table | Composite PK |
|---|---|
| `dashboard_roles` | `(dashboard_id, role_id)` |
| `dashboard_slices` | `(dashboard_id, slice_id)` |
| `dashboard_user` | `(user_id, dashboard_id)` |
| `report_schedule_user` | `(user_id, report_schedule_id)` |
| `rls_filter_roles` | `(role_id, rls_filter_id)` |
| `rls_filter_tables` | `(table_id, rls_filter_id)` |
| `slice_user` | `(user_id, slice_id)` |
| `sqlatable_user` | `(user_id, table_id)` |
**Before upgrading:**
- The migration **deletes** two classes of pre-existing rows the composite PK cannot accommodate: duplicate `(fk1, fk2)` pairs (it keeps the lowest `id` and removes the rest) and rows with `NULL` in either FK column. Both are meaningless for `secondary=` association tables, but export the affected rows first if you need an audit record.
- External tooling (BI tools, backup scripts) that references the surrogate `id` on these tables will break; no application code references it.
- Downgrade restores the `id` column (and the original `UNIQUE` on the two tables that had it) but leaves the FK columns `NOT NULL` (intentional — a `NULL` FK in a junction row is meaningless).
For large `dashboard_slices` / `report_schedule_user` tables, see the operator runbook in [#39859](https://github.com/apache/superset/pull/39859) — pre-flight inventory queries, per-dialect lock-window sizing, and the duplicate / NULL-FK roll-up — to plan the maintenance window.
## 6.0.0
- [33055](https://github.com/apache/superset/pull/33055): Upgrades Flask-AppBuilder to 5.0.0. The AUTH_OID authentication type has been deprecated and is no longer available as an option in Flask-AppBuilder. OpenID (OID) is considered a deprecated authentication protocol - if you are using AUTH_OID, you will need to migrate to an alternative authentication method such as OAuth, LDAP, or database authentication before upgrading.
- [34871](https://github.com/apache/superset/pull/34871): Fixed Jest test hanging issue from Ant Design v5 upgrade. MessageChannel is now mocked in test environment to prevent rc-overflow from causing Jest to hang. Test environment only - no production impact.

View File

@@ -91,22 +91,28 @@ _ASF_LICENSE_HEADER = """\
LANGUAGE_NAMES: dict[str, str] = {
"ar": "Arabic",
"ca": "Catalan",
"cs": "Czech",
"de": "German",
"es": "Spanish",
"fa": "Persian (Farsi)",
"fi": "Finnish",
"fr": "French",
"it": "Italian",
"ja": "Japanese",
"ko": "Korean",
"lv": "Latvian",
"mi": "Māori",
"nl": "Dutch",
"pl": "Polish",
"pt": "Portuguese",
"pt_BR": "Brazilian Portuguese",
"ro": "Romanian",
"ru": "Russian",
"sk": "Slovak",
"sl": "Slovenian",
"sr": "Serbian",
"sr_Latn": "Serbian (Latin script)",
"th": "Thai",
"tr": "Turkish",
"uk": "Ukrainian",
"zh": "Chinese (Simplified)",
@@ -346,6 +352,97 @@ def translate_batch(
return parse_response(result.stdout.strip(), len(batch))
def _translate_single_plaintext(
model: str,
target_lang: str,
item: dict[str, Any],
index: dict[str, Any],
) -> str | None:
"""Translate a single entry with a plain-text prompt (no JSON envelope).
Fallback for an entry whose JSON batch response cannot be parsed — typically
because the source string contains literal double-quotes that the model
echoes back unescaped, corrupting the surrounding JSON. Asking for a bare
string sidesteps the JSON contract entirely. Returns the translation text,
or None if the CLI call fails.
"""
claude_bin = shutil.which("claude")
if not claude_bin:
raise RuntimeError(
"claude CLI not found. Install Claude Code or add it to PATH."
)
lines = [
"You are a professional translator specializing in software UI strings.",
f"Translate the following English string into {_lang_name(target_lang)} "
f"({target_lang}).",
"Return ONLY the translation as plain text — no surrounding quotes, no "
"JSON, no markdown fences, no explanation.",
"Preserve all format placeholders exactly (%(name)s, {name}, %s, %d), any "
"HTML tags, and any inner quotation marks.",
"",
f"English: {item['msgid']}",
]
if item.get("msgid_plural"):
lines.append(f"English plural: {item['msgid_plural']}")
refs = index.get(item["index_key"], {})
ref_lines = [
f"{_lang_name(lang)}: {val}"
for lang, val in sorted(refs.items())
if lang != target_lang and isinstance(val, str) and val
]
if ref_lines:
lines.append("")
lines.append("Reference translations in other languages:")
lines.extend(ref_lines)
prompt = "\n".join(lines)
# claude_bin is resolved via shutil.which — not user-controlled input
result = subprocess.run( # noqa: S603
[claude_bin, "--model", model, "-p"],
input=prompt,
capture_output=True,
text=True,
check=False,
)
if result.returncode != 0:
return None
text = result.stdout.strip()
# Strip accidental markdown fences or wrapping quotes the model may add.
text = re.sub(r"^```[^\n]*\n?", "", text)
text = re.sub(r"\n?```$", "", text).strip()
if len(text) >= 2 and text[0] == '"' and text[-1] == '"':
text = text[1:-1]
return text or None
def _resilient_translate(
model: str,
target_lang: str,
batch: list[dict[str, Any]],
index: dict[str, Any],
) -> dict[int, str]:
"""Translate a batch, isolating entries that break the JSON response contract.
``translate_batch`` sends the whole batch in one request and parses a single
JSON object back. A source string containing literal double-quotes can make
the model emit unescaped quotes, so ``json.loads`` fails and the ENTIRE batch
would be lost. To salvage the rest, on a parse failure (ValueError) we bisect
the batch and recurse; a lone entry that still fails falls back to a
plain-text prompt via ``_translate_single_plaintext``. Returned keys are
positions within ``batch``. RuntimeError (CLI failure) is left to propagate
to the caller, preserving the existing per-batch failure handling.
"""
try:
return translate_batch(model, target_lang, batch, index)
except ValueError:
if len(batch) == 1:
text = _translate_single_plaintext(model, target_lang, batch[0], index)
return {0: text} if text else {}
mid = len(batch) // 2
left = _resilient_translate(model, target_lang, batch[:mid], index)
right = _resilient_translate(model, target_lang, batch[mid:], index)
return {**left, **{k + mid: v for k, v in right.items()}}
def _apply_plural_translation(entry: polib.POEntry, translation: str) -> None:
"""Distribute a model response across the entry's plural forms.
@@ -462,7 +559,7 @@ def _process_batches(
file=sys.stderr,
)
try:
translations = translate_batch(model, lang, batch_items, index)
translations = _resilient_translate(model, lang, batch_items, index)
except (ValueError, RuntimeError) as exc:
print(f" ERROR in batch starting at {batch_start}: {exc}", file=sys.stderr)
failed_count += len(batch_entries)

View File

@@ -19,9 +19,8 @@
export const DASHBOARD_LIST = '/dashboard/list/';
export const CHART_LIST = '/chart/list/';
export const WORLD_HEALTH_DASHBOARD = '/superset/dashboard/world_health/';
export const SAMPLE_DASHBOARD_1 = '/superset/dashboard/1-sample-dashboard/';
export const SUPPORTED_CHARTS_DASHBOARD =
'/superset/dashboard/supported_charts_dash/';
export const TABBED_DASHBOARD = '/superset/dashboard/tabbed_dash/';
export const WORLD_HEALTH_DASHBOARD = '/dashboard/world_health/';
export const SAMPLE_DASHBOARD_1 = '/dashboard/1-sample-dashboard/';
export const SUPPORTED_CHARTS_DASHBOARD = '/dashboard/supported_charts_dash/';
export const TABBED_DASHBOARD = '/dashboard/tabbed_dash/';
export const DATABASE_LIST = '/databaseview/list';

View File

@@ -8439,9 +8439,6 @@
"arm64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8459,9 +8456,6 @@
"arm64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8479,9 +8473,6 @@
"ppc64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8499,9 +8490,6 @@
"riscv64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8519,9 +8507,6 @@
"riscv64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8539,9 +8524,6 @@
"s390x"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8559,9 +8541,6 @@
"x64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8579,9 +8558,6 @@
"x64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@@ -9498,9 +9474,9 @@
}
},
"node_modules/@sigstore/core": {
"version": "3.2.0",
"resolved": "https://registry.npmjs.org/@sigstore/core/-/core-3.2.0.tgz",
"integrity": "sha512-kxHrDQ9YgfrWUSXU0cjsQGv8JykOFZQ9ErNKbFPWzk3Hgpwu8x2hHrQ9IdA8yl+j9RTLTC3sAF3Tdq1IQCP4oA==",
"version": "3.2.1",
"resolved": "https://registry.npmjs.org/@sigstore/core/-/core-3.2.1.tgz",
"integrity": "sha512-qRsxPnCrbC/puegGxKuynfnxgLiHqWStrSjxkoB4YKqq3Z3s4cyZyj42ZdWFAEblNP65C+rBH8EuREHIXoi83g==",
"dev": true,
"license": "Apache-2.0",
"engines": {
@@ -9666,14 +9642,14 @@
}
},
"node_modules/@sigstore/verify": {
"version": "3.1.0",
"resolved": "https://registry.npmjs.org/@sigstore/verify/-/verify-3.1.0.tgz",
"integrity": "sha512-mNe0Iigql08YupSOGv197YdHpPPr+EzDZmfCgMc7RPNaZTw5aLN01nBl6CHJOh3BGtnMIj83EeN4butBchc8Ag==",
"version": "3.1.1",
"resolved": "https://registry.npmjs.org/@sigstore/verify/-/verify-3.1.1.tgz",
"integrity": "sha512-qv7+G3J2cc6wwFj3yKvXOamzqhMwSk1ogPGmhpS8iXllcPrJaIIBA+4HbttlHVu1pqWTdmaCH/WE7UOC51kdoA==",
"dev": true,
"license": "Apache-2.0",
"dependencies": {
"@sigstore/bundle": "^4.0.0",
"@sigstore/core": "^3.1.0",
"@sigstore/core": "^3.2.1",
"@sigstore/protobuf-specs": "^0.5.0"
},
"engines": {
@@ -10651,9 +10627,6 @@
"cpu": [
"arm64"
],
"libc": [
"glibc"
],
"license": "Apache-2.0 AND MIT",
"optional": true,
"os": [
@@ -10670,9 +10643,6 @@
"cpu": [
"arm64"
],
"libc": [
"musl"
],
"license": "Apache-2.0 AND MIT",
"optional": true,
"os": [
@@ -10689,9 +10659,6 @@
"cpu": [
"ppc64"
],
"libc": [
"glibc"
],
"license": "Apache-2.0 AND MIT",
"optional": true,
"os": [
@@ -10708,9 +10675,6 @@
"cpu": [
"s390x"
],
"libc": [
"glibc"
],
"license": "Apache-2.0 AND MIT",
"optional": true,
"os": [
@@ -10727,9 +10691,6 @@
"cpu": [
"x64"
],
"libc": [
"glibc"
],
"license": "Apache-2.0 AND MIT",
"optional": true,
"os": [
@@ -10746,9 +10707,6 @@
"cpu": [
"x64"
],
"libc": [
"musl"
],
"license": "Apache-2.0 AND MIT",
"optional": true,
"os": [
@@ -20315,9 +20273,9 @@
"license": "MIT"
},
"node_modules/fast-uri": {
"version": "3.1.2",
"resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.2.tgz",
"integrity": "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ==",
"version": "3.1.3",
"resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.3.tgz",
"integrity": "sha512-i70LwGWUduXqzicKXWshooq+sWL1K3WUU5rKZNG/0i3a1OSoX3HqhH5WbWwTmqWfor4urUakGPiRQcleRZTwOg==",
"funding": [
{
"type": "github",
@@ -38475,18 +38433,18 @@
"license": "ISC"
},
"node_modules/sigstore": {
"version": "4.1.0",
"resolved": "https://registry.npmjs.org/sigstore/-/sigstore-4.1.0.tgz",
"integrity": "sha512-/fUgUhYghuLzVT/gaJoeVehLCgZiUxPCPMcyVNY0lIf/cTCz58K/WTI7PefDarXxp9nUKpEwg1yyz3eSBMTtgA==",
"version": "4.1.1",
"resolved": "https://registry.npmjs.org/sigstore/-/sigstore-4.1.1.tgz",
"integrity": "sha512-endqECJkfhozrXMK5ngu/UAA0xVcVEFdnHJCElGaExypjW+HK5i6zu3NteLoaX/iFbRUbC3+DjttQs0GARr+5w==",
"dev": true,
"license": "Apache-2.0",
"dependencies": {
"@sigstore/bundle": "^4.0.0",
"@sigstore/core": "^3.1.0",
"@sigstore/core": "^3.2.1",
"@sigstore/protobuf-specs": "^0.5.0",
"@sigstore/sign": "^4.1.0",
"@sigstore/tuf": "^4.0.1",
"@sigstore/verify": "^3.1.0"
"@sigstore/sign": "^4.1.1",
"@sigstore/tuf": "^4.0.2",
"@sigstore/verify": "^3.1.1"
},
"engines": {
"node": "^20.17.0 || >=22.9.0"

View File

@@ -413,6 +413,7 @@
"@luma.gl/shadertools": "~9.2.5",
"@luma.gl/webgl": "~9.2.5",
"fast-xml-parser": "^5.8.0",
"fast-uri": "^3.1.3",
"jest-mock": "^30.4.0",
"jest-runtime": "^30.4.0",
"@jest/globals": "^30.4.0",

View File

@@ -238,11 +238,16 @@ export const xAxisForceCategoricalControl = {
return state?.form_data?.x_axis_sort !== undefined || control.value;
},
renderTrigger: true,
// Expose the toggle for numeric and temporal x-axes. Temporal columns
// default to a continuous time scale, where ECharts places ticks at "nice"
// intervals that don't align with the actual buckets (e.g. weekly grain
// markers landing between month ticks). Treating the axis as categorical
// lets each bucket map to a discrete, tick-aligned category.
visibility: ({ controls }: { controls: ControlStateMapping }) =>
checkColumnType(
getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
controls?.datasource?.datasource,
[GenericDataType.Numeric],
[GenericDataType.Numeric, GenericDataType.Temporal],
),
shouldMapStateToProps: () => true,
},

View File

@@ -20,7 +20,11 @@
import { GenericDataType } from '@apache-superset/core/common';
import { xAxisForceCategoricalControl } from '../../src/shared-controls/customControls';
import { checkColumnType } from '../../src/utils/checkColumnType';
import type { ControlState } from '@superset-ui/chart-controls';
import type {
ControlPanelState,
ControlState,
ControlStateMapping,
} from '@superset-ui/chart-controls';
jest.mock('../../src/utils/checkColumnType');
jest.mock('@superset-ui/core', () => ({
@@ -39,12 +43,12 @@ test('xAxisForceCategoricalControl should not treat temporal columns as categori
controls: {
x_axis: { value: 'date_column' },
datasource: { datasource: {} },
},
};
} as unknown as ControlStateMapping,
} as unknown as ControlPanelState;
const result = xAxisForceCategoricalControl.config.initialValue!(
control,
state as any,
state,
);
// Verify: should return control value (false) for non-numeric columns
@@ -55,3 +59,27 @@ test('xAxisForceCategoricalControl should not treat temporal columns as categori
mockCheckColumnType.mockClear();
});
test('xAxisForceCategoricalControl is visible for numeric and temporal x-axes', () => {
const mockCheckColumnType = jest.mocked(checkColumnType);
mockCheckColumnType.mockReturnValue(true);
const controls = {
x_axis: { value: 'date_column' },
datasource: { datasource: {} },
} as unknown as ControlStateMapping;
const visible = xAxisForceCategoricalControl.config.visibility!({
controls,
});
expect(visible).toBe(true);
// Temporal columns must be included so the toggle is exposed for time-grain
// charts (e.g. weekly grain), where the time scale misaligns ticks/markers.
expect(mockCheckColumnType).toHaveBeenCalledWith('date_column', {}, [
GenericDataType.Numeric,
GenericDataType.Temporal,
]);
mockCheckColumnType.mockClear();
});

View File

@@ -109,7 +109,7 @@ export default class ChartClient {
(await buildQueryRegistry.get(visType)) ?? (() => formData);
const requestConfig: RequestConfig = useLegacyApi
? {
endpoint: '/superset/explore_json/',
endpoint: '/explore_json/',
postPayload: {
form_data: buildQuery(formData),
},
@@ -139,7 +139,7 @@ export default class ChartClient {
): Promise<Datasource> {
return this.client
.get({
endpoint: `/superset/fetch_datasource_metadata?datasourceKey=${datasourceKey}`,
endpoint: `/fetch_datasource_metadata?datasourceKey=${datasourceKey}`,
...options,
} as RequestConfig)
.then(response => response.json as Datasource);

View File

@@ -263,9 +263,7 @@ export default function StatefulChart(props: StatefulChartProps) {
if (!useLegacyApi && !queryContext.queries) {
queryContext = { queries: [queryContext] };
}
const endpoint = useLegacyApi
? '/superset/explore_json/'
: '/api/v1/chart/data';
const endpoint = useLegacyApi ? '/explore_json/' : '/api/v1/chart/data';
const requestConfig: RequestConfig = {
endpoint,

View File

@@ -82,7 +82,11 @@ export default class SupersetClientClass {
unauthorizedHandler = undefined,
}: ClientConfig = {}) {
const url = new URL(`${protocol || 'https:'}//${host || 'localhost'}`);
this.appRoot = appRoot;
// Strip a trailing slash so the getUrl dedupe comparisons and the final
// `${this.appRoot}/${...}` build stay correct regardless of how the root
// was supplied. Mirrors normalizeBackendUrlString / AppRootMiddleware /
// LegacyPrefixRedirectMiddleware, which all rstrip the root.
this.appRoot = appRoot.replace(/\/$/, '');
this.host = url.host;
this.protocol = url.protocol as Protocol;
this.headers = { Accept: 'application/json', ...headers }; // defaulting accept to json
@@ -296,8 +300,26 @@ export default class SupersetClientClass {
const host = inputHost ?? this.host;
const cleanHost = host.slice(-1) === '/' ? host.slice(0, -1) : host; // no backslash
// Strip a single leading appRoot segment so callers that accidentally
// pre-prefix their endpoint (e.g. by wrapping with ensureAppRoot before
// passing to the client) do not produce a doubled `/superset/superset/...`
// URL. Single-pass strip mirrors
// `stripAppRoot` in `src/utils/pathUtils` and `normalizeBackendUrlString`
// exactly: a genuine `/superset/superset/<slug>` is a legitimate route, not
// a double-prefix bug. The L2 static invariant still flags pre-prefixing as
// a migration issue; this is the runtime safety net.
let cleanEndpoint = endpoint;
const root = this.appRoot;
if (root) {
if (cleanEndpoint === root) {
cleanEndpoint = '';
} else if (cleanEndpoint.startsWith(`${root}/`)) {
cleanEndpoint = cleanEndpoint.slice(root.length);
}
}
return `${this.protocol}//${cleanHost}${this.appRoot}/${
endpoint[0] === '/' ? endpoint.slice(1) : endpoint
cleanEndpoint[0] === '/' ? cleanEndpoint.slice(1) : cleanEndpoint
}`;
}
}

View File

@@ -55,24 +55,25 @@ export default async function parseResponse<T extends ParseMethod = 'json'>(
if (parseMethod === 'json-bigint') {
const rawData = await response.text();
const json = JSONbig.parse(rawData);
const decoded = cloneDeepWith(json, (value: any) => {
if (
value?.isInteger?.() === true &&
(value?.isGreaterThan?.(Number.MAX_SAFE_INTEGER) ||
value?.isLessThan?.(Number.MIN_SAFE_INTEGER))
) {
// toFixed() avoids scientific notation, which BigInt() rejects.
return BigInt(value.toFixed());
}
// // `json-bigint` could not handle floats well, see sidorares/json-bigint#62
// // TODO: clean up after json-bigint>1.0.1 is released
if (value?.isNaN?.() === false) {
return value?.toNumber?.();
}
return undefined;
});
const result: JsonResponse = {
response,
json: cloneDeepWith(json, (value: any) => {
if (
value?.isInteger?.() === true &&
(value?.isGreaterThan?.(Number.MAX_SAFE_INTEGER) ||
value?.isLessThan?.(Number.MIN_SAFE_INTEGER))
) {
// toFixed() avoids scientific notation, which BigInt() rejects.
return BigInt(value.toFixed());
}
// // `json-bigint` could not handle floats well, see sidorares/json-bigint#62
// // TODO: clean up after json-bigint>1.0.1 is released
if (value?.isNaN?.() === false) {
return value?.toNumber?.();
}
return undefined;
}),
json: decoded,
};
return result as ReturnType;
}

View File

@@ -21,6 +21,9 @@ export { default as callApi } from './callApi';
export { default as SupersetClient } from './SupersetClient';
export { default as SupersetClientClass } from './SupersetClientClass';
export { normalizeBackendUrlString } from './normalizeBackendUrls';
export type { NormalizeOptions } from './normalizeBackendUrls';
export * from './types';
export * from './constants';
export { default as __hack_reexport_connection } from './types';

View File

@@ -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.
*/
/**
* Strips the configured application root from a single backend-supplied URL
* string so the frontend speaks router-relative paths. Apply it at the few call
* sites that surface a router-relative URL from an API response (e.g. a
* dataset's `explore_url`) before handing the value to a consumer that
* re-prefixes the root — `SupersetClient.getUrl`, `makeUrl`, or a react-router
* `<Link>` resolving against the Router `basename`. Without it those consumers
* would re-prefix an already-rooted path into `/superset/superset/...`.
*
* Absolute (`https:`, `ftp:`, `mailto:`, `tel:`) and protocol-relative (`//`)
* URLs pass through untouched, so an operator-configured external
* `default_endpoint` on a dataset is left alone.
*/
export interface NormalizeOptions {
/** Application root to strip. Empty string disables normalisation. */
applicationRoot: string;
}
const SAFE_ABSOLUTE_URL_RE = /^(?:https?|ftp|mailto|tel):/i;
function stripTrailingSlash(root: string): string {
return root.endsWith('/') ? root.slice(0, -1) : root;
}
/** Normalise a single router-relative URL string. */
export function normalizeBackendUrlString(
value: string,
options: NormalizeOptions,
): string {
const root = stripTrailingSlash(options.applicationRoot);
if (!root) return value;
if (SAFE_ABSOLUTE_URL_RE.test(value)) return value;
if (value.startsWith('//')) return value;
if (value === root) return '/';
if (value.startsWith(`${root}/`)) {
return value.slice(root.length);
}
return value;
}

View File

@@ -32,7 +32,7 @@ export default function getDatasourceMetadata({
}: Params) {
return client
.get({
endpoint: `/superset/fetch_datasource_metadata?datasourceKey=${datasourceKey}`,
endpoint: `/fetch_datasource_metadata?datasourceKey=${datasourceKey}`,
...requestConfig,
})
.then(response => response.json as Datasource);

View File

@@ -106,6 +106,7 @@ export type ChartCustomization = {
};
description?: string;
removed?: boolean;
time_grains?: string[];
};
export type ChartCustomizationDivider = Partial<

View File

@@ -283,6 +283,22 @@ test('lookalike OpenStreetMap hostnames do not receive OSM attribution', () => {
}
});
test('relative raster tile templates do not receive OSM attribution', () => {
// A host-relative template cannot be parsed by `new URL`, so the OSM
// hostname check must fall through to "not OSM" rather than throw.
const relativeTileUrl = '/local-tiles/{z}/{x}/{y}.png';
const style = resolveMapStyle(
`tile://${relativeTileUrl}`,
'default-style.json',
);
expect(typeof style).toBe('object');
if (typeof style !== 'string') {
expect(style.sources['osm-raster-tiles'].tiles).toEqual([relativeTileUrl]);
expect(style.sources['osm-raster-tiles']).not.toHaveProperty('attribution');
}
});
test('style JSON URLs pass through without raster wrapping', () => {
const styleUrl = 'https://example.com/styles/custom-style.json';

View File

@@ -88,6 +88,9 @@ type BootstrapData = {
};
export function getBootstrapDataFromDocument(): unknown {
/* istanbul ignore if -- a missing document only occurs in SSR/worker
contexts, which Jest cannot simulate: jsdom pins `document` as a
non-configurable global */
if (typeof document === 'undefined') {
return undefined;
}

View File

@@ -176,7 +176,9 @@ describe('ChartClient', () => {
Promise.reject(new Error('Unexpected all to v1 API')),
);
fetchMock.post('glob:*/superset/explore_json/', {
// post `Superset.route_base = ""`, the legacy endpoint
// collapsed from `/superset/explore_json/` to `/explore_json/`.
fetchMock.post('glob:*/explore_json/', {
field1: 'abc',
field2: 'def',
});
@@ -198,13 +200,10 @@ describe('ChartClient', () => {
describe('.loadDatasource(datasourceKey, options)', () => {
test('fetches datasource', () => {
fetchMock.get(
'glob:*/superset/fetch_datasource_metadata?datasourceKey=1__table',
{
field1: 'abc',
field2: 'def',
},
);
fetchMock.get('glob:*/fetch_datasource_metadata?datasourceKey=1__table', {
field1: 'abc',
field2: 'def',
});
return expect(chartClient.loadDatasource('1__table')).resolves.toEqual({
field1: 'abc',
@@ -264,13 +263,10 @@ describe('ChartClient', () => {
color: 'living-coral',
});
fetchMock.get(
'glob:*/superset/fetch_datasource_metadata?datasourceKey=1__table',
{
name: 'transactions',
schema: 'staging',
},
);
fetchMock.get('glob:*/fetch_datasource_metadata?datasourceKey=1__table', {
name: 'transactions',
schema: 'staging',
});
fetchMock.post('glob:*/api/v1/chart/data', {
lorem: 'ipsum',

View File

@@ -0,0 +1,113 @@
/**
* 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 { SupersetClientClass } from '@superset-ui/core';
// SupersetClient is expected to apply the configured appRoot exactly once.
// Callers must pass router-relative endpoints; pre-prefixing causes the
// double-prefix bug documented below.
describe('SupersetClient applies the application root exactly once', () => {
const buildClient = () =>
new SupersetClientClass({
protocol: 'https:',
host: 'config_host',
appRoot: '/superset',
});
test('endpoint without leading slash is concatenated correctly', () => {
expect(buildClient().getUrl({ endpoint: 'api/v1/chart' })).toBe(
'https://config_host/superset/api/v1/chart',
);
});
test('endpoint with leading slash is normalised to a single root segment', () => {
expect(buildClient().getUrl({ endpoint: '/api/v1/chart' })).toBe(
'https://config_host/superset/api/v1/chart',
);
});
// A trailing slash on the configured appRoot is stripped at construction
// (SupersetClientClass `appRoot.replace(/\/$/, '')`). Without it, a root of
// '/superset/' produced 'https://host/superset//foo', and the dedupe block's
// `startsWith('/superset//')` check silently failed to dedupe a pre-prefixed
// endpoint. This pins both behaviours against regression.
test('trailing-slash appRoot is normalised to a single root segment', () => {
const client = new SupersetClientClass({
protocol: 'https:',
host: 'config_host',
appRoot: '/superset/',
});
expect(client.getUrl({ endpoint: '/api/v1/chart' })).toBe(
'https://config_host/superset/api/v1/chart',
);
// and a pre-prefixed endpoint is still deduped, not doubled
expect(client.getUrl({ endpoint: '/superset/api/v1/chart' })).toBe(
'https://config_host/superset/api/v1/chart',
);
});
// Runtime safety net: if a caller pre-prefixes the endpoint (e.g. by wrapping
// with ensureAppRoot before calling), getUrl strips the duplicate. The L2
// static invariant still flags the pattern at the call site — this guards
// against the bug reaching production if the static check is bypassed.
test('dedupes a leading application-root segment from a pre-prefixed endpoint', () => {
expect(buildClient().getUrl({ endpoint: '/superset/api/v1/chart' })).toBe(
'https://config_host/superset/api/v1/chart',
);
});
// Single-pass strip preserves a legitimate `/superset/superset/<slug>`
// route. Backend-supplied router-relative URLs are stripped of the root at
// the call sites that surface them (via `normalizeBackendUrlString`) before
// any re-prefixing helper sees them, so a doubled leading segment reaching
// `getUrl` is a real route, not a double-prefix bug. This pin guards against
// silent regression to a greedy strip.
test('strips exactly one application-root segment (single-pass)', () => {
expect(
buildClient().getUrl({ endpoint: '/superset/superset/api/v1/chart' }),
).toBe('https://config_host/superset/superset/api/v1/chart');
expect(
buildClient().getUrl({
endpoint: '/superset/superset/superset/api/v1/chart',
}),
).toBe('https://config_host/superset/superset/superset/api/v1/chart');
});
test('dedupe is segment-boundary aware — `/supersetfoo` is not a prefix match', () => {
expect(buildClient().getUrl({ endpoint: '/supersetfoo/x' })).toBe(
'https://config_host/superset/supersetfoo/x',
);
});
test('dedupes the bare application root to an empty endpoint', () => {
expect(buildClient().getUrl({ endpoint: '/superset' })).toBe(
'https://config_host/superset/',
);
});
test('empty application root produces no prefix segment', () => {
const client = new SupersetClientClass({
protocol: 'https:',
host: 'config_host',
});
expect(client.getUrl({ endpoint: '/api/v1/chart' })).toBe(
'https://config_host/api/v1/chart',
);
});
});

View File

@@ -0,0 +1,89 @@
/**
* 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 { normalizeBackendUrlString } from '../../src/connection/normalizeBackendUrls';
const PREFIX = '/superset';
describe('normalizeBackendUrlString', () => {
test('strips application root from a router-relative path', () => {
expect(
normalizeBackendUrlString('/superset/explore/?slice_id=1', {
applicationRoot: PREFIX,
}),
).toBe('/explore/?slice_id=1');
});
test('strips a value that equals the application root exactly', () => {
expect(
normalizeBackendUrlString('/superset', { applicationRoot: PREFIX }),
).toBe('/');
});
test('tolerates a trailing slash on applicationRoot', () => {
expect(
normalizeBackendUrlString('/superset/foo', {
applicationRoot: '/superset/',
}),
).toBe('/foo');
});
// The negative cases below prove the helper is conservative: it doesn't
// mutate external URLs or path segments that merely share text with the root.
test('passes absolute URLs through unchanged', () => {
expect(
normalizeBackendUrlString('https://external.example.com/superset/foo', {
applicationRoot: PREFIX,
}),
).toBe('https://external.example.com/superset/foo');
});
test('passes protocol-relative URLs through unchanged', () => {
expect(
normalizeBackendUrlString('//cdn.example.com/superset/foo', {
applicationRoot: PREFIX,
}),
).toBe('//cdn.example.com/superset/foo');
});
test('does not strip a similar-but-different prefix segment', () => {
// /superset-public/... shares text with /superset but is a different path
// segment. Only /superset followed by / or end-of-string counts.
expect(
normalizeBackendUrlString('/superset-public/explore/?slice_id=1', {
applicationRoot: PREFIX,
}),
).toBe('/superset-public/explore/?slice_id=1');
});
test('is a no-op when application root is empty', () => {
expect(
normalizeBackendUrlString('/superset/explore/?slice_id=1', {
applicationRoot: '',
}),
).toBe('/superset/explore/?slice_id=1');
});
test('is idempotent: normalize(normalize(x)) === normalize(x)', () => {
const once = normalizeBackendUrlString('/superset/explore/?id=1', {
applicationRoot: PREFIX,
});
const twice = normalizeBackendUrlString(once, { applicationRoot: PREFIX });
expect(twice).toBe(once);
});
});

View File

@@ -35,8 +35,10 @@ describe('getFormData()', () => {
field2: 'def',
};
// post-`route_base=""`, the legacy endpoint collapsed
// from `/superset/fetch_datasource_metadata` to `/fetch_datasource_metadata`.
fetchMock.get(
'glob:*/superset/fetch_datasource_metadata?datasourceKey=1__table',
'glob:*/fetch_datasource_metadata?datasourceKey=1__table',
mockData,
);

View File

@@ -44,7 +44,7 @@ export class DashboardPage {
* @param slug - The dashboard slug (e.g., 'world_health')
*/
async gotoBySlug(slug: string): Promise<void> {
await gotoWithRetry(this.page, `superset/dashboard/${slug}/`);
await gotoWithRetry(this.page, `dashboard/${slug}/`);
}
/**
@@ -52,7 +52,7 @@ export class DashboardPage {
* @param id - The dashboard ID
*/
async gotoById(id: number): Promise<void> {
await gotoWithRetry(this.page, `superset/dashboard/${id}/`);
await gotoWithRetry(this.page, `dashboard/${id}/`);
}
/**

View File

@@ -29,6 +29,7 @@ import { waitForPost } from '../../helpers/api/intercepts';
import { expectStatusOneOf } from '../../helpers/api/assertions';
import { getDatabaseByName } from '../../helpers/api/database';
import { apiExecuteSql } from '../../helpers/api/sqllab';
import { TIMEOUT } from '../../utils/constants';
interface ExamplesSetupResult {
tableName: string;
@@ -116,7 +117,7 @@ async function dropTempTable(
// Uses test.describe only because Playwright's serial mode API requires it -
// (Deviation from "avoid describe" guideline is necessary for functional reasons)
test.describe('create dataset wizard', () => {
test.describe.configure({ mode: 'serial' });
test.describe.configure({ mode: 'serial', timeout: TIMEOUT.SLOW_TEST });
test('should create a dataset via wizard', async ({ page, testAssets }) => {
const { tableName, dbId, createDatasetPage } = await setupExamplesDataset(

View File

@@ -35,5 +35,5 @@ export const URL = {
LOGIN: 'login/',
SAVED_QUERIES_LIST: 'savedqueryview/list/',
SQLLAB: 'sqllab',
WELCOME: 'superset/welcome/',
WELCOME: 'welcome/',
} as const;

View File

@@ -1573,6 +1573,47 @@ test('xAxisForceCategorical forces Category axis regardless of Numeric coltype',
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.
// Forcing categorical maps each bucket to a discrete, tick-aligned category
// while still formatting the labels as dates rather than raw timestamps.
const ts1 = 1745784000000;
const ts2 = 1745870400000;
const chartProps = createTestChartProps({
formData: {
metrics: ['metric'],
granularity_sqla: 'ds',
x_axis: '__timestamp',
xAxisForceCategorical: true,
},
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 {
type: string;
axisLabel: { formatter: (v: Date) => string };
};
expect(xAxis.type).toBe(AxisType.Category);
const label = xAxis.axisLabel.formatter(new Date(ts1));
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
expect(label).not.toBe(String(ts1));
});
test('temporal x coltype wires the time formatter and Time axis', () => {
// Regression guard: the happy path for time-series charts. Ensures that
// Temporal coltype keeps routing through the TimeFormatter so a refactor

View File

@@ -56,6 +56,7 @@ import { PAGE_SIZE_OPTIONS } from '../consts';
import { sortAlphanumericCaseInsensitive } from './utils/sortAlphanumericCaseInsensitive';
import { SearchOption, SortByItem } from '../types';
import SearchSelectDropdown from './components/SearchSelectDropdown';
import { SupersetTheme, css } from '@apache-superset/core/theme';
export interface DataTableProps<D extends object> extends TableOptions<D> {
tableClassName?: string;
@@ -561,6 +562,9 @@ export default typedMemo(function DataTable<D extends object>({
align="center"
justify="space-between"
gap="middle"
css={(theme: SupersetTheme) => css`
font-size: ${theme.fontSizeSM}px;
`}
>
{hasPagination ? (
<SelectPageSize
@@ -576,30 +580,32 @@ export default typedMemo(function DataTable<D extends object>({
/>
) : null}
<Flex wrap align="center" gap="middle">
{serverPagination && searchInput && (
<Space size="small" className="search-select-container">
<span className="search-by-label">{t('Search by')}:</span>
<SearchSelectDropdown
searchOptions={searchOptions}
value={serverPaginationData?.searchColumn || ''}
onChange={onSearchColChange}
/>
</Space>
)}
{searchInput && (
<GlobalFilter<D>
searchInput={
typeof searchInput === 'boolean' ? undefined : searchInput
}
preGlobalFilteredRows={preGlobalFilteredRows}
setGlobalFilter={
manualSearch ? handleSearchChange : setGlobalFilter
}
filterValue={manualSearch ? initialSearchText : filterValue}
id={searchInputId}
serverPagination={!!serverPagination}
rowCount={rowCount}
/>
<>
{serverPagination && (
<Space direction="vertical" size={4}>
{t('Search by')}
<SearchSelectDropdown
searchOptions={searchOptions}
value={serverPaginationData?.searchColumn || ''}
onChange={onSearchColChange}
/>
</Space>
)}
<GlobalFilter<D>
searchInput={
typeof searchInput === 'boolean' ? undefined : searchInput
}
preGlobalFilteredRows={preGlobalFilteredRows}
setGlobalFilter={
manualSearch ? handleSearchChange : setGlobalFilter
}
filterValue={manualSearch ? initialSearchText : filterValue}
id={searchInputId}
serverPagination={!!serverPagination}
rowCount={rowCount}
/>
</>
)}
{renderTimeComparisonDropdown
? renderTimeComparisonDropdown()

View File

@@ -17,14 +17,9 @@
* under the License.
*/
/* eslint-disable import/no-extraneous-dependencies */
import { styled } from '@apache-superset/core/theme';
import { RawAntdSelect } from '@superset-ui/core/components';
import { RawAntdSelect as Select } from '@superset-ui/core/components';
import { SearchOption } from '../../types';
const StyledSelect = styled(RawAntdSelect)`
width: 120px;
margin-right: 8px;
`;
import { SupersetTheme, css } from '@apache-superset/core/theme';
interface SearchSelectDropdownProps {
/** The currently selected search column value */
@@ -41,10 +36,14 @@ function SearchSelectDropdown({
searchOptions,
}: SearchSelectDropdownProps) {
return (
<StyledSelect
<Select
className="search-select"
value={value || (searchOptions?.[0]?.value ?? '')}
css={(theme: SupersetTheme) => css`
width: ${theme.sizeUnit * 30}px;
`}
value={value ?? searchOptions?.[0]?.value}
options={searchOptions}
size="small"
onChange={onChange}
/>
);

View File

@@ -58,7 +58,7 @@ import {
useTheme,
SupersetTheme,
} from '@apache-superset/core/theme';
import { t, tn } from '@apache-superset/core/translation';
import { t } from '@apache-superset/core/translation';
import { GenericDataType } from '@apache-superset/core/common';
import {
Input,
@@ -254,12 +254,13 @@ function SearchInput({
inputRef,
}: SearchInputProps) {
return (
<Space direction="horizontal" size={4} className="dt-global-filter">
{t('Search')}
<Space direction="vertical" size={4} className="dt-global-filter">
<span aria-hidden="true">{t('Search')}</span>
<Input
aria-label={t('Search %s records', count)}
placeholder={tn('%s record', '%s records...', count, count)}
aria-label={t('Search records')}
placeholder={t('Search records')}
value={value}
size="small"
onChange={onChange}
onBlur={onBlur}
ref={inputRef}
@@ -276,18 +277,18 @@ function SelectPageSize({
const { Option } = Select;
return (
<span className="dt-select-page-size">
<Space direction="vertical" size={4} className="dt-select-page-size">
<VisuallyHidden htmlFor="pageSizeSelect">
{t('Select page size')}
</VisuallyHidden>
{t('Show')}{' '}
{t('Entries per page')}
<Select<number>
id="pageSizeSelect"
value={current}
onChange={value => onChange(value)}
size="small"
css={(theme: SupersetTheme) => css`
width: ${theme.sizeUnit * 18}px;
width: ${theme.sizeUnit * 30}px;
`}
aria-label={t('Show entries per page')}
>
@@ -301,9 +302,8 @@ function SelectPageSize({
</Option>
);
})}
</Select>{' '}
{t('entries per page')}
</span>
</Select>
</Space>
);
}

View File

@@ -2106,7 +2106,7 @@ describe('plugin-chart-table', () => {
await waitFor(() => {
expect(screen.getByRole('textbox')).toHaveValue('Michael');
expect(screen.getByLabelText('Search 0 records')).toHaveValue(
expect(screen.getByLabelText('Search records')).toHaveValue(
'Michael',
);
});

View File

@@ -0,0 +1,183 @@
/**
* 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 { readdirSync, readFileSync, statSync } from 'fs';
import { join, relative, resolve, sep } from 'path';
const DEFAULT_ROOTS = ['src', 'packages/superset-ui-core/src'];
const ALWAYS_SKIP_SEGMENTS = new Set([
'node_modules',
'dist',
'build',
'coverage',
'__mocks__',
'cypress-base',
'playwright',
]);
const ALWAYS_SKIP_SUFFIXES = [
'.test.ts',
'.test.tsx',
'.stories.ts',
'.stories.tsx',
];
const SOURCE_EXTENSIONS = ['.ts', '.tsx'];
export interface ScanOptions {
/** Workspace-relative directories to scan. Defaults to the source tree. */
roots?: string[];
/** Extra path segments to skip on top of {@link ALWAYS_SKIP_SEGMENTS}. */
ignoreSegments?: string[];
/** Regex run against each line of each file. */
pattern: RegExp;
/** Workspace-relative paths (forward slashes) exempt from this scan. */
allowlist?: string[];
}
export interface ScanHit {
/** Workspace-relative path with forward slashes. */
file: string;
/** 1-based line number. */
line: number;
/** The text of the matching line, trimmed. */
text: string;
/** The substring captured by `pattern`. */
match: string;
}
// __dirname resolves to <workspace>/spec/helpers regardless of cwd.
const WORKSPACE_ROOT = resolve(__dirname, '..', '..');
function isSourceFile(name: string): boolean {
return (
SOURCE_EXTENSIONS.some(ext => name.endsWith(ext)) &&
!ALWAYS_SKIP_SUFFIXES.some(suffix => name.endsWith(suffix))
);
}
function walk(directory: string, ignoreSegments: Set<string>): string[] {
const found: string[] = [];
let entries;
try {
entries = readdirSync(directory, { withFileTypes: true });
} catch {
return found;
}
for (const entry of entries) {
if (ignoreSegments.has(entry.name)) continue;
const absolute = join(directory, entry.name);
if (entry.isDirectory()) {
found.push(...walk(absolute, ignoreSegments));
} else if (entry.isFile() && isSourceFile(entry.name)) {
found.push(absolute);
}
}
return found;
}
function toForwardSlashes(path: string): string {
return sep === '/' ? path : path.split(sep).join('/');
}
/**
* Line-by-line regex scan over the source tree. Returns one {@link ScanHit}
* per matching line. Textual (not AST-based) — false positives on string
* literals should be fixed by tightening the regex.
*/
export function scanSource(options: ScanOptions): ScanHit[] {
const {
roots = DEFAULT_ROOTS,
ignoreSegments = [],
pattern,
allowlist = [],
} = options;
const ignoreSet = new Set([...ALWAYS_SKIP_SEGMENTS, ...ignoreSegments]);
const allowSet = new Set(allowlist);
const hits: ScanHit[] = [];
const seen = new Set<string>();
for (const root of roots) {
const absoluteRoot = resolve(WORKSPACE_ROOT, root);
let stat;
try {
stat = statSync(absoluteRoot);
} catch {
continue;
}
if (!stat.isDirectory()) continue;
for (const absoluteFile of walk(absoluteRoot, ignoreSet)) {
if (seen.has(absoluteFile)) continue;
seen.add(absoluteFile);
const relativePath = toForwardSlashes(
relative(WORKSPACE_ROOT, absoluteFile),
);
if (allowSet.has(relativePath)) continue;
const contents = readFileSync(absoluteFile, 'utf8');
const lines = contents.split('\n');
// Reuse the regex per file. Without the `g` flag, `.exec` ignores
// lastIndex, so recompiling per-line was wasted allocation.
const lineRegex = pattern.flags.includes('g')
? new RegExp(pattern.source, pattern.flags.replace('g', ''))
: pattern;
for (let index = 0; index < lines.length; index += 1) {
const lineText = lines[index];
const match = lineRegex.exec(lineText);
if (match) {
hits.push({
file: relativePath,
line: index + 1,
text: lineText.trim(),
match: match[0],
});
}
}
}
}
return hits;
}
/** Format hits as a multi-line failure message: ` file:line — text`. */
export function formatHits(hits: ScanHit[], header: string): string {
if (hits.length === 0) return header;
const lines = hits
.slice(0, 50)
.map(hit => ` ${hit.file}:${hit.line}${hit.text}`);
const overflow =
hits.length > 50 ? `\n ... and ${hits.length - 50} more` : '';
return `${header}\n${lines.join('\n')}${overflow}`;
}
/** Throw with a formatted message if `hits` is non-empty. */
export function expectNoHits(hits: ScanHit[], header: string): void {
if (hits.length > 0) {
throw new Error(formatHits(hits, header));
}
}

View File

@@ -0,0 +1,53 @@
/**
* 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.
*/
/**
* Run `callback` with `getBootstrapData().common.application_root` set to
* `applicationRoot`. Resets modules so any imports inside the callback see
* the configured value, then restores the prior DOM and module cache on exit.
* Pass `''` to simulate the default root-of-domain deployment.
*/
export async function withApplicationRoot<T>(
applicationRoot: string,
callback: () => Promise<T> | T,
): Promise<T> {
const previousBody = document.body.innerHTML;
try {
const bootstrapData = { common: { application_root: applicationRoot } };
document.body.innerHTML = `<div id="app" data-bootstrap='${JSON.stringify(bootstrapData)}'></div>`;
jest.resetModules();
await import('src/utils/getBootstrapData');
return await callback();
} finally {
document.body.innerHTML = previousBody;
jest.resetModules();
}
}
/** Run `body` once per scenario, each under a different application root. */
export async function applicationRootScenarios<S extends { root: string }>(
scenarios: S[],
body: (scenario: S) => Promise<void> | void,
): Promise<void> {
for (const scenario of scenarios) {
// eslint-disable-next-line no-await-in-loop -- intentional: scenarios share document state.
await withApplicationRoot(scenario.root, () => body(scenario));
}
}

View File

@@ -20,6 +20,11 @@
import { Global } from '@emotion/react';
import { css } from '@apache-superset/core/theme';
// Class applied to the SQL Lab tab bar's overflow ("...") dropdown so its menu
// items truncate long tab names. The dropdown is portaled to the body, outside
// the tabs' emotion scope, so it is styled here via a global rule.
export const SQLLAB_TAB_OVERFLOW_POPUP_CLASS = 'sqllab-tab-overflow-popup';
export const SqlLabGlobalStyles = () => (
<Global
styles={theme => css`
@@ -30,6 +35,31 @@ export const SqlLabGlobalStyles = () => (
); // Set a min height so the gutter is always visible when resizing
overflow: hidden;
}
// The tab label is a flex node (icon menu + title + status icon). antd's
// overflow dropdown styles each menu item for a plain-text label, so the
// nested flex defeats its ellipsis and very long names render blank. Cap
// the item width and let the title truncate inside it.
.${SQLLAB_TAB_OVERFLOW_POPUP_CLASS} {
.ant-tabs-dropdown-menu-item {
max-width: ${theme.sizeUnit * 80}px;
}
.ant-tabs-dropdown-menu-item > span {
min-width: 0;
overflow: hidden;
}
[data-test='sql-editor-tab-header'] {
min-width: 0;
width: 100%;
}
[data-test='sql-editor-tab-title'] {
flex: 1;
min-width: 0;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
}
}
`}
/>
);

View File

@@ -44,7 +44,7 @@ import {
import { fDuration, extendedDayjs } from '@superset-ui/core/utils/dates';
import { SqlLabRootState } from 'src/SqlLab/types';
import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
import { makeUrl } from 'src/utils/pathUtils';
import { openInNewTab } from 'src/utils/navigationUtils';
import ResultSet from '../ResultSet';
import HighlightedSql from '../HighlightedSql';
import { StaticPosition, StyledTooltip, ModalResultSetWrapper } from './styles';
@@ -80,8 +80,7 @@ interface QueryTableProps {
}
const openQuery = (id: number) => {
const url = makeUrl(`/sqllab?queryId=${id}`);
window.open(url);
openInNewTab(`/sqllab?queryId=${id}`);
};
const QueryTable = ({

View File

@@ -53,7 +53,28 @@ jest.mock('@superset-ui/core', () => ({
isFeatureEnabled: jest.fn().mockReturnValue(false),
}));
// Mock openInNewTab so the Create-chart "new window" branch can be asserted
// without spawning a real window. The rest of navigationUtils stays real so
// existing CSV-download tests keep using the genuine `redirect`/`makeUrl`.
jest.mock('src/utils/navigationUtils', () => ({
...jest.requireActual('src/utils/navigationUtils'),
openInNewTab: jest.fn(),
}));
// eslint-disable-next-line import/order, import/first
import { openInNewTab } from 'src/utils/navigationUtils';
// Stub postFormData so the Create-chart click resolves quickly; this lets
// the test focus on the URL composition that happens after the resolve.
jest.mock('src/explore/exploreUtils/formData', () => ({
...jest.requireActual('src/explore/exploreUtils/formData'),
postFormData: jest.fn(),
}));
// eslint-disable-next-line import/order, import/first
import { postFormData } from 'src/explore/exploreUtils/formData';
const mockIsFeatureEnabled = isFeatureEnabled as jest.Mock;
const mockOpenInNewTab = openInNewTab as jest.Mock;
const mockPostFormData = postFormData as jest.Mock;
jest.mock('src/components/ErrorMessage', () => ({
ErrorMessageWithStackTrace: () => <div data-test="error-message">Error</div>,
@@ -160,6 +181,9 @@ describe('ResultSet', () => {
beforeEach(() => {
applicationRootMock.mockReturnValue('');
mockStartExport.mockClear();
mockOpenInNewTab.mockClear();
mockPostFormData.mockReset();
mockPostFormData.mockResolvedValue('test-form-data-key');
});
// Add cleanup after each test
@@ -1009,4 +1033,103 @@ describe('ResultSet', () => {
screen.getByRole('button', { name: 'Results Action' }),
).toBeInTheDocument();
});
test('Create chart in new window opens single-prefixed explore URL under subdirectory deployment', async () => {
// When the user metaKey-clicks "Create chart", the SQL-Lab result handoff
// composes an explore URL via mountExploreUrl(..., includeAppRoot=true).
// Under SUPERSET_APP_ROOT=/superset, the resulting URL must contain the
// prefix exactly once. A doubled prefix (/superset/superset/explore/…)
// produces a blank Explore page.
const appRoot = '/superset';
applicationRootMock.mockReturnValue(appRoot);
const queryWithId = {
...queries[0],
results: {
...queries[0].results,
query_id: 42,
},
};
const { getByTestId } = setup(
{
...mockedProps,
queryId: queryWithId.id,
database: { allows_subquery: true, allows_virtual_table_explore: true },
},
mockStore({
...initialState,
user,
sqlLab: {
...initialState.sqlLab,
queries: {
[queryWithId.id]: queryWithId,
},
},
}),
);
const exploreButton = await waitFor(() =>
getByTestId('explore-results-button'),
);
fireEvent.click(exploreButton, { metaKey: true });
await waitFor(() => {
expect(mockOpenInNewTab).toHaveBeenCalledTimes(1);
});
const url = mockOpenInNewTab.mock.calls[0][0] as string;
expect(url).toMatch(/^\/superset\/explore\/\?.*form_data_key=/);
expect(url).not.toMatch(/\/superset\/superset\//);
});
test('Create chart in same window pushes router-relative explore URL under subdirectory deployment', async () => {
// Same-tab click (no metaKey) goes through history.push under the SPA
// basename Router, so mountExploreUrl is called with includeAppRoot=false.
// The composed URL must NOT carry an app-root prefix — the router applies
// it once via <Router basename={applicationRoot()}>. A premature prefix
// here would compound with the basename and yield /superset/superset/…
const appRoot = '/superset';
applicationRootMock.mockReturnValue(appRoot);
const queryWithId = {
...queries[0],
results: {
...queries[0].results,
query_id: 99,
},
};
const store = mockStore({
...initialState,
user,
sqlLab: {
...initialState.sqlLab,
queries: {
[queryWithId.id]: queryWithId,
},
},
});
const { getByTestId } = render(
<ResultSet
{...mockedProps}
queryId={queryWithId.id}
database={{
allows_subquery: true,
allows_virtual_table_explore: true,
}}
/>,
{ useRedux: true, store, useRouter: true },
);
const exploreButton = await waitFor(() =>
getByTestId('explore-results-button'),
);
fireEvent.click(exploreButton);
await waitFor(() => {
expect(mockPostFormData).toHaveBeenCalledTimes(1);
});
expect(mockOpenInNewTab).not.toHaveBeenCalled();
});
});

View File

@@ -16,7 +16,6 @@
* specific language governing permissions and limitations
* under the License.
*/
import { sanitizeUrl } from '@braintree/sanitize-url';
import {
useCallback,
useEffect,
@@ -88,7 +87,7 @@ import { usePermissions } from 'src/hooks/usePermissions';
import { StreamingExportModal } from 'src/components/StreamingExportModal';
import { useStreamingExport } from 'src/components/StreamingExportModal/useStreamingExport';
import { useConfirmModal } from 'src/hooks/useConfirmModal';
import { makeUrl } from 'src/utils/pathUtils';
import { makeUrl, openInNewTab, redirect } from 'src/utils/navigationUtils';
import ExploreCtasResultsButton from '../ExploreCtasResultsButton';
import ExploreResultsButton from '../ExploreResultsButton';
import HighlightedSql from '../HighlightedSql';
@@ -312,7 +311,9 @@ const ResultSet = ({
includeAppRoot,
);
if (openInNewWindow) {
window.open(url, '_blank', 'noreferrer');
// `url` is from `mountExploreUrl(..., includeAppRoot=true)`; the
// helper re-applies `ensureAppRoot` idempotently.
openInNewTab(url);
} else {
history.push(url);
}
@@ -379,7 +380,13 @@ const ResultSet = ({
{ rows: rowsCount.toLocaleString() },
),
onConfirm: () => {
window.location.href = sanitizeUrl(getExportCsvUrl(query.id));
// `getExportCsvUrl` already runs the path through `makeUrl`;
// `redirect` re-applies `ensureAppRoot` idempotently and routes
// the sink through navigationUtils' barriers (scheme allowlist,
// userinfo rejection, backslash rejection), which is a
// strict superset of what `sanitizeUrl` from master PR #40546
// provides.
redirect(getExportCsvUrl(query.id));
},
confirmText: t('OK'),
cancelText: t('Close'),

View File

@@ -16,7 +16,6 @@
* specific language governing permissions and limitations
* under the License.
*/
import { sanitizeUrl } from '@braintree/sanitize-url';
import { useCallback, useState, FormEvent } from 'react';
import { ModalTitleWithIcon } from 'src/components/ModalTitleWithIcon';
import { Radio, RadioChangeEvent } from '@superset-ui/core/components/Radio';
@@ -58,6 +57,7 @@ import { postFormData } from 'src/explore/exploreUtils/formData';
import { URL_PARAMS } from 'src/constants';
import { isEmpty } from 'lodash-es';
import { clearDatasetCache } from 'src/utils/cachedSupersetGet';
import { openInNewTab, redirect } from 'src/utils/navigationUtils';
interface QueryDatabase {
id?: number;
@@ -244,10 +244,16 @@ export const SaveDatasetModal = ({
useState(false);
const createWindow = (url: string) => {
// `url` is from `mountExploreUrl(..., includeAppRoot=true)`; the
// navigationUtils helpers re-apply `ensureAppRoot` idempotently.
if (openWindow) {
window.open(sanitizeUrl(url), '_blank', 'noreferrer');
// `openInNewTab` / `redirect` route the sink through navigationUtils'
// barriers (scheme allowlist, userinfo rejection, backslash
// rejection) — strictly stronger than master PR #40546's `sanitizeUrl`
// wrap, which only rejects `javascript:` / `data:` / `vbscript:`.
openInNewTab(url);
} else {
window.location.href = sanitizeUrl(url);
redirect(url);
}
};
const formDataWithDefaults = {

View File

@@ -55,306 +55,303 @@ const setup = (queryEditor: QueryEditor, store?: Store) =>
...(store && { store }),
});
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
describe('SqlEditorTabHeader', () => {
test('renders name', () => {
const { queryByText } = setup(defaultQueryEditor, mockStore(initialState));
expect(queryByText(defaultQueryEditor.name)).toBeInTheDocument();
expect(queryByText(extraQueryEditor1.name)).not.toBeInTheDocument();
expect(queryByText(extraQueryEditor2.name)).not.toBeInTheDocument();
});
// Renders the header and opens its "..." dropdown menu, returning the store so
// each test can assert on the actions it dispatches.
const openTabDropdown = () => {
const store = mockStore(initialState);
const { getByTestId } = setup(defaultQueryEditor, store);
userEvent.click(getByTestId('dropdown-trigger'));
return store;
};
test('renders name from unsaved changes', () => {
const expectedTitle = 'updated title';
const { queryByText } = setup(
defaultQueryEditor,
mockStore({
...initialState,
sqlLab: {
...initialState.sqlLab,
unsavedQueryEditor: {
id: defaultQueryEditor.id,
name: expectedTitle,
},
},
}),
);
expect(queryByText(expectedTitle)).toBeInTheDocument();
expect(queryByText(defaultQueryEditor.name)).not.toBeInTheDocument();
expect(queryByText(extraQueryEditor1.name)).not.toBeInTheDocument();
expect(queryByText(extraQueryEditor2.name)).not.toBeInTheDocument();
});
test('renders current name for unrelated unsaved changes', () => {
const unrelatedTitle = 'updated title';
const { queryByText } = setup(
defaultQueryEditor,
mockStore({
...initialState,
sqlLab: {
...initialState.sqlLab,
unsavedQueryEditor: {
id: `${defaultQueryEditor.id}-other`,
name: unrelatedTitle,
},
},
}),
);
expect(queryByText(defaultQueryEditor.name)).toBeInTheDocument();
expect(queryByText(unrelatedTitle)).not.toBeInTheDocument();
expect(queryByText(extraQueryEditor1.name)).not.toBeInTheDocument();
expect(queryByText(extraQueryEditor2.name)).not.toBeInTheDocument();
});
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
describe('with dropdown menus', () => {
let store = mockStore();
beforeEach(async () => {
store = mockStore(initialState);
const { getByTestId } = setup(defaultQueryEditor, store);
const dropdown = getByTestId('dropdown-trigger');
userEvent.click(dropdown);
});
test('should dispatch removeQueryEditor action', async () => {
await waitFor(() =>
expect(screen.getByTestId('close-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('close-tab-menu-option'));
const actions = store.getActions();
await waitFor(() =>
expect(actions[0]).toEqual({
type: REMOVE_QUERY_EDITOR,
queryEditor: defaultQueryEditor,
}),
);
});
test('should dispatch queryEditorSetTitle action', async () => {
await waitFor(() =>
expect(
screen.getByTestId('rename-tab-menu-option'),
).toBeInTheDocument(),
);
const expectedTitle = 'typed text';
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
fireEvent.change(input, { target: { value: expectedTitle } });
fireEvent.click(screen.getByRole('button', { name: 'Save' }));
const actions = store.getActions();
await waitFor(() =>
expect(actions[0]).toEqual({
type: QUERY_EDITOR_SET_TITLE,
name: expectedTitle,
queryEditor: expect.objectContaining({
id: defaultQueryEditor.id,
}),
}),
);
});
test('prefills the rename input with the current tab name', async () => {
await waitFor(() =>
expect(
screen.getByTestId('rename-tab-menu-option'),
).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
expect(input).toHaveValue(defaultQueryEditor.name);
});
test('focuses the rename input when the modal opens', async () => {
await waitFor(() =>
expect(
screen.getByTestId('rename-tab-menu-option'),
).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
await waitFor(() => expect(input).toHaveFocus());
});
test('disables Save when the input is empty or whitespace', async () => {
await waitFor(() =>
expect(
screen.getByTestId('rename-tab-menu-option'),
).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
fireEvent.change(input, { target: { value: ' ' } });
expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled();
});
test('does not dispatch or dismiss on Enter when the input is empty', async () => {
await waitFor(() =>
expect(
screen.getByTestId('rename-tab-menu-option'),
).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
fireEvent.change(input, { target: { value: ' ' } });
fireEvent.keyDown(input, { key: 'Enter', keyCode: 13, charCode: 13 });
const dispatchedTitleChange = store
.getActions()
.some(action => action.type === QUERY_EDITOR_SET_TITLE);
expect(dispatchedTitleChange).toBe(false);
// the modal must stay open so the user can correct the name,
// mirroring the disabled Save button rather than dismissing like Escape
expect(screen.queryByRole('dialog')).toBeInTheDocument();
});
test('does not dispatch a title change when the modal is cancelled', async () => {
await waitFor(() =>
expect(
screen.getByTestId('rename-tab-menu-option'),
).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
fireEvent.change(input, { target: { value: 'discarded text' } });
fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
expect(store.getActions()).toEqual([]);
});
test('does not dispatch a title change when dismissed with the close button', async () => {
await waitFor(() =>
expect(
screen.getByTestId('rename-tab-menu-option'),
).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
fireEvent.change(input, { target: { value: 'discarded text' } });
fireEvent.click(screen.getByTestId('close-modal-btn'));
expect(store.getActions()).toEqual([]);
});
test('returns focus to the tab header after the modal is cancelled', async () => {
await waitFor(() =>
expect(
screen.getByTestId('rename-tab-menu-option'),
).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
await screen.findByTestId('rename-tab-input');
fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
await waitFor(() =>
expect(screen.getByTestId('sql-editor-tab-header')).toHaveFocus(),
);
});
test('returns focus to the tab header after a successful rename', async () => {
await waitFor(() =>
expect(
screen.getByTestId('rename-tab-menu-option'),
).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
fireEvent.change(input, { target: { value: 'renamed tab' } });
fireEvent.click(screen.getByRole('button', { name: 'Save' }));
await waitFor(() =>
expect(screen.getByTestId('sql-editor-tab-header')).toHaveFocus(),
);
});
test('should dispatch removeAllOtherQueryEditors action', async () => {
await waitFor(() =>
expect(screen.getByTestId('close-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('close-all-other-menu-option'));
const actions = store.getActions();
await waitFor(() =>
expect(actions).toEqual([
{
type: REMOVE_QUERY_EDITOR,
queryEditor: initialState.sqlLab.queryEditors[1],
},
{
type: REMOVE_QUERY_EDITOR,
queryEditor: initialState.sqlLab.queryEditors[2],
},
]),
);
});
test('should dispatch cloneQueryToNewTab action', async () => {
await waitFor(() =>
expect(screen.getByTestId('close-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('clone-tab-menu-option'));
const actions = store.getActions();
await waitFor(() =>
expect(actions[0]).toEqual({
type: ADD_QUERY_EDITOR,
queryEditor: expect.objectContaining({
name: `Copy of ${defaultQueryEditor.name}`,
sql: defaultQueryEditor.sql,
autorun: false,
}),
}),
);
});
});
test('does not leak tab-editing keystrokes from the rename input to the surrounding tabs', async () => {
const onContainerKeyDown = jest.fn();
const store = mockStore(initialState);
render(
<div onKeyDown={onContainerKeyDown}>
<SqlEditorTabHeader queryEditor={defaultQueryEditor} />
</div>,
{ useRedux: true, store },
);
userEvent.click(screen.getByTestId('dropdown-trigger'));
await waitFor(() =>
expect(screen.getByTestId('rename-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
// The modal portals over the editable-card tabs, whose keyboard handler would
// otherwise remove, navigate, or activate a tab (and swallow Space). None of
// these keys should escape the modal to the surrounding container.
[
'Delete',
'Backspace',
'ArrowLeft',
'ArrowRight',
'Home',
'End',
' ',
].forEach(key => fireEvent.keyDown(input, { key }));
expect(onContainerKeyDown).not.toHaveBeenCalled();
// Escape (close) and Tab (focus trap) must still reach the Modal.
fireEvent.keyDown(input, { key: 'Tab' });
fireEvent.keyDown(input, { key: 'Escape' });
const reached = onContainerKeyDown.mock.calls.map(call => call[0].key);
expect(reached).toEqual(expect.arrayContaining(['Tab', 'Escape']));
});
test('renders name', () => {
const { queryByText } = setup(defaultQueryEditor, mockStore(initialState));
expect(queryByText(defaultQueryEditor.name)).toBeInTheDocument();
expect(queryByText(extraQueryEditor1.name)).not.toBeInTheDocument();
expect(queryByText(extraQueryEditor2.name)).not.toBeInTheDocument();
});
test('exposes the name on a dedicated node the overflow dropdown can truncate', () => {
// The overflow ("...") menu reuses this label and styles the title node by
// its data-test to keep very long names from rendering blank.
const { getByTestId } = setup(defaultQueryEditor, mockStore(initialState));
expect(getByTestId('sql-editor-tab-title')).toHaveTextContent(
defaultQueryEditor.name,
);
});
test('renders name from unsaved changes', () => {
const expectedTitle = 'updated title';
const { queryByText } = setup(
defaultQueryEditor,
mockStore({
...initialState,
sqlLab: {
...initialState.sqlLab,
unsavedQueryEditor: {
id: defaultQueryEditor.id,
name: expectedTitle,
},
},
}),
);
expect(queryByText(expectedTitle)).toBeInTheDocument();
expect(queryByText(defaultQueryEditor.name)).not.toBeInTheDocument();
expect(queryByText(extraQueryEditor1.name)).not.toBeInTheDocument();
expect(queryByText(extraQueryEditor2.name)).not.toBeInTheDocument();
});
test('renders current name for unrelated unsaved changes', () => {
const unrelatedTitle = 'updated title';
const { queryByText } = setup(
defaultQueryEditor,
mockStore({
...initialState,
sqlLab: {
...initialState.sqlLab,
unsavedQueryEditor: {
id: `${defaultQueryEditor.id}-other`,
name: unrelatedTitle,
},
},
}),
);
expect(queryByText(defaultQueryEditor.name)).toBeInTheDocument();
expect(queryByText(unrelatedTitle)).not.toBeInTheDocument();
expect(queryByText(extraQueryEditor1.name)).not.toBeInTheDocument();
expect(queryByText(extraQueryEditor2.name)).not.toBeInTheDocument();
});
test('should dispatch removeQueryEditor action', async () => {
const store = openTabDropdown();
await waitFor(() =>
expect(screen.getByTestId('close-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('close-tab-menu-option'));
const actions = store.getActions();
await waitFor(() =>
expect(actions[0]).toEqual({
type: REMOVE_QUERY_EDITOR,
queryEditor: defaultQueryEditor,
}),
);
});
test('should dispatch queryEditorSetTitle action', async () => {
const store = openTabDropdown();
await waitFor(() =>
expect(screen.getByTestId('rename-tab-menu-option')).toBeInTheDocument(),
);
const expectedTitle = 'typed text';
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
fireEvent.change(input, { target: { value: expectedTitle } });
fireEvent.click(screen.getByRole('button', { name: 'Save' }));
const actions = store.getActions();
await waitFor(() =>
expect(actions[0]).toEqual({
type: QUERY_EDITOR_SET_TITLE,
name: expectedTitle,
queryEditor: expect.objectContaining({
id: defaultQueryEditor.id,
}),
}),
);
});
test('prefills the rename input with the current tab name', async () => {
openTabDropdown();
await waitFor(() =>
expect(screen.getByTestId('rename-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
expect(input).toHaveValue(defaultQueryEditor.name);
});
test('focuses the rename input when the modal opens', async () => {
openTabDropdown();
await waitFor(() =>
expect(screen.getByTestId('rename-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
await waitFor(() => expect(input).toHaveFocus());
});
test('disables Save when the input is empty or whitespace', async () => {
openTabDropdown();
await waitFor(() =>
expect(screen.getByTestId('rename-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
fireEvent.change(input, { target: { value: ' ' } });
expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled();
});
test('does not dispatch or dismiss on Enter when the input is empty', async () => {
const store = openTabDropdown();
await waitFor(() =>
expect(screen.getByTestId('rename-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
fireEvent.change(input, { target: { value: ' ' } });
fireEvent.keyDown(input, { key: 'Enter', keyCode: 13, charCode: 13 });
const dispatchedTitleChange = store
.getActions()
.some(action => action.type === QUERY_EDITOR_SET_TITLE);
expect(dispatchedTitleChange).toBe(false);
// the modal must stay open so the user can correct the name,
// mirroring the disabled Save button rather than dismissing like Escape
expect(screen.queryByRole('dialog')).toBeInTheDocument();
});
test('does not dispatch a title change when the modal is cancelled', async () => {
const store = openTabDropdown();
await waitFor(() =>
expect(screen.getByTestId('rename-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
fireEvent.change(input, { target: { value: 'discarded text' } });
fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
expect(store.getActions()).toEqual([]);
});
test('does not dispatch a title change when dismissed with the close button', async () => {
const store = openTabDropdown();
await waitFor(() =>
expect(screen.getByTestId('rename-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
fireEvent.change(input, { target: { value: 'discarded text' } });
fireEvent.click(screen.getByTestId('close-modal-btn'));
expect(store.getActions()).toEqual([]);
});
test('returns focus to the tab header after the modal is cancelled', async () => {
openTabDropdown();
await waitFor(() =>
expect(screen.getByTestId('rename-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
await screen.findByTestId('rename-tab-input');
fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
await waitFor(() =>
expect(screen.getByTestId('sql-editor-tab-header')).toHaveFocus(),
);
});
test('returns focus to the tab header after a successful rename', async () => {
openTabDropdown();
await waitFor(() =>
expect(screen.getByTestId('rename-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
fireEvent.change(input, { target: { value: 'renamed tab' } });
fireEvent.click(screen.getByRole('button', { name: 'Save' }));
await waitFor(() =>
expect(screen.getByTestId('sql-editor-tab-header')).toHaveFocus(),
);
});
test('should dispatch removeAllOtherQueryEditors action', async () => {
const store = openTabDropdown();
await waitFor(() =>
expect(screen.getByTestId('close-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('close-all-other-menu-option'));
const actions = store.getActions();
await waitFor(() =>
expect(actions).toEqual([
{
type: REMOVE_QUERY_EDITOR,
queryEditor: initialState.sqlLab.queryEditors[1],
},
{
type: REMOVE_QUERY_EDITOR,
queryEditor: initialState.sqlLab.queryEditors[2],
},
]),
);
});
test('should dispatch cloneQueryToNewTab action', async () => {
const store = openTabDropdown();
await waitFor(() =>
expect(screen.getByTestId('close-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('clone-tab-menu-option'));
const actions = store.getActions();
await waitFor(() =>
expect(actions[0]).toEqual({
type: ADD_QUERY_EDITOR,
queryEditor: expect.objectContaining({
name: `Copy of ${defaultQueryEditor.name}`,
sql: defaultQueryEditor.sql,
autorun: false,
}),
}),
);
});
test('does not leak tab-editing keystrokes from the rename input to the surrounding tabs', async () => {
const onContainerKeyDown = jest.fn();
const store = mockStore(initialState);
render(
<div onKeyDown={onContainerKeyDown}>
<SqlEditorTabHeader queryEditor={defaultQueryEditor} />
</div>,
{ useRedux: true, store },
);
userEvent.click(screen.getByTestId('dropdown-trigger'));
await waitFor(() =>
expect(screen.getByTestId('rename-tab-menu-option')).toBeInTheDocument(),
);
fireEvent.click(screen.getByTestId('rename-tab-menu-option'));
const input = await screen.findByTestId('rename-tab-input');
// The modal portals over the editable-card tabs, whose keyboard handler would
// otherwise remove, navigate, or activate a tab (and swallow Space). None of
// these keys should escape the modal to the surrounding container.
[
'Delete',
'Backspace',
'ArrowLeft',
'ArrowRight',
'Home',
'End',
' ',
].forEach(key => fireEvent.keyDown(input, { key }));
expect(onContainerKeyDown).not.toHaveBeenCalled();
// Escape (close) and Tab (focus trap) must still reach the Modal.
fireEvent.keyDown(input, { key: 'Tab' });
fireEvent.keyDown(input, { key: 'Escape' });
const reached = onContainerKeyDown.mock.calls.map(call => call[0].key);
expect(reached).toEqual(expect.arrayContaining(['Tab', 'Escape']));
});

View File

@@ -245,7 +245,7 @@ const SqlEditorTabHeader: FC<Props> = ({ queryEditor }) => {
/>
}
/>
<TabTitle>{qe.name}</TabTitle>{' '}
<TabTitle data-test="sql-editor-tab-title">{qe.name}</TabTitle>{' '}
<StatusIcon
className="status-icon"
iconSize="m"

View File

@@ -29,6 +29,7 @@ import { ErrorBoundary } from 'src/components/ErrorBoundary';
import { detectOS } from 'src/utils/common';
import * as Actions from 'src/SqlLab/actions/sqlLab';
import { Icons } from '@superset-ui/core/components/Icons';
import { SQLLAB_TAB_OVERFLOW_POPUP_CLASS } from 'src/SqlLab/SqlLabGlobalStyles';
import SqlEditor from '../SqlEditor';
import SqlEditorTabHeader from '../SqlEditorTabHeader';
@@ -262,6 +263,7 @@ function TabbedSqlEditors({
hideAdd={offline}
onTabClick={onTabClicked}
onEdit={handleEdit}
popupClassName={SQLLAB_TAB_OVERFLOW_POPUP_CLASS}
type={queryEditors?.length === 0 ? 'card' : 'editable-card'}
addIcon={
<Tooltip

View File

@@ -0,0 +1,153 @@
/**
* 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 { ChartCustomization, ChartCustomizationType } from '@superset-ui/core';
import { getInitialDataMask } from 'src/dataMask/reducer';
import type { ChartCustomizationsFormItem } from 'src/dashboard/components/nativeFilters/FiltersConfigModal/types';
import { transformCustomizationForSave } from 'src/dashboard/components/nativeFilters/FiltersConfigModal/transformers';
const createTimeGrainCustomizationInput = (
timeGrains?: string[],
): Omit<ChartCustomizationsFormItem, 'type'> => ({
scope: {
rootPath: ['ROOT_ID'],
excluded: [],
},
name: 'Time Grain Display Control',
filterType: 'chart_customization_timegrain',
dataset: {
value: 10,
label: 'main.dataset',
},
column: 'dttm',
controlValues: {},
requiredFirst: {},
defaultValue: null,
defaultDataMask: getInitialDataMask(),
sortMetric: null,
time_grains: timeGrains,
description: '',
});
test('transformCustomizationForSave persists time_grains when a subset is selected', () => {
const transformed = transformCustomizationForSave(
'CHART_CUSTOMIZATION-subset',
createTimeGrainCustomizationInput([
'PT1H',
'P1D',
'P1W',
]) as ChartCustomizationsFormItem,
);
expect(transformed).toBeDefined();
expect(transformed?.type).toBe(ChartCustomizationType.ChartCustomization);
expect(transformed && 'time_grains' in transformed).toBe(true);
expect(
transformed && 'time_grains' in transformed
? transformed.time_grains
: undefined,
).toEqual(['PT1H', 'P1D', 'P1W']);
});
test('transformCustomizationForSave omits time_grains from API payload when all are selected', () => {
const transformed = transformCustomizationForSave(
'CHART_CUSTOMIZATION-all',
createTimeGrainCustomizationInput(undefined) as ChartCustomizationsFormItem,
);
expect(transformed).toBeDefined();
expect(
transformed && 'time_grains' in transformed
? transformed.time_grains
: undefined,
).toBeUndefined();
// API boundary: undefined keys are omitted from JSON payloads.
const serialized = JSON.parse(JSON.stringify(transformed));
expect(serialized).not.toHaveProperty('time_grains');
});
test('transformCustomizationForSave remains backward compatible when time_grains is missing', () => {
const formInput = createTimeGrainCustomizationInput();
delete (formInput as Partial<ChartCustomizationsFormItem>).time_grains;
const transformed = transformCustomizationForSave(
'CHART_CUSTOMIZATION-legacy',
formInput as ChartCustomizationsFormItem,
);
expect(transformed).toBeDefined();
expect(
transformed && 'time_grains' in transformed
? transformed.time_grains
: undefined,
).toBeUndefined();
const serialized = JSON.parse(JSON.stringify(transformed));
expect(serialized).not.toHaveProperty('time_grains');
});
test('transformCustomizationForSave omits time_grains when an empty array is provided', () => {
const transformed = transformCustomizationForSave(
'CHART_CUSTOMIZATION-empty-array',
createTimeGrainCustomizationInput([]) as ChartCustomizationsFormItem,
);
expect(transformed).toBeDefined();
expect(
transformed && 'time_grains' in transformed
? transformed.time_grains
: undefined,
).toBeUndefined();
// API boundary: empty allowlist should behave like unrestricted and be omitted.
const serialized = JSON.parse(JSON.stringify(transformed));
expect(serialized).not.toHaveProperty('time_grains');
});
test('transformCustomizationForSave preserves saved time_grains when reloading an existing customization', () => {
// Simulates the edit round-trip: a previously saved ChartCustomization is
// passed back through the transformer. The allowlist must survive intact so
// the edit modal can restore it.
const savedCustomization: ChartCustomization = {
id: 'CHART_CUSTOMIZATION-existing',
type: ChartCustomizationType.ChartCustomization,
name: 'Time Grain Display Control',
filterType: 'chart_customization_timegrain',
description: '',
targets: [{ datasetId: 10, column: { name: 'dttm' } }],
scope: { rootPath: ['ROOT_ID'], excluded: [] },
controlValues: {},
defaultDataMask: getInitialDataMask(),
removed: false,
time_grains: ['PT1H', 'P1D'],
};
const transformed = transformCustomizationForSave(
savedCustomization.id,
savedCustomization,
);
expect(transformed).toBeDefined();
expect(
transformed && 'time_grains' in transformed
? transformed.time_grains
: undefined,
).toEqual(['PT1H', 'P1D']);
});

View File

@@ -97,11 +97,6 @@ export default function PluginFilterTimegrain(
handleChange(filterState.value ?? []);
}, [JSON.stringify(filterState.value)]);
const placeholderText =
(data || []).length === 0
? t('No data')
: tn('%s option', '%s options', data.length, data.length);
const formItemData: FormItemProps = {};
if (filterState.validateMessage) {
formItemData.extra = (
@@ -111,15 +106,27 @@ export default function PluginFilterTimegrain(
);
}
const options = (data || []).map(
(row: { name: string; duration: string }) => {
const options = (data || [])
.map((row: { name: string; duration: string }) => {
const { name, duration } = row;
return {
label: name,
value: duration,
};
},
);
})
// Apply allowlist filter if timeGrains is configured, but keep current selection visible
.filter(option => {
const allowlist = formData.timeGrains;
if (!allowlist || allowlist.length === 0) {
return true;
}
return allowlist.includes(option.value) || value.includes(option.value);
});
const placeholderText =
options.length === 0
? t('No data')
: tn('%s option', '%s options', options.length, options.length);
const sortComparator = useCallback(
(a: LabeledValue, b: LabeledValue) => {

View File

@@ -0,0 +1,181 @@
/**
* 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.
*/
/**
* Integration Test: Time Grain Pre-filter Feature (Customization plugin)
*
* Mirrors src/filters/components/TimeGrain/TimeGrainPreFilter.integration.test.tsx
* but targets the customization plugin at
* src/chartCustomizations/components/TimeGrain/TimeGrainFilterPlugin and adds
* a case for the escape hatch the customization version introduces.
*/
import {
render,
screen,
userEvent,
waitFor,
} from 'spec/helpers/testing-library';
import PluginFilterTimegrain from 'src/chartCustomizations/components/TimeGrain/TimeGrainFilterPlugin';
import type { PluginFilterTimeGrainProps } from 'src/chartCustomizations/components/TimeGrain/types';
const data = [
{ duration: 'PT1M', name: 'Minute' },
{ duration: 'PT1H', name: 'Hour' },
{ duration: 'P1D', name: 'Day' },
{ duration: 'P1W', name: 'Week' },
{ duration: 'P1M', name: 'Month' },
];
const baseConfig = {
height: 100,
width: 300,
setFilterActive: jest.fn(),
setHoveredFilter: jest.fn(),
unsetHoveredFilter: jest.fn(),
setFocusedFilter: jest.fn(),
unsetFocusedFilter: jest.fn(),
inputRef: { current: null },
};
/**
* Scenario: Dashboard owner configures a time grain filter to show only Hour, Day, Week.
* End-user opens the dashboard and can only select from those three options.
*/
test('time grain pre-filter restricts dashboard filter options', async () => {
const setDataMask = jest.fn();
const dashboardConfig: PluginFilterTimeGrainProps = {
...baseConfig,
data,
formData: {
datasource: '3__table',
height: 100,
width: 300,
nativeFilterId: 'time_grain_1',
defaultValue: null,
viz_type: 'filter_timegrain',
timeGrains: ['PT1H', 'P1D', 'P1W'],
},
filterState: {
value: null,
validateStatus: undefined,
validateMessage: undefined,
},
setDataMask,
};
render(<PluginFilterTimegrain {...dashboardConfig} />);
expect(screen.getByText('3 options')).toBeInTheDocument();
setDataMask.mockClear();
await userEvent.click(screen.getByRole('combobox'));
await waitFor(() => {
const labels = screen.getAllByRole('option').map(o => o.textContent);
expect(labels).toEqual(['Hour', 'Day', 'Week']);
expect(labels).not.toContain('Minute');
expect(labels).not.toContain('Month');
});
await userEvent.click(screen.getByText('Day'));
await waitFor(() => {
expect(setDataMask).toHaveBeenCalledWith({
extraFormData: { time_grain_sqla: 'P1D' },
filterState: { label: 'Day', value: ['P1D'] },
});
});
});
/**
* Scenario: Dashboard owner disables pre-filter (unchecks the CollapsibleControl).
* No restrictions: all time grains appear in the runtime filter.
*/
test('all time grains appear when pre-filter is unchecked', async () => {
const dashboardConfig: PluginFilterTimeGrainProps = {
...baseConfig,
data,
formData: {
datasource: '3__table',
height: 100,
width: 300,
nativeFilterId: 'time_grain_1',
defaultValue: null,
viz_type: 'filter_timegrain',
timeGrains: undefined,
},
filterState: {
value: null,
validateStatus: undefined,
validateMessage: undefined,
},
setDataMask: jest.fn(),
};
render(<PluginFilterTimegrain {...dashboardConfig} />);
await userEvent.click(screen.getByRole('combobox'));
await waitFor(() => {
const labels = screen.getAllByRole('option').map(o => o.textContent);
expect(labels).toEqual(['Minute', 'Hour', 'Day', 'Week', 'Month']);
});
});
/**
* Scenario: Dashboard owner narrowed the pre-filter after an end-user had
* already selected a value that now falls outside the allowlist.
* The current selection stays visible so the filter does not silently drop it.
*/
test('current selection stays visible when it is outside the pre-filter allowlist', async () => {
const dashboardConfig: PluginFilterTimeGrainProps = {
...baseConfig,
data,
formData: {
datasource: '3__table',
height: 100,
width: 300,
nativeFilterId: 'time_grain_1',
defaultValue: null,
viz_type: 'filter_timegrain',
timeGrains: ['PT1H', 'P1D', 'P1W'],
},
filterState: {
value: ['P1M'],
validateStatus: undefined,
validateMessage: undefined,
},
setDataMask: jest.fn(),
};
render(<PluginFilterTimegrain {...dashboardConfig} />);
await userEvent.click(screen.getByRole('combobox'));
await waitFor(() => {
const labels = screen.getAllByRole('option').map(o => o.textContent);
expect(labels).toHaveLength(4);
expect(labels).toEqual(
expect.arrayContaining(['Hour', 'Day', 'Week', 'Month']),
);
expect(labels).not.toContain('Minute');
});
});

View File

@@ -24,6 +24,7 @@ import { PluginFilterHooks, PluginFilterStylesProps } from '../types';
interface PluginFilterTimeGrainCustomizeProps {
defaultValue?: string[] | null;
inputRef?: RefObject<HTMLInputElement>;
timeGrains?: string[];
}
export type PluginFilterTimeGrainQueryFormData = QueryFormData &

View File

@@ -84,7 +84,7 @@ import {
} from 'src/database/actions';
import Mousetrap from 'mousetrap';
import { clearDatasetCache } from 'src/utils/cachedSupersetGet';
import { makeUrl } from 'src/utils/pathUtils';
import { makeUrl, openInNewTab } from 'src/utils/navigationUtils';
import {
OwnerSelectLabel,
OWNER_TEXT_LABEL_PROP,
@@ -1181,7 +1181,9 @@ function DatasourceEditor({
}, [datasource]);
const openOnSqlLab = useCallback(() => {
window.open(getSQLLabUrl(), '_blank', 'noopener,noreferrer');
// `getSQLLabUrl()` already runs the path through `makeUrl`; `openInNewTab`
// re-applies `ensureAppRoot`, which is idempotent on already-prefixed paths.
openInNewTab(getSQLLabUrl());
}, [getSQLLabUrl]);
const onQueryRun = useCallback(async () => {

View File

@@ -66,7 +66,7 @@ test('renders single dashboard link correctly', () => {
const link = screen.getByText('Sales Dashboard');
expect(link).toBeInTheDocument();
expect(link.closest('a')).toHaveAttribute('href', '/superset/dashboard/1/');
expect(link.closest('a')).toHaveAttribute('href', '/dashboard/1/');
expect(link.closest('a')).toHaveAttribute('target', '_blank');
});
@@ -98,9 +98,9 @@ test('links have correct href attributes', () => {
.getByText(', Very Long Dashboard Name That Should Be Truncated')
.closest('a');
expect(salesLink).toHaveAttribute('href', '/superset/dashboard/1/');
expect(analyticsLink).toHaveAttribute('href', '/superset/dashboard/2/');
expect(longNameLink).toHaveAttribute('href', '/superset/dashboard/3/');
expect(salesLink).toHaveAttribute('href', '/dashboard/1/');
expect(analyticsLink).toHaveAttribute('href', '/dashboard/2/');
expect(longNameLink).toHaveAttribute('href', '/dashboard/3/');
});
test('applies correct styling classes', () => {
@@ -124,5 +124,5 @@ test('handles dashboard with empty title', () => {
const link = screen.getByRole('link');
expect(link).toHaveTextContent('');
expect(link).toHaveAttribute('href', '/superset/dashboard/1/');
expect(link).toHaveAttribute('href', '/dashboard/1/');
});

View File

@@ -62,7 +62,7 @@ const DashboardLinksExternal = ({
{dashboards.map((dashboard, index) => (
<GenericLink
key={dashboard.id}
to={`/superset/dashboard/${dashboard.id}/`}
to={`/dashboard/${dashboard.id}/`}
target="_blank"
>
{index === 0

View File

@@ -25,6 +25,7 @@ import {
within,
} from 'spec/helpers/testing-library';
import { DatasourceType, isFeatureEnabled } from '@superset-ui/core';
import * as getBootstrapData from 'src/utils/getBootstrapData';
import {
createProps,
DATASOURCE_ENDPOINT,
@@ -822,3 +823,57 @@ test('calculated column search is case-insensitive', async () => {
expect(screen.getByDisplayValue('upper_name')).toBeInTheDocument();
});
});
test('Open in SQL lab href is single-prefixed under subdirectory deployment', () => {
// The Open-in-SQL-Lab link's href is produced by `getSQLLabUrl()`:
// return makeUrl(`/sqllab/?${queryParams.toString()}`);
// `makeUrl` is the idempotent app-root prefix helper from
// `src/utils/navigationUtils`. Rendering the link requires both the
// virtual datasourceType state AND a populated Redux `database.queryResult`
// slice (which is not part of the default test reducer tree). Calling
// `makeUrl` directly with a `/superset` mock exercises the exact path the
// component takes and pins the dedupe invariant for the underlying helper.
const applicationRootSpy = jest
.spyOn(getBootstrapData, 'applicationRoot')
.mockReturnValue('/superset');
try {
const { makeUrl } = jest.requireActual('src/utils/navigationUtils');
const queryParams = new URLSearchParams({
dbid: '1',
sql: 'SELECT * FROM users',
name: 'Vehicle Sales',
schema: 'public',
autorun: 'true',
isDataset: 'true',
});
const url = makeUrl(`/sqllab/?${queryParams.toString()}`);
expect(url).toMatch(/^\/superset\/sqllab\/\?/);
expect(url).not.toMatch(/\/superset\/superset\//);
} finally {
applicationRootSpy.mockRestore();
}
});
test('DatasourceEditor source pins getSQLLabUrl/openOnSqlLab to the makeUrl + openInNewTab helpers', () => {
// Source-pin: lock the exact two-line shape the runtime behaviour depends
// on. `getSQLLabUrl` MUST wrap its `/sqllab/?...` path in `makeUrl` so the
// Layer-2 idempotent prefix runs at the click boundary; `openOnSqlLab`
// MUST delegate to `openInNewTab` so `ensureAppRoot` runs again (idempotent
// dedupe, see `navigationUtils.appRoot.test.tsx`). A refactor that drops
// either layer would let a doubled-prefix URL escape into a new tab.
// eslint-disable-next-line global-require
const { readFileSync } = require('fs');
// eslint-disable-next-line global-require
const { join } = require('path');
const src = readFileSync(
join(__dirname, '..', 'DatasourceEditor.tsx'),
'utf8',
);
expect(src).toMatch(
/return makeUrl\(`\/sqllab\/\?\$\{queryParams\.toString\(\)\}`\);/,
);
expect(src).toMatch(/openInNewTab\(getSQLLabUrl\(\)\);/);
expect(src).toMatch(
/import \{ makeUrl, openInNewTab \} from 'src\/utils\/navigationUtils';/,
);
});

View File

@@ -24,7 +24,7 @@ import {
} from '@superset-ui/core';
import getOwnerName from 'src/utils/getOwnerName';
import { Avatar, AvatarGroup, Tooltip } from '@superset-ui/core/components';
import { ensureAppRoot } from 'src/utils/pathUtils';
import { ensureAppRoot } from 'src/utils/navigationUtils';
import { getRandomColor } from './utils';
import type { FacePileProps } from './types';

View File

@@ -68,10 +68,9 @@ test('should render the link with just one item', () => {
],
});
expect(screen.getByText('Test dashboard')).toBeInTheDocument();
expect(screen.getByRole('link')).toHaveAttribute(
'href',
`/superset/dashboard/1`,
);
// default `linkPrefix` is now `/dashboard/` (post-route_base);
// legacy `/superset/dashboard/...` was the pre-collapse route.
expect(screen.getByRole('link')).toHaveAttribute('href', `/dashboard/1`);
});
test('should render a custom prefix link', () => {

View File

@@ -65,7 +65,7 @@ const StyledCrossLinks = styled.div`
function CrossLinks({
crossLinks,
maxLinks = 20,
linkPrefix = '/superset/dashboard/',
linkPrefix = '/dashboard/',
external = false,
}: CrossLinksProps) {
const [crossLinksRef, plusRef, elementsTruncated, hasHiddenElements] =

View File

@@ -19,7 +19,7 @@
import { useState, useCallback, useRef, useEffect } from 'react';
import { SupersetClient } from '@superset-ui/core';
import { ExportStatus, StreamingProgress } from './StreamingExportModal';
import { makeUrl } from 'src/utils/pathUtils';
import { makeUrl } from 'src/utils/navigationUtils';
import { applicationRoot } from 'src/utils/getBootstrapData';
interface UseStreamingExportOptions {
@@ -38,8 +38,8 @@ interface StreamingExportParams {
* The API endpoint URL for the export request.
*
* URLs should be prefixed with the application root at the call site using
* `makeUrl()` from 'src/utils/pathUtils'. This ensures proper handling for
* subdirectory deployments (e.g., /superset/api/v1/...).
* `makeUrl()` from `src/utils/navigationUtils`. This ensures proper handling
* for subdirectory deployments (e.g., /superset/api/v1/...).
*
* A defensive guard (`ensureUrlPrefix`) will apply the prefix if missing,
* but callers should not rely on this fallback behavior.

View File

@@ -82,7 +82,7 @@ const SupersetTag = ({
{' '}
{id ? (
<Link
to={`/superset/all_entities/?id=${id}`}
to={`/all_entities/?id=${id}`}
target="_blank"
rel="noreferrer"
>

View File

@@ -35,12 +35,12 @@ test('getPage falls back to "home" for the welcome page and unknown pathnames',
const { navigation, notifyLocationChanged } = await importNavigation();
// The default pathname ('/') is not enumerated and falls back to home.
expect(navigation.getPage()).toBe('home');
notifyLocationChanged('/superset/welcome/');
notifyLocationChanged('/welcome/');
expect(navigation.getPage()).toBe('home');
});
test('getPage derives the page from window.location.pathname', async () => {
window.location.pathname = '/superset/dashboard/42/';
window.location.pathname = '/dashboard/42/';
const { navigation } = await importNavigation();
expect(navigation.getPage()).toBe('dashboard');
});
@@ -56,19 +56,19 @@ test('notifyLocationChanged fires listeners on page type change', async () => {
const listener = jest.fn();
const disposable = navigation.onDidChangePage(listener);
notifyLocationChanged('/superset/dashboard/1/');
notifyLocationChanged('/dashboard/1/');
expect(listener).toHaveBeenCalledWith('dashboard');
disposable.dispose();
});
test('notifyLocationChanged does not fire listeners when page type is unchanged', async () => {
window.location.pathname = '/superset/dashboard/1/';
window.location.pathname = '/dashboard/1/';
const { navigation, notifyLocationChanged } = await importNavigation();
const listener = jest.fn();
navigation.onDidChangePage(listener);
notifyLocationChanged('/superset/dashboard/2/');
notifyLocationChanged('/dashboard/2/');
expect(listener).not.toHaveBeenCalled();
});
@@ -78,7 +78,7 @@ test('onDidChangePage listener is removed after dispose', async () => {
const disposable = navigation.onDidChangePage(listener);
disposable.dispose();
notifyLocationChanged('/superset/dashboard/1/');
notifyLocationChanged('/dashboard/1/');
expect(listener).not.toHaveBeenCalled();
});

View File

@@ -36,8 +36,12 @@ type Page = navigationApi.Page;
/** Maps route path patterns to their corresponding Page type. */
const PAGE_ROUTES: { path: string; page: Page }[] = [
{ path: RoutePaths.DASHBOARD, page: 'dashboard' },
// List routes must precede their parameterised detail counterparts: with
// prefix-free paths, `matchPath(exact: false)` lets `/dashboard/:idOrSlug/`
// greedily capture `/dashboard/list/` (idOrSlug='list'), so the more specific
// list route has to win first — mirroring the `routes.tsx` Switch precedence.
{ path: RoutePaths.DASHBOARD_LIST, page: 'dashboard_list' },
{ path: RoutePaths.DASHBOARD, page: 'dashboard' },
{ path: RoutePaths.QUERY_HISTORY, page: 'query_history' },
{ path: RoutePaths.SAVED_QUERIES, page: 'saved_queries' },
{ path: RoutePaths.SQLLAB, page: 'sqllab' },

View File

@@ -54,7 +54,7 @@ import {
} from 'spec/fixtures/mockSliceEntities';
import { emptyFilters } from 'spec/fixtures/mockDashboardFilters';
import mockDashboardData from 'spec/fixtures/mockDashboardData';
import { navigateTo } from 'src/utils/navigationUtils';
import { navigateTo, navigateWithState } from 'src/utils/navigationUtils';
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
@@ -72,6 +72,7 @@ jest.mock('src/utils/navigationUtils', () => ({
const mockIsFeatureEnabled = isFeatureEnabled as jest.Mock;
const mockNavigateTo = navigateTo as jest.Mock;
const mockNavigateWithState = navigateWithState as jest.Mock;
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
describe('dashboardState actions', () => {
@@ -253,7 +254,48 @@ describe('dashboardState actions', () => {
await waitFor(() => expect(postStub.mock.calls.length).toBe(1));
expect(mockNavigateTo).toHaveBeenCalledWith(
`/superset/dashboard/${newDashboardId}/`,
`/dashboard/${newDashboardId}/`,
);
});
// `navigateWithState` regression for the
// dashboard-properties-changed save path. Two assertions in one shape:
// (a) the emitted path is router-relative (`/dashboard/<id>/`), not
// the pre-migration `/superset/dashboard/<id>/` literal that under
// subdirectory deployment would double-prefix to
// `/superset/superset/dashboard/<id>/`;
// (b) the `event: 'dashboard_properties_changed'` history-state arg is
// preserved verbatim. A previous attempt to swap `navigateWithState`
// for a plain `navigateTo` would silently drop this state object and
// the dashboard would lose its post-save UX cue.
test('saves dashboard properties via navigateWithState with state preserved', async () => {
const updatedId = 777;
const { getState, dispatch } = setup({
dashboardState: { hasUnsavedChanges: true },
});
mockNavigateWithState.mockClear();
putStub.mockRestore();
putStub = jest.spyOn(SupersetClient, 'put').mockResolvedValue({
json: {
result: { ...mockDashboardData, id: updatedId, slug: null },
last_modified_time: 0,
},
} as any);
const thunk = saveDashboardRequest(
newDashboardData,
updatedId,
SAVE_TYPE_OVERWRITE,
);
await thunk(dispatch, getState);
await waitFor(() => expect(putStub.mock.calls.length).toBe(1));
await waitFor(() => expect(mockNavigateWithState).toHaveBeenCalled());
expect(mockNavigateWithState).toHaveBeenCalledWith(
`/dashboard/${updatedId}/`,
{ event: 'dashboard_properties_changed' },
);
});
});
@@ -513,13 +555,11 @@ describe('dashboardState actions', () => {
});
getStub.mockRestore();
getStub = jest
.spyOn(SupersetClient, 'get')
.mockRejectedValue(
new Response(JSON.stringify({ message: 'Not found' }), {
status: 404,
}),
);
getStub = jest.spyOn(SupersetClient, 'get').mockRejectedValue(
new Response(JSON.stringify({ message: 'Not found' }), {
status: 404,
}),
);
await fetchFaveStar(id)(dispatch, getState);
@@ -536,13 +576,11 @@ describe('dashboardState actions', () => {
});
getStub.mockRestore();
getStub = jest
.spyOn(SupersetClient, 'get')
.mockRejectedValue(
new Response(JSON.stringify({ message: 'Server error' }), {
status: 500,
}),
);
getStub = jest.spyOn(SupersetClient, 'get').mockRejectedValue(
new Response(JSON.stringify({ message: 'Server error' }), {
status: 500,
}),
);
await fetchFaveStar(id)(dispatch, getState);

View File

@@ -584,9 +584,7 @@ export function saveDashboardRequest(
}),
);
dispatch(saveDashboardFinished());
navigateTo(
`/superset/dashboard/${(response.json as JsonObject).result?.id}/`,
);
navigateTo(`/dashboard/${(response.json as JsonObject).result?.id}/`);
dispatch(addSuccessToast(t('This dashboard was saved successfully.')));
return response;
};
@@ -639,7 +637,7 @@ export function saveDashboardRequest(
}
dispatch(saveDashboardFinished());
// redirect to the new slug or id
navigateWithState(`/superset/dashboard/${slug || id}/`, {
navigateWithState(`/dashboard/${slug || id}/`, {
event: 'dashboard_properties_changed',
});

View File

@@ -62,7 +62,7 @@ export function fetchDatasourceMetadata(key: string) {
}
return SupersetClient.get({
endpoint: `/superset/fetch_datasource_metadata?datasourceKey=${key}`,
endpoint: `/fetch_datasource_metadata?datasourceKey=${key}`,
}).then(({ json }) => dispatch(setDatasource(json as Datasource, key)));
};
}

View File

@@ -48,7 +48,7 @@ import * as useNativeFiltersModule from './state';
fetchMock.get('glob:*/csstemplateasyncmodelview/api/read', {});
fetchMock.put('glob:*/api/v1/dashboard/*', {});
// Add mock for logging endpoint
fetchMock.post('glob:*/superset/log/?*', {});
fetchMock.post('glob:*/log/?*', {});
jest.mock('src/dashboard/actions/dashboardState', () => ({
...jest.requireActual('src/dashboard/actions/dashboardState'),

View File

@@ -29,7 +29,7 @@ import * as chartCustomizationActions from '../../actions/chartCustomizationActi
fetchMock.get('glob:*/csstemplateasyncmodelview/api/read', {});
fetchMock.put('glob:*/api/v1/dashboard/*/colors*', {});
fetchMock.post('glob:*/superset/log/?*', {});
fetchMock.post('glob:*/log/?*', {});
jest.mock('@visx/responsive', () => ({
useParentSize: () => ({ parentRef: { current: null }, width: 800 }),

View File

@@ -0,0 +1,172 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { render, screen, userEvent } from 'spec/helpers/testing-library';
import { VizType } from '@superset-ui/core';
import mockState from 'spec/fixtures/mockState';
import SliceHeaderControls, { SliceHeaderControlsProps } from '.';
// Subdirectory-specific regressions live here so the existing 676-line
// SliceHeaderControls.test.tsx doesn't need to mock getBootstrapData.
// DO NOT switch this file to
// `spec/helpers/withApplicationRoot.ts`. The fixture does
// `jest.resetModules()` + dynamic `import('src/utils/getBootstrapData')` to
// install a fixture-configured applicationRoot. But `SliceHeaderControls` is
// imported statically at the top of this file; its transitive dependency
// chain (`SliceHeaderControls` → `navigationUtils` → `pathUtils` →
// `getBootstrapData::applicationRoot`) is bound to the pre-reset module
// instance. After `withApplicationRoot('/superset')` resets modules and
// re-imports getBootstrapData on the test side, the statically-imported
// component continues to reach the OLD module whose `application_root` was
// empty at first evaluation — so the rendered tree resolves
// `applicationRoot()` to `''`, NOT `/superset`. Gate (a) of the M7 go/no-go
// fails ("the rendered SliceHeaderControls tree must resolve
// applicationRoot() to the fixture-configured value"). The hand-rolled
// `jest.mock('src/utils/getBootstrapData', ...)` below remains until a
// later slice either (i) defers the SliceHeaderControls import into the
// withApplicationRoot callback or (ii) plumbs application_root through
// React context rather than a module-scoped cache.
// Name must start with `mock` so Jest's hoisted jest.mock() factory may
// reference it. `default` returns a static shape (not mockApplicationRoot)
// because consumers like setupClient.ts call getBootstrapData() at import
// time — calling mockApplicationRoot inside `default` hits TDZ.
const mockApplicationRoot = jest.fn<string, []>(() => '');
jest.mock('src/utils/getBootstrapData', () => ({
__esModule: true,
default: () => ({
common: { application_root: '', static_assets_prefix: '' },
}),
applicationRoot: () => mockApplicationRoot(),
staticAssetsPrefix: () => '',
}));
const SLICE_ID = 371;
const buildProps = (): SliceHeaderControlsProps =>
({
addDangerToast: jest.fn(),
addSuccessToast: jest.fn(),
exploreChart: jest.fn(),
exportCSV: jest.fn(),
exportFullCSV: jest.fn(),
exportXLSX: jest.fn(),
exportFullXLSX: jest.fn(),
exportPivotExcel: jest.fn(),
forceRefresh: jest.fn(),
handleToggleFullSize: jest.fn(),
toggleExpandSlice: jest.fn(),
logEvent: jest.fn(),
logExploreChart: jest.fn(),
slice: {
slice_id: SLICE_ID,
slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20371%7D',
slice_name: 'Subdirectory regression chart',
slice_description: '',
form_data: {
slice_id: SLICE_ID,
datasource: '58__table',
viz_type: VizType.Sunburst,
},
viz_type: VizType.Sunburst,
datasource: '58__table',
description: '',
description_markeddown: '',
owners: [],
modified: '',
changed_on: 0,
},
isCached: [false],
isExpanded: false,
cachedDttm: [''],
updatedDttm: 0,
supersetCanExplore: true,
supersetCanCSV: true,
componentId: 'CHART-subdir',
dashboardId: 26,
isFullSize: false,
chartStatus: 'rendered',
showControls: true,
supersetCanShare: true,
formData: {
slice_id: SLICE_ID,
datasource: '58__table',
viz_type: VizType.Sunburst,
},
exploreUrl: '/explore/?dashboard_page_id=abc&slice_id=371',
defaultOpen: true,
}) as unknown as SliceHeaderControlsProps;
const renderControls = (): void => {
render(<SliceHeaderControls {...buildProps()} />, {
useRedux: true,
useRouter: true,
initialState: {
...mockState,
user: {
...mockState.user,
roles: { Admin: [['can_samples', 'Datasource']] },
},
},
});
};
describe('SliceHeaderControls — Cmd-click "Edit chart" under subdirectory deployment', () => {
let openSpy: jest.SpyInstance;
beforeEach(() => {
mockApplicationRoot.mockReturnValue('');
openSpy = jest.spyOn(window, 'open').mockImplementation(() => null);
});
afterEach(() => {
openSpy.mockRestore();
});
test('opens the unprefixed exploreUrl when application root is empty', async () => {
mockApplicationRoot.mockReturnValue('');
renderControls();
userEvent.click(screen.getByRole('button', { name: 'More Options' }));
const editChart = await screen.findByText('Edit chart');
userEvent.click(editChart, { metaKey: true });
expect(openSpy).toHaveBeenCalledWith(
'/explore/?dashboard_page_id=abc&slice_id=371',
'_blank',
'noopener noreferrer',
);
});
test('opens the prefixed exploreUrl when deployed under a subdirectory', async () => {
mockApplicationRoot.mockReturnValue('/superset');
renderControls();
userEvent.click(screen.getByRole('button', { name: 'More Options' }));
const editChart = await screen.findByText('Edit chart');
userEvent.click(editChart, { metaKey: true });
expect(openSpy).toHaveBeenCalledWith(
'/superset/explore/?dashboard_page_id=abc&slice_id=371',
'_blank',
'noopener noreferrer',
);
});
});

View File

@@ -57,6 +57,7 @@ import { useDrillDetailMenuItems } from 'src/components/Chart/useDrillDetailMenu
import { LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE } from 'src/logger/LogUtils';
import { MenuKeys, RootState } from 'src/dashboard/types';
import DrillDetailModal from 'src/components/Chart/DrillDetail/DrillDetailModal';
import { openInNewTab } from 'src/utils/navigationUtils';
import { usePermissions } from 'src/hooks/usePermissions';
import { useDatasetDrillInfo } from 'src/hooks/apiResources/datasets';
import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
@@ -263,7 +264,7 @@ const SliceHeaderControls = (
props.logExploreChart?.(props.slice.slice_id);
if (domEvent.metaKey || domEvent.ctrlKey) {
domEvent.preventDefault();
window.open(props.exploreUrl, '_blank');
openInNewTab(props.exploreUrl);
} else {
history.push(props.exploreUrl);
}

View File

@@ -26,6 +26,9 @@ const PERMALINK_PAYLOAD = {
key: '123',
url: 'http://fakeurl.com/123',
};
// rewritePermalinkOrigin substitutes window.location.origin (jsdom: http://localhost)
// for the permalink's origin while preserving the path. See urlUtils.ts.
const REWRITTEN_URL = `${window.location.origin}/123`;
const FILTER_STATE_PAYLOAD = {
value: '{}',
};
@@ -58,9 +61,7 @@ test('renders overlay on click', async () => {
test('obtains short url', async () => {
render(<URLShortLinkButton {...props} />, { useRedux: true });
userEvent.click(screen.getByRole('button'));
expect(await screen.findByRole('tooltip')).toHaveTextContent(
PERMALINK_PAYLOAD.url,
);
expect(await screen.findByRole('tooltip')).toHaveTextContent(REWRITTEN_URL);
});
test('creates email anchor', async () => {
@@ -78,7 +79,7 @@ test('creates email anchor', async () => {
},
);
const href = `mailto:?Subject=${subject}%20&Body=${content}${PERMALINK_PAYLOAD.url}`;
const href = `mailto:?Subject=${subject}%20&Body=${content}${REWRITTEN_URL}`;
userEvent.click(screen.getByRole('button'));
expect(await screen.findByRole('link')).toHaveAttribute('href', href);
});

View File

@@ -0,0 +1,146 @@
/**
* 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 type { FC } from 'react';
import { render, screen } from 'spec/helpers/testing-library';
// Tab.tsx is statically imported below; the mock pattern intercepts
// applicationRoot() rather than relying on withApplicationRoot (which is for
// dynamic-import unit tests).
const mockApplicationRoot = jest.fn<string, []>(() => '');
jest.mock('src/utils/getBootstrapData', () => {
const actual = jest.requireActual<
typeof import('src/utils/getBootstrapData')
>('src/utils/getBootstrapData');
return {
__esModule: true,
default: actual.default,
applicationRoot: () => mockApplicationRoot(),
staticAssetsPrefix: actual.staticAssetsPrefix,
};
});
jest.mock('src/dashboard/util/getChartIdsFromComponent', () =>
jest.fn(() => []),
);
jest.mock('src/dashboard/containers/DashboardComponent', () =>
jest.fn(() => <div data-test="DashboardComponent" />),
);
jest.mock('@superset-ui/core/components/EditableTitle', () => ({
__esModule: true,
EditableTitle: jest.fn(() => <div data-test="EditableTitle" />),
}));
jest.mock('src/dashboard/components/dnd/DragDroppable', () => ({
...jest.requireActual('src/dashboard/components/dnd/DragDroppable'),
Droppable: jest.fn(props => (
<div>{props.children ? props.children({}) : null}</div>
)),
}));
// eslint-disable-next-line import/first
import ActualTab from './Tab';
const Tab = ActualTab as unknown as FC<Record<string, unknown>>;
const DASHBOARD_ID = 23;
const buildProps = () => ({
id: 'TAB-empty-',
parentId: 'TABS-empty-',
depth: 2,
index: 0,
renderType: 'RENDER_TAB_CONTENT',
availableColumnCount: 12,
columnWidth: 120,
isFocused: false,
component: {
children: [],
id: 'TAB-empty-',
meta: { text: 'Empty Tab' },
parents: ['ROOT_ID', 'GRID_ID', 'TABS-empty-'],
type: 'TAB',
},
parentComponent: {
children: ['TAB-empty-'],
id: 'TABS-empty-',
meta: {},
parents: ['ROOT_ID', 'GRID_ID'],
type: 'TABS',
},
editMode: true,
embeddedMode: false,
undoLength: 0,
redoLength: 0,
filters: {},
directPathToChild: [],
directPathLastUpdated: 0,
dashboardId: DASHBOARD_ID,
focusedFilterScope: null,
isComponentVisible: true,
onDropOnTab: jest.fn(),
handleComponentDrop: jest.fn(),
updateComponents: jest.fn(),
setDirectPathToChild: jest.fn(),
onResizeStart: jest.fn(),
onResize: jest.fn(),
onResizeStop: jest.fn(),
});
const renderEmptyEditModeTab = () =>
render(<Tab {...buildProps()} />, {
useRedux: true,
useDnd: true,
initialState: {
dashboardInfo: { dash_edit_perm: true },
},
});
beforeEach(() => {
mockApplicationRoot.mockReturnValue('');
});
test('Tab — empty edit-mode "create a new chart" link is unprefixed when application root is empty', () => {
mockApplicationRoot.mockReturnValue('');
renderEmptyEditModeTab();
expect(
screen.getByRole('link', { name: 'create a new chart' }),
).toHaveAttribute('href', `/chart/add?dashboard_id=${DASHBOARD_ID}`);
});
test('Tab — empty edit-mode "create a new chart" link carries the application root under subdirectory deployment', () => {
mockApplicationRoot.mockReturnValue('/superset');
renderEmptyEditModeTab();
// Single prefix — not /superset/superset/ — verifying ensureAppRoot's
// dedupe boundary holds against the path's leading slash.
expect(
screen.getByRole('link', { name: 'create a new chart' }),
).toHaveAttribute('href', `/superset/chart/add?dashboard_id=${DASHBOARD_ID}`);
});
test('Tab — empty edit-mode "create a new chart" link prefixes correctly for nested subdirectory roots', () => {
mockApplicationRoot.mockReturnValue('/a/b/c');
renderEmptyEditModeTab();
expect(
screen.getByRole('link', { name: 'create a new chart' }),
).toHaveAttribute('href', `/a/b/c/chart/add?dashboard_id=${DASHBOARD_ID}`);
});

View File

@@ -27,6 +27,7 @@ import {
import DashboardComponent from 'src/dashboard/containers/DashboardComponent';
import { EditableTitle } from '@superset-ui/core/components';
import { setEditMode, onRefresh } from 'src/dashboard/actions/dashboardState';
import * as getBootstrapData from 'src/utils/getBootstrapData';
import type { FC } from 'react';
import ActualTab from './Tab';
@@ -488,6 +489,36 @@ test('Render tab content with no children, editMode: true, canEdit: true', () =>
).toHaveAttribute('href', '/chart/add?dashboard_id=23');
});
test('empty-tab "create a new chart" link is single-prefixed under subdirectory deployment', () => {
// The empty-tab CTA composes the chart-add URL via ensureAppRoot. Under
// SUPERSET_APP_ROOT=/superset the rendered href must be exactly
// `/superset/chart/add?dashboard_id=23` — not `/chart/add?…` (no prefix)
// and not `/superset/superset/chart/add?…` (double prefix). The link uses
// target="_blank", so basename routing does NOT re-apply the prefix.
const applicationRootSpy = jest
.spyOn(getBootstrapData, 'applicationRoot')
.mockReturnValue('/superset');
try {
const props = createProps();
props.editMode = true;
props.component.children = [];
render(<Tab {...props} />, {
useRedux: true,
useDnd: true,
initialState: {
dashboardInfo: { dash_edit_perm: true },
},
});
expect(
screen.getByRole('link', { name: 'create a new chart' }),
).toHaveAttribute('href', '/superset/chart/add?dashboard_id=23');
} finally {
applicationRootSpy.mockRestore();
}
});
test('Drag to empty state, editMode: true, canEdit: true', async () => {
const props = createProps();
props.editMode = true;

View File

@@ -36,6 +36,7 @@ import getChartIdsFromComponent from 'src/dashboard/util/getChartIdsFromComponen
import DashboardComponent from 'src/dashboard/containers/DashboardComponent';
import AnchorLink from 'src/dashboard/components/AnchorLink';
import { Typography } from '@superset-ui/core/components/Typography';
import { ensureAppRoot } from 'src/utils/navigationUtils';
import {
useIsAutoRefreshing,
useIsRefreshInFlight,
@@ -333,7 +334,9 @@ const Tab = (props: TabProps): ReactElement => {
<span>
{t('You can')}{' '}
<Typography.Link
href={`/chart/add?dashboard_id=${dashboardId}`}
href={ensureAppRoot(
`/chart/add?dashboard_id=${dashboardId}`,
)}
rel="noopener noreferrer"
target="_blank"
>

View File

@@ -0,0 +1,207 @@
/**
* 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.
*/
// `FilterBar/index.tsx::publishDataMask` is one of the five sanctioned
// `applicationRoot()` callers (memory `project_supersetclient_approot_dedupe`).
// It runs after a filter mutation to push the updated filter cache key into
// the URL via `history.replace`. Two appRoot-aware operations gate that
// replace:
//
// 1. The path-matching guard — only fire when the current pathname is a
// dashboard route under the configured appRoot. The bug class this
// catches is "filter writes pollute Explore's URL after navigation".
//
// 2. The prefix-strip — React Router applies `basename` internally, so
// `history.replace({ pathname })` must receive a path WITHOUT the
// appRoot. The bug class this catches is `/superset/superset/dashboard/...`
// in the URL bar after the first filter change.
//
// `publishDataMask` is module-private (declared as a `const debounce(...)`).
// Testing it through a rendered FilterBar requires the Redux store, the
// filter cache API, and the debounce timer — heavyweight relative to what
// the contract actually says. Instead this test does two things:
//
// A. Reads FilterBar/index.tsx as source and pins the two patterns that
// embody the contract. A future refactor that drops the guard or the
// strip fails here loudly with the exact line that drifted.
// B. Tests the *equivalent* pure logic (re-implementation of the same
// pattern) across every appRoot × pathname × Explore-vs-Dashboard
// input shape that matters in practice. If the actual code drifts
// from the documented invariant, the source-pin in (A) fires; if the
// documented invariant itself is wrong, (B) fires.
import { readFileSync } from 'fs';
import { join } from 'path';
const FILTERBAR_SRC = readFileSync(join(__dirname, 'index.tsx'), 'utf8');
// ---------------------------------------------------------------------------
// (A) Source-pin: the two patterns that implement the contract.
// ---------------------------------------------------------------------------
test('FilterBar/index.tsx guards history.replace by the configured app root', () => {
// The guard short-circuits the URL mutation when the current path is not a
// dashboard route under the appRoot — e.g. when the user navigated to
// Explore (`/explore/?slice_id=...`), the FilterBar's debounced commit must
// not stomp Explore's query string with native_filters_key.
expect(FILTERBAR_SRC).toContain(
'window.location.pathname.startsWith(`${applicationRoot()}/dashboard`)',
);
});
test('FilterBar/index.tsx strips the app root before history.replace', () => {
// Both halves of the strip survive together — the appRoot != "/" check
// and the startsWith-before-substring guard. Each is load-bearing on its
// own (without the first, root deploy hits `.substring(1)` and clips off
// the leading slash; without the second, paths that diverge from the
// appRoot get incorrectly truncated).
expect(FILTERBAR_SRC).toContain(
"if (appRoot !== '/' && replacementPathname.startsWith(appRoot))",
);
expect(FILTERBAR_SRC).toContain(
'replacementPathname = replacementPathname.substring(appRoot.length);',
);
});
test('FilterBar/index.tsx imports applicationRoot from getBootstrapData', () => {
// Centralised symbol — the static-scan invariant in
// navigationUtils.invariants.test.ts enumerates the sanctioned import
// sites. If FilterBar's import path drifts, that scan also fires; this
// one anchors the import locally so a `git blame` on FilterBar tells the
// story without needing to cross-reference the scan ledger.
expect(FILTERBAR_SRC).toMatch(
/import\s+\{\s*applicationRoot\s*\}\s+from\s+'src\/utils\/getBootstrapData'/,
);
});
// ---------------------------------------------------------------------------
// (B) Characterisation: the documented invariant, exercised across the
// appRoot × pathname matrix.
// ---------------------------------------------------------------------------
//
// Re-implementation of the FilterBar guard + strip, kept here so the test
// can fail loudly if the *invariant itself* is wrong (rather than a typo in
// the implementation). The source-pin above catches the inverse case
// (implementation drifts away from the invariant).
interface Scenario {
description: string;
appRoot: string;
pathname: string;
shouldReplace: boolean;
replacementPathname?: string;
}
function applyFilterBarPathLogic(
appRoot: string,
pathname: string,
): { shouldReplace: boolean; replacementPathname?: string } {
if (!pathname.startsWith(`${appRoot}/dashboard`)) {
return { shouldReplace: false };
}
let replacement = pathname;
if (appRoot !== '/' && replacement.startsWith(appRoot)) {
replacement = replacement.substring(appRoot.length);
}
return { shouldReplace: true, replacementPathname: replacement };
}
const SCENARIOS: ReadonlyArray<Scenario> = [
// Root deploy — pathname matches `/dashboard` directly.
{
description: 'root deploy on a dashboard page',
appRoot: '',
pathname: '/dashboard/1/',
shouldReplace: true,
replacementPathname: '/dashboard/1/',
},
{
description: 'root deploy on Explore — guard short-circuits',
appRoot: '',
pathname: '/explore/?slice_id=42',
shouldReplace: false,
},
// Subdir deploy — appRoot is `/superset`, pathname carries the prefix.
{
description: 'subdir deploy on a dashboard page',
appRoot: '/superset',
pathname: '/superset/dashboard/2/',
shouldReplace: true,
// Stripped: React Router re-applies basename so the strip MUST happen.
replacementPathname: '/dashboard/2/',
},
{
description: 'subdir deploy on a dashboard permalink',
appRoot: '/superset',
pathname: '/superset/dashboard/p/abc123/',
shouldReplace: true,
replacementPathname: '/dashboard/p/abc123/',
},
{
description: 'subdir deploy on Explore — guard short-circuits',
appRoot: '/superset',
pathname: '/superset/explore/?slice_id=7',
shouldReplace: false,
},
{
description:
'subdir deploy on bare app root (no /dashboard) — short-circuits',
appRoot: '/superset',
pathname: '/superset/',
shouldReplace: false,
},
// Operator deploy under a deeply nested basename.
{
description: 'deep-nested deploy on a dashboard page',
appRoot: '/tenant-a/superset',
pathname: '/tenant-a/superset/dashboard/9/',
shouldReplace: true,
replacementPathname: '/dashboard/9/',
},
// Adversarial: appRoot `/dash` is a substring of `/dashboard`. The guard
// template is `${appRoot}/dashboard` so the prefix is `/dash/dashboard`,
// which (correctly) does NOT match a bare `/dashboard/1/` path. Pin the
// case so a maintainer doesn't "fix" the guard to also match prefix-free
// paths (which would re-introduce the Explore-stomp regression for
// operators whose root happens to share characters with `/dashboard`).
{
description:
'appRoot is a substring prefix of /dashboard — guard does NOT match a bare /dashboard path',
appRoot: '/dash',
pathname: '/dashboard/5/',
shouldReplace: false,
},
];
test.each(SCENARIOS)(
'publishDataMask path logic: $description',
({ appRoot, pathname, shouldReplace, replacementPathname }: Scenario) => {
const result = applyFilterBarPathLogic(appRoot, pathname);
expect(result.shouldReplace).toBe(shouldReplace);
if (shouldReplace) {
expect(result.replacementPathname).toBe(replacementPathname);
// The dedupe contract: no `/superset/superset/...` ever reaches React
// Router. Even if the source-pin drifts, this catches the user-visible
// symptom.
expect(result.replacementPathname).not.toMatch(/\/superset\/superset\//);
} else {
expect(result.replacementPathname).toBeUndefined();
}
},
);

View File

@@ -1,60 +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.
*/
import { ChartDataResponseResult } from '@superset-ui/core';
import { applyTimeGrainAllowlist } from './FilterValue';
const baseResults = [
{
data: [
{ duration: 'PT1H', name: 'Hour' },
{ duration: 'P1D', name: 'Day' },
{ duration: 'P1W', name: 'Week' },
{ duration: 'P1M', name: 'Month' },
],
},
] as unknown as ChartDataResponseResult[];
test('applyTimeGrainAllowlist should filter to configured durations', () => {
const filtered = applyTimeGrainAllowlist(
'filter_timegrain',
['PT1H', 'P1D', 'P1W'],
baseResults,
);
expect(filtered[0].data).toEqual([
{ duration: 'PT1H', name: 'Hour' },
{ duration: 'P1D', name: 'Day' },
{ duration: 'P1W', name: 'Week' },
]);
});
test('applyTimeGrainAllowlist should return unfiltered results for non-timegrain filters', () => {
const filtered = applyTimeGrainAllowlist(
'filter_select',
['PT1H'],
baseResults,
);
expect(filtered).toEqual(baseResults);
});
test('applyTimeGrainAllowlist should return unfiltered results when allowlist is empty', () => {
const filtered = applyTimeGrainAllowlist('filter_timegrain', [], baseResults);
expect(filtered).toEqual(baseResults);
});

View File

@@ -85,35 +85,6 @@ const StyledDiv = styled.div<{
const queriesDataPlaceholder = [{ data: [{}] }];
type TimeGrainFilterConfig = {
time_grains?: string[];
};
export const applyTimeGrainAllowlist = (
filterType: string,
allowedTimeGrains: string[] | undefined,
results: ChartDataResponseResult[],
): ChartDataResponseResult[] => {
if (filterType !== 'filter_timegrain' || !allowedTimeGrains?.length) {
return results;
}
return results.map(result => {
if (!Array.isArray(result.data)) {
return result;
}
return {
...result,
data: result.data.filter(row =>
allowedTimeGrains.includes(
(row as { duration?: string }).duration ?? '',
),
),
};
});
};
const useShouldFilterRefresh = () => {
const isDashboardRefreshing = useSelector<RootState, boolean>(
state => state.dashboardState.isRefreshing,
@@ -145,9 +116,6 @@ const FilterValue: FC<FilterValueProps> = ({
const theme = useTheme() as SupersetTheme;
const { id, targets, filterType } = filter;
const isCustomization = isChartCustomization(filter);
const allowedTimeGrains = isCustomization
? undefined
: (filter as TimeGrainFilterConfig).time_grains;
const adhocFilters = isCustomization ? undefined : filter.adhoc_filters;
const timeRange = isCustomization ? undefined : filter.time_range;
const granularitySqla = isCustomization ? undefined : filter.granularity_sqla;
@@ -285,23 +253,13 @@ const FilterValue: FC<FilterValueProps> = ({
// deal with getChartDataRequest transforming the response data
const result = 'result' in json ? json.result[0] : json;
if (response.status === 200) {
setState(
applyTimeGrainAllowlist(filterType, allowedTimeGrains, [
result as ChartDataResponseResult,
]),
);
setState([result as ChartDataResponseResult]);
setError(undefined);
handleFilterLoadFinish();
} else if (response.status === 202) {
waitForAsyncData(result as Parameters<typeof waitForAsyncData>[0])
.then((asyncResult: ChartDataResponseResult[]) => {
setState(
applyTimeGrainAllowlist(
filterType,
allowedTimeGrains,
asyncResult,
),
);
setState(asyncResult);
setError(undefined);
handleFilterLoadFinish();
})
@@ -317,13 +275,7 @@ const FilterValue: FC<FilterValueProps> = ({
);
}
} else {
setState(
applyTimeGrainAllowlist(
filterType,
allowedTimeGrains,
json.result as ChartDataResponseResult[],
),
);
setState(json.result as ChartDataResponseResult[]);
setError(undefined);
handleFilterLoadFinish();
}
@@ -342,7 +294,6 @@ const FilterValue: FC<FilterValueProps> = ({
groupby,
handleFilterLoadFinish,
filter,
allowedTimeGrains,
hasDataSource,
isRefreshing,
shouldRefresh,

View File

@@ -142,9 +142,11 @@ const publishDataMask = debounce(
// pathname could be updated somewhere else through window.history
// keep react router history in sync with window history
// replace params only when current page is /superset/dashboard
// replace params only when current page is a dashboard route under the
// configured applicationRoot (e.g. `/dashboard/...` for root deploy,
// `/superset/dashboard/...` for the legacy subdir deploy).
// this prevents a race condition between updating filters and navigating to Explore
if (window.location.pathname.includes('/superset/dashboard')) {
if (window.location.pathname.startsWith(`${applicationRoot()}/dashboard`)) {
// The history API is part of React router and understands that a basename may exist.
// Internally it treats all paths as if they are relative to the root and appends
// it when necessary. We strip any prefix so that history.replace adds it back and doesn't

View File

@@ -329,9 +329,8 @@ const FiltersConfigForm = (
const formFilterWithTimeGrains = formFilter as typeof formFilter & {
time_grains?: string[];
};
const filterToEditWithTimeGrains = filterToEdit as
| (Filter & { time_grains?: string[] })
| undefined;
const savedTimeGrains =
filterToEdit?.time_grains ?? customizationToEdit?.time_grains;
const handleModifyFilter = useCallback(() => {
if (onModifyFilter) {
@@ -594,8 +593,7 @@ const FiltersConfigForm = (
!!filterToEdit?.time_range;
const hasTimeGrainPreFilter = !!(
formFilterWithTimeGrains?.time_grains?.length ||
filterToEditWithTimeGrains?.time_grains?.length
formFilterWithTimeGrains?.time_grains?.length || savedTimeGrains?.length
);
const hasEnableSingleValue =
@@ -1297,7 +1295,9 @@ const FiltersConfigForm = (
</CollapsibleControl>
</FormItem>
)}
{itemTypeField === 'filter_timegrain' &&
{(itemTypeField === 'filter_timegrain' ||
itemTypeField ===
'chart_customization_timegrain') &&
hasDataset &&
datasetDetails?.time_grain_sqla &&
datasetDetails.time_grain_sqla.length > 0 && (
@@ -1332,9 +1332,7 @@ const FiltersConfigForm = (
filterId,
'time_grains',
]}
initialValue={
filterToEditWithTimeGrains?.time_grains
}
initialValue={savedTimeGrains}
{...getFiltersConfigModalTestId(
'time-grain-allowlist',
)}

View File

@@ -108,7 +108,7 @@ function transformFormInput(
excluded: [],
};
return {
const result: ChartCustomization = {
id,
type: ChartCustomizationType.ChartCustomization,
name: formInputs.name,
@@ -120,6 +120,12 @@ function transformFormInput(
defaultDataMask: formInputs.defaultDataMask ?? {},
removed: false,
};
if (formInputs.time_grains?.length) {
result.time_grains = formInputs.time_grains;
}
return result;
}
function transformSavedCustomization(

View File

@@ -89,6 +89,7 @@ export interface ChartCustomizationsFormItem {
adhoc_filters?: AdhocFilter[];
time_range?: string;
granularity_sqla?: string;
time_grains?: string[];
type: typeof NativeFilterType.NativeFilter;
description: string;
datasetInfo?: {

View File

@@ -23,6 +23,7 @@ import {
DataMaskStateWithId,
} from '@superset-ui/core';
import rison from 'rison';
import { navigateWithState } from 'src/utils/navigationUtils';
/**
* Synthetic dataMask key for URL Rison filters that don't match any native
@@ -238,7 +239,17 @@ export function prettifyRisonFilterUrl(): void {
const prettifiedUrl = `${beforeRison}${separator}f=${risonValue}${afterRison}`;
if (prettifiedUrl !== currentUrl) {
window.history.replaceState(window.history.state, '', prettifiedUrl);
// Route through navigateWithState so the navigationUtils guards
// (`assertSafeNavigationUrl` scheme/userinfo barriers + the
// CodeQL-recognised inline sanitisers) apply at the
// `window.history.replaceState` sink. The URL constructor inside
// `navigateWithState` is conservative about re-encoding: sub-delims
// like `(`, `)`, `:`, `!` (the meaningful Rison glyphs) survive,
// so the prettification's visual win is preserved for every
// character the prettifier actually targets.
navigateWithState(prettifiedUrl, window.history.state ?? {}, {
replace: true,
});
}
} catch (error) {
console.warn('Failed to prettify Rison URL:', error);
@@ -351,12 +362,11 @@ export function updateUrlWithUnmatchedFilters(
// With a real `BrowserRouter`, `history.replace` would do this too — but
// under a `createMemoryHistory` (used in tests, or in some embedded
// contexts) it does not, and we'd leak the stale URL into the next
// `getRisonFilterParam()` call.
window.history.replaceState(
window.history.state,
'',
currentUrl.toString(),
);
// `getRisonFilterParam()` call. Routed through navigateWithState so the
// navigationUtils scheme/userinfo barriers gate the sink.
navigateWithState(currentUrl.toString(), window.history.state ?? {}, {
replace: true,
});
if (history) {
history.replace({
pathname: currentUrl.pathname,

View File

@@ -0,0 +1,78 @@
/**
* 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 fetchMock from 'fetch-mock';
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import EmbedCodeContent from 'src/explore/components/EmbedCodeContent';
// The chart-embed iframe `src` is produced by:
// 1. Backend `url_for(_external=True)` → absolute URL whose origin is the
// backend `Host` header (often the internal docker hostname under
// `superset-light:8088` when `ENABLE_PROXY_FIX` is off).
// 2. Frontend `rewritePermalinkOrigin` swaps the origin for
// `window.location.origin` so the iframe `src` is reachable from the
// browser that pasted the embed code.
// 3. The path segment (`/superset/explore/p/<key>/`) survives unchanged —
// the application_root must therefore be applied exactly once.
//
// This composition was previously verified only via manual QA
// (memory `project_supersetclient_approot_dedupe` records the discovery).
// This test pins the iframe-src shape so a future change to the permalink
// API, the origin-rewrite helper, or the EmbedCodeContent template would
// surface in CI rather than in a user-reported broken embed.
const SUBDIR_PERMALINK_URL =
'http://superset-light:8088/superset/explore/p/abc123/';
fetchMock.post('glob:*/api/v1/explore/permalink', {
url: SUBDIR_PERMALINK_URL,
});
const mockFormData = {
datasource: 'table__1',
viz_type: 'table',
};
test('iframe src under subdir deployment uses browser origin + single prefix', async () => {
render(<EmbedCodeContent formData={mockFormData} />, { useRedux: true });
// The textarea `value` contains the full iframe HTML once the permalink
// promise resolves. `data-test="embed-code-textarea"` is the stable hook.
const textarea = await screen.findByTestId('embed-code-textarea');
// Wait for the asynchronous permalink fetch to land in the textarea.
await waitFor(() =>
expect((textarea as HTMLTextAreaElement).value).toContain('<iframe'),
);
const html = (textarea as HTMLTextAreaElement).value;
const srcMatch = html.match(/src="([^"]+)"/);
expect(srcMatch).not.toBeNull();
const src = (srcMatch as RegExpMatchArray)[1];
// Two contracts: origin is the browser-side origin (jsdom default
// `http://localhost`), and the `/superset/` prefix from the backend
// payload survives — exactly once.
const parsed = new URL(src);
expect(parsed.origin).toBe(window.location.origin);
expect(parsed.pathname).toBe('/superset/explore/p/abc123/');
expect(src).not.toContain('/superset/superset/');
// Standalone + height controls are appended additively by the component.
expect(parsed.searchParams.get('standalone')).toBe('1');
expect(parsed.searchParams.get('height')).toBe('400');
});

View File

@@ -41,7 +41,7 @@ import TextControl from 'src/explore/components/controls/TextControl';
import CheckboxControl from 'src/explore/components/controls/CheckboxControl';
import PopoverSection from '@superset-ui/core/components/PopoverSection';
import ControlHeader from 'src/explore/components/ControlHeader';
import { ensureAppRoot } from 'src/utils/pathUtils';
import { ensureAppRoot } from 'src/utils/navigationUtils';
import {
ANNOTATION_SOURCE_TYPES,
ANNOTATION_TYPES,
@@ -119,7 +119,13 @@ const NotFoundContent = () => (
<span>
{t('Add an annotation layer')}{' '}
<a
href={ensureAppRoot('/annotationlayer/list')}
// encodeURI wraps the DOM-derived application-root prefix so
// CodeQL's `js/html-injection` sees a recognised through-function
// sanitiser between `applicationRoot()` (reads `data-bootstrap`
// from the DOM) and the `<a href>` sink. The string fed in is a
// URL-normalised path (`/seg/seg`) so encodeURI is idempotent in
// practice — it does not alter the navigation target.
href={encodeURI(ensureAppRoot('/annotationlayer/list'))}
target="_blank"
rel="noopener noreferrer"
>

View File

@@ -185,6 +185,7 @@ test('opens SQL Lab in a new tab when View in SQL Lab button is clicked with met
expect(window.open).toHaveBeenCalledWith(
`/sqllab?datasourceKey=${datasource}&sql=${encodeURIComponent(sql)}`,
'_blank',
'noopener noreferrer',
);
});

View File

@@ -40,7 +40,7 @@ import {
import { CopyToClipboard } from 'src/components';
import { RootState } from 'src/dashboard/types';
import { findPermission } from 'src/utils/findPermission';
import { makeUrl } from 'src/utils/pathUtils';
import { openInNewTab } from 'src/utils/navigationUtils';
import CodeSyntaxHighlighter, {
SupportedLanguage,
preloadLanguages,
@@ -140,11 +140,8 @@ const ViewQuery: FC<ViewQueryProps> = props => {
};
if (domEvent.metaKey || domEvent.ctrlKey) {
domEvent.preventDefault();
window.open(
makeUrl(
`/sqllab?datasourceKey=${datasource}&sql=${encodeURIComponent(currentSQL)}`,
),
'_blank',
openInNewTab(
`/sqllab?datasourceKey=${datasource}&sql=${encodeURIComponent(currentSQL)}`,
);
} else {
history.push({ pathname: '/sqllab', state: { requestedQuery } });

View File

@@ -39,7 +39,7 @@ const TestDashboardsMenuItems = ({
<div data-test="menu-items">
{menuItems.map(item => (
<div key={item.key} data-test={`menu-item-${item!.key}`}>
{typeof item.label === 'string' ? item!.label : 'Complex Label'}
{item!.label}
{item!.disabled && <span data-test="disabled">disabled</span>}
</div>
))}
@@ -173,6 +173,35 @@ describe('DashboardsSubMenu', () => {
expect(screen.getByTestId('menu-item-5')).toBeInTheDocument();
});
test('renders SPA-relative dashboard links without the /superset/ prefix', () => {
// Regression: prior to the route_base="" alignment, this menu emitted
// `to="/superset/dashboard/<id>"` which, combined with the React Router
// `basename={applicationRoot()}`, produced a doubled `/superset/superset/`
// path on subdirectory deployments and a backend 404.
const dashboards = [{ id: 9, dashboard_title: 'Sales Dashboard' }];
render(
<TestDashboardsMenuItems
chartId={102}
dashboards={dashboards}
searchTerm=""
/>,
{ useRouter: true },
);
const link = screen.getByRole('link', { name: /Sales Dashboard/ });
expect(link).toHaveAttribute('href', '/dashboard/9?focused_chart=102');
});
test('omits the focused_chart query when chartId is undefined', () => {
const dashboards = [{ id: 9, dashboard_title: 'Sales Dashboard' }];
render(<TestDashboardsMenuItems dashboards={dashboards} searchTerm="" />, {
useRouter: true,
});
const link = screen.getByRole('link', { name: /Sales Dashboard/ });
expect(link).toHaveAttribute('href', '/dashboard/9');
});
test('partial string search works correctly', () => {
const dashboards = [
{ id: 1, dashboard_title: 'Revenue Report' },

View File

@@ -71,8 +71,8 @@ export const useDashboardsMenuItems = ({
label: (
<Link
target="_blank"
rel="noreferer noopener"
to={`/superset/dashboard/${dashboard.id}${urlQueryString}`}
rel="noreferrer noopener"
to={`/dashboard/${dashboard.id}${urlQueryString}`}
css={css`
display: flex;
flex-direction: row;

View File

@@ -66,7 +66,7 @@ describe('exploreUtils', () => {
force: false,
curUrl: 'http://superset.com',
});
compareURI(URI(url!), URI('/superset/explore_json/'));
compareURI(URI(url!), URI('/explore_json/'));
});
test('generates proper json forced url', () => {
const url = getExploreUrl({
@@ -75,10 +75,7 @@ describe('exploreUtils', () => {
force: true,
curUrl: 'superset.com',
});
compareURI(
URI(url!),
URI('/superset/explore_json/').search({ force: 'true' }),
);
compareURI(URI(url!), URI('/explore_json/').search({ force: 'true' }));
});
test('generates proper csv URL', () => {
const url = getExploreUrl({
@@ -87,10 +84,7 @@ describe('exploreUtils', () => {
force: false,
curUrl: 'superset.com',
});
compareURI(
URI(url!),
URI('/superset/explore_json/').search({ csv: 'true' }),
);
compareURI(URI(url!), URI('/explore_json/').search({ csv: 'true' }));
});
test('generates proper standalone URL', () => {
const url = getExploreUrl({
@@ -113,10 +107,7 @@ describe('exploreUtils', () => {
force: false,
curUrl: 'superset.com?foo=bar',
});
compareURI(
URI(url!),
URI('/superset/explore_json/').search({ foo: 'bar' }),
);
compareURI(URI(url!), URI('/explore_json/').search({ foo: 'bar' }));
});
test('generate proper save slice url', () => {
const url = getExploreUrl({
@@ -125,10 +116,7 @@ describe('exploreUtils', () => {
force: false,
curUrl: 'superset.com?foo=bar',
});
compareURI(
URI(url!),
URI('/superset/explore_json/').search({ foo: 'bar' }),
);
compareURI(URI(url!), URI('/explore_json/').search({ foo: 'bar' }));
});
});

View File

@@ -201,9 +201,12 @@ test('exportChart legacy API (useLegacyApi=true) passes prefixed URL to onStartS
expect(onStartStreamingExport).toHaveBeenCalledTimes(1);
const callArgs = onStartStreamingExport.mock.calls[0][0];
// The legacy blueprint path is /superset/explore_json/; with appRoot=/superset the
// full streaming URL is /superset/superset/explore_json/ (appRoot + blueprint prefix).
expect(callArgs.url).toBe('/superset/superset/explore_json/?csv=true');
// post `Superset.route_base = ""`, the blueprint path is
// `/explore_json/`. With appRoot=/superset, the prefixed URL is
// `/superset/explore_json/?csv=true` — single-prefix, not the legacy
// doubled `/superset/superset/explore_json/...` that this test used to
// pin (which was the bug, now fixed at source).
expect(callArgs.url).toBe('/superset/explore_json/?csv=true');
expect(callArgs.exportType).toBe('csv');
});
@@ -228,7 +231,7 @@ test('exportChart legacy API builds relative URL for CSV export without app root
expect(onStartStreamingExport).toHaveBeenCalledTimes(1);
const callArgs = onStartStreamingExport.mock.calls[0][0];
expect(callArgs.url).toBe('/superset/explore_json/?csv=true');
expect(callArgs.url).toBe('/explore_json/?csv=true');
});
test('exportChart legacy API builds relative URL for xlsx export', async () => {
@@ -253,7 +256,7 @@ test('exportChart legacy API builds relative URL for xlsx export', async () => {
expect(onStartStreamingExport).toHaveBeenCalledTimes(1);
const callArgs = onStartStreamingExport.mock.calls[0][0];
expect(callArgs.url).toBe('/superset/explore_json/?xlsx=true');
expect(callArgs.url).toBe('/explore_json/?xlsx=true');
});
test('exportChart legacy API calls postBlob with relative URL', async () => {
@@ -278,7 +281,7 @@ test('exportChart legacy API calls postBlob with relative URL', async () => {
expect(SupersetClient.postBlob).toHaveBeenCalledTimes(1);
const [url] = SupersetClient.postBlob.mock.calls[0];
expect(url).toBe('/superset/explore_json/?csv=true');
expect(url).toBe('/explore_json/?csv=true');
expect(url).not.toMatch(/^https?:\/\//);
expect(downloadBlob).toHaveBeenCalled();
});
@@ -305,7 +308,7 @@ test('exportChart legacy API includes force param when force=true', async () =>
expect(onStartStreamingExport).toHaveBeenCalledTimes(1);
const callArgs = onStartStreamingExport.mock.calls[0][0];
expect(callArgs.url).toBe('/superset/explore_json/?force=true&csv=true');
expect(callArgs.url).toBe('/explore_json/?force=true&csv=true');
});
test('exportChart successfully exports chart as CSV', async () => {

View File

@@ -39,7 +39,7 @@ test('Get ExploreUrl with default params', () => {
test('Get ExploreUrl with endpointType:full', () => {
const params = createParams();
expect(getExploreUrl({ ...params, endpointType: 'full' })).toBe(
'http://localhost/superset/explore_json/',
'http://localhost/explore_json/',
);
});
@@ -47,21 +47,21 @@ test('Get ExploreUrl with endpointType:full and method:GET', () => {
const params = createParams();
expect(
getExploreUrl({ ...params, endpointType: 'full', method: 'GET' }),
).toBe('http://localhost/superset/explore_json/');
).toBe('http://localhost/explore_json/');
});
test('Get relative ExploreUrl with endpointType:csv', () => {
const params = createParams();
expect(
getExploreUrl({ ...params, endpointType: 'csv', relative: true }),
).toBe('/superset/explore_json/?csv=true');
).toBe('/explore_json/?csv=true');
});
test('Get relative ExploreUrl with endpointType:xlsx', () => {
const params = createParams();
expect(
getExploreUrl({ ...params, endpointType: 'xlsx', relative: true }),
).toBe('/superset/explore_json/?xlsx=true');
).toBe('/explore_json/?xlsx=true');
});
test('Get relative ExploreUrl with force:true', () => {
@@ -73,7 +73,7 @@ test('Get relative ExploreUrl with force:true', () => {
force: true,
relative: true,
}),
).toBe('/superset/explore_json/?force=true&csv=true');
).toBe('/explore_json/?force=true&csv=true');
});
test('Get relative ExploreUrl with endpointType:base', () => {

View File

@@ -19,8 +19,12 @@
import { getURIDirectory } from '.';
test('Cases in which the "explore_json" will be returned', () => {
// post `Superset.route_base = ""` collapse the legacy
// `/superset/explore_json/` endpoint is gone; `getURIDirectory` now
// returns the bare `/explore_json/` path (appRoot is applied at the
// outer `ensureAppRoot` layer when `includeAppRoot=true`).
['full', 'json', 'csv', 'query', 'results', 'samples'].forEach(name => {
expect(getURIDirectory(name)).toBe('/superset/explore_json/');
expect(getURIDirectory(name)).toBe('/explore_json/');
});
});

View File

@@ -33,7 +33,7 @@ import {
import { availableDomains } from 'src/utils/hostNamesConfig';
import { safeStringify } from 'src/utils/safeStringify';
import { optionLabel } from 'src/utils/common';
import { ensureAppRoot } from 'src/utils/pathUtils';
import { ensureAppRoot } from 'src/utils/navigationUtils';
import { downloadBlob, getFilenameFromResponse } from 'src/utils/export';
import { URL_PARAMS } from 'src/constants';
import {
@@ -166,7 +166,7 @@ export function getURIDirectory(
'results',
'samples',
].includes(endpointType)
? '/superset/explore_json/'
? '/explore_json/'
: '/explore/';
return includeAppRoot ? ensureAppRoot(uri) : uri;
}

View File

@@ -23,6 +23,19 @@ import ExtensionsLoader from './ExtensionsLoader';
type Extension = core.Extension;
const mockApplicationRoot = jest.fn<string, []>(() => '');
jest.mock('src/utils/getBootstrapData', () => {
const actual = jest.requireActual<
typeof import('src/utils/getBootstrapData')
>('src/utils/getBootstrapData');
return {
__esModule: true,
...actual,
applicationRoot: () => mockApplicationRoot(),
};
});
function createMockExtension(overrides: Partial<Extension> = {}): Extension {
return {
id: 'test-extension',
@@ -37,8 +50,39 @@ function createMockExtension(overrides: Partial<Extension> = {}): Extension {
beforeEach(() => {
(ExtensionsLoader as any).instance = undefined;
mockApplicationRoot.mockReturnValue('');
});
/**
* Capture the src attribute of the remote-entry script element and trigger
* its onerror handler so loadModule short-circuits without webpack module
* federation globals.
*/
function captureRemoteEntryScript(): {
getSrc: () => string | null;
restore: () => void;
} {
let capturedSrc: string | null = null;
const appendChildSpy = jest
.spyOn(document.head, 'appendChild')
.mockImplementation((element: Node) => {
if (element instanceof HTMLScriptElement) {
capturedSrc = element.getAttribute('src');
if (element.onerror) {
const errorHandler = element.onerror;
setTimeout(() => {
errorHandler('Script load halted by test');
}, 0);
}
}
return element;
});
return {
getSrc: () => capturedSrc,
restore: () => appendChildSpy.mockRestore(),
};
}
test('creates a singleton instance', () => {
const instance1 = ExtensionsLoader.getInstance();
const instance2 = ExtensionsLoader.getInstance();
@@ -126,6 +170,70 @@ test('logs success after initializeExtensions completes', async () => {
infoSpy.mockRestore();
});
// Subdirectory regression (gap review 2026-06-10): the backend emits a
// router-relative remoteEntry URL; assigning it raw to `script.src` resolved
// it against the domain root, 404ing every extension under a subdirectory
// deployment.
test('prefixes a router-relative remoteEntry with the application root', async () => {
mockApplicationRoot.mockReturnValue('/superset');
const loader = ExtensionsLoader.getInstance();
const errorSpy = jest.spyOn(logging, 'error').mockImplementation();
const script = captureRemoteEntryScript();
await loader.initializeExtension(
createMockExtension({
id: 'sub-ext',
remoteEntry: '/api/v1/extensions/pub/sub-ext/remoteEntry.js',
}),
);
expect(script.getSrc()).toBe(
'/superset/api/v1/extensions/pub/sub-ext/remoteEntry.js',
);
errorSpy.mockRestore();
script.restore();
});
test('leaves the remoteEntry unprefixed on root deployments', async () => {
const loader = ExtensionsLoader.getInstance();
const errorSpy = jest.spyOn(logging, 'error').mockImplementation();
const script = captureRemoteEntryScript();
await loader.initializeExtension(
createMockExtension({
id: 'root-ext',
remoteEntry: '/api/v1/extensions/pub/root-ext/remoteEntry.js',
}),
);
expect(script.getSrc()).toBe(
'/api/v1/extensions/pub/root-ext/remoteEntry.js',
);
errorSpy.mockRestore();
script.restore();
});
test('passes an absolute remoteEntry URL through unchanged', async () => {
mockApplicationRoot.mockReturnValue('/superset');
const loader = ExtensionsLoader.getInstance();
const errorSpy = jest.spyOn(logging, 'error').mockImplementation();
const script = captureRemoteEntryScript();
await loader.initializeExtension(
createMockExtension({
id: 'cdn-ext',
remoteEntry: 'https://cdn.example.com/remoteEntry.js',
}),
);
expect(script.getSrc()).toBe('https://cdn.example.com/remoteEntry.js');
errorSpy.mockRestore();
script.restore();
});
test('logs error when initializeExtensions fails', async () => {
const loader = ExtensionsLoader.getInstance();
const errorSpy = jest.spyOn(logging, 'error').mockImplementation();

View File

@@ -19,6 +19,7 @@
import { SupersetClient } from '@superset-ui/core';
import { logging } from '@apache-superset/core/utils';
import type { common as core } from '@apache-superset/core';
import { makeUrl } from 'src/utils/navigationUtils';
type Extension = core.Extension;
@@ -107,10 +108,12 @@ class ExtensionsLoader {
private async loadModule(extension: Extension): Promise<void> {
const { remoteEntry, id } = extension;
// Load the remote entry script
// Load the remote entry script. The backend emits a router-relative
// remoteEntry URL; `makeUrl` applies the application root for
// subdirectory deployments (idempotent, and absolute URLs pass through).
await new Promise<void>((resolve, reject) => {
const element = document.createElement('script');
element.src = remoteEntry;
element.src = makeUrl(remoteEntry);
element.type = 'text/javascript';
element.async = true;
element.onload = () => resolve();

View File

@@ -1427,7 +1427,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
const openDashboardInNewTab = (dashboardId?: number | string | null) => {
if (!dashboardId) return;
navigateTo(`/superset/dashboard/${dashboardId}`, { newWindow: true });
navigateTo(`/dashboard/${dashboardId}/`, { newWindow: true });
};
const onChartChange = (chart: SelectValue) => {

View File

@@ -0,0 +1,152 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { render } from 'spec/helpers/testing-library';
import type { TaggedObjects } from 'src/types/TaggedObject';
import AllEntitiesTable from './AllEntitiesTable';
// Regression for the tag list page (sc-108439): backend `.url` properties on
// Dashboard / Slice / SavedQuery are router-relative by contract
// (see `superset/models/dashboard.py` `dashboard_link` docstring: "`Dashboard.url`
// itself stays router-relative so frontend callers can apply ensureAppRoot
// exactly once"). The TagDAO emits those router-relative paths verbatim as
// `o.url` on tagged-object responses. AllEntitiesTable therefore must wrap
// `o.url` with `ensureAppRoot` exactly once; otherwise the row hrefs lack
// the `SUPERSET_APP_ROOT` prefix and clicks land on broken links.
//
// Mocking note (same as SliceHeaderControls.subdirectory.test.tsx): the
// component is imported statically, so the `withApplicationRoot` fixture's
// `jest.resetModules()` + dynamic re-import pattern can't retroactively
// change which `getBootstrapData` instance the already-bound module graph
// sees. We mock `src/utils/getBootstrapData` directly with a reconfigurable
// `mockApplicationRoot` factory and flip it per scenario.
//
// Name must start with `mock` so Jest's hoisted jest.mock() factory may
// reference it. `default` returns a STATIC shape (not mockApplicationRoot)
// because consumers like the reducer chain call getBootstrapData() at
// import time — calling mockApplicationRoot inside `default` hits TDZ.
const mockApplicationRoot = jest.fn<string, []>(() => '');
jest.mock('src/utils/getBootstrapData', () => ({
__esModule: true,
default: () => ({
common: { application_root: '', static_assets_prefix: '' },
}),
applicationRoot: () => mockApplicationRoot(),
staticAssetsPrefix: () => '',
}));
const BACKEND_URLS = {
dashboard: '/dashboard/sales/',
chart: '/explore/?slice_id=42',
query: '/sqllab?savedQueryId=7',
};
const SAMPLE_OBJECTS: TaggedObjects = {
dashboard: [
{
id: 1,
type: 'dashboard',
name: 'Sales',
url: BACKEND_URLS.dashboard,
changed_on: '2026-06-01T00:00:00Z',
created_by: 1,
creator: 'admin',
owners: [],
tags: [],
},
],
chart: [
{
id: 42,
type: 'chart',
name: 'Top Customers',
url: BACKEND_URLS.chart,
changed_on: '2026-06-01T00:00:00Z',
created_by: 1,
creator: 'admin',
owners: [],
tags: [],
},
],
query: [
{
id: 7,
type: 'query',
name: 'Daily Revenue',
url: BACKEND_URLS.query,
changed_on: '2026-06-01T00:00:00Z',
created_by: 1,
creator: 'admin',
owners: [],
tags: [],
},
],
};
const renderAndCollectHrefs = (): string[] => {
const { container } = render(
<AllEntitiesTable
objects={SAMPLE_OBJECTS}
setShowTagModal={() => {}}
canEditTag
/>,
{ useRedux: true, useTheme: true },
);
return Array.from(container.querySelectorAll<HTMLAnchorElement>('a[href]'))
.map(a => a.getAttribute('href') ?? '')
.filter(href => href !== '' && href !== '#');
};
beforeEach(() => {
mockApplicationRoot.mockReset();
});
test('row hrefs carry the application root under /superset', () => {
mockApplicationRoot.mockReturnValue('/superset');
const hrefs = renderAndCollectHrefs();
expect(hrefs).toEqual(
expect.arrayContaining([
`/superset${BACKEND_URLS.dashboard}`,
`/superset${BACKEND_URLS.chart}`,
`/superset${BACKEND_URLS.query}`,
]),
);
hrefs.forEach(href => {
expect(href).not.toMatch(/^\/superset\/superset\//);
expect(href.startsWith('/superset')).toBe(true);
});
});
test('row hrefs are unchanged under the default root-of-domain deployment', () => {
mockApplicationRoot.mockReturnValue('');
const hrefs = renderAndCollectHrefs();
expect(hrefs).toEqual(
expect.arrayContaining([
BACKEND_URLS.dashboard,
BACKEND_URLS.chart,
BACKEND_URLS.query,
]),
);
hrefs.forEach(href => {
expect(href).not.toMatch(/^\/superset/);
});
});

View File

@@ -27,6 +27,7 @@ import { EmptyState } from '@superset-ui/core/components';
import { FacePile, TagsList, type TagType } from 'src/components';
import { TaggedObject, TaggedObjects } from 'src/types/TaggedObject';
import { Typography } from '@superset-ui/core/components/Typography';
import { ensureAppRoot } from 'src/utils/navigationUtils';
const MAX_TAGS_TO_SHOW = 3;
const PAGE_SIZE = 10;
@@ -73,7 +74,9 @@ export default function AllEntitiesTable({
const renderTable = (type: objectType) => {
const data = objects[type].map((o: TaggedObject) => ({
[type]: <Typography.Link href={o.url}>{o.name}</Typography.Link>,
[type]: (
<Typography.Link href={ensureAppRoot(o.url)}>{o.name}</Typography.Link>
),
modified: o.changed_on ? extendedDayjs.utc(o.changed_on).fromNow() : '',
tags: o.tags,
owners: o.owners,

View File

@@ -0,0 +1,160 @@
/**
* 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.
*/
// Subdirectory regression coverage for the DatabaseModal "post-connection"
// call-to-action buttons (Create dataset + Query data in SQL Lab).
//
// Original bug class: under a subdir deployment (`applicationRoot = '/superset'`)
// the "Query data in SQL Lab" button navigated to a double-prefixed URL
// (`/superset/superset/sqllab?db=true`). The fix routes both CTA buttons
// through `redirectURL`, which delegates to `useHistory().push(...)`. React
// Router's `BrowserRouter basename={applicationRoot()}` re-applies the prefix
// internally, so the argument to `history.push` MUST be a router-relative
// path (no leading `${applicationRoot}` and no `ensureAppRoot` wrap).
//
// Reaching `renderCTABtns()` through a rendered modal requires walking the
// full "select engine → fill SQLAlchemy form → submit → wait for success"
// flow with every fetch mocked. The contract under test is much smaller
// than that surface, so this file uses the same source-pin + pure-logic
// characterisation pattern as
// `dashboard/components/nativeFilters/FilterBar/FilterBar.subdirectory.test.ts`.
import { readFileSync } from 'fs';
import { join } from 'path';
const MODAL_SRC = readFileSync(join(__dirname, 'index.tsx'), 'utf8');
// ---------------------------------------------------------------------------
// Source-pin: redirectURL receives router-relative paths.
// ---------------------------------------------------------------------------
test('DatabaseModal redirectURL delegates to history.push', () => {
// The redirectURL helper is the single funnel for post-connection
// navigation. Both CTA buttons call it; the only safe argument shape is
// a router-relative path, because BrowserRouter's basename re-applies the
// app root. Pinning the helper body here means a future refactor that
// re-introduces `applicationRoot()` or `ensureAppRoot` at this layer
// fails loudly with the exact callsite.
expect(MODAL_SRC).toMatch(
/const redirectURL = \(url: string\) => \{\s*history\.push\(url\);\s*\};/,
);
});
test('DatabaseModal "Query data in SQL Lab" pushes a router-relative /sqllab', () => {
// The exact string we want in source. If someone "fixes" subdir support
// by wrapping this in ensureAppRoot or hard-coding `/superset/sqllab`,
// this test fires before the bad change ships.
expect(MODAL_SRC).toContain("redirectURL('/sqllab?db=true')");
});
test('DatabaseModal "Create dataset" pushes a router-relative /dataset/add/', () => {
// Symmetric invariant for the sibling CTA button — same risk class
// (basename double-prefix) applies. Pinning prevents drift where someone
// "fixes" one button and leaves the other inconsistent.
expect(MODAL_SRC).toContain("redirectURL('/dataset/add/')");
});
test('DatabaseModal CTA buttons do NOT prefix the app root themselves', () => {
// The buttons must not call applicationRoot()/ensureAppRoot/makeUrl on
// these specific paths — basename handles the prefix, and any extra
// prefixing produces `/superset/superset/...`. Search the renderCTABtns
// block (lines ~1855-1879 at time of writing) for offending patterns.
const ctaMatch = MODAL_SRC.match(
/const renderCTABtns = \(\) =>[\s\S]*?<\/StyledBtns>\s*\);/,
);
expect(ctaMatch).not.toBeNull();
const ctaSrc = ctaMatch![0];
expect(ctaSrc).not.toMatch(/applicationRoot\s*\(/);
expect(ctaSrc).not.toMatch(/ensureAppRoot\s*\(/);
expect(ctaSrc).not.toMatch(/makeUrl\s*\(/);
// And the specific double-prefixed string the original bug produced —
// pin it so a regression that hard-codes the app root never ships.
expect(ctaSrc).not.toContain('/superset/sqllab');
expect(ctaSrc).not.toContain('/superset/dataset/add');
});
// ---------------------------------------------------------------------------
// Characterisation: the documented invariant exercised across app-roots.
// ---------------------------------------------------------------------------
//
// Re-implements the behaviour the source-pin describes: a router-relative
// path pushed through history under BrowserRouter's basename produces a
// single-prefixed URL. If the documented invariant itself is wrong the
// source-pin is useless; this matrix catches that.
interface Scenario {
description: string;
basename: string;
pushArg: string;
expected: string;
}
// Mirror of how React Router composes basename + pushed path. React Router
// requires the pushed path to start with '/' and concatenates basename in
// front (stripping a trailing slash on basename if present).
function composeUnderBasename(basename: string, pushArg: string): string {
const normalisedBase =
basename === '/' || basename === '' ? '' : basename.replace(/\/+$/, '');
return `${normalisedBase}${pushArg}`;
}
const SCENARIOS: ReadonlyArray<Scenario> = [
{
description: 'root deploy: bare basename + /sqllab?db=true',
basename: '',
pushArg: '/sqllab?db=true',
expected: '/sqllab?db=true',
},
{
description: 'subdir deploy: /superset + /sqllab?db=true',
basename: '/superset',
pushArg: '/sqllab?db=true',
expected: '/superset/sqllab?db=true',
},
{
description: 'subdir deploy: /superset + /dataset/add/',
basename: '/superset',
pushArg: '/dataset/add/',
expected: '/superset/dataset/add/',
},
{
description: 'deep-nested deploy: /tenant-a/superset + /sqllab?db=true',
basename: '/tenant-a/superset',
pushArg: '/sqllab?db=true',
expected: '/tenant-a/superset/sqllab?db=true',
},
{
description: 'subdir deploy: trailing slash on basename collapses cleanly',
basename: '/superset/',
pushArg: '/sqllab?db=true',
expected: '/superset/sqllab?db=true',
},
];
test.each(SCENARIOS)(
'redirectURL navigation: $description',
({ basename, pushArg, expected }: Scenario) => {
const url = composeUnderBasename(basename, pushArg);
expect(url).toBe(expected);
// The dedupe contract: no `/superset/superset/...` ever reaches the
// browser. Even if the source-pin drifts, this catches the user-visible
// symptom for the subdir case.
expect(url).not.toMatch(/\/superset\/superset\//);
},
);

View File

@@ -0,0 +1,114 @@
/**
* 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 userEvent from '@testing-library/user-event';
import { render, screen } from 'spec/helpers/testing-library';
// DatasetPanel opens the dataset via openInNewTab -> ensureAppRoot, which reads
// applicationRoot() statically. Intercept it to simulate a subdirectory deploy.
const mockApplicationRoot = jest.fn<string, []>(() => '');
jest.mock('src/utils/getBootstrapData', () => {
const actual = jest.requireActual<
typeof import('src/utils/getBootstrapData')
>('src/utils/getBootstrapData');
return {
__esModule: true,
default: actual.default,
applicationRoot: () => mockApplicationRoot(),
staticAssetsPrefix: actual.staticAssetsPrefix,
};
});
// eslint-disable-next-line import/first
import DatasetPanel from 'src/features/datasets/AddDataset/DatasetPanel/DatasetPanel';
// eslint-disable-next-line import/first
import { exampleColumns } from './fixtures';
// eslint-disable-next-line import/first
import { DatasetObject } from 'src/features/datasets/AddDataset/types';
const APP_ROOT = '/superset';
let openSpy: jest.SpyInstance;
beforeEach(() => {
mockApplicationRoot.mockReturnValue(APP_ROOT);
openSpy = jest.spyOn(window, 'open').mockImplementation(() => null);
});
afterEach(() => {
jest.restoreAllMocks();
});
const datasetWith = (exploreUrl: string): DatasetObject[] => [
{
db: {
id: 1,
database_name: 'test_database',
owners: [1],
backend: 'test_backend',
},
schema: 'test_schema',
dataset_name: 'example_dataset',
table_name: 'example_table',
explore_url: exploreUrl,
},
];
test('View Dataset opens a single-prefixed URL under a subdirectory deployment', async () => {
// The backend emits explore_url already carrying the application root. The
// strip must run before ensureAppRoot re-prefixes, otherwise the opened tab
// would point at a doubled /superset/superset/... path.
render(
<DatasetPanel
tableName="example_table"
hasError={false}
columnList={exampleColumns}
loading={false}
datasets={datasetWith(`${APP_ROOT}/explore/?datasource=1__table`)}
/>,
{ useRouter: true },
);
await userEvent.click(screen.getByText('View Dataset'));
expect(openSpy).toHaveBeenCalledTimes(1);
const openedUrl = openSpy.mock.calls[0][0];
expect(openedUrl).toBe(`${APP_ROOT}/explore/?datasource=1__table`);
expect(openedUrl).not.toContain('/superset/superset');
});
test('View Dataset passes an external explore_url through unprefixed', async () => {
render(
<DatasetPanel
tableName="example_table"
hasError={false}
columnList={exampleColumns}
loading={false}
datasets={datasetWith('https://external.example.com/custom-endpoint')}
/>,
{ useRouter: true },
);
await userEvent.click(screen.getByText('View Dataset'));
expect(openSpy).toHaveBeenCalledTimes(1);
expect(openSpy.mock.calls[0][0]).toBe(
'https://external.example.com/custom-endpoint',
);
});

View File

@@ -26,6 +26,7 @@ import Table, {
TableSize,
} from '@superset-ui/core/components/Table';
import { DatasetObject } from 'src/features/datasets/AddDataset/types';
import { openInNewTab, stripAppRoot } from 'src/utils/navigationUtils';
import { ITableColumn } from './types';
import MessageContent from './MessageContent';
@@ -227,11 +228,12 @@ const renderExistingDatasetAlert = (dataset?: DatasetObject) => (
<span
role="button"
onClick={() => {
window.open(
dataset?.explore_url,
'_blank',
'noreferrer noopener popup=false',
);
if (dataset?.explore_url) {
// `explore_url` is router-relative from the backend (rooted under
// a subdirectory deployment); strip the root so openInNewTab's
// ensureAppRoot re-prefixes it once rather than doubling it.
openInNewTab(stripAppRoot(dataset.explore_url));
}
}}
tabIndex={0}
className="view-dataset-button"

View File

@@ -29,8 +29,8 @@ import {
DatasetObject,
} from 'src/features/datasets/AddDataset/types';
import { Table } from 'src/hooks/apiResources';
import { ensureAppRoot } from 'src/utils/pathUtils';
import { Typography } from '@superset-ui/core/components/Typography';
import { ensureAppRoot } from 'src/utils/navigationUtils';
interface LeftPanelProps {
setDataset: Dispatch<SetStateAction<object>>;

View File

@@ -0,0 +1,230 @@
/**
* 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 * as reactRedux from 'react-redux';
import fetchMock from 'fetch-mock';
import { BrowserRouter } from 'react-router-dom';
import { QueryParamProvider } from 'use-query-params';
import { ReactRouter5Adapter } from 'use-query-params/adapters/react-router-5';
import { render, screen } from 'spec/helpers/testing-library';
import * as CoreTheme from '@apache-superset/core/theme';
// Menu's brand link statically reads applicationRoot(); intercept it to
// simulate a subdirectory deployment.
const mockApplicationRoot = jest.fn<string, []>(() => '');
jest.mock('src/utils/getBootstrapData', () => {
const actual = jest.requireActual<
typeof import('src/utils/getBootstrapData')
>('src/utils/getBootstrapData');
return {
__esModule: true,
default: actual.default,
applicationRoot: () => mockApplicationRoot(),
staticAssetsPrefix: actual.staticAssetsPrefix,
};
});
jest.mock('@apache-superset/core/theme', () => ({
...jest.requireActual('@apache-superset/core/theme'),
useTheme: jest.fn(),
}));
jest.mock('antd', () => ({
...jest.requireActual('antd'),
Grid: {
...jest.requireActual('antd').Grid,
useBreakpoint: () => ({ md: true }),
},
}));
// IMPORTANT: unlike Menu.test.tsx this file does NOT mock GenericLink. It
// renders the real react-router-dom <Link> under a real
// `<BrowserRouter basename={APP_ROOT}>` — the same seam production builds via
// `<Router basename={applicationRoot()}>` in src/views/App.tsx. BrowserRouter
// honours basename in createHref and re-prepends the root WITHOUT deduping, so
// this exercises the actual rendered href the user clicks. The seam mock in
// Menu.test.tsx (no basename) is blind to that re-prepend; this file covers it.
// eslint-disable-next-line import/first
import { Menu } from './Menu';
const APP_ROOT = '/superset';
const user = {
createdOn: '2021-04-27T18:12:38.952304',
email: 'admin',
firstName: 'admin',
isActive: true,
lastName: 'admin',
permissions: {},
roles: {
Admin: [
['can_sqllab', 'Superset'],
['can_write', 'Dashboard'],
['can_write', 'Chart'],
],
},
userId: 1,
username: 'admin',
};
const mockedProps = {
user,
data: {
menu: [],
brand: {
path: '/superset/welcome/',
icon: '/static/assets/images/superset-logo-horiz.png',
alt: 'Apache Superset',
width: '126',
tooltip: '',
text: '',
},
environment_tag: {
text: 'Production',
color: '#000',
},
navbar_right: {
show_watermark: false,
bug_report_url: '/report/',
documentation_url: '/docs/',
languages: {
en: { flag: 'us', name: 'English', url: '/lang/en' },
},
show_language_picker: true,
user_is_anonymous: true,
user_info_url: '/users/userinfo/',
user_logout_url: '/logout/',
user_login_url: '/login/',
locale: 'en',
version_string: '1.0.0',
version_sha: 'randomSHA',
build_number: 'randomBuildNumber',
},
settings: [],
},
};
const useSelectorMock = jest.spyOn(reactRedux, 'useSelector');
const useThemeMock = CoreTheme.useTheme as jest.Mock;
fetchMock.get(
'glob:*api/v1/database/?q=(filters:!((col:allow_file_upload,opr:upload_is_enabled,value:!t)))',
{},
);
const renderUnderSubdirectory = () =>
render(
// Mirror production's `<Router basename={applicationRoot()}>`
// (views/App.tsx). BrowserRouter honours basename in createHref, so a
// router-relative `to` is re-prefixed exactly once — the seam the brand
// link's strip must feed correctly.
<BrowserRouter basename={APP_ROOT}>
<QueryParamProvider adapter={ReactRouter5Adapter}>
<Menu {...mockedProps} />
</QueryParamProvider>
</BrowserRouter>,
{ useRedux: true, useTheme: true },
);
beforeEach(() => {
mockApplicationRoot.mockReturnValue(APP_ROOT);
useSelectorMock.mockReturnValue({ roles: user.roles });
// Keep the page URL under the basename so BrowserRouter is consistent.
window.history.replaceState({}, '', `${APP_ROOT}/welcome/`);
});
afterEach(() => {
window.history.replaceState({}, '', '/');
useSelectorMock.mockClear();
jest.restoreAllMocks();
});
test('brand logo link is single-prefixed when brandLogoHref arrives already rooted', async () => {
// The backend emits brandLogoHref already carrying the app root. The Router
// basename re-prepends the root, so the strip must run first to avoid a
// doubled /superset/superset/... href in the rendered anchor.
useThemeMock.mockReturnValue({
...CoreTheme.supersetTheme,
brandLogoUrl: '/static/assets/images/custom-logo.png',
brandLogoHref: '/superset/welcome/',
});
renderUnderSubdirectory();
const brandLink = await screen.findByRole('link', {
name: /apache superset/i,
});
expect(brandLink).toHaveAttribute('href', '/superset/welcome/');
expect(brandLink.getAttribute('href')).not.toContain('/superset/superset');
});
test('brand logo link is single-prefixed when brandLogoHref is a bare path', async () => {
// A bare (un-rooted) brandLogoHref must end up prefixed exactly once after
// the Router basename re-prepend.
useThemeMock.mockReturnValue({
...CoreTheme.supersetTheme,
brandLogoUrl: '/static/assets/images/custom-logo.png',
brandLogoHref: '/welcome/',
});
renderUnderSubdirectory();
const brandLink = await screen.findByRole('link', {
name: /apache superset/i,
});
expect(brandLink).toHaveAttribute('href', '/superset/welcome/');
expect(brandLink.getAttribute('href')).not.toContain('/superset/superset');
});
test('brand logo link renders without crashing when brandLogoHref is unset (partial theme override)', async () => {
// A partial theme override can set brandLogoUrl (the image) while leaving
// brandLogoHref undefined. The internal branch must stay null-safe: stripping
// the ensureAppRoot'd value (which falls back to the app root for an undefined
// input) avoids a `undefined.startsWith` TypeError that would white-screen the
// nav chrome. The rendered href resolves to the app root, never doubled.
useThemeMock.mockReturnValue({
...CoreTheme.supersetTheme,
brandLogoUrl: '/static/assets/images/custom-logo.png',
brandLogoHref: undefined,
});
renderUnderSubdirectory();
const brandLink = await screen.findByRole('link', {
name: /apache superset/i,
});
expect(brandLink.getAttribute('href')).toMatch(/^\/superset\/?$/);
expect(brandLink.getAttribute('href')).not.toContain('/superset/superset');
});
test('external brandLogoHref passes through without app-root prefixing', async () => {
useThemeMock.mockReturnValue({
...CoreTheme.supersetTheme,
brandLogoUrl: '/static/assets/images/custom-logo.png',
brandLogoHref: 'https://external.example.com',
});
renderUnderSubdirectory();
const brandLink = await screen.findByRole('link', {
name: /apache superset/i,
});
expect(brandLink).toHaveAttribute('href', 'https://external.example.com');
});

View File

@@ -25,6 +25,35 @@ import * as CoreTheme from '@apache-superset/core/theme';
import { Menu } from './Menu';
import * as getBootstrapData from 'src/utils/getBootstrapData';
// Capture what `<GenericLink to={...}>` receives so the SPA-route regression
// tests can assert on the value handed to react-router-dom (which applies its
// own basename in production via `<Router basename={applicationRoot()}>` in
// src/views/App.tsx). The test harness's `<BrowserRouter>` has no basename,
// so asserting on the rendered `<a href>` wouldn't catch the double-prefix.
let observedGenericLinkTo: unknown = null;
jest.mock('src/components/GenericLink', () => ({
__esModule: true,
GenericLink: ({
to,
children,
...rest
}: {
to: unknown;
children: React.ReactNode;
[k: string]: unknown;
}) => {
observedGenericLinkTo = to;
return (
<a
href={typeof to === 'string' ? to : '#'}
{...(rest as Record<string, unknown>)}
>
{children}
</a>
);
},
}));
jest.mock('@apache-superset/core/theme', () => ({
...jest.requireActual('@apache-superset/core/theme'),
useTheme: jest.fn(),
@@ -797,6 +826,161 @@ test('brand link falls back to brand.path when theme brandLogoUrl is absent', as
expect(brandLink).toHaveAttribute('href', '/superset/welcome/');
});
// Regression: the real backend emits `brand.path` and `brand.icon` already
// carrying the app root (because they pass through `url_for`). The frontend
// must not double-prefix them — neither via
// `ensureAppRoot`/`ensureStaticPrefix` nor via React Router's `basename`
// re-prepend.
//
// In production the SPA-route branch goes through `<GenericLink to={...}> ->
// react-router-dom <Link>`, and the Router's `basename={applicationRoot()}`
// (src/views/App.tsx) re-prepends the app root to the rendered `href`. The
// test harness's `<BrowserRouter>` has no basename, so asserting on the
// rendered `<a href>` wouldn't catch the bug. Instead we mock `GenericLink`
// at module load (top of this file) and assert the path *handed to it*
// already has the root stripped — the value the production Router will then
// safely re-prepend.
describe('brand link single-prefix regressions (subdirectory deployment)', () => {
beforeEach(() => {
observedGenericLinkTo = null;
});
test('brand link hands a root-stripped path to GenericLink when brand.path arrives already rooted (SPA route)', async () => {
applicationRootMock.mockReturnValue('/superset');
staticAssetsPrefixMock.mockReturnValue('/superset');
useSelectorMock.mockReturnValue({ roles: user.roles });
const propsWithRootedBrand = {
...mockedProps,
isFrontendRoute: () => true,
data: {
...mockedProps.data,
brand: {
...mockedProps.data.brand,
path: '/superset/welcome/',
icon: '/superset/static/assets/images/superset-logo-horiz.png',
},
},
};
render(<Menu {...propsWithRootedBrand} />, {
useRedux: true,
useQueryParams: true,
useRouter: true,
useTheme: true,
});
// Wait for the mocked GenericLink to render.
await screen.findByRole('link', {
name: new RegExp(propsWithRootedBrand.data.brand.alt, 'i'),
});
expect(observedGenericLinkTo).toBe('/welcome/');
});
test('brand link is single-prefix when brand.path arrives already rooted (non-SPA route)', async () => {
applicationRootMock.mockReturnValue('/superset');
staticAssetsPrefixMock.mockReturnValue('/superset');
useSelectorMock.mockReturnValue({ roles: user.roles });
const propsWithRootedBrand = {
...mockedProps,
isFrontendRoute: () => false,
data: {
...mockedProps.data,
brand: {
...mockedProps.data.brand,
path: '/superset/welcome/',
icon: '/superset/static/assets/images/superset-logo-horiz.png',
},
},
};
render(<Menu {...propsWithRootedBrand} />, {
useRedux: true,
useQueryParams: true,
useRouter: true,
useTheme: true,
});
const brandLink = await screen.findByRole('link', {
name: new RegExp(propsWithRootedBrand.data.brand.alt, 'i'),
});
expect(brandLink).toHaveAttribute('href', '/superset/welcome/');
const brandImg = brandLink.querySelector('img');
expect(brandImg).toHaveAttribute(
'src',
'/superset/static/assets/images/superset-logo-horiz.png',
);
});
test('brand link strips a nested application root before handing to GenericLink', async () => {
applicationRootMock.mockReturnValue('/preset/superset');
staticAssetsPrefixMock.mockReturnValue('/preset/superset');
useSelectorMock.mockReturnValue({ roles: user.roles });
const propsWithRootedBrand = {
...mockedProps,
isFrontendRoute: () => true,
data: {
...mockedProps.data,
brand: {
...mockedProps.data.brand,
path: '/preset/superset/welcome/',
icon: '/preset/superset/static/assets/images/superset-logo-horiz.png',
},
},
};
render(<Menu {...propsWithRootedBrand} />, {
useRedux: true,
useQueryParams: true,
useRouter: true,
useTheme: true,
});
await screen.findByRole('link', {
name: new RegExp(propsWithRootedBrand.data.brand.alt, 'i'),
});
expect(observedGenericLinkTo).toBe('/welcome/');
});
test('brand link from theme.brandLogoHref hands a root-stripped path to GenericLink when already rooted', async () => {
applicationRootMock.mockReturnValue('/superset');
staticAssetsPrefixMock.mockReturnValue('/superset');
useSelectorMock.mockReturnValue({ roles: user.roles });
useThemeMock.mockReturnValue({
...CoreTheme.supersetTheme,
brandLogoUrl: '/superset/static/assets/images/custom-logo.png',
brandLogoHref: '/superset/welcome/',
});
render(<Menu {...mockedProps} />, {
useRedux: true,
useQueryParams: true,
useRouter: true,
useTheme: true,
});
const brandLink = await screen.findByRole('link', {
name: /apache superset/i,
});
// The internal brand logo link goes through StyledBrandLink -> GenericLink
// -> react-router <Link>. The production Router re-prepends the app root, so
// the value handed to GenericLink must already be stripped to avoid a
// doubled /superset/superset/... href. (Real-basename coverage of the
// resulting rendered href lives in Menu.subdirectory.test.tsx.)
expect(observedGenericLinkTo).toBe('/welcome/');
expect(brandLink).toHaveAttribute('href', '/welcome/');
const brandImg = brandLink.querySelector('img');
expect(brandImg).toHaveAttribute(
'src',
'/superset/static/assets/images/custom-logo.png',
);
});
});
// --- Active tab highlighting (regression tests for issue #36403) ---
//
// The active top-level tab is highlighted by matching the current route to a

View File

@@ -20,7 +20,7 @@ import { useState, useEffect } from 'react';
import { styled, css, useTheme } from '@apache-superset/core/theme';
import { t } from '@apache-superset/core/translation';
import { ensureStaticPrefix } from 'src/utils/assetUrl';
import { ensureAppRoot } from 'src/utils/pathUtils';
import { ensureAppRoot, stripAppRoot } from 'src/utils/navigationUtils';
import { getUrlParam, isUrlExternal } from 'src/utils/urlUtils';
import { MainNav, MenuItem } from '@superset-ui/core/components/Menu';
import { Tooltip, Grid, Row, Col, Image } from '@superset-ui/core/components';
@@ -258,10 +258,18 @@ export function Menu({
// of the localized label. Fall back to the label when no name is provided.
const key = name ?? label;
if (url && isFrontendRoute) {
// `<Router basename={applicationRoot()}>` re-prepends the app root to
// `to`, so handing it the already-rooted `url` from bootstrap_data
// would render a doubled `/superset/superset/...` anchor. Strip the
// root first; mirrors the brand-link treatment below.
return {
key,
label: (
<NavLink role="button" to={url} activeClassName="is-active">
<NavLink
role="button"
to={stripAppRoot(url)}
activeClassName="is-active"
>
{label}
</NavLink>
),
@@ -288,7 +296,11 @@ export function Menu({
// collide with that parent. Fall back to the label when no name.
key: child.name ?? `${child.label}`,
label: child.isFrontendRoute ? (
<NavLink to={child.url || ''} exact activeClassName="is-active">
<NavLink
to={stripAppRoot(child.url || '')}
exact
activeClassName="is-active"
>
{child.label}
</NavLink>
) : (
@@ -327,7 +339,18 @@ export function Menu({
{brandImage}
</Typography.Link>
) : (
<StyledBrandLink to={brandHref}>{brandImage}</StyledBrandLink>
// StyledBrandLink wraps GenericLink -> react-router <Link>, and
// `<Router basename={applicationRoot()}>` re-prepends the app root
// to `to`. Strip the root so the rendered anchor is single-prefixed
// rather than a doubled `/superset/superset/...`. Strip `brandHref`
// (the ensureAppRoot'd value) rather than the raw
// `theme.brandLogoHref` so an unset href (partial theme override)
// stays null-safe — `ensureAppRoot(undefined)` yields the app root,
// which `stripAppRoot` then reduces to `/`. Mirrors the brand.path
// branch's single-prefix treatment.
<StyledBrandLink to={stripAppRoot(brandHref)}>
{brandImage}
</StyledBrandLink>
)}
</StyledBrandWrapper>
);
@@ -335,8 +358,12 @@ export function Menu({
// ---------------------------------------------------------------------------------
// TODO: deprecate this once Theme is fully rolled out
// Kept as is for backwards compatibility with the old theme system / superset_config.py
//
// `<Router basename={applicationRoot()}>` re-prepends the app root to the
// `to` prop, so handing it an already-rooted `brand.path` would render a
// doubled `/superset/superset/...` href. Strip the root first.
link = (
<GenericLink className="navbar-brand" to={brand.path}>
<GenericLink className="navbar-brand" to={stripAppRoot(brand.path)}>
<StyledImage
preview={false}
src={ensureStaticPrefix(brand.icon)}

View File

@@ -26,6 +26,7 @@ import {
} from 'spec/helpers/testing-library';
import { isFeatureEnabled, FeatureFlag, CACHE_KEY } from '@superset-ui/core';
import { isEmbedded } from 'src/dashboard/util/isEmbedded';
import * as getBootstrapData from 'src/utils/getBootstrapData';
import RightMenu from './RightMenu';
import { GlobalMenuDataOptions, RightMenuProps } from './types';
@@ -495,3 +496,66 @@ test('hides logout button when embedded and flag is enabled', async () => {
userEvent.hover(await screen.findByText(/Settings/i));
expect(screen.queryByText('Logout')).not.toBeInTheDocument();
});
test('Info link href is single-prefixed under subdirectory deployment', async () => {
// Backend emits a bare leading-slash path (`/user_info/` or `/users/userinfo/`).
// RightMenu wraps it with ensureAppRoot, which reads applicationRoot()
// dynamically. Under SUPERSET_APP_ROOT=/superset the rendered href must
// be exactly `/superset/users/userinfo/` — not `/users/userinfo/` (no
// prefix → 404) or `/superset/superset/users/userinfo/` (double prefix).
const applicationRootSpy = jest
.spyOn(getBootstrapData, 'applicationRoot')
.mockReturnValue('/superset');
try {
resetUseSelectorMock();
render(<RightMenu {...createProps()} />, {
useRedux: true,
useQueryParams: true,
useRouter: true,
useTheme: true,
});
userEvent.hover(await screen.findByText(/Settings/i));
const infoLink = await screen.findByText('Info');
expect(infoLink.closest('a')).toHaveAttribute(
'href',
'/superset/users/userinfo/',
);
} finally {
applicationRootSpy.mockRestore();
}
});
test('Logout link href is single-prefixed under subdirectory deployment', async () => {
// The logout URL is built by Flask-AppBuilder's get_url_for_logout, which
// is SCRIPT_NAME-aware and returns `/superset/logout/` under app_root.
// The frontend then routes it through ensureAppRoot, whose idempotence
// contract (see pathUtils.parity.test.ts) must prevent doubling.
const applicationRootSpy = jest
.spyOn(getBootstrapData, 'applicationRoot')
.mockReturnValue('/superset');
try {
const props = createProps();
// Mirror the SCRIPT_NAME-prefixed value the backend would emit under
// APPLICATION_ROOT=/superset.
props.navbarRight.user_logout_url = '/superset/logout/';
resetUseSelectorMock();
render(<RightMenu {...props} />, {
useRedux: true,
useQueryParams: true,
useRouter: true,
useTheme: true,
});
userEvent.hover(await screen.findByText(/Settings/i));
const logoutLink = await screen.findByText('Logout');
expect(logoutLink.closest('a')).toHaveAttribute(
'href',
'/superset/logout/',
);
} finally {
applicationRootSpy.mockRestore();
}
});

View File

@@ -53,7 +53,7 @@ import {
TelemetryPixel,
} from '@superset-ui/core/components';
import type { ItemType, MenuItem } from '@superset-ui/core/components/Menu';
import { ensureAppRoot } from 'src/utils/pathUtils';
import { ensureAppRoot, stripAppRoot } from 'src/utils/navigationUtils';
import { isEmbedded } from 'src/dashboard/util/isEmbedded';
import { findPermission } from 'src/utils/findPermission';
import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
@@ -427,7 +427,7 @@ const RightMenu = ({
items.push({
key: menu.label,
label: isFrontendRoute(menu.url) ? (
<Link to={menu.url || ''}>{menu.label}</Link>
<Link to={stripAppRoot(menu.url || '')}>{menu.label}</Link>
) : (
<Typography.Link href={ensureAppRoot(menu.url || '')}>
{menu.label}
@@ -443,7 +443,7 @@ const RightMenu = ({
items.push({
key: menu.label,
label: isFrontendRoute(menu.url) ? (
<Link to={menu.url || ''}>{menu.label}</Link>
<Link to={stripAppRoot(menu.url || '')}>{menu.label}</Link>
) : (
<Typography.Link href={ensureAppRoot(menu.url || '')}>
{menu.label}
@@ -478,7 +478,9 @@ const RightMenu = ({
sectionItems.push({
key: child.label,
label: isFrontendRoute(child.url) ? (
<Link to={child.url || ''}>{menuItemDisplay}</Link>
<Link to={stripAppRoot(child.url || '')}>
{menuItemDisplay}
</Link>
) : (
<Typography.Link
href={child.url || ''}

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