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

refactor(sqllab): Remove tableOptions from redux state #23488

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions superset-frontend/src/SqlLab/actions/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export const EXPAND_TABLE = 'EXPAND_TABLE';
export const COLLAPSE_TABLE = 'COLLAPSE_TABLE';
export const QUERY_EDITOR_SETDB = 'QUERY_EDITOR_SETDB';
export const QUERY_EDITOR_SET_SCHEMA = 'QUERY_EDITOR_SET_SCHEMA';
export const QUERY_EDITOR_SET_TABLE_OPTIONS = 'QUERY_EDITOR_SET_TABLE_OPTIONS';
export const QUERY_EDITOR_SET_TITLE = 'QUERY_EDITOR_SET_TITLE';
export const QUERY_EDITOR_SET_AUTORUN = 'QUERY_EDITOR_SET_AUTORUN';
export const QUERY_EDITOR_SET_SQL = 'QUERY_EDITOR_SET_SQL';
Expand Down Expand Up @@ -952,10 +951,6 @@ export function queryEditorSetSchema(queryEditor, schema) {
};
}

export function queryEditorSetTableOptions(queryEditor, options) {
return { type: QUERY_EDITOR_SET_TABLE_OPTIONS, queryEditor, options };
}

export function queryEditorSetAutorun(queryEditor, autorun) {
return function (dispatch) {
const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)
Expand Down
16 changes: 12 additions & 4 deletions superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import {
FullSQLEditor as AceEditor,
} from 'src/components/AsyncAceEditor';
import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor';
import { useSchemas } from 'src/hooks/apiResources';
import { useSchemas, useTables } from 'src/hooks/apiResources';

type HotKey = {
key: string;
Expand Down Expand Up @@ -99,11 +99,19 @@ const AceEditorWrapper = ({
'dbId',
'sql',
'functionNames',
'tableOptions',
'validationResult',
'schema',
]);
const { data: schemaOptions } = useSchemas({ dbId: queryEditor.dbId });
const { data: schemaOptions } = useSchemas({
...(autocomplete && { dbId: queryEditor.dbId }),
});
const { data: tableData } = useTables({
...(autocomplete && {
dbId: queryEditor.dbId,
schema: queryEditor.schema,
}),
});

const currentSql = queryEditor.sql ?? '';
const functionNames = queryEditor.functionNames ?? [];

Expand All @@ -120,7 +128,7 @@ const AceEditorWrapper = ({
}),
[schemaOptions],
);
const tables = queryEditor.tableOptions ?? [];
const tables = tableData?.options ?? [];

const [sql, setSql] = useState(currentSql);
const [words, setWords] = useState<AceCompleterKeyword[]>([]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ const SaveQuery = ({
'schema',
'selectedText',
'sql',
'tableOptions',
'templateParams',
]);
const query = useMemo(
Expand Down
11 changes: 0 additions & 11 deletions superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import {
collapseTable,
expandTable,
queryEditorSetSchema,
queryEditorSetTableOptions,
setDatabases,
addDangerToast,
resetState,
Expand Down Expand Up @@ -218,15 +217,6 @@ const SqlEditorLeftBar = ({
[dispatch, queryEditor],
);

const handleTablesLoad = useCallback(
(options: Array<any>) => {
if (queryEditor) {
dispatch(queryEditorSetTableOptions(queryEditor, options));
}
},
[dispatch, queryEditor],
);

const handleDbList = useCallback(
(result: DatabaseObject) => {
dispatch(setDatabases(result));
Expand Down Expand Up @@ -256,7 +246,6 @@ const SqlEditorLeftBar = ({
onDbChange={onDbChange}
onSchemaChange={handleSchemaChange}
onTableSelectChange={onTablesChange}
onTablesLoad={handleTablesLoad}
schema={schema}
tableValue={selectedTableNames}
sqlLabMode
Expand Down
1 change: 0 additions & 1 deletion superset-frontend/src/SqlLab/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ export const defaultQueryEditor = {
name: 'Untitled Query 1',
schema: 'main',
remoteId: null,
tableOptions: [],
functionNames: [],
hideLeftBar: false,
templateParams: '{}',
Expand Down
12 changes: 0 additions & 12 deletions superset-frontend/src/SqlLab/reducers/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -587,18 +587,6 @@ export default function sqlLabReducer(state = {}, action) {
),
};
},
[actions.QUERY_EDITOR_SET_TABLE_OPTIONS]() {
return {
...state,
...alterUnsavedQueryEditorState(
state,
{
tableOptions: action.options,
},
action.queryEditor.id,
),
};
},
[actions.QUERY_EDITOR_SET_TITLE]() {
return {
...state,
Expand Down
1 change: 0 additions & 1 deletion superset-frontend/src/SqlLab/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export interface QueryEditor {
autorun: boolean;
sql: string;
remoteId: number | null;
tableOptions: any[];
functionNames: string[];
validationResult?: {
completed: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import React from 'react';
import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
import { queryClient } from 'src/views/QueryProvider';
import fetchMock from 'fetch-mock';
import { act } from 'react-dom/test-utils';
import userEvent from '@testing-library/user-event';
import TableSelector, { TableSelectorMultiple } from '.';

Expand All @@ -36,11 +35,6 @@ const createProps = (props = {}) => ({
...props,
});

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

const getTableMockFunction = () =>
({
count: 4,
Expand Down Expand Up @@ -124,47 +118,6 @@ test('renders disabled without schema', async () => {
});
});

test('table options are notified after schema selection', async () => {
fetchMock.get(schemaApiRoute, 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 to search schemas',
});
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();

fetchMock.get(tablesApiRoute, 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' },
{ label: 'table_c', value: 'table_c' },
{ label: 'table_d', value: 'table_d' },
]);
});
});

test('table select retain value if not in SQL Lab mode', async () => {
fetchMock.get(schemaApiRoute, { result: ['test_schema'] });
fetchMock.get(tablesApiRoute, getTableMockFunction());
Expand Down
10 changes: 0 additions & 10 deletions superset-frontend/src/components/TableSelector/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ interface TableSelectorProps {
isDatabaseSelectEnabled?: boolean;
onDbChange?: (db: DatabaseObject) => void;
onSchemaChange?: (schema?: string) => void;
onTablesLoad?: (options: Array<any>) => void;
readOnly?: boolean;
schema?: string;
onEmptyResults?: (searchText?: string) => void;
Expand Down Expand Up @@ -158,7 +157,6 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
isDatabaseSelectEnabled = true,
onDbChange,
onSchemaChange,
onTablesLoad,
readOnly = false,
onEmptyResults,
schema,
Expand Down Expand Up @@ -199,14 +197,6 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
},
});

useEffect(() => {
// Set the tableOptions in the queryEditor so autocomplete
// works on new tabs
if (data && isFetched) {
onTablesLoad?.(data.options);
}
}, [data, isFetched, onTablesLoad]);

const tableOptions = useMemo<TableOption[]>(
() =>
data
Expand Down