Merge ~newell-jensen/maas:lp1727419 into maas:master

[Newell Jensen]

LP: #1727419 -- Uncapitalize letters in middle of error message sentences.

[Andres Rodriguez (andreserl)]

To what messages does it apply? Only E-mail?

review: Needs Information

05a8a62... by Newell Jensen on 2019-05-18

Add unit test showing that when trying to add zone with the same name as an already existing one, that the error message has lower case words in rest of the sentence.

[Newell Jensen (newell-jensen)]

> To what messages does it apply? Only E-mail?

It applies to all forms that inherit from MAASModelForm. I added a unit test showing that when trying to add a zone with the same name as an already existing one, that the error message has lower case words in rest of the sentence.

[Lee Trager (ltrager)]

Couple of questions below

review: Needs Information

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1727419 lp:~newell-jensen/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 05a8a62a8b07a3bf70280d94d1d66251a9393e27

review: Approve

Unmerged commits

05a8a62... by Newell Jensen on 2019-05-18

Add unit test showing that when trying to add zone with the same name as an already existing one, that the error message has lower case words in rest of the sentence.

4071ceb... by Newell Jensen on 2019-05-17

Update unit test.

971e135... by Newell Jensen on 2019-05-17

LP: #1727419 -- Uncapitalize letters in middle of error message sentences.

diff --git a/src/maasserver/api/zones.py b/src/maasserver/api/zones.py
index 6d56264..c233d01 100644
--- a/src/maasserver/api/zones.py
+++ b/src/maasserver/api/zones.py
@@ -159,7 +159,7 @@ class ZonesHandler(ModelCollectionOperationsHandler):
         @error (http-status-code) "400" 400
         @error (content) "alreadyexists" The zone already exists
         @error-example "alreadyexists"
-            {"name": ["Physical zone with this Name already exists."]}
+            {"name": ["Physical zone with this name already exists."]}
         """
         return super().create(request)
 
diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py
index 2341511..659655e 100644
--- a/src/maasserver/forms/__init__.py
+++ b/src/maasserver/forms/__init__.py
@@ -445,10 +445,23 @@ class MAASModelForm(
 
     def _update_errors(self, errors):
         """Provide Django 1.11-like behaviour in 1.8 as well."""
+        # Sometimes Django capitalizes the field name while it is in
+        # the middle of the error messages.  As these errors are
+        # surfaced to the UI, we want well formed sentences.
         if hasattr(errors, 'error_dict'):
-            error_dict = errors
+            error_dict = errors.error_dict
+            for field, validation_errors in error_dict.items():
+                new_messages = []
+                for messages in validation_errors:
+                    for message in messages:
+                        if field.capitalize() in message:
+                            new_messages.append(
+                                message.replace(field.capitalize(), field))
+                        else:
+                            new_messages.append(message)
+                error_dict[field] = new_messages
         else:
-            error_dict = ValidationError({NON_FIELD_ERRORS: errors})
+            error_dict = {NON_FIELD_ERRORS: errors}
         super(MAASModelForm, self)._update_errors(error_dict)
 
     def use_perms(self):
@@ -1462,7 +1475,7 @@ class NewUserCreationForm(UserCreationForm):
         email = self.cleaned_data['email']
         if User.objects.filter(email__iexact=email).exists():
             raise forms.ValidationError(
-                "User with this E-mail address already exists.")
+                "User with this email address already exists.")
         return email
 
 
@@ -1486,6 +1499,27 @@ class EditUserForm(UserChangeForm):
         if 'password' in self.fields:
             del self.fields['password']
 
+    def _update_errors(self, errors):
+        """Provide Django 1.11-like behaviour in 1.8 as well."""
+        # Sometimes Django capitalizes the field name while it is in
+        # the middle of the error messages.  As these errors are
+        # surfaced to the UI, we want well formed sentences.
+        if hasattr(errors, 'error_dict'):
+            error_dict = errors.error_dict
+            for field, validation_errors in error_dict.items():
+                new_messages = []
+                for messages in validation_errors:
+                    for message in messages:
+                        if field.capitalize() in message:
+                            new_messages.append(
+                                message.replace(field.capitalize(), field))
+                        else:
+                            new_messages.append(message)
+                error_dict[field] = new_messages
+        else:
+            error_dict = {NON_FIELD_ERRORS: errors}
+        super(EditUserForm, self)._update_errors(error_dict)
+
 
 class DeleteUserForm(Form):
     """A form to remove a user."""
diff --git a/src/maasserver/forms/tests/test_user.py b/src/maasserver/forms/tests/test_user.py
index 3e08f7a..721deeb 100644
--- a/src/maasserver/forms/tests/test_user.py
+++ b/src/maasserver/forms/tests/test_user.py
@@ -1,4 +1,4 @@
-# Copyright 2014-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for user-creation forms."""
@@ -14,7 +14,6 @@ from maasserver.forms import (
 from maasserver.models import Config
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
-from testtools.matchers import MatchesRegex
 
 
 class TestUniqueEmailForms(MAASServerTestCase):
@@ -24,10 +23,9 @@ class TestUniqueEmailForms(MAASServerTestCase):
         self.assertIn('email', form._errors)
         self.assertEqual(1, len(form._errors['email']))
         # Cope with 'Email' and 'E-mail' in error message.
-        self.assertThat(
+        self.assertEqual(
             form._errors['email'][0],
-            MatchesRegex(
-                r'User with this E-{0,1}mail address already exists.'))
+            'User with this email address already exists.')
 
     def test_ProfileForm_fails_validation_if_email_taken(self):
         another_email = '[e-mail]' % factory.make_string()
diff --git a/src/maasserver/forms/tests/test_zone.py b/src/maasserver/forms/tests/test_zone.py
index 9e93239..efd3464 100644
--- a/src/maasserver/forms/tests/test_zone.py
+++ b/src/maasserver/forms/tests/test_zone.py
@@ -1,4 +1,4 @@
-# Copyright 2014-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for `ZoneForm`."""
@@ -56,3 +56,14 @@ class TestZoneForm(MAASServerTestCase):
             data={'name': factory.make_name('zone')},
             instance=zone)
         self.assertTrue(form.is_valid())
+
+    def test_zone_create_errors_with_lowercase_words_in_middle(self):
+        name = factory.make_name('zone')
+        form = ZoneForm(data={'name': name})
+        form.save()
+        self.assertTrue(form.is_valid(), form._errors)
+        new_form = ZoneForm(data={'name': name})
+        new_form.is_valid()
+        self.assertEquals(
+            new_form._errors['name'][0],
+            'Physical zone with this name already exists.')