From c98a60c80d6cbe6cee6ec7e4d2cccb602a66d7c9 Mon Sep 17 00:00:00 2001 From: Stuart George Date: Wed, 18 Oct 2017 00:48:05 +0100 Subject: [PATCH 1/4] example commit - for testing --- wagtailstreamforms/blocks.py | 41 +++++++++++++++++++++++------ wagtailstreamforms/forms.py | 1 + wagtailstreamforms/models/mixins.py | 8 +++--- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/wagtailstreamforms/blocks.py b/wagtailstreamforms/blocks.py index 4da1a467..2fa2c1ed 100644 --- a/wagtailstreamforms/blocks.py +++ b/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('
%s
' % 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 diff --git a/wagtailstreamforms/forms.py b/wagtailstreamforms/forms.py index c8f8c7e1..1be92e7f 100644 --- a/wagtailstreamforms/forms.py +++ b/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(): diff --git a/wagtailstreamforms/models/mixins.py b/wagtailstreamforms/models/mixins.py index b343aa59..b52d92a8 100644 --- a/wagtailstreamforms/models/mixins.py +++ b/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) From 106b8dd5b6a8184aa4a19518009818747f34e09b Mon Sep 17 00:00:00 2001 From: Stuart George Date: Wed, 18 Oct 2017 02:03:56 +0100 Subject: [PATCH 2/4] tests to ensure correct form validted --- tests/blocks/test_form_block.py | 35 ++++++++++++++++++++++++++----- tests/blocks/test_info_block.py | 33 +++++++++++++++++++++++++++++ tests/models/test_base_form.py | 9 +++++--- tests/models/test_email_form.py | 15 +++++++++++-- tests/models/test_mixins.py | 18 +++++++++++++--- wagtailstreamforms/models/form.py | 5 ++++- 6 files changed, 101 insertions(+), 14 deletions(-) create mode 100644 tests/blocks/test_info_block.py diff --git a/tests/blocks/test_form_block.py b/tests/blocks/test_form_block.py index 65f3975f..989f781b 100644 --- a/tests/blocks/test_form_block.py +++ b/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([ '

Form

', '
', '' % self.form.pk, + '', '
', '', '', @@ -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 diff --git a/tests/blocks/test_info_block.py b/tests/blocks/test_info_block.py new file mode 100644 index 00000000..fba295a3 --- /dev/null +++ b/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([ + '
foo
' + ]) + 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([ + '
some help
' + ]) + 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([ + '
foo
' + ]) + self.assertInHTML(expected_html, test_form_html) diff --git a/tests/models/test_base_form.py b/tests/models/test_base_form.py index 7cabd0f3..5f0416fb 100644 --- a/tests/models/test_base_form.py +++ b/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() diff --git a/tests/models/test_email_form.py b/tests/models/test_email_form.py index 3b150b14..bd1a4880 100644 --- a/tests/models/test_email_form.py +++ b/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,7 +61,11 @@ 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) @@ -66,7 +73,11 @@ def test_process_form_submission__sends_an_email(self): 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) diff --git a/tests/models/test_mixins.py b/tests/models/test_mixins.py index 2e9f36df..8fcff276 100644 --- a/tests/models/test_mixins.py +++ b/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) diff --git a/wagtailstreamforms/models/form.py b/wagtailstreamforms/models/form.py index 484c741a..3a4a1418 100644 --- a/wagtailstreamforms/models/form.py +++ b/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 From 8a1e014700d5d9f0a68314e9012f45edcf6cf195 Mon Sep 17 00:00:00 2001 From: Stuart George Date: Wed, 18 Oct 2017 08:13:40 +0100 Subject: [PATCH 3/4] lock postgres version to 9.6 --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 39337a8f..9a6c587a 100644 --- a/docker-compose.yml +++ b/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 From efc7212cddef6989c9d31fe5cd33faf2d9143b6c Mon Sep 17 00:00:00 2001 From: Stuart George Date: Wed, 18 Oct 2017 08:49:41 +0100 Subject: [PATCH 4/4] codecov --- README.rst | 6 +++--- circle.yml | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.rst b/README.rst index 96d510c0..951010c3 100644 --- a/README.rst +++ b/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 diff --git a/circle.yml b/circle.yml index 53e96940..24205178 100644 --- a/circle.yml +++ b/circle.yml @@ -11,7 +11,7 @@ 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: @@ -19,4 +19,4 @@ test: - tox -v --recreate post: - cp -r htmlcov $CIRCLE_ARTIFACTS - - coveralls \ No newline at end of file + - codecov \ No newline at end of file