Skip to content

Commit

Permalink
[PP-1589] Always block expired cards for Sirsidynix Horizon regardles…
Browse files Browse the repository at this point in the history 
…s of ignore blocks flag. (#1987)
  • Loading branch information
@dbernstein
dbernstein authored Aug 14, 2024
1 parent f053734 commit f04138e
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 83 deletions.
86 changes: 46 additions & 40 deletions src/palace/manager/api/sirsidynix_authentication_provider.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
},
),
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

###
Expand Down
105 changes: 62 additions & 43 deletions tests/manager/api/test_sirsidynix_auth_provider.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit f04138e

Please sign in to comment.