Merge pull request #871 from avdi/refactoring-with-josh-and-avdi

Various refactorings towards Ruby/Rails idiom from Josh Susser and Avdi Grimm

app/models/category.rb

@@ -111,7 +111,7 @@ class Category < ActiveRecord::Base
   def group_names=(names)
     # this line bothers me, destroying in AR can not seem to be queued, thinking of extending it
     category_groups.destroy_all unless new_record?
-    ids = Group.where(name: names.split(",")).select(:id).map(&:id)
+    ids = Group.where(name: names.split(",")).pluck(:id)
     ids.each do |id|
       category_groups.build(group_id: id)
     end

app/models/group.rb

@@ -130,11 +130,7 @@ class Group < ActiveRecord::Base
   end
 
   def usernames
-    users.select("username").map(&:username).join(",")
-  end
-
-  def user_ids
-    users.select('users.id').map(&:id)
+    users.pluck(:username).join(",")
   end
 
   def add(user)

app/models/post_action.rb

@@ -26,7 +26,7 @@ class PostAction < ActiveRecord::Base
                                     .count('DISTINCT posts.id')
 
     $redis.set('posts_flagged_count', posts_flagged_count)
-    user_ids = User.staff.select(:id).map {|u| u.id}
+    user_ids = User.staff.pluck(:id)
     MessageBus.publish('/flagged_counts', { total: posts_flagged_count }, { user_ids: user_ids })
   end
 

app/models/user.rb

@@ -69,16 +69,14 @@ class User < ActiveRecord::Base
   end
 
   def self.sanitize_username!(name)
-    name.gsub!(/^[^A-Za-z0-9]+|[^A-Za-z0-9_]+$/, "")
-    name.gsub!(/[^A-Za-z0-9_]+/, "_")
+    name.gsub!(/^[^[:alnum:]]+|\W+$/, "")
+    name.gsub!(/\W+/, "_")
   end
 
-  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
 
   def self.find_available_username_based_on(name)
+    sanitize_username!(name)
+    name = rightsize_username(name)
     i = 1
     attempt = name
     until username_available?(attempt)
@@ -103,14 +101,13 @@ class User < ActiveRecord::Base
       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.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]

lib/post_destroyer.rb

@@ -43,7 +43,7 @@ class PostDestroyer
       @post.update_flagged_posts_count
 
       # Remove any reply records that point to deleted posts
-      post_ids = PostReply.select(:post_id).where(reply_id: @post.id).map(&:post_id)
+      post_ids = PostReply.where(reply_id: @post.id).pluck(:post_id)
       PostReply.delete_all reply_id: @post.id
 
       if post_ids.present?