Merge remote-tracking branch 'origin/master' into HEAD

This commit is contained in:
Joe Li
2026-07-15 00:19:58 -07:00
42 changed files with 2515 additions and 1388 deletions

View File

@@ -11,7 +11,15 @@ updates:
- package-ecosystem: "npm"
ignore:
- dependency-name: "@rjsf/*"
# TODO: remove below entries until React >= 19.0.0
# TODO: remove below entries once the application supports React >= 19.0.0
- dependency-name: "react"
update-types: ["version-update:semver-major"]
- dependency-name: "react-dom"
update-types: ["version-update:semver-major"]
- dependency-name: "@types/react"
update-types: ["version-update:semver-major"]
- dependency-name: "@types/react-dom"
update-types: ["version-update:semver-major"]
- dependency-name: "react-icons"
# JSDOM v30 doesn't play well with Jest v30
# Source: https://jestjs.io/blog#known-issues

View File

@@ -64,7 +64,7 @@ jobs:
# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@54f647b7e1bb85c95cddabcd46b0c578ec92bc1a # v4.36.3
uses: github/codeql-action/init@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4.37.0
with:
languages: ${{ matrix.language }}
# If you wish to specify custom queries, you can do so here or in a config file.
@@ -75,6 +75,6 @@ jobs:
# queries: security-extended,security-and-quality
- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@54f647b7e1bb85c95cddabcd46b0c578ec92bc1a # v4.36.3
uses: github/codeql-action/analyze@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4.37.0
with:
category: "/language:${{matrix.language}}"

View File

@@ -7,6 +7,11 @@ on:
- "[0-9].[0-9]*"
pull_request:
types: [synchronize, opened, reopened, ready_for_review]
# Nightly full-tree sweep. Per-PR runs only lint changed files, so a change
# that invalidates an untouched file (e.g. a type change that breaks an
# importing test) can pass every PR yet leave master red. This catches that.
schedule:
- cron: "0 6 * * *"
permissions:
contents: read
@@ -32,6 +37,9 @@ jobs:
with:
persist-credentials: false
submodules: recursive
# Full history so we can diff a PR/push against its base commit to
# determine changed files (see "Determine changed files" below).
fetch-depth: 0
- name: Setup Python
uses: ./.github/actions/setup-backend/
@@ -69,19 +77,82 @@ jobs:
restore-keys: |
pre-commit-v2-${{ runner.os }}-py${{ matrix.python-version }}-
- name: Get changed files
- name: Determine changed files
id: changed_files
uses: ./.github/actions/file-changes-action
with:
output: " "
env:
EVENT_NAME: ${{ github.event_name }}
BASE_SHA: ${{ github.event.pull_request.base.sha }}
BEFORE_SHA: ${{ github.event.before }}
run: |
set -euo pipefail
# Scheduled runs check the whole tree (see the pre-commit step).
if [ "${EVENT_NAME}" = "schedule" ]; then
echo "mode=all" >> "$GITHUB_OUTPUT"
exit 0
fi
# Resolve the commit to diff against.
base=""
if [ "${EVENT_NAME}" = "pull_request" ]; then
base="${BASE_SHA}"
elif [ -n "${BEFORE_SHA:-}" ] && \
[ "${BEFORE_SHA}" != "0000000000000000000000000000000000000000" ]; then
base="${BEFORE_SHA}"
fi
# Fail closed: if the diff base can't be resolved, check every file
# instead of silently checking nothing. Previously an empty file list
# made `pre-commit run --files` a no-op that still reported success,
# which let unlinted code reach master.
if [ -z "${base}" ] || ! git cat-file -e "${base}^{commit}" 2>/dev/null; then
echo "::notice::Could not resolve a diff base; falling back to --all-files."
echo "mode=all" >> "$GITHUB_OUTPUT"
exit 0
fi
# Files present in HEAD that changed since the base (drop deletions).
files="$(git diff --name-only --diff-filter=ACMRT "${base}...HEAD")"
if [ -z "${files}" ]; then
echo "mode=none" >> "$GITHUB_OUTPUT"
else
echo "mode=files" >> "$GITHUB_OUTPUT"
{
echo "files<<__CHANGED_FILES_EOF__"
echo "${files}"
echo "__CHANGED_FILES_EOF__"
} >> "$GITHUB_OUTPUT"
fi
- name: pre-commit
env:
MODE: ${{ steps.changed_files.outputs.mode }}
CHANGED_FILES: ${{ steps.changed_files.outputs.files }}
run: |
set +e # Don't exit immediately on failure
export SKIP=type-checking-frontend
pre-commit run --files $CHANGED_FILES
case "${MODE}" in
all)
echo " Running pre-commit on all files."
pre-commit run --all-files
;;
files)
echo " Running pre-commit on changed files:"
echo "${CHANGED_FILES}"
# shellcheck disable=SC2086
pre-commit run --files ${CHANGED_FILES}
;;
none)
echo " No source files changed; nothing for pre-commit to check."
exit 0
;;
*)
echo "⚠️ Unrecognized changed-files mode '${MODE}'; checking all files."
pre-commit run --all-files
;;
esac
PRE_COMMIT_EXIT_CODE=$?
git diff --quiet --exit-code
GIT_DIFF_EXIT_CODE=$?

3
.gitmodules vendored
View File

@@ -21,9 +21,6 @@
[submodule ".github/actions/pr-lint-action"]
path = .github/actions/pr-lint-action
url = https://github.com/morrisoncole/pr-lint-action
[submodule ".github/actions/file-changes-action"]
path = .github/actions/file-changes-action
url = https://github.com/trilom/file-changes-action
[submodule ".github/actions/cached-dependencies"]
path = .github/actions/cached-dependencies
url = https://github.com/apache-superset/cached-dependencies

View File

@@ -45,7 +45,7 @@ dependencies = [
# without the ``base.txt`` lock file (#40962).
"cachetools>=7.1.4, <8",
"celery>=5.6.3, <6.0.0",
"click>=8.4.0",
"click>=8.4.2",
"click-option-group",
"colorama",
"flask-cors>=6.0.5, <7.0",
@@ -155,7 +155,7 @@ elasticsearch = ["elasticsearch-dbapi>=0.2.13, <0.3.0"]
exasol = ["sqlalchemy-exasol>=2.4.0, <8.0"]
excel = ["xlrd>=2.0.2, <2.1"]
fastmcp = [
"fastmcp>=3.4.2,<4.0",
"fastmcp>=3.4.3,<4.0",
# tiktoken backs the response-size-guard token estimator. Without
# it, the middleware falls back to a coarser character-based
# heuristic that under-counts JSON-heavy MCP responses.
@@ -199,7 +199,7 @@ risingwave = ["sqlalchemy-risingwave"]
shillelagh = ["shillelagh[all]>=1.4.4, <2"]
singlestore = ["sqlalchemy-singlestoredb>=1.2.1, <2"]
snowflake = ["snowflake-sqlalchemy>=1.10.2, <2"]
sqlite = ["syntaqlite>=0.6.0,<0.7.0"]
sqlite = ["syntaqlite>=0.7.0,<0.8.0"]
spark = [
"pyhive[hive]>=0.6.5;python_version<'3.11'",
"pyhive[hive_pure_sasl]>=0.7",
@@ -244,7 +244,7 @@ development = [
"ruff",
"sqloxide",
"statsd",
"syntaqlite>=0.6.0,<0.7.0",
"syntaqlite>=0.7.0,<0.8.0",
]
[project.urls]

View File

@@ -60,7 +60,7 @@ cffi==2.0.0
# pynacl
charset-normalizer==3.4.2
# via requests
click==8.4.1
click==8.4.2
# via
# apache-superset (pyproject.toml)
# celery

View File

@@ -130,7 +130,7 @@ charset-normalizer==3.4.2
# via
# -c requirements/base-constraint.txt
# requests
click==8.4.1
click==8.4.2
# via
# -c requirements/base-constraint.txt
# apache-superset
@@ -238,9 +238,9 @@ et-xmlfile==2.0.0
# openpyxl
exceptiongroup==1.3.0
# via fastmcp-slim
fastmcp==3.4.2
fastmcp==3.4.4
# via apache-superset
fastmcp-slim==3.4.2
fastmcp-slim==3.4.4
# via fastmcp
filelock==3.20.3
# via
@@ -1017,7 +1017,7 @@ starlette==1.3.1
# mcp
statsd==4.0.1
# via apache-superset
syntaqlite==0.6.0
syntaqlite==0.7.1
# via apache-superset
tabulate==0.10.0
# via

View File

@@ -17,45 +17,38 @@
* under the License.
*/
import { Component, ReactNode } from 'react';
import { useState, useCallback, ReactNode } from 'react';
export type Props = {
children: ReactNode;
expandableWhat?: string;
};
type State = {
open: boolean;
};
export default function Expandable({ children, expandableWhat }: Props) {
const [open, setOpen] = useState(false);
export default class Expandable extends Component<Props, State> {
constructor(props: Props) {
super(props);
this.state = { open: false };
this.handleToggle = this.handleToggle.bind(this);
}
const handleToggle = useCallback(() => {
setOpen(prevOpen => !prevOpen);
}, []);
handleToggle() {
this.setState(({ open }) => ({ open: !open }));
}
const label = expandableWhat
? `${open ? 'Hide' : 'Show'} ${expandableWhat}`
: open
? 'Hide'
: 'Show';
render() {
const { open } = this.state;
const { children, expandableWhat } = this.props;
return (
<div>
<button
type="button"
className="btn btn-primary btn-sm"
onClick={this.handleToggle}
>
{`${open ? 'Hide' : 'Show'} ${expandableWhat}`}
</button>
<br />
<br />
{open ? children : null}
</div>
);
}
return (
<div>
<button
type="button"
className="btn btn-primary btn-sm"
onClick={handleToggle}
>
{label}
</button>
<br />
<br />
{open ? children : null}
</div>
);
}

View File

@@ -17,7 +17,7 @@
* under the License.
*/
import { Component, ReactNode } from 'react';
import { useState, useEffect, useCallback, useRef, ReactNode } from 'react';
import { t } from '@apache-superset/core/translation';
import {
SupersetClient,
@@ -36,12 +36,6 @@ export type Props = {
postPayload?: string;
};
type State = {
didVerify: boolean;
error?: Error | SupersetApiError;
payload?: object;
};
export const renderError = (error: Error) => (
<div>
The following error occurred, make sure you have <br />
@@ -54,29 +48,37 @@ export const renderError = (error: Error) => (
</div>
);
export default class VerifyCORS extends Component<Props, State> {
constructor(props: Props) {
super(props);
this.state = { didVerify: false };
this.handleVerify = this.handleVerify.bind(this);
}
export default function VerifyCORS({
children,
endpoint,
host,
method,
postPayload,
}: Props): JSX.Element {
const [didVerify, setDidVerify] = useState(false);
const [error, setError] = useState<Error | SupersetApiError | undefined>(
undefined,
);
const [payload, setPayload] = useState<object | undefined>(undefined);
componentDidUpdate(prevProps: Props) {
const { endpoint, host, postPayload, method } = this.props;
const prevPropsRef = useRef({ endpoint, host, postPayload, method });
useEffect(() => {
const prevProps = prevPropsRef.current;
if (
(this.state.didVerify || this.state.error) &&
(didVerify || error) &&
(prevProps.endpoint !== endpoint ||
prevProps.host !== host ||
prevProps.postPayload !== postPayload ||
prevProps.method !== method)
) {
// eslint-disable-next-line react/no-did-update-set-state
this.setState({ didVerify: false, error: undefined });
setDidVerify(false);
setError(undefined);
}
}
prevPropsRef.current = { endpoint, host, postPayload, method };
}, [endpoint, host, postPayload, method, didVerify, error]);
handleVerify() {
const { endpoint, host, postPayload, method } = this.props;
const handleVerify = useCallback(() => {
SupersetClient.reset();
SupersetClient.configure({
credentials: 'include',
@@ -94,43 +96,40 @@ export default class VerifyCORS extends Component<Props, State> {
}
return { error: 'Must provide valid endpoint and payload.' };
})
.then(result =>
this.setState({ didVerify: true, error: undefined, payload: result }),
)
.catch(error => this.setState({ error }));
}
.then(result => {
setDidVerify(true);
setError(undefined);
setPayload(result);
})
.catch(err => setError(err));
}, [endpoint, host, method, postPayload]);
render() {
const { didVerify, error, payload } = this.state;
const { children } = this.props;
return didVerify ? (
children({ payload })
) : (
<div className="row">
<div className="col-md-10">
This example requires CORS requests from this domain. <br />
<br />
1) enable CORS requests in your Superset App from{' '}
{`${window.location.origin}`}
<br />
2) configure your Superset App host name below <br />
3) click below to verify authentication. You may debug CORS further
using the `@superset-ui/connection` story. <br />
<br />
<Button type="primary" size="small" onClick={this.handleVerify}>
{t('Verify')}
</Button>
<br />
<br />
</div>
{error && (
<div className="col-md-8">
<ErrorMessage error={error} />
</div>
)}
return didVerify ? (
<>{children({ payload })}</>
) : (
<div className="row">
<div className="col-md-10">
This example requires CORS requests from this domain. <br />
<br />
1) enable CORS requests in your Superset App from{' '}
{`${window.location.origin}`}
<br />
2) configure your Superset App host name below <br />
3) click below to verify authentication. You may debug CORS further
using the `@superset-ui/connection` story. <br />
<br />
<Button type="primary" size="small" onClick={handleVerify}>
{t('Verify')}
</Button>
<br />
<br />
</div>
);
}
{error && (
<div className="col-md-8">
<ErrorMessage error={error} />
</div>
)}
</div>
);
}

View File

@@ -17,126 +17,108 @@
* under the License.
*/
import { PureComponent } from 'react';
import { useState, useCallback } from 'react';
import { formatNumber } from '@superset-ui/core';
interface NumberFormatValidatorState {
formatString: string;
testValues: (number | null | undefined)[];
}
const testValues: (number | null | undefined)[] = [
987654321,
12345.6789,
3000,
400.14,
70.00002,
1,
0,
-1,
-70.00002,
-400.14,
-3000,
-12345.6789,
-987654321,
Number.POSITIVE_INFINITY,
Number.NEGATIVE_INFINITY,
NaN,
null,
undefined,
];
class NumberFormatValidator extends PureComponent<
Record<string, never>,
NumberFormatValidatorState
> {
state: NumberFormatValidatorState = {
formatString: '.3~s',
testValues: [
987654321,
12345.6789,
3000,
400.14,
70.00002,
1,
0,
-1,
-70.00002,
-400.14,
-3000,
-12345.6789,
-987654321,
Number.POSITIVE_INFINITY,
Number.NEGATIVE_INFINITY,
NaN,
null,
undefined,
],
};
function NumberFormatValidator() {
const [formatString, setFormatString] = useState('.3~s');
constructor(props: Record<string, never>) {
super(props);
const handleFormatChange = useCallback(
(event: React.ChangeEvent<HTMLInputElement>) => {
setFormatString(event.target.value);
},
[],
);
this.handleFormatChange = this.handleFormatChange.bind(this);
}
handleFormatChange(event: React.ChangeEvent<HTMLInputElement>) {
this.setState({
formatString: event.target.value,
});
}
render() {
const { formatString, testValues } = this.state;
return (
<div className="container">
<div className="row" style={{ margin: '40px 20px 0 20px' }}>
<div className="col-sm">
<p>
This <code>@superset-ui/number-format</code> package enriches{' '}
<code>d3-format</code>
to handle invalid formats as well as edge case values. Use the
validator below to preview outputs from the specified format
string. See
<a
href="https://github.com/d3/d3-format#locale_format"
target="_blank"
rel="noopener noreferrer"
>
D3 Format Reference
</a>
for how to write a D3 format string.
</p>
</div>
</div>
<div className="row" style={{ margin: '10px 0 30px 0' }}>
<div className="col-sm" />
<div className="col-sm-8">
<div className="form">
<div className="form-group">
{/* eslint-disable-next-line jsx-a11y/label-has-associated-control */}
<label>
Enter D3 format string:
<input
id="formatString"
className="form-control form-control-lg"
type="text"
value={formatString}
onChange={this.handleFormatChange}
/>
</label>
</div>
</div>
</div>
<div className="col-sm" />
</div>
<div className="row">
<div className="col-sm">
<table className="table table-striped table-sm">
<thead>
<tr>
<th>Input (number)</th>
<th>Formatted output (string)</th>
</tr>
</thead>
<tbody>
{testValues.map((v, index) => (
<tr key={index}>
<td>
<code>{`${v}`}</code>
</td>
<td>
<code>&quot;{formatNumber(formatString, v)}&quot;</code>
</td>
</tr>
))}
</tbody>
</table>
</div>
return (
<div className="container">
<div className="row" style={{ margin: '40px 20px 0 20px' }}>
<div className="col-sm">
<p>
This <code>@superset-ui/number-format</code> package enriches{' '}
<code>d3-format</code>
to handle invalid formats as well as edge case values. Use the
validator below to preview outputs from the specified format string.
See
<a
href="https://github.com/d3/d3-format#locale_format"
target="_blank"
rel="noopener noreferrer"
>
D3 Format Reference
</a>
for how to write a D3 format string.
</p>
</div>
</div>
);
}
<div className="row" style={{ margin: '10px 0 30px 0' }}>
<div className="col-sm" />
<div className="col-sm-8">
<div className="form">
<div className="form-group">
{/* eslint-disable-next-line jsx-a11y/label-has-associated-control */}
<label>
Enter D3 format string:
<input
id="formatString"
className="form-control form-control-lg"
type="text"
value={formatString}
onChange={handleFormatChange}
/>
</label>
</div>
</div>
</div>
<div className="col-sm" />
</div>
<div className="row">
<div className="col-sm">
<table className="table table-striped table-sm">
<thead>
<tr>
<th>Input (number)</th>
<th>Formatted output (string)</th>
</tr>
</thead>
<tbody>
{testValues.map((v, index) => (
<tr key={index}>
<td>
<code>{`${v}`}</code>
</td>
<td>
<code>&quot;{formatNumber(formatString, v)}&quot;</code>
</td>
</tr>
))}
</tbody>
</table>
</div>
</div>
</div>
);
}
export default {

View File

@@ -17,115 +17,96 @@
* under the License.
*/
import { PureComponent } from 'react';
import { useState, useCallback } from 'react';
import { formatTime } from '@superset-ui/core';
interface TimeFormatValidatorState {
formatString: string;
testValues: (Date | number | null | undefined)[];
}
const testValues: (Date | number | null | undefined)[] = [
new Date(Date.UTC(1986, 5, 14, 8, 30, 53)),
new Date(Date.UTC(2001, 9, 27, 13, 45, 2, 678)),
new Date(Date.UTC(2009, 1, 1, 0, 0, 0)),
new Date(Date.UTC(2018, 1, 1, 10, 20, 33)),
0,
null,
undefined,
];
class TimeFormatValidator extends PureComponent<
Record<string, never>,
TimeFormatValidatorState
> {
state: TimeFormatValidatorState = {
formatString: '%Y-%m-%d %H:%M:%S',
testValues: [
new Date(Date.UTC(1986, 5, 14, 8, 30, 53)),
new Date(Date.UTC(2001, 9, 27, 13, 45, 2, 678)),
new Date(Date.UTC(2009, 1, 1, 0, 0, 0)),
new Date(Date.UTC(2018, 1, 1, 10, 20, 33)),
0,
null,
undefined,
],
};
function TimeFormatValidator() {
const [formatString, setFormatString] = useState('%Y-%m-%d %H:%M:%S');
constructor(props: Record<string, never>) {
super(props);
this.handleFormatChange = this.handleFormatChange.bind(this);
}
const handleFormatChange = useCallback(
(event: React.ChangeEvent<HTMLInputElement>) => {
setFormatString(event.target.value);
},
[],
);
handleFormatChange(event: React.ChangeEvent<HTMLInputElement>) {
this.setState({
formatString: event.target.value,
});
}
render() {
const { formatString, testValues } = this.state;
return (
<div className="container">
<div className="row" style={{ margin: '40px 20px 0 20px' }}>
<div className="col-sm">
<p>
This <code>@superset-ui/time-format</code> package enriches
<code>d3-time-format</code> to handle invalid formats as well as
edge case values. Use the validator below to preview outputs from
the specified format string. See &nbsp;
<a
href="https://github.com/d3/d3-time-format#locale_format"
target="_blank"
rel="noopener noreferrer"
>
D3 Time Format Reference
</a>
&nbsp;for how to write a D3 time format string.
</p>
</div>
</div>
<div className="row" style={{ margin: '10px 0 30px 0' }}>
<div className="col-sm" />
<div className="col-sm-8">
<div className="form">
<div className="form-group">
{/* eslint-disable-next-line jsx-a11y/label-has-associated-control */}
<label>
Enter D3 time format string:
<input
id="formatString"
className="form-control form-control-lg"
type="text"
value={formatString}
onChange={this.handleFormatChange}
/>
</label>
</div>
</div>
</div>
<div className="col-sm" />
</div>
<div className="row">
<div className="col-sm">
<table className="table table-striped table-sm">
<thead>
<tr>
<th>Input (time)</th>
<th>Formatted output (string)</th>
</tr>
</thead>
<tbody>
{testValues.map((v, index) => (
<tr key={index}>
<td>
<code>
{v instanceof Date ? v.toUTCString() : `${v}`}
</code>
</td>
<td>
<code>&quot;{formatTime(formatString, v)}&quot;</code>
</td>
</tr>
))}
</tbody>
</table>
</div>
return (
<div className="container">
<div className="row" style={{ margin: '40px 20px 0 20px' }}>
<div className="col-sm">
<p>
This <code>@superset-ui/time-format</code> package enriches
<code>d3-time-format</code> to handle invalid formats as well as
edge case values. Use the validator below to preview outputs from
the specified format string. See &nbsp;
<a
href="https://github.com/d3/d3-time-format#locale_format"
target="_blank"
rel="noopener noreferrer"
>
D3 Time Format Reference
</a>
&nbsp;for how to write a D3 time format string.
</p>
</div>
</div>
);
}
<div className="row" style={{ margin: '10px 0 30px 0' }}>
<div className="col-sm" />
<div className="col-sm-8">
<div className="form">
<div className="form-group">
{/* eslint-disable-next-line jsx-a11y/label-has-associated-control */}
<label>
Enter D3 time format string:
<input
id="formatString"
className="form-control form-control-lg"
type="text"
value={formatString}
onChange={handleFormatChange}
/>
</label>
</div>
</div>
</div>
<div className="col-sm" />
</div>
<div className="row">
<div className="col-sm">
<table className="table table-striped table-sm">
<thead>
<tr>
<th>Input (time)</th>
<th>Formatted output (string)</th>
</tr>
</thead>
<tbody>
{testValues.map((v, index) => (
<tr key={index}>
<td>
<code>{v instanceof Date ? v.toUTCString() : `${v}`}</code>
</td>
<td>
<code>&quot;{formatTime(formatString, v)}&quot;</code>
</td>
</tr>
))}
</tbody>
</table>
</div>
</div>
</div>
);
}
export default {

View File

@@ -26,9 +26,17 @@ import {
import { FeatureFlag, VizType } from '@superset-ui/core';
import mockState from 'spec/fixtures/mockState';
import { cachedSupersetGet } from 'src/utils/cachedSupersetGet';
import downloadAsImage from 'src/utils/downloadAsImage';
import downloadAsPdf from 'src/utils/downloadAsPdf';
import SliceHeaderControls, { SliceHeaderControlsProps } from '.';
jest.mock('src/utils/cachedSupersetGet');
jest.mock('src/utils/downloadAsImage', () =>
jest.fn(() => jest.fn().mockResolvedValue(undefined)),
);
jest.mock('src/utils/downloadAsPdf', () =>
jest.fn(() => jest.fn().mockResolvedValue(undefined)),
);
const mockCachedSupersetGet = cachedSupersetGet as jest.MockedFunction<
typeof cachedSupersetGet
@@ -127,6 +135,12 @@ const openMenu = () => {
userEvent.click(screen.getByRole('button', { name: 'More Options' }));
};
const mockDownloadAsImage = downloadAsImage as jest.MockedFunction<
typeof downloadAsImage
>;
const mockDownloadAsPdf = downloadAsPdf as jest.MockedFunction<
typeof downloadAsPdf
>;
const mockFullscreenElement = (getElement: () => Element | null) => {
Object.defineProperty(document, 'fullscreenElement', {
configurable: true,
@@ -136,6 +150,8 @@ const mockFullscreenElement = (getElement: () => Element | null) => {
beforeEach(() => {
mockCachedSupersetGet.mockClear();
mockDownloadAsImage.mockClear();
mockDownloadAsPdf.mockClear();
mockCachedSupersetGet.mockResolvedValue({
response: {} as Response,
json: {
@@ -675,6 +691,109 @@ test('Should pass formData to Share menu for embed code feature', () => {
expect(screen.getByText('Share')).toBeInTheDocument();
});
test('Download submenu shows standardized export screenshot and PDF labels', async () => {
const props = createProps();
renderWrapper(props);
openMenu();
userEvent.hover(screen.getByText('Download'));
expect(
await screen.findByText('Export screenshot (jpeg)'),
).toBeInTheDocument();
expect(screen.getByText('Export screenshot (png)')).toBeInTheDocument();
expect(screen.getByText('Export as PDF')).toBeInTheDocument();
});
test('Clicking "Export screenshot (jpeg)" calls downloadAsImage and logEvent', async () => {
const props = createProps();
renderWrapper(props);
openMenu();
userEvent.hover(screen.getByText('Download'));
userEvent.click(await screen.findByText('Export screenshot (jpeg)'));
expect(downloadAsImage).toHaveBeenCalledWith(
`.dashboard-chart-id-${SLICE_ID}`,
props.slice.slice_name,
true,
expect.anything(),
);
expect(props.logEvent).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ chartId: SLICE_ID }),
);
});
test('Export screenshot (png) submenu shows Transparent and Solid options', async () => {
const props = createProps();
renderWrapper(props);
openMenu();
userEvent.hover(screen.getByText('Download'));
userEvent.hover(await screen.findByText('Export screenshot (png)'));
expect(await screen.findByText('Transparent background')).toBeInTheDocument();
expect(screen.getByText('Solid background')).toBeInTheDocument();
});
test('Clicking "Transparent background" calls downloadAsImage with transparent option and logEvent', async () => {
const props = createProps();
renderWrapper(props);
openMenu();
userEvent.hover(screen.getByText('Download'));
userEvent.hover(await screen.findByText('Export screenshot (png)'));
userEvent.click(await screen.findByText('Transparent background'));
expect(downloadAsImage).toHaveBeenCalledWith(
`.dashboard-chart-id-${SLICE_ID}`,
props.slice.slice_name,
true,
expect.anything(),
{ format: 'png', backgroundType: 'transparent' },
);
expect(props.logEvent).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
chartId: SLICE_ID,
backgroundType: 'transparent',
}),
);
});
test('Clicking "Solid background" calls downloadAsImage with solid option and logEvent', async () => {
const props = createProps();
renderWrapper(props);
openMenu();
userEvent.hover(screen.getByText('Download'));
userEvent.hover(await screen.findByText('Export screenshot (png)'));
userEvent.click(await screen.findByText('Solid background'));
expect(downloadAsImage).toHaveBeenCalledWith(
`.dashboard-chart-id-${SLICE_ID}`,
props.slice.slice_name,
true,
expect.anything(),
{ format: 'png', backgroundType: 'solid' },
);
expect(props.logEvent).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
chartId: SLICE_ID,
backgroundType: 'solid',
}),
);
});
test('Clicking "Export as PDF" calls downloadAsPdf and logEvent', async () => {
const props = createProps();
renderWrapper(props);
openMenu();
userEvent.hover(screen.getByText('Download'));
userEvent.click(await screen.findByText('Export as PDF'));
expect(downloadAsPdf).toHaveBeenCalledWith(
`.dashboard-chart-id-${SLICE_ID}`,
props.slice.slice_name,
true,
);
expect(props.logEvent).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ chartId: SLICE_ID }),
);
});
test('Should show single fetched query tooltip with timestamp', async () => {
const updatedDttm = Date.parse('2024-01-28T10:00:00.000Z');
const props = createProps();

View File

@@ -50,12 +50,17 @@ import {
} from '@superset-ui/core/components';
import { useShareMenuItems } from 'src/dashboard/components/menu/ShareMenuItems';
import downloadAsImage from 'src/utils/downloadAsImage';
import downloadAsPdf from 'src/utils/downloadAsPdf';
import { getSliceHeaderTooltip } from 'src/dashboard/util/getSliceHeaderTooltip';
import { Icons } from '@superset-ui/core/components/Icons';
import ViewQueryModal from 'src/explore/components/controls/ViewQueryModal';
import { ResultsPaneOnDashboard } from 'src/explore/components/DataTablesPane';
import { useDrillDetailMenuItems } from 'src/components/Chart/useDrillDetailMenuItems';
import { LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE } from 'src/logger/LogUtils';
import {
LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE,
LOG_ACTIONS_CHART_DOWNLOAD_AS_PNG,
LOG_ACTIONS_CHART_DOWNLOAD_AS_PDF,
} 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';
@@ -302,29 +307,90 @@ const SliceHeaderControls = (
props.exportXLSX?.(props.slice.slice_id);
break;
case MenuKeys.DownloadAsImage: {
// menu closes with a delay, we need to hide it manually,
// so that we don't capture it on the screenshot
// Hide the dropdown menu so it is not captured in the screenshot
const menu = document.querySelector(
'.ant-dropdown:not(.ant-dropdown-hidden)',
) as HTMLElement;
) as HTMLElement | null;
if (menu) {
menu.style.visibility = 'hidden';
}
downloadAsImage(
getScreenshotNodeSelector(props.slice.slice_id),
props.slice.slice_name,
true,
theme,
)(domEvent).then(() => {
Promise.resolve(
downloadAsImage(
getScreenshotNodeSelector(props.slice.slice_id),
props.slice.slice_name,
true,
theme,
)(domEvent),
).finally(() => {
if (menu) {
menu.style.visibility = 'visible';
}
});
// eslint-disable-next-line no-unused-expressions
props.logEvent?.(LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE, {
chartId: props.slice.slice_id,
});
break;
}
case MenuKeys.DownloadAsPngTransparent:
case MenuKeys.DownloadAsPngSolid: {
const menu = document.querySelector(
'.ant-dropdown:not(.ant-dropdown-hidden)',
) as HTMLElement | null;
if (menu) {
menu.style.visibility = 'hidden';
}
const backgroundType =
key === MenuKeys.DownloadAsPngTransparent ? 'transparent' : 'solid';
Promise.resolve(
downloadAsImage(
getScreenshotNodeSelector(props.slice.slice_id),
props.slice.slice_name,
true,
theme,
{ format: 'png', backgroundType },
)(domEvent),
).finally(() => {
if (menu) {
menu.style.visibility = 'visible';
}
});
// eslint-disable-next-line no-unused-expressions
props.logEvent?.(LOG_ACTIONS_CHART_DOWNLOAD_AS_PNG, {
chartId: props.slice.slice_id,
backgroundType,
});
break;
}
case MenuKeys.DownloadAsPdf: {
const menu = document.querySelector(
'.ant-dropdown:not(.ant-dropdown-hidden)',
) as HTMLElement | null;
if (menu) {
menu.style.visibility = 'hidden';
}
Promise.resolve(
downloadAsPdf(
getScreenshotNodeSelector(props.slice.slice_id),
props.slice.slice_name,
true,
)(domEvent),
).finally(() => {
if (menu) {
menu.style.visibility = 'visible';
}
});
// eslint-disable-next-line no-unused-expressions
props.logEvent?.(LOG_ACTIONS_CHART_DOWNLOAD_AS_PDF, {
chartId: props.slice.slice_id,
});
break;
}
case MenuKeys.ExportPivotXlsx: {
const sliceSelector = `#chart-id-${props.slice.slice_id}`;
props.exportPivotExcel?.(
@@ -614,9 +680,30 @@ const SliceHeaderControls = (
: []),
{
key: MenuKeys.DownloadAsImage,
label: t('Download as image'),
label: t('Export screenshot (jpeg)'),
icon: <Icons.FileImageOutlined css={dropdownIconsStyles} />,
},
{
type: 'submenu',
key: 'download_as_png_submenu',
label: t('Export screenshot (png)'),
icon: <Icons.FileImageOutlined css={dropdownIconsStyles} />,
children: [
{
key: MenuKeys.DownloadAsPngTransparent,
label: t('Transparent background'),
},
{
key: MenuKeys.DownloadAsPngSolid,
label: t('Solid background'),
},
],
},
{
key: MenuKeys.DownloadAsPdf,
label: t('Export as PDF'),
icon: <Icons.FileOutlined css={dropdownIconsStyles} />,
},
],
});
}

View File

@@ -367,6 +367,9 @@ export interface SliceEntitiesState {
export enum MenuKeys {
DownloadAsImage = 'download_as_image',
DownloadAsPngTransparent = 'download_as_png_transparent',
DownloadAsPngSolid = 'download_as_png_solid',
DownloadAsPdf = 'download_as_pdf',
ExploreChart = 'explore_chart',
ExportCsv = 'export_csv',
ExportPivotCsv = 'export_pivot_csv',

View File

@@ -49,6 +49,7 @@ import { useToasts } from 'src/components/MessageToasts/withToasts';
import { DEFAULT_CSV_STREAMING_ROW_THRESHOLD } from 'src/constants';
import { exportChart, getChartKey } from 'src/explore/exploreUtils';
import downloadAsImage from 'src/utils/downloadAsImage';
import downloadAsPdf from 'src/utils/downloadAsPdf';
import { getChartPermalink } from 'src/utils/urlUtils';
import copyTextToClipboard from 'src/utils/copy';
import { useHeaderReportMenuItems } from 'src/features/reports/ReportModal/HeaderReportDropdown';
@@ -56,6 +57,8 @@ import { MenuItemTooltip } from 'src/components/Chart/DisabledMenuItemTooltip';
import { logEvent } from 'src/logger/actions';
import {
LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE,
LOG_ACTIONS_CHART_DOWNLOAD_AS_PNG,
LOG_ACTIONS_CHART_DOWNLOAD_AS_PDF,
LOG_ACTIONS_CHART_DOWNLOAD_AS_JSON,
LOG_ACTIONS_CHART_DOWNLOAD_AS_CSV,
LOG_ACTIONS_CHART_DOWNLOAD_AS_CSV_PIVOTED,
@@ -88,9 +91,15 @@ const MENU_KEYS = {
EXPORT_TO_JSON: 'export_to_json',
EXPORT_TO_XLSX: 'export_to_xlsx',
EXPORT_ALL_SCREENSHOT: 'export_all_screenshot',
EXPORT_ALL_PNG_TRANSPARENT: 'export_all_png_transparent',
EXPORT_ALL_PNG_SOLID: 'export_all_png_solid',
EXPORT_ALL_PDF: 'export_all_pdf',
EXPORT_CURRENT_TO_CSV: 'export_current_to_csv',
EXPORT_CURRENT_TO_JSON: 'export_current_to_json',
EXPORT_CURRENT_SCREENSHOT: 'export_current_screenshot',
EXPORT_CURRENT_PNG_TRANSPARENT: 'export_current_png_transparent',
EXPORT_CURRENT_PNG_SOLID: 'export_current_png_solid',
EXPORT_CURRENT_PDF: 'export_current_pdf',
EXPORT_CURRENT_XLSX: 'export_current_xlsx',
SHARE_SUBMENU: 'share_submenu',
COPY_PERMALINK: 'copy_permalink',
@@ -107,6 +116,98 @@ const MENU_KEYS = {
const VIZ_TYPES_PIVOTABLE = [VizType.PivotTable];
const CHART_EXPORT_SELECTOR = '.panel-body .chart-container';
export function getExportScreenshotMenuItems({
chartSelector,
sliceName,
chartId,
theme,
setIsDropdownVisible,
dispatch,
submenuKey,
transparentKey,
solidKey,
pdfKey,
}: {
chartSelector: string;
sliceName: string;
chartId?: number;
theme: ReturnType<typeof useTheme>;
setIsDropdownVisible: (visible: boolean) => void;
dispatch: Dispatch<any>;
submenuKey: string;
transparentKey: string;
solidKey: string;
pdfKey: string;
}) {
return [
{
type: 'submenu' as const,
key: submenuKey,
label: t('Export screenshot (png)'),
icon: <Icons.FileImageOutlined />,
children: [
{
key: transparentKey,
label: t('Transparent background'),
onClick: (e: {
domEvent: React.MouseEvent | React.KeyboardEvent;
}) => {
downloadAsImage(chartSelector, sliceName, true, theme, {
format: 'png',
backgroundType: 'transparent',
})(e.domEvent);
setIsDropdownVisible(false);
dispatch(
logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_PNG, {
chartId,
chartName: sliceName,
backgroundType: 'transparent',
}),
);
},
},
{
key: solidKey,
label: t('Solid background'),
onClick: (e: {
domEvent: React.MouseEvent | React.KeyboardEvent;
}) => {
downloadAsImage(chartSelector, sliceName, true, theme, {
format: 'png',
backgroundType: 'solid',
})(e.domEvent);
setIsDropdownVisible(false);
dispatch(
logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_PNG, {
chartId,
chartName: sliceName,
backgroundType: 'solid',
}),
);
},
},
],
},
{
key: pdfKey,
label: t('Export as PDF'),
icon: <Icons.FileOutlined />,
onClick: (e: { domEvent: React.MouseEvent | React.KeyboardEvent }) => {
downloadAsPdf(chartSelector, sliceName, true)(e.domEvent);
setIsDropdownVisible(false);
dispatch(
logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_PDF, {
chartId,
chartName: sliceName,
}),
);
},
},
];
}
export const MenuItemWithCheckboxContainer = styled.div`
${({ theme }) => css`
display: flex;
@@ -811,7 +912,7 @@ export const useExploreAdditionalActionsMenu = (
disabled: imageExportDisabled,
onClick: (e: { domEvent: React.MouseEvent | React.KeyboardEvent }) => {
downloadAsImage(
'.panel-body .chart-container',
CHART_EXPORT_SELECTOR,
slice?.slice_name ?? t('New chart'),
true,
theme,
@@ -825,6 +926,18 @@ export const useExploreAdditionalActionsMenu = (
);
},
},
...getExportScreenshotMenuItems({
chartSelector: CHART_EXPORT_SELECTOR,
sliceName: slice?.slice_name ?? t('New chart'),
chartId: slice?.slice_id,
theme,
setIsDropdownVisible,
dispatch,
submenuKey: 'export_all_png_submenu',
transparentKey: MENU_KEYS.EXPORT_ALL_PNG_TRANSPARENT,
solidKey: MENU_KEYS.EXPORT_ALL_PNG_SOLID,
pdfKey: MENU_KEYS.EXPORT_ALL_PDF,
}),
{
key: MENU_KEYS.EXPORT_TO_XLSX,
label: dataExportLabel(t('Export to Excel')),
@@ -920,7 +1033,7 @@ export const useExploreAdditionalActionsMenu = (
disabled: imageExportDisabled,
onClick: (e: { domEvent: React.MouseEvent | React.KeyboardEvent }) => {
downloadAsImage(
'.panel-body .chart-container',
CHART_EXPORT_SELECTOR,
slice?.slice_name ?? t('New chart'),
true,
theme,
@@ -934,6 +1047,18 @@ export const useExploreAdditionalActionsMenu = (
);
},
},
...getExportScreenshotMenuItems({
chartSelector: CHART_EXPORT_SELECTOR,
sliceName: slice?.slice_name ?? t('New chart'),
chartId: slice?.slice_id,
theme,
setIsDropdownVisible,
dispatch,
submenuKey: 'export_current_png_submenu',
transparentKey: MENU_KEYS.EXPORT_CURRENT_PNG_TRANSPARENT,
solidKey: MENU_KEYS.EXPORT_CURRENT_PNG_SOLID,
pdfKey: MENU_KEYS.EXPORT_CURRENT_PDF,
}),
{
key: MENU_KEYS.EXPORT_CURRENT_XLSX,
label: dataExportLabel(t('Export to Excel')),

View File

@@ -19,7 +19,12 @@
import { ComponentType } from 'react';
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { useExploreAdditionalActionsMenu } from './index';
import downloadAsImage from 'src/utils/downloadAsImage';
import downloadAsPdf from 'src/utils/downloadAsPdf';
import {
useExploreAdditionalActionsMenu,
getExportScreenshotMenuItems,
} from './index';
import * as exploreUtils from 'src/explore/exploreUtils';
jest.mock('src/explore/exploreUtils', () => ({
@@ -29,6 +34,22 @@ jest.mock('src/explore/exploreUtils', () => ({
getChartKey: jest.fn(() => 'test_chart_key'),
}));
jest.mock('src/utils/downloadAsImage', () => ({
__esModule: true,
default: jest.fn(() => jest.fn()),
}));
jest.mock('src/utils/downloadAsPdf', () => ({
__esModule: true,
default: jest.fn(() => jest.fn()),
}));
const mockDownloadAsImage = downloadAsImage as jest.MockedFunction<
typeof downloadAsImage
>;
const mockDownloadAsPdf = downloadAsPdf as jest.MockedFunction<
typeof downloadAsPdf
>;
const mockExportChart = exploreUtils.exportChart as jest.Mock;
const mockAddDangerToast = jest.fn();
@@ -148,3 +169,83 @@ test('shows 413 error toast when Export Current View CSV server path fails with
);
});
});
const CHART_SELECTOR = '.panel-body .chart-container';
const SLICE_NAME = 'My chart';
const CHART_ID = 42;
const domEvent = {} as React.MouseEvent;
const buildScreenshotItems = () => {
const setIsDropdownVisible = jest.fn();
const dispatch = jest.fn();
const items = getExportScreenshotMenuItems({
chartSelector: CHART_SELECTOR,
sliceName: SLICE_NAME,
chartId: CHART_ID,
theme: {} as any,
setIsDropdownVisible,
dispatch,
submenuKey: 'export_png_submenu',
transparentKey: 'export_png_transparent',
solidKey: 'export_png_solid',
pdfKey: 'export_pdf',
}) as any[];
return { items, setIsDropdownVisible, dispatch };
};
test('getExportScreenshotMenuItems builds the PNG submenu and PDF item with the provided keys', () => {
const { items } = buildScreenshotItems();
const [pngSubmenu, pdfItem] = items;
expect(pngSubmenu.key).toBe('export_png_submenu');
expect(pngSubmenu.children).toHaveLength(2);
expect(pngSubmenu.children[0].key).toBe('export_png_transparent');
expect(pngSubmenu.children[1].key).toBe('export_png_solid');
expect(pdfItem.key).toBe('export_pdf');
});
test('getExportScreenshotMenuItems transparent option downloads a transparent PNG and dispatches a log event', () => {
const { items, setIsDropdownVisible, dispatch } = buildScreenshotItems();
items[0].children[0].onClick({ domEvent });
expect(mockDownloadAsImage).toHaveBeenCalledWith(
CHART_SELECTOR,
SLICE_NAME,
true,
expect.anything(),
{ format: 'png', backgroundType: 'transparent' },
);
expect(setIsDropdownVisible).toHaveBeenCalledWith(false);
expect(dispatch).toHaveBeenCalledTimes(1);
});
test('getExportScreenshotMenuItems solid option downloads a solid PNG and dispatches a log event', () => {
const { items, setIsDropdownVisible, dispatch } = buildScreenshotItems();
items[0].children[1].onClick({ domEvent });
expect(mockDownloadAsImage).toHaveBeenCalledWith(
CHART_SELECTOR,
SLICE_NAME,
true,
expect.anything(),
{ format: 'png', backgroundType: 'solid' },
);
expect(setIsDropdownVisible).toHaveBeenCalledWith(false);
expect(dispatch).toHaveBeenCalledTimes(1);
});
test('getExportScreenshotMenuItems PDF option calls downloadAsPdf and dispatches a log event', () => {
const { items, setIsDropdownVisible, dispatch } = buildScreenshotItems();
items[1].onClick({ domEvent });
expect(mockDownloadAsPdf).toHaveBeenCalledWith(
CHART_SELECTOR,
SLICE_NAME,
true,
);
expect(setIsDropdownVisible).toHaveBeenCalledWith(false);
expect(dispatch).toHaveBeenCalledTimes(1);
});

View File

@@ -861,9 +861,9 @@ test('opens Schedule Section on click', async () => {
useRedux: true,
});
userEvent.click(screen.getByTestId('schedule-panel'));
const scheduleHeader = within(
const [scheduleHeader] = within(
screen.getByRole('tab', { expanded: true }),
).queryAllByText(/schedule/i)[0];
).queryAllByText(/schedule/i);
expect(scheduleHeader).toBeInTheDocument();
});
test('renders default Schedule fields', async () => {
@@ -929,9 +929,9 @@ test('opens Notification Method Section on click', async () => {
useRedux: true,
});
userEvent.click(screen.getByTestId('notification-method-panel'));
const notificationMethodHeader = within(
const [notificationMethodHeader] = within(
screen.getByRole('tab', { expanded: true }),
).queryAllByText(/notification method/i)[0];
).queryAllByText(/notification method/i);
expect(notificationMethodHeader).toBeInTheDocument();
});

View File

@@ -54,6 +54,8 @@ export const LOG_ACTIONS_DASHBOARD_DOWNLOAD_AS_IMAGE =
export const LOG_ACTIONS_DASHBOARD_DOWNLOAD_AS_PDF =
'dashboard_download_as_pdf';
export const LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE = 'chart_download_as_image';
export const LOG_ACTIONS_CHART_DOWNLOAD_AS_PNG = 'chart_download_as_png';
export const LOG_ACTIONS_CHART_DOWNLOAD_AS_PDF = 'chart_download_as_pdf';
export const LOG_ACTIONS_CHART_DOWNLOAD_AS_CSV = 'chart_download_as_csv';
export const LOG_ACTIONS_CHART_DOWNLOAD_AS_CSV_PIVOTED =
'chart_download_as_csv_pivoted';
@@ -112,6 +114,8 @@ export const LOG_EVENT_TYPE_USER = new Set([
LOG_ACTIONS_DASHBOARD_DOWNLOAD_AS_IMAGE,
LOG_ACTIONS_DASHBOARD_DOWNLOAD_AS_PDF,
LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE,
LOG_ACTIONS_CHART_DOWNLOAD_AS_PNG,
LOG_ACTIONS_CHART_DOWNLOAD_AS_PDF,
]);
export const LOG_EVENT_DATASET_TYPE_DATASET_CREATION = [

View File

@@ -24,7 +24,7 @@ import downloadAsImageOptimized, {
jest.mock('dom-to-image-more', () => ({
__esModule: true,
default: { toJpeg: jest.fn() },
default: { toJpeg: jest.fn(), toPng: jest.fn() },
}));
jest.mock('src/components/MessageToasts/actions', () => ({
@@ -36,6 +36,7 @@ jest.mock('@apache-superset/core/translation', () => ({
}));
const mockToJpeg = domToImage.toJpeg as jest.Mock;
const mockToPng = domToImage.toPng as jest.Mock;
const mockAddWarningToast = addWarningToast as jest.Mock;
// document.fonts.ready is not implemented in jsdom; provide a resolved promise
@@ -81,6 +82,7 @@ function attachMockApi(
beforeEach(() => {
jest.clearAllMocks();
mockToJpeg.mockResolvedValue('data:image/jpeg;base64,test');
mockToPng.mockResolvedValue('data:image/png;base64,test');
});
afterEach(() => {
@@ -650,6 +652,55 @@ test('ag-grid path uses theme colorBgContainer as background', async () => {
jest.useRealTimers();
});
test('ag-grid path exports PNG (transparent) via toPng when format is png', async () => {
jest.useFakeTimers();
const { container, agContainer, cleanup } = buildAgGridElement();
attachMockApi(agContainer);
const theme = { colorBgContainer: '#1a1a2e' } as any;
const handler = downloadAsImageOptimized('div', 'My Chart', false, theme, {
format: 'png',
backgroundType: 'transparent',
});
const exportPromise = handler(syntheticEventFor(container));
await jest.runAllTimersAsync();
await exportPromise;
// format=png must route to toPng (not toJpeg) with a transparent background
expect(mockToPng).toHaveBeenCalledWith(
expect.any(HTMLElement),
expect.objectContaining({ bgcolor: 'transparent' }),
);
expect(mockToJpeg).not.toHaveBeenCalled();
cleanup();
jest.useRealTimers();
});
test('ag-grid path exports PNG (solid) via toPng using theme background', async () => {
jest.useFakeTimers();
const { container, agContainer, cleanup } = buildAgGridElement();
attachMockApi(agContainer);
const theme = { colorBgContainer: '#1a1a2e' } as any;
const handler = downloadAsImageOptimized('div', 'My Chart', false, theme, {
format: 'png',
backgroundType: 'solid',
});
const exportPromise = handler(syntheticEventFor(container));
await jest.runAllTimersAsync();
await exportPromise;
expect(mockToPng).toHaveBeenCalledWith(
expect.any(HTMLElement),
expect.objectContaining({ bgcolor: '#1a1a2e' }),
);
expect(mockToJpeg).not.toHaveBeenCalled();
cleanup();
jest.useRealTimers();
});
test('ag-grid path falls back to white background when theme is absent', async () => {
jest.useFakeTimers();
const { container, agContainer, cleanup } = buildAgGridElement();

View File

@@ -25,6 +25,8 @@ import { addWarningToast } from 'src/components/MessageToasts/actions';
import type { AgGridContainerElement } from '@superset-ui/core/components';
const IMAGE_DOWNLOAD_QUALITY = 0.95;
const PNG_SCALE = 2; // Higher quality for PNG
export type BackgroundType = 'transparent' | 'solid';
const TRANSPARENT_RGBA = 'transparent';
const POLL_INTERVAL_MS = 100;
@@ -268,6 +270,13 @@ const createEnhancedClone = (
return { clone, cleanup };
};
export type ImageFormat = 'jpeg' | 'png';
export interface DownloadImageOptions {
format?: ImageFormat;
backgroundType?: BackgroundType;
}
// Polls until scrollHeight is stable for minStablePolls consecutive intervals or maxMs elapses.
// ag-grid has no "layout settled" event, so polling is the recommended workaround.
export const waitForStableScrollHeight = (
@@ -312,7 +321,10 @@ export default function downloadAsImageOptimized(
description: string,
isExactSelector = false,
theme?: SupersetTheme,
options: DownloadImageOptions = {},
) {
const { format = 'jpeg', backgroundType = 'solid' } = options;
return async (event: SyntheticEvent) => {
const elementToPrint = isExactSelector
? document.querySelector(selector)
@@ -331,6 +343,13 @@ export default function downloadAsImageOptimized(
!node.className.includes('header-controls')
: true;
const isPng = format === 'png';
const scale = isPng ? PNG_SCALE : 1;
const bgcolor =
isPng && backgroundType === 'transparent'
? 'transparent'
: theme?.colorBgContainer;
// Only apply ag-grid path for single-chart captures.
// Skip entirely for dashboard-level exports (selector targets the .dashboard root).
const isDashboardCapture = (
@@ -420,17 +439,29 @@ export default function downloadAsImageOptimized(
const imageHeight = agRootWrapper.scrollHeight;
const dataUrl = await domToImage.toJpeg(agRootWrapper, {
bgcolor: theme?.colorBgContainer,
const agImageOptions = {
bgcolor,
filter,
quality: IMAGE_DOWNLOAD_QUALITY,
height: imageHeight,
width: originalWidth,
height: imageHeight * scale,
width: originalWidth * scale,
cacheBust: true,
});
...(isPng && {
style: {
transform: `scale(${PNG_SCALE})`,
transformOrigin: 'top left',
width: `${originalWidth}px`,
height: `${imageHeight}px`,
},
}),
};
const dataUrl = isPng
? await domToImage.toPng(agRootWrapper, agImageOptions)
: await domToImage.toJpeg(agRootWrapper, agImageOptions);
const link = document.createElement('a');
link.download = `${generateFileStem(description)}.jpg`;
link.download = `${generateFileStem(description)}.${isPng ? 'png' : 'jpg'}`;
link.href = dataUrl;
link.click();
} catch (error) {
@@ -466,20 +497,33 @@ export default function downloadAsImageOptimized(
);
cleanup = cleanupFn;
const dataUrl = await domToImage.toJpeg(clone, {
bgcolor: theme?.colorBgContainer,
const imageOptions = {
bgcolor,
filter,
quality: IMAGE_DOWNLOAD_QUALITY,
height: clone.scrollHeight,
width: clone.scrollWidth,
height: clone.scrollHeight * scale,
width: clone.scrollWidth * scale,
cacheBust: true,
});
...(isPng && {
style: {
transform: `scale(${PNG_SCALE})`,
transformOrigin: 'top left',
width: `${clone.scrollWidth}px`,
height: `${clone.scrollHeight}px`,
},
}),
};
const dataUrl = isPng
? await domToImage.toPng(clone, imageOptions)
: await domToImage.toJpeg(clone, imageOptions);
cleanup();
cleanup = null;
const extension = isPng ? 'png' : 'jpg';
const link = document.createElement('a');
link.download = `${generateFileStem(description)}.jpg`;
link.download = `${generateFileStem(description)}.${extension}`;
link.href = dataUrl;
link.click();
} catch (error) {

View File

@@ -444,9 +444,8 @@ test.each(doublePrefixTestCases)(
const expectedEndpoint = `/api/v1/${resource}/export/?q=!(${ids.join(',')})`;
// Explicitly verify no prefix in endpoint - this will fail if ensureAppRoot is used
const callArgs = (SupersetClient.get as jest.Mock).mock.calls.slice(
-1,
)[0][0];
const [lastCall] = (SupersetClient.get as jest.Mock).mock.calls.slice(-1);
const [callArgs] = lastCall;
expect(callArgs.endpoint).not.toContain(appRoot);
expect(callArgs.endpoint).toBe(expectedEndpoint);

View File

@@ -103,6 +103,19 @@ class DatasourceTypeUpdateRequiredValidationError(ValidationError):
)
class ChartQueryContextDatasourceMismatchValidationError(ValidationError):
"""
Raised when a query-context-only update carries a datasource that does not
match the chart's own datasource.
"""
def __init__(self) -> None:
super().__init__(
_("The query context datasource does not match the chart datasource"),
field_name="query_context",
)
class ChartNotFoundError(CommandException):
message = "Chart not found."

View File

@@ -29,6 +29,7 @@ from superset.commands.chart.exceptions import (
ChartForbiddenError,
ChartInvalidError,
ChartNotFoundError,
ChartQueryContextDatasourceMismatchValidationError,
ChartUpdateFailedError,
DashboardsForbiddenError,
DashboardsNotFoundValidationError,
@@ -46,6 +47,7 @@ from superset.exceptions import SupersetSecurityException
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.tags.models import ObjectType
from superset.utils import json
from superset.utils.decorators import on_error, transaction
logger = logging.getLogger(__name__)
@@ -106,6 +108,55 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
if not security_manager.is_editor(dash):
raise DashboardsForbiddenError()
def _validate_query_context_datasource(
self, exceptions: list[ValidationError]
) -> None:
"""
Ensure a query-context-only update keeps the chart's own datasource.
The submitted query context is only verified when it carries a parseable
``datasource`` object; a payload that references a different datasource than
the chart's persisted one is rejected. Payloads without a datasource fall
back to the chart's datasource at execution time and need no check.
"""
if not self._model:
return
raw_query_context = self._properties.get("query_context")
if not raw_query_context:
return
try:
query_context = json.loads(raw_query_context)
except (TypeError, ValueError):
# TypeError covers a query_context that isn't a string (e.g. an
# already-parsed dict); that shape is intentionally out of scope
# here since the schema serializes it as a JSON string. ValueError
# covers a string that failed to parse. Either way, an unverifiable
# payload is left for downstream handling rather than guessed at.
return
datasource = (
query_context.get("datasource") if isinstance(query_context, dict) else None
)
if not isinstance(datasource, dict):
return
try:
ids_match = int(datasource["id"]) == self._model.datasource_id
except (KeyError, TypeError, ValueError):
ids_match = False
# A datasource object must carry a type that matches the chart's own.
# Treating a missing type as valid would let an id-only payload through,
# and query-context loading reads datasource["type"] directly, so that
# payload raises KeyError when the saved context is later replayed.
datasource_type = datasource.get("type")
types_match = str(datasource_type) == self._model.datasource_type
if not ids_match or not types_match:
exceptions.append(ChartQueryContextDatasourceMismatchValidationError())
def validate(self) -> None: # noqa: C901
exceptions: list[ValidationError] = []
dashboard_ids = self._properties.get("dashboards")
@@ -140,6 +191,9 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
security_manager.raise_for_access(chart=self._model)
except SupersetSecurityException as ex:
raise ChartForbiddenError() from ex
# Keep the refreshed payload bound to the chart's own datasource so it
# cannot be repointed at an unrelated one.
self._validate_query_context_datasource(exceptions)
# validate tags
try:

View File

@@ -37,6 +37,7 @@ from superset.commands.report.exceptions import (
AlertQueryMultipleRowsError,
AlertQueryTimeout,
AlertValidatorConfigError,
ReportScheduleExecutorNotFoundError,
)
from superset.reports.models import ReportSchedule, ReportScheduleValidatorType
from superset.tasks.utils import get_executor
@@ -193,6 +194,7 @@ class AlertCommand(BaseCommand):
database=self._report_schedule.database
)
rendered_sql = sql_template.process_template(self._report_schedule.sql)
try:
limited_rendered_sql = self._report_schedule.database.apply_limit_to_sql(
rendered_sql, ALERT_SQL_LIMIT
@@ -210,6 +212,13 @@ class AlertCommand(BaseCommand):
model=self._report_schedule,
)
user = security_manager.find_user(username)
# A deleted/disabled executor user makes find_user return None. Raise
# the dedicated error so the handler below re-surfaces it instead of
# masking it as an opaque AlertQueryError (or letting the missing user
# surface as a NoneType error from the downstream auth flow).
if user is None:
raise ReportScheduleExecutorNotFoundError(username)
with override_user(user):
start = default_timer()
df = self._report_schedule.database.get_df(sql=limited_rendered_sql)
@@ -223,6 +232,10 @@ class AlertCommand(BaseCommand):
except SoftTimeLimitExceeded as ex:
logger.warning("A timeout occurred while executing the alert query: %s", ex)
raise AlertQueryTimeout() from ex
except ReportScheduleExecutorNotFoundError:
# A missing executor user is a configuration problem, not a transient
# query error; surface the typed error rather than masking it.
raise
except Exception as ex:
logger.warning("An error occurred when running alert query")
# The exception message here can reveal to much information to malicious

View File

@@ -17,7 +17,9 @@
from __future__ import annotations
import logging
import re
from datetime import datetime
from re import Pattern
from typing import Any, Callable, cast, TYPE_CHECKING, TypedDict, Union
from apispec import APISpec
@@ -56,6 +58,10 @@ if TYPE_CHECKING:
logger = logging.getLogger(__name__)
INSUFFICIENT_PERMISSIONS_REGEX: Pattern[str] = re.compile(
r"\[INSUFFICIENT_PERMISSIONS\]|\bSQLSTATE:\s*42501\b"
)
try:
from databricks.sql.utils import ParamEscaper
@@ -263,6 +269,25 @@ class DatabricksBaseEngineSpec(BaseEngineSpec):
def epoch_to_dttm(cls) -> str:
return HiveEngineSpec.epoch_to_dttm()
@classmethod
def extract_errors(
cls,
ex: Exception,
context: dict[str, Any] | None = None,
database_name: str | None = None,
) -> list[SupersetError]:
raw_message = cls._extract_error_message(ex)
if INSUFFICIENT_PERMISSIONS_REGEX.search(raw_message):
return [
SupersetError(
error_type=SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR,
message=raw_message,
level=ErrorLevel.WARNING,
extra={"engine_name": cls.engine_name},
)
]
return super().extract_errors(ex, context, database_name)
class DatabricksODBCEngineSpec(DatabricksBaseEngineSpec):
"""Databricks engine spec using ODBC driver for SQL Endpoints."""
@@ -490,46 +515,15 @@ class DatabricksDynamicBaseEngineSpec(BasicParametersMixin, DatabricksBaseEngine
context: dict[str, Any] | None = None,
database_name: str | None = None,
) -> list[SupersetError]:
raw_message = cls._extract_error_message(ex)
context = context or {}
# access_token isn't currently parseable from the
# databricks error response, but adding it in here
# for reference if their error message changes
for key, value in cls.context_key_mapping.items():
context[key] = context.get(value)
db_engine_custom_errors = cls.get_database_custom_errors(database_name)
if not isinstance(db_engine_custom_errors, dict):
db_engine_custom_errors = {}
for regex, (message, error_type, extra) in [
*db_engine_custom_errors.items(),
*cls.custom_errors.items(),
]:
match = regex.search(raw_message)
if match:
params = {**context, **match.groupdict()}
extra["engine_name"] = cls.engine_name
return [
SupersetError(
error_type=error_type,
message=message % params,
level=ErrorLevel.ERROR,
extra=extra,
)
]
return [
SupersetError(
error_type=SupersetErrorType.GENERIC_DB_ENGINE_ERROR,
message=cls._extract_error_message(ex),
level=ErrorLevel.ERROR,
extra={"engine_name": cls.engine_name},
)
]
return super().extract_errors(ex, context, database_name)
@classmethod
def validate_parameters( # type: ignore

View File

@@ -306,6 +306,8 @@ class GetChartInfoRequest(BaseModel):
current chart configuration from cache.
"""
model_config = ConfigDict(populate_by_name=True)
identifier: Annotated[
int | str | None,
Field(
@@ -314,6 +316,7 @@ class GetChartInfoRequest(BaseModel):
"Chart identifier - can be numeric ID or UUID string. "
"Optional when form_data_key is provided (for unsaved charts)."
),
validation_alias=AliasChoices("identifier", "id", "chart_id"),
),
]
form_data_key: str | None = Field(
@@ -344,6 +347,7 @@ class GetChartInfoRequest(BaseModel):
"set that excludes 'form_data' (the full chart config, can be 50KB+). "
"Add 'form_data' explicitly when you need the raw chart configuration."
),
validation_alias=AliasChoices("select_columns", "columns"),
),
]
@@ -1937,6 +1941,8 @@ class ChartRequestNormalizerMixin(BaseModel):
class ListChartsRequest(EditedByMeMixin, CreatedByMeMixin, MetadataCacheControl):
"""Request schema for list_charts with clear, unambiguous types."""
model_config = ConfigDict(populate_by_name=True)
filters: Annotated[
List[ChartFilter],
Field(
@@ -1952,6 +1958,7 @@ class ListChartsRequest(EditedByMeMixin, CreatedByMeMixin, MetadataCacheControl)
default_factory=list,
description="List of columns to select. Defaults to common columns if not "
"specified.",
validation_alias=AliasChoices("select_columns", "columns"),
),
]
@@ -2129,6 +2136,8 @@ class GenerateChartRequest(ChartRequestNormalizerMixin, QueryCacheControl):
class GenerateExploreLinkRequest(ChartRequestNormalizerMixin, FormDataCacheControl):
model_config = ConfigDict(populate_by_name=True)
dataset_id: int | str = Field(..., description="Dataset identifier (ID, UUID)")
config: ChartConfig | None = Field(
None,
@@ -2143,7 +2152,11 @@ class GenerateExploreLinkRequest(ChartRequestNormalizerMixin, FormDataCacheContr
class UpdateChartRequest(ChartRequestNormalizerMixin, QueryCacheControl):
model_config = ConfigDict(populate_by_name=True)
identifier: int | str = Field(..., description="Chart ID or UUID")
identifier: int | str = Field(
...,
description="Chart ID or UUID",
validation_alias=AliasChoices("identifier", "id", "chart_id"),
)
config: ChartConfig | None = Field(
None,
description="Chart configuration. Optional; omit to only update chart_name.",
@@ -2188,6 +2201,8 @@ class UpdateChartRequest(ChartRequestNormalizerMixin, QueryCacheControl):
class UpdateChartPreviewRequest(ChartRequestNormalizerMixin, FormDataCacheControl):
model_config = ConfigDict(populate_by_name=True)
form_data_key: str | None = Field(
None,
description=(
@@ -2215,12 +2230,15 @@ class GetChartDataRequest(QueryCacheControl):
the current chart configuration from cache.
"""
model_config = ConfigDict(populate_by_name=True)
identifier: int | str | None = Field(
default=None,
description=(
"Chart identifier (ID, UUID). "
"Optional when form_data_key is provided (for unsaved charts)."
),
validation_alias=AliasChoices("identifier", "id", "chart_id"),
)
form_data_key: str | None = Field(
default=None,
@@ -2345,12 +2363,15 @@ class GetChartPreviewRequest(QueryCacheControl):
using the current chart configuration from cache.
"""
model_config = ConfigDict(populate_by_name=True)
identifier: int | str | None = Field(
default=None,
description=(
"Chart identifier (ID, UUID). "
"Optional when form_data_key is provided (for unsaved charts)."
),
validation_alias=AliasChoices("identifier", "id", "chart_id"),
)
form_data_key: str | None = Field(
default=None,
@@ -2564,12 +2585,15 @@ class GetChartSqlRequest(BaseModel):
to get the SQL for the unsaved chart state from the Explore view.
"""
model_config = ConfigDict(populate_by_name=True)
identifier: int | str | None = Field(
default=None,
description=(
"Chart identifier - can be numeric ID or UUID string. "
"Optional when form_data_key is provided (for unsaved charts)."
),
validation_alias=AliasChoices("identifier", "id", "chart_id"),
)
form_data_key: str | None = Field(
default=None,

View File

@@ -195,6 +195,8 @@ class DashboardFilter(ColumnOperator):
class ListDashboardsRequest(EditedByMeMixin, CreatedByMeMixin, MetadataCacheControl):
"""Request schema for list_dashboards with clear, unambiguous types."""
model_config = ConfigDict(populate_by_name=True)
filters: Annotated[
List[DashboardFilter],
Field(
@@ -210,6 +212,7 @@ class ListDashboardsRequest(EditedByMeMixin, CreatedByMeMixin, MetadataCacheCont
default_factory=list,
description="List of columns to select. Defaults to common columns "
"if not specified.",
validation_alias=AliasChoices("select_columns", "columns"),
),
]
@@ -321,10 +324,15 @@ class GetDashboardInfoRequest(MetadataCacheControl):
in a dashboard but the URL contains a permalink_key.
"""
model_config = ConfigDict(populate_by_name=True)
identifier: Annotated[
int | str,
Field(
description="Dashboard identifier - can be numeric ID, UUID string, or slug"
description=(
"Dashboard identifier - can be numeric ID, UUID string, or slug"
),
validation_alias=AliasChoices("identifier", "id", "dashboard_id"),
),
]
permalink_key: str | None = Field(
@@ -347,6 +355,7 @@ class GetDashboardInfoRequest(MetadataCacheControl):
"to override, e.g. ['id','dashboard_title','charts'] for minimal "
"output, or add 'css' to include raw dashboard CSS."
),
validation_alias=AliasChoices("select_columns", "columns"),
),
]
@@ -585,10 +594,18 @@ class DashboardList(BaseModel):
class AddChartToDashboardRequest(BaseModel):
"""Request schema for adding a chart to an existing dashboard."""
model_config = ConfigDict(populate_by_name=True)
dashboard_id: int = Field(
..., description="ID of the dashboard to add the chart to"
...,
description="ID of the dashboard to add the chart to",
validation_alias=AliasChoices("dashboard_id", "dashboard", "id"),
)
chart_id: int = Field(
...,
description="ID of the chart to add to the dashboard",
validation_alias=AliasChoices("chart_id", "chart"),
)
chart_id: int = Field(..., description="ID of the chart to add to the dashboard")
target_tab: str | None = Field(
None,
min_length=1,

View File

@@ -25,6 +25,7 @@ from datetime import datetime, timezone
from typing import Annotated, Any, Dict, List, Literal
from pydantic import (
AliasChoices,
BaseModel,
ConfigDict,
Field,
@@ -262,6 +263,8 @@ class DatasetList(BaseModel):
class ListDatasetsRequest(EditedByMeMixin, CreatedByMeMixin, MetadataCacheControl):
"""Request schema for list_datasets with clear, unambiguous types."""
model_config = ConfigDict(populate_by_name=True)
filters: Annotated[
List[DatasetFilter],
Field(
@@ -277,6 +280,7 @@ class ListDatasetsRequest(EditedByMeMixin, CreatedByMeMixin, MetadataCacheContro
default_factory=list,
description="List of columns to select. Defaults to common columns if not "
"specified.",
validation_alias=AliasChoices("select_columns", "columns"),
),
]
search: Annotated[
@@ -370,9 +374,14 @@ DEFAULT_GET_DATASET_INFO_COLUMN_FIELDS: List[str] = [
class GetDatasetInfoRequest(MetadataCacheControl):
"""Request schema for get_dataset_info with support for ID or UUID."""
model_config = ConfigDict(populate_by_name=True)
identifier: Annotated[
int | str,
Field(description="Dataset identifier - can be numeric ID or UUID string"),
Field(
description="Dataset identifier - can be numeric ID or UUID string",
validation_alias=AliasChoices("identifier", "id", "dataset_id"),
),
]
select_columns: Annotated[
List[str],
@@ -391,6 +400,7 @@ class GetDatasetInfoRequest(MetadataCacheControl):
"Pass an explicit list to select only what you need "
"(e.g. ['id', 'table_name', 'columns', 'metrics'])."
),
validation_alias=AliasChoices("select_columns", "columns"),
),
]
column_fields: Annotated[

View File

@@ -0,0 +1,110 @@
# 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.
"""strip metricSqlExpressions from ag_grid_table params
Before PR #41555 the AG Grid table plugin leaked ``metricSqlExpressions``
(a mapping of every datasource metric/column name to its SQL expression) into
``extra_form_data`` on every filter interaction, which was then serialised into
the ``params`` and ``query_context`` columns on save. For datasources with
many metrics this bloated each chart record by hundreds of MB.
This migration strips the field from existing rows so those records are no
longer loaded eagerly on every dashboard request.
Revision ID: d24e6b0a9c7f
Revises: 8f3a1b2c4d5e
Create Date: 2026-06-30 00:00:00.000000
"""
from alembic import op
from sqlalchemy import Column, Integer, String, Text
from sqlalchemy.orm import declarative_base
from superset import db
from superset.utils import json
# revision identifiers, used by Alembic.
revision = "d24e6b0a9c7f"
down_revision = "8f3a1b2c4d5e"
Base = declarative_base()
_FIELD = "metricSqlExpressions"
_VIZ_TYPE = "ag_grid_table"
class Slice(Base):
__tablename__ = "slices"
id = Column(Integer, primary_key=True)
viz_type = Column(String(250))
params = Column(Text)
query_context = Column(Text)
def _strip_params(slc: Slice) -> bool:
"""Remove _FIELD from extra_form_data in params. Returns True if changed."""
if not slc.params:
return False
try:
params = json.loads(slc.params)
except Exception:
return False
extra = params.get("extra_form_data", {})
if _FIELD not in extra:
return False
del extra[_FIELD]
params["extra_form_data"] = extra
slc.params = json.dumps(params)
return True
def _strip_query_context(slc: Slice) -> bool:
"""Remove _FIELD from query_context.form_data.extra_form_data."""
if not slc.query_context:
return False
try:
qc = json.loads(slc.query_context)
except Exception:
return False
extra = qc.get("form_data", {}).get("extra_form_data", {})
if _FIELD not in extra:
return False
del extra[_FIELD]
slc.query_context = json.dumps(qc)
return True
def upgrade() -> None:
bind = op.get_bind()
session = db.Session(bind=bind)
for slc in session.query(Slice).filter(Slice.viz_type == _VIZ_TYPE):
_strip_params(slc)
_strip_query_context(slc)
session.flush()
def downgrade() -> None:
# The stripped data was runtime-derived and is regenerated automatically
# by the chart plugin on the next render — there is nothing to restore.
pass

View File

@@ -107,9 +107,13 @@ class SynchronousSqlJsonExecutor(SqlJsonExecutorBase):
if data.get("status") == QueryStatus.FAILED: # type: ignore
# new error payload with rich context
if data["errors"]: # type: ignore
raise SupersetErrorsException(
[SupersetError(**params) for params in data["errors"]] # type: ignore
errors = [SupersetError(**params) for params in data["errors"]] # type: ignore
status = (
500
if any(error.level == ErrorLevel.ERROR for error in errors)
else 400
)
raise SupersetErrorsException(errors, status=status)
# old string-only error message
raise SupersetGenericDBErrorException(data["error"]) # type: ignore

File diff suppressed because it is too large Load Diff

View File

@@ -299,6 +299,17 @@ def _inject_action_meta_record(
logger.exception("version_changes: malformed ACTION_META_KEY payload")
def _write_action_kind(
session: Session, tx_table: sa.Table, tx_id: int, action_kind: str
) -> None:
"""Write action metadata through the transaction's existing connection."""
session.connection().execute(
sa.update(tx_table)
.where(tx_table.c.id == tx_id)
.values(action_kind=action_kind)
)
def _stamp_action_kind_on_transaction(session: Session, tx_id: int) -> None:
"""Pop the per-tx action_kind from ``session.info`` and stamp it
onto the ``version_transaction`` row identified by *tx_id*.
@@ -312,10 +323,11 @@ def _stamp_action_kind_on_transaction(session: Session, tx_id: int) -> None:
The action_kind is popped (not just read) so a long-lived session
can't accidentally carry the value into the next transaction. A
failed stamp is logged and swallowed — action_kind is a
descriptive enrichment, not a correctness invariant; refusing to
write change records because an UPDATE on a single column failed
would punish the user save for an audit-log nicety.
failed stamp is rolled back to a SAVEPOINT, logged, and swallowed:
action_kind is descriptive enrichment, not a correctness invariant.
The SAVEPOINT is opened after the final flush, so a failed metadata
statement cannot poison the user transaction or disturb Continuum's
canonical shadows.
"""
# pylint: disable=import-outside-toplevel
from sqlalchemy_continuum import versioning_manager
@@ -325,11 +337,8 @@ def _stamp_action_kind_on_transaction(session: Session, tx_id: int) -> None:
return
tx_tbl = versioning_manager.transaction_cls.__table__
try:
session.connection().execute(
sa.update(tx_tbl)
.where(tx_tbl.c.id == tx_id)
.values(action_kind=action_kind)
)
with session.connection().begin_nested():
_write_action_kind(session, tx_tbl, tx_id, action_kind)
except Exception: # pylint: disable=broad-except
logger.exception(
"version_changes: failed to stamp action_kind=%s on tx %s",

View File

@@ -16,6 +16,8 @@
# under the License.
"""Transaction-level correctness tests for entity version history."""
from unittest.mock import patch
import pytest
import sqlalchemy as sa
@@ -23,6 +25,8 @@ from superset import db
from superset.connectors.sqla.models import SqlaTable
from superset.models.slice import Slice
from superset.utils import json
from superset.versioning.changes import listener
from superset.versioning.changes.listener import ACTION_KIND_KEY
from superset.versioning.changes.table import version_changes_table
from tests.integration_tests.base_tests import SupersetTestCase
from tests.integration_tests.fixtures.birth_names_dashboard import ( # noqa: F401
@@ -222,3 +226,83 @@ class TestVersionHistoryTransactionCorrectness(SupersetTestCase):
def test_nested_rollback_preserves_outer_initial_state(self) -> None:
"""Rolling back a SAVEPOINT keeps the outer transaction registry."""
self._assert_nested_transaction_preserves_outer_initial_state("rollback")
def test_action_kind_sql_failure_is_isolated_and_consumed(self) -> None:
"""Failed descriptive metadata cannot poison or leak from the save."""
chart = self._chart("Girls")
chart_id = chart.id
boundary = db.session.scalar(
sa.text("SELECT coalesce(max(id), 0) FROM version_transaction")
)
def fail_action_kind(*_args: object, **_kwargs: object) -> None:
db.session.connection().execute(
sa.text("UPDATE __missing_action_kind_target__ SET value = 1")
)
try:
db.session.info[ACTION_KIND_KEY] = "restore"
chart.slice_name = "Girls survives action metadata failure"
with patch.object(listener, "_write_action_kind", fail_action_kind):
db.session.commit()
db.session.expire_all()
saved = db.session.get(Slice, chart_id)
assert saved is not None
assert saved.slice_name == "Girls survives action metadata failure"
assert ACTION_KIND_KEY not in db.session.info
first_rows = db.session.execute(
sa.text(
"SELECT sv.transaction_id, vt.action_kind "
"FROM slices_version sv "
"JOIN version_transaction vt ON vt.id = sv.transaction_id "
"WHERE sv.id = :id AND sv.operation_type = 1 "
"AND sv.slice_name = :name "
"AND sv.transaction_id > :boundary"
),
{
"id": chart_id,
"name": "Girls survives action metadata failure",
"boundary": boundary,
},
).all()
assert len(first_rows) == 1
assert first_rows[0].action_kind is None
failed_metadata_tx_id = first_rows[0].transaction_id
semantic_changes = _changes_for_chart(
chart_id, after_transaction_id=boundary
)
name_changes = [
row
for row in semantic_changes
if row["path"] == ["slice_name"]
and row["to_value"] == "Girls survives action metadata failure"
]
assert len(name_changes) == 1
assert name_changes[0]["transaction_id"] == failed_metadata_tx_id
saved.slice_name = "Girls next transaction"
db.session.commit()
next_rows = db.session.execute(
sa.text(
"SELECT vt.action_kind FROM slices_version sv "
"JOIN version_transaction vt ON vt.id = sv.transaction_id "
"WHERE sv.id = :id AND sv.operation_type = 1 "
"AND sv.slice_name = :name "
"AND sv.transaction_id > :boundary"
),
{
"id": chart_id,
"name": "Girls next transaction",
"boundary": failed_metadata_tx_id,
},
).all()
assert len(next_rows) == 1
assert next_rows[0].action_kind is None
finally:
chart = db.session.get(Slice, chart_id)
assert chart is not None
chart.slice_name = "Girls"
db.session.commit()

View File

@@ -17,10 +17,11 @@
import pytest
from pytest_mock import MockerFixture
from superset.commands.chart.exceptions import ChartForbiddenError
from superset.commands.chart.exceptions import ChartForbiddenError, ChartInvalidError
from superset.commands.chart.update import UpdateChartCommand
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetSecurityException
from superset.utils import json
def _editorship_exc() -> SupersetSecurityException:
@@ -149,3 +150,91 @@ def test_update_chart_editor_can_perform_regular_update(
exceptions = compute_subjects.call_args.args[2]
assert properties["editors"] == [2]
assert exceptions == []
def _query_context_payload(datasource: object) -> dict[str, object]:
"""Build a query-context-only update payload targeting ``datasource``."""
return {
"query_context": json.dumps({"datasource": datasource, "queries": []}),
"query_context_generation": True,
}
@pytest.mark.parametrize(
"datasource_type",
[
"table",
"query", # non-table datasource types must also be accepted when matching
],
)
def test_update_chart_query_context_matching_datasource_is_allowed(
mocker: MockerFixture,
datasource_type: str,
) -> None:
"""A query context that targets the chart's own datasource is accepted."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
find_by_id.return_value = mocker.MagicMock(
id=1,
tags=[],
dashboards=[],
datasource_id=42,
datasource_type=datasource_type,
)
mocker.patch("superset.commands.chart.update.security_manager.raise_for_editorship")
mocker.patch("superset.commands.chart.update.security_manager.raise_for_access")
UpdateChartCommand(
1, _query_context_payload({"id": 42, "type": datasource_type})
).validate()
@pytest.mark.parametrize(
"datasource",
[
{"id": 99, "type": "table"}, # different id
{"id": 42, "type": "query"}, # different type
{"id": "99", "type": "table"}, # different id as string
{"id": 42}, # matching id but missing type
{"id": "5f7b3c1a-...-uuid", "type": "table"}, # non-numeric id
],
)
def test_update_chart_query_context_mismatched_datasource_is_rejected(
mocker: MockerFixture,
datasource: dict[str, object],
) -> None:
"""A query context pointing at a different datasource is rejected with a 4xx."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
find_by_id.return_value = mocker.MagicMock(
id=1, tags=[], dashboards=[], datasource_id=42, datasource_type="table"
)
mocker.patch("superset.commands.chart.update.security_manager.raise_for_editorship")
mocker.patch("superset.commands.chart.update.security_manager.raise_for_access")
with pytest.raises(ChartInvalidError):
UpdateChartCommand(1, _query_context_payload(datasource)).validate()
@pytest.mark.parametrize(
"query_context",
[
"{}", # no datasource key
'{"datasource": null}', # null datasource
"not-json", # unparseable payload
],
)
def test_update_chart_query_context_without_datasource_is_allowed(
mocker: MockerFixture,
query_context: str,
) -> None:
"""Payloads with no verifiable datasource fall back to the chart's own."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
find_by_id.return_value = mocker.MagicMock(
id=1, tags=[], dashboards=[], datasource_id=42, datasource_type="table"
)
mocker.patch("superset.commands.chart.update.security_manager.raise_for_editorship")
mocker.patch("superset.commands.chart.update.security_manager.raise_for_access")
UpdateChartCommand(
1,
{"query_context": query_context, "query_context_generation": True},
).validate()

View File

@@ -23,7 +23,10 @@ import pytest
from pytest_mock import MockerFixture
from superset.commands.report.alert import AlertCommand
from superset.commands.report.exceptions import AlertValidatorConfigError
from superset.commands.report.exceptions import (
AlertValidatorConfigError,
ReportScheduleExecutorNotFoundError,
)
from superset.reports.models import ReportScheduleValidatorType, ReportState
@@ -494,3 +497,38 @@ def test_not_null_validator_with_empty_result_does_not_trigger(
assert triggered is False, "NOT_NULL with empty result should not trigger"
assert command._result is None
def test_execute_query_raises_when_executor_user_missing(
mocker: MockerFixture,
) -> None:
"""
When the configured executor user cannot be resolved
(``security_manager.find_user`` returns ``None``), the alert-query path
raises a dedicated ``ReportScheduleExecutorNotFoundError`` naming the
username, rather than swallowing it into an opaque ``AlertQueryError`` (or
surfacing a NoneType/AttributeError from the downstream auth flow).
"""
mocker.patch(
"superset.commands.report.alert.jinja_context.get_template_processor",
)
mocker.patch(
"superset.commands.report.alert.get_executor",
return_value=("executor", "ghost_user"),
)
mocker.patch(
"superset.commands.report.alert.security_manager.find_user",
return_value=None,
)
report_schedule_mock = mocker.Mock()
report_schedule_mock.id = 1
report_schedule_mock.sql = "SELECT value FROM metrics"
command = AlertCommand(
report_schedule=report_schedule_mock,
execution_id=uuid4(),
)
with pytest.raises(ReportScheduleExecutorNotFoundError, match="ghost_user"):
command._execute_query()

View File

@@ -235,6 +235,80 @@ def test_extract_errors_with_context() -> None:
]
@pytest.mark.parametrize(
"msg",
[
# tag only
"[INSUFFICIENT_PERMISSIONS] Insufficient privileges: User does not "
"have USE CATALOG on Catalog 'platform_production'.",
# SQLSTATE only
"Insufficient privileges: User does not have USE CATALOG on Catalog "
"'platform_production'. SQLSTATE: 42501.",
# SQLSTATE with irregular spacing
"Insufficient privileges on Catalog 'platform_production'. SQLSTATE:42501",
# both, as seen in real Databricks responses
"[INSUFFICIENT_PERMISSIONS] Insufficient privileges: User does not "
"have USE CATALOG on Catalog 'platform_production'. SQLSTATE: 42501.",
],
)
def test_extract_errors_insufficient_permissions(msg: str) -> None:
"""
Test that Databricks catalog/schema permission errors are classified as
a client-side, warning-level error instead of a generic server error,
regardless of which half of the pattern is present in the raw message.
"""
result = DatabricksNativeEngineSpec.extract_errors(Exception(msg))
assert result == [
SupersetError(
message=msg,
error_type=SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR,
level=ErrorLevel.WARNING,
extra={
"engine_name": "Databricks (legacy)",
"issue_codes": [
{
"code": 1017,
"message": "Issue 1017 - User doesn't have the proper permissions.", # noqa: E501
}
],
},
)
]
def test_extract_errors_insufficient_permissions_covers_odbc() -> None:
"""
The insufficient-permissions classification lives on the shared
``DatabricksBaseEngineSpec`` so that the ODBC ("SQL Endpoint") connector
benefits too, not just the native/Python-connector engines.
"""
from superset.db_engine_specs.databricks import DatabricksODBCEngineSpec
msg = (
"[INSUFFICIENT_PERMISSIONS] Insufficient privileges: User does not "
"have USE CATALOG on Catalog 'platform_production'. SQLSTATE: 42501."
)
result = DatabricksODBCEngineSpec.extract_errors(Exception(msg))
assert result == [
SupersetError(
message=msg,
error_type=SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR,
level=ErrorLevel.WARNING,
extra={
"engine_name": "Databricks SQL Endpoint",
"issue_codes": [
{
"code": 1017,
"message": "Issue 1017 - User doesn't have the proper permissions.", # noqa: E501
}
],
},
)
]
@pytest.mark.parametrize(
"target_type,expected_result",
[

View File

@@ -28,10 +28,16 @@ from superset.mcp_service.chart.schemas import (
FilterConfig,
GenerateChartRequest,
GenerateChartResponse,
GetChartDataRequest,
GetChartInfoRequest,
GetChartPreviewRequest,
GetChartSqlRequest,
ListChartsRequest,
MixedTimeseriesChartConfig,
PieChartConfig,
PivotTableChartConfig,
TableChartConfig,
UpdateChartRequest,
XYChartConfig,
)
@@ -1137,3 +1143,64 @@ class TestSqlMetricLlmContextWrapping:
assert "<UNTRUSTED-CONTENT>" in metric["label"]
assert metric["expressionType"] == "SQL"
assert "<UNTRUSTED-CONTENT>" not in metric["optionName"]
class TestRequestSchemaAliasChoices:
"""Test that LLM-friendly field name variants are accepted on the
chart MCP tool request schemas, so callers sending 'id'/'chart_id'
instead of 'identifier' (or 'columns' instead of 'select_columns')
don't silently have the field dropped."""
def test_get_chart_info_identifier_id_alias(self) -> None:
req = GetChartInfoRequest.model_validate({"id": 42})
assert req.identifier == 42
def test_get_chart_info_identifier_chart_id_alias(self) -> None:
req = GetChartInfoRequest.model_validate({"chart_id": 42})
assert req.identifier == 42
def test_get_chart_info_identifier_still_works(self) -> None:
req = GetChartInfoRequest.model_validate({"identifier": 42})
assert req.identifier == 42
def test_get_chart_info_select_columns_columns_alias(self) -> None:
req = GetChartInfoRequest.model_validate(
{"id": 42, "columns": ["id", "slice_name"]}
)
assert req.select_columns == ["id", "slice_name"]
def test_get_chart_data_identifier_id_alias(self) -> None:
req = GetChartDataRequest.model_validate({"id": 7})
assert req.identifier == 7
def test_get_chart_data_identifier_chart_id_alias(self) -> None:
req = GetChartDataRequest.model_validate({"chart_id": 7})
assert req.identifier == 7
def test_get_chart_preview_identifier_id_alias(self) -> None:
req = GetChartPreviewRequest.model_validate({"id": 7})
assert req.identifier == 7
def test_get_chart_preview_identifier_chart_id_alias(self) -> None:
req = GetChartPreviewRequest.model_validate({"chart_id": 7})
assert req.identifier == 7
def test_get_chart_sql_identifier_id_alias(self) -> None:
req = GetChartSqlRequest.model_validate({"id": 7})
assert req.identifier == 7
def test_get_chart_sql_identifier_chart_id_alias(self) -> None:
req = GetChartSqlRequest.model_validate({"chart_id": 7})
assert req.identifier == 7
def test_update_chart_identifier_id_alias(self) -> None:
req = UpdateChartRequest.model_validate({"id": 7})
assert req.identifier == 7
def test_update_chart_identifier_chart_id_alias(self) -> None:
req = UpdateChartRequest.model_validate({"chart_id": 7})
assert req.identifier == 7
def test_list_charts_select_columns_columns_alias(self) -> None:
req = ListChartsRequest.model_validate({"columns": ["id", "slice_name"]})
assert req.select_columns == ["id", "slice_name"]

View File

@@ -31,11 +31,14 @@ from superset.mcp_service.dashboard.schemas import (
_extract_cross_filters_enabled,
_extract_native_filters,
_safe_user_label,
AddChartToDashboardRequest,
dashboard_serializer,
DashboardInfo,
DuplicateDashboardRequest,
DuplicateDashboardResponse,
GenerateDashboardRequest,
GetDashboardInfoRequest,
ListDashboardsRequest,
serialize_chart_summary,
serialize_dashboard_object,
UpdateDashboardRequest,
@@ -931,3 +934,46 @@ class TestDuplicateDashboardResponse:
"""A null error stays null rather than being wrapped."""
resp = DuplicateDashboardResponse(dashboard_url="http://host/d/1/")
assert resp.error is None
class TestRequestSchemaAliasChoices:
"""Test that LLM-friendly field name variants are accepted on the
dashboard MCP tool request schemas, so callers sending 'id'/'dashboard_id'
instead of 'identifier' (or 'columns' instead of 'select_columns')
don't silently have the field dropped."""
def test_get_dashboard_info_identifier_id_alias(self) -> None:
req = GetDashboardInfoRequest.model_validate({"id": 42})
assert req.identifier == 42
def test_get_dashboard_info_identifier_dashboard_id_alias(self) -> None:
req = GetDashboardInfoRequest.model_validate({"dashboard_id": 42})
assert req.identifier == 42
def test_get_dashboard_info_identifier_still_works(self) -> None:
req = GetDashboardInfoRequest.model_validate({"identifier": 42})
assert req.identifier == 42
def test_get_dashboard_info_select_columns_columns_alias(self) -> None:
req = GetDashboardInfoRequest.model_validate(
{"id": 42, "columns": ["id", "dashboard_title"]}
)
assert req.select_columns == ["id", "dashboard_title"]
def test_list_dashboards_select_columns_columns_alias(self) -> None:
req = ListDashboardsRequest.model_validate(
{"columns": ["id", "dashboard_title"]}
)
assert req.select_columns == ["id", "dashboard_title"]
def test_add_chart_to_dashboard_dashboard_alias(self) -> None:
req = AddChartToDashboardRequest.model_validate({"dashboard": 1, "chart_id": 2})
assert req.dashboard_id == 1
def test_add_chart_to_dashboard_id_alias(self) -> None:
req = AddChartToDashboardRequest.model_validate({"id": 1, "chart_id": 2})
assert req.dashboard_id == 1
def test_add_chart_to_dashboard_chart_alias(self) -> None:
req = AddChartToDashboardRequest.model_validate({"dashboard_id": 1, "chart": 2})
assert req.chart_id == 2

View File

@@ -0,0 +1,54 @@
# 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.
"""
Unit tests for MCP dataset schema validation.
"""
from superset.mcp_service.dataset.schemas import (
GetDatasetInfoRequest,
ListDatasetsRequest,
)
class TestRequestSchemaAliasChoices:
"""Test that LLM-friendly field name variants are accepted on the
dataset MCP tool request schemas, so callers sending 'id'/'dataset_id'
instead of 'identifier' (or 'columns' instead of 'select_columns')
don't silently have the field dropped."""
def test_get_dataset_info_identifier_id_alias(self) -> None:
req = GetDatasetInfoRequest.model_validate({"id": 42})
assert req.identifier == 42
def test_get_dataset_info_identifier_dataset_id_alias(self) -> None:
req = GetDatasetInfoRequest.model_validate({"dataset_id": 42})
assert req.identifier == 42
def test_get_dataset_info_identifier_still_works(self) -> None:
req = GetDatasetInfoRequest.model_validate({"identifier": 42})
assert req.identifier == 42
def test_get_dataset_info_select_columns_columns_alias(self) -> None:
req = GetDatasetInfoRequest.model_validate(
{"id": 42, "columns": ["id", "table_name"]}
)
assert req.select_columns == ["id", "table_name"]
def test_list_datasets_select_columns_columns_alias(self) -> None:
req = ListDatasetsRequest.model_validate({"columns": ["id", "table_name"]})
assert req.select_columns == ["id", "table_name"]

View File

@@ -0,0 +1,203 @@
# 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.
"""Tests for migration ``d24e6b0a9c7f_strip_metricsqlexpressions_from_ag_grid_params``.
Covers the two helper functions (_strip_params, _strip_query_context) and the
full upgrade() path that seeds an ag_grid_table slice with metricSqlExpressions
in both params and query_context, then asserts both fields are stripped.
"""
from __future__ import annotations
from importlib import import_module
from unittest.mock import patch
import pytest
from sqlalchemy import create_engine
from sqlalchemy.orm import Session
from superset.utils import json
migration = import_module(
"superset.migrations.versions."
"2026-06-30_00-00_d24e6b0a9c7f_strip_metricsqlexpressions_from_ag_grid_params"
)
Slice = migration.Slice
_strip_params = migration._strip_params
_strip_query_context = migration._strip_query_context
_FIELD = migration._FIELD
_VIZ_TYPE = migration._VIZ_TYPE
_SQL_EXPR = {"col1": "SELECT col1 FROM t", "col2": "SUM(revenue)"}
def _contaminated_params() -> str:
return json.dumps(
{
"viz_type": _VIZ_TYPE,
"extra_form_data": {
_FIELD: _SQL_EXPR,
"filters": [{"col": "col1", "op": "==", "val": "a"}],
},
}
)
def _contaminated_query_context() -> str:
return json.dumps(
{
"form_data": {
"extra_form_data": {
_FIELD: _SQL_EXPR,
},
},
"queries": [],
}
)
@pytest.fixture
def engine():
engine = create_engine("sqlite:///:memory:")
migration.Base.metadata.create_all(engine)
return engine
# ---------------------------------------------------------------------------
# Helper-function unit tests
# ---------------------------------------------------------------------------
def test_strip_params_removes_field() -> None:
slc = Slice(params=_contaminated_params())
changed = _strip_params(slc)
assert changed
result = json.loads(slc.params)
assert _FIELD not in result["extra_form_data"]
assert result["extra_form_data"]["filters"] == [
{"col": "col1", "op": "==", "val": "a"}
]
def test_strip_params_noop_when_field_absent() -> None:
params = json.dumps({"extra_form_data": {"filters": []}})
slc = Slice(params=params)
changed = _strip_params(slc)
assert not changed
assert json.loads(slc.params) == {"extra_form_data": {"filters": []}}
def test_strip_params_noop_when_params_empty() -> None:
assert not _strip_params(Slice(params=None))
def test_strip_params_noop_on_invalid_json() -> None:
slc = Slice(params="not-json")
changed = _strip_params(slc)
assert not changed
assert slc.params == "not-json"
def test_strip_query_context_removes_field() -> None:
slc = Slice(query_context=_contaminated_query_context())
changed = _strip_query_context(slc)
assert changed
result = json.loads(slc.query_context)
assert _FIELD not in result["form_data"]["extra_form_data"]
def test_strip_query_context_noop_when_field_absent() -> None:
qc = json.dumps({"form_data": {"extra_form_data": {}}})
slc = Slice(query_context=qc)
assert not _strip_query_context(slc)
def test_strip_query_context_noop_when_query_context_empty() -> None:
assert not _strip_query_context(Slice(query_context=None))
def test_strip_query_context_noop_on_invalid_json() -> None:
slc = Slice(query_context="not-json")
changed = _strip_query_context(slc)
assert not changed
assert slc.query_context == "not-json"
# ---------------------------------------------------------------------------
# Full upgrade() integration test
# ---------------------------------------------------------------------------
def test_upgrade_strips_both_fields(engine) -> None:
"""upgrade() must strip _FIELD from params and query_context for ag_grid_table
slices, and must not touch slices of other viz types."""
with Session(engine) as seed:
seed.add_all(
[
Slice(
id=1,
viz_type=_VIZ_TYPE,
params=_contaminated_params(),
query_context=_contaminated_query_context(),
),
# Different viz type — must be left unchanged.
Slice(
id=2,
viz_type="table",
params=_contaminated_params(),
query_context=_contaminated_query_context(),
),
]
)
seed.commit()
# Simulate the Alembic transaction: bind the migration session to an explicit
# connection opened via engine.begin() so the transaction auto-commits on
# context exit. The session.flush() inside upgrade() must emit the SQL
# UPDATEs before the connection is committed; without it the outer commit
# has nothing to write and the data is left unchanged.
with engine.begin() as conn:
upgrade_session = Session(bind=conn)
with (
patch.object(migration, "op") as mock_op,
patch.object(migration, "db") as mock_db,
):
mock_op.get_bind.return_value = conn
mock_db.Session.return_value = upgrade_session
migration.upgrade()
with Session(engine) as verify:
slc1 = verify.get(Slice, 1)
params1 = json.loads(slc1.params)
assert _FIELD not in params1["extra_form_data"], (
"upgrade() must strip _FIELD from params.extra_form_data"
)
assert params1["extra_form_data"]["filters"] == [
{"col": "col1", "op": "==", "val": "a"}
]
qc1 = json.loads(slc1.query_context)
assert _FIELD not in qc1["form_data"]["extra_form_data"], (
"upgrade() must strip _FIELD from query_context.form_data.extra_form_data"
)
slc2 = verify.get(Slice, 2)
params2 = json.loads(slc2.params)
assert _FIELD in params2["extra_form_data"], (
"upgrade() must not touch non-ag_grid_table slices"
)

View File

@@ -0,0 +1,150 @@
# 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.
from typing import Any
from unittest.mock import MagicMock
import pytest
from superset.exceptions import SupersetErrorsException
from superset.sqllab.sql_json_executer import SynchronousSqlJsonExecutor
from superset.utils.core import QueryStatus
def _make_executor(data: dict[str, Any]) -> SynchronousSqlJsonExecutor:
query_dao = MagicMock()
get_sql_results_task = MagicMock(return_value=data)
return SynchronousSqlJsonExecutor(
query_dao,
get_sql_results_task,
timeout_duration_in_seconds=60,
sqllab_backend_persistence_feature_enable=False,
)
def _make_execution_context() -> MagicMock:
execution_context = MagicMock()
execution_context.query.id = 1
execution_context.expand_data = False
execution_context.select_as_cta = False
return execution_context
def test_execute_raises_400_when_all_errors_are_warnings() -> None:
"""
All-WARNING rich errors (e.g. a Databricks insufficient-permissions
error) should surface as a 4xx client error, not a 500.
"""
data = {
"status": QueryStatus.FAILED,
"errors": [
{
"message": "Insufficient privileges on Catalog 'foo'.",
"error_type": "CONNECTION_DATABASE_PERMISSIONS_ERROR",
"level": "warning",
"extra": {},
}
],
}
executor = _make_executor(data)
with pytest.raises(SupersetErrorsException) as excinfo:
executor.execute(_make_execution_context(), "SELECT 1", None)
assert excinfo.value.status == 400
def test_execute_raises_400_when_errors_are_warning_and_info() -> None:
"""
A mix of WARNING and INFO errors (no ERROR-level entries) should still
surface as a 4xx, since nothing in the list indicates a server fault.
"""
data = {
"status": QueryStatus.FAILED,
"errors": [
{
"message": "Insufficient privileges on Catalog 'foo'.",
"error_type": "CONNECTION_DATABASE_PERMISSIONS_ERROR",
"level": "warning",
"extra": {},
},
{
"message": "For your information.",
"error_type": "GENERIC_DB_ENGINE_ERROR",
"level": "info",
"extra": {},
},
],
}
executor = _make_executor(data)
with pytest.raises(SupersetErrorsException) as excinfo:
executor.execute(_make_execution_context(), "SELECT 1", None)
assert excinfo.value.status == 400
def test_execute_raises_500_when_any_error_is_error_level() -> None:
"""
Existing behavior is preserved: a single ERROR-level rich error still
surfaces as a 500.
"""
data = {
"status": QueryStatus.FAILED,
"errors": [
{
"message": "Something went wrong.",
"error_type": "GENERIC_DB_ENGINE_ERROR",
"level": "error",
"extra": {},
},
],
}
executor = _make_executor(data)
with pytest.raises(SupersetErrorsException) as excinfo:
executor.execute(_make_execution_context(), "SELECT 1", None)
assert excinfo.value.status == 500
def test_execute_raises_500_when_errors_are_mixed_warning_and_error() -> None:
"""
A mix of WARNING and ERROR still surfaces as a 500: any ERROR wins.
"""
data = {
"status": QueryStatus.FAILED,
"errors": [
{
"message": "Insufficient privileges on Catalog 'foo'.",
"error_type": "CONNECTION_DATABASE_PERMISSIONS_ERROR",
"level": "warning",
"extra": {},
},
{
"message": "Something went wrong.",
"error_type": "GENERIC_DB_ENGINE_ERROR",
"level": "error",
"extra": {},
},
],
}
executor = _make_executor(data)
with pytest.raises(SupersetErrorsException) as excinfo:
executor.execute(_make_execution_context(), "SELECT 1", None)
assert excinfo.value.status == 500