From 85f1a0b9cb099f487dcfb1f57d3ab2b9519f765a Mon Sep 17 00:00:00 2001 From: imperosol Date: Mon, 2 Mar 2026 15:44:37 +0100 Subject: [PATCH] refactor InvoiceItem and BasketItem models --- eboutic/admin.py | 14 +- ..._product_name_basketitem_label_and_more.py | 53 ++++++ eboutic/models.py | 167 ++++++++---------- 3 files changed, 135 insertions(+), 99 deletions(-) create mode 100644 eboutic/migrations/0003_rename_product_name_basketitem_label_and_more.py diff --git a/eboutic/admin.py b/eboutic/admin.py index b9be2774..d06e2ca1 100644 --- a/eboutic/admin.py +++ b/eboutic/admin.py @@ -22,23 +22,22 @@ from eboutic.models import Basket, BasketItem, Invoice, InvoiceItem class BasketAdmin(admin.ModelAdmin): list_display = ("user", "date", "total") autocomplete_fields = ("user",) + date_hierarchy = "date" def get_queryset(self, request): return ( super() .get_queryset(request) .annotate( - total=Sum( - F("items__quantity") * F("items__product_unit_price"), default=0 - ) + total=Sum(F("items__quantity") * F("items__unit_price"), default=0) ) ) @admin.register(BasketItem) class BasketItemAdmin(admin.ModelAdmin): - list_display = ("basket", "product_name", "product_unit_price", "quantity") - search_fields = ("product_name",) + list_display = ("label", "unit_price", "quantity") + search_fields = ("label",) @admin.register(Invoice) @@ -50,5 +49,6 @@ class InvoiceAdmin(admin.ModelAdmin): @admin.register(InvoiceItem) class InvoiceItemAdmin(admin.ModelAdmin): - list_display = ("invoice", "product_name", "product_unit_price", "quantity") - search_fields = ("product_name",) + list_display = ("label", "unit_price", "quantity") + search_fields = ("label",) + list_select_related = ("price",) diff --git a/eboutic/migrations/0003_rename_product_name_basketitem_label_and_more.py b/eboutic/migrations/0003_rename_product_name_basketitem_label_and_more.py new file mode 100644 index 00000000..d3b29d14 --- /dev/null +++ b/eboutic/migrations/0003_rename_product_name_basketitem_label_and_more.py @@ -0,0 +1,53 @@ +# Generated by Django 5.2.11 on 2026-02-22 18:13 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [("counter", "0038_price"), ("eboutic", "0002_auto_20221005_2243")] + + operations = [ + migrations.RenameField( + model_name="basketitem", old_name="product_name", new_name="label" + ), + migrations.RenameField( + model_name="basketitem", + old_name="product_unit_price", + new_name="unit_price", + ), + migrations.RenameField( + model_name="basketitem", old_name="product_id", new_name="product" + ), + migrations.RenameField( + model_name="invoiceitem", old_name="product_name", new_name="label" + ), + migrations.RenameField( + model_name="invoiceitem", + old_name="product_unit_price", + new_name="unit_price", + ), + migrations.RenameField( + model_name="invoiceitem", old_name="product_id", new_name="product" + ), + migrations.RemoveField(model_name="basketitem", name="type_id"), + migrations.RemoveField(model_name="invoiceitem", name="type_id"), + migrations.AlterField( + model_name="basketitem", + name="product", + field=models.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + to="counter.product", + verbose_name="product", + ), + ), + migrations.AlterField( + model_name="invoiceitem", + name="product", + field=models.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + to="counter.product", + verbose_name="product", + ), + ), + ] diff --git a/eboutic/models.py b/eboutic/models.py index e35b1ddf..a5c42bb8 100644 --- a/eboutic/models.py +++ b/eboutic/models.py @@ -17,12 +17,12 @@ from __future__ import annotations import hmac from datetime import datetime from enum import Enum -from typing import Any, Self +from typing import Self from dict2xml import dict2xml from django.conf import settings from django.db import DataError, models -from django.db.models import F, OuterRef, Subquery, Sum +from django.db.models import F, OuterRef, Q, Subquery, Sum from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ @@ -30,8 +30,8 @@ from core.models import User from counter.fields import CurrencyField from counter.models import ( BillingInfo, - Counter, Customer, + Price, Product, Refilling, Selling, @@ -39,20 +39,28 @@ from counter.models import ( ) -def get_eboutic_products(user: User) -> list[Product]: - products = ( - get_eboutic() - .products.filter(product_type__isnull=False) - .filter(archived=False, limit_age__lte=user.age) - .annotate( - order=F("product_type__order"), - category=F("product_type__name"), - category_comment=F("product_type__comment"), - price=F("selling_price"), # <-- selected price for basket validation +def get_eboutic_prices(user: User) -> list[Price]: + return list( + Price.objects.filter( + Q(is_always_shown=True, groups__in=user.all_groups) + | Q( + id=Subquery( + Price.objects.filter( + product_id=OuterRef("product_id"), groups__in=user.all_groups + ) + .order_by("amount") + .values("id")[:1] + ) + ), + product__product_type__isnull=False, + product__archived=False, + product__limit_age__lte=user.age, + product__counters=get_eboutic(), ) - .prefetch_related("buying_groups") # <-- used in `Product.can_be_sold_to` + .select_related("product", "product__product_type") + .order_by("product__product_type__order", "product_id", "amount") + .distinct() ) - return [p for p in products if p.can_be_sold_to(user)] class BillingInfoState(Enum): @@ -94,21 +102,21 @@ class Basket(models.Model): def __str__(self): return f"{self.user}'s basket ({self.items.all().count()} items)" - def can_be_viewed_by(self, user): + def can_be_viewed_by(self, user: User): return self.user == user @cached_property def contains_refilling_item(self) -> bool: return self.items.filter( - type_id=settings.SITH_COUNTER_PRODUCTTYPE_REFILLING + product__product_type_id=settings.SITH_COUNTER_PRODUCTTYPE_REFILLING ).exists() @cached_property def total(self) -> float: return float( - self.items.aggregate( - total=Sum(F("quantity") * F("product_unit_price"), default=0) - )["total"] + self.items.aggregate(total=Sum(F("quantity") * F("unit_price"), default=0))[ + "total" + ] ) def generate_sales( @@ -120,7 +128,8 @@ class Basket(models.Model): Example: ```python counter = Counter.objects.get(name="Eboutic") - sales = basket.generate_sales(counter, "SITH_ACCOUNT") + user = User.objects.get(username="bibou") + sales = basket.generate_sales(counter, user, Selling.PaymentMethod.SITH_ACCOUNT) # here the basket is in the same state as before the method call with transaction.atomic(): @@ -131,31 +140,23 @@ class Basket(models.Model): # thus only the sales remain ``` """ - # I must proceed with two distinct requests instead of - # only one with a join because the AbstractBaseItem model has been - # poorly designed. If you refactor the model, please refactor this too. - items = self.items.order_by("product_id") - ids = [item.product_id for item in items] - products = Product.objects.filter(id__in=ids).order_by("id") - # items and products are sorted in the same order - sales = [] - for item, product in zip(items, products, strict=False): - sales.append( - Selling( - label=product.name, - counter=counter, - club=product.club, - product=product, - seller=seller, - customer=Customer.get_or_create(self.user)[0], - unit_price=item.product_unit_price, - quantity=item.quantity, - payment_method=payment_method, - ) + customer = Customer.get_or_create(self.user)[0] + return [ + Selling( + label=item.label, + counter=counter, + club_id=item.product.club_id, + product=item.product, + seller=seller, + customer=customer, + unit_price=item.unit_price, + quantity=item.quantity, + payment_method=payment_method, ) - return sales + for item in self.items.select_related("product") + ] - def get_e_transaction_data(self) -> list[tuple[str, Any]]: + def get_e_transaction_data(self) -> list[tuple[str, str]]: user = self.user if not hasattr(user, "customer"): raise Customer.DoesNotExist @@ -201,7 +202,7 @@ class InvoiceQueryset(models.QuerySet): def annotate_total(self) -> Self: """Annotate the queryset with the total amount of each invoice. - The total amount is the sum of (product_unit_price * quantity) + The total amount is the sum of (unit_price * quantity) for all items related to the invoice. """ # aggregates within subqueries require a little bit of black magic, @@ -211,7 +212,7 @@ class InvoiceQueryset(models.QuerySet): total=Subquery( InvoiceItem.objects.filter(invoice_id=OuterRef("pk")) .values("invoice_id") - .annotate(total=Sum(F("product_unit_price") * F("quantity"))) + .annotate(total=Sum(F("unit_price") * F("quantity"))) .values("total") ) ) @@ -221,11 +222,7 @@ class Invoice(models.Model): """Invoices are generated once the payment has been validated.""" user = models.ForeignKey( - User, - related_name="invoices", - verbose_name=_("user"), - blank=False, - on_delete=models.CASCADE, + User, related_name="invoices", verbose_name=_("user"), on_delete=models.CASCADE ) date = models.DateTimeField(_("date"), auto_now=True) validated = models.BooleanField(_("validated"), default=False) @@ -246,53 +243,44 @@ class Invoice(models.Model): if self.validated: raise DataError(_("Invoice already validated")) customer, _created = Customer.get_or_create(user=self.user) - eboutic = Counter.objects.filter(type="EBOUTIC").first() - for i in self.items.all(): - if i.type_id == settings.SITH_COUNTER_PRODUCTTYPE_REFILLING: - new = Refilling( - counter=eboutic, - customer=customer, - operator=self.user, - amount=i.product_unit_price * i.quantity, - payment_method=Refilling.PaymentMethod.CARD, - date=self.date, + kwargs = { + "counter": get_eboutic(), + "customer": customer, + "date": self.date, + "payment_method": Selling.PaymentMethod.CARD, + } + for i in self.items.select_related("product"): + if i.product.product_type_id == settings.SITH_COUNTER_PRODUCTTYPE_REFILLING: + Refilling.objects.create( + **kwargs, operator=self.user, amount=i.unit_price * i.quantity ) - new.save() else: - product = Product.objects.filter(id=i.product_id).first() - new = Selling( - label=i.product_name, - counter=eboutic, - club=product.club, - product=product, + Selling.objects.create( + **kwargs, + label=i.label, + club_id=i.product.club_id, + product=i.product, seller=self.user, - customer=customer, - unit_price=i.product_unit_price, + unit_price=i.unit_price, quantity=i.quantity, - payment_method=Selling.PaymentMethod.CARD, - date=self.date, ) - new.save() self.validated = True self.save() class AbstractBaseItem(models.Model): - product_id = models.IntegerField(_("product id")) - product_name = models.CharField(_("product name"), max_length=255) - type_id = models.IntegerField(_("product type id")) - product_unit_price = CurrencyField(_("unit price")) + product = models.ForeignKey( + Product, verbose_name=_("product"), on_delete=models.PROTECT + ) + label = models.CharField(_("product name"), max_length=255) + unit_price = CurrencyField(_("unit price")) quantity = models.PositiveIntegerField(_("quantity")) class Meta: abstract = True def __str__(self): - return "Item: %s (%s) x%d" % ( - self.product_name, - self.product_unit_price, - self.quantity, - ) + return "Item: %s (%s) x%d" % (self.product.name, self.unit_price, self.quantity) class BasketItem(AbstractBaseItem): @@ -301,21 +289,16 @@ class BasketItem(AbstractBaseItem): ) @classmethod - def from_product(cls, product: Product, quantity: int, basket: Basket): + def from_price(cls, price: Price, quantity: int, basket: Basket): """Create a BasketItem with the same characteristics as the - product passed in parameters, with the specified quantity. - - Warning: - the basket field is not filled, so you must set - it yourself before saving the model. + product price passed in parameters, with the specified quantity. """ return cls( basket=basket, - product_id=product.id, - product_name=product.name, - type_id=product.product_type_id, + label=price.full_label, + product_id=price.product_id, quantity=quantity, - product_unit_price=product.selling_price, + unit_price=price.amount, )