Report info/generation share collection selection function.

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(

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] = {

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="[email protected]", 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,27 +129,56 @@ 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,
     ):
         controller = ReportController(db.session)
         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
         ):