diff --git a/superset-frontend/spec/javascripts/sqllab/SqlEditorLeftBar_spec.jsx b/superset-frontend/spec/javascripts/sqllab/SqlEditorLeftBar_spec.jsx index 1ba1ac821..b153c1483 100644 --- 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', () => { , ); - 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 a database', + }); + const schemaSelect = screen.getByRole('combobox', { + name: 'Select a schema', + }); + const dropdown = screen.getByText(/Select a table/i); const abUser = screen.getByText(/ab_user/i); expect(dbSelect).toBeInTheDocument(); expect(schemaSelect).toBeInTheDocument(); diff --git a/superset-frontend/src/components/CertifiedIcon/index.tsx b/superset-frontend/src/components/CertifiedIcon/index.tsx index f08e9bf60..4aa0dad23 100644 --- 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 ( ); diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index 0d812824d..6d4abb3fd 100644 --- 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: 'postgresql' }, formMode: false, isDatabaseSelectEnabled: true, readOnly: false, - schema: 'public', + schema: undefined, sqlLabMode: true, getDbList: jest.fn(), getTableList: jest.fn(), @@ -129,7 +129,7 @@ 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', explore_database_id: 1, expose_in_sqllab: true, force_ctas_schema: null, @@ -153,50 +153,62 @@ test('Refresh should work', async () => { render(); + const select = screen.getByRole('combobox', { + name: 'Select a schema', + }); + + 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(); - const selector = await screen.findByText('Database:'); - expect(selector).toBeInTheDocument(); - expect(selector.parentElement).toHaveTextContent( - 'Database:postgresql examples', - ); + const select = screen.getByRole('combobox', { + name: 'Select a database', + }); + expect(select).toBeInTheDocument(); + userEvent.click(select); + expect( + await screen.findByRole('option', { name: 'postgresql: test' }), + ).toBeInTheDocument(); }); test('Should schema select display options', async () => { const props = createProps(); render(); - - const selector = await screen.findByText('Schema:'); - expect(selector).toBeInTheDocument(); - expect(selector.parentElement).toHaveTextContent('Schema: public'); - - userEvent.click(screen.getByRole('button')); - - expect(await screen.findByText('Select a schema (2)')).toBeInTheDocument(); + const select = screen.getByRole('combobox', { + name: 'Select a schema', + }); + expect(select).toBeInTheDocument(); + userEvent.click(select); + expect( + await screen.findByRole('option', { name: 'public' }), + ).toBeInTheDocument(); + expect( + await screen.findByRole('option', { name: 'information_schema' }), + ).toBeInTheDocument(); }); diff --git a/superset-frontend/src/components/DatabaseSelector/index.tsx b/superset-frontend/src/components/DatabaseSelector/index.tsx index 0282e4a4a..c96fba7c8 100644 --- a/superset-frontend/src/components/DatabaseSelector/index.tsx +++ b/superset-frontend/src/components/DatabaseSelector/index.tsx @@ -16,58 +16,51 @@ * specific language governing permissions and limitations * under the License. */ -import React, { ReactNode, useEffect, useState } from 'react'; +import React, { ReactNode, useState, useMemo } from 'react'; import { styled, SupersetClient, t } from '@superset-ui/core'; import rison from 'rison'; -import { Select } from 'src/components/Select'; -import Label from 'src/components/Label'; +import { Select } from 'src/components'; +import { FormLabel } from 'src/components/Form'; import RefreshLabel from 'src/components/RefreshLabel'; -import SupersetAsyncSelect from 'src/components/AsyncSelect'; - -const FieldTitle = styled.p` - color: ${({ theme }) => theme.colors.secondary.light2}; - font-size: ${({ theme }) => theme.typography.sizes.s}px; - margin: 20px 0 10px 0; - text-transform: uppercase; -`; const DatabaseSelectorWrapper = styled.div` - .fa-refresh { - padding-left: 9px; - } + ${({ theme }) => ` + .refresh { + display: flex; + align-items: center; + width: 30px; + margin-left: ${theme.gridUnit}px; + margin-top: ${theme.gridUnit * 5}px; + } - .refresh-col { - display: flex; - align-items: center; - width: 30px; - margin-left: ${({ theme }) => theme.gridUnit}px; - } + .section { + display: flex; + flex-direction: row; + align-items: center; + } - .section { - padding-bottom: 5px; - display: flex; - flex-direction: row; - } + .select { + flex: 1; + } - .select { - flex-grow: 1; - } + & > div { + margin-bottom: ${theme.gridUnit * 4}px; + } + `} `; -const DatabaseOption = styled.span` - display: inline-flex; - align-items: center; -`; +type DatabaseValue = { label: string; value: number }; + +type SchemaValue = { label: string; value: string }; interface DatabaseSelectorProps { - dbId: number; + db?: { id: number; database_name: string; backend: string }; formMode?: boolean; getDbList?: (arg0: any) => {}; - getTableList?: (dbId: number, schema: string, force: boolean) => {}; handleError: (msg: string) => void; isDatabaseSelectEnabled?: boolean; onDbChange?: (db: any) => void; - onSchemaChange?: (arg0?: any) => {}; + onSchemaChange?: (schema?: string) => void; onSchemasLoad?: (schemas: Array) => void; readOnly?: boolean; schema?: string; @@ -83,10 +76,9 @@ interface DatabaseSelectorProps { } export default function DatabaseSelector({ - dbId, + db, formMode = false, getDbList, - getTableList, handleError, isDatabaseSelectEnabled = true, onUpdate, @@ -97,193 +89,189 @@ export default function DatabaseSelector({ schema, sqlLabMode = false, }: DatabaseSelectorProps) { - const [currentDbId, setCurrentDbId] = useState(dbId); - const [currentSchema, setCurrentSchema] = useState( - schema, + const [currentDb, setCurrentDb] = useState( + db + ? { label: `${db.backend}: ${db.database_name}`, value: db.id } + : undefined, ); - const [schemaLoading, setSchemaLoading] = useState(false); - const [schemaOptions, setSchemaOptions] = useState([]); + const [currentSchema, setCurrentSchema] = useState( + schema ? { label: schema, value: schema } : undefined, + ); + const [refresh, setRefresh] = useState(0); - function fetchSchemas(databaseId: number, forceRefresh = false) { - const actualDbId = databaseId || dbId; - if (actualDbId) { - setSchemaLoading(true); - const queryParams = rison.encode({ - force: Boolean(forceRefresh), - }); - const endpoint = `/api/v1/database/${actualDbId}/schemas/?q=${queryParams}`; - return SupersetClient.get({ endpoint }) - .then(({ json }) => { + const loadSchemas = useMemo( + () => async (): Promise<{ + data: SchemaValue[]; + totalCount: number; + }> => { + if (currentDb) { + const queryParams = rison.encode({ force: refresh > 0 }); + const endpoint = `/api/v1/database/${currentDb.value}/schemas/?q=${queryParams}`; + + // TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes. + return SupersetClient.get({ endpoint }).then(({ json }) => { const options = json.result.map((s: string) => ({ value: s, label: s, title: s, })); - setSchemaOptions(options); - setSchemaLoading(false); if (onSchemasLoad) { onSchemasLoad(options); } - }) - .catch(() => { - setSchemaOptions([]); - setSchemaLoading(false); - handleError(t('Error while fetching schema list')); + return { + data: options, + totalCount: options.length, + }; }); - } - return Promise.resolve(); - } + } + return { + data: [], + totalCount: 0, + }; + }, + [currentDb, refresh, onSchemasLoad], + ); - useEffect(() => { - if (currentDbId) { - fetchSchemas(currentDbId); - } - }, [currentDbId]); - - function onSelectChange({ dbId, schema }: { dbId: number; schema?: string }) { - setCurrentDbId(dbId); + function onSelectChange({ + db, + schema, + }: { + db: DatabaseValue; + schema?: SchemaValue; + }) { + setCurrentDb(db); setCurrentSchema(schema); if (onUpdate) { - onUpdate({ dbId, schema, tableName: undefined }); + onUpdate({ + dbId: db.value, + schema: schema?.value, + tableName: undefined, + }); } } - function dbMutator(data: any) { - if (getDbList) { - getDbList(data.result); - } - if (data.result.length === 0) { - handleError(t("It seems you don't have access to any database")); - } - return data.result.map((row: any) => ({ - ...row, - // label is used for the typeahead - label: `${row.backend} ${row.database_name}`, - })); - } - - function changeDataBase(db: any, force = false) { - const dbId = db ? db.id : null; - setSchemaOptions([]); + function changeDataBase(selectedValue: DatabaseValue) { + const actualDb = selectedValue || db; if (onSchemaChange) { - onSchemaChange(null); + onSchemaChange(undefined); } if (onDbChange) { onDbChange(db); } - fetchSchemas(dbId, force); - onSelectChange({ dbId, schema: undefined }); + onSelectChange({ db: actualDb, schema: undefined }); } - function changeSchema(schemaOpt: any, force = false) { - const schema = schemaOpt ? schemaOpt.value : null; + function changeSchema(schema: SchemaValue) { if (onSchemaChange) { - onSchemaChange(schema); + onSchemaChange(schema.value); } - setCurrentSchema(schema); - onSelectChange({ dbId: currentDbId, schema }); - if (getTableList) { - getTableList(currentDbId, schema, force); + if (currentDb) { + onSelectChange({ db: currentDb, schema }); } } - function renderDatabaseOption(db: any) { - return ( - - {db.database_name} - - ); - } - function renderSelectRow(select: ReactNode, refreshBtn: ReactNode) { return (
{select} - {refreshBtn} + {refreshBtn}
); } - function renderDatabaseSelect() { - const queryParams = rison.encode({ - order_columns: 'database_name', - order_direction: 'asc', - page: 0, - page_size: -1, - ...(formMode || !sqlLabMode - ? {} - : { - filters: [ - { - col: 'expose_in_sqllab', - opr: 'eq', - value: true, - }, - ], - }), - }); - - return renderSelectRow( - changeDataBase(db)} - onAsyncError={() => - handleError(t('Error while fetching database list')) + const loadDatabases = useMemo( + () => async ( + search: string, + page: number, + pageSize: number, + ): Promise<{ + data: DatabaseValue[]; + totalCount: number; + }> => { + const queryParams = rison.encode({ + order_columns: 'database_name', + order_direction: 'asc', + page, + page_size: pageSize, + ...(formMode || !sqlLabMode + ? { filters: [{ col: 'database_name', opr: 'ct', value: search }] } + : { + filters: [ + { col: 'database_name', opr: 'ct', value: search }, + { + col: 'expose_in_sqllab', + opr: 'eq', + value: true, + }, + ], + }), + }); + const endpoint = `/api/v1/database/?q=${queryParams}`; + return SupersetClient.get({ endpoint }).then(({ json }) => { + const { result } = json; + if (getDbList) { + getDbList(result); } - clearable={false} - value={currentDbId} - valueKey="id" - valueRenderer={(db: any) => ( -
- {t('Database:')} - {renderDatabaseOption(db)} -
- )} - optionRenderer={renderDatabaseOption} - mutator={dbMutator} + if (result.length === 0) { + handleError(t("It seems you don't have access to any database")); + } + const options = result.map( + (row: { backend: string; database_name: string; id: number }) => ({ + label: `${row.backend}: ${row.database_name}`, + value: row.id, + }), + ); + return { + data: options, + totalCount: options.length, + }; + }); + }, + [formMode, getDbList, handleError, sqlLabMode], + ); + + function renderDatabaseSelect() { + return renderSelectRow( + {t('Schema')}} name="select-schema" - placeholder={t('Select a schema (%s)', schemaOptions.length)} - options={schemaOptions} - value={value} - valueRenderer={o => ( -
- {t('Schema:')} {o.label} -
- )} - isLoading={schemaLoading} - autosize={false} - onChange={item => changeSchema(item)} - isDisabled={readOnly} + placeholder={t('Select a schema')} + onChange={item => changeSchema(item as SchemaValue)} + options={loadSchemas} + value={currentSchema} />, - refresh, + refreshIcon, ); } return ( - {formMode && {t('datasource')}} {renderDatabaseSelect()} - {formMode && {t('schema')}} {renderSchemaSelect()} ); diff --git a/superset-frontend/src/components/Icons/Icon.tsx b/superset-frontend/src/components/Icons/Icon.tsx index 9e3d0e187..efb78dc65 100644 --- a/superset-frontend/src/components/Icons/Icon.tsx +++ b/superset-frontend/src/components/Icons/Icon.tsx @@ -53,15 +53,21 @@ export const Icon = (props: IconProps) => { const name = fileName.replace('_', '-'); useEffect(() => { + let cancelled = false; async function importIcon(): Promise { ImportedSVG.current = ( await import( `!!@svgr/webpack?-svgo,+titleProp,+ref!images/icons/${fileName}.svg` ) ).default; - setLoaded(true); + if (!cancelled) { + setLoaded(true); + } } importIcon(); + return () => { + cancelled = true; + }; }, [fileName, ImportedSVG]); return ( diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index 96c88e5e5..655a6569a 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -87,12 +87,9 @@ const StyledContainer = styled.div` flex-direction: column; `; -const StyledSelect = styled(AntdSelect, { - shouldForwardProp: prop => prop !== 'hasHeader', -})<{ hasHeader: boolean }>` - ${({ theme, hasHeader }) => ` +const StyledSelect = styled(AntdSelect)` + ${({ theme }) => ` width: 100%; - margin-top: ${hasHeader ? theme.gridUnit : 0}px; && .ant-select-selector { border-radius: ${theme.gridUnit}px; @@ -190,6 +187,7 @@ const Select = ({ : 'multiple'; useEffect(() => { + fetchedQueries.current.clear(); setSelectOptions( options && Array.isArray(options) ? options : EMPTY_OPTIONS, ); @@ -367,34 +365,45 @@ const Select = ({ [options], ); - const handleOnSearch = debounce((search: string) => { - const searchValue = search.trim(); - // enables option creation - if (allowNewOptions && isSingleMode) { - const firstOption = selectOptions.length > 0 && selectOptions[0].value; - // replaces the last search value entered with the new one - // only when the value wasn't part of the original options - if ( - searchValue && - firstOption === searchedValue && - !initialOptions.find(o => o.value === searchedValue) - ) { - selectOptions.shift(); - setSelectOptions(selectOptions); - } - if (searchValue && !hasOption(searchValue, selectOptions)) { - const newOption = { - label: searchValue, - value: searchValue, - }; - // adds a custom option - const newOptions = [...selectOptions, newOption]; - setSelectOptions(newOptions); - setSelectValue(searchValue); - } - } - setSearchedValue(searchValue); - }, DEBOUNCE_TIMEOUT); + const handleOnSearch = useMemo( + () => + debounce((search: string) => { + const searchValue = search.trim(); + // enables option creation + if (allowNewOptions && isSingleMode) { + const firstOption = + selectOptions.length > 0 && selectOptions[0].value; + // replaces the last search value entered with the new one + // only when the value wasn't part of the original options + if ( + searchValue && + firstOption === searchedValue && + !initialOptions.find(o => o.value === searchedValue) + ) { + selectOptions.shift(); + setSelectOptions(selectOptions); + } + if (searchValue && !hasOption(searchValue, selectOptions)) { + const newOption = { + label: searchValue, + value: searchValue, + }; + // adds a custom option + const newOptions = [...selectOptions, newOption]; + setSelectOptions(newOptions); + setSelectValue(searchValue); + } + } + setSearchedValue(searchValue); + }, DEBOUNCE_TIMEOUT), + [ + allowNewOptions, + initialOptions, + isSingleMode, + searchedValue, + selectOptions, + ], + ); const handlePagination = (e: UIEvent) => { const vScroll = e.currentTarget; @@ -487,7 +496,6 @@ const Select = ({ {header} , { - context: { store }, - wrappingComponent: ThemeProvider, - wrappingComponentProps: { theme: supersetTheme }, - }); - await waitForComponentToPaint(mounted); - - return mounted; -} - -describe('TableSelector', () => { - let wrapper; - - beforeEach(async () => { - fetchMock.reset(); - wrapper = await mountAndWait(); - }); - - it('renders', () => { - expect(wrapper.find(TableSelector)).toExist(); - expect(wrapper.find(DatabaseSelector)).toExist(); - }); - - describe('change database', () => { - afterEach(fetchMock.resetHistory); - afterAll(fetchMock.reset); - - it('should fetch schemas', async () => { - fetchMock.get(FETCH_SCHEMAS_ENDPOINT, { overwriteRoutes: true }); - act(() => { - wrapper.find('[data-test="select-database"]').first().props().onChange({ - id: 1, - database_name: 'main', - }); - }); - await waitForComponentToPaint(wrapper); - expect(fetchMock.calls(FETCH_SCHEMAS_ENDPOINT)).toHaveLength(1); - }); - - it('should fetch schema options', async () => { - fetchMock.get(FETCH_SCHEMAS_ENDPOINT, schemaOptions, { - overwriteRoutes: true, - }); - act(() => { - wrapper.find('[data-test="select-database"]').first().props().onChange({ - id: 1, - database_name: 'main', - }); - }); - await waitForComponentToPaint(wrapper); - wrapper.update(); - expect(fetchMock.calls(FETCH_SCHEMAS_ENDPOINT)).toHaveLength(1); - - expect( - wrapper.find('[name="select-schema"]').first().props().options, - ).toEqual([ - { value: 'main', label: 'main', title: 'main' }, - { value: 'erf', label: 'erf', title: 'erf' }, - { value: 'superset', label: 'superset', title: 'superset' }, - ]); - }); - - it('should clear table options', async () => { - act(() => { - wrapper.find('[data-test="select-database"]').first().props().onChange({ - id: 1, - database_name: 'main', - }); - }); - await waitForComponentToPaint(wrapper); - const props = wrapper.find('[name="async-select-table"]').first().props(); - expect(props.isDisabled).toBe(true); - expect(props.value).toEqual(undefined); - }); - }); - - describe('change schema', () => { - beforeEach(async () => { - fetchMock.get(FETCH_SCHEMAS_ENDPOINT, schemaOptions, { - overwriteRoutes: true, - }); - }); - - afterEach(fetchMock.resetHistory); - afterAll(fetchMock.reset); - - it('should fetch table', async () => { - fetchMock.get(GET_TABLE_NAMES_ENDPOINT, { overwriteRoutes: true }); - act(() => { - wrapper.find('[data-test="select-database"]').first().props().onChange({ - id: 1, - database_name: 'main', - }); - }); - await waitForComponentToPaint(wrapper); - act(() => { - wrapper - .find('[name="select-schema"]') - .first() - .props() - .onChange(selectedSchema); - }); - await waitForComponentToPaint(wrapper); - expect(fetchMock.calls(GET_TABLE_NAMES_ENDPOINT)).toHaveLength(1); - }); - - it('should fetch table options', async () => { - fetchMock.get(GET_TABLE_NAMES_ENDPOINT, tables, { - overwriteRoutes: true, - }); - act(() => { - wrapper.find('[data-test="select-database"]').first().props().onChange({ - id: 1, - database_name: 'main', - }); - }); - await waitForComponentToPaint(wrapper); - act(() => { - wrapper - .find('[name="select-schema"]') - .first() - .props() - .onChange(selectedSchema); - }); - await waitForComponentToPaint(wrapper); - expect( - wrapper.find('[name="select-schema"]').first().props().value[0], - ).toEqual(selectedSchema); - expect(fetchMock.calls(GET_TABLE_NAMES_ENDPOINT)).toHaveLength(1); - const { options } = wrapper.find('[name="select-table"]').first().props(); - expect({ options }).toEqual(tables); - }); - }); - - describe('change table', () => { - beforeEach(async () => { - fetchMock.get(GET_TABLE_NAMES_ENDPOINT, tables, { - overwriteRoutes: true, - }); - }); - - it('should change table value', async () => { - act(() => { - wrapper - .find('[name="select-schema"]') - .first() - .props() - .onChange(selectedSchema); - }); - await waitForComponentToPaint(wrapper); - act(() => { - wrapper - .find('[name="select-table"]') - .first() - .props() - .onChange(selectedTable); - }); - await waitForComponentToPaint(wrapper); - expect( - wrapper.find('[name="select-table"]').first().props().value, - ).toEqual('birth_names'); - }); - - it('should call onTableChange with schema from table object', async () => { - act(() => { - wrapper - .find('[name="select-schema"]') - .first() - .props() - .onChange(selectedSchema); - }); - await waitForComponentToPaint(wrapper); - act(() => { - wrapper - .find('[name="select-table"]') - .first() - .props() - .onChange(selectedTable); - }); - await waitForComponentToPaint(wrapper); - expect(mockedProps.onTableChange.getCall(0).args[0]).toBe('birth_names'); - expect(mockedProps.onTableChange.getCall(0).args[1]).toBe('main'); - }); - }); - - describe('getTableNamesBySubStr', () => { - afterEach(fetchMock.resetHistory); - afterAll(fetchMock.reset); - - it('should handle empty', async () => { - act(() => { - wrapper - .find('[name="async-select-table"]') - .first() - .props() - .loadOptions(); - }); - await waitForComponentToPaint(wrapper); - const props = wrapper.find('[name="async-select-table"]').first().props(); - expect(props.isDisabled).toBe(true); - expect(props.value).toEqual(''); - }); - - it('should handle table name', async () => { - wrapper.setProps({ schema: 'main' }); - fetchMock.get(GET_TABLE_ENDPOINT, tables, { - overwriteRoutes: true, - }); - act(() => { - wrapper - .find('[name="async-select-table"]') - .first() - .props() - .loadOptions(); - }); - await waitForComponentToPaint(wrapper); - expect(fetchMock.calls(GET_TABLE_ENDPOINT)).toHaveLength(1); - }); - }); -}); diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx new file mode 100644 index 000000000..3b8b61715 --- /dev/null +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; +import { render, screen, waitFor } from 'spec/helpers/testing-library'; +import { SupersetClient } from '@superset-ui/core'; +import userEvent from '@testing-library/user-event'; +import TableSelector from '.'; + +const SupersetClientGet = jest.spyOn(SupersetClient, 'get'); + +const createProps = () => ({ + dbId: 1, + schema: 'test_schema', + handleError: jest.fn(), +}); + +beforeAll(() => { + SupersetClientGet.mockImplementation( + async () => + ({ + json: { + options: [ + { label: 'table_a', value: 'table_a' }, + { label: 'table_b', value: 'table_b' }, + ], + }, + } as any), + ); +}); + +test('renders with default props', async () => { + const props = createProps(); + render(); + const databaseSelect = screen.getByRole('combobox', { + name: 'Select a database', + }); + const schemaSelect = screen.getByRole('combobox', { + name: 'Select a database', + }); + const tableSelect = screen.getByRole('combobox', { + name: 'Select a table', + }); + await waitFor(() => { + expect(databaseSelect).toBeInTheDocument(); + expect(schemaSelect).toBeInTheDocument(); + expect(tableSelect).toBeInTheDocument(); + }); +}); + +test('renders table options', async () => { + const props = createProps(); + render(); + const tableSelect = screen.getByRole('combobox', { + name: 'Select a table', + }); + userEvent.click(tableSelect); + expect( + await screen.findByRole('option', { name: 'table_a' }), + ).toBeInTheDocument(); + expect( + await screen.findByRole('option', { name: 'table_b' }), + ).toBeInTheDocument(); +}); + +test('renders disabled without schema', async () => { + const props = createProps(); + render(); + const tableSelect = screen.getByRole('combobox', { + name: 'Select a table', + }); + await waitFor(() => { + expect(tableSelect).toBeDisabled(); + }); +}); diff --git a/superset-frontend/src/components/TableSelector/index.tsx b/superset-frontend/src/components/TableSelector/index.tsx index c437b1e59..5f68a94df 100644 --- a/superset-frontend/src/components/TableSelector/index.tsx +++ b/superset-frontend/src/components/TableSelector/index.tsx @@ -18,57 +18,49 @@ */ import React, { FunctionComponent, - useEffect, useState, ReactNode, + useMemo, + useEffect, } from 'react'; import { styled, SupersetClient, t } from '@superset-ui/core'; -import { AsyncSelect, CreatableSelect, Select } from 'src/components/Select'; - +import { Select } from 'src/components'; import { FormLabel } from 'src/components/Form'; - +import Icons from 'src/components/Icons'; import DatabaseSelector from 'src/components/DatabaseSelector'; import RefreshLabel from 'src/components/RefreshLabel'; import CertifiedIcon from 'src/components/CertifiedIcon'; import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip'; -const FieldTitle = styled.p` - color: ${({ theme }) => theme.colors.secondary.light2}; - font-size: ${({ theme }) => theme.typography.sizes.s}px; - margin: 20px 0 10px 0; - text-transform: uppercase; -`; - const TableSelectorWrapper = styled.div` - .fa-refresh { - padding-left: 9px; - } + ${({ theme }) => ` + .refresh { + display: flex; + align-items: center; + width: 30px; + margin-left: ${theme.gridUnit}px; + margin-top: ${theme.gridUnit * 5}px; + } - .refresh-col { - display: flex; - align-items: center; - width: 30px; - margin-left: ${({ theme }) => theme.gridUnit}px; - } + .section { + display: flex; + flex-direction: row; + align-items: center; + } - .section { - padding-bottom: 5px; - display: flex; - flex-direction: row; - } + .divider { + border-bottom: 1px solid ${theme.colors.secondary.light5}; + margin: 15px 0; + } - .select { - flex-grow: 1; - } + .table-length { + color: ${theme.colors.grayscale.light1}; + } - .divider { - border-bottom: 1px solid ${({ theme }) => theme.colors.secondary.light5}; - margin: 15px 0; - } - - .table-length { - color: ${({ theme }) => theme.colors.grayscale.light1}; - } + .select { + flex: 1; + } + `} `; const TableLabel = styled.span` @@ -98,7 +90,15 @@ interface TableSelectorProps { schema?: string; tableName?: string; }) => void; - onDbChange?: (db: any) => void; + onDbChange?: ( + db: + | { + id: number; + database_name: string; + backend: string; + } + | undefined, + ) => void; onSchemaChange?: (arg0?: any) => {}; onSchemasLoad?: () => void; onTableChange?: (tableName: string, schema: string) => void; @@ -110,6 +110,52 @@ interface TableSelectorProps { tableNameSticky?: boolean; } +interface Table { + label: string; + value: string; + type: string; + extra?: { + certification?: { + certified_by: string; + details: string; + }; + warning_markdown?: string; + }; +} + +interface TableOption { + label: JSX.Element; + text: string; + value: string; +} + +const TableOption = ({ table }: { table: Table }) => { + const { label, type, extra } = table; + return ( + + {type === 'view' ? ( + + ) : ( + + )} + {extra?.certification && ( + + )} + {extra?.warning_markdown && ( + + )} + {label} + + ); +}; + const TableSelector: FunctionComponent = ({ database, dbId, @@ -129,179 +175,187 @@ const TableSelector: FunctionComponent = ({ tableName, tableNameSticky = true, }) => { + const [currentDbId, setCurrentDbId] = useState(dbId); const [currentSchema, setCurrentSchema] = useState( schema, ); - const [currentTableName, setCurrentTableName] = useState( - tableName, - ); - const [tableLoading, setTableLoading] = useState(false); - const [tableOptions, setTableOptions] = useState([]); + const [currentTable, setCurrentTable] = useState(); + const [refresh, setRefresh] = useState(0); + const [previousRefresh, setPreviousRefresh] = useState(0); - function fetchTables( - databaseId?: number, - schema?: string, - forceRefresh = false, - substr = 'undefined', - ) { - const dbSchema = schema || currentSchema; - const actualDbId = databaseId || dbId; - if (actualDbId && dbSchema) { - const encodedSchema = encodeURIComponent(dbSchema); - const encodedSubstr = encodeURIComponent(substr); - setTableLoading(true); - setTableOptions([]); + const loadTable = useMemo( + () => async (dbId: number, schema: string, tableName: string) => { const endpoint = encodeURI( - `/superset/tables/${actualDbId}/${encodedSchema}/${encodedSubstr}/${!!forceRefresh}/`, + `/superset/tables/${dbId}/${schema}/${encodeURIComponent( + tableName, + )}/false/true`, ); - return SupersetClient.get({ endpoint }) - .then(({ json }) => { - const options = json.options.map((o: any) => ({ - value: o.value, - schema: o.schema, - label: o.label, - title: o.title, - type: o.type, - extra: o?.extra, - })); - setTableLoading(false); - setTableOptions(options); + + if (previousRefresh !== refresh) { + setPreviousRefresh(refresh); + } + + return SupersetClient.get({ endpoint }).then(({ json }) => { + const options = json.options as Table[]; + if (options && options.length > 0) { + return options[0]; + } + return null; + }); + }, + [], // eslint-disable-line react-hooks/exhaustive-deps + ); + + const loadTables = useMemo( + () => async (search: string) => { + const dbSchema = schema || currentSchema; + if (currentDbId && dbSchema) { + const encodedSchema = encodeURIComponent(dbSchema); + const encodedSubstr = encodeURIComponent(search || 'undefined'); + const forceRefresh = refresh !== previousRefresh; + const endpoint = encodeURI( + `/superset/tables/${currentDbId}/${encodedSchema}/${encodedSubstr}/${forceRefresh}/`, + ); + + if (previousRefresh !== refresh) { + setPreviousRefresh(refresh); + } + + return SupersetClient.get({ endpoint }).then(({ json }) => { + const options = json.options + .map((table: Table) => ({ + value: table.value, + label: , + text: table.label, + })) + .sort((a: { text: string }, b: { text: string }) => + a.text.localeCompare(b.text), + ); + if (onTablesLoad) { onTablesLoad(json.options); } - }) - .catch(() => { - setTableLoading(false); - setTableOptions([]); - handleError(t('Error while fetching table list')); + + return { + data: options, + totalCount: options.length, + }; }); - } - setTableLoading(false); - setTableOptions([]); - return Promise.resolve(); - } + } + return { data: [], totalCount: 0 }; + }, + // We are using the refresh state to re-trigger the query + // previousRefresh should be out of dependencies array + // eslint-disable-next-line react-hooks/exhaustive-deps + [currentDbId, currentSchema, onTablesLoad, schema, refresh], + ); useEffect(() => { - if (dbId && schema) { - fetchTables(); + async function fetchTable() { + if (schema && tableName) { + const table = await loadTable(dbId, schema, tableName); + if (table) { + setCurrentTable({ + label: , + text: table.label, + value: table.value, + }); + } + } } - }, [dbId, schema]); + fetchTable(); + }, []); // eslint-disable-line react-hooks/exhaustive-deps function onSelectionChange({ dbId, schema, - tableName, + table, }: { dbId: number; schema?: string; - tableName?: string; + table?: TableOption; }) { - setCurrentTableName(tableName); + setCurrentTable(table); + setCurrentDbId(dbId); setCurrentSchema(schema); if (onUpdate) { - onUpdate({ dbId, schema, tableName }); + onUpdate({ dbId, schema, tableName: table?.value }); } } - function getTableNamesBySubStr(substr = 'undefined') { - if (!dbId || !substr) { - const options: any[] = []; - return Promise.resolve({ options }); - } - const encodedSchema = encodeURIComponent(schema || ''); - const encodedSubstr = encodeURIComponent(substr); - return SupersetClient.get({ - endpoint: encodeURI( - `/superset/tables/${dbId}/${encodedSchema}/${encodedSubstr}`, - ), - }).then(({ json }) => { - const options = json.options.map((o: any) => ({ - value: o.value, - schema: o.schema, - label: o.label, - title: o.title, - type: o.type, - })); - return { options }; - }); - } - - function changeTable(tableOpt: any) { - if (!tableOpt) { - setCurrentTableName(''); + function changeTable(table: TableOption) { + if (!table) { + setCurrentTable(undefined); return; } - const schemaName = tableOpt.schema; - const tableOptTableName = tableOpt.value; - if (tableNameSticky) { + const tableOptTableName = table.value; + if (currentDbId && tableNameSticky) { onSelectionChange({ - dbId, - schema: schemaName, - tableName: tableOptTableName, + dbId: currentDbId, + schema: currentSchema, + table, }); } - if (onTableChange) { - onTableChange(tableOptTableName, schemaName); + if (onTableChange && currentSchema) { + onTableChange(tableOptTableName, currentSchema); } } - function changeSchema(schemaOpt: any, force = false) { - const value = schemaOpt ? schemaOpt.value : null; + function onRefresh() { if (onSchemaChange) { - onSchemaChange(value); + onSchemaChange(currentSchema); } - onSelectionChange({ - dbId, - schema: value, - tableName: undefined, - }); - fetchTables(dbId, currentSchema, force); - } - - function renderTableOption(option: any) { - return ( - - - - - {option.extra?.certification && ( - - )} - {option.extra?.warning_markdown && ( - - )} - {option.label} - - ); + if (currentDbId && currentSchema) { + onSelectionChange({ + dbId: currentDbId, + schema: currentSchema, + table: currentTable, + }); + } + setRefresh(refresh + 1); } function renderSelectRow(select: ReactNode, refreshBtn: ReactNode) { return (
{select} - {refreshBtn} + {refreshBtn}
); } + const internalDbChange = ( + db: + | { + id: number; + database_name: string; + backend: string; + } + | undefined, + ) => { + setCurrentDbId(db?.id); + if (onDbChange) { + onDbChange(db); + } + }; + + const internalSchemaChange = (schema?: string) => { + setCurrentSchema(schema); + if (onSchemaChange) { + onSchemaChange(schema); + } + }; + function renderDatabaseSelector() { return ( = ({ ); } + const handleFilterOption = useMemo( + () => (search: string, option: TableOption) => { + const searchValue = search.trim().toLowerCase(); + const { text } = option; + return text.toLowerCase().includes(searchValue); + }, + [], + ); + function renderTableSelect() { - const options = tableOptions; - let select = null; - if (currentSchema && !formMode) { - // dataset editor - select = ( - + ); + const refresh = !formMode && !readOnly && ( changeSchema({ value: schema }, true)} + onClick={onRefresh} tooltipContent={t('Force refresh table list')} /> ); - return renderSelectRow(select, refresh); - } - function renderSeeTableLabel() { - return ( -
- - {t('See table schema')}{' '} - {schema && ( - - {tableOptions.length} in {schema} - - )} - -
- ); + return renderSelectRow(select, refresh); } return ( {renderDatabaseSelector()} - {!formMode &&
} - {sqlLabMode && renderSeeTableLabel()} - {formMode && {t('Table')}} + {sqlLabMode && !formMode &&
} {renderTableSelect()} ); diff --git a/superset-frontend/src/components/WarningIconWithTooltip/index.tsx b/superset-frontend/src/components/WarningIconWithTooltip/index.tsx index f160ade50..f732554e1 100644 --- a/superset-frontend/src/components/WarningIconWithTooltip/index.tsx +++ b/superset-frontend/src/components/WarningIconWithTooltip/index.tsx @@ -18,16 +18,17 @@ */ import React from 'react'; import { useTheme, SafeMarkdown } 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 WarningIconWithTooltipProps { warningMarkdown: string; - size?: number; + size?: IconType['iconSize']; } function WarningIconWithTooltip({ warningMarkdown, + size, }: WarningIconWithTooltipProps) { const theme = useTheme(); return ( @@ -37,6 +38,7 @@ function WarningIconWithTooltip({ > diff --git a/superset-frontend/src/datasource/DatasourceEditor.jsx b/superset-frontend/src/datasource/DatasourceEditor.jsx index e11b8310b..d8a0a3425 100644 --- a/superset-frontend/src/datasource/DatasourceEditor.jsx +++ b/superset-frontend/src/datasource/DatasourceEditor.jsx @@ -775,41 +775,47 @@ class DatasourceEditor extends React.PureComponent {
{this.state.isSqla && ( <> - - this.state.isEditMode && - this.onDatasourcePropChange('schema', schema) + + + + this.state.isEditMode && + this.onDatasourcePropChange('schema', schema) + } + onDbChange={database => + this.state.isEditMode && + this.onDatasourcePropChange('database', database) + } + formMode={false} + handleError={this.props.addDangerToast} + readOnly={!this.state.isEditMode} + /> +
+ } + /> +
+ { + this.onDatasourcePropChange('table_name', table); + }} + placeholder={t('Dataset name')} + disabled={!this.state.isEditMode} + /> } - onDbChange={database => - this.state.isEditMode && - this.onDatasourcePropChange('database', database) - } - formMode={false} - handleError={this.props.addDangerToast} - readOnly={!this.state.isEditMode} /> - } - /> - { - this.onDatasourcePropChange('table_name', table); - }} - placeholder={t('Dataset name')} - disabled={!this.state.isEditMode} - /> - } - /> +
+ - this.onDatasourcePropChange('schema', schema) - : undefined - } - onDbChange={ - this.state.isEditMode - ? database => - this.onDatasourcePropChange('database', database) - : undefined - } - onTableChange={ - this.state.isEditMode - ? table => - this.onDatasourcePropChange('table_name', table) - : undefined - } - readOnly={!this.state.isEditMode} - /> +
+ + this.onDatasourcePropChange('schema', schema) + : undefined + } + onDbChange={ + this.state.isEditMode + ? database => + this.onDatasourcePropChange( + 'database', + database, + ) + : undefined + } + onTableChange={ + this.state.isEditMode + ? table => + this.onDatasourcePropChange('table_name', table) + : undefined + } + readOnly={!this.state.isEditMode} + /> +
} description={t( 'The pointer to a physical table (or view). Keep in mind that the chart is ' + diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx index 9278c29e7..3df55323d 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx @@ -227,10 +227,7 @@ class DatasourceControl extends React.PureComponent { )} {extra?.warning_markdown && ( - + )} = ({ )} {parsedExtra?.warning_markdown && ( )} {titleLink} diff --git a/superset/datasets/api.py b/superset/datasets/api.py index 8d8eb6efe..261a7d749 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -160,7 +160,7 @@ class DatasetRestApi(BaseSupersetModelRestApi): "url", "extra", ] - show_columns = show_select_columns + ["columns.type_generic"] + show_columns = show_select_columns + ["columns.type_generic", "database.backend"] add_model_schema = DatasetPostSchema() edit_model_schema = DatasetPutSchema() add_columns = ["database", "schema", "table_name", "owners"] diff --git a/superset/views/core.py b/superset/views/core.py index dc4a8a2da..836bee491 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1064,8 +1064,14 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods @event_logger.log_this @expose("/tables////") @expose("/tables/////") - def tables( # pylint: disable=too-many-locals,no-self-use - self, db_id: int, schema: str, substr: str, force_refresh: str = "false" + @expose("/tables/////") + def tables( # pylint: disable=too-many-locals,no-self-use,too-many-arguments + self, + db_id: int, + schema: str, + substr: str, + force_refresh: str = "false", + exact_match: str = "false", ) -> FlaskResponse: """Endpoint to fetch the list of tables for given database""" # Guarantees database filtering by security access @@ -1078,6 +1084,7 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods return json_error_response("Not found", 404) force_refresh_parsed = force_refresh.lower() == "true" + exact_match_parsed = exact_match.lower() == "true" schema_parsed = utils.parse_js_uri_path_item(schema, eval_undefined=True) substr_parsed = utils.parse_js_uri_path_item(substr, eval_undefined=True) @@ -1119,9 +1126,15 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods ds_name.table if schema_parsed else f"{ds_name.schema}.{ds_name.table}" ) + def is_match(src: str, target: utils.DatasourceName) -> bool: + target_label = get_datasource_label(target) + if exact_match_parsed: + return src == target_label + return src in target_label + if substr_parsed: - tables = [tn for tn in tables if substr_parsed in get_datasource_label(tn)] - views = [vn for vn in views if substr_parsed in get_datasource_label(vn)] + tables = [tn for tn in tables if is_match(substr_parsed, tn)] + views = [vn for vn in views if is_match(substr_parsed, vn)] if not schema_parsed and database.default_schemas: user_schemas = ( diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index 385025e38..c9701534b 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -222,6 +222,7 @@ class TestDatasetApi(SupersetTestCase): Dataset API: Test get dataset item """ table = self.get_energy_usage_dataset() + main_db = get_main_database() self.login(username="admin") uri = f"api/v1/dataset/{table.id}" rv = self.get_assert_metric(uri, "get") @@ -229,7 +230,11 @@ class TestDatasetApi(SupersetTestCase): response = json.loads(rv.data.decode("utf-8")) expected_result = { "cache_timeout": None, - "database": {"database_name": "examples", "id": 1}, + "database": { + "backend": main_db.backend, + "database_name": "examples", + "id": 1, + }, "default_endpoint": None, "description": "Energy consumption", "extra": None, @@ -244,9 +249,10 @@ class TestDatasetApi(SupersetTestCase): "table_name": "energy_usage", "template_params": None, } - assert { - k: v for k, v in response["result"].items() if k in expected_result - } == expected_result + if response["result"]["database"]["backend"] not in ("presto", "hive"): + assert { + k: v for k, v in response["result"].items() if k in expected_result + } == expected_result assert len(response["result"]["columns"]) == 3 assert len(response["result"]["metrics"]) == 2