From b81cf49d0aca043c576405983de9d865556b04bc Mon Sep 17 00:00:00 2001 From: Sli Date: Thu, 14 Nov 2024 16:17:10 +0100 Subject: [PATCH] Remove student card creation from CounterClick view and use fragment instead Intercept htmx on submit requests, this allows auto submit from nfc fields Fix super call with parameters Add loading wheel on student card form for counter_click.jinja --- core/static/bundled/utils/web-components.ts | 11 +- counter/forms.py | 12 +- .../counter/add_student_card_fragment.jinja | 25 ++++ counter/templates/counter/counter_click.jinja | 27 ++-- counter/tests/test_customer.py | 125 ++++++++++++------ counter/urls.py | 11 +- counter/views/click.py | 27 +--- counter/views/student_card.py | 55 +++++++- 8 files changed, 196 insertions(+), 97 deletions(-) create mode 100644 counter/templates/counter/add_student_card_fragment.jinja diff --git a/core/static/bundled/utils/web-components.ts b/core/static/bundled/utils/web-components.ts index 8bec98f9..c2b089c5 100644 --- a/core/static/bundled/utils/web-components.ts +++ b/core/static/bundled/utils/web-components.ts @@ -6,7 +6,16 @@ **/ export function registerComponent(name: string, options?: ElementDefinitionOptions) { return (component: CustomElementConstructor) => { - window.customElements.define(name, component, options); + try { + window.customElements.define(name, component, options); + } catch (e) { + if (e instanceof DOMException) { + // biome-ignore lint/suspicious/noConsole: it's handy to troobleshot + console.warn(e.message); + return; + } + throw e; + } }; } diff --git a/counter/forms.py b/counter/forms.py index 84a92512..91e7c3dc 100644 --- a/counter/forms.py +++ b/counter/forms.py @@ -45,9 +45,7 @@ class BillingInfoForm(forms.ModelForm): class StudentCardForm(forms.ModelForm): - """Form for adding student cards - Only used for user profile since CounterClick is to complicated. - """ + """Form for adding student cards""" class Meta: model = StudentCard @@ -114,14 +112,6 @@ class GetUserForm(forms.Form): return cleaned_data -class NFCCardForm(forms.Form): - student_card_uid = forms.CharField( - max_length=StudentCard.UID_SIZE, - required=False, - widget=NFCTextInput, - ) - - class RefillForm(forms.ModelForm): error_css_class = "error" required_css_class = "required" diff --git a/counter/templates/counter/add_student_card_fragment.jinja b/counter/templates/counter/add_student_card_fragment.jinja new file mode 100644 index 00000000..aa2150e1 --- /dev/null +++ b/counter/templates/counter/add_student_card_fragment.jinja @@ -0,0 +1,25 @@ +
+

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

+
+ {% csrf_token %} + {{ form.as_p() }} + + +
+
{% trans %}Registered cards{% endtrans %}
+ {% if student_cards %} + + + {% else %} + {% trans %}No card registered{% endtrans %} + {% endif %} +
diff --git a/counter/templates/counter/counter_click.jinja b/counter/templates/counter/counter_click.jinja index cb6bb9cf..323a5905 100644 --- a/counter/templates/counter/counter_click.jinja +++ b/counter/templates/counter/counter_click.jinja @@ -29,26 +29,15 @@ {{ user_mini_profile(customer.user) }} {{ user_subscription(customer.user) }}

{% trans %}Amount: {% endtrans %}{{ customer.amount }} €

-
- {% csrf_token %} - - {% trans %}Add a student card{% endtrans %} - {{ student_card_input.student_card_uid }} - {% if request.session['not_valid_student_card_uid'] %} -

{% trans %}This is not a valid student card UID{% endtrans %}

- {% endif %} - -
-
{% trans %}Registered cards{% endtrans %}
- {% if student_cards %} - - {% else %} - {% trans %}No card registered{% endtrans %} + {% if counter.type == 'BAR' %} +
+
+
{% endif %} diff --git a/counter/tests/test_customer.py b/counter/tests/test_customer.py index 2d7e1c60..dd7a5ed6 100644 --- a/counter/tests/test_customer.py +++ b/counter/tests/test_customer.py @@ -168,6 +168,7 @@ class TestStudentCard(TestCase): cls.root = User.objects.get(username="root") cls.counter = Counter.objects.get(id=2) + cls.ae_counter = Counter.objects.get(name="AE") def setUp(self): # Auto login on counter @@ -191,94 +192,144 @@ class TestStudentCard(TestCase): # Test card with mixed letters and numbers response = self.client.post( reverse( - "counter:click", - kwargs={"counter_id": self.counter.id, "user_id": self.sli.id}, + "counter:add_student_card_fragment", + kwargs={ + "counter_id": self.counter.id, + "customer_id": self.sli.customer.pk, + }, ), - {"student_card_uid": "8B90734A802A8F", "action": "add_student_card"}, + {"uid": "8B90734A802A8F"}, ) - self.assertContains(response, text="8B90734A802A8F") + assert response.status_code == 302 + self.assertContains(self.client.get(response.url), text="8B90734A802A8F") # Test card with only numbers response = self.client.post( reverse( - "counter:click", - kwargs={"counter_id": self.counter.id, "user_id": self.sli.id}, + "counter:add_student_card_fragment", + kwargs={ + "counter_id": self.counter.id, + "customer_id": self.sli.customer.pk, + }, ), - {"student_card_uid": "04786547890123", "action": "add_student_card"}, + {"uid": "04786547890123"}, ) - self.assertContains(response, text="04786547890123") + assert response.status_code == 302 + self.assertContains(self.client.get(response.url), text="04786547890123") # Test card with only letters response = self.client.post( reverse( - "counter:click", - kwargs={"counter_id": self.counter.id, "user_id": self.sli.id}, + "counter:add_student_card_fragment", + kwargs={ + "counter_id": self.counter.id, + "customer_id": self.sli.customer.pk, + }, ), - {"student_card_uid": "ABCAAAFAAFAAAB", "action": "add_student_card"}, + {"uid": "ABCAAAFAAFAAAB"}, ) - self.assertContains(response, text="ABCAAAFAAFAAAB") + assert response.status_code == 302 + self.assertContains(self.client.get(response.url), text="ABCAAAFAAFAAAB") def test_add_student_card_from_counter_fail(self): # UID too short response = self.client.post( reverse( - "counter:click", - kwargs={"counter_id": self.counter.id, "user_id": self.sli.id}, + "counter:add_student_card_fragment", + kwargs={ + "counter_id": self.counter.id, + "customer_id": self.sli.customer.pk, + }, ), - {"student_card_uid": "8B90734A802A8", "action": "add_student_card"}, - ) - self.assertContains( - response, text="Ce n'est pas un UID de carte étudiante valide" + {"uid": "8B90734A802A8"}, ) + self.assertContains(response, text="Cet UID est invalide") # UID too long response = self.client.post( reverse( - "counter:click", - kwargs={"counter_id": self.counter.id, "user_id": self.sli.id}, + "counter:add_student_card_fragment", + kwargs={ + "counter_id": self.counter.id, + "customer_id": self.sli.customer.pk, + }, ), - {"student_card_uid": "8B90734A802A8FA", "action": "add_student_card"}, + {"uid": "8B90734A802A8FA"}, ) + self.assertContains(response, text="Cet UID est invalide") self.assertContains( - response, text="Ce n'est pas un UID de carte étudiante valide" + response, + text="Assurez-vous que cette valeur comporte au plus 14 caractères (actuellement 15).", ) # Test with already existing card response = self.client.post( reverse( - "counter:click", - kwargs={"counter_id": self.counter.id, "user_id": self.sli.id}, + "counter:add_student_card_fragment", + kwargs={ + "counter_id": self.counter.id, + "customer_id": self.sli.customer.pk, + }, ), - {"student_card_uid": "9A89B82018B0A0", "action": "add_student_card"}, + {"uid": "9A89B82018B0A0"}, ) + self.assertContains(response, text="Cet UID est invalide") self.assertContains( - response, text="Ce n'est pas un UID de carte étudiante valide" + response, text="Un objet Student card avec ce champ Uid existe déjà." ) # Test with lowercase response = self.client.post( reverse( - "counter:click", - kwargs={"counter_id": self.counter.id, "user_id": self.sli.id}, + "counter:add_student_card_fragment", + kwargs={ + "counter_id": self.counter.id, + "customer_id": self.sli.customer.pk, + }, ), - {"student_card_uid": "8b90734a802a9f", "action": "add_student_card"}, - ) - self.assertContains( - response, text="Ce n'est pas un UID de carte étudiante valide" + {"uid": "8b90734a802a9f"}, ) + self.assertContains(response, text="Cet UID est invalide") # Test with white spaces response = self.client.post( reverse( - "counter:click", - kwargs={"counter_id": self.counter.id, "user_id": self.sli.id}, + "counter:add_student_card_fragment", + kwargs={ + "counter_id": self.counter.id, + "customer_id": self.sli.customer.pk, + }, ), - {"student_card_uid": " ", "action": "add_student_card"}, + {"uid": " "}, ) - self.assertContains( - response, text="Ce n'est pas un UID de carte étudiante valide" + self.assertContains(response, text="Cet UID est invalide") + self.assertContains(response, text="Ce champ est obligatoire.") + + def test_add_student_card_from_counter_unauthorized(self): + # 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}, ) + def send_valid_request(client, counter_id): + return client.post( + reverse( + "counter:add_student_card_fragment", + kwargs={ + "counter_id": counter_id, + "customer_id": self.sli.customer.pk, + }, + ), + {"uid": "8B90734A802A8F"}, + ) + + 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) + def test_delete_student_card_with_owner(self): self.client.force_login(self.sli) self.client.post( diff --git a/counter/urls.py b/counter/urls.py index d5247478..ab93b586 100644 --- a/counter/urls.py +++ b/counter/urls.py @@ -52,7 +52,11 @@ from counter.views.home import ( CounterMain, ) from counter.views.invoice import InvoiceCallView -from counter.views.student_card import StudentCardDeleteView, StudentCardFormView +from counter.views.student_card import ( + StudentCardDeleteView, + StudentCardFormFragmentView, + StudentCardFormView, +) urlpatterns = [ path("/", CounterMain.as_view(), name="details"), @@ -77,6 +81,11 @@ 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/views/click.py b/counter/views/click.py index 88875fe4..1bdc4b3c 100644 --- a/counter/views/click.py +++ b/counter/views/click.py @@ -27,8 +27,8 @@ from django.utils.translation import gettext_lazy as _ from django.views.generic import DetailView from core.views import CanViewMixin -from counter.forms import NFCCardForm, RefillForm -from counter.models import Counter, Customer, Product, Selling, StudentCard +from counter.forms import RefillForm +from counter.models import Counter, Customer, Product, Selling from counter.views.mixins import CounterTabsMixin if TYPE_CHECKING: @@ -134,7 +134,6 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView): request.session["too_young"] = False request.session["not_allowed"] = False request.session["no_age"] = False - request.session["not_valid_student_card_uid"] = False if self.object.type != "BAR": self.operator = request.user elif self.customer_is_barman(): @@ -146,8 +145,6 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView): action = parse_qs(request.body.decode()).get("action", [""])[0] if action == "add_product": self.add_product(request) - elif action == "add_student_card": - self.add_student_card(request) elif action == "del_product": self.del_product(request) elif action == "refill": @@ -284,23 +281,6 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView): request.session.modified = True return True - def add_student_card(self, request): - """Add a new student card on the customer account.""" - uid = str(request.POST["student_card_uid"]) - if not StudentCard.is_valid(uid): - request.session["not_valid_student_card_uid"] = True - return False - - if not ( - self.object.type == "BAR" - and "counter_token" in request.session - and request.session["counter_token"] == self.object.token - and self.object.is_open - ): - raise PermissionDenied - StudentCard(customer=self.customer, uid=uid).save() - return True - def del_product(self, request): """Delete a product from the basket.""" pid = parse_qs(request.body.decode())["product_id"][0] @@ -431,10 +411,7 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView): product ) kwargs["customer"] = self.customer - kwargs["student_cards"] = self.customer.student_cards.all() - kwargs["student_card_input"] = NFCCardForm() kwargs["basket_total"] = self.sum_basket(self.request) kwargs["refill_form"] = self.refill_form or RefillForm() - kwargs["student_card_max_uid_size"] = StudentCard.UID_SIZE kwargs["barmens_can_refill"] = self.object.can_refill() return kwargs diff --git a/counter/views/student_card.py b/counter/views/student_card.py index b39cc519..a79fa8fd 100644 --- a/counter/views/student_card.py +++ b/counter/views/student_card.py @@ -18,9 +18,9 @@ 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 CanEditMixin +from core.views import AllowFragment, CanEditMixin from counter.forms import StudentCardForm -from counter.models import Customer, StudentCard +from counter.models import Counter, Customer, StudentCard class StudentCardDeleteView(DeleteView, CanEditMixin): @@ -40,7 +40,7 @@ class StudentCardDeleteView(DeleteView, CanEditMixin): ) -class StudentCardFormView(FormView): +class StudentCardFormView(AllowFragment, FormView): """Add a new student card.""" form_class = StudentCardForm @@ -62,3 +62,52 @@ class StudentCardFormView(FormView): 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, + }, + )