From d4b9c3afb1b83d3e0c143fadb590197b3150599e Mon Sep 17 00:00:00 2001 From: Sli Date: Sat, 7 Dec 2024 17:28:34 +0100 Subject: [PATCH] Make StudentCardFormView fragment only --- core/templates/core/user_preferences.jinja | 37 +-- core/views/user.py | 4 - counter/templates/counter/counter_click.jinja | 2 +- .../create_student_card.jinja} | 10 +- counter/tests/test_customer.py | 280 ++++++++++++------ counter/urls.py | 6 - counter/utils.py | 12 +- counter/views/student_card.py | 71 ++--- 8 files changed, 234 insertions(+), 188 deletions(-) rename counter/templates/counter/{add_student_card_fragment.jinja => fragments/create_student_card.jinja} (61%) diff --git a/core/templates/core/user_preferences.jinja b/core/templates/core/user_preferences.jinja index 0cf4bd57..38749fde 100644 --- a/core/templates/core/user_preferences.jinja +++ b/core/templates/core/user_preferences.jinja @@ -38,32 +38,17 @@ {% if profile.customer %}

{% trans %}Student cards{% endtrans %}

- {% if profile.customer.student_cards.exists() %} - - {% else %} - {% trans %}No student card registered.{% endtrans %} -

- {% trans %}You can add a card by asking at a counter or add it yourself here. If you want to manually - add a student card yourself, you'll need a NFC reader. We store the UID of the card which is 14 characters long.{% endtrans %} -

- {% endif %} - -
- {% csrf_token %} - {{ student_card_form.as_p() }} - -
+

+ {% trans %}You can add a card by asking at a counter or add it yourself here. If you want to manually + add a student card yourself, you'll need a NFC reader. We store the UID of the card which is 14 characters long.{% endtrans %} +

+
+
+
{% endif %} {% endblock %} \ No newline at end of file diff --git a/core/views/user.py b/core/views/user.py index e9694a92..5a797620 100644 --- a/core/views/user.py +++ b/core/views/user.py @@ -70,7 +70,6 @@ from core.views.forms import ( UserGodfathersForm, UserProfileForm, ) -from counter.forms import StudentCardForm from counter.models import Refilling, Selling from eboutic.models import Invoice from subscription.models import Subscription @@ -576,9 +575,6 @@ class UserPreferencesView(UserTabsMixin, CanEditMixin, UpdateView): hasattr(self.object, "trombi_user") and self.request.user.trombi_user.trombi ): kwargs["trombi_form"] = UserTrombiForm() - - if hasattr(self.object, "customer"): - kwargs["student_card_form"] = StudentCardForm() return kwargs diff --git a/counter/templates/counter/counter_click.jinja b/counter/templates/counter/counter_click.jinja index 323a5905..9c95292e 100644 --- a/counter/templates/counter/counter_click.jinja +++ b/counter/templates/counter/counter_click.jinja @@ -32,7 +32,7 @@ {% if counter.type == 'BAR' %}
diff --git a/counter/templates/counter/add_student_card_fragment.jinja b/counter/templates/counter/fragments/create_student_card.jinja similarity index 61% rename from counter/templates/counter/add_student_card_fragment.jinja rename to counter/templates/counter/fragments/create_student_card.jinja index aa2150e1..7cd05ba9 100644 --- a/counter/templates/counter/add_student_card_fragment.jinja +++ b/counter/templates/counter/fragments/create_student_card.jinja @@ -1,7 +1,6 @@

{% trans %}Add a student card{% endtrans %}

{% for card in student_cards %} -
  • {{ card.uid }}
    • +
  • + {{ card.uid }} + + {% trans %}Delete{% endtrans %} + +
    • {% endfor %} {% else %} - {% trans %}No card registered{% endtrans %} + {% trans %}No student card registered.{% endtrans %} {% endif %}
    diff --git a/counter/tests/test_customer.py b/counter/tests/test_customer.py index dd7a5ed6..f7e599e6 100644 --- a/counter/tests/test_customer.py +++ b/counter/tests/test_customer.py @@ -1,15 +1,27 @@ import json import string +from datetime import timedelta import pytest +from django.conf import settings +from django.contrib.auth.base_user import make_password from django.test import Client, TestCase from django.urls import reverse +from django.utils.timezone import now from model_bakery import baker -from core.baker_recipes import subscriber_user +from club.models import Membership +from core.baker_recipes import board_user, subscriber_user from core.models import User from counter.baker_recipes import refill_recipe, sale_recipe -from counter.models import BillingInfo, Counter, Customer, Refilling, Selling +from counter.models import ( + BillingInfo, + Counter, + Customer, + Refilling, + Selling, + StudentCard, +) @pytest.mark.django_db @@ -162,43 +174,65 @@ class TestStudentCard(TestCase): @classmethod def setUpTestData(cls): - cls.krophil = User.objects.get(username="krophil") - cls.sli = User.objects.get(username="sli") - cls.skia = User.objects.get(username="skia") - cls.root = User.objects.get(username="root") + cls.customer = subscriber_user.make() + cls.customer.save() + cls.barmen = subscriber_user.make(password=make_password("plop")) + cls.board_admin = board_user.make() + cls.club_admin = baker.make(User) + cls.root = baker.make(User, is_superuser=True) + cls.subscriber = subscriber_user.make() - cls.counter = Counter.objects.get(id=2) - cls.ae_counter = Counter.objects.get(name="AE") + cls.counter = baker.make(Counter, type="BAR") + cls.counter.sellers.add(cls.barmen) + + cls.club_counter = baker.make(Counter) + baker.make( + Membership, + start_date=now() - timedelta(days=30), + club=cls.club_counter.club, + role=settings.SITH_CLUB_ROLES_ID["Board member"], + user=cls.club_admin, + ) + + cls.valid_card = baker.make( + StudentCard, customer=cls.customer.customer, uid="8A89B82018B0A0" + ) def setUp(self): # Auto login on counter self.client.post( reverse("counter:login", args=[self.counter.id]), - {"username": "krophil", "password": "plop"}, + {"username": self.barmen.username, "password": "plop"}, ) def test_search_user_with_student_card(self): response = self.client.post( reverse("counter:details", args=[self.counter.id]), - {"code": "9A89B82018B0A0"}, + {"code": self.valid_card.uid}, ) assert response.url == reverse( "counter:click", - kwargs={"counter_id": self.counter.id, "user_id": self.sli.id}, + kwargs={"counter_id": self.counter.id, "user_id": self.customer.id}, ) def test_add_student_card_from_counter(self): # Test card with mixed letters and numbers response = self.client.post( reverse( - "counter:add_student_card_fragment", + "counter:add_student_card", kwargs={ - "counter_id": self.counter.id, - "customer_id": self.sli.customer.pk, + "customer_id": self.customer.customer.pk, }, ), {"uid": "8B90734A802A8F"}, + HTTP_REFERER=reverse( + "counter:click", + kwargs={ + "counter_id": self.counter.id, + "user_id": self.customer.customer.pk, + }, + ), ) assert response.status_code == 302 self.assertContains(self.client.get(response.url), text="8B90734A802A8F") @@ -206,13 +240,19 @@ class TestStudentCard(TestCase): # Test card with only numbers response = self.client.post( reverse( - "counter:add_student_card_fragment", + "counter:add_student_card", kwargs={ - "counter_id": self.counter.id, - "customer_id": self.sli.customer.pk, + "customer_id": self.customer.customer.pk, }, ), {"uid": "04786547890123"}, + HTTP_REFERER=reverse( + "counter:click", + kwargs={ + "counter_id": self.counter.id, + "user_id": self.customer.customer.pk, + }, + ), ) assert response.status_code == 302 self.assertContains(self.client.get(response.url), text="04786547890123") @@ -220,13 +260,19 @@ class TestStudentCard(TestCase): # Test card with only letters response = self.client.post( reverse( - "counter:add_student_card_fragment", + "counter:add_student_card", kwargs={ - "counter_id": self.counter.id, - "customer_id": self.sli.customer.pk, + "customer_id": self.customer.customer.pk, }, ), {"uid": "ABCAAAFAAFAAAB"}, + HTTP_REFERER=reverse( + "counter:click", + kwargs={ + "counter_id": self.counter.id, + "user_id": self.customer.customer.pk, + }, + ), ) assert response.status_code == 302 self.assertContains(self.client.get(response.url), text="ABCAAAFAAFAAAB") @@ -235,26 +281,38 @@ class TestStudentCard(TestCase): # UID too short response = self.client.post( reverse( - "counter:add_student_card_fragment", + "counter:add_student_card", kwargs={ - "counter_id": self.counter.id, - "customer_id": self.sli.customer.pk, + "customer_id": self.customer.customer.pk, }, ), {"uid": "8B90734A802A8"}, + HTTP_REFERER=reverse( + "counter:click", + kwargs={ + "counter_id": self.counter.id, + "user_id": self.customer.customer.pk, + }, + ), ) self.assertContains(response, text="Cet UID est invalide") # UID too long response = self.client.post( reverse( - "counter:add_student_card_fragment", + "counter:add_student_card", kwargs={ - "counter_id": self.counter.id, - "customer_id": self.sli.customer.pk, + "customer_id": self.customer.customer.pk, }, ), {"uid": "8B90734A802A8FA"}, + HTTP_REFERER=reverse( + "counter:click", + kwargs={ + "counter_id": self.counter.id, + "user_id": self.customer.customer.pk, + }, + ), ) self.assertContains(response, text="Cet UID est invalide") self.assertContains( @@ -265,13 +323,19 @@ class TestStudentCard(TestCase): # Test with already existing card response = self.client.post( reverse( - "counter:add_student_card_fragment", + "counter:add_student_card", kwargs={ - "counter_id": self.counter.id, - "customer_id": self.sli.customer.pk, + "customer_id": self.customer.customer.pk, + }, + ), + {"uid": self.valid_card.uid}, + HTTP_REFERER=reverse( + "counter:click", + kwargs={ + "counter_id": self.counter.id, + "user_id": self.customer.customer.pk, }, ), - {"uid": "9A89B82018B0A0"}, ) self.assertContains(response, text="Cet UID est invalide") self.assertContains( @@ -281,26 +345,38 @@ class TestStudentCard(TestCase): # Test with lowercase response = self.client.post( reverse( - "counter:add_student_card_fragment", + "counter:add_student_card", kwargs={ - "counter_id": self.counter.id, - "customer_id": self.sli.customer.pk, + "customer_id": self.customer.customer.pk, }, ), {"uid": "8b90734a802a9f"}, + HTTP_REFERER=reverse( + "counter:click", + kwargs={ + "counter_id": self.counter.id, + "user_id": self.customer.customer.pk, + }, + ), ) self.assertContains(response, text="Cet UID est invalide") # Test with white spaces response = self.client.post( reverse( - "counter:add_student_card_fragment", + "counter:add_student_card", kwargs={ - "counter_id": self.counter.id, - "customer_id": self.sli.customer.pk, + "customer_id": self.customer.customer.pk, }, ), {"uid": " "}, + HTTP_REFERER=reverse( + "counter:click", + kwargs={ + "counter_id": self.counter.id, + "user_id": self.customer.customer.pk, + }, + ), ) self.assertContains(response, text="Cet UID est invalide") self.assertContains(response, text="Ce champ est obligatoire.") @@ -309,52 +385,58 @@ class TestStudentCard(TestCase): # Send to a counter where you aren't logged in self.client.post( reverse("counter:logout", args=[self.counter.id]), - {"user_id": self.krophil.id}, + {"user_id": self.barmen.id}, ) def send_valid_request(client, counter_id): return client.post( reverse( - "counter:add_student_card_fragment", + "counter:add_student_card", kwargs={ - "counter_id": counter_id, - "customer_id": self.sli.customer.pk, + "customer_id": self.customer.customer.pk, }, ), {"uid": "8B90734A802A8F"}, + HTTP_REFERER=reverse( + "counter:click", + kwargs={ + "counter_id": counter_id, + "user_id": self.customer.customer.pk, + }, + ), ) assert send_valid_request(self.client, self.counter.id).status_code == 403 # Send to a non bar counter - self.client.force_login(self.skia) - assert send_valid_request(self.client, self.ae_counter.id) + self.client.force_login(self.club_admin) + assert send_valid_request(self.client, self.club_counter.id).status_code == 403 def test_delete_student_card_with_owner(self): - self.client.force_login(self.sli) + self.client.force_login(self.customer) self.client.post( reverse( "counter:delete_student_card", kwargs={ - "customer_id": self.sli.customer.pk, - "card_id": self.sli.customer.student_cards.first().id, + "customer_id": self.customer.customer.pk, + "card_id": self.customer.customer.student_cards.first().id, }, ) ) - assert not self.sli.customer.student_cards.exists() + assert not self.customer.customer.student_cards.exists() def test_delete_student_card_with_board_member(self): - self.client.force_login(self.skia) + self.client.force_login(self.board_admin) self.client.post( reverse( "counter:delete_student_card", kwargs={ - "customer_id": self.sli.customer.pk, - "card_id": self.sli.customer.student_cards.first().id, + "customer_id": self.customer.customer.pk, + "card_id": self.customer.customer.student_cards.first().id, }, ) ) - assert not self.sli.customer.student_cards.exists() + assert not self.customer.customer.student_cards.exists() def test_delete_student_card_with_root(self): self.client.force_login(self.root) @@ -362,100 +444,107 @@ class TestStudentCard(TestCase): reverse( "counter:delete_student_card", kwargs={ - "customer_id": self.sli.customer.pk, - "card_id": self.sli.customer.student_cards.first().id, + "customer_id": self.customer.customer.pk, + "card_id": self.customer.customer.student_cards.first().id, }, ) ) - assert not self.sli.customer.student_cards.exists() + assert not self.customer.customer.student_cards.exists() def test_delete_student_card_fail(self): - self.client.force_login(self.krophil) + self.client.force_login(self.subscriber) response = self.client.post( reverse( "counter:delete_student_card", kwargs={ - "customer_id": self.sli.customer.pk, - "card_id": self.sli.customer.student_cards.first().id, + "customer_id": self.customer.customer.pk, + "card_id": self.customer.customer.student_cards.first().id, }, ) ) assert response.status_code == 403 - assert self.sli.customer.student_cards.exists() + assert self.customer.customer.student_cards.exists() def test_add_student_card_from_user_preferences(self): # Test with owner of the card - self.client.force_login(self.sli) - self.client.post( + self.client.force_login(self.customer) + response = self.client.post( reverse( - "counter:add_student_card", kwargs={"customer_id": self.sli.customer.pk} + "counter:add_student_card", + kwargs={"customer_id": self.customer.customer.pk}, ), {"uid": "8B90734A802A8F"}, ) - response = self.client.get( - reverse("core:user_prefs", kwargs={"user_id": self.sli.id}) - ) + assert response.status_code == 302 + + response = self.client.get(response.url) self.assertContains(response, text="8B90734A802A8F") # Test with board member - self.client.force_login(self.skia) - self.client.post( + self.client.force_login(self.board_admin) + response = self.client.post( reverse( - "counter:add_student_card", kwargs={"customer_id": self.sli.customer.pk} + "counter:add_student_card", + kwargs={"customer_id": self.customer.customer.pk}, ), {"uid": "8B90734A802A8A"}, ) - response = self.client.get( - reverse("core:user_prefs", kwargs={"user_id": self.sli.id}) - ) + assert response.status_code == 302 + + response = self.client.get(response.url) self.assertContains(response, text="8B90734A802A8A") # Test card with only numbers - self.client.post( + response = self.client.post( reverse( - "counter:add_student_card", kwargs={"customer_id": self.sli.customer.pk} + "counter:add_student_card", + kwargs={"customer_id": self.customer.customer.pk}, ), {"uid": "04786547890123"}, ) - response = self.client.get( - reverse("core:user_prefs", kwargs={"user_id": self.sli.id}) - ) + assert response.status_code == 302 + + response = self.client.get(response.url) self.assertContains(response, text="04786547890123") # Test card with only letters - self.client.post( + response = self.client.post( reverse( - "counter:add_student_card", kwargs={"customer_id": self.sli.customer.pk} + "counter:add_student_card", + kwargs={"customer_id": self.customer.customer.pk}, ), {"uid": "ABCAAAFAAFAAAB"}, ) - response = self.client.get( - reverse("core:user_prefs", kwargs={"user_id": self.sli.id}) - ) + + assert response.status_code == 302 + + response = self.client.get(response.url) self.assertContains(response, text="ABCAAAFAAFAAAB") # Test with root self.client.force_login(self.root) - self.client.post( + response = self.client.post( reverse( - "counter:add_student_card", kwargs={"customer_id": self.sli.customer.pk} + "counter:add_student_card", + kwargs={"customer_id": self.customer.customer.pk}, ), {"uid": "8B90734A802A8B"}, ) - response = self.client.get( - reverse("core:user_prefs", kwargs={"user_id": self.sli.id}) - ) + assert response.status_code == 302 + + response = self.client.get(response.url) self.assertContains(response, text="8B90734A802A8B") def test_add_student_card_from_user_preferences_fail(self): - self.client.force_login(self.sli) + self.client.force_login(self.customer) # UID too short response = self.client.post( reverse( - "counter:add_student_card", kwargs={"customer_id": self.sli.customer.pk} + "counter:add_student_card", + kwargs={"customer_id": self.customer.customer.pk}, ), {"uid": "8B90734A802A8"}, ) @@ -465,7 +554,8 @@ class TestStudentCard(TestCase): # UID too long response = self.client.post( reverse( - "counter:add_student_card", kwargs={"customer_id": self.sli.customer.pk} + "counter:add_student_card", + kwargs={"customer_id": self.customer.customer.pk}, ), {"uid": "8B90734A802A8FA"}, ) @@ -474,9 +564,10 @@ class TestStudentCard(TestCase): # Test with already existing card response = self.client.post( reverse( - "counter:add_student_card", kwargs={"customer_id": self.sli.customer.pk} + "counter:add_student_card", + kwargs={"customer_id": self.customer.customer.pk}, ), - {"uid": "9A89B82018B0A0"}, + {"uid": self.valid_card.uid}, ) self.assertContains( response, text="Un objet Student card avec ce champ Uid existe déjà." @@ -485,7 +576,8 @@ class TestStudentCard(TestCase): # Test with lowercase response = self.client.post( reverse( - "counter:add_student_card", kwargs={"customer_id": self.sli.customer.pk} + "counter:add_student_card", + kwargs={"customer_id": self.customer.customer.pk}, ), {"uid": "8b90734a802a9f"}, ) @@ -494,17 +586,19 @@ class TestStudentCard(TestCase): # Test with white spaces response = self.client.post( reverse( - "counter:add_student_card", kwargs={"customer_id": self.sli.customer.pk} + "counter:add_student_card", + kwargs={"customer_id": self.customer.customer.pk}, ), {"uid": " " * 14}, ) self.assertContains(response, text="Cet UID est invalide") # Test with unauthorized user - self.client.force_login(self.krophil) + self.client.force_login(self.subscriber) response = self.client.post( reverse( - "counter:add_student_card", kwargs={"customer_id": self.sli.customer.pk} + "counter:add_student_card", + kwargs={"customer_id": self.customer.customer.pk}, ), {"uid": "8B90734A802A8F"}, ) diff --git a/counter/urls.py b/counter/urls.py index ab93b586..e196894f 100644 --- a/counter/urls.py +++ b/counter/urls.py @@ -54,7 +54,6 @@ from counter.views.home import ( from counter.views.invoice import InvoiceCallView from counter.views.student_card import ( StudentCardDeleteView, - StudentCardFormFragmentView, StudentCardFormView, ) @@ -81,11 +80,6 @@ urlpatterns = [ StudentCardFormView.as_view(), name="add_student_card", ), - path( - "customer//card/add/counter//", - StudentCardFormFragmentView.as_view(), - name="add_student_card_fragment", - ), path( "customer//card/delete//", StudentCardDeleteView.as_view(), diff --git a/counter/utils.py b/counter/utils.py index 2b9b6fd6..499b2d8e 100644 --- a/counter/utils.py +++ b/counter/utils.py @@ -22,14 +22,22 @@ def is_logged_in_counter(request: HttpRequest) -> bool: to the counter) - The current session has a counter token associated with it. - A counter with this token exists. + - The counter is open """ referer_ok = ( "HTTP_REFERER" in request.META and resolve(urlparse(request.META["HTTP_REFERER"]).path).app_name == "counter" ) - return ( + has_token = ( (referer_ok or request.resolver_match.app_name == "counter") and "counter_token" in request.session and request.session["counter_token"] - and Counter.objects.filter(token=request.session["counter_token"]).exists() + ) + if not has_token: + return False + + return ( + Counter.objects.annotate_is_open() + .filter(token=request.session["counter_token"], is_open=True) + .exists() ) diff --git a/counter/views/student_card.py b/counter/views/student_card.py index a79fa8fd..882069da 100644 --- a/counter/views/student_card.py +++ b/counter/views/student_card.py @@ -13,14 +13,17 @@ # # + from django.core.exceptions import PermissionDenied +from django.http import HttpRequest from django.shortcuts import get_object_or_404 from django.urls import reverse_lazy from django.views.generic.edit import DeleteView, FormView -from core.views import AllowFragment, CanEditMixin +from core.views import CanEditMixin from counter.forms import StudentCardForm -from counter.models import Counter, Customer, StudentCard +from counter.models import Customer, StudentCard +from counter.utils import is_logged_in_counter class StudentCardDeleteView(DeleteView, CanEditMixin): @@ -40,16 +43,22 @@ class StudentCardDeleteView(DeleteView, CanEditMixin): ) -class StudentCardFormView(AllowFragment, FormView): - """Add a new student card.""" +class StudentCardFormView(FormView): + """Add a new student card. This is a fragment view !""" form_class = StudentCardForm - template_name = "core/create.jinja" + template_name = "counter/fragments/create_student_card.jinja" - def dispatch(self, request, *args, **kwargs): - self.customer = get_object_or_404(Customer, pk=kwargs["customer_id"]) - if not StudentCard.can_create(self.customer, request.user): + def dispatch(self, request: HttpRequest, *args, **kwargs): + self.customer = get_object_or_404( + Customer.objects.prefetch_related("student_cards"), pk=kwargs["customer_id"] + ) + + if not is_logged_in_counter(request) and not StudentCard.can_create( + self.customer, request.user + ): raise PermissionDenied + return super().dispatch(request, *args, **kwargs) def form_valid(self, form): @@ -58,56 +67,12 @@ class StudentCardFormView(AllowFragment, FormView): StudentCard(customer=self.customer, uid=data["uid"]).save() return res - def get_success_url(self, **kwargs): - return reverse_lazy( - "core:user_prefs", kwargs={"user_id": self.customer.user.pk} - ) - - -class StudentCardFormFragmentView(FormView): - """ - Add a new student card from a counter - This is a fragment only view which integrates with counter_click.jinja - """ - - form_class = StudentCardForm - template_name = "counter/add_student_card_fragment.jinja" - - def dispatch(self, request, *args, **kwargs): - self.counter = get_object_or_404( - Counter.objects.annotate_is_open(), pk=kwargs["counter_id"] - ) - self.customer = get_object_or_404( - Customer.objects.prefetch_related("student_cards"), pk=kwargs["customer_id"] - ) - if not ( - self.counter.type == "BAR" - and "counter_token" in request.session - and request.session["counter_token"] == self.counter.token - and self.counter.is_open - ): - raise PermissionDenied - return super().dispatch(request, *args, **kwargs) - - def form_valid(self, form): - data = form.clean() - res = super().form_valid(form) - StudentCard(customer=self.customer, uid=data["uid"]).save() - return res - def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) - context["counter"] = self.counter context["customer"] = self.customer context["action"] = self.request.path context["student_cards"] = self.customer.student_cards.all() return context def get_success_url(self, **kwargs): - return reverse_lazy( - "counter:add_student_card_fragment", - kwargs={ - "customer_id": self.customer.pk, - "counter_id": self.counter.id, - }, - ) + return self.request.path