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)