endimonan opened a new pull request, #43280: URL: https://github.com/apache/superset/pull/43280
### SUMMARY Exporting a saved chart to CSV/XLSX produces a generic filename like `superset_table_20260818_101010.csv`, while the streaming CSV path (rows >= `CSV_STREAMING_ROW_THRESHOLD`) names the file after the chart, because that filename is built client side. This inconsistency is what https://github.com/apache/superset/discussions/21859 has been asking about since 2022. #42193 tried to close this by reading `form_data["slice_name"]` in `_get_default_export_filename`, but no real client ever sends `slice_name` inside the form data: `Slice.form_data` only injects `slice_id`, `viz_type` and `datasource`, and the frontend's `QueryFormData` has no `slice_name` field either. So that branch is unreachable in practice and every non-streaming export still falls back to the `viz_type`. This PR makes the chart name actually reach the filename: `_get_default_export_filename` now also accepts the resolved `Slice` and picks the first usable candidate: an explicit `slice_name` in the form data (keeps the #42193 contract), the chart's own name, the `viz_type`, and finally a generic "export". A candidate is only used if it is a plain string and survives `secure_filename` (a name written entirely in a non-latin alphabet sanitizes to an empty string, in which case we fall through to the `viz_type` rather than produce a nameless file). The name segment is also capped at 150 characters: chart names can be up to 250 characters, and without the cap the composed filename would exceed the 255-character single-component limit of NTFS/ext4/APFS, breaking consumers that honor `Content-Disposition` verbatim (`curl -OJ`, `wget --content-disposition`). Where the `Slice` comes from depends on the route. `POST /api/v1/chart/data` relies on the query context factory, which already resolves the slice from `form_data.slice_id` through `ChartDAO.find_by_id` (so the requesting user's `ChartFilter` still applies). `GET /api/v1/chart/<pk>/data/` passes the chart it just loaded, because a chart's saved query context rarely carries a `slice_id` (0 of the 104 example charts have one). The streaming CSV fallback filename (used when the client doesn't send one) gets the same treatment, and `GET /api/v1/chart/data/<cache_key>` benefits from the factory-resolved slice as well. Behavior that intentionally does not change: a client-supplied `filename` still wins everywhere, requests without any `slice_id` still fall back to the `viz_type`, and JSON responses are untouched. Drill to Detail downloads still get the generic name because that pane only submits `{dashboardId}` as form data; that's a separate (frontend) change if wanted. The client-built streaming filename format (`<Chart_name>_<date>_<time>.csv`) also stays as is; after this change both paths contain the chart name, which is what the discussion asks for. This picks up what #25648 and #27409 (@nandwalritik) set out to do, as invited in https://github.com/apache/superset/discussions/21859. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Before: `superset_table_20260818_061705.csv` After: `superset_Filename_Probe_Chart_20260818_061705.csv` ### TESTING INSTRUCTIONS Save a chart with a recognizable name, open it in Explore and use Download > Export to .CSV (with fewer rows than `CSV_STREAMING_ROW_THRESHOLD`, i.e. the default path). The downloaded file is named `superset_<Chart_name>_<timestamp>.csv` instead of `superset_<viz_type>_<timestamp>.csv`. Same for XLSX. `GET /api/v1/chart/<pk>/data/?format=csv` shows the same name in `Content-Disposition`. `POST /api/v1/chart/data` with a query context whose `form_data` has a `slice_id` uses the chart name; without a `slice_id` it keeps the `viz_type` fallback. Unit tests: `pytest tests/unit_tests/charts/test_chart_data_api.py` ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: https://github.com/apache/superset/discussions/21859 - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
