Validate against duplicate form field names - fixes #585

Thanks to @tacitus for this fix (#2445).

CHANGELOG.txt

@@ -9,6 +9,7 @@ Changelog
  * oEmbed URL for audioBoom was updated (Janneke Janssen)
  * Remember tree location in page chooser when switching between Internal / External / Email link (Matt Westcott)
  * `FieldRowPanel` now creates equal-width columns automatically if `col*` classnames are not specified (Chris Rogers)
+ * Form builder now validates against multiple fields with the same name (Richard McMillan)
  * Fix: Email templates and document uploader now support custom `STATICFILES_STORAGE` (Jonny Scholes)
 
 

CONTRIBUTORS.rst

@@ -141,6 +141,7 @@ Contributors
 * Behzad Nategh
 * Yann Fouillat (Gagaro)
 * Jonny Scholes
+* Richard McMillan
 
 Translators
 ===========

docs/releases/1.6.rst

@@ -19,6 +19,7 @@ Minor features
  * oEmbed URL for audioBoom was updated (Janneke Janssen)
  * Remember tree location in page chooser when switching between Internal / External / Email link (Matt Westcott)
  * ``FieldRowPanel`` now creates equal-width columns automatically if ``col*`` classnames are not specified (Chris Rogers)
+ * Form builder now validates against multiple fields with the same name (Richard McMillan)
 
 
 Bug fixes

wagtail/wagtailadmin/forms.py

@@ -289,6 +289,7 @@ class WagtailAdminPageForm(WagtailAdminModelForm):
             return self.cleaned_data['seo_title'].strip()
 
     def clean(self):
+
         cleaned_data = super(WagtailAdminPageForm, self).clean()
         if 'slug' in self.cleaned_data:
             if not Page._slug_is_available(

wagtail/wagtailforms/forms.py

@@ -3,6 +3,9 @@ from __future__ import absolute_import, unicode_literals
 from collections import OrderedDict
 
 import django.forms
+from django.utils.translation import ugettext_lazy as _
+
+from wagtail.wagtailadmin.forms import WagtailAdminPageForm
 
 
 class BaseForm(django.forms.Form):
@@ -111,3 +114,27 @@ class SelectDateForm(django.forms.Form):
         required=False,
         widget=django.forms.DateInput(attrs={'placeholder': 'Date to'})
     )
+
+
+class WagtailAdminFormPageForm(WagtailAdminPageForm):
+
+    def clean(self):
+
+        super(WagtailAdminFormPageForm, self).clean()
+
+        # Check for dupe form field labels - fixes #585
+        if 'form_fields' in self.formsets:
+            _forms = self.formsets['form_fields'].forms
+            for f in _forms:
+                f.is_valid()
+
+            for i, form in enumerate(_forms):
+                if 'label' in form.changed_data:
+                    label = form.cleaned_data.get('label')
+                    for idx, ff in enumerate(_forms):
+                        # Exclude self
+                        if idx != i and label == ff.cleaned_data.get('label'):
+                            form.add_error(
+                                'label',
+                                django.forms.ValidationError(_('There is another field with the label %s, please change one of them.' % label))
+                            )

wagtail/wagtailforms/models.py

@@ -17,7 +17,7 @@ from wagtail.wagtailadmin.edit_handlers import FieldPanel
 from wagtail.wagtailadmin.utils import send_mail
 from wagtail.wagtailcore.models import Orderable, Page, UserPagePermissionsProxy, get_page_models
 
-from .forms import FormBuilder
+from .forms import FormBuilder, WagtailAdminFormPageForm
 
 FORM_FIELD_CHOICES = (
     ('singleline', _('Single line text')),
@@ -135,6 +135,8 @@ class AbstractForm(Page):
 
     form_builder = FormBuilder
 
+    base_form_class = WagtailAdminFormPageForm
+
     def __init__(self, *args, **kwargs):
         super(AbstractForm, self).__init__(*args, **kwargs)
         if not hasattr(self, 'landing_page_template'):

wagtail/wagtailforms/tests.py

@@ -678,3 +678,43 @@ class TestIssue798(TestCase):
 
         # Check that form submission was saved correctly
         self.assertTrue(FormSubmission.objects.filter(page=self.form_page, form_data__contains='7.3').exists())
+
+
+class TestIssue585(TestCase):
+    fixtures = ['test.json']
+
+    def setUp(self):
+
+        self.assertTrue(self.client.login(username='superuser', password='password'))
+        # Find root page
+        self.root_page = Page.objects.get(id=2)
+
+    def test_adding_duplicate_form_labels(self):
+        post_data = {
+            'title': "Form page!",
+            'content': "Some content",
+            'slug': 'contact-us',
+            'form_fields-TOTAL_FORMS': '3',
+            'form_fields-INITIAL_FORMS': '3',
+            'form_fields-MIN_NUM_FORMS': '0',
+            'form_fields-MAX_NUM_FORMS': '1000',
+            'form_fields-0-id': '',
+            'form_fields-0-label': 'foo',
+            'form_fields-0-field_type': 'singleline',
+            'form_fields-1-id': '',
+            'form_fields-1-label': 'foo',
+            'form_fields-1-field_type': 'singleline',
+            'form_fields-2-id': '',
+            'form_fields-2-label': 'bar',
+            'form_fields-2-field_type': 'singleline',
+        }
+        response = self.client.post(
+            reverse('wagtailadmin_pages:add', args=('tests', 'formpage', self.root_page.id)), post_data
+        )
+
+        self.assertEqual(response.status_code, 200)
+
+        self.assertContains(
+            response,
+            text="There is another field with the label foo, please change one of them.",
+        )