You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2018/08/15 18:25:07 UTC
[GitHub] graceguo-supercat closed pull request #5100: Fix form data issue
switching viz types
graceguo-supercat closed pull request #5100: Fix form data issue switching viz types
URL: https://github.com/apache/incubator-superset/pull/5100
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/superset/assets/src/explore/components/ExploreViewContainer.jsx b/superset/assets/src/explore/components/ExploreViewContainer.jsx
index 315cc80ad2..720e72176a 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 241852a33b..8dd72a712b 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 78f0d4e219..111954a3d2 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 4c5ae01f76..9f3e4e069d 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 2371abaf32..62c48771fc 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -933,7 +933,7 @@ def clean_fulfilled_requests(session):
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 @@ def get_form_data(self, slice_id=None):
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 @@ def get_viz(
@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 @@ def datasource_info(datasource_id, datasource_type, form_data):
@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 ed66af70f2..f03c51f2b3 100644
--- a/tests/core_tests.py
+++ b/tests/core_tests.py
@@ -191,10 +191,11 @@ def test_save_slice(self):
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 @@ def test_save_slice(self):
)
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 @@ def test_slice_id_is_always_logged_correctly_on_web_request(self):
# 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 @@ def test_slice_id_is_always_logged_correctly_on_ajax_request(self):
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 @@ def test_comments_in_sqlatable_query(self):
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']
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org