* Fix mypy

* Remove commented code
* Add a test to cover case when remote_patron_lookup is called with Patron parameter.

src/palace/manager/api/admin/controller/patron.py

@@ -36,7 +36,7 @@ def _load_patrondata(self, authenticator=None):
             )
 
         patron_data = PatronData(authorization_identifier=identifier)
-        complete_patron_data = None
+        remote_patron_data = None
         patron_lookup_providers = list(authenticator.unique_patron_lookup_providers)
 
         if not patron_lookup_providers:
@@ -45,12 +45,12 @@ def _load_patrondata(self, authenticator=None):
             )
 
         for provider in patron_lookup_providers:
-            complete_patron_data = provider.remote_patron_lookup(patron_data)
-            if complete_patron_data:
-                return complete_patron_data
+            remote_patron_data = provider.remote_patron_lookup(patron_data)
+            if remote_patron_data:
+                return remote_patron_data
 
         # If we get here, none of the providers succeeded.
-        if not complete_patron_data:
+        if not remote_patron_data:
             return NO_SUCH_PATRON.detailed(
                 _(
                     "No patron with identifier %(patron_identifier)s was found at your library",

src/palace/manager/api/sirsidynix_authentication_provider.py

@@ -25,13 +25,12 @@
     FormField,
 )
 from palace.manager.service.analytics.analytics import Analytics
+from palace.manager.sqlalchemy.model.patron import Patron
 from palace.manager.util.http import HTTP
 
 if TYPE_CHECKING:
     from requests import Response
 
-    from palace.manager.sqlalchemy.model.patron import Patron
-
 
 class SirsiBlockReasons:
     NOT_APPROVED = _("Patron has not yet been approved")
@@ -194,11 +193,16 @@ def remote_authenticate(
 
     def remote_patron_lookup(
         self, patron_or_patrondata: Patron | PatronData
-    ) -> None | SirsiDynixPatronData:
+    ) -> None | SirsiDynixPatronData | PatronData:
         """Do a remote patron lookup, this method can only look up a patron with a patrondata object
         with a session_token already setup within it.
         This method also checks all the reasons that a patron may be blocked for.
         """
+
+        # Return None if a patron object is passed in.
+        if isinstance(patron_or_patrondata, Patron):
+            return None
+
         # if the patron data object is not authenticated just pass it back after ensuring the
         # complete flag is set to False
         if (
@@ -208,17 +212,11 @@ def remote_patron_lookup(
             patron_or_patrondata.complete = False
             return patron_or_patrondata
 
-        # We cannot do a remote lookup without a session token
-        # if :
-        #     return None
-        # elif not patron_or_patrondata.session_token:
-        #     return None
-
         patrondata = patron_or_patrondata
         # Pull and parse the basic patron information
         data = self.api_read_patron_data(
             patron_key=patrondata.permanent_id,
-            session_token=patrondata.session_token,
+            session_token=patrondata.session_token,  # type: ignore[attr-defined]
         )
         if not data or "fields" not in data:
             return None
@@ -245,7 +243,7 @@ def remote_patron_lookup(
         # Get patron "fines" information
         status = self.api_patron_status_info(
             patron_key=patrondata.permanent_id,
-            session_token=patrondata.session_token,
+            session_token=patrondata.session_token,  # type: ignore[attr-defined]
         )
 
         if not status or "fields" not in status:

tests/manager/api/test_sirsidynix_auth_provider.py

@@ -288,19 +288,25 @@ def test_remote_patron_lookup_blocks(
         assert patrondata.block_reason == block_reason
 
     def test_remote_patron_lookup_bad_patrondata(
-        self, sirsi_auth_fixture: SirsiAuthFixture
+        self,
+        sirsi_auth_fixture: SirsiAuthFixture,
+        db: DatabaseTransactionFixture,
     ):
         # Test no session token
         provider = sirsi_auth_fixture.provider_mocked_api().provider
         patron_data = provider.remote_patron_lookup(
             SirsiDynixPatronData(permanent_id="xxxx", session_token=None)
         )
 
-        assert not patron_data.complete
+        assert patron_data and not patron_data.complete
 
         # Test incorrect patrondata type
         patron_data = provider.remote_patron_lookup(PatronData(permanent_id="xxxx"))
-        assert not patron_data.complete
+        assert patron_data and not patron_data.complete
+
+        # Test remote_patron_lookup with Patron object
+        patron = db.patron()
+        assert provider.remote_patron_lookup(patron) is None
 
     def test_remote_patron_lookup_bad_patron_read_data(
         self, sirsi_auth_fixture: SirsiAuthFixture