This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 732248e4755 fix(logging): record dashboard_id and slice_id for REST
mutation endpoints (#44553)
732248e4755 is described below
commit 732248e4755b972ef0a1c04b8d7698b707777ff5
Author: Evan Rusackas <[email protected]>
AuthorDate: Thu Sep 24 11:24:11 2026 -0700
fix(logging): record dashboard_id and slice_id for REST mutation endpoints
(#44553)
Co-authored-by: vikash kumar <[email protected]>
Co-authored-by: Claude Fable 5.1 <[email protected]>
---
superset/charts/api.py | 11 +-
superset/dashboards/api.py | 9 +-
superset/utils/log.py | 101 +++++++++++++-
tests/integration_tests/base_tests.py | 14 +-
tests/integration_tests/charts/api_tests.py | 11 ++
tests/integration_tests/dashboards/api_tests.py | 27 +++-
.../dashboards/soft_delete_tests.py | 7 +
tests/unit_tests/utils/log_tests.py | 150 ++++++++++++++++++++-
8 files changed, 320 insertions(+), 10 deletions(-)
diff --git a/superset/charts/api.py b/superset/charts/api.py
index fdcc0d6a2d8..0501420b0b3 100644
--- a/superset/charts/api.py
+++ b/superset/charts/api.py
@@ -19,7 +19,7 @@ import logging
from contextvars import ContextVar
from datetime import datetime
from io import BytesIO
-from typing import Any, cast, ClassVar, Optional
+from typing import Any, Callable, cast, ClassVar, Optional
from zipfile import is_zipfile, ZipFile
from flask import current_app, redirect, request, Response, url_for
@@ -692,9 +692,13 @@ class ChartRestApi(SoftDeleteApiMixin,
BaseSupersetModelRestApi):
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post",
log_to_statsd=False,
+ allow_extra_payload=True,
)
@requires_json
- def post(self) -> Response:
+ def post(
+ self,
+ add_extra_log_payload: Callable[..., None] = lambda **kwargs: None,
+ ) -> Response:
"""Create a new chart.
---
post:
@@ -736,6 +740,9 @@ class ChartRestApi(SoftDeleteApiMixin,
BaseSupersetModelRestApi):
return self.response_400(message=error.messages)
try:
new_model = CreateChartCommand(item).run()
+ # The id only exists once the command has run, so the event
+ # logger cannot derive it from the route.
+ add_extra_log_payload(slice_id=new_model.id)
return self.response(201, id=new_model.id, result=item,
uuid=new_model.uuid)
except DashboardsForbiddenError as ex:
return self.response(ex.status, message=ex.message)
diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py
index c12ed00661c..e63739cd05a 100644
--- a/superset/dashboards/api.py
+++ b/superset/dashboards/api.py
@@ -983,9 +983,13 @@ class DashboardRestApi(
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post",
log_to_statsd=False,
+ allow_extra_payload=True,
)
@requires_json
- def post(self) -> Response:
+ def post(
+ self,
+ add_extra_log_payload: Callable[..., None] = lambda **kwargs: None,
+ ) -> Response:
"""Create a new dashboard.
---
post:
@@ -1025,6 +1029,9 @@ class DashboardRestApi(
return self.response_400(message=error.messages)
try:
new_model = CreateDashboardCommand(item).run()
+ # The id only exists once the command has run, so the event
+ # logger cannot derive it from the route.
+ add_extra_log_payload(dashboard_id=new_model.id)
return self.response(201, id=new_model.id, result=item,
uuid=new_model.uuid)
except DashboardInvalidError as ex:
return self.response_422(message=ex.normalized_messages())
diff --git a/superset/utils/log.py b/superset/utils/log.py
index 58051600fbe..e1cece42e29 100644
--- a/superset/utils/log.py
+++ b/superset/utils/log.py
@@ -20,6 +20,7 @@ import functools
import inspect
import logging
import textwrap
+import uuid
from abc import ABC, abstractmethod
from collections.abc import Iterator
from contextlib import contextmanager
@@ -31,12 +32,98 @@ from flask_appbuilder.const import API_URI_RIS_KEY
from sqlalchemy import inspect as sa_inspect
from sqlalchemy.exc import SQLAlchemyError
+from superset.constants import SKIP_VISIBILITY_FILTER_CLASSES
from superset.extensions import stats_logger_manager
from superset.utils import json
from superset.utils.core import get_user_id, LoggerLevel, to_int
logger = logging.getLogger(__name__)
+# The ``logs`` table has an integer column for the dashboard or chart a request
+# touched. This maps the model behind a REST API's ``datamodel`` to that column
+# so every route on the matching API populates it without per-endpoint
plumbing.
+LOG_OBJECT_ID_COLUMNS: dict[str, str] = {
+ "Dashboard": "dashboard_id",
+ "Slice": "slice_id",
+}
+
+# Route parameters that identify the single object a REST API route acts on.
+OBJECT_ID_VIEW_ARGS: tuple[str, ...] = (
+ "pk",
+ "id_or_slug",
+ "id_or_uuid",
+ "uuid",
+ "uuid_str",
+)
+
+
+def _resolve_object_id(model: Any, identifier: Any) -> int | None:
+ """
+ Turn a route identifier (id, UUID or slug) into the model's integer id.
+
+ Slugs and UUIDs are looked up bypassing the soft-delete visibility filter
so
+ that restore and purge routes can still identify the archived row they act
+ on. Lookup failures never propagate: an unlogged id must not fail a
request.
+ """
+ # pylint: disable=import-outside-toplevel
+ from superset import db
+
+ try:
+ return int(identifier)
+ except (TypeError, ValueError):
+ pass
+
+ try:
+ criterion = model.uuid == uuid.UUID(str(identifier))
+ except ValueError:
+ if not hasattr(model, "slug"):
+ return None
+ criterion = model.slug == str(identifier)
+
+ try:
+ return (
+ db.session.query(model.id)
+ .filter(criterion)
+ .execution_options(**{SKIP_VISIBILITY_FILTER_CLASSES: {model}})
+ .scalar()
+ )
+ except SQLAlchemyError:
+ logger.debug(
+ "Could not resolve %s %r for event logging", model.__name__,
identifier
+ )
+ return None
+
+
+def get_object_ids_from_view_args(
+ view: Any, view_args: dict[str, Any]
+) -> dict[str, Any]:
+ """
+ Derive the ``dashboard_id`` / ``slice_id`` log fields for a REST API route.
+
+ ``view`` is the API instance the logged route was called on and
+ ``view_args`` are the keyword arguments Flask passed to it. The result is
+ empty unless the API is backed by a model that ``logs`` has a column for.
+
+ A single-object route (``/<pk>``, ``/<id_or_slug>``, ``/<uuid>``, ...)
+ yields e.g. ``{"dashboard_id": 42}``. A bulk route identified by a rison
+ list of ids yields ``{"dashboard_ids": [...]}`` for the JSON payload
+ instead, since the integer column can only hold one id.
+ """
+ model = getattr(getattr(view, "datamodel", None), "obj", None)
+ column = LOG_OBJECT_ID_COLUMNS.get(getattr(model, "__name__", ""))
+ if column is None:
+ return {}
+
+ for key in OBJECT_ID_VIEW_ARGS:
+ if key in view_args:
+ object_id = _resolve_object_id(model, view_args[key])
+ return {column: object_id} if object_id is not None else {}
+
+ ids = view_args.get("rison")
+ if isinstance(ids, list) and ids and all(isinstance(i, int) for i in ids):
+ return {f"{column}s": ids}
+ return {}
+
def collect_request_payload(include_request_data: bool = True) -> dict[str,
Any]:
"""Collect log payload identifiable from request context"""
@@ -321,7 +408,19 @@ class AbstractEventLogger(ABC):
with self.log_context(
action=action_str, object_ref=object_ref_str, **wrapper_kwargs
) as log:
- log(**kwargs)
+ # Resolve the object's id before the route runs so that delete
+ # and purge can still identify the row they are about to
remove.
+ # Read the URL's own view args (Flask fills request.view_args
+ # from the route regardless of what a decorator above this one
+ # does to the wrapped function's signature) rather than only
+ # this wrapper's own kwargs, which a decorator like
+ # with_dashboard can empty out by calling the wrapped function
+ # positionally (e.g. f(self, dash)) after resolving the id.
+ view = args[0] if args else None
+ route_args = dict(kwargs)
+ if has_request_context() and request:
+ route_args.update(request.view_args or {})
+ log(**kwargs, **get_object_ids_from_view_args(view,
route_args))
if allow_extra_payload:
# add a payload updater to the decorated function
value = f(*args, add_extra_log_payload=log, **kwargs)
diff --git a/tests/integration_tests/base_tests.py
b/tests/integration_tests/base_tests.py
index 8bef7da4dff..4dae108ad1e 100644
--- a/tests/integration_tests/base_tests.py
+++ b/tests/integration_tests/base_tests.py
@@ -39,7 +39,7 @@ from superset import db, security_manager
from superset.connectors.sqla.models import BaseDatasource, SqlaTable
from superset.constants import SKIP_VISIBILITY_FILTER_CLASSES
from superset.models import core as models
-from superset.models.core import Database
+from superset.models.core import Database, Log
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.sql.parse import CTASMethod
@@ -311,6 +311,18 @@ class SupersetTestCase(TestCase):
username, first_name, last_name, email, role_admin, password
)
+ @staticmethod
+ def get_latest_log(action: str) -> Log:
+ """Return the newest ``logs`` row recorded for ``action``."""
+ log = (
+ db.session.query(Log)
+ .filter_by(action=action)
+ .order_by(Log.id.desc())
+ .first()
+ )
+ assert log is not None, f"no log row recorded for {action}"
+ return log
+
@staticmethod
def get_user(username: str) -> ab_models.User:
user = (
diff --git a/tests/integration_tests/charts/api_tests.py
b/tests/integration_tests/charts/api_tests.py
index b9501de7ddd..1bfcab0b5d0 100644
--- a/tests/integration_tests/charts/api_tests.py
+++ b/tests/integration_tests/charts/api_tests.py
@@ -336,6 +336,8 @@ class TestChartApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCase):
assert rv.status_code == 200
model = db.session.query(Slice).get(chart_id)
assert model is None
+ log = self.get_latest_log("ChartRestApi.delete")
+ assert log.slice_id == chart_id
def test_delete_bulk_charts(self):
"""
@@ -359,6 +361,11 @@ class TestChartApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCase):
for chart_id in chart_ids:
model = db.session.query(Slice).get(chart_id)
assert model is None
+ # a single integer column cannot hold every id, so the full list is
+ # recorded in the JSON payload instead
+ log = self.get_latest_log("ChartRestApi.bulk_delete")
+ assert log.slice_id is None
+ assert json.loads(log.json)["slice_ids"] == chart_ids
def test_delete_bulk_chart_bad_request(self):
"""
@@ -579,6 +586,8 @@ class TestChartApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCase):
# uuid should be returned in the response
assert "uuid" in data
assert str(model.uuid) == str(data["uuid"])
+ log = self.get_latest_log("ChartRestApi.post")
+ assert log.slice_id == model.id
db.session.delete(model)
db.session.commit()
@@ -867,6 +876,8 @@ class TestChartApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCase):
uri = f"api/v1/chart/{chart_id}"
rv = self.put_assert_metric(uri, chart_data, "put")
assert rv.status_code == 200
+ log = self.get_latest_log("ChartRestApi.put")
+ assert log.slice_id == chart_id
model = db.session.query(Slice).get(chart_id)
related_dashboard =
db.session.query(Dashboard).filter_by(slug="births").first()
assert model.created_by == admin
diff --git a/tests/integration_tests/dashboards/api_tests.py
b/tests/integration_tests/dashboards/api_tests.py
index 8cbe5763029..00d4853b082 100644
--- a/tests/integration_tests/dashboards/api_tests.py
+++ b/tests/integration_tests/dashboards/api_tests.py
@@ -1663,6 +1663,8 @@ class TestDashboardApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCa
assert rv.status_code == 200
model = db.session.query(Dashboard).get(dashboard_id)
assert model is None
+ log = self.get_latest_log("DashboardRestApi.delete")
+ assert log.dashboard_id == dashboard_id
def test_delete_bulk_dashboards(self):
"""
@@ -1690,6 +1692,11 @@ class TestDashboardApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCa
for dashboard_id in dashboard_ids:
model = db.session.query(Dashboard).get(dashboard_id)
assert model is None
+ # a single integer column cannot hold every id, so the full list is
+ # recorded in the JSON payload instead
+ log = self.get_latest_log("DashboardRestApi.bulk_delete")
+ assert log.dashboard_id is None
+ assert json.loads(log.json)["dashboard_ids"] == dashboard_ids
def test_delete_bulk_embedded_dashboards(self):
"""
@@ -1967,6 +1974,8 @@ class TestDashboardApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCa
# uuid should be returned in the response
assert "uuid" in data
assert str(model.uuid) == str(data["uuid"])
+ log = self.get_latest_log("DashboardRestApi.post")
+ assert log.dashboard_id == model.id
db.session.delete(model)
db.session.commit()
@@ -2330,6 +2339,8 @@ class TestDashboardApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCa
uri = f"api/v1/dashboard/{dashboard_id}"
rv = self.put_assert_metric(uri, self.dashboard_data, "put")
assert rv.status_code == 200
+ log = self.get_latest_log("DashboardRestApi.put")
+ assert log.dashboard_id == dashboard_id
model = db.session.query(Dashboard).get(dashboard_id)
assert model.dashboard_title == self.dashboard_data["dashboard_title"]
assert model.slug == self.dashboard_data["slug"]
@@ -2449,6 +2460,8 @@ class TestDashboardApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCa
uri = f"api/v1/dashboard/{dashboard_id}/filters"
rv = self.put_assert_metric(uri, self.dashboard_put_filters_data,
"put_filters")
assert rv.status_code == 200
+ log = self.get_latest_log("DashboardRestApi.put_filters")
+ assert log.dashboard_id == dashboard_id
model = db.session.query(Dashboard).get(dashboard_id)
json_metadata = model.json_metadata
native_filter_config =
json.loads(json_metadata)["native_filter_configuration"]
@@ -2622,6 +2635,8 @@ class TestDashboardApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCa
}
rv = self.put_assert_metric(uri, put_data, "put_chart_customizations")
assert rv.status_code == 200
+ log = self.get_latest_log("DashboardRestApi.put_chart_customizations")
+ assert log.dashboard_id == dashboard_id
model = db.session.query(Dashboard).get(dashboard_id)
json_metadata = model.json_metadata
chart_customization_config = json.loads(json_metadata)[
@@ -4419,6 +4434,8 @@ class TestDashboardApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCa
assert rv.status_code == 200
response = json.loads(rv.data.decode("utf-8"))
assert response == {"result": {"id": ANY, "last_modified_time": ANY}}
+ log = self.get_latest_log("DashboardRestApi.copy_dash")
+ assert log.dashboard_id == pk
dash = (
db.session.query(Dashboard)
@@ -4849,8 +4866,8 @@ class TestDashboardApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCa
mock_get_from_cache_key.side_effect = lambda cache_key, **_kwargs:
payloads.get(
cache_key
)
- mock_store_cache_payload.side_effect = (
- lambda cache_key, payload: payloads.__setitem__(cache_key, payload)
+ mock_store_cache_payload.side_effect = lambda cache_key, payload: (
+ payloads.__setitem__(cache_key, payload)
)
def publish(_request_key, cache_key, _scope, _previous_cache_key):
@@ -5074,8 +5091,8 @@ class TestDashboardApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCa
mock_get_from_cache_key.side_effect = lambda cache_key, **_kwargs:
payloads.get(
cache_key
)
- mock_store_cache_payload.side_effect = (
- lambda cache_key, payload: payloads.__setitem__(cache_key, payload)
+ mock_store_cache_payload.side_effect = lambda cache_key, payload: (
+ payloads.__setitem__(cache_key, payload)
)
def publish(_request_key, cache_key, _scope, previous_cache_key):
@@ -6236,6 +6253,8 @@ class TestDashboardApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCa
uri = f"api/v1/dashboard/{dashboard.id}/colors"
rv = self.client.put(uri, json=colors)
assert rv.status_code == 200
+ log = self.get_latest_log("DashboardRestApi.put_colors")
+ assert log.dashboard_id == dashboard.id
updated_dashboard = db.session.query(Dashboard).get(dashboard.id)
updated_label_colors = json.loads(updated_dashboard.json_metadata).get(
diff --git a/tests/integration_tests/dashboards/soft_delete_tests.py
b/tests/integration_tests/dashboards/soft_delete_tests.py
index 9174a6d59f0..4869137a12e 100644
--- a/tests/integration_tests/dashboards/soft_delete_tests.py
+++ b/tests/integration_tests/dashboards/soft_delete_tests.py
@@ -389,6 +389,9 @@ class TestDashboardRestore(SupersetTestCase):
self.client.delete(f"/api/v1/dashboard/{dashboard_id}")
rv = self.client.post(f"/api/v1/dashboard/{dashboard_uuid}/restore")
assert rv.status_code == 200
+ # the UUID route still resolves to the archived row's integer id
+ log = self.get_latest_log("DashboardRestApi.restore")
+ assert log.dashboard_id == dashboard_id
rv = self.client.get(f"/api/v1/dashboard/{dashboard_id}")
assert rv.status_code == 200
@@ -876,6 +879,10 @@ class TestDashboardArchiveListing(SupersetTestCase):
rv = self.client.post(f"/api/v1/dashboard/{dashboard_uuid}/purge")
assert rv.status_code == 200, rv.data
+ # the UUID resolves to the row's integer id before it's gone for good
+ log = self.get_latest_log("DashboardRestApi.purge")
+ assert log.dashboard_id == dashboard_id
+
row = (
db.session.query(Dashboard)
.execution_options(**{SKIP_VISIBILITY_FILTER_CLASSES: {Dashboard}})
diff --git a/tests/unit_tests/utils/log_tests.py
b/tests/unit_tests/utils/log_tests.py
index 5b031b57788..121b26552ae 100644
--- a/tests/unit_tests/utils/log_tests.py
+++ b/tests/unit_tests/utils/log_tests.py
@@ -15,8 +15,21 @@
# specific language governing permissions and limitations
# under the License.
+import uuid
+from datetime import datetime, timezone
+from types import SimpleNamespace
+from typing import Any
-from superset.utils.log import get_logger_from_status
+import pytest
+from flask import current_app
+from pytest_mock import MockerFixture
+from sqlalchemy.orm.session import Session
+
+from superset.utils.log import (
+ DBEventLogger,
+ get_logger_from_status,
+ get_object_ids_from_view_args,
+)
def test_log_from_status_exception() -> None:
@@ -35,3 +48,138 @@ def test_log_from_status_info() -> None:
(func, log_level) = get_logger_from_status(300)
assert func.__name__ == "info"
assert log_level == "info"
+
+
+# Stand-ins for the models behind ``DashboardRestApi`` / ``ChartRestApi``
+# ``datamodel``: the helper only inspects the class name, so no ORM is needed.
+_Dashboard = type("Dashboard", (), {})
+_Slice = type("Slice", (), {})
+# A model that ``logs`` has no id column for.
+_Database = type("Database", (), {})
+
+
+def _view_for(model: type) -> SimpleNamespace:
+ """Build the minimal REST API shape the event logger inspects."""
+ return SimpleNamespace(datamodel=SimpleNamespace(obj=model))
+
+
[email protected](
+ "model,view_args,expected",
+ [
+ (_Dashboard, {"pk": 42}, {"dashboard_id": 42}),
+ (_Dashboard, {"pk": "42"}, {"dashboard_id": 42}),
+ (_Dashboard, {"id_or_slug": "7"}, {"dashboard_id": 7}),
+ (_Slice, {"pk": "3"}, {"slice_id": 3}),
+ (_Slice, {"id_or_uuid": 3}, {"slice_id": 3}),
+ (_Dashboard, {"rison": [1, 2, 3]}, {"dashboard_ids": [1, 2, 3]}),
+ (_Slice, {"rison": [5]}, {"slice_ids": [5]}),
+ # rison payloads that are not a list of ids (list endpoints,
thumbnails)
+ (_Dashboard, {"rison": {"columns": ["id"]}}, {}),
+ (_Dashboard, {"rison": []}, {}),
+ (_Dashboard, {"rison": [1, "a"]}, {}),
+ # routes with no object identifier at all (create, import, list)
+ (_Dashboard, {}, {}),
+ # a route parameter takes precedence over a rison list
+ (_Dashboard, {"pk": 9, "rison": [1, 2]}, {"dashboard_id": 9}),
+ # models without a ``logs`` column never contribute ids
+ (_Database, {"pk": 1}, {}),
+ (_Database, {"rison": [1, 2]}, {}),
+ ],
+)
+def test_get_object_ids_from_view_args(
+ model: type, view_args: dict[str, Any], expected: dict[str, Any]
+) -> None:
+ assert get_object_ids_from_view_args(_view_for(model), view_args) ==
expected
+
+
+def test_get_object_ids_from_view_args_without_datamodel() -> None:
+ """Plain views and free functions decorated with the logger are ignored."""
+ assert get_object_ids_from_view_args(None, {"pk": 1}) == {}
+ assert get_object_ids_from_view_args(object(), {"pk": 1}) == {}
+
+
+def test_get_object_ids_from_view_args_resolves_slug_and_uuid(
+ session: Session,
+) -> None:
+ """Slug and UUID routes resolve to the integer id, even when archived."""
+ from superset.models.core import FavStar # noqa: F401
+ from superset.models.dashboard import Dashboard
+
+ Dashboard.metadata.create_all(session.get_bind()) # pylint:
disable=no-member
+ dashboard = Dashboard(
+ id=100,
+ dashboard_title="audited",
+ slug="audited-slug",
+ uuid=uuid.uuid4(),
+ deleted_at=datetime.now(timezone.utc),
+ )
+ session.add(dashboard)
+ session.commit()
+
+ view = _view_for(Dashboard)
+ assert get_object_ids_from_view_args(view, {"id_or_slug": "audited-slug"})
== {
+ "dashboard_id": 100
+ }
+ assert get_object_ids_from_view_args(view, {"uuid": str(dashboard.uuid)})
== {
+ "dashboard_id": 100
+ }
+ assert get_object_ids_from_view_args(view, {"uuid_str":
str(uuid.uuid4())}) == {}
+ assert get_object_ids_from_view_args(view, {"id_or_slug": "missing"}) == {}
+
+
+def test_log_this_with_context_derives_object_id_from_route(
+ app_context: None, mocker: MockerFixture
+) -> None:
+ """``log_this_with_context`` fills ``dashboard_id`` from the route's pk."""
+ mock_log = mocker.patch.object(DBEventLogger, "log")
+ logger = DBEventLogger()
+
+ class FakeDashboardRestApi: # pylint: disable=too-few-public-methods
+ datamodel = SimpleNamespace(obj=_Dashboard)
+
+ @logger.log_this_with_context(action="DashboardRestApi.delete")
+ def delete(self, pk: int) -> str:
+ return f"deleted {pk}"
+
+ with current_app.test_request_context("/api/v1/dashboard/42",
method="DELETE"):
+ assert FakeDashboardRestApi().delete(pk="42") == "deleted 42"
+
+ payload = mock_log.call_args[1]
+ assert payload["dashboard_id"] == 42
+ # ``to_int`` turns a missing id into 0; the DB logger stores that as NULL.
+ assert not payload["slice_id"]
+ assert payload["records"][0]["dashboard_id"] == 42
+ assert payload["records"][0]["pk"] == "42"
+
+
+def test_log_this_with_context_derives_object_id_despite_outer_decorator(
+ app_context: None, mocker: MockerFixture
+) -> None:
+ """``DashboardRestApi.get`` sits behind ``with_dashboard``, which calls the
+ logged view positionally (``f(self, dash)``) after resolving the id,
+ leaving the logger's own ``kwargs`` empty. The id must still come from
+ ``request.view_args``, with no ``add_extra_log_payload`` call in the view.
+ """
+ mock_log = mocker.patch.object(DBEventLogger, "log")
+ logger = DBEventLogger()
+
+ class FakeDashboardRestApi: # pylint: disable=too-few-public-methods
+ datamodel = SimpleNamespace(obj=_Dashboard)
+
+ @logger.log_this_with_context(action="DashboardRestApi.get")
+ def get(self, dash_id: int) -> str:
+ return f"got {dash_id}"
+
+ def with_dashboard(f: Any) -> Any:
+ def wraps(self: Any, id_or_slug: str) -> Any:
+ return f(self, int(id_or_slug))
+
+ return wraps
+
+ FakeDashboardRestApi.get = with_dashboard(FakeDashboardRestApi.get) #
type: ignore[method-assign]
+
+ with current_app.test_request_context("/api/v1/dashboard/42",
method="GET"):
+ assert FakeDashboardRestApi().get("42") == "got 42"
+
+ payload = mock_log.call_args[1]
+ assert payload["dashboard_id"] == 42
