Merge pull request #1035 from svthalia/pizza-payment-none

Delete payments when Payment.NONE in pizzas

website/events/services.py

@@ -7,7 +7,6 @@
 from events import emails
 from events.exceptions import RegistrationError
 from events.models import Registration, RegistrationInformationField, Event
-from payments.exceptions import PaymentError
 from payments.models import Payment
 from payments.services import create_payment, delete_payment
 from utils.snippets import datetime_to_lectureyear

website/payments/services.py

@@ -28,6 +28,10 @@ def create_payment(
         raise PaymentError(_("This user does not have Thalia Pay enabled"))
 
     if payable.payment is not None:
+        payable.payment.amount = payable.payment_amount
+        payable.payment.notes = payable.payment_notes
+        payable.payment.topic = payable.payment_topic
+        payable.payment.paid_by = payable.payment_payer
         payable.payment.type = pay_type
         payable.payment.save()
     else:

website/payments/tests/test_services.py

@@ -65,15 +65,15 @@ def test_create_payment(self):
             self.assertEqual(p.paid_by, self.member)
             self.assertEqual(p.processed_by, self.member)
             self.assertEqual(p.type, Payment.CASH)
-        with self.subTest("Does not create new payment if one already exists"):
+        with self.subTest("Updates payment if one already exists"):
             existing_payment = Payment(amount=2)
             p = services.create_payment(
                 MockPayable(payer=self.member, payment=existing_payment),
                 self.member,
                 Payment.CASH,
             )
             self.assertEqual(p, existing_payment)
-            self.assertEqual(p.amount, 2)
+            self.assertEqual(p.amount, 5)
         with self.subTest("Does not allow Thalia Pay when not enabled"):
             with self.assertRaises(PaymentError):
                 services.create_payment(

website/pizzas/api/serializers.py

@@ -69,24 +69,9 @@ def validate(self, attrs):
         return super().validate(attrs)
 
     def create(self, validated_data: Any) -> Any:
-        if "payment" in validated_data:
-            payment_type = validated_data["payment"].get("type", Payment.NONE)
-            del validated_data["payment"]
-        else:
-            payment_type = Payment.NONE
-
-        instance = super().create(validated_data)
-        instance.payment.type = payment_type
-        instance.payment.save()
-        return instance
+        del validated_data["payment"]
+        return super().create(validated_data)
 
     def update(self, instance: Model, validated_data: Any) -> Any:
-        if (
-            validated_data.get("payment", {}).get("type", instance.payment.type)
-            != instance.payment.type
-        ):
-            instance.payment.type = validated_data["payment"]["type"]
-            instance.payment.save()
-        if "payment" in validated_data:
-            del validated_data["payment"]
+        del validated_data["payment"]
         return super().update(instance, validated_data)

website/pizzas/api/viewsets.py

@@ -8,6 +8,7 @@
 from rest_framework.viewsets import GenericViewSet, ModelViewSet
 
 from payments.models import Payment
+from payments.services import delete_payment, create_payment
 from pizzas.api import serializers
 from pizzas.models import Product, PizzaEvent, Order
 from pizzas.services import can_change_order
@@ -52,9 +53,7 @@ def get_queryset(self):
                 return Order.objects.none()
 
             return Order.objects.filter(
-                member=self.request.member,
-                payment__type=Payment.NONE,
-                pizza_event=event,
+                member=self.request.member, payment=None, pizza_event=event,
             )
         return Order.objects.filter(member=self.request.member, pizza_event=event)
 
@@ -80,13 +79,39 @@ def perform_create(self, serializer):
             if serializer.validated_data.get("name"):
                 serializer.save(pizza_event=PizzaEvent.current())
             else:
-                if self.action.endswith("update") and can_change_order(
-                    self.request.member, PizzaEvent.current()
-                ):
-                    serializer.save(pizza_event=PizzaEvent.current())
+                if can_change_order(self.request.member, PizzaEvent.current()):
+                    order = serializer.save(pizza_event=PizzaEvent.current())
+                    if "payment" in serializer.validated_data:
+                        payment_type = serializer.validated_data["payment"]["type"]
+                    else:
+                        payment_type = Payment.NONE
+
+                    self._update_payment(order, payment_type, self.request.user)
                 else:
                     serializer.save(
                         member=self.request.member, pizza_event=PizzaEvent.current()
                     )
-        except IntegrityError:
-            raise ValidationError("Something went wrong when saving the order")
+        except IntegrityError as e:
+            raise ValidationError("Something went wrong when saving the order" + str(e))
+
+    def perform_update(self, serializer):
+        order = serializer.save()
+        if "payment" in serializer.validated_data and can_change_order(
+            self.request.member, PizzaEvent.current()
+        ):
+            self._update_payment(
+                order, serializer.validated_data["payment"]["type"], self.request.user,
+            )
+
+    @staticmethod
+    def _update_payment(order, payment_type=None, processed_by=None):
+        if order.payment and payment_type == Payment.NONE:
+            delete_payment(order)
+        elif order.payment:
+            if payment_type is None:
+                payment_type = order.payment.type
+            order.payment = create_payment(order, processed_by, payment_type)
+            order.payment.save()
+        else:
+            order.payment = create_payment(order, processed_by, payment_type)
+            order.save()

website/pizzas/migrations/0012_auto_20191116_2329.py

@@ -0,0 +1,19 @@
+# Generated by Django 2.2.6 on 2019-11-16 22:29
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('pizzas', '0011_auto_20191129_1634'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='order',
+            name='payment',
+            field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='pizzas_order', to='payments.Payment', verbose_name='payment'),
+        ),
+    ]

website/pizzas/models.py

@@ -9,7 +9,7 @@
 from events.models import Event
 import members
 from members.models import Member
-from payments.models import Payment
+from payments.models import Payment, Payable
 from pushnotifications.models import ScheduledMessage, Category
 from utils.translation import ModelTranslateMeta, MultilingualField
 
@@ -170,7 +170,7 @@ class Meta:
         permissions = (("order_restricted_products", _("Order restricted products")),)
 
 
-class Order(models.Model):
+class Order(models.Model, Payable):
     """Describes an order of an item during an event"""
 
     member = models.ForeignKey(
@@ -189,7 +189,9 @@ class Order(models.Model):
         verbose_name=_("payment"),
         to="payments.Payment",
         related_name="pizzas_order",
-        on_delete=models.CASCADE,
+        on_delete=models.PROTECT,
+        blank=True,
+        null=True,
     )
 
     product = models.ForeignKey(
@@ -200,6 +202,26 @@ class Order(models.Model):
         verbose_name=_("event"), to=PizzaEvent, on_delete=models.CASCADE
     )
 
+    @property
+    def payment_amount(self):
+        return self.product.price
+
+    @property
+    def payment_topic(self):
+        start_date = date(self.pizza_event.start, "Y-m-d")
+        return f"Pizzas {self.pizza_event.event.title_en} [{start_date}]"
+
+    @property
+    def payment_notes(self):
+        return (
+            f"Pizza order by {self.member_name} "
+            f"for {self.pizza_event.event.title_en}"
+        )
+
+    @property
+    def payment_payer(self):
+        return self.member
+
     def clean(self):
         if (self.member is None and not self.name) or (self.member and self.name):
             raise ValidationError(
@@ -209,38 +231,6 @@ def clean(self):
                 }
             )
 
-    def save(self, *args, **kwargs):
-        if not self.id and self.pizza_event.end_reminder:
-            self.pizza_event.end_reminder.users.remove(self.member)
-
-        notes = (
-            f"Pizza order by {self.member_name} "
-            f"for {self.pizza_event.event.title_en}"
-        )
-        try:
-            self.payment.notes = notes
-            self.payment.paid_by = self.member
-            self.payment.amount = self.product.price
-            self.payment.save()
-        except ObjectDoesNotExist:
-            self.payment = Payment.objects.create(
-                amount=self.product.price,
-                notes=notes,
-                paid_by=self.member,
-                topic=f"Pizzas {self.pizza_event.event.title_en} "
-                f'[{date(self.pizza_event.start, "Y-m-d")}]',
-            )
-
-        super().save(*args, **kwargs)
-
-    def delete(self, using=None, keep_parents=False):
-        if not self.id and self.pizza_event.end_reminder:
-            self.pizza_event.end_reminder.users.add(self.member)
-
-        payment = self.payment
-        super().delete(using, keep_parents)
-        payment.delete()
-
     @property
     def member_name(self):
         if self.member is not None:
@@ -263,12 +253,8 @@ def member_first_name(self):
     def can_be_changed(self):
         try:
             return (
-                self.payment
-                and not (
-                    self.payment.processed and not self.payment.type == Payment.TPAY
-                )
-                and not self.pizza_event.has_ended
-            )
+                not self.payment or self.payment.type == Payment.TPAY
+            ) and not self.pizza_event.has_ended
         except ObjectDoesNotExist:
             return False
 

website/pizzas/signals.py

@@ -1,5 +1,5 @@
 """The signals checked by the pizzas package"""
-from django.db.models.signals import post_save
+from django.db.models.signals import post_save, post_delete, pre_delete, pre_save
 
 from utils.models.signals import suspendingreceiver
 
@@ -19,3 +19,23 @@ def post_registration_save(sender, instance, **kwargs):
             event.pizzaevent.end_reminder.users.remove(instance.member)
         else:
             event.pizzaevent.end_reminder.users.add(instance.member)
+
+
+@suspendingreceiver(pre_save, sender="pizzas.Order", dispatch_uid="pizzas_order_save")
+def pre_order_save(sender, instance, **kwargs):
+    """Remove user from the order reminder when saved"""
+    if (
+        not instance.pk
+        and instance.pizza_event.end_reminder
+        and not instance.pizza_event.end_reminder.sent
+    ):
+        instance.pizza_event.end_reminder.users.remove(instance.member)
+
+
+@suspendingreceiver(
+    pre_delete, sender="pizzas.Order", dispatch_uid="pizzas_order_delete"
+)
+def pre_order_delete(sender, instance, **kwargs):
+    """Re-add user to reminder to on order deletion"""
+    if instance.pizza_event.end_reminder and not instance.pizza_event.end_reminder.sent:
+        instance.pizza_event.end_reminder.users.add(instance.member)

website/pizzas/templates/pizzas/admin/orders.html

@@ -106,16 +106,16 @@
                                 <td>
                                     <select name="payment"
                                             data-none="{{ payment.NONE }}"
-                                            {% if order.payment.type != payment.NONE %}class="paid"{% endif %}>
+                                            {% if order.payment is not None %}class="paid"{% endif %}>
                                         <option value="{{ payment.NONE }}"
-                                                {% if order.payment.type == payment.NONE %}selected{% endif %}>{% trans 'None' %}</option>
+                                                {% if order.payment is None %}selected{% endif %}>{% trans 'None' %}</option>
                                         <option value="{{ payment.CARD }}"
                                                 {% if order.payment.type == payment.CARD %}selected{% endif %}>{% trans 'Card' %}</option>
                                         <option value="{{ payment.CASH }}"
                                                 {% if order.payment.type == payment.CASH %}selected{% endif %}>{% trans 'Cash' %}</option>
                                         <option value="{{ payment.TPAY }}"
                                                 {% if order.payment.type == payment.TPAY %}selected{% endif %}
-                                                {% if not order.payment.paid_by.tpay_enabled %} disabled {% endif %}>{% trans 'Thalia Pay' %}</option>
+                                                {% if not order.member.tpay_enabled %} disabled {% endif %}>{% trans 'Thalia Pay' %}</option>
                                     </select>
                                 </td>
                                 <td>

website/pizzas/views.py

@@ -7,6 +7,7 @@
 from django.views.decorators.http import require_http_methods
 
 from payments.models import Payment
+from payments.services import create_payment
 from .models import Order, PizzaEvent, Product
 
 
@@ -34,8 +35,6 @@ def cancel_order(request):
             if not order.can_be_changed:
                 messages.error(request, _("You can no longer cancel."))
             elif order.member == request.member:
-                if order.payment.type == Payment.TPAY:
-                    order.payment.type = Payment.NONE
                 order.delete()
                 messages.success(request, _("Your order has been cancelled."))
         except Http404:
@@ -50,8 +49,7 @@ def pay_order(request):
         try:
             order = get_object_or_404(Order, pk=int(request.POST["order"]))
             if order.member == request.member:
-                order.payment.type = Payment.TPAY
-                order.save()
+                create_payment(order, Payment.TPAY, order.member)
                 messages.success(
                     request, _("Your order has been paid with " "Thalia Pay.")
                 )