Empty email for users gets saved as an empty string

The actual behavior is that horizon sends an empty string
on the "email" field when creating a new user if the field
isn't filled by the user.
To match the keystone client behaviour, instead of sending
an empty string, this patch sends a "None" value when the
field isn't filled by the user.
By doing this, when creating a new user from both,
keystone client and horizon, a "null" value is stored
under the same conditions.
in the database. This will keep the data consistency.
From the presentation point of view, when a "null" value
is received, it is presented as an empty string instead
of showing "None" on the "email" field of the users table.

Change-Id: Ib5df774b3fe9d82c084f380c9c11aea46b2984aa
Closes-Bug: #1265805

openstack_dashboard/dashboards/admin/users/forms.py

@@ -115,6 +115,8 @@ class CreateUserForm(BaseUserForm):
         domain = api.keystone.get_default_domain(self.request)
         try:
             LOG.info('Creating user with name "%s"' % data['name'])
+            if "email" in data:
+                data['email'] = data['email'] or None
             new_user = api.keystone.user_create(request,
                                                 name=data['name'],
                                                 email=data['email'],
@@ -190,6 +192,8 @@ class UpdateUserForm(BaseUserForm):
         data.pop('domain_name')
 
         try:
+            if "email" in data:
+                data['email'] = data['email'] or None
             response = api.keystone.user_update(request, user, **data)
             messages.success(request,
                              _('User has been updated successfully.'))

openstack_dashboard/dashboards/admin/users/tables.py

@@ -129,7 +129,9 @@ class UsersTable(tables.DataTable):
     )
     name = tables.Column('name', verbose_name=_('User Name'))
     email = tables.Column('email', verbose_name=_('Email'),
-                          filters=[defaultfilters.urlize])
+                          filters=(lambda v: defaultfilters
+                                   .default_if_none(v, ""),
+                                   defaultfilters.urlize))
     # Default tenant is not returned from Keystone currently.
     #default_tenant = tables.Column('default_tenant',
     #                               verbose_name=_('Default Project'))

openstack_dashboard/dashboards/admin/users/tests.py

@@ -56,13 +56,11 @@ class UsersViewTests(test.BaseAdminViewTests):
         domain = self._get_default_domain()
         domain_id = domain.id
         users = self._get_users(domain_id)
-        api.keystone.user_list(IgnoreArg(), domain=domain_id) \
-                .AndReturn(users)
+        api.keystone.user_list(IgnoreArg(),
+                               domain=domain_id).AndReturn(users)
 
         self.mox.ReplayAll()
-
         res = self.client.get(USERS_INDEX_URL)
-
         self.assertTemplateUsed(res, 'admin/users/index.html')
         self.assertItemsEqual(res.context['table'].data, users)
 
@@ -91,7 +89,9 @@ class UsersViewTests(test.BaseAdminViewTests):
 
         api.keystone.get_default_domain(IgnoreArg()) \
             .MultipleTimes().AndReturn(domain)
-        api.keystone.tenant_list(IgnoreArg(), domain=domain_id, user=None) \
+        api.keystone.tenant_list(IgnoreArg(),
+                                 domain=domain_id,
+                                 user=None) \
             .AndReturn([self.tenants.list(), False])
         api.keystone.user_create(IgnoreArg(),
                                  name=user.name,
@@ -126,6 +126,49 @@ class UsersViewTests(test.BaseAdminViewTests):
                               domain_context_name=domain.name)
         self.test_create()
 
+    @test.create_stubs({api.keystone: ('user_create',
+                                       'get_default_domain',
+                                       'tenant_list',
+                                       'add_tenant_user_role',
+                                       'get_default_role',
+                                       'role_list')})
+    def test_create_with_empty_email(self):
+        user = self.users.get(id="5")
+        domain = self._get_default_domain()
+        domain_id = domain.id
+        role = self.roles.first()
+        api.keystone.get_default_domain(IgnoreArg()) \
+            .MultipleTimes().AndReturn(domain)
+        api.keystone.tenant_list(IgnoreArg(),
+                                 domain=domain_id,
+                                 user=None) \
+            .AndReturn([self.tenants.list(), False])
+        api.keystone.user_create(IgnoreArg(),
+                                 name=user.name,
+                                 email=user.email,
+                                 password=user.password,
+                                 project=self.tenant.id,
+                                 enabled=True,
+                                 domain=domain_id).AndReturn(user)
+        api.keystone.role_list(IgnoreArg()).AndReturn(self.roles.list())
+        api.keystone.get_default_role(IgnoreArg()).AndReturn(role)
+        api.keystone.add_tenant_user_role(IgnoreArg(), self.tenant.id,
+                                          user.id, role.id)
+
+        self.mox.ReplayAll()
+        formData = {'method': 'CreateUserForm',
+                    'domain_id': domain_id,
+                    'name': user.name,
+                    'email': "",
+                    'password': user.password,
+                    'project': self.tenant.id,
+                    'role_id': self.roles.first().id,
+                    'confirm_password': user.password}
+        res = self.client.post(USER_CREATE_URL, formData)
+
+        self.assertNoFormErrors(res)
+        self.assertMessageCount(success=1)
+
     @test.create_stubs({api.keystone: ('get_default_domain',
                                        'tenant_list',
                                        'role_list',
@@ -236,6 +279,7 @@ class UsersViewTests(test.BaseAdminViewTests):
                                        'user_update',
                                        'roles_for_user', )})
     def _update(self, user):
+        user = self.users.get(id="1")
         domain_id = user.domain_id
         domain = self.domains.get(id=domain_id)
         test_password = 'normalpwd'
@@ -243,9 +287,11 @@ class UsersViewTests(test.BaseAdminViewTests):
 
         api.keystone.user_get(IsA(http.HttpRequest), '1',
                               admin=True).AndReturn(user)
-        api.keystone.domain_get(IsA(http.HttpRequest), domain_id) \
-            .AndReturn(domain)
-        api.keystone.tenant_list(IgnoreArg(), domain=domain_id, user=user.id) \
+        api.keystone.domain_get(IsA(http.HttpRequest),
+                                domain_id).AndReturn(domain)
+        api.keystone.tenant_list(IgnoreArg(),
+                                 domain=domain_id,
+                                 user=user.id) \
             .AndReturn([self.tenants.list(), False])
         api.keystone.user_update(IsA(http.HttpRequest),
                                  user.id,
@@ -268,14 +314,46 @@ class UsersViewTests(test.BaseAdminViewTests):
 
         self.assertNoFormErrors(res)
 
-    def test_update(self):
-        user = self.users.get(id="1")
-        self._update(user)
-
+    @test.create_stubs({api.keystone: ('user_get',
+                                       'domain_get',
+                                       'tenant_list',
+                                       'user_update_tenant',
+                                       'user_update_password',
+                                       'user_update',
+                                       'roles_for_user', )})
     def test_update_with_no_email_attribute(self):
-        user = self.users.get(id="1")
-        del user.email
-        self._update(user)
+        user = self.users.get(id="5")
+        domain_id = user.domain_id
+        domain = self.domains.get(id=domain_id)
+
+        api.keystone.user_get(IsA(http.HttpRequest), '1',
+                              admin=True).AndReturn(user)
+        api.keystone.domain_get(IsA(http.HttpRequest),
+                                domain_id).AndReturn(domain)
+        api.keystone.tenant_list(IgnoreArg(),
+                                 domain=domain_id,
+                                 user=user.id) \
+            .AndReturn([self.tenants.list(), False])
+        api.keystone.user_update(IsA(http.HttpRequest),
+                                 user.id,
+                                 email=user.email,
+                                 name=user.name,
+                                 password=user.password,
+                                 project=self.tenant.id).AndReturn(None)
+
+        self.mox.ReplayAll()
+
+        formData = {'method': 'UpdateUserForm',
+                    'id': user.id,
+                    'name': user.name,
+                    'email': "",
+                    'password': user.password,
+                    'project': self.tenant.id,
+                    'confirm_password': user.password}
+
+        res = self.client.post(USER_UPDATE_URL, formData)
+
+        self.assertNoFormErrors(res)
 
     @test.create_stubs({api.keystone: ('user_get',
                                        'domain_get',

openstack_dashboard/test/test_data/keystone_data.py

@@ -194,7 +194,16 @@ def data(TEST):
                  'enabled': True,
                  'domain_id': "2"}
     user4 = users.User(users.UserManager(None), user_dict)
-    TEST.users.add(user, user2, user3, user4)
+    user_dict = {'id': "5",
+                 'name': 'user_five',
+                 'email': None,
+                 'password': 'password',
+                 'token': 'test_token',
+                 'project_id': '2',
+                 'enabled': True,
+                 'domain_id': "1"}
+    user5 = users.User(users.UserManager(None), user_dict)
+    TEST.users.add(user, user2, user3, user4, user5)
     TEST.user = user  # Your "current" user
     TEST.user.service_catalog = SERVICE_CATALOG