Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Changes the DatabaseSelector and TableSelector to use the new Select component #16483

Merged
merged 1 commit into from
Sep 22, 2021
Merged

refactor: Changes the DatabaseSelector and TableSelector to use the new Select component #16483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions superset-frontend/spec/javascripts/sqllab/SqlEditorLeftBar_spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion superset-frontend/spec/javascripts/sqllab/SqlEditor_spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -53,6 +54,7 @@ describe('SqlEditor', () => {
actions: {
queryEditorSetFunctionNames,
queryEditorSetSelectedText,
queryEditorSetSchemaOptions,
addDangerToast: jest.fn(),
},
database: {},
Expand Down
9 changes: 7 additions & 2 deletions superset-frontend/src/SqlLab/components/SqlEditorLeftBar.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -171,7 +177,6 @@ export default class SqlEditorLeftBar extends React.PureComponent {
onTablesLoad={this.onTablesLoad}
schema={qe.schema}
sqlLabMode
tableNameSticky={false}
/>
<div className="divider" />
<StyledScrollbarContainer>
Expand Down
9 changes: 4 additions & 5 deletions superset-frontend/src/components/CertifiedIcon/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -48,8 +48,7 @@ function CertifiedIcon({
>
<Icons.Certified
iconColor={supersetTheme.colors.primary.base}
height={size}
width={size}
iconSize={size}
/>
</Tooltip>
);
Expand Down
115 changes: 90 additions & 25 deletions superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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',
Expand Down Expand Up @@ -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;
Expand All @@ -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'),
);
});
Loading