Stop storing patron neighborhood

alembic/versions/20250307_80a70e6ec724_stop_storing_location_information.py

@@ -0,0 +1,45 @@
+"""Stop storing location information
+
+Revision ID: 80a70e6ec724
+Revises: 61df6012a5e6
+Create Date: 2025-03-07 02:32:46.583364+00:00
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "80a70e6ec724"
+down_revision = "61df6012a5e6"
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    op.drop_index("ix_circulationevents_location", table_name="circulationevents")
+    op.drop_column("circulationevents", "location")
+    op.drop_index("ix_patrons_cached_neighborhood", table_name="patrons")
+    op.drop_column("patrons", "cached_neighborhood")
+
+
+def downgrade() -> None:
+    op.add_column(
+        "patrons",
+        sa.Column(
+            "cached_neighborhood", sa.VARCHAR(), autoincrement=False, nullable=True
+        ),
+    )
+    op.create_index(
+        "ix_patrons_cached_neighborhood",
+        "patrons",
+        ["cached_neighborhood"],
+        unique=False,
+    )
+    op.add_column(
+        "circulationevents",
+        sa.Column("location", sa.VARCHAR(), autoincrement=False, nullable=True),
+    )
+    op.create_index(
+        "ix_circulationevents_location", "circulationevents", ["location"], unique=False
+    )

src/palace/manager/api/admin/controller/dashboard.py

@@ -93,14 +93,11 @@ def get_date(field):
         # though, we should use the date provided by the user.
         date_end_label = get_date("dateEnd")
         date_end = date_end_label + timedelta(days=1)
-        locations = flask.request.args.get("locations", None)
         library = get_request_library(default=None)
         library_short_name = library.short_name if library else None
 
         analytics_exporter = analytics_exporter or LocalAnalyticsExporter()
-        data = analytics_exporter.export(
-            self._db, date_start, date_end, locations, library
-        )
+        data = analytics_exporter.export(self._db, date_start, date_end, library)
         return (
             data,
             date_start.strftime(date_format),

src/palace/manager/api/authentication/base.py

@@ -175,8 +175,6 @@ def __init__(
         fines=None,
         block_reason=None,
         library_identifier=None,
-        neighborhood=None,
-        cached_neighborhood=None,
         complete=True,
     ):
         """Store basic information about a patron.
@@ -232,24 +230,6 @@ def __init__(
         :param library_identifier: A string pulled from the ILS that
         is used to determine if this user belongs to the current library.
 
-        :param neighborhood: A string pulled from the ILS that
-        identifies the patron's geographic location in a deliberately
-        imprecise way that makes sense to the library -- maybe the
-        patron's ZIP code or the name of their home branch. This data
-        is never stored in a way that can be associated with an
-        individual patron. Depending on library policy, this data may
-        be associated with circulation events -- but a circulation
-        event is not associated with the patron who triggered it.
-
-        :param cached_neighborhood: This is the same as neighborhood,
-        but it _will_ be cached in the patron's database record, for
-        up to twelve hours. This should only be used by ILS systems
-        that would have performance problems fetching patron
-        neighborhood on demand.
-
-        If cached_neighborhood is set but neighborhood is not,
-        cached_neighborhood will be used as neighborhood.
-
         :param complete: Does this PatronData represent the most
         complete data we are likely to get for this patron from this
         data source, or is it an abbreviated version of more complete
@@ -274,12 +254,6 @@ def __init__(
         # to have it available for notifications.
         self.email_address = email_address
 
-        # If cached_neighborhood (cached in the database) is provided
-        # but neighborhood (destroyed at end of request) is not, use
-        # cached_neighborhood as neighborhood.
-        self.neighborhood = neighborhood or cached_neighborhood
-        self.cached_neighborhood = cached_neighborhood
-
     def __eq__(self, other):
         """
         Compares two PatronData objects
@@ -305,8 +279,6 @@ def __eq__(self, other):
             and self.complete == other.complete
             and self.personal_name == other.personal_name
             and self.email_address == other.email_address
-            and self.neighborhood == other.neighborhood
-            and self.cached_neighborhood == other.cached_neighborhood
         )
 
     def __repr__(self):
@@ -336,11 +308,6 @@ def apply(self, patron: Patron):
         self.set_value(patron, "authorization_expires", self.authorization_expires)
         self.set_value(patron, "fines", self.fines)
         self.set_value(patron, "block_reason", self.block_reason)
-        self.set_value(patron, "cached_neighborhood", self.cached_neighborhood)
-
-        # Patron neighborhood (not a database field) is set as a
-        # convenience.
-        patron.neighborhood = self.neighborhood or self.cached_neighborhood
 
         # Now handle authorization identifier.
         if self.complete:

src/palace/manager/api/authentication/basic.py

@@ -862,12 +862,6 @@ def authenticated_patron(
             return patron
         if PatronUtility.needs_external_sync(patron):
             self.update_patron_metadata(patron)
-        if patron.cached_neighborhood and not patron.neighborhood:
-            # Patron.neighborhood (which is not a model field) was not
-            # set, probably because we avoided an expensive metadata
-            # update. But we have a cached_neighborhood (which _is_ a
-            # model field) to use in situations like this.
-            patron.neighborhood = patron.cached_neighborhood
         return patron
 
     def update_patron_metadata(self, patron: Patron) -> Patron | None:

src/palace/manager/api/circulation.py

@@ -943,7 +943,6 @@ def _collect_event(
         patron: Patron | None,
         licensepool: LicensePool | None,
         name: str,
-        include_neighborhood: bool = False,
     ) -> None:
         """Collect an analytics event.
 
@@ -952,19 +951,12 @@ def _collect_event(
             patron will be used.
         :param licensepool: The LicensePool associated with the event.
         :param name: The name of the event.
-        :param include_neighborhood: If this is True, _and_ the
-            current request's authenticated patron is the same as the
-            patron in `patron`, _and_ the authenticated patron has
-            associated neighborhood information obtained from the ILS,
-            then that neighborhood information (but not the patron's
-            identity) will be associated with the circulation event.
         """
         if not self.analytics:
             return
 
-        # It would be really useful to know which patron caused this
-        # this event -- this will help us get a library and
-        # potentially a neighborhood.
+        # It would be really useful to know which patron caused
+        # this event -- this will help us get a library
         if flask.request:
             request_patron = getattr(flask.request, "patron", None)
         else:
@@ -983,20 +975,10 @@ def _collect_event(
             # have a library associated with it.
             library = get_request_library(default=self.library)
 
-        neighborhood = None
-        if (
-            include_neighborhood
-            and flask.request
-            and request_patron
-            and request_patron == patron
-        ):
-            neighborhood = getattr(request_patron, "neighborhood", None)
-
         self.analytics.collect_event(
             library,
             licensepool,
             name,
-            neighborhood=neighborhood,
             patron=patron,
         )
 
@@ -1006,9 +988,7 @@ def _collect_checkout_event(self, patron: Patron, licensepool: LicensePool) -> N
         This is called in two different places -- one when loaning
         licensed books and one when 'loaning' open-access books.
         """
-        return self._collect_event(
-            patron, licensepool, CirculationEvent.CM_CHECKOUT, include_neighborhood=True
-        )
+        return self._collect_event(patron, licensepool, CirculationEvent.CM_CHECKOUT)
 
     def borrow(
         self,
@@ -1179,7 +1159,6 @@ def borrow(
                     patron=patron,
                     licensepool=licensepool,
                     name=CirculationEvent.CM_HOLD_CONVERTED_TO_LOAN,
-                    include_neighborhood=True,
                 )
 
                 # Delete the record of the hold.
@@ -1412,9 +1391,7 @@ def fulfill(
         # Send out an analytics event to record the fact that
         # a fulfillment was initiated through the circulation
         # manager.
-        self._collect_event(
-            patron, licensepool, CirculationEvent.CM_FULFILL, include_neighborhood=True
-        )
+        self._collect_event(patron, licensepool, CirculationEvent.CM_FULFILL)
 
         # Make sure the delivery mechanism we just used is associated
         # with the loan, if any.

src/palace/manager/api/controller/analytics.py

@@ -22,9 +22,6 @@ def track_event(self, identifier_type, identifier, event_type):
             # Authentication on the AnalyticsController is optional,
             # so we may not have a patron.
             patron = get_request_patron(default=None)
-            neighborhood = None
-            if patron:
-                neighborhood = getattr(patron, "neighborhood", None)
             pools = self.load_licensepools(library, identifier_type, identifier)
             if isinstance(pools, ProblemDetail):
                 return pools
@@ -33,7 +30,6 @@ def track_event(self, identifier_type, identifier, event_type):
                 pools[0],
                 event_type,
                 utc_now(),
-                neighborhood=neighborhood,
                 patron=patron,
             )
             return Response({}, 200)

src/palace/manager/api/local_analytics_exporter.py

@@ -19,9 +19,9 @@
 class LocalAnalyticsExporter:
     """Export large numbers of analytics events in CSV format."""
 
-    def export(self, _db, start, end, locations=None, library=None):
+    def export(self, _db, start, end, library=None):
         # Get the results from the database.
-        query = self.analytics_query(start, end, locations, library)
+        query = self.analytics_query(start, end, library)
         results = _db.execute(query)
 
         # Write the CSV file to a BytesIO.
@@ -39,7 +39,6 @@ def export(self, _db, start, end, locations=None, library=None):
             "language",
             "target_age",
             "genres",
-            "location",
             "collection_name",
             "library_short_name",
             "library_name",
@@ -53,7 +52,7 @@ def export(self, _db, start, end, locations=None, library=None):
         writer.writerows(results)
         return output.getvalue().decode("utf-8")
 
-    def analytics_query(self, start, end, locations=None, library=None):
+    def analytics_query(self, start, end, library=None):
         """Build a database query that fetches rows of analytics data.
 
         This method uses low-level SQLAlchemy code to do all
@@ -70,19 +69,6 @@ def analytics_query(self, start, end, locations=None, library=None):
             CirculationEvent.start < end,
         ]
 
-        if locations:
-            event_types = [
-                CirculationEvent.CM_CHECKOUT,
-                CirculationEvent.CM_FULFILL,
-                CirculationEvent.OPEN_BOOK,
-            ]
-            locations = locations.strip().split(",")
-
-            clauses += [
-                CirculationEvent.type.in_(event_types),
-                CirculationEvent.location.in_(locations),
-            ]
-
         if library:
             clauses += [CirculationEvent.library == library]
 
@@ -109,7 +95,6 @@ def analytics_query(self, start, end, locations=None, library=None):
                     Edition.publisher,
                     Edition.imprint,
                     Edition.language,
-                    CirculationEvent.location,
                     IntegrationConfiguration.name.label("collection_name"),
                     Library.short_name.label("library_short_name"),
                     Library.name.label("library_name"),
@@ -213,7 +198,6 @@ def analytics_query(self, start, end, locations=None, library=None):
                 events.language,
                 target_age_string.label("target_age"),
                 genres.label("genres"),
-                events.location,
                 events.collection_name,
                 events.library_short_name,
                 events.library_name,