codeant-ai-for-open-source[bot] commented on code in PR #38384:
URL: https://github.com/apache/superset/pull/38384#discussion_r2893125724
##########
superset-frontend/src/dashboard/reducers/dashboardInfo.ts:
##########
@@ -124,22 +122,11 @@ export default function dashboardInfoReducer(
case DASHBOARD_INFO_UPDATED: {
const dashAction = action as DashboardInfoAction;
const newInfo = dashAction.newInfo || {};
- const { theme_id: themeId, ...otherInfo } = newInfo;
- const updatedState: DashboardInfoState = {
+ return {
...state,
- ...otherInfo,
+ ...newInfo,
last_modified_time: Math.round(new Date().getTime() / 1000),
};
Review Comment:
**Suggestion:** When handling the generic dashboard info update action, the
reducer now shallow-merges `newInfo.metadata` into the existing state without
preserving `chartsInScope`/`tabsInScope` for `native_filter_configuration`,
even though the `preserveScopes` helper exists and is used for HYDRATE. This
means that whenever `dashboardInfoChanged` is dispatched with a
`metadata.native_filter_configuration` coming from the server (which may omit
those scope fields), the reducer will overwrite the in-memory
`native_filter_configuration` array and drop the existing scope data, causing
native filters to lose their chart/tab scoping after such updates. To avoid
this regression, the DASHBOARD_INFO_UPDATED branch should merge
`metadata.native_filter_configuration` via `preserveScopes` instead of blindly
overwriting it. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Saving dashboard overwrites native filter chart/tab scoping.
- ⚠️ Native filter bar shows filters as globally scoped again.
- ⚠️ Cross-filter and scoping UX becomes inconsistent after saves.
- ⚠️ HYDRATE preserves scopes but later updates silently discard them.
```
</details>
```suggestion
case DASHBOARD_INFO_UPDATED: {
const dashAction = action as DashboardInfoAction;
const newInfo = dashAction.newInfo || {};
let mergedMetadata = newInfo.metadata;
if (
newInfo.metadata?.native_filter_configuration &&
state.metadata?.native_filter_configuration
) {
mergedMetadata = {
...newInfo.metadata,
native_filter_configuration: preserveScopes(
state.metadata.native_filter_configuration,
newInfo.metadata.native_filter_configuration,
),
} as DashboardInfo['metadata'];
}
return {
...state,
...newInfo,
...(mergedMetadata && { metadata: mergedMetadata }),
last_modified_time: Math.round(new Date().getTime() / 1000),
};
}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a dashboard with native filters and adjust their chart/tab scoping
via the Native
Filters UI. The scoping update is handled by `setInScopeStatusOfFilters` in
`superset-frontend/src/dashboard/actions/nativeFilters.ts:106-147`, which
merges the new
`chartsInScope`/`tabsInScope` into `metadata.native_filter_configuration`
and dispatches
`dashboardInfoChanged({ metadata })` (lines 141-145), populating
`state.dashboardInfo.metadata.native_filter_configuration` with those scope
fields.
2. In the same session, click the "Save" button in the dashboard header.
This triggers
`overwriteDashboard` in
`superset-frontend/src/dashboard/components/Header/index.tsx:431-501`, which
calls
`boundActionCreators.onSave(data, dashboardInfo.id, SAVE_TYPE_OVERWRITE)`
(line 479).
`onSave` is bound to `saveDashboardRequest` from
`superset-frontend/src/dashboard/actions/dashboardState.ts` (import and
binding at lines
70-73 and 318-319).
3. Inside `saveDashboardRequest`
(`superset-frontend/src/dashboard/actions/dashboardState.ts:424-732`), after
a successful
`PUT /api/v1/dashboard/:id`, `onUpdateSuccess` (lines 548-603) parses the
backend
`json_metadata` into `parsedMetadata` (lines 554-557) and dispatches
`setDashboardMetadata(parsedMetadata)` (line 558). `setDashboardMetadata`
(lines 391-402)
in the same file then dispatches `dashboardInfoChanged({ metadata: {
...dashboardInfo?.metadata, ...updatedMetadata } })`, where
`updatedMetadata` comes from
the server and includes a `native_filter_configuration` that does not
contain the
client-only `chartsInScope`/`tabsInScope` fields.
4. The `dashboardInfoChanged` action creator in
`superset-frontend/src/dashboard/actions/dashboardInfo.ts:44-47` emits `{
type:
DASHBOARD_INFO_UPDATED, newInfo }`, which is handled by
`dashboardInfoReducer` in
`superset-frontend/src/dashboard/reducers/dashboardInfo.ts:117-129`. In the
`DASHBOARD_INFO_UPDATED` case (lines 122-129), the reducer shallow-merges
`...newInfo`
into `state`, so `state.metadata` is entirely replaced by
`newInfo.metadata`. Because
`newInfo.metadata.native_filter_configuration` came from the server without
`chartsInScope`/`tabsInScope`, the existing scopes stored in
`state.metadata.native_filter_configuration` are dropped. On subsequent
renders, any
component relying on
`state.dashboardInfo.metadata.native_filter_configuration` for
scoping (e.g., via `nativeFilters` state and Filter Bar components
referenced in
`superset-frontend/src/dashboard/components/nativeFilters/*`) will see
filters as if they
have lost their chart/tab scoping.
```
</details>
<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/dashboardInfo.ts
**Line:** 122:129
**Comment:**
*Logic Error: When handling the generic dashboard info update action,
the reducer now shallow-merges `newInfo.metadata` into the existing state
without preserving `chartsInScope`/`tabsInScope` for
`native_filter_configuration`, even though the `preserveScopes` helper exists
and is used for HYDRATE. This means that whenever `dashboardInfoChanged` is
dispatched with a `metadata.native_filter_configuration` coming from the server
(which may omit those scope fields), the reducer will overwrite the in-memory
`native_filter_configuration` array and drop the existing scope data, causing
native filters to lose their chart/tab scoping after such updates. To avoid
this regression, the DASHBOARD_INFO_UPDATED branch should merge
`metadata.native_filter_configuration` via `preserveScopes` instead of blindly
overwriting 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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38384&comment_hash=8b86cd6edb7f3f7a349372b7072b8beef3aeda4f8e122c7ae03025e3fa38ccce&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38384&comment_hash=8b86cd6edb7f3f7a349372b7072b8beef3aeda4f8e122c7ae03025e3fa38ccce&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]
