From 3f5d5aeba8c3ec2cd101ecae6465454b08545091 Mon Sep 17 00:00:00 2001 From: dbernstein Date: Wed, 4 Sep 2024 13:52:23 -0700 Subject: [PATCH] [PP-1456] Add loan count column to audio book playback time reports. (#2002) --- ...an_identifier_column_to_playtime_tables.py | 59 ++++++ .../api/controller/playtime_entries.py | 44 ++++- .../manager/core/query/playtime_entries.py | 2 + .../manager/scripts/playtime_entries.py | 96 +++++++-- .../manager/sqlalchemy/model/time_tracking.py | 6 + .../api/controller/test_playtime_entries.py | 39 +++- .../manager/scripts/test_playtime_entries.py | 182 ++++++++++++++++-- .../sqlalchemy/model/test_time_tracking.py | 17 +- 8 files changed, 407 insertions(+), 38 deletions(-) create mode 100644 alembic/versions/20240821_7a2fcaac8b63_add_loan_identifier_column_to_playtime_tables.py diff --git a/alembic/versions/20240821_7a2fcaac8b63_add_loan_identifier_column_to_playtime_tables.py b/alembic/versions/20240821_7a2fcaac8b63_add_loan_identifier_column_to_playtime_tables.py new file mode 100644 index 0000000000..d305375e5c --- /dev/null +++ b/alembic/versions/20240821_7a2fcaac8b63_add_loan_identifier_column_to_playtime_tables.py @@ -0,0 +1,59 @@ +"""Add loan_identifier column to playtime tables. + +Revision ID: 7a2fcaac8b63 +Revises: 7ba553f3f80d +Create Date: 2024-08-21 23:23:48.085451+00:00 + +""" +import sqlalchemy as sa +from alembic import op +from sqlalchemy.orm.session import Session + +# revision identifiers, used by Alembic. +revision = "7a2fcaac8b63" +down_revision = "7ba553f3f80d" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + session = Session(bind=op.get_bind()) + conn = session.connection() + + op.add_column( + "playtime_entries", + sa.Column("loan_identifier", sa.String(length=40), nullable=False, default=""), + ) + + op.add_column( + "playtime_summaries", + sa.Column("loan_identifier", sa.String(length=40), nullable=False, default=""), + ) + + op.drop_constraint("unique_playtime_summary", "playtime_summaries", type_="unique") + + op.create_unique_constraint( + "unique_playtime_summary", + "playtime_summaries", + [ + "timestamp", + "identifier_str", + "collection_name", + "library_name", + "loan_identifier", + ], + ) + + +def downgrade() -> None: + op.drop_column("playtime_entries", "loan_identifier") + + op.drop_constraint("unique_playtime_summary", "playtime_summaries", type_="unique") + + op.drop_column("playtime_summaries", "loan_identifier") + + op.create_unique_constraint( + "unique_playtime_summary", + "playtime_summaries", + ["timestamp", "identifier_str", "collection_name", "library_name"], + ) diff --git a/src/palace/manager/api/controller/playtime_entries.py b/src/palace/manager/api/controller/playtime_entries.py index faa08b932d..0bae2039d4 100644 --- a/src/palace/manager/api/controller/playtime_entries.py +++ b/src/palace/manager/api/controller/playtime_entries.py @@ -1,7 +1,10 @@ from __future__ import annotations +import hashlib + import flask from pydantic import ValidationError +from sqlalchemy import select from palace.manager.api.controller.circulation_manager import ( CirculationManagerController, @@ -16,8 +19,23 @@ from palace.manager.sqlalchemy.model.collection import Collection from palace.manager.sqlalchemy.model.identifier import Identifier from palace.manager.sqlalchemy.model.library import Library +from palace.manager.sqlalchemy.model.licensing import LicensePool +from palace.manager.sqlalchemy.model.patron import Loan from palace.manager.sqlalchemy.util import get_one +MISSING_LOAN_IDENTIFIER = "LOAN_NOT_FOUND" + + +def resolve_loan_identifier(loan: Loan | None) -> str: + def sha1(msg): + return + + return ( + hashlib.sha1(f"loan: {loan.id}".encode()).hexdigest() + if loan + else MISSING_LOAN_IDENTIFIER + ) + class PlaytimeEntriesController(CirculationManagerController): def track_playtimes(self, collection_id, identifier_type, identifier_idn): @@ -49,8 +67,32 @@ def track_playtimes(self, collection_id, identifier_type, identifier_idn): except ValidationError as ex: return INVALID_INPUT.detailed(ex.json()) + # attempt to resolve a loan associated with the patron, identifier, in the time period + entry_max_start_time = max([x.during_minute for x in data.time_entries]) + entry_min_end_time = min([x.during_minute for x in data.time_entries]) + + loan = self._db.scalars( + select(Loan) + .select_from(Loan) + .join(LicensePool) + .where( + LicensePool.identifier == identifier, + Loan.patron == flask.request.patron, + Loan.start <= entry_max_start_time, + Loan.end > entry_min_end_time, + ) + .order_by(Loan.start.desc()) + ).first() + + loan_identifier = resolve_loan_identifier(loan=loan) + responses, summary = PlaytimeEntries.insert_playtime_entries( - self._db, identifier, collection, library, data + self._db, + identifier, + collection, + library, + data, + loan_identifier, ) response_data = PlaytimeEntriesPostResponse( diff --git a/src/palace/manager/core/query/playtime_entries.py b/src/palace/manager/core/query/playtime_entries.py index 9faec6277a..13f5122dfe 100644 --- a/src/palace/manager/core/query/playtime_entries.py +++ b/src/palace/manager/core/query/playtime_entries.py @@ -28,6 +28,7 @@ def insert_playtime_entries( collection: Collection, library: Library, data: PlaytimeEntriesPost, + loan_identifier: str, ) -> tuple[list, PlaytimeEntriesPostSummary]: """Insert into the database playtime entries from a request""" responses = [] @@ -59,6 +60,7 @@ def insert_playtime_entries( library_name=library.name, timestamp=entry.during_minute, total_seconds_played=entry.seconds_played, + loan_identifier=loan_identifier, ) except IntegrityError as ex: logging.getLogger("Time Tracking").error( diff --git a/src/palace/manager/scripts/playtime_entries.py b/src/palace/manager/scripts/playtime_entries.py index b940f589aa..b73c96e305 100644 --- a/src/palace/manager/scripts/playtime_entries.py +++ b/src/palace/manager/scripts/playtime_entries.py @@ -11,7 +11,9 @@ import dateutil.parser import pytz -from sqlalchemy.sql.expression import false, true +from sqlalchemy.sql.expression import and_, distinct, false, select, true +from sqlalchemy.sql.functions import coalesce, count +from sqlalchemy.sql.functions import max as sql_max from sqlalchemy.sql.functions import sum from palace.manager.core.config import Configuration @@ -57,7 +59,7 @@ def do_run(self): PlaytimeEntry.timestamp <= cut_off, ) - # Aggregate entries per identifier-timestamp-collection-library grouping. + # Aggregate entries per identifier-timestamp-collection-library-loan_identifier grouping. # The label forms of the identifier, collection, and library are also # factored in, in case any of the foreign keys are missing. # Since timestamps should be on minute-boundaries the aggregation @@ -71,6 +73,7 @@ def group_key_for_entry(e: PlaytimeEntry) -> tuple: e.identifier_str, e.collection_name, e.library_name, + e.loan_identifier, ) by_group = defaultdict(int) @@ -88,6 +91,7 @@ def group_key_for_entry(e: PlaytimeEntry) -> tuple: identifier_str, collection_name, library_name, + loan_identifier, ) = group # Update the playtime summary. @@ -101,9 +105,11 @@ def group_key_for_entry(e: PlaytimeEntry) -> tuple: identifier_str=identifier_str, collection_name=collection_name, library_name=library_name, + loan_identifier=loan_identifier, ) self.log.info( - f"Added {seconds} to {identifier_str} ({collection_name} in {library_name}) for {timestamp}: new total {playtime.total_seconds_played}." + f"Added {seconds} to {identifier_str} ({collection_name} in {library_name} with loan id of " + f"{loan_identifier}) for {timestamp}: new total {playtime.total_seconds_played}." ) self._db.commit() @@ -214,33 +220,80 @@ def do_run(self): self.log.warning(temp.read()) def _fetch_report_records(self, start: datetime, until: datetime) -> Query: - return ( - self._db.query(PlaytimeSummary) - .with_entities( + # The loan count query returns only non-empty string isbns and titles if there is more + # than one row returned with the grouping. This way we ensure that we do not + # count the same loan twice in the case we have when a + # 1. a single loan with identifier A + # 2. and one or more playtime summaries with title A or no title or isbn A or no isbn + # 3. and one more playtime summaries with title B, isbn B + # This situation can occur when the title and isbn metadata associated with an ID changes due to a feed + # update that occurs between playlist entry posts. + # in this case we just associate the loan identifier with one unique combination of the list of titles and isbn + # values. + loan_count_query = ( + select( + PlaytimeSummary.identifier_str.label("identifier_str2"), + PlaytimeSummary.collection_name.label("collection_name2"), + PlaytimeSummary.library_name.label("library_name2"), + sql_max(coalesce(PlaytimeSummary.isbn, "")).label("isbn2"), + sql_max(coalesce(PlaytimeSummary.title, "")).label("title2"), + count(distinct(PlaytimeSummary.loan_identifier)).label("loan_count"), + ) + .where(PlaytimeSummary.timestamp.between(start, until)) + .group_by( PlaytimeSummary.identifier_str, PlaytimeSummary.collection_name, PlaytimeSummary.library_name, - PlaytimeSummary.isbn, - PlaytimeSummary.title, - sum(PlaytimeSummary.total_seconds_played), + PlaytimeSummary.identifier_id, ) - .filter( - PlaytimeSummary.timestamp >= start, - PlaytimeSummary.timestamp < until, + .subquery() + ) + + seconds_query = ( + select( + PlaytimeSummary.identifier_str, + PlaytimeSummary.collection_name, + PlaytimeSummary.library_name, + coalesce(PlaytimeSummary.isbn, "").label("isbn"), + coalesce(PlaytimeSummary.title, "").label("title"), + sum(PlaytimeSummary.total_seconds_played).label("total_seconds_played"), ) + .where(PlaytimeSummary.timestamp.between(start, until)) .group_by( PlaytimeSummary.identifier_str, PlaytimeSummary.collection_name, PlaytimeSummary.library_name, - PlaytimeSummary.identifier_id, PlaytimeSummary.isbn, PlaytimeSummary.title, + PlaytimeSummary.identifier_id, ) - .order_by( - PlaytimeSummary.collection_name, - PlaytimeSummary.library_name, - PlaytimeSummary.identifier_str, - ) + .subquery() + ) + + combined = self._db.query(seconds_query, loan_count_query).outerjoin( + loan_count_query, + and_( + seconds_query.c.identifier_str == loan_count_query.c.identifier_str2, + seconds_query.c.collection_name == loan_count_query.c.collection_name2, + seconds_query.c.library_name == loan_count_query.c.library_name2, + seconds_query.c.isbn == loan_count_query.c.isbn2, + seconds_query.c.title == loan_count_query.c.title2, + ), + ) + combined_sq = combined.subquery() + + return self._db.query( + combined_sq.c.identifier_str, + combined_sq.c.collection_name, + combined_sq.c.library_name, + combined_sq.c.isbn, + combined_sq.c.title, + combined_sq.c.total_seconds_played, + coalesce(combined_sq.c.loan_count, 0), + ).order_by( + combined_sq.c.collection_name, + combined_sq.c.library_name, + combined_sq.c.identifier_str, ) @@ -256,6 +309,7 @@ def _produce_report(writer: Writer, date_label, records=None) -> None: "library", "title", "total seconds", + "loan count", ) ) for ( @@ -265,15 +319,17 @@ def _produce_report(writer: Writer, date_label, records=None) -> None: isbn, title, total, + loan_count, ) in records: row = ( date_label, identifier_str, - isbn, + None if isbn == "" else isbn, collection_name, library_name, - title, + None if title == "" else title, total, + loan_count, ) # Write the row to the CSV writer.writerow(row) diff --git a/src/palace/manager/sqlalchemy/model/time_tracking.py b/src/palace/manager/sqlalchemy/model/time_tracking.py index f264b6f076..c636d0b231 100644 --- a/src/palace/manager/sqlalchemy/model/time_tracking.py +++ b/src/palace/manager/sqlalchemy/model/time_tracking.py @@ -74,6 +74,8 @@ class PlaytimeEntry(Base): collection: Mapped[Collection] = relationship("Collection", uselist=False) library: Mapped[Library] = relationship("Library", uselist=False) + loan_identifier = Column(String(40), nullable=False) + __table_args__ = ( UniqueConstraint( "tracking_id", @@ -128,6 +130,7 @@ class PlaytimeSummary(Base): title = Column(String) isbn = Column(String) + loan_identifier = Column(String(40), nullable=False) identifier: Mapped[Identifier] = relationship("Identifier", uselist=False) collection: Mapped[Collection] = relationship("Collection", uselist=False) @@ -139,6 +142,7 @@ class PlaytimeSummary(Base): "identifier_str", "collection_name", "library_name", + "loan_identifier", name="unique_playtime_summary", ), ) @@ -155,6 +159,7 @@ def add( identifier_str: str, collection_name: str, library_name: str | None, + loan_identifier: str, ) -> PlaytimeSummary: """Add playtime (in seconds) to it's associated minute-level summary record.""" # Update each label with its current value, if its foreign key is present. @@ -178,6 +183,7 @@ def add( "collection_name": None if collection else collection_name, "library_id": library.id if library else None, "library_name": None if library else library_name, + "loan_identifier": loan_identifier, } lookup_keys = {k: v for k, v in _potential_lookup_keys.items() if v is not None} additional_columns = { diff --git a/tests/manager/api/controller/test_playtime_entries.py b/tests/manager/api/controller/test_playtime_entries.py index 297ba075c4..e13278d768 100644 --- a/tests/manager/api/controller/test_playtime_entries.py +++ b/tests/manager/api/controller/test_playtime_entries.py @@ -1,8 +1,15 @@ +import datetime +import hashlib from unittest.mock import patch import flask from sqlalchemy.exc import IntegrityError +from palace.manager.api.controller.playtime_entries import ( + MISSING_LOAN_IDENTIFIER, + resolve_loan_identifier, +) +from palace.manager.sqlalchemy.model.patron import Loan from palace.manager.sqlalchemy.model.time_tracking import PlaytimeEntry from palace.manager.sqlalchemy.util import get_one from palace.manager.util.datetime_helpers import utc_now @@ -33,11 +40,23 @@ def test_track_playtime(self, circulation_fixture: CirculationControllerFixture) ) patron = db.patron() + loan_exists_date_str = date_string(hour=12, minute=0) + inscope_loan_start = datetime.datetime.fromisoformat(loan_exists_date_str) + inscope_loan_end = inscope_loan_start + datetime.timedelta(days=14) + + loan, _ = pool.loan_to( + patron, + inscope_loan_start, + inscope_loan_end, + ) + + expected_loan_identifier = resolve_loan_identifier(loan=loan) + data = dict( timeEntries=[ { "id": "tracking-id-0", - "during_minute": date_string(hour=12, minute=0), + "during_minute": loan_exists_date_str, "seconds_played": 12, }, { @@ -83,6 +102,7 @@ def test_track_playtime(self, circulation_fixture: CirculationControllerFixture) assert entry.library == db.default_library() assert entry.total_seconds_played == 12 assert entry.timestamp.isoformat() == date_string(hour=12, minute=0) + assert entry.loan_identifier == expected_loan_identifier entry = get_one(db.session, PlaytimeEntry, tracking_id="tracking-id-1") assert entry is not None @@ -91,12 +111,23 @@ def test_track_playtime(self, circulation_fixture: CirculationControllerFixture) assert entry.library == db.default_library() assert entry.total_seconds_played == 17 assert entry.timestamp.isoformat() == date_string(hour=12, minute=1) + assert entry.loan_identifier == expected_loan_identifier # The very old entry does not get recorded assert None == get_one( db.session, PlaytimeEntry, tracking_id="tracking-id-2" ) + def test_resolve_loan_identifier(self): + no_loan = resolve_loan_identifier(loan=None) + test_id = 1 + test_loan_identifier = resolve_loan_identifier(Loan(id=test_id)) + assert no_loan == MISSING_LOAN_IDENTIFIER + assert ( + test_loan_identifier + == hashlib.sha1(f"loan: {test_id}".encode()).hexdigest() + ) + def test_track_playtime_duplicate_id_ok( self, circulation_fixture: CirculationControllerFixture ): @@ -104,7 +135,7 @@ def test_track_playtime_duplicate_id_ok( identifier = db.identifier() collection = db.default_collection() library = db.default_library() - + patron = db.patron() # Attach the identifier to the collection pool = db.licensepool( db.edition( @@ -112,7 +143,8 @@ def test_track_playtime_duplicate_id_ok( ), collection=collection, ) - patron = db.patron() + + loan_identifier = resolve_loan_identifier(loan=None) db.session.add( PlaytimeEntry( @@ -125,6 +157,7 @@ def test_track_playtime_duplicate_id_ok( identifier_str=identifier.urn, collection_name=collection.name, library_name=library.name, + loan_identifier=loan_identifier, ) ) diff --git a/tests/manager/scripts/test_playtime_entries.py b/tests/manager/scripts/test_playtime_entries.py index 0718d94089..fac8fe9dbc 100644 --- a/tests/manager/scripts/test_playtime_entries.py +++ b/tests/manager/scripts/test_playtime_entries.py @@ -33,6 +33,7 @@ def create_playtime_entries( identifier: Identifier, collection: Collection, library: Library, + loan_identifier: str, *entries: PlaytimeTimeEntry, ) -> list[PlaytimeEntry]: all_inserted = [] @@ -47,6 +48,7 @@ def create_playtime_entries( identifier_str=identifier.urn, collection_name=collection.name, library_name=library.name, + loan_identifier=loan_identifier, ) db.session.add(inserted) all_inserted.append(inserted) @@ -83,12 +85,15 @@ def test_summation(self, db: DatabaseTransactionFixture): ) collection2 = db.collection(name=c2_old_name) library2 = db.library(name=l2_old_name) - + loan_identifier, loan_identifier2, loan_identifier3, loan_identifier4 = ( + f"loan-id:{x}" for x in range(0, 4) + ) entries = create_playtime_entries( db, identifier, collection, library, + loan_identifier, P(id="0", during_minute=dk(m=0), seconds_played=30), P(id="1", during_minute=dk(m=0), seconds_played=30), P(id="2", during_minute=dk(m=0), seconds_played=30), @@ -99,6 +104,7 @@ def test_summation(self, db: DatabaseTransactionFixture): identifier2, collection, library, + loan_identifier2, P(id="0", during_minute=dk(m=0), seconds_played=30), P(id="1", during_minute=dk(m=0), seconds_played=30), P(id="2", during_minute=dk(m=0), seconds_played=30), @@ -113,6 +119,7 @@ def test_summation(self, db: DatabaseTransactionFixture): identifier2, collection2, library, + loan_identifier3, P(id="0", during_minute=dk(m=0), seconds_played=30), P(id="1", during_minute=dk(m=1), seconds_played=40), ) @@ -123,6 +130,7 @@ def test_summation(self, db: DatabaseTransactionFixture): identifier2, collection2, library2, + loan_identifier4, P(id="0", during_minute=dk(m=0), seconds_played=30), P(id="1", during_minute=dk(m=0), seconds_played=40), P(id="2", during_minute=dk(m=0), seconds_played=30), @@ -134,6 +142,7 @@ def test_summation(self, db: DatabaseTransactionFixture): identifier2, collection, library, + loan_identifier2, P(id="5", during_minute=utc_now(), seconds_played=30), ) @@ -143,6 +152,7 @@ def test_summation(self, db: DatabaseTransactionFixture): identifier2, collection, library, + loan_identifier2, P(id="6", during_minute=dk(m=10), seconds_played=30), ) processed_entry.processed = True @@ -214,6 +224,7 @@ def test_summation(self, db: DatabaseTransactionFixture): assert id1time.identifier_str == identifier.urn assert id1time.collection_name == collection.name assert id1time.library_name == library.name + assert id1time.loan_identifier == loan_identifier assert id1time.timestamp == dk() assert id2time1.identifier == identifier2 @@ -223,6 +234,8 @@ def test_summation(self, db: DatabaseTransactionFixture): assert id2time1.identifier_str == id2_new_urn assert id2time1.collection_name == collection.name assert id2time1.library_name == library.name + assert id2time1.loan_identifier == loan_identifier2 + assert id2time1.timestamp == dk() assert id2time2.identifier == identifier2 @@ -231,6 +244,7 @@ def test_summation(self, db: DatabaseTransactionFixture): assert id2time2.identifier_str == id2_new_urn assert id2time2.collection_name == collection.name assert id2time2.library_name == library.name + assert id2time2.loan_identifier == loan_identifier2 assert id2time2.total_seconds_played == 30 assert id2time2.timestamp == dk(m=1) @@ -240,6 +254,7 @@ def test_summation(self, db: DatabaseTransactionFixture): assert id2col2time.identifier_str == id2_new_urn assert id2col2time.collection_name == c2_new_name assert id2col2time.library_name == library.name + assert id2col2time.loan_identifier == loan_identifier3 assert id2col2time.total_seconds_played == 30 assert id2col2time.timestamp == dk() @@ -249,6 +264,7 @@ def test_summation(self, db: DatabaseTransactionFixture): assert id2col2time1.identifier_str == id2_new_urn assert id2col2time1.collection_name == c2_new_name assert id2col2time1.library_name == library.name + assert id2col2time1.loan_identifier == loan_identifier3 assert id2col2time1.total_seconds_played == 40 assert id2col2time1.timestamp == dk(m=1) @@ -258,6 +274,7 @@ def test_summation(self, db: DatabaseTransactionFixture): assert id2c2l2time.identifier_str == id2_new_urn assert id2c2l2time.collection_name == c2_new_name assert id2c2l2time.library_name == l2_new_name + assert id2c2l2time.loan_identifier == loan_identifier4 assert id2c2l2time.total_seconds_played == 100 assert id2c2l2time.timestamp == dk() @@ -267,11 +284,13 @@ def test_reap_processed_entries(self, db: DatabaseTransactionFixture): identifier = db.identifier() collection = db.default_collection() library = db.default_library() + loan_identifier = "loan-id" entries = create_playtime_entries( db, identifier, collection, library, + loan_identifier, P(id="0", during_minute=dk(m=0), seconds_played=30), P(id="1", during_minute=dk(m=0), seconds_played=30), P(id="2", during_minute=dk(m=0), seconds_played=30), @@ -341,6 +360,8 @@ def related_rows(table, present: Literal["all"] | Literal["none"]): c2 = db.collection(name=c2_name) l1 = db.library(name=l1_name) l2 = db.library(name=l2_name) + loan1_id = "loan1" + loan2_id = "loan2" P = PlaytimeTimeEntry dk = date2k @@ -351,6 +372,7 @@ def related_rows(table, present: Literal["all"] | Literal["none"]): id1, c1, l1, + loan1_id, P(id="0", during_minute=dk(m=0), seconds_played=30), P(id="1", during_minute=dk(m=0), seconds_played=30), ) @@ -359,6 +381,7 @@ def related_rows(table, present: Literal["all"] | Literal["none"]): id2, c2, l2, + loan2_id, P(id="2", during_minute=dk(m=0), seconds_played=12), P(id="3", during_minute=dk(m=0), seconds_played=17), ) @@ -389,6 +412,7 @@ def related_rows(table, present: Literal["all"] | Literal["none"]): assert b1sum1.identifier_id == id1.id assert b1sum1.collection_id == c1.id assert b1sum1.library_id == l1.id + assert b1sum1.loan_identifier == loan1_id assert b2sum1.total_seconds_played == 29 assert b2sum1.identifier_str == id2_urn @@ -397,6 +421,7 @@ def related_rows(table, present: Literal["all"] | Literal["none"]): assert b2sum1.identifier_id == id2.id assert b2sum1.collection_id == c2.id assert b2sum1.library_id == l2.id + assert b2sum1.loan_identifier == loan2_id # Add some new client playtime entries. book1_round2 = create_playtime_entries( @@ -404,6 +429,7 @@ def related_rows(table, present: Literal["all"] | Literal["none"]): id1, c1, l1, + loan1_id, P(id="4", during_minute=dk(m=0), seconds_played=30), P(id="5", during_minute=dk(m=0), seconds_played=30), ) @@ -412,6 +438,7 @@ def related_rows(table, present: Literal["all"] | Literal["none"]): id2, c2, l2, + loan2_id, P(id="6", during_minute=dk(m=0), seconds_played=22), P(id="7", during_minute=dk(m=0), seconds_played=46), ) @@ -456,6 +483,7 @@ def related_rows(table, present: Literal["all"] | Literal["none"]): assert b1sum1.identifier_id is None assert b1sum1.collection_id is None assert b1sum1.library_id is None + assert b1sum1.loan_identifier == loan1_id assert b2sum1.total_seconds_played == 97 assert b2sum1.identifier_str == id2_urn @@ -464,6 +492,7 @@ def related_rows(table, present: Literal["all"] | Literal["none"]): assert b2sum1.identifier_id is None assert b2sum1.collection_id is None assert b2sum1.library_id is None + assert b2sum1.loan_identifier == loan2_id def date1m(days) -> date: @@ -483,6 +512,7 @@ def playtime( library: Library, timestamp: datetime, total_seconds: int, + loan_identifier: str, ): return PlaytimeSummary.add( session, @@ -494,6 +524,7 @@ def playtime( identifier_str=identifier.urn, collection_name=collection.name, library_name=library.name, + loan_identifier=loan_identifier, ) @@ -511,6 +542,18 @@ def test_do_run( collection2 = db.collection() library2 = db.library() + identifier3 = db.identifier() + + loan_identifiers = [f"loan_id:{x}" for x in range(1, 7)] + ( + loan_identifier, + loan_identifier2, + loan_identifier3, + loan_identifier4, + loan_identifier5, + loan_identifier6, + ) = loan_identifiers + isbn_ids: dict[str, Identifier] = { "i1": db.identifier( identifier_type=Identifier.ISBN, foreign_id="080442957X" @@ -533,23 +576,85 @@ def test_do_run( # We're using the RecursiveEquivalencyCache, so must refresh it. EquivalentIdentifiersCoverageProvider(db.session).run() - playtime(db.session, identifier, collection, library, dt1m(3), 1) - playtime(db.session, identifier, collection, library, dt1m(31), 2) playtime( - db.session, identifier, collection, library, dt1m(-31), 60 + db.session, identifier, collection, library, dt1m(3), 1, loan_identifier + ) + playtime( + db.session, identifier, collection, library, dt1m(31), 2, loan_identifier + ) + playtime( + db.session, identifier, collection, library, dt1m(-31), 60, loan_identifier ) # out of range: prior to the beginning of the default reporting period playtime( - db.session, identifier, collection, library, dt1m(95), 60 + db.session, + identifier, + collection, + library, + dt1m(95), + 60, + loan_identifier, ) # out of range: future - playtime(db.session, identifier2, collection, library, dt1m(3), 5) - playtime(db.session, identifier2, collection, library, dt1m(4), 6) + playtime( + db.session, identifier2, collection, library, dt1m(3), 5, loan_identifier2 + ) + playtime( + db.session, identifier2, collection, library, dt1m(4), 6, loan_identifier2 + ) # Collection2 - playtime(db.session, identifier, collection2, library, dt1m(3), 100) + playtime( + db.session, identifier, collection2, library, dt1m(3), 100, loan_identifier3 + ) # library2 - playtime(db.session, identifier, collection, library2, dt1m(3), 200) + playtime( + db.session, identifier, collection, library2, dt1m(3), 200, loan_identifier4 + ) # collection2 library2 - playtime(db.session, identifier, collection2, library2, dt1m(3), 300) + playtime( + db.session, + identifier, + collection2, + library2, + dt1m(3), + 300, + loan_identifier5, + ) + + playtime( + db.session, + identifier3, + collection2, + library2, + dt1m(10), + 800, + loan_identifier6, + ) + + # log a summary where a title that was previously unavailable is now available for the same loan + edition2 = db.edition(title="A test") + edition2.primary_identifier = identifier3 + + playtime( + db.session, + identifier3, + collection2, + library2, + dt1m(15), + 13, + loan_identifier6, + ) + + edition2.title = "Z test" + + playtime( + db.session, + identifier3, + collection2, + library2, + dt1m(20), + 4, + loan_identifier6, + ) reporting_name = "test cm" with ( @@ -567,7 +672,7 @@ def test_do_run( # Assert assert ( - writer().writerow.call_count == 6 + writer().writerow.call_count == 9 ) # 1 header, 5 identifier,collection,library entries cutoff = date1m(0).replace(day=1) @@ -584,6 +689,7 @@ def test_do_run( "library", "title", "total seconds", + "loan count", ) ), call( @@ -595,6 +701,43 @@ def test_do_run( library2.name, None, 300, + 1, + ) + ), + call( + ( + column1, + identifier3.urn, + None, + collection2.name, + library2.name, + None, + 800, + 0, + ) + ), + call( + ( + column1, + identifier3.urn, + None, + collection2.name, + library2.name, + "A test", + 13, + 0, + ) + ), + call( + ( + column1, + identifier3.urn, + None, + collection2.name, + library2.name, + "Z test", + 4, + 1, ) ), call( @@ -606,6 +749,7 @@ def test_do_run( library.name, None, 100, + 1, ) ), call( @@ -617,6 +761,7 @@ def test_do_run( library2.name, None, 200, + 1, ) ), call( @@ -627,6 +772,7 @@ def test_do_run( collection.name, library.name, None, + 3, 1, ) ), # Identifier without edition @@ -639,10 +785,13 @@ def test_do_run( library.name, edition.title, 11, + 1, ) ), # Identifier with edition ] + # verify the number of unique loans + assert len(loan_identifiers) == sum([x.args[0][7] for x in call_args[1:]]) assert services_email_fixture.mock_emailer.send.call_count == 1 assert services_email_fixture.mock_emailer.send.call_args == call( subject=f"{reporting_name}: Playtime Summaries {cutoff} - {until}", @@ -659,7 +808,16 @@ def test_no_reporting_email(self, db: DatabaseTransactionFixture): identifier = db.identifier() collection = db.default_collection() library = db.default_library() - _ = playtime(db.session, identifier, collection, library, dt1m(20), 1) + loan_id = "loan-id" + _ = playtime( + db.session, + identifier, + collection, + library, + dt1m(20), + 1, + loan_id, + ) with patch("palace.manager.scripts.playtime_entries.os.environ", new={}): script = PlaytimeEntriesEmailReportsScript(db.session) diff --git a/tests/manager/sqlalchemy/model/test_time_tracking.py b/tests/manager/sqlalchemy/model/test_time_tracking.py index 19079f0a42..3703eeaa38 100644 --- a/tests/manager/sqlalchemy/model/test_time_tracking.py +++ b/tests/manager/sqlalchemy/model/test_time_tracking.py @@ -36,6 +36,7 @@ def test_create(self, db: DatabaseTransactionFixture): timestamp=now, total_seconds_played=30, tracking_id="tracking-id", + loan_identifier="loan-id", ) assert entry.identifier == identifier @@ -47,12 +48,14 @@ def test_create(self, db: DatabaseTransactionFixture): assert entry.total_seconds_played == 30 assert entry.timestamp == now assert entry.tracking_id == "tracking-id" + assert entry.loan_identifier == "loan-id" def test_constraints(self, db: DatabaseTransactionFixture): identifier = db.identifier() collection = db.default_collection() library = db.default_library() now = utc_now() + loan_id = "loan-id" # > 60 second playtime (per minute) with pytest.raises(IntegrityError) as raised: @@ -68,6 +71,7 @@ def test_constraints(self, db: DatabaseTransactionFixture): timestamp=now, total_seconds_played=61, tracking_id="tracking-id", + loan_identifier=loan_id, ) assert "max_total_seconds_played_constraint" in raised.exconly() db.session.rollback() @@ -90,6 +94,7 @@ def test_constraints(self, db: DatabaseTransactionFixture): timestamp=now, total_seconds_played=60, tracking_id="tracking-id-0", + loan_identifier=loan_id, ) # Different identifier same tracking id is ok create( @@ -104,6 +109,7 @@ def test_constraints(self, db: DatabaseTransactionFixture): timestamp=now, total_seconds_played=60, tracking_id="tracking-id-0", + loan_identifier="loan-id-2", ) # Same identifier different tracking id is ok create( @@ -118,6 +124,7 @@ def test_constraints(self, db: DatabaseTransactionFixture): timestamp=now, total_seconds_played=60, tracking_id="tracking-id-1", + loan_identifier=loan_id, ) with pytest.raises(IntegrityError) as raised: # Same identifier same tracking id is not ok @@ -133,6 +140,7 @@ def test_constraints(self, db: DatabaseTransactionFixture): timestamp=now, total_seconds_played=60, tracking_id="tracking-id-0", + loan_identifier=loan_id, ) assert ( f"Key (tracking_id, identifier_str, collection_name, library_name)=(tracking-id-0, {identifier.urn}, {collection.name}, {library.name}) already exists" @@ -145,6 +153,7 @@ def test_contraints(self, db: DatabaseTransactionFixture): identifier = db.identifier() collection = db.default_collection() library = db.default_library() + loan_id = "loan-id" create( db.session, @@ -157,6 +166,7 @@ def test_contraints(self, db: DatabaseTransactionFixture): library_name=library.name, timestamp=datetime.datetime(2000, 1, 1, 12, 00, 00), total_seconds_played=600, + loan_identifier=loan_id, ) # Same identifier string with same timestamp @@ -172,9 +182,10 @@ def test_contraints(self, db: DatabaseTransactionFixture): library_name=library.name, timestamp=datetime.datetime(2000, 1, 1, 12, 00, 00), total_seconds_played=600, + loan_identifier=loan_id, ) assert ( - f'Key ("timestamp", identifier_str, collection_name, library_name)=(2000-01-01 12:00:00+00, {identifier.urn}, {collection.name}, {library.name}) already exists' + f'Key ("timestamp", identifier_str, collection_name, library_name, loan_identifier)=(2000-01-01 12:00:00+00, {identifier.urn}, {collection.name}, {library.name}, {loan_id}) already exists' in raised.exconly() ) @@ -193,6 +204,7 @@ def test_contraints(self, db: DatabaseTransactionFixture): library_name=library.name, timestamp=datetime.datetime(2000, 1, 1, 12, 00, 1), total_seconds_played=600, + loan_identifier=loan_id, ) assert "timestamp_minute_boundary_constraint" in raised.exconly() @@ -200,7 +212,7 @@ def test_cascades(self, db: DatabaseTransactionFixture): identifier = db.identifier() collection = db.default_collection() library = db.default_library() - + loan_id = "loan-id" urn = identifier.urn entry, _ = create( db.session, @@ -213,6 +225,7 @@ def test_cascades(self, db: DatabaseTransactionFixture): library_name=library.name, timestamp=datetime.datetime(2000, 1, 1, 12, 00, 00), total_seconds_played=600, + loan_identifier=loan_id, ) assert entry.identifier == identifier