codeant-ai-for-open-source[bot] commented on code in PR #32958:
URL: https://github.com/apache/superset/pull/32958#discussion_r3464389164
##########
tests/integration_tests/dashboards/api_tests.py:
##########
@@ -84,7 +84,7 @@ class TestDashboardApi(ApiOwnersTestCaseMixin,
InsertChartMixin, SupersetTestCas
"description": "desc_changed",
"position_json": '{"b": "B"}',
"css": "css_changed",
- "json_metadata": '{"refresh_frequency": 30,
"timed_refresh_immune_slices": [], "expanded_slices": {}, "color_scheme": "",
"label_colors": {}, "shared_label_colors": [], "map_label_colors": {},
"color_scheme_domain": [], "cross_filters_enabled": false}', # noqa: E501
+ "json_metadata": '{"refresh_frequency": 30,
"timed_refresh_immune_slices": [], "expanded_slices": {}, "expand_all_slices":
False, "color_scheme": "", "label_colors": {}, "shared_label_colors": [],
"map_label_colors": {}, "color_scheme_domain": [], "cross_filters_enabled":
false}', # noqa: E501
Review Comment:
**Suggestion:** The `json_metadata` payload is not valid JSON because it
contains Python `False` inside a JSON string. The dashboard PUT tests that
reuse this fixture will fail validation (`JSON not valid`) before update logic
runs. Use JSON booleans (`false`/`true`) consistently in this string. [type
error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Dashboard update integration test `test_update_dashboard` fails.
- ❌ Partial update test `test_update_partial_dashboard` also fails.
- ⚠️ Invalid json_metadata blocked before update logic executes.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `tests/integration_tests/dashboards/api_tests.py:80-90` and observe
`TestDashboardApi.dashboard_data`, where the `"json_metadata"` field is a
string
containing `"expand_all_slices": False` (capital-F `False`) alongside other
JSON fields
like `"cross_filters_enabled": false`.
2. In `test_update_dashboard` (`api_tests.py:7-26` around lines 7-26), the
test creates a
dashboard, logs in as admin, and calls `rv = self.put_assert_metric(uri,
self.dashboard_data, "put")`, sending `self.dashboard_data` as the JSON body
to `PUT
/api/v1/dashboard/{dashboard_id}`.
3. The Dashboard API schema (`superset/dashboards/schemas.py:430-437`)
defines
`json_metadata` as a `fields.String` with `validate=validate_json_metadata`,
and
`validate_json_metadata` (`schemas.py:108-18`) calls `json.loads(value)` on
the
`json_metadata` string, raising `json.JSONDecodeError` for invalid JSON
before converting
that into a `ValidationError("JSON not valid")`.
4. When the request body includes the string with `"expand_all_slices":
False`,
`json.loads` fails because `False` is not a valid JSON literal (JSON
requires `false`),
causing the API to return a 400/422 validation error instead of the 200
status expected by
both `test_update_dashboard` and `test_update_partial_dashboard`
(`api_tests.py:2595-2627`), so these tests fail before reaching any update
logic.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9bcbde94502943fb9598be096673db57&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=9bcbde94502943fb9598be096673db57&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:** tests/integration_tests/dashboards/api_tests.py
**Line:** 87:87
**Comment:**
*Type Error: The `json_metadata` payload is not valid JSON because it
contains Python `False` inside a JSON string. The dashboard PUT tests that
reuse this fixture will fail validation (`JSON not valid`) before update logic
runs. Use JSON booleans (`false`/`true`) consistently in this string.
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%2F32958&comment_hash=afa64ce263031dbf3099bf0483456d052ece6b784c5c3aa898609c9567a0e018&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32958&comment_hash=afa64ce263031dbf3099bf0483456d052ece6b784c5c3aa898609c9567a0e018&reaction=dislike'>👎</a>
##########
tests/integration_tests/fixtures/importexport.py:
##########
@@ -69,7 +69,7 @@
"css": "",
"dashboard_title": "Births 2",
"description": None,
- "json_metadata": '{"timed_refresh_immune_slices": [],
"expanded_slices": {}, "refresh_frequency": 0, "default_filters": "{}",
"color_scheme": null, "remote_id": 1}', # noqa: E501
+ "json_metadata": '{"timed_refresh_immune_slices": [],
"expanded_slices": {}, "expand_all_slices": "False", "refresh_frequency": 0,
"default_filters": "{}", "color_scheme": null, "remote_id": 1}', # noqa: E501
Review Comment:
**Suggestion:** `expand_all_slices` is being serialized as the string
`"False"` instead of a JSON boolean. This value can pass through import
unchanged and later be treated as truthy (`"False" || false`) in frontend
hydration, causing all chart descriptions to expand when the setting is
intended to be disabled. Store it as a real boolean (`false`) in the fixture
payload. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dashboard metadata flag stored with wrong JSON type.
- ⚠️ Dashboard hydration may expand descriptions despite setting false.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In `tests/integration_tests/fixtures/importexport.py:26-34` (line 72), the
`dashboard_export` fixture defines `__Dashboard__["json_metadata"]` as a
JSON string where
`"expand_all_slices": "False"` is stored as a string instead of a JSON
boolean.
2. Use this fixture to build a dashboard export bundle and import it through
the V1
dashboard import API `POST api/v1/dashboard/import/`, as exercised in
`tests/integration_tests/dashboards/api_tests.py:test_import_dashboard`
(lines 10-16),
which persists the `json_metadata` string directly into the
`Dashboard.json_metadata`
column.
3. Fetch the imported dashboard via `GET /api/v1/dashboard/<id>`; this is
simulated in
`superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx:24-27`,
where `json_metadata` is returned as `mockedJsonMetadata` containing
`"expand_all_slices":
"false"` as a string.
4. During client-side hydration,
`superset-frontend/src/dashboard/actions/hydrate.ts:378`
computes `expandAllSlices` as `metadata?.expand_all_slices || false`; with
`metadata.expand_all_slices` being a non-empty string (`"False"`/`"false"`),
the
expression is truthy, so the Redux state treats `expandAllSlices` as enabled
and all chart
descriptions expand even though the saved setting is logically false.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d4d53d1131c14c85bdd6f4f444313a05&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=d4d53d1131c14c85bdd6f4f444313a05&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:** tests/integration_tests/fixtures/importexport.py
**Line:** 72:72
**Comment:**
*Type Error: `expand_all_slices` is being serialized as the string
`"False"` instead of a JSON boolean. This value can pass through import
unchanged and later be treated as truthy (`"False" || false`) in frontend
hydration, causing all chart descriptions to expand when the setting is
intended to be disabled. Store it as a real boolean (`false`) in the fixture
payload.
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%2F32958&comment_hash=00d1c8824ceb8a557f1c6353c58a5faa9919f20dd0444db6ddddc62fb58e24a9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32958&comment_hash=00d1c8824ceb8a557f1c6353c58a5faa9919f20dd0444db6ddddc62fb58e24a9&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]
