From c34f476b31b3716d8e66f7e9cd47500103f821e3 Mon Sep 17 00:00:00 2001 From: Philipp Weissensteiner Date: Sun, 31 Mar 2013 18:51:13 +0200 Subject: [PATCH] Improve suggest_username method in user.rb The suggest_username method showed up on codeclimate so I thought I'd give it some love and make it more readable. In the process removed trailing whitespaces and had to fix a terrible spelling error :) --- app/models/user.rb | 52 ++++++++++++++++++++++++---------------- spec/models/user_spec.rb | 20 +++++++++------- 2 files changed, 42 insertions(+), 30 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 83f6b528b45..d43c2b9bda7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -59,31 +59,20 @@ class User < ActiveRecord::Base 3..15 end - def self.suggest_username(name) - return unless name.present? - - # If it's an email - if name =~ /([^@]+)@([^\.]+)/ - name = Regexp.last_match[1] - - # Special case, if it's "me" or "i" @ something, take the something. - name = Regexp.last_match[2] if ['i', 'me'].include?(name) - end - - name.gsub!(/^[^A-Za-z0-9]+/, "") - name.gsub!(/[^A-Za-z0-9_]+$/, "") + def self.sanitize_username!(name) + name.gsub!(/^[^A-Za-z0-9]+|[^A-Za-z0-9_]+$/, "") name.gsub!(/[^A-Za-z0-9_]+/, "_") + end - # Pad the length with 1s + def self.pad_missing_chars_with_1s!(name) missing_chars = User.username_length.begin - name.length name << ('1' * missing_chars) if missing_chars > 0 + end - # Trim extra length - name = name[0..User.username_length.end-1] - + def self.find_available_username_based_on(name) i = 1 attempt = name - while !username_available?(attempt) + until username_available?(attempt) suffix = i.to_s max_length = User.username_length.end - suffix.length - 1 attempt = "#{name[0..max_length]}#{suffix}" @@ -92,6 +81,27 @@ class User < ActiveRecord::Base attempt 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 + + sanitize_username!(name) + pad_missing_chars_with_1s!(name) + + # Trim extra length + name = name[0..User.username_length.end-1] + find_available_username_based_on(name) + end + def self.create_for_email(email, opts={}) username = suggest_username(email) @@ -240,7 +250,7 @@ class User < ActiveRecord::Base end def moderator? - # this saves us from checking both, admins are always moderators + # this saves us from checking both, admins are always moderators # # in future we may split this out admin || moderator @@ -409,7 +419,7 @@ class User < ActiveRecord::Base end # a touch faster than automatic - def admin? + def admin? admin end @@ -545,7 +555,7 @@ class User < ActiveRecord::Base end end end - + def email_in_restriction_setting?(setting) domains = setting.gsub('.', '\.') regexp = Regexp.new("@(#{domains})", true) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 798d721e6fe..554e74ccc66 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -443,7 +443,7 @@ describe User do end it 'corrects weird characters' do - User.suggest_username("Darth%^Vadar").should == "Darth_Vadar" + User.suggest_username("Darth%^Vader").should == "Darth_Vader" end it 'adds 1 to an existing username' do @@ -455,8 +455,9 @@ describe User do User.suggest_username('a').should == 'a11' end - it "has a special case for me emails" do + 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 @@ -511,12 +512,12 @@ describe User do Fabricate.build(:user, email: 'notgood@trashmail.net').should_not be_valid Fabricate.build(:user, email: 'mailinator.com@gmail.com').should be_valid end - + it 'should not reject partial matches' do SiteSetting.stubs(:email_domains_blacklist).returns('mail.com') Fabricate.build(:user, email: 'mailinator@gmail.com').should be_valid end - + it 'should reject some emails based on the email_domains_blacklist site setting ignoring case' do SiteSetting.stubs(:email_domains_blacklist).returns('trashmail.net') Fabricate.build(:user, email: 'notgood@TRASHMAIL.NET').should_not be_valid @@ -539,7 +540,7 @@ describe User do u.email = 'nope@mailinator.com' u.should_not be_valid end - + it 'whitelist should reject some emails based on the email_domains_whitelist site setting' do SiteSetting.stubs(:email_domains_whitelist).returns('vaynermedia.com') Fabricate.build(:user, email: 'notgood@mailinator.com').should_not be_valid @@ -565,7 +566,7 @@ describe User do u.should be_valid end - it 'email whitelist should be used when email is being changed' do + it 'email whitelist should be used when email is being changed' do SiteSetting.stubs(:email_domains_whitelist).returns('vaynermedia.com') u = Fabricate(:user, email: 'good@vaynermedia.com') u.email = 'nope@mailinator.com' @@ -735,11 +736,12 @@ describe User do end describe '#create_for_email' do - let(:subject) { User.create_for_email('test@email.com') } + let(:subject) { User.create_for_email('walter.white@email.com') } it { should be_present } - its(:username) { should == 'test' } - its(:name) { should == 'test'} + its(:username) { should == 'walter_white' } + its(:name) { should == 'walter_white'} it { should_not be_active } + its(:email) { should == 'walter.white@email.com' } end describe 'email_confirmed?' do