From 1f2b94bdf322e9ae67922808f9fe30203568a910 Mon Sep 17 00:00:00 2001 From: Kegan Maher Date: Fri, 1 Dec 2023 20:15:20 +0000 Subject: [PATCH 1/3] fix(enrollment): update re-enrollment error handling do not rely on specific text of the error message instead check for presence of customer ID and group ID --- benefits/enrollment/api.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/benefits/enrollment/api.py b/benefits/enrollment/api.py index 525d74393..964a90da3 100644 --- a/benefits/enrollment/api.py +++ b/benefits/enrollment/api.py @@ -65,7 +65,7 @@ def __init__(self, response): class GroupResponse: """Benefits Enrollment Customer Group API response.""" - def __init__(self, response, requested_id, payload=None): + def __init__(self, response, requested_id, group_id, payload=None): if payload is None: try: payload = response.json() @@ -74,18 +74,12 @@ def __init__(self, response, requested_id, payload=None): else: try: # Group API uses an error response (500) to indicate that the customer already exists in the group (!!!) - # The error message should contain the customer ID we sent via payload and start with "Duplicate" + # The error message should contain the customer ID and group ID we sent via payload error = response.json()["errors"][0] customer_id = payload[0] detail = error["detail"] - failure = ( - customer_id is None - or detail is None - or customer_id not in detail - or customer_id in detail - and not detail.startswith("Duplicate") - ) + failure = customer_id is None or detail is None or not (customer_id in detail and group_id in detail) if failure: raise ApiError("Invalid response format") @@ -269,10 +263,10 @@ def enroll(self, customer_token, group_id): if r.status_code in (200, 201): logger.info("Customer enrolled in group") - return GroupResponse(r, customer.id) + return GroupResponse(r, customer.id, group_id) elif r.status_code == 500: logger.info("Customer already exists in group") - return GroupResponse(r, customer.id, payload=payload) + return GroupResponse(r, customer.id, group_id, payload=payload) else: r.raise_for_status() except requests.ConnectionError: From 87b2671fa43c4539e60514d74a0017b08711d05c Mon Sep 17 00:00:00 2001 From: Kegan Maher Date: Fri, 1 Dec 2023 20:16:02 +0000 Subject: [PATCH 2/3] test(enrollment): update tests for error condition --- .../enrollment/test_api_GroupResponse.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/pytest/enrollment/test_api_GroupResponse.py b/tests/pytest/enrollment/test_api_GroupResponse.py index 20807773e..4a5aaeeb0 100644 --- a/tests/pytest/enrollment/test_api_GroupResponse.py +++ b/tests/pytest/enrollment/test_api_GroupResponse.py @@ -8,14 +8,14 @@ def test_no_payload_invalid_response(mocker): mock_response.json.side_effect = ValueError with pytest.raises(ApiError, match=r"response"): - GroupResponse(mock_response, 0) + GroupResponse(mock_response, "customer", "group") def test_no_payload_valid_response_single_matching_id(mocker): mock_response = mocker.Mock() mock_response.json.return_value = ["0"] - response = GroupResponse(mock_response, "0") + response = GroupResponse(mock_response, "0", "group") assert response.customer_ids == ["0"] assert response.updated_customer_id == "0" @@ -27,7 +27,7 @@ def test_no_payload_valid_response_single_unmatching_id(mocker): mock_response = mocker.Mock() mock_response.json.return_value = ["1"] - response = GroupResponse(mock_response, "0") + response = GroupResponse(mock_response, "0", "group") assert response.customer_ids == ["1"] assert response.updated_customer_id == "1" @@ -39,7 +39,7 @@ def test_no_payload_valid_response_multiple_ids(mocker): mock_response = mocker.Mock() mock_response.json.return_value = ["0", "1"] - response = GroupResponse(mock_response, "0") + response = GroupResponse(mock_response, "0", "group") assert response.customer_ids == ["0", "1"] assert not response.updated_customer_id @@ -53,14 +53,14 @@ def test_payload_invalid_response(mocker, exception): mock_response.json.side_effect = exception with pytest.raises(ApiError, match=r"response"): - GroupResponse(mock_response, "0", []) + GroupResponse(mock_response, "0", "group", []) def test_payload_valid_response(mocker): mock_response = mocker.Mock() - mock_response.json.return_value = {"errors": [{"detail": "Duplicate id 0"}]} + mock_response.json.return_value = {"errors": [{"detail": "0 group"}]} - response = GroupResponse(mock_response, "0", ["0"]) + response = GroupResponse(mock_response, "0", "group", ["0"]) assert response.customer_ids == ["0"] assert response.updated_customer_id == "0" @@ -69,13 +69,13 @@ def test_payload_valid_response(mocker): failure_conditions = [ - # customer_id is None - ({"detail": "Duplicate"}, [None]), # detail is None ({"detail": None}, ["0"]), + # customer_id is None + ({"detail": "0 group"}, [None]), # customer_id not in detail - ({"detail": "1"}, ["0"]), - # customer_id in detail, detail doesn't start with Duplicate + ({"detail": "1 group"}, ["0"]), + # group_id not in detail ({"detail": "0"}, ["0"]), ] @@ -86,4 +86,4 @@ def test_payload_failure_response(mocker, error, payload): mock_response.json.return_value = {"errors": [error]} with pytest.raises(ApiError, match=r"response"): - GroupResponse(mock_response, "0", payload) + GroupResponse(mock_response, "0", "group", payload) From 79474763463d3ea61db873a3048578f2ff973428 Mon Sep 17 00:00:00 2001 From: Angela Tran Date: Fri, 1 Dec 2023 21:14:37 +0000 Subject: [PATCH 3/3] chore: bump version to 2023.12.1 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index ff3fc46af..6fd1054ac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "benefits" -version = "2023.09.1" +version = "2023.12.1" description = "Cal-ITP Benefits is an application that enables automated eligibility verification and enrollment for transit benefits onto customers’ existing contactless bank (credit/debit) cards." readme = "README.md" license = { file = "LICENSE" }