Refactored user_name suggestion methods into a module to reduce the complexity of User model

app/controllers/users/omniauth_callbacks_controller.rb

@@ -1,6 +1,7 @@
 # -*- encoding : utf-8 -*-
 require_dependency 'email'
 require_dependency 'enum'
+require_dependency 'user_name_suggester'
 
 class Users::OmniauthCallbacksController < ApplicationController
   skip_before_filter :redirect_to_login_if_required
@@ -87,7 +88,7 @@ class Users::OmniauthCallbacksController < ApplicationController
     fb_uid = auth_token["uid"]
 
 
-    username = User.suggest_username(name)
+    username = UserNameSuggester.suggest(name)
 
     session[:authentication] = {
       facebook: {
@@ -232,7 +233,7 @@ class Users::OmniauthCallbacksController < ApplicationController
       @data = {
         email: email,
         name: User.suggest_name(name),
-        username: User.suggest_username(username),
+        username: UserNameSuggester.suggest(username),
         email_valid: true ,
         auth_provider: data[:provider] || params[:provider].try(:capitalize)
       }
@@ -306,7 +307,7 @@ class Users::OmniauthCallbacksController < ApplicationController
         email: email,
         email_valid: true,
         name: User.suggest_name(email),
-        username: User.suggest_username(email),
+        username: UserNameSuggester.suggest(email),
         auth_provider: params[:provider].try(:capitalize)
       }
 

app/controllers/users_controller.rb

@@ -1,4 +1,5 @@
 require_dependency 'discourse_hub'
+require_dependency 'user_name_suggester'
 
 class UsersController < ApplicationController
 
@@ -102,7 +103,7 @@ class UsersController < ApplicationController
       if User.username_available?(params[:username])
         render json: {available: true}
       else
-        render json: {available: false, suggestion: User.suggest_username(params[:username])}
+        render json: {available: false, suggestion: UserNameSuggester.suggest(params[:username])}
       end
     else
 
@@ -133,7 +134,7 @@ class UsersController < ApplicationController
         end
       elsif available_globally && !available_locally
         # Already registered on this site with the matching nickname and email address. Why are you signing up again?
-        render json: {available: false, suggestion: User.suggest_username(params[:username])}
+        render json: {available: false, suggestion: UserNameSuggester.suggest(params[:username])}
       else
         # Not available anywhere.
         render json: {available: false, suggestion: suggestion_from_discourse_hub}
@@ -202,7 +203,7 @@ class UsersController < ApplicationController
       message: I18n.t(
         "login.errors",
         errors:I18n.t(
-          "login.not_available", suggestion: User.suggest_username(params[:username])
+          "login.not_available", suggestion: UserNameSuggester.suggest(params[:username])
         )
       )
     }

app/models/user.rb

@@ -5,6 +5,7 @@ require_dependency 'pbkdf2'
 require_dependency 'summarize'
 require_dependency 'discourse'
 require_dependency 'post_destroyer'
+require_dependency 'user_name_suggester'
 
 class User < ActiveRecord::Base
   attr_accessible :name, :username, :password, :email, :bio_raw, :website
@@ -73,46 +74,13 @@ class User < ActiveRecord::Base
     3..15
   end
 
-  def self.sanitize_username!(name)
-    name.gsub!(/^[^[:alnum:]]+|\W+$/, "")
-    name.gsub!(/\W+/, "_")
-  end
-
-
-  def self.find_available_username_based_on(name)
-    sanitize_username!(name)
-    name = rightsize_username(name)
-    i = 1
-    attempt = name
-    until username_available?(attempt)
-      suffix = i.to_s
-      max_length = User.username_length.end - suffix.length - 1
-      attempt = "#{name[0..max_length]}#{suffix}"
-      i += 1
-    end
-    attempt
+  def self.username_available?(username)
+    lower = username.downcase
+    User.where(username_lower: lower).blank?
   end
 
   EMAIL = %r{([^@]+)@([^\.]+)}
 
-  def self.suggest_username(name)
-    return unless name.present?
-
-    if name =~ EMAIL
-      # When 'walter@white.com' take 'walter'
-      name = Regexp.last_match[1]
-
-      # When 'me@eviltrout.com' take 'eviltrout'
-      name = Regexp.last_match[2] if ['i', 'me'].include?(name)
-    end
-
-    find_available_username_based_on(name)
-  end
-
-  def self.rightsize_username(name)
-    name.ljust(username_length.begin, '1')[0,username_length.end]
-  end
-
   def self.new_from_params(params)
     user = User.new
     user.name = params[:name]
@@ -123,7 +91,7 @@ class User < ActiveRecord::Base
   end
 
   def self.create_for_email(email, opts={})
-    username = suggest_username(email)
+    username = UserNameSuggester.suggest(email)
 
     if SiteSetting.call_discourse_hub?
       begin
@@ -149,18 +117,6 @@ class User < ActiveRecord::Base
     user
   end
 
-  def self.username_available?(username)
-    lower = username.downcase
-    User.where(username_lower: lower).blank?
-  end
-
-  def self.username_valid?(username)
-    u = User.new(username: username)
-    u.username_format_validator
-    u.errors[:username].blank?
-  end
-
-
   def self.suggest_name(email)
     return "" unless email
     name = email.split(/[@\+]/)[0]

lib/user_name_suggester.rb

@@ -0,0 +1,44 @@
+module UserNameSuggester
+
+  def self.suggest(name)
+    return unless name.present?
+    name = parse_name_from_email(name)
+    find_available_username_based_on(name)
+  end
+
+  private
+
+  def self.parse_name_from_email(name)
+    if name =~ User::EMAIL
+      # When 'walter@white.com' take 'walter'
+      name = Regexp.last_match[1]
+      # When 'me@eviltrout.com' take 'eviltrout'
+      name = Regexp.last_match[2] if ['i', 'me'].include?(name)
+    end
+    name
+  end
+
+  def self.find_available_username_based_on(name)
+    sanitize_username!(name)
+    name = rightsize_username(name)
+    i = 1
+    attempt = name
+    until User.username_available?(attempt)
+      suffix = i.to_s
+      max_length = User.username_length.end - suffix.length - 1
+      attempt = "#{name[0..max_length]}#{suffix}"
+      i += 1
+    end
+    attempt
+  end
+
+  def self.sanitize_username!(name)
+    name.gsub!(/^[^[:alnum:]]+|\W+$/, "")
+    name.gsub!(/\W+/, "_")
+  end
+
+  def self.rightsize_username(name)
+    name.ljust(User.username_length.begin, '1')[0, User.username_length.end]
+  end
+
+end

spec/components/user_name_suggester_spec.rb

@@ -0,0 +1,70 @@
+require 'spec_helper'
+require 'user_name_suggester'
+
+describe UserNameSuggester do
+
+  describe 'name heuristics' do
+    it 'is able to guess a decent username from an email' do
+      UserNameSuggester.suggest('bob@bob.com').should == 'bob'
+    end
+  end
+
+  describe '.suggest' do
+
+    it "doesn't raise an error on nil username" do
+      UserNameSuggester.suggest(nil).should be_nil
+    end
+
+    it 'corrects weird characters' do
+      UserNameSuggester.suggest("Darth%^Vader").should == 'Darth_Vader'
+    end
+
+    it 'adds 1 to an existing username' do
+      user = Fabricate(:user)
+      UserNameSuggester.suggest(user.username).should == "#{user.username}1"
+    end
+
+    it "adds numbers if it's too short" do
+      UserNameSuggester.suggest('a').should == 'a11'
+    end
+
+    it "has a special case for me and i emails" do
+      UserNameSuggester.suggest('me@eviltrout.com').should == 'eviltrout'
+      UserNameSuggester.suggest('i@eviltrout.com').should == 'eviltrout'
+    end
+
+    it "shortens very long suggestions" do
+      UserNameSuggester.suggest("myreallylongnameisrobinwardesquire").should == 'myreallylongnam'
+    end
+
+    it "makes room for the digit added if the username is too long" do
+      User.create(username: 'myreallylongnam', email: 'fake@discourse.org')
+      UserNameSuggester.suggest("myreallylongnam").should == 'myreallylongna1'
+    end
+
+    it "removes leading character if it is not alphanumeric" do
+      UserNameSuggester.suggest("_myname").should == 'myname'
+    end
+
+    it "removes trailing characters if they are invalid" do
+      UserNameSuggester.suggest("myname!^$=").should == 'myname'
+    end
+
+    it "replace dots" do
+      UserNameSuggester.suggest("my.name").should == 'my_name'
+    end
+
+    it "remove leading dots" do
+      UserNameSuggester.suggest(".myname").should == 'myname'
+    end
+
+    it "remove trailing dots" do
+      UserNameSuggester.suggest("myname.").should == 'myname'
+    end
+
+    it 'should handle typical facebook usernames' do
+      UserNameSuggester.suggest('roger.nelson.3344913').should == 'roger_nelson_33'
+    end
+  end
+
+end

spec/models/user_spec.rb

@@ -409,10 +409,6 @@ describe User do
   end
 
   describe 'name heuristics' do
-    it 'is able to guess a decent username from an email' do
-      User.suggest_username('bob@bob.com').should == 'bob'
-    end
-
     it 'is able to guess a decent name from an email' do
       User.suggest_name('sam.saffron@gmail.com').should == 'Sam Saffron'
     end
@@ -474,64 +470,6 @@ describe User do
     end
   end
 
-  describe '.suggest_username' do
-
-    it "doesn't raise an error on nil username" do
-      User.suggest_username(nil).should be_nil
-    end
-
-    it 'corrects weird characters' do
-      User.suggest_username("Darth%^Vader").should == "Darth_Vader"
-    end
-
-    it 'adds 1 to an existing username' do
-      user = Fabricate(:user)
-      User.suggest_username(user.username).should == "#{user.username}1"
-    end
-
-    it "adds numbers if it's too short" do
-      User.suggest_username('a').should == 'a11'
-    end
-
-    it "has a special case for me and i emails" do
-      User.suggest_username('me@eviltrout.com').should == 'eviltrout'
-      User.suggest_username('i@eviltrout.com').should == 'eviltrout'
-    end
-
-    it "shortens very long suggestions" do
-      User.suggest_username("myreallylongnameisrobinwardesquire").should == 'myreallylongnam'
-    end
-
-    it "makes room for the digit added if the username is too long" do
-      User.create(username: 'myreallylongnam', email: 'fake@discourse.org')
-      User.suggest_username("myreallylongnam").should == 'myreallylongna1'
-    end
-
-    it "removes leading character if it is not alphanumeric" do
-      User.suggest_username("_myname").should == 'myname'
-    end
-
-    it "removes trailing characters if they are invalid" do
-      User.suggest_username("myname!^$=").should == 'myname'
-    end
-
-    it "replace dots" do
-      User.suggest_username("my.name").should == 'my_name'
-    end
-
-    it "remove leading dots" do
-      User.suggest_username(".myname").should == 'myname'
-    end
-
-    it "remove trailing dots" do
-      User.suggest_username("myname.").should == 'myname'
-    end
-
-    it 'should handle typical facebook usernames' do
-      User.suggest_username('roger.nelson.3344913').should == 'roger_nelson_33'
-    end
-  end
-
   describe 'email_validator' do
     it 'should allow good emails' do
       user = Fabricate.build(:user, email: 'good@gmail.com')