Merge pull request #11 from AccentDesign/bug_#3

Bug #3

README.rst

@@ -1,7 +1,7 @@
 Wagtail StreamForms
 ===================
 
-|CircleCI| |Coverage Status|
+|CircleCI| |Codecov|
 
 This package is currently a concept but allows you to add add forms that
 are built in the cms admin area to any streamfield. You can also create
@@ -109,5 +109,5 @@ or run for a single environment
 
 .. |CircleCI| image:: https://circleci.com/gh/AccentDesign/wagtailstreamforms/tree/master.svg?style=svg
    :target: https://circleci.com/gh/AccentDesign/wagtailstreamforms/tree/master
-.. |Coverage Status| image:: https://coveralls.io/repos/github/AccentDesign/wagtailstreamforms/badge.svg?branch=master
-   :target: https://coveralls.io/github/AccentDesign/wagtailstreamforms?branch=master
+.. |Codecov| image:: https://codecov.io/gh/AccentDesign/wagtailstreamforms/branch/master/graph/badge.svg
+   :target: https://codecov.io/gh/AccentDesign/wagtailstreamforms

circle.yml

@@ -11,12 +11,12 @@ dependencies:
     - sudo apt-get install python-dev
   override:
     - pip -V
-    - pip install -U tox coveralls
+    - pip install -U tox codecov
     - pyenv local $TOX_PY34 $TOX_PY35 $TOX_PY36
 
 test:
   override:
     - tox -v --recreate
   post:
     - cp -r htmlcov $CIRCLE_ARTIFACTS
-    - coveralls
+    - codecov

docker-compose.yml

@@ -26,7 +26,7 @@ services:
     ports:
     - 8000:8000
   db:
-    image: postgres
+    image: postgres:9.6
     environment:
     - POSTGRES_USER=postgres
     - POSTGRES_PASSWORD=password

tests/blocks/test_form_block.py

@@ -23,13 +23,15 @@ def test_render(self):
 
         html = block.render(block.to_python({
             'form': self.form.pk,
-            'form_action': '/foo/'
+            'form_action': '/foo/',
+            'form_reference': 'some-ref'
         }))
 
         expected_html = '\n'.join([
             '<h2>Form</h2>',
             '<form action="/foo/" method="post" novalidate>',
             '<input id="id_form_id" name="form_id" type="hidden" value="%s">' % self.form.pk,
+            '<input id="id_form_reference" name="form_reference" type="hidden" value="some-ref">',
             '<div class="field-row">',
             '<label for="id_name">name</label>',
             '<input type="text" name="name" maxlength="255" required id="id_name" />',
@@ -40,25 +42,47 @@ def test_render(self):
 
         self.assertHTMLEqual(html, expected_html)
 
+    def test_clean_adds_form_reference(self):
+        block = WagtailFormBlock()
+
+        value = block.clean({
+            'form': self.form.pk,
+            'form_action': '/foo/'
+        })
+
+        self.assertIsNotNone(value.get('form_reference'))
+
+    def test_clean_keeps_existing_form_reference(self):
+        block = WagtailFormBlock()
+
+        value = block.clean({
+            'form': self.form.pk,
+            'form_action': '/foo/',
+            'form_reference': 'some-ref'
+        })
+
+        self.assertEquals(value.get('form_reference'), 'some-ref')
+
     def test_context_has_form(self):
         block = WagtailFormBlock()
 
         context = block.get_context(block.to_python({
             'form': self.form.pk,
-            'form_action': '/foo/'
+            'form_action': '/foo/',
+            'form_reference': 'some-ref'
         }))
 
         self.assertIsNotNone(context['form'])
 
     def test_context_form_is_invalid_when_parent_context_has_this_form_with_errors(self):
-        invalid_form = self.form.get_form({'form_id': self.form.id})
+        invalid_form = self.form.get_form({'form_id': self.form.id, 'form_reference': 'some-ref'})
         assert not invalid_form.is_valid()
 
         self.assertEquals(invalid_form.errors, {'name': ['This field is required.']})
 
         # this is the context a page will set for an invalid form
         parent_context = {
-            'invalid_stream_form_id': self.form.id,
+            'invalid_stream_form_reference': 'some-ref',
             'invalid_stream_form': invalid_form
         }
 
@@ -67,7 +91,8 @@ def test_context_form_is_invalid_when_parent_context_has_this_form_with_errors(s
         # get a new block context
         context = block.get_context(block.to_python({
             'form': self.form.pk,
-            'form_action': '/foo/'
+            'form_action': '/foo/',
+            'form_reference': 'some-ref'
         }), parent_context)
 
         # finally make sure the form in the block is the one with errors

tests/blocks/test_info_block.py

@@ -0,0 +1,33 @@
+from wagtailstreamforms.blocks import InfoBlock
+
+from ..test_case import AppTestCase
+
+
+class TestInfoBlockTestCase(AppTestCase):
+
+    def test_form_render_with_value(self):
+        block = InfoBlock()
+
+        test_form_html = block.render_form('foo')
+        expected_html = '\n'.join([
+            '<div style="margin-top:5px;padding:0.9em 1.2em;">foo</div>'
+        ])
+        self.assertInHTML(expected_html, test_form_html)
+
+    def test_form_render_no_value_with_help_text(self):
+        block = InfoBlock(help_text='some help')
+
+        test_form_html = block.render_form('')
+        expected_html = '\n'.join([
+            '<div style="margin-top:5px;padding:0.9em 1.2em;">some help</div>'
+        ])
+        self.assertInHTML(expected_html, test_form_html)
+
+    def test_form_render_value_and_help_text(self):
+        block = InfoBlock(help_text='some help')
+
+        test_form_html = block.render_form('foo')
+        expected_html = '\n'.join([
+            '<div style="margin-top:5px;padding:0.9em 1.2em;">foo</div>'
+        ])
+        self.assertInHTML(expected_html, test_form_html)

tests/models/test_base_form.py

@@ -130,7 +130,8 @@ def test_get_form(self):
             'date',
             'datetime',
             'regexfield',
-            'form_id'
+            'form_id',
+            'form_reference'
         ]
         self.assertEqual(actual_fields, expected_fields)
 
@@ -154,7 +155,8 @@ def test_process_form_submission__saves_record_when_store_submission_is_true(sel
             'date': '2017-01-01',
             'datetime': '2017-01-01 00:00:00',
             'regexfield': 'text',
-            'form_id': form.pk
+            'form_id': form.pk,
+            'form_reference': 'some-ref'
         }
         form_class = form.get_form(data)
         assert form_class.is_valid()
@@ -179,7 +181,8 @@ def test_process_form_submission__does_not_save_record_when_store_submission_is_
             'date': '2017-01-01',
             'datetime': '2017-01-01 00:00:00',
             'regexfield': 'text',
-            'form_id': form.pk
+            'form_id': form.pk,
+            'form_reference': 'some-ref'
         }
         form_class = form.get_form(data)
         assert form_class.is_valid()

tests/models/test_email_form.py

@@ -12,6 +12,9 @@ class ModelGenericTests(AppTestCase):
     def test_inheritance(self):
         self.assertTrue(issubclass(EmailForm, BaseForm))
 
+    def test_ignored_fields(self):
+        self.assertEquals(EmailForm.ignored_fields, ['recaptcha', 'form_id', 'form_reference'])
+
 
 class ModelFieldTests(AppTestCase):
 
@@ -58,15 +61,23 @@ def test_form(self, store_submission=False):
 
     def test_process_form_submission__sends_an_email(self):
         form = self.test_form()
-        form_class = form.get_form({'name': 'foo', 'form_id': form.pk})
+        form_class = form.get_form({
+            'name': 'foo',
+            'form_id': form.pk,
+            'form_reference': 'some-ref'
+        })
         assert form_class.is_valid()
         form.process_form_submission(form_class)
         self.assertEqual(len(mail.outbox), 1)
         self.assertEqual(mail.outbox[0].subject, form.subject)
 
     def test_process_form_submission__still_saves_submission(self):
         form = self.test_form(True)
-        form_class = form.get_form({'name': 'foo', 'form_id': form.pk})
+        form_class = form.get_form({
+            'name': 'foo',
+            'form_id': form.pk,
+            'form_reference': 'some-ref'
+        })
         assert form_class.is_valid()
         form.process_form_submission(form_class)
         self.assertEquals(form.get_submission_class().objects.count(), 1)

tests/models/test_mixins.py

@@ -40,7 +40,11 @@ def test_get_responds(self):
 
     def test_post_responds(self):
         form = self.test_form()
-        fake_request = self.rf.post('/fake/', {'form_id': form.pk, 'name': 'Bill'})
+        fake_request = self.rf.post('/fake/', {
+            'name': 'Bill',
+            'form_id': form.pk,
+            'form_reference': 'some-ref'
+        })
         fake_request.user = AnonymousUser()
 
         response = SomePage().serve(fake_request)
@@ -49,7 +53,11 @@ def test_post_responds(self):
 
     def test_post_saves_submission(self):
         form = self.test_form(True)
-        fake_request = self.rf.post('/fake/', {'form_id': form.pk, 'name': 'Bill'})
+        fake_request = self.rf.post('/fake/', {
+            'name': 'Bill',
+            'form_id': form.pk,
+            'form_reference': 'some-ref'
+        })
         fake_request.user = AnonymousUser()
 
         SomePage().serve(fake_request)
@@ -76,7 +84,11 @@ def test_no_form_id_does_not_break_view(self):
 
     def test_invalid_data_does_not_break_view(self):
         form = self.test_form()
-        fake_request = self.rf.post('/fake/', {'form_id': form.pk, 'name': ''})
+        fake_request = self.rf.post('/fake/', {
+            'name': '',
+            'form_id': form.pk,
+            'form_reference': 'some-ref'
+        })
         fake_request.user = AnonymousUser()
 
         response = SomePage().serve(fake_request)

wagtailstreamforms/blocks.py

@@ -1,10 +1,19 @@
+import uuid
+
 from django import forms
+from django.utils.safestring import mark_safe
 
 from wagtail.wagtailcore import blocks
-
 from wagtailstreamforms.models import BaseForm
 
 
+class InfoBlock(blocks.CharBlock):
+    def render_form(self, value, prefix='', errors=None):
+        field = self.field
+        shown_value = value if value else field.help_text
+        return mark_safe('<div style="margin-top:5px;padding:0.9em 1.2em;">%s</div>' % shown_value)
+
+
 class FormChooserBlock(blocks.ChooserBlock):
     target_model = BaseForm
     widget = forms.Select
@@ -32,8 +41,12 @@ def to_python(self, value):
 class WagtailFormBlock(blocks.StructBlock):
     form = FormChooserBlock()
     form_action = blocks.CharBlock(
-        default='.',
-        help_text='The action to use in the form. "." means it will post to the current page.'
+        required=False,
+        help_text='The form post action. "" or "." for the current page or a url'
+    )
+    form_reference = InfoBlock(
+        required=False,
+        help_text='This form will be given a unique reference once saved'
     )
 
     class Meta:
@@ -47,16 +60,28 @@ def render(self, value, context=None):
 
     def get_context(self, value, parent_context=None):
         context = super(WagtailFormBlock, self).get_context(value, parent_context)
+
         form = value['form']
+        form_reference = value.get('form_reference')
 
-        # see if the page has an invalid form in the context and
-        # use it if its the same id
-        invalid_form_id = context.get('invalid_stream_form_id')
+        # check the context for an invalid form submitted to the page.
+        # Use that instead if it has the same unique form_reference number
+        invalid_form_reference = context.get('invalid_stream_form_reference')
         invalid_form = context.get('invalid_stream_form')
 
-        if invalid_form_id and invalid_form and invalid_form_id == form.id:
+        if invalid_form_reference and invalid_form and invalid_form_reference == form_reference:
             context['form'] = invalid_form
         else:
-            context['form'] = form.get_form(initial={'form_id': form.id})
+            context['form'] = form.get_form(initial={'form_id': form.id, 'form_reference': form_reference})
 
         return context
+
+    def clean(self, value):
+        result = super(WagtailFormBlock, self).clean(value)
+
+        # set to a new uuid so we can ensure we can identify this form
+        # against other forms of the same type in the page
+        if not result.get('form_reference'):
+            result['form_reference'] = uuid.uuid4()
+
+        return result

wagtailstreamforms/forms.py

@@ -34,6 +34,7 @@ def formfields(self):
 
         # add form id to identify the form type
         fields['form_id'] = django.forms.CharField(widget=django.forms.HiddenInput)
+        fields['form_reference'] = django.forms.CharField(widget=django.forms.HiddenInput)
 
         # if enabled add recaptcha field
         if self.add_recaptcha and recaptcha_enabled():

wagtailstreamforms/models/form.py

@@ -146,6 +146,9 @@ class BasicForm(BaseForm):
 class EmailForm(BaseForm):
     """ A form that sends and email. """
 
+    # do not add these fields to the email
+    ignored_fields = ['recaptcha', 'form_id', 'form_reference']
+
     subject = models.CharField(
         max_length=255
     )
@@ -180,7 +183,7 @@ def send_form_mail(self, form):
         for name, field in form.fields.items():
             data = form.cleaned_data.get(name)
 
-            if name == 'recaptcha' or name == 'form_id' or not data:
+            if name in self.ignored_fields or not data:
                 continue
 
             label = field.label or name

wagtailstreamforms/models/mixins.py

@@ -10,7 +10,7 @@ class StreamFormPageMixin(object):
     Pages that require processing forms within their own streafields should inherit from it.
     """
 
-    invalid_stream_form_id = None
+    invalid_stream_form_reference = None
     invalid_stream_form = None
 
     def get_context(self, request, *args, **kwargs):
@@ -19,7 +19,7 @@ def get_context(self, request, *args, **kwargs):
         context = super(StreamFormPageMixin, self).get_context(request, *args, **kwargs)
 
         context.update({
-            'invalid_stream_form_id': self.invalid_stream_form_id,
+            'invalid_stream_form_reference': self.invalid_stream_form_reference,
             'invalid_stream_form': self.invalid_stream_form
         })
 
@@ -43,7 +43,7 @@ def serve(self, request, *args, **kwargs):
         """ If we are posting and there is a form, process it. """
 
         # reset these each time so we don't hang onto invalid forms on page round trips
-        self.invalid_stream_form_id = None
+        self.invalid_stream_form_reference = None
         self.invalid_stream_form = None
 
         if request.method == 'POST':
@@ -65,7 +65,7 @@ def serve(self, request, *args, **kwargs):
                 else:
                     # the form is invalid, set these so the FormChooserBlock
                     # can pick them up in the context
-                    self.invalid_stream_form_id = form_def.pk
+                    self.invalid_stream_form_reference = form.data.get('form_reference')
                     self.invalid_stream_form = form
 
         return super(StreamFormPageMixin, self).serve(request, *args, **kwargs)