apply review comments

This commit is contained in:
imperosol
2026-06-11 18:13:03 +02:00
parent 998efc7c6b
commit caa2bf66be
8 changed files with 66 additions and 51 deletions
+33 -12
View File
@@ -3,6 +3,7 @@ import math
import uuid import uuid
from collections import defaultdict from collections import defaultdict
from datetime import date, datetime, timezone from datetime import date, datetime, timezone
from typing import ClassVar
from dateutil.relativedelta import relativedelta from dateutil.relativedelta import relativedelta
from django import forms from django import forms
@@ -11,6 +12,7 @@ from django.core.exceptions import ValidationError
from django.db.models import Exists, OuterRef, Q from django.db.models import Exists, OuterRef, Q
from django.forms import BaseModelFormSet from django.forms import BaseModelFormSet
from django.http import HttpRequest from django.http import HttpRequest
from django.utils.functional import cached_property
from django.utils.timezone import now from django.utils.timezone import now
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext_lazy as _
from django_celery_beat.models import ClockedSchedule from django_celery_beat.models import ClockedSchedule
@@ -600,6 +602,10 @@ class BasketItemForm(forms.Form):
class BaseBasketForm(forms.BaseFormSet): class BaseBasketForm(forms.BaseFormSet):
# Minimum amount of money there must be on the account after the transaction
# If None, the min balance check is skipped
min_result_balance: ClassVar[int | None] = 0
def __init__(self, *args, customer: Customer, counter: Counter, **kwargs): def __init__(self, *args, customer: Customer, counter: Counter, **kwargs):
super().__init__(*args, **kwargs) super().__init__(*args, **kwargs)
self.customer = customer self.customer = customer
@@ -614,8 +620,7 @@ class BaseBasketForm(forms.BaseFormSet):
self._check_forms_have_errors() self._check_forms_have_errors()
self._check_product_are_unique() self._check_product_are_unique()
self._check_recorded_products() self._check_recorded_products()
self._check_enough_money() self._check_account_balance()
self._check_refills()
def _check_forms_have_errors(self): def _check_forms_have_errors(self):
if any(len(form.errors) > 0 for form in self): if any(len(form.errors) > 0 for form in self):
@@ -626,10 +631,33 @@ class BaseBasketForm(forms.BaseFormSet):
if len(price_ids) != len(self.forms): if len(price_ids) != len(self.forms):
raise forms.ValidationError(_("Duplicated product entries.")) raise forms.ValidationError(_("Duplicated product entries."))
def _check_enough_money(self): @cached_property
self.total_price = sum([data["total_price"] for data in self.cleaned_data]) def total_price(self):
if self.total_price > self.customer.amount: refill = settings.SITH_COUNTER_PRODUCTTYPE_REFILLING
total_other = sum(
form.cleaned_data["total_price"]
for form in self.forms
if form.price.product.product_type_id != refill
)
total_refill = sum(
form.cleaned_data["total_price"]
for form in self.forms
if form.price.product.product_type_id == refill
)
return total_other - total_refill
def _check_account_balance(self):
result_balance = self.customer.amount - self.total_price
if (
self.min_result_balance is not None
and self.min_result_balance > result_balance
):
raise forms.ValidationError(_("Not enough money")) raise forms.ValidationError(_("Not enough money"))
if result_balance > settings.SITH_ACCOUNT_MAX_MONEY:
raise ValidationError(
_("There cannot be more than %(money)d€ on an AE account")
% {"money": settings.SITH_ACCOUNT_MAX_MONEY}
)
def _check_recorded_products(self): def _check_recorded_products(self):
"""Check for, among other things, ecocups and pitchers""" """Check for, among other things, ecocups and pitchers"""
@@ -659,13 +687,6 @@ class BaseBasketForm(forms.BaseFormSet):
% ", ".join([str(p) for p in limit_reached]) % ", ".join([str(p) for p in limit_reached])
) )
def _check_refills(self):
refill_type_id = settings.SITH_COUNTER_PRODUCTTYPE_REFILLING
if any(f.price.product.product_type_id == refill_type_id for f in self.forms):
raise ValidationError(
_("Refill bonds cannot be purchased outside of the eboutic")
)
BasketForm = forms.formset_factory( BasketForm = forms.formset_factory(
BasketItemForm, formset=BaseBasketForm, absolute_max=None, min_num=1 BasketItemForm, formset=BaseBasketForm, absolute_max=None, min_num=1
+1 -1
View File
@@ -99,7 +99,7 @@ class Customer(models.Model):
user = models.OneToOneField(User, primary_key=True, on_delete=models.CASCADE) user = models.OneToOneField(User, primary_key=True, on_delete=models.CASCADE)
account_id = models.CharField(_("account id"), max_length=10, unique=True) account_id = models.CharField(_("account id"), max_length=10, unique=True)
amount = CurrencyField( amount: CurrencyField = CurrencyField(
_("amount"), max_value=settings.SITH_ACCOUNT_MAX_MONEY, default=0 _("amount"), max_value=settings.SITH_ACCOUNT_MAX_MONEY, default=0
) )
+13
View File
@@ -535,6 +535,19 @@ class TestCounterClick(TestFullClickBase):
assert self.updated_amount(self.customer) == Decimal(10) assert self.updated_amount(self.customer) == Decimal(10)
def test_unrecord_above_limit_fails(self):
"""Test that it's forbidden to give back a recorded product
if it puts the account balance above the limit.
"""
self.login_in_bar()
limit = settings.SITH_ACCOUNT_MAX_MONEY
# put the account balance just at the limit
baker.make(Refilling, customer=self.customer.customer, amount=limit)
response = self.submit_basket(self.customer, [BasketItem(self.dcons.id, 1)])
assert response.status_code == 200 # no redirect = failure
self.customer.customer.refresh_from_db()
assert self.updated_amount(self.customer) == limit
def test_annotate_has_barman_queryset(self): def test_annotate_has_barman_queryset(self):
"""Test if the custom queryset method `annotate_has_barman` works as intended.""" """Test if the custom queryset method `annotate_has_barman` works as intended."""
counters = Counter.objects.annotate_has_barman(self.barmen) counters = Counter.objects.annotate_has_barman(self.barmen)
@@ -65,11 +65,15 @@ document.addEventListener("alpine:init", () => {
); );
}, },
getTotalRefill() { /**
* Get the total of money that would be added to the AE account on basket purchase.
*/
getTotalAdded() {
return this.basket return this.basket
.filter((item) => item.isRefill) .filter((item) => item.isRefill || item.unitPrice < 0)
.reduce( .reduce(
(acc: number, item: BasketItem) => acc + item.quantity * item.unitPrice, (acc: number, item: BasketItem) =>
acc + Math.abs(item.quantity * item.unitPrice),
0, 0,
); );
}, },
+2 -2
View File
@@ -58,7 +58,7 @@
</div> </div>
</div> </div>
{% endif %} {% endif %}
<template x-if="(getTotalRefill() + {{ customer_amount }}) > {{ settings.SITH_ACCOUNT_MAX_MONEY }}"> <template x-if="(getTotalAdded() + {{ customer_amount }}) > {{ settings.SITH_ACCOUNT_MAX_MONEY }}">
<div class="alert alert-red"> <div class="alert alert-red">
<div class="alert-main"> <div class="alert-main">
{% trans trimmed limit=settings.SITH_ACCOUNT_MAX_MONEY %} {% trans trimmed limit=settings.SITH_ACCOUNT_MAX_MONEY %}
@@ -122,7 +122,7 @@
</button> </button>
<button <button
class="btn btn-blue" class="btn btn-blue"
:disabled="(getTotalRefill() + {{ customer_amount }}) > {{ settings.SITH_ACCOUNT_MAX_MONEY }}" :disabled="(getTotalAdded() + {{ customer_amount }}) > {{ settings.SITH_ACCOUNT_MAX_MONEY }}"
> >
<i class="fa fa-check"></i> <i class="fa fa-check"></i>
{% trans %}Validate{% endtrans %} {% trans %}Validate{% endtrans %}
+1 -23
View File
@@ -66,29 +66,7 @@ if TYPE_CHECKING:
class BaseEbouticBasketForm(BaseBasketForm): class BaseEbouticBasketForm(BaseBasketForm):
def _check_enough_money(self, *args, **kwargs): min_result_balance = None # user can pay by card, so no minimum enforced
# Disable money check
...
def _check_refills(self):
"""Check that this basket won't put customer balance above the limit."""
refill_type_id = settings.SITH_COUNTER_PRODUCTTYPE_REFILLING
total_refill = sum(
f.price.amount * f.cleaned_data["quantity"]
for f in self.forms
if f.price.product.product_type_id == refill_type_id
)
total_other = sum(
f.price.amount * f.cleaned_data["quantity"]
for f in self.forms
if f.price.product.product_type_id != refill_type_id
)
limit = settings.SITH_ACCOUNT_MAX_MONEY
if (total_refill - total_other + self.customer.amount) > limit:
raise ValidationError(
_("There cannot be more than %(money)d€ on an AE account")
% {"money": limit}
)
EbouticBasketForm = forms.formset_factory( EbouticBasketForm = forms.formset_factory(
+5 -10
View File
@@ -3306,6 +3306,11 @@ msgstr "Saisie de produit dupliquée"
msgid "Not enough money" msgid "Not enough money"
msgstr "Solde insuffisant" msgstr "Solde insuffisant"
#: counter/forms.py counter/models.py
#, python-format
msgid "There cannot be more than %(money)d€ on an AE account"
msgstr "Il ne peut pas y avoir plus de %(money)d€ sur un compte AE"
#: counter/forms.py #: counter/forms.py
#, python-format #, python-format
msgid "" msgid ""
@@ -3314,11 +3319,6 @@ msgstr ""
"Cet utilisateur a atteint sa limite de déconsigne pour les produits " "Cet utilisateur a atteint sa limite de déconsigne pour les produits "
"suivants : %s" "suivants : %s"
#: counter/forms.py
msgid "Refill bonds cannot be purchased outside of the eboutic"
msgstr ""
"Les bons de rechargement ne peuvent pas être achetés en dehors de l'eboutic"
#: counter/management/commands/dump_accounts.py #: counter/management/commands/dump_accounts.py
msgid "Your AE account has been emptied" msgid "Your AE account has been emptied"
msgstr "Votre compte AE a été vidé" msgstr "Votre compte AE a été vidé"
@@ -3524,11 +3524,6 @@ msgstr "méthode de paiement"
msgid "refilling" msgid "refilling"
msgstr "rechargement" msgstr "rechargement"
#: counter/models.py eboutic/views.py
#, python-format
msgid "There cannot be more than %(money)d€ on an AE account"
msgstr "Il ne peut pas y avoir plus de %(money)d€ sur un compte AE"
#: counter/models.py #: counter/models.py
msgid "Sith account" msgid "Sith account"
msgstr "Compte utilisateur" msgstr "Compte utilisateur"
+4
View File
@@ -504,6 +504,10 @@ SITH_ACCOUNT_DUMP_DELTA = timedelta(days=30)
"""timedelta between the warning mail and the actual account dump""" """timedelta between the warning mail and the actual account dump"""
SITH_ACCOUNT_MAX_MONEY = 250 # € SITH_ACCOUNT_MAX_MONEY = 250 # €
"""Maximum amount of money a sith account can hold.
This amount is defined by the AE's Terms and Conditions of Sale.
"""
# Defines which product type is the refilling type, # Defines which product type is the refilling type,
# and thus increases the account amount # and thus increases the account amount