Merge branch '3.17' of github.com:saleor/saleor into fix-transaction-…

…update-metadata-317

package.json

@@ -1,6 +1,6 @@
 {
   "name": "saleor",
-  "version": "3.17.40",
+  "version": "3.17.41",
   "engines": {
     "node": ">=16 <17",
     "npm": ">=7"

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "saleor"
-version = "3.17.40"
+version = "3.17.41"
 description = "A modular, high performance, headless e-commerce platform built with Python, GraphQL, Django, and React."
 authors = [ "Saleor Commerce <hello@saleor.io>" ]
 license = "BSD-3-Clause"

saleor/__init__.py

@@ -3,7 +3,7 @@
 from .celeryconf import app as celery_app
 
 __all__ = ["celery_app"]
-__version__ = "3.17.40"
+__version__ = "3.17.41"
 
 
 class PatchedSubscriberExecutionContext(object):

saleor/checkout/complete_checkout.py

@@ -116,25 +116,62 @@ def _process_voucher_data_for_order(checkout_info: "CheckoutInfo") -> dict:
     checkout = checkout_info.checkout
     voucher, code = get_voucher_for_checkout_info(checkout_info, with_lock=True)
 
-    if checkout.voucher_code and not voucher:
+    if code and not voucher:
         msg = "Voucher expired in meantime. Order placement aborted."
         raise NotApplicable(msg)
 
-    if not voucher:
+    if not voucher or not code:
         return {}
 
-    code = cast(VoucherCode, code)
-    if voucher.usage_limit:
-        increase_voucher_usage(voucher, code)
-    if voucher.apply_once_per_customer:
-        customer_email = cast(str, checkout_info.get_customer_email())
-        add_voucher_usage_by_customer(voucher, code, customer_email)
+    customer_email = cast(str, checkout_info.get_customer_email())
+
+    _increase_checkout_voucher_usage(checkout, code, voucher, customer_email)
     return {
         "voucher": voucher,
         "voucher_code": voucher.code,
     }
 
 
+@traced_atomic_transaction()
+def _increase_checkout_voucher_usage(
+    checkout: "Checkout",
+    voucher_code: "VoucherCode",
+    voucher: "Voucher",
+    customer_email: str,
+):
+    # Prevent race condition when two different threads are processing the same checkout
+    # with limited usage voucher assigned, both threads increasing the
+    # voucher usage which causing `NotApplicable` error for voucher.
+    if checkout.is_voucher_usage_increased or not voucher:
+        return
+
+    if voucher.usage_limit:
+        increase_voucher_usage(voucher, voucher_code)
+    if voucher.apply_once_per_customer:
+        add_voucher_usage_by_customer(voucher, voucher_code, customer_email)
+    checkout.is_voucher_usage_increased = True
+    checkout.save(update_fields=["is_voucher_usage_increased"])
+
+
+@traced_atomic_transaction()
+def _release_checkout_voucher_usage(
+    checkout: "Checkout",
+    voucher_code: Optional["VoucherCode"],
+    voucher: Optional["Voucher"],
+    user_email: Optional[str],
+):
+    if not checkout.is_voucher_usage_increased:
+        return
+
+    release_voucher_usage(
+        voucher,
+        voucher_code,
+        user_email,
+    )
+    checkout.is_voucher_usage_increased = False
+    checkout.save(update_fields=["is_voucher_usage_increased"])
+
+
 def _process_shipping_data_for_order(
     checkout_info: "CheckoutInfo",
     base_shipping_price: Money,
@@ -514,9 +551,10 @@ def _prepare_order_data(
     try:
         manager.preprocess_order_creation(checkout_info, lines)
     except TaxError:
-        release_voucher_usage(
-            checkout_info.voucher,
+        _release_checkout_voucher_usage(
+            checkout,
             checkout_info.voucher_code,
+            checkout_info.voucher,
             order_data.get("user_email"),
         )
         raise
@@ -805,6 +843,7 @@ def _get_order_data(
 
 
 def _process_payment(
+    checkout: Checkout,
     payment: Payment,
     customer_id: Optional[str],
     store_source: bool,
@@ -839,10 +878,8 @@ def _process_payment(
         if not txn.is_success:
             raise PaymentError(txn.error)
     except PaymentError as e:
-        release_voucher_usage(
-            voucher,
-            voucher_code,
-            order_data.get("user_email"),
+        _release_checkout_voucher_usage(
+            checkout, voucher_code, voucher, order_data.get("user_email")
         )
         raise ValidationError(str(e), code=CheckoutErrorCode.PAYMENT_ERROR.value)
     return txn
@@ -918,9 +955,10 @@ def complete_checkout_post_payment_part(
         action_required = txn.action_required
         if action_required:
             action_data = txn.action_required_data
-            release_voucher_usage(
-                checkout_info.voucher,
+            _release_checkout_voucher_usage(
+                checkout_info.checkout,
                 checkout_info.voucher_code,
+                checkout_info.voucher,
                 order_data.get("user_email"),
             )
 
@@ -941,9 +979,10 @@ def complete_checkout_post_payment_part(
             # remove checkout after order is successfully created
             checkout_info.checkout.delete()
         except InsufficientStock as e:
-            release_voucher_usage(
-                checkout_info.voucher,
+            _release_checkout_voucher_usage(
+                checkout_info.checkout,
                 checkout_info.voucher_code,
+                checkout_info.voucher,
                 order_data.get("user_email"),
             )
             gateway.payment_refund_or_void(
@@ -952,9 +991,10 @@ def complete_checkout_post_payment_part(
             error = prepare_insufficient_stock_checkout_validation_error(e)
             raise error
         except GiftCardNotApplicable as e:
-            release_voucher_usage(
-                checkout_info.voucher,
+            _release_checkout_voucher_usage(
+                checkout_info.checkout,
                 checkout_info.voucher_code,
+                checkout_info.voucher,
                 order_data.get("user_email"),
             )
             gateway.payment_refund_or_void(
@@ -987,19 +1027,14 @@ def _is_refund_ongoing(payment):
 def _increase_voucher_usage(checkout_info: "CheckoutInfo"):
     """Increase a voucher usage applied to the checkout."""
     voucher, code = get_voucher_for_checkout_info(checkout_info, with_lock=True)
-    if not voucher:
+    if not voucher or not code:
         return None
 
-    if voucher.apply_once_per_customer:
-        customer_email = cast(str, checkout_info.get_customer_email())
-        # type-ignore added as we ensure that voucher code instance is created
-        add_voucher_usage_by_customer(
-            voucher, code, customer_email  # type: ignore[arg-type]
-        )
+    customer_email = cast(str, checkout_info.get_customer_email())
 
-    if voucher.usage_limit:
-        # type-ignore added as we ensure that voucher code instance is created
-        increase_voucher_usage(voucher, code)  # type: ignore[arg-type]
+    checkout = checkout_info.checkout
+    _increase_checkout_voucher_usage(checkout, code, voucher, customer_email)
+    return code
 
 
 def _create_order_lines_from_checkout_lines(
@@ -1339,16 +1374,18 @@ def create_order_from_checkout(
                 checkout_info.checkout.delete()
             return order
         except InsufficientStock:
-            release_voucher_usage(
-                checkout_info.voucher,
+            _release_checkout_voucher_usage(
+                checkout,
                 checkout_info.voucher_code,
+                checkout_info.voucher,
                 checkout_info.checkout.get_customer_email(),
             )
             raise
         except GiftCardNotApplicable:
-            release_voucher_usage(
-                checkout_info.voucher,
+            _release_checkout_voucher_usage(
+                checkout,
                 checkout_info.voucher_code,
+                checkout_info.voucher,
                 checkout_info.checkout.get_customer_email(),
             )
             raise
@@ -1516,9 +1553,17 @@ def complete_checkout_with_payment(
     voucher_code = checkout_info.voucher_code
     if payment:
         with transaction_with_commit_on_errors():
-            Checkout.objects.select_for_update().filter(pk=checkout_pk).first()
+            checkout = (
+                Checkout.objects.select_for_update().filter(pk=checkout_pk).first()
+            )
+
+            if not checkout:
+                order = Order.objects.get_by_checkout_token(checkout_pk)
+                return order, False, {}
+
             payment = Payment.objects.select_for_update().get(id=payment.id)
             txn = _process_payment(
+                checkout=checkout,
                 payment=payment,
                 customer_id=customer_id,
                 store_source=store_source,

saleor/checkout/fetch.py

@@ -426,8 +426,8 @@ def fetch_checkout_info(
     if shipping_channel_listings is None:
         shipping_channel_listings = channel.shipping_method_listings.all()
 
-    voucher_code = None
     if not voucher:
+        voucher_code = None
         voucher, voucher_code = get_voucher_for_checkout(
             checkout, channel_slug=channel.slug
         )

saleor/checkout/migrations/0064_checkout_is_voucher_usage_increased.py

@@ -0,0 +1,25 @@
+# Generated by Django 3.2.25 on 2024-04-23 06:43
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("checkout", "0063_auto_20240402_1114"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="checkout",
+            name="is_voucher_usage_increased",
+            field=models.BooleanField(default=False),
+        ),
+        migrations.RunSQL(
+            """
+                ALTER TABLE checkout_checkout
+                ALTER COLUMN is_voucher_usage_increased
+                SET DEFAULT false;
+            """,
+            migrations.RunSQL.noop,
+        ),
+    ]

saleor/checkout/models.py

@@ -170,8 +170,13 @@ class Checkout(models.Model):
     discount_name = models.CharField(max_length=255, blank=True, null=True)
 
     translated_discount_name = models.CharField(max_length=255, blank=True, null=True)
-    voucher_code = models.CharField(max_length=255, blank=True, null=True)
     gift_cards = models.ManyToManyField(GiftCard, blank=True, related_name="checkouts")
+    voucher_code = models.CharField(max_length=255, blank=True, null=True)
+
+    # The field is to prevent race condition when two different threads are processing
+    # the same checkout with limited usage voucher assigned, both threads increasing the
+    # voucher usage which causing `NotApplicable` error for voucher
+    is_voucher_usage_increased = models.BooleanField(default=False)
 
     redirect_url = models.URLField(blank=True, null=True)
     tracking_code = models.CharField(max_length=255, blank=True, null=True)

saleor/checkout/tests/test_checkout.py

@@ -47,6 +47,7 @@
     get_checkout_metadata,
     get_external_shipping_id,
     get_voucher_discount_for_checkout,
+    get_voucher_for_checkout,
     get_voucher_for_checkout_info,
     is_fully_paid,
     recalculate_checkout_discount,
@@ -1150,6 +1151,60 @@ def test_get_voucher_for_checkout_info_no_voucher_code(checkout):
     assert code is None
 
 
+def test_get_voucher_for_checkout(checkout_with_voucher, voucher):
+    # given
+    checkout = checkout_with_voucher
+    voucher.usage_limit = 1
+    voucher.save(update_fields=["usage_limit"])
+
+    # when
+    checkout_voucher, _ = get_voucher_for_checkout(checkout, checkout.channel.slug)
+
+    # then
+    assert checkout_voucher == voucher
+
+
+def test_get_voucher_for_checkout_voucher_used(checkout_with_voucher, voucher):
+    # given
+    checkout = checkout_with_voucher
+    voucher.usage_limit = 1
+    voucher.used = 1
+    voucher.save(update_fields=["usage_limit", "used"])
+
+    code = voucher.codes.first()
+    code.used = 1
+    code.save(update_fields=["used"])
+
+    # when
+    checkout_voucher, _ = get_voucher_for_checkout(checkout, checkout.channel.slug)
+
+    # then
+    assert checkout_voucher is None
+
+
+def test_get_voucher_for_checkout_voucher_used_voucher_usage_already_increased(
+    checkout_with_voucher, voucher
+):
+    # given
+    checkout = checkout_with_voucher
+    checkout.is_voucher_usage_increased = True
+    checkout.save(update_fields=["is_voucher_usage_increased"])
+
+    voucher.usage_limit = 1
+    voucher.used = 1
+    voucher.save(update_fields=["usage_limit", "used"])
+
+    code = voucher.codes.first()
+    code.used = 1
+    code.save(update_fields=["used"])
+
+    # when
+    checkout_voucher, _ = get_voucher_for_checkout(checkout, checkout.channel.slug)
+
+    # then
+    assert checkout_voucher == voucher
+
+
 def test_remove_voucher_from_checkout(checkout_with_voucher, voucher_translation_fr):
     checkout = checkout_with_voucher
     remove_voucher_from_checkout(checkout)