bito-code-review[bot] commented on code in PR #37107:
URL: https://github.com/apache/superset/pull/37107#discussion_r2691744897
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -1717,6 +2007,7 @@ class DatasourceEditor extends PureComponent {
label={t('Metric currency')}
control={
<CurrencyControl
+ onChange={() => {}}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Broken currency selection</b></div>
<div id="fix">
The CurrencyControl's onChange is set to a no-op, so selecting a currency
won't update the metric's currency field. This breaks currency selection
functionality.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
no_patch_reason: code_fix_patch_generator tool failed due to insufficient
context for the item renderer scope, but the fix is straightforward: replace
onChange={() => {}} with value={v} onChange={onChange}
```
</div>
</details>
</div>
<small><i>Code Review Run #e8610b</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/components/Datasource/types.ts:
##########
@@ -54,20 +54,26 @@ export interface CRUDCollectionProps {
expandFieldset?: ReactNode;
extraButtons?: ReactNode;
itemGenerator?: () => any;
- itemCellProps?: ((
- val: unknown,
- label: string,
- record: any,
- ) => DetailedHTMLProps<
- TdHTMLAttributes<HTMLTableCellElement>,
- HTMLTableCellElement
- >)[];
- itemRenderers?: ((
- val: unknown,
- onChange: () => void,
- label: string,
- record: any,
- ) => ReactNode)[];
+ itemCellProps?: Record<
+ string,
+ (
+ val: unknown,
+ label: string,
+ record: any,
+ ) => DetailedHTMLProps<
+ TdHTMLAttributes<HTMLTableCellElement>,
+ HTMLTableCellElement
+ >
+ >;
+ itemRenderers?: Record<
+ string,
+ (
+ val: unknown,
+ onChange: (value: unknown) => void,
+ label: string,
+ record: any,
+ ) => ReactNode
+ >;
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Type mismatch in onCellChange</b></div>
<div id="fix">
The type change from array to Record improves maintainability by keying
renderers/cell props by column name rather than order. The onChange signature
update to pass the value is correct for usages like TextControl. However, the
onCellChange implementation expects boolean, creating a type mismatch if
renderers pass non-boolean values.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
- onCellChange(id: number, col: string, val: boolean) {
+ onCellChange(id: number, col: string, val: unknown) {
```
</div>
</details>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/e9e5826/.cursor/rules/dev-standard.mdc#L16";>dev-standard.mdc:16</a>
</li>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/e9e5826/AGENTS.md#L57";>AGENTS.md:57</a>
</li>
</ul>
</details>
<small><i>Code Review Run #e8610b</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/components/Datasource/components/Field/index.tsx:
##########
@@ -26,16 +26,16 @@ import {
FormLabel,
} from '@superset-ui/core/components';
-interface FieldProps<V> {
+export interface FieldProps<V> {
fieldKey: string;
value?: V;
label: string;
description?: ReactNode;
control: ReactElement;
additionalControl?: ReactElement;
- onChange: (fieldKey: string, newValue: V) => void;
- compact: boolean;
- inline: boolean;
+ onChange?: (fieldKey: string, newValue: V) => void;
+ compact?: boolean;
+ inline?: boolean;
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing default for optional prop</b></div>
<div id="fix">
Making 'inline' optional in FieldProps is fine, but the function lacks a
default value, which could lead to inconsistent layout behavior if omitted. It
looks like undefined would act as false, but adding a default ensures clarity
and prevents potential issues.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
- onChange = () => {},
- compact = false,
- inline,
- errorMessage,
+ onChange = () => {},
+ compact = false,
+ inline = false,
+ errorMessage,
```
</div>
</details>
</div>
<small><i>Code Review Run #e8610b</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
