aminghadersohi commented on code in PR #42655:
URL: https://github.com/apache/superset/pull/42655#discussion_r3717944980
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -2347,6 +2347,14 @@ class UpdateChartRequest(ChartRequestNormalizerMixin,
QueryCacheControl):
None,
description="Chart configuration. Optional; omit to only update
chart_name.",
)
+ add_columns: List[ColumnRef] | None = Field(
+ None,
+ description=(
+ "Table columns or metrics to append while preserving every
existing "
+ "column and metric. Use this instead of config.columns when adding
"
+ "columns to an existing table chart."
+ ),
+ )
Review Comment:
Partly. The `add_columns` field itself is covered — `test_update_chart.py`
has `test_add_columns_preserves_existing_columns_and_metrics`,
`test_add_columns_rebinds_requested_dataset`, and a parametrized
`test_add_metric_to_raw_table_returns_actionable_error`. They live next to the
other `_build_update_payload` tests rather than in `test_chart_schemas.py`,
which is why the search missed them.
The validator was a real gap though, so I've added
`TestUpdateChartRequestColumnPatchValidation` covering both error branches and
the valid additive shape (see the reply on the `validate_column_patch` thread).
Writing the coverage you asked for also turned up an actual bug, so thanks —
details in the other thread and in the PR comment.
--
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]
