diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 786a0590780..a728352117c 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -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" diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 01b848564ad..46b59362baa 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -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", diff --git a/superset-frontend/src/dashboard/hooks/useDownloadScreenshot.ts b/superset-frontend/src/dashboard/hooks/useDownloadScreenshot.ts index bc8ef7d1ffa..f3b21691b39 100644 --- a/superset-frontend/src/dashboard/hooks/useDownloadScreenshot.ts +++ b/superset-frontend/src/dashboard/hooks/useDownloadScreenshot.ts @@ -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); diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index a739e44c271..83387ee3372 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -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() diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 1e896d88541..a6551024eb9 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -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):