Improve URLs

run_specific_test.sh

@@ -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

run_tests.sh

@@ -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

superset/assets/javascripts/addSlice/AddSliceContainer.jsx

@@ -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() {

superset/assets/javascripts/explore/exploreUtils.js

@@ -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;
 }
 

superset/models/core.py

@@ -201,26 +201,30 @@ def form_data(self):
         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):

superset/views/core.py

@@ -943,24 +943,39 @@ def clean_fulfilled_requests(session):
         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 @@ def get_viz(
     @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 @@ def slice_json(self, slice_id):
             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 @@ def slice_json(self, slice_id):
     @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 @@ def annotation_json(self, layer_id):
     @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 @@ def import_dashboards(self):
     @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 @@ def explore(self, datasource_type, datasource_id):
                 # 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 @@ def save_or_overwrite_slice(
         """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:

tests/core_tests.py

@@ -97,7 +97,7 @@ def test_cache_key(self):
         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 @@ def test_slice_json_endpoint(self):
         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 @@ def test_slice_csv_endpoint(self):
         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 @@ def test_slices(self):
         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 @@ 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
+
 
 if __name__ == '__main__':
     unittest.main()