From fa5fb56b3dd9b5639ae13f6225d628dcde123681 Mon Sep 17 00:00:00 2001 From: julianajlk Date: Fri, 2 Feb 2024 15:19:21 -0500 Subject: [PATCH 01/11] refactor: Enable DPM and handle PI in other states for retry with InProgressProcessorResponse --- ecommerce/extensions/checkout/mixins.py | 21 +++++-- .../extensions/payment/processors/__init__.py | 4 ++ .../extensions/payment/processors/stripe.py | 62 ++++++++++++++++++- ecommerce/extensions/payment/views/stripe.py | 17 ++++- 4 files changed, 98 insertions(+), 6 deletions(-) diff --git a/ecommerce/extensions/checkout/mixins.py b/ecommerce/extensions/checkout/mixins.py index 8366f295b0c..d583ba93c23 100644 --- a/ecommerce/extensions/checkout/mixins.py +++ b/ecommerce/extensions/checkout/mixins.py @@ -93,7 +93,7 @@ def add_payment_event(self, event): # pylint: disable = arguments-differ self._payment_events = [] self._payment_events.append(event) - def handle_payment(self, response, basket): # pylint: disable=arguments-differ + def handle_payment(self, response, basket): # pylint: disable=arguments-differ, inconsistent-return-statements """ Handle any payment processing and record payment sources and events. @@ -111,14 +111,27 @@ def handle_payment(self, response, basket): # pylint: disable=arguments-differ # If payment didn't go through, the handle_processor_response function will raise an error. We want to # send the event regardless of if the payment didn't go through. try: - handled_processor_response = self.payment_processor.handle_processor_response(response, basket=basket) + processor_response = self.payment_processor.handle_processor_response(response, basket=basket) except Exception as ex: properties.update({'success': False, 'payment_error': type(ex).__name__, }) raise else: + # If the processor_response has a status, it's a InProgressProcessorResponse, + # which means the payment is part of dynamic payment methods + if 'status' in processor_response._fields: + logger.info( + 'Dynamic Payment Method in progress for edX order %s and basket %s, ' + 'returning Payment Intent %s with status %s to the payment MFE.', + processor_response.order_number, + processor_response.basket_id, + processor_response.transaction_id, + processor_response.status, + ) + # TODO: do we track this event if in progress? + return processor_response # We only record successful payments in the database. - self.record_payment(basket, handled_processor_response) - properties.update({'total': handled_processor_response.total, 'success': True, }) + self.record_payment(basket, processor_response) + properties.update({'total': processor_response.total, 'success': True, }) finally: track_segment_event(basket.site, basket.owner, 'Payment Processor Response', properties) diff --git a/ecommerce/extensions/payment/processors/__init__.py b/ecommerce/extensions/payment/processors/__init__.py index 31caf91b540..aa63f2e72c0 100644 --- a/ecommerce/extensions/payment/processors/__init__.py +++ b/ecommerce/extensions/payment/processors/__init__.py @@ -14,6 +14,10 @@ HandledProcessorResponse = namedtuple('HandledProcessorResponse', ['transaction_id', 'total', 'currency', 'card_number', 'card_type']) +InProgressProcessorResponse = namedtuple('InProgressProcessorResponse', + ['basket_id', 'order_number', 'transaction_id', 'confirmation_client_secret', + 'status', 'payment_method', 'total']) + logger = logging.getLogger(__name__) diff --git a/ecommerce/extensions/payment/processors/stripe.py b/ecommerce/extensions/payment/processors/stripe.py index caa47828e6b..812b8d11f12 100644 --- a/ecommerce/extensions/payment/processors/stripe.py +++ b/ecommerce/extensions/payment/processors/stripe.py @@ -4,6 +4,7 @@ import logging import stripe +from django.conf import settings from oscar.apps.payment.exceptions import GatewayError from oscar.core.loading import get_model @@ -19,7 +20,8 @@ from ecommerce.extensions.payment.processors import ( ApplePayMixin, BaseClientSidePaymentProcessor, - HandledProcessorResponse + HandledProcessorResponse, + InProgressProcessorResponse ) logger = logging.getLogger(__name__) @@ -103,6 +105,30 @@ def _build_payment_intent_parameters(self, basket): }, } + def create_new_payment_intent_for_basket(self, basket, payment_intent_id): + """ + Create a new Stripe payment intent to associate with the current basket. + This is used as a reset of the payment to allow payment retries when the intent gets into unexpected states. + """ + # Cancel existing Payment Intent + stripe.PaymentIntent.cancel(payment_intent_id) + + # Create a new Payment Intent and add to Basket + new_payment_intent = stripe.PaymentIntent.create( + **self._build_payment_intent_parameters(basket), + # This means this payment intent can only be confirmed with secret key (as in, from ecommerce) + secret_key_confirmation='required', + # don't create a new intent for the same basket + idempotency_key=self.generate_basket_pi_idempotency_key(basket), + # Enable dynamic payment methods, w/o payment method configuration ID due to Custom Actions Beta: + # 'allow_redirects' is default to 'always', + # 'enabled' is not default to True with CAB, only for Deferred Intents. + automatic_payment_methods={'enabled': True}, + ) + new_payment_intent_id = new_payment_intent['id'] + basket_add_payment_intent_id_attribute(basket, new_payment_intent_id) + return new_payment_intent + def generate_basket_pi_idempotency_key(self, basket): """ Generate an idempotency key for creating a PaymentIntent for a Basket. @@ -132,6 +158,10 @@ def get_capture_context(self, request): secret_key_confirmation='required', # don't create a new intent for the same basket idempotency_key=self.generate_basket_pi_idempotency_key(basket), + # Enable dynamic payment methods, w/o payment method configuration ID due to Custom Actions Beta + # 'allow_redirects' is default to 'always' + # 'enabled' is not default to True with CAB, only for Deferred Intents + automatic_payment_methods={'enabled': True}, ) # id is the payment_intent_id from Stripe @@ -179,6 +209,7 @@ def handle_processor_response(self, response, basket=None): # pretty sure we should simply return/error if basket is None, as not # sure what it would mean if there payment_intent_id = response['payment_intent_id'] + dynamic_payment_methods_enabled = response['dynamic_payment_methods_enabled'] # NOTE: In the future we may want to get/create a Customer. See https://stripe.com/docs/api#customers. # rewrite order amount so it's updated for coupon & quantity and unchanged by the user @@ -186,18 +217,47 @@ def handle_processor_response(self, response, basket=None): payment_intent_id, **self._build_payment_intent_parameters(basket), ) + + # Need a return_url for dynamic payment methods that require action outside of the payment MFE + return_url = settings.PAYMENT_MICROFRONTEND_URL + try: confirm_api_response = stripe.PaymentIntent.confirm( payment_intent_id, # stop on complicated payments MFE can't handle yet error_on_requires_action=True, expand=['payment_method'], + return_url=return_url, ) except stripe.error.CardError as err: self.record_processor_response(err.json_body, transaction_id=payment_intent_id, basket=basket) logger.exception('Card Error for basket [%d]: %s}', basket.id, err) raise + # If the payment has another status other than 'succeeded', we want to return to the MFE something it can handle + if dynamic_payment_methods_enabled: + if confirm_api_response['status'] == 'requires_action': + # The Payment Intent in this status will result in 'payment_intent_unexpected_state', since it cannot + # be confirmed again unless in 'requires_confirmation' or 'requires_payment_intent' status. + # Need to create a new one so the MFE can let the learner retry payment + new_payment_intent = self.create_new_payment_intent_for_basket(confirm_api_response['id'], basket) + logger.info( + 'Dynamic Payment Method received requires additional action for edX order %s, ' + 'created a new Payment Intent %s with status %s.', + confirm_api_response['metadata']['order_number'], + new_payment_intent['id'], + new_payment_intent['status'], + ) + return InProgressProcessorResponse( + basket_id=basket.id, + order_number=basket.order_number, + status=new_payment_intent['status'], + confirmation_client_secret=new_payment_intent['client_secret'], + transaction_id=new_payment_intent['id'], + payment_method=new_payment_intent['payment_method'], + total=new_payment_intent['amount'], + ) + # proceed only if payment went through assert confirm_api_response['status'] == "succeeded" self.record_processor_response(confirm_api_response, transaction_id=payment_intent_id, basket=basket) diff --git a/ecommerce/extensions/payment/views/stripe.py b/ecommerce/extensions/payment/views/stripe.py index feb71b0cf38..70106de4518 100644 --- a/ecommerce/extensions/payment/views/stripe.py +++ b/ecommerce/extensions/payment/views/stripe.py @@ -228,13 +228,16 @@ def post(self, request): try: with transaction.atomic(): try: - self.handle_payment(stripe_response, basket) + in_progress_payment = self.handle_payment(stripe_response, basket) except CardError as err: return self.stripe_error_response(err) except: # pylint: disable=bare-except logger.exception('Attempts to handle payment for basket [%d] failed.', basket.id) return self.error_page_response() + # We'll only have a return value if the payment is a dynamic payment methods in progress + if in_progress_payment: + return self.dynamic_payment_methods_response(in_progress_payment) try: billing_address = self.create_billing_address( user=self.request.user, @@ -293,3 +296,15 @@ def stripe_error_response(self, error): 'error_code': error.code, 'user_message': error.user_message, }, status=400) + + def dynamic_payment_methods_response(self, in_progress_payment): + """Tell the frontend the Payment Intent status and it will decide what to do.""" + if 'status' in in_progress_payment._fields: + response_data = { + 'status': in_progress_payment.status, + 'confirmation_client_secret': in_progress_payment.confirmation_client_secret, + 'payment_method': in_progress_payment.payment_method, + 'total': in_progress_payment.total, + 'transaction_id': in_progress_payment.transaction_id, + } + return JsonResponse(response_data, status=201) From 015594fed8697d1d57769e8d39dc7561da33fc87 Mon Sep 17 00:00:00 2001 From: julianajlk Date: Fri, 9 Feb 2024 11:35:59 -0500 Subject: [PATCH 02/11] fix: Set PAYMENT_MICROFRONTEND_URL in base/devstack.py --- ecommerce/settings/base.py | 2 +- ecommerce/settings/devstack.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ecommerce/settings/base.py b/ecommerce/settings/base.py index 6e2ebf1d542..c25cd5c4488 100644 --- a/ecommerce/settings/base.py +++ b/ecommerce/settings/base.py @@ -865,7 +865,7 @@ ECOMMERCE_MICROFRONTEND_URL = os.environ.get('ECOMMERCE_MICROFRONTEND_URL') # Needed to link to the payment micro-frontend -PAYMENT_MICROFRONTEND_URL = None +PAYMENT_MICROFRONTEND_URL = os.environ.get('PAYMENT_MICROFRONTEND_URL') # For Enterprise purchases to send purchase information to HubSpot for marketing leads HUBSPOT_FORMS_API_URI = "SET-ME-PLEASE" diff --git a/ecommerce/settings/devstack.py b/ecommerce/settings/devstack.py index d45291a1635..5ce82962b2a 100644 --- a/ecommerce/settings/devstack.py +++ b/ecommerce/settings/devstack.py @@ -63,6 +63,8 @@ ) CORS_ALLOW_CREDENTIALS = True +PAYMENT_MICROFRONTEND_URL = 'http://localhost:1998' + ECOMMERCE_MICROFRONTEND_URL = 'http://localhost:1996' ENTERPRISE_CATALOG_API_URL = urljoin(f"{ENTERPRISE_CATALOG_SERVICE_URL}/", 'api/v1/') From dcef1f6fd8a45359ebd648e1e620fcb61cc2bd4b Mon Sep 17 00:00:00 2001 From: julianajlk Date: Mon, 4 Mar 2024 13:03:28 -0500 Subject: [PATCH 03/11] refactor: Add Payment Intent ID, status and DPM boolean to basket --- ecommerce/extensions/basket/constants.py | 2 + ecommerce/extensions/basket/utils.py | 34 ++++++++++++++ ecommerce/extensions/basket/views.py | 45 ++++++++++++++++++- .../extensions/payment/processors/stripe.py | 17 +++++-- ecommerce/extensions/payment/views/stripe.py | 11 ++++- 5 files changed, 103 insertions(+), 6 deletions(-) diff --git a/ecommerce/extensions/basket/constants.py b/ecommerce/extensions/basket/constants.py index a642b04ad68..04715ca7eca 100644 --- a/ecommerce/extensions/basket/constants.py +++ b/ecommerce/extensions/basket/constants.py @@ -2,6 +2,8 @@ EMAIL_OPT_IN_ATTRIBUTE = "email_opt_in" PURCHASER_BEHALF_ATTRIBUTE = "purchased_for_organization" PAYMENT_INTENT_ID_ATTRIBUTE = "payment_intent_id" +PAYMENT_INTENT_STATUS_ATTRIBUTE = "payment_intent_status" +DYNAMIC_PAYMENT_METHODS_ENABLED = "dynamic_payment_methods_enabled" # .. toggle_name: enable_stripe_payment_processor # .. toggle_type: waffle_flag diff --git a/ecommerce/extensions/basket/utils.py b/ecommerce/extensions/basket/utils.py index 389169c0de8..1108ced84ef 100644 --- a/ecommerce/extensions/basket/utils.py +++ b/ecommerce/extensions/basket/utils.py @@ -19,9 +19,11 @@ from ecommerce.courses.utils import mode_for_product from ecommerce.extensions.analytics.utils import track_segment_event from ecommerce.extensions.basket.constants import ( + DYNAMIC_PAYMENT_METHODS_ENABLED, EMAIL_OPT_IN_ATTRIBUTE, ENABLE_STRIPE_PAYMENT_PROCESSOR, PAYMENT_INTENT_ID_ATTRIBUTE, + PAYMENT_INTENT_STATUS_ATTRIBUTE, PURCHASER_BEHALF_ATTRIBUTE, REDIRECT_WITH_WAFFLE_TESTING_QUERYSTRING ) @@ -394,6 +396,38 @@ def basket_add_organization_attribute(basket, request_data): ) +@newrelic.agent.function_trace() +def basket_add_payment_intent_status(basket, payment_intent_status): + payment_intent_status_attribute, __ = BasketAttributeType.objects.get_or_create( + name=PAYMENT_INTENT_STATUS_ATTRIBUTE) + # Do a get_or_create and update value_text after (instead of update_or_create) + # to prevent a particularly slow full table scan that uses a LIKE + basket_attribute, __ = BasketAttribute.objects.get_or_create( + attribute_type=payment_intent_status_attribute, + basket=basket, + ) + basket_attribute.value_text = payment_intent_status + basket_attribute.save() + + +@newrelic.agent.function_trace() +def basket_add_dynamic_payment_methods_enabled(basket, payment_intent): + """ + Adds a boolean value which is True if there is more than + 'card' payment method type in the Stripe Payment Intent. + """ + dynamic_payment_methods_enabled_attribute, __ = BasketAttributeType.objects.get_or_create( + name=DYNAMIC_PAYMENT_METHODS_ENABLED) + # Do a get_or_create and update value_text after (instead of update_or_create) + # to prevent a particularly slow full table scan that uses a LIKE + basket_attribute, __ = BasketAttribute.objects.get_or_create( + attribute_type=dynamic_payment_methods_enabled_attribute, + basket=basket, + ) + basket_attribute.value_text = len(payment_intent['payment_method_types']) > 1 + basket_attribute.save() + + @newrelic.agent.function_trace() def basket_add_payment_intent_id_attribute(basket, payment_intent_id): """ diff --git a/ecommerce/extensions/basket/views.py b/ecommerce/extensions/basket/views.py index 8ed49ad0462..0ce60aff07a 100644 --- a/ecommerce/extensions/basket/views.py +++ b/ecommerce/extensions/basket/views.py @@ -47,7 +47,12 @@ translate_basket_line_for_segment ) from ecommerce.extensions.basket import message_utils -from ecommerce.extensions.basket.constants import ENABLE_STRIPE_PAYMENT_PROCESSOR +from ecommerce.extensions.basket.constants import ( + DYNAMIC_PAYMENT_METHODS_ENABLED, + ENABLE_STRIPE_PAYMENT_PROCESSOR, + PAYMENT_INTENT_ID_ATTRIBUTE, + PAYMENT_INTENT_STATUS_ATTRIBUTE +) from ecommerce.extensions.basket.exceptions import BadRequestException, RedirectException, VoucherException from ecommerce.extensions.basket.utils import ( add_invalid_code_message_to_url, @@ -677,6 +682,9 @@ def _serialize_context(self, context, lines_data): self._add_offers(response) self._add_coupons(response, context) self._add_enable_stripe_payment_processor(response) + self._add_payment_intent_id(response, self.request.basket) + self._add_payment_intent_status(response, self.request.basket) + self._add_is_dynamic_payment_methods(response, self.request.basket) return response def _add_products(self, response, lines_data): @@ -739,6 +747,41 @@ def _add_enable_stripe_payment_processor(self, response): self.request, ENABLE_STRIPE_PAYMENT_PROCESSOR ) + def _add_payment_intent_id(self, response, basket): + try: + payment_intent_id_attribute = BasketAttributeType.objects.get(name=PAYMENT_INTENT_ID_ATTRIBUTE) + payment_intent_attr = BasketAttribute.objects.get( + basket=basket, + attribute_type=payment_intent_id_attribute + ) + response['payment_intent_id'] = payment_intent_attr.value_text.strip() + except (BasketAttribute.DoesNotExist, BasketAttributeType.DoesNotExist): + response['payment_intent_id'] = None + + def _add_payment_intent_status(self, response, basket): + try: + payment_intent_status_attribute = BasketAttributeType.objects.get(name=PAYMENT_INTENT_STATUS_ATTRIBUTE) + payment_intent_attr = BasketAttribute.objects.get( + basket=basket, + attribute_type=payment_intent_status_attribute + ) + response['payment_intent_status'] = payment_intent_attr.value_text.strip() + except (BasketAttribute.DoesNotExist, BasketAttributeType.DoesNotExist): + response['payment_intent_status'] = None + + def _add_is_dynamic_payment_methods(self, response, basket): + try: + dynamic_payment_methods_enabled_attribute = BasketAttributeType.objects.get( + name=DYNAMIC_PAYMENT_METHODS_ENABLED) + payment_intent_attr = BasketAttribute.objects.get( + basket=basket, + attribute_type=dynamic_payment_methods_enabled_attribute + ) + is_dynamic_payment_methods_enabled = payment_intent_attr.value_text.strip() + response['is_dynamic_payment_methods_enabled'] = is_dynamic_payment_methods_enabled == 'True' + except (BasketAttribute.DoesNotExist, BasketAttributeType.DoesNotExist): + response['is_dynamic_payment_methods_enabled'] = None + def _get_response_status(self, response): return message_utils.get_response_status(response['messages']) diff --git a/ecommerce/extensions/payment/processors/stripe.py b/ecommerce/extensions/payment/processors/stripe.py index 812b8d11f12..024d95cf33a 100644 --- a/ecommerce/extensions/payment/processors/stripe.py +++ b/ecommerce/extensions/payment/processors/stripe.py @@ -12,7 +12,9 @@ from ecommerce.extensions.basket.constants import PAYMENT_INTENT_ID_ATTRIBUTE from ecommerce.extensions.basket.models import Basket from ecommerce.extensions.basket.utils import ( + basket_add_dynamic_payment_methods_enabled, basket_add_payment_intent_id_attribute, + basket_add_payment_intent_status, get_basket_courses_list, get_billing_address_from_payment_intent_data ) @@ -118,15 +120,22 @@ def create_new_payment_intent_for_basket(self, basket, payment_intent_id): **self._build_payment_intent_parameters(basket), # This means this payment intent can only be confirmed with secret key (as in, from ecommerce) secret_key_confirmation='required', - # don't create a new intent for the same basket - idempotency_key=self.generate_basket_pi_idempotency_key(basket), # Enable dynamic payment methods, w/o payment method configuration ID due to Custom Actions Beta: # 'allow_redirects' is default to 'always', # 'enabled' is not default to True with CAB, only for Deferred Intents. automatic_payment_methods={'enabled': True}, ) new_payment_intent_id = new_payment_intent['id'] + logger.info( + 'Canceled Payment Intent [%s] and created new Payment Intent [%s] for basket [%d]', + payment_intent_id, + new_payment_intent_id, + basket.id, + ) + new_payment_intent_status = new_payment_intent['status'] basket_add_payment_intent_id_attribute(basket, new_payment_intent_id) + basket_add_payment_intent_status(basket, new_payment_intent_status) + basket_add_dynamic_payment_methods_enabled(basket, new_payment_intent) return new_payment_intent def generate_basket_pi_idempotency_key(self, basket): @@ -175,6 +184,8 @@ def get_capture_context(self, request): ) basket_add_payment_intent_id_attribute(basket, transaction_id) + basket_add_payment_intent_status(basket, stripe_response['status']) + basket_add_dynamic_payment_methods_enabled(basket, stripe_response) # for when basket was already created, but with different amount except stripe.error.IdempotencyError: # if this PI has been created before, we should be able to retrieve @@ -242,7 +253,7 @@ def handle_processor_response(self, response, basket=None): # Need to create a new one so the MFE can let the learner retry payment new_payment_intent = self.create_new_payment_intent_for_basket(confirm_api_response['id'], basket) logger.info( - 'Dynamic Payment Method received requires additional action for edX order %s, ' + 'Dynamic Payment Method received, requires additional action for edX order %s, ' 'created a new Payment Intent %s with status %s.', confirm_api_response['metadata']['order_number'], new_payment_intent['id'], diff --git a/ecommerce/extensions/payment/views/stripe.py b/ecommerce/extensions/payment/views/stripe.py index 70106de4518..98ab5a4edba 100644 --- a/ecommerce/extensions/payment/views/stripe.py +++ b/ecommerce/extensions/payment/views/stripe.py @@ -237,6 +237,14 @@ def post(self, request): # We'll only have a return value if the payment is a dynamic payment methods in progress if in_progress_payment: + logger.info( + 'Dynamic Payment Method received for edX order %s with basket %d, ' + 'returning Payment Intent %s with status %s to the payment MFE', + in_progress_payment.order_number, + in_progress_payment.basket_id, + in_progress_payment.transaction_id, + in_progress_payment.status, + ) return self.dynamic_payment_methods_response(in_progress_payment) try: billing_address = self.create_billing_address( @@ -298,13 +306,12 @@ def stripe_error_response(self, error): }, status=400) def dynamic_payment_methods_response(self, in_progress_payment): - """Tell the frontend the Payment Intent status and it will decide what to do.""" + """Tell the frontend the Payment Intent ID and status and it will decide what to do.""" if 'status' in in_progress_payment._fields: response_data = { 'status': in_progress_payment.status, 'confirmation_client_secret': in_progress_payment.confirmation_client_secret, 'payment_method': in_progress_payment.payment_method, - 'total': in_progress_payment.total, 'transaction_id': in_progress_payment.transaction_id, } return JsonResponse(response_data, status=201) From b097032d1e26f922c717b5027d07520020b12b6d Mon Sep 17 00:00:00 2001 From: julianajlk Date: Mon, 5 Feb 2024 15:33:01 -0500 Subject: [PATCH 04/11] test: Add test for in progress payment and modify fixture data --- .../extensions/basket/tests/test_views.py | 48 ++- .../test_stripe_test_payment_flow.json | 314 ++++++++++++++++++ .../payment/tests/views/test_stripe.py | 58 +++- 3 files changed, 413 insertions(+), 7 deletions(-) diff --git a/ecommerce/extensions/basket/tests/test_views.py b/ecommerce/extensions/basket/tests/test_views.py index cbf6c237982..1b6bb5faea9 100644 --- a/ecommerce/extensions/basket/tests/test_views.py +++ b/ecommerce/extensions/basket/tests/test_views.py @@ -36,7 +36,13 @@ from ecommerce.extensions.basket.constants import EMAIL_OPT_IN_ATTRIBUTE, ENABLE_STRIPE_PAYMENT_PROCESSOR from ecommerce.extensions.basket.tests.mixins import BasketMixin from ecommerce.extensions.basket.tests.test_utils import TEST_BUNDLE_ID -from ecommerce.extensions.basket.utils import _set_basket_bundle_status, apply_voucher_on_basket_and_check_discount +from ecommerce.extensions.basket.utils import ( + _set_basket_bundle_status, + apply_voucher_on_basket_and_check_discount, + basket_add_dynamic_payment_methods_enabled, + basket_add_payment_intent_id_attribute, + basket_add_payment_intent_status +) from ecommerce.extensions.catalogue.tests.mixins import DiscoveryTestMixin from ecommerce.extensions.offer.constants import DYNAMIC_DISCOUNT_FLAG from ecommerce.extensions.offer.utils import format_benefit_value @@ -325,6 +331,14 @@ def create_basket_and_add_product(self, product): basket.add_product(product, 1) return basket + def create_basket_and_add_product_stripe(self, product, payment_intent_status, payment_intent_id, payment_intent): + basket = self.create_empty_basket() + basket.add_product(product, 1) + basket_add_dynamic_payment_methods_enabled(basket, payment_intent) + basket_add_payment_intent_id_attribute(basket, payment_intent_id) + basket_add_payment_intent_status(basket, payment_intent_status) + return basket + def create_seat(self, course, seat_price=100, cert_type='verified'): return course.create_or_update_seat(cert_type, True, seat_price) @@ -404,6 +418,7 @@ def assert_expected_response( self, basket, enable_stripe_payment_processor=False, + is_dynamic_payment_methods_enabled=None, url=None, response=None, status_code=200, @@ -421,6 +436,8 @@ def assert_expected_response( subject=None, messages=None, summary_discounts=None, + payment_intent_id=None, + payment_intent_status=None, **kwargs ): if response is None: @@ -458,6 +475,7 @@ def assert_expected_response( 'basket_id': basket.id, 'currency': currency, 'enable_stripe_payment_processor': enable_stripe_payment_processor, + 'is_dynamic_payment_methods_enabled': is_dynamic_payment_methods_enabled, 'offers': offers, 'coupons': coupons, 'messages': messages if messages else [], @@ -466,6 +484,8 @@ def assert_expected_response( 'summary_discounts': summary_discounts, 'summary_price': summary_price, 'order_total': order_total, + 'payment_intent_id': payment_intent_id, + 'payment_intent_status': payment_intent_status, 'products': [ { 'product_type': product_type, @@ -712,6 +732,32 @@ def test_enable_stripe_payment_processor_flag(self, enable_stripe_payment_proces enable_stripe_payment_processor=enable_stripe_payment_processor, ) + def test_cart_with_stripe_data(self): + """ + For Dynamic Payment Methods, the basket will contain Payment Intent information. + Verify that the basket contains Payment Intent ID, Payment Intent status, and if DPM is enabled. + """ + payment_intent_status = 'requires_payment_method' + payment_intent_id = 'pi_3OqcQ5H4caH7G0X11y8NKNa4' + payment_intent = { + 'payment_method_types': [ + 'card', + 'klarna', + ] + } + seat = self.create_seat(self.course) + basket = self.create_basket_and_add_product_stripe( + seat, payment_intent_status, payment_intent_id, payment_intent + ) + response = self.client.get(self.path) + self.assert_expected_response( + basket, + response=response, + is_dynamic_payment_methods_enabled=len(payment_intent['payment_method_types']) > 1, + payment_intent_status=payment_intent_status, + payment_intent_id=payment_intent_id + ) + @responses.activate def test_enterprise_free_basket_redirect(self): """ diff --git a/ecommerce/extensions/payment/tests/views/fixtures/test_stripe_test_payment_flow.json b/ecommerce/extensions/payment/tests/views/fixtures/test_stripe_test_payment_flow.json index a220dabf98d..411ed622d8e 100644 --- a/ecommerce/extensions/payment/tests/views/fixtures/test_stripe_test_payment_flow.json +++ b/ecommerce/extensions/payment/tests/views/fixtures/test_stripe_test_payment_flow.json @@ -153,6 +153,133 @@ "transfer_data": null, "transfer_group": null }, + "confirm_resp_in_progress": { + "id": "pi_3LsftNIadiFyUl1x2TWxaADZ", + "object": "payment_intent", + "amount": 14900, + "amount_capturable": 0, + "amount_details": { + "tip": {} + }, + "amount_received": 0, + "application": null, + "application_fee_amount": null, + "automatic_payment_methods": { + "allow_redirects": "always", + "enabled": true + }, + "canceled_at": null, + "cancellation_reason": null, + "capture_method": "automatic", + "charges": { + "object": "list", + "data": [], + "has_more": false, + "total_count": 0, + "url": "/v1/charges?payment_intent=pi_3LsftNIadiFyUl1x2TWxaADZ" + }, + "client_secret": "pi_3LsftNIadiFyUl1x2TWxaADZ_secret_VxRx7Y1skyp0jKtq7Gdu80Xnh", + "confirmation_method": "automatic", + "created": 1665723185, + "currency": "usd", + "customer": null, + "description": "EDX-100011", + "invoice": null, + "last_payment_error": null, + "livemode": false, + "metadata": { + "order_number": "EDX-100011", + "courses": "[{'course_id': 'course-v1:edX+DemoX+Demo_Course', 'course_name': 'edX Demonstration Course'}]" + }, + "next_action": { + "redirect_to_url": { + "return_url": "http://localhost:1998", + "url": "https://pm-redirects.stripe.com/authorize/acct_1Ls7QSH4caH7G0X1/pa_nonce_PfyM6Ous6q0DWlvWSStpSEBAONfxwOL" + }, + "type": "redirect_to_url" + }, + "on_behalf_of": null, + "payment_method": { + "id": "pm_1OqcPrH4caH7G0X1YGu1IGJf", + "object": "payment_method", + "billing_details": { + "address": { + "city": "Beverly Hills", + "country": "US", + "line1": "Amsterdam Ave", + "line2": "Apt 214", + "postal_code": "10024", + "state": "NY" + }, + "email": "customer@email.us", + "name": "Test Person-us", + "phone": null + }, + "created": 1709562175, + "customer": null, + "klarna": { + }, + "livemode": false, + "metadata": { + }, + "type": "klarna" + }, + "payment_method_configuration_details": { + "id": "pmc_1LspDWH4caH7G0X1LXrN8QMJ", + "parent": null + }, + "payment_method_options": { + "affirm": { + }, + "afterpay_clearpay": { + "reference": null + }, + "card": { + "installments": null, + "mandate_options": null, + "network": null, + "request_three_d_secure": "automatic" + }, + "klarna": { + "preferred_locale": null + }, + "link": { + "persistent_token": null + } + }, + "payment_method_types": [ + "card", + "afterpay_clearpay", + "klarna", + "link", + "affirm" + ], + "processing": null, + "receipt_email": null, + "review": null, + "secret_key_confirmation": "required", + "setup_future_usage": null, + "shipping": { + "address": { + "city": "Beverly Hills", + "country": "US", + "line1": "Amsterdam Ave", + "line2": "Apt 214", + "postal_code": "10024", + "state": "NY" + }, + "carrier": null, + "name": "Test Person-us", + "phone": null, + "tracking_number": null + }, + "source": null, + "statement_descriptor": null, + "statement_descriptor_suffix": null, + "status": "requires_action", + "transfer_data": null, + "transfer_group": null + }, "create_resp": { "id": "pi_3LsftNIadiFyUl1x2TWxaADZ", "object": "payment_intent", @@ -208,6 +335,93 @@ "transfer_data": null, "transfer_group": null }, + "create_resp_in_progress": { + "id": "pi_3OqcQ5H4caH7G0X11y8NKNa4", + "object": "payment_intent", + "amount": 14900, + "amount_capturable": 0, + "amount_details": { + "tip": { + } + }, + "amount_received": 0, + "application": null, + "application_fee_amount": null, + "automatic_payment_methods": { + "allow_redirects": "always", + "enabled": true + }, + "canceled_at": null, + "cancellation_reason": null, + "capture_method": "automatic", + "charges": { + "object": "list", + "data": [ + ], + "has_more": false, + "total_count": 0, + "url": "/v1/charges?payment_intent=pi_3OqcQ5H4caH7G0X11y8NKNa4" + }, + "client_secret": "pi_3OqcQ5H4caH7G0X11y8NKNa4_secret_kbBQP5DZGLccIESMInIq5GECO", + "confirmation_method": "automatic", + "created": 1709562189, + "currency": "usd", + "customer": null, + "description": "EDX-100011", + "invoice": null, + "last_payment_error": null, + "latest_charge": null, + "livemode": false, + "metadata": { + "order_number": "EDX-100011", + "courses": "[{'course_id': 'course-v1:edX+DemoX+Demo_Course', 'course_name': 'edX Demonstration Course'}]" + }, + "next_action": null, + "on_behalf_of": null, + "payment_method": null, + "payment_method_configuration_details": { + "id": "pmc_1LspDWH4caH7G0X1LXrN8QMJ", + "parent": null + }, + "payment_method_options": { + "affirm": { + }, + "afterpay_clearpay": { + "reference": null + }, + "card": { + "installments": null, + "mandate_options": null, + "network": null, + "request_three_d_secure": "automatic" + }, + "klarna": { + "preferred_locale": null + }, + "link": { + "persistent_token": null + } + }, + "payment_method_types": [ + "card", + "afterpay_clearpay", + "klarna", + "link", + "affirm" + ], + "processing": null, + "receipt_email": null, + "review": null, + "secret_key_confirmation": "required", + "setup_future_usage": null, + "shipping": null, + "source": null, + "statement_descriptor": null, + "statement_descriptor_suffix": null, + "status": "requires_payment_method", + "transfer_data": null, + "transfer_group": null + }, "modify_resp": { "id": "pi_3LsftNIadiFyUl1x2TWxaADZ", "object": "payment_intent", @@ -263,6 +477,106 @@ "transfer_data": null, "transfer_group": null }, + "cancel_resp": { + "id": "pi_3LsftNIadiFyUl1x2TWxaADZ", + "object": "payment_intent", + "amount": 14900, + "amount_capturable": 0, + "amount_details": { + "tip": { + } + }, + "amount_received": 0, + "application": null, + "application_fee_amount": null, + "automatic_payment_methods": { + "allow_redirects": "always", + "enabled": true + }, + "canceled_at": 1709562188, + "cancellation_reason": null, + "capture_method": "automatic", + "charges": { + "object": "list", + "data": [ + ], + "has_more": false, + "total_count": 0, + "url": "/v1/charges?payment_intent=pi_3LsftNIadiFyUl1x2TWxaADZ" + }, + "client_secret": "pi_3LsftNIadiFyUl1x2TWxaADZ_secret_VxRx7Y1skyp0jKtq7Gdu80Xnh", + "confirmation_method": "automatic", + "created": 1709561818, + "currency": "usd", + "customer": null, + "description": "EDX-100011", + "invoice": null, + "last_payment_error": null, + "latest_charge": null, + "livemode": false, + "metadata": { + "order_number": "EDX-100011", + "courses": "[{'course_id': 'course-v1:edX+DemoX+Demo_Course', 'course_name': 'edX Demonstration Course'}]" + }, + "next_action": null, + "on_behalf_of": null, + "payment_method": "pm_1OqcPrH4caH7G0X1YGu1IGJf", + "payment_method_configuration_details": { + "id": "pmc_1LspDWH4caH7G0X1LXrN8QMJ", + "parent": null + }, + "payment_method_options": { + "affirm": { + }, + "afterpay_clearpay": { + "reference": null + }, + "card": { + "installments": null, + "mandate_options": null, + "network": null, + "request_three_d_secure": "automatic" + }, + "klarna": { + "preferred_locale": null + }, + "link": { + "persistent_token": null + } + }, + "payment_method_types": [ + "card", + "afterpay_clearpay", + "klarna", + "link", + "affirm" + ], + "processing": null, + "receipt_email": null, + "review": null, + "secret_key_confirmation": "required", + "setup_future_usage": null, + "shipping": { + "address": { + "city": "Beverly Hills", + "country": "US", + "line1": "Amsterdam Ave", + "line2": "Apt 214", + "postal_code": "10024", + "state": "NY" + }, + "carrier": null, + "name": "Test Person-us", + "phone": null, + "tracking_number": null + }, + "source": null, + "statement_descriptor": null, + "statement_descriptor_suffix": null, + "status": "canceled", + "transfer_data": null, + "transfer_group": null + }, "refund_resp": { "id": "re_3LsftNIadiFyUl1x2KvTM7FO", "object": "refund", diff --git a/ecommerce/extensions/payment/tests/views/test_stripe.py b/ecommerce/extensions/payment/tests/views/test_stripe.py index e5ecc91478a..52ac0120cf6 100644 --- a/ecommerce/extensions/payment/tests/views/test_stripe.py +++ b/ecommerce/extensions/payment/tests/views/test_stripe.py @@ -61,7 +61,8 @@ def payment_flow_with_mocked_stripe_calls( create_side_effect=None, retrieve_side_effect=None, confirm_side_effect=None, - modify_side_effect=None): + modify_side_effect=None, + in_progress_payment=None): """ Helper function to mock all stripe calls with successful responses. @@ -95,6 +96,8 @@ def payment_flow_with_mocked_stripe_calls( with mock.patch('stripe.PaymentIntent.confirm') as mock_confirm: if confirm_side_effect is not None: mock_confirm.side_effect = confirm_side_effect + elif in_progress_payment is not None: + mock_confirm.side_effect = [fixture_data['confirm_resp_in_progress']] else: mock_confirm.side_effect = [fixture_data['confirm_resp']] with mock.patch('stripe.PaymentIntent.modify') as mock_modify: @@ -102,11 +105,19 @@ def payment_flow_with_mocked_stripe_calls( mock_modify.side_effect = modify_side_effect else: mock_modify.side_effect = [fixture_data['modify_resp']] - # make your call - return self.client.post( - url, - data=data - ) + # If Payment Intent gets into 'requires_action' status without confirmation from the BNPL, + # we create a new Payment Intent for retry payment in the MFE + with mock.patch('stripe.PaymentIntent.cancel') as mock_cancel: + if in_progress_payment is not None: + mock_cancel.side_effect = [fixture_data['cancel_resp']] + with mock.patch('stripe.PaymentIntent.create') as mock_create: + if in_progress_payment is not None: + mock_create.side_effect = [fixture_data['create_resp_in_progress']] + # make your call + return self.client.post( + url, + data=data + ) def assert_successful_order_response(self, response, order_number): assert response.status_code == 201 @@ -157,8 +168,11 @@ def test_login_required(self): def test_payment_flow( self, confirm_resp, + confirm_resp_in_progress, # pylint: disable=unused-argument create_resp, + create_resp_in_progress, # pylint: disable=unused-argument modify_resp, + cancel_resp, # pylint: disable=unused-argument refund_resp, # pylint: disable=unused-argument retrieve_addr_resp): """ @@ -205,6 +219,7 @@ def test_payment_flow( data={ 'payment_intent_id': 'pi_3LsftNIadiFyUl1x2TWxaADZ', 'skus': basket.lines.first().stockrecord.partner_sku, + 'dynamic_payment_methods_enabled': False, }, ) assert mock_retrieve.call_count == 1 @@ -380,6 +395,7 @@ def test_payment_error_no_basket(self): data={ 'payment_intent_id': 'pi_3LsftNIadiFyUl1x2TWxaADZ', 'skus': '', + 'dynamic_payment_methods_enabled': False, }, ) assert response.status_code == 302 @@ -397,6 +413,7 @@ def test_payment_error_sku_mismatch(self): { 'payment_intent_id': 'pi_3LsftNIadiFyUl1x2TWxaADZ', 'skus': 'totally_the_wrong_sku', + 'dynamic_payment_methods_enabled': False, }, ) assert response.json() == {'sku_error': True} @@ -423,11 +440,38 @@ def test_payment_check_sdn_returns_hits(self): { 'payment_intent_id': 'pi_3LsftNIadiFyUl1x2TWxaADZ', 'skus': basket.lines.first().stockrecord.partner_sku, + 'dynamic_payment_methods_enabled': False, }, ) assert response.status_code == 400 assert response.json() == {'sdn_check_failure': {'hit_count': 1}} + def test_payment_handle_payment_intent_in_progress(self): + """ + Verify the POST endpoint handles a Payment Intent that is not succeeded yet, + with a 'requires_action' status without confirmation from the BNPL payment. + """ + basket = self.create_basket(product_class=SEAT_PRODUCT_CLASS_NAME) + + response = self.payment_flow_with_mocked_stripe_calls( + self.stripe_checkout_url, + { + 'payment_intent_id': 'pi_3LsftNIadiFyUl1x2TWxaADZ', + 'skus': basket.lines.first().stockrecord.partner_sku, + 'dynamic_payment_methods_enabled': True, + }, + in_progress_payment=True, + ) + + assert response.status_code == 201 + # A new Payment Intent was created for retry payment + assert response.json() == { + 'status': 'requires_payment_method', + 'transaction_id': 'pi_3OqcQ5H4caH7G0X11y8NKNa4', + 'confirmation_client_secret': 'pi_3OqcQ5H4caH7G0X11y8NKNa4_secret_kbBQP5DZGLccIESMInIq5GECO', + 'payment_method': None, + } + def test_handle_payment_fails_with_carderror(self): """ Verify handle payment failing with CardError returns correct error JSON. @@ -439,6 +483,7 @@ def test_handle_payment_fails_with_carderror(self): { 'payment_intent_id': 'pi_3LsftNIadiFyUl1x2TWxaADZ', 'skus': basket.lines.first().stockrecord.partner_sku, + 'dynamic_payment_methods_enabled': False, }, confirm_side_effect=stripe.error.CardError('Oops!', {}, 'card_declined'), ) @@ -459,6 +504,7 @@ def test_handle_payment_fails_with_unexpected_error(self): { 'payment_intent_id': 'pi_3LsftNIadiFyUl1x2TWxaADZ', 'skus': basket.lines.first().stockrecord.partner_sku, + 'dynamic_payment_methods_enabled': False, }, ) assert response.status_code == 400 From de9e9e6a14688f37981556b701b3aeb7f0bf1317 Mon Sep 17 00:00:00 2001 From: julianajlk Date: Tue, 26 Mar 2024 18:19:33 -0400 Subject: [PATCH 05/11] fix: Move create new PI to capture-context --- .../extensions/payment/processors/stripe.py | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/ecommerce/extensions/payment/processors/stripe.py b/ecommerce/extensions/payment/processors/stripe.py index 024d95cf33a..d786577bd22 100644 --- a/ecommerce/extensions/payment/processors/stripe.py +++ b/ecommerce/extensions/payment/processors/stripe.py @@ -177,7 +177,8 @@ def get_capture_context(self, request): transaction_id = stripe_response['id'] logger.info( - "Capture-context: succesfully created a Stripe Payment Intent [%s] for basket [%s] and order [%s]", + "Capture-context: succesfully created a Stripe Payment Intent [%s] " + "for basket [%s] and order [%s]", transaction_id, basket.id, basket.order_number @@ -186,6 +187,14 @@ def get_capture_context(self, request): basket_add_payment_intent_id_attribute(basket, transaction_id) basket_add_payment_intent_status(basket, stripe_response['status']) basket_add_dynamic_payment_methods_enabled(basket, stripe_response) + + # Check if payment intent is in unexpected state, ie. 'requires_action' + # If the user closes the DPM BNPL window, they are redirected back to payment MFE, + # and Stripe will change the status of the intent back to 'requires_payment_method'. + # This is here as an added protection against potential edge cases. + if stripe_response['status'] == 'requires_action': + stripe_response = self.create_new_payment_intent_for_basket(basket, transaction_id) + # for when basket was already created, but with different amount except stripe.error.IdempotencyError: # if this PI has been created before, we should be able to retrieve @@ -248,25 +257,14 @@ def handle_processor_response(self, response, basket=None): # If the payment has another status other than 'succeeded', we want to return to the MFE something it can handle if dynamic_payment_methods_enabled: if confirm_api_response['status'] == 'requires_action': - # The Payment Intent in this status will result in 'payment_intent_unexpected_state', since it cannot - # be confirmed again unless in 'requires_confirmation' or 'requires_payment_intent' status. - # Need to create a new one so the MFE can let the learner retry payment - new_payment_intent = self.create_new_payment_intent_for_basket(confirm_api_response['id'], basket) - logger.info( - 'Dynamic Payment Method received, requires additional action for edX order %s, ' - 'created a new Payment Intent %s with status %s.', - confirm_api_response['metadata']['order_number'], - new_payment_intent['id'], - new_payment_intent['status'], - ) return InProgressProcessorResponse( basket_id=basket.id, order_number=basket.order_number, - status=new_payment_intent['status'], - confirmation_client_secret=new_payment_intent['client_secret'], - transaction_id=new_payment_intent['id'], - payment_method=new_payment_intent['payment_method'], - total=new_payment_intent['amount'], + status=confirm_api_response['status'], + confirmation_client_secret=confirm_api_response['client_secret'], + transaction_id=confirm_api_response['id'], + payment_method=confirm_api_response['payment_method'], + total=confirm_api_response['amount'], ) # proceed only if payment went through From 66d1c96bed6c155b8fa7074dddd68822c7f6da14 Mon Sep 17 00:00:00 2001 From: julianajlk Date: Wed, 27 Mar 2024 17:34:14 -0400 Subject: [PATCH 06/11] test: Update tests move create new PI --- .../test_stripe_test_payment_flow.json | 8 +- .../payment/tests/views/test_stripe.py | 74 +++++++++++++------ 2 files changed, 57 insertions(+), 25 deletions(-) diff --git a/ecommerce/extensions/payment/tests/views/fixtures/test_stripe_test_payment_flow.json b/ecommerce/extensions/payment/tests/views/fixtures/test_stripe_test_payment_flow.json index 411ed622d8e..f2ec8a0f1d3 100644 --- a/ecommerce/extensions/payment/tests/views/fixtures/test_stripe_test_payment_flow.json +++ b/ecommerce/extensions/payment/tests/views/fixtures/test_stripe_test_payment_flow.json @@ -418,7 +418,7 @@ "source": null, "statement_descriptor": null, "statement_descriptor_suffix": null, - "status": "requires_payment_method", + "status": "requires_action", "transfer_data": null, "transfer_group": null }, @@ -478,7 +478,7 @@ "transfer_group": null }, "cancel_resp": { - "id": "pi_3LsftNIadiFyUl1x2TWxaADZ", + "id": "pi_3OqcQ5H4caH7G0X11y8NKNa4", "object": "payment_intent", "amount": 14900, "amount_capturable": 0, @@ -502,9 +502,9 @@ ], "has_more": false, "total_count": 0, - "url": "/v1/charges?payment_intent=pi_3LsftNIadiFyUl1x2TWxaADZ" + "url": "/v1/charges?payment_intent=pi_3OqcQ5H4caH7G0X11y8NKNa4" }, - "client_secret": "pi_3LsftNIadiFyUl1x2TWxaADZ_secret_VxRx7Y1skyp0jKtq7Gdu80Xnh", + "client_secret": "pi_3OqcQ5H4caH7G0X11y8NKNa4_secret_kbBQP5DZGLccIESMInIq5GECO", "confirmation_method": "automatic", "created": 1709561818, "currency": "usd", diff --git a/ecommerce/extensions/payment/tests/views/test_stripe.py b/ecommerce/extensions/payment/tests/views/test_stripe.py index 52ac0120cf6..e2165391ad8 100644 --- a/ecommerce/extensions/payment/tests/views/test_stripe.py +++ b/ecommerce/extensions/payment/tests/views/test_stripe.py @@ -16,6 +16,7 @@ ) from ecommerce.courses.tests.factories import CourseFactory from ecommerce.entitlements.utils import create_or_update_course_entitlement +from ecommerce.extensions.basket.constants import PAYMENT_INTENT_ID_ATTRIBUTE from ecommerce.extensions.checkout.utils import get_receipt_page_url from ecommerce.extensions.order.constants import PaymentEventTypeName from ecommerce.extensions.payment.constants import STRIPE_CARD_TYPE_MAP @@ -105,19 +106,11 @@ def payment_flow_with_mocked_stripe_calls( mock_modify.side_effect = modify_side_effect else: mock_modify.side_effect = [fixture_data['modify_resp']] - # If Payment Intent gets into 'requires_action' status without confirmation from the BNPL, - # we create a new Payment Intent for retry payment in the MFE - with mock.patch('stripe.PaymentIntent.cancel') as mock_cancel: - if in_progress_payment is not None: - mock_cancel.side_effect = [fixture_data['cancel_resp']] - with mock.patch('stripe.PaymentIntent.create') as mock_create: - if in_progress_payment is not None: - mock_create.side_effect = [fixture_data['create_resp_in_progress']] - # make your call - return self.client.post( - url, - data=data - ) + # make your call + return self.client.post( + url, + data=data + ) def assert_successful_order_response(self, response, order_number): assert response.status_code == 201 @@ -385,6 +378,49 @@ def test_capture_context_large_characters_basket(self): # The metadata must be less than 500 characters assert len(mock_create.call_args.kwargs['metadata']['courses']) < 500 + @file_data('fixtures/test_stripe_test_payment_flow.json') + def test_capture_context_in_progress_payment( + self, + confirm_resp, # pylint: disable=unused-argument + confirm_resp_in_progress, # pylint: disable=unused-argument + create_resp, + create_resp_in_progress, + modify_resp, # pylint: disable=unused-argument + cancel_resp, + refund_resp, # pylint: disable=unused-argument + retrieve_addr_resp): # pylint: disable=unused-argument + """ + Verify that hitting capture-context with a Payment Intent that already exists but it's + in 'requires_action' state, that a new Payment Intent is created and associated to the basket. + """ + basket = self.create_basket(product_class=SEAT_PRODUCT_CLASS_NAME) + + with mock.patch('stripe.PaymentIntent.create') as mock_create: + mock_create.return_value = create_resp_in_progress + # If Payment Intent gets into 'requires_action' status without confirmation from the BNPL, + # we create a new Payment Intent for retry payment in the MFE + with mock.patch('stripe.PaymentIntent.cancel') as mock_cancel: + mock_cancel.return_value = cancel_resp + with mock.patch('stripe.PaymentIntent.create') as mock_create_new: + mock_create_new.return_value = create_resp + response = self.client.get(self.capture_context_url) + + # Basket should have the new Payment Intent ID + mock_create_new.assert_called_once() + payment_intent_id = BasketAttribute.objects.get( + basket=basket, + attribute_type__name=PAYMENT_INTENT_ID_ATTRIBUTE + ).value_text + assert payment_intent_id == mock_create_new.return_value['id'] + + # Response should have the same order_number and new client secret + assert response.json() == { + 'capture_context': { + 'key_id': mock_create_new.return_value['client_secret'], + 'order_id': basket.order_number, + } + } + def test_payment_error_no_basket(self): """ Verify view redirects to error page if no basket exists for payment_intent_id. @@ -449,7 +485,7 @@ def test_payment_check_sdn_returns_hits(self): def test_payment_handle_payment_intent_in_progress(self): """ Verify the POST endpoint handles a Payment Intent that is not succeeded yet, - with a 'requires_action' status without confirmation from the BNPL payment. + with a 'requires_action' for a BNPL payment, which will be handled in the MFE. """ basket = self.create_basket(product_class=SEAT_PRODUCT_CLASS_NAME) @@ -464,13 +500,9 @@ def test_payment_handle_payment_intent_in_progress(self): ) assert response.status_code == 201 - # A new Payment Intent was created for retry payment - assert response.json() == { - 'status': 'requires_payment_method', - 'transaction_id': 'pi_3OqcQ5H4caH7G0X11y8NKNa4', - 'confirmation_client_secret': 'pi_3OqcQ5H4caH7G0X11y8NKNa4_secret_kbBQP5DZGLccIESMInIq5GECO', - 'payment_method': None, - } + # Should return 'requires_action' to the MFE with the same Payment Intent + assert response.json()['status'] == 'requires_action' + assert response.json()['transaction_id'] == 'pi_3LsftNIadiFyUl1x2TWxaADZ' def test_handle_payment_fails_with_carderror(self): """ From 7fb9e375dff3fb35db63798e65bc1e7acd86609c Mon Sep 17 00:00:00 2001 From: julianajlk Date: Thu, 28 Mar 2024 17:54:58 -0400 Subject: [PATCH 07/11] fix: Remove no longer needed PI status from basket attr --- ecommerce/extensions/basket/constants.py | 1 - ecommerce/extensions/basket/tests/test_views.py | 14 ++++---------- ecommerce/extensions/basket/utils.py | 15 --------------- ecommerce/extensions/basket/views.py | 15 +-------------- ecommerce/extensions/payment/processors/stripe.py | 8 ++------ 5 files changed, 7 insertions(+), 46 deletions(-) diff --git a/ecommerce/extensions/basket/constants.py b/ecommerce/extensions/basket/constants.py index 04715ca7eca..436071f33da 100644 --- a/ecommerce/extensions/basket/constants.py +++ b/ecommerce/extensions/basket/constants.py @@ -2,7 +2,6 @@ EMAIL_OPT_IN_ATTRIBUTE = "email_opt_in" PURCHASER_BEHALF_ATTRIBUTE = "purchased_for_organization" PAYMENT_INTENT_ID_ATTRIBUTE = "payment_intent_id" -PAYMENT_INTENT_STATUS_ATTRIBUTE = "payment_intent_status" DYNAMIC_PAYMENT_METHODS_ENABLED = "dynamic_payment_methods_enabled" # .. toggle_name: enable_stripe_payment_processor diff --git a/ecommerce/extensions/basket/tests/test_views.py b/ecommerce/extensions/basket/tests/test_views.py index 1b6bb5faea9..52a56f7306e 100644 --- a/ecommerce/extensions/basket/tests/test_views.py +++ b/ecommerce/extensions/basket/tests/test_views.py @@ -40,8 +40,7 @@ _set_basket_bundle_status, apply_voucher_on_basket_and_check_discount, basket_add_dynamic_payment_methods_enabled, - basket_add_payment_intent_id_attribute, - basket_add_payment_intent_status + basket_add_payment_intent_id_attribute ) from ecommerce.extensions.catalogue.tests.mixins import DiscoveryTestMixin from ecommerce.extensions.offer.constants import DYNAMIC_DISCOUNT_FLAG @@ -331,12 +330,11 @@ def create_basket_and_add_product(self, product): basket.add_product(product, 1) return basket - def create_basket_and_add_product_stripe(self, product, payment_intent_status, payment_intent_id, payment_intent): + def create_basket_and_add_product_stripe(self, product, payment_intent_id, payment_intent): basket = self.create_empty_basket() basket.add_product(product, 1) basket_add_dynamic_payment_methods_enabled(basket, payment_intent) basket_add_payment_intent_id_attribute(basket, payment_intent_id) - basket_add_payment_intent_status(basket, payment_intent_status) return basket def create_seat(self, course, seat_price=100, cert_type='verified'): @@ -437,7 +435,6 @@ def assert_expected_response( messages=None, summary_discounts=None, payment_intent_id=None, - payment_intent_status=None, **kwargs ): if response is None: @@ -485,7 +482,6 @@ def assert_expected_response( 'summary_price': summary_price, 'order_total': order_total, 'payment_intent_id': payment_intent_id, - 'payment_intent_status': payment_intent_status, 'products': [ { 'product_type': product_type, @@ -735,9 +731,8 @@ def test_enable_stripe_payment_processor_flag(self, enable_stripe_payment_proces def test_cart_with_stripe_data(self): """ For Dynamic Payment Methods, the basket will contain Payment Intent information. - Verify that the basket contains Payment Intent ID, Payment Intent status, and if DPM is enabled. + Verify that the basket contains Payment Intent ID and if DPM is enabled. """ - payment_intent_status = 'requires_payment_method' payment_intent_id = 'pi_3OqcQ5H4caH7G0X11y8NKNa4' payment_intent = { 'payment_method_types': [ @@ -747,14 +742,13 @@ def test_cart_with_stripe_data(self): } seat = self.create_seat(self.course) basket = self.create_basket_and_add_product_stripe( - seat, payment_intent_status, payment_intent_id, payment_intent + seat, payment_intent_id, payment_intent ) response = self.client.get(self.path) self.assert_expected_response( basket, response=response, is_dynamic_payment_methods_enabled=len(payment_intent['payment_method_types']) > 1, - payment_intent_status=payment_intent_status, payment_intent_id=payment_intent_id ) diff --git a/ecommerce/extensions/basket/utils.py b/ecommerce/extensions/basket/utils.py index 1108ced84ef..cd7669e05e1 100644 --- a/ecommerce/extensions/basket/utils.py +++ b/ecommerce/extensions/basket/utils.py @@ -23,7 +23,6 @@ EMAIL_OPT_IN_ATTRIBUTE, ENABLE_STRIPE_PAYMENT_PROCESSOR, PAYMENT_INTENT_ID_ATTRIBUTE, - PAYMENT_INTENT_STATUS_ATTRIBUTE, PURCHASER_BEHALF_ATTRIBUTE, REDIRECT_WITH_WAFFLE_TESTING_QUERYSTRING ) @@ -396,20 +395,6 @@ def basket_add_organization_attribute(basket, request_data): ) -@newrelic.agent.function_trace() -def basket_add_payment_intent_status(basket, payment_intent_status): - payment_intent_status_attribute, __ = BasketAttributeType.objects.get_or_create( - name=PAYMENT_INTENT_STATUS_ATTRIBUTE) - # Do a get_or_create and update value_text after (instead of update_or_create) - # to prevent a particularly slow full table scan that uses a LIKE - basket_attribute, __ = BasketAttribute.objects.get_or_create( - attribute_type=payment_intent_status_attribute, - basket=basket, - ) - basket_attribute.value_text = payment_intent_status - basket_attribute.save() - - @newrelic.agent.function_trace() def basket_add_dynamic_payment_methods_enabled(basket, payment_intent): """ diff --git a/ecommerce/extensions/basket/views.py b/ecommerce/extensions/basket/views.py index 0ce60aff07a..241655e789d 100644 --- a/ecommerce/extensions/basket/views.py +++ b/ecommerce/extensions/basket/views.py @@ -50,8 +50,7 @@ from ecommerce.extensions.basket.constants import ( DYNAMIC_PAYMENT_METHODS_ENABLED, ENABLE_STRIPE_PAYMENT_PROCESSOR, - PAYMENT_INTENT_ID_ATTRIBUTE, - PAYMENT_INTENT_STATUS_ATTRIBUTE + PAYMENT_INTENT_ID_ATTRIBUTE ) from ecommerce.extensions.basket.exceptions import BadRequestException, RedirectException, VoucherException from ecommerce.extensions.basket.utils import ( @@ -683,7 +682,6 @@ def _serialize_context(self, context, lines_data): self._add_coupons(response, context) self._add_enable_stripe_payment_processor(response) self._add_payment_intent_id(response, self.request.basket) - self._add_payment_intent_status(response, self.request.basket) self._add_is_dynamic_payment_methods(response, self.request.basket) return response @@ -758,17 +756,6 @@ def _add_payment_intent_id(self, response, basket): except (BasketAttribute.DoesNotExist, BasketAttributeType.DoesNotExist): response['payment_intent_id'] = None - def _add_payment_intent_status(self, response, basket): - try: - payment_intent_status_attribute = BasketAttributeType.objects.get(name=PAYMENT_INTENT_STATUS_ATTRIBUTE) - payment_intent_attr = BasketAttribute.objects.get( - basket=basket, - attribute_type=payment_intent_status_attribute - ) - response['payment_intent_status'] = payment_intent_attr.value_text.strip() - except (BasketAttribute.DoesNotExist, BasketAttributeType.DoesNotExist): - response['payment_intent_status'] = None - def _add_is_dynamic_payment_methods(self, response, basket): try: dynamic_payment_methods_enabled_attribute = BasketAttributeType.objects.get( diff --git a/ecommerce/extensions/payment/processors/stripe.py b/ecommerce/extensions/payment/processors/stripe.py index d786577bd22..745733f01b0 100644 --- a/ecommerce/extensions/payment/processors/stripe.py +++ b/ecommerce/extensions/payment/processors/stripe.py @@ -14,7 +14,6 @@ from ecommerce.extensions.basket.utils import ( basket_add_dynamic_payment_methods_enabled, basket_add_payment_intent_id_attribute, - basket_add_payment_intent_status, get_basket_courses_list, get_billing_address_from_payment_intent_data ) @@ -113,7 +112,7 @@ def create_new_payment_intent_for_basket(self, basket, payment_intent_id): This is used as a reset of the payment to allow payment retries when the intent gets into unexpected states. """ # Cancel existing Payment Intent - stripe.PaymentIntent.cancel(payment_intent_id) + cancelled_payment_intent = stripe.PaymentIntent.cancel(payment_intent_id) # Create a new Payment Intent and add to Basket new_payment_intent = stripe.PaymentIntent.create( @@ -128,13 +127,11 @@ def create_new_payment_intent_for_basket(self, basket, payment_intent_id): new_payment_intent_id = new_payment_intent['id'] logger.info( 'Canceled Payment Intent [%s] and created new Payment Intent [%s] for basket [%d]', - payment_intent_id, + cancelled_payment_intent['id'], new_payment_intent_id, basket.id, ) - new_payment_intent_status = new_payment_intent['status'] basket_add_payment_intent_id_attribute(basket, new_payment_intent_id) - basket_add_payment_intent_status(basket, new_payment_intent_status) basket_add_dynamic_payment_methods_enabled(basket, new_payment_intent) return new_payment_intent @@ -185,7 +182,6 @@ def get_capture_context(self, request): ) basket_add_payment_intent_id_attribute(basket, transaction_id) - basket_add_payment_intent_status(basket, stripe_response['status']) basket_add_dynamic_payment_methods_enabled(basket, stripe_response) # Check if payment intent is in unexpected state, ie. 'requires_action' From fb99ec47557ffb06e839f89adb833aeae05a2db0 Mon Sep 17 00:00:00 2001 From: julianajlk Date: Thu, 28 Mar 2024 22:34:01 -0400 Subject: [PATCH 08/11] refactor: Capture-context new PI check existing first bc of Stripe API --- .../extensions/payment/processors/stripe.py | 118 ++++++++++-------- 1 file changed, 66 insertions(+), 52 deletions(-) diff --git a/ecommerce/extensions/payment/processors/stripe.py b/ecommerce/extensions/payment/processors/stripe.py index 745733f01b0..8daa5dd0245 100644 --- a/ecommerce/extensions/payment/processors/stripe.py +++ b/ecommerce/extensions/payment/processors/stripe.py @@ -158,59 +158,73 @@ def get_capture_context(self, request): } else: try: - stripe_response = stripe.PaymentIntent.create( - **self._build_payment_intent_parameters(basket), - # This means this payment intent can only be confirmed with secret key (as in, from ecommerce) - secret_key_confirmation='required', - # don't create a new intent for the same basket - idempotency_key=self.generate_basket_pi_idempotency_key(basket), - # Enable dynamic payment methods, w/o payment method configuration ID due to Custom Actions Beta - # 'allow_redirects' is default to 'always' - # 'enabled' is not default to True with CAB, only for Deferred Intents - automatic_payment_methods={'enabled': True}, - ) - - # id is the payment_intent_id from Stripe - transaction_id = stripe_response['id'] - - logger.info( - "Capture-context: succesfully created a Stripe Payment Intent [%s] " - "for basket [%s] and order [%s]", - transaction_id, - basket.id, - basket.order_number - ) - - basket_add_payment_intent_id_attribute(basket, transaction_id) - basket_add_dynamic_payment_methods_enabled(basket, stripe_response) - - # Check if payment intent is in unexpected state, ie. 'requires_action' - # If the user closes the DPM BNPL window, they are redirected back to payment MFE, - # and Stripe will change the status of the intent back to 'requires_payment_method'. - # This is here as an added protection against potential edge cases. - if stripe_response['status'] == 'requires_action': - stripe_response = self.create_new_payment_intent_for_basket(basket, transaction_id) - - # for when basket was already created, but with different amount - except stripe.error.IdempotencyError: - # if this PI has been created before, we should be able to retrieve - # it from Stripe using the payment_intent_id BasketAttribute. - # Note that we update the PI's price in handle_processor_response - # before hitting the confirm endpoint, so we don't need to do that here - payment_intent_id_attribute = BasketAttributeType.objects.get(name=PAYMENT_INTENT_ID_ATTRIBUTE) - payment_intent_attr = BasketAttribute.objects.get( + # Check if payment intent is in unexpected state, ie. 'requires_action'. + # This check is here for the situation where a BNPL is not finalized in a window, + # but another window is opened and the checkout page is loaded. + # First need to check for the presence of a Payment Intent in the basket. + # We need to do this before creating a Payment Intent, even with the idempotency key + # because Stripe will change a 'requires_action' status to 'requires_payment_method' if + # we call create on it. To avoid that, we must check the status prior to calling create. + payment_intent_id = BasketAttribute.objects.get( basket=basket, - attribute_type=payment_intent_id_attribute - ) - transaction_id = payment_intent_attr.value_text.strip() - logger.info( - 'Idempotency Error: Retrieving existing Payment Intent for basket [%d]' - ' with transaction ID [%s] and order number [%s].', - basket.id, - transaction_id, - basket.order_number, - ) - stripe_response = stripe.PaymentIntent.retrieve(id=transaction_id) + attribute_type__name=PAYMENT_INTENT_ID_ATTRIBUTE + ).value_text + except BasketAttribute.DoesNotExist: + payment_intent_id = None + if payment_intent_id: + stripe_response = stripe.PaymentIntent.retrieve(id=payment_intent_id) + status = stripe_response['status'] + if status != 'requires_payment_method' or status != 'requires_confirmation': + # Payment Intent is not in a comfirmable status, must create a new one + stripe_response = self.create_new_payment_intent_for_basket(basket, payment_intent_id) + else: + try: + stripe_response = stripe.PaymentIntent.create( + **self._build_payment_intent_parameters(basket), + # This means this payment intent can only be confirmed with secret key (as in, from ecommerce) + secret_key_confirmation='required', + # don't create a new intent for the same basket + idempotency_key=self.generate_basket_pi_idempotency_key(basket), + # Enable dynamic payment methods, w/o payment method configuration ID due to Custom Actions Beta + # 'allow_redirects' is default to 'always' + # 'enabled' is not default to True with CAB, only for Deferred Intents + automatic_payment_methods={'enabled': True}, + ) + + # id is the payment_intent_id from Stripe + transaction_id = stripe_response['id'] + + logger.info( + "Capture-context: succesfully created a Stripe Payment Intent [%s] " + "for basket [%s] and order [%s]", + transaction_id, + basket.id, + basket.order_number + ) + + basket_add_payment_intent_id_attribute(basket, transaction_id) + basket_add_dynamic_payment_methods_enabled(basket, stripe_response) + + # for when basket was already created, but with different amount + except stripe.error.IdempotencyError: + # if this PI has been created before, we should be able to retrieve + # it from Stripe using the payment_intent_id BasketAttribute. + # Note that we update the PI's price in handle_processor_response + # before hitting the confirm endpoint, so we don't need to do that here + payment_intent_id_attribute = BasketAttributeType.objects.get(name=PAYMENT_INTENT_ID_ATTRIBUTE) + payment_intent_attr = BasketAttribute.objects.get( + basket=basket, + attribute_type=payment_intent_id_attribute + ) + transaction_id = payment_intent_attr.value_text.strip() + logger.info( + 'Idempotency Error: Retrieving existing Payment Intent for basket [%d]' + ' with transaction ID [%s] and order number [%s].', + basket.id, + transaction_id, + basket.order_number, + ) + stripe_response = stripe.PaymentIntent.retrieve(id=transaction_id) new_capture_context = { 'key_id': stripe_response['client_secret'], From f760e835bbaa85be9b8880ce109b6c5643c6105f Mon Sep 17 00:00:00 2001 From: julianajlk Date: Thu, 28 Mar 2024 23:12:58 -0400 Subject: [PATCH 09/11] test: Update test to reflect retrieve first on capture-context --- .../test_stripe_test_payment_flow.json | 174 +++++++++--------- .../payment/tests/views/test_stripe.py | 26 +-- 2 files changed, 102 insertions(+), 98 deletions(-) diff --git a/ecommerce/extensions/payment/tests/views/fixtures/test_stripe_test_payment_flow.json b/ecommerce/extensions/payment/tests/views/fixtures/test_stripe_test_payment_flow.json index f2ec8a0f1d3..9f586c760e1 100644 --- a/ecommerce/extensions/payment/tests/views/fixtures/test_stripe_test_payment_flow.json +++ b/ecommerce/extensions/payment/tests/views/fixtures/test_stripe_test_payment_flow.json @@ -335,93 +335,6 @@ "transfer_data": null, "transfer_group": null }, - "create_resp_in_progress": { - "id": "pi_3OqcQ5H4caH7G0X11y8NKNa4", - "object": "payment_intent", - "amount": 14900, - "amount_capturable": 0, - "amount_details": { - "tip": { - } - }, - "amount_received": 0, - "application": null, - "application_fee_amount": null, - "automatic_payment_methods": { - "allow_redirects": "always", - "enabled": true - }, - "canceled_at": null, - "cancellation_reason": null, - "capture_method": "automatic", - "charges": { - "object": "list", - "data": [ - ], - "has_more": false, - "total_count": 0, - "url": "/v1/charges?payment_intent=pi_3OqcQ5H4caH7G0X11y8NKNa4" - }, - "client_secret": "pi_3OqcQ5H4caH7G0X11y8NKNa4_secret_kbBQP5DZGLccIESMInIq5GECO", - "confirmation_method": "automatic", - "created": 1709562189, - "currency": "usd", - "customer": null, - "description": "EDX-100011", - "invoice": null, - "last_payment_error": null, - "latest_charge": null, - "livemode": false, - "metadata": { - "order_number": "EDX-100011", - "courses": "[{'course_id': 'course-v1:edX+DemoX+Demo_Course', 'course_name': 'edX Demonstration Course'}]" - }, - "next_action": null, - "on_behalf_of": null, - "payment_method": null, - "payment_method_configuration_details": { - "id": "pmc_1LspDWH4caH7G0X1LXrN8QMJ", - "parent": null - }, - "payment_method_options": { - "affirm": { - }, - "afterpay_clearpay": { - "reference": null - }, - "card": { - "installments": null, - "mandate_options": null, - "network": null, - "request_three_d_secure": "automatic" - }, - "klarna": { - "preferred_locale": null - }, - "link": { - "persistent_token": null - } - }, - "payment_method_types": [ - "card", - "afterpay_clearpay", - "klarna", - "link", - "affirm" - ], - "processing": null, - "receipt_email": null, - "review": null, - "secret_key_confirmation": "required", - "setup_future_usage": null, - "shipping": null, - "source": null, - "statement_descriptor": null, - "statement_descriptor_suffix": null, - "status": "requires_action", - "transfer_data": null, - "transfer_group": null - }, "modify_resp": { "id": "pi_3LsftNIadiFyUl1x2TWxaADZ", "object": "payment_intent", @@ -693,6 +606,93 @@ "status": "requires_confirmation", "transfer_data": null, "transfer_group": null + }, + "retrieve_resp_in_progress": { + "id": "pi_3OqcQ5H4caH7G0X11y8NKNa4", + "object": "payment_intent", + "amount": 14900, + "amount_capturable": 0, + "amount_details": { + "tip": { + } + }, + "amount_received": 0, + "application": null, + "application_fee_amount": null, + "automatic_payment_methods": { + "allow_redirects": "always", + "enabled": true + }, + "canceled_at": null, + "cancellation_reason": null, + "capture_method": "automatic", + "charges": { + "object": "list", + "data": [ + ], + "has_more": false, + "total_count": 0, + "url": "/v1/charges?payment_intent=pi_3OqcQ5H4caH7G0X11y8NKNa4" + }, + "client_secret": "pi_3OqcQ5H4caH7G0X11y8NKNa4_secret_kbBQP5DZGLccIESMInIq5GECO", + "confirmation_method": "automatic", + "created": 1709562189, + "currency": "usd", + "customer": null, + "description": "EDX-100011", + "invoice": null, + "last_payment_error": null, + "latest_charge": null, + "livemode": false, + "metadata": { + "order_number": "EDX-100011", + "courses": "[{'course_id': 'course-v1:edX+DemoX+Demo_Course', 'course_name': 'edX Demonstration Course'}]" + }, + "next_action": null, + "on_behalf_of": null, + "payment_method": null, + "payment_method_configuration_details": { + "id": "pmc_1LspDWH4caH7G0X1LXrN8QMJ", + "parent": null + }, + "payment_method_options": { + "affirm": { + }, + "afterpay_clearpay": { + "reference": null + }, + "card": { + "installments": null, + "mandate_options": null, + "network": null, + "request_three_d_secure": "automatic" + }, + "klarna": { + "preferred_locale": null + }, + "link": { + "persistent_token": null + } + }, + "payment_method_types": [ + "card", + "afterpay_clearpay", + "klarna", + "link", + "affirm" + ], + "processing": null, + "receipt_email": null, + "review": null, + "secret_key_confirmation": "required", + "setup_future_usage": null, + "shipping": null, + "source": null, + "statement_descriptor": null, + "statement_descriptor_suffix": null, + "status": "requires_action", + "transfer_data": null, + "transfer_group": null } } } diff --git a/ecommerce/extensions/payment/tests/views/test_stripe.py b/ecommerce/extensions/payment/tests/views/test_stripe.py index e2165391ad8..6c96388fbab 100644 --- a/ecommerce/extensions/payment/tests/views/test_stripe.py +++ b/ecommerce/extensions/payment/tests/views/test_stripe.py @@ -17,6 +17,7 @@ from ecommerce.courses.tests.factories import CourseFactory from ecommerce.entitlements.utils import create_or_update_course_entitlement from ecommerce.extensions.basket.constants import PAYMENT_INTENT_ID_ATTRIBUTE +from ecommerce.extensions.basket.utils import basket_add_payment_intent_id_attribute from ecommerce.extensions.checkout.utils import get_receipt_page_url from ecommerce.extensions.order.constants import PaymentEventTypeName from ecommerce.extensions.payment.constants import STRIPE_CARD_TYPE_MAP @@ -163,11 +164,11 @@ def test_payment_flow( confirm_resp, confirm_resp_in_progress, # pylint: disable=unused-argument create_resp, - create_resp_in_progress, # pylint: disable=unused-argument modify_resp, cancel_resp, # pylint: disable=unused-argument refund_resp, # pylint: disable=unused-argument - retrieve_addr_resp): + retrieve_addr_resp, + retrieve_resp_in_progress): # pylint: disable=unused-argument """ Verify that the stripe payment flow, hitting capture-context and stripe-checkout urls, results in a basket associated with the correct @@ -384,39 +385,42 @@ def test_capture_context_in_progress_payment( confirm_resp, # pylint: disable=unused-argument confirm_resp_in_progress, # pylint: disable=unused-argument create_resp, - create_resp_in_progress, modify_resp, # pylint: disable=unused-argument cancel_resp, refund_resp, # pylint: disable=unused-argument - retrieve_addr_resp): # pylint: disable=unused-argument + retrieve_addr_resp, # pylint: disable=unused-argument + retrieve_resp_in_progress): """ Verify that hitting capture-context with a Payment Intent that already exists but it's in 'requires_action' state, that a new Payment Intent is created and associated to the basket. """ basket = self.create_basket(product_class=SEAT_PRODUCT_CLASS_NAME) + payment_intent_id = retrieve_resp_in_progress['id'] + basket_add_payment_intent_id_attribute(basket, payment_intent_id) - with mock.patch('stripe.PaymentIntent.create') as mock_create: - mock_create.return_value = create_resp_in_progress + with mock.patch('stripe.PaymentIntent.retrieve') as mock_retrieve: + mock_retrieve.return_value = retrieve_resp_in_progress # If Payment Intent gets into 'requires_action' status without confirmation from the BNPL, # we create a new Payment Intent for retry payment in the MFE with mock.patch('stripe.PaymentIntent.cancel') as mock_cancel: mock_cancel.return_value = cancel_resp - with mock.patch('stripe.PaymentIntent.create') as mock_create_new: - mock_create_new.return_value = create_resp + with mock.patch('stripe.PaymentIntent.create') as mock_create: + mock_create.return_value = create_resp response = self.client.get(self.capture_context_url) # Basket should have the new Payment Intent ID - mock_create_new.assert_called_once() + mock_create.assert_called_once() + mock_cancel.assert_called_once() payment_intent_id = BasketAttribute.objects.get( basket=basket, attribute_type__name=PAYMENT_INTENT_ID_ATTRIBUTE ).value_text - assert payment_intent_id == mock_create_new.return_value['id'] + assert payment_intent_id == mock_create.return_value['id'] # Response should have the same order_number and new client secret assert response.json() == { 'capture_context': { - 'key_id': mock_create_new.return_value['client_secret'], + 'key_id': mock_create.return_value['client_secret'], 'order_id': basket.order_number, } } From d1994e0a8366a14951d2fcb6ed2475c99cc022e4 Mon Sep 17 00:00:00 2001 From: julianajlk Date: Tue, 9 Apr 2024 21:22:46 -0400 Subject: [PATCH 10/11] fix: Feedback edits and comments --- ecommerce/extensions/checkout/mixins.py | 1 - .../extensions/payment/processors/stripe.py | 20 ++++++++++++++----- ecommerce/extensions/payment/views/stripe.py | 13 ++++++------ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/ecommerce/extensions/checkout/mixins.py b/ecommerce/extensions/checkout/mixins.py index d583ba93c23..34db38266ec 100644 --- a/ecommerce/extensions/checkout/mixins.py +++ b/ecommerce/extensions/checkout/mixins.py @@ -127,7 +127,6 @@ def handle_payment(self, response, basket): # pylint: disable=arguments-differ, processor_response.transaction_id, processor_response.status, ) - # TODO: do we track this event if in progress? return processor_response # We only record successful payments in the database. self.record_payment(basket, processor_response) diff --git a/ecommerce/extensions/payment/processors/stripe.py b/ecommerce/extensions/payment/processors/stripe.py index 8daa5dd0245..cf2f2db7e3b 100644 --- a/ecommerce/extensions/payment/processors/stripe.py +++ b/ecommerce/extensions/payment/processors/stripe.py @@ -106,7 +106,7 @@ def _build_payment_intent_parameters(self, basket): }, } - def create_new_payment_intent_for_basket(self, basket, payment_intent_id): + def cancel_and_create_new_payment_intent_for_basket(self, basket, payment_intent_id): """ Create a new Stripe payment intent to associate with the current basket. This is used as a reset of the payment to allow payment retries when the intent gets into unexpected states. @@ -114,7 +114,9 @@ def create_new_payment_intent_for_basket(self, basket, payment_intent_id): # Cancel existing Payment Intent cancelled_payment_intent = stripe.PaymentIntent.cancel(payment_intent_id) - # Create a new Payment Intent and add to Basket + # Create a new Payment Intent and add to the basket. + # Pls note: idempotency_key is not used because we found that Stripe will "resuscitate" the + # Payment Intent and an intent with the same ID would be created - this will fail later on confirm. new_payment_intent = stripe.PaymentIntent.create( **self._build_payment_intent_parameters(basket), # This means this payment intent can only be confirmed with secret key (as in, from ecommerce) @@ -171,19 +173,27 @@ def get_capture_context(self, request): ).value_text except BasketAttribute.DoesNotExist: payment_intent_id = None + + # If the basket has a Payment Intent, get the status if payment_intent_id: stripe_response = stripe.PaymentIntent.retrieve(id=payment_intent_id) status = stripe_response['status'] if status != 'requires_payment_method' or status != 'requires_confirmation': - # Payment Intent is not in a comfirmable status, must create a new one - stripe_response = self.create_new_payment_intent_for_basket(basket, payment_intent_id) + # Payment Intent is in a non-confirmable status, must create a new one + stripe_response = self.cancel_and_create_new_payment_intent_for_basket(basket, payment_intent_id) + # If a Payment Intent exists in a confirmable status, it will skip the below else statement, + # aka not create another intent with the idempotency key this time around. + + # The basket does not have a Payment Intent, create one else: try: stripe_response = stripe.PaymentIntent.create( **self._build_payment_intent_parameters(basket), # This means this payment intent can only be confirmed with secret key (as in, from ecommerce) secret_key_confirmation='required', - # don't create a new intent for the same basket + # Use idempotency to not create a new intent for the same basket, as + # this endpoint is called every time the checkout page is loaded, otherwise + # it would create a new payment intent every time idempotency_key=self.generate_basket_pi_idempotency_key(basket), # Enable dynamic payment methods, w/o payment method configuration ID due to Custom Actions Beta # 'allow_redirects' is default to 'always' diff --git a/ecommerce/extensions/payment/views/stripe.py b/ecommerce/extensions/payment/views/stripe.py index 98ab5a4edba..a86362a38e9 100644 --- a/ecommerce/extensions/payment/views/stripe.py +++ b/ecommerce/extensions/payment/views/stripe.py @@ -307,11 +307,10 @@ def stripe_error_response(self, error): def dynamic_payment_methods_response(self, in_progress_payment): """Tell the frontend the Payment Intent ID and status and it will decide what to do.""" - if 'status' in in_progress_payment._fields: - response_data = { - 'status': in_progress_payment.status, - 'confirmation_client_secret': in_progress_payment.confirmation_client_secret, - 'payment_method': in_progress_payment.payment_method, - 'transaction_id': in_progress_payment.transaction_id, - } + response_data = { + 'status': in_progress_payment.status, + 'confirmation_client_secret': in_progress_payment.confirmation_client_secret, + 'payment_method': in_progress_payment.payment_method, + 'transaction_id': in_progress_payment.transaction_id, + } return JsonResponse(response_data, status=201) From 9e69195cca86ac95b32c97410b293b4417e8e483 Mon Sep 17 00:00:00 2001 From: julianajlk Date: Tue, 16 Apr 2024 14:23:51 -0400 Subject: [PATCH 11/11] fix: Add event property for DPM processor response and siteconfiguration --- ecommerce/extensions/payment/processors/webhooks.py | 7 +++++-- .../extensions/payment/tests/processors/test_webhooks.py | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/ecommerce/extensions/payment/processors/webhooks.py b/ecommerce/extensions/payment/processors/webhooks.py index dfb638c5da8..05086466a57 100644 --- a/ecommerce/extensions/payment/processors/webhooks.py +++ b/ecommerce/extensions/payment/processors/webhooks.py @@ -72,6 +72,9 @@ def handle_webhooks_payment(self, request, payment_intent, payment_method_type): Upon receipt of the payment_intent.succeeded event from Stripe, create an order, create a billing address, fulfill order, and save payment processor data. """ + # Adding the request site needed for getting the partner.short_code from siteconfiguration + self.site = request.site + # Get basket associated to the Payment Intent payment_intent_id = payment_intent['id'] order_number = payment_intent['description'] @@ -98,7 +101,7 @@ def handle_webhooks_payment(self, request, payment_intent, payment_method_type): try: self.record_processor_response(payment_intent, transaction_id=payment_intent_id, basket=basket) logger.info( - '[Dynamic Payment Methods] Successfully recorded Stripe payment intent [%s] ' + '[Dynamic Payment Methods] Successfully recorded Stripe response for payment intent [%s] ' 'for basket [%d] and order number [%s].', payment_intent_id, basket.id, @@ -120,7 +123,7 @@ def handle_webhooks_payment(self, request, payment_intent, payment_method_type): self.record_payment(basket, handled_processor_response) properties.update({'total': handled_processor_response.total, 'success': True, }) finally: - # TODO: Differentiate event from regular payments? + properties.update({'payment_method': payment_method_type}) track_segment_event(basket.site, basket.owner, 'Payment Processor Response', properties) # Create Billing Address diff --git a/ecommerce/extensions/payment/tests/processors/test_webhooks.py b/ecommerce/extensions/payment/tests/processors/test_webhooks.py index 246c42ef3fe..3203bc82350 100644 --- a/ecommerce/extensions/payment/tests/processors/test_webhooks.py +++ b/ecommerce/extensions/payment/tests/processors/test_webhooks.py @@ -161,7 +161,7 @@ def test_handle_webhooks_payment(self, mock_track, mock_retrieve, mock_handle_po }, } self.processor_class(self.site).handle_webhooks_payment( - self.request, succeeded_payment_intent, 'afterpay_clearpay' + self.request, succeeded_payment_intent, 'affirm' ) properties = { 'basket_id': self.basket.id, @@ -169,6 +169,7 @@ def test_handle_webhooks_payment(self, mock_track, mock_retrieve, mock_handle_po 'stripe_enabled': True, 'total': self.basket.total_incl_tax, 'success': True, + 'payment_method': succeeded_payment_intent['charges']['data'][0]['payment_method_details']['type'], } mock_track.assert_called_once_with( self.basket.site, self.basket.owner, 'Payment Processor Response', properties