codeant-ai-for-open-source[bot] commented on code in PR #38865:
URL: https://github.com/apache/superset/pull/38865#discussion_r2994408549
##########
superset-frontend/src/dataMask/reducer.ts:
##########
@@ -245,35 +246,39 @@ const dataMaskReducer = produce(
if (!isChartCustomizationItem(item)) {
return;
}
+ if (isDynamicTitleCustomization(item)) {
+ return;
+ }
+ const customizationItem = item as ChartCustomization;
- const customizationFilterId = item.id;
+ const customizationFilterId = customizationItem.id;
const dataMask = loadedDataMask || {};
cleanState[customizationFilterId] = {
...getInitialDataMask(customizationFilterId),
- ...item.defaultDataMask,
+ ...customizationItem.defaultDataMask,
...dataMask[customizationFilterId],
};
if (
draft[customizationFilterId] &&
- item.defaultDataMask &&
+ customizationItem.defaultDataMask &&
!areObjectsEqual(
- item.defaultDataMask,
+ customizationItem.defaultDataMask,
draft[customizationFilterId],
{ ignoreUndefined: true },
)
) {
cleanState[customizationFilterId] = {
...cleanState[customizationFilterId],
- ...item.defaultDataMask,
+ ...customizationItem.defaultDataMask,
};
}
Review Comment:
**Suggestion:** The `if` block that compares
`customizationItem.defaultDataMask` to `draft[customizationFilterId]` and then
re-applies `defaultDataMask` will almost always evaluate as "not equal",
causing default values to be reapplied over any persisted data mask for that
customization on every hydrate, effectively discarding saved user state for
chart customizations. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Chart customization plugins may lose persisted state on hydrate.
- ⚠️ Users can see default customization instead of last selection.
```
</details>
```suggestion
// customizationItem.defaultDataMask is already applied above when
initializing cleanState
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open any dashboard in the UI, which triggers `hydrateDashboard` in
`src/dashboard/actions/hydrate.ts:104-113`, passing `dataMask` from the
backend and
`dashboard.metadata.chart_customization_config` into the Redux store.
2. Ensure the dashboard metadata contains at least one non-dynamic-title
chart
customization with an `id` and a `defaultDataMask` (these customizations are
read via
`selectChartCustomizationConfiguration` in
`src/dashboard/components/nativeFilters/state.ts:98-125`).
3. Dispatch the `HYDRATE_DASHBOARD` action so `dataMaskReducer` in
`src/dataMask/reducer.ts:200-219` runs its `HYDRATE_DASHBOARD` case, iterates
`chartCustomizationConfig`, and initializes
`cleanState[customizationFilterId]` with
`getInitialDataMask`, `customizationItem.defaultDataMask`, and the backend
`loadedDataMask[customizationFilterId]` (lines 55-62).
4. When there is already a `draft[customizationFilterId]` entry (previous
in-memory
`DataMask` for that customization), the condition at
`src/dataMask/reducer.ts:64-71`
compares `customizationItem.defaultDataMask` against the full
`draft[customizationFilterId]` object; this almost always fails equality and
re-applies
`defaultDataMask` over `cleanState` at lines 73-76, overriding any
overlapping fields from
`loadedDataMask[customizationFilterId]` and effectively discarding persisted
state for
that customization on hydrate.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dataMask/reducer.ts
**Line:** 263:276
**Comment:**
*Logic Error: The `if` block that compares
`customizationItem.defaultDataMask` to `draft[customizationFilterId]` and then
re-applies `defaultDataMask` will almost always evaluate as "not equal",
causing default values to be reapplied over any persisted data mask for that
customization on every hydrate, effectively discarding saved user state for
chart customizations.
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%2F38865&comment_hash=721a49fbb6a9d16aa083ace92ce0f8fe26979b2d46b4d0fdc531258f1cff38e5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38865&comment_hash=721a49fbb6a9d16aa083ace92ce0f8fe26979b2d46b4d0fdc531258f1cff38e5&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]