codeant-ai-for-open-source[bot] commented on code in PR #39968:
URL: https://github.com/apache/superset/pull/39968#discussion_r3209328771
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts:
##########
@@ -778,6 +779,7 @@ const transformProps = (
colorPositiveNegative,
totals,
showTotals,
+ showRowGroupCounts,
Review Comment:
**Suggestion:** The new row-group count setting is forwarded
unconditionally, so a value saved in Aggregate mode is still applied in Raw
Records mode even though the control is hidden there. Because the control uses
`resetOnHide: false`, users can switch to Raw mode with counts suppressed and
no visible way to re-enable them. Gate this value by query mode (e.g., only
honor it in Aggregate mode, otherwise force the default behavior). [incorrect
condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Raw Records view inherits hidden Aggregate toggle state.
- ⚠️ Row group counts can be disabled with no Raw-mode control.
- ⚠️ UX confusion: Raw behavior depends on invisible Aggregate option.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In the AG Grid Table control panel
(`superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx`
lines
95–106), uncheck the `show_row_group_counts` checkbox while in Aggregate
query mode. This
writes `show_row_group_counts: false` into the chart form_data and, because
`resetOnHide:
false`, the value will persist even when the control is later hidden.
2. Switch the same chart to Raw query mode using the `queryMode` radio
control defined in
`controlPanel.tsx` lines 26–35 and the `isAggMode`/`isRawMode` helpers at
lines 13–14,
which hide the `show_row_group_counts` control via `visibility: isAggMode`
but do not
alter its saved value.
3. When the chart re-renders in Raw mode, `transformProps`
(`superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts`
lines
493–511) merges `formData` and destructures `show_row_group_counts:
showRowGroupCounts =
true` without checking `queryMode`, then returns `showRowGroupCounts`
unchanged in the
props object (`transformProps.ts` lines 754–782). Thus `showRowGroupCounts`
remains
`false` in Raw mode if it was previously turned off in Aggregate mode.
4. `AgGridTableChart`
(`superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx`
lines
1–31 and 420–33) forwards `showRowGroupCounts` to `AgGridTable`, which uses
it to build
`autoGroupColumnDef` with `suppressCount: !showRowGroupCounts`
(`superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx`
lines
12–18, 56–66). As a result, in Raw Records mode the group row counts remain
hidden with no
visible control to re-enable them there; users must switch back to Aggregate
mode to
change the setting.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Fplugin-chart-ag-grid-table%2Fsrc%2FtransformProps.ts%0A%2A%2ALine%3A%2A%2A%20782%3A782%0A%2A%2AComment%3A%2A%2A%0A%09%2AIncorrect%20Condition%20Logic%3A%20The%20new%20row-group%20count%20setting%20is%20forwarded%20unconditionally%2C%20so%20a%20value%20saved%20in%20Aggregate%20mode%20is%20still%20applied%20in%20Raw%20Records%20mode%20even%20though%20the%20control%20is%20hidden%20there.%20Because%20the%20control%20uses%20%60resetOnHide%3A%20false%60%2C%20users%20can%20switch%20to%20Raw%20mode%20with%20counts%20suppressed%20and%20no%20visible%20way%20to%20re-enable%20them.%20Gate%20this%20value%20by%20query%20mode%20%28e.g.%2C%20only%20honor%20it%20in%20Aggregate%20mode%2C%20otherwise%20force%20the%20default%20behavior%29.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct
%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Fplugin-chart-ag-grid-table%2Fsrc%2FtransformProps.ts%0A%2A%2ALine%3A%2A%2A%20782%3A782%0A%2A%2AComment%3A%2A%2A%0A%09%2AIncorrect%20Condition%20Logic%3A%20The%20new%20row-group%20count%20setting%20is%20forwarded%20unconditionally%2C%20so%20a%20value%20saved%20in%20Aggregate%20mode%20is%20still%20applied%20in%20Raw%20Records%20mode%2
0even%20though%20the%20control%20is%20hidden%20there.%20Because%20the%20control%20uses%20%60resetOnHide%3A%20false%60%2C%20users%20can%20switch%20to%20Raw%20mode%20with%20counts%20suppressed%20and%20no%20visible%20way%20to%20re-enable%20them.%20Gate%20this%20value%20by%20query%20mode%20%28e.g.%2C%20only%20honor%20it%20in%20Aggregate%20mode%2C%20otherwise%20force%20the%20default%20behavior%29.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(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/plugins/plugin-chart-ag-grid-table/src/transformProps.ts
**Line:** 782:782
**Comment:**
*Incorrect Condition Logic: The new row-group count setting is
forwarded unconditionally, so a value saved in Aggregate mode is still applied
in Raw Records mode even though the control is hidden there. Because the
control uses `resetOnHide: false`, users can switch to Raw mode with counts
suppressed and no visible way to re-enable them. Gate this value by query mode
(e.g., only honor it in Aggregate mode, otherwise force the default behavior).
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%2F39968&comment_hash=510d0b7180712959850289725800f2b002e157cd8bba5171ce150a7e28271387&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39968&comment_hash=510d0b7180712959850289725800f2b002e157cd8bba5171ce150a7e28271387&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]
