Repository: django-oscar/django-oscar

Issue:
Error if create voucher name matching to voucher name deleted
Got error 500 when create voucher

<img width="844" alt="Screen Shot 2020-01-09 at 9 37 03 AM" src="https://user-images.githubusercontent.com/10126934/72033027-a49d3880-32c3-11ea-819e-cbc0c8a9bbde.png">

### Steps to Reproduce
1. Create new voucher
2. Delete that voucher
3. Create voucher again with same **voucher name** deleted




Discussion:

Fix patch:
diff --git a/src/oscar/apps/dashboard/vouchers/views.py b/src/oscar/apps/dashboard/vouchers/views.py
--- a/src/oscar/apps/dashboard/vouchers/views.py
+++ b/src/oscar/apps/dashboard/vouchers/views.py
@@ -10,6 +10,7 @@
 from django.utils.translation import gettext_lazy as _
 from django.views import generic
 
+from oscar.apps.voucher.utils import get_offer_name
 from oscar.core.loading import get_class, get_model
 from oscar.core.utils import slugify
 from oscar.views import sort_queryset
@@ -113,7 +114,7 @@ def form_valid(self, form):
         )
         name = form.cleaned_data['name']
         offer = ConditionalOffer.objects.create(
-            name=_("Offer for voucher '%s'") % name,
+            name=get_offer_name(name),
             offer_type=ConditionalOffer.VOUCHER,
             benefit=benefit,
             condition=condition,
@@ -199,6 +200,7 @@ def form_valid(self, form):
         offer.condition.save()
 
         offer.exclusive = form.cleaned_data['exclusive']
+        offer.name = get_offer_name(voucher.name)
         offer.save()
 
         benefit = voucher.benefit
@@ -253,7 +255,7 @@ def form_valid(self, form):
         )
         name = form.cleaned_data['name']
         offer = ConditionalOffer.objects.create(
-            name=_("Offer for voucher '%s'") % name,
+            name=get_offer_name(name),
             offer_type=ConditionalOffer.VOUCHER,
             benefit=benefit,
             condition=condition,
@@ -322,7 +324,7 @@ def form_valid(self, form):
             )
             name = form.cleaned_data['name']
             offer, __ = ConditionalOffer.objects.update_or_create(
-                name=_("Offer for voucher '%s'") % name,
+                name=get_offer_name(name),
                 defaults=dict(
                     offer_type=ConditionalOffer.VOUCHER,
                     benefit=benefit,
diff --git a/src/oscar/apps/voucher/apps.py b/src/oscar/apps/voucher/apps.py
--- a/src/oscar/apps/voucher/apps.py
+++ b/src/oscar/apps/voucher/apps.py
@@ -10,3 +10,4 @@ class VoucherConfig(OscarConfig):
 
     def ready(self):
         from . import receivers  # noqa
+        from . import signals  # noqa
diff --git a/src/oscar/apps/voucher/signals.py b/src/oscar/apps/voucher/signals.py
new file mode 100644
--- /dev/null
+++ b/src/oscar/apps/voucher/signals.py
@@ -0,0 +1,25 @@
+from django.db.models.signals import post_delete
+from django.dispatch import receiver
+
+from oscar.apps.voucher.utils import get_offer_name
+from oscar.core.loading import get_model
+
+Voucher = get_model('voucher', 'Voucher')
+ConditionalOffer = get_model('offer', 'ConditionalOffer')
+
+
+@receiver(post_delete, sender=Voucher)
+def delete_unused_related_conditional_offer(instance, **kwargs):
+    voucher = instance  # the object is no longer in the database
+
+    try:
+        conditional_offer = ConditionalOffer.objects.get(
+            name=get_offer_name(voucher.name),
+            offer_type=ConditionalOffer.VOUCHER
+        )
+    except (ConditionalOffer.DoesNotExist, ConditionalOffer.MultipleObjectsReturned):
+        pass
+    else:
+        # Only delete if not used by other vouchers
+        if not conditional_offer.vouchers.exists():
+            conditional_offer.delete()
diff --git a/src/oscar/apps/voucher/utils.py b/src/oscar/apps/voucher/utils.py
--- a/src/oscar/apps/voucher/utils.py
+++ b/src/oscar/apps/voucher/utils.py
@@ -2,6 +2,7 @@
 
 from django.db import connection
 from django.utils.crypto import get_random_string
+from django.utils.translation import gettext_lazy as _
 
 
 def generate_code(length, chars='ABCDEFGHJKLMNPQRSTUVWXYZ23456789',
@@ -33,3 +34,11 @@ def get_unused_code(length=12, group_length=4, separator='-'):
             "SELECT 1 FROM voucher_voucher WHERE code=%s", [code])
         if not cursor.fetchall():
             return code
+
+
+def get_offer_name(voucher_name):
+    """
+    Return the name used for the auto-generated offer created
+    when a voucher is created through the dashboard.
+    """
+    return _("Offer for voucher '%s'") % voucher_name


Test patch:
diff --git a/tests/integration/voucher/test_models.py b/tests/integration/voucher/test_models.py
--- a/tests/integration/voucher/test_models.py
+++ b/tests/integration/voucher/test_models.py
@@ -5,9 +5,11 @@
 from django.core import exceptions
 from django.test import TestCase
 from django.utils.timezone import utc
+from django.utils.translation import gettext_lazy as _
 
 from oscar.apps.voucher.models import Voucher
 from oscar.core.compat import get_user_model
+from oscar.core.loading import get_model
 from oscar.test.factories import (
     ConditionFactory, OrderFactory, RangeFactory, UserFactory, VoucherFactory,
     VoucherSetFactory, create_basket, create_offer, create_product)
@@ -15,6 +17,7 @@
 START_DATETIME = datetime.datetime(2011, 1, 1).replace(tzinfo=utc)
 END_DATETIME = datetime.datetime(2012, 1, 1).replace(tzinfo=utc)
 User = get_user_model()
+ConditionalOffer = get_model('offer', 'ConditionalOffer')
 
 
 class TestSavingAVoucher(TestCase):
@@ -98,6 +101,93 @@ def test_is_available_to_different_users(self):
             self.assertFalse(is_voucher_available_to_user)
 
 
+class TestVoucherDelete(TestCase):
+
+    def setUp(self):
+        product = create_product(price=100)
+        self.offer_range = RangeFactory(products=[product])
+        self.offer_condition = ConditionFactory(range=self.offer_range, value=2)
+
+    def test_related_offer_deleted(self):
+        # Voucher with offer name corresponding to it as used in the dashboard
+        voucher_name = "Voucher"
+        voucher = VoucherFactory(name=voucher_name, code="VOUCHER")
+        voucher.offers.add(
+            create_offer(
+                name=_("Offer for voucher '%s'") % voucher_name,
+                offer_type='Voucher',
+                range=self.offer_range,
+                condition=self.offer_condition
+            )
+        )
+
+        voucher.delete()
+        self.assertFalse(
+            ConditionalOffer.objects.filter(
+                name=_("Offer for voucher '%s'") % voucher_name,
+                offer_type=ConditionalOffer.VOUCHER
+            ).exists())
+
+    def test_related_offer_different_name_not_deleted(self):
+        # Voucher with offer named differently
+        voucher = VoucherFactory(name="Voucher", code="VOUCHER")
+        voucher.offers.add(
+            create_offer(
+                name="Different name test",
+                offer_type='Voucher',
+                range=self.offer_range,
+                condition=self.offer_condition
+            )
+        )
+
+        offer_ids = list(voucher.offers.all().values_list('pk', flat=True))
+
+        voucher.delete()
+        count_offers = ConditionalOffer.objects.filter(id__in=offer_ids).count()
+        assert len(offer_ids) == count_offers
+
+    def test_related_offer_different_type_not_deleted(self):
+        # Voucher with offer not of type "Voucher"
+        voucher_name = "Voucher"
+        voucher = VoucherFactory(name=voucher_name, code="VOUCHER")
+        voucher.offers.add(
+            create_offer(
+                name=_("Offer for voucher '%s'") % voucher_name,
+                offer_type='Site',
+                range=self.offer_range,
+                condition=self.offer_condition
+            )
+        )
+
+        offer_ids = list(voucher.offers.all().values_list('pk', flat=True))
+
+        voucher.delete()
+        count_offers = ConditionalOffer.objects.filter(id__in=offer_ids).count()
+        assert len(offer_ids) == count_offers
+
+    def test_multiple_related_offers_not_deleted(self):
+        # Voucher with already used offer
+        voucher_name = "Voucher 1"
+        offer = create_offer(
+            name=_("Offer for voucher '%s'") % voucher_name,
+            offer_type='Voucher',
+            range=self.offer_range,
+            condition=self.offer_condition
+        )
+
+        voucher1 = VoucherFactory(name=voucher_name, code="VOUCHER1")
+        voucher1.offers.add(offer)
+
+        voucher2 = VoucherFactory(name="Voucher 2", code="VOUCHER2")
+        voucher2.offers.add(offer)
+
+        offer_ids = list(voucher1.offers.all().values_list('pk', flat=True))
+
+        voucher1.delete()
+        count_offers = ConditionalOffer.objects.filter(id__in=offer_ids).count()
+        assert len(offer_ids) == count_offers
+
+
 class TestAvailableForBasket(TestCase):
 
     def setUp(self):