You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kg...@apache.org on 2026/05/29 16:26:15 UTC

(superset) 08/23: refactor(activity-view): extract resolve_endpoint_path_entity helper

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

kgabryje pushed a commit to branch sc-107283-versioning-activity-view
in repository https://gitbox.apache.org/repos/asf/superset.git

commit a412f1d9e6ab93c49a859adcd163d4bff36f14ce
Author: Mike Bridge <mi...@ext.preset.io>
AuthorDate: Tue May 26 15:46:20 2026 -0600

    refactor(activity-view): extract resolve_endpoint_path_entity helper
    
    Make the change easy, then make the easy change. Phase 4 (T028 chart
    /activity/ endpoint, T033 dataset endpoint) duplicates the UUID-parse
    + find_active_by_uuid + raise_for_ownership dance from the existing
    DashboardRestApi.activity. Extracting now keeps the next two endpoints
    single-line affairs.
    
    * New PathEntityResponseError carries a pre-built error Response.
      Subclassing Exception (not ValueError) so the endpoint's catch is
      precise and the standard hierarchy stays unpolluted.
    * New resolve_endpoint_path_entity(api, model_cls, uuid_str) runs the
      three-step preflight and raises PathEntityResponseError with the
      right 400/404/403 response carried inside.
    * DashboardRestApi.activity refactored to use the helper. 12 lines of
      imports + try/except boilerplate collapse to a single try/except
      block that maps every preflight failure to its response.
    
    No behaviour change: 10/10 dashboard activity integration tests still
    green. Pure structural change, per Beck's tidy-first discipline (the
    T028 endpoint code follows in the next commit).
    
    Co-Authored-By: Claude Opus 4.7 (1M context) <no...@anthropic.com>
---
 superset/dashboards/api.py      | 22 +++++------------
 superset/versioning/activity.py | 55 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 16 deletions(-)

diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py
index db051b5ee08..576a799fcca 100644
--- a/superset/dashboards/api.py
+++ b/superset/dashboards/api.py
@@ -2500,32 +2500,22 @@ class DashboardRestApi(CustomTagsOptimizationMixin, BaseSupersetModelRestApi):
               $ref: '#/components/responses/404'
         """
         # pylint: disable=import-outside-toplevel
-        from uuid import UUID
-
-        from superset.daos.version import VersionDAO
-        from superset.exceptions import SupersetSecurityException
         from superset.versioning import activity as activity_module
         from superset.versioning.schemas import ActivityResponseSchema
 
         try:
-            entity_uuid = UUID(uuid_str)
-        except ValueError:
-            return self.response_400(message="Invalid UUID")
+            entity = activity_module.resolve_endpoint_path_entity(
+                self, Dashboard, uuid_str
+            )
+        except activity_module.PathEntityResponseError as exc:
+            return exc.response
 
         try:
             params = activity_module.parse_activity_query_params(request.args)
         except activity_module.ActivityParamsError as exc:
             return self.response_400(message=str(exc))
 
-        entity = VersionDAO.find_active_by_uuid(Dashboard, entity_uuid)
-        if entity is None:
-            return self.response_404()
-        try:
-            security_manager.raise_for_ownership(entity)
-        except SupersetSecurityException:
-            return self.response_403()
-
-        records, count = activity_module.get_activity(Dashboard, entity_uuid, **params)
+        records, count = activity_module.get_activity(Dashboard, entity.uuid, **params)
         payload = ActivityResponseSchema().dump({"result": records, "count": count})
         return self.response(200, **payload)
 
diff --git a/superset/versioning/activity.py b/superset/versioning/activity.py
index 07719b21089..73b319924d7 100644
--- a/superset/versioning/activity.py
+++ b/superset/versioning/activity.py
@@ -881,6 +881,61 @@ class ActivityParamsError(ValueError):
     no other callers should depend on the exception type."""
 
 
+class PathEntityResponseError(Exception):
+    """Carries a pre-built error ``Response`` from
+    :func:`resolve_endpoint_path_entity`. The endpoint catches this and
+    returns the carried response directly. The shape exists so the
+    UUID-parse + find-by-uuid + ownership-check dance can live in one
+    place across the three activity-view endpoint families."""
+
+    def __init__(self, response: Any) -> None:
+        super().__init__("PathEntityResponseError")
+        self.response = response
+
+
+def resolve_endpoint_path_entity(api: Any, model_cls: type, uuid_str: str) -> Any:
+    """Run the standard path-entity preflight for an activity endpoint:
+
+    1. Parse *uuid_str* into a UUID (or raise → 400).
+    2. Look up the live entity via ``VersionDAO.find_active_by_uuid``
+       (or raise → 404).
+    3. Run ``security_manager.raise_for_ownership`` (or raise → 403).
+
+    Returns the live entity on success. Raises
+    :class:`PathEntityResponseError` carrying the appropriate error
+    Response on any failure; the endpoint method should::
+
+        try:
+            entity = resolve_endpoint_path_entity(self, Dashboard, uuid_str)
+        except PathEntityResponseError as exc:
+            return exc.response
+
+    *api* is the FAB ``ModelRestApi`` instance — we call
+    ``api.response_400`` / ``api.response_403`` / ``api.response_404``
+    on it. Pass ``self`` from the endpoint method.
+    """
+    # pylint: disable=import-outside-toplevel
+    from superset import security_manager
+    from superset.daos.version import VersionDAO
+    from superset.exceptions import SupersetSecurityException
+
+    try:
+        entity_uuid = UUID(uuid_str)
+    except ValueError as exc:
+        raise PathEntityResponseError(api.response_400(message="Invalid UUID")) from exc
+
+    entity = VersionDAO.find_active_by_uuid(model_cls, entity_uuid)
+    if entity is None:
+        raise PathEntityResponseError(api.response_404())
+
+    try:
+        security_manager.raise_for_ownership(entity)
+    except SupersetSecurityException as exc:
+        raise PathEntityResponseError(api.response_403()) from exc
+
+    return entity
+
+
 def parse_activity_query_params(args: Any) -> dict[str, Any]:
     """Parse the ``since`` / ``until`` / ``include`` / ``page`` / ``page_size``
     query parameters into the kwargs ``get_activity`` accepts.