Merge pull request #121 from MITLibraries/handle-timeout

Handle errors when marking invoices paid

sapinvoices/alma.py

@@ -196,7 +196,7 @@ def mark_invoice_paid(
         payment_date: datetime,
         payment_amount: str,
         payment_currency: str,
-    ) -> dict:
+    ) -> None:
         """Mark an invoice as paid using the invoice process endpoint."""
         endpoint = f"acq/invoices/{invoice_id}"
         params = {"op": "paid"}
@@ -216,7 +216,8 @@ def mark_invoice_paid(
         )
         result.raise_for_status()
         time.sleep(0.1)
-        return result.json()
+        if not result.json()["payment"]["payment_status"]["value"] == "PAID":
+            raise ValueError
 
     def process_invoice(self, invoice_id: str) -> dict:
         """Move an invoice to in process using the invoice process endpoint."""

sapinvoices/sap.py

@@ -11,6 +11,7 @@
 
 import fabric
 import flatdict  # type: ignore
+import requests.exceptions
 from paramiko import RSAKey
 
 from sapinvoices.alma import AlmaClient
@@ -630,18 +631,18 @@ def mark_invoices_paid(
         logger.debug("date: %s", date)
         logger.debug("total amount: %s", invoice["total amount"])
         logger.debug("currency: %s", invoice["currency"])
-        response = alma_client.mark_invoice_paid(
-            invoice_id, date, invoice["total amount"], invoice["currency"]
-        )
-        if response["payment"]["payment_status"]["value"] == "PAID":
-            logger.debug("Invoice '%s' marked as paid in Alma", invoice_id)
+        try:
+            alma_client.mark_invoice_paid(
+                invoice_id, date, invoice["total amount"], invoice["currency"]
+            )
+
             paid_invoice_count += 1
-        else:
+        except (requests.exceptions.RequestException, ValueError):
             logger.error(
-                "Something went wrong marking invoice '%s' paid in Alma, it "
-                "should be investigated manually",
+                "Something went wrong marking invoice '%s' paid in Alma.",
                 invoice_id,
             )
+
     return paid_invoice_count
 
 

tests/test_alma.py

@@ -1,6 +1,8 @@
 import datetime
 import urllib.parse
 
+import pytest
+import requests.exceptions
 import requests_mock
 
 
@@ -119,13 +121,68 @@ def test_mark_invoice_paid(alma_client):
                 payment_amount=payment_amount,
                 payment_currency=payment_currency,
             )
-            == mocked_response
+            is None
         )
         assert mocker.last_request.url == test_url
         assert mocker.last_request.method == "POST"
         assert mocker.last_request.json() == test_payload
 
 
+def test_mark_invoice_paid_request_read_timeout(alma_client):
+    test_url = "https://example.com/acq/invoices/558809630001021?op=paid"
+    invoice_id = "558809630001021"
+    payment_date = datetime.datetime(2021, 7, 22)
+    payment_amount = "120"
+    payment_currency = "USD"
+    with requests_mock.Mocker(case_sensitive=True) as mocker:
+        mocker.post(
+            test_url,
+            exc=requests.exceptions.ReadTimeout,
+        )
+        with pytest.raises(requests.exceptions.RequestException):
+            alma_client.mark_invoice_paid(
+                invoice_id,
+                payment_date=payment_date,
+                payment_amount=payment_amount,
+                payment_currency=payment_currency,
+            )
+
+
+def test_mark_invoice_paid_request_status_error(alma_client):
+    test_url = "https://example.com/acq/invoices/558809630001021?op=paid"
+    invoice_id = "558809630001021"
+    payment_date = datetime.datetime(2021, 7, 22)
+    payment_amount = "120"
+    payment_currency = "USD"
+    with requests_mock.Mocker(case_sensitive=True) as mocker:
+        mocker.post(test_url, json={}, status_code=404)
+        with pytest.raises(requests.exceptions.RequestException):
+            alma_client.mark_invoice_paid(
+                invoice_id,
+                payment_date=payment_date,
+                payment_amount=payment_amount,
+                payment_currency=payment_currency,
+            )
+
+
+def test_mark_invoice_paid_request_value_error(alma_client):
+    test_url = "https://example.com/acq/invoices/558809630001021?op=paid"
+    invoice_id = "558809630001021"
+    payment_date = datetime.datetime(2021, 7, 22)
+    payment_amount = "120"
+    payment_currency = "USD"
+    mocked_response = {"payment": {"payment_status": {"value": "FOO"}}}
+    with requests_mock.Mocker(case_sensitive=True) as mocker:
+        mocker.post(test_url, json=mocked_response)
+        with pytest.raises(ValueError):
+            alma_client.mark_invoice_paid(
+                invoice_id,
+                payment_date=payment_date,
+                payment_amount=payment_amount,
+                payment_currency=payment_currency,
+            )
+
+
 def test_get_invoices_by_status(alma_client):
     invoice_records = {
         "invoice": [{"record_number": i} for i in range(5)],

tests/test_cli.py

@@ -99,9 +99,6 @@ def test_sap_invoices_final_run_real_run(
     assert "Starting SAP invoices process with options" in caplog.text
     assert "Final run: True" in caplog.text
     assert "Real run: True" in caplog.text
-    assert (
-        "Something went wrong marking invoice '02' paid in Alma, it "
-        "should be investigated manually" in caplog.text
-    )
+    assert "Something went wrong marking invoice '02' paid in Alma." in caplog.text
     assert "SAP invoice process completed for a final run" in caplog.text
     assert "2 monograph invoices retrieved and processed:" in caplog.text

tests/test_sap.py

@@ -4,6 +4,7 @@
 from unittest.mock import MagicMock, call
 
 import pytest
+import requests
 
 from sapinvoices import sap
 
@@ -607,17 +608,17 @@ def test_mark_invoices_paid_all_successful(alma_client):
         {"id": "2", "total amount": "200", "currency": "GBH"},
         {"id": "3", "total amount": "300", "currency": "GBH"},
     ]
-    alma_client.mark_invoice_paid = MagicMock(
-        return_value={"payment": {"payment_status": {"value": "PAID"}}}
-    )
+    alma_client.mark_invoice_paid = MagicMock(return_value=None)
     expected_calls = [
         call("1", date, "100", "USD"),
         call("2", date, "200", "GBH"),
         call("3", date, "300", "GBH"),
     ]
-    result = sap.mark_invoices_paid(alma_client, invoices, datetime(2022, 1, 7))
+    paid_invoice_count = sap.mark_invoices_paid(
+        alma_client, invoices, datetime(2022, 1, 7)
+    )
     assert alma_client.mark_invoice_paid.call_args_list == expected_calls
-    assert result == 3
+    assert paid_invoice_count == 3
 
 
 def test_mark_invoices_paid_error(alma_client, caplog):
@@ -630,7 +631,7 @@ def test_mark_invoices_paid_error(alma_client, caplog):
     alma_client.mark_invoice_paid = MagicMock(
         side_effect=[
             {"payment": {"payment_status": {"value": "PAID"}}},
-            {"payment": {"payment_status": {"value": "WRONG"}}},
+            ValueError,
             {"payment": {"payment_status": {"value": "PAID"}}},
         ]
     )
@@ -640,14 +641,42 @@ def test_mark_invoices_paid_error(alma_client, caplog):
         call("2", date, "200", "GBH"),
         call("3", date, "300", "GBH"),
     ]
-    result = sap.mark_invoices_paid(alma_client, invoices, datetime(2022, 1, 7))
+
+    paid_invoice_count = sap.mark_invoices_paid(
+        alma_client, invoices, datetime(2022, 1, 7)
+    )
     assert alma_client.mark_invoice_paid.call_args_list == expected_calls
-    assert result == 2
-    assert (
-        "Something went wrong marking invoice '2' paid in Alma, it "
-        "should be investigated manually" in caplog.text
+    assert paid_invoice_count == 2
+    assert "Something went wrong marking invoice '2' paid in Alma." in caplog.text
+
+
+def test_mark_invoices_paid_handles_request_exception(alma_client, caplog):
+    date = datetime(2022, 1, 7)
+    invoices = [
+        {"id": "1", "total amount": "100", "currency": "USD"},
+        {"id": "2", "total amount": "200", "currency": "GBH"},
+        {"id": "3", "total amount": "300", "currency": "GBH"},
+    ]
+    alma_client.mark_invoice_paid = MagicMock(
+        side_effect=[
+            {"payment": {"payment_status": {"value": "PAID"}}},
+            {"payment": {"payment_status": {"value": "PAID"}}},
+            requests.exceptions.RequestException,
+        ]
     )
 
+    expected_calls = [
+        call("1", date, "100", "USD"),
+        call("2", date, "200", "GBH"),
+        call("3", date, "300", "GBH"),
+    ]
+    paid_invoice_count = sap.mark_invoices_paid(
+        alma_client, invoices, datetime(2022, 1, 7)
+    )
+    assert alma_client.mark_invoice_paid.call_args_list == expected_calls
+    assert paid_invoice_count == 2
+    assert "Something went wrong marking invoice '3' paid in Alma." in caplog.text
+
 
 def test_run_not_final_not_real(
     alma_client,