diff --git a/subscription/forms.py b/subscription/forms.py index 0d808997..7f8cd7d7 100644 --- a/subscription/forms.py +++ b/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( diff --git a/subscription/templates/subscription/forms/create_existing_user.jinja b/subscription/templates/subscription/forms/create_existing_user.jinja deleted file mode 100644 index 380abb01..00000000 --- a/subscription/templates/subscription/forms/create_existing_user.jinja +++ /dev/null @@ -1,28 +0,0 @@ -{% load static %} -{% load i18n %} - - -
-
- {{ errors }} - {% for field, errors in fields %} - - {{ field.label_tag }} - {{ field }} - {% if field.help_text %} - {{ field.help_text }} - {% endif %} -

- {% if field.name == "payment_method" %} - - {% blocktranslate %}If the subscription is done using the AE account, you must also click it on the AE counter.{% endblocktranslate %} - - {% endif %} - {% endfor %} -
-
-
diff --git a/subscription/templates/subscription/forms/create_new_user.jinja b/subscription/templates/subscription/forms/create_new_user.jinja deleted file mode 100644 index c22df09b..00000000 --- a/subscription/templates/subscription/forms/create_new_user.jinja +++ /dev/null @@ -1 +0,0 @@ -{{ form.as_p }} \ No newline at end of file diff --git a/subscription/templates/subscription/fragments/creation_form_existing_user.jinja b/subscription/templates/subscription/fragments/creation_form_existing_user.jinja new file mode 100644 index 00000000..ae82bdd3 --- /dev/null +++ b/subscription/templates/subscription/fragments/creation_form_existing_user.jinja @@ -0,0 +1,38 @@ +
+ {% csrf_token %} + +
+
+ {{ form.errors }} + {% for field in form %} +
+ {{ field.label_tag() }} + {{ field }} + {% if field.help_text %} + {{ field.help_text }} + {% endif %} + {% if field.name == "payment_method" %} + + {% trans trimmed %} + If the subscription is done using the AE account, + you must also click it on the AE counter. + {% endtrans %} + + {% endif %} +
+ {% endfor %} +
+
+
+ + +
diff --git a/subscription/templates/subscription/fragments/creation_form.jinja b/subscription/templates/subscription/fragments/creation_form_new_user.jinja similarity index 77% rename from subscription/templates/subscription/fragments/creation_form.jinja rename to subscription/templates/subscription/fragments/creation_form_new_user.jinja index 697c04bc..e1c08e28 100644 --- a/subscription/templates/subscription/fragments/creation_form.jinja +++ b/subscription/templates/subscription/fragments/creation_form_new_user.jinja @@ -1,5 +1,5 @@
{% 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 %}

{% trans %}New subscription{% endtrans %}

- {{ form_fragment(existing_user_form, existing_user_post_url) }} + {{ existing_user_fragment }} - {{ form_fragment(new_user_form, new_user_post_url) }} + {{ new_user_fragment }} {% endblock %} diff --git a/subscription/tests/test_new_subscription.py b/subscription/tests/test_new_subscription.py index 3833e48b..a02a1d49 100644 --- a/subscription/tests/test_new_subscription.py +++ b/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( - user_factory: Callable[[], User], settings: SettingsWrapper -): +def test_form_existing_user_valid(user_factory: Callable[[], User]): """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, diff --git a/subscription/views.py b/subscription/views.py index fb4cde07..505f9614 100644 --- a/subscription/views.py +++ b/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): - 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" +class CreateSubscriptionFragment(PermissionRequiredMixin, FragmentMixin, CreateView): 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"