FIX: username suggester to account for the more relaxed username rules (closes #3907)

app/models/username_validator.rb

@@ -36,6 +36,8 @@ class UsernameValidator
     errors.empty?
   end
 
+  CONFUSING_EXTENSIONS ||= /\.(js|json|css|htm|html|xml|jpg|jpeg|png|gif|bmp|ico|tif|tiff|woff)$/i
+
   private
 
   def username_exist?
@@ -68,29 +70,29 @@ class UsernameValidator
 
   def username_first_char_valid?
     return unless errors.empty?
-    if username[0] =~ /[^\w]/
+    if username[0] =~ /\W/
       self.errors << I18n.t(:'user.username.must_begin_with_alphanumeric')
     end
   end
 
   def username_last_char_valid?
     return unless errors.empty?
-    if username[-1] =~ /[^\w]/
+    if username[-1] =~ /[^A-Za-z0-9]/
       self.errors << I18n.t(:'user.username.must_end_with_alphanumeric')
     end
   end
 
   def username_no_double_special?
     return unless errors.empty?
-    if username =~ /[\-_\.]{2,}/
+    if username =~ /[-_.]{2,}/
       self.errors << I18n.t(:'user.username.must_not_contain_two_special_chars_in_seq')
     end
   end
 
   def username_does_not_end_with_confusing_suffix?
     return unless errors.empty?
-    if username =~ /\.(json|gif|jpeg|png|htm|js|json|xml|woff|tif|html|ico)/i
-      self.errors << I18n.t(:'user.username.must_not_contain_confusing_suffix')
+    if username =~ CONFUSING_EXTENSIONS
+      self.errors << I18n.t(:'user.username.must_not_end_with_confusing_suffix')
     end
   end
 end

config/locales/server.en.yml

@@ -1374,7 +1374,7 @@ en:
       must_begin_with_alphanumeric: "must begin with a letter or number or an underscore"
       must_end_with_alphanumeric: "must end with a letter or number or an underscore"
       must_not_contain_two_special_chars_in_seq: "must not contain a sequence of 2 or more special chars (.-_)"
-      must_not_contain_confusing_suffix: "must not contain a confusing suffix like .json or .png etc."
+      must_not_end_with_confusing_suffix: "must not end with a confusing suffix like .json or .png etc."
     email:
       not_allowed: "is not allowed from that email provider. Please use another email address."
       blocked: "is not allowed."

lib/user_name_suggester.rb

@@ -34,11 +34,17 @@ module UserNameSuggester
   end
 
   def self.sanitize_username(name)
-    ActiveSupport::Inflector.transliterate(name)
-                            .gsub(/^[^[:alnum:]]+|\W+$/, "")
-                            .gsub(/\W+/, "_")
-                            .gsub(/^\_+/, '')
-                            .gsub(/[\-_\.]{2,}/, "_")
+    name = ActiveSupport::Inflector.transliterate(name)
+    # 1. replace characters that aren't allowed with '_'
+    name.gsub!(UsernameValidator::CONFUSING_EXTENSIONS, "_")
+    name.gsub!(/[^\w.-]/, "_")
+    # 2. removes unallowed leading characters
+    name.gsub!(/^\W+/, "")
+    # 3. removes unallowed trailing characters
+    name.gsub!(/[^A-Za-z0-9]+$/, "")
+    # 4. unify special characters
+    name.gsub!(/[-_.]{2,}/, "_")
+    name
   end
 
   def self.rightsize_username(name)

spec/components/email/receiver_spec.rb

@@ -225,7 +225,7 @@ describe Email::Receiver do
 
       user = topic.user
       expect(user.staged).to eq(true)
-      expect(user.username).to eq("random_name")
+      expect(user.username).to eq("random.name")
       expect(user.name).to eq("Случайная Имя")
     end
 

spec/components/user_name_suggester_spec.rb

@@ -56,15 +56,19 @@ describe UserNameSuggester do
     end
 
     it "removes leading character if it is not alphanumeric" do
-      expect(UserNameSuggester.suggest("_myname")).to eq('myname')
+      expect(UserNameSuggester.suggest(".myname")).to eq('myname')
+    end
+
+    it "allows leading _" do
+      expect(UserNameSuggester.suggest("_myname")).to eq('_myname')
     end
 
     it "removes trailing characters if they are invalid" do
       expect(UserNameSuggester.suggest("myname!^$=")).to eq('myname')
     end
 
-    it "replace dots" do
-      expect(UserNameSuggester.suggest("my.name")).to eq('my_name')
+    it "allows dots in the middle" do
+      expect(UserNameSuggester.suggest("my.name")).to eq('my.name')
     end
 
     it "remove leading dots" do
@@ -81,7 +85,7 @@ describe UserNameSuggester do
     end
 
     it 'should handle typical facebook usernames' do
-      expect(UserNameSuggester.suggest('roger.nelson.3344913')).to eq('roger_nelson_33')
+      expect(UserNameSuggester.suggest('roger.nelson.3344913')).to eq('roger.nelson.33')
     end
   end