Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add more semantics to API exceptions related to subsidy fulfillment #207

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iloveagent57
Copy link
Contributor

@iloveagent57 iloveagent57 commented Jun 27, 2023

This change helps to thread error responses from GEAG back through the response payloads of enterprise-subsidy and enterprise-access in our redemption flow.
It also sets up a more generic framework for describing, to the caller of the policy redeem endpoint, the semantics of failures that occur in the downstream redeem request of the subsidy API.
The structure of the error response will be like so (should be backwards compatible):

{'code': 'fulfillment_error',
 'detail': {'metadata': {'enterprise_administrators': [{'email': '[email protected]'}],
                         'subsidy_error_detail': [{'message': 'woozit '
                                                              'duplicate order '
                                                              'woohoo!'}]},
            'reason': 'duplicate_fulfillment',
            'user_message': 'A legacy fulfillment already exists for this '
                            'content.'}}

@iloveagent57 iloveagent57 force-pushed the aed/redeem-error-msg branch from 08b78e4 to 9707b2a Compare June 28, 2023 16:01
@iloveagent57 iloveagent57 changed the title checkpoint add more semantics to API exceptions related to subsidy fulfillment Jun 28, 2023
Comment on lines +114 to +117
``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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just the result of the subsidy API being indecisive about error formatting and being written by >1 person?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof yes, can we fix the offending call?

Copy link
Contributor Author

@iloveagent57 iloveagent57 Jun 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote it to be defensive about the types of messages that can exist from a DRF APIException: https://github.com/encode/django-rest-framework/blob/master/rest_framework/exceptions.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants