bito-code-review[bot] commented on code in PR #40778:
URL: https://github.com/apache/superset/pull/40778#discussion_r3365966233
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/NativeFiltersModal.test.tsx:
##########
@@ -110,4 +110,22 @@ describe('createNewOnOpen', () => {
expect(getByRole('alert')).toBeInTheDocument();
expect(getByRole('alert')).toHaveTextContent('There are unsaved changes.');
});
+
+ test('confirm-cancel button proceeds with cancel after the unsaved alert',
async () => {
+ const onCancel = jest.fn();
+ const { getByRole, getByTestId, findByRole } = setup({
+ onCancel,
+ createNewOnOpen: false,
+ });
+ const dropdownButton = getByTestId('new-item-dropdown-button');
+ fireEvent.mouseEnter(dropdownButton);
+ const addFilterMenuItem = await findByRole('menuitem', {
+ name: /add filter/i,
+ });
+ fireEvent.click(addFilterMenuItem);
+ fireEvent.click(getByRole('button', { name: 'Cancel' }));
+ // Alert is shown; user clicks the confirm-cancel button to discard
changes.
+ fireEvent.click(getByTestId('native-filter-modal-confirm-cancel-button'));
+ expect(onCancel).toHaveBeenCalledTimes(1);
+ });
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Duplicated test setup code</b></div>
<div id="fix">
The new test duplicates 7 lines of setup code (lines 120-126) identical to
lines 102-108 in the preceding test. Extract shared setup logic (dropdown
interaction, add filter flow) into a beforeEach hook or helper to reduce
maintenance burden and divergence risk.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
---
a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/NativeFiltersModal.test.tsx
+++
b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/NativeFiltersModal.test.tsx
@@ -85,6 +85,19 @@ test('the form validates required fields', async () => {
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
describe('createNewOnOpen', () => {
+
+ // Helper to add a filter via dropdown menu
+ async function addFilterViaDropdown(getByTestId: any, findByRole: any) {
+ const dropdownButton = getByTestId('new-item-dropdown-button');
+ fireEvent.mouseEnter(dropdownButton);
+ const addFilterMenuItem = await findByRole('menuitem', {
+ name: /add filter/i,
+ });
+ fireEvent.click(addFilterMenuItem);
+ }
+
test('does not show alert when there is no unsaved filters', async () =>
{
const onCancel = jest.fn();
const { getByRole } = setup({ onCancel, createNewOnOpen: false });
@@ -96,24 +109,14 @@ describe('createNewOnOpen', () => {
test('shows correct alert message for unsaved filters', async () => {
const onCancel = jest.fn();
const { getByRole, getByTestId, findByRole } = setup({
onCancel,
createNewOnOpen: false,
});
- const dropdownButton = getByTestId('new-item-dropdown-button');
- fireEvent.mouseEnter(dropdownButton);
- const addFilterMenuItem = await findByRole('menuitem', {
- name: /add filter/i,
- });
- fireEvent.click(addFilterMenuItem);
+ await addFilterViaDropdown(getByTestId, findByRole);
fireEvent.click(getByRole('button', { name: 'Cancel' }));
expect(onCancel).toHaveBeenCalledTimes(0);
expect(getByRole('alert')).toBeInTheDocument();
expect(getByRole('alert')).toHaveTextContent('There are unsaved
changes.');
});
- test('confirm-cancel button proceeds with cancel after the unsaved
alert', async () => {
+ test('confirm-cancel button proceeds with cancel after the unsaved
alert', async (): Promise<void> => {
const onCancel = jest.fn();
const { getByRole, getByTestId, findByRole } = setup({
onCancel,
createNewOnOpen: false,
});
- const dropdownButton = getByTestId('new-item-dropdown-button');
- fireEvent.mouseEnter(dropdownButton);
- const addFilterMenuItem = await findByRole('menuitem', {
- name: /add filter/i,
- });
- fireEvent.click(addFilterMenuItem);
+ await addFilterViaDropdown(getByTestId, findByRole);
fireEvent.click(getByRole('button', { name: 'Cancel' }));
// Alert is shown; user clicks the confirm-cancel button to discard
changes.
fireEvent.click(getByTestId('native-filter-modal-confirm-cancel-button'));
expect(onCancel).toHaveBeenCalledTimes(1);
});
});
```
</div>
</details>
</div>
<small><i>Code Review Run #43b3ac</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]
