codeant-ai-for-open-source[bot] commented on code in PR #32958:
URL: https://github.com/apache/superset/pull/32958#discussion_r3591725920
##########
superset/daos/dashboard.py:
##########
@@ -397,6 +397,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:** The new metadata assignment stores `expand_all_slices`
without normalizing its type, so legacy/string values like `"False"` are
persisted as strings and later evaluated as truthy in the frontend. Coerce this
field to a real boolean before saving (or validate/coerce via schema load, not
just schema validate) to prevent dashboards from incorrectly expanding all
descriptions. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Copied dashboards may show all chart descriptions.
- ⚠️ Advanced expand-all setting can invert for legacy metadata.
- ⚠️ Frontend state receives truthy strings instead of booleans.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Trigger the dashboard copy flow so that `CopyDashboardCommand` receives a
payload with
`json_metadata` containing `"expand_all_slices": "False"` (for example,
using JSON shaped
like the fixture in
`tests/integration_tests/fixtures/importexport.py:26-34`, where
`__Dashboard__.json_metadata` embeds `"expand_all_slices": "False"`), and
the command at
`superset/commands/dashboard/copy.py:35-41` passes this `json_metadata`
string into
`DashboardDAO.copy_dashboard`.
2. In `DashboardDAO.copy_dashboard` at `superset/daos/dashboard.py:45-61`,
the code
executes `metadata = json.loads(data["json_metadata"])`, so the parsed
`metadata`
dictionary now contains `{"expand_all_slices": "False"}` where the value is
a Python
string, not a boolean.
3. The same method then calls `DashboardDAO.set_dash_metadata(dash, metadata,
old_to_new_slice_ids)` at `superset/daos/dashboard.py:7, 45-47`, and inside
`set_dash_metadata` the new line at `superset/daos/dashboard.py:400`
(`md["expand_all_slices"] = data.get("expand_all_slices", False)`) copies
the string
`"False"` directly into `md`, which is built from `dashboard.params_dict`
(`superset/daos/dashboard.py:47`), and persists it via
`dashboard.json_metadata =
json.dumps(md)` at `superset/daos/dashboard.py:29`.
4. When the copied dashboard is viewed, the frontend hydration logic in
`superset-frontend/src/dashboard/actions/hydrate.ts:22-30` reads the
server-side metadata
and constructs `dashboardState` with `expandAllSlices:
metadata?.expand_all_slices ||
false` at line 29; because the string `"False"` is truthy in JavaScript,
`expandAllSlices`
becomes the truthy string instead of the intended `false`.
5. Chart components then compute `isExpanded` using the selector in
`superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:10-15`,
which
does `!!((state.dashboardState as JsonObject).expandedSlices?.[props.id] ??
(state.dashboardState as JsonObject).expandAllSlices)`, so `!!('False')`
evaluates to
`true` and every chart description is treated as expanded even though the
original JSON
metadata logically represented a disabled `expand_all_slices` setting.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5f730c4c769445fcaf4b5ab25d9b3d99&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=5f730c4c769445fcaf4b5ab25d9b3d99&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:** 400:400
**Comment:**
*Type Error: The new metadata assignment stores `expand_all_slices`
without normalizing its type, so legacy/string values like `"False"` are
persisted as strings and later evaluated as truthy in the frontend. Coerce this
field to a real boolean before saving (or validate/coerce via schema load, not
just schema validate) to prevent dashboards from incorrectly expanding all
descriptions.
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=2dfbfcc18760cc4ccbc16b6918dda5d82281d4e897955431e816d9dca279ba63&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32958&comment_hash=2dfbfcc18760cc4ccbc16b6918dda5d82281d4e897955431e816d9dca279ba63&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]
