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

Filtering out SQLLab views out of table list view by default #4746

Merged
merged 3 commits into from
Apr 12, 2018
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
86 changes: 47 additions & 39 deletions superset/assets/javascripts/addSlice/AddSliceContainer.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import PropTypes from 'prop-types';
import { Button, Panel, Grid, Row, Col } from 'react-bootstrap';
import { Button, Panel } from 'react-bootstrap';
import Select from 'react-virtualized-select';
import visTypes from '../explore/stores/visTypes';
import { t } from '../locales';
Expand All @@ -12,6 +12,8 @@ const propTypes = {
})).isRequired,
};

const styleSelectWidth = { width: 300 };

export default class AddSliceContainer extends React.PureComponent {
constructor(props) {
super(props);
Expand Down Expand Up @@ -55,44 +57,50 @@ export default class AddSliceContainer extends React.PureComponent {
return (
<div className="container">
<Panel header={<h3>{t('Create a new slice')}</h3>}>
<Grid>
<Row>
<Col xs={12} sm={6}>
<div>
<p>{t('Choose a datasource')}</p>
<Select
clearable={false}
name="select-datasource"
onChange={this.changeDatasource.bind(this)}
options={this.props.datasources}
placeholder={t('Choose a datasource')}
value={this.state.datasourceValue}
/>
</div>
<br />
<div>
<p>{t('Choose a visualization type')}</p>
<Select
clearable={false}
name="select-vis-type"
onChange={this.changeVisType.bind(this)}
options={this.vizTypeOptions}
placeholder={t('Choose a visualization type')}
value={this.state.visType}
/>
</div>
<br />
<Button
bsStyle="primary"
disabled={this.isBtnDisabled()}
onClick={this.gotoSlice.bind(this)}
>
{t('Create new slice')}
</Button>
<br /><br />
</Col>
</Row>
</Grid>
<div>
<p>{t('Choose a datasource')}</p>
<div style={styleSelectWidth}>
<Select
clearable={false}
style={styleSelectWidth}
name="select-datasource"
onChange={this.changeDatasource.bind(this)}
options={this.props.datasources}
placeholder={t('Choose a datasource')}
value={this.state.datasourceValue}
width={200}
/>
</div>
<p className="text-muted">
{t(
'If the datasource your are looking for is not ' +
'available in the list, ' +
'follow the instructions on the how to add it on the ')}
<a href="http://superset.apache.org/tutorial.html">{t('Superset tutorial')}</a>
</p>
</div>
<br />
<div>
<p>{t('Choose a visualization type')}</p>
<Select
clearable={false}
name="select-vis-type"
style={styleSelectWidth}
onChange={this.changeVisType.bind(this)}
options={this.vizTypeOptions}
placeholder={t('Choose a visualization type')}
value={this.state.visType}
/>
</div>
<br />
<Button
bsStyle="primary"
disabled={this.isBtnDisabled()}
onClick={this.gotoSlice.bind(this)}
>
{t('Create new slice')}
</Button>
<br /><br />
</Panel>
</div>
);
Expand Down
4 changes: 4 additions & 0 deletions superset/connectors/base/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ def values_for_column(self, column_name, limit=10000):
values in filters in the explore view"""
raise NotImplementedError()

@staticmethod
def default_query(qry):
return qry


class BaseColumn(AuditMixinNullable, ImportMixin):
"""Interface for column"""
Expand Down
6 changes: 4 additions & 2 deletions superset/connectors/connector_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ def get_datasource(cls, datasource_type, datasource_id, session):
def get_all_datasources(cls, session):
datasources = []
for source_type in ConnectorRegistry.sources:
datasources.extend(
session.query(ConnectorRegistry.sources[source_type]).all())
source_class = ConnectorRegistry.sources[source_type]
qry = session.query(source_class)
qry = source_class.default_query(qry)
datasources.extend(qry.all())
return datasources

@classmethod
Expand Down
5 changes: 5 additions & 0 deletions superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ class SqlaTable(Model, BaseDatasource):
foreign_keys=[database_id])
schema = Column(String(255))
sql = Column(Text)
is_sqllab_view = Column(Boolean, default=False)

baselink = 'tablemodelview'

Expand Down Expand Up @@ -819,6 +820,10 @@ def query_datasources_by_name(
query = query.filter_by(schema=schema)
return query.all()

@staticmethod
def default_query(qry):
return qry.filter_by(is_sqllab_view=False)


sa.event.listen(SqlaTable, 'after_insert', set_perm)
sa.event.listen(SqlaTable, 'after_update', set_perm)
21 changes: 14 additions & 7 deletions superset/connectors/sqla/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,15 @@ class TableModelView(DatasourceModelView, DeleteMixin, YamlExportMixin): # noqa
'table_name', 'sql', 'filter_select_enabled', 'slices',
'fetch_values_predicate', 'database', 'schema',
'description', 'owner',
'main_dttm_col', 'default_endpoint', 'offset', 'cache_timeout']
'main_dttm_col', 'default_endpoint', 'offset', 'cache_timeout',
'is_sqllab_view',
]
base_filters = [['id', DatasourceFilter, lambda: []]]
show_columns = edit_columns + ['perm']
related_views = [TableColumnInlineView, SqlMetricInlineView]
base_order = ('changed_on', 'desc')
search_columns = (
'database', 'schema', 'table_name', 'owner',
'database', 'schema', 'table_name', 'owner', 'is_sqllab_view',
)
description_columns = {
'slices': _(
Expand Down Expand Up @@ -213,8 +216,10 @@ class TableModelView(DatasourceModelView, DeleteMixin, YamlExportMixin): # noqa
"Whether to populate the filter's dropdown in the explore "
"view's filter section with a list of distinct values fetched "
'from the backend on the fly'),
'is_sqllab_view': _(
Copy link
Member

Choose a reason for hiding this comment

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

Is there anyway to infer this rather than it be a form option?

Copy link
Member Author

Choose a reason for hiding this comment

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

I could remove it from the edit_columns and have it only in the show_columns if you think it's better

"Whether the table was generated by the 'Visualize' flow "
'in SQL Lab'),
}
base_filters = [['id', DatasourceFilter, lambda: []]]
label_columns = {
'slices': _('Associated Charts'),
'link': _('Table'),
Expand All @@ -231,6 +236,7 @@ class TableModelView(DatasourceModelView, DeleteMixin, YamlExportMixin): # noqa
'owner': _('Owner'),
'main_dttm_col': _('Main Datetime Column'),
'description': _('Description'),
'is_sqllab_view': _('SQL Lab View'),
}

def pre_add(self, table):
Expand Down Expand Up @@ -296,13 +302,14 @@ def refresh(self, tables):
return redirect('/tablemodelview/list/')


appbuilder.add_view(
TableModelView,
appbuilder.add_view_no_menu(TableModelView)
appbuilder.add_link(
'Tables',
label=__('Tables'),
href='/tablemodelview/list/?_flt_1_is_sqllab_view=y',
icon='fa-upload',
category='Sources',
category_label=__('Sources'),
icon='fa-table',
)
category_icon='fa-table')

appbuilder.add_separator('Sources')
54 changes: 54 additions & 0 deletions superset/migrations/versions/130915240929_is_sqllab_viz_flow.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
"""is_sqllab_view

Revision ID: 130915240929
Revises: f231d82b9b26
Create Date: 2018-04-03 08:19:34.098789

"""
from alembic import op
import sqlalchemy as sa
from sqlalchemy.ext.declarative import declarative_base

from superset import db

# revision identifiers, used by Alembic.
revision = '130915240929'
down_revision = 'f231d82b9b26'

Base = declarative_base()


class Table(Base):
"""Declarative class to do query in upgrade"""
__tablename__ = 'tables'
id = sa.Column(sa.Integer, primary_key=True)
sql = sa.Column(sa.Text)
is_sqllab_view = sa.Column(sa.Boolean())


def upgrade():
bind = op.get_bind()
op.add_column(
'tables',
sa.Column(
'is_sqllab_view',
sa.Boolean(),
nullable=True,
default=False,
server_default=sa.false(),
),
)

session = db.Session(bind=bind)

# Use Slice class defined here instead of models.Slice
for tbl in session.query(Table).all():
if tbl.sql:
tbl.is_sqllab_view = True
Copy link
Member

Choose a reason for hiding this comment

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

I guess the presence of custom SQL doesn't always imply that the SQL corresponds to a SQL Lab view.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, I'm not sure how else to identify it. It's the best proxy we have. Note that this does not change any behavior other than being filtered out by default in the Table list view, but the filter can be removed anyways, so I didn't think it had to be perfect. Goal here is just to hide some of the clutter this creates.

session.merge(tbl)
session.commit()
db.session.close()


def downgrade():
op.drop_column('tables', 'is_sqllab_view')
1 change: 1 addition & 0 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -2162,6 +2162,7 @@ def sqllab_viz(self):
table = SqlaTable(table_name=table_name)
table.database_id = data.get('dbId')
table.schema = data.get('schema')
table.is_sqllab_view = True
q = SupersetQuery(data.get('sql'))
table.sql = q.stripped()
db.session.add(table)
Expand Down