Skip to content

Commit 941d61f

Browse files
authored
[Bugfix] Réparation du problème du clean_name vide sur les champs de formulaire (#492)
* Fix issue where forms had an empty clean_name * Update release number
1 parent 4288b52 commit 941d61f

5 files changed

Lines changed: 918 additions & 837 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
from django.db import migrations
2+
3+
from wagtail.contrib.forms.utils import get_field_clean_name
4+
5+
6+
def fix_empty_clean_names(apps, schema_editor):
7+
FormField = apps.get_model("forms", "FormField")
8+
for field in FormField.objects.filter(clean_name=""):
9+
field.clean_name = get_field_clean_name(field.label)
10+
field.save(update_fields=["clean_name"])
11+
12+
13+
class Migration(migrations.Migration):
14+
15+
dependencies = [
16+
("forms", "0004_formfield_locale_formfield_translation_key_and_more"),
17+
]
18+
19+
operations = [
20+
migrations.RunPython(fix_empty_clean_names, reverse_code=migrations.RunPython.noop),
21+
]

forms/models.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from wagtail.contrib.forms.forms import BaseForm, FormBuilder
1010
from wagtail.contrib.forms.models import AbstractEmailForm, AbstractFormField
1111
from wagtail.contrib.forms.panels import FormSubmissionsPanel
12+
from wagtail.contrib.forms.utils import get_field_clean_name
1213
from wagtail.fields import RichTextField
1314
from wagtail.models import TranslatableMixin
1415
from wagtail_honeypot.models import HoneypotFormMixin, HoneypotFormSubmissionMixin
@@ -43,6 +44,13 @@ class FormField(TranslatableMixin, AbstractFormField):
4344
SynchronizedField("clean_name", overridable=False),
4445
]
4546

47+
def save(self, *args, **kwargs):
48+
# Guarantee clean_name is always set, even if pk is already assigned (e.g. when
49+
# reconstructed from a revision via from_serializable_data before the first DB insert).
50+
if not self.clean_name:
51+
self.clean_name = get_field_clean_name(self.label)
52+
super().save(*args, **kwargs)
53+
4654
class Meta(TranslatableMixin.Meta, AbstractFormField.Meta):
4755
verbose_name = _("Form field")
4856
verbose_name_plural = _("Form fields")

forms/tests/test_forms.py

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from wagtail.test.utils import WagtailPageTestCase
44
from wagtail_localize.models import TranslationSource
55

6-
from forms.models import FormPage
6+
from forms.models import FormField, FormPage
77

88

99
class FormsTestCase(WagtailPageTestCase):
@@ -56,6 +56,57 @@ def test_incorrect_form_is_rejected(self):
5656
)
5757
# Updates sometimes mess with the order of the translations and so the displayed translation. Both are fine.
5858

59+
def test_form_field_clean_name_set_on_save(self):
60+
"""
61+
Regression test: FormField.clean_name must never be empty after save.
62+
63+
Certain code paths (e.g. reconstruction from a page revision via
64+
from_serializable_data) can produce FormField instances whose pk is
65+
already set before save() is called, causingAbstractFormField.save()
66+
to skip the clean_name computation (it only runs when pk is None).
67+
This leaves clean_name as "" in the database, making all BO responses
68+
appear as None.
69+
"""
70+
form_page = FormPage.objects.first()
71+
72+
# Simulate the problematic path: a FormField whose pk is pre-assigned
73+
# (non-None) but whose clean_name is still empty, as happens when
74+
# from_serializable_data reconstructs a child object from a revision.
75+
field = FormField.objects.filter(page=form_page).first()
76+
original_clean_name = field.clean_name
77+
78+
field.clean_name = ""
79+
field.save()
80+
81+
field.refresh_from_db()
82+
self.assertNotEqual(field.clean_name, "", "clean_name should not be empty after save")
83+
self.assertEqual(field.clean_name, original_clean_name)
84+
85+
def test_get_data_fields_has_no_empty_clean_names(self):
86+
"""
87+
Regression test: every FormField returned by get_data_fields() must
88+
have a non-empty clean_name.
89+
90+
If clean_name is "" in the DB, get_data_fields() returns ("", label)
91+
tuples. The BO then calls form_data.get("") which is always None, and
92+
the xlsx export collapses all "" keys so every column header becomes
93+
the last field's label.
94+
"""
95+
form_page = FormPage.objects.first()
96+
97+
# Force empty clean_names on all fields to simulate the buggy state.
98+
FormField.objects.filter(page=form_page).update(clean_name="")
99+
100+
# Re-save each field; the overridden save() must restore clean_name.
101+
for field in FormField.objects.filter(page=form_page):
102+
field.save()
103+
104+
data_fields = form_page.get_data_fields()
105+
for name, label in data_fields:
106+
if name == "submit_time":
107+
continue
108+
self.assertNotEqual(name, "", f"Field '{label}' has an empty clean_name after save()")
109+
59110
def test_form_field_labels_are_translatable(self):
60111
form_page = FormPage.objects.first()
61112
source, _ = TranslationSource.update_or_create_from_instance(form_page)

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "sites-conformes"
3-
version = "3.1.0"
3+
version = "3.1.1"
44
description = "Gestionnaire de contenu permettant de créer et gérer un site internet basé sur le Système de design de l’État, accessible et responsive"
55
authors = [
66
{ name = "Sites Conformes dev team", email = "contact@sites.beta.gouv.fr" }

0 commit comments

Comments
 (0)