refactor: Changes the DatabaseSelector and TableSelector to use the n…

…ew Select component (#16483)
apache · Sep 22, 2021 · 596e1cd · 596e1cd
1 parent 1d5100d
commit 596e1cd
Show file tree

Hide file tree

Showing 18 changed files with 734 additions and 870 deletions.
diff --git a/superset-frontend/spec/javascripts/sqllab/SqlEditorLeftBar_spec.jsx b/superset-frontend/spec/javascripts/sqllab/SqlEditorLeftBar_spec.jsx
@@ -81,9 +81,13 @@ describe('Left Panel Expansion', () => {
         </Provider>
       </ThemeProvider>,
     );
-    const dbSelect = screen.getByText(/select a database/i);
-    const schemaSelect = screen.getByText(/select a schema \(0\)/i);
-    const dropdown = screen.getByText(/Select table/i);
+    const dbSelect = screen.getByRole('combobox', {
+      name: 'Select database or type database name',
+    });
+    const schemaSelect = screen.getByRole('combobox', {
+      name: 'Select schema or type schema name',
+    });
+    const dropdown = screen.getByText(/Select table or type table name/i);
     const abUser = screen.getByText(/ab_user/i);
     expect(dbSelect).toBeInTheDocument();
     expect(schemaSelect).toBeInTheDocument();

diff --git a/superset-frontend/spec/javascripts/sqllab/SqlEditor_spec.jsx b/superset-frontend/spec/javascripts/sqllab/SqlEditor_spec.jsx
@@ -36,13 +36,14 @@ import { Dropdown } from 'src/common/components';
 import {
   queryEditorSetFunctionNames,
   queryEditorSetSelectedText,
+  queryEditorSetSchemaOptions,
 } from 'src/SqlLab/actions/sqlLab';
 import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
 import { initialState, queries, table } from './fixtures';
 
 const MOCKED_SQL_EDITOR_HEIGHT = 500;
 
-fetchMock.get('glob:*/api/v1/database/*', {});
+fetchMock.get('glob:*/api/v1/database/*', { result: [] });
 
 const middlewares = [thunk];
 const mockStore = configureStore(middlewares);
@@ -53,6 +54,7 @@ describe('SqlEditor', () => {
     actions: {
       queryEditorSetFunctionNames,
       queryEditorSetSelectedText,
+      queryEditorSetSchemaOptions,
       addDangerToast: jest.fn(),
     },
     database: {},

diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar.jsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar.jsx
@@ -91,7 +91,13 @@ export default class SqlEditorLeftBar extends React.PureComponent {
   }
 
   onTableChange(tableName, schemaName) {
-    this.props.actions.addTable(this.props.queryEditor, tableName, schemaName);
+    if (tableName && schemaName) {
+      this.props.actions.addTable(
+        this.props.queryEditor,
+        tableName,
+        schemaName,
+      );
+    }
   }
 
   onToggleTable(tables) {
@@ -171,7 +177,6 @@ export default class SqlEditorLeftBar extends React.PureComponent {
           onTablesLoad={this.onTablesLoad}
           schema={qe.schema}
           sqlLabMode
-          tableNameSticky={false}
         />
         <div className="divider" />
         <StyledScrollbarContainer>

diff --git a/superset-frontend/src/components/CertifiedIcon/index.tsx b/superset-frontend/src/components/CertifiedIcon/index.tsx
@@ -18,19 +18,19 @@
  */
 import React from 'react';
 import { t, supersetTheme } from '@superset-ui/core';
-import Icons from 'src/components/Icons';
+import Icons, { IconType } from 'src/components/Icons';
 import { Tooltip } from 'src/components/Tooltip';
 
 export interface CertifiedIconProps {
   certifiedBy?: string;
   details?: string;
-  size?: number;
+  size?: IconType['iconSize'];
 }
 
 function CertifiedIcon({
   certifiedBy,
   details,
-  size = 24,
+  size = 'l',
 }: CertifiedIconProps) {
   return (
     <Tooltip
@@ -48,8 +48,7 @@ function CertifiedIcon({
     >
       <Icons.Certified
         iconColor={supersetTheme.colors.primary.base}
-        height={size}
-        width={size}
+        iconSize={size}
       />
     </Tooltip>
   );

diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx
@@ -26,11 +26,11 @@ import DatabaseSelector from '.';
 const SupersetClientGet = jest.spyOn(SupersetClient, 'get');
 
 const createProps = () => ({
-  dbId: 1,
+  db: { id: 1, database_name: 'test', backend: 'test-postgresql' },
   formMode: false,
   isDatabaseSelectEnabled: true,
   readOnly: false,
-  schema: 'public',
+  schema: undefined,
   sqlLabMode: true,
   getDbList: jest.fn(),
   getTableList: jest.fn(),
@@ -57,9 +57,9 @@ beforeEach(() => {
       }
       return {
         json: {
-          count: 1,
+          count: 2,
           description_columns: {},
-          ids: [1],
+          ids: [1, 2],
           label_columns: {
             allow_csv_upload: 'Allow Csv Upload',
             allow_ctas: 'Allow Ctas',
@@ -129,12 +129,32 @@ beforeEach(() => {
               changed_on: '2021-03-09T19:02:07.141095',
               changed_on_delta_humanized: 'a day ago',
               created_by: null,
-              database_name: 'examples',
+              database_name: 'test-postgres',
               explore_database_id: 1,
               expose_in_sqllab: true,
               force_ctas_schema: null,
               id: 1,
             },
+            {
+              allow_csv_upload: false,
+              allow_ctas: false,
+              allow_cvas: false,
+              allow_dml: false,
+              allow_multi_schema_metadata_fetch: false,
+              allow_run_async: false,
+              allows_cost_estimate: null,
+              allows_subquery: true,
+              allows_virtual_table_explore: true,
+              backend: 'mysql',
+              changed_on: '2021-03-09T19:02:07.141095',
+              changed_on_delta_humanized: 'a day ago',
+              created_by: null,
+              database_name: 'test-mysql',
+              explore_database_id: 1,
+              expose_in_sqllab: true,
+              force_ctas_schema: null,
+              id: 2,
+            },
           ],
         },
       } as any;
@@ -153,50 +173,95 @@ test('Refresh should work', async () => {
 
   render(<DatabaseSelector {...props} />);
 
+  const select = screen.getByRole('combobox', {
+    name: 'Select schema or type schema name',
+  });
+
+  userEvent.click(select);
+
   await waitFor(() => {
-    expect(SupersetClientGet).toBeCalledTimes(2);
-    expect(props.getDbList).toBeCalledTimes(1);
+    expect(SupersetClientGet).toBeCalledTimes(1);
+    expect(props.getDbList).toBeCalledTimes(0);
     expect(props.getTableList).toBeCalledTimes(0);
     expect(props.handleError).toBeCalledTimes(0);
     expect(props.onDbChange).toBeCalledTimes(0);
     expect(props.onSchemaChange).toBeCalledTimes(0);
-    expect(props.onSchemasLoad).toBeCalledTimes(1);
+    expect(props.onSchemasLoad).toBeCalledTimes(0);
     expect(props.onUpdate).toBeCalledTimes(0);
   });
 
-  userEvent.click(screen.getByRole('button'));
+  userEvent.click(screen.getByRole('button', { name: 'refresh' }));
 
   await waitFor(() => {
-    expect(SupersetClientGet).toBeCalledTimes(3);
-    expect(props.getDbList).toBeCalledTimes(1);
+    expect(SupersetClientGet).toBeCalledTimes(2);
+    expect(props.getDbList).toBeCalledTimes(0);
     expect(props.getTableList).toBeCalledTimes(0);
     expect(props.handleError).toBeCalledTimes(0);
-    expect(props.onDbChange).toBeCalledTimes(1);
-    expect(props.onSchemaChange).toBeCalledTimes(1);
+    expect(props.onDbChange).toBeCalledTimes(0);
+    expect(props.onSchemaChange).toBeCalledTimes(0);
     expect(props.onSchemasLoad).toBeCalledTimes(2);
-    expect(props.onUpdate).toBeCalledTimes(1);
+    expect(props.onUpdate).toBeCalledTimes(0);
   });
 });
 
 test('Should database select display options', async () => {
   const props = createProps();
   render(<DatabaseSelector {...props} />);
-  const selector = await screen.findByText('Database:');
-  expect(selector).toBeInTheDocument();
-  expect(selector.parentElement).toHaveTextContent(
-    'Database:postgresql examples',
-  );
+  const select = screen.getByRole('combobox', {
+    name: 'Select database or type database name',
+  });
+  expect(select).toBeInTheDocument();
+  userEvent.click(select);
+  expect(await screen.findByText('test-mysql')).toBeInTheDocument();
 });
 
 test('Should schema select display options', async () => {
   const props = createProps();
   render(<DatabaseSelector {...props} />);
+  const select = screen.getByRole('combobox', {
+    name: 'Select schema or type schema name',
+  });
+  expect(select).toBeInTheDocument();
+  userEvent.click(select);
+  expect(
+    await screen.findByRole('option', { name: 'public' }),
+  ).toBeInTheDocument();
+  expect(
+    await screen.findByRole('option', { name: 'information_schema' }),
+  ).toBeInTheDocument();
+});
 
-  const selector = await screen.findByText('Schema:');
-  expect(selector).toBeInTheDocument();
-  expect(selector.parentElement).toHaveTextContent('Schema: public');
-
-  userEvent.click(screen.getByRole('button'));
+test('Sends the correct db when changing the database', async () => {
+  const props = createProps();
+  render(<DatabaseSelector {...props} />);
+  const select = screen.getByRole('combobox', {
+    name: 'Select database or type database name',
+  });
+  expect(select).toBeInTheDocument();
+  userEvent.click(select);
+  userEvent.click(await screen.findByText('test-mysql'));
+  await waitFor(() =>
+    expect(props.onDbChange).toHaveBeenCalledWith(
+      expect.objectContaining({
+        id: 2,
+        database_name: 'test-mysql',
+        backend: 'mysql',
+      }),
+    ),
+  );
+});
 
-  expect(await screen.findByText('Select a schema (2)')).toBeInTheDocument();
+test('Sends the correct schema when changing the schema', async () => {
+  const props = createProps();
+  render(<DatabaseSelector {...props} />);
+  const select = screen.getByRole('combobox', {
+    name: 'Select schema or type schema name',
+  });
+  expect(select).toBeInTheDocument();
+  userEvent.click(select);
+  const schemaOption = await screen.findAllByText('information_schema');
+  userEvent.click(schemaOption[1]);
+  await waitFor(() =>
+    expect(props.onSchemaChange).toHaveBeenCalledWith('information_schema'),
+  );
 });