codeant-ai-for-open-source[bot] commented on code in PR #37893:
URL: https://github.com/apache/superset/pull/37893#discussion_r2793651545
##########
superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx:
##########
@@ -283,7 +283,7 @@ const config: ControlPanelConfig = {
return true;
}
if (isPhysicalColumn(selection)) {
- return !!dttmLookup[selection];
+ return !!dttmLookup[(selection || '').toLowerCase()];
Review Comment:
**Suggestion:** The visibility check assumes that any value passing
`isPhysicalColumn` is a string and calls `.toLowerCase()` on it, but
`isPhysicalColumn` can also return true for column objects (e.g., with a
`column_name` field); in that case `selection` is an object and `(selection ||
'').toLowerCase()` will throw at runtime. To avoid this, derive the key from
`selection.column_name` when `selection` is an object and only call
`.toLowerCase()` on a string. [type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Table chart Explore panel can throw on render.
- ❌ Time grain control may not render for valid groupbys.
- ⚠️ Users blocked from adjusting aggregation time grain.
```
</details>
```suggestion
const physicalColumnName =
typeof selection === 'string'
? selection
: selection.column_name || '';
return !!dttmLookup[physicalColumnName.toLowerCase()];
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Superset frontend and open a Table chart in Explore that uses the
Table plugin
config from
`superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx`.
2. In the Query section, set `Query mode` to Aggregate so that the `groupby`
and
`time_grain_sqla` controls are active (config block starting around lines
240–310 in
`controlPanel.tsx`).
3. In the `Dimensions`/`Group by` control (which uses
`sharedControls.groupby` overridden
in this file), select a physical column that is represented as a column
object (a
`ColumnMeta` with a `column_name` field) so that `controls.groupby.value`
contains that
object rather than just the column name string.
4. When the control panel re-renders, the `time_grain_sqla` control's
`visibility`
function (defined in `controlPanel.tsx` around lines 260–290) executes: for
the object
`selection`, `isAdhocColumn(selection)` returns false,
`isPhysicalColumn(selection)`
returns true, and then `(selection || '').toLowerCase()` is evaluated,
causing a runtime
error `selection.toLowerCase is not a function` and breaking rendering of
the time grain
control (and possibly the panel row).
```
</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-table/src/controlPanel.tsx
**Line:** 286:286
**Comment:**
*Type Error: The visibility check assumes that any value passing
`isPhysicalColumn` is a string and calls `.toLowerCase()` on it, but
`isPhysicalColumn` can also return true for column objects (e.g., with a
`column_name` field); in that case `selection` is an object and `(selection ||
'').toLowerCase()` will throw at runtime. To avoid this, derive the key from
`selection.column_name` when `selection` is an object and only call
`.toLowerCase()` on a string.
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%2F37893&comment_hash=d283853f3a66f2ccf73ef673601a4e89bc5ff53d1f2248343c382882d117fabc&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37893&comment_hash=d283853f3a66f2ccf73ef673601a4e89bc5ff53d1f2248343c382882d117fabc&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]
