From 29236e0b0e15cb225b5ade11f3d7dae3dafb01d2 Mon Sep 17 00:00:00 2001 From: Ludovic Stephan Date: Sun, 28 Feb 2021 02:02:31 +0100 Subject: [PATCH] Nouvelle gestion des erreurs JSON --- kfet/forms.py | 2 +- kfet/views.py | 275 +++++++++++++++++++++++++++++++++----------------- 2 files changed, 182 insertions(+), 95 deletions(-) diff --git a/kfet/forms.py b/kfet/forms.py index 4f91680f..5cc9d83f 100644 --- a/kfet/forms.py +++ b/kfet/forms.py @@ -537,7 +537,7 @@ class TransferForm(forms.ModelForm): def clean_amount(self): amount = self.cleaned_data["amount"] if amount <= 0: - raise forms.ValidationError("Montant invalide") + raise forms.ValidationError("Le montant d'un transfert doit être positif") return amount class Meta: diff --git a/kfet/views.py b/kfet/views.py index 76643809..e403505a 100644 --- a/kfet/views.py +++ b/kfet/views.py @@ -15,7 +15,7 @@ from django.contrib.messages.views import SuccessMessageMixin from django.core.exceptions import SuspiciousOperation, ValidationError from django.db import transaction from django.db.models import Count, F, Max, OuterRef, Prefetch, Q, Subquery, Sum -from django.forms import formset_factory +from django.forms import ValidationError, formset_factory from django.http import ( Http404, HttpResponseBadRequest, @@ -964,15 +964,18 @@ def kpsul_checkout_data(request): @kfet_password_auth def kpsul_update_addcost(request): addcost_form = AddcostForm(request.POST) + data = {"errors": []} if not addcost_form.is_valid(): - data = {"errors": {"addcost": list(addcost_form.errors)}} + for (field, errors) in addcost_form.errors.items(): + for error in errors: + data["errors"].append({"code": f"invalid_{field}", "message": error}) + return JsonResponse(data, status=400) + required_perms = ["kfet.manage_addcosts"] if not request.user.has_perms(required_perms): - data = { - "errors": {"missing_perms": get_missing_perms(required_perms, request.user)} - } + data["missing_perms"] = get_missing_perms(required_perms, request.user) return JsonResponse(data, status=403) trigramme = addcost_form.cleaned_data["trigramme"] @@ -987,14 +990,13 @@ def kpsul_update_addcost(request): def get_missing_perms(required_perms: List[str], user: User) -> List[str]: - def get_perm_description(app_label: str, codename: str) -> str: - name = Permission.objects.values_list("name", flat=True).get( + def get_perm_name(app_label: str, codename: str) -> str: + return Permission.objects.values_list("name", flat=True).get( codename=codename, content_type__app_label=app_label ) - return "[{}] {}".format(app_label, name) missing_perms = [ - get_perm_description(*perm.split(".")) + get_perm_name(*perm.split(".")) for perm in required_perms if not user.has_perm(perm) ] @@ -1006,17 +1008,31 @@ def get_missing_perms(required_perms: List[str], user: User) -> List[str]: @kfet_password_auth def kpsul_perform_operations(request): # Initializing response data - data = {"operationgroup": 0, "operations": [], "warnings": {}, "errors": {}} + data = {"errors": []} # Checking operationgroup operationgroup_form = KPsulOperationGroupForm(request.POST) if not operationgroup_form.is_valid(): - data["errors"]["operation_group"] = list(operationgroup_form.errors) + for field in operationgroup_form.errors: + verbose_field, feminin = ( + ("compte", "") if field == "on_acc" else ("caisse", "e") + ) + data["errors"].append( + { + "code": f"invalid_{field}", + "message": f"Pas de {verbose_field} sélectionné{feminin}", + } + ) # Checking operation_formset operation_formset = KPsulOperationFormSet(request.POST) if not operation_formset.is_valid(): - data["errors"]["operations"] = list(operation_formset.errors) + data["errors"].append( + { + "code": "invalid_formset", + "message": "Formulaire d'opérations vide ou invalide", + } + ) # Returning BAD REQUEST if errors if data["errors"]: @@ -1025,6 +1041,7 @@ def kpsul_perform_operations(request): # Pre-saving (no commit) operationgroup = operationgroup_form.save(commit=False) operations = operation_formset.save(commit=False) + on_acc = operationgroup.on_acc # Retrieving COF grant cof_grant = kfet_config.subvention_cof @@ -1038,13 +1055,13 @@ def kpsul_perform_operations(request): 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 = all( - (addcost_for, addcost_amount, addcost_for != operationgroup.on_acc) - ) - need_comment = operationgroup.on_acc.need_comment + is_addcost = all((addcost_for, addcost_amount, addcost_for != on_acc)) + need_comment = on_acc.need_comment - if operationgroup.on_acc.is_frozen: - data["errors"]["frozen"] = [operationgroup.on_acc.trigramme] + if on_acc.is_frozen: + data["errors"].append( + {"code": "frozen_acc", "message": f"Le compte {on_acc.trigramme} est gelé"} + ) # Filling data of each operations # + operationgroup + calculating other stuffs @@ -1056,19 +1073,23 @@ def kpsul_perform_operations(request): operation.addcost_amount = addcost_amount * operation.article_nb operation.amount -= operation.addcost_amount to_addcost_for_balance += operation.addcost_amount - if operationgroup.on_acc.is_cash: + if on_acc.is_cash: to_checkout_balance += -operation.amount - if ( - operationgroup.on_acc.is_cof - and operation.article.category.has_reduction - ): + if on_acc.is_cof and operation.article.category.has_reduction: if is_addcost and operation.article.category.has_addcost: operation.addcost_amount /= cof_grant_divisor operation.amount = operation.amount / cof_grant_divisor to_articles_stocks[operation.article] -= operation.article_nb else: - if operationgroup.on_acc.is_cash: - data["errors"]["account"] = "LIQ" + if on_acc.is_cash: + data["errors"].append( + { + "code": "invalid_liq", + "message": ( + "Impossible de compter autre chose que des achats sur LIQ" + ), + } + ) if operation.type != Operation.EDIT: to_checkout_balance += operation.amount operationgroup.amount += operation.amount @@ -1077,41 +1098,42 @@ def kpsul_perform_operations(request): if operation.type == Operation.EDIT: required_perms.add("kfet.edit_balance_account") need_comment = True - if operationgroup.on_acc.is_cof: + if account.is_cof: to_addcost_for_balance = to_addcost_for_balance / cof_grant_divisor - (perms, stop) = operationgroup.on_acc.perms_to_perform_operation( - amount=operationgroup.amount - ) + (perms, stop) = account.perms_to_perform_operation(amount=operationgroup.amount) required_perms |= perms + if stop: + data["errors"].append( + { + "code": "negative", + "message": f"Le compte {account.trigramme} a un solde insuffisant.", + } + ) + if need_comment: operationgroup.comment = operationgroup.comment.strip() if not operationgroup.comment: - data["errors"]["need_comment"] = True + data["need_comment"] = True - if data["errors"]: + if data["errors"] or "need_comment" in data: return JsonResponse(data, status=400) - 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"] = [operationgroup.on_acc.trigramme] + if not request.user.has_perms(required_perms): + data["missing_perms"] = get_missing_perms(required_perms, request.user) 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 + operationgroup.is_cof = on_acc.is_cof # Starting transaction to ensure data consistency with transaction.atomic(): # If not cash account, # saving account's balance and adding to Negative if not in - on_acc = operationgroup.on_acc if not on_acc.is_cash: ( Account.objects.filter(pk=on_acc.pk).update( @@ -1135,13 +1157,10 @@ def kpsul_perform_operations(request): # Saving operation group operationgroup.save() - data["operationgroup"] = operationgroup.pk - # 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: @@ -1164,7 +1183,7 @@ def kpsul_perform_operations(request): "valid_by__trigramme": ( operationgroup.valid_by and operationgroup.valid_by.trigramme or None ), - "on_acc__trigramme": operationgroup.on_acc.trigramme, + "on_acc__trigramme": on_acc.trigramme, "entries": [], } ] @@ -1205,7 +1224,7 @@ def kpsul_perform_operations(request): @kfet_password_auth def cancel_operations(request): # Pour la réponse - data = {"canceled": [], "warnings": {}, "errors": {}} + data = {"canceled": [], "warnings": {}, "errors": []} # Checking if BAD REQUEST (opes_pk not int or not existing) try: @@ -1214,29 +1233,41 @@ def cancel_operations(request): map(int, filter(None, request.POST.getlist("operations[]", []))) ) except ValueError: + data["errors"].append( + {"code": "invalid_request", "message": "Requête invalide !"} + ) return JsonResponse(data, status=400) + opes_all = Operation.objects.select_related( "group", "group__on_acc", "group__on_acc__negative" ).filter(pk__in=opes_post) opes_pk = [ope.pk for ope in opes_all] opes_notexisting = [ope for ope in opes_post if ope not in opes_pk] if opes_notexisting: - data["errors"]["opes_notexisting"] = opes_notexisting + data["errors"].append( + { + "code": "cancel_missing", + "message": "Opérations inexistantes : {}".format( + ", ".join(map(str, opes_notexisting)) + ), + } + ) return JsonResponse(data, status=400) opes_already_canceled = [] # Déjà annulée opes = [] # Pas déjà annulée required_perms = set() - stop_all = False cancel_duration = kfet_config.cancel_duration - to_accounts_balances = defaultdict( - lambda: 0 - ) # Modifs à faire sur les balances des comptes - to_groups_amounts = defaultdict( - lambda: 0 - ) # ------ sur les montants des groupes d'opé - to_checkouts_balances = defaultdict(lambda: 0) # ------ sur les balances de caisses - to_articles_stocks = defaultdict(lambda: 0) # ------ sur les stocks d'articles + + # Modifs à faire sur les balances des comptes + to_accounts_balances = defaultdict(int) + # ------ sur les montants des groupes d'opé + to_groups_amounts = defaultdict(int) + # ------ sur les balances de caisses + to_checkouts_balances = defaultdict(int) + # ------ sur les stocks d'articles + to_articles_stocks = defaultdict(int) + for ope in opes_all: if ope.canceled_at: # Opération déjà annulée, va pour un warning en Response @@ -1307,16 +1338,22 @@ def cancel_operations(request): amount=to_accounts_balances[account] ) required_perms |= perms - stop_all = stop_all or stop if stop: negative_accounts.append(account.trigramme) - if stop_all 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_all: - data["errors"]["negative"] = negative_accounts + if negative_accounts: + data["errors"].append( + { + "code": "negative", + "message": "Solde insuffisant pour les comptes suivants : {}".format( + ", ".join(negative_accounts) + ), + } + ) + return JsonResponse(data, status=400) + + if not request.user.has_perms(required_perms): + data["missing_perms"] = get_missing_perms(required_perms, request.user) return JsonResponse(data, status=403) canceled_by = required_perms and request.user.profile.account_kfet or None @@ -1644,12 +1681,36 @@ def transfers_create(request): @teamkfet_required @kfet_password_auth def perform_transfers(request): - data = {"errors": {}, "transfers": [], "transfergroup": 0} + data = {"errors": []} # Checking transfer_formset transfer_formset = TransferFormSet(request.POST) - if not transfer_formset.is_valid(): - return JsonResponse({"errors": list(transfer_formset.errors)}, status=400) + try: + if not transfer_formset.is_valid(): + for form_errors in transfer_formset.errors: + for (field, errors) in form_errors.items(): + if field == "amount": + for error in errors: + data["errors"].append({"code": "amount", "message": error}) + else: + # C'est compliqué de trouver le compte qui pose problème... + acc_error = True + + if acc_error: + data["errors"].append( + { + "code": "invalid_acc", + "message": "L'un des comptes est invalide ou manquant", + } + ) + + return JsonResponse(data, status=400) + + except ValidationError: + data["errors"].append( + {"code": "invalid_request", "message": "Requête invalide"} + ) + return JsonResponse(data, status=400) transfers = transfer_formset.save(commit=False) @@ -1657,14 +1718,12 @@ def perform_transfers(request): required_perms = set( ["kfet.add_transfer"] ) # Required perms to perform all transfers - to_accounts_balances = defaultdict(lambda: 0) # For balances of accounts + to_accounts_balances = defaultdict(int) # For balances of accounts for transfer in transfers: to_accounts_balances[transfer.from_acc] -= transfer.amount to_accounts_balances[transfer.to_acc] += transfer.amount - stop_all = False - negative_accounts = [] # Checking if ok on all accounts frozen = set() @@ -1676,20 +1735,34 @@ def perform_transfers(request): amount=to_accounts_balances[account] ) required_perms |= perms - stop_all = stop_all or stop if stop: negative_accounts.append(account.trigramme) - if len(frozen): - data["errors"]["frozen"] = list(frozen) + if frozen: + data["errors"].append( + { + "code": "frozen", + "message": "Les comptes suivants sont gelés : {}".format( + ", ".join(frozen) + ), + } + ) + + if negative_accounts: + data["errors"].append( + { + "code": "negative", + "message": "Solde insuffisant pour les comptes suivants : {}".format( + ", ".join(negative_accounts) + ), + } + ) + + if data["errors"]: return JsonResponse(data, status=400) - if stop_all 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_all: - data["errors"]["negative"] = negative_accounts + if not request.user.has_perms(required_perms): + data["missing_perms"] = get_missing_perms(required_perms, request.user) return JsonResponse(data, status=403) # Creating transfer group @@ -1711,22 +1784,20 @@ def perform_transfers(request): # Saving transfer group transfergroup.save() - data["transfergroup"] = transfergroup.pk # Saving all transfers with group for transfer in transfers: transfer.group = transfergroup transfer.save() - data["transfers"].append(transfer.pk) - return JsonResponse(data) + return JsonResponse({}) @teamkfet_required @kfet_password_auth def cancel_transfers(request): # Pour la réponse - data = {"canceled": [], "warnings": {}, "errors": {}} + data = {"canceled": [], "warnings": {}, "errors": []} # Checking if BAD REQUEST (transfers_pk not int or not existing) try: @@ -1735,7 +1806,11 @@ def cancel_transfers(request): map(int, filter(None, request.POST.getlist("transfers[]", []))) ) except ValueError: + data["errors"].append( + {"code": "invalid_request", "message": "Requête invalide !"} + ) return JsonResponse(data, status=400) + transfers_all = Transfer.objects.select_related( "group", "from_acc", "from_acc__negative", "to_acc", "to_acc__negative" ).filter(pk__in=transfers_post) @@ -1744,17 +1819,23 @@ def cancel_transfers(request): transfer for transfer in transfers_post if transfer not in transfers_pk ] if transfers_notexisting: - data["errors"]["transfers_notexisting"] = transfers_notexisting + data["errors"].append( + { + "code": "cancel_missing", + "message": "Transferts inexistants : {}".format( + ", ".join(map(str, transfers_notexisting)) + ), + } + ) return JsonResponse(data, status=400) - transfers_already_canceled = [] # Déjà annulée - transfers = [] # Pas déjà annulée + transfers_already_canceled = [] # Déjà annulés + transfers = [] # Pas déjà annulés required_perms = set() - stop_all = False cancel_duration = kfet_config.cancel_duration - to_accounts_balances = defaultdict( - lambda: 0 - ) # Modifs à faire sur les balances des comptes + + # Modifs à faire sur les balances des comptes + to_accounts_balances = defaultdict(int) for transfer in transfers_all: if transfer.canceled_at: # Transfert déjà annulé, va pour un warning en Response @@ -1782,16 +1863,22 @@ def cancel_transfers(request): amount=to_accounts_balances[account] ) required_perms |= perms - stop_all = stop_all or stop if stop: negative_accounts.append(account.trigramme) - if stop_all 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_all: - data["errors"]["negative"] = negative_accounts + if negative_accounts: + data["errors"].append( + { + "code": "negative", + "message": "Solde insuffisant pour les comptes suivants : {}".format( + ", ".join(negative_accounts) + ), + } + ) + return JsonResponse(data, status=400) + + if not request.user.has_perms(required_perms): + data["missing_perms"] = get_missing_perms(required_perms, request.user) return JsonResponse(data, status=403) canceled_by = required_perms and request.user.profile.account_kfet or None