feat: Use dashboard name for screenshot download (#34988)

This commit is contained in:
Vitor Avila
2025-09-05 02:16:45 -03:00
committed by GitHub
parent 9424538bb1
commit ce74ae095d
5 changed files with 118 additions and 5 deletions

View File

@@ -57,6 +57,7 @@
"antd": "^5.24.6",
"chrono-node": "^2.7.8",
"classnames": "^2.2.5",
"content-disposition": "^0.5.4",
"d3-color": "^3.1.0",
"d3-scale": "^2.1.2",
"dayjs": "^1.11.13",
@@ -174,6 +175,7 @@
"@testing-library/react": "^12.1.5",
"@testing-library/react-hooks": "^8.0.1",
"@testing-library/user-event": "^12.8.3",
"@types/content-disposition": "^0.5.9",
"@types/dom-to-image": "^2.6.7",
"@types/jest": "^29.5.14",
"@types/js-levenshtein": "^1.1.3",
@@ -15250,6 +15252,13 @@
"@types/node": "*"
}
},
"node_modules/@types/content-disposition": {
"version": "0.5.9",
"resolved": "https://registry.npmjs.org/@types/content-disposition/-/content-disposition-0.5.9.tgz",
"integrity": "sha512-8uYXI3Gw35MhiVYhG3s295oihrxRyytcRHjSjqnqZVDDy/xcGBRny7+Xj1Wgfhv5QzRtN2hB2dVRBUX9XW3UcQ==",
"dev": true,
"license": "MIT"
},
"node_modules/@types/conventional-commits-parser": {
"version": "5.0.1",
"resolved": "https://registry.npmjs.org/@types/conventional-commits-parser/-/conventional-commits-parser-5.0.1.tgz",
@@ -21928,7 +21937,6 @@
"version": "0.5.4",
"resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-0.5.4.tgz",
"integrity": "sha512-FveZTNuGw04cxlAiWbzi6zTAL/lhehaWbTtgluJh4/E95DqMwTmha3KZN1aAWA8cFIhHzMZUvLevkw5Rqk+tSQ==",
"dev": true,
"license": "MIT",
"dependencies": {
"safe-buffer": "5.2.1"

View File

@@ -125,6 +125,7 @@
"antd": "^5.24.6",
"chrono-node": "^2.7.8",
"classnames": "^2.2.5",
"content-disposition": "^0.5.4",
"d3-color": "^3.1.0",
"d3-scale": "^2.1.2",
"dayjs": "^1.11.13",
@@ -242,6 +243,7 @@
"@testing-library/react": "^12.1.5",
"@testing-library/react-hooks": "^8.0.1",
"@testing-library/user-event": "^12.8.3",
"@types/content-disposition": "^0.5.9",
"@types/dom-to-image": "^2.6.7",
"@types/jest": "^29.5.14",
"@types/js-levenshtein": "^1.1.3",

View File

@@ -20,6 +20,7 @@ import { useCallback, useEffect, useRef } from 'react';
import { useSelector } from 'react-redux';
import { useToasts } from 'src/components/MessageToasts/withToasts';
import { last } from 'lodash';
import contentDisposition from 'content-disposition';
import {
logging,
t,
@@ -98,12 +99,31 @@ export const useDownloadScreenshot = (
headers: { Accept: 'application/pdf, image/png' },
parseMethod: 'raw',
})
.then((response: Response) => response.blob())
.then(blob => {
.then((response: Response) => {
const disposition = response.headers.get('Content-Disposition');
let fileName = `screenshot.${format}`; // default filename
if (disposition) {
try {
const parsed = contentDisposition.parse(disposition);
if (parsed?.parameters?.filename) {
fileName = parsed.parameters.filename;
}
} catch (error) {
console.warn(
'Failed to parse Content-Disposition header:',
error,
);
}
}
return response.blob().then(blob => ({ blob, fileName }));
})
.then(({ blob, fileName }) => {
const url = window.URL.createObjectURL(blob);
const a = document.createElement('a');
a.href = url;
a.download = `screenshot.${format}`;
a.download = fileName;
document.body.appendChild(a);
a.click();
document.body.removeChild(a);

View File

@@ -113,6 +113,7 @@ from superset.tasks.thumbnails import (
from superset.tasks.utils import get_current_user
from superset.utils import json
from superset.utils.core import parse_boolean_string
from superset.utils.file import get_filename
from superset.utils.pdf import build_pdf_from_screenshots
from superset.utils.screenshots import (
DashboardScreenshot,
@@ -1204,6 +1205,10 @@ class DashboardRestApi(BaseSupersetModelRestApi):
image = cache_payload.get_image()
except ScreenshotImageNotAvailableException:
return self.response_404()
filename = get_filename(
dashboard.dashboard_title or "screenshot", dashboard.id, skip_id=True
)
if download_format == "pdf":
pdf_img = image.getvalue()
# Convert the screenshot to PDF
@@ -1212,13 +1217,18 @@ class DashboardRestApi(BaseSupersetModelRestApi):
return Response(
pdf_data,
mimetype="application/pdf",
headers={"Content-Disposition": "inline; filename=dashboard.pdf"},
headers={
"Content-Disposition": f'attachment; filename="{filename}.pdf"'
},
direct_passthrough=True,
)
if download_format == "png":
return Response(
FileWrapper(image),
mimetype="image/png",
headers={
"Content-Disposition": f'attachment; filename="{filename}.png"'
},
direct_passthrough=True,
)
return self.response_404()

View File

@@ -3180,6 +3180,79 @@ class TestDashboardApi(ApiOwnersTestCaseMixin, InsertChartMixin, SupersetTestCas
response = self._get_screenshot(dashboard.id, cache_key, "invalid")
assert response.status_code == 404
@with_feature_flags(THUMBNAILS=True, ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True)
@pytest.mark.usefixtures("create_dashboard_with_tag")
@patch("superset.dashboards.api.cache_dashboard_screenshot")
@patch("superset.dashboards.api.build_pdf_from_screenshots")
@patch("superset.dashboards.api.DashboardScreenshot.get_from_cache_key")
def test_screenshot_filename_in_header(
self, mock_get_from_cache_key, mock_build_pdf, mock_cache_task
):
"""
Dashboard API: Test that screenshot download includes proper filename in header
"""
self.login(ADMIN_USERNAME)
mock_cache_task.return_value = None
mock_get_from_cache_key.return_value = ScreenshotCachePayload(
b"fake image data"
)
mock_build_pdf.return_value = b"fake pdf data"
dashboard = (
db.session.query(Dashboard)
.filter(Dashboard.dashboard_title == "dash with tag")
.first()
)
cache_resp = self._cache_screenshot(dashboard.id)
assert cache_resp.status_code == 200
cache_key = json.loads(cache_resp.data.decode("utf-8"))["cache_key"]
for format, mt in {"png": "image/png", "pdf": "application/pdf"}.items():
response = self._get_screenshot(dashboard.id, cache_key, format)
assert response.status_code == 200
assert response.mimetype == mt
assert (
response.headers["Content-Disposition"]
== f'attachment; filename="dash_with_tag.{format}"'
)
@with_feature_flags(THUMBNAILS=True, ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True)
@pytest.mark.usefixtures("create_dashboard_with_tag")
@patch("superset.dashboards.api.cache_dashboard_screenshot")
@patch("superset.dashboards.api.build_pdf_from_screenshots")
@patch("superset.dashboards.api.DashboardScreenshot.get_from_cache_key")
def test_screenshot_filename_in_header_dashboard_with_no_title(
self, mock_get_from_cache_key, mock_build_pdf, mock_cache_task
):
"""
Dashboard API: Test that filename in header for screenshot download defaults
to screenshot if dashboard does not have a title.
"""
self.login(ADMIN_USERNAME)
mock_cache_task.return_value = None
mock_get_from_cache_key.return_value = ScreenshotCachePayload(
b"fake image data"
)
mock_build_pdf.return_value = b"fake pdf data"
dashboard = Dashboard()
db.session.add(dashboard)
db.session.commit()
cache_resp = self._cache_screenshot(dashboard.id)
assert cache_resp.status_code == 200
cache_key = json.loads(cache_resp.data.decode("utf-8"))["cache_key"]
for format, mt in {"png": "image/png", "pdf": "application/pdf"}.items():
response = self._get_screenshot(dashboard.id, cache_key, format)
assert response.status_code == 200
assert response.mimetype == mt
assert (
response.headers["Content-Disposition"]
== f'attachment; filename="screenshot.{format}"'
)
@with_feature_flags(THUMBNAILS=False, ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True)
@pytest.mark.usefixtures("create_dashboard_with_tag")
def test_cache_dashboard_screenshot_feature_thumbnails_ff_disabled(self):