fix: address review comments

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"
 

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]:

commerce_coordinator/apps/commercetools/pipeline.py

@@ -12,10 +12,9 @@
 from openedx_filters.exceptions import OpenEdxFilterException
 from requests import HTTPError
 
-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 +109,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,

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'

commerce_coordinator/apps/paypal/pipeline.py

@@ -3,7 +3,7 @@
 """
 
 import logging
-from urllib.parse import urlencode, urljoin
+from urllib.parse import urlencode
 
 from django.conf import settings
 from openedx_filters import PipelineStep
@@ -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,

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'
@@ -21,7 +20,8 @@ def test_pipeline_step(self):
         result: dict = paypal_payment_pipe.run_filter(
             edx_lms_user_id=1,
             psp='paypal_edx',
+            payment_intent_id="00001",
             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'])

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,7 @@ def run_filter(self, payment_intent_id=None, psp=None, **params):
                 'payment_intent': payment_intent,
                 'redirect_url': receipt_url
             }
-        else:
-            return {
-                'psp': psp
-            }
+        return None
 
 
 class RefundPaymentIntent(PipelineStep):

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 = ""