Refactor controller tests to use flask_app_fixture (PP-893) (#1634)

* Remove uses of the admin settings controller.
* Convert discovery_services tests.
* Fix up library settings tests
* Fix up catalog services controller tests.
* Replace admin controller fixture for TestAdminSearchController
* Replace admin controller fixture for DiscoveryServiceLibraryRegistrationsController
* Replace fixture for TestAdminPermissionsControllerMixin
* Replace fixtures in DeviceTokensController tests.

api/admin/controller/__init__.py

@@ -30,7 +30,6 @@ def setup_admin_controllers(manager: CirculationManager):
     from api.admin.controller.patron import PatronController
     from api.admin.controller.patron_auth_services import PatronAuthServicesController
     from api.admin.controller.reset_password import ResetPasswordController
-    from api.admin.controller.settings import SettingsController
     from api.admin.controller.sign_in import SignInController
     from api.admin.controller.sitewide_settings import (
         SitewideConfigurationSettingsController,
@@ -48,7 +47,6 @@ def setup_admin_controllers(manager: CirculationManager):
     manager.admin_custom_lists_controller = CustomListsController(manager)
     manager.admin_lanes_controller = LanesController(manager)
     manager.admin_dashboard_controller = DashboardController(manager)
-    manager.admin_settings_controller = SettingsController(manager)
     manager.admin_patron_controller = PatronController(manager)
     manager.admin_discovery_services_controller = DiscoveryServicesController(manager)
     manager.admin_discovery_service_library_registrations_controller = (

api/circulation_manager.py

@@ -64,7 +64,6 @@
     from api.admin.controller.patron_auth_services import PatronAuthServicesController
     from api.admin.controller.quicksight import QuickSightController
     from api.admin.controller.reset_password import ResetPasswordController
-    from api.admin.controller.settings import SettingsController
     from api.admin.controller.sign_in import SignInController
     from api.admin.controller.sitewide_settings import (
         SitewideConfigurationSettingsController,
@@ -100,7 +99,6 @@ class CirculationManager(LoggerMixin):
     admin_custom_lists_controller: CustomListsController
     admin_lanes_controller: LanesController
     admin_dashboard_controller: DashboardController
-    admin_settings_controller: SettingsController
     admin_patron_controller: PatronController
     admin_discovery_services_controller: DiscoveryServicesController
     admin_discovery_service_library_registrations_controller: DiscoveryServiceLibraryRegistrationsController

tests/api/admin/controller/test_admin_search_controller.py

@@ -1,19 +1,22 @@
+from unittest.mock import MagicMock
+
 import pytest
 
+from api.admin.controller.admin_search import AdminSearchController
 from core.model.classification import Subject
 from core.model.datasource import DataSource
 from core.model.licensing import LicensePool
 from core.model.work import Work
-from tests.fixtures.api_admin import AdminControllerFixture
+from tests.fixtures.database import DatabaseTransactionFixture
+from tests.fixtures.flask import FlaskAppFixture
 
 
 class AdminSearchFixture:
-    def __init__(self, admin_ctrl_fixture: AdminControllerFixture):
-        self.admin_ctrl_fixture = admin_ctrl_fixture
-        self.manager = admin_ctrl_fixture.manager
-        self.db = self.admin_ctrl_fixture.ctrl.db
-
-        db = self.db
+    def __init__(self, db: DatabaseTransactionFixture):
+        self.db = db
+        mock_manager = MagicMock()
+        mock_manager._db = db.session
+        self.controller = AdminSearchController(mock_manager)
 
         # Setup works with subjects, languages, audiences etc...
         gutenberg = DataSource.lookup(db.session, DataSource.GUTENBERG)
@@ -77,20 +80,23 @@ def __init__(self, admin_ctrl_fixture: AdminControllerFixture):
 
 @pytest.fixture(scope="function")
 def admin_search_fixture(
-    admin_ctrl_fixture: AdminControllerFixture,
+    db: DatabaseTransactionFixture,
 ) -> AdminSearchFixture:
-    return AdminSearchFixture(admin_ctrl_fixture)
+    return AdminSearchFixture(db)
 
 
 class TestAdminSearchController:
-    def test_search_field_values(self, admin_search_fixture: AdminSearchFixture):
-        with admin_search_fixture.admin_ctrl_fixture.request_context_with_library_and_admin(
+    def test_search_field_values(
+        self,
+        admin_search_fixture: AdminSearchFixture,
+        flask_app_fixture: FlaskAppFixture,
+        db: DatabaseTransactionFixture,
+    ):
+        with flask_app_fixture.test_request_context(
             "/",
-            library=admin_search_fixture.admin_ctrl_fixture.ctrl.db.default_library(),
+            library=db.default_library(),
         ):
-            response = (
-                admin_search_fixture.manager.admin_search_controller.search_field_values()
-            )
+            response = admin_search_fixture.controller.search_field_values()
 
         assert response["subjects"] == {
             "subject 1": 1,
@@ -104,14 +110,19 @@ def test_search_field_values(self, admin_search_fixture: AdminSearchFixture):
         assert response["publishers"] == {"Publisher 1": 3, "Publisher 10": 10}
         assert response["distributors"] == {"Gutenberg": 13}
 
-    def test_different_license_types(self, admin_search_fixture: AdminSearchFixture):
+    def test_different_license_types(
+        self,
+        admin_search_fixture: AdminSearchFixture,
+        flask_app_fixture: FlaskAppFixture,
+        db: DatabaseTransactionFixture,
+    ):
         # Remove the cache
-        admin_search_fixture.manager.admin_search_controller.__class__._search_field_values_cached.ttls = (  # type: ignore
+        admin_search_fixture.controller.__class__._search_field_values_cached.ttls = (  # type: ignore
             0
         )
 
         w = (
-            admin_search_fixture.db.session.query(Work)
+            db.session.query(Work)
             .filter(Work.presentation_edition.has(title="work3"))
             .first()
         )
@@ -122,24 +133,20 @@ def test_different_license_types(self, admin_search_fixture: AdminSearchFixture)
 
         # A pool without licenses should not attribute to the count
         pool.licenses_owned = 0
-        with admin_search_fixture.admin_ctrl_fixture.request_context_with_library_and_admin(
+        with flask_app_fixture.test_request_context(
             "/",
-            library=admin_search_fixture.admin_ctrl_fixture.ctrl.db.default_library(),
+            library=db.default_library(),
         ):
-            response = (
-                admin_search_fixture.manager.admin_search_controller.search_field_values()
-            )
+            response = admin_search_fixture.controller.search_field_values()
             assert "Horror" not in response["genres"]
             assert "Spanish" not in response["languages"]
 
         # An open access license should get counted even without owned licenses
         pool.open_access = True
-        with admin_search_fixture.admin_ctrl_fixture.request_context_with_library_and_admin(
+        with flask_app_fixture.test_request_context(
             "/",
-            library=admin_search_fixture.admin_ctrl_fixture.ctrl.db.default_library(),
+            library=db.default_library(),
         ):
-            response = (
-                admin_search_fixture.manager.admin_search_controller.search_field_values()
-            )
+            response = admin_search_fixture.controller.search_field_values()
             assert "Horror" in response["genres"]
             assert "Spanish" in response["languages"]

tests/api/admin/controller/test_base.py

@@ -1,59 +1,83 @@
 import pytest
 
+from api.admin.controller.base import AdminPermissionsControllerMixin
 from api.admin.exceptions import AdminNotAuthorized
 from core.model import AdminRole
-from tests.fixtures.api_admin import AdminControllerFixture
+from tests.fixtures.database import DatabaseTransactionFixture
+from tests.fixtures.flask import FlaskAppFixture
+
+
+@pytest.fixture()
+def controller() -> AdminPermissionsControllerMixin:
+    return AdminPermissionsControllerMixin()
 
 
 class TestAdminPermissionsControllerMixin:
-    def test_require_system_admin(self, admin_ctrl_fixture: AdminControllerFixture):
-        with admin_ctrl_fixture.request_context_with_admin("/admin"):
+    def test_require_system_admin(
+        self,
+        controller: AdminPermissionsControllerMixin,
+        flask_app_fixture: FlaskAppFixture,
+    ):
+        with flask_app_fixture.test_request_context("/admin"):
             pytest.raises(
                 AdminNotAuthorized,
-                admin_ctrl_fixture.manager.admin_work_controller.require_system_admin,
+                controller.require_system_admin,
             )
 
-            admin_ctrl_fixture.admin.add_role(AdminRole.SYSTEM_ADMIN)
-            admin_ctrl_fixture.manager.admin_work_controller.require_system_admin()
+        with flask_app_fixture.test_request_context_system_admin("/admin"):
+            controller.require_system_admin()
 
     def test_require_sitewide_library_manager(
-        self, admin_ctrl_fixture: AdminControllerFixture
+        self,
+        controller: AdminPermissionsControllerMixin,
+        flask_app_fixture: FlaskAppFixture,
     ):
-        with admin_ctrl_fixture.request_context_with_admin("/admin"):
+        with flask_app_fixture.test_request_context("/admin"):
             pytest.raises(
                 AdminNotAuthorized,
-                admin_ctrl_fixture.manager.admin_work_controller.require_sitewide_library_manager,
+                controller.require_sitewide_library_manager,
             )
 
-            admin_ctrl_fixture.admin.add_role(AdminRole.SITEWIDE_LIBRARY_MANAGER)
-            admin_ctrl_fixture.manager.admin_work_controller.require_sitewide_library_manager()
+        library_manager = flask_app_fixture.admin_user(
+            role=AdminRole.SITEWIDE_LIBRARY_MANAGER
+        )
+        with flask_app_fixture.test_request_context("/admin", admin=library_manager):
+            controller.require_sitewide_library_manager()
 
-    def test_require_library_manager(self, admin_ctrl_fixture: AdminControllerFixture):
-        with admin_ctrl_fixture.request_context_with_admin("/admin"):
+    def test_require_library_manager(
+        self,
+        controller: AdminPermissionsControllerMixin,
+        flask_app_fixture: FlaskAppFixture,
+        db: DatabaseTransactionFixture,
+    ):
+        with flask_app_fixture.test_request_context("/admin"):
             pytest.raises(
                 AdminNotAuthorized,
-                admin_ctrl_fixture.manager.admin_work_controller.require_library_manager,
-                admin_ctrl_fixture.ctrl.db.default_library(),
+                controller.require_library_manager,
+                db.default_library(),
             )
 
-            admin_ctrl_fixture.admin.add_role(
-                AdminRole.LIBRARY_MANAGER, admin_ctrl_fixture.ctrl.db.default_library()
-            )
-            admin_ctrl_fixture.manager.admin_work_controller.require_library_manager(
-                admin_ctrl_fixture.ctrl.db.default_library()
-            )
+        library_manager = flask_app_fixture.admin_user(
+            role=AdminRole.LIBRARY_MANAGER, library=db.default_library()
+        )
+        with flask_app_fixture.test_request_context("/admin", admin=library_manager):
+            controller.require_library_manager(db.default_library())
 
-    def test_require_librarian(self, admin_ctrl_fixture: AdminControllerFixture):
-        with admin_ctrl_fixture.request_context_with_admin("/admin"):
+    def test_require_librarian(
+        self,
+        controller: AdminPermissionsControllerMixin,
+        flask_app_fixture: FlaskAppFixture,
+        db: DatabaseTransactionFixture,
+    ):
+        with flask_app_fixture.test_request_context("/admin"):
             pytest.raises(
                 AdminNotAuthorized,
-                admin_ctrl_fixture.manager.admin_work_controller.require_librarian,
-                admin_ctrl_fixture.ctrl.db.default_library(),
+                controller.require_librarian,
+                db.default_library(),
             )
 
-            admin_ctrl_fixture.admin.add_role(
-                AdminRole.LIBRARIAN, admin_ctrl_fixture.ctrl.db.default_library()
-            )
-            admin_ctrl_fixture.manager.admin_work_controller.require_librarian(
-                admin_ctrl_fixture.ctrl.db.default_library()
-            )
+        librarian = flask_app_fixture.admin_user(
+            role=AdminRole.LIBRARIAN, library=db.default_library()
+        )
+        with flask_app_fixture.test_request_context("/admin", admin=librarian):
+            controller.require_librarian(db.default_library())