mirror of
https://github.com/apache/superset.git
synced 2026-08-13 11:40:37 +00:00
Compare commits
10
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
276d6f04f5 | ||
|
|
524d99159a | ||
|
|
657a2a4cb2 | ||
|
|
eefb3e3835 | ||
|
|
07accd56f2 | ||
|
|
22caf221b7 | ||
|
|
de2276225a | ||
|
|
685f26b1bf | ||
|
|
739365979d | ||
|
|
bc1a8e0858 |
@@ -37,7 +37,7 @@ jobs:
|
||||
persist-credentials: false
|
||||
submodules: recursive
|
||||
- name: Setup Java
|
||||
uses: actions/setup-java@03ad4de0992f5dab5e18fcb136590ce7c4a0ac95 # v5.6.0
|
||||
uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0
|
||||
with:
|
||||
distribution: "temurin"
|
||||
java-version: "11"
|
||||
|
||||
@@ -23,7 +23,7 @@ jobs:
|
||||
persist-credentials: false
|
||||
submodules: recursive
|
||||
- name: Setup Java
|
||||
uses: actions/setup-java@03ad4de0992f5dab5e18fcb136590ce7c4a0ac95 # v5.6.0
|
||||
uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0
|
||||
with:
|
||||
distribution: "temurin"
|
||||
java-version: "11"
|
||||
|
||||
@@ -76,7 +76,7 @@ jobs:
|
||||
node-version-file: "./docs/.nvmrc"
|
||||
- name: Setup Python
|
||||
uses: ./.github/actions/setup-backend/
|
||||
- uses: actions/setup-java@03ad4de0992f5dab5e18fcb136590ce7c4a0ac95 # v5.6.0
|
||||
- uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0
|
||||
with:
|
||||
distribution: "zulu"
|
||||
java-version: "21"
|
||||
|
||||
@@ -26,6 +26,7 @@ assists people when migrating to a new version.
|
||||
|
||||
- [42935](https://github.com/apache/superset/pull/42935): The MCP service now refuses to start (`MCPAuthConfigError`) when `MCP_JWT_ISSUER` trusts more than one issuer and no `MCP_USER_RESOLVER` is configured, instead of only logging a warning. This was already a documented misconfiguration (the default resolver isn't issuer-scoped, so distinct trusted issuers minting the same username/email would resolve to the same Superset user); deployments trusting multiple issuers must configure an `MCP_USER_RESOLVER` that derives its identity from the token's `iss` claim before upgrading. Single-issuer deployments are unaffected.
|
||||
- [42393](https://github.com/apache/superset/pull/42393): Exported dataset YAML now carries a `uuid` for each metric and column so that custom folder assignments (which reference metrics/columns by UUID) survive an import into another workspace. This affects any export bundle that contains datasets, not just a dataset export: chart, dashboard, database and full-asset exports all embed the same dataset YAML, so a dashboard exported from this release also fails to import into an older one even though no dataset was exported directly. As with `folders` and `currency_code_column`, the affected `datasets/` files fail schema validation (`Unknown field: uuid`) when imported into Superset releases that predate this change; regenerate or hand-edit exports for older targets in mixed-version fleets.
|
||||
- [42300](https://github.com/apache/superset/pull/42300): Timeseries charts (line/area/bar) with a Y-axis bound in effect — either an explicit `yAxisBounds` or one derived from `truncateYAxis` — now clamp out-of-range data points to that bound instead of letting ECharts drop the point (and the line segments around it) entirely. Any existing chart with a configured Y-axis bound and data outside it will look different after upgrading: a gap becomes a point pinned to the boundary. The clamp also rewrites the value ECharts reads for that point's tooltip and data label, so the displayed value is the bound rather than the true observation.
|
||||
- [42087](https://github.com/apache/superset/pull/42087): Stored calculated-column and metric expressions are validated when a query is built, under the same sub-query policy already applied to adhoc expressions. Previously only the dataset update path checked them on save, so expressions written by v1 import, by dataset duplication, or before that check existed were never validated. Since `ALLOW_ADHOC_SUBQUERY` defaults to `False` (see [19242](https://github.com/apache/superset/pull/19242)), a dataset whose stored expression contains a sub-query works before upgrading and afterwards fails at chart render with `Custom SQL fields cannot contain sub-queries.` There is no migration step, and the error does not name the offending dataset column, so audit stored expressions before upgrading: either rewrite them without the sub-query, or set `ALLOW_ADHOC_SUBQUERY = True` to keep the previous behaviour for both stored and adhoc expressions.
|
||||
|
||||
### Selenium support removed — Playwright is now required for screenshots
|
||||
|
||||
+8
-7
@@ -3215,16 +3215,17 @@
|
||||
"integrity": "sha512-Fc8Ne62jJlKHiG/ajlonC4Sd66Pq68fFwK4ihJGNZpGqboc324SQk+lRvMzpPRuJOmfrJefdG8/7JdWX4bzJ2Q=="
|
||||
},
|
||||
"node_modules/brace-expansion": {
|
||||
"version": "5.0.7",
|
||||
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.7.tgz",
|
||||
"integrity": "sha512-7oFy703dxfY3/NLxC1fh2SUCQ0H9rmAY+5EpDVfXjUTTs+HEwR2nYaqLv+GWcTsumwxPfiz6CzCNkwXwBUwqCA==",
|
||||
"version": "5.0.8",
|
||||
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz",
|
||||
"integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"peer": true,
|
||||
"dependencies": {
|
||||
"balanced-match": "^4.0.2"
|
||||
},
|
||||
"engines": {
|
||||
"node": "18 || 20 || >=22"
|
||||
"node": "20 || >=22"
|
||||
}
|
||||
},
|
||||
"node_modules/browserslist": {
|
||||
@@ -11184,9 +11185,9 @@
|
||||
"integrity": "sha512-Fc8Ne62jJlKHiG/ajlonC4Sd66Pq68fFwK4ihJGNZpGqboc324SQk+lRvMzpPRuJOmfrJefdG8/7JdWX4bzJ2Q=="
|
||||
},
|
||||
"brace-expansion": {
|
||||
"version": "5.0.7",
|
||||
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.7.tgz",
|
||||
"integrity": "sha512-7oFy703dxfY3/NLxC1fh2SUCQ0H9rmAY+5EpDVfXjUTTs+HEwR2nYaqLv+GWcTsumwxPfiz6CzCNkwXwBUwqCA==",
|
||||
"version": "5.0.8",
|
||||
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz",
|
||||
"integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==",
|
||||
"dev": true,
|
||||
"peer": true,
|
||||
"requires": {
|
||||
|
||||
@@ -36,6 +36,9 @@
|
||||
"cypress": {
|
||||
"form-data": "^2.3.4"
|
||||
},
|
||||
"minimatch@>=10": {
|
||||
"brace-expansion": ">=5.0.8"
|
||||
},
|
||||
"qs": "^6.14.2",
|
||||
"uuid": "^11.1.1"
|
||||
},
|
||||
|
||||
Generated
+21
-28
@@ -100,7 +100,7 @@
|
||||
"geostyler-style": "11.0.2",
|
||||
"geostyler-wfs-parser": "^3.0.1",
|
||||
"google-auth-library": "^11.0.0",
|
||||
"immer": "^11.1.15",
|
||||
"immer": "^11.1.16",
|
||||
"interweave": "^13.1.1",
|
||||
"jquery": "^4.0.0",
|
||||
"js-levenshtein": "^1.1.6",
|
||||
@@ -108,7 +108,7 @@
|
||||
"json-stringify-pretty-compact": "^4.0.0",
|
||||
"lodash": "^4.18.1",
|
||||
"lodash-es": "^4.18.1",
|
||||
"mapbox-gl": "^3.27.0",
|
||||
"mapbox-gl": "^3.28.0",
|
||||
"markdown-to-jsx": "^9.10.2",
|
||||
"match-sorter": "^8.3.0",
|
||||
"memoize-one": "^6.0.0",
|
||||
@@ -272,7 +272,7 @@
|
||||
"swc-loader": "^0.2.7",
|
||||
"ts-jest": "^29.4.12",
|
||||
"tscw-config": "^1.1.2",
|
||||
"tsx": "^4.23.7",
|
||||
"tsx": "^4.23.9",
|
||||
"typescript": "5.4.5",
|
||||
"unzipper": "^0.12.5",
|
||||
"wait-on": "^9.1.0",
|
||||
@@ -18005,11 +18005,14 @@
|
||||
}
|
||||
},
|
||||
"node_modules/core-js": {
|
||||
"version": "3.49.0",
|
||||
"resolved": "https://registry.npmjs.org/core-js/-/core-js-3.49.0.tgz",
|
||||
"integrity": "sha512-es1U2+YTtzpwkxVLwAFdSpaIMyQaq0PBgm3YD1W3Qpsn1NAmO3KSgZfu+oGSWVu6NvLHoHCV/aYcsE5wiB7ALg==",
|
||||
"version": "3.50.0",
|
||||
"resolved": "https://registry.npmjs.org/core-js/-/core-js-3.50.0.tgz",
|
||||
"integrity": "sha512-BRWgOLKkFeCgRudR6zrs8p9XJZcE14grzKMMssoYrk6krtuEZ7MTKPIY5RzOnqsEKIR9kst7wNzphttraT+Yqw==",
|
||||
"hasInstallScript": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": "*"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/core-js"
|
||||
@@ -24184,9 +24187,9 @@
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/immer": {
|
||||
"version": "11.1.15",
|
||||
"resolved": "https://registry.npmjs.org/immer/-/immer-11.1.15.tgz",
|
||||
"integrity": "sha512-VrNANlmnWQnh5COXIIOQXM9oOJw7naGKlBT74ZOOR6lpVXc3gFEu9FJLDFcpCJ2j+NWr8TIwtWD//T6ZX6TKiQ==",
|
||||
"version": "11.1.16",
|
||||
"resolved": "https://registry.npmjs.org/immer/-/immer-11.1.16.tgz",
|
||||
"integrity": "sha512-Xs7H9rBc+kti1J6RueUvbEBkmOz7jqj11XYgf+YMXAYzu8EeE7hwZ9poLXdVfVnGmJu7QAf41T7H2KuF6QoK6Q==",
|
||||
"license": "MIT",
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
@@ -28745,15 +28748,14 @@
|
||||
}
|
||||
},
|
||||
"node_modules/mapbox-gl": {
|
||||
"version": "3.27.0",
|
||||
"resolved": "https://registry.npmjs.org/mapbox-gl/-/mapbox-gl-3.27.0.tgz",
|
||||
"integrity": "sha512-K8W9LTTjFEJsg9qsnJbKk+zbXrmSqa+nU1EiFXez5gQ0T0RMtylZUelgg1/RE6vCUMvHX0gaYfWU9g2mTWuA0g==",
|
||||
"version": "3.28.0",
|
||||
"resolved": "https://registry.npmjs.org/mapbox-gl/-/mapbox-gl-3.28.0.tgz",
|
||||
"integrity": "sha512-WEbvl2ju0MUZ+R83HeCosmJBTyYdhmFcajeQ7kwLyJ0EHUw9YG/k2QLcMmAQ8sXZpkWq1BbmfjT5lh/oInOnCw==",
|
||||
"license": "SEE LICENSE IN LICENSE.txt",
|
||||
"workspaces": [
|
||||
"src/style-spec",
|
||||
"plugins/mapbox-gl-pmtiles-provider",
|
||||
"test/build/vite",
|
||||
"test/build/webpack",
|
||||
"test/bundlers/*",
|
||||
"test/build/typings"
|
||||
]
|
||||
},
|
||||
@@ -39964,9 +39966,9 @@
|
||||
"license": "0BSD"
|
||||
},
|
||||
"node_modules/tsx": {
|
||||
"version": "4.23.7",
|
||||
"resolved": "https://registry.npmjs.org/tsx/-/tsx-4.23.7.tgz",
|
||||
"integrity": "sha512-3f/u/+UDCNQ7iwUZW9FCMnNGIHzElGJYh0S/yy8IvWSsn5O7fEO/897FaG7FA2W8yryiRyuwXZ1PYLAKYaqSuQ==",
|
||||
"version": "4.23.9",
|
||||
"resolved": "https://registry.npmjs.org/tsx/-/tsx-4.23.9.tgz",
|
||||
"integrity": "sha512-6q8uTORRGauQVjqMQnKUucLFoeXZAfw6zKvG35GLbdKWbLdeOtZ3H4mhyA5mxuUd2o2cRTskhj59nLLQseUvUw==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
@@ -43233,7 +43235,7 @@
|
||||
"ag-grid-react": "36.0.2",
|
||||
"brace": "^0.11.1",
|
||||
"classnames": "^2.5.1",
|
||||
"core-js": "^3.49.0",
|
||||
"core-js": "^3.50.0",
|
||||
"csstype": "^3.2.3",
|
||||
"d3-format": "^3.1.2",
|
||||
"d3-interpolate": "^3.0.1",
|
||||
@@ -43351,15 +43353,6 @@
|
||||
"node": ">=12"
|
||||
}
|
||||
},
|
||||
"packages/superset-ui-core/node_modules/dompurify": {
|
||||
"version": "3.4.13",
|
||||
"resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.4.13.tgz",
|
||||
"integrity": "sha512-2vmYIoqjze2d+kakP8S/nS5shfsl587kzwEjcGlTdiksUVgFHnFCsLYDVj/JNqJVOQZGSYBTmuycv0PodwmnMQ==",
|
||||
"license": "(MPL-2.0 OR Apache-2.0)",
|
||||
"optionalDependencies": {
|
||||
"@types/trusted-types": "^2.0.7"
|
||||
}
|
||||
},
|
||||
"packages/superset-ui-core/node_modules/react-ace": {
|
||||
"version": "14.0.1",
|
||||
"resolved": "https://registry.npmjs.org/react-ace/-/react-ace-14.0.1.tgz",
|
||||
@@ -43943,7 +43936,7 @@
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@math.gl/web-mercator": "^4.1.0",
|
||||
"mapbox-gl": "^3.27.0",
|
||||
"mapbox-gl": "^3.28.0",
|
||||
"maplibre-gl": "^5.24.0",
|
||||
"react-map-gl": "^8.1.2",
|
||||
"supercluster": "^8.0.1"
|
||||
|
||||
@@ -177,7 +177,7 @@
|
||||
"geostyler-style": "11.0.2",
|
||||
"geostyler-wfs-parser": "^3.0.1",
|
||||
"google-auth-library": "^11.0.0",
|
||||
"immer": "^11.1.15",
|
||||
"immer": "^11.1.16",
|
||||
"interweave": "^13.1.1",
|
||||
"jquery": "^4.0.0",
|
||||
"js-levenshtein": "^1.1.6",
|
||||
@@ -185,7 +185,7 @@
|
||||
"json-stringify-pretty-compact": "^4.0.0",
|
||||
"lodash": "^4.18.1",
|
||||
"lodash-es": "^4.18.1",
|
||||
"mapbox-gl": "^3.27.0",
|
||||
"mapbox-gl": "^3.28.0",
|
||||
"markdown-to-jsx": "^9.10.2",
|
||||
"match-sorter": "^8.3.0",
|
||||
"memoize-one": "^6.0.0",
|
||||
@@ -349,7 +349,7 @@
|
||||
"swc-loader": "^0.2.7",
|
||||
"ts-jest": "^29.4.12",
|
||||
"tscw-config": "^1.1.2",
|
||||
"tsx": "^4.23.7",
|
||||
"tsx": "^4.23.9",
|
||||
"typescript": "5.4.5",
|
||||
"unzipper": "^0.12.5",
|
||||
"wait-on": "^9.1.0",
|
||||
@@ -412,6 +412,9 @@
|
||||
"lerna": {
|
||||
"js-yaml": "^4.3.0"
|
||||
},
|
||||
"minimatch@>=10": {
|
||||
"brace-expansion": ">=5.0.8"
|
||||
},
|
||||
"nwsapi": "^2.2.13",
|
||||
"puppeteer": "^22.4.1",
|
||||
"tar": "^7.5.16",
|
||||
|
||||
@@ -60,7 +60,7 @@
|
||||
"ag-grid-react": "36.0.2",
|
||||
"brace": "^0.11.1",
|
||||
"classnames": "^2.5.1",
|
||||
"core-js": "^3.49.0",
|
||||
"core-js": "^3.50.0",
|
||||
"csstype": "^3.2.3",
|
||||
"d3-format": "^3.1.2",
|
||||
"d3-interpolate": "^3.0.1",
|
||||
|
||||
@@ -0,0 +1,162 @@
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Regression coverage for the Drill to Detail modal's results table.
|
||||
*
|
||||
* The drill pane measures its available height with a resize detector and
|
||||
* hands it to a virtualized table, so the rows only render if the modal's
|
||||
* internal height chain (resizable wrapper -> modal container -> modal body ->
|
||||
* flex pane) actually resolves to a real height. That chain is wired together
|
||||
* with CSS selectors targeting Ant Design's internal modal classes, which
|
||||
* TypeScript cannot see and unit tests do not exercise: when the antd v6
|
||||
* upgrade renamed `.ant-modal-content` to `.ant-modal-container`, the chain
|
||||
* silently broke, the pane measured ~0, and the table briefly flashed its rows
|
||||
* before collapsing to an empty body with only the header and pagination
|
||||
* visible.
|
||||
*
|
||||
* Only a real browser sees layout, so this is pinned here rather than in the
|
||||
* DOM-contract unit suite. Because the failure mode is
|
||||
* render-then-collapse, a single "rows are visible" read could pass during
|
||||
* the initial flash — the assertion therefore lets the height settle once,
|
||||
* then requires it to hold across further spaced reads with no more
|
||||
* retrying, so a later or partial collapse cannot be masked by an early-exit
|
||||
* retry that stopped at the first passing sample.
|
||||
*
|
||||
* CI green => the drill modal's table renders rows at a stable, non-collapsed
|
||||
* height.
|
||||
* CI red => the modal height chain broke again (or drill-to-detail failed to
|
||||
* open/load at all).
|
||||
*/
|
||||
import { testWithAssets, expect } from '../../helpers/fixtures';
|
||||
import { TIMEOUT } from '../../utils/constants';
|
||||
import { DashboardPage } from '../../pages/DashboardPage';
|
||||
import { createDashboardWithCharts } from './dashboard-test-helpers';
|
||||
|
||||
const MIN_STABLE_BODY_HEIGHT = 100;
|
||||
// Max fraction the body height may drift from the settled baseline below; a
|
||||
// partial collapse (e.g. 400px -> 150px) still clears MIN_STABLE_BODY_HEIGHT
|
||||
// but fails this, so the check enforces stability, not just a floor.
|
||||
const HEIGHT_DRIFT_TOLERANCE = 0.25;
|
||||
// Extra spaced reads taken *after* the height has settled, and the gap
|
||||
// between them. These are plain assertions, not wrapped in a retrying
|
||||
// helper: once settled, a retry would return on the first passing sample
|
||||
// and could mask a collapse that only shows up later in the window.
|
||||
const HEIGHT_SAMPLE_COUNT = 2;
|
||||
const HEIGHT_SAMPLE_INTERVAL_MS = 300;
|
||||
|
||||
testWithAssets(
|
||||
'drill to detail modal renders result rows at a stable height',
|
||||
async ({ page, testAssets }) => {
|
||||
testWithAssets.setTimeout(TIMEOUT.SLOW_TEST);
|
||||
|
||||
const { dashboardId } = await createDashboardWithCharts(
|
||||
page,
|
||||
testAssets,
|
||||
testWithAssets.info(),
|
||||
{
|
||||
datasetName: 'birth_names',
|
||||
chartNamePrefix: 'drill_detail',
|
||||
dashboardTitlePrefix: 'drill_detail_modal',
|
||||
chartSpecs: [
|
||||
{
|
||||
viz_type: 'pie',
|
||||
params: {
|
||||
groupby: ['gender'],
|
||||
metric: 'count',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
);
|
||||
|
||||
const dashboard = new DashboardPage(page);
|
||||
await dashboard.gotoById(dashboardId);
|
||||
await dashboard.waitForLoad();
|
||||
await dashboard.waitForChartsToLoad();
|
||||
|
||||
// Open the chart context menu. The ECharts canvas exposes no data-test
|
||||
// hooks for its regions, so right-click the centre of the chart container;
|
||||
// the exact-text match below then works whether the click landed on a
|
||||
// slice (which adds "Drill to detail by" items) or on the chart background.
|
||||
// The first right-click after load can be swallowed by a chart re-render
|
||||
// closing the menu, so retry the click until the menu actually shows.
|
||||
const chart = page.locator('[data-test="chart-container"]').first();
|
||||
await chart.scrollIntoViewIfNeeded();
|
||||
const contextMenu = page.locator('[data-test="chart-context-menu"]');
|
||||
await expect(async () => {
|
||||
const box = await chart.boundingBox();
|
||||
if (!box) {
|
||||
throw new Error('chart container has no bounding box');
|
||||
}
|
||||
await page.mouse.click(box.x + box.width / 2, box.y + box.height / 2, {
|
||||
button: 'right',
|
||||
});
|
||||
await expect(contextMenu).toBeVisible({ timeout: 2000 });
|
||||
}).toPass({ timeout: TIMEOUT.CHART_RENDER });
|
||||
await page
|
||||
.getByRole('menuitem', { name: 'Drill to detail', exact: true })
|
||||
.click();
|
||||
|
||||
const modal = page.locator('.ant-modal:visible');
|
||||
await expect(modal).toBeVisible({ timeout: TIMEOUT.FORM_LOAD });
|
||||
|
||||
// Wait for the samples request to resolve into a rendered table: the row
|
||||
// count pill and the virtualized body both come from the loaded page.
|
||||
const tableBody = modal.locator('.virtual-grid');
|
||||
await expect(tableBody).toBeAttached({ timeout: TIMEOUT.CHART_RENDER });
|
||||
|
||||
// The regression collapses the body *after* first paint, so first let the
|
||||
// height settle above the floor (retrying is safe here: the collapse is
|
||||
// persistent, so a broken build never finds a passing read and this
|
||||
// still times out red), then, without any further retrying, take extra
|
||||
// spaced reads and require each to hold within tolerance of that settled
|
||||
// baseline — a delayed or partial collapse can no longer be masked by an
|
||||
// early-exit retry that stopped at the first passing sample.
|
||||
let baselineHeight = 0;
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
baselineHeight = (await tableBody.boundingBox())?.height ?? 0;
|
||||
return baselineHeight;
|
||||
},
|
||||
{ timeout: TIMEOUT.CHART_RENDER },
|
||||
)
|
||||
.toBeGreaterThan(MIN_STABLE_BODY_HEIGHT);
|
||||
for (let sample = 0; sample < HEIGHT_SAMPLE_COUNT; sample += 1) {
|
||||
// eslint-disable-next-line no-await-in-loop -- reads must be sequential
|
||||
// and spaced out to observe a delayed collapse; there is nothing to
|
||||
// parallelize.
|
||||
await page.waitForTimeout(HEIGHT_SAMPLE_INTERVAL_MS);
|
||||
// eslint-disable-next-line no-await-in-loop -- see above
|
||||
const box = await tableBody.boundingBox();
|
||||
const height = box?.height ?? 0;
|
||||
expect(height).toBeGreaterThan(MIN_STABLE_BODY_HEIGHT);
|
||||
expect(Math.abs(height - baselineHeight)).toBeLessThanOrEqual(
|
||||
baselineHeight * HEIGHT_DRIFT_TOLERANCE,
|
||||
);
|
||||
}
|
||||
|
||||
// And the rows are real data, not just an expanded empty scroller:
|
||||
// birth_names sample rows always carry a gender value.
|
||||
await expect(modal.getByText(/^(boy|girl)$/).first()).toBeVisible({
|
||||
timeout: TIMEOUT.API_RESPONSE,
|
||||
});
|
||||
},
|
||||
);
|
||||
@@ -827,6 +827,14 @@ export default function transformProps(
|
||||
|
||||
series.unshift(baselineSeries);
|
||||
}
|
||||
|
||||
// Snapshot the observation-series count before annotation layers are
|
||||
// appended below. Annotation series (formula/interval/event/timeseries)
|
||||
// carry their own configured values, which the Y axis clamp further
|
||||
// below must not rewrite, or an annotation could be moved to a location
|
||||
// that doesn't match its configuration.
|
||||
const observationSeriesCount = series.length;
|
||||
|
||||
const selectedValues = (filterState.selectedValues || []).reduce(
|
||||
(acc: Record<string, number>, selectedValue: string) => {
|
||||
const index = series.findIndex(({ name }) => name === selectedValue);
|
||||
@@ -986,6 +994,60 @@ export default function transformProps(
|
||||
}
|
||||
}
|
||||
|
||||
// Whenever a Y axis bound is defined, whether explicitly configured or
|
||||
// derived above from the data, clamp series values to those bounds
|
||||
// instead of leaving raw out-of-range values in place. ECharts axis
|
||||
// clipping can otherwise drop an out-of-bounds point (and the line
|
||||
// segments around it) entirely rather than truncating it at the
|
||||
// boundary (see https://github.com/apache/superset/issues/27449).
|
||||
if (yAxisMin !== undefined || yAxisMax !== undefined) {
|
||||
const valueIndex = isHorizontal ? 0 : 1;
|
||||
type AxisValue = string | number | null | undefined;
|
||||
type AxisPoint = AxisValue[];
|
||||
const clampAxisValue = (value: AxisValue): AxisValue => {
|
||||
if (typeof value !== 'number' || Number.isNaN(value)) return value;
|
||||
let clamped = value;
|
||||
if (yAxisMin !== undefined) clamped = Math.max(clamped, yAxisMin);
|
||||
if (yAxisMax !== undefined) clamped = Math.min(clamped, yAxisMax);
|
||||
return clamped;
|
||||
};
|
||||
const clampPoint = (point: AxisPoint): AxisPoint => {
|
||||
const newPoint = [...point];
|
||||
newPoint[valueIndex] = clampAxisValue(newPoint[valueIndex]);
|
||||
return newPoint;
|
||||
};
|
||||
series.forEach((s, index) => {
|
||||
// Skip annotation series appended above; only clamp the chart's own
|
||||
// observation/legend/baseline series.
|
||||
if (index >= observationSeriesCount) return;
|
||||
if (!Array.isArray(s.data)) return;
|
||||
const clampedData = (
|
||||
s.data as (AxisPoint | Record<string, unknown>)[]
|
||||
).map(point => {
|
||||
if (Array.isArray(point)) {
|
||||
return clampPoint(point);
|
||||
}
|
||||
// Some series paths (e.g. colorByPrimaryAxis, or negative bar
|
||||
// label positioning) wrap the tuple in an object of the shape
|
||||
// `{ value: [x, y], ... }` instead of passing the tuple
|
||||
// directly; clamp the wrapped tuple in place so those points
|
||||
// aren't skipped and left to be dropped by ECharts axis clipping.
|
||||
if (
|
||||
point &&
|
||||
typeof point === 'object' &&
|
||||
Array.isArray((point as { value?: unknown }).value)
|
||||
) {
|
||||
return {
|
||||
...point,
|
||||
value: clampPoint((point as { value: AxisPoint }).value),
|
||||
};
|
||||
}
|
||||
return point;
|
||||
});
|
||||
s.data = clampedData as typeof s.data;
|
||||
});
|
||||
}
|
||||
|
||||
// A dashboard-level time grain override (e.g. via a filter or the temporal
|
||||
// range control) is delivered in extraFormData and should take precedence
|
||||
// over the chart's own time grain when formatting temporal axes/tooltips.
|
||||
|
||||
+192
@@ -1555,6 +1555,198 @@ test('computes row-contribution axis padding per stack when time_compare splits
|
||||
expect(xAxisRaw.max).toBeLessThan(1.5);
|
||||
});
|
||||
|
||||
test('clamps series values to the yAxis max instead of dropping out-of-range points (#27449)', () => {
|
||||
const queriesData: ChartDataResponseResult[] = [
|
||||
createTestQueryData(
|
||||
createTestData(
|
||||
[
|
||||
{ 'Series A': 1 },
|
||||
{ 'Series A': 2 },
|
||||
{ 'Series A': 3 },
|
||||
{ 'Series A': 4 },
|
||||
{ 'Series A': 1000 },
|
||||
{ 'Series A': 4 },
|
||||
{ 'Series A': 2 },
|
||||
],
|
||||
{ intervalMs: 300000000 },
|
||||
),
|
||||
),
|
||||
];
|
||||
|
||||
const chartProps = createTestChartProps({
|
||||
formData: {
|
||||
...formData,
|
||||
groupby: [],
|
||||
seriesType: EchartsTimeseriesSeriesType.Line,
|
||||
truncateYAxis: true,
|
||||
yAxisBounds: [0, 10],
|
||||
},
|
||||
queriesData,
|
||||
});
|
||||
|
||||
const transformedProps = transformProps(chartProps);
|
||||
const series = transformedProps.echartOptions.series as SeriesOption[];
|
||||
const seriesA = series.find(s => s.name === 'Series A');
|
||||
expect(seriesA).toBeDefined();
|
||||
const data = seriesA!.data as [number, number][];
|
||||
|
||||
// The point that was 1000 should be present (not dropped) and clamped to
|
||||
// the configured yAxis max of 10, rather than disappearing entirely.
|
||||
expect(data).toHaveLength(7);
|
||||
expect(data[4][1]).toBe(10);
|
||||
});
|
||||
|
||||
test('clamps series values to the yAxis min when a value falls below it', () => {
|
||||
const queriesData: ChartDataResponseResult[] = [
|
||||
createTestQueryData(
|
||||
createTestData(
|
||||
[{ 'Series A': -1000 }, { 'Series A': 2 }, { 'Series A': 3 }],
|
||||
{ intervalMs: 300000000 },
|
||||
),
|
||||
),
|
||||
];
|
||||
|
||||
const chartProps = createTestChartProps({
|
||||
formData: {
|
||||
...formData,
|
||||
groupby: [],
|
||||
seriesType: EchartsTimeseriesSeriesType.Line,
|
||||
truncateYAxis: true,
|
||||
yAxisBounds: [0, 10],
|
||||
},
|
||||
queriesData,
|
||||
});
|
||||
|
||||
const transformedProps = transformProps(chartProps);
|
||||
const series = transformedProps.echartOptions.series as SeriesOption[];
|
||||
const seriesA = series.find(s => s.name === 'Series A');
|
||||
expect(seriesA).toBeDefined();
|
||||
const data = seriesA!.data as [number, number][];
|
||||
|
||||
expect(data).toHaveLength(3);
|
||||
expect(data[0][1]).toBe(0);
|
||||
});
|
||||
|
||||
test('clamps series values to the yAxis bounds when colorByPrimaryAxis wraps points in objects (#27449)', () => {
|
||||
const queriesData: ChartDataResponseResult[] = [
|
||||
createTestQueryData(
|
||||
createTestData(
|
||||
[{ 'Series A': 1 }, { 'Series A': 1000 }, { 'Series A': 2 }],
|
||||
{ intervalMs: 300000000 },
|
||||
),
|
||||
),
|
||||
];
|
||||
|
||||
const chartProps = createTestChartProps({
|
||||
formData: {
|
||||
...formData,
|
||||
groupby: [],
|
||||
seriesType: EchartsTimeseriesSeriesType.Line,
|
||||
truncateYAxis: true,
|
||||
yAxisBounds: [0, 10],
|
||||
colorByPrimaryAxis: true,
|
||||
},
|
||||
queriesData,
|
||||
});
|
||||
|
||||
const transformedProps = transformProps(chartProps);
|
||||
const series = transformedProps.echartOptions.series as SeriesOption[];
|
||||
const seriesA = series.find(s => s.name === 'Series A');
|
||||
expect(seriesA).toBeDefined();
|
||||
const data = seriesA!.data as { value: [number, number] }[];
|
||||
|
||||
// colorByPrimaryAxis wraps each point as `{ value: [x, y], itemStyle }`
|
||||
// rather than a bare tuple; the wrapped value must still be clamped
|
||||
// instead of being skipped and left for ECharts to drop.
|
||||
expect(data).toHaveLength(3);
|
||||
expect(data[1].value[1]).toBe(10);
|
||||
});
|
||||
|
||||
test('does not clamp a timeseries annotation series to the Y axis bounds (#27449)', () => {
|
||||
const timeseries: TimeseriesAnnotationLayer = {
|
||||
annotationType: AnnotationType.Timeseries,
|
||||
name: 'My Timeseries',
|
||||
show: true,
|
||||
showLabel: true,
|
||||
sourceType: AnnotationSourceType.Line,
|
||||
style: AnnotationStyle.Solid,
|
||||
titleColumn: '',
|
||||
value: 3,
|
||||
};
|
||||
const annotationData = {
|
||||
'My Timeseries': {
|
||||
records: [
|
||||
{ x: 0, y: 11000 },
|
||||
{ x: 300000000, y: 21000 },
|
||||
],
|
||||
},
|
||||
};
|
||||
const queriesData: ChartDataResponseResult[] = [
|
||||
createTestQueryData(
|
||||
createTestData([{ 'Series A': 1 }, { 'Series A': 2 }], {
|
||||
intervalMs: 300000000,
|
||||
}),
|
||||
{ annotation_data: annotationData },
|
||||
),
|
||||
];
|
||||
|
||||
const chartProps = createTestChartProps({
|
||||
formData: {
|
||||
...formData,
|
||||
groupby: [],
|
||||
seriesType: EchartsTimeseriesSeriesType.Line,
|
||||
truncateYAxis: true,
|
||||
yAxisBounds: [0, 10],
|
||||
annotationLayers: [timeseries],
|
||||
},
|
||||
annotationData,
|
||||
queriesData,
|
||||
});
|
||||
|
||||
const transformedProps = transformProps(chartProps);
|
||||
const series = transformedProps.echartOptions.series as SeriesOption[];
|
||||
const annotationSeries = series.find(s => s.id === 'My Timeseries');
|
||||
expect(annotationSeries).toBeDefined();
|
||||
const data = annotationSeries!.data as [number, number][];
|
||||
|
||||
// The annotation carries its own configured values (11000, 21000), which
|
||||
// are unrelated to the chart's own out-of-range-data problem this PR
|
||||
// fixes. They must be left untouched by the Y axis clamp rather than
|
||||
// rewritten to the yAxisBounds max of 10.
|
||||
expect(data[0][1]).toBe(11000);
|
||||
expect(data[1][1]).toBe(21000);
|
||||
});
|
||||
|
||||
test('clamps series values at the correct tuple index for horizontal bar charts (#27449)', () => {
|
||||
const queriesData: ChartDataResponseResult[] = [
|
||||
createTestQueryData(
|
||||
createTestData(
|
||||
[{ 'Series A': 15000 }, { 'Series A': 20000 }, { 'Series A': 18000 }],
|
||||
{ intervalMs: 300000000 },
|
||||
),
|
||||
),
|
||||
];
|
||||
|
||||
const chartProps = createTestChartProps({
|
||||
formData: {
|
||||
...baseFormDataHorizontalBar,
|
||||
yAxisBounds: [0, 16000],
|
||||
},
|
||||
queriesData,
|
||||
});
|
||||
|
||||
const transformedProps = transformProps(chartProps);
|
||||
const series = transformedProps.echartOptions.series as SeriesOption[];
|
||||
const seriesA = series.find(s => s.name === 'Series A');
|
||||
expect(seriesA).toBeDefined();
|
||||
const data = seriesA!.data as [number, number][];
|
||||
|
||||
// In horizontal orientation the value sits at tuple index 0 (the axes are
|
||||
// swapped), so the clamp must target that index rather than index 1.
|
||||
expect(data).toHaveLength(3);
|
||||
expect(data[1][0]).toBe(16000);
|
||||
});
|
||||
|
||||
test('legend is visible on tall charts when enabled by the user', () => {
|
||||
const chartProps = createTestChartProps({
|
||||
height: 400,
|
||||
|
||||
@@ -30,7 +30,7 @@
|
||||
},
|
||||
"dependencies": {
|
||||
"@math.gl/web-mercator": "^4.1.0",
|
||||
"mapbox-gl": "^3.27.0",
|
||||
"mapbox-gl": "^3.28.0",
|
||||
"maplibre-gl": "^5.24.0",
|
||||
"react-map-gl": "^8.1.2",
|
||||
"supercluster": "^8.0.1"
|
||||
|
||||
@@ -91,6 +91,21 @@ class ExportChartsCommand(ExportModelsCommand):
|
||||
def enable_tag_export(cls) -> None:
|
||||
cls._include_tags = True
|
||||
|
||||
def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
yield from super().run()
|
||||
|
||||
# Tags are exported once for all requested charts (rather than per
|
||||
# chart in `_export`) so a multi-chart export doesn't lose tags to
|
||||
# the parent's per-file-name de-duplication of `tags.yaml`.
|
||||
if (
|
||||
self.export_related
|
||||
and ExportChartsCommand._include_tags
|
||||
and feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM")
|
||||
):
|
||||
yield from ExportTagsCommand(
|
||||
chart_ids=[model.id for model in self._models]
|
||||
).run()
|
||||
|
||||
@staticmethod
|
||||
def _export(
|
||||
model: Slice, export_related: bool = True
|
||||
@@ -102,12 +117,3 @@ class ExportChartsCommand(ExportModelsCommand):
|
||||
|
||||
if model.table and export_related:
|
||||
yield from ExportDatasetsCommand([model.table.id]).run()
|
||||
|
||||
# Check if the calling class is ExportDashboardCommands
|
||||
if (
|
||||
export_related
|
||||
and ExportChartsCommand._include_tags
|
||||
and feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM")
|
||||
):
|
||||
chart_id = model.id
|
||||
yield from ExportTagsCommand().export(chart_ids=[chart_id])
|
||||
|
||||
@@ -68,8 +68,9 @@ class ChartWarmUpCacheCommand(BaseCommand):
|
||||
# Apply dashboard filters if dashboard_id is provided
|
||||
if dashboard_filters := self._get_dashboard_filters(chart.id):
|
||||
for query in query_context.queries:
|
||||
query.filter.extend(
|
||||
query.filter = (
|
||||
cast(list[QueryObjectFilterClause], dashboard_filters)
|
||||
+ query.filter
|
||||
)
|
||||
|
||||
query_context.force = True
|
||||
|
||||
@@ -398,9 +398,9 @@ class ExportDashboardsCommand(ExportModelsCommand):
|
||||
yield from command.run()
|
||||
command.enable_tag_export()
|
||||
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
|
||||
yield from ExportTagsCommand.export(
|
||||
yield from ExportTagsCommand(
|
||||
dashboard_ids=dashboard_ids, chart_ids=chart_ids
|
||||
)
|
||||
).run()
|
||||
|
||||
# Export related theme
|
||||
if model.theme:
|
||||
|
||||
@@ -70,12 +70,10 @@ class ExportAssetsCommand(BaseCommand):
|
||||
elif command == ExportChartsCommand:
|
||||
chart_ids = ids
|
||||
|
||||
# FIXME: It would probably be better to align the tags export
|
||||
# command with the other export commands
|
||||
yield from ExportTagsCommand.export(
|
||||
yield from ExportTagsCommand(
|
||||
dashboard_ids=dashboard_ids,
|
||||
chart_ids=chart_ids,
|
||||
)
|
||||
).run()
|
||||
|
||||
def validate(self) -> None:
|
||||
pass
|
||||
|
||||
@@ -23,33 +23,78 @@ from collections.abc import Iterator
|
||||
import yaml
|
||||
from superset.daos.chart import ChartDAO
|
||||
from superset.daos.dashboard import DashboardDAO
|
||||
from superset.daos.tag import TagDAO
|
||||
from superset.extensions import feature_flag_manager
|
||||
from superset.tags.models import TagType
|
||||
from superset.tags.models import ObjectType, TagType
|
||||
from superset.commands.export.models import ExportModelsCommand
|
||||
from superset.commands.tag.exceptions import TagNotFoundError
|
||||
|
||||
|
||||
# pylint: disable=too-few-public-methods
|
||||
class ExportTagsCommand:
|
||||
class ExportTagsCommand(ExportModelsCommand):
|
||||
dao = TagDAO
|
||||
not_found = TagNotFoundError
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
model_ids: Optional[list[int]] = None,
|
||||
export_related: bool = True,
|
||||
*,
|
||||
dashboard_ids: Optional[Union[int, List[Union[int, str]]]] = None,
|
||||
chart_ids: Optional[Union[int, List[Union[int, str]]]] = None,
|
||||
):
|
||||
super().__init__(model_ids=model_ids or [], export_related=export_related)
|
||||
self.dashboard_ids = dashboard_ids
|
||||
self.chart_ids = chart_ids
|
||||
|
||||
def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
if not feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
|
||||
return
|
||||
|
||||
self.validate()
|
||||
|
||||
dashboard_ids: list[Union[int, str]] = (
|
||||
[self.dashboard_ids]
|
||||
if isinstance(self.dashboard_ids, int)
|
||||
else list(self.dashboard_ids or [])
|
||||
)
|
||||
chart_ids: list[Union[int, str]] = (
|
||||
[self.chart_ids]
|
||||
if isinstance(self.chart_ids, int)
|
||||
else list(self.chart_ids or [])
|
||||
)
|
||||
|
||||
if self.model_ids:
|
||||
for tag in self._models:
|
||||
for tagged_object in tag.objects:
|
||||
if tagged_object.object_type == ObjectType.dashboard:
|
||||
dashboard_ids.append(tagged_object.object_id)
|
||||
elif tagged_object.object_type == ObjectType.chart:
|
||||
chart_ids.append(tagged_object.object_id)
|
||||
|
||||
dashboard_ids = list(dict.fromkeys(dashboard_ids))
|
||||
chart_ids = list(dict.fromkeys(chart_ids))
|
||||
|
||||
yield (
|
||||
ExportTagsCommand._file_name(),
|
||||
lambda: ExportTagsCommand._file_content(
|
||||
dashboard_ids or None, chart_ids or None
|
||||
),
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _file_name() -> str:
|
||||
# Use the model to determine the filename
|
||||
def _file_name(model: Any = None) -> str:
|
||||
return "tags.yaml"
|
||||
|
||||
@staticmethod
|
||||
def _merge_tags(
|
||||
dashboard_tags: List[dict[str, Any]], chart_tags: List[dict[str, Any]]
|
||||
) -> List[dict[str, Any]]:
|
||||
# Create a dictionary to prevent duplicates based on tag name
|
||||
tags_dict = {tag["tag_name"]: tag for tag in dashboard_tags}
|
||||
|
||||
# Add chart tags, preserving unique tag names
|
||||
for tag in chart_tags:
|
||||
if tag["tag_name"] not in tags_dict:
|
||||
tags_dict[tag["tag_name"]] = tag
|
||||
|
||||
# Return merged tags as a list
|
||||
return list(tags_dict.values())
|
||||
|
||||
@staticmethod
|
||||
@@ -62,13 +107,9 @@ class ExportTagsCommand:
|
||||
dashboard_tags = []
|
||||
chart_tags = []
|
||||
|
||||
# Fetch dashboard tags if provided
|
||||
if dashboard_ids:
|
||||
# Ensure dashboard_ids is a list
|
||||
if isinstance(dashboard_ids, int):
|
||||
dashboard_ids = [
|
||||
dashboard_ids
|
||||
] # Convert single int to list for consistency
|
||||
dashboard_ids = [dashboard_ids]
|
||||
|
||||
dashboards = [
|
||||
dashboard
|
||||
@@ -88,11 +129,9 @@ class ExportTagsCommand:
|
||||
]
|
||||
dashboard_tags.extend(filtered_tags)
|
||||
|
||||
# Fetch chart tags if provided
|
||||
if chart_ids:
|
||||
# Ensure chart_ids is a list
|
||||
if isinstance(chart_ids, int):
|
||||
chart_ids = [chart_ids] # Convert single int to list for consistency
|
||||
chart_ids = [chart_ids]
|
||||
|
||||
charts = [
|
||||
chart
|
||||
@@ -109,23 +148,8 @@ class ExportTagsCommand:
|
||||
]
|
||||
chart_tags.extend(filtered_tags)
|
||||
|
||||
# Merge the tags from both dashboards and charts
|
||||
merged_tags = ExportTagsCommand._merge_tags(dashboard_tags, chart_tags)
|
||||
payload["tags"].extend(merged_tags)
|
||||
|
||||
# Convert to YAML format
|
||||
file_content = yaml.safe_dump(payload, sort_keys=False)
|
||||
return file_content
|
||||
|
||||
@staticmethod
|
||||
def export(
|
||||
dashboard_ids: Optional[Union[int, List[Union[int, str]]]] = None,
|
||||
chart_ids: Optional[Union[int, List[Union[int, str]]]] = None,
|
||||
) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
if not feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
|
||||
return
|
||||
|
||||
yield (
|
||||
ExportTagsCommand._file_name(),
|
||||
lambda: ExportTagsCommand._file_content(dashboard_ids, chart_ids),
|
||||
)
|
||||
|
||||
+23
-3
@@ -37,6 +37,9 @@ from flask_babel import _
|
||||
from werkzeug.exceptions import BadRequest
|
||||
|
||||
from superset import appbuilder, dataframe, db, result_set
|
||||
from superset.charts.data.dashboard_filter_context import (
|
||||
get_dashboard_filter_context,
|
||||
)
|
||||
from superset.common.db_query_status import QueryStatus
|
||||
from superset.exceptions import (
|
||||
SerializationError,
|
||||
@@ -502,11 +505,20 @@ def get_dashboard_extra_filters(
|
||||
return []
|
||||
|
||||
with contextlib.suppress(json.JSONDecodeError):
|
||||
# does this dashboard have default filters?
|
||||
json_metadata = json.loads(dashboard.json_metadata)
|
||||
native_filters = [
|
||||
flt
|
||||
for flt in get_dashboard_filter_context(
|
||||
dashboard_id=dashboard_id,
|
||||
chart_id=slice_id,
|
||||
).extra_form_data.get("filters", [])
|
||||
if isinstance(flt, dict)
|
||||
]
|
||||
|
||||
# does this dashboard have legacy default filters?
|
||||
default_filters = json.loads(json_metadata.get("default_filters", "null"))
|
||||
if not default_filters:
|
||||
return []
|
||||
return native_filters
|
||||
|
||||
# are default filters applicable to the given slice?
|
||||
filter_scopes = json_metadata.get("filter_scopes", {})
|
||||
@@ -517,7 +529,15 @@ def get_dashboard_extra_filters(
|
||||
and isinstance(filter_scopes, dict)
|
||||
and isinstance(default_filters, dict)
|
||||
):
|
||||
return build_extra_filters(layout, filter_scopes, default_filters, slice_id)
|
||||
return [
|
||||
*build_extra_filters(
|
||||
layout,
|
||||
filter_scopes,
|
||||
default_filters,
|
||||
slice_id,
|
||||
),
|
||||
*native_filters,
|
||||
]
|
||||
return []
|
||||
|
||||
|
||||
|
||||
@@ -2266,6 +2266,79 @@ class TestChartApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCase):
|
||||
{"chart_id": slc.id, "viz_error": None, "viz_status": "success"}
|
||||
]
|
||||
|
||||
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
|
||||
def test_warm_up_cache_native_defaults_hit_browser_query_cache(self) -> None:
|
||||
self.login(ADMIN_USERNAME)
|
||||
chart = self.get_slice("Pivot Table v2")
|
||||
dashboard = self.get_dash_by_slug("births")
|
||||
|
||||
saved_query_context = json.loads(chart.query_context)
|
||||
chart_filter = {"col": "name", "op": "!=", "val": "__missing_name__"}
|
||||
for query in saved_query_context["queries"]:
|
||||
query["filters"] = [*(query.get("filters") or []), chart_filter]
|
||||
chart.query_context = json.dumps(saved_query_context)
|
||||
|
||||
metadata = json.loads(dashboard.json_metadata or "{}")
|
||||
legacy_filter = {"col": "name", "op": "in", "val": ["Alice"]}
|
||||
metadata["default_filters"] = json.dumps(
|
||||
{"-1": {legacy_filter["col"]: legacy_filter["val"]}}
|
||||
)
|
||||
metadata["filter_scopes"] = {}
|
||||
native_filter = {"col": "gender", "op": "IN", "val": ["girl"]}
|
||||
metadata["native_filter_configuration"] = [
|
||||
{
|
||||
"id": "NATIVE_FILTER-gender",
|
||||
"name": "Gender",
|
||||
"type": "NATIVE_FILTER",
|
||||
"filterType": "filter_select",
|
||||
"scope": {"rootPath": ["ROOT_ID"], "excluded": []},
|
||||
"targets": [
|
||||
{
|
||||
"datasetId": chart.datasource_id,
|
||||
"column": {"name": "gender"},
|
||||
}
|
||||
],
|
||||
"defaultDataMask": {
|
||||
"extraFormData": {"filters": [native_filter]},
|
||||
"filterState": {"value": ["girl"]},
|
||||
},
|
||||
"controlValues": {},
|
||||
}
|
||||
]
|
||||
dashboard.json_metadata = json.dumps(metadata)
|
||||
db.session.commit()
|
||||
|
||||
warm_up_response = self.client.put(
|
||||
"/api/v1/chart/warm_up_cache",
|
||||
json={"chart_id": chart.id, "dashboard_id": dashboard.id},
|
||||
)
|
||||
assert warm_up_response.status_code == 200
|
||||
assert warm_up_response.json["result"] == [
|
||||
{"chart_id": chart.id, "viz_error": None, "viz_status": "success"}
|
||||
]
|
||||
|
||||
browser_query_context = json.loads(chart.query_context)
|
||||
browser_query_context["force"] = False
|
||||
for query in browser_query_context["queries"]:
|
||||
query["filters"] = [
|
||||
legacy_filter,
|
||||
native_filter,
|
||||
*(query.get("filters") or []),
|
||||
]
|
||||
|
||||
assert browser_query_context["queries"][0]["filters"] == [
|
||||
legacy_filter,
|
||||
native_filter,
|
||||
chart_filter,
|
||||
]
|
||||
|
||||
chart_data_response = self.client.post(
|
||||
"/api/v1/chart/data",
|
||||
json=browser_query_context,
|
||||
)
|
||||
assert chart_data_response.status_code == 200
|
||||
assert chart_data_response.json["result"][0]["is_cached"] is True
|
||||
|
||||
def test_warm_up_cache_chart_id_required(self):
|
||||
self.login(ADMIN_USERNAME)
|
||||
rv = self.client.put("/api/v1/chart/warm_up_cache", json={"dashboard_id": 1})
|
||||
|
||||
@@ -30,14 +30,14 @@ def mock_security_manager():
|
||||
|
||||
@patch("superset.commands.chart.warm_up_cache.get_dashboard_extra_filters")
|
||||
@patch("superset.commands.chart.warm_up_cache.ChartDataCommand")
|
||||
def test_applies_dashboard_filters_to_non_legacy_chart(
|
||||
def test_prepends_dashboard_filters_to_non_legacy_chart(
|
||||
mock_chart_data_command, mock_get_dashboard_filters
|
||||
):
|
||||
"""Verify dashboard filters are added to query.filter for non-legacy viz"""
|
||||
"""Verify dashboard filters precede saved chart filters."""
|
||||
# Setup: Mock dashboard filters response
|
||||
mock_get_dashboard_filters.return_value = [
|
||||
{"col": "country", "op": "in", "val": ["USA", "France"]}
|
||||
]
|
||||
legacy_filter = {"col": "region", "op": "in", "val": ["Americas"]}
|
||||
native_filter = {"col": "country", "op": "IN", "val": ["USA", "France"]}
|
||||
mock_get_dashboard_filters.return_value = [legacy_filter, native_filter]
|
||||
|
||||
# Create a chart with non-legacy viz type
|
||||
chart = Slice(
|
||||
@@ -48,9 +48,10 @@ def test_applies_dashboard_filters_to_non_legacy_chart(
|
||||
datasource_type="table",
|
||||
)
|
||||
|
||||
# Create mock query with empty filter list
|
||||
# Create mock query with a saved chart filter
|
||||
chart_filter = {"col": "state", "op": "==", "val": "CA"}
|
||||
mock_query = Mock()
|
||||
mock_query.filter = []
|
||||
mock_query.filter = [chart_filter]
|
||||
mock_qc = Mock()
|
||||
mock_qc.queries = [mock_query]
|
||||
mock_qc.force = False
|
||||
@@ -64,13 +65,8 @@ def test_applies_dashboard_filters_to_non_legacy_chart(
|
||||
# Execute with dashboard_id
|
||||
result = ChartWarmUpCacheCommand(chart, 42, None).run()
|
||||
|
||||
# VALIDATE: Filters were added to query.filter
|
||||
assert len(mock_query.filter) == 1, "Filter should be added to query"
|
||||
assert mock_query.filter[0] == {
|
||||
"col": "country",
|
||||
"op": "in",
|
||||
"val": ["USA", "France"],
|
||||
}, "Filter content should match dashboard filter"
|
||||
# VALIDATE: Dashboard filters precede the chart's saved filters
|
||||
assert mock_query.filter == [legacy_filter, native_filter, chart_filter]
|
||||
|
||||
# VALIDATE: get_dashboard_extra_filters was called correctly
|
||||
mock_get_dashboard_filters.assert_called_once_with(123, 42)
|
||||
|
||||
@@ -102,10 +102,10 @@ def test_export_assets_command(mocker: MockerFixture) -> None:
|
||||
]
|
||||
|
||||
ExportTagsCommand = mocker.patch( # noqa: N806
|
||||
"superset.commands.export.assets.ExportTagsCommand.export"
|
||||
"superset.commands.export.assets.ExportTagsCommand"
|
||||
)
|
||||
|
||||
ExportTagsCommand.return_value = [
|
||||
ExportTagsCommand.return_value.run.return_value = [
|
||||
("tags.yaml", lambda: "<TAGS CONTENTS>"),
|
||||
]
|
||||
|
||||
|
||||
@@ -16,9 +16,17 @@
|
||||
# under the License.
|
||||
"""Tests for superset.views.utils module"""
|
||||
|
||||
from flask import current_app
|
||||
from unittest.mock import patch
|
||||
|
||||
from superset.views.utils import get_form_data
|
||||
from flask import current_app
|
||||
from sqlalchemy.orm.session import Session
|
||||
|
||||
from superset import db
|
||||
from superset.connectors.sqla.models import Database, SqlaTable
|
||||
from superset.models.dashboard import Dashboard
|
||||
from superset.models.slice import Slice
|
||||
from superset.utils import json
|
||||
from superset.views.utils import get_dashboard_extra_filters, get_form_data
|
||||
|
||||
|
||||
def test_get_form_data_handles_non_json_body_with_json_content_type() -> None:
|
||||
@@ -51,3 +59,78 @@ def test_get_form_data_handles_non_dict_json_body() -> None:
|
||||
|
||||
assert form_data == {}
|
||||
assert slc is None
|
||||
|
||||
|
||||
def test_get_dashboard_extra_filters_includes_native_filter_defaults(
|
||||
session: Session,
|
||||
) -> None:
|
||||
"""Native filter defaults are included in dashboard cache warming."""
|
||||
Dashboard.metadata.create_all(session.get_bind())
|
||||
|
||||
dataset = SqlaTable(
|
||||
table_name="extra_filters_table",
|
||||
database=Database(database_name="extra_filters_db", sqlalchemy_uri="sqlite://"),
|
||||
)
|
||||
db.session.add(dataset)
|
||||
db.session.flush()
|
||||
|
||||
chart = Slice(
|
||||
slice_name="chart_with_native_filter",
|
||||
datasource_id=dataset.id,
|
||||
datasource_type="table",
|
||||
)
|
||||
dashboard = Dashboard(
|
||||
dashboard_title="native_filter_dash",
|
||||
slices=[chart],
|
||||
published=True,
|
||||
json_metadata=json.dumps(
|
||||
{
|
||||
"native_filter_configuration": [
|
||||
{
|
||||
"id": "NATIVE_FILTER-1",
|
||||
"name": "Region filter",
|
||||
"type": "NATIVE_FILTER",
|
||||
"scope": {"rootPath": ["ROOT_ID"], "excluded": []},
|
||||
"targets": [{"column": {"name": "region"}}],
|
||||
"defaultDataMask": {
|
||||
"extraFormData": {
|
||||
"filters": [
|
||||
{"col": "region", "op": "IN", "val": ["APAC"]}
|
||||
]
|
||||
},
|
||||
"filterState": {"value": ["APAC"]},
|
||||
},
|
||||
"controlValues": {},
|
||||
}
|
||||
]
|
||||
}
|
||||
),
|
||||
position_json="{}",
|
||||
)
|
||||
db.session.add_all([chart, dashboard])
|
||||
db.session.flush()
|
||||
|
||||
with patch("superset.charts.data.dashboard_filter_context._check_dashboard_access"):
|
||||
extra_filters = get_dashboard_extra_filters(chart.id, dashboard.id)
|
||||
|
||||
assert extra_filters == [{"col": "region", "op": "IN", "val": ["APAC"]}]
|
||||
|
||||
legacy_filter = {"col": "country", "op": "in", "val": ["Brazil"]}
|
||||
metadata = json.loads(dashboard.json_metadata)
|
||||
metadata["default_filters"] = json.dumps({"legacy-filter": {"country": ["Brazil"]}})
|
||||
metadata["filter_scopes"] = {}
|
||||
dashboard.json_metadata = json.dumps(metadata)
|
||||
|
||||
with (
|
||||
patch("superset.charts.data.dashboard_filter_context._check_dashboard_access"),
|
||||
patch(
|
||||
"superset.views.utils.build_extra_filters",
|
||||
return_value=[legacy_filter],
|
||||
),
|
||||
):
|
||||
extra_filters = get_dashboard_extra_filters(chart.id, dashboard.id)
|
||||
|
||||
assert extra_filters == [
|
||||
legacy_filter,
|
||||
{"col": "region", "op": "IN", "val": ["APAC"]},
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user