codeant-ai-for-open-source[bot] commented on code in PR #36076:
URL: https://github.com/apache/superset/pull/36076#discussion_r3487531543
##########
superset/utils/schema.py:
##########
@@ -83,3 +84,35 @@ def validate_external_url(value: Optional[str]) -> None:
)
if not parsed.netloc:
raise ValidationError("URL must be absolute and include a host.")
+
+
+def validate_query_context_metadata(value: Union[bytes, bytearray, str, None])
-> None:
+ """
+ Validator for query_context field to ensure it contains required metadata.
+
+ Validates that the query_context JSON contains the required 'datasource'
and
+ 'queries' fields needed for chart data retrieval.
+
+ :raises ValidationError: if value is not valid JSON or missing required
fields
+ :param value: a JSON string that should contain datasource and queries
metadata
+ """
+ if value is None or value == "":
+ return # Allow None values and empty strings
Review Comment:
**Suggestion:** Treating an empty string as valid `query_context` bypasses
the new metadata checks and still allows persisting blank payloads; those
charts later fail data retrieval paths that expect saved query context JSON.
Only `None` should bypass validation, while empty strings should be rejected as
invalid input. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
❌ /api/v1/chart/<id>/data fails for blank query_context.
⚠️ MCP get_chart_data treats chart as missing query_context.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create a chart via the Charts REST API endpoint `POST /api/v1/chart/`,
implemented by
`ChartRestApi` in `superset/charts/api.py` where `add_model_schema =
ChartPostSchema()` is
defined at lines 36-37, using a request body that includes `"query_context":
""` (empty
string).
2. During request validation, `ChartPostSchema` in
`superset/charts/schemas.py` (lines
52-56) maps the `query_context` field to
`utils.validate_query_context_metadata`, which
executes `validate_query_context_metadata` in `superset/utils/schema.py`
(definition at
lines 89-98).
3. Inside `validate_query_context_metadata`, the check at
`superset/utils/schema.py:99-100` (`if value is None or value == "":
return`) treats the
empty string as valid, so no `ValidationError` is raised and the chart is
persisted with
`Slice.query_context` (column defined at `superset/models/slice.py:84`) set
to `""`
instead of `NULL`.
4. Later, when fetching chart data via `GET /api/v1/chart/<pk>/data` in
`superset/charts/data/api.py`, the handler at
`superset/charts/data/api.py:160-167`
executes `json_body = json.loads(chart.query_context)` (line 162); for a
blank string this
raises `json.JSONDecodeError`, is caught, sets `json_body = None`, and then
returns HTTP
400 with message `"Chart has no query context saved. Please save the chart
again."`,
meaning a chart that passed validation cannot be used until its
query_context is fixed.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=eec011208cee46ed9d9c22b0ad38ef2e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=eec011208cee46ed9d9c22b0ad38ef2e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/schema.py
**Line:** 99:100
**Comment:**
*Incomplete Implementation: Treating an empty string as valid
`query_context` bypasses the new metadata checks and still allows persisting
blank payloads; those charts later fail data retrieval paths that expect saved
query context JSON. Only `None` should bypass validation, while empty strings
should be rejected as invalid input.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=8d25455bce127ba597821ebe0a180dc61456f53567aad413193557fefe474fdd&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=8d25455bce127ba597821ebe0a180dc61456f53567aad413193557fefe474fdd&reaction=dislike'>👎</a>
--
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]
