Skip to content

Commit

Permalink
Suppress works on a per library basis (PP-939) (#1680)
Browse files Browse the repository at this point in the history
Works can be suppressed at the library level.
  • Loading branch information
jonathangreen authored Feb 20, 2024
1 parent 1d8c2b1 commit 1be90db
Show file tree
Hide file tree
Showing 12 changed files with 446 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
"""Ability to suppress works per library
Revision ID: 9d2dccb0d6ff
Revises: 1c9f519415b5
Create Date: 2024-02-16 17:08:52.146860+00:00
"""
import sqlalchemy as sa

from alembic import op

# revision identifiers, used by Alembic.
revision = "9d2dccb0d6ff"
down_revision = "1c9f519415b5"
branch_labels = None
depends_on = None


def upgrade() -> None:
op.create_table(
"work_library_suppressions",
sa.Column("work_id", sa.Integer(), nullable=False),
sa.Column("library_id", sa.Integer(), nullable=False),
sa.ForeignKeyConstraint(["library_id"], ["libraries.id"], ondelete="CASCADE"),
sa.ForeignKeyConstraint(["work_id"], ["works.id"], ondelete="CASCADE"),
sa.PrimaryKeyConstraint("work_id", "library_id"),
)


def downgrade() -> None:
op.drop_table("work_library_suppressions")
10 changes: 10 additions & 0 deletions bin/configuration/suppress_work_for_library
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#!/usr/bin/env python
import os
import sys

bin_dir = os.path.split(__file__)[0]
package_dir = os.path.join(bin_dir, "..", "..")
sys.path.append(os.path.abspath(package_dir))
from core.scripts import SuppressWorkForLibraryScript

SuppressWorkForLibraryScript().run()
9 changes: 9 additions & 0 deletions core/external_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -1575,6 +1575,7 @@ def from_worklist(cls, _db, worklist, facets):
allow_holds=allow_holds,
license_datasource=license_datasource_id,
lane_building=True,
library=library,
)

def __init__(
Expand Down Expand Up @@ -1723,6 +1724,9 @@ def __init__(

self.lane_building = kwargs.pop("lane_building", False)

library = kwargs.pop("library", None)
self.library_id = library.id if library else None

# At this point there should be no keyword arguments -- you can't pass
# whatever you want into this method.
if kwargs:
Expand Down Expand Up @@ -1847,6 +1851,11 @@ def build(self, _chain_filters=None):
if self.author is not None:
nested_filters["contributors"].append(self.author_filter)

if self.library_id:
f = chain(
f, Bool(must_not=[Terms(**{"suppressed_for": [self.library_id]})])
)

if self.media:
f = chain(f, Terms(medium=scrub_list(self.media)))

Expand Down
9 changes: 8 additions & 1 deletion core/lane.py
Original file line number Diff line number Diff line change
Expand Up @@ -2387,10 +2387,17 @@ def only_show_ready_deliverable_works(self, _db, query, show_suppressed=False):
Note that this assumes the query has an active join against
LicensePool.
"""
return Collection.restrict_to_ready_deliverable_works(
query = Collection.restrict_to_ready_deliverable_works(
query, show_suppressed=show_suppressed, collection_ids=self.collection_ids
)

if not show_suppressed and self.library_id is not None:
query = query.filter(
not_(Work.suppressed_for.contains(self.get_library(_db)))
)

return query

def bibliographic_filter_clauses(self, _db, qu):
"""Create a SQLAlchemy filter that excludes books whose bibliographic
metadata doesn't match what we're looking for.
Expand Down
7 changes: 7 additions & 0 deletions core/model/listeners.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,13 @@ def licensepool_removed_from_work(target, value, initiator):
target.external_index_needs_updating()


@event.listens_for(Work.suppressed_for, "append")
@event.listens_for(Work.suppressed_for, "remove")
def work_suppressed_for_library(target, value, initiator):
if target:
target.external_index_needs_updating()


@Listener.before_flush(LicensePool, ListenerState.deleted)
def licensepool_deleted(session: Session, instance: LicensePool) -> None:
"""A LicensePool is deleted only when its collection is deleted.
Expand Down
17 changes: 17 additions & 0 deletions core/model/work.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
ForeignKey,
Integer,
Numeric,
Table,
Unicode,
)
from sqlalchemy.dialects.postgresql import INT4RANGE
Expand Down Expand Up @@ -209,6 +210,11 @@ class Work(Base):
# will be made to make the Work presentation ready.
presentation_ready_exception = Column(Unicode, default=None, index=True)

# Supress this work from appearing in any feeds for a specific library.
suppressed_for: Mapped[Library] = relationship(
"Library", secondary="work_library_suppressions", passive_deletes=True
)

# These fields are potentially large and can be deferred if you
# don't need all the data in a Work.
LARGE_FIELDS = [
Expand Down Expand Up @@ -1601,6 +1607,7 @@ def _set_value(parent, key, target):
result["_id"] = getattr(doc, "id")
result["work_id"] = getattr(doc, "id")
result["summary"] = getattr(doc, "summary_text")
result["suppressed_for"] = [int(l.id) for l in getattr(doc, "suppressed_for")]
result["fiction"] = (
"Fiction" if getattr(doc, "fiction") is True else "Nonfiction"
)
Expand Down Expand Up @@ -1806,6 +1813,16 @@ def delete(
_db.delete(self)


work_library_suppressions = Table(
"work_library_suppressions",
Base.metadata,
Column("work_id", ForeignKey("works.id", ondelete="CASCADE"), primary_key=True),
Column(
"library_id", ForeignKey("libraries.id", ondelete="CASCADE"), primary_key=True
),
)


def add_work_to_customlists_for_collection(pool_or_work: LicensePool | Work) -> None:
if isinstance(pool_or_work, Work):
work = pool_or_work
Expand Down
90 changes: 90 additions & 0 deletions core/scripts.py
Original file line number Diff line number Diff line change
Expand Up @@ -2668,6 +2668,96 @@ def process_loan(self, loan: Loan):
self.notifications.send_loan_expiry_message(loan, delta.days, tokens)


class SuppressWorkForLibraryScript(Script):
"""Suppress works from a library by identifier"""

BY_DATABASE_ID = "Database ID"

@classmethod
def arg_parser(cls, _db: Session | None) -> argparse.ArgumentParser: # type: ignore[override]
parser = argparse.ArgumentParser()
if _db is None:
raise ValueError("No database session provided.")
library_name_list = sorted(str(l.short_name) for l in _db.query(Library))
library_names = '"' + '", "'.join(library_name_list) + '"'
parser.add_argument(
"-l",
"--library",
help="Short name of the library. Libraries on this system: %s."
% library_names,
required=True,
metavar="SHORT_NAME",
)
parser.add_argument(
"-t",
"--identifier-type",
help="Identifier type (default: ISBN). "
f'To name identifiers by their database ID, use --identifier-type="{cls.BY_DATABASE_ID}".',
default="ISBN",
)
parser.add_argument(
"-i",
"--identifier",
help="The identifier to suppress.",
required=True,
)
return parser

@classmethod
def parse_command_line(
cls, _db: Session | None = None, cmd_args: list[str] | None = None
):
parser = cls.arg_parser(_db)
return parser.parse_known_args(cmd_args)[0]

def load_library(self, library_short_name: str) -> Library:
library_short_name = library_short_name.strip()
library = self._db.scalars(
select(Library).where(Library.short_name == library_short_name)
).one_or_none()
if not library:
raise ValueError(f"Unknown library: {library_short_name}")
return library

def load_identifier(self, identifier_type: str, identifier: str) -> Identifier:
query = select(Identifier)
identifier_type = identifier_type.strip()
identifier = identifier.strip()
if identifier_type == self.BY_DATABASE_ID:
query = query.where(Identifier.id == int(identifier))
else:
query = query.where(Identifier.type == identifier_type).where(
Identifier.identifier == identifier
)

identifier_obj = self._db.scalars(query).unique().one_or_none()
if not identifier_obj:
raise ValueError(f"Unknown identifier: {identifier_type}/{identifier}")

return identifier_obj

def do_run(self, cmd_args: list[str] | None = None) -> None:
parsed = self.parse_command_line(self._db, cmd_args=cmd_args)

library = self.load_library(parsed.library)
identifier = self.load_identifier(parsed.identifier_type, parsed.identifier)

self.suppress_work(library, identifier)

def suppress_work(self, library: Library, identifier: Identifier) -> None:
work = identifier.work
if not work:
self.log.warning(f"No work found for {identifier}")
return

work.suppressed_for.append(library)
self.log.info(
f"Suppressed {identifier.type}/{identifier.identifier} (work id: {work.id}) for {library.short_name}."
)

self._db.commit()


class MockStdin:
"""Mock a list of identifiers passed in on standard input."""

Expand Down
19 changes: 19 additions & 0 deletions tests/core/models/test_listeners.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,22 @@ def test_licensepool_storage_status_change(
== work.coverage_records[0].operation
)
assert WorkCoverageRecord.REGISTERED == work.coverage_records[0].status

def test_work_suppressed_for_library(self, db: DatabaseTransactionFixture):
work = db.work(with_license_pool=True)
library = db.library()

# Clear out any WorkCoverageRecords created as the work was initialized.
work.coverage_records = []

# Act
work.suppressed_for.append(library)

# Assert
assert 1 == len(work.coverage_records)
assert work.id == work.coverage_records[0].work_id
assert (
WorkCoverageRecord.UPDATE_SEARCH_INDEX_OPERATION
== work.coverage_records[0].operation
)
assert WorkCoverageRecord.REGISTERED == work.coverage_records[0].status
50 changes: 49 additions & 1 deletion tests/core/models/test_work.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pytest
import pytz
from psycopg2.extras import NumericRange
from sqlalchemy import select

from core.classifier import Classifier, Fantasy, Romance, Science_Fiction
from core.equivalents_coverage import EquivalentIdentifiersCoverageProvider
Expand All @@ -16,7 +17,7 @@
from core.model.identifier import Identifier
from core.model.licensing import LicensePool
from core.model.resource import Hyperlink, Representation, Resource
from core.model.work import Work, WorkGenre
from core.model.work import Work, WorkGenre, work_library_suppressions
from core.util.datetime_helpers import datetime_utc, from_timestamp, utc_now
from tests.fixtures.database import DatabaseTransactionFixture
from tests.fixtures.sample_covers import SampleCoversFixture
Expand Down Expand Up @@ -806,6 +807,53 @@ def test_work_updates_info_on_pool_suppressed(self, db: DatabaseTransactionFixtu
assert "Alice Adder, Bob Bitshifter" == work.author
assert "Adder, Alice ; Bitshifter, Bob" == work.sort_author

def test_suppressed_for_delete_work(self, db: DatabaseTransactionFixture):
work = db.work()
library1 = db.library()
library2 = db.library()

work.suppressed_for.append(library1)
work.suppressed_for.append(library2)
db.session.flush()

assert len(db.session.execute(select(work_library_suppressions)).all()) == 2

db.session.delete(work)
db.session.flush()

# The libraries are not deleted.
assert library1 in db.session
assert library2 in db.session

# The work is deleted.
assert work not in db.session

# The references in the work_library_suppressions table to the work are deleted.
assert len(db.session.execute(select(work_library_suppressions)).all()) == 0

def test_suppressed_for_delete_library(self, db: DatabaseTransactionFixture):
work = db.work()
library1 = db.library()
library2 = db.library()

work.suppressed_for.append(library1)
work.suppressed_for.append(library2)
db.session.flush()

assert len(db.session.execute(select(work_library_suppressions)).all()) == 2

db.session.delete(library1)
db.session.flush()
db.session.expire_all()

assert library1 not in db.session

assert library2 in db.session
assert work in db.session

assert len(db.session.execute(select(work_library_suppressions)).all()) == 1
assert work.suppressed_for == [library2]

def test_different_language_means_different_work(
self, db: DatabaseTransactionFixture
):
Expand Down
Loading

0 comments on commit 1be90db

Please sign in to comment.