tien238lnd opened a new issue, #45210: URL: https://github.com/apache/superset/issues/45210
### Bug description Updating a saved chart through the MCP `update_chart` tool changes more than the caller named. A `config` that names a few fields resets the other saved settings to the mapper's defaults, and a change of viz type inside one chart type silently replaces the chart. `merge_chart_form_data` only protects `filters`, `color_scheme` and `row_limit` when they are left out. Of the 15 chart plugins, only gauge, treemap and gantt override `merge_update_form_data`; the other 12 fall through to `_merge_shared_form_data`, so every other control a mapper writes unconditionally is overwritten. Measured through the save path of `update_chart` (`resolve_update_config` followed by `_build_update_payload`) on `549508b34` (#44656), with a saved chart and an update that names one or two fields: | Saved chart, update | Result on master | |---|---| | `ag-grid-table`, config without `viz_type` | becomes `table`; loses `adhoc_filters`, `color_scheme`, `page_length`, `table_timestamp_format` | | xy bar, config without `kind` | becomes a line chart; loses `adhoc_filters`, `color_scheme`, `show_legend`, `legendOrientation`, `show_value`, `row_limit` | | xy bar to line (`kind: "line"`) | user filters, `color_scheme`, `show_legend`, `legendOrientation`, `row_limit` lost | | pie, only `row_limit` named | `donut`, `show_labels`, `label_type`, `sort_by_metric`, `show_legend`, `show_total`, `labels_outside` reset | | big number with trendline, `show_trendline` omitted | becomes `big_number_total`; loses `granularity_sqla`, `show_trend_line`, `start_y_axis_at_zero`, `compare_lag` | Cases that already behave: xy with the same `kind` and `filters` omitted keeps the user filters, and a change of chart type (pie to table) leaves no pie keys behind. Steps to reproduce: 1. Save a chart of one of the types above with a few non-default settings (for example a pie with `donut: true` and `show_legend: false`). 2. Call `update_chart` with `generate_preview: false` and a `config` that has `chart_type` and only the field to change (for example `row_limit: 200`). 3. Read the saved `params`: the settings that were not named are back to the mapper defaults. Expected: an update changes only the fields it names; a viz change inside one chart type (xy `kind`, table `viz_type`) keeps the other saved settings. Only a different `chart_type` replaces the chart. ### Screenshots/recordings Not applicable. ### Superset version master / latest-dev ### Python version 3.11 ### Node version Not applicable ### Browser Not applicable ### Additional context A fix that fits the existing hook: each plugin declares which `config` field writes which `form_data` key, and the default `merge_update_form_data` keeps every saved key that no named field writes. A guard test can require every field of a plugin's config model to appear in that table or in an exemption list with a reason. `ag-grid-table`, xy `kind` and the big number trendline also need their `viz_type` taken from the saved chart when the caller leaves it out, in `resolve_update_config`. A patch along these lines exists but has not been proposed as a PR; happy to open one if maintainers agree with the direction. ### Checklist - [x] I have searched Superset docs and Slack and didn't find a solution to my problem. - [x] I have searched the GitHub issue tracker and didn't find a similar bug report. - [x] I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section. -- 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]
