From e81a048bc226005c9a6d12c04c943a611833f7e5 Mon Sep 17 00:00:00 2001 From: Tim DiLauro Date: Mon, 13 May 2024 19:04:46 -0400 Subject: [PATCH] Report info/generation share collection selection function. --- .../manager/api/admin/controller/report.py | 26 ++------ .../generate_inventory_and_hold_reports.py | 64 +++++++++++++------ .../api/admin/controller/test_report.py | 49 +++++++++++--- 3 files changed, 91 insertions(+), 48 deletions(-) diff --git a/src/palace/manager/api/admin/controller/report.py b/src/palace/manager/api/admin/controller/report.py index 9954ad7669..fc223d7a7f 100644 --- a/src/palace/manager/api/admin/controller/report.py +++ b/src/palace/manager/api/admin/controller/report.py @@ -3,7 +3,6 @@ import flask from flask import Response -from sqlalchemy import not_, select from sqlalchemy.orm import Session from palace.manager.api.admin.model.inventory_report import ( @@ -13,16 +12,11 @@ from palace.manager.api.admin.problem_details import ADMIN_NOT_AUTHORIZED from palace.manager.celery.tasks.generate_inventory_and_hold_reports import ( generate_inventory_and_hold_reports, + library_report_integrations, ) from palace.manager.core.problem_details import INTERNAL_SERVER_ERROR -from palace.manager.integration.goals import Goals from palace.manager.sqlalchemy.constants import MediaTypes from palace.manager.sqlalchemy.model.admin import Admin -from palace.manager.sqlalchemy.model.collection import Collection -from palace.manager.sqlalchemy.model.integration import ( - IntegrationConfiguration, - IntegrationLibraryConfiguration, -) from palace.manager.sqlalchemy.model.library import Library from palace.manager.util.log import LoggerMixin from palace.manager.util.problem_detail import ProblemDetail, ProblemDetailException @@ -49,20 +43,12 @@ def inventory_report_info(self) -> Response: if not admin.is_librarian(library): raise ProblemDetailException(ADMIN_NOT_AUTHORIZED) - collections = self._db.scalars( - select(Collection) - .join(IntegrationConfiguration) - .join(IntegrationLibraryConfiguration) - .where( - IntegrationLibraryConfiguration.library_id == library.id, - IntegrationConfiguration.goal == Goals.LICENSE_GOAL, - not_( - IntegrationConfiguration.settings_dict.contains( - {"include_in_inventory_report": False} - ) - ), + collections = [ + integration.collection + for integration in library_report_integrations( + library=library, session=self._db ) - ).all() + ] info = InventoryReportInfo( collections=[ InventoryReportCollectionInfo( diff --git a/src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py b/src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py index 52c0235391..bb2d8911f4 100644 --- a/src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py +++ b/src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py @@ -28,6 +28,45 @@ from palace.manager.sqlalchemy.util import get_one +def eligible_integrations( + integrations: list[IntegrationConfiguration], +) -> list[IntegrationConfiguration]: + """Subset a list of integrations to only those that are eligible for the inventory report.""" + registry = LicenseProvidersRegistry() + + def is_eligible(integration: IntegrationConfiguration) -> bool: + if integration.protocol is None: + return False + settings = registry[integration.protocol].settings_load(integration) + return isinstance(settings, OPDSImporterSettings) + + return [integration for integration in integrations if is_eligible(integration)] + + +def library_report_integrations( + library: Library, session: Session | None = None +) -> list[IntegrationConfiguration]: + """Return a list of collections to report for the given library.""" + if session is None: + session = Session.object_session(library) + + # resolve integrations + integrations = session.scalars( + select(IntegrationConfiguration) + .join(IntegrationLibraryConfiguration) + .where( + IntegrationLibraryConfiguration.library_id == library.id, + IntegrationConfiguration.goal == Goals.LICENSE_GOAL, + not_( + IntegrationConfiguration.settings_dict.contains( + {"include_in_inventory_report": False} + ) + ), + ) + ).all() + return sorted(eligible_integrations(integrations), key=lambda i: i.name) + + class GenerateInventoryAndHoldsReportsJob(Job): def __init__( self, @@ -63,27 +102,12 @@ def run(self) -> None: file_name_modifier = f"{library.short_name}-{date_str}" - # resolve integrations - integrations = session.scalars( - select(IntegrationConfiguration) - .join(IntegrationLibraryConfiguration) - .where( - IntegrationLibraryConfiguration.library_id == self.library_id, - IntegrationConfiguration.goal == Goals.LICENSE_GOAL, - not_( - IntegrationConfiguration.settings_dict.contains( - {"include_in_inventory_report": False} - ) - ), + integration_ids = [ + integration.id + for integration in library_report_integrations( + library=library, session=session ) - ).all() - registry = LicenseProvidersRegistry() - integration_ids: list[int] = [] - for integration in integrations: - settings = registry[integration.protocol].settings_load(integration) - if not isinstance(settings, OPDSImporterSettings): - continue - integration_ids.append(integration.id) + ] # generate inventory report csv file sql_params: dict[str, Any] = { diff --git a/tests/manager/api/admin/controller/test_report.py b/tests/manager/api/admin/controller/test_report.py index 664872954d..28cd0071ac 100644 --- a/tests/manager/api/admin/controller/test_report.py +++ b/tests/manager/api/admin/controller/test_report.py @@ -10,6 +10,8 @@ InventoryReportInfo, ) from palace.manager.api.admin.problem_details import ADMIN_NOT_AUTHORIZED +from palace.manager.api.overdrive import OverdriveAPI +from palace.manager.core.opds_import import OPDSAPI from palace.manager.sqlalchemy.model.admin import Admin, AdminRole from palace.manager.sqlalchemy.util import create from palace.manager.util.problem_detail import ProblemDetailException @@ -77,7 +79,10 @@ def test_inventory_report_info( email="librarian@example.org", role=AdminRole.LIBRARIAN, library=library1 ) - collection = db.collection() + collection = db.collection( + protocol=OPDSAPI.label(), + settings={"data_source": "test", "external_account_id": "http://url"}, + ) collection.libraries = [library1, library2] success_payload_dict = InventoryReportInfo( @@ -124,17 +129,47 @@ def test_inventory_report_info( assert admin_response_none.status_code == 404 @pytest.mark.parametrize( - "settings, expect_collection", + "protocol, settings, expect_collection", ( - ({}, True), - ({"include_in_inventory_report": False}, False), - ({"include_in_inventory_report": True}, True), + ( + OPDSAPI.label(), + {"data_source": "test", "external_account_id": "http://url"}, + True, + ), + ( + OPDSAPI.label(), + { + "include_in_inventory_report": False, + "data_source": "test", + "external_account_id": "http://test.url", + }, + False, + ), + ( + OPDSAPI.label(), + { + "include_in_inventory_report": True, + "data_source": "test", + "external_account_id": "http://test.url", + }, + True, + ), + ( + OverdriveAPI.label(), + { + "overdrive_website_id": "test", + "overdrive_client_key": "test", + "overdrive_client_secret": "test", + }, + False, + ), ), ) def test_inventory_report_info_reportable_collections( self, db: DatabaseTransactionFixture, flask_app_fixture: FlaskAppFixture, + protocol: str, settings: dict, expect_collection: bool, ): @@ -142,9 +177,8 @@ def test_inventory_report_info_reportable_collections( sysadmin = flask_app_fixture.admin_user(role=AdminRole.SYSTEM_ADMIN) library = db.library() - collection = db.collection() + collection = db.collection(protocol=protocol, settings=settings) collection.libraries = [library] - collection._set_settings(**settings) expected_collections = ( [InventoryReportCollectionInfo(id=collection.id, name=collection.name)] @@ -157,7 +191,6 @@ def test_inventory_report_info_reportable_collections( ).api_dict() assert len(expected_collections) == expected_collection_count - # Sysadmin can get info for any library. with flask_app_fixture.test_request_context( "/", admin=sysadmin, library=library ):