Skip to content

Commit

Permalink
Add better bibliotheca error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathangreen committed Sep 3, 2024
1 parent c80f10a commit 4759f6f
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 10 deletions.
16 changes: 8 additions & 8 deletions src/palace/manager/api/bibliotheca.py
Original file line number Diff line number Diff line change
Expand Up @@ -1003,28 +1003,28 @@ def process_one(
expected = expected.split(",")

if actual == "CAN_WISH":
return NoLicenses(message)
return NoLicenses(debug_info=message)

if "CAN_LOAN" in expected and actual == "CAN_HOLD":
return NoAvailableCopies(message)
return NoAvailableCopies(debug_info=message)

if "CAN_LOAN" in expected and actual == "HOLD":
return AlreadyOnHold(message)
return AlreadyOnHold(debug_info=message)

if "CAN_LOAN" in expected and actual == "LOAN":
return AlreadyCheckedOut(message)
return AlreadyCheckedOut(debug_info=message)

if "CAN_HOLD" in expected and actual == "CAN_LOAN":
return CurrentlyAvailable(message)
return CurrentlyAvailable(debug_info=message)

if "CAN_HOLD" in expected and actual == "HOLD":
return AlreadyOnHold(message)
return AlreadyOnHold(debug_info=message)

if "CAN_HOLD" in expected:
return CannotHold(message)
return CannotHold(debug_info=message)

if "CAN_LOAN" in expected:
return CannotLoan(message)
return CannotLoan(debug_info=message)

return RemoteInitiatedServerError(message, BibliothecaAPI.SERVICE_NAME)

Expand Down
24 changes: 24 additions & 0 deletions src/palace/manager/api/circulation_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,24 +322,44 @@ class NoAvailableCopies(CannotLoan):
copies are already checked out.
"""

@property
def base(self) -> ProblemDetail:
return CHECKOUT_FAILED.detailed(detail="No available copies to check out.")


class AlreadyCheckedOut(CannotLoan):
"""The patron can't put check this book out because they already have
it checked out.
"""

@property
def base(self) -> ProblemDetail:
return CHECKOUT_FAILED.detailed(
detail="You already have this book checked out."
)


class AlreadyOnHold(CannotHold):
"""The patron can't put this book on hold because they already have
it on hold.
"""

@property
def base(self) -> ProblemDetail:
return HOLD_FAILED.detailed(detail="You already have this book on hold.")


class NotCheckedOut(CannotReturn):
"""The patron can't return this book because they don't
have it checked out in the first place.
"""

@property
def base(self) -> ProblemDetail:
return COULD_NOT_MIRROR_TO_REMOTE.detailed(
title="Unable to return", detail="You don't have this book checked out."
)


class NotOnHold(CannotReleaseHold):
"""The patron can't release a hold for this book because they don't
Expand All @@ -350,6 +370,10 @@ class NotOnHold(CannotReleaseHold):
class CurrentlyAvailable(CannotHold):
"""The patron can't put this book on hold because it's available now."""

@property
def base(self) -> ProblemDetail:
return HOLD_FAILED.detailed(detail="Cannot place a hold on an available title.")


class HoldOnUnlimitedAccess(CannotHold):
"""The patron can't put this book on hold because it's an unlimited
Expand Down
27 changes: 25 additions & 2 deletions tests/manager/api/test_bibliotheca.py
Original file line number Diff line number Diff line change
Expand Up @@ -858,62 +858,85 @@ class TestErrorParser:
)

@pytest.mark.parametrize(
"incoming_message, error_class",
"incoming_message, error_class, message, debug_message",
[
(
"Patron cannot loan more than 12 documents",
PatronLoanLimitReached,
"Patron cannot loan more than 12 documents",
None,
),
(
"Patron cannot have more than 15 holds",
PatronHoldLimitReached,
"Patron cannot have more than 15 holds",
None,
),
(
"the patron document status was CAN_WISH and not one of CAN_LOAN,RESERVATION",
NoLicenses,
"The library currently has no licenses for this book.",
"the patron document status was CAN_WISH and not one of CAN_LOAN,RESERVATION",
),
(
"the patron document status was CAN_HOLD and not one of CAN_LOAN,RESERVATION",
NoAvailableCopies,
"No available copies to check out.",
"the patron document status was CAN_HOLD and not one of CAN_LOAN,RESERVATION",
),
(
"the patron document status was LOAN and not one of CAN_LOAN,RESERVATION",
AlreadyCheckedOut,
"You already have this book checked out.",
"the patron document status was LOAN and not one of CAN_LOAN,RESERVATION",
),
(
"The patron has no eBooks checked out",
NotCheckedOut,
"The patron has no eBooks checked out",
None,
),
(
"the patron document status was CAN_LOAN and not one of CAN_HOLD",
CurrentlyAvailable,
"Cannot place a hold on an available title.",
"the patron document status was CAN_LOAN and not one of CAN_HOLD",
),
(
"the patron document status was HOLD and not one of CAN_HOLD",
AlreadyOnHold,
"You already have this book on hold.",
"the patron document status was HOLD and not one of CAN_HOLD",
),
(
"The patron does not have the book on hold",
NotOnHold,
"The patron does not have the book on hold",
None,
),
# This is such a weird case we don't have a special exception for it.
(
"the patron document status was LOAN and not one of CAN_HOLD",
CannotHold,
"Could not place hold (reason unknown).",
"the patron document status was LOAN and not one of CAN_HOLD",
),
],
)
def test_exception(
self,
incoming_message: str,
error_class: type[CirculationException],
message: str,
debug_message: str | None,
):
document = self.BIBLIOTHECA_ERROR_RESPONSE_BODY_TEMPLATE.format(
message=incoming_message
)
error = ErrorParser().process_first(document)
assert error.__class__ is error_class
assert incoming_message == str(error)
assert error.problem_detail.detail == message
assert error.problem_detail.debug_message == debug_message

@pytest.mark.parametrize(
"incoming_message, incoming_message_from_file, error_string",
Expand Down

0 comments on commit 4759f6f

Please sign in to comment.