[log] Updating form-data logic (#10197)

Co-authored-by: John Bodley <john.bodley@airbnb.com>

superset/utils/log.py

@@ -36,31 +36,32 @@ class AbstractEventLogger(ABC):
         pass
 
     def log_this(self, f: Callable[..., Any]) -> Callable[..., Any]:
+        from superset.views.core import get_form_data
+
         @functools.wraps(f)
         def wrapper(*args: Any, **kwargs: Any) -> Any:
             user_id = None
             if g.user:
                 user_id = g.user.get_id()
-            form_data = request.form.to_dict() or {}
+            payload = request.form.to_dict() or {}
 
             # request parameters can overwrite post body
             request_params = request.args.to_dict()
-            form_data.update(request_params)
-            form_data.update(kwargs)
+            payload.update(request_params)
+            payload.update(kwargs)
 
-            slice_id = form_data.get("slice_id")
-            dashboard_id = form_data.get("dashboard_id")
+            dashboard_id = payload.get("dashboard_id")
+
+            if "form_data" in payload:
+                form_data, _ = get_form_data()
+                payload["form_data"] = form_data
+                slice_id = form_data.get("slice_id")
+            else:
+                slice_id = payload.get("slice_id")
 
             try:
-                slice_id = int(
-                    slice_id
-                    or json.loads(
-                        form_data.get("form_data")  # type: ignore
-                    ).get(
-                        "slice_id"
-                    )
-                )
-            except (ValueError, TypeError):
+                slice_id = int(slice_id)  # type: ignore
+            except (TypeError, ValueError):
                 slice_id = 0
 
             self.stats_logger.incr(f.__name__)
@@ -70,10 +71,10 @@ class AbstractEventLogger(ABC):
 
             # bulk insert
             try:
-                explode_by = form_data.get("explode")
-                records = json.loads(form_data.get(explode_by))  # type: ignore
+                explode_by = payload.get("explode")
+                records = json.loads(payload.get(explode_by))  # type: ignore
             except Exception:  # pylint: disable=broad-except
-                records = [form_data]
+                records = [payload]
 
             referrer = request.referrer[:1000] if request.referrer else None
 

tests/utils_tests.py

@@ -32,7 +32,7 @@ from sqlalchemy.exc import ArgumentError
 import tests.test_app
 from superset import app, db, security_manager
 from superset.exceptions import CertificateException, SupersetException
-from superset.models.core import Database
+from superset.models.core import Database, Log
 from superset.utils.cache_manager import CacheManager
 from superset.utils.core import (
     base_json_conv,
@@ -1316,3 +1316,31 @@ class TestUtils(SupersetTestCase):
             )
 
             self.assertEqual(slc, None)
+
+    def test_log_this(self) -> None:
+        # TODO: Add additional scenarios.
+        self.login(username="admin")
+        slc = self.get_slice("Girls", db.session)
+        dashboard_id = 1
+
+        resp = self.get_json_resp(
+            f"/superset/explore_json/{slc.datasource_type}/{slc.datasource_id}/"
+            + f'?form_data={{"slice_id": {slc.id}}}&dashboard_id={dashboard_id}',
+            {"form_data": json.dumps(slc.viz.form_data)},
+        )
+
+        record = (
+            db.session.query(Log)
+            .filter_by(action="explore_json", slice_id=slc.id)
+            .order_by(Log.dttm.desc())
+            .first()
+        )
+
+        self.assertEqual(record.dashboard_id, dashboard_id)
+        self.assertEqual(json.loads(record.json)["dashboard_id"], str(dashboard_id))
+        self.assertEqual(json.loads(record.json)["form_data"]["slice_id"], slc.id)
+
+        self.assertEqual(
+            json.loads(record.json)["form_data"]["viz_type"],
+            slc.viz.form_data["viz_type"],
+        )