diff --git a/app/assets/javascripts/discourse/controllers/preferences_username_controller.js.coffee b/app/assets/javascripts/discourse/controllers/preferences_username_controller.js.coffee index 114694094..4bc2bd97b 100644 --- a/app/assets/javascripts/discourse/controllers/preferences_username_controller.js.coffee +++ b/app/assets/javascripts/discourse/controllers/preferences_username_controller.js.coffee @@ -1,6 +1,7 @@ Discourse.PreferencesUsernameController = Ember.ObjectController.extend Discourse.Presence, taken: false + invalid: false saving: false error: false @@ -8,8 +9,10 @@ Discourse.PreferencesUsernameController = Ember.ObjectController.extend Discours return true if @get('saving') return true if @blank('newUsername') return true if @get('taken') + return true if @get('invalid') return true if @get('unchanged') - ).property('newUsername', 'taken', 'unchanged', 'saving') + false + ).property('newUsername', 'taken', 'invalid', 'unchanged', 'saving') unchanged: (-> @get('newUsername') == @get('content.username') @@ -17,10 +20,14 @@ Discourse.PreferencesUsernameController = Ember.ObjectController.extend Discours checkTaken: (-> @set('taken', false) + @set('invalid', false) return if @blank('newUsername') return if @get('unchanged') Discourse.User.checkUsername(@get('newUsername')).then (result) => - @set('taken', true) unless result.available + if result.errors + @set('invalid', true) + else if result.available == false + @set('taken', true) ).observes('newUsername') saveButtonText: (-> diff --git a/app/assets/javascripts/discourse/templates/user/username.js.handlebars b/app/assets/javascripts/discourse/templates/user/username.js.handlebars index 8deef5783..affaa1998 100644 --- a/app/assets/javascripts/discourse/templates/user/username.js.handlebars +++ b/app/assets/javascripts/discourse/templates/user/username.js.handlebars @@ -23,6 +23,9 @@ {{#if controller.taken}} {{i18n user.change_username.taken}} {{/if}} + {{#if controller.invalid}} + {{i18n user.change_username.invalid}} + {{/if}} diff --git a/app/assets/javascripts/discourse/views/user/preferences_username_view.js.coffee b/app/assets/javascripts/discourse/views/user/preferences_username_view.js.coffee index 128279ce5..2f37bd14a 100644 --- a/app/assets/javascripts/discourse/views/user/preferences_username_view.js.coffee +++ b/app/assets/javascripts/discourse/views/user/preferences_username_view.js.coffee @@ -2,6 +2,13 @@ window.Discourse.PreferencesUsernameView = Ember.View.extend templateName: 'user/username' classNames: ['user-preferences'] - didInsertElement: -> $('#change_username').focus() + + keyDown: (e) -> + if e.keyCode is 13 + unless @get('controller').get('saveDisabled') + @get('controller').changeUsername() + else + e.preventDefault() + return false diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 24d2fe99e..d9c4bd2c0 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -76,7 +76,9 @@ class UsersController < ApplicationController def check_username requires_parameter(:username) - if !SiteSetting.call_mothership? + if !User.username_valid?(params[:username]) + render json: {errors: [I18n.t("user.username.characters")]} + elsif !SiteSetting.call_mothership? if User.username_available?(params[:username]) render json: {available: true} else diff --git a/app/models/user.rb b/app/models/user.rb index fb55d91a6..deb6271ea 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -112,6 +112,12 @@ class User < ActiveRecord::Base !User.where(username_lower: lower).exists? end + def self.username_valid?(username) + u = User.new(username: username) + u.username_format_validator + !u.errors[:username].present? + end + def enqueue_welcome_message(message_type) return unless SiteSetting.send_welcome_message? Jobs.enqueue(:send_system_message, user_id: self.id, message_type: message_type) @@ -367,6 +373,30 @@ class User < ActiveRecord::Base Guardian.new(self) end + def username_format_validator + unless username + return errors.add(:username, I18n.t(:'user.username.blank')) + end + + if username.length < User.username_length.begin + return errors.add(:username, I18n.t(:'user.username.short', min: User.username_length.begin)) + end + + if username.length > User.username_length.end + return errors.add(:username, I18n.t(:'user.username.long', max: User.username_length.end)) + end + + if username =~ /[^A-Za-z0-9_]/ + return errors.add(:username, I18n.t(:'user.username.characters')) + end + + if username[0,1] =~ /[^A-Za-z0-9]/ + return errors.add(:username, I18n.t(:'user.username.must_begin_with_alphanumeric')) + end + nil + end + + protected def cook @@ -426,38 +456,19 @@ class User < ActiveRecord::Base self.username_lower = username.downcase end + def username_validator + username_format_validator || begin + lower = username.downcase + if username_changed? && User.where(username_lower: lower).exists? + return errors.add(:username, I18n.t(:'user.username.unique')) + end + end + end + def password_validator if @raw_password return errors.add(:password, "must be 6 letters or longer") if @raw_password.length < 6 end end - def username_validator - unless username - return errors.add(:username, I18n.t(:'user.username.blank')) - end - - if username.length < User.username_length.begin - return errors.add(:username, I18n.t(:'user.username.short', min: User.username_length.begin)) - end - - if username.length > User.username_length.end - return errors.add(:username, I18n.t(:'user.username.long', max: User.username_length.end)) - end - - if username =~ /[^A-Za-z0-9_]/ - return errors.add(:username, I18n.t(:'user.username.characters')) - end - - if username[0,1] =~ /[^A-Za-z0-9]/ - return errors.add(:username, I18n.t(:'user.username.must_begin_with_alphanumeric')) - end - - lower = username.downcase - if username_changed? && User.where(username_lower: lower).exists? - return errors.add(:username, I18n.t(:'user.username.unique')) - end - - end - end diff --git a/config/locales/en.yml b/config/locales/en.yml index 439c15424..d3c5290ce 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -474,6 +474,7 @@ en: confirm: "There could be consequences to changing your username. Are you absolutely sure you want to?" taken: "Sorry that username is taken." error: "There was an error changing your username." + invalid: "That username is invalid. It must only include numbers and letters" change_email: action: 'change email' title: "Change Email" diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 69996d5a5..56835191b 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -483,6 +483,24 @@ describe UsersController do end it_should_behave_like 'when username is unavailable locally' end + + context 'has invalid characters' do + before do + xhr :get, :check_username, username: 'bad username' + end + + it 'should return success' do + response.should be_success + end + + it 'should not return an available key' do + ::JSON.parse(response.body)['available'].should be_nil + end + + it 'should return an error message' do + ::JSON.parse(response.body)['errors'].should_not be_empty + end + end end context 'when call_mothership is enabled' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 28b25af35..d0df536f2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -394,6 +394,20 @@ describe User do end end + context '.username_valid?' do + it 'returns true when username is both valid and available' do + User.username_valid?('Available').should be_true + end + + it 'returns true when the username is valid but not available' do + User.username_valid?(Fabricate(:user).username).should be_true + end + + it 'returns false when the username is not valid' do + User.username_valid?('not valid.name').should be_false + end + end + describe '.suggest_username' do it 'corrects weird characters' do User.suggest_username("Darth%^Vadar").should == "Darth_Vadar"