From commits-return-1320-archive-asf-public=cust-asf.ponee.io@superset.incubator.apache.org Wed Aug 15 20:25:11 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id BCE0E180626 for ; Wed, 15 Aug 2018 20:25:10 +0200 (CEST) Received: (qmail 84187 invoked by uid 500); 15 Aug 2018 18:25:09 -0000 Mailing-List: contact commits-help@superset.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@superset.incubator.apache.org Delivered-To: mailing list commits@superset.incubator.apache.org Received: (qmail 84178 invoked by uid 99); 15 Aug 2018 18:25:09 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 15 Aug 2018 18:25:09 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 4D51380675; Wed, 15 Aug 2018 18:25:09 +0000 (UTC) Date: Wed, 15 Aug 2018 18:25:09 +0000 To: "commits@superset.apache.org" Subject: [incubator-superset] branch master updated: Fix form data issue switching viz types (#5100) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <153435750910.7275.4861742029281321980@gitbox.apache.org> From: graceguo@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: incubator-superset X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 4e3b2e7cbc2a8a79aebb7f53e06758b0a0da0f9a X-Git-Newrev: bf0afef7a9b0212e339d823b632a7e2ddbf46f6d X-Git-Rev: bf0afef7a9b0212e339d823b632a7e2ddbf46f6d X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated This is an automated email from the ASF dual-hosted git repository. graceguo 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 bf0afef Fix form data issue switching viz types (#5100) bf0afef is described below commit bf0afef7a9b0212e339d823b632a7e2ddbf46f6d Author: michellethomas AuthorDate: Wed Aug 15 11:25:06 2018 -0700 Fix form data issue switching viz types (#5100) * Fixing issue with extra params in formData * Pass in param use_slice_data to decide whether to use slice data * Fixing core_tests to not use explore_json overrides --- .../explore/components/ExploreViewContainer.jsx | 6 --- superset/assets/src/explore/controls.jsx | 7 +++ superset/assets/src/explore/store.js | 7 --- superset/assets/src/explore/visTypes.jsx | 2 +- superset/views/core.py | 13 ++++-- tests/core_tests.py | 50 +++++++++------------- 6 files changed, 37 insertions(+), 48 deletions(-) diff --git a/superset/assets/src/explore/components/ExploreViewContainer.jsx b/superset/assets/src/explore/components/ExploreViewContainer.jsx index 315cc80..720e721 100644 --- a/superset/assets/src/explore/components/ExploreViewContainer.jsx +++ b/superset/assets/src/explore/components/ExploreViewContainer.jsx @@ -293,12 +293,6 @@ ExploreViewContainer.propTypes = propTypes; function mapStateToProps({ explore, charts, impressionId }) { const form_data = getFormDataFromControls(explore.controls); - // fill in additional params stored in form_data but not used by control - Object.keys(explore.rawFormData).forEach((key) => { - if (form_data[key] === undefined) { - form_data[key] = explore.rawFormData[key]; - } - }); const chartKey = Object.keys(charts)[0]; const chart = charts[chartKey]; return { diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index 241852a..8dd72a7 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -1888,6 +1888,13 @@ export const controls = { description: t('The number of seconds before expiring the cache'), }, + url_params: { + type: 'HiddenControl', + label: t('URL Parameters'), + hidden: true, + description: t('Extra parameters for use in jinja templated queries'), + }, + order_by_entity: { type: 'CheckboxControl', label: t('Order by entity id'), diff --git a/superset/assets/src/explore/store.js b/superset/assets/src/explore/store.js index 78f0d4e..111954a 100644 --- a/superset/assets/src/explore/store.js +++ b/superset/assets/src/explore/store.js @@ -121,13 +121,6 @@ export function applyDefaultFormData(form_data) { formData[k] = form_data[k]; } }); - // fill in additional params stored in form_data but not used by control - Object.keys(form_data) - .forEach((key) => { - if (formData[key] === undefined) { - formData[key] = form_data[key]; - } - }); return formData; } diff --git a/superset/assets/src/explore/visTypes.jsx b/superset/assets/src/explore/visTypes.jsx index 4c5ae01..9f3e4e0 100644 --- a/superset/assets/src/explore/visTypes.jsx +++ b/superset/assets/src/explore/visTypes.jsx @@ -23,7 +23,7 @@ export const sections = { controlSetRows: [ ['datasource'], ['viz_type'], - ['slice_id', 'cache_timeout'], + ['slice_id', 'cache_timeout', 'url_params'], ], }, colorScheme: { diff --git a/superset/views/core.py b/superset/views/core.py index 2371aba..62c4877 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -933,7 +933,7 @@ class Superset(BaseSupersetView): session.commit() return redirect('/accessrequestsmodelview/list/') - def get_form_data(self, slice_id=None): + def get_form_data(self, slice_id=None, use_slice_data=False): form_data = {} post_data = request.form.get('form_data') request_args_data = request.args.get('form_data') @@ -970,7 +970,12 @@ class Superset(BaseSupersetView): slice_id = form_data.get('slice_id') or slice_id slc = None - if slice_id: + # Check if form data only contains slice_id + contains_only_slc_id = not any(key != 'slice_id' for key in form_data) + + # Include the slice_form_data if request from explore or slice calls + # or if form_data only contains slice_id + if slice_id and (use_slice_data or contains_only_slc_id): slc = db.session.query(models.Slice).filter_by(id=slice_id).first() slice_form_data = slc.form_data.copy() # allow form_data in request override slice from_data @@ -1010,7 +1015,7 @@ class Superset(BaseSupersetView): @has_access @expose('/slice//') def slice(self, slice_id): - form_data, slc = self.get_form_data(slice_id) + form_data, slc = self.get_form_data(slice_id, use_slice_data=True) endpoint = '/superset/explore/?form_data={}'.format( parse.quote(json.dumps(form_data)), ) @@ -1217,7 +1222,7 @@ class Superset(BaseSupersetView): @expose('/explore/', methods=['GET', 'POST']) def explore(self, datasource_type=None, datasource_id=None): user_id = g.user.get_id() if g.user else None - form_data, slc = self.get_form_data() + form_data, slc = self.get_form_data(use_slice_data=True) datasource_id, datasource_type = self.datasource_info( datasource_id, datasource_type, form_data) diff --git a/tests/core_tests.py b/tests/core_tests.py index ed66af7..f03c51f 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -191,10 +191,11 @@ class CoreTests(SupersetTestCase): form_data = { 'viz_type': 'sankey', - 'groupby': 'target', + 'groupby': 'source', 'metric': 'sum__value', 'row_limit': 5000, 'slice_id': new_slice_id, + 'time_range': 'now', } # Setting the name back to its original name by overwriting new slice self.get_resp( @@ -207,6 +208,7 @@ class CoreTests(SupersetTestCase): ) slc = db.session.query(models.Slice).filter_by(id=new_slice_id).first() assert slc.slice_name == new_slice_name + assert slc.viz.form_data == form_data db.session.delete(slc) def test_filter_endpoint(self): @@ -557,7 +559,7 @@ class CoreTests(SupersetTestCase): # superset/explore case slc = db.session.query(models.Slice).filter_by(slice_name='Girls').one() qry = db.session.query(models.Log).filter_by(slice_id=slc.id) - self.get_resp(slc.slice_url, {'form_data': json.dumps(slc.viz.form_data)}) + self.get_resp(slc.slice_url, {'form_data': json.dumps(slc.form_data)}) self.assertEqual(1, qry.count()) def test_slice_id_is_always_logged_correctly_on_ajax_request(self): @@ -566,7 +568,7 @@ class CoreTests(SupersetTestCase): slc = db.session.query(models.Slice).filter_by(slice_name='Girls').one() qry = db.session.query(models.Log).filter_by(slice_id=slc.id) slc_url = slc.slice_url.replace('explore', 'explore_json') - self.get_json_resp(slc_url, {'form_data': json.dumps(slc.viz.form_data)}) + self.get_json_resp(slc_url, {'form_data': json.dumps(slc.form_data)}) self.assertEqual(1, qry.count()) def test_slice_query_endpoint(self): @@ -654,46 +656,34 @@ class CoreTests(SupersetTestCase): rendered_query = text_type(table.get_from_clause()) self.assertEqual(clean_query, rendered_query) - def test_slice_url_overrides(self): - # No override - self.login(username='admin') - slice_name = 'Girls' - slc = self.get_slice(slice_name, db.session) - resp = self.get_resp(slc.explore_json_url) - assert '"Jennifer"' in resp - - # Overriding groupby - url = slc.get_explore_url( - base_url='/superset/explore_json', - overrides={'groupby': ['state']}) - resp = self.get_resp(url) - assert '"CA"' in resp - def test_slice_payload_no_data(self): self.login(username='admin') slc = self.get_slice('Girls', db.session) + json_endpoint = '/superset/explore_json/' + form_data = slc.form_data + form_data.update({ + 'filters': [{'col': 'state', 'op': 'in', 'val': ['N/A']}], + }) - url = slc.get_explore_url( - base_url='/superset/explore_json', - overrides={ - 'filters': [{'col': 'state', 'op': 'in', 'val': ['N/A']}], - }, + data = self.get_json_resp( + json_endpoint, + {'form_data': json.dumps(form_data)}, ) - - data = self.get_json_resp(url) self.assertEqual(data['status'], utils.QueryStatus.SUCCESS) self.assertEqual(data['error'], 'No data') def test_slice_payload_invalid_query(self): self.login(username='admin') slc = self.get_slice('Girls', db.session) + form_data = slc.form_data + form_data.update({ + 'groupby': ['N/A'], + }) - url = slc.get_explore_url( - base_url='/superset/explore_json', - overrides={'groupby': ['N/A']}, + data = self.get_json_resp( + '/superset/explore_json/', + {'form_data': json.dumps(form_data)}, ) - - data = self.get_json_resp(url) self.assertEqual(data['status'], utils.QueryStatus.FAILED) assert 'KeyError' in data['stacktrace']