Skip to content

Commit

Permalink
Fix ODL License status document checks (PP-1331) (#1885)
Browse files Browse the repository at this point in the history
* ODL logging is making loans fail when the response code is 201.

* Fix log message

* Fix status code

* Code review feedback.
  • Loading branch information
jonathangreen authored Jun 4, 2024
1 parent 8c601de commit 1c1d1ea
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
4 changes: 2 additions & 2 deletions src/palace/manager/api/odl.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ def get_license_status_document(self, loan: Loan) -> dict[str, Any]:
)

response = self._get(url)
if response.status_code != 200:
if not (200 <= response.status_code < 300):
header_string = ", ".join(
{f"{k}: {v}" for k, v in response.headers.items()}
)
Expand All @@ -341,7 +341,7 @@ def get_license_status_document(self, loan: Loan) -> dict[str, Any]:
)
self.log.error(
f"Error getting License Status Document for loan ({loan.id}): Url '{url}' returned "
f"status code {response.status_code}. Expected 200. Response headers: {header_string}. "
f"status code {response.status_code}. Expected 2XX. Response headers: {header_string}. "
f"Response content: {response_string}."
)
raise BadResponseException(url, "License Status Document request failed.")
Expand Down
8 changes: 4 additions & 4 deletions tests/manager/api/test_odl.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ class TestODLAPI:
def test_get_license_status_document_success(
self, odl_api_test_fixture: ODLAPITestFixture
) -> None:
# With a new loan.
# With a new loan. New loan returns a 201 status.
loan, _ = odl_api_test_fixture.license.loan_to(odl_api_test_fixture.patron)
odl_api_test_fixture.api.queue_response(
200, content=json.dumps(dict(status="ready"))
201, content=json.dumps(dict(status="ready"))
)
odl_api_test_fixture.api.get_license_status_document(loan)
requested_url = odl_api_test_fixture.api.requests[0][0]
Expand Down Expand Up @@ -101,7 +101,7 @@ def test_get_license_status_document_success(
== notification_url
)

# With an existing loan.
# With an existing loan. Existing loan returns a 200 status.
loan, _ = odl_api_test_fixture.license.loan_to(odl_api_test_fixture.patron)
loan.external_identifier = odl_api_test_fixture.db.fresh_str()

Expand Down Expand Up @@ -139,7 +139,7 @@ def test_get_license_status_document_errors(
odl_api_test_fixture.api.get_license_status_document,
loan,
)
assert "returned status code 403. Expected 200." in caplog.text
assert "returned status code 403. Expected 2XX." in caplog.text
assert "just junk ..." in caplog.text

def test_checkin_success(
Expand Down

0 comments on commit 1c1d1ea

Please sign in to comment.