From 51c1767ffe911e8d22b12eeb28d52c2487f88184 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Aur=C3=A9lien=20Delobelle?= <aurelien.delobelle@ens.fr>
Date: Thu, 11 Aug 2016 06:13:31 +0200
Subject: [PATCH] =?UTF-8?q?Rework=20validation=20et=20enregistrement=20op?=
 =?UTF-8?q?=C3=A9s=20K-Psul?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Utilisations d'update pour les balances et autres afin d'éviter les
  lost update
- Validation tout en un puis enregistrement tout en un
- Ajout du lien vers K-Psul dans le nav
---
 kfet/templates/kfet/base_nav.html |   1 +
 kfet/views.py                     | 165 ++++++++++++------------------
 2 files changed, 69 insertions(+), 97 deletions(-)

diff --git a/kfet/templates/kfet/base_nav.html b/kfet/templates/kfet/base_nav.html
index 22f11450..bc433a8b 100644
--- a/kfet/templates/kfet/base_nav.html
+++ b/kfet/templates/kfet/base_nav.html
@@ -13,6 +13,7 @@
     {% if perms.kfet.is_team %}
     Equipe
     <ul>
+        <li><a href="{% url "kfet.kpsul" %}">K-Psul</a></li>
         <li><a href="{% url "kfet.account" %}">Comptes</a></li>
         <li><a href="{% url "kfet.checkout" %}">Caisses</a></li>
         <li><a href="{% url "kfet.article" %}">Articles</a></li>
diff --git a/kfet/views.py b/kfet/views.py
index 842e372a..ac980ecb 100644
--- a/kfet/views.py
+++ b/kfet/views.py
@@ -398,34 +398,27 @@ def get_missing_perms(required_perms, user):
 @permission_required('kfet.is_team')
 def kpsul_perform_operations(request):
     # Initializing response data
-    data = defaultdict(list)
+    data = { 'operation_group': 0, 'operations': [],
+             'warnings': {}, 'errors': {} }
 
     # Checking operationgroup
     operationgroup_form = KPsulOperationGroupForm(request.POST)
     if not operationgroup_form.is_valid():
-        data['errors'].append({'operation_group': list(operationgroup_form.errors)})
+        data['errors']['operation_group'] = list(operationgroup_form.errors)
 
     # Checking operation_formset
     operation_formset   = KPsulOperationFormSet(request.POST)
     if not operation_formset.is_valid():
-        data['errors'].append({'operations': list(operation_formset.errors) })
+        data['errors']['operations'] = list(operation_formset.errors)
 
     # Returning BAD REQUEST if errors
-    if 'errors' in data:
+    if data['errors']:
         return JsonResponse(data, status=400)
 
     # Pre-saving (no commit)
     operationgroup = operationgroup_form.save(commit = False)
     operations     = operation_formset.save(commit = False)
 
-    # Specific account's checking
-    if operationgroup.on_acc.is_cash:
-        for operation in operations:
-            if operation.type in [Operation.DEPOSIT, Operation.WITHDRAW]:
-                data['errors'].append(
-                    {'account': 'Charge et retrait impossible sur LIQ'})
-                return JsonResponse(data, status=400)
-
     # Retrieving COF grant
     cof_grant = Settings.SUBVENTION_COF()
     # Retrieving addcosts data
@@ -433,121 +426,99 @@ def kpsul_perform_operations(request):
     addcost_for    = Settings.ADDCOST_FOR()
 
     # Initializing vars
-    required_perms    = set()
+    required_perms    = set() # Required perms to perform all operations
     cof_grant_divisor = 1 + cof_grant / 100
+    to_addcost_for_balance = 0 # For balance of addcost_for
+    to_checkout_balance = 0 # For balance of selected checkout
+    to_articles_stocks  = defaultdict(lambda:0) # For stocks articles
     is_addcost = (addcost_for and addcost_amount
                     and addcost_for != operationgroup.on_acc)
-    addcost_total = 0
-    to_checkout_balance = 0
 
-    # 1. Calculating amount of each PURCHASE operations
-    # 1.1 Standard price for n articles
-    # 1.2 Adding addcost if there is one
-    # 1.3 Taking into account cof status
-    # 2. Updating (no commit) stock of article for PURCHASE operations
-    # 3. Calculating amount of operation group
-    # 4. Adding required permissions to perform each operation
-    # 5. Calculating total addcost
-    #    and adding addcost_for in operation instance
-    # 6. Calculating diff for checkout's balance
+    # Filling data of each operations + operationgroup + calculating other stuffs
     for operation in operations:
         if operation.type == Operation.PURCHASE:
-            # 1.1
             operation.amount = - operation.article.price * operation.article_nb
             if is_addcost:
-                # 1.2
+                operation.addcost_for    = addcost_for
                 operation.addcost_amount = addcost_amount * operation.article_nb
-                operation.amount -= operation.addcost_amount
-                # 5
-                addcost_total += operation.addcost_amount
-                operation.addcost_for = addcost_for
-            # 6
+                operation.amount        -= operation.addcost_amounty
+                to_addcost_for_balance  += operation.addcost_amount
             if operationgroup.on_acc.is_cash:
                 to_checkout_balance += -operation.amount
-            # 1.3
             if operationgroup.on_acc.is_cof:
                 operation.amount = operation.amount / cof_grant_divisor
-            # 2
-            operation.article.stock -= operation.article_nb
+            to_articles_stocks[operation.article] -= operation.article_nb
         else:
-            # Ope.type is deposit or withdraw
-            # 6 too
+            if operationgroup.on_acc.is_cash:
+                data['errors']['account'] = 'Charge et retrait impossible sur LIQ'
             to_checkout_balance += operation.amount
-        # 3
         operationgroup.amount += operation.amount
-        # 4
         if operation.type == Operation.DEPOSIT:
             required_perms.add('kfet.perform_deposit')
 
+    (perms, stop) = operationgroup.on_acc.perms_to_perform_operation(
+            amount = operationgroup.amount)
+    required_perms |= perms
+
+    if stop or not request.user.has_perms(required_perms):
+        missing_perms = get_missing_perms(required_perms, request.user)
+        if missing_perms:
+            data['errors']['missing_perms'] = missing_perms
+        if stop:
+            data['errors']['negative'] = True
+        return JsonResponse(data, status=403)
+
+    # If 1 perm is required, filling who perform the operations
+    if required_perms:
+        operationgroup.valid_by = request.user.profile.account_kfet
+    # Filling cof status for statistics
+    operationgroup.is_cof = operationgroup.on_acc.is_cof
+
     # Starting transaction to ensure data consistency
-    # Using select_for_update where it is critical
-    try:
-        with transaction.atomic():
-            on_acc = operationgroup.on_acc
-            on_acc = Account.objects.select_for_update().get(pk=on_acc.pk)
-            # Adding required permissions to perform operation group
-            (opegroup_perms, stop_ope) = on_acc.perms_to_perform_operation(
-                amount = operationgroup.amount)
-            required_perms |= opegroup_perms
-
-            # Checking authenticated user has all perms
-            if stop_ope or not request.user.has_perms(required_perms):
-                raise PermissionDenied
-
-            # If 1 perm is required, saving who perform the operations
-            if len(required_perms) > 0:
-                operationgroup.valid_by = request.user.profile.account_kfet
-
-            # Filling cof status for statistics
-            operationgroup.is_cof = on_acc.is_cof
-
-            # If not cash account,
-            # saving account's balance and adding to Negative if not in
-            if not on_acc.is_cash:
-                on_acc.balance += operationgroup.amount
-                on_acc.save()
-                if on_acc.balance < 0:
-                    if hasattr(on_acc, 'negative'):
-                        if not on_acc.negative.start:
-                            on_acc.negative.start = timezone.now()
-                            on_acc.negative.save()
+    with transaction.atomic():
+        # If not cash account,
+        # saving account's balance and adding to Negative if not in
+        if not operationgroup.on_acc.is_cash:
+            Account.objects.filter(pk=operationgroup.on_acc.pk).update(
+                balance = F('balance') + operationgroup.amount)
+            operationgroup.on_acc.refresh_from_db()
+            if operationgroup.on_acc.balance < 0:
+                if hasattr(on_acc, 'negative'):
+                    if not on_acc.negative.start:
+                        on_acc.negative.start = timezone.now()
+                        on_acc.negative.save()
                     else:
                         negative = AccountNegative(
-                            account = on_acc, start = timezone.now())
+                            account = operationgroup.on_acc, start = timezone.now())
                         negative.save()
                 elif (hasattr(on_acc, 'negative')
                         and not on_acc.negative.balance_offset):
                     on_acc.negative.delete()
 
-            # Updating checkout's balance
-            operationgroup.checkout.balance += to_checkout_balance
-            operationgroup.checkout.save()
+        # Updating checkout's balance
+        if to_checkout_balance:
+            Checkout.objects.filter(pk=operationgroup.checkout.pk).update(
+                balance = F('balance') + to_checkout_balance)
 
-            # Saving addcost_for with new balance if there is one
-            if is_addcost:
-                addcost_for.balance += addcost_total
-                addcost_for.save()
+        # Saving addcost_for with new balance if there is one
+        if is_addcost and to_addcost_for_balance:
+            Account.objects.filter(pk=addcost_for.pk).update(
+                balance = F('balance') + to_addcost_for_balance)
 
-            # Saving operation group
-            operationgroup.save()
-            data['operationgroup'] = operationgroup.pk
+        # Saving operation group
+        operationgroup.save()
+        data['operationgroup'] = operationgroup.pk
 
-            # Filling operationgroup id for each operations and saving
-            # Saving articles with new stock
-            for operation in operations:
-                operation.group = operationgroup
-                operation.save()
-                if operation.type == Operation.PURCHASE:
-                    operation.article.save()
-                data['operations'].append(operation.pk)
-    except PermissionDenied:
-        # Sending BAD_REQUEST with missing perms or url to manage negative
-        missing_perms = get_missing_perms(required_perms, request.user)
-        if missing_perms:
-            data['errors'].append({'missing_perms': missing_perms })
-        if stop_ope:
-            data['errors'].append({'negative': 'url to manage negative'})
-        return JsonResponse(data, status=403)
+        # Filling operationgroup id for each operations and saving
+        for operation in operations:
+            operation.group = operationgroup
+            operation.save()
+            data['operations'].append(operation.pk)
+
+        # Updating articles stock
+        for article in to_articles_stocks:
+            Article.objects.filter(pk=article.pk).update(
+                stock = F('stock') + to_articles_stocks[article])
 
     return JsonResponse(data)