Merge pull request #29 from incuna/form-buttons

Use <button type=submit> instead of <input>

changelog.md

@@ -1,3 +1,8 @@
+Unreleased
+----------
+
+* Use html `buttons` instead of `inputs` for form submission.
+
 v1.1.0
 ------
 

groups/forms.py

@@ -1,6 +1,6 @@
-from crispy_forms.bootstrap import FormActions
+from crispy_forms.bootstrap import FormActions, StrictButton
 from crispy_forms.helper import FormHelper
-from crispy_forms.layout import Layout, Submit
+from crispy_forms.layout import Layout
 from django import forms
 from django.core.urlresolvers import reverse_lazy
 
@@ -22,7 +22,7 @@ def build_helper(self):
         helper.layout = Layout(
             'body',
             FormActions(
-                Submit('comment-submit', 'Post comment'),
+                StrictButton('Post comment', type='submit'),
             ),
         )
         return helper
@@ -44,7 +44,7 @@ def build_helper(self):
         helper.layout = Layout(
             'file',
             FormActions(
-                Submit('comment-submit', 'Upload this file'),
+                StrictButton('Upload this file', type='submit'),
             ),
         )
         return helper
@@ -65,7 +65,7 @@ class DiscussionCreate(forms.Form):
         'name',
         'comment',
         FormActions(
-            Submit('submit', 'Create Discussion'),
+            StrictButton('Create Discussion', type='submit'),
         ),
     )
 
@@ -95,7 +95,7 @@ def __init__(self, user, instance, url_name, *args, **kwargs):
         self.helper.form_class = 'form-horizontal'
         self.helper.layout = Layout(
             FormActions(
-                Submit('subscribe-submit', button_text),
+                StrictButton(button_text, type='submit'),
             ),
         )
         self.helper.form_action = reverse_lazy(url_name, kwargs={'pk': instance.pk})

groups/tests/test_forms.py

@@ -1,3 +1,5 @@
+from bs4 import BeautifulSoup
+from crispy_forms.utils import render_crispy_form
 from incuna_test_utils.compat import Python2AssertMixin
 from incuna_test_utils.factories.images import uploadable_file
 
@@ -6,6 +8,18 @@
 from .. import forms, models
 
 
+def has_submit(form):
+    rendered_form = render_crispy_form(form)
+    parser = BeautifulSoup(rendered_form, 'html.parser')
+    submits = parser.findAll('input', {'type': 'submit'})
+    return any(submits)
+
+
+def get_button(form):
+    rendered_form = render_crispy_form(form)
+    return BeautifulSoup(rendered_form, 'html.parser').findAll('button')[0]
+
+
 class TestAddTextComment(Python2AssertMixin, RequestTestCase):
     form = forms.AddTextComment
     model = models.TextComment
@@ -17,7 +31,6 @@ def test_form_fields(self):
 
     def test_form_valid(self):
         data = {'body': 'I am a comment'}
-
         form = self.form(data=data)
         self.assertTrue(form.is_valid(), msg=form.errors)
 
@@ -26,6 +39,20 @@ def test_form_not_valid(self):
         self.assertFalse(form.is_valid())
         self.assertIn('body', form.errors)
 
+    def test_submit_not_input(self):
+        """The form does not have a submit <input>."""
+        form = self.form()
+        self.assertFalse(has_submit(form))
+
+    def test_submit_button(self):
+        """
+        The form has a submit <button>.
+
+        This allows for more flexibility in styling.
+        """
+        button = get_button(self.form())
+        self.assertEqual(button.get('type'), 'submit')
+
 
 class TestAddFileComment(Python2AssertMixin, RequestTestCase):
     form = forms.AddFileComment
@@ -38,7 +65,6 @@ def test_form_fields(self):
 
     def test_form_valid(self):
         file_data = {'file': uploadable_file()}
-
         form = self.form(files=file_data)
         self.assertTrue(form.is_valid(), msg=form.errors)
 
@@ -47,6 +73,20 @@ def test_form_not_valid(self):
         self.assertFalse(form.is_valid())
         self.assertIn('file', form.errors)
 
+    def test_submit_not_input(self):
+        """The form does not have a submit <input>."""
+        form = self.form()
+        self.assertFalse(has_submit(form))
+
+    def test_submit_button(self):
+        """
+        The form has a submit <button>.
+
+        This allows for more flexibility in styling.
+        """
+        button = get_button(self.form())
+        self.assertEqual(button.get('type'), 'submit')
+
 
 class TestDiscussionCreateForm(Python2AssertMixin, RequestTestCase):
     form = forms.DiscussionCreate
@@ -70,6 +110,20 @@ def test_form_not_valid(self):
         self.assertFalse(form.is_valid())
         self.assertIn('name', form.errors)
 
+    def test_submit_not_input(self):
+        """The form does not have a submit <input>."""
+        form = self.form()
+        self.assertFalse(has_submit(form))
+
+    def test_submit_button(self):
+        """
+        The form has a submit <button>.
+
+        This allows for more flexibility in styling.
+        """
+        button = get_button(self.form())
+        self.assertEqual(button.get('type'), 'submit')
+
 
 class TestSubscribeForm(Python2AssertMixin, RequestTestCase):
     form = forms.SubscribeForm
@@ -88,17 +142,38 @@ def get_form(self, **kwargs):
 
     def test_form_valid(self):
         form = self.get_form(data={})
-
         self.assertTrue(form.is_valid(), msg=form.errors)
 
     def test_initial_subscribe(self):
         form = self.get_form()
-
         self.assertTrue(form.initial['subscribe'])
 
     def test_initial_unsubscribe(self):
         self.discussion.subscribers.add(self.user)
+        form = self.get_form()
+        self.assertFalse(form.initial['subscribe'])
 
+    def test_submit_not_input(self):
+        """The form does not have a submit <input>."""
         form = self.get_form()
+        self.assertFalse(has_submit(form))
 
-        self.assertFalse(form.initial['subscribe'])
+    def test_submit_button(self):
+        """
+        The form has a submit <button>.
+
+        This allows for more flexibility in styling.
+        """
+        button = get_button(self.get_form())
+        self.assertEqual(button.get('type'), 'submit')
+
+    def test_button_text_subscribe(self):
+        """The button says 'Subscribe' when the user is not subscribed."""
+        button = get_button(self.get_form())
+        self.assertEqual(button.string, 'Subscribe')
+
+    def test_button_text_unsubscribe(self):
+        """The button says 'Unsubscribe' when the user is subscribed."""
+        self.discussion.subscribers.add(self.user)
+        button = get_button(self.get_form())
+        self.assertEqual(button.string, 'Unsubscribe')

groups/tests/test_views.py

@@ -93,29 +93,18 @@ def test_form_unsubscribe(self):
 class TestGroupSubscribeIntegration(WebTest):
     def setUp(self):
         self.user = factories.UserFactory.create()
+        self.group = factories.GroupFactory.create()
+        self.group_url = reverse('group-detail', kwargs={'pk': self.group.pk})
 
     def test_subscribe_unsubscribe(self):
         """A user can subscribe and unsubscribe from a group."""
-        group = factories.GroupFactory.create()
-        group_url = reverse('group-detail', kwargs={'pk': group.pk})
-
-        # A user not yet subscribed sees a Subscribe button
-        form = self.app.get(group_url, user=self.user).form
-        submit_button = form.fields['subscribe-submit'][0]
-        self.assertEqual(submit_button._value, 'Subscribe')
-
         # A user is subscribed to the group if they click the button
-        form.submit()
-        self.assertIn(self.user, group.watchers.all())
-
-        # A subscribed user sees an Unsubscribe button
-        form = self.app.get(group_url, user=self.user).form
-        submit_button = form.fields['subscribe-submit'][0]
-        self.assertEqual(submit_button._value, 'Unsubscribe')
+        self.app.get(self.group_url, user=self.user).form.submit()
+        self.assertIn(self.user, self.group.watchers.all())
 
         # A user is unsubscribed from the group if they click the button
-        form.submit()
-        self.assertNotIn(self.user, group.watchers.all())
+        self.app.get(self.group_url, user=self.user).form.submit()
+        self.assertNotIn(self.user, self.group.watchers.all())
 
 
 class TestCommentEmailMixin(RequestTestCase):

requirements.txt

@@ -1,4 +1,5 @@
 -e .
+beautifulsoup4==4.4.1
 colour-runner==0.0.4
 coverage==3.7.1
 dj-database-url==0.3.0