Fix: retry command, feat: remove specific transaction for handle_payment and refactor: callback server and success

paygate/admin.py

@@ -25,7 +25,7 @@ def check_if_is_payed(self, request, queryset):
         for basket in queryset:
             site = basket.site
             paygate = PayGate(site)
-            success = paygate.send_callback_to_itself_to_retry(basket=basket)
+            success = paygate.send_callback_to_itself_to_retry(basket)
             if success:
                 self.message_user(
                     request,

paygate/management/commands/retry_baskets_payed_in_paygate.py

@@ -32,16 +32,16 @@ def add_arguments(self, parser):
         parser.add_argument(
             "--start",
             type=str,
-            help="The start date period to retry, incompatible with the --delta-in-minutes",
+            help="The start date period to retry, incompatible with the --delta_in_minutes",
         )
         parser.add_argument(
             "--end",
             type=str,
             default=None,
-            help="The end date period to retry, incompatible with the --delta-in-minutes",
+            help="The end date period to retry, incompatible with the --delta_in_minutes",
         )
         parser.add_argument(
-            "--delta-in-minutes",
+            "--delta_in_minutes",
             type=str,
             default=1440,  # 1 day
             help="The number of seconds to retry, default to last day",
@@ -57,12 +57,12 @@ def handle(self, *args, **kwargs):
             start = datetime.strptime(kwargs["start"], "%Y-%m-%d %H:%M:%S")
             end = datetime.strptime(kwargs["end"], "%Y-%m-%d %H:%M:%S")
         else:
-            delta_in_minutes = kwargs["delta-in-minutes"]
+            delta_in_minutes = int(kwargs["delta_in_minutes"])
             now = datetime.now()
             end = now
             start = now - timedelta(minutes=delta_in_minutes)
 
         site_domain = kwargs["site"]
-        site = Site.objects.filter(domain=site_domain)
+        site = Site.objects.filter(domain=site_domain).first()
         paygate = PayGate(site)
         paygate.retry_baskets_payed_in_paygate(start, end)

paygate/processors.py

@@ -14,7 +14,7 @@
 from django.urls import reverse
 from oscar.apps.payment.exceptions import GatewayError
 from oscar.core.loading import get_class, get_model
-from paygate.utils import get_basket, order_exist
+from paygate.utils import get_basket_from_payment_ref, order_exist
 
 from ecommerce.core.url_utils import get_ecommerce_url
 from ecommerce.extensions.payment.processors import (BasePaymentProcessor,
@@ -649,13 +649,7 @@ def retry_baskets_payed_in_paygate(
         # it should be a single item that have been payed
         for paygate_transaction in response_data:
             payment_ref = paygate_transaction.get("PAYMENT_REF")
-            basket_id = OrderNumberGenerator().basket_id(payment_ref)
-            if not basket_id:
-                logger.warning(
-                    "Can't reverse the basket_id from payment_ref=%s", payment_ref
-                )
-                continue
-            basket = get_basket(basket_id)
+            basket = get_basket_from_payment_ref(payment_ref)
             if not basket:
                 logger.warning("Can't find Basket for payment_ref=%s", payment_ref)
                 continue
@@ -667,7 +661,7 @@ def retry_baskets_payed_in_paygate(
             logger.info("Retrying callback server for payment_ref=%s", payment_ref)
 
             # call the callback server to retry
-            self.send_callback_to_itself_to_retry(payment_ref=payment_ref)
+            self.send_callback_to_itself_to_retry(basket)
 
         if len(response_data) == next_rows:
             self.retry_baskets_payed_in_paygate(
@@ -677,7 +671,7 @@ def retry_baskets_payed_in_paygate(
                 next_rows=next_rows,
             )
 
-    def send_callback_to_itself_to_retry(self, payment_ref=None, basket=None) -> bool:
+    def send_callback_to_itself_to_retry(self, basket) -> bool:
         """
         Send the success callback to itself.
         The callback will also call the PayGate to double check.
@@ -687,7 +681,7 @@ def send_callback_to_itself_to_retry(self, payment_ref=None, basket=None) -> boo
 
         logger.info("Sending callback to check if payment_ref=%s has been payed", payment_ref)
         # call the callback server to retry
-        ecommerce_callback_server = get_ecommerce_url(
+        ecommerce_callback_server = basket.site.siteconfiguration.build_ecommerce_url(
             reverse("ecommerce_plugin_paygate:callback_server")
         )
         request_data = {

paygate/utils.py

@@ -6,6 +6,7 @@
 Order = get_model("order", "Order")
 Basket = get_model("basket", "Basket")
 Applicator = get_class("offer.applicator", "Applicator")
+OrderNumberGenerator = get_class("order.utils", "OrderNumberGenerator")
 
 
 def order_exist(basket: Basket) -> bool:
@@ -25,7 +26,16 @@ def get_basket(basket_id, request=None):
         basket_id = int(basket_id)
         basket = Basket.objects.get(id=basket_id)
         basket.strategy = strategy.Selector().strategy()
-        Applicator().apply(basket, basket.owner, request=request)
+        if request:
+            Applicator().apply(basket, basket.owner, request=request)
         return basket
     except (ValueError, ObjectDoesNotExist):
         return None
+
+
+def get_basket_from_payment_ref(payment_ref):
+    """
+    Get the Django Oscar Basket from payment_ref used by PayGate.
+    """
+    basket_id = OrderNumberGenerator().basket_id(payment_ref)
+    return get_basket(basket_id)

paygate/views.py

@@ -17,14 +17,13 @@
 from django.views.generic import View
 from oscar.apps.payment.exceptions import PaymentError
 from oscar.core.loading import get_class, get_model
-from paygate.utils import get_basket
 
 from ecommerce.extensions.checkout.mixins import EdxOrderPlacementMixin
 from ecommerce.extensions.checkout.utils import get_receipt_page_url
 
 from .ip import allowed_client_ip, get_client_ip
 from .processors import PayGate
-from .utils import order_exist
+from .utils import get_basket_from_payment_ref, order_exist
 
 logger = logging.getLogger(__name__)
 
@@ -34,6 +33,10 @@
 PaymentProcessorResponse = get_model("payment", "PaymentProcessorResponse")
 
 
+class PayGateCallbackException(Exception):
+    pass
+
+
 class PayGateCallbackBaseResponseView(
     EdxOrderPlacementMixin, View, metaclass=abc.ABCMeta
 ):
@@ -85,20 +88,70 @@ def get_basket_and_record_response(self, request):
         basket = None
         ppr = None
 
-        transaction_id = paygate_response.get("payment_ref")
-        if transaction_id:
-            basket_id = OrderNumberGenerator().basket_id(transaction_id)
-            basket = get_basket(basket_id)
+        payment_ref = paygate_response.get("payment_ref")
+        if payment_ref:
+            basket = get_basket_from_payment_ref(payment_ref)
 
             ppr = self.payment_processor.record_processor_response(
                 paygate_response,
-                transaction_id=transaction_id,
+                transaction_id=payment_ref,
                 basket=basket,
             )
         else:
             logger.warning("Missing 'payment_ref' parameter from request")
         return basket, ppr
 
+    def handle_payment_and_create_order(self, request, basket, payment_processor_response):
+        """
+        Handle payment and if need create_order
+        """
+        if order_exist(basket):
+            # the basket already contains an order.
+            # we could receive duplicated server callbacks.
+            logger.warning(
+                "PayGate callback the basket already has an order for basket [%d]",
+                basket.id,
+            )
+            return False
+
+        try:
+            # This method have to be invoked in order to handle a payment,
+            # this method could raise an PaymentError exception.
+            self.handle_payment(payment_processor_response.response, basket)
+        except PaymentError as exc:
+            logger.exception(
+                "PayGate server callback error while handling payment with a payment error for basket [%d]",
+                basket.id,
+            )
+            raise PayGateCallbackException(
+                "Error while handling payment - payment error"
+            ) from exc
+        except Exception as exc:  # pylint: disable=broad-except
+            logger.exception(
+                "PayGate server callback error while handling payment with another error for basket [%d]",
+                basket.id,
+            )
+            logger.error(traceback.format_exc())
+            raise PayGateCallbackException("Error while handling payment - other error") from exc
+
+        # create an order for the basket
+        try:
+            order = self.create_order(request, basket)
+        except Exception as exc:  # pylint: disable=broad-except
+            logger.exception(
+                "PayGate server callback error while creating order for basket [%d]",
+                basket.id,
+            )
+            raise PayGateCallbackException("Error while creating order") from exc
+
+        # post order
+        try:
+            self.handle_post_order(order)
+        except Exception:  # pylint: disable=broad-except
+            self.log_order_placement_exception(basket.order_number, basket.id)
+
+        return True
+
 
 class PayGateCallbackServerResponseView(PayGateCallbackBaseResponseView):
     """
@@ -158,50 +211,9 @@ def post(self, request, *args, **kwargs):  # pylint: disable=unused-argument
             )
 
         try:
-            # Explicitly delimit operations which will be rolled back if an exception occurs.
-            with transaction.atomic():
-                # This method have to be invoked in order to handle a payment,
-                # this method could raise an PaymentError exception.
-                self.handle_payment(payment_processor_response.response, basket)
-        except PaymentError:
-            logger.exception(
-                "PayGate server callback error while handling payment with a payment error for basket [%d]",
-                basket.id,
-            )
-            return HttpResponseServerError(
-                "Error while handling payment - payment error"
-            )
-        except Exception:  # pylint: disable=broad-except
-            logger.exception(
-                "PayGate server callback error while handling payment with another error for basket [%d]",
-                basket.id,
-            )
-            logger.error(traceback.format_exc())
-            return HttpResponseServerError("Error while handling payment - other error")
-
-        # if the basket hasn't already contain an order, create one
-        if order_exist(basket):
-            # the basket already contains an order.
-            # we could receive duplicated server callbacks.
-            logger.warning(
-                "PayGate server callback the basket already has an order for basket [%d]",
-                basket.id,
-            )
-        else:
-            # create an order for the basket
-            try:
-                order = self.create_order(request, basket)
-            except Exception:  # pylint: disable=broad-except
-                logger.exception(
-                    "PayGate server callback error while creating order for basket [%d]",
-                    basket.id,
-                )
-                return HttpResponseServerError("Error while creating order")
-
-            try:
-                self.handle_post_order(order)
-            except Exception:  # pylint: disable=broad-except
-                self.log_order_placement_exception(basket.order_number, basket.id)
+            self.handle_payment_and_create_order(request, basket, payment_processor_response)
+        except PayGateCallbackException as exp:
+            return HttpResponseServerError(str(exp))
 
         return HttpResponse("Received server callback with success")
 
@@ -236,38 +248,10 @@ def get(
             disable_back_button=True,
         )
 
-        if not order_exist(basket):
-            # Received the frontend success callback before received the server-to-server callback
-
-            try:
-                # Explicitly delimit operations which will be rolled back if an exception occurs.
-                with transaction.atomic():
-                    # This method have to be invoked in order to handle a payment,
-                    # this method could raise an PaymentError exception.
-                    self.handle_payment(payment_processor_response.response, basket)
-            except PaymentError:
-                return redirect(self.payment_processor.error_url)
-            except Exception:  # pylint: disable=broad-except
-                logger.exception(
-                    "Attempts to handle payment for basket [%d] failed.", basket.id
-                )
-                logger.error(traceback.format_exc())
-                return redirect(receipt_url)
-
-            try:
-                order = self.create_order(request, basket)
-            except Exception:  # pylint: disable=broad-except
-                return redirect(receipt_url)
-
-            try:
-                self.handle_post_order(order)
-            except Exception:  # pylint: disable=broad-except
-                self.log_order_placement_exception(basket.order_number, basket.id)
-
-            return redirect(receipt_url)
-        # else
-        #   basked already has an order, ok the PayGate already has successfully called the server
-        #      callback.
+        try:
+            self.handle_payment_and_create_order(request, basket, payment_processor_response)
+        except PayGateCallbackException:
+            return redirect(self.payment_processor.error_url)
 
         return redirect(receipt_url)