codeant-ai-for-open-source[bot] commented on code in PR #42088:
URL: https://github.com/apache/superset/pull/42088#discussion_r3632064663
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx:
##########
@@ -717,24 +750,70 @@ const config: ControlPanelConfig = {
: [];
const chartStatus = chart?.chartStatus;
+ // Normalize legacy `toAllRow`/`toTextColor` flags saved before
+ // `columnFormatting`/`objectFormatting` existed, so "entire
row"
+ // formatters set under the old schema keep working.
+ const value = _?.value ?? [];
+ if (value && Array.isArray(value)) {
+ value.forEach(
+ (item: ConditionalFormattingConfig, index, array) => {
+ if (
+ item.colorScheme &&
+ !['Green', 'Red'].includes(item.colorScheme)
+ ) {
+ if (item.columnFormatting === undefined) {
+ // eslint-disable-next-line no-param-reassign
+ array[index] = {
+ ...item,
+ ...(item.toTextColor === true && {
+ objectFormatting:
ObjectFormattingEnum.TEXT_COLOR,
+ }),
+ ...(item.toAllRow === true && {
+ columnFormatting:
ObjectFormattingEnum.ENTIRE_ROW,
+ }),
+ };
+ }
+ }
+ },
+ );
+ }
const { colnames, coltypes } =
chart?.queriesResponse?.[0] ?? {};
- const numericColumns =
- Array.isArray(colnames) && Array.isArray(coltypes)
- ? colnames
- .filter(
- (colname: string, index: number) =>
- coltypes[index] === GenericDataType.Numeric,
- )
- .map((colname: string) => ({
- value: colname,
- label: Array.isArray(verboseMap)
- ? colname
- : (verboseMap[colname] ?? colname),
- dataType:
- colnames && coltypes[colnames?.indexOf(colname)],
- }))
- : [];
+ const hasColumns =
+ Array.isArray(colnames) && Array.isArray(coltypes);
+ const allColumns = hasColumns
+ ? [
+ {
+ value: ObjectFormattingEnum.ENTIRE_ROW,
+ label: t('entire row'),
+ dataType: GenericDataType.String,
+ },
+ ...colnames.map((colname: string, index: number) => ({
+ value: colname,
+ label: Array.isArray(verboseMap)
+ ? colname
+ : (verboseMap[colname] ?? colname),
Review Comment:
**Suggestion:** The new label resolution dereferences `verboseMap` without a
null check, so when metadata is missing this will throw at runtime while
building column options in the control panel. Guard this access with optional
chaining or a default object before indexing. [null pointer]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Conditional formatting control can crash Explore control panel.
- ⚠️ Users unable to configure row-level formatting.
- ⚠️ Migration to Table V2 hampers due to broken UI.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Explore view for a Table V2 (AG Grid) chart so that the control
panel
configuration from
`superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx`
is used via
`mapStateToProps()` in
`superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx:569`.
2. Trigger the conditional formatting control so its `mapStateToProps` runs
the block
starting at `controlPanel.tsx:740`, reaching the construction of
`allColumns` and
`numericColumns` around lines 783–816.
3. In this execution, `chart?.queriesResponse?.[0]?.colnames` and `coltypes`
are present
(so `hasColumns` is true), but `verboseMap` is undefined or missing (e.g.,
datasource
metadata not yet loaded or not provided), so `Array.isArray(verboseMap)` at
line 793
returns false.
4. The ternary expression then evaluates `(verboseMap[colname] ?? colname)`
at line 795,
which dereferences `verboseMap` directly; with `verboseMap` undefined, this
throws a
`TypeError: Cannot read properties of undefined` and breaks control-panel
rendering for
the conditional formatting UI.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=676f16bce11444f39ba0380258cb56bc&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=676f16bce11444f39ba0380258cb56bc&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/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx
**Line:** 793:795
**Comment:**
*Null Pointer: The new label resolution dereferences `verboseMap`
without a null check, so when metadata is missing this will throw at runtime
while building column options in the control panel. Guard this access with
optional chaining or a default object before indexing.
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%2F42088&comment_hash=97b545fca491f8318ceae0ea89d193e6fdb31c9fa872905b3f6b1cebc66b9a70&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42088&comment_hash=97b545fca491f8318ceae0ea89d193e6fdb31c9fa872905b3f6b1cebc66b9a70&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]
