Forms Template/JS Refactoring

ephios/core/forms/events.py

@@ -10,7 +10,6 @@
 from django.contrib.auth.models import Group
 from django.core.exceptions import ValidationError
 from django.db.models import Q
-from django.template.loader import render_to_string
 from django.utils.safestring import mark_safe
 from django.utils.timezone import make_aware
 from django.utils.translation import gettext as _
@@ -264,18 +263,12 @@ class EventTypePreferenceForm(PreferenceForm):
 
 
 class BasePluginFormMixin:
+    template_name = "core/fragments/plugin_form.html"
+
     @property
     def heading(self):
         raise NotImplementedError
 
-    def render(self):
-        try:
-            self.helper.form_tag = False
-        except AttributeError:
-            self.helper = FormHelper(self)
-            self.helper.form_tag = False
-        return render_to_string("core/fragments/plugin_form.html", context={"form": self})
-
     def is_function_active(self):
         """
         When building forms for additional features, return whether that feature is enabled for the form instance.

ephios/core/templates/core/event_form.html

@@ -51,7 +51,7 @@ <h1>{% blocktranslate with title=eventtype.title %}Create new {{ title }}{% endb
         </div>
 
         {% for plugin_form in plugin_forms %}
-            {{ plugin_form.render }}
+            {{ plugin_form }}
         {% endfor %}
 
         <div class="form-group">

ephios/core/templates/core/fragments/plugin_form.html

@@ -5,7 +5,9 @@
     </div>
     <div id="plugin-form-{{ form.prefix }}" class="collapse{% if form.is_function_active %} show{% endif %}">
         <div class="card-body">
-            {% crispy form %}
+            {% block card-body %}
+                {{ form|crispy }}
+            {% endblock %}
         </div>
     </div>
 </div>

ephios/core/templates/core/shift_form.html

@@ -31,7 +31,7 @@ <h1>{% trans "Create new shift for event" %}</h1>
                 <hr class="border border-secondary border-dark opacity-75">
                 <div class="plugin-forms">
                     {% for plugin_form in plugin_forms %}
-                        {{ plugin_form.render }}
+                        {{ plugin_form }}
                     {% endfor %}
                 </div>
                 <div class="form-group">

ephios/core/templates/core/userprofile_form.html

@@ -34,7 +34,7 @@ <h1>{% trans "Add new user" %}</h1>
                     <h4 class="card-title">{% translate "Qualifications" %}</h4>
                     <ul class="list-group list-group-flush" data-formset-body>
                         {% for form in qualification_formset %}
-                            <div class="list-group-item" data-formset-form>
+                            <li class="list-group-item" data-formset-form>
                                 {{ form.id }}
                                 <div class="row align-items-center">
                                     <div class="col-md-5">
@@ -61,7 +61,7 @@ <h4 class="card-title">{% translate "Qualifications" %}</h4>
                                     </div>
                                 </div>
                                 <div class="d-none">{{ form.DELETE }}</div>
-                            </div>
+                            </li>
                         {% endfor %}
                     </ul>
 
@@ -73,7 +73,7 @@ <h4 class="card-title">{% translate "Qualifications" %}</h4>
 
             <script type="form-template" data-formset-empty-form>
                 {% escapescript %}
-                    <div class="list-group-item" data-formset-form>
+                    <li class="list-group-item" data-formset-form>
                         <div class="row align-items-center">
                             <div class="col-md-5">
                                 {% crispy_field qualification_formset.empty_form.qualification label_class="col-md-4" field_class="col-md-8" wrapper_class="row" %}
@@ -90,7 +90,7 @@ <h4 class="card-title">{% translate "Qualifications" %}</h4>
                             </div>
                         </div>
                         <div class="d-none">{{ qualification_formset.empty_form.DELETE }}</div>
-                    </div>
+                    </li>
                 {% endescapescript %}
             </script>
 

ephios/extra/mixins.py

@@ -1,9 +1,12 @@
 import functools
+from typing import Collection
 
 from django.contrib.auth.mixins import AccessMixin, PermissionRequiredMixin
 from django.shortcuts import redirect
 from django.urls import resolve
 
+from ephios.core.forms.events import BasePluginFormMixin
+
 
 class CustomPermissionRequiredMixin(PermissionRequiredMixin):
     """
@@ -147,5 +150,5 @@ def save_plugin_forms(self):
         for plugin_form in self.plugin_forms:
             plugin_form.save()
 
-    def get_plugin_forms(self):
+    def get_plugin_forms(self) -> Collection[BasePluginFormMixin]:
         raise NotImplementedError

ephios/plugins/basesignup/templates/basesignup/section_based/configuration_form.html

@@ -14,7 +14,7 @@ <h3 class="card-title">{% translate "Sections" %}</h3>
             {{ form.sections_formset|formset_errors }}
             <ul class="list-group list-group-flush" data-formset-body>
                 {% for section_form in form.sections_formset %}
-                    <div class="list-group-item" data-formset-form>
+                    <li class="list-group-item" data-formset-form>
                         <div class="row align-items-center">
                             <div class="col-md-3">
                                 {{ section_form.title|as_crispy_field }}
@@ -38,7 +38,7 @@ <h3 class="card-title">{% translate "Sections" %}</h3>
                             {{ section_form.DELETE }}
                             {{ section_form.uuid }}
                         </div>
-                    </div>
+                    </li>
                 {% endfor %}
             </ul>
 
@@ -50,7 +50,7 @@ <h3 class="card-title">{% translate "Sections" %}</h3>
 
     <script type="form-template" data-formset-empty-form>
         {% escapescript %}
-            <div class="list-group-item" data-formset-form>
+            <li class="list-group-item" data-formset-form>
                 <div class="row align-items-center">
                     <div class="col-md-3">
                         {{ form.sections_formset.empty_form.title|as_crispy_field }}
@@ -74,7 +74,7 @@ <h3 class="card-title">{% translate "Sections" %}</h3>
                     {{ form.sections_formset.empty_form.DELETE }}
                     {{ form.sections_formset.empty_form.uuid }}
                 </div>
-            </div>
+            </li>
         {% endescapescript %}
     </script>
 </div>

ephios/plugins/eventautoqualification/forms.py

@@ -1,5 +1,3 @@
-from crispy_forms.helper import FormHelper
-from crispy_forms.layout import HTML
 from django import forms
 from django.utils.translation import gettext_lazy as _
 from django_select2.forms import Select2Widget
@@ -10,6 +8,8 @@
 
 
 class EventAutoQualificationForm(BasePluginFormMixin, forms.ModelForm):
+    template_name = "eventautoqualification/auto_qualification_setup_form.html"
+
     class Meta:
         model = EventAutoQualificationConfiguration
         fields = ["qualification", "expiration_date", "mode", "extend_only", "needs_confirmation"]
@@ -31,23 +31,16 @@ def __init__(self, *args, event=None, edit_permission=False, **kwargs):
         super().__init__(*args, **kwargs)
 
         self.fields["qualification"].required = False
-        self.helper = FormHelper(self)
+        self.edit_permission = edit_permission
         if not edit_permission:
-            self.helper.layout.insert(
-                0,
-                HTML(
-                    "<p>"
-                    + str(
-                        _(
-                            "You don't have permission to grant qualifications, so you can't edit these settings."
-                        )
-                    )
-                    + "</p>"
-                ),
-            )
             for field in self.fields.values():
                 field.disabled = True
 
+    def get_context(self):
+        context = super().get_context()
+        context["edit_permission"] = self.edit_permission
+        return context
+
     def save(self, commit=True):
         if self.cleaned_data.get("qualification"):
             self.instance.event = self.event

ephios/plugins/eventautoqualification/templates/eventautoqualification/auto_qualification_setup_form.html

@@ -0,0 +1,11 @@
+{% extends "core/fragments/plugin_form.html" %}
+{% load i18n %}
+
+{% block card-body %}
+    {% if not edit_permission %}
+        <div class="alert alert-warning">
+            {% translate "You don't have permission to grant qualifications, so you can't edit these settings." %}
+        </div>
+    {% endif %}
+    {{ block.super }}
+{% endblock %}

ephios/plugins/qualification_management/templates/core/qualificationcategories_form.html

@@ -20,7 +20,7 @@ <h2>{% trans "Edit qualification categories" %}</h2>
                 <div class="card-body">
                     <ul class="list-group list-group-flush" data-formset-body>
                         {% for category_form in form %}
-                            <div class="list-group-item" data-formset-form>
+                            <li class="list-group-item" data-formset-form>
                                 {{ category_form.id }}
                                 <div class="row align-items-center">
                                     <div class="col-lg-4">
@@ -44,7 +44,7 @@ <h2>{% trans "Edit qualification categories" %}</h2>
                                 <div class="d-none">
                                     {{ category_form.DELETE }}
                                 </div>
-                            </div>
+                            </li>
                         {% endfor %}
                     </ul>
 
@@ -57,7 +57,7 @@ <h2>{% trans "Edit qualification categories" %}</h2>
             {% with category_form=form.empty_form %}
                 <script type="form-template" data-formset-empty-form>
                     {% escapescript %}
-                        <div class="list-group-item" data-formset-form>
+                        <li class="list-group-item" data-formset-form>
                             {{ category_form.id }}
                             <div class="row align-items-center">
                                 <div class="col-lg-4">
@@ -75,7 +75,7 @@ <h2>{% trans "Edit qualification categories" %}</h2>
                             <div class="d-none">
                                 {{ category_form.DELETE }}
                             </div>
-                        </div>
+                        </li>
                     {% endescapescript %}
                 </script>
             {% endwith %}

ephios/plugins/simpleresource/forms.py

@@ -1,4 +1,3 @@
-from crispy_forms.helper import FormHelper
 from django.forms import BaseModelFormSet, BooleanField, ModelForm, modelformset_factory
 from django.forms.formsets import DELETION_FIELD_NAME
 from django.utils.translation import gettext as _
@@ -17,9 +16,8 @@ class Meta:
         }
 
     def __init__(self, *args, shift, **kwargs):
+        kwargs.setdefault("prefix", "simple_resource")
         self.shift = shift
-        self.helper = FormHelper()
-        self.helper.include_media = False
         try:
             kwargs.setdefault("instance", ResourceAllocation.objects.get(shift_id=shift.id))
         except (AttributeError, ResourceAllocation.DoesNotExist):

ephios/plugins/simpleresource/templates/simpleresource/resourcecategories_form.html

@@ -24,7 +24,7 @@ <h1>{% trans "Edit resource categories" %}</h1>
                 <div class="card-body pt-2">
                     <ul class="list-group list-group-flush" data-formset-body>
                         {% for category_form in form %}
-                            <div class="list-group-item" data-formset-form>
+                            <li class="list-group-item" data-formset-form>
                                 {{ category_form.id }}
                                 <div class="row align-items-start">
                                     <div class="col-lg-6 pt-3">
@@ -48,7 +48,7 @@ <h1>{% trans "Edit resource categories" %}</h1>
                                 <div class="d-none">
                                     {{ category_form.DELETE }}
                                 </div>
-                            </div>
+                            </li>
                         {% endfor %}
                     </ul>
 
@@ -61,7 +61,7 @@ <h1>{% trans "Edit resource categories" %}</h1>
             {% with category_form=form.empty_form %}
                 <script type="form-template" data-formset-empty-form>
                     {% escapescript %}
-                        <div class="list-group-item" data-formset-form>
+                        <li class="list-group-item" data-formset-form>
                             {{ category_form.id }}
                             <div class="row align-items-start">
                                 <div class="col-lg-6 pt-3">
@@ -79,7 +79,7 @@ <h1>{% trans "Edit resource categories" %}</h1>
                             <div class="d-none">
                                 {{ category_form.DELETE }}
                             </div>
-                        </div>
+                        </li>
                     {% endescapescript %}
                 </script>
             {% endwith %}

ephios/static/ephios/js/formset/formset.js

@@ -1,5 +1,5 @@
 /**
- * Django formset helper, adapted from django-formset-js-improved
+ * Django formset helper, adapted from django-formset-js-improved for use in ephios
  */
 (function ($) {
     "use strict";
@@ -77,7 +77,14 @@
             .replace(new RegExp('<\\\\/script>', 'g'), '</script>');
 
         var $newFormFragment = $($.parseHTML(newFormHtml, this.$body.document, true));
+
+        if (this.opts.animateForms) {
+            // Hide the new form before adding it to the DOM to avoid flickering
+            $newFormFragment.hide();
+        }
+
         this.$body.append($newFormFragment);
+        $newFormFragment.trigger("prepareNewFormFragment")
 
         var $newForm = $newFormFragment.filter(this.opts.form);
         this.bindForm($newForm, newIndex);

ephios/static/ephios/js/main.js

@@ -23,7 +23,10 @@ function handleForms(elem) {
     elem.find("[data-formset]").formset({
         animateForms: true,
         reorderMode: 'dom',
-    }).on("formAdded", "div", function (event) {
+    }).on("prepareNewFormFragment", "[data-formset-form]", function (event) {
+        // Handle any forms that were added to the template with this custom event and
+        // not 'formAdded' as that would be called after animation leading to
+        // the slideDown animation not using the correct height
         handleForms($(event.target));
     });
 }

ephios/templates/base.html

@@ -53,7 +53,7 @@
     {# as of 2021-06-14, the statici18n tag misbehaves with compression when 500 errors occur #}
     <script type="text/javascript" src="{% statici18n LANGUAGE_CODE %}"></script>
     {% compress js %}
-        <script type="text/javascript" src="{% static "jquery/js/jquery-3.5.1.min.js" %}"></script>
+        <script type="text/javascript" src="{% static "jquery/js/jquery-3.7.0.min.js" %}"></script>
         <script type="text/javascript" src="{% static "bootstrap/js/bootstrap.bundle.min.js" %}"></script>
         <script type="text/javascript" src="{% static "select2/js/select2.js" %}"></script>
         <script type="text/javascript" src="{% static "django_select2/django_select2.js" %}"></script>

tests/plugins/simpleresource/test_resources.py

@@ -66,7 +66,7 @@ def test_resource_allocation_edit(django_app, superuser, groups, event, resource
     form = django_app.get(
         reverse("core:shift_edit", kwargs={"pk": event.shifts.first().pk}), user=superuser
     ).form
-    form["resources"].select_multiple([resources[0].pk])
+    form["simple_resource-resources"].select_multiple([resources[0].pk])
     form.submit()
     assert ResourceAllocation.objects.get(shift=event.shifts.first()).resources.count() == 1
     assert (