From a17a2d3abec06f8550254b53de18b743220a4620 Mon Sep 17 00:00:00 2001 From: dojutsu-user Date: Sat, 17 Nov 2018 00:27:02 +0530 Subject: [PATCH 1/6] Add form validation --- readthedocs/core/forms.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/readthedocs/core/forms.py b/readthedocs/core/forms.py index 6899afa1295..bb7f5e2aab0 100644 --- a/readthedocs/core/forms.py +++ b/readthedocs/core/forms.py @@ -34,6 +34,18 @@ def __init__(self, *args, **kwargs): except AttributeError: pass + def clean_first_name(self): + first_name = self.cleaned_data.get('first_name') + if not len(first_name) <= 30: + raise forms.ValidationError('Ensure that first name has at most 30 characters.') + return first_name + + def clean_last_name(self): + last_name = self.cleaned_data.get('last_name') + if not len(last_name) <= 30: + raise forms.ValidationError('Ensure that last name has at most 30 characters.') + return last_name + def save(self, commit=True): first_name = self.cleaned_data.pop('first_name', None) last_name = self.cleaned_data.pop('last_name', None) From f1417061cc800d3bd8ca7bcb1d521cc4e18822f6 Mon Sep 17 00:00:00 2001 From: dojutsu-user Date: Sat, 17 Nov 2018 00:27:23 +0530 Subject: [PATCH 2/6] Add tests --- .../rtd_tests/tests/test_profile_views.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_profile_views.py b/readthedocs/rtd_tests/tests/test_profile_views.py index cdef5d20906..88198af1c03 100644 --- a/readthedocs/rtd_tests/tests/test_profile_views.py +++ b/readthedocs/rtd_tests/tests/test_profile_views.py @@ -35,6 +35,23 @@ def test_edit_profile(self): self.assertEqual(self.user.last_name, 'Docs') self.assertEqual(self.user.profile.homepage, 'readthedocs.org') + def test_edit_profile_with_invalid_values(self): + resp = self.client.get( + reverse('profiles_profile_edit'), + ) + self.assertTrue(resp.status_code, 200) + resp = self.client.post( + reverse('profiles_profile_edit'), + data={ + 'first_name': 'this-is-first_name-whose-length-is-greater-than-30', + 'last_name': 'this-is-last_name-whose-length-is-greater-than-30', + 'homepage': 'readthedocs.org', + } + ) + + self.assertFormError(resp, form='form', field='first_name', errors='Ensure that first name has at most 30 characters.') + self.assertFormError(resp, form='form', field='last_name', errors='Ensure that last name has at most 30 characters.') + def test_delete_account(self): resp = self.client.get( reverse('delete_account') From 2fff2594562f645793b64d586ffb212e528f8817 Mon Sep 17 00:00:00 2001 From: dojutsu-user Date: Sat, 17 Nov 2018 13:33:18 +0530 Subject: [PATCH 3/6] Use max_length --- readthedocs/core/forms.py | 16 ++-------------- .../rtd_tests/tests/test_profile_views.py | 17 ----------------- 2 files changed, 2 insertions(+), 31 deletions(-) diff --git a/readthedocs/core/forms.py b/readthedocs/core/forms.py index bb7f5e2aab0..bc062286547 100644 --- a/readthedocs/core/forms.py +++ b/readthedocs/core/forms.py @@ -18,8 +18,8 @@ class UserProfileForm(forms.ModelForm): - first_name = CharField(label=_('First name'), required=False) - last_name = CharField(label=_('Last name'), required=False) + first_name = CharField(label=_('First name'), required=False, max_length=30) + last_name = CharField(label=_('Last name'), required=False, max_length=30) class Meta(object): model = UserProfile @@ -34,18 +34,6 @@ def __init__(self, *args, **kwargs): except AttributeError: pass - def clean_first_name(self): - first_name = self.cleaned_data.get('first_name') - if not len(first_name) <= 30: - raise forms.ValidationError('Ensure that first name has at most 30 characters.') - return first_name - - def clean_last_name(self): - last_name = self.cleaned_data.get('last_name') - if not len(last_name) <= 30: - raise forms.ValidationError('Ensure that last name has at most 30 characters.') - return last_name - def save(self, commit=True): first_name = self.cleaned_data.pop('first_name', None) last_name = self.cleaned_data.pop('last_name', None) diff --git a/readthedocs/rtd_tests/tests/test_profile_views.py b/readthedocs/rtd_tests/tests/test_profile_views.py index 88198af1c03..cdef5d20906 100644 --- a/readthedocs/rtd_tests/tests/test_profile_views.py +++ b/readthedocs/rtd_tests/tests/test_profile_views.py @@ -35,23 +35,6 @@ def test_edit_profile(self): self.assertEqual(self.user.last_name, 'Docs') self.assertEqual(self.user.profile.homepage, 'readthedocs.org') - def test_edit_profile_with_invalid_values(self): - resp = self.client.get( - reverse('profiles_profile_edit'), - ) - self.assertTrue(resp.status_code, 200) - resp = self.client.post( - reverse('profiles_profile_edit'), - data={ - 'first_name': 'this-is-first_name-whose-length-is-greater-than-30', - 'last_name': 'this-is-last_name-whose-length-is-greater-than-30', - 'homepage': 'readthedocs.org', - } - ) - - self.assertFormError(resp, form='form', field='first_name', errors='Ensure that first name has at most 30 characters.') - self.assertFormError(resp, form='form', field='last_name', errors='Ensure that last name has at most 30 characters.') - def test_delete_account(self): resp = self.client.get( reverse('delete_account') From b890b1f06775691a4cea868921eea0c12e29596e Mon Sep 17 00:00:00 2001 From: dojutsu-user Date: Sun, 18 Nov 2018 16:57:54 +0530 Subject: [PATCH 4/6] Add tests --- .../rtd_tests/tests/test_profile_views.py | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_profile_views.py b/readthedocs/rtd_tests/tests/test_profile_views.py index cdef5d20906..2b9c411dbf1 100644 --- a/readthedocs/rtd_tests/tests/test_profile_views.py +++ b/readthedocs/rtd_tests/tests/test_profile_views.py @@ -35,6 +35,33 @@ def test_edit_profile(self): self.assertEqual(self.user.last_name, 'Docs') self.assertEqual(self.user.profile.homepage, 'readthedocs.org') + def test_edit_profile_with_invalid_values(self): + resp = self.client.get( + reverse('profiles_profile_edit'), + ) + self.assertTrue(resp.status_code, 200) + + long_first_name = 'this-is-first_name-whose-length-is-greater-than-30' + long_last_name = 'this-is-last_name-whose-length-is-greater-than-30' + long_homepage = 'this-is-the-the-link-to-the-homepage-of-the-user-which-is-pretty-long-and-that-is-why-wrong-value.org' + + self.assertTrue(len(long_first_name) > 30) + self.assertTrue(len(long_last_name) > 30) + self.assertTrue(len(long_homepage) > 100) + + resp = self.client.post( + reverse('profiles_profile_edit'), + data={ + 'first_name': long_first_name, + 'last_name': long_last_name, + 'homepage': long_homepage, + } + ) + + self.assertFormError(resp, form='form', field='first_name', errors='Ensure this value has at most 30 characters (it has {}).'.format(len(long_first_name))) + self.assertFormError(resp, form='form', field='last_name', errors='Ensure this value has at most 30 characters (it has {}).'.format(len(long_last_name))) + self.assertFormError(resp, form='form', field='homepage', errors='Ensure this value has at most 100 characters (it has {}).'.format(len(long_homepage))) + def test_delete_account(self): resp = self.client.get( reverse('delete_account') From 10a0eb36af8a9562e16e9e842c4025dfde6b9396 Mon Sep 17 00:00:00 2001 From: dojutsu-user Date: Sun, 18 Nov 2018 17:22:57 +0530 Subject: [PATCH 5/6] reduce redundant code --- readthedocs/rtd_tests/tests/test_profile_views.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_profile_views.py b/readthedocs/rtd_tests/tests/test_profile_views.py index 2b9c411dbf1..5e5c016334d 100644 --- a/readthedocs/rtd_tests/tests/test_profile_views.py +++ b/readthedocs/rtd_tests/tests/test_profile_views.py @@ -57,10 +57,12 @@ def test_edit_profile_with_invalid_values(self): 'homepage': long_homepage, } ) - - self.assertFormError(resp, form='form', field='first_name', errors='Ensure this value has at most 30 characters (it has {}).'.format(len(long_first_name))) - self.assertFormError(resp, form='form', field='last_name', errors='Ensure this value has at most 30 characters (it has {}).'.format(len(long_last_name))) - self.assertFormError(resp, form='form', field='homepage', errors='Ensure this value has at most 100 characters (it has {}).'.format(len(long_homepage))) + + FORM_ERROR_FORMAT = 'Ensure this value has at most {} characters (it has {}).' + + self.assertFormError(resp, form='form', field='first_name', errors=FORM_ERROR_FORMAT.format(30, len(long_first_name))) + self.assertFormError(resp, form='form', field='last_name', errors=FORM_ERROR_FORMAT.format(30, len(long_last_name))) + self.assertFormError(resp, form='form', field='homepage', errors=FORM_ERROR_FORMAT.format(100, len(long_homepage))) def test_delete_account(self): resp = self.client.get( From 734823411cd7364a551a5afcb259ca9e2c619cdd Mon Sep 17 00:00:00 2001 From: dojutsu-user Date: Sun, 18 Nov 2018 22:33:23 +0530 Subject: [PATCH 6/6] Improve test --- .../rtd_tests/tests/test_profile_views.py | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_profile_views.py b/readthedocs/rtd_tests/tests/test_profile_views.py index 5e5c016334d..ba7a2989b24 100644 --- a/readthedocs/rtd_tests/tests/test_profile_views.py +++ b/readthedocs/rtd_tests/tests/test_profile_views.py @@ -41,28 +41,20 @@ def test_edit_profile_with_invalid_values(self): ) self.assertTrue(resp.status_code, 200) - long_first_name = 'this-is-first_name-whose-length-is-greater-than-30' - long_last_name = 'this-is-last_name-whose-length-is-greater-than-30' - long_homepage = 'this-is-the-the-link-to-the-homepage-of-the-user-which-is-pretty-long-and-that-is-why-wrong-value.org' - - self.assertTrue(len(long_first_name) > 30) - self.assertTrue(len(long_last_name) > 30) - self.assertTrue(len(long_homepage) > 100) - resp = self.client.post( reverse('profiles_profile_edit'), data={ - 'first_name': long_first_name, - 'last_name': long_last_name, - 'homepage': long_homepage, + 'first_name': 'a' * 31, + 'last_name': 'b' * 31, + 'homepage': 'c' * 101, } ) FORM_ERROR_FORMAT = 'Ensure this value has at most {} characters (it has {}).' - self.assertFormError(resp, form='form', field='first_name', errors=FORM_ERROR_FORMAT.format(30, len(long_first_name))) - self.assertFormError(resp, form='form', field='last_name', errors=FORM_ERROR_FORMAT.format(30, len(long_last_name))) - self.assertFormError(resp, form='form', field='homepage', errors=FORM_ERROR_FORMAT.format(100, len(long_homepage))) + self.assertFormError(resp, form='form', field='first_name', errors=FORM_ERROR_FORMAT.format(30, 31)) + self.assertFormError(resp, form='form', field='last_name', errors=FORM_ERROR_FORMAT.format(30, 31)) + self.assertFormError(resp, form='form', field='homepage', errors=FORM_ERROR_FORMAT.format(100, 101)) def test_delete_account(self): resp = self.client.get(