Ensure location is captured in s3 analytics provider and improve type…

… hints and docstrings

src/palace/manager/api/s3_analytics_provider.py

@@ -4,7 +4,7 @@
 import json
 import random
 import string
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Any
 
 from palace.manager.core.config import CannotLoadConfiguration
 from palace.manager.core.local_analytics_provider import LocalAnalyticsProvider
@@ -141,21 +141,18 @@ def _create_event_object(
 
     def collect_event(
         self,
-        library,
-        license_pool,
-        event_type,
-        time,
-        old_value=None,
-        new_value=None,
+        library: Library,
+        license_pool: LicensePool,
+        event_type: str,
+        time: datetime,
+        old_value: Any = None,
+        new_value: Any = None,
         user_agent: str | None = None,
         patron: Patron | None = None,
         **kwargs,
     ):
         """Log the event using the appropriate for the specific provider's mechanism.
 
-        :param db: Database session
-        :type db: sqlalchemy.orm.session.Session
-
         :param library: Library associated with the event
         :type library: core.model.library.Library
 
@@ -168,9 +165,6 @@ def collect_event(
         :param time: Event's timestamp
         :type time: datetime.datetime
 
-        :param neighborhood: Geographic location of the event
-        :type neighborhood: str
-
         :param old_value: Old value of the metric changed by the event
         :type old_value: Any
 
@@ -179,6 +173,9 @@ def collect_event(
 
         :param user_agent: The user_agent of the caller.
         :type user_agent:  str
+
+        :param patron: The patron associated with the event, where applicable
+        :type patron: Patron
         """
 
         if not library and not license_pool:
@@ -193,6 +190,7 @@ def collect_event(
             new_value,
             user_agent=user_agent,
             patron=patron,
+            **kwargs,
         )
         content = json.dumps(
             event,

src/palace/manager/service/analytics/analytics.py

@@ -31,7 +31,7 @@ def __init__(
                     "S3 analytics is not configured: No analytics bucket was specified."
                 )
 
-    def collect_event(self, library, license_pool, event_type, time=None, patron: Patron | None = None, **kwargs):  # type: ignore[no-untyped-def]
+    def collect_event(self, library: Library, license_pool: LicensePool, event_type: str, time: datetime = None, patron: Patron | None = None, **kwargs):  # type: ignore[no-untyped-def]
         if not time:
             time = utc_now()
 

tests/manager/core/test_s3_analytics_provider.py

@@ -112,6 +112,8 @@ def test_analytics_data_with_associated_license_pool_is_correctly_stored_in_s3(
             audience=Classifier.AUDIENCE_ADULT,
             with_license_pool=True,
         )
+
+        neighborhood = "test neighborhood"
         license_pool = work.license_pools[0]
         edition = work.presentation_edition
 
@@ -130,6 +132,7 @@ def test_analytics_data_with_associated_license_pool_is_correctly_stored_in_s3(
             event_time,
             user_agent=user_agent,
             patron=patron,
+            neighborhood=neighborhood,
         )
 
         # Assert
@@ -185,3 +188,4 @@ def test_analytics_data_with_associated_license_pool_is_correctly_stored_in_s3(
         assert event["language"] == work.language
         assert event["user_agent"] == user_agent
         assert event["patron_uuid"] == str(patron.uuid)
+        assert event["location"] == neighborhood