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
@@ -65,10 +65,14 @@ def get_edx_payment_intent_id(order: CTOrder) -> Union[str, None]:
     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_info,
     get_edx_payment_intent_id,
-    get_edx_payment_service_provider,
     get_edx_refund_amount
 )
 from commerce_coordinator.apps.commercetools.clients import CommercetoolsAPIClient
@@ -110,26 +109,13 @@ 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,
                 "psp": psp,
                 "payment_intent_id": intent_id
             }
-            if intent_id:
-                ct_payment = ct_api_client.get_payment_by_key(intent_id)
-                ret_val['amount_in_cents'] = get_edx_refund_amount(ct_order)
-                ret_val['has_been_refunded'] = has_refund_transaction(ct_payment)
-                ret_val['payment_data'] = ct_payment
-            else:
-                ret_val['amount_in_cents'] = decimal.Decimal(0.00)
-                ret_val['has_been_refunded'] = False
-                ret_val['payment_data'] = None
 
             return ret_val
         except CommercetoolsError as err:  # pragma no cover

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,29 +3,32 @@
 """
 
 import logging
-from urllib.parse import urlencode, urljoin
+from urllib.parse import urlencode
 
 from django.conf import settings
 from openedx_filters import PipelineStep
 
 from commerce_coordinator.apps.commercetools.catalog_info.constants import EDX_PAYPAL_PAYMENT_INTERFACE_NAME
+from commerce_coordinator.apps.core.constants import PipelineCommand
 
 logger = logging.getLogger(__name__)
 
 
 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
-            query_params = {'free_text_search': params.get('order_number')}
+    def run_filter(self, psp=None, payment_intent_id=None, **params):
+        tag = type(self).__name__
 
-            redirect_url = urljoin(base_url, activities_url) + '?' + urlencode(query_params)
+        if payment_intent_id is None or psp != EDX_PAYPAL_PAYMENT_INTERFACE_NAME:
+            logger.debug(f'[{tag}] payment_intent_id not set, skipping.')
+            return PipelineCommand.CONTINUE.value
 
-            return {
-                'redirect_url': redirect_url,
-            }
+        activity_page_url = settings.PAYMENT_PROCESSOR_CONFIG['edx']['paypal']['user_activity_page_url']
+        query_params = {'free_text_search': params.get('order_number')}
 
-        return None
+        redirect_url = activity_page_url + '?' + urlencode(query_params)
+
+        return {
+            'redirect_url': redirect_url,
+        }

commerce_coordinator/apps/paypal/tests/test_pipeline.py

@@ -6,22 +6,25 @@
 
 from commerce_coordinator.apps.paypal.pipeline import GetPayPalPaymentReceipt
 
+TEST_PAYMENT_PROCESSOR_CONFIG = settings.PAYMENT_PROCESSOR_CONFIG
+ACTIVITY_URL = "https://paypal.com/myaccount/activities/"
+TEST_PAYMENT_PROCESSOR_CONFIG['edx']['paypal']['user_activity_page_url'] = ACTIVITY_URL
+
 
 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/"
-    )
+    @override_settings(PAYMENT_PROCESSOR_CONFIG=TEST_PAYMENT_PROCESSOR_CONFIG)
     def test_pipeline_step(self):
         order_number = '123'
         paypal_payment_pipe = GetPayPalPaymentReceipt("test_pipe", None)
 
         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}"
+        url = settings.PAYMENT_PROCESSOR_CONFIG['edx']['paypal']['user_activity_page_url']
+        redirect_url = f"{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=None, 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',
 )
@@ -431,6 +431,9 @@ def root(*path_fragments):
             'source_system_identifier': 'edx/commerce_coordinator?v=1',
             'webhook_endpoint_secret': STRIPE_WEBHOOK_ENDPOINT_SECRET,
         },
+        'paypal': {
+            'user_activity_page_url': '',
+        },
     },
 }
 # END PAYMENT PROCESSING
@@ -476,7 +479,3 @@ def root(*path_fragments):
 SEGMENT_KEY = None
 
 FAVICON_URL = "https://edx-cdn.org/v3/prod/favicon.ico"
-
-# PAYPAL SETTINIS
-PAYPAL_BASE_URL = ""
-PAYPAL_USER_ACTIVITES_URL = ""

commerce_coordinator/settings/local.py

@@ -144,6 +144,9 @@
             'source_system_identifier': 'edx/commerce_coordinator?v=1',
             'webhook_endpoint_secret': 'SET-ME-PLEASE',
         },
+        'paypal': {
+            'user_activity_page_url': 'https://paypal.com/myaccount/activities/',
+        },
     },
 }
 

commerce_coordinator/settings/test.py

@@ -15,6 +15,9 @@
             'source_system_identifier': 'edx/commerce_coordinator?v=1',
             'webhook_endpoint_secret': 'SET-ME-PLEASE',
         },
+        'paypal': {
+            'user_activity_page_url': 'https://paypal.com/myaccount/activities/',
+        },
     },
 }
 # END PAYMENT PROCESSING