[incubator-superset] branch master updated: [explore] Fix and test slice id logging issue (#3339)

[ma...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin 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 8d877e8  [explore] Fix and test slice id  logging  issue (#3339)
8d877e8 is described below

commit 8d877e8a35e503a27d24d84bdc54f2822625cc91
Author: timifasubaa <30...@users.noreply.github.com>
AuthorDate: Thu Aug 24 09:11:41 2017 -0700

    [explore] Fix and test slice id  logging  issue (#3339)
    
    * [explore] fixed padding bug on filter section
    
    * fix slice_id logging issue
    
    * [superset-sqllab] fix slice_id population in appropriate column
    
    * [explore-logging] test the slice_id logging fix
    
    * fix travis errors
    
    * fix nits pointed out in PR comments
    
    * cleanup tests
    
    * made python more beautiful
    
    * made python even more beautiful
    
    * made python even more more beautiful
    
    * made python even more more more beautiful
    
    * fix lint error
    
    * make exception handling more specific
    
    * fixed silly error
    
    * fixed argument indentation
---
 superset/models/core.py | 17 +++++++++--------
 tests/core_tests.py     | 18 +++++++++++++++++-
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/superset/models/core.py b/superset/models/core.py
index 4f76c79..e38c8cd 100644
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -86,8 +86,7 @@ class CssTemplate(Model, AuditMixinNullable):
 slice_user = Table('slice_user', metadata,
                    Column('id', Integer, primary_key=True),
                    Column('user_id', Integer, ForeignKey('ab_user.id')),
-                   Column('slice_id', Integer, ForeignKey('slices.id'))
-                   )
+                   Column('slice_id', Integer, ForeignKey('slices.id')))
 
 
 class Slice(Model, AuditMixinNullable, ImportMixin):
@@ -435,7 +434,7 @@ class Dashboard(Model, AuditMixinNullable, ImportMixin):
                 new_filter_immune_slices.append(new_slc_id_str)
             if ('timed_refresh_immune_slices' in i_params_dict and
                     old_slc_id_str in
-                        i_params_dict['timed_refresh_immune_slices']):
+                    i_params_dict['timed_refresh_immune_slices']):
                 new_timed_refresh_immune_slices.append(new_slc_id_str)
             if ('expanded_slices' in i_params_dict and
                     old_slc_id_str in i_params_dict['expanded_slices']):
@@ -761,11 +760,14 @@ class Log(Model):
             post_data = request.form or {}
             d.update(post_data)
             d.update(kwargs)
-            slice_id = d.get('slice_id', 0)
+            slice_id = d.get('slice_id')
+
             try:
-                slice_id = int(slice_id) if slice_id else 0
-            except ValueError:
+                slice_id = int(
+                    slice_id or json.loads(d.get('form_data')).get('slice_id'))
+            except (ValueError, TypeError):
                 slice_id = 0
+
             params = ""
             try:
                 params = json.dumps(d)
@@ -773,12 +775,11 @@ class Log(Model):
                 pass
             stats_logger.incr(f.__name__)
             value = f(*args, **kwargs)
-
             sesh = db.session()
             log = cls(
                 action=f.__name__,
                 json=params,
-                dashboard_id=d.get('dashboard_id') or None,
+                dashboard_id=d.get('dashboard_id'),
                 slice_id=slice_id,
                 duration_ms=(
                     datetime.now() - start_dttm).total_seconds() * 1000,
diff --git a/tests/core_tests.py b/tests/core_tests.py
index 4a7efc4..245bd88 100644
--- a/tests/core_tests.py
+++ b/tests/core_tests.py
@@ -41,6 +41,7 @@ class CoreTests(SupersetTestCase):
     def setUp(self):
         db.session.query(Query).delete()
         db.session.query(models.DatasourceAccessRequest).delete()
+        db.session.query(models.Log).delete()
 
     def tearDown(self):
         db.session.query(Query).delete()
@@ -726,7 +727,22 @@ class CoreTests(SupersetTestCase):
         data = self.get_json_resp('/superset/fave_dashboards_by_username/{}/'.format(username))
         self.assertNotIn('message', data)
 
+    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)
+        self.assertEqual(1, qry.count())
+
+    def test_slice_id_is_always_logged_correctly_on_ajax_request(self):
+        # superset/explore_json case
+        self.login(username="admin")
+        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)
+        self.assertEqual(1, qry.count())
+
 
 if __name__ == '__main__':
     unittest.main()
-

-- 
To stop receiving notification emails like this one, please contact
['"commits@superset.apache.org" <co...@superset.apache.org>'].