From 0aa17279f0eadaaaa1fe62ea24d1c8ff616e953b 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 --- core/static/webpack/utils/web-components.ts | 11 +- counter/forms.py | 12 +- .../counter/add_student_card_fragment.jinja | 26 ++++ counter/templates/counter/counter_click.jinja | 25 +--- counter/tests/test_customer.py | 125 ++++++++++++------ counter/urls.py | 6 + counter/views.py | 77 +++++++---- 7 files changed, 187 insertions(+), 95 deletions(-) create mode 100644 counter/templates/counter/add_student_card_fragment.jinja diff --git a/core/static/webpack/utils/web-components.ts b/core/static/webpack/utils/web-components.ts index 8bec98f9..c2b089c5 100644 --- a/core/static/webpack/utils/web-components.ts +++ b/core/static/webpack/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..6446ae01 --- /dev/null +++ b/counter/templates/counter/add_student_card_fragment.jinja @@ -0,0 +1,26 @@ +
+

{% 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..e72eb77e 100644 --- a/counter/templates/counter/counter_click.jinja +++ b/counter/templates/counter/counter_click.jinja @@ -29,27 +29,12 @@ {{ 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 %} - {% 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 a2732925..8b7417c7 100644 --- a/counter/urls.py +++ b/counter/urls.py @@ -45,6 +45,7 @@ from counter.views import ( RefillingDeleteView, SellingDeleteView, StudentCardDeleteView, + StudentCardFormFragmentView, StudentCardFormView, counter_login, counter_logout, @@ -73,6 +74,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.py b/counter/views.py index 9483d335..4176e3ba 100644 --- a/counter/views.py +++ b/counter/views.py @@ -53,14 +53,13 @@ from django.views.generic.edit import ( from accounting.models import CurrencyField from core.utils import get_semester_code, get_start_of_semester -from core.views import CanEditMixin, CanViewMixin, TabedViewMixin +from core.views import AllowFragment, CanEditMixin, CanViewMixin, TabedViewMixin from core.views.forms import LoginForm from counter.forms import ( CashSummaryFormBase, CounterEditForm, EticketForm, GetUserForm, - NFCCardForm, ProductEditForm, RefillForm, StudentCardForm, @@ -330,7 +329,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(): @@ -342,8 +340,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": @@ -480,23 +476,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] @@ -627,11 +606,8 @@ 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 @@ -1513,7 +1489,7 @@ class CounterRefillingListView(CounterAdminTabsMixin, CounterAdminMixin, ListVie return kwargs -class StudentCardFormView(FormView): +class StudentCardFormView(AllowFragment, FormView): """Add a new student card.""" form_class = StudentCardForm @@ -1535,3 +1511,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(FormView, self).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, + }, + )