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.

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

fix: auto-complete of tables and names are not working in SQL lab #19152

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -171,17 +171,20 @@ class AceEditorWrapper extends React.PureComponent<Props, State> {
meta: 'schema',
}));
const columns = {};
const tables = props.extendedTables || props.tables || [];

const tables = props.tables || [];
const extendedTables = props.extendedTables || [];

const tableWords = tables.map(t => {
const tableName = t.name;
const cols = t.columns || [];
const tableName = t.value;
const extendedTable = extendedTables.find(et => et.name === tableName);
const cols = (extendedTable && extendedTable.columns) || [];
cols.forEach(col => {
columns[col.name] = null; // using an object as a unique set
});

return {
name: tableName,
name: t.label,
value: tableName,
score: TABLE_AUTOCOMPLETE_SCORE,
meta: 'table',
Expand Down
38 changes: 31 additions & 7 deletions superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import React from 'react';
import React, { useEffect, useRef, useCallback } from 'react';
import Button from 'src/components/Button';
import { t, styled, css, SupersetTheme } from '@superset-ui/core';
import Collapse from 'src/components/Collapse';
Expand All @@ -41,7 +41,10 @@ interface actionsTypes {
addDangerToast: (msg: string) => void;
queryEditorSetSchema: (queryEditor: QueryEditor, schema?: string) => void;
queryEditorSetSchemaOptions: () => void;
queryEditorSetTableOptions: (options: Array<any>) => void;
queryEditorSetTableOptions: (
queryEditor: QueryEditor,
options: Array<any>,
) => void;
resetState: () => void;
}

Expand Down Expand Up @@ -86,6 +89,13 @@ export default function SqlEditorLeftBar({
tables = [],
height = 500,
}: SqlEditorLeftBarProps) {
// Ref needed to avoid infinite rerenders on handlers
// that require and modify the queryEditor
const queryEditorRef = useRef<QueryEditor>(queryEditor);
useEffect(() => {
queryEditorRef.current = queryEditor;
}, [queryEditor]);

const onDbChange = ({ id: dbId }: { id: number }) => {
actions.queryEditorSetDb(queryEditor, dbId);
actions.queryEditorSetFunctionNames(queryEditor, dbId);
Expand Down Expand Up @@ -132,9 +142,23 @@ export default function SqlEditorLeftBar({
const shouldShowReset = window.location.search === '?reset=1';
const tableMetaDataHeight = height - 130; // 130 is the height of the selects above

const onSchemaChange = (schema: string) => {
actions.queryEditorSetSchema(queryEditor, schema);
};
const handleSchemaChange = useCallback(
(schema: string) => {
if (queryEditorRef.current) {
actions.queryEditorSetSchema(queryEditorRef.current, schema);
}
},
[actions],
);

const handleTablesLoad = React.useCallback(
(options: Array<any>) => {
if (queryEditorRef.current) {
actions.queryEditorSetTableOptions(queryEditorRef.current, options);
}
},
[actions],
);

return (
<div className="SqlEditorLeftBar">
Expand All @@ -143,10 +167,10 @@ export default function SqlEditorLeftBar({
getDbList={actions.setDatabases}
handleError={actions.addDangerToast}
onDbChange={onDbChange}
onSchemaChange={onSchemaChange}
onSchemaChange={handleSchemaChange}
onSchemasLoad={actions.queryEditorSetSchemaOptions}
onTableChange={onTableChange}
onTablesLoad={actions.queryEditorSetTableOptions}
onTablesLoad={handleTablesLoad}
schema={queryEditor.schema}
sqlLabMode
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@
import React from 'react';
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import { SupersetClient } from '@superset-ui/core';
import { act } from 'react-dom/test-utils';
import userEvent from '@testing-library/user-event';
import TableSelector from '.';

const SupersetClientGet = jest.spyOn(SupersetClient, 'get');

const createProps = () => ({
const createProps = (props = {}) => ({
database: {
id: 1,
database_name: 'main',
Expand All @@ -34,23 +35,33 @@ const createProps = () => ({
},
schema: 'test_schema',
handleError: jest.fn(),
...props,
});

beforeAll(() => {
SupersetClientGet.mockImplementation(
async () =>
({
json: {
options: [
{ label: 'table_a', value: 'table_a' },
{ label: 'table_b', value: 'table_b' },
],
},
} as any),
);
afterEach(() => {
jest.clearAllMocks();
});

const getSchemaMockFunction = async () =>
({
json: {
result: ['schema_a', 'schema_b'],
},
} as any);

const getTableMockFunction = async () =>
({
json: {
options: [
{ label: 'table_a', value: 'table_a' },
{ label: 'table_b', value: 'table_b' },
],
},
} as any);

test('renders with default props', async () => {
SupersetClientGet.mockImplementation(getTableMockFunction);

const props = createProps();
render(<TableSelector {...props} />, { useRedux: true });
const databaseSelect = screen.getByRole('combobox', {
Expand All @@ -70,6 +81,8 @@ test('renders with default props', async () => {
});

test('renders table options', async () => {
SupersetClientGet.mockImplementation(getTableMockFunction);

const props = createProps();
render(<TableSelector {...props} />, { useRedux: true });
const tableSelect = screen.getByRole('combobox', {
Expand All @@ -85,6 +98,8 @@ test('renders table options', async () => {
});

test('renders disabled without schema', async () => {
SupersetClientGet.mockImplementation(getTableMockFunction);

const props = createProps();
render(<TableSelector {...props} schema={undefined} />, { useRedux: true });
const tableSelect = screen.getByRole('combobox', {
Expand All @@ -94,3 +109,42 @@ test('renders disabled without schema', async () => {
expect(tableSelect).toBeDisabled();
});
});

test('table options are notified after schema selection', async () => {
SupersetClientGet.mockImplementation(getSchemaMockFunction);

const callback = jest.fn();
const props = createProps({
onTablesLoad: callback,
schema: undefined,
});
render(<TableSelector {...props} />, { useRedux: true });

const schemaSelect = screen.getByRole('combobox', {
name: 'Select schema or type schema name',
});
expect(schemaSelect).toBeInTheDocument();
expect(callback).not.toHaveBeenCalled();

userEvent.click(schemaSelect);

expect(
await screen.findByRole('option', { name: 'schema_a' }),
).toBeInTheDocument();
expect(
await screen.findByRole('option', { name: 'schema_b' }),
).toBeInTheDocument();

SupersetClientGet.mockImplementation(getTableMockFunction);

act(() => {
userEvent.click(screen.getAllByText('schema_a')[1]);
});

await waitFor(() => {
expect(callback).toHaveBeenCalledWith([
{ label: 'table_a', value: 'table_a' },
{ label: 'table_b', value: 'table_b' },
]);
});
});
7 changes: 3 additions & 4 deletions superset-frontend/src/components/TableSelector/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -208,15 +208,14 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
currentTable = option;
}
});
if (onTablesLoad) {
onTablesLoad(json.options);
}

onTablesLoad?.(json.options);
setTableOptions(options);
setCurrentTable(currentTable);
setLoadingTables(false);
if (forceRefresh) addSuccessToast('List updated');
})
.catch(e => {
.catch(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the extra cleanup along the way!

setLoadingTables(false);
handleError(t('There was an error loading the tables'));
});
Expand Down