test: add new RTL and integration tests for DatasetList (#36681)

Co-authored-by: Claude <noreply@anthropic.com>

superset-frontend/src/features/datasets/DuplicateDatasetModal.test.tsx

@@ -0,0 +1,282 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { render, screen, waitFor } from 'spec/helpers/testing-library';
+import userEvent from '@testing-library/user-event';
+import { ThemeProvider, supersetTheme } from '@apache-superset/core';
+import DuplicateDatasetModal from './DuplicateDatasetModal';
+
+// Test-only fixture type that includes all fields from API responses
+// Matches VirtualDataset structure from DatasetList but defined locally for tests
+interface VirtualDatasetFixture {
+  id: number;
+  table_name: string;
+  kind: string;
+  schema: string;
+  database: {
+    id: string;
+    database_name: string;
+  };
+  owners: Array<{ first_name: string; last_name: string; id: number }>;
+  changed_by_name: string;
+  changed_by: string;
+  changed_on_delta_humanized: string;
+  explore_url: string;
+  extra: string;
+  sql: string | null;
+}
+
+// Test fixture with extra/sql fields that exist in actual API responses
+const mockDataset: VirtualDatasetFixture = {
+  id: 1,
+  table_name: 'original_dataset',
+  kind: 'virtual',
+  schema: 'public',
+  database: {
+    id: '1',
+    database_name: 'PostgreSQL',
+  },
+  owners: [],
+  changed_by_name: 'Admin',
+  changed_by: 'Admin User',
+  changed_on_delta_humanized: '1 day ago',
+  explore_url: '/explore/?datasource=1__table',
+  extra: '{}',
+  sql: 'SELECT * FROM table',
+};
+
+const Wrapper = ({
+  dataset,
+  onHide,
+  onDuplicate,
+}: {
+  dataset: VirtualDatasetFixture | null;
+  onHide: jest.Mock;
+  onDuplicate: jest.Mock;
+}) => (
+  <ThemeProvider theme={supersetTheme}>
+    <DuplicateDatasetModal
+      dataset={dataset}
+      onHide={onHide}
+      onDuplicate={onDuplicate}
+    />
+  </ThemeProvider>
+);
+
+const renderModal = (
+  dataset: VirtualDatasetFixture | null,
+  onHide: jest.Mock,
+  onDuplicate: jest.Mock,
+) =>
+  render(
+    <Wrapper dataset={dataset} onHide={onHide} onDuplicate={onDuplicate} />,
+  );
+
+test('modal opens when dataset is provided', async () => {
+  const onHide = jest.fn();
+  const onDuplicate = jest.fn();
+
+  renderModal(mockDataset, onHide, onDuplicate);
+
+  // Modal should be visible
+  expect(await screen.findByText('Duplicate dataset')).toBeInTheDocument();
+
+  // Input field should be present
+  expect(screen.getByTestId('duplicate-modal-input')).toBeInTheDocument();
+
+  // Duplicate button should be present
+  expect(
+    screen.getByRole('button', { name: /duplicate/i }),
+  ).toBeInTheDocument();
+});
+
+test('modal does not open when dataset is null', () => {
+  const onHide = jest.fn();
+  const onDuplicate = jest.fn();
+
+  renderModal(null, onHide, onDuplicate);
+
+  // Modal should not be visible
+  expect(screen.queryByText('Duplicate dataset')).not.toBeInTheDocument();
+});
+
+test('duplicate button disabled after clearing input', async () => {
+  const onHide = jest.fn();
+  const onDuplicate = jest.fn();
+
+  renderModal(mockDataset, onHide, onDuplicate);
+
+  const input = await screen.findByTestId('duplicate-modal-input');
+
+  // Type some text first
+  await userEvent.type(input, 'test');
+
+  // Then clear it
+  await userEvent.clear(input);
+
+  // Duplicate button should now be disabled (empty input)
+  const duplicateButton = screen.getByRole('button', { name: /duplicate/i });
+  expect(duplicateButton).toBeDisabled();
+});
+
+test('duplicate button enabled when name is entered', async () => {
+  const onHide = jest.fn();
+  const onDuplicate = jest.fn();
+
+  renderModal(mockDataset, onHide, onDuplicate);
+
+  const input = await screen.findByTestId('duplicate-modal-input');
+
+  // Type a new name
+  await userEvent.type(input, 'new_dataset_copy');
+
+  // Duplicate button should now be enabled
+  const duplicateButton = await screen.findByRole('button', {
+    name: /duplicate/i,
+  });
+  expect(duplicateButton).toBeEnabled();
+});
+
+test('clicking Duplicate calls onDuplicate with new name', async () => {
+  const onHide = jest.fn();
+  const onDuplicate = jest.fn();
+
+  renderModal(mockDataset, onHide, onDuplicate);
+
+  const input = await screen.findByTestId('duplicate-modal-input');
+
+  // Type a new name
+  await userEvent.type(input, 'new_dataset_copy');
+
+  // Click Duplicate button
+  const duplicateButton = await screen.findByRole('button', {
+    name: /duplicate/i,
+  });
+  await userEvent.click(duplicateButton);
+
+  // onDuplicate should be called with the new name
+  await waitFor(() => {
+    expect(onDuplicate).toHaveBeenCalledWith('new_dataset_copy');
+  });
+});
+
+test('pressing Enter key triggers duplicate action', async () => {
+  const onHide = jest.fn();
+  const onDuplicate = jest.fn();
+
+  renderModal(mockDataset, onHide, onDuplicate);
+
+  const input = await screen.findByTestId('duplicate-modal-input');
+
+  // Clear any existing value and type new name with Enter at end
+  await userEvent.clear(input);
+  await userEvent.type(input, 'new_dataset_copy{enter}');
+
+  // onDuplicate should be called by onPressEnter handler
+  await waitFor(() => {
+    expect(onDuplicate).toHaveBeenCalledWith('new_dataset_copy');
+  });
+});
+
+test('modal closes when onHide is called', async () => {
+  const onHide = jest.fn();
+  const onDuplicate = jest.fn();
+
+  const { rerender } = renderModal(mockDataset, onHide, onDuplicate);
+
+  expect(await screen.findByText('Duplicate dataset')).toBeInTheDocument();
+
+  // Simulate closing the modal by setting dataset to null
+  rerender(
+    <Wrapper dataset={null} onHide={onHide} onDuplicate={onDuplicate} />,
+  );
+
+  // Modal should no longer be visible (Ant Design keeps it in DOM but hides it)
+  await waitFor(() => {
+    expect(screen.queryByText('Duplicate dataset')).not.toBeVisible();
+  });
+});
+
+test('cancel button clears input and closes modal', async () => {
+  const onHide = jest.fn();
+  const onDuplicate = jest.fn();
+
+  const { rerender } = renderModal(mockDataset, onHide, onDuplicate);
+
+  const input = await screen.findByTestId('duplicate-modal-input');
+
+  // Type some text
+  await userEvent.type(input, 'test_name');
+
+  expect(input).toHaveValue('test_name');
+
+  // Click cancel button
+  const cancelButton = await screen.findByRole('button', { name: /cancel/i });
+  await userEvent.click(cancelButton);
+
+  // onHide should be called
+  expect(onHide).toHaveBeenCalled();
+
+  // Simulate closing the modal (parent sets dataset to null)
+  rerender(
+    <Wrapper dataset={null} onHide={onHide} onDuplicate={onDuplicate} />,
+  );
+
+  // Modal should be hidden
+  await waitFor(() => {
+    expect(screen.queryByText('Duplicate dataset')).not.toBeVisible();
+  });
+
+  // Reopen with same dataset - input should be cleared
+  rerender(
+    <Wrapper dataset={mockDataset} onHide={onHide} onDuplicate={onDuplicate} />,
+  );
+
+  const reopenedInput = await screen.findByTestId('duplicate-modal-input');
+  expect(reopenedInput).toHaveValue('');
+});
+
+test('input field clears when new dataset is provided', async () => {
+  const onHide = jest.fn();
+  const onDuplicate = jest.fn();
+
+  const { rerender } = renderModal(mockDataset, onHide, onDuplicate);
+
+  const input = await screen.findByTestId('duplicate-modal-input');
+
+  // Type a name
+  await userEvent.type(input, 'old_name');
+
+  expect(input).toHaveValue('old_name');
+
+  // Switch to different dataset
+  const newDataset: VirtualDatasetFixture = {
+    ...mockDataset,
+    id: 2,
+    table_name: 'different_dataset',
+  };
+
+  rerender(
+    <Wrapper dataset={newDataset} onHide={onHide} onDuplicate={onDuplicate} />,
+  );
+
+  // Input should be cleared
+  await waitFor(() => {
+    expect(input).toHaveValue('');
+  });
+});

superset-frontend/src/features/datasets/hooks/useDatasetLists.test.ts

@@ -17,7 +17,8 @@
  * under the License.
  */
 import { renderHook } from '@testing-library/react-hooks';
-import { SupersetClient } from '@superset-ui/core';
+import { waitFor } from '@testing-library/dom';
+import { SupersetClient, JsonResponse } from '@superset-ui/core';
 import rison from 'rison';
 import useDatasetsList from './useDatasetLists';
 
@@ -52,15 +53,14 @@ test('useDatasetsList fetches first page of datasets successfully', async () =>
       count: 2,
       result: mockDatasets,
     },
-  } as any);
+  } as unknown as JsonResponse);
 
-  const { result, waitForNextUpdate } = renderHook(() =>
-    useDatasetsList(mockDb, 'public'),
-  );
+  const { result } = renderHook(() => useDatasetsList(mockDb, 'public'));
 
-  await waitForNextUpdate();
+  await waitFor(() => {
+    expect(result.current.datasets).toEqual(mockDatasets);
+  });
 
-  expect(result.current.datasets).toEqual(mockDatasets);
   expect(result.current.datasetNames).toEqual(['table1', 'table2']);
   expect(getSpy).toHaveBeenCalledTimes(1);
 });
@@ -79,21 +79,20 @@ test('useDatasetsList fetches multiple pages (pagination) until count reached',
         count: 3,
         result: page1Data,
       },
-    } as any)
+    } as unknown as JsonResponse)
     .mockResolvedValueOnce({
       json: {
         count: 3,
         result: page2Data,
       },
-    } as any);
+    } as unknown as JsonResponse);
 
-  const { result, waitForNextUpdate } = renderHook(() =>
-    useDatasetsList(mockDb, 'public'),
-  );
+  const { result } = renderHook(() => useDatasetsList(mockDb, 'public'));
 
-  await waitForNextUpdate();
+  await waitFor(() => {
+    expect(result.current.datasets).toEqual([...page1Data, ...page2Data]);
+  });
 
-  expect(result.current.datasets).toEqual([...page1Data, ...page2Data]);
   expect(result.current.datasetNames).toEqual(['table1', 'table2', 'table3']);
   expect(getSpy).toHaveBeenCalledTimes(2);
 });
@@ -108,15 +107,18 @@ test('useDatasetsList extracts dataset names correctly', async () => {
         { id: 3, table_name: 'products' },
       ],
     },
-  } as any);
+  } as unknown as JsonResponse);
 
-  const { result, waitForNextUpdate } = renderHook(() =>
-    useDatasetsList(mockDb, 'public'),
-  );
+  const { result } = renderHook(() => useDatasetsList(mockDb, 'public'));
 
-  await waitForNextUpdate();
+  await waitFor(() => {
+    expect(result.current.datasetNames).toEqual([
+      'users',
+      'orders',
+      'products',
+    ]);
+  });
 
-  expect(result.current.datasetNames).toEqual(['users', 'orders', 'products']);
   expect(getSpy).toHaveBeenCalledTimes(1);
 });
 
@@ -126,17 +128,16 @@ test('useDatasetsList handles API 500 error gracefully', async () => {
     .spyOn(SupersetClient, 'get')
     .mockRejectedValue(new Error('Internal Server Error'));
 
-  const { result, waitForNextUpdate } = renderHook(() =>
-    useDatasetsList(mockDb, 'public'),
-  );
+  const { result } = renderHook(() => useDatasetsList(mockDb, 'public'));
 
-  await waitForNextUpdate();
+  await waitFor(() => {
+    expect(mockAddDangerToast).toHaveBeenCalledWith(
+      'There was an error fetching dataset',
+    );
+  });
 
   expect(result.current.datasets).toEqual([]);
   expect(result.current.datasetNames).toEqual([]);
-  expect(mockAddDangerToast).toHaveBeenCalledWith(
-    'There was an error fetching dataset',
-  );
   // Should only be called once - error causes break
   expect(getSpy).toHaveBeenCalledTimes(1);
 });
@@ -147,17 +148,16 @@ test('useDatasetsList handles empty dataset response', async () => {
       count: 0,
       result: [],
     },
-  } as any);
+  } as unknown as JsonResponse);
 
-  const { result, waitForNextUpdate } = renderHook(() =>
-    useDatasetsList(mockDb, 'public'),
-  );
+  const { result } = renderHook(() => useDatasetsList(mockDb, 'public'));
 
-  await waitForNextUpdate();
+  await waitFor(() => {
+    expect(getSpy).toHaveBeenCalledTimes(1);
+  });
 
   expect(result.current.datasets).toEqual([]);
   expect(result.current.datasetNames).toEqual([]);
-  expect(getSpy).toHaveBeenCalledTimes(1);
 });
 
 test('useDatasetsList stops pagination when results reach count', async () => {
@@ -172,21 +172,20 @@ test('useDatasetsList stops pagination when results reach count', async () => {
           { id: 2, table_name: 'table2' },
         ],
       },
-    } as any)
+    } as unknown as JsonResponse)
     .mockResolvedValueOnce({
       json: {
         count: 2,
         result: [], // No more results
       },
-    } as any);
+    } as unknown as JsonResponse);
 
-  const { result, waitForNextUpdate } = renderHook(() =>
-    useDatasetsList(mockDb, 'public'),
-  );
+  const { result } = renderHook(() => useDatasetsList(mockDb, 'public'));
 
-  await waitForNextUpdate();
+  await waitFor(() => {
+    expect(result.current.datasets).toHaveLength(2);
+  });
 
-  expect(result.current.datasets).toHaveLength(2);
   expect(result.current.datasetNames).toEqual(['table1', 'table2']);
   // Should stop after results.length >= count
   expect(getSpy).toHaveBeenCalledTimes(1);
@@ -203,58 +202,60 @@ test('useDatasetsList resets datasets when schema changes', async () => {
           { id: 2, table_name: 'public_table2' },
         ],
       },
-    } as any)
+    } as unknown as JsonResponse)
     .mockResolvedValueOnce({
       json: {
         count: 1,
         result: [{ id: 3, table_name: 'private_table1' }],
       },
-    } as any);
+    } as unknown as JsonResponse);
 
-  const { result, waitForNextUpdate, rerender } = renderHook(
+  const { result, rerender } = renderHook(
     ({ db, schema }) => useDatasetsList(db, schema),
     {
       initialProps: { db: mockDb, schema: 'public' },
     },
   );
 
-  await waitForNextUpdate();
-
-  expect(result.current.datasetNames).toEqual([
-    'public_table1',
-    'public_table2',
-  ]);
+  await waitFor(() => {
+    expect(result.current.datasetNames).toEqual([
+      'public_table1',
+      'public_table2',
+    ]);
+  });
 
   // Change schema
   rerender({ db: mockDb, schema: 'private' });
 
-  await waitForNextUpdate();
+  await waitFor(() => {
+    // Should have new datasets from private schema
+    expect(result.current.datasetNames).toEqual(['private_table1']);
+  });
 
-  // Should have new datasets from private schema
-  expect(result.current.datasetNames).toEqual(['private_table1']);
   expect(getSpy).toHaveBeenCalledTimes(2);
 });
 
 test('useDatasetsList handles network timeout gracefully', async () => {
   // Mock timeout/abort error (status: 0)
-  const timeoutError = new Error('Network timeout');
-  (timeoutError as any).status = 0;
+  const timeoutError = new Error('Network timeout') as Error & {
+    status: number;
+  };
+  timeoutError.status = 0;
 
   const getSpy = jest
     .spyOn(SupersetClient, 'get')
     .mockRejectedValue(timeoutError);
 
-  const { result, waitForNextUpdate } = renderHook(() =>
-    useDatasetsList(mockDb, 'public'),
-  );
+  const { result } = renderHook(() => useDatasetsList(mockDb, 'public'));
 
-  await waitForNextUpdate();
+  await waitFor(() => {
+    expect(mockAddDangerToast).toHaveBeenCalledWith(
+      'There was an error fetching dataset',
+    );
+  });
 
   expect(result.current.datasets).toEqual([]);
   expect(result.current.datasetNames).toEqual([]);
-  expect(mockAddDangerToast).toHaveBeenCalledWith(
-    'There was an error fetching dataset',
-  );
   // Should only be called once - error causes break
   expect(getSpy).toHaveBeenCalledTimes(1);
 });
@@ -265,19 +266,18 @@ test('useDatasetsList breaks pagination loop on persistent API errors', async ()
     .spyOn(SupersetClient, 'get')
     .mockRejectedValue(new Error('Persistent server error'));
 
-  const { result, waitForNextUpdate } = renderHook(() =>
-    useDatasetsList(mockDb, 'public'),
-  );
+  const { result } = renderHook(() => useDatasetsList(mockDb, 'public'));
 
-  await waitForNextUpdate();
+  await waitFor(() => {
+    expect(mockAddDangerToast).toHaveBeenCalledWith(
+      'There was an error fetching dataset',
+    );
+  });
 
   // Should only attempt once, then break (not infinite loop)
   expect(getSpy).toHaveBeenCalledTimes(1);
   expect(result.current.datasets).toEqual([]);
   expect(result.current.datasetNames).toEqual([]);
-  expect(mockAddDangerToast).toHaveBeenCalledWith(
-    'There was an error fetching dataset',
-  );
   expect(mockAddDangerToast).toHaveBeenCalledTimes(1);
 });
 
@@ -290,23 +290,22 @@ test('useDatasetsList handles error on second page gracefully', async () => {
         count: 3, // Indicates more data exists
         result: [{ id: 1, table_name: 'table1' }],
       },
-    } as any)
+    } as unknown as JsonResponse)
     .mockRejectedValue(new Error('Second page error'));
 
-  const { result, waitForNextUpdate } = renderHook(() =>
-    useDatasetsList(mockDb, 'public'),
-  );
+  const { result } = renderHook(() => useDatasetsList(mockDb, 'public'));
 
-  await waitForNextUpdate();
+  await waitFor(() => {
+    expect(mockAddDangerToast).toHaveBeenCalledWith(
+      'There was an error fetching dataset',
+    );
+  });
 
   // Should have first page data, then stop on error
   expect(getSpy).toHaveBeenCalledTimes(2);
   expect(result.current.datasets).toHaveLength(1);
   expect(result.current.datasets[0].table_name).toBe('table1');
   expect(result.current.datasetNames).toEqual(['table1']);
-  expect(mockAddDangerToast).toHaveBeenCalledWith(
-    'There was an error fetching dataset',
-  );
   expect(mockAddDangerToast).toHaveBeenCalledTimes(1);
 });
 
@@ -316,7 +315,7 @@ test('useDatasetsList skips fetching when schema is null or undefined', () => {
   // Test with null schema
   const { result: resultNull, rerender } = renderHook(
     ({ db, schema }) => useDatasetsList(db, schema),
-    { initialProps: { db: mockDb, schema: null as any } },
+    { initialProps: { db: mockDb, schema: null as unknown as string } },
   );
 
   // Schema is null - should NOT call API
@@ -325,7 +324,7 @@ test('useDatasetsList skips fetching when schema is null or undefined', () => {
   expect(resultNull.current.datasetNames).toEqual([]);
 
   // Change to undefined - still should NOT call API
-  rerender({ db: mockDb, schema: undefined as any });
+  rerender({ db: mockDb, schema: undefined as unknown as string });
   expect(getSpy).not.toHaveBeenCalled();
   expect(resultNull.current.datasets).toEqual([]);
   expect(resultNull.current.datasetNames).toEqual([]);
@@ -349,7 +348,7 @@ test('useDatasetsList skips fetching when db.id is undefined', () => {
   const dbWithoutId = {
     database_name: 'test_db',
     owners: [1] as [number],
-  } as any;
+  } as typeof mockDb;
 
   const { result } = renderHook(() => useDatasetsList(dbWithoutId, 'public'));
 
@@ -362,16 +361,15 @@ test('useDatasetsList skips fetching when db.id is undefined', () => {
 test('useDatasetsList encodes schemas with spaces and special characters in endpoint URL', async () => {
   const getSpy = jest.spyOn(SupersetClient, 'get').mockResolvedValue({
     json: { count: 0, result: [] },
-  } as any);
+  } as unknown as JsonResponse);
 
-  const { waitForNextUpdate } = renderHook(() =>
-    useDatasetsList(mockDb, 'sales analytics'),
-  );
+  renderHook(() => useDatasetsList(mockDb, 'sales analytics'));
 
-  await waitForNextUpdate();
+  await waitFor(() => {
+    expect(getSpy).toHaveBeenCalledTimes(1);
+  });
 
   // Verify API was called with encoded schema
-  expect(getSpy).toHaveBeenCalledTimes(1);
   const callArg = getSpy.mock.calls[0]?.[0]?.endpoint;
   expect(callArg).toBeDefined();
 
@@ -379,9 +377,15 @@ test('useDatasetsList encodes schemas with spaces and special characters in endp
   // Schema 'sales analytics' -> encodeURIComponent -> 'sales%20analytics' -> rison.encode_uri -> 'sales%2520analytics'
   expect(callArg).toContain('sales%2520analytics');
 
-  // Decode rison to verify filter structure
-  const risonParam = callArg!.split('?q=')[1];
-  const decoded = rison.decode(decodeURIComponent(risonParam)) as any;
+  // Decode rison to verify filter structure using URL parser
+  // searchParams.get() already URL-decodes, so pass directly to rison.decode
+  const risonParam = new URL(callArg!, 'http://localhost').searchParams.get(
+    'q',
+  );
+  expect(risonParam).toBeTruthy();
+  const decoded = rison.decode(risonParam!) as {
+    filters: Array<{ col: string; opr: string; value: string }>;
+  };
 
   // After rison decoding, the schema should be the encoded version (encodeURIComponent output)
   expect(decoded.filters[1]).toEqual({