Merge branch 'develop' into missing-migrations

Author: ntrncic

src/hope/apps/utils/sentry.py

@@ -1,28 +1,26 @@
 from functools import wraps
 import logging
-from typing import TYPE_CHECKING, Callable, ParamSpec, TypeVar
+from typing import TYPE_CHECKING, Callable
 
-from sentry_sdk import configure_scope, set_tag
+import sentry_sdk
+from sentry_sdk import set_tag
 
 if TYPE_CHECKING:
     from sentry_sdk.types import Event, Hint
 
 log = logging.getLogger(__name__)
 
-P = ParamSpec("P")
-R = TypeVar("R")
-
 
 def sentry_tags[**P, R](func: Callable[P, R]) -> Callable[P, R]:
     """Add sentry tags 'celery' and 'celery_task'."""
 
     @wraps(func)
     def wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
-        with configure_scope() as scope:
-            scope.set_tag("celery", True)
-            scope.set_tag("celery_task", func.__name__)
+        scope = sentry_sdk.get_isolation_scope()
+        scope.set_tag("celery", True)
+        scope.set_tag("celery_task", func.__name__)
 
-            return func(*args, **kwargs)
+        return func(*args, **kwargs)
 
     return wrapper
 

src/hope/middlewares/sentry.py

@@ -4,7 +4,7 @@
 
 from django.http import HttpRequest
 import pycountry
-from sentry_sdk import configure_scope
+import sentry_sdk
 
 
 def is_country_name(country_name: str) -> bool:
@@ -25,14 +25,14 @@ def __init__(self, get_response: Callable) -> None:
     # Note: must be listed AFTER AuthenticationMiddleware
     def __call__(self, request: HttpRequest) -> Any:
         sys.stderr.isatty = lambda: False  # type: ignore # I guess this is a hack to make Sentry not use colors in the terminal?
-        with configure_scope() as scope:
-            business_area = request.headers.get("Business-Area")
-            if not business_area:
-                # example: api/rest/ukraine/rdi/upload/; api/admin/account/role/;
-                # all api urls with BA has pattern 'api/rest/BA/etc'
-                pattern = r"api/rest/(?P<country>[^/]+)/"
-                match = re.search(pattern, request.path)
-                business_area = match.group("country") if match else self.business_area
-            scope.set_tag("username", request.user.username)
-            scope.set_tag("business_area", business_area)
-            return self.get_response(request)
+        scope = sentry_sdk.get_isolation_scope()
+        business_area = request.headers.get("Business-Area")
+        if not business_area:
+            # example: api/rest/ukraine/rdi/upload/; api/admin/account/role/;
+            # all api urls with BA has pattern 'api/rest/BA/etc'
+            pattern = r"api/rest/(?P<country>[^/]+)/"
+            match = re.search(pattern, request.path)
+            business_area = match.group("country") if match else self.business_area
+        scope.set_tag("username", request.user.username)
+        scope.set_tag("business_area", business_area)
+        return self.get_response(request)

tests/unit/apps/utils/test_sentry.py

@@ -0,0 +1,23 @@
+from unittest.mock import MagicMock, patch
+
+from hope.apps.utils.sentry import sentry_tags
+
+
+@patch("hope.apps.utils.sentry.sentry_sdk")
+def test_sentry_tags_sets_celery_scope_tags_and_preserves_return_value(
+    mocked_sentry: MagicMock,
+) -> None:
+    scope = MagicMock()
+    mocked_sentry.get_isolation_scope.return_value = scope
+
+    @sentry_tags
+    def my_task(value: int) -> int:
+        return value * 2
+
+    result = my_task(21)
+
+    assert result == 42
+    assert my_task.__name__ == "my_task"
+    mocked_sentry.get_isolation_scope.assert_called_once_with()
+    scope.set_tag.assert_any_call("celery", True)
+    scope.set_tag.assert_any_call("celery_task", "my_task")

tests/unit/middlewares/__init__.py

@@ -0,0 +1,61 @@
+from unittest.mock import MagicMock, patch
+
+from hope.middlewares.sentry import SentryScopeMiddleware
+
+
+@patch("hope.middlewares.sentry.sentry_sdk")
+def test_call_uses_business_area_header_when_present(mocked_sentry: MagicMock) -> None:
+    scope = MagicMock()
+    mocked_sentry.get_isolation_scope.return_value = scope
+    response_sentinel = object()
+    get_response = MagicMock(return_value=response_sentinel)
+    middleware = SentryScopeMiddleware(get_response)
+    request = MagicMock()
+    request.headers = {"Business-Area": "ukraine"}
+    request.path = "/api/rest/afghanistan/rdi/upload/"
+    request.user.username = "alice"
+
+    result = middleware(request)
+
+    assert result is response_sentinel
+    get_response.assert_called_once_with(request)
+    scope.set_tag.assert_any_call("username", "alice")
+    scope.set_tag.assert_any_call("business_area", "ukraine")
+
+
+@patch("hope.middlewares.sentry.sentry_sdk")
+def test_call_derives_business_area_from_api_rest_path_when_header_missing(
+    mocked_sentry: MagicMock,
+) -> None:
+    scope = MagicMock()
+    mocked_sentry.get_isolation_scope.return_value = scope
+    get_response = MagicMock()
+    middleware = SentryScopeMiddleware(get_response)
+    request = MagicMock()
+    request.headers = {}
+    request.path = "/api/rest/ukraine/rdi/upload/"
+    request.user.username = "bob"
+
+    middleware(request)
+
+    scope.set_tag.assert_any_call("username", "bob")
+    scope.set_tag.assert_any_call("business_area", "ukraine")
+
+
+@patch("hope.middlewares.sentry.sentry_sdk")
+def test_call_falls_back_to_default_business_area_when_path_does_not_match(
+    mocked_sentry: MagicMock,
+) -> None:
+    scope = MagicMock()
+    mocked_sentry.get_isolation_scope.return_value = scope
+    get_response = MagicMock()
+    middleware = SentryScopeMiddleware(get_response)
+    request = MagicMock()
+    request.headers = {}
+    request.path = "/api/admin/account/role/"
+    request.user.username = "carol"
+
+    middleware(request)
+
+    scope.set_tag.assert_any_call("username", "carol")
+    scope.set_tag.assert_any_call("business_area", "NO_BA")