fix(dataset): sync columns checkbox not responding to clicks in virtual dataset modal (#35650)

superset-frontend/src/components/Datasource/DatasourceModal/DatasourceModal.test.jsx

@@ -122,7 +122,7 @@ describe('DatasourceModal', () => {
   });
 
   test('should render error dialog', async () => {
-    jest
+    const putSpy = jest
       .spyOn(SupersetClient, 'put')
       .mockRejectedValue(new Error('Something went wrong'));
     await act(async () => {
@@ -135,5 +135,172 @@ describe('DatasourceModal', () => {
       const errorTitle = await screen.findByText('Error saving dataset');
       expect(errorTitle).toBeInTheDocument();
     });
+    putSpy.mockRestore();
+  });
+
+  test('shows sync columns checkbox when SQL changes', async () => {
+    cleanup();
+    const datasourceWithSQL = {
+      ...mockedProps.datasource,
+      sql: 'SELECT * FROM original_table',
+    };
+    const modifiedDatasource = {
+      ...datasourceWithSQL,
+      sql: 'SELECT * FROM new_table', // Different SQL to trigger checkbox
+    };
+
+    const { rerender } = render(
+      <DatasourceModal {...mockedProps} datasource={datasourceWithSQL} />,
+      { store, useRouter: true },
+    );
+
+    // Update with modified SQL
+    rerender(
+      <DatasourceModal {...mockedProps} datasource={modifiedDatasource} />,
+    );
+
+    const saveButton = screen.getByTestId('datasource-modal-save');
+    fireEvent.click(saveButton);
+
+    // Wait for confirmation modal to appear
+    await waitFor(() => {
+      expect(screen.getByText('Confirm save')).toBeInTheDocument();
+    });
+
+    // Checkbox should be present and checked by default when SQL changes
+    const checkbox = await screen.findByRole('checkbox');
+    expect(checkbox).toBeInTheDocument();
+    expect(checkbox).toBeChecked();
+
+    // Should show the sync columns message
+    expect(screen.getByText('Automatically sync columns')).toBeInTheDocument();
+  });
+
+  test('syncs columns when checkbox is checked and submits with override_columns=true', async () => {
+    const datasourceWithSQL = {
+      ...mockedProps.datasource,
+      sql: 'SELECT * FROM original_table',
+    };
+    const modifiedDatasource = {
+      ...datasourceWithSQL,
+      sql: 'SELECT * FROM new_table',
+    };
+
+    // Render with the initial datasource
+    cleanup();
+    fetchMock.reset();
+    fetchMock.post(SAVE_ENDPOINT, SAVE_PAYLOAD);
+    fetchMock.put(SAVE_DATASOURCE_ENDPOINT, {});
+    fetchMock.get(GET_DATASOURCE_ENDPOINT, { result: {} });
+    fetchMock.get(GET_DATABASE_ENDPOINT, { result: [] });
+
+    const { rerender } = render(
+      <DatasourceModal {...mockedProps} datasource={datasourceWithSQL} />,
+      { store, useRouter: true },
+    );
+
+    // Update with modified SQL to trigger checkbox
+    rerender(
+      <DatasourceModal {...mockedProps} datasource={modifiedDatasource} />,
+    );
+
+    const saveButton = screen.getByTestId('datasource-modal-save');
+    fireEvent.click(saveButton);
+
+    // Wait for confirmation modal to appear
+    await waitFor(() => {
+      expect(screen.getByText('Confirm save')).toBeInTheDocument();
+    });
+
+    // Checkbox should be present and checked by default when SQL changes
+    const checkbox = await screen.findByRole('checkbox');
+    expect(checkbox).toBeChecked();
+
+    // Click OK to submit
+    const okButton = screen.getByRole('button', { name: 'OK' });
+    fireEvent.click(okButton);
+
+    // Verify the PUT request was made with override_columns=true
+    await waitFor(() => {
+      const putCalls = fetchMock
+        .calls()
+        .filter(
+          call =>
+            call[0].includes('/api/v1/dataset/7') &&
+            call[0].includes('override_columns') &&
+            call[1]?.method === 'PUT',
+        );
+      expect(putCalls.length).toBeGreaterThan(0);
+      expect(putCalls[putCalls.length - 1][0]).toContain(
+        'override_columns=true',
+      );
+    });
+  });
+
+  test('does not sync columns when checkbox is unchecked and submits with override_columns=false', async () => {
+    const datasourceWithSQL = {
+      ...mockedProps.datasource,
+      sql: 'SELECT * FROM original_table',
+    };
+    const modifiedDatasource = {
+      ...datasourceWithSQL,
+      sql: 'SELECT * FROM new_table',
+    };
+
+    // Render with the initial datasource
+    cleanup();
+    fetchMock.reset();
+    fetchMock.post(SAVE_ENDPOINT, SAVE_PAYLOAD);
+    fetchMock.put(SAVE_DATASOURCE_ENDPOINT, {});
+    fetchMock.get(GET_DATASOURCE_ENDPOINT, { result: {} });
+    fetchMock.get(GET_DATABASE_ENDPOINT, { result: [] });
+
+    const { rerender } = render(
+      <DatasourceModal {...mockedProps} datasource={datasourceWithSQL} />,
+      { store, useRouter: true },
+    );
+
+    // Update with modified SQL to trigger checkbox
+    rerender(
+      <DatasourceModal {...mockedProps} datasource={modifiedDatasource} />,
+    );
+
+    const saveButton = screen.getByTestId('datasource-modal-save');
+    fireEvent.click(saveButton);
+
+    // Wait for confirmation modal to appear
+    await waitFor(() => {
+      expect(screen.getByText('Confirm save')).toBeInTheDocument();
+    });
+
+    // Checkbox should be present and checked by default when SQL changes
+    const checkbox = await screen.findByRole('checkbox');
+    expect(checkbox).toBeChecked();
+
+    // Uncheck the checkbox
+    fireEvent.click(checkbox);
+
+    // Verify checkbox is now unchecked
+    expect(checkbox).not.toBeChecked();
+
+    // Click OK to submit
+    const okButton = screen.getByRole('button', { name: 'OK' });
+    fireEvent.click(okButton);
+
+    // Verify the PUT request was made with override_columns=false
+    await waitFor(() => {
+      const putCalls = fetchMock
+        .calls()
+        .filter(
+          call =>
+            call[0].includes('/api/v1/dataset/7') &&
+            call[0].includes('override_columns') &&
+            call[1]?.method === 'PUT',
+        );
+      expect(putCalls.length).toBeGreaterThan(0);
+      expect(putCalls[putCalls.length - 1][0]).toContain(
+        'override_columns=false',
+      );
+    });
   });
 });

superset-frontend/src/components/Datasource/DatasourceModal/index.tsx

@@ -108,6 +108,7 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
   const [isSaving, setIsSaving] = useState(false);
   const [isEditing, setIsEditing] = useState<boolean>(false);
   const [modal, contextHolder] = Modal.useModal();
+  const [confirmModalOpen, setConfirmModalOpen] = useState(false);
   const buildPayload = (datasource: Record<string, any>) => {
     const payload: Record<string, any> = {
       table_name: datasource.table_name,
@@ -275,7 +276,7 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
             <Checkbox
               checked={syncColumns}
               onChange={() => {
-                setSyncColumns(!syncColumns);
+                setSyncColumns(prev => !prev);
               }}
             />
             <span
@@ -300,14 +301,17 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
   }, [datasource.sql, currentDatasource.sql]);
 
   const onClickSave = () => {
-    modal.confirm({
-      title: t('Confirm save'),
-      content: getSaveDialog(),
-      onOk: onConfirmSave,
-      icon: null,
-      okText: t('OK'),
-      cancelText: t('Cancel'),
-    });
+    setConfirmModalOpen(true);
+  };
+
+  const handleConfirmModalClose = () => {
+    setConfirmModalOpen(false);
+  };
+
+  const handleConfirmSave = async () => {
+    await onConfirmSave();
+    // Note: on success, onConfirmSave calls onHide() which closes parent modal
+    // On error, confirmModal stays open so user can see the error and try again or cancel
   };
 
   return (
@@ -371,6 +375,16 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
         currencies={currencies}
       />
       {contextHolder}
+      <Modal
+        title={t('Confirm save')}
+        show={confirmModalOpen}
+        onHide={handleConfirmModalClose}
+        onHandledPrimaryAction={handleConfirmSave}
+        primaryButtonName={t('OK')}
+        primaryButtonLoading={isSaving}
+      >
+        {getSaveDialog()}
+      </Modal>
     </StyledDatasourceModal>
   );
 };