From 4d53c9b5a5f6d26bf0e657b22b0f771ff34222ff Mon Sep 17 00:00:00 2001 From: mubbsharanwar Date: Fri, 13 Dec 2024 19:10:28 +0500 Subject: [PATCH] fix: address review comments --- .../apps/commercetools/catalog_info/constants.py | 2 +- .../apps/commercetools/catalog_info/edx_utils.py | 16 ++++++++++------ .../apps/commercetools/pipeline.py | 8 ++------ commerce_coordinator/apps/paypal/apps.py | 2 +- commerce_coordinator/apps/paypal/pipeline.py | 9 ++++----- .../apps/paypal/tests/test_pipeline.py | 5 ++--- commerce_coordinator/apps/stripe/pipeline.py | 10 +++------- commerce_coordinator/settings/base.py | 7 +++---- 8 files changed, 26 insertions(+), 33 deletions(-) diff --git a/commerce_coordinator/apps/commercetools/catalog_info/constants.py b/commerce_coordinator/apps/commercetools/catalog_info/constants.py index d49190ec..f02d31ae 100644 --- a/commerce_coordinator/apps/commercetools/catalog_info/constants.py +++ b/commerce_coordinator/apps/commercetools/catalog_info/constants.py @@ -78,7 +78,7 @@ class Languages: "directDiscounts[*]" ) -STRIPE_PAYMENT_STATUS_INTERFACE_CODE_SUCCEEDED = "succeeded" +PAYMENT_STATUS_INTERFACE_CODE_SUCCEEDED = "succeeded" EDX_STRIPE_PAYMENT_INTERFACE_NAME = "stripe_edx" diff --git a/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py b/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py index 3987f375..85378325 100644 --- a/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py +++ b/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py @@ -11,7 +11,7 @@ from commerce_coordinator.apps.commercetools.catalog_info.constants import ( EDX_STRIPE_PAYMENT_INTERFACE_NAME, - STRIPE_PAYMENT_STATUS_INTERFACE_CODE_SUCCEEDED, + PAYMENT_STATUS_INTERFACE_CODE_SUCCEEDED, EdXFieldNames, TwoUKeys ) @@ -51,7 +51,7 @@ def get_edx_lms_user_name(customer: CTCustomer): def get_edx_successful_stripe_payment(order: CTOrder) -> Union[CTPayment, None]: for pr in order.payment_info.payments: pmt = pr.obj - if pmt.payment_status.interface_code == STRIPE_PAYMENT_STATUS_INTERFACE_CODE_SUCCEEDED \ + if pmt.payment_status.interface_code == PAYMENT_STATUS_INTERFACE_CODE_SUCCEEDED \ and pmt.payment_method_info.payment_interface == EDX_STRIPE_PAYMENT_INTERFACE_NAME and \ pmt.interface_id: return pmt @@ -64,11 +64,15 @@ def get_edx_payment_intent_id(order: CTOrder) -> Union[str, None]: return pmt.interface_id return None - -def get_edx_payment_service_provider(order: CTOrder) -> Union[str, None]: +# TODO update get_edx_successful_stripe_payment to accommodate this util logic +# and replace it with that. +def get_edx_payment_info(order: CTOrder): for pr in order.payment_info.payments: - return pr.obj.payment_method_info.payment_interface - return None + pmt = pr.obj + if pmt.payment_status.interface_code == PAYMENT_STATUS_INTERFACE_CODE_SUCCEEDED \ + and pmt.interface_id: + return pmt.interface_id, pmt.payment_method_info.payment_interface + return None, None def get_edx_order_workflow_state_key(order: CTOrder) -> Optional[str]: diff --git a/commerce_coordinator/apps/commercetools/pipeline.py b/commerce_coordinator/apps/commercetools/pipeline.py index 30876f6b..1c67cab2 100644 --- a/commerce_coordinator/apps/commercetools/pipeline.py +++ b/commerce_coordinator/apps/commercetools/pipeline.py @@ -15,7 +15,7 @@ from commerce_coordinator.apps.commercetools.catalog_info.constants import EDX_STRIPE_PAYMENT_INTERFACE_NAME from commerce_coordinator.apps.commercetools.catalog_info.edx_utils import ( get_edx_payment_intent_id, - get_edx_payment_service_provider, + get_edx_payment_info, get_edx_refund_amount ) from commerce_coordinator.apps.commercetools.clients import CommercetoolsAPIClient @@ -110,11 +110,7 @@ def run_filter(self, active_order_management_system, order_number, **kwargs): # duration = (datetime.now() - start_time).total_seconds() log.info(f"[Performance Check] get_order_by_number call took {duration} seconds") - psp = get_edx_payment_service_provider(ct_order) - - intent_id = None - if psp == EDX_STRIPE_PAYMENT_INTERFACE_NAME: - intent_id = get_edx_payment_intent_id(ct_order) + intent_id, psp = get_edx_payment_info(ct_order) ret_val = { "order_data": ct_order, diff --git a/commerce_coordinator/apps/paypal/apps.py b/commerce_coordinator/apps/paypal/apps.py index 02594da0..4c5ff5e2 100644 --- a/commerce_coordinator/apps/paypal/apps.py +++ b/commerce_coordinator/apps/paypal/apps.py @@ -4,5 +4,5 @@ from django.apps import AppConfig -class PaypalConfig(AppConfig): +class PayPalConfig(AppConfig): name = 'commerce_coordinator.apps.paypal' diff --git a/commerce_coordinator/apps/paypal/pipeline.py b/commerce_coordinator/apps/paypal/pipeline.py index 27d04907..45befab7 100644 --- a/commerce_coordinator/apps/paypal/pipeline.py +++ b/commerce_coordinator/apps/paypal/pipeline.py @@ -16,13 +16,12 @@ class GetPayPalPaymentReceipt(PipelineStep): """ Purpare PayPal payment recipt """ - def run_filter(self, psp=None, **params): - if psp == EDX_PAYPAL_PAYMENT_INTERFACE_NAME: - base_url = settings.PAYPAL_BASE_URL - activities_url = settings.PAYPAL_USER_ACTIVITES_URL + def run_filter(self, psp, payment_intent_id, **params): + if payment_intent_id is None or psp != EDX_PAYPAL_PAYMENT_INTERFACE_NAME: + activity_page_url = settings.PAYPAL_USER_ACTIVITY_PAGE_URL query_params = {'free_text_search': params.get('order_number')} - redirect_url = urljoin(base_url, activities_url) + '?' + urlencode(query_params) + redirect_url = activity_page_url + '?' + urlencode(query_params) return { 'redirect_url': redirect_url, diff --git a/commerce_coordinator/apps/paypal/tests/test_pipeline.py b/commerce_coordinator/apps/paypal/tests/test_pipeline.py index 36ed5da2..f85d3f8e 100644 --- a/commerce_coordinator/apps/paypal/tests/test_pipeline.py +++ b/commerce_coordinator/apps/paypal/tests/test_pipeline.py @@ -11,8 +11,7 @@ class TestGetPayPalPaymentReceipt(TestCase): """A pytest Test case for the GetPayPalPaymentReceipt Pipeline Step""" @override_settings( - PAYPAL_BASE_URL="https://paypal.com/", - PAYPAL_USER_ACTIVITES_URL="myaccount/activities/" + PAYPAL_USER_ACTIVITY_PAGE_URL="https://paypal.com/myaccount/activities/" ) def test_pipeline_step(self): order_number = '123' @@ -23,5 +22,5 @@ def test_pipeline_step(self): psp='paypal_edx', order_number=order_number ) - redirect_url = f"{settings.PAYPAL_BASE_URL}{settings.PAYPAL_USER_ACTIVITES_URL}?free_text_search={order_number}" + redirect_url = f"{settings.PAYPAL_USER_ACTIVITY_PAGE_URL}?free_text_search={order_number}" self.assertEqual(redirect_url, result['redirect_url']) diff --git a/commerce_coordinator/apps/stripe/pipeline.py b/commerce_coordinator/apps/stripe/pipeline.py index ebfd74bc..22368ad5 100644 --- a/commerce_coordinator/apps/stripe/pipeline.py +++ b/commerce_coordinator/apps/stripe/pipeline.py @@ -215,14 +215,14 @@ class GetPaymentIntentReceipt(PipelineStep): """ Pull the receipt if the payment_intent is set """ # pylint: disable=unused-argument - def run_filter(self, payment_intent_id=None, psp=None, **params): + def run_filter(self, psp, payment_intent_id=None, **params): tag = type(self).__name__ - if psp == EDX_STRIPE_PAYMENT_INTERFACE_NAME and payment_intent_id is None: + if payment_intent_id is None: logger.debug(f'[{tag}] payment_intent_id not set, skipping.') return PipelineCommand.CONTINUE.value - elif psp == EDX_STRIPE_PAYMENT_INTERFACE_NAME: + if psp == EDX_STRIPE_PAYMENT_INTERFACE_NAME: stripe_api_client = StripeAPIClient() payment_intent = stripe_api_client.retrieve_payment_intent( payment_intent_id, @@ -234,10 +234,6 @@ def run_filter(self, payment_intent_id=None, psp=None, **params): 'payment_intent': payment_intent, 'redirect_url': receipt_url } - else: - return { - 'psp': psp - } class RefundPaymentIntent(PipelineStep): diff --git a/commerce_coordinator/settings/base.py b/commerce_coordinator/settings/base.py index 9a7dc2e2..11d985d2 100644 --- a/commerce_coordinator/settings/base.py +++ b/commerce_coordinator/settings/base.py @@ -53,7 +53,7 @@ def root(*path_fragments): 'commerce_coordinator.apps.frontend_app_payment.apps.FrontendAppPaymentConfig', 'commerce_coordinator.apps.lms.apps.LmsConfig', 'commerce_coordinator.apps.stripe.apps.StripeConfig', - 'commerce_coordinator.apps.paypal.apps.PaypalConfig', + 'commerce_coordinator.apps.paypal.apps.PayPalConfig', 'commerce_coordinator.apps.titan.apps.TitanConfig', 'commerce_coordinator.apps.commercetools', ) @@ -477,6 +477,5 @@ def root(*path_fragments): FAVICON_URL = "https://edx-cdn.org/v3/prod/favicon.ico" -# PAYPAL SETTINIS -PAYPAL_BASE_URL = "" -PAYPAL_USER_ACTIVITES_URL = "" +# PAYPAL SETTINGS +PAYPAL_USER_ACTIVITY_PAGE_URL = ""