[ui-importer] add table name validation (#4194)

Author: ramprasadagarwal

desktop/core/src/desktop/js/apps/newimporter/ImporterFilePreview/DestinationSettings/DestinationSettings.scss

@@ -19,13 +19,18 @@
 .antd.cuix {
   .importer-destination-settings {
     display: flex;
+    flex-direction: column;
     gap: vars.$cdl-spacing-s;
 
     .ant-form-item {
       margin-bottom: 0;
     }
 
-    &__select-dropdown,
+    &__form-container {
+      display: flex;
+      gap: vars.$cdl-spacing-s;
+    }
+
     &__input {
       border: 1px solid vars.$fluidx-gray-600;
       border-radius: vars.$border-radius-base;

desktop/core/src/desktop/js/apps/newimporter/ImporterFilePreview/DestinationSettings/DestinationSettings.test.tsx

@@ -22,7 +22,12 @@ import { DestinationConfig } from '../../types';
 import { useDataCatalog } from '../../../../utils/hooks/useDataCatalog/useDataCatalog';
 
 const mockUseDataCatalog = {
-  loading: false,
+  loading: {
+    connector: false,
+    compute: false,
+    database: false,
+    table: false
+  },
   databases: ['database1', 'database2'],
   database: 'database1',
   connectors: [
@@ -35,6 +40,7 @@ const mockUseDataCatalog = {
     { id: 'compute2', name: 'Compute 2' }
   ],
   compute: { id: 'compute1', name: 'Compute 1' },
+  tables: [],
   setCompute: jest.fn(),
   setConnector: jest.fn(),
   setDatabase: jest.fn()
@@ -79,20 +85,26 @@ describe('DestinationSettings Component', () => {
     expect(screen.getByDisplayValue('test_table')).toBeInTheDocument();
   });
 
-  it('should hide compute field when only one compute is available', () => {
+  it('should not have compute field when only one compute is available', async () => {
     const mockUseDataCatalogSingleCompute = {
       ...mockUseDataCatalog,
-      computes: [{ id: 'compute1', name: 'Compute 1' }]
+      computes: [{ id: 'compute1', name: 'Compute 1' }],
+      compute: { id: 'compute1', name: 'Compute 1' }
     };
 
-    (useDataCatalog as jest.Mock).mockReturnValueOnce(mockUseDataCatalogSingleCompute);
+    (useDataCatalog as jest.Mock).mockReturnValue(mockUseDataCatalogSingleCompute);
 
     render(<DestinationSettings {...defaultProps} />);
 
-    expect(screen.queryByLabelText('Compute')).not.toBeInTheDocument();
+    await waitFor(() => {
+      expect(screen.getByLabelText('Engine')).toBeInTheDocument();
+      expect(screen.getByLabelText('Database')).toBeInTheDocument();
+      expect(screen.getByLabelText('Table Name')).toBeInTheDocument();
+      expect(screen.queryByLabelText('Compute')).not.toBeInTheDocument();
+    });
   });
 
-  it('should show compute field when multiple computes are available', () => {
+  it('should show compute field when multiple computes are available', async () => {
     render(<DestinationSettings {...defaultProps} />);
 
     expect(screen.getByLabelText('Compute')).toBeInTheDocument();
@@ -149,54 +161,20 @@ describe('DestinationSettings Component', () => {
       id: 'compute2',
       name: 'Compute 2'
     });
-    expect(defaultProps.onChange).toHaveBeenCalledWith('compute', 'compute2');
+    expect(defaultProps.onChange).toHaveBeenCalledWith('computeId', 'compute2');
   });
 
-  it('should update table name input and not call onChange immediately', () => {
+  it('should update table name input and call onChange when input changes', async () => {
     render(<DestinationSettings {...defaultProps} />);
 
     const tableNameInput = screen.getByLabelText('Table Name');
     fireEvent.change(tableNameInput, { target: { value: 'new_table_name' } });
 
     expect(screen.getByDisplayValue('new_table_name')).toBeInTheDocument();
-    expect(defaultProps.onChange).not.toHaveBeenCalled();
+    expect(defaultProps.onChange).toHaveBeenCalledWith('tableName', 'new_table_name');
   });
 
-  it('should show loader in select dropdown when loading state is true', () => {
-    const mockUseDataCatalogLoading = {
-      ...mockUseDataCatalog,
-      loading: true
-    };
-
-    (useDataCatalog as jest.Mock).mockReturnValueOnce(mockUseDataCatalogLoading);
-
-    render(<DestinationSettings {...defaultProps} />);
-
-    const selectDropdowns = document.querySelectorAll('.ant-select');
-    selectDropdowns.forEach(select => {
-      expect(select).toBeInTheDocument();
-      expect(select).toHaveClass('ant-select-loading');
-    });
-  });
-
-  it('should not show loader in select dropdown when loading state is false', () => {
-    const mockUseDataCatalogLoading = {
-      ...mockUseDataCatalog,
-      loading: false
-    };
-
-    (useDataCatalog as jest.Mock).mockReturnValueOnce(mockUseDataCatalogLoading);
-
-    render(<DestinationSettings {...defaultProps} />);
-
-    const selectDropdowns = document.querySelectorAll('.ant-select');
-    selectDropdowns.forEach(select => {
-      expect(select).toBeInTheDocument();
-      expect(select).not.toHaveClass('ant-select-loading');
-    });
-  });
-
-  it('should set default values from props on component mount', () => {
+  it('should set default values from props on component mount', async () => {
     const mockSetConnector = jest.fn();
     const mockSetDatabase = jest.fn();
     const mockSetCompute = jest.fn();
@@ -223,7 +201,7 @@ describe('DestinationSettings Component', () => {
     });
   });
 
-  it('should not call setters when no matching items found in defaultValues', () => {
+  it('should not call setters when no matching items found in defaultValues', async () => {
     const mockSetConnector = jest.fn();
     const mockSetDatabase = jest.fn();
     const mockSetCompute = jest.fn();
@@ -246,4 +224,49 @@ describe('DestinationSettings Component', () => {
     expect(mockSetDatabase).not.toHaveBeenCalled();
     expect(mockSetCompute).not.toHaveBeenCalled();
   });
+
+  it('should show alert when table name already exists', async () => {
+    const mockUseDataCatalogWithExistingTables = {
+      ...mockUseDataCatalog,
+      tables: [{ name: 'existing_table' }, { name: 'another_table' }]
+    };
+
+    (useDataCatalog as jest.Mock).mockReturnValue(mockUseDataCatalogWithExistingTables);
+
+    render(<DestinationSettings {...defaultProps} />);
+
+    const tableNameInput = screen.getByLabelText('Table Name');
+
+    fireEvent.change(tableNameInput, { target: { value: 'existing_table' } });
+
+    await waitFor(() => {
+      expect(screen.getByRole('alert')).toBeInTheDocument();
+      expect(screen.getByText('Table name already exists in the database')).toBeInTheDocument();
+    });
+  });
+
+  it('should hide alert when table name is unique', async () => {
+    const mockUseDataCatalogWithExistingTables = {
+      ...mockUseDataCatalog,
+      tables: [{ name: 'existing_table' }, { name: 'another_table' }]
+    };
+
+    (useDataCatalog as jest.Mock).mockReturnValue(mockUseDataCatalogWithExistingTables);
+
+    render(<DestinationSettings {...defaultProps} />);
+
+    const tableNameInput = screen.getByLabelText('Table Name');
+
+    fireEvent.change(tableNameInput, { target: { value: 'existing_table' } });
+
+    await waitFor(() => {
+      expect(screen.getByRole('alert')).toBeInTheDocument();
+    });
+
+    fireEvent.change(tableNameInput, { target: { value: 'unique_table_name' } });
+
+    await waitFor(() => {
+      expect(screen.queryByRole('alert')).not.toBeInTheDocument();
+    });
+  });
 });

desktop/core/src/desktop/js/apps/newimporter/ImporterFilePreview/DestinationSettings/DestinationSettings.tsx

@@ -15,12 +15,13 @@
 // limitations under the License.
 
 import React, { useEffect } from 'react';
-import { Form, Input, Select } from 'antd';
+import { Alert, Form } from 'antd';
 import { i18nReact } from '../../../../utils/i18nReact';
 import { useDataCatalog } from '../../../../utils/hooks/useDataCatalog/useDataCatalog';
 import { DestinationConfig } from '../../types';
 
 import './DestinationSettings.scss';
+import FormInput, { FieldType } from '../../../../reactComponents/FormInput/FormInput';
 
 interface DestinationSettingsProps {
   defaultValues: DestinationConfig;
@@ -32,6 +33,9 @@ const DestinationSettings = ({
   onChange
 }: DestinationSettingsProps): JSX.Element => {
   const { t } = i18nReact.useTranslation();
+  const [error, setError] = React.useState<{
+    tableName?: string;
+  }>({});
   const [tableName, setTableName] = React.useState<string | undefined>(defaultValues?.tableName);
 
   const {
@@ -42,6 +46,7 @@ const DestinationSettings = ({
     connector,
     computes,
     compute,
+    tables,
     setCompute,
     setConnector,
     setDatabase
@@ -51,16 +56,18 @@ const DestinationSettings = ({
     {
       label: t('Engine'),
       name: 'connectorId',
-      type: 'select',
+      type: FieldType.SELECT,
+      tooltip: t('Select the engine to use for the destination'),
       options: connectors.map(connector => ({
         label: connector.displayName,
         value: connector.id
       }))
     },
     {
       label: t('Compute'),
-      name: 'compute',
-      type: 'select',
+      name: 'computeId',
+      type: FieldType.SELECT,
+      tooltip: t('Select the compute to use for the destination'),
       options:
         computes?.map(compute => ({
           label: compute.name,
@@ -71,7 +78,8 @@ const DestinationSettings = ({
     {
       label: t('Database'),
       name: 'database',
-      type: 'select',
+      type: FieldType.SELECT,
+      tooltip: t('Select the database to use for the destination'),
       options:
         databases?.map(database => ({
           label: database,
@@ -81,7 +89,8 @@ const DestinationSettings = ({
     {
       label: t('Table Name'),
       name: 'tableName',
-      type: 'input'
+      type: FieldType.INPUT,
+      tooltip: t('Enter the name of the table to use for the destination')
     }
   ].filter(({ hidden }) => !hidden);
 
@@ -91,23 +100,27 @@ const DestinationSettings = ({
       if (selectedConnector) {
         setConnector(selectedConnector);
       }
-    } else if (name === 'database') {
-      const selectedDatabase = databases?.find(database => database === value);
-      if (selectedDatabase) {
-        setDatabase(selectedDatabase);
-      }
-    } else if (name === 'compute') {
+    } else if (name === 'computeId') {
       const selectedCompute = computes?.find(compute => compute.id === value);
       if (selectedCompute) {
         setCompute(selectedCompute);
       }
+    } else if (name === 'database') {
+      setDatabase(value);
+    } else if (name === 'tableName') {
+      setTableName(value);
     }
 
     onChange(name, value);
   };
 
-  const handleTableChange = (value: string) => {
-    setTableName(value);
+  const validateTableName = (name: string) => {
+    const tableExists = tables.some(table => table.name.toLowerCase() === name.toLowerCase());
+    if (tableExists) {
+      setError(prev => ({ ...prev, tableName: t('Table name already exists in the database') }));
+    } else {
+      setError(prev => ({ ...prev, tableName: undefined }));
+    }
   };
 
   useEffect(() => {
@@ -164,49 +177,46 @@ const DestinationSettings = ({
     }
   }, [defaultValues?.tableName]);
 
-  const selectedSettings = {
+  useEffect(() => {
+    if (tableName) {
+      validateTableName(tableName);
+    }
+  }, [tableName, tables]);
+
+  const selectedSettings: DestinationConfig = {
     connectorId: connector?.id,
-    compute: compute?.id,
+    computeId: compute?.id,
     database: database,
     tableName: tableName
   };
 
+  const loadingState = {
+    connectorId: loading.connector,
+    computeId: loading.compute,
+    database: loading.database,
+    tableName: loading.table
+  };
+
   return (
     <div className="importer-destination-settings">
-      {inputConfig.map(({ label, name, type, options }) => {
-        if (type === 'select') {
-          return (
-            <Form layout="vertical" key={name}>
-              <Form.Item key={name} label={label} htmlFor={name}>
-                <Select
-                  getPopupContainer={triggerNode => triggerNode.parentElement}
-                  options={options}
-                  id={name}
-                  loading={loading}
-                  value={selectedSettings[name]}
-                  className="importer-destination-settings__select-dropdown"
-                  onChange={value => handleDropdownChange(name, value)}
-                />
-              </Form.Item>
-            </Form>
-          );
-        }
-        if (type === 'input') {
-          return (
-            <Form layout="vertical" key={name}>
-              <Form.Item key={name} label={label} htmlFor={name}>
-                <Input
-                  id={name}
-                  value={tableName}
-                  className="importer-destination-settings__input"
-                  onChange={e => handleTableChange(e.target.value)}
-                />
-              </Form.Item>
-            </Form>
-          );
-        }
-        return <></>;
-      })}
+      {error && Object.values(error).some(Boolean) && (
+        <Alert message={error.tableName} type="error" showIcon />
+      )}
+      <div className="importer-destination-settings__form-container">
+        {inputConfig.map(field => (
+          <Form key={field.name} layout="vertical">
+            <FormInput<string, DestinationConfig>
+              field={field}
+              defaultValue={selectedSettings[field.name]}
+              value={selectedSettings[field.name]}
+              onChange={handleDropdownChange}
+              className="importer-destination-settings__input"
+              loading={loadingState[field.name]}
+              error={error[field.name]}
+            />
+          </Form>
+        ))}
+      </div>
     </div>
   );
 };