rusackas commented on code in PR #44553:
URL: https://github.com/apache/superset/pull/44553#discussion_r4082938301
##########
tests/unit_tests/utils/log_tests.py:
##########
@@ -35,3 +48,105 @@ 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(),
Review Comment:
Fixed - switched to `datetime.now(timezone.utc)` to match the convention in
`dao/dashboard_test.py` and the other soft-delete fixtures.
##########
tests/unit_tests/utils/log_tests.py:
##########
@@ -35,3 +48,105 @@ 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
Review Comment:
Function-scoped model imports are the pattern the rest of this test file
(and sibling unit tests like test_subdirectory_url_for.py,
datasource/dao_tests.py) follow to keep the module import-light and only pull
in real ORM models where a test needs app/db context. Leaving it local.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
