codeant-ai-for-open-source[bot] commented on code in PR #32958:
URL: https://github.com/apache/superset/pull/32958#discussion_r3485734883
##########
superset/dashboards/schemas.py:
##########
@@ -197,6 +197,7 @@ class DashboardJSONMetadataSchema(Schema):
# deprecated wrt dashboard-native filters
filter_scopes = fields.Dict()
expanded_slices = fields.Dict()
+ expand_all_slices = fields.Boolean()
Review Comment:
**Suggestion:** This permissive boolean field accepts string values like
`"false"`, but dashboard metadata is later consumed as raw JSON and string
`"false"` is truthy in frontend checks, causing all descriptions to appear
expanded unexpectedly. Enforce strict boolean-only input for this key (reject
non-boolean types) so persisted metadata cannot store string booleans. [type
error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dashboards misinterpret expand-all flag when typed as string.
- ⚠️ Frontend shows all descriptions expanded despite metadata “false”.
- ⚠️ External clients sending string booleans persist inconsistent state.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Send a dashboard create or update request where the `json_metadata`
payload (validated
by `validate_json_metadata` in `superset/dashboards/schemas.py:108-118`)
contains
`"expand_all_slices": "false"` as a string, so `json.loads` in
`validate_json_metadata`
produces a Python dict with `{"expand_all_slices": "false"}`.
2. `validate_json_metadata` calls
`DashboardJSONMetadataSchema().validate(value_obj,
partial=False)` at `superset/dashboards/schemas.py:116-117`, and the
`expand_all_slices =
fields.Boolean()` definition in `DashboardJSONMetadataSchema` at
`superset/dashboards/schemas.py:88-102` accepts the string `"false"` as a
valid boolean
value, so validation passes and the original JSON string is stored unchanged.
3. When persisting metadata, `DashboardDAO.set_dash_metadata` in
`superset/daos/dashboard.py:4-14,61-67` is given the decoded metadata dict
and assigns
`md["expand_all_slices"] = data.get("expand_all_slices", False)` at
`dashboard.py:5-7,339-418`; because `data["expand_all_slices"]` is the
string `"false"`,
that exact string is written into `md` and then into
`dashboard.json_metadata` via
`json.dumps(md)`.
4. On dashboard load, `hydrateDashboard()` in
`superset-frontend/src/dashboard/actions/hydrate.ts:360-419` reads `metadata
=
JSON.parse(dashboard.json_metadata)` and computes `expandAllSlices:
metadata?.expand_all_slices || false` at `hydrate.ts:378`; with
`metadata.expand_all_slices === "false"`, the JavaScript `||` treats the
string as truthy,
so `expandAllSlices` becomes `"false"`, and `Chart`’s `isExpanded` selector
at
`superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:209-215`
evaluates `!!(expandedSlices?.[props.id] ?? expandAllSlices)` to `true`,
causing chart
descriptions to expand even though metadata attempted to disable them.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3bcd4c4c02e243c0aa16e0aa52269133&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=3bcd4c4c02e243c0aa16e0aa52269133&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/dashboards/schemas.py
**Line:** 200:200
**Comment:**
*Type Error: This permissive boolean field accepts string values like
`"false"`, but dashboard metadata is later consumed as raw JSON and string
`"false"` is truthy in frontend checks, causing all descriptions to appear
expanded unexpectedly. Enforce strict boolean-only input for this key (reject
non-boolean types) so persisted metadata cannot store string booleans.
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=e11b0c3ea8ac17cb67662a6cd594d1fd7b4dd0719fa32acbed1e3ee4ea3fdd62&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32958&comment_hash=e11b0c3ea8ac17cb67662a6cd594d1fd7b4dd0719fa32acbed1e3ee4ea3fdd62&reaction=dislike'>👎</a>
##########
superset-frontend/src/dashboard/reducers/dashboardState.ts:
##########
@@ -258,11 +258,7 @@ export default function dashboardStateReducer(
const updatedExpandedSlices = { ...state.expandedSlices };
const { sliceId } = action;
if (sliceId !== undefined) {
- if (updatedExpandedSlices[sliceId]) {
- delete updatedExpandedSlices[sliceId];
- } else {
- updatedExpandedSlices[sliceId] = true;
- }
+ updatedExpandedSlices[sliceId] = !updatedExpandedSlices[sliceId];
Review Comment:
**Suggestion:** This toggle inverts only the per-slice override value, not
the effective expanded state. When global expand is enabled and a slice has no
explicit override, the first click on “Hide chart description” flips
`undefined` to `true`, so the description stays visible instead of hiding.
Compute the current state using the global fallback (`expandedSlices[sliceId]
?? expandAllSlices`) and then invert that effective value. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Global expand-all descriptions toggle fails to hide initially.
- ⚠️ Chart header “Hide description” appears unresponsive to users.
- ⚠️ Dashboard UX inconsistent when per-slice overrides are first used.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Load a dashboard whose JSON metadata has `"expand_all_slices": true`, so
`hydrateDashboard()` in
`superset-frontend/src/dashboard/actions/hydrate.ts:360-420` sets
`dashboardState.expandAllSlices` via `expandAllSlices:
metadata?.expand_all_slices ||
false` at `hydrate.ts:378`.
2. On the same dashboard, pick a chart with a description where there is no
per-slice
override in metadata (no entry for that slice id in `expanded_slices`), so
`expandedSlices[props.id]` is `undefined` when `Chart` computes `isExpanded`
in
`superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:209-215`
using
`expandedSlices?.[props.id] ?? expandAllSlices`, resulting in `isExpanded
=== true` from
the global flag.
3. In the chart header controls, click “Hide chart description”, which
triggers the
`MenuKeys.ToggleChartDescription` case in `handleMenuClick` in
`superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:251-260`,
calling `props.toggleExpandSlice(props.slice.slice_id)`, with
`toggleExpandSlice` bound in
`Chart` via `bindActionCreators` at `Chart.tsx:57-69`.
4. The `TOGGLE_EXPAND_SLICE` action is handled by `dashboardStateReducer` in
`superset-frontend/src/dashboard/reducers/dashboardState.ts:257-263`, where
the reducer
executes `updatedExpandedSlices[sliceId] = !updatedExpandedSlices[sliceId];`
(line 261),
turning an `undefined` entry into `true`; on the next render `isExpanded` is
still true
because `expandedSlices[sliceId]` is now `true`, so the description remains
visible and
the first “Hide” click appears to have no effect.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1f1dc00aa21242fdaad4da7d0ae059e3&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=1f1dc00aa21242fdaad4da7d0ae059e3&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/reducers/dashboardState.ts
**Line:** 261:261
**Comment:**
*Logic Error: This toggle inverts only the per-slice override value,
not the effective expanded state. When global expand is enabled and a slice has
no explicit override, the first click on “Hide chart description” flips
`undefined` to `true`, so the description stays visible instead of hiding.
Compute the current state using the global fallback (`expandedSlices[sliceId]
?? expandAllSlices`) and then invert that effective value.
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=7ba833baa08f036638c6c70a410c799629d071162b217351e3442a9602ae1283&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32958&comment_hash=7ba833baa08f036638c6c70a410c799629d071162b217351e3442a9602ae1283&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]
