Merge pull request #1380 from ae-utbm/fragment-subscription

refactor: use `FragmentMixin` for subscription fragments

subscription/forms.py

@@ -79,7 +79,6 @@ class SubscriptionNewUserForm(SubscriptionForm):
     """
 
     allowed_payment_methods = ["CARD", "CASH"]
-    template_name = "subscription/forms/create_new_user.jinja"
 
     __user_fields = forms.fields_for_model(
         User,
@@ -121,6 +120,12 @@ class SubscriptionNewUserForm(SubscriptionForm):
             email=self.cleaned_data.get("email"),
             date_of_birth=self.cleaned_data.get("date_of_birth"),
         )
+        if self.errors:
+            # don't bother generating username, password and other data.
+            # The form validation failed anyway, so using a dummy User
+            # (just for Subscription.clean not to crash) is enough
+            self.instance.member = member
+            return super().clean()
         if self.cleaned_data.get("subscription_type") in [
             "un-semestre",
             "deux-semestres",
@@ -153,7 +158,6 @@ class SubscriptionNewUserForm(SubscriptionForm):
 class SubscriptionExistingUserForm(SubscriptionForm):
     """Form to add a subscription to an existing user."""
 
-    template_name = "subscription/forms/create_existing_user.jinja"
     required_css_class = "required"
 
     birthdate = forms.fields_for_model(

subscription/templates/subscription/forms/create_existing_user.jinja

@@ -1,28 +0,0 @@
-{% load static %}
-{% load i18n %}
-
-
-<div x-data="existing_user_subscription_form" class="form-content existing-user">
-  <fieldset>
-    {{ errors }}
-    {% for field, errors in fields %}
-      <p{% with classes=field.css_classes %}{% if classes %} class="{{ classes }}"{% endif %}{% endwith %}>
-        {{ field.label_tag }}
-        {{ field }}
-        {% if field.help_text %}
-          <span class="helptext">{{ field.help_text }}</span>
-        {% endif %}
-      </p>
-      {% if field.name == "payment_method" %}
-        <i>
-          {% blocktranslate %}If the subscription is done using the AE account, you must also click it on the AE counter.{% endblocktranslate %}
-        </i>
-      {% endif %}
-    {% endfor %}
-  </fieldset>
-  <div
-    id="subscription-form-user-mini-profile"
-    x-html="profileFragment"
-    :aria-busy="loading"
-  ></div>
-</div>

subscription/templates/subscription/forms/create_new_user.jinja

@@ -1 +0,0 @@
-{{ form.as_p }}

subscription/templates/subscription/fragments/creation_form_existing_user.jinja

@@ -0,0 +1,38 @@
+<form
+  hx-post="{{ url("subscription:fragment-existing-user") }}"
+  hx-target="this"
+  hx-disabled-elt="find input[type='submit']"
+  hx-swap="outerHTML"
+>
+  {% csrf_token %}
+
+  <div x-data="existing_user_subscription_form" class="form-content existing-user">
+    <fieldset>
+      {{ form.errors }}
+      {% for field in form %}
+        <div class="form-group">
+          {{ field.label_tag() }}
+          {{ field }}
+          {% if field.help_text %}
+            <span class="helptext">{{ field.help_text }}</span>
+          {% endif %}
+          {% if field.name == "payment_method" %}
+            <i>
+              {% trans trimmed %}
+                If the subscription is done using the AE account,
+                you must also click it on the AE counter.
+              {% endtrans %}
+            </i>
+          {% endif %}
+        </div>
+      {% endfor %}
+    </fieldset>
+    <div
+      id="subscription-form-user-mini-profile"
+      x-html="profileFragment"
+      :aria-busy="loading"
+    ></div>
+  </div>
+
+  <input type="submit" value="{% trans %}Save{% endtrans %}">
+</form>

subscription/templates/subscription/fragments/creation_form_new_user.jinja

@@ -1,5 +1,5 @@
 <form
-  hx-post="{{ post_url }}"
+  hx-post="{{ url("subscription:fragment-new-user") }}"
   hx-target="this"
   hx-disabled-elt="find input[type='submit']"
   hx-swap="outerHTML"

subscription/templates/subscription/subscription.jinja

@@ -18,22 +18,14 @@
   <link rel="stylesheet" href="{{ static("subscription/css/subscription.scss") }}">
 {% endblock %}
 
-{% macro form_fragment(form_object, post_url) %}
-  {# Include the form fragment inside a with block,
-  in order to inject the right form in the right place #}
-  {% with form=form_object, post_url=post_url %}
-    {% include "subscription/fragments/creation_form.jinja" %}
-  {% endwith %}
-{% endmacro %}
-
 {% block content %}
   <h3>{% trans %}New subscription{% endtrans %}</h3>
   <ui-tab-group id="subscription-form">
     <ui-tab title="{% trans %}Existing member{% endtrans %}" active>
-      {{ form_fragment(existing_user_form, existing_user_post_url) }}
+      {{ existing_user_fragment }}
     </ui-tab>
     <ui-tab title="{% trans %}New member{% endtrans %}">
-      {{ form_fragment(new_user_form, new_user_post_url) }}
+      {{ new_user_fragment }}
     </ui-tab>
   </ui-tab-group>
 {% endblock %}

subscription/tests/test_new_subscription.py

@@ -12,7 +12,6 @@ from django.urls import reverse
 from django.utils.timezone import localdate
 from model_bakery import baker
 from pytest_django.asserts import assertRedirects
-from pytest_django.fixtures import SettingsWrapper
 
 from core.baker_recipes import board_user, old_subscriber_user, subscriber_user
 from core.models import Group, User
@@ -26,9 +25,7 @@ from subscription.models import Subscription
     "user_factory",
     [old_subscriber_user.make, lambda: baker.make(User)],
 )
-def test_form_existing_user_valid(
+def test_form_existing_user_valid(user_factory: Callable[[], User]):
-    user_factory: Callable[[], User], settings: SettingsWrapper
-):
     """Test `SubscriptionExistingUserForm`"""
     user = user_factory()
     user.date_of_birth = date(year=1967, month=3, day=14)
@@ -48,7 +45,7 @@ def test_form_existing_user_valid(
 
 
 @pytest.mark.django_db
-def test_form_existing_user_with_birthdate(settings: SettingsWrapper):
+def test_form_existing_user_with_birthdate():
     """Test `SubscriptionExistingUserForm`"""
     user = baker.make(User, date_of_birth=None)
     data = {
@@ -70,7 +67,7 @@ def test_form_existing_user_with_birthdate(settings: SettingsWrapper):
 
 
 @pytest.mark.django_db
-def test_form_existing_user_invalid(settings: SettingsWrapper):
+def test_form_existing_user_invalid():
     """Test `SubscriptionExistingUserForm`, with users that shouldn't subscribe."""
     user = subscriber_user.make()
     # make sure the current subscription will end in a long time
@@ -91,7 +88,7 @@ def test_form_existing_user_invalid(settings: SettingsWrapper):
 
 
 @pytest.mark.django_db
-def test_form_new_user(settings: SettingsWrapper):
+def test_form_new_user():
     data = {
         "first_name": "John",
         "last_name": "Doe",
@@ -121,7 +118,7 @@ def test_form_new_user(settings: SettingsWrapper):
     "subscription_type",
     ["un-semestre", "deux-semestres", "cursus-tronc-commun", "cursus-branche"],
 )
-def test_form_set_new_user_as_student(settings: SettingsWrapper, subscription_type):
+def test_form_set_new_user_as_student(subscription_type):
     """Test that new users have the student role by default."""
     data = {
         "first_name": "John",
@@ -165,7 +162,7 @@ def test_page_access_with_get_data(client: Client):
 
 
 @pytest.mark.django_db
-def test_submit_form_existing_user(client: Client, settings: SettingsWrapper):
+def test_submit_form_existing_user(client: Client):
     client.force_login(
         baker.make(
             User,
@@ -196,7 +193,7 @@ def test_submit_form_existing_user(client: Client, settings: SettingsWrapper):
 
 
 @pytest.mark.django_db
-def test_submit_form_new_user(client: Client, settings: SettingsWrapper):
+def test_submit_form_new_user(client: Client):
     client.force_login(
         baker.make(
             User,

subscription/views.py

@@ -23,6 +23,7 @@ from django.utils.translation import gettext_lazy as _
 from django.views.generic import CreateView, DetailView, TemplateView
 from django.views.generic.edit import FormView
 
+from core.views import FragmentMixin, UseFragmentsMixin
 from core.views.group import PermissionGroupsUpdateView
 from subscription.forms import (
     SelectionDateForm,
@@ -32,24 +33,9 @@ from subscription.forms import (
 from subscription.models import Subscription
 
 
-class NewSubscription(PermissionRequiredMixin, TemplateView):
+class CreateSubscriptionFragment(PermissionRequiredMixin, FragmentMixin, CreateView):
-    template_name = "subscription/subscription.jinja"
-    permission_required = "subscription.add_subscription"
-
-    def get_context_data(self, **kwargs):
-        return super().get_context_data(**kwargs) | {
-            "existing_user_form": SubscriptionExistingUserForm(
-                initial={"member": self.request.GET.get("member")}
-            ),
-            "new_user_form": SubscriptionNewUserForm(),
-            "existing_user_post_url": reverse("subscription:fragment-existing-user"),
-            "new_user_post_url": reverse("subscription:fragment-new-user"),
-        }
-
-
-class CreateSubscriptionFragment(PermissionRequiredMixin, CreateView):
-    template_name = "subscription/fragments/creation_form.jinja"
     permission_required = "subscription.add_subscription"
+    object = None
 
     def get_success_url(self):
         return reverse(
@@ -61,14 +47,14 @@ class CreateSubscriptionExistingUserFragment(CreateSubscriptionFragment):
     """Create a subscription for a user who already exists."""
 
     form_class = SubscriptionExistingUserForm
-    extra_context = {"post_url": reverse_lazy("subscription:fragment-existing-user")}
+    template_name = "subscription/fragments/creation_form_existing_user.jinja"
 
 
 class CreateSubscriptionNewUserFragment(CreateSubscriptionFragment):
     """Create a subscription for a user who doesn't exist yet."""
 
     form_class = SubscriptionNewUserForm
-    extra_context = {"post_url": reverse_lazy("subscription:fragment-new-user")}
+    template_name = "subscription/fragments/creation_form_new_user.jinja"
 
     def form_valid(self, form):
         res = super().form_valid(form)
@@ -83,6 +69,15 @@ class CreateSubscriptionNewUserFragment(CreateSubscriptionFragment):
         return res
 
 
+class NewSubscription(PermissionRequiredMixin, UseFragmentsMixin, TemplateView):
+    template_name = "subscription/subscription.jinja"
+    permission_required = "subscription.add_subscription"
+    fragments = {
+        "new_user_fragment": CreateSubscriptionNewUserFragment,
+        "existing_user_fragment": CreateSubscriptionExistingUserFragment,
+    }
+
+
 class SubscriptionCreatedFragment(PermissionRequiredMixin, DetailView):
     template_name = "subscription/fragments/creation_success.jinja"
     permission_required = "subscription.add_subscription"