iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[WiP] Cleanup & fix URL scheme for the explore view (#4490) 

* Improve URLs

* Fix js tests
This commit is contained in:
Maxime Beauchemin 2018-02-27 15:08:06 -08:00 committed by Grace Guo
parent bcca1717f2
commit 83524f97d7
9 changed files with 114 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
run_specific_test.sh
View File

@ -1,6 +1,7 @@
#!/usr/bin/env bash
echo $DB
rm -f .coverage
export PYTHONPATH=./
export SUPERSET_CONFIG=tests.superset_test_config
set -e
superset/bin/superset version -v

1
run_tests.sh
View File

@ -4,6 +4,7 @@ rm ~/.superset/unittests.db
rm ~/.superset/celerydb.sqlite
rm ~/.superset/celery_results.sqlite
rm -f .coverage
export PYTHONPATH=./
export SUPERSET_CONFIG=tests.superset_test_config
set -e
superset/bin/superset db upgrade

3
superset/assets/javascripts/addSlice/AddSliceContainer.jsx
View File

@ -23,9 +23,8 @@ export default class AddSliceContainer extends React.PureComponent {
}
exploreUrl() {
const baseUrl = `/superset/explore/${this.state.datasourceType}/${this.state.datasourceId}`;
const formData = encodeURIComponent(JSON.stringify({ viz_type: this.state.visType }));
return `${baseUrl}?form_data=${formData}`;
return `/superset/explore/?form_data=${formData}`;
}
gotoSlice() {

3
superset/assets/javascripts/explore/exploreUtils.js
View File

@ -25,9 +25,6 @@ export function getURIDirectory(formData, endpointType = 'base') {
if (['json', 'csv', 'query'].indexOf(endpointType) >= 0) {
directory = '/superset/explore_json/';
}
const [datasource_id, datasource_type] = formData.datasource.split('__');
directory += `${datasource_type}/${datasource_id}/`;
return directory;
}

2
superset/assets/spec/javascripts/addSlice/AddSliceContainer_spec.jsx
View File

@ -53,7 +53,7 @@ describe('AddSliceContainer', () => {
datasourceId: datasourceValue.split('__')[0],
datasourceType: datasourceValue.split('__')[1],
});
const formattedUrl = '/superset/explore/table/1?form_data=%7B%22viz_type%22%3A%22table%22%7D';
const formattedUrl = '/superset/explore/?form_data=%7B%22viz_type%22%3A%22table%22%7D';
expect(wrapper.instance().exploreUrl()).to.equal(formattedUrl);
});
});

18
superset/assets/spec/javascripts/explore/utils_spec.jsx
View File

@ -27,7 +27,7 @@ describe('utils', () => {
});
compareURI(
URI(url),
URI('/superset/explore/table/1/'),
URI('/superset/explore/'),
);
expect(payload).to.deep.equals(formData);
});
@ -40,7 +40,7 @@ describe('utils', () => {
});
compareURI(
URI(url),
URI('/superset/explore_json/table/1/'),
URI('/superset/explore_json/'),
);
expect(payload).to.deep.equals(formData);
});
@ -53,7 +53,7 @@ describe('utils', () => {
});
compareURI(
URI(url),
URI('/superset/explore_json/table/1/')
URI('/superset/explore_json/')
.search({ force: 'true' }),
);
expect(payload).to.deep.equals(formData);
@ -67,7 +67,7 @@ describe('utils', () => {
});
compareURI(
URI(url),
URI('/superset/explore_json/table/1/')
URI('/superset/explore_json/')
.search({ csv: 'true' }),
);
expect(payload).to.deep.equals(formData);
@ -81,7 +81,7 @@ describe('utils', () => {
});
compareURI(
URI(url),
URI('/superset/explore/table/1/')
URI('/superset/explore/')
.search({ standalone: 'true' }),
);
expect(payload).to.deep.equals(formData);
@ -95,7 +95,7 @@ describe('utils', () => {
});
compareURI(
URI(url),
URI('/superset/explore_json/table/1/')
URI('/superset/explore_json/')
.search({ foo: 'bar' }),
);
expect(payload).to.deep.equals(formData);
@ -109,7 +109,7 @@ describe('utils', () => {
});
compareURI(
URI(url),
URI('/superset/explore_json/table/1/')
URI('/superset/explore_json/')
.search({ foo: 'bar' }),
);
expect(payload).to.deep.equals(formData);
@ -123,7 +123,7 @@ describe('utils', () => {
});
compareURI(
URI(url),
URI('/superset/explore_json/table/1/')
URI('/superset/explore_json/')
.search({ foo: 'bar' }),
);
expect(payload).to.deep.equals(formData);
@ -134,7 +134,7 @@ describe('utils', () => {
it('generates proper base url with form_data', () => {
compareURI(
URI(getExploreLongUrl(formData, 'base')),
URI('/superset/explore/table/1/').search({ form_data: sFormData }),
URI('/superset/explore/').search({ form_data: sFormData }),
);
});
});

24
superset/models/core.py
View File

@ -201,26 +201,30 @@ class Slice(Model, AuditMixinNullable, ImportMixin):
form_data.update({
'slice_id': self.id,
'viz_type': self.viz_type,
'datasource': str(self.datasource_id) + '__' + self.datasource_type,
'datasource': '{}__{}'.format(
self.datasource_id, self.datasource_type),
})
if self.cache_timeout:
form_data['cache_timeout'] = self.cache_timeout
return form_data
def get_explore_url(self, base_url='/superset/explore', overrides=None):
overrides = overrides or {}
form_data = {'slice_id': self.id}
form_data.update(overrides)
params = parse.quote(json.dumps(form_data))
return (
'{base_url}/?form_data={params}'.format(**locals()))
@property
def slice_url(self):
"""Defines the url to access the slice"""
form_data = {'slice_id': self.id}
return (
'/superset/explore/{obj.datasource_type}/'
'{obj.datasource_id}/?form_data={params}'.format(
obj=self, params=parse.quote(json.dumps(form_data))))
return self.get_explore_url()
@property
def slice_id_url(self):
return (
'/superset/{slc.datasource_type}/{slc.datasource_id}/{slc.id}/'
).format(slc=self)
def explore_json_url(self):
"""Defines the url to access the slice"""
return self.get_explore_url('/superset/explore_json')
@property
def edit_url(self):

84
superset/views/core.py
View File

@ -943,24 +943,39 @@ class Superset(BaseSupersetView):
session.commit()
return redirect('/accessrequestsmodelview/list/')
def get_form_data(self):
d = {}
def get_form_data(self, slice_id=None):
form_data = {}
post_data = request.form.get('form_data')
request_args_data = request.args.get('form_data')
# Supporting POST
if post_data:
d.update(json.loads(post_data))
form_data.update(json.loads(post_data))
# request params can overwrite post body
if request_args_data:
d.update(json.loads(request_args_data))
form_data.update(json.loads(request_args_data))
if request.args.get('viz_type'):
# Converting old URLs
d = cast_form_data(d)
form_data = cast_form_data(form_data)
d = {k: v for k, v in d.items() if k not in FORM_DATA_KEY_BLACKLIST}
form_data = {
k: v
for k, v in form_data.items()
if k not in FORM_DATA_KEY_BLACKLIST
}
return d
# When a slice_id is present, load from DB and override
# the form_data from the DB with the other form_data provided
slice_id = form_data.get('slice_id') or slice_id
slc = None
if slice_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
slice_form_data.update(form_data)
form_data = slice_form_data
return form_data, slc
def get_viz(
self,
@ -991,11 +1006,9 @@ class Superset(BaseSupersetView):
@has_access
@expose('/slice/<slice_id>/')
def slice(self, slice_id):
viz_obj = self.get_viz(slice_id)
endpoint = '/superset/explore/{}/{}?form_data={}'.format(
viz_obj.datasource.type,
viz_obj.datasource.id,
parse.quote(json.dumps(viz_obj.form_data)),
form_data, slc = self.get_form_data(slice_id)
endpoint = '/superset/explore/?form_data={}'.format(
parse.quote(json.dumps(form_data)),
)
if request.args.get('standalone') == 'true':
endpoint += '&standalone=true'
@ -1075,10 +1088,7 @@ class Superset(BaseSupersetView):
viz_obj = self.get_viz(slice_id)
datasource_type = viz_obj.datasource.type
datasource_id = viz_obj.datasource.id
form_data = viz_obj.form_data
# This allows you to override the saved slice form data with
# data from the current request (e.g. change the time window)
form_data.update(self.get_form_data())
form_data, slc = self.get_form_data()
except Exception as e:
return json_error_response(
utils.error_msg_from_exception(e),
@ -1091,7 +1101,7 @@ class Superset(BaseSupersetView):
@has_access_api
@expose('/annotation_json/<layer_id>')
def annotation_json(self, layer_id):
form_data = self.get_form_data()
form_data = self.get_form_data()[0]
form_data['layer_id'] = layer_id
form_data['filters'] = [{'col': 'layer_id',
'op': '==',
@ -1115,12 +1125,15 @@ class Superset(BaseSupersetView):
@log_this
@has_access_api
@expose('/explore_json/<datasource_type>/<datasource_id>/', methods=['GET', 'POST'])
def explore_json(self, datasource_type, datasource_id):
@expose('/explore_json/', methods=['GET', 'POST'])
def explore_json(self, datasource_type=None, datasource_id=None):
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()
form_data = self.get_form_data()[0]
datasource_id, datasource_type = self.datasource_info(
datasource_id, datasource_type, form_data)
except Exception as e:
logging.exception(e)
return json_error_response(
@ -1157,19 +1170,36 @@ class Superset(BaseSupersetView):
@has_access
@expose('/explorev2/<datasource_type>/<datasource_id>/')
def explorev2(self, datasource_type, datasource_id):
"""Deprecated endpoint, here for backward compatibility of urls"""
return redirect(url_for(
'Superset.explore',
datasource_type=datasource_type,
datasource_id=datasource_id,
**request.args))
@staticmethod
def datasource_info(datasource_id, datasource_type, form_data):
"""Compatibility layer for handling of datasource info
datasource_id & datasource_type used to be passed in the URL
directory, now they should come as part of the form_data,
This function allows supporting both without duplicating code"""
datasource = form_data.get('datasource', '')
if '__' in datasource:
datasource_id, datasource_type = datasource.split('__')
datasource_id = int(datasource_id)
return datasource_id, datasource_type
@log_this
@has_access
@expose('/explore/<datasource_type>/<datasource_id>/', methods=['GET', 'POST'])
def explore(self, datasource_type, datasource_id):
datasource_id = int(datasource_id)
@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 = self.get_form_data()
form_data, slc = self.get_form_data()
datasource_id, datasource_type = self.datasource_info(
datasource_id, datasource_type, form_data)
saved_url = None
url_id = request.args.get('r')
@ -1182,14 +1212,6 @@ class Superset(BaseSupersetView):
# allow form_date in request override saved url
url_form_data.update(form_data)
form_data = url_form_data
slice_id = form_data.get('slice_id')
slc = None
if slice_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
slice_form_data.update(form_data)
form_data = slice_form_data
error_redirect = '/slicemodelview/list/'
datasource = ConnectorRegistry.get_datasource(
@ -1308,7 +1330,7 @@ class Superset(BaseSupersetView):
"""Save or overwrite a slice"""
slice_name = args.get('slice_name')
action = args.get('action')
form_data = self.get_form_data()
form_data, _ = self.get_form_data()
if action in ('saveas'):
if 'slice_id' in form_data:

38
tests/core_tests.py
View File

@ -97,7 +97,7 @@ class CoreTests(SupersetTestCase):
qobj['groupby'] = []
self.assertNotEqual(cache_key, viz.cache_key(qobj))
def test_slice_json_endpoint(self):
def test_old_slice_json_endpoint(self):
self.login(username='admin')
slc = self.get_slice('Girls', db.session)
@ -108,7 +108,13 @@ class CoreTests(SupersetTestCase):
resp = self.get_resp(json_endpoint, {'form_data': json.dumps(slc.viz.form_data)})
assert '"Jennifer"' in resp
def test_slice_csv_endpoint(self):
def test_slice_json_endpoint(self):
self.login(username='admin')
slc = self.get_slice('Girls', db.session)
resp = self.get_resp(slc.explore_json_url)
assert '"Jennifer"' in resp
def test_old_slice_csv_endpoint(self):
self.login(username='admin')
slc = self.get_slice('Girls', db.session)
@ -119,6 +125,15 @@ class CoreTests(SupersetTestCase):
resp = self.get_resp(csv_endpoint, {'form_data': json.dumps(slc.viz.form_data)})
assert 'Jennifer,' in resp
def test_slice_csv_endpoint(self):
self.login(username='admin')
slc = self.get_slice('Girls', db.session)
csv_endpoint = '/superset/explore_json/?csv=true'
resp = self.get_resp(
csv_endpoint, {'form_data': json.dumps({'slice_id': slc.id})})
assert 'Jennifer,' in resp
def test_admin_only_permissions(self):
def assert_admin_permission_in(role_name, assert_func):
role = sm.find_role(role_name)
@ -225,8 +240,8 @@ class CoreTests(SupersetTestCase):
urls = []
for slc in db.session.query(Slc).all():
urls += [
(slc.slice_name, 'slice_url', slc.slice_url),
(slc.slice_name, 'slice_id_url', slc.slice_id_url),
(slc.slice_name, 'explore', slc.slice_url),
(slc.slice_name, 'explore_json', slc.explore_json_url),
]
for name, method, url in urls:
logging.info('[{name}]/[{method}]: {url}'.format(**locals()))
@ -879,6 +894,21 @@ 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
if __name__ == '__main__':
unittest.main()