From 59cde9163f3fc08d113e493b720b74720a781ca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=AD=C3=B0ir=20Valberg=20Gu=C3=B0mundsson?= Date: Wed, 27 Mar 2019 22:53:23 +0100 Subject: [PATCH] Use a modelformset for the order detail view to be able to validate stock on updates of quantities and payment of the order. --- src/profiles/templates/shop/order_detail.html | 32 ++--- src/shop/forms.py | 28 +++++ src/shop/models.py | 9 ++ src/shop/order_cleanup_worker.py | 30 ----- src/shop/templates/product_detail.html | 2 +- src/shop/templates/shop_index.html | 19 ++- src/shop/views.py | 115 ++++++++++-------- 7 files changed, 135 insertions(+), 100 deletions(-) delete mode 100644 src/shop/order_cleanup_worker.py diff --git a/src/profiles/templates/shop/order_detail.html b/src/profiles/templates/shop/order_detail.html index c659be04..7ad6491b 100644 --- a/src/profiles/templates/shop/order_detail.html +++ b/src/profiles/templates/shop/order_detail.html @@ -12,6 +12,7 @@ {% if not order.paid %}
{% csrf_token %} + {{ order_product_formset.management_form }} {% endif %} @@ -25,28 +26,29 @@ Price - {% for order_product in order.orderproductrelation_set.all %} + {% for form in order_product_formset %} + {{ form.id }}
Total +
- {{ order_product.product.name }} - - {% if not order.open == None %} - - {% bootstrap_button '' button_type="submit" button_class="btn-danger" name="remove_product" value=order_product.pk %} - {% else %} - {{ order_product.quantity }} + {{ form.instance.product.name }} + {% if form.instance.product.stock_amount %} +
{{ form.instance.product.left_in_stock }} left in stock {% endif %} 		- {{ order_product.product.price|currency }} + {% if not order.open == None %} + {% bootstrap_field form.quantity show_label=False %} + {% else %} + {{ form.instance.quantity }} + {% endif %} - {{ order_product.total|currency }} + {{ form.instance.product.price|currency }} + + {{ form.instance.total|currency }} + + {% bootstrap_button '' button_type="submit" button_class="btn-danger" name="remove_product" value=order_product.pk %} {% endfor %} diff --git a/src/shop/forms.py b/src/shop/forms.py index 3d8dcbcd..7ed44bcb 100644 --- a/src/shop/forms.py +++ b/src/shop/forms.py @@ -1,6 +1,34 @@ from django import forms +from django.forms import modelformset_factory + +from shop.models import OrderProductRelation class AddToOrderForm(forms.Form): quantity = forms.IntegerField(initial=1) + +class OrderProductRelationForm(forms.ModelForm): + class Meta: + model = OrderProductRelation + fields = ['quantity'] + + def clean_quantity(self): + product = self.instance.product + new_quantity = self.cleaned_data['quantity'] + + if product.left_in_stock < new_quantity: + raise forms.ValidationError( + "Only {} left in stock.".format( + product.left_in_stock, + ) + ) + + return new_quantity + + +OrderProductRelationFormSet = modelformset_factory( + OrderProductRelation, + form=OrderProductRelationForm, + extra=0 +) diff --git a/src/shop/models.py b/src/shop/models.py index 7ab42d04..c4f6f5c3 100644 --- a/src/shop/models.py +++ b/src/shop/models.py @@ -80,6 +80,8 @@ class Order(CreatedUpdatedModel): default=False, ) + # We are using a NullBooleanField here to ensure that we only have one open order per user at a time. + # This "hack" is possible since postgres treats null values as different, and thus we have database level integrity. open = models.NullBooleanField( verbose_name=_('Open?'), help_text=_('Whether this shop order is open or not. "None" means closed.'), @@ -416,8 +418,15 @@ class Product(CreatedUpdatedModel, UUIDModel): @property def left_in_stock(self): if self.stock_amount: + # All orders that are not open and not cancelled count towards what has + # been "reserved" from stock. + # + # This means that an order has either been paid (by card or blockchain) + # or is marked to be paid with cash or bank transfer, meaning it is a + # "reservation" of the product in question. sold = OrderProductRelation.objects.filter( product=self, + order__open=None, order__cancelled=False, ).aggregate(Sum('quantity'))['quantity__sum'] diff --git a/src/shop/order_cleanup_worker.py b/src/shop/order_cleanup_worker.py deleted file mode 100644 index ff4e94c5..00000000 --- a/src/shop/order_cleanup_worker.py +++ /dev/null @@ -1,30 +0,0 @@ -from dateutil import relativedelta -from django.conf import settings -from django.utils import timezone - -from shop.models import Order -from shop.email import add_order_cancelled_email - -import logging - -logging.basicConfig(level=logging.INFO) -logger = logging.getLogger('bornhack.%s' % __name__) - - -def do_work(): - """ - The order cleanup worker scans for orders that are still open - but are older than ORDER_TTL, and marks those as closed. - """ - - time_threshold = timezone.now() - relativedelta.relativedelta(**{settings.ORDER_TTL_UNIT: settings.ORDER_TTL}) - - orders_to_delete = Order.objects.filter(open=True, cancelled=False, created__lt=time_threshold) - - for order in orders_to_delete: - logger.info( - "Cancelling order %s since it has been open for more than %s %s" % - (order.pk, settings.ORDER_TTL, settings.ORDER_TTL_UNIT) - ) - order.mark_as_cancelled() - add_order_cancelled_email(order) diff --git a/src/shop/templates/product_detail.html b/src/shop/templates/product_detail.html index 87d77ab9..34fc50c2 100644 --- a/src/shop/templates/product_detail.html +++ b/src/shop/templates/product_detail.html @@ -31,7 +31,7 @@ {% if product.left_in_stock > 0 %} {{ product.left_in_stock }} available
{% else %} - Sold out. + Sold out.
{% endif %} diff --git a/src/shop/templates/shop_index.html b/src/shop/templates/shop_index.html index d7b9e072..1c67d61f 100644 --- a/src/shop/templates/shop_index.html +++ b/src/shop/templates/shop_index.html @@ -55,15 +55,24 @@ Shop | {{ block.super }} {{ product.name }} + + {% if product.stock_amount %} -
- {% if product.left_in_stock == 0 %} - Sold out! - {% elif product.left_in_stock <= 10 %} + + {% if product.left_in_stock <= 10 %} +
Only {{ product.left_in_stock }} left! - {% endif %}
{% endif %} + + {% if product.left_in_stock == 0 %} +
+ Sold out! +
+ {% endif %} + + {% endif %} +
diff --git a/src/shop/views.py b/src/shop/views.py index 1c97e9c6..18fda6a3 100644 --- a/src/shop/views.py +++ b/src/shop/views.py @@ -1,7 +1,9 @@ +import logging +from collections import OrderedDict + from django.conf import settings from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin -from django.urls import reverse, reverse_lazy from django.db.models import Count, F from django.http import ( HttpResponse, @@ -10,6 +12,10 @@ from django.http import ( Http404 ) from django.shortcuts import get_object_or_404 +from django.urls import reverse, reverse_lazy +from django.utils import timezone +from django.utils.decorators import method_decorator +from django.views.decorators.csrf import csrf_exempt from django.views.generic import ( View, ListView, @@ -18,9 +24,6 @@ from django.views.generic import ( ) from django.views.generic.base import RedirectView from django.views.generic.detail import SingleObjectMixin -from django.utils.decorators import method_decorator -from django.views.decorators.csrf import csrf_exempt -from django.utils import timezone from shop.models import ( Order, @@ -31,16 +34,15 @@ from shop.models import ( EpayPayment, CreditNote, ) -from .forms import AddToOrderForm -from .epay import calculate_epay_hash, validate_epay_callback -from collections import OrderedDict from vendor.coinify.coinify_callback import CoinifyCallback from .coinify import ( create_coinify_invoice, save_coinify_callback, process_coinify_invoice_json ) -import logging +from .epay import calculate_epay_hash, validate_epay_callback +from .forms import AddToOrderForm, OrderProductRelationFormSet + logger = logging.getLogger("bornhack.%s" % __name__) @@ -291,10 +293,63 @@ class OrderDetailView(LoginRequiredMixin, EnsureUserOwnsOrderMixin, EnsureOrderH template_name = 'shop/order_detail.html' context_object_name = 'order' - def post(self, request, *args, **kwargs): - order = self.get_object() - payment_method = request.POST.get('payment_method') + def get_context_data(self, **kwargs): + if 'order_product_formset' not in kwargs: + kwargs['order_product_formset'] = OrderProductRelationFormSet( + queryset=OrderProductRelation.objects.filter(order=self.get_object()), + ) + return super().get_context_data(**kwargs) + + def post(self, request, *args, **kwargs): + self.object = self.get_object() + order = self.object + + # First check if the user is removing a product from the order. + product_remove = request.POST.get('remove_product') + if product_remove: + order.orderproductrelation_set.filter(pk=product_remove).delete() + if not order.products.count() > 0: + order.mark_as_cancelled() + messages.info(request, 'Order cancelled!') + return HttpResponseRedirect(reverse_lazy('shop:index')) + + # Then see if the user is cancelling the order. + if 'cancel_order' in request.POST: + order.mark_as_cancelled() + messages.info(request, 'Order cancelled!') + return HttpResponseRedirect(reverse_lazy('shop:index')) + + # The user is not removing products or cancelling the order, + # so from now on we do stuff that require us to check stock. + # We use a formset for this to be able to display exactly + # which product is not in stock if that is the case. + formset = OrderProductRelationFormSet( + request.POST, + queryset=OrderProductRelation.objects.filter(order=order), + ) + + # If the formset is not valid it means that we cannot fulfill the order, so return and inform the user. + if not formset.is_valid(): + messages.error( + request, + "Some of the products you are ordering are out of stock. Review the order and try again." + ) + return self.render_to_response( + self.get_context_data(order_product_formset=formset) + ) + + # No stock issues, proceed to check if the user is updating the order. + if 'update_order' in request.POST: + # We have already made sure the formset is valid, so just save it to update quantities. + formset.save() + + order.customer_comment = request.POST.get('customer_comment') or '' + order.invoice_address = request.POST.get('invoice_address') or '' + order.save() + + # Then at last see if the user is paying for the order. + payment_method = request.POST.get('payment_method') if payment_method in order.PAYMENT_METHODS: if not request.POST.get('accept_terms'): messages.error(request, "You need to accept the general terms and conditions before you can continue!") @@ -330,44 +385,6 @@ class OrderDetailView(LoginRequiredMixin, EnsureUserOwnsOrderMixin, EnsureOrderH return HttpResponseRedirect(reverses[payment_method]) - if 'update_order' in request.POST: - for order_product in order.orderproductrelation_set.all(): - order_product_id = str(order_product.pk) - if order_product_id in request.POST: - new_quantity = int(request.POST.get(order_product_id)) - - if order_product.quantity < new_quantity: - # We are incrementing and thus need to check stock - incrementing_by = new_quantity - order_product.quantity - if incrementing_by > order_product.product.left_in_stock: - messages.error( - request, - "Sadly we only have {} '{}' left in stock.".format( - order_product.product.left_in_stock, - order_product.product.name, - ) - ) - return super(OrderDetailView, self).get(request, *args, **kwargs) - - order_product.quantity = new_quantity - order_product.save() - order.customer_comment = request.POST.get('customer_comment') or '' - order.invoice_address = request.POST.get('invoice_address') or '' - order.save() - - product_remove = request.POST.get('remove_product') - if product_remove: - order.orderproductrelation_set.filter(pk=product_remove).delete() - if not order.products.count() > 0: - order.mark_as_cancelled() - messages.info(request, 'Order cancelled!') - return HttpResponseRedirect(reverse_lazy('shop:index')) - - if 'cancel_order' in request.POST: - order.mark_as_cancelled() - messages.info(request, 'Order cancelled!') - return HttpResponseRedirect(reverse_lazy('shop:index')) - return super(OrderDetailView, self).get(request, *args, **kwargs)