codeant-ai-for-open-source[bot] commented on code in PR #36990:
URL: https://github.com/apache/superset/pull/36990#discussion_r2682116249
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx:
##########
@@ -252,7 +252,7 @@ const config: ControlPanelConfig = {
visibility: ({ controls }) => {
const dttmLookup = Object.fromEntries(
ensureIsArray(controls?.groupby?.options).map(option => [
- option.column_name,
+ (option.column_name || '').toLowerCase(),
option.is_dttm,
]),
Review Comment:
**Suggestion:** Robustness bug: the mapping uses `option.column_name`
without guarding against `option` being null/other types; accessing
`.column_name` on `null`/`undefined` would throw. Normalize option names
defensively (handle string vs object vs null) before calling `.toLowerCase()`.
[possible bug]
**Severity Level:** Critical 🚨
```suggestion
ensureIsArray(controls?.groupby?.options).map(option => {
const name =
typeof option === 'string' ? option :
option?.column_name ?? '';
return [name.toLowerCase(), option?.is_dttm];
}),
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The suggestion to defensive-check option (string vs object vs null) is
sensible — the current code assumes option is an object with column_name.
Using a small normalization (typeof option === 'string' ? option :
option?.column_name ?? '') prevents potential exceptions and handles more input
shapes robustly.
</details>
<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:** 254:257
**Comment:**
*Possible Bug: Robustness bug: the mapping uses `option.column_name`
without guarding against `option` being null/other types; accessing
`.column_name` on `null`/`undefined` would throw. Normalize option names
defensively (handle string vs object vs null) before calling `.toLowerCase()`.
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>
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx:
##########
@@ -263,7 +263,7 @@ const config: ControlPanelConfig = {
return true;
}
if (isPhysicalColumn(selection)) {
- return !!dttmLookup[selection];
+ return !!dttmLookup[(selection || '').toLowerCase()];
Review Comment:
**Suggestion:** Runtime TypeError: the code calls `.toLowerCase()` on
`selection` without ensuring it's a string. When `selection` is an object (e.g.
a column object), `(selection || '').toLowerCase()` will attempt to call
`toLowerCase` on an object and throw. Extract the column name safely (handle
string vs object) before lowercasing and looking up in `dttmLookup`. [type
error]
**Severity Level:** Minor ⚠️
```suggestion
const colKey =
typeof selection === 'string'
? selection.toLowerCase()
: (selection?.column_name ||
'').toLowerCase();
return !!dttmLookup[colKey];
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This is a valid runtime bug: selection can be an object (column descriptor)
not a string,
so (selection || '').toLowerCase() may attempt to call toLowerCase on an
object and throw.
The improved code safely extracts a string key (handles string vs object)
before lowercasing
and fixes the potential TypeError.
</details>
<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:** 266:266
**Comment:**
*Type Error: Runtime TypeError: the code calls `.toLowerCase()` on
`selection` without ensuring it's a string. When `selection` is an object (e.g.
a column object), `(selection || '').toLowerCase()` will attempt to call
`toLowerCase` on an object and throw. Extract the column name safely (handle
string vs object) before lowercasing and looking up in `dttmLookup`.
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>
--
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]
