From 3eac584c7c6a73db9cf8c238c806357d1f3b2c3e Mon Sep 17 00:00:00 2001 From: LB Date: Wed, 29 Nov 2017 17:27:26 +0800 Subject: [PATCH] Make FormBuilder more easy to extend --- CHANGELOG.txt | 1 + .../reference/contrib/forms/customisation.rst | 64 ++++++++++++++- docs/releases/2.0.rst | 1 + wagtail/contrib/forms/forms.py | 43 ++++++----- wagtail/contrib/forms/tests/test_forms.py | 52 ++++++++++++- wagtail/contrib/forms/tests/test_models.py | 77 ++++++++++++++++++- wagtail/contrib/forms/tests/test_views.py | 61 ++++++++++++++- .../migrations/0027_auto_20180110_1727.py | 53 +++++++++++++ wagtail/tests/testapp/models.py | 53 ++++++++++++- .../form_page_with_custom_form_builder.html | 11 +++ ...page_with_custom_form_builder_landing.html | 10 +++ 11 files changed, 396 insertions(+), 30 deletions(-) create mode 100644 wagtail/tests/testapp/migrations/0027_auto_20180110_1727.py create mode 100644 wagtail/tests/testapp/templates/tests/form_page_with_custom_form_builder.html create mode 100644 wagtail/tests/testapp/templates/tests/form_page_with_custom_form_builder_landing.html diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 0f302b6d67..04b7d1e6b1 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -31,6 +31,7 @@ Changelog * Rich text fields now take feature lists into account when whitelisting HTML elements (Matt Westcott) * FormPage lists and Form submission lists in admin now use class based views for easy overriding (Johan Arensman) * Form submission csv exports now have the export date in the filename and can be customized (Johan Arensman) + * FormBuilder class now uses bound methods for field generation, adding custom fields is now easier and documented (LB (Ben) Johnston) * Fix: Do not remove stopwords when generating slugs from non-ASCII titles, to avoid issues with incorrect word boundaries (Sævar Öfjörð Magnússon) * Fix: The PostgreSQL search backend now preserves ordering of the `QuerySet` when searching with `order_by_relevance=False` (Bertrand Bordage) * Fix: Using `modeladmin_register` as a decorator no longer replaces the decorated class with `None` (Tim Heap) diff --git a/docs/reference/contrib/forms/customisation.rst b/docs/reference/contrib/forms/customisation.rst index 1a0f198bad..870637992c 100644 --- a/docs/reference/contrib/forms/customisation.rst +++ b/docs/reference/contrib/forms/customisation.rst @@ -549,7 +549,6 @@ Next, you need to transform your template to display the results: You can also show the results on the landing page. - Custom landing page redirect ---------------------------- @@ -605,7 +604,6 @@ Finally, we add a URL param of `id` based on the ``form_submission`` if it exist ], 'Email'), ] - Customise form submissions listing in Wagtail Admin --------------------------------------------------- @@ -628,7 +626,7 @@ Example: # override the method to generate csv filename def get_csv_filename(self): - """ Returns the filename for CSV file with page title at start""" + """ Returns the filename for CSV file with page slug at start""" filename = super().get_csv_filename() return self.form_page.slug + '-' + filename @@ -647,3 +645,63 @@ Example: thank_you_text = RichTextField(blank=True) # content_panels = ... + +Adding a custom field type +-------------------------- + +First, make the new field type available in the page editor by changing your ``FormField`` model. + +* Create a new set of choices which includes the original ``FORM_FIELD_CHOICES`` along with new field types you want to make available. +* Each choice must contain a unique key and a human readable name of the field, e.g. ``('slug', 'URL Slug')`` +* Override the ``field_type`` field in your ``FormField`` model with ``choices`` attribute using these choices. +* You will need to run ``./manage.py makemigrations`` and ``./manage.py migrate`` after this step. + + +Then, create and use a new form builder class. + +* Define a new form builder class that extends the ``FormBuilder`` class. +* Add a method that will return a created Django form field for the new field type. +* Its name must be in the format: ``create__field``, e.g. ``create_slug_field`` +* Override the ``form_builder`` attribute in your form page model to use your new form builder class. + +Example: + +.. code-block:: python + + from django import forms + from django.db import models + from modelcluster.fields import ParentalKey + from wagtail.contrib.forms.forms import FormBuilder + from wagtail.contrib.forms.models import ( + AbstractEmailForm, AbstractFormField, FORM_FIELD_CHOICES) + + + class FormField(AbstractFormField): + # extend the built in field type choices + # our field type key will be 'ipaddress' + CHOICES = FORM_FIELD_CHOICES + (('ipaddress', 'IP Address'),) + + page = ParentalKey('FormPage', related_name='form_fields') + # override the field_type field with extended choices + field_type = models.CharField( + verbose_name='field type', + max_length=16, + # use the choices tuple defined above + choices=CHOICES + ) + + + class CustomFormBuilder(FormBuilder): + # create a function that returns an instanced Django form field + # function name must match create__field + def create_ipaddress_field(self, field, options): + # return `forms.GenericIPAddressField(**options)` not `forms.SlugField` + # returns created a form field with the options passed in + return forms.GenericIPAddressField(**options) + + + class FormPage(AbstractEmailForm): + # intro, thank_you_text, edit_handlers, etc... + + # use custom form builder defined above + form_builder = CustomFormBuilder diff --git a/docs/releases/2.0.rst b/docs/releases/2.0.rst index e5f51071ec..5260fb3887 100644 --- a/docs/releases/2.0.rst +++ b/docs/releases/2.0.rst @@ -44,6 +44,7 @@ Other features * Rich text fields now take feature lists into account when whitelisting HTML elements (Matt Westcott) * FormPage lists and Form submission lists in admin now use class based views for easy overriding (Johan Arensman) * Form submission csv exports now have the export date in the filename and can be customized (Johan Arensman) + * FormBuilder class now uses bound methods for field generation, adding custom fields is now easier and documented (LB (Ben Johnston)) Bug fixes ~~~~~~~~~ diff --git a/wagtail/contrib/forms/forms.py b/wagtail/contrib/forms/forms.py index ac6620e1ed..c8ad844ae0 100644 --- a/wagtail/contrib/forms/forms.py +++ b/wagtail/contrib/forms/forms.py @@ -77,21 +77,27 @@ class FormBuilder: def create_hidden_field(self, field, options): return django.forms.CharField(widget=django.forms.HiddenInput, **options) - FIELD_TYPES = { - 'singleline': create_singleline_field, - 'multiline': create_multiline_field, - 'date': create_date_field, - 'datetime': create_datetime_field, - 'email': create_email_field, - 'url': create_url_field, - 'number': create_number_field, - 'dropdown': create_dropdown_field, - 'multiselect': create_multiselect_field, - 'radio': create_radio_field, - 'checkboxes': create_checkboxes_field, - 'checkbox': create_checkbox_field, - 'hidden': create_hidden_field, - } + def get_create_field_function(self, type): + """ + Takes string of field type and returns a Django Form Field Instance. + Assumes form field creation functions are in the format: + 'create_fieldtype_field' + """ + create_field_function = getattr(self, 'create_%s_field' % type, None) + if create_field_function: + return create_field_function + else: + import inspect + method_list = [ + f[0] for f in + inspect.getmembers(self.__class__, inspect.isfunction) + if f[0].startswith('create_') and f[0].endswith('_field') + ] + raise AttributeError( + "Could not find function matching format \ + create__field for type: " + type, + "Must be one of: " + ", ".join(method_list) + ) @property def formfields(self): @@ -99,11 +105,8 @@ class FormBuilder: for field in self.fields: options = self.get_field_options(field) - - if field.field_type in self.FIELD_TYPES: - formfields[field.clean_name] = self.FIELD_TYPES[field.field_type](self, field, options) - else: - raise Exception("Unrecognised field type: " + field.field_type) + create_field = self.get_create_field_function(field.field_type) + formfields[field.clean_name] = create_field(field, options) return formfields diff --git a/wagtail/contrib/forms/tests/test_forms.py b/wagtail/contrib/forms/tests/test_forms.py index 81784497bb..8f149c5c56 100644 --- a/wagtail/contrib/forms/tests/test_forms.py +++ b/wagtail/contrib/forms/tests/test_forms.py @@ -2,9 +2,10 @@ from django import forms from django.test import TestCase -from wagtail.tests.testapp.models import FormField, FormPage -from wagtail.core.models import Page from wagtail.contrib.forms.forms import FormBuilder +from wagtail.core.models import Page +from wagtail.tests.testapp.models import ( + ExtendedFormField, FormField, FormPage, FormPageWithCustomFormBuilder) class TestFormBuilder(TestCase): @@ -162,3 +163,50 @@ class TestFormBuilder(TestCase): self.assertIsInstance(form_class.base_fields['your-favourite-python-ide'].widget, forms.RadioSelect) self.assertIsInstance(form_class.base_fields['your-choices'].widget, forms.CheckboxSelectMultiple) self.assertIsInstance(form_class.base_fields['a-hidden-field'].widget, forms.HiddenInput) + + +class TestCustomFormBuilder(TestCase): + def setUp(self): + # Create a form page + home_page = Page.objects.get(url_path='/home/') + + self.form_page = home_page.add_child( + instance=FormPageWithCustomFormBuilder( + title='IT Support Request', + slug='it-support-request', + to_address='it@jenkins.com', + from_address='support@jenkins.com', + subject='Support Request Submitted', + ) + ) + ExtendedFormField.objects.create( + page=self.form_page, + sort_order=1, + label='Name', + field_type='singleline', + required=True, + ) + + def test_using_custom_form_builder(self): + """Tests that charfield max_length is 120 characters.""" + form_class = self.form_page.get_form_class() + form = form_class() + # check name field exists + self.assertIsInstance(form.base_fields['name'], forms.CharField) + # check max_length is set + self.assertEqual(form.base_fields['name'].max_length, 120) + + def test_adding_custom_field(self): + """Tests that we can add the ipaddress field, which is an extended choice.""" + ExtendedFormField.objects.create( + page=self.form_page, + sort_order=1, + label='Device IP Address', + field_type='ipaddress', + required=True, + ) + form_class = self.form_page.get_form_class() + form = form_class() + # check ip address field used + self.assertIsInstance( + form.base_fields['device-ip-address'], forms.GenericIPAddressField) diff --git a/wagtail/contrib/forms/tests/test_models.py b/wagtail/contrib/forms/tests/test_models.py index 9e17c74d0a..fd923e621e 100644 --- a/wagtail/contrib/forms/tests/test_models.py +++ b/wagtail/contrib/forms/tests/test_models.py @@ -8,7 +8,8 @@ from wagtail.contrib.forms.models import FormSubmission from wagtail.contrib.forms.tests.utils import ( make_form_page, make_form_page_with_custom_submission, make_form_page_with_redirect) from wagtail.core.models import Page -from wagtail.tests.testapp.models import CustomFormPageSubmission, FormField, JadeFormPage +from wagtail.tests.testapp.models import ( + CustomFormPageSubmission, ExtendedFormField, FormField, FormPageWithCustomFormBuilder, JadeFormPage) from wagtail.tests.utils import WagtailTestUtils @@ -400,6 +401,80 @@ class TestFormWithRedirect(TestCase): self.assertTrue(FormSubmission.objects.filter(page=form_page, form_data__contains='hello world').exists()) +class TestFormPageWithCustomFormBuilder(TestCase, WagtailTestUtils): + + def setUp(self): + + home_page = Page.objects.get(url_path='/home/') + form_page = home_page.add_child( + instance=FormPageWithCustomFormBuilder( + title='Support Request', + slug='support-request', + to_address='it@jenkins.com', + from_address='support@jenkins.com', + subject='Support Request Submitted', + ) + ) + ExtendedFormField.objects.create( + page=form_page, + sort_order=1, + label='Name', + field_type='singleline', # singleline field will be max_length 120 + required=True, + ) + ExtendedFormField.objects.create( + page=form_page, + sort_order=1, + label='Device IP Address', + field_type='ipaddress', + required=True, + ) + + def test_get_form(self): + response = self.client.get('/support-request/') + + # Check response + self.assertTemplateUsed(response, 'tests/form_page_with_custom_form_builder.html') + self.assertTemplateNotUsed(response, 'tests/form_page_with_custom_form_builder_landing.html') + self.assertContains(response, 'Support Request', html=True) + # check that max_length attribute has been passed into form + self.assertContains(response, '', html=True) + # check ip address field has rendered + self.assertContains(response, '', html=True) + + def test_post_invalid_form(self): + response = self.client.post('/support-request/', { + 'name': 'very long name longer than 120 characters' * 3, # invalid + 'device-ip-address': '192.0.2.30', # valid + }) + # Check response with invalid character count + self.assertContains(response, 'Ensure this value has at most 120 characters (it has 123)') + self.assertTemplateUsed(response, 'tests/form_page_with_custom_form_builder.html') + self.assertTemplateNotUsed(response, 'tests/form_page_with_custom_form_builder_landing.html') + + response = self.client.post('/support-request/', { + 'name': 'Ron Johnson', # valid + 'device-ip-address': '3300.192.0.2.30', # invalid + }) + # Check response with invalid character count + self.assertContains(response, 'Enter a valid IPv4 or IPv6 address.') + self.assertTemplateUsed(response, 'tests/form_page_with_custom_form_builder.html') + self.assertTemplateNotUsed(response, 'tests/form_page_with_custom_form_builder_landing.html') + + def test_post_valid_form(self): + response = self.client.post('/support-request/', { + 'name': 'Ron Johnson', + 'device-ip-address': '192.0.2.30', + }) + + # Check response + self.assertContains(response, 'Thank you for submitting a Support Request.') + self.assertContains(response, 'Ron Johnson') + self.assertContains(response, '192.0.2.30') + self.assertTemplateNotUsed(response, 'tests/form_page_with_custom_form_builder.html') + self.assertTemplateUsed(response, 'tests/form_page_with_custom_form_builder_landing.html') + + class TestIssue798(TestCase): fixtures = ['test.json'] diff --git a/wagtail/contrib/forms/tests/test_views.py b/wagtail/contrib/forms/tests/test_views.py index 5113b6e72b..a810b2e006 100644 --- a/wagtail/contrib/forms/tests/test_views.py +++ b/wagtail/contrib/forms/tests/test_views.py @@ -12,9 +12,9 @@ from wagtail.contrib.forms.models import FormSubmission from wagtail.contrib.forms.tests.utils import make_form_page, make_form_page_with_custom_submission from wagtail.core.models import Page from wagtail.tests.testapp.models import ( - CustomFormPageSubmission, FormField, FormFieldForCustomListViewPage, - FormFieldWithCustomSubmission, FormPage, FormPageWithCustomSubmission, - FormPageWithCustomSubmissionListView) + CustomFormPageSubmission, ExtendedFormField, FormField, FormFieldForCustomListViewPage, + FormFieldWithCustomSubmission, FormPage, FormPageWithCustomFormBuilder, + FormPageWithCustomSubmission, FormPageWithCustomSubmissionListView) from wagtail.tests.utils import WagtailTestUtils @@ -1132,6 +1132,61 @@ class TestFormsWithCustomSubmissionsList(TestCase, WagtailTestUtils): self.assertTrue('Old chocolate idea' in first_row_values) +class TestFormsWithCustomFormBuilderSubmissionsList(TestCase, WagtailTestUtils): + + def setUp(self): + home_page = Page.objects.get(url_path='/home/') + form_page = home_page.add_child( + instance=FormPageWithCustomFormBuilder( + title='Support Request', + slug='support-request', + to_address='it@jenkins.com', + from_address='support@jenkins.com', + subject='Support Request Submitted', + ) + ) + ExtendedFormField.objects.create( + page=form_page, + sort_order=1, + label='Name', + field_type='singleline', # singleline field will be max_length 120 + required=True, + ) + ExtendedFormField.objects.create( + page=form_page, + sort_order=1, + label='Device IP Address', + field_type='ipaddress', + required=True, + ) + + for i in range(20): + submission = FormSubmission.objects.create( + page=form_page, + form_data=json.dumps({ + 'name': 'John %s' % i, + 'device-ip-address': '192.0.2.%s' % i, + }), + ) + submission.save() + self.form_page = form_page + # Login + self.login() + + def test_list_submissions(self): + response = self.client.get( + reverse('wagtailforms:list_submissions', args=(self.form_page.id,))) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailforms/index_submissions.html') + self.assertEqual(len(response.context['data_rows']), 20) + + # check display of list values within form submissions + self.assertContains(response, '192.0.2.1') + self.assertContains(response, '192.0.2.15') + + class TestIssue585(TestCase): fixtures = ['test.json'] diff --git a/wagtail/tests/testapp/migrations/0027_auto_20180110_1727.py b/wagtail/tests/testapp/migrations/0027_auto_20180110_1727.py new file mode 100644 index 0000000000..9618ed7226 --- /dev/null +++ b/wagtail/tests/testapp/migrations/0027_auto_20180110_1727.py @@ -0,0 +1,53 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.6 on 2018-01-10 08:27 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion +import modelcluster.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('wagtailcore', '0040_page_draft_title'), + ('tests', '0026_auto_20171207_1657'), + ] + + operations = [ + migrations.CreateModel( + name='ExtendedFormField', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('sort_order', models.IntegerField(blank=True, editable=False, null=True)), + ('label', models.CharField(help_text='The label of the form field', max_length=255, verbose_name='label')), + ('required', models.BooleanField(default=True, verbose_name='required')), + ('choices', models.TextField(blank=True, help_text='Comma separated list of choices. Only applicable in checkboxes, radio and dropdown.', verbose_name='choices')), + ('default_value', models.CharField(blank=True, help_text='Default value. Comma separated values supported for checkboxes.', max_length=255, verbose_name='default value')), + ('help_text', models.CharField(blank=True, max_length=255, verbose_name='help text')), + ('field_type', models.CharField(choices=[('singleline', 'Single line text'), ('multiline', 'Multi-line text'), ('email', 'Email'), ('number', 'Number'), ('url', 'URL'), ('checkbox', 'Checkbox'), ('checkboxes', 'Checkboxes'), ('dropdown', 'Drop down'), ('multiselect', 'Multiple select'), ('radio', 'Radio buttons'), ('date', 'Date'), ('datetime', 'Date/time'), ('hidden', 'Hidden field'), ('ipaddress', 'IP Address')], max_length=16, verbose_name='field type')), + ], + options={ + 'ordering': ['sort_order'], + 'abstract': False, + }, + ), + migrations.CreateModel( + name='FormPageWithCustomFormBuilder', + fields=[ + ('page_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='wagtailcore.Page')), + ('to_address', models.CharField(blank=True, help_text='Optional - form submissions will be emailed to these addresses. Separate multiple addresses by comma.', max_length=255, verbose_name='to address')), + ('from_address', models.CharField(blank=True, max_length=255, verbose_name='from address')), + ('subject', models.CharField(blank=True, max_length=255, verbose_name='subject')), + ], + options={ + 'abstract': False, + }, + bases=('wagtailcore.page',), + ), + migrations.AddField( + model_name='extendedformfield', + name='page', + field=modelcluster.fields.ParentalKey(on_delete=django.db.models.deletion.CASCADE, related_name='form_fields', to='tests.FormPageWithCustomFormBuilder'), + ), + ] diff --git a/wagtail/tests/testapp/models.py b/wagtail/tests/testapp/models.py index 44418a43ac..21acea43c2 100644 --- a/wagtail/tests/testapp/models.py +++ b/wagtail/tests/testapp/models.py @@ -2,6 +2,7 @@ import hashlib import json import os +from django import forms from django.conf import settings from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType @@ -21,7 +22,9 @@ from wagtail.admin.edit_handlers import ( StreamFieldPanel, TabbedInterface) from wagtail.admin.forms import WagtailAdminPageForm from wagtail.admin.utils import send_mail -from wagtail.contrib.forms.models import AbstractEmailForm, AbstractFormField, AbstractFormSubmission +from wagtail.contrib.forms.forms import FormBuilder +from wagtail.contrib.forms.models import ( + AbstractEmailForm, AbstractFormField, AbstractFormSubmission, FORM_FIELD_CHOICES) from wagtail.contrib.settings.models import BaseSetting, register_setting from wagtail.contrib.table_block.blocks import TableBlock from wagtail.core.blocks import CharBlock, RichTextBlock @@ -610,6 +613,54 @@ class FormPageWithCustomSubmissionListView(AbstractEmailForm): ] +# FormPage with cutom FormBuilder + +EXTENDED_CHOICES = FORM_FIELD_CHOICES + (('ipaddress', 'IP Address'),) + + +class ExtendedFormField(AbstractFormField): + """Override the field_type field with extended choices.""" + page = ParentalKey( + 'FormPageWithCustomFormBuilder', + related_name='form_fields', + on_delete=models.CASCADE) + field_type = models.CharField( + verbose_name='field type', max_length=16, choices=EXTENDED_CHOICES) + + +class CustomFormBuilder(FormBuilder): + """ + A custom FormBuilder that has an 'ipaddress' field with + customised create_singleline_field with shorter max_length + """ + + def create_singleline_field(self, field, options): + options['max_length'] = 120 # usual default is 255 + return forms.CharField(**options) + + def create_ipaddress_field(self, field, options): + return forms.GenericIPAddressField(**options) + + +class FormPageWithCustomFormBuilder(AbstractEmailForm): + """ + A Form page that has a custom form builder and uses a custom + form field model with additional field_type choices. + """ + + form_builder = CustomFormBuilder + + content_panels = [ + FieldPanel('title', classname="full title"), + InlinePanel('form_fields', label="Form fields"), + MultiFieldPanel([ + FieldPanel('to_address', classname="full"), + FieldPanel('from_address', classname="full"), + FieldPanel('subject', classname="full"), + ], "Email") + ] + + # Snippets class AdvertPlacement(models.Model): page = ParentalKey('wagtailcore.Page', related_name='advert_placements', on_delete=models.CASCADE) diff --git a/wagtail/tests/testapp/templates/tests/form_page_with_custom_form_builder.html b/wagtail/tests/testapp/templates/tests/form_page_with_custom_form_builder.html new file mode 100644 index 0000000000..386c72ada5 --- /dev/null +++ b/wagtail/tests/testapp/templates/tests/form_page_with_custom_form_builder.html @@ -0,0 +1,11 @@ +{% extends "tests/base.html" %} +{% load wagtailcore_tags %} + +{% block content %} +

{{ greeting }}

+
+ {% csrf_token %} + {{ form.as_p }} + +
+{% endblock %} diff --git a/wagtail/tests/testapp/templates/tests/form_page_with_custom_form_builder_landing.html b/wagtail/tests/testapp/templates/tests/form_page_with_custom_form_builder_landing.html new file mode 100644 index 0000000000..dff020a3b9 --- /dev/null +++ b/wagtail/tests/testapp/templates/tests/form_page_with_custom_form_builder_landing.html @@ -0,0 +1,10 @@ +{% extends "tests/base.html" %} + +{% block content %} +

Thank you for submitting a Support Request.

+
    + {% for key, value in form_submission.get_data.items %} +
  • {{ key }}: {{ value }}
    • + {% endfor %} +
+{% endblock %}