From 169045b186f15352a9526f4c5cf07329676e7c69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A6var=20=C3=96fj=C3=B6r=C3=B0=20Magn=C3=BAsson?= Date: Tue, 25 Oct 2016 21:55:45 +0000 Subject: [PATCH] Loop through all multiple value fields when sending form submission by email. A form field with multiple checkboxes may introduce submissions that have more than one value. The `send_email` function would loop through all form fields and grab only the first item of each field, ignoring fields that may have multiple values selected. This is a side effect of using `QueryDict.get()`, which will only return the last value of a list. This commit fixes that by first converting the `QueryDict` instance to a regular dict of lists so that `get` will return all values, then joining multiple items if needed. --- CHANGELOG.txt | 1 + docs/releases/1.8.rst | 1 + wagtail/wagtailforms/models.py | 8 +++++++- wagtail/wagtailforms/tests/test_models.py | 9 ++++++++- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 8c1b31777a..d7efbad1e0 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -19,6 +19,7 @@ Changelog * Fix: ModelAdmin views now consistently call `get_context_data` (Andy Babic) * Fix: Header for search results on the redirects index page now shows the correct count when the listing is paginated (Nick Smith) * Fix: `set_url_paths` management command is now compatible with Django 1.10 (Benjamin Bach) + * Fix: Form builder email notifications now output multiple values correctly (Sævar Öfjörð Magnússon) 1.7 (20.10.2016) diff --git a/docs/releases/1.8.rst b/docs/releases/1.8.rst index 48ef138376..e5beced50f 100644 --- a/docs/releases/1.8.rst +++ b/docs/releases/1.8.rst @@ -41,6 +41,7 @@ Bug fixes * ModelAdmin views now consistently call ``get_context_data`` (Andy Babic) * Header for search results on the redirects index page now shows the correct count when the listing is paginated (Nick Smith) * ``set_url_paths`` management command is now compatible with Django 1.10 (Benjamin Bach) + * Form builder email notifications now output multiple values correctly (Sævar Öfjörð Magnússon) Upgrade considerations ====================== diff --git a/wagtail/wagtailforms/models.py b/wagtail/wagtailforms/models.py index fab52b5cb3..d3ac520646 100644 --- a/wagtail/wagtailforms/models.py +++ b/wagtail/wagtailforms/models.py @@ -295,7 +295,13 @@ class AbstractEmailForm(AbstractForm): def send_mail(self, form): addresses = [x.strip() for x in self.to_address.split(',')] - content = '\n'.join([x[1].label + ': ' + text_type(form.data.get(x[0])) for x in form.fields.items()]) + content = [] + for field in form: + value = field.value() + if isinstance(value, list): + value = ', '.join(value) + content.append('{}: {}'.format(field.label, value)) + content = '\n'.join(content) send_mail(self.subject, content, addresses, self.from_address,) class Meta: diff --git a/wagtail/wagtailforms/tests/test_models.py b/wagtail/wagtailforms/tests/test_models.py index 065ec66340..a960be1cca 100644 --- a/wagtail/wagtailforms/tests/test_models.py +++ b/wagtail/wagtailforms/tests/test_models.py @@ -104,6 +104,13 @@ class TestFormSubmission(TestCase): self.assertIn("bar", submission[0].form_data) self.assertIn("baz", submission[0].form_data) + # Check that the all the multiple checkbox values are serialised in the + # email correctly + self.assertEqual(len(mail.outbox), 1) + self.assertIn("bar", mail.outbox[0].body) + self.assertIn("foo", mail.outbox[0].body) + self.assertIn("baz", mail.outbox[0].body) + def test_post_blank_checkbox(self): response = self.client.post('/contact-us/', { 'your-email': 'bob@example.com', @@ -118,7 +125,7 @@ class TestFormSubmission(TestCase): # Check that the checkbox was serialised in the email correctly self.assertEqual(len(mail.outbox), 1) - self.assertIn("Your choices: None", mail.outbox[0].body) + self.assertIn("Your choices: ", mail.outbox[0].body) class TestFormWithCustomSubmission(TestCase, WagtailTestUtils):