EnxDev opened a new pull request, #45236: URL: https://github.com/apache/superset/pull/45236
### SUMMARY Follow-up to #45221, which made `update_chart` and `update_chart_preview` keep a saved big number trendline when the config leaves out `show_trendline`. One case was left converting silently, and this closes it. A big number with trendline can have a SQL expression as its time column (Explore saves it as an adhoc `x_axis`). `BigNumberChartConfig.temporal_column` is a column name, so there's nothing the update can inherit. The hook gave up, `show_trendline` fell back to `False`, and the chart was saved as a plain big number. The viz type changed, so the update also went through the cross-viz merge and dropped `compare_lag` and the other trendline settings. The caller only asked to change a metric. That update is now refused with a validation error that names the two ways forward: > The saved trendline uses a SQL expression as its time column, which a Big Number config cannot carry. Set 'temporal_column' to a dataset column to keep a trendline, or set 'show_trendline' to false to remove it. Keeping the expression itself would need the typed config to carry an adhoc column, which is a schema change and not something to slip into a follow-up. Refusing is the smaller step: the trendline is no longer lost without the caller saying so. What doesn't change: - Naming `temporal_column` or `show_trendline` works as before, on these charts too. - A dataset rebind still maps the chart as a plain big number. The saved time column belongs to the previous dataset either way. - A saved trendline chart with no time column at all still becomes a plain big number. It has no trendline to keep. The other gap I'd noted on #45221, clients filling in advertised defaults for `kind`, `viz_type` and `show_trendline`, needs no change. Since #44656 the `update_chart` and `update_chart_preview` input schemas only carry `chart_type` for `config`, so there are no defaults for a client to fill in. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Not applicable, backend only. ### TESTING INSTRUCTIONS ```bash pytest tests/unit_tests/mcp_service/chart/tool/test_update_chart.py tests/unit_tests/mcp_service/chart/tool/test_update_chart_preview.py -k "sql_trendline or without_a_saved_time_column" ``` Eight tests. Four fail on master: the hook-level one, `update_chart` through the MCP client for both the save and the default preview path, and `update_chart_preview` on cached form data. The other four pass on master and pin what must stay the same: the two explicit choices, the rebind, and the chart with no saved time column. Manually, through an MCP client: 1. In Explore, save a Big Number with Trendline whose x-axis is a custom SQL expression, for example `DATE(ds)`. 2. Call `update_chart` with `generate_preview: false` and a config that has only `chart_type: "big_number"` and a `metric`. 3. The call returns a `ValidationError` and the chart is untouched. On master it succeeds and the chart is saved as `big_number_total`. 4. Repeat with `temporal_column` set to a dataset column, or with `show_trendline: false`. Both succeed. ### ADDITIONAL INFORMATION - [x] Has associated issue: #45210 - [ ] 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
