fix username validator

ckan/logic/validators.py

@@ -544,10 +544,9 @@ def user_name_validator(key, data, errors, context):
         raise Invalid(_('User names must be strings'))
 
     user = model.User.get(new_user_name)
+    user_obj_from_context = context.get('user_obj')
     if user is not None:
         # A user with new_user_name already exists in the database.
-
-        user_obj_from_context = context.get('user_obj')
         if user_obj_from_context and user_obj_from_context.id == user.id:
             # If there's a user_obj in context with the same id as the user
             # found in the db, then we must be doing a user_update and not
@@ -558,6 +557,12 @@ def user_name_validator(key, data, errors, context):
             # name, so you can create a new user with that name or update an
             # existing user's name to that name.
             errors[key].append(_('That login name is not available.'))
+    elif user_obj_from_context:
+        old_user = model.User.get(user_obj_from_context.id)
+        if old_user is not None:
+            errors[key].append(_('That login name can not be modified.'))
+        else:
+            return
 
 def user_both_passwords_entered(key, data, errors, context):
 

ckan/tests/logic/action/test_update.py

@@ -80,24 +80,12 @@ def test_user_update_name(self):
 
         # 1. Setup.
         user = factories.User()
+        user['name'] = 'updated'
 
-        # 2. Call the function that's being tested, once only.
-        # FIXME we have to pass the email address and password to user_update
-        # even though we're not updating those fields, otherwise validation
-        # fails.
-        helpers.call_action('user_update', id=user['name'],
-                            email=user['email'],
-                            password=factories.User.attributes()['password'],
-                            name='updated',
-                            )
-
-        # 3. Make assertions about the return value and/or side-effects.
-        updated_user = helpers.call_action('user_show', id=user['id'])
-        # Note that we check just the field we were trying to update, not the
-        # entire dict, only assert what we're actually testing.
-        assert updated_user['name'] == 'updated'
-
-        # 4. Do nothing else!
+        # 2. Make assertions about the return value and/or side-effects.
+        assert_raises(logic.ValidationError,
+                      helpers.call_action, 'user_update',
+                      **user)
 
     # END-BEFORE
 
@@ -267,7 +255,7 @@ def test_user_update_activity_stream(self):
         helpers.call_action('user_update', id=user['name'],
                             email=user['email'],
                             password=factories.User.attributes()['password'],
-                            name='updated',
+                            fullname='updated full name',
                             )
 
         activity_stream = helpers.call_action('user_activity_list',
@@ -310,7 +298,7 @@ def test_user_update_with_custom_schema(self):
         helpers.call_action('user_update', context={'schema': schema},
                             id=user['name'], email=user['email'],
                             password=factories.User.attributes()['password'],
-                            name='updated',
+                            fullname='updated full name',
                             )
 
         # Since we passed user['name'] to user_update as the 'id' param,
@@ -325,7 +313,6 @@ def test_user_update_multiple(self):
 
         params = {
             'id': user['id'],
-            'name': 'updated_name',
             'fullname': 'updated full name',
             'about': 'updated about',
             # FIXME: We shouldn't have to put email here since we're not
@@ -339,7 +326,6 @@ def test_user_update_multiple(self):
         helpers.call_action('user_update', **params)
 
         updated_user = helpers.call_action('user_show', id=user['id'])
-        assert updated_user['name'] == 'updated_name'
         assert updated_user['fullname'] == 'updated full name'
         assert updated_user['about'] == 'updated about'
 
@@ -351,7 +337,6 @@ def test_user_update_does_not_return_password(self):
 
         params = {
             'id': user['id'],
-            'name': 'updated_name',
             'fullname': 'updated full name',
             'about': 'updated about',
             'email': user['email'],
@@ -369,7 +354,6 @@ def test_user_update_does_not_return_apikey(self):
 
         params = {
             'id': user['id'],
-            'name': 'updated_name',
             'fullname': 'updated full name',
             'about': 'updated about',
             'email': user['email'],
@@ -391,7 +375,6 @@ def test_user_update_does_not_return_reset_key(self):
 
         params = {
             'id': user['id'],
-            'name': 'updated_name',
             'fullname': 'updated full name',
             'about': 'updated about',
             'email': user['email'],