bito-code-review[bot] commented on code in PR #37107:
URL: https://github.com/apache/superset/pull/37107#discussion_r2706367939
##########
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx:
##########
@@ -89,7 +91,7 @@ export const useResultsPane = ({
ownState,
})
.then(({ json }) => {
- setResultResp(ensureIsArray(json.result));
+ setResultResp(ensureIsArray(json.result) as QueryResultInterface[]);
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Type assertion mismatch</b></div>
<div id="fix">
Similar to the cache case, this type assertion mismatches the data
structure, potentially causing issues when processing query results.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
- data: Record<string, any>[][];
+ data: Record<string, any>[];
```
</div>
</details>
</div>
<small><i>Code Review Run #6c4885</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/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx:
##########
@@ -168,27 +177,40 @@ describe('ExploreChartHeader', () => {
const props = createProps();
render(<ExploreHeader {...props} />, { useRedux: true });
const newChartName = 'New chart name';
- const prevChartName = props.slice_name;
+ const prevChartName = props.sliceName;
+
+ // Wait for the component to render with the chart title
expect(
- await screen.findByText(/add the name of the chart/i),
+ await screen.findByDisplayValue(prevChartName ?? ''),
).toBeInTheDocument();
- userEvent.click(screen.getByLabelText('Menu actions trigger'));
- userEvent.click(screen.getByText('Edit chart properties'));
+ await userEvent.click(screen.getByLabelText('Menu actions trigger'));
+ await userEvent.click(screen.getByText('Edit chart properties'));
const nameInput = await screen.findByRole('textbox', { name: 'Name' });
- userEvent.clear(nameInput);
- userEvent.type(nameInput, newChartName);
+ await userEvent.clear(nameInput);
+ await userEvent.type(nameInput, newChartName);
expect(screen.getByDisplayValue(newChartName)).toBeInTheDocument();
- userEvent.click(screen.getByRole('button', { name: 'Cancel' }));
+ await userEvent.click(screen.getByRole('button', { name: 'Cancel' }));
- userEvent.click(screen.getByLabelText('Menu actions trigger'));
- userEvent.click(screen.getByText('Edit chart properties'));
+ // Wait for the modal to close
+ await waitFor(() => {
+ expect(
+ screen.queryByRole('textbox', { name: 'Name' }),
+ ).not.toBeInTheDocument();
+ });
- expect(await screen.findByDisplayValue(prevChartName)).toBeInTheDocument();
+ await userEvent.click(screen.getByLabelText('Menu actions trigger'));
+ await userEvent.click(screen.getByText('Edit chart properties'));
+
+ // Wait for the modal to reopen and verify the name was reset
+ const reopenedNameInput = await screen.findByRole('textbox', {
+ name: 'Name',
+ });
+ expect(reopenedNameInput).toHaveValue(prevChartName ?? '');
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Test expectation mismatch</b></div>
<div id="fix">
The test expects the chart name to reset to the original value after
canceling edits and reopening the modal. However, the PropertiesModal persists
form state across opens since it lacks a reset mechanism on show. This will
cause the test to fail. Consider adding a useEffect in PropertiesModal to reset
state when show is true, or adjust the test to match current behavior.
</div>
</div>
<small><i>Code Review Run #6c4885</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]
