From 0ad9cbb83b3a70ad31b0de168b13ab74cd901bb4 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Thu, 20 Sep 2018 13:51:39 -0700 Subject: [PATCH] [explore] add "View samples" modal to action buttons (#5770) * [explore] add "View samples" modal to action buttons Also broke down the `View query` and `View results` as different request so that viewing the query does not require fetching the results anymore * fix js tests * lint (cherry picked from commit 73d1e4596de21ac3c3be63e44d1afa656a7ac460) --- .../components/DisplayQueryButton_spec.jsx | 2 +- .../javascripts/sqllab/TableElement_spec.jsx | 4 +- .../explore/components/DisplayQueryButton.jsx | 103 +++++++++++++----- .../src/explore/components/RowCountLabel.jsx | 7 +- superset/assets/src/explore/exploreUtils.js | 8 +- superset/assets/stylesheets/superset.less | 7 +- superset/views/base.py | 6 +- superset/views/core.py | 69 ++++++++---- superset/viz.py | 29 +++-- 9 files changed, 170 insertions(+), 65 deletions(-) diff --git a/superset/assets/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx b/superset/assets/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx index ab43ddf8643c7..fd42912bff1a9 100644 --- a/superset/assets/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx +++ b/superset/assets/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx @@ -25,6 +25,6 @@ describe('DisplayQueryButton', () => { }); it('renders a dropdown', () => { const wrapper = mount(); - expect(wrapper.find(ModalTrigger)).to.have.lengthOf(2); + expect(wrapper.find(ModalTrigger)).to.have.lengthOf(3); }); }); diff --git a/superset/assets/spec/javascripts/sqllab/TableElement_spec.jsx b/superset/assets/spec/javascripts/sqllab/TableElement_spec.jsx index 9909bd6af0a63..b5d8b768f5356 100644 --- a/superset/assets/spec/javascripts/sqllab/TableElement_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/TableElement_spec.jsx @@ -54,8 +54,6 @@ describe('TableElement', () => { expect(wrapper.state().expanded).to.equal(true); wrapper.find('.table-remove').simulate('click'); expect(wrapper.state().expanded).to.equal(false); - setTimeout(() => { - expect(mockedActions.removeTable.called).to.equal(true); - }, 10); + expect(mockedActions.removeDataPreview.called).to.equal(true); }); }); diff --git a/superset/assets/src/explore/components/DisplayQueryButton.jsx b/superset/assets/src/explore/components/DisplayQueryButton.jsx index 9c8454d32237a..b7f293f1ebad9 100644 --- a/superset/assets/src/explore/components/DisplayQueryButton.jsx +++ b/superset/assets/src/explore/components/DisplayQueryButton.jsx @@ -6,9 +6,9 @@ import markdown from 'react-syntax-highlighter/languages/hljs/markdown'; import sql from 'react-syntax-highlighter/languages/hljs/sql'; import json from 'react-syntax-highlighter/languages/hljs/json'; import github from 'react-syntax-highlighter/styles/hljs/github'; -import { DropdownButton, MenuItem } from 'react-bootstrap'; -import { BootstrapTable, TableHeaderColumn } from 'react-bootstrap-table'; -import 'react-bootstrap-table/css/react-bootstrap-table.css'; +import { DropdownButton, MenuItem, Row, Col, FormControl } from 'react-bootstrap'; +import { Table } from 'reactable'; +import $ from 'jquery'; import CopyToClipboard from './../../components/CopyToClipboard'; import { getExploreUrlAndPayload } from '../exploreUtils'; @@ -17,14 +17,13 @@ import Loading from '../../components/Loading'; import ModalTrigger from './../../components/ModalTrigger'; import Button from '../../components/Button'; import { t } from '../../locales'; +import RowCountLabel from './RowCountLabel'; registerLanguage('markdown', markdown); registerLanguage('html', html); registerLanguage('sql', sql); registerLanguage('json', json); -const $ = (window.$ = require('jquery')); - const propTypes = { onOpenInEditor: PropTypes.func, animation: PropTypes.bool, @@ -46,15 +45,17 @@ export default class DisplayQueryButton extends React.PureComponent { data: null, isLoading: false, error: null, + filterText: '', sqlSupported: datasource && datasource.split('__')[1] === 'table', }; this.beforeOpen = this.beforeOpen.bind(this); + this.changeFilterText = this.changeFilterText.bind(this); } - beforeOpen() { + beforeOpen(endpointType) { this.setState({ isLoading: true }); const { url, payload } = getExploreUrlAndPayload({ formData: this.props.latestQueryFormData, - endpointType: 'query', + endpointType, }); $.ajax({ type: 'POST', @@ -79,6 +80,9 @@ export default class DisplayQueryButton extends React.PureComponent { }, }); } + changeFilterText(event) { + this.setState({ filterText: event.target.value }); + } redirectSQLLab() { this.props.onOpenInEditor(this.props.latestQueryFormData); } @@ -111,7 +115,7 @@ export default class DisplayQueryButton extends React.PureComponent { if (this.state.isLoading) { return (Loading...); } else if (this.state.error) { @@ -120,33 +124,72 @@ export default class DisplayQueryButton extends React.PureComponent { if (this.state.data.length === 0) { return 'No data'; } - const headers = Object.keys(this.state.data[0]).map((k, i) => ( - {k} - )); - return ( - - {headers} - - ); + return this.renderDataTable(this.state.data); + } + return null; + } + renderDataTable(data) { + return ( +
+ + + + + + + + + + + ); + } + renderSamplesModalBody() { + if (this.state.isLoading) { + return (Loading...); + } else if (this.state.error) { + return
{this.state.error}
; + } else if (this.state.data) { + return this.renderDataTable(this.state.data); } return null; } render() { return ( - + +   + } + bsSize="sm" + pullRight + id="query" + > {t('View query')}} modalTitle={t('View query')} bsSize="large" - beforeOpen={this.beforeOpen} + beforeOpen={() => this.beforeOpen('query')} modalBody={this.renderQueryModalBody()} eventKey="1" /> @@ -156,10 +199,20 @@ export default class DisplayQueryButton extends React.PureComponent { triggerNode={{t('View results')}} modalTitle={t('View results')} bsSize="large" - beforeOpen={this.beforeOpen} + beforeOpen={() => this.beforeOpen('results')} modalBody={this.renderResultsModalBody()} eventKey="2" /> + {t('View samples')}} + modalTitle={t('View samples')} + bsSize="large" + beforeOpen={() => this.beforeOpen('samples')} + modalBody={this.renderSamplesModalBody()} + eventKey="2" + /> {this.state.sqlSupported && - {formattedRowCount} rows + {formattedRowCount}{' '}{suffix} ); diff --git a/superset/assets/src/explore/exploreUtils.js b/superset/assets/src/explore/exploreUtils.js index fcab33f121acb..dcf562d13b038 100644 --- a/superset/assets/src/explore/exploreUtils.js +++ b/superset/assets/src/explore/exploreUtils.js @@ -22,7 +22,7 @@ export function getAnnotationJsonUrl(slice_id, form_data, isNative) { export function getURIDirectory(formData, endpointType = 'base') { // Building the directory part of the URI let directory = '/superset/explore/'; - if (['json', 'csv', 'query'].indexOf(endpointType) >= 0) { + if (['json', 'csv', 'query', 'results', 'samples'].indexOf(endpointType) >= 0) { directory = '/superset/explore_json/'; } return directory; @@ -81,6 +81,12 @@ export function getExploreUrlAndPayload({ if (endpointType === 'query') { search.query = 'true'; } + if (endpointType === 'results') { + search.results = 'true'; + } + if (endpointType === 'samples') { + search.samples = 'true'; + } const paramNames = Object.keys(requestParams); if (paramNames.length) { paramNames.forEach((name) => { diff --git a/superset/assets/stylesheets/superset.less b/superset/assets/stylesheets/superset.less index ef729577446ae..6c52282589eb4 100644 --- a/superset/assets/stylesheets/superset.less +++ b/superset/assets/stylesheets/superset.less @@ -425,8 +425,8 @@ g.annotation-container { font: normal normal normal 14px/1 FontAwesome; content: "\f0dc"; position: absolute; - top: 17px; - right: 15px; + top: 6px; + right: 5px; color: @brand-primary; } .reactable-header-sort-asc::before{ @@ -437,6 +437,9 @@ g.annotation-container { content: "\f0dd"; color: @brand-primary; } +tr.reactable-column-header th.reactable-header-sortable { + padding-right: 17px; +} .explore-chart-overlay { position: absolute; diff --git a/superset/views/base.py b/superset/views/base.py index a809f4d4b4d54..f41315f5e1cda 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -7,7 +7,6 @@ from datetime import datetime import functools -import json import logging import traceback @@ -19,6 +18,7 @@ from flask_babel import get_locale from flask_babel import gettext as __ from flask_babel import lazy_gettext as _ +import simplejson as json import yaml from superset import conf, db, security_manager, utils @@ -52,7 +52,7 @@ def json_error_response(msg=None, status=500, stacktrace=None, payload=None, lin payload['link'] = link return Response( - json.dumps(payload, default=utils.json_iso_dttm_ser), + json.dumps(payload, default=utils.json_iso_dttm_ser, ignore_nan=True), status=status, mimetype='application/json') @@ -99,7 +99,7 @@ class BaseSupersetView(BaseView): def json_response(self, obj, status=200): return Response( - json.dumps(obj, default=utils.json_int_dttm_ser), + json.dumps(obj, default=utils.json_int_dttm_ser, ignore_nan=True), status=status, mimetype='application/json') diff --git a/superset/views/core.py b/superset/views/core.py index c5cfa4dd226cb..8ed539c9f81d5 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1050,17 +1050,26 @@ def get_query_string_response(self, viz_obj): else: query = 'No query.' - return Response( - json.dumps({ - 'query': query, - 'language': viz_obj.datasource.query_language, - 'data': viz_obj.get_df().to_dict('records'), # TODO, split into endpoint - }, default=utils.json_iso_dttm_ser), - status=200, - mimetype='application/json') + return self.json_response({ + 'query': query, + 'language': viz_obj.datasource.query_language, + }) + + def get_raw_results(self, viz_obj): + return self.json_response({ + 'data': viz_obj.get_df().to_dict('records'), + }) + + def get_samples(self, viz_obj): + return self.json_response({ + 'data': viz_obj.get_samples(), + }) - def generate_json(self, datasource_type, datasource_id, form_data, - csv=False, query=False, force=False): + def generate_json( + self, datasource_type, datasource_id, form_data, + csv=False, query=False, force=False, results=False, + samples=False, + ): try: viz_obj = self.get_viz( datasource_type=datasource_type, @@ -1090,6 +1099,12 @@ def generate_json(self, datasource_type, datasource_id, form_data, if query: return self.get_query_string_response(viz_obj) + if results: + return self.get_raw_results(viz_obj) + + if samples: + return self.get_samples(viz_obj) + try: payload = viz_obj.get_payload() except SupersetException as se: @@ -1156,10 +1171,22 @@ def annotation_json(self, layer_id): @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) def explore_json(self, datasource_type=None, datasource_id=None): + """Serves all request that GET or POST form_data + + This endpoint evolved to be the entry point of many different + requests that GETs or POSTs a form_data. + + `self.generate_json` receives this input and returns different + payloads based on the request args in the first block + + TODO: break into one endpoint for each return shape""" + csv = request.args.get('csv') == 'true' + query = request.args.get('query') == 'true' + results = request.args.get('results') == 'true' + samples = request.args.get('samples') == 'true' + force = request.args.get('force') == 'true' + try: - csv = request.args.get('csv') == 'true' - query = request.args.get('query') == 'true' - force = request.args.get('force') == 'true' form_data = self.get_form_data()[0] datasource_id, datasource_type = self.datasource_info( datasource_id, datasource_type, form_data) @@ -1168,12 +1195,16 @@ def explore_json(self, datasource_type=None, datasource_id=None): return json_error_response( utils.error_msg_from_exception(e), stacktrace=traceback.format_exc()) - return self.generate_json(datasource_type=datasource_type, - datasource_id=datasource_id, - form_data=form_data, - csv=csv, - query=query, - force=force) + return self.generate_json( + datasource_type=datasource_type, + datasource_id=datasource_id, + form_data=form_data, + csv=csv, + query=query, + results=results, + force=force, + samples=samples, + ) @log_this @has_access diff --git a/superset/viz.py b/superset/viz.py index e2a0f27411c0b..0802ed14ec9f2 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -187,6 +187,17 @@ def get_fillna_for_columns(self, columns=None): } return fillna + def get_samples(self): + query_obj = self.query_obj() + query_obj.update({ + 'groupby': [], + 'metrics': [], + 'row_limit': 1000, + 'columns': [o.column_name for o in self.datasource.columns], + }) + df = self.get_df(query_obj) + return df.to_dict(orient='records') + def get_df(self, query_obj=None): """Returns a pandas dataframe based on the query object""" if not query_obj: @@ -240,9 +251,15 @@ def df_metrics_to_num(self, df): if dtype.type == np.object_ and col in metrics: df[col] = pd.to_numeric(df[col], errors='coerce') + def process_query_filters(self): + utils.convert_legacy_filters_into_adhoc(self.form_data) + merge_extra_filters(self.form_data) + utils.split_adhoc_filters_into_base_filters(self.form_data) + def query_obj(self): """Building a query object""" form_data = self.form_data + self.process_query_filters() gb = form_data.get('groupby') or [] if not isinstance(gb, list): gb = [gb] @@ -258,12 +275,6 @@ def query_obj(self): groupby.remove(DTTM_ALIAS) is_timeseries = True - # extras are used to query elements specific to a datasource type - # for instance the extra where clause that applies only to Tables - - utils.convert_legacy_filters_into_adhoc(form_data) - merge_extra_filters(form_data) - utils.split_adhoc_filters_into_base_filters(form_data) granularity = ( form_data.get('granularity') or form_data.get('granularity_sqla') @@ -286,8 +297,8 @@ def query_obj(self): self.from_dttm = from_dttm self.to_dttm = to_dttm - filters = form_data.get('filters', []) - + # extras are used to query elements specific to a datasource type + # for instance the extra where clause that applies only to Tables extras = { 'where': form_data.get('where', ''), 'having': form_data.get('having', ''), @@ -304,7 +315,7 @@ def query_obj(self): 'groupby': groupby, 'metrics': metrics, 'row_limit': row_limit, - 'filter': filters, + 'filter': self.form_data.get('filters', []), 'timeseries_limit': limit, 'extras': extras, 'timeseries_limit_metric': timeseries_limit_metric,