diff --git a/src/palace/manager/api/sirsidynix_authentication_provider.py b/src/palace/manager/api/sirsidynix_authentication_provider.py index 821de8fdf..38f38b240 100644 --- a/src/palace/manager/api/sirsidynix_authentication_provider.py +++ b/src/palace/manager/api/sirsidynix_authentication_provider.py @@ -56,15 +56,20 @@ class SirsiDynixHorizonAuthSettings(BasicAuthProviderSettings): alias="CLIENT_ID", ) - patron_status_block: bool = FormField( + patron_blocks_enforced: bool = FormField( True, form=ConfigurationFormItem( - label="Patron Status Block", + label="Enforce Patron ILS Blocks", description=( - "Block patrons from borrowing based on the status of the ILS's patron block field?" + "Block patrons from borrowing based on the approved, hasMaxDaysWithFines, hasMaxFines, hasMaxLostItem, " + "hasMaxOverdueDays, hasMaxItemsCheckedOut fields from the ILS.(Note: expired accounts are always " + "blocked)." ), type=ConfigurationFormItemType.SELECT, - options={"true": "Yes, block.", "false": "No, do not block."}, + options={ + "true": "Patron blocks enforced", + "false": "Patron blocks NOT enforced.", + }, ), ) @@ -166,7 +171,7 @@ def __init__( self.sirsi_library_id = library_settings.library_id # Check if patrons should be blocked based on ILS status - self.patron_status_should_block = settings.patron_status_block + self.patron_blocks_enforced = settings.patron_blocks_enforced def remote_authenticate( self, username: str | None, password: str | None @@ -228,44 +233,45 @@ def remote_patron_lookup( patrondata.block_reason = SirsiBlockReasons.PATRON_BLOCKED return patrondata - if self.patron_status_should_block: - # Get patron "fines" information - status = self.api_patron_status_info( - patron_key=patrondata.permanent_id, - session_token=patrondata.session_token, - ) + # Get patron "fines" information + status = self.api_patron_status_info( + patron_key=patrondata.permanent_id, + session_token=patrondata.session_token, + ) - if not status or "fields" not in status: - return None - - status_fields: dict = status["fields"] - fines = status_fields.get("estimatedFines") - if fines is not None: - # We ignore currency for now, and assume USD - patrondata.fines = float(fines.get("amount", 0)) - - # Blockable statuses - if status_fields.get("hasMaxDaysWithFines") or status_fields.get( - "hasMaxFines" - ): - patrondata.block_reason = PatronData.EXCESSIVE_FINES - elif status_fields.get("hasMaxLostItem"): - patrondata.block_reason = PatronData.TOO_MANY_LOST - elif status_fields.get("hasMaxOverdueDays") or status_fields.get( - "hasMaxOverdueItem" - ): - patrondata.block_reason = PatronData.TOO_MANY_OVERDUE - elif status_fields.get("hasMaxItemsCheckedOut"): - patrondata.block_reason = PatronData.TOO_MANY_LOANS - elif status_fields.get("expired"): - patrondata.block_reason = SirsiBlockReasons.EXPIRED - - # If previously, the patron was blocked this should unset the value in the DB - if patrondata.block_reason is None: - patrondata.block_reason = PatronData.NO_VALUE - else: + if not status or "fields" not in status: + return None + + status_fields: dict = status["fields"] + fines = status_fields.get("estimatedFines") + if fines is not None: + # We ignore currency for now, and assume USD + patrondata.fines = float(fines.get("amount", 0)) + + if status_fields.get("expired"): + patrondata.block_reason = SirsiBlockReasons.EXPIRED + elif status_fields.get("hasMaxDaysWithFines") or status_fields.get( + "hasMaxFines" + ): + patrondata.block_reason = PatronData.EXCESSIVE_FINES + elif status_fields.get("hasMaxLostItem"): + patrondata.block_reason = PatronData.TOO_MANY_LOST + elif status_fields.get("hasMaxOverdueDays") or status_fields.get( + "hasMaxOverdueItem" + ): + patrondata.block_reason = PatronData.TOO_MANY_OVERDUE + elif status_fields.get("hasMaxItemsCheckedOut"): + patrondata.block_reason = PatronData.TOO_MANY_LOANS + + # If previously, the patron was blocked this should unset the value in the DB + if patrondata.block_reason is None: patrondata.block_reason = PatronData.NO_VALUE + if not self.patron_blocks_enforced: + # if blocks are ignored and patron is not expired, treat patron as unblocked. + if patrondata.block_reason != SirsiBlockReasons.EXPIRED: + patrondata.block_reason = PatronData.NO_VALUE + return patrondata ### diff --git a/tests/manager/api/test_sirsidynix_auth_provider.py b/tests/manager/api/test_sirsidynix_auth_provider.py index b01ca6b6e..9dc283b7e 100644 --- a/tests/manager/api/test_sirsidynix_auth_provider.py +++ b/tests/manager/api/test_sirsidynix_auth_provider.py @@ -212,61 +212,80 @@ def test_remote_patron_lookup(self, sirsi_auth_fixture: SirsiAuthFixture): assert patrondata.block_reason == PatronData.NO_VALUE assert patrondata.library_identifier == "testtype" - def test_remote_patron_lookup_unblocked_user_with_blocks_ignored( - self, sirsi_auth_fixture: SirsiAuthFixture - ): - settings = sirsi_auth_fixture.settings(patron_status_block=False) - provider = sirsi_auth_fixture.provider(settings=settings) - provider_mock = sirsi_auth_fixture.provider_mocked_api(provider) - patrondata = provider_mock.provider.remote_patron_lookup( - SirsiDynixPatronData(permanent_id="xxxx", session_token="xxx") - ) - - assert provider_mock.api_read_patron_data.call_count == 1 - assert provider_mock.api_patron_status_info.call_count == 0 - assert isinstance(patrondata, PatronData) - assert patrondata.personal_name == "Test User" - assert patrondata.block_reason == PatronData.NO_VALUE - - def test_remote_patron_lookup_blocked_user_with_blocks_enabled( - self, sirsi_auth_fixture: SirsiAuthFixture + @pytest.mark.parametrize( + "patron_data, patron_blocks_enforced, block_reason", + [ + ( + None, + False, + PatronData.NO_VALUE, + ), + ( + None, + True, + PatronData.NO_VALUE, + ), + ( + {"fields": {"hasMaxDaysWithFines": True}}, + True, + PatronData.EXCESSIVE_FINES, + ), + ( + {"fields": {"hasMaxDaysWithFines": True}}, + False, + PatronData.NO_VALUE, + ), + ( + {"fields": {"privilegeExpiresDate": "9999-01-01"}}, + True, + PatronData.NO_VALUE, + ), + ( + {"fields": {"expired": True}}, + True, + SirsiBlockReasons.EXPIRED, + ), + ( + {"fields": {"expired": True}}, + False, + SirsiBlockReasons.EXPIRED, + ), + ( + {"fields": {"expired": False}}, + True, + PatronData.NO_VALUE, + ), + ( + {"fields": {"expired": False}}, + False, + PatronData.NO_VALUE, + ), + ], + ) + def test_remote_patron_lookup_blocks( + self, + sirsi_auth_fixture: SirsiAuthFixture, + patron_data: dict[Any, Any], + patron_blocks_enforced: bool, + block_reason: str, ): - """Tests that remote patron block status is called when blocks are enabled.""" - patron_info = {"fields": {"hasMaxDaysWithFines": True}} - provider_mock = sirsi_auth_fixture.provider_mocked_api( - patron_status_info=patron_info + settings = sirsi_auth_fixture.settings( + patron_blocks_enforced=patron_blocks_enforced ) - - patrondata = provider_mock.provider.remote_patron_lookup( - SirsiDynixPatronData(permanent_id="xxxx", session_token="xxx") - ) - - assert provider_mock.api_read_patron_data.call_count == 1 - assert provider_mock.api_patron_status_info.call_count == 1 - assert isinstance(patrondata, PatronData) - assert patrondata.personal_name == "Test User" - assert patrondata.block_reason == PatronData.EXCESSIVE_FINES - - def test_remote_patron_lookup_blocked_user_with_blocks_ignored( - self, sirsi_auth_fixture: SirsiAuthFixture - ): - """ "Test that patron block status is not called when blocks are ignored.""" - settings = sirsi_auth_fixture.settings(patron_status_block=False) provider = sirsi_auth_fixture.provider(settings=settings) provider_mock = sirsi_auth_fixture.provider_mocked_api(provider) - provider_mock.api_patron_status_info.return_value = { - "fields": {"hasMaxDaysWithFines": True} - } + if patron_data: + provider_mock.api_patron_status_info.return_value = patron_data patrondata = provider_mock.provider.remote_patron_lookup( SirsiDynixPatronData(permanent_id="xxxx", session_token="xxx") ) assert provider_mock.api_read_patron_data.call_count == 1 - assert provider_mock.api_patron_status_info.call_count == 0 + assert provider_mock.api_patron_status_info.call_count == 1 assert isinstance(patrondata, PatronData) assert patrondata.personal_name == "Test User" - assert patrondata.block_reason == PatronData.NO_VALUE + assert patrondata.block_reason == block_reason def test_remote_patron_lookup_bad_patrondata( self, sirsi_auth_fixture: SirsiAuthFixture