Merge pull request #742 from ae-utbm/refactor-eboutic

Eboutic big refactor

counter/models.py

@@ -310,11 +310,26 @@ class Product(models.Model):
 
         Returns:
             True if the user can buy this product else False
+
+        Warnings:
+            This performs a db query, thus you can quickly have
+            a N+1 queries problem if you call it in a loop.
+            Hopefully, you can avoid that if you prefetch the buying_groups :
+
+            ```python
+            user = User.objects.get(username="foobar")
+            products = [
+                p
+                for p in Product.objects.prefetch_related("buying_groups")
+                if p.can_be_sold_to(user)
+            ]
+            ```
         """
-        if not self.buying_groups.exists():
+        buying_groups = list(self.buying_groups.all())
+        if not buying_groups:
             return True
-        for group_id in self.buying_groups.values_list("pk", flat=True):
-            if user.is_in_group(pk=group_id):
+        for group in buying_groups:
+            if user.is_in_group(pk=group.id):
                 return True
         return False
 
@@ -690,14 +705,14 @@ class Selling(models.Model):
             self.customer.amount -= self.quantity * self.unit_price
             self.customer.save(allow_negative=allow_negative, is_selling=True)
             self.is_validated = True
-        u = User.objects.filter(id=self.customer.user.id).first()
-        if u.was_subscribed:
+        user = self.customer.user
+        if user.was_subscribed:
             if (
                 self.product
                 and self.product.id == settings.SITH_PRODUCT_SUBSCRIPTION_ONE_SEMESTER
             ):
                 sub = Subscription(
-                    member=u,
+                    member=user,
                     subscription_type="un-semestre",
                     payment_method="EBOUTIC",
                     location="EBOUTIC",
@@ -719,9 +734,8 @@ class Selling(models.Model):
                 self.product
                 and self.product.id == settings.SITH_PRODUCT_SUBSCRIPTION_TWO_SEMESTERS
             ):
-                u = User.objects.filter(id=self.customer.user.id).first()
                 sub = Subscription(
-                    member=u,
+                    member=user,
                     subscription_type="deux-semestres",
                     payment_method="EBOUTIC",
                     location="EBOUTIC",
@@ -739,13 +753,13 @@ class Selling(models.Model):
                     start=sub.subscription_start,
                 )
                 sub.save()
-        if self.customer.user.preferences.notify_on_click:
+        if user.preferences.notify_on_click:
             Notification(
-                user=self.customer.user,
+                user=user,
                 url=reverse(
                     "core:user_account_detail",
                     kwargs={
-                        "user_id": self.customer.user.id,
+                        "user_id": user.id,
                         "year": self.date.year,
                         "month": self.date.month,
                     },
@@ -754,19 +768,15 @@ class Selling(models.Model):
                 type="SELLING",
             ).save()
         super().save(*args, **kwargs)
-        try:
-            # The product has no id until it's saved
-            if self.product.eticket:
-                self.send_mail_customer()
-        except:
-            pass
+        if hasattr(self.product, "eticket"):
+            self.send_mail_customer()
 
-    def is_owned_by(self, user):
+    def is_owned_by(self, user: User) -> bool:
         if user.is_anonymous:
             return False
-        return user.is_owner(self.counter) and self.payment_method != "CARD"
+        return self.payment_method != "CARD" and user.is_owner(self.counter)
 
-    def can_be_viewed_by(self, user):
+    def can_be_viewed_by(self, user: User) -> bool:
         if (
             not hasattr(self, "customer") or self.customer is None
         ):  # Customer can be set to Null
@@ -812,7 +822,9 @@ class Selling(models.Model):
             "url": self.customer.get_full_url(),
             "eticket": self.get_eticket_full_url(),
         }
-        self.customer.user.email_user(subject, message_txt, html_message=message_html)
+        self.customer.user.email_user(
+            subject, message_txt, html_message=message_html, fail_silently=True
+        )
 
     def get_eticket_full_url(self):
         eticket_url = reverse("counter:eticket_pdf", kwargs={"selling_id": self.id})

counter/tests.py

@@ -16,8 +16,9 @@ import json
 import re
 import string
 
+import pytest
 from django.core.cache import cache
-from django.test import TestCase
+from django.test import Client, TestCase
 from django.urls import reverse
 from django.utils import timezone
 from django.utils.timezone import timedelta
@@ -303,18 +304,11 @@ class TestCounterStats(TestCase):
         ]
 
 
-class TestBillingInfo(TestCase):
-    @classmethod
-    def setUpTestData(cls):
-        cls.payload_1 = {
-            "first_name": "Subscribed",
-            "last_name": "User",
-            "address_1": "1 rue des Huns",
-            "zip_code": "90000",
-            "city": "Belfort",
-            "country": "FR",
-        }
-        cls.payload_2 = {
+@pytest.mark.django_db
+class TestBillingInfo:
+    @pytest.fixture
+    def payload(self):
+        return {
             "first_name": "Subscribed",
             "last_name": "User",
             "address_1": "3, rue de Troyes",
@@ -322,213 +316,80 @@ class TestBillingInfo(TestCase):
             "city": "Sète",
             "country": "FR",
         }
-        cls.root = User.objects.get(username="root")
-        cls.subscriber = User.objects.get(username="subscriber")
 
-    def test_edit_infos(self):
-        user = self.subscriber
-        BillingInfo.objects.get_or_create(
-            customer=user.customer, defaults=self.payload_1
-        )
-        self.client.force_login(user)
-        response = self.client.post(
-            reverse("counter:edit_billing_info", args=[user.id]),
-            json.dumps(self.payload_2),
+    def test_edit_infos(self, client: Client, payload: dict):
+        user = subscriber_user.make()
+        baker.make(BillingInfo, customer=user.customer)
+        client.force_login(user)
+        response = client.put(
+            reverse("api:put_billing_info", args=[user.id]),
+            json.dumps(payload),
             content_type="application/json",
         )
-        user = User.objects.get(username="subscriber")
+        user.refresh_from_db()
         infos = BillingInfo.objects.get(customer__user=user)
         assert response.status_code == 200
-        self.assertJSONEqual(response.content, {"errors": None})
         assert hasattr(user.customer, "billing_infos")
         assert infos.customer == user.customer
-        assert infos.first_name == "Subscribed"
-        assert infos.last_name == "User"
-        assert infos.address_1 == "3, rue de Troyes"
-        assert infos.address_2 is None
-        assert infos.zip_code == "34301"
-        assert infos.city == "Sète"
-        assert infos.country == "FR"
+        for key, val in payload.items():
+            assert getattr(infos, key) == val
 
-    def test_create_infos_for_user_with_account(self):
-        user = User.objects.get(username="subscriber")
-        if hasattr(user.customer, "billing_infos"):
-            user.customer.billing_infos.delete()
-        self.client.force_login(user)
-        response = self.client.post(
-            reverse("counter:create_billing_info", args=[user.id]),
-            json.dumps(self.payload_1),
+    @pytest.mark.parametrize(
+        "user_maker", [subscriber_user.make, lambda: baker.make(User)]
+    )
+    @pytest.mark.django_db
+    def test_create_infos(self, client: Client, user_maker, payload):
+        user = user_maker()
+        client.force_login(user)
+        assert not BillingInfo.objects.filter(customer__user=user).exists()
+        response = client.put(
+            reverse("api:put_billing_info", args=[user.id]),
+            json.dumps(payload),
             content_type="application/json",
         )
-        user = User.objects.get(username="subscriber")
-        infos = BillingInfo.objects.get(customer__user=user)
         assert response.status_code == 200
-        self.assertJSONEqual(response.content, {"errors": None})
-        assert hasattr(user.customer, "billing_infos")
-        assert infos.customer == user.customer
-        assert infos.first_name == "Subscribed"
-        assert infos.last_name == "User"
-        assert infos.address_1 == "1 rue des Huns"
-        assert infos.address_2 is None
-        assert infos.zip_code == "90000"
-        assert infos.city == "Belfort"
-        assert infos.country == "FR"
-
-    def test_create_infos_for_user_without_account(self):
-        user = User.objects.get(username="subscriber")
-        if hasattr(user, "customer"):
-            user.customer.delete()
-        self.client.force_login(user)
-        response = self.client.post(
-            reverse("counter:create_billing_info", args=[user.id]),
-            json.dumps(self.payload_1),
-            content_type="application/json",
-        )
-        user = User.objects.get(username="subscriber")
+        user.refresh_from_db()
         assert hasattr(user, "customer")
-        assert hasattr(user.customer, "billing_infos")
-        assert response.status_code == 200
-        self.assertJSONEqual(response.content, {"errors": None})
         infos = BillingInfo.objects.get(customer__user=user)
-        self.assertEqual(user.customer, infos.customer)
-        assert infos.first_name == "Subscribed"
-        assert infos.last_name == "User"
-        assert infos.address_1 == "1 rue des Huns"
-        assert infos.address_2 is None
-        assert infos.zip_code == "90000"
-        assert infos.city == "Belfort"
-        assert infos.country == "FR"
-
-    def test_create_invalid(self):
-        user = User.objects.get(username="subscriber")
-        if hasattr(user.customer, "billing_infos"):
-            user.customer.billing_infos.delete()
-        self.client.force_login(user)
-        # address_1, zip_code and country are missing
-        payload = {
-            "first_name": user.first_name,
-            "last_name": user.last_name,
-            "city": "Belfort",
-        }
-        response = self.client.post(
-            reverse("counter:create_billing_info", args=[user.id]),
-            json.dumps(payload),
-            content_type="application/json",
-        )
-        user = User.objects.get(username="subscriber")
-        self.assertEqual(400, response.status_code)
-        assert not hasattr(user.customer, "billing_infos")
-        expected_errors = {
-            "errors": [
-                {"field": "Adresse 1", "messages": ["Ce champ est obligatoire."]},
-                {"field": "Code postal", "messages": ["Ce champ est obligatoire."]},
-                {"field": "Country", "messages": ["Ce champ est obligatoire."]},
-            ]
-        }
-        self.assertJSONEqual(response.content, expected_errors)
-
-    def test_edit_invalid(self):
-        user = User.objects.get(username="subscriber")
-        BillingInfo.objects.get_or_create(
-            customer=user.customer, defaults=self.payload_1
-        )
-        self.client.force_login(user)
-        # address_1, zip_code and country are missing
-        payload = {
-            "first_name": user.first_name,
-            "last_name": user.last_name,
-            "city": "Belfort",
-        }
-        response = self.client.post(
-            reverse("counter:edit_billing_info", args=[user.id]),
-            json.dumps(payload),
-            content_type="application/json",
-        )
-        user = User.objects.get(username="subscriber")
-        self.assertEqual(400, response.status_code)
-        assert hasattr(user.customer, "billing_infos")
-        expected_errors = {
-            "errors": [
-                {"field": "Adresse 1", "messages": ["Ce champ est obligatoire."]},
-                {"field": "Code postal", "messages": ["Ce champ est obligatoire."]},
-                {"field": "Country", "messages": ["Ce champ est obligatoire."]},
-            ]
-        }
-        self.assertJSONEqual(response.content, expected_errors)
-
-    def test_edit_other_user(self):
-        user = User.objects.get(username="sli")
-        self.client.login(username="subscriber", password="plop")
-        BillingInfo.objects.get_or_create(
-            customer=user.customer, defaults=self.payload_1
-        )
-        response = self.client.post(
-            reverse("counter:edit_billing_info", args=[user.id]),
-            json.dumps(self.payload_2),
-            content_type="application/json",
-        )
-        self.assertEqual(403, response.status_code)
-
-    def test_edit_not_existing_infos(self):
-        user = User.objects.get(username="subscriber")
-        if hasattr(user.customer, "billing_infos"):
-            user.customer.billing_infos.delete()
-        self.client.force_login(user)
-        response = self.client.post(
-            reverse("counter:edit_billing_info", args=[user.id]),
-            json.dumps(self.payload_2),
-            content_type="application/json",
-        )
-        self.assertEqual(404, response.status_code)
-
-    def test_edit_by_root(self):
-        user = User.objects.get(username="subscriber")
-        BillingInfo.objects.get_or_create(
-            customer=user.customer, defaults=self.payload_1
-        )
-        self.client.force_login(self.root)
-        response = self.client.post(
-            reverse("counter:edit_billing_info", args=[user.id]),
-            json.dumps(self.payload_2),
-            content_type="application/json",
-        )
-        assert response.status_code == 200
-        user = User.objects.get(username="subscriber")
-        infos = BillingInfo.objects.get(customer__user=user)
-        self.assertJSONEqual(response.content, {"errors": None})
-        assert hasattr(user.customer, "billing_infos")
-        self.assertEqual(user.customer, infos.customer)
-        self.assertEqual("Subscribed", infos.first_name)
-        self.assertEqual("User", infos.last_name)
-        self.assertEqual("3, rue de Troyes", infos.address_1)
-        self.assertEqual(None, infos.address_2)
-        self.assertEqual("34301", infos.zip_code)
-        self.assertEqual("Sète", infos.city)
-        self.assertEqual("FR", infos.country)
-
-    def test_create_by_root(self):
-        user = User.objects.get(username="subscriber")
-        if hasattr(user.customer, "billing_infos"):
-            user.customer.billing_infos.delete()
-        self.client.force_login(self.root)
-        response = self.client.post(
-            reverse("counter:create_billing_info", args=[user.id]),
-            json.dumps(self.payload_2),
-            content_type="application/json",
-        )
-        assert response.status_code == 200
-        user = User.objects.get(username="subscriber")
-        infos = BillingInfo.objects.get(customer__user=user)
-        self.assertJSONEqual(response.content, {"errors": None})
         assert hasattr(user.customer, "billing_infos")
         assert infos.customer == user.customer
-        assert infos.first_name == "Subscribed"
-        assert infos.last_name == "User"
-        assert infos.address_1 == "3, rue de Troyes"
-        assert infos.address_2 is None
-        assert infos.zip_code == "34301"
-        assert infos.city == "Sète"
-        assert infos.country == "FR"
+        for key, val in payload.items():
+            assert getattr(infos, key) == val
+
+    def test_invalid_data(self, client: Client, payload):
+        user = subscriber_user.make()
+        client.force_login(user)
+        # address_1, zip_code and country are missing
+        del payload["city"]
+        response = client.put(
+            reverse("api:put_billing_info", args=[user.id]),
+            json.dumps(payload),
+            content_type="application/json",
+        )
+        assert response.status_code == 422
+        user.customer.refresh_from_db()
+        assert not hasattr(user.customer, "billing_infos")
+
+    @pytest.mark.parametrize(
+        ("operator_maker", "expected_code"),
+        [
+            (subscriber_user.make, 403),
+            (lambda: baker.make(User), 403),
+            (lambda: baker.make(User, is_superuser=True), 200),
+        ],
+    )
+    def test_edit_other_user(
+        self, client: Client, operator_maker, expected_code: int, payload: dict
+    ):
+        user = subscriber_user.make()
+        client.force_login(operator_maker())
+        baker.make(BillingInfo, customer=user.customer)
+        response = client.put(
+            reverse("api:put_billing_info", args=[user.id]),
+            json.dumps(payload),
+            content_type="application/json",
+        )
+        assert response.status_code == expected_code
 
 
 class TestBarmanConnection(TestCase):

counter/urls.py

@@ -57,16 +57,6 @@ urlpatterns = [
         StudentCardDeleteView.as_view(),
         name="delete_student_card",
     ),
-    path(
-        "customer/<int:user_id>/billing_info/create",
-        create_billing_info,
-        name="create_billing_info",
-    ),
-    path(
-        "customer/<int:user_id>/billing_info/edit",
-        edit_billing_info,
-        name="edit_billing_info",
-    ),
     path("admin/<int:counter_id>/", CounterEditView.as_view(), name="admin"),
     path(
         "admin/<int:counter_id>/prop/",

counter/views.py

@@ -12,7 +12,6 @@
 # OR WITHIN THE LOCAL FILE "LICENSE"
 #
 #
-import json
 import re
 from datetime import datetime, timedelta
 from datetime import timezone as tz
@@ -21,7 +20,6 @@ from urllib.parse import parse_qs
 
 from django import forms
 from django.conf import settings
-from django.contrib.auth.decorators import login_required
 from django.core.exceptions import PermissionDenied
 from django.db import DataError, transaction
 from django.db.models import F
@@ -56,7 +54,6 @@ from core.utils import get_semester_code, get_start_of_semester
 from core.views import CanEditMixin, CanViewMixin, TabedViewMixin
 from core.views.forms import LoginForm
 from counter.forms import (
-    BillingInfoForm,
     CashSummaryFormBase,
     CounterEditForm,
     EticketForm,
@@ -67,7 +64,6 @@ from counter.forms import (
     StudentCardForm,
 )
 from counter.models import (
-    BillingInfo,
     CashRegisterSummary,
     CashRegisterSummaryItem,
     Counter,
@@ -1569,51 +1565,3 @@ class StudentCardFormView(FormView):
         return reverse_lazy(
             "core:user_prefs", kwargs={"user_id": self.customer.user.pk}
         )
-
-
-def __manage_billing_info_req(request, user_id, *, delete_if_fail=False):
-    data = json.loads(request.body)
-    form = BillingInfoForm(data)
-    if not form.is_valid():
-        if delete_if_fail:
-            Customer.objects.get(user__id=user_id).billing_infos.delete()
-        errors = [
-            {"field": str(form.fields[k].label), "messages": v}
-            for k, v in form.errors.items()
-        ]
-        content = json.dumps({"errors": errors})
-        return HttpResponse(status=400, content=content)
-    if form.is_valid():
-        infos = Customer.objects.get(user__id=user_id).billing_infos
-        for field in form.fields:
-            infos.__dict__[field] = form[field].value()
-        infos.save()
-        content = json.dumps({"errors": None})
-        return HttpResponse(status=200, content=content)
-
-
-@login_required
-@require_POST
-def create_billing_info(request, user_id):
-    user = request.user
-    if user.id != user_id and not user.has_perm("counter:add_billinginfo"):
-        raise PermissionDenied()
-    user = get_object_or_404(User, pk=user_id)
-    customer, _ = Customer.get_or_create(user)
-    BillingInfo.objects.create(customer=customer)
-    return __manage_billing_info_req(request, user_id, delete_if_fail=True)
-
-
-@login_required
-@require_POST
-def edit_billing_info(request, user_id):
-    user = request.user
-    if user.id != user_id and not user.has_perm("counter:change_billinginfo"):
-        raise PermissionDenied()
-    user = get_object_or_404(User, pk=user_id)
-    if not hasattr(user, "customer"):
-        raise Http404
-    if not hasattr(user.customer, "billing_infos"):
-        raise Http404
-
-    return __manage_billing_info_req(request, user_id)

eboutic/admin.py

@@ -19,9 +19,20 @@ from eboutic.models import *
 
 @admin.register(Basket)
 class BasketAdmin(admin.ModelAdmin):
-    list_display = ("user", "date", "get_total")
+    list_display = ("user", "date", "total")
     autocomplete_fields = ("user",)
 
+    def get_queryset(self, request):
+        return (
+            super()
+            .get_queryset(request)
+            .annotate(
+                total=Sum(
+                    F("items__quantity") * F("items__product_unit_price"), default=0
+                )
+            )
+        )
+
 
 @admin.register(BasketItem)
 class BasketItemAdmin(admin.ModelAdmin):

eboutic/api.py

@@ -0,0 +1,38 @@
+from django.shortcuts import get_object_or_404
+from ninja_extra import ControllerBase, api_controller, route
+from ninja_extra.exceptions import NotFound, PermissionDenied
+from ninja_extra.permissions import IsAuthenticated
+from pydantic import NonNegativeInt
+
+from core.models import User
+from counter.models import BillingInfo, Customer
+from eboutic.models import Basket
+from eboutic.schemas import BillingInfoSchema
+
+
+@api_controller("/etransaction", permissions=[IsAuthenticated])
+class EtransactionInfoController(ControllerBase):
+    @route.put("/billing-info/{user_id}", url_name="put_billing_info")
+    def put_user_billing_info(self, user_id: NonNegativeInt, info: BillingInfoSchema):
+        """Update or create the billing info of this user."""
+        if user_id == self.context.request.user.id:
+            user = self.context.request.user
+        elif self.context.request.user.is_root:
+            user = get_object_or_404(User, pk=user_id)
+        else:
+            raise PermissionDenied
+        customer, _ = Customer.get_or_create(user)
+        BillingInfo.objects.update_or_create(
+            customer=customer, defaults=info.model_dump(exclude_none=True)
+        )
+
+    @route.get("/data", url_name="etransaction_data", include_in_schema=False)
+    def fetch_etransaction_data(self):
+        """Generate the data to pay an eboutic command with paybox.
+
+        The data is generated with the basket that is used by the current session.
+        """
+        basket = Basket.from_session(self.context.request.session)
+        if basket is None:
+            raise NotFound
+        return dict(basket.get_e_transaction_data())

eboutic/forms.py

@@ -20,17 +20,15 @@
 # Place - Suite 330, Boston, MA 02111-1307, USA.
 #
 #
-
-import json
-import re
-import typing
+from functools import cached_property
 from urllib.parse import unquote
 
 from django.http import HttpRequest
 from django.utils.translation import gettext as _
-from sentry_sdk import capture_message
+from pydantic import ValidationError
 
 from eboutic.models import get_eboutic_products
+from eboutic.schemas import PurchaseItemList, PurchaseItemSchema
 
 
 class BasketForm:
@@ -43,8 +41,7 @@ class BasketForm:
     Thus this class is a pure standalone and performs its operations by its own means.
     However, it still tries to share some similarities with a standard django Form.
 
-    Example:
-    -------
+    Examples:
         ::
 
             def my_view(request):
@@ -62,28 +59,13 @@ class BasketForm:
     You can also use a little shortcut by directly calling `form.is_valid()`
     without calling `form.clean()`. In this case, the latter method shall be
     implicitly called.
-
     """
 
-    # check the json is an array containing non-nested objects.
-    # values must be strings or numbers
-    # this is matched :
-    # [{"id": 4, "name": "[PROMO 22] badges", "unit_price": 2.3, "quantity": 2}]
-    # but this is not :
-    # [{"id": {"nested_id": 10}, "name": "[PROMO 22] badges", "unit_price": 2.3, "quantity": 2}]
-    # and neither does this :
-    # [{"id": ["nested_id": 10], "name": "[PROMO 22] badges", "unit_price": 2.3, "quantity": 2}]
-    # and neither does that :
-    # [{"id": null, "name": "[PROMO 22] badges", "unit_price": 2.3, "quantity": 2}]
-    json_cookie_re = re.compile(
-        r"^\[\s*(\{\s*(\"[^\"]*\":\s*(\"[^\"]{0,64}\"|\d{0,5}\.?\d+),?\s*)*\},?\s*)*\s*\]$"
-    )
-
     def __init__(self, request: HttpRequest):
         self.user = request.user
         self.cookies = request.COOKIES
         self.error_messages = set()
-        self.correct_cookie = []
+        self.correct_items = []
 
     def clean(self) -> None:
         """Perform all the checks, but return nothing.
@@ -98,70 +80,29 @@ class BasketForm:
             - all the ids refer to products the user is allowed to buy
             - all the quantities are positive integers
         """
-        # replace escaped double quotes by single quotes, as the RegEx used to check the json
-        # does not support escaped double quotes
-        basket = unquote(self.cookies.get("basket_items", "")).replace('\\"', "'")
-
-        if basket in ("[]", ""):
-            self.error_messages.add(_("You have no basket."))
-            return
-
-        # check that the json is not nested before parsing it to make sure
-        # malicious user can't DDoS the server with deeply nested json
-        if not BasketForm.json_cookie_re.match(basket):
-            # As the validation of the cookie goes through a rather boring regex,
-            # we can regularly have to deal with subtle errors that we hadn't forecasted,
-            # so we explicitly lay a Sentry message capture here.
-            capture_message(
-                "Eboutic basket regex checking failed to validate basket json",
-                level="error",
+        try:
+            basket = PurchaseItemList.validate_json(
+                unquote(self.cookies.get("basket_items", "[]"))
             )
+        except ValidationError:
             self.error_messages.add(_("The request was badly formatted."))
             return
-
-        try:
-            basket = json.loads(basket)
-        except json.JSONDecodeError:
-            self.error_messages.add(_("The basket cookie was badly formatted."))
-            return
-
-        if type(basket) is not list or len(basket) == 0:
+        if len(basket) == 0:
             self.error_messages.add(_("Your basket is empty."))
             return
-
+        existing_ids = {product.id for product in get_eboutic_products(self.user)}
         for item in basket:
-            expected_keys = {"id", "quantity", "name", "unit_price"}
-            if type(item) is not dict or set(item.keys()) != expected_keys:
-                self.error_messages.add("One or more items are badly formatted.")
-                continue
-            # check the id field is a positive integer
-            if type(item["id"]) is not int or item["id"] < 0:
-                self.error_messages.add(
-                    _("%(name)s : this product does not exist.")
-                    % {"name": item["name"]}
-                )
-                continue
             # check a product with this id does exist
-            ids = {product.id for product in get_eboutic_products(self.user)}
-            if not item["id"] in ids:
+            if item.product_id in existing_ids:
+                self.correct_items.append(item)
+            else:
                 self.error_messages.add(
                     _(
                         "%(name)s : this product does not exist or may no longer be available."
                     )
-                    % {"name": item["name"]}
+                    % {"name": item.name}
                 )
                 continue
-            if type(item["quantity"]) is not int or item["quantity"] < 0:
-                self.error_messages.add(
-                    _("You cannot buy %(nbr)d %(name)s.")
-                    % {"nbr": item["quantity"], "name": item["name"]}
-                )
-                continue
-
-            # if we arrive here, it means this item has passed all tests
-            self.correct_cookie.append(item)
-        # for loop for item checking ends here
-
         # this function does not return anything.
         # instead, it fills a set containing the collected error messages
         # an empty set means that no error was seen thus everything is ok
@@ -174,16 +115,16 @@ class BasketForm:
 
         If the `clean()` method has not been called beforehand, call it.
         """
-        if self.error_messages == set() and self.correct_cookie == []:
+        if not self.error_messages and not self.correct_items:
             self.clean()
         if self.error_messages:
             return False
         return True
 
-    def get_error_messages(self) -> typing.List[str]:
+    @cached_property
+    def errors(self) -> list[str]:
         return list(self.error_messages)
 
-    def get_cleaned_cookie(self) -> str:
-        if not self.correct_cookie:
-            return ""
-        return json.dumps(self.correct_cookie)
+    @cached_property
+    def cleaned_data(self) -> list[PurchaseItemSchema]:
+        return self.correct_items

eboutic/models.py

@@ -16,6 +16,7 @@ from __future__ import annotations
 
 import hmac
 from datetime import datetime
+from typing import Any
 
 from dict2xml import dict2xml
 from django.conf import settings
@@ -38,6 +39,7 @@ def get_eboutic_products(user: User) -> list[Product]:
         .annotate(priority=F("product_type__priority"))
         .annotate(category=F("product_type__name"))
         .annotate(category_comment=F("product_type__comment"))
+        .prefetch_related("buying_groups")  # <-- used in `Product.can_be_sold_to`
     )
     return [p for p in products if p.can_be_sold_to(user)]
 
@@ -57,66 +59,25 @@ class Basket(models.Model):
     def __str__(self):
         return f"{self.user}'s basket ({self.items.all().count()} items)"
 
-    def add_product(self, p: Product, q: int = 1):
-        """Given p an object of the Product model and q an integer,
-        add q items corresponding to this Product from the basket.
-
-        If this function is called with a product not in the basket, no error will be raised
-        """
-        item = self.items.filter(product_id=p.id).first()
-        if item is None:
-            BasketItem(
-                basket=self,
-                product_id=p.id,
-                product_name=p.name,
-                type_id=p.product_type.id,
-                quantity=q,
-                product_unit_price=p.selling_price,
-            ).save()
-        else:
-            item.quantity += q
-            item.save()
-
-    def del_product(self, p: Product, q: int = 1):
-        """Given p an object of the Product model and q an integer
-        remove q items corresponding to this Product from the basket.
-
-        If this function is called with a product not in the basket, no error will be raised
-        """
-        try:
-            item = self.items.get(product_id=p.id)
-        except BasketItem.DoesNotExist:
-            return
-        item.quantity -= q
-        if item.quantity <= 0:
-            item.delete()
-        else:
-            item.save()
-
-    def clear(self) -> None:
-        """Remove all items from this basket without deleting the basket."""
-        self.items.all().delete()
-
     @cached_property
     def contains_refilling_item(self) -> bool:
         return self.items.filter(
             type_id=settings.SITH_COUNTER_PRODUCTTYPE_REFILLING
         ).exists()
 
-    def get_total(self) -> float:
-        total = self.items.aggregate(
-            total=Sum(F("quantity") * F("product_unit_price"))
-        )["total"]
-        return float(total) if total is not None else 0
+    @cached_property
+    def total(self) -> float:
+        return float(
+            self.items.aggregate(
+                total=Sum(F("quantity") * F("product_unit_price"), default=0)
+            )["total"]
+        )
 
     @classmethod
     def from_session(cls, session) -> Basket | None:
         """The basket stored in the session object, if it exists."""
         if "basket_id" in session:
-            try:
-                return cls.objects.get(id=session["basket_id"])
-            except cls.DoesNotExist:
-                return None
+            return cls.objects.filter(id=session["basket_id"]).first()
         return None
 
     def generate_sales(self, counter, seller: User, payment_method: str):
@@ -161,18 +122,24 @@ class Basket(models.Model):
             )
         return sales
 
-    def get_e_transaction_data(self):
+    def get_e_transaction_data(self) -> list[tuple[str, Any]]:
         user = self.user
         if not hasattr(user, "customer"):
             raise Customer.DoesNotExist
         customer = user.customer
         if not hasattr(user.customer, "billing_infos"):
             raise BillingInfo.DoesNotExist
+        cart = {
+            "shoppingcart": {"total": {"totalQuantity": min(self.items.count(), 99)}}
+        }
+        cart = '<?xml version="1.0" encoding="UTF-8" ?>' + dict2xml(
+            cart, newlines=False
+        )
         data = [
             ("PBX_SITE", settings.SITH_EBOUTIC_PBX_SITE),
             ("PBX_RANG", settings.SITH_EBOUTIC_PBX_RANG),
             ("PBX_IDENTIFIANT", settings.SITH_EBOUTIC_PBX_IDENTIFIANT),
-            ("PBX_TOTAL", str(int(self.get_total() * 100))),
+            ("PBX_TOTAL", str(int(self.total * 100))),
             ("PBX_DEVISE", "978"),  # This is Euro
             ("PBX_CMD", str(self.id)),
             ("PBX_PORTEUR", user.email),
@@ -181,14 +148,6 @@ class Basket(models.Model):
             ("PBX_TYPEPAIEMENT", "CARTE"),
             ("PBX_TYPECARTE", "CB"),
             ("PBX_TIME", datetime.now().replace(microsecond=0).isoformat("T")),
-        ]
-        cart = {
-            "shoppingcart": {"total": {"totalQuantity": min(self.items.count(), 99)}}
-        }
-        cart = '<?xml version="1.0" encoding="UTF-8" ?>' + dict2xml(
-            cart, newlines=False
-        )
-        data += [
             ("PBX_SHOPPINGCART", cart),
             ("PBX_BILLING", customer.billing_infos.to_3dsv2_xml()),
         ]
@@ -218,10 +177,11 @@ class Invoice(models.Model):
         return f"{self.user} - {self.get_total()} - {self.date}"
 
     def get_total(self) -> float:
-        total = self.items.aggregate(
-            total=Sum(F("quantity") * F("product_unit_price"))
-        )["total"]
-        return float(total) if total is not None else 0
+        return float(
+            self.items.aggregate(
+                total=Sum(F("quantity") * F("product_unit_price"), default=0)
+            )["total"]
+        )
 
     def validate(self):
         if self.validated:
@@ -284,7 +244,7 @@ class BasketItem(AbstractBaseItem):
     )
 
     @classmethod
-    def from_product(cls, product: Product, quantity: int):
+    def from_product(cls, product: Product, quantity: int, basket: Basket):
         """Create a BasketItem with the same characteristics as the
         product passed in parameters, with the specified quantity.
 
@@ -293,9 +253,10 @@ class BasketItem(AbstractBaseItem):
             it yourself before saving the model.
         """
         return cls(
+            basket=basket,
             product_id=product.id,
             product_name=product.name,
-            type_id=product.product_type.id,
+            type_id=product.product_type_id,
             quantity=quantity,
             product_unit_price=product.selling_price,
         )

eboutic/schemas.py

@@ -0,0 +1,33 @@
+from ninja import ModelSchema, Schema
+from pydantic import Field, NonNegativeInt, PositiveInt, TypeAdapter
+
+from counter.models import BillingInfo
+
+
+class PurchaseItemSchema(Schema):
+    product_id: NonNegativeInt = Field(alias="id")
+    name: str
+    unit_price: float
+    quantity: PositiveInt
+
+
+# The eboutic deals with data that is dict mixed with JSON.
+# Hence it would be a hassle to manage it with a proper Schema class,
+# and we use a TypeAdapter instead
+PurchaseItemList = TypeAdapter(list[PurchaseItemSchema])
+
+
+class BillingInfoSchema(ModelSchema):
+    class Meta:
+        model = BillingInfo
+        fields = [
+            "customer",
+            "first_name",
+            "last_name",
+            "address_1",
+            "address_2",
+            "zip_code",
+            "city",
+            "country",
+        ]
+        fields_optional = ["customer"]

eboutic/static/eboutic/js/eboutic.js

@@ -30,22 +30,17 @@ function getCookie(name) {
  */
 function get_starting_items() {
     const cookie = getCookie(BASKET_ITEMS_COOKIE_NAME);
-    let output = [];
-
-    try {
-        // Django cookie backend does an utter mess on non-trivial data types
-        // so we must perform a conversion of our own
-        const biscuit = JSON.parse(cookie.replace(/\\054/g, ','));
-        output = Array.isArray(biscuit) ? biscuit : [];
-
-    } catch (e) {}
-
-    output.forEach(item => {
-        let el = document.getElementById(item.id);
-        el.classList.add("selected");
-    });
-
-    return output;
+    if (!cookie) {
+        return []
+    }
+    // Django cookie backend converts `,` to `\054`
+    let parsed = JSON.parse(cookie.replace(/\\054/g, ','));
+    if (typeof parsed === "string") {
+        // In some conditions, a second parsing is needed
+        parsed = JSON.parse(parsed);
+    }
+    const res = Array.isArray(parsed) ? parsed : [];
+    return res.filter((i) => !!document.getElementById(i.id))
 }
 
 document.addEventListener('alpine:init', () => {
@@ -63,7 +58,7 @@ document.addEventListener('alpine:init', () => {
 
         /**
          * Add 1 to the quantity of an item in the basket
-         * @param {BasketItem} item 
+         * @param {BasketItem} item
          */
         add(item) {
             item.quantity++;
@@ -72,18 +67,15 @@ document.addEventListener('alpine:init', () => {
 
         /**
          * Remove 1 to the quantity of an item in the basket
-         * @param {BasketItem} item_id 
+         * @param {BasketItem} item_id
          */
         remove(item_id) {
             const index = this.items.findIndex(e => e.id === item_id);
-            
+
             if (index < 0) return;
             this.items[index].quantity -= 1;
 
             if (this.items[index].quantity === 0) {
-                let el = document.getElementById(this.items[index].id);
-                el.classList.remove("selected");
-
                 this.items = this.items.filter((e) => e.id !== this.items[index].id);
             }
             this.set_cookies();
@@ -93,12 +85,6 @@ document.addEventListener('alpine:init', () => {
          * Remove all the items from the basket & cleans the catalog CSS classes
          */
         clear_basket() {
-            // We remove the class "selected" from all the items in the catalog
-            this.items.forEach(item => {
-                let el = document.getElementById(item.id);
-                el.classList.remove("selected");
-            })
-
             this.items = [];
             this.set_cookies();
         },
@@ -108,8 +94,11 @@ document.addEventListener('alpine:init', () => {
          * ! the cookie survives an hour
          */
         set_cookies() {
-            if (this.items.length === 0) document.cookie = `${BASKET_ITEMS_COOKIE_NAME}=;Max-Age=0`;
-            else document.cookie = `${BASKET_ITEMS_COOKIE_NAME}=${encodeURIComponent(JSON.stringify(this.items))};Max-Age=3600`;
+            if (this.items.length === 0) {
+                document.cookie = `${BASKET_ITEMS_COOKIE_NAME}=;Max-Age=0`;
+            } else {
+                document.cookie = `${BASKET_ITEMS_COOKIE_NAME}=${encodeURIComponent(JSON.stringify(this.items))};Max-Age=3600`;
+            }
         },
 
         /**
@@ -145,12 +134,10 @@ document.addEventListener('alpine:init', () => {
 
             // if the item is not in the basket, we create it
             // else we add + 1 to it
-            if (item === undefined) item = this.create_item(id, name, price);
-            else this.add(item);
-
-            if (item.quantity > 0) {
-                let el = document.getElementById(item.id);
-                el.classList.add("selected");
+            if (!item) {
+                item = this.create_item(id, name, price);
+            } else {
+                this.add(item);
             }
         },
     }))

eboutic/static/eboutic/js/makecommand.js

@@ -1,73 +1,70 @@
-document.addEventListener('alpine:init', () => {
-    Alpine.store('bank_payment_enabled', false)
+/**
+ * @readonly
+ * @enum {number}
+ */
+const BillingInfoReqState = {
+    SUCCESS: 1,
+    FAILURE: 2,
+    SENDING: 3,
+};
 
-    Alpine.store('billing_inputs', {
-        data: JSON.parse(et_data)["data"],
+document.addEventListener("alpine:init", () => {
+    Alpine.store("billing_inputs", {
+        data: et_data,
 
         async fill() {
             document.getElementById("bank-submit-button").disabled = true;
-            const request = new Request(et_data_url, {
-                method: "GET",
-                headers: {
-                    'Accept': 'application/json',
-                    'Content-Type': 'application/json',
-                },
-            });
-            const res = await fetch(request);
+            const res = await fetch(et_data_url);
             if (res.ok) {
-                const json = await res.json();
-                if (json["data"]) {
-                    this.data = json["data"];
-                }
+                this.data = await res.json();
                 document.getElementById("bank-submit-button").disabled = false;
             }
-        }
-    })
+        },
+    });
 
-    Alpine.data('billing_infos', () => ({
-        errors: [],
-        successful: false,
-        url: billing_info_exist ? edit_billing_info_url : create_billing_info_url,
+    Alpine.data("billing_infos", () => ({
+        /** @type {BillingInfoReqState | null} */
+        req_state: null,
 
         async send_form() {
+            this.req_state = BillingInfoReqState.SENDING;
             const form = document.getElementById("billing_info_form");
-            const submit_button = form.querySelector("input[type=submit]")
-            submit_button.disabled = true;
             document.getElementById("bank-submit-button").disabled = true;
-            this.successful = false
-
-            let payload = {};
-            for (const elem of form.querySelectorAll("input")) {
-                if (elem.type === "text" && elem.value) {
-                    payload[elem.name] = elem.value;
-                }
-            }
-            const country = form.querySelector("select");
-            if (country && country.value) {
-                payload[country.name] = country.value;
-            }
-            const request = new Request(this.url, {
-                method: "POST",
-                headers: {
-                    'Accept': 'application/json',
-                    'Content-Type': 'application/json',
-                    'X-CSRFToken': getCSRFToken(),
-                },
+            let payload = Object.fromEntries(
+                Array.from(form.querySelectorAll("input, select"))
+                    .filter((elem) => elem.type !== "submit" && elem.value)
+                    .map((elem) => [elem.name, elem.value]),
+            );
+            const res = await fetch(billing_info_url, {
+                method: "PUT",
                 body: JSON.stringify(payload),
             });
-            const res = await fetch(request);
-            const json = await res.json();
-            if (json["errors"]) {
-                this.errors = json["errors"];
-            } else {
-                this.errors = [];
-                this.successful = true;
-                this.url = edit_billing_info_url;
+            this.req_state = res.ok
+                ? BillingInfoReqState.SUCCESS
+                : BillingInfoReqState.FAILURE;
+            if (res.ok) {
                 Alpine.store("billing_inputs").fill();
             }
-            submit_button.disabled = false;
-        }
-    }))
-})
+        },
 
+        get_alert_color() {
+            if (this.req_state === BillingInfoReqState.SUCCESS) {
+                return "green";
+            }
+            if (this.req_state === BillingInfoReqState.FAILURE) {
+                return "red";
+            }
+            return "";
+        },
 
+        get_alert_message() {
+            if (this.req_state === BillingInfoReqState.SUCCESS) {
+                return billing_info_success_message;
+            }
+            if (this.req_state === BillingInfoReqState.FAILURE) {
+                return billing_info_failure_message;
+            }
+            return "";
+        },
+    }));
+});

eboutic/templates/eboutic/eboutic_main.jinja

@@ -29,7 +29,6 @@
             {% for error in errors %}
               <p style="margin: 0">{{ error }}</p>
             {% endfor %}
-            {% trans %}Your basket has been cleaned accordingly to those errors.{% endtrans %}
           </div>
         </div>
       {% endif %}
@@ -103,8 +102,12 @@
               </div>
               <div class="product-group">
                 {% for p in items %}
-                  <button id="{{ p.id }}" class="product-button"
-                          @click='add_from_catalog({{ p.id }}, {{ p.name|tojson }}, {{ p.selling_price }})'>
+                  <button
+                    id="{{ p.id }}"
+                    class="product-button"
+                    :class="{selected: items.some((i) => i.id === {{ p.id }})}"
+                    @click='add_from_catalog({{ p.id }}, {{ p.name|tojson }}, {{ p.selling_price }})'
+                  >
                     {% if p.icon %}
                       <img class="product-image" src="{{ p.icon.url }}"
                            alt="image de {{ p.name }}">

eboutic/templates/eboutic/eboutic_makecommand.jinja

@@ -37,7 +37,7 @@
         </table>
 
         <p>
-          <strong>{% trans %}Basket amount: {% endtrans %}{{ "%0.2f"|format(basket.get_total()) }} €</strong>
+          <strong>{% trans %}Basket amount: {% endtrans %}{{ "%0.2f"|format(basket.total) }} €</strong>
 
           {% if customer_amount != None %}
             <br>
@@ -47,49 +47,54 @@
             {% if not basket.contains_refilling_item %}
               <br>
               {% trans %}Remaining account amount: {% endtrans %}
-              <strong>{{ "%0.2f"|format(customer_amount|float - basket.get_total()) }} €</strong>
+              <strong>{{ "%0.2f"|format(customer_amount|float - basket.total) }} €</strong>
             {% endif %}
           {% endif %}
         </p>
         <br>
         {% if settings.SITH_EBOUTIC_CB_ENABLED %}
-          <div class="collapse" :class="{'shadow': collapsed}" x-data="{collapsed: false}" x-cloak>
+          <div
+            class="collapse"
+            :class="{'shadow': collapsed}"
+            x-data="{collapsed: !billing_info_exist}"
+            x-cloak
+          >
             <div class="collapse-header clickable" @click="collapsed = !collapsed">
               <span class="collapse-header-text">
-                {% trans %}Edit billing information{% endtrans %}
+                {% trans %}Billing information{% endtrans %}
               </span>
               <span class="collapse-header-icon" :class="{'reverse': collapsed}">
                 <i class="fa fa-caret-down"></i>
               </span>
             </div>
-            <form class="collapse-body" id="billing_info_form" method="post"
-                  x-show="collapsed" x-data="billing_infos"
-                  x-transition.scale.origin.top
-                  @submit.prevent="send_form()">
+            <form
+              class="collapse-body"
+              id="billing_info_form"
+              x-data="billing_infos"
+              x-show="collapsed"
+              x-transition.scale.origin.top
+              @submit.prevent="await send_form()"
+            >
               {% csrf_token %}
               {{ billing_form }}
               <br>
               <br>
-              <div x-show="errors.length > 0" class="alert alert-red" x-transition>
-                <div class="alert-main">
-                  <template x-for="error in errors">
-                    <div x-text="error.field + ' : ' + error.messages.join(', ')"></div>
-                  </template>
-                </div>
-                <div class="clickable" @click="errors = []">
+              <div
+                x-show="[BillingInfoReqState.SUCCESS, BillingInfoReqState.FAILURE].includes(req_state)"
+                class="alert"
+                :class="'alert-' + get_alert_color()"
+                x-transition
+              >
+                <div class="alert-main" x-text="get_alert_message()"></div>
+                <div class="clickable" @click="req_state = null">
                   <i class="fa fa-close"></i>
                 </div>
               </div>
-              <div x-show="successful" class="alert alert-green" x-transition>
-                <div class="alert-main">
-                  Informations de facturation enregistrées
-                </div>
-                <div class="clickable" @click="successful = false">
-                  <i class="fa fa-close"></i>
-                </div>
-              </div>
-              <input type="submit" class="btn btn-blue clickable"
-                     value="{% trans %}Validate{% endtrans %}">
+              <input
+                type="submit" class="btn btn-blue clickable"
+                value="{% trans %}Validate{% endtrans %}"
+                :disabled="req_state === BillingInfoReqState.SENDING"
+              >
             </form>
           </div>
           <br>
@@ -102,16 +107,21 @@
             </p>
           {% endif %}
           <form method="post" action="{{ settings.SITH_EBOUTIC_ET_URL }}" name="bank-pay-form">
-            <template x-data x-for="input in $store.billing_inputs.data">
-              <input type="hidden" :name="input['key']" :value="input['value']">
+            <template x-data x-for="[key, value] in Object.entries($store.billing_inputs.data)">
+              <input type="hidden" :name="key" :value="value">
             </template>
-            <input type="submit" id="bank-submit-button"
-                   {% if must_fill_billing_infos %}disabled="disabled"{% endif %}
-                   value="{% trans %}Pay with credit card{% endtrans %}"/>
+            <input
+              type="submit"
+              id="bank-submit-button"
+              {% if must_fill_billing_infos %}disabled="disabled"{% endif %}
+              value="{% trans %}Pay with credit card{% endtrans %}"
+            />
           </form>
         {% endif %}
         {% if basket.contains_refilling_item %}
           <p>{% trans %}AE account payment disabled because your basket contains refilling items.{% endtrans %}</p>
+        {% elif basket.total > user.account_balance %}
+          <p>{% trans %}AE account payment disabled because you do not have enough money remaining.{% endtrans %}</p>
         {% else %}
           <form method="post" action="{{ url('eboutic:pay_with_sith') }}" name="sith-pay-form">
             {% csrf_token %}
@@ -124,15 +134,16 @@
 
 {% block script %}
   <script>
-    const create_billing_info_url = '{{ url("counter:create_billing_info", user_id=request.user.id) }}'
-    const edit_billing_info_url = '{{ url("counter:edit_billing_info", user_id=request.user.id) }}';
-    const et_data_url = '{{ url("eboutic:et_data") }}'
-    let billing_info_exist = {{ "true" if billing_infos else "false" }}
+    const billing_info_url = '{{ url("api:put_billing_info", user_id=request.user.id) }}';
+    const et_data_url = '{{ url("api:etransaction_data") }}';
+    const billing_info_exist = {{ "true" if billing_infos else "false" }};
+    const billing_info_success_message = "{% trans %}Billing info registration success{% endtrans %}";
+    const billing_info_failure_message = "{% trans %}Billing info registration failure{% endtrans %}";
 
     {% if billing_infos %}
-      const et_data = {{ billing_infos|tojson }}
+      const et_data = {{ billing_infos|safe }}
     {% else %}
-      const et_data = '{"data": []}'
+      const et_data = {}
     {% endif %}
   </script>
   {{ super() }}

eboutic/tests/tests.py

@@ -36,7 +36,7 @@ from django.urls import reverse
 
 from core.models import User
 from counter.models import Counter, Customer, Product, Selling
-from eboutic.models import Basket
+from eboutic.models import Basket, BasketItem
 
 
 class TestEboutic(TestCase):
@@ -60,14 +60,14 @@ class TestEboutic(TestCase):
         basket = Basket.objects.create(user=user)
         session["basket_id"] = basket.id
         session.save()
-        basket.add_product(self.barbar, 3)
-        basket.add_product(self.cotis)
+        BasketItem.from_product(self.barbar, 3, basket).save()
+        BasketItem.from_product(self.cotis, 1, basket).save()
         return basket
 
     def generate_bank_valid_answer(self) -> str:
         basket = Basket.from_session(self.client.session)
         basket_id = basket.id
-        amount = int(basket.get_total() * 100)
+        amount = int(basket.total * 100)
         query = f"Amount={amount}&BasketID={basket_id}&Auto=42&Error=00000"
         with open("./eboutic/tests/private_key.pem", "br") as f:
             PRIVKEY = f.read()
@@ -88,7 +88,7 @@ class TestEboutic(TestCase):
         self.subscriber.customer.amount = 100  # give money before test
         self.subscriber.customer.save()
         basket = self.get_busy_basket(self.subscriber)
-        amount = basket.get_total()
+        amount = basket.total
         response = self.client.post(reverse("eboutic:pay_with_sith"))
         self.assertRedirects(response, "/eboutic/pay/success/")
         new_balance = Customer.objects.get(user=self.subscriber).amount
@@ -99,7 +99,7 @@ class TestEboutic(TestCase):
     def test_buy_with_sith_account_no_money(self):
         self.client.force_login(self.subscriber)
         basket = self.get_busy_basket(self.subscriber)
-        initial = basket.get_total() - 1  # just not enough to complete the sale
+        initial = basket.total - 1  # just not enough to complete the sale
         self.subscriber.customer.amount = initial
         self.subscriber.customer.save()
         response = self.client.post(reverse("eboutic:pay_with_sith"))
@@ -135,7 +135,7 @@ class TestEboutic(TestCase):
         cotis = basket.items.filter(product_name="Cotis 2 semestres").first()
         assert cotis is not None
         assert cotis.quantity == 1
-        assert basket.get_total() == 3 * 1.7 + 28
+        assert basket.total == 3 * 1.7 + 28
 
     def test_submit_empty_basket(self):
         self.client.force_login(self.subscriber)
@@ -151,7 +151,7 @@ class TestEboutic(TestCase):
         ]"""
         response = self.client.get(reverse("eboutic:command"))
         cookie = self.client.cookies["basket_items"].OutputString()
-        assert 'basket_items=""' in cookie
+        assert 'basket_items="[]"' in cookie
         assert "Path=/eboutic" in cookie
         self.assertRedirects(response, "/eboutic/")
 

eboutic/views.py

@@ -16,7 +16,6 @@
 import base64
 import json
 from datetime import datetime
-from urllib.parse import unquote
 
 import sentry_sdk
 from cryptography.exceptions import InvalidSignature
@@ -26,6 +25,7 @@ from cryptography.hazmat.primitives.hashes import SHA1
 from cryptography.hazmat.primitives.serialization import load_pem_public_key
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
+from django.contrib.auth.mixins import LoginRequiredMixin
 from django.core.exceptions import SuspiciousOperation
 from django.db import DatabaseError, transaction
 from django.http import HttpRequest, HttpResponse
@@ -37,7 +37,14 @@ from django.views.generic import TemplateView, View
 from counter.forms import BillingInfoForm
 from counter.models import Counter, Customer, Product
 from eboutic.forms import BasketForm
-from eboutic.models import Basket, Invoice, InvoiceItem, get_eboutic_products
+from eboutic.models import (
+    Basket,
+    BasketItem,
+    Invoice,
+    InvoiceItem,
+    get_eboutic_products,
+)
+from eboutic.schemas import PurchaseItemList, PurchaseItemSchema
 
 
 @login_required
@@ -75,43 +82,46 @@ def payment_result(request, result: str) -> HttpResponse:
     return render(request, "eboutic/eboutic_payment_result.jinja", context)
 
 
-class EbouticCommand(TemplateView):
+class EbouticCommand(LoginRequiredMixin, TemplateView):
     template_name = "eboutic/eboutic_makecommand.jinja"
+    basket: Basket
 
     @method_decorator(login_required)
     def post(self, request, *args, **kwargs):
         return redirect("eboutic:main")
 
-    @method_decorator(login_required)
     def get(self, request: HttpRequest, *args, **kwargs):
         form = BasketForm(request)
         if not form.is_valid():
-            request.session["errors"] = form.get_error_messages()
+            request.session["errors"] = form.errors
             request.session.modified = True
             res = redirect("eboutic:main")
-            res.set_cookie("basket_items", form.get_cleaned_cookie(), path="/eboutic")
+            res.set_cookie(
+                "basket_items",
+                PurchaseItemList.dump_json(form.cleaned_data, by_alias=True).decode(),
+                path="/eboutic",
+            )
             return res
-
         basket = Basket.from_session(request.session)
         if basket is not None:
-            basket.clear()
+            basket.items.all().delete()
         else:
             basket = Basket.objects.create(user=request.user)
             request.session["basket_id"] = basket.id
             request.session.modified = True
 
-        items = json.loads(unquote(request.COOKIES["basket_items"]))
-        items.sort(key=lambda item: item["id"])
-        ids = [item["id"] for item in items]
-        quantities = [item["quantity"] for item in items]
-        products = Product.objects.filter(id__in=ids)
-        for product, qty in zip(products, quantities):
-            basket.add_product(product, qty)
-        kwargs["basket"] = basket
-        return self.render_to_response(self.get_context_data(**kwargs))
+        items: list[PurchaseItemSchema] = form.cleaned_data
+        pks = {item.product_id for item in items}
+        products = {p.pk: p for p in Product.objects.filter(pk__in=pks)}
+        db_items = []
+        for pk in pks:
+            quantity = sum(i.quantity for i in items if i.product_id == pk)
+            db_items.append(BasketItem.from_product(products[pk], quantity, basket))
+        BasketItem.objects.bulk_create(db_items)
+        self.basket = basket
+        return super().get(request)
 
     def get_context_data(self, **kwargs):
-        # basket is already in kwargs when the method is called
         default_billing_info = None
         if hasattr(self.request.user, "customer"):
             customer = self.request.user.customer
@@ -124,9 +134,8 @@ class EbouticCommand(TemplateView):
         if not kwargs["must_fill_billing_infos"]:
             # the user has already filled its billing_infos, thus we can
             # get it without expecting an error
-            data = kwargs["basket"].get_e_transaction_data()
-            data = {"data": [{"key": key, "value": val} for key, val in data]}
-            kwargs["billing_infos"] = json.dumps(data)
+            kwargs["billing_infos"] = dict(self.basket.get_e_transaction_data())
+        kwargs["basket"] = self.basket
         kwargs["billing_form"] = BillingInfoForm(instance=default_billing_info)
         return kwargs
 
@@ -149,29 +158,32 @@ def pay_with_sith(request):
     refilling = settings.SITH_COUNTER_PRODUCTTYPE_REFILLING
     if basket is None or basket.items.filter(type_id=refilling).exists():
         return redirect("eboutic:main")
-    c = Customer.objects.filter(user__id=basket.user.id).first()
+    c = Customer.objects.filter(user__id=basket.user_id).first()
     if c is None:
         return redirect("eboutic:main")
-    if c.amount < basket.get_total():
+    if c.amount < basket.total:
+        res = redirect("eboutic:payment_result", "failure")
+        res.delete_cookie("basket_items", "/eboutic")
+        return res
+    eboutic = Counter.objects.get(type="EBOUTIC")
+    sales = basket.generate_sales(eboutic, c.user, "SITH_ACCOUNT")
+    try:
+        with transaction.atomic():
+            # Selling.save has some important business logic in it.
+            # Do not bulk_create this
+            for sale in sales:
+                sale.save()
+            basket.delete()
+        request.session.pop("basket_id", None)
+        res = redirect("eboutic:payment_result", "success")
+    except DatabaseError as e:
+        with sentry_sdk.push_scope() as scope:
+            scope.user = {"username": request.user.username}
+            scope.set_extra("someVariable", e.__repr__())
+            sentry_sdk.capture_message(
+                f"Erreur le {datetime.now()} dans eboutic.pay_with_sith"
+            )
         res = redirect("eboutic:payment_result", "failure")
-    else:
-        eboutic = Counter.objects.filter(type="EBOUTIC").first()
-        sales = basket.generate_sales(eboutic, c.user, "SITH_ACCOUNT")
-        try:
-            with transaction.atomic():
-                for sale in sales:
-                    sale.save()
-                basket.delete()
-            request.session.pop("basket_id", None)
-            res = redirect("eboutic:payment_result", "success")
-        except DatabaseError as e:
-            with sentry_sdk.push_scope() as scope:
-                scope.user = {"username": request.user.username}
-                scope.set_extra("someVariable", e.__repr__())
-                sentry_sdk.capture_message(
-                    f"Erreur le {datetime.now()} dans eboutic.pay_with_sith"
-                )
-            res = redirect("eboutic:payment_result", "failure")
     res.delete_cookie("basket_items", "/eboutic")
     return res
 
@@ -205,7 +217,7 @@ class EtransactionAutoAnswer(View):
                     )
                     if b is None:
                         raise SuspiciousOperation("Basket does not exists")
-                    if int(b.get_total() * 100) != int(request.GET["Amount"]):
+                    if int(b.total * 100) != int(request.GET["Amount"]):
                         raise SuspiciousOperation(
                             "Basket total and amount do not match"
                         )

sith/settings.py

@@ -618,11 +618,14 @@ SITH_EBOUTIC_CB_ENABLED = True
 SITH_EBOUTIC_ET_URL = (
     "https://preprod-tpeweb.e-transactions.fr/cgi/MYchoix_pagepaiement.cgi"
 )
-SITH_EBOUTIC_PBX_SITE = "4000666"
-SITH_EBOUTIC_PBX_RANG = "42"
-SITH_EBOUTIC_PBX_IDENTIFIANT = "123456789"
+SITH_EBOUTIC_PBX_SITE = "1999888"
+SITH_EBOUTIC_PBX_RANG = "32"
+SITH_EBOUTIC_PBX_IDENTIFIANT = "2"
 SITH_EBOUTIC_HMAC_KEY = binascii.unhexlify(
-    "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF"
+    "0123456789ABCDEF0123456789ABCDEF"
+    "0123456789ABCDEF0123456789ABCDEF"
+    "0123456789ABCDEF0123456789ABCDEF"
+    "0123456789ABCDEF0123456789ABCDEF"
 )
 SITH_EBOUTIC_PUB_KEY = ""
 with open(os.path.join(os.path.dirname(__file__), "et_keys/pubkey.pem")) as f: