codeant-ai-for-open-source[bot] commented on code in PR #32958:
URL: https://github.com/apache/superset/pull/32958#discussion_r3464381190
##########
superset-frontend/src/dashboard/actions/hydrate.ts:
##########
@@ -375,6 +375,7 @@ export const hydrateDashboard =
directPathLastUpdated: Date.now(),
focusedFilterField: null,
expandedSlices: metadata?.expanded_slices || {},
+ expandAllSlices: metadata?.expand_all_slices || false,
Review Comment:
**Suggestion:** This defaulting logic uses `||`, so a legacy string value
like `"false"` is treated as truthy and carried into state. Downstream, the
chart visibility check casts this value with `!!`, which makes `"false"` behave
as `true` and incorrectly expands all chart descriptions on load. Normalize
this to a real boolean (and use nullish/default semantics rather than
truthy/falsy coercion). [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dashboards can expand descriptions despite metadata saying false.
- ⚠️ Advanced users editing JSON metadata see confusing behavior.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Update a dashboard via the REST API using `DashboardPutSchema`
(superset/dashboards/schemas.py:470-39) by sending a `PUT
/api/v1/dashboard/<id>` with a
`json_metadata` string containing `"expand_all_slices": "false"` (note the
quoted string).
2. `DashboardPutSchema.json_metadata` uses `validate_json_metadata`
(superset/dashboards/schemas.py:35-39 and 108-18), which loads the JSON and
validates it
with `DashboardJSONMetadataSchema.expand_all_slices = fields.Boolean()`
(superset/dashboards/schemas.py:187-22); the `"false"` string is accepted as
boolean-like
but the original JSON string remains unchanged.
3. `UpdateDashboardCommand.run`
(superset/commands/dashboard/update.py:19-40) then loads
this same `json_metadata` string with `json.loads` and passes the resulting
dict (where
`expand_all_slices` is still the string `"false"`) into
`DashboardDAO.set_dash_metadata`
(superset/daos/dashboard.py:4-8), which assigns `md["expand_all_slices"] =
data.get("expand_all_slices", False)` (superset/daos/dashboard.py:84-86),
persisting the
`"false"` string into `dashboard.json_metadata`.
4. When a user opens the dashboard, `DashboardRestApi.get`
(superset/dashboards/api.py:28-40) returns this metadata to the frontend;
`DashboardPage.tsx` calls `hydrateDashboard`
(superset-frontend/src/dashboard/containers/DashboardPage.tsx:32-45), which
sets
`expandAllSlices: metadata?.expand_all_slices || false`
(superset-frontend/src/dashboard/actions/hydrate.ts:31-40). Because
`"false"` is truthy,
`expandAllSlices` becomes the string `"false"`, and `Chart.tsx` computes
`isExpanded` as
`!!(state.dashboardState.expandedSlices?.[props.id] ??
state.dashboardState.expandAllSlices)`
(superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:89-94),
so
`!!"false"` is `true`, incorrectly expanding all chart descriptions despite
the user
specifying `"false"` in metadata.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=001b27ed5d80424cb40daafa8f3c7c3f&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=001b27ed5d80424cb40daafa8f3c7c3f&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-frontend/src/dashboard/actions/hydrate.ts
**Line:** 378:378
**Comment:**
*Incorrect Condition Logic: This defaulting logic uses `||`, so a
legacy string value like `"false"` is treated as truthy and carried into state.
Downstream, the chart visibility check casts this value with `!!`, which makes
`"false"` behave as `true` and incorrectly expands all chart descriptions on
load. Normalize this to a real boolean (and use nullish/default semantics
rather than truthy/falsy coercion).
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=8deafc3e4b8fa61b0b94be63f5194a5e76c1abb6d3a08a26b1cb89f276befc43&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32958&comment_hash=8deafc3e4b8fa61b0b94be63f5194a5e76c1abb6d3a08a26b1cb89f276befc43&reaction=dislike'>👎</a>
##########
superset/daos/dashboard.py:
##########
@@ -341,6 +341,7 @@ def set_dash_metadata(
md["color_namespace"] = data.get("color_namespace")
md["expanded_slices"] = data.get("expanded_slices", {})
+ md["expand_all_slices"] = data.get("expand_all_slices", False)
Review Comment:
**Suggestion:** This writes `expand_all_slices` without type normalization,
so non-boolean inputs (for example legacy `"false"` strings) can be persisted
into dashboard metadata. That malformed type later reaches the frontend and can
be interpreted incorrectly as enabled. Coerce/validate this value to a strict
boolean before storing it. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dashboards persist string expand_all_slices values in metadata.
- ⚠️ Frontend misinterprets persisted strings, altering chart visibility.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Issue a dashboard update through the REST API using `DashboardPutSchema`
(superset/dashboards/schemas.py:470-39) with a body where `json_metadata` is
a string such
as `'{"expand_all_slices": "false"}'`, e.g. via `PUT /api/v1/dashboard/<id>`.
2. `DashboardPutSchema.json_metadata` calls `validate_json_metadata`
(superset/dashboards/schemas.py:35-39 and 108-18), which parses the JSON and
validates it
with `DashboardJSONMetadataSchema` (`expand_all_slices = fields.Boolean()` at
superset/dashboards/schemas.py:187-22); the `"false"` string passes
validation as a
boolean-like value even though the underlying JSON still stores it as a
string.
3. In `UpdateDashboardCommand.run`
(superset/commands/dashboard/update.py:19-40), Superset
again calls `json.loads(self._properties.get("json_metadata", "{}"))`,
producing a Python
dict where `data["expand_all_slices"]` is the string `"false"`, and then
calls
`DashboardDAO.set_dash_metadata` with this dict
(superset/daos/dashboard.py:4-8).
4. Inside `set_dash_metadata`, the assignment `md["expand_all_slices"] =
data.get("expand_all_slices", False)` (superset/daos/dashboard.py:84-86)
writes the
unnormalized `"false"` string directly into `dashboard.json_metadata`, which
later reaches
the frontend, where `hydrateDashboard`
(superset-frontend/src/dashboard/actions/hydrate.ts:31-40) and `Chart.tsx`
(superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:89-94)
treat
this truthy string as enabled and expand chart descriptions unexpectedly.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=53493538d3f64e14b9dfa6d605c8dd2d&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=53493538d3f64e14b9dfa6d605c8dd2d&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/daos/dashboard.py
**Line:** 344:344
**Comment:**
*Type Error: This writes `expand_all_slices` without type
normalization, so non-boolean inputs (for example legacy `"false"` strings) can
be persisted into dashboard metadata. That malformed type later reaches the
frontend and can be interpreted incorrectly as enabled. Coerce/validate this
value to a strict boolean before storing it.
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=0fe6771667b7e6e2cd81d9bdb11cf1b4c020feed66c9d2f81f857174c08b00e3&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32958&comment_hash=0fe6771667b7e6e2cd81d9bdb11cf1b4c020feed66c9d2f81f857174c08b00e3&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]
