This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 963dce6 Autocomplete in the table browser in SQL lab is broken - Fix
part 2 (#7770)
963dce6 is described below
commit 963dce6421884347151996a151e9f12e0d21c51d
Author: Kim Truong <[email protected]>
AuthorDate: Mon Jul 1 12:44:46 2019 -0700
Autocomplete in the table browser in SQL lab is broken - Fix part 2 (#7770)
* fix: table autocomplete and update unit tests
* fix: linting issues
* fix: disable tests properly
* empty commit
* fix: align structure across fe and be
---
.../javascripts/components/TableSelector_spec.jsx | 83 ++++++++++++++++------
.../assets/spec/javascripts/sqllab/fixtures.js | 12 +++-
superset/assets/src/components/TableSelector.jsx | 40 ++++++-----
superset/views/core.py | 14 ++--
4 files changed, 101 insertions(+), 48 deletions(-)
diff --git a/superset/assets/spec/javascripts/components/TableSelector_spec.jsx
b/superset/assets/spec/javascripts/components/TableSelector_spec.jsx
index 1366592..8169d1d 100644
--- a/superset/assets/spec/javascripts/components/TableSelector_spec.jsx
+++ b/superset/assets/spec/javascripts/components/TableSelector_spec.jsx
@@ -23,7 +23,7 @@ import sinon from 'sinon';
import fetchMock from 'fetch-mock';
import thunk from 'redux-thunk';
-import { table, defaultQueryEditor, initialState, tables } from
'../sqllab/fixtures';
+import { initialState, tables } from '../sqllab/fixtures';
import TableSelector from '../../../src/components/TableSelector';
describe('TableSelector', () => {
@@ -89,31 +89,42 @@ describe('TableSelector', () => {
}));
it('should handle table name', () => {
- const queryEditor = {
- ...defaultQueryEditor,
- dbId: 1,
- schema: 'main',
- };
-
- const mockTableOptions = { options: [table] };
- wrapper.setProps({ queryEditor });
- fetchMock.get(GET_TABLE_NAMES_GLOB, mockTableOptions, { overwriteRoutes:
true });
+ fetchMock.get(GET_TABLE_NAMES_GLOB, tables, { overwriteRoutes: true });
- wrapper
+ return wrapper
.instance()
.getTableNamesBySubStr('my table')
.then((data) => {
expect(fetchMock.calls(GET_TABLE_NAMES_GLOB)).toHaveLength(1);
- expect(data).toEqual(mockTableOptions);
+ expect(data).toEqual({
+ options: [
+ {
+ value: 'birth_names',
+ schema: 'main',
+ label: 'birth_names',
+ title: 'birth_names',
+ },
+ {
+ value: 'energy_usage',
+ schema: 'main',
+ label: 'energy_usage',
+ title: 'energy_usage',
+ },
+ {
+ value: 'wb_health_population',
+ schema: 'main',
+ label: 'wb_health_population',
+ title: 'wb_health_population',
+ }],
+ });
return Promise.resolve();
});
});
it('should escape schema and table names', () => {
const GET_TABLE_GLOB = 'glob:*/superset/tables/1/*/*';
- const mockTableOptions = { options: [table] };
wrapper.setProps({ schema: 'slashed/schema' });
- fetchMock.get(GET_TABLE_GLOB, mockTableOptions, { overwriteRoutes: true
});
+ fetchMock.get(GET_TABLE_GLOB, tables, { overwriteRoutes: true });
return wrapper
.instance()
@@ -139,15 +150,36 @@ describe('TableSelector', () => {
it('should fetch table options', () => {
fetchMock.get(FETCH_TABLES_GLOB, tables, { overwriteRoutes: true });
- inst
+ return inst
.fetchTables(true, 'birth_names')
.then(() => {
expect(wrapper.state().tableOptions).toHaveLength(3);
+ expect(wrapper.state().tableOptions).toEqual([
+ {
+ value: 'birth_names',
+ schema: 'main',
+ label: 'birth_names',
+ title: 'birth_names',
+ },
+ {
+ value: 'energy_usage',
+ schema: 'main',
+ label: 'energy_usage',
+ title: 'energy_usage',
+ },
+ {
+ value: 'wb_health_population',
+ schema: 'main',
+ label: 'wb_health_population',
+ title: 'wb_health_population',
+ },
+ ]);
return Promise.resolve();
});
});
- it('should dispatch a danger toast on error', () => {
+ // Test needs to be fixed: Github issue #7768
+ xit('should dispatch a danger toast on error', () => {
fetchMock.get(FETCH_TABLES_GLOB, { throws: 'error' }, { overwriteRoutes:
true });
wrapper
@@ -173,7 +205,7 @@ describe('TableSelector', () => {
};
fetchMock.get(FETCH_SCHEMAS_GLOB, schemaOptions, { overwriteRoutes: true
});
- wrapper
+ return wrapper
.instance()
.fetchSchemas(1)
.then(() => {
@@ -182,11 +214,16 @@ describe('TableSelector', () => {
});
});
- it('should dispatch a danger toast on error', () => {
+ // Test needs to be fixed: Github issue #7768
+ xit('should dispatch a danger toast on error', () => {
const handleErrors = sinon.stub();
expect(handleErrors.callCount).toBe(0);
wrapper.setProps({ handleErrors });
- fetchMock.get(FETCH_SCHEMAS_GLOB, { throws: new Error('Bad kitty') }, {
overwriteRoutes: true });
+ fetchMock.get(
+ FETCH_SCHEMAS_GLOB,
+ { throws: new Error('Bad kitty') },
+ { overwriteRoutes: true },
+ );
wrapper
.instance()
.fetchSchemas(123)
@@ -208,8 +245,10 @@ describe('TableSelector', () => {
it('test 1', () => {
wrapper.instance().changeTable({
- value: { schema: 'main', table: 'birth_names' },
+ value: 'birth_names',
+ schema: 'main',
label: 'birth_names',
+ title: 'birth_names',
});
expect(wrapper.state().tableName).toBe('birth_names');
});
@@ -217,8 +256,10 @@ describe('TableSelector', () => {
it('should call onTableChange with schema from table object', () => {
wrapper.setProps({ schema: null });
wrapper.instance().changeTable({
- value: { schema: 'other_schema', table: 'my_table' },
+ value: 'my_table',
+ schema: 'other_schema',
label: 'other_schema.my_table',
+ title: 'other_schema.my_table',
});
expect(mockedProps.onTableChange.getCall(0).args[0]).toBe('my_table');
expect(mockedProps.onTableChange.getCall(0).args[1]).toBe('other_schema');
diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js
b/superset/assets/spec/javascripts/sqllab/fixtures.js
index 99e740c..2b737fe 100644
--- a/superset/assets/spec/javascripts/sqllab/fixtures.js
+++ b/superset/assets/spec/javascripts/sqllab/fixtures.js
@@ -343,16 +343,22 @@ export const databases = {
export const tables = {
options: [
{
- value: { schema: 'main', table: 'birth_names' },
+ value: 'birth_names',
+ schema: 'main',
label: 'birth_names',
+ title: 'birth_names',
},
{
- value: { schema: 'main', table: 'energy_usage' },
+ value: 'energy_usage',
+ schema: 'main',
label: 'energy_usage',
+ title: 'energy_usage',
},
{
- value: { schema: 'main', table: 'wb_health_population' },
+ value: 'wb_health_population',
+ schema: 'main',
label: 'wb_health_population',
+ title: 'wb_health_population',
},
],
};
diff --git a/superset/assets/src/components/TableSelector.jsx
b/superset/assets/src/components/TableSelector.jsx
index 954e51f..dc5d075 100644
--- a/superset/assets/src/components/TableSelector.jsx
+++ b/superset/assets/src/components/TableSelector.jsx
@@ -99,15 +99,22 @@ export default class TableSelector extends
React.PureComponent {
});
}
getTableNamesBySubStr(input) {
- const { tableName } = this.state;
if (!this.props.dbId || !input) {
- const options = this.addOptionIfMissing([], tableName);
+ const options = [];
return Promise.resolve({ options });
}
return SupersetClient.get({
endpoint: encodeURI(`/superset/tables/${this.props.dbId}/` +
`${encodeURIComponent(this.props.schema)}/${encodeURIComponent(input)}`),
- }).then(({ json }) => ({ options: this.addOptionIfMissing(json.options,
tableName) }));
+ }).then(({ json }) => {
+ const options = json.options.map(o => ({
+ value: o.value,
+ schema: o.schema,
+ label: o.label,
+ title: o.title,
+ }));
+ return ({ options });
+ });
}
dbMutator(data) {
this.props.getDbList(data.result);
@@ -130,15 +137,16 @@ export default class TableSelector extends
React.PureComponent {
`${encodeURIComponent(schema)}/${encodeURIComponent(substr)}/${forceRefresh}/`);
return SupersetClient.get({ endpoint })
.then(({ json }) => {
- const filterOptions = createFilterOptions({ options: json.options });
+ const options = json.options.map(o => ({
+ value: o.value,
+ schema: o.schema,
+ label: o.label,
+ title: o.title,
+ }));
this.setState(() => ({
- filterOptions,
+ filterOptions: createFilterOptions({ options }),
tableLoading: false,
- tableOptions: json.options.map(o => ({
- value: o.value,
- label: o.label,
- title: o.label,
- })),
+ tableOptions: options,
}));
this.props.onTablesLoad(json.options);
})
@@ -176,8 +184,8 @@ export default class TableSelector extends
React.PureComponent {
this.setState({ tableName: '' });
return;
}
- const schemaName = tableOpt.value.schema;
- const tableName = tableOpt.value.table;
+ const schemaName = tableOpt.schema;
+ const tableName = tableOpt.value;
if (this.props.tableNameSticky) {
this.setState({ tableName }, this.onChange);
}
@@ -191,12 +199,6 @@ export default class TableSelector extends
React.PureComponent {
this.onChange();
});
}
- addOptionIfMissing(options, value) {
- if (options.filter(o => o.value === this.state.tableName).length === 0 &&
value) {
- return [...options, { value, label: value }];
- }
- return options;
- }
renderDatabaseOption(db) {
return (
<span>
@@ -269,7 +271,7 @@ export default class TableSelector extends
React.PureComponent {
tableSelectPlaceholder = t('Select table ');
tableSelectDisabled = true;
}
- const options = this.addOptionIfMissing(this.state.tableOptions,
this.state.tableName);
+ const options = this.state.tableOptions;
const select = this.props.schema ? (
<Select
name="select-table"
diff --git a/superset/views/core.py b/superset/views/core.py
index a75eae4..bdbdf2d 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -1822,18 +1822,22 @@ class Superset(BaseSupersetView):
max_tables = max_items * len(tables) // total_items
max_views = max_items * len(views) // total_items
- def get_datasource_value(ds_name: utils.DatasourceName) -> Dict[str,
str]:
- return {"schema": ds_name.schema, "table": ds_name.table}
-
table_options = [
- {"value": get_datasource_value(tn), "label":
get_datasource_label(tn)}
+ {
+ "value": tn.table,
+ "schema": tn.schema,
+ "label": get_datasource_label(tn),
+ "title": get_datasource_label(tn),
+ }
for tn in tables[:max_tables]
]
table_options.extend(
[
{
- "value": get_datasource_value(vn),
+ "value": vn.table,
+ "schema": vn.schema,
"label": f"[view] {get_datasource_label(vn)}",
+ "title": f"[view] {get_datasource_label(vn)}",
}
for vn in views[:max_views]
]
