You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by gr...@apache.org on 2018/08/15 18:25:09 UTC
[incubator-superset] branch master updated: Fix form data issue
switching viz types (#5100)
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 <mi...@gmail.com>
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/<slice_id>/')
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']