aminghadersohi commented on PR #45575: URL: https://github.com/apache/superset/pull/45575#issuecomment-6322095238
Local end-to-end check at 409886ec39d83e61eea05ce9b29c89e279c2000a, compared with the merge base cdc9430597c84ce1c89b400cf3215247b372d657. I ran the MCP service (`superset mcp run`, `call_tool` over streamable HTTP) on example data as Admin, Alpha and Gamma users, and read every saved title back from the metadata DB. Result: looks good to ship. - Over-limit titles (501/600-char dashboard titles, 251–600-char chart names, including the `name`/`slice_name`/`title` aliases) report "String exceeds the allowed maximum length" in 66/66 cases. The merge base reported "Contains disallowed content" for dashboards and `generate_chart`. The error shape is unchanged. - A 250-character chart name saves at 250 characters through all four aliases. 251 and 255 are rejected with no row written and the stored name unchanged. These are the only outcome changes from the merge base: 30 of 303 paired calls. - Blank or whitespace-only titles/names on `duplicate_dashboard` (including its aliases), `create_virtual_dataset`, `update_dataset`, `create_dataset_metric`, `update_dataset_metric`, `create_theme` and whitespace-only `save_sql_query` labels report "Text must not be empty" at the right field. - `<script>`, `javascript:` and `data:text/html,` titles are still rejected (45/45, same class as the merge base). Valid titles with curly quotes, apostrophes, `&`, emoji and surrounding spaces save as before. Small things I noticed, none blocking: 1. A 250-character chart name sent with leading/trailing spaces is now rejected, although the trimmed name would fit (the merge base saved it at 250). `Field(max_length=250)` checks the raw value, while dashboard titles are trimmed before the length check. Writing the trimmed value back in the chart-name before-validator would make the two consistent. 2. `create_theme` has no length cap, but `Theme.theme_name` is `String(250)`. A 310-character name saved on SQLite, so on PostgreSQL/MySQL it would fail at the database instead of returning a length reason. This was already the case before this PR. 3. `save_sql_query` with `label: ""` reports "String is shorter than the allowed minimum", while whitespace-only reports "Text must not be empty". This matches the docs, which only cover whitespace-only labels. Not run: a real LLM in the loop, PostgreSQL/MySQL metadata DB, deployed auth modes. -- 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]
