From 9707b2aff8238425e9296ae606327195e6d6788e Mon Sep 17 00:00:00 2001 From: Alex Dusenbery Date: Wed, 21 Jun 2023 11:00:23 -0400 Subject: [PATCH] feat: add more semantics to API exceptions related to subsidy fulfillment. ENT-7271 --- enterprise_access/apps/api/v1/exceptions.py | 143 ++++++++++++++++++ .../tests/test_subsidy_access_policy_views.py | 62 ++++++++ .../api/v1/views/subsidy_access_policy.py | 50 +++--- .../apps/subsidy_access_policy/constants.py | 55 +++++++ 4 files changed, 279 insertions(+), 31 deletions(-) create mode 100644 enterprise_access/apps/api/v1/exceptions.py diff --git a/enterprise_access/apps/api/v1/exceptions.py b/enterprise_access/apps/api/v1/exceptions.py new file mode 100644 index 00000000..910bc036 --- /dev/null +++ b/enterprise_access/apps/api/v1/exceptions.py @@ -0,0 +1,143 @@ +""" +Version 1 API Exceptions. +""" +from rest_framework import status +from rest_framework.exceptions import APIException + +from enterprise_access.apps.api_client.lms_client import LmsApiClient +from enterprise_access.apps.subsidy_access_policy import constants + + +class RedemptionRequestException(APIException): + status_code = status.HTTP_422_UNPROCESSABLE_ENTITY + default_detail = 'Could not redeem' + + +class SubsidyAPIRedemptionRequestException(RedemptionRequestException): + """ + An API exception that has a response payload structured like + { + 'code': 'some_error_code', + 'detail': { + 'reason': 'reason_for_error', + 'user_message': 'User friendly string describing the error.', + # additional metadata describing the error, possibly including admin emails. + 'metadata': { + 'key': 'value', + } + } + } + + There are some sane defaults set at initialization for the reason, code, and user_message + values. + """ + default_detail = 'Error redeeming through Subsidy API' + default_code = constants.SubsidyRedemptionErrorCodes.DEFAULT_ERROR + + # Custom keys of the `detail` field returned in the response payload. + default_reason = constants.SubsidyRedemptionErrorReasons.DEFAULT_REASON + default_user_message = constants.SubsidyRedemptionErrorReasons.USER_MESSAGES_BY_REASON[default_reason] + + def __init__(self, code=None, detail=None, policy=None, subsidy_api_error=None): + """ + Initializes all of the attributes of this exception instance. + + args: + code (str): A reusable error code constant. + detail ([list,str,dict]): Details about the exception we're raising. + policy (SubsidyAccessPolicy): A policy object, from which we can fetch admin email addresses. + subsidy_api_error (SubsidyAPIHTTPError): The exception object that was caught, from which + we can infer more specific causes about the redemption error this exception encapsulates. + """ + super().__init__(code=code, detail=detail) + + self.reason = self.default_reason + self.user_message = self.default_user_message + self.metadata = {} + + if policy and subsidy_api_error: + try: + self._build_subsidy_api_error_payload(policy, subsidy_api_error) + except Exception: # pylint: disable=broad-except + self.metadata = { + 'subsidy_error_detail_raw': str(subsidy_api_error.error_response.content), + } + + self.detail = { + 'code': self.code, + 'detail': { + 'reason': self.reason, + 'user_message': self.user_message, + 'metadata': self.metadata, + } + } + + def _build_subsidy_api_error_payload(self, policy, subsidy_api_error): + """ + Helper to build error response payload on Subsidy API errors. + """ + subsidy_error_detail = subsidy_api_error.error_payload().get('detail') + subsidy_error_code = subsidy_api_error.error_payload().get('code') + + self.metadata = { + 'enterprise_administrators': LmsApiClient().get_enterprise_customer_data( + policy.enterprise_customer_uuid + ).get('admin_users') + } + + # We currently only have enough data to say more specific things + # about fulfillment errors during subsidy API redemption. + if subsidy_error_code == constants.SubsidyRedemptionErrorCodes.FULFILLMENT_ERROR: + self._set_subsidy_fulfillment_error_reason(subsidy_error_detail) + + def _set_subsidy_fulfillment_error_reason(self, subsidy_error_detail): + """ + Helper to set the reason, user_message, and metadata + for the given subsidy_error_detail. + """ + self.metadata['subsidy_error_detail'] = subsidy_error_detail + self.reason = constants.SubsidyFulfillmentErrorReasons.DEFAULT_REASON + + if subsidy_error_detail: + message_string = self._get_subsidy_fulfillment_error_message(subsidy_error_detail) + if cause_of_message := constants.SubsidyFulfillmentErrorReasons.get_cause_from_error_message( + message_string + ): + self.reason = cause_of_message + # pylint: disable=attribute-defined-outside-init + self.code = constants.SubsidyRedemptionErrorCodes.FULFILLMENT_ERROR + + self.user_message = constants.SubsidyFulfillmentErrorReasons.USER_MESSAGES_BY_REASON.get(self.reason) + + def _get_subsidy_fulfillment_error_message(self, subsidy_error_detail): + """ + ``subsidy_error_detail`` is either a string describing an error message, + a dict with a "message" key describing an error message, or a list of such + dicts. This helper method widdles any of those things down into a single + error message string. + """ + if isinstance(subsidy_error_detail, str): + return subsidy_error_detail + + subsidy_message_dict = subsidy_error_detail + if isinstance(subsidy_error_detail, list): + subsidy_message_dict = subsidy_error_detail[0] + + return subsidy_message_dict.get('message') + + +class SubsidyAccessPolicyLockedException(APIException): + """ + Throw this exception when an attempt to acquire a policy lock failed because it was already locked by another agent. + + Note: status.HTTP_423_LOCKED is NOT acceptable as a status code for delivery to web browsers. According to Mozilla: + + > The ability to lock a resource is specific to some WebDAV servers. Browsers accessing web pages will never + > encounter this status code; in the erroneous cases it happens, they will handle it as a generic 400 status code. + + See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/423 + + HTTP 429 Too Many Requests is the next best thing, and implies retryability. + """ + status_code = status.HTTP_429_TOO_MANY_REQUESTS + default_detail = 'Enrollment currently locked for this subsidy access policy.' diff --git a/enterprise_access/apps/api/v1/tests/test_subsidy_access_policy_views.py b/enterprise_access/apps/api/v1/tests/test_subsidy_access_policy_views.py index b22c9693..7759cbb5 100644 --- a/enterprise_access/apps/api/v1/tests/test_subsidy_access_policy_views.py +++ b/enterprise_access/apps/api/v1/tests/test_subsidy_access_policy_views.py @@ -7,6 +7,7 @@ from uuid import UUID, uuid4 import ddt +import requests from django.conf import settings from rest_framework import status from rest_framework.reverse import reverse @@ -22,6 +23,8 @@ AccessMethods, MissingSubsidyAccessReasonUserMessages, PolicyTypes, + SubsidyFulfillmentErrorReasons, + SubsidyRedemptionErrorCodes, TransactionStateChoices ) from enterprise_access.apps.subsidy_access_policy.tests.factories import ( @@ -682,6 +685,65 @@ def test_redeem_policy(self, mock_transactions_cache_for_learner): # pylint: di ), ) + @mock.patch('enterprise_access.apps.subsidy_access_policy.models.get_and_cache_transactions_for_learner') + @mock.patch('enterprise_access.apps.api.v1.exceptions.LmsApiClient') + @ddt.data( + { + 'subsidy_error_code': 'fulfillment_error', + 'subsidy_error_detail': [ + {'message': 'woozit duplicate order woohoo!'}, + ], + 'expected_redeem_error_detail': { + 'reason': SubsidyFulfillmentErrorReasons.DUPLICATE_FULFILLMENT, + 'user_message': SubsidyFulfillmentErrorReasons.USER_MESSAGES_BY_REASON[ + SubsidyFulfillmentErrorReasons.DUPLICATE_FULFILLMENT + ], + 'metadata': { + 'enterprise_administrators': [{'email': 'edx@example.com'}], + 'subsidy_error_detail': [ + {'message': 'woozit duplicate order woohoo!'} + ], + }, + }, + 'expected_redeem_error_code': SubsidyRedemptionErrorCodes.FULFILLMENT_ERROR, + }, + ) + @ddt.unpack + def test_redeem_policy_subsidy_api_error( + self, mock_lms_api_client, mock_transactions_cache_for_learner, # pylint: disable=unused-argument + subsidy_error_code, subsidy_error_detail, + expected_redeem_error_detail, expected_redeem_error_code + ): + """ + Verify that SubsidyAccessPolicyRedeemViewset redeem endpoint returns a well-structured + error response payload when the subsidy API call to redeem/fulfill responds with an error. + """ + mock_lms_api_client().get_enterprise_customer_data.return_value = { + 'slug': 'the-slug', + 'admin_users': [{'email': 'edx@example.com'}], + } + self.mock_get_content_metadata.return_value = {'content_price': 123} + mock_response = mock.MagicMock() + mock_response.json.return_value = { + 'code': subsidy_error_code, + 'detail': subsidy_error_detail, + } + self.redeemable_policy.subsidy_client.create_subsidy_transaction.side_effect = requests.exceptions.HTTPError( + response=mock_response + ) + + payload = { + 'lms_user_id': 1234, + 'content_key': 'course-v1:edX+edXPrivacy101+3T2020', + } + + response = self.client.post(self.subsidy_access_policy_redeem_endpoint, payload) + + response_json = self.load_json(response.content) + self.maxDiff = None + self.assertEqual(response_json['detail'], expected_redeem_error_detail) + self.assertEqual(response_json['code'], expected_redeem_error_code) + @mock.patch('enterprise_access.apps.subsidy_access_policy.models.get_and_cache_transactions_for_learner') def test_redeem_policy_with_metadata(self, mock_transactions_cache_for_learner): # pylint: disable=unused-argument """ diff --git a/enterprise_access/apps/api/v1/views/subsidy_access_policy.py b/enterprise_access/apps/api/v1/views/subsidy_access_policy.py index b575a1d8..6a0bf4b3 100644 --- a/enterprise_access/apps/api/v1/views/subsidy_access_policy.py +++ b/enterprise_access/apps/api/v1/views/subsidy_access_policy.py @@ -21,7 +21,7 @@ from rest_framework import serializers as rest_serializers from rest_framework import status, viewsets from rest_framework.decorators import action -from rest_framework.exceptions import APIException, NotFound +from rest_framework.exceptions import NotFound from rest_framework.generics import get_object_or_404 from rest_framework.response import Response @@ -57,6 +57,11 @@ ) from enterprise_access.apps.subsidy_access_policy.subsidy_api import get_redemptions_by_content_and_policy_for_learner +from ..exceptions import ( + RedemptionRequestException, + SubsidyAccessPolicyLockedException, + SubsidyAPIRedemptionRequestException +) from .utils import PaginationWithPageCount logger = logging.getLogger(__name__) @@ -244,28 +249,6 @@ def get_queryset(self): return queryset.filter(enterprise_customer_uuid=enterprise_customer_uuid) -class RedemptionRequestException(APIException): - status_code = status.HTTP_422_UNPROCESSABLE_ENTITY - default_detail = 'Could not redeem' - - -class SubsidyAccessPolicyLockedException(APIException): - """ - Throw this exception when an attempt to acquire a policy lock failed because it was already locked by another agent. - - Note: status.HTTP_423_LOCKED is NOT acceptable as a status code for delivery to web browsers. According to Mozilla: - - > The ability to lock a resource is specific to some WebDAV servers. Browsers accessing web pages will never - > encounter this status code; in the erroneous cases it happens, they will handle it as a generic 400 status code. - - See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/423 - - HTTP 429 Too Many Requests is the next best thing, and implies retryability. - """ - status_code = status.HTTP_429_TOO_MANY_REQUESTS - default_detail = 'Enrollment currently locked for this subsidy access policy.' - - class SubsidyAccessPolicyRedeemViewset(UserDetailsFromJwtMixin, PermissionRequiredMixin, viewsets.GenericViewSet): """ Viewset for Subsidy Access Policy APIs. @@ -484,11 +467,10 @@ def redeem(self, request, *args, **kwargs): logger.exception(exc) raise SubsidyAccessPolicyLockedException() from exc except SubsidyAPIHTTPError as exc: - logger.exception(f'{exc} when creating transaction in subsidy API') - error_payload = exc.error_payload() - error_payload['detail'] = f"Subsidy Transaction API error: {error_payload['detail']}" - raise RedemptionRequestException( - detail=error_payload, + logger.exception(f'{exc} when creating transaction in subsidy API with payload {exc.error_payload()}') + raise SubsidyAPIRedemptionRequestException( + policy=policy, + subsidy_api_error=exc, ) from exc def get_existing_redemptions(self, policies, lms_user_id): @@ -673,9 +655,7 @@ def _get_reasons_for_no_redeemable_policies(self, enterprise_customer_uuid, non_ for each non-redeemable policy. """ reasons = [] - lms_client = LmsApiClient() - enterprise_customer_data = lms_client.get_enterprise_customer_data(enterprise_customer_uuid) - enterprise_admin_users = enterprise_customer_data.get('admin_users') + enterprise_admin_users = self._get_enterprise_admin_users(enterprise_customer_uuid) for reason, policies in non_redeemable_policies.items(): reasons.append({ @@ -689,6 +669,14 @@ def _get_reasons_for_no_redeemable_policies(self, enterprise_customer_uuid, non_ return reasons + def _get_enterprise_admin_users(self, enterprise_customer_uuid): + """ + Helper to fetch admin users for the given customer uuid. + """ + lms_client = LmsApiClient() + enterprise_customer_data = lms_client.get_enterprise_customer_data(enterprise_customer_uuid) + return enterprise_customer_data.get('admin_users') + def _get_list_price(self, enterprise_customer_uuid, content_key): """ Determine the price for content for display purposes only. diff --git a/enterprise_access/apps/subsidy_access_policy/constants.py b/enterprise_access/apps/subsidy_access_policy/constants.py index a7183ac4..be0328b4 100644 --- a/enterprise_access/apps/subsidy_access_policy/constants.py +++ b/enterprise_access/apps/subsidy_access_policy/constants.py @@ -1,4 +1,5 @@ """ Constants for the subsidy_access_policy app. """ +import re class AccessMethods: @@ -99,3 +100,57 @@ class MissingSubsidyAccessReasonUserMessages: REASON_LEARNER_MAX_SPEND_REACHED = "learner_max_spend_reached" REASON_POLICY_SPEND_LIMIT_REACHED = "policy_spend_limit_reached" REASON_LEARNER_MAX_ENROLLMENTS_REACHED = "learner_max_enrollments_reached" + + +class SubsidyRedemptionErrorCodes: + """ + Collection of error ``code`` values that the subsidy API's + redeem endpoint might return in an error response payload. + """ + DEFAULT_ERROR = 'subsidy_redemption_error' + FULFILLMENT_ERROR = 'fulfillment_error' + + +class SubsidyRedemptionErrorReasons: + """ + Somewhat more generic collection of reasons that redemption may have + failed in ways that are *not* related to fulfillment. + """ + DEFAULT_REASON = 'default_subsidy_redemption_error' + + USER_MESSAGES_BY_REASON = { + DEFAULT_REASON: "Something went wrong during subsidy redemption", + } + + +class SubsidyFulfillmentErrorReasons: + """ + Codifies standard reasons that fulfillment may have failed, + along with a mapping of those reasons to user-friendly display messages. + """ + DEFAULT_REASON = 'default_fulfillment_error' + DUPLICATE_FULFILLMENT = 'duplicate_fulfillment' + + USER_MESSAGES_BY_REASON = { + DEFAULT_REASON: "Something went wrong during fulfillment", + DUPLICATE_FULFILLMENT: "A legacy fulfillment already exists for this content.", + } + + CAUSES_REGEXP_BY_REASON = { + DUPLICATE_FULFILLMENT: re.compile(".*duplicate order.*"), + } + + @classmethod + def get_cause_from_error_message(cls, message_string): + """ + Helper to find the cause of a given error message string + by matching against the regexs mapped above. + """ + if not message_string: + return None + + for cause_of_message, regex in cls.CAUSES_REGEXP_BY_REASON.items(): + if regex.match(message_string): + return cause_of_message + + return None