fix(charts): use chart name in CSV/XLSX/zip export filenames#42193
fix(charts): use chart name in CSV/XLSX/zip export filenames#42193rusackas wants to merge 2 commits into
Conversation
…rt headers The /data endpoint already extracts a sanitized filename (falling back to a chart-name-based default) for the streaming CSV export path, but the non-streaming CSV, XLSX, and multi-query zip branches of _send_chart_response ignored it and always called generate_download_headers() with no filename, producing generic timestamp-only download names. Fixes #37487
Code Review Agent Run #b012f0Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #42193 +/- ##
==========================================
- Coverage 65.08% 65.08% -0.01%
==========================================
Files 2758 2758
Lines 155229 155235 +6
Branches 35572 35572
==========================================
+ Hits 101026 101029 +3
- Misses 52288 52291 +3
Partials 1915 1915
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
The flagged issue is correct. When a user provides a filename with an extension (e.g., To resolve this, you should strip the extension from the provided Proposed FixIn import os
# ... inside _send_chart_response ...
export_filename = filename or self._get_default_export_filename(form_data)
base_filename = os.path.splitext(export_filename)[0]
if is_csv_format:
return CsvResponse(
data, headers=generate_download_headers("csv", base_filename)
)Since there are no other comments in the provided PR context, there are no additional issues to address. superset/charts/data/api.py |
…lename The non-streaming CSV/XLSX/zip branch of _send_chart_response passed a client-supplied filename straight into generate_download_headers(), which always appends the format extension, producing doubled extensions like "export.csv.csv" whenever the caller's filename already ends in .csv/.xlsx/.zip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #809fb6Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
SUMMARY
Chart data exports (CSV/XLSX/zip) from
/api/v1/chart/datawere alwaysnamed with a bare timestamp (e.g.
20260127_160233.csv), even though theendpoint already extracts a sanitized
filenamefrom the request. Onlythe large-dataset streaming CSV path honored that filename (and its
chart-name-based fallback); the regular non-streaming CSV, XLSX, and
multi-query zip branches in
_send_chart_responsecalledgenerate_download_headers()with no filename at all, dropping it on thefloor.
This PR:
filenamethrough togenerate_download_headers()in the CSV, XLSX, and zip branches.
timestamp default already used by the streaming CSV path (extracted into
a shared
_get_default_export_filenamehelper to avoid duplicating thatlogic).
scoped to the chart data export endpoints only.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (backend header change only)
TESTING INSTRUCTIONS
just a timestamp.
Also see the added unit tests in
tests/unit_tests/charts/test_chart_data_api.py, which pin the bug byasserting the
Content-Dispositionheader contains the chart name forthe CSV, XLSX, and zip export branches.
ADDITIONAL INFORMATION