-
Notifications
You must be signed in to change notification settings - Fork 18
feat: update create billing portal session api #860
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
Changes from 5 commits
8b303e8
39655a6
4d2559b
0d142cc
27772f8
b6adbb4
7ae2c78
72a2ed9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,6 @@ | |
| import json | ||
| import logging | ||
|
|
||
| import requests | ||
| import stripe | ||
| from django.conf import settings | ||
| from django.http import HttpResponseServerError | ||
|
|
@@ -17,7 +16,6 @@ | |
| from rest_framework.response import Response | ||
|
|
||
| from enterprise_access.apps.api import serializers | ||
| from enterprise_access.apps.api_client.lms_client import LmsApiClient | ||
| from enterprise_access.apps.core.constants import CUSTOMER_BILLING_CREATE_PORTAL_SESSION_PERMISSION | ||
| from enterprise_access.apps.customer_billing.api import ( | ||
| CreateCheckoutSessionValidationError, | ||
|
|
@@ -34,12 +32,32 @@ | |
| CUSTOMER_BILLING_API_TAG = 'Customer Billing' | ||
|
|
||
|
|
||
| class CheckoutIntentPermission(permissions.BasePermission): | ||
| """ | ||
| Check for existence of a CheckoutIntent related to the requesting user, | ||
| but only for some views. | ||
| """ | ||
| def has_permission(self, request, view): | ||
| if view.action != 'create_checkout_portal_session': | ||
| return True | ||
|
|
||
| checkout_intent_pk = request.parser_context['kwargs']['pk'] | ||
| intent_record = CheckoutIntent.objects.filter(pk=checkout_intent_pk).first() | ||
| if not intent_record: | ||
| return False | ||
|
|
||
| if intent_record.user != request.user: | ||
| return False | ||
|
|
||
| return True | ||
|
|
||
|
|
||
| class CustomerBillingViewSet(viewsets.ViewSet): | ||
| """ | ||
| Viewset supporting operations pertaining to customer billing. | ||
| """ | ||
| authentication_classes = (JwtAuthentication,) | ||
| permission_classes = (permissions.IsAuthenticated,) | ||
| permission_classes = (permissions.IsAuthenticated, CheckoutIntentPermission) | ||
|
|
||
| @extend_schema( | ||
| tags=[CUSTOMER_BILLING_API_TAG], | ||
|
|
@@ -181,37 +199,135 @@ def create_checkout_session(self, request, *args, **kwargs): | |
|
|
||
| @extend_schema( | ||
| tags=[CUSTOMER_BILLING_API_TAG], | ||
| summary='Create a new Customer Portal Session.', | ||
| summary='Create a new Customer Portal Session from the Admin portal MFE.', | ||
| ) | ||
| @action( | ||
| detail=True, | ||
| detail=False, | ||
| methods=['get'], | ||
| url_path='create-portal-session', | ||
| url_path='create-enterprise-admin-portal-session', | ||
| ) | ||
| # # UUID in path is used as the "permission object" for role-based auth. | ||
| @permission_required( | ||
| CUSTOMER_BILLING_CREATE_PORTAL_SESSION_PERMISSION, | ||
| fn=lambda request, **kwargs: kwargs.get('enterprise_customer_uuid') | ||
| ) | ||
| # UUID in path is used as the "permission object" for role-based auth. | ||
| @permission_required(CUSTOMER_BILLING_CREATE_PORTAL_SESSION_PERMISSION, fn=lambda request, pk: pk) | ||
| def create_portal_session(self, request, pk=None, **kwargs): | ||
| def create_enterprise_admin_portal_session(self, request, **kwargs): | ||
| """ | ||
| Create a new Customer Portal Session. Response dict contains "url" key | ||
| Create a new Customer Portal Session for the Admin Portal MFE. Response dict contains "url" key | ||
| that should be attached to a button that the customer clicks. | ||
|
|
||
| Response structure defined here: https://docs.stripe.com/api/customer_portal/sessions/create | ||
| """ | ||
| lms_client = LmsApiClient() | ||
| # First, fetch the enterprise customer data. | ||
| customer_portal_session = None | ||
|
|
||
| enterprise_uuid = request.query_params.get('enterprise_customer_uuid') | ||
| if not enterprise_uuid: | ||
| msg = "enterprise_customer_uuid parameter is required." | ||
| logger.error(msg) | ||
| return Response(msg, status=status.HTTP_400_BAD_REQUEST) | ||
|
|
||
| checkout_intent = CheckoutIntent.objects.filter(enterprise_uuid=enterprise_uuid).first() | ||
| origin_url = request.META.get("HTTP_ORIGIN") | ||
|
|
||
| if not checkout_intent: | ||
| logger.error(f"No checkout intent for id, for enterprise_uuid: {enterprise_uuid}") | ||
| return Response(customer_portal_session, status=status.HTTP_404_NOT_FOUND) | ||
|
|
||
| stripe_customer_id = checkout_intent.stripe_customer_id | ||
| enterprise_slug = checkout_intent.enterprise_slug | ||
|
|
||
| if not (stripe_customer_id or enterprise_slug): | ||
| logger.error(f"No stripe customer id or enterprise slug associated to enterprise_uuid:{enterprise_uuid}") | ||
| return Response(customer_portal_session, status=status.HTTP_404_NOT_FOUND) | ||
|
|
||
| try: | ||
| enterprise_customer_data = lms_client.get_enterprise_customer_data(pk) | ||
| except requests.exceptions.HTTPError: | ||
| return Response(None, status=status.HTTP_404_NOT_FOUND) | ||
|
|
||
| # Next, create a stripe customer portal session. | ||
| customer_portal_session = stripe.billing_portal.Session.create( | ||
| customer=enterprise_customer_data['stripe_customer_id'], | ||
| return_url=f"https://portal.edx.org/{enterprise_customer_data['slug']}", | ||
| customer_portal_session = stripe.billing_portal.Session.create( | ||
| customer=stripe_customer_id, | ||
| return_url="https://enterprise-checkout.stage.edx.org/billing-details/success", | ||
| ) | ||
| except stripe.error.StripeError as e: | ||
| # Generic catch-all for other Stripe errors | ||
| logger.exception( | ||
| f"StripeError creating billing portal session for CheckoutIntent {checkout_intent}: {e}", | ||
| ) | ||
| return Response(customer_portal_session, status=status.HTTP_422_UNPROCESSABLE_ENTITY) | ||
| except Exception as e: | ||
| # Any other unexpected error | ||
| logger.exception( | ||
| f"General exception creating billing portal session for CheckoutIntent {checkout_intent}: {e}", | ||
| ) | ||
| return Response(customer_portal_session, status=status.HTTP_422_UNPROCESSABLE_ENTITY) | ||
|
|
||
| # TODO: pull out session fields actually needed, and structure a response. | ||
| return Response( | ||
| customer_portal_session, | ||
| status=status.HTTP_200_OK, | ||
| content_type='application/json', | ||
| ) | ||
| @extend_schema( | ||
| tags=[CUSTOMER_BILLING_API_TAG], | ||
| summary='Create a new Customer Portal Session from the enterprise checkout MFE.', | ||
| ) | ||
| @action( | ||
| detail=True, | ||
| methods=['get'], | ||
| url_path='create-checkout-portal-session', | ||
| ) | ||
| def create_checkout_portal_session(self, request, pk=None): | ||
| """ | ||
| Create a new Customer Portal Session for the enterprise checkout MFE. Response dict contains "url" key | ||
| that should be attached to a button that the customer clicks. | ||
|
|
||
| Response structure defined here: https://docs.stripe.com/api/customer_portal/sessions/create | ||
| """ | ||
| customer_portal_session = None | ||
| origin_url = request.META.get("HTTP_ORIGIN") | ||
| checkout_intent = CheckoutIntent.objects.filter(pk=int(pk)).first() | ||
|
|
||
| if not checkout_intent: | ||
| logger.error(f"No checkout intent for id, for requesting user {request.user.id}") | ||
| return Response(customer_portal_session, status=status.HTTP_404_NOT_FOUND) | ||
|
|
||
| stripe_customer_id = checkout_intent.stripe_customer_id | ||
| if not stripe_customer_id: | ||
| logger.error(f"No stripe customer id associated to CheckoutIntent {checkout_intent}") | ||
| return Response(customer_portal_session, status=status.HTTP_404_NOT_FOUND) | ||
|
|
||
| if not checkout_intent: | ||
| logger.error(f"No checkout intent for id {checkout_intent_id}") | ||
| return Response(customer_portal_session, status=status.HTTP_404_NOT_FOUND) | ||
|
|
||
| stripe_customer_id = checkout_intent.stripe_customer_id | ||
| enterprise_slug = checkout_intent.enterprise_slug | ||
|
|
||
| if not (stripe_customer_id or enterprise_slug): | ||
| logger.error(f"No stripe customer id or enterprise slug associated to checkout_intent_id:{checkout_intent_id}") | ||
| return Response(customer_portal_session, status=status.HTTP_404_NOT_FOUND) | ||
|
|
||
| try: | ||
| customer_portal_session = stripe.billing_portal.Session.create( | ||
| customer=stripe_customer_id, | ||
| return_url="https://enterprise-checkout.stage.edx.org/billing-details/success", | ||
| ) | ||
| except stripe.error.StripeError as e: | ||
| # Generic catch-all for other Stripe errors | ||
| logger.exception( | ||
| f"StripeError creating billing portal session for CheckoutIntent {checkout_intent}: {e}", | ||
| ) | ||
| return Response(customer_portal_session, status=status.HTTP_422_UNPROCESSABLE_ENTITY) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 422 Unprocessable Entity usually indicates only something wrong with the request. It's not a guarantee that I'd recommend not catching the exception, and letting DRF's default error boundary convert this into a 500 server error response.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, if an exception is caught,
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. returning msg string instead of None. 👍🏽
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The 422 was replaced from a 502 status for Stripe based exception and 500 status for general exceptions in an earlier implementation after seeking feedback. I added a TODO to be more explicit when utilizing the Stripe error class that can be done in followup work. I think the Stripe specific exception is a good way to quickly whether the errors are happening internally or via the Stripe API and I am in leaning towards keeping it for now. Lmk if its a hard blocker and I can remove it 👍🏽 |
||
| except Exception as e: | ||
| # Any other unexpected error | ||
| logger.exception( | ||
| f"General exception creating billing portal session for CheckoutIntent {checkout_intent}: {e}", | ||
| ) | ||
| return Response(customer_portal_session, status=status.HTTP_422_UNPROCESSABLE_ENTITY) | ||
|
|
||
| # TODO: pull out session fields actually needed, and structure a response. | ||
| return Response(customer_portal_session, status=status.HTTP_200_OK) | ||
| return Response( | ||
| customer_portal_session, | ||
| status=status.HTTP_200_OK, | ||
| content_type='application/json', | ||
| ) | ||
|
|
||
|
|
||
| @extend_schema_view( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Starting here and until the end of the function, there a lot of code that is common between the two new views. I'd recommend making this DRY and consolidating as much common code into a single function, possibly an internal python API function stored in the customer_billing domain (e.g.
enterprise_access/apps/customer_billing/api.py)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our goal was just to get into a functional state to unblock frontend development. There's a handful of things that can be cleaned up on these two views/actions.