check_username api now returns correct error message for invalid lengths etc

app/controllers/users_controller.rb

@@ -76,8 +76,9 @@ class UsersController < ApplicationController
   def check_username
     requires_parameter(:username)
 
-    if !UsernameValidator.new(params[:username]).valid_format?
+    validator = UsernameValidator.new(params[:username])
-      render json: {errors: [I18n.t("user.username.characters")]}
+    if !validator.valid_format?
+      render json: {errors: validator.errors}
     elsif !SiteSetting.call_mothership?
       if User.username_available?(params[:username])
         render json: {available: true}

app/models/user.rb

@@ -376,7 +376,7 @@ class User < ActiveRecord::Base
   def username_format_validator
     validator = UsernameValidator.new(username)
     unless validator.valid_format?
-      errors.add(:username, validator.error)
+      validator.errors.each { |e| errors.add(:username, e) }
     end
   end
 

app/models/username_validator.rb

@@ -2,9 +2,9 @@ class UsernameValidator
 
   def initialize(username)
     @username = username
-    @error = []
+    @errors = []
   end
-  attr_accessor :error
+  attr_accessor :errors
   attr_reader :username
 
   def user
@@ -17,43 +17,43 @@ class UsernameValidator
     username_length_max?
     username_char_valid?
     username_first_char_valid?
-    error.blank?
+    errors.empty?
   end
 
   private
 
   def username_exist?
-    return unless error.empty?
+    return unless errors.empty?
     unless username
-      self.error = I18n.t(:'user.username.blank')
+      self.errors << I18n.t(:'user.username.blank')
     end
   end
 
   def username_length_min?
-    return unless error.empty?
+    return unless errors.empty?
     if username.length < User.username_length.begin
-      self.error = I18n.t(:'user.username.short', min: User.username_length.begin)
+      self.errors << I18n.t(:'user.username.short', min: User.username_length.begin)
     end
   end
 
   def username_length_max?
-    return unless error.empty?
+    return unless errors.empty?
     if username.length > User.username_length.end
-      self.error = I18n.t(:'user.username.long', max: User.username_length.end)
+      self.errors << I18n.t(:'user.username.long', max: User.username_length.end)
     end
   end
 
   def username_char_valid?
-    return unless error.empty?
+    return unless errors.empty?
     if username =~ /[^A-Za-z0-9_]/
-      self.error = I18n.t(:'user.username.characters')
+      self.errors << I18n.t(:'user.username.characters')
     end
   end
 
   def username_first_char_valid?
-    return unless error.empty?
+    return unless errors.empty?
     if username[0,1] =~ /[^A-Za-z0-9]/
-      self.error = I18n.t(:'user.username.must_begin_with_alphanumeric')
+      self.errors << I18n.t(:'user.username.must_begin_with_alphanumeric')
     end
   end
 

spec/controllers/users_controller_spec.rb

@@ -484,11 +484,7 @@ describe UsersController do
         it_should_behave_like 'when username is unavailable locally'
       end
 
-      context 'has invalid characters' do
+      shared_examples_for 'checking an invalid username' do
-        before do
-          xhr :get, :check_username, username: 'bad username'
-        end
-
         it 'should return success' do
           response.should be_success
         end
@@ -501,6 +497,28 @@ describe UsersController do
           ::JSON.parse(response.body)['errors'].should_not be_empty
         end
       end
+
+      context 'has invalid characters' do
+        before do
+          xhr :get, :check_username, username: 'bad username'
+        end
+        it_should_behave_like 'checking an invalid username'
+
+        it 'should return the invalid characters message' do
+          ::JSON.parse(response.body)['errors'].should include(I18n.t(:'user.username.characters'))
+        end
+      end
+
+      context 'is too long' do
+        before do
+          xhr :get, :check_username, username: 'abcdefghijklmnop'
+        end
+        it_should_behave_like 'checking an invalid username'
+
+        it 'should return the "too short" message' do
+          ::JSON.parse(response.body)['errors'].should include(I18n.t(:'user.username.long', max: User.username_length.end))
+        end
+      end
     end
 
     context 'when call_mothership is enabled' do