aminghadersohi commented on PR #40473:
URL: https://github.com/apache/superset/pull/40473#issuecomment-4631882289
Addressed all remaining open review comments:
**1. `filter_state` in `DEFAULT_GET_DASHBOARD_INFO_COLUMNS`** (Copilot
#3358257445 and duplicates): Removed `filter_state` from the lean default. The
docstring already said it was excluded; now the code matches. The
`get_dashboard_info` logic that auto-adds `filter_state` when `permalink_key`
is present was previously dead code — it is now live and functional.
**2. Empty-list disables filtering** (Copilot #3358257483, #3358257509,
#3358257533 and duplicates): All three `_parse_select_columns` validators
(dashboard, chart, dataset) and the dataset `_parse_column_fields` validator
now fall back to the lean default when `parse_json_or_list` returns `[]`.
Previously, `""` or `[]` input bypassed all filtering because
`_filter_fields_by_context` only filters when `select_columns` is truthy.
**3. Unsaved-chart path in `get_chart_info`** (Copilot #3358257584): The
`form_data_key`-only branch (no `identifier`) now calls
`model_dump(mode="json", context={"select_columns": request.select_columns})`
before returning, consistent with the saved-chart path. Large `form_data`
payloads are now excluded by the lean default on this path too.
**4. Tests updated**: `test_column_fields_empty_list_stays_empty` →
`test_column_fields_empty_list_falls_back_to_default`; privacy test assertion
updated (`form_data` is now absent from the response rather than `None` since
it's not in the default `select_columns`); added
`test_unsaved_chart_select_columns_filters_response` covering the unsaved-chart
select_columns path.
**Bito "dead code" comment** (#3358883587): The `and "filter_state" not in
effective_select_columns` condition was dead code when `filter_state` was in
the default, but is now live code after fix #1 — keeping it as is.
All 118 affected tests pass.
--
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]
