make User.find_by_slack_mention a bang method that fails if the user …

…is not found\n refactor Leaderboard into its own class

README.md

@@ -116,6 +116,17 @@ gamebot leaderboard
 
 The leaderboard contains 3 topmost players ranked by [Elo](http://en.wikipedia.org/wiki/Elo_rating_system), use _leaderboard 10_ or _leaderboard infinity_ to see 10 players or more, respectively.
 
+#### gamebot rank [<user>, ...]
+
+Show the smallest range of ranks for a list of players.  If no user is passed, your rank is shown.
+
+```
+gamebot rank @WangHoe @DengYaping
+
+2. Deng Yaping: 1 win, 3 losses (elo: 24)
+3. Wang Hoe: 0 wins, 1 loss (elo: -12)
+```
+
 #### gamebot reset [secret]
 
 Direct-message gamebot to reset all users and pending challenges.

app/models.rb

@@ -3,3 +3,4 @@
 require 'models/challenge_state'
 require 'models/challenge'
 require 'models/match'
+require 'models/leaderboard'

app/models/challenge.rb

@@ -30,9 +30,7 @@ def self.split_teammates_and_opponents(challenger, names, separator = 'with')
       if name == separator
         current_side = teammates
       else
-        user = ::User.find_by_slack_mention(name)
-        fail ArgumentError, "I don't know who #{name} is! Ask them to _#{SlackGamebot.config.user} register_." unless user
-        current_side << user
+        current_side << ::User.find_by_slack_mention!(name)
       end
     end
 

app/models/leaderboard.rb

@@ -0,0 +1,28 @@
+class Leaderboard
+  def self.standings(max = nil)
+    players = User.all.any_of({ :wins.gt => 0 }, :losses.gt => 0)
+    players = players.limit(max) if max && max > 0
+    players = players.desc(:elo).asc(:_id).to_a
+    rank = 1
+    standings = []
+    players.each_with_index do |player, index|
+      rank += 1 if index > 0 && players[index - 1].elo != player.elo
+      standings << [rank, player]
+    end
+    standings
+  end
+
+  def self.leaderboard(max = 3)
+    stringify_leaderboard(standings(max))
+  end
+
+  def self.rank_section(users)
+    leaderboard = standings.drop_while { |row| !users.member? row.last }
+    leaderboard = leaderboard.reverse.drop_while { |row| !users.member? row.last }.reverse
+    stringify_leaderboard(leaderboard)
+  end
+
+  def self.stringify_leaderboard(leaderboard)
+    leaderboard.any? ? leaderboard.map { |a| a.join('. ') }.join("\n") : 'No players.'
+  end
+end

app/models/user.rb

@@ -15,16 +15,14 @@ def slack_mention
     "<@#{user_id}>"
   end
 
-  def self.find_by_slack_mention(user_name)
-    User.where(user_name =~ /^<@(.*)>$/ ? { user_id: Regexp.last_match[1] } : { user_name: user_name }).first
+  def self.find_by_slack_mention!(user_name)
+    user = User.where(user_name =~ /^<@(.*)>$/ ? { user_id: Regexp.last_match[1] } : { user_name: user_name }).first
+    fail ArgumentError, "I don't know who #{user_name} is! Ask them to _#{SlackGamebot.config.user} register_." unless user
+    user
   end
 
-  def self.find_many_by_slack_mention(user_names)
-    user_names.map do |user_name|
-      user = find_by_slack_mention(user_name)
-      fail ArgumentError, "I don't know who #{user_name} is! Ask them to _#{SlackGamebot.config.user} register_." unless user
-      user
-    end
+  def self.find_many_by_slack_mention!(user_names)
+    user_names.map { |user| find_by_slack_mention!(user) }
   end
 
   # Find an existing record, update the username if necessary, otherwise create a user record.
@@ -36,39 +34,11 @@ def self.find_create_or_update_by_slack_id!(slack_id)
     instance
   end
 
-  def self.standings(max = nil)
-    players = User.all.any_of({ :wins.gt => 0 }, :losses.gt => 0)
-    players = players.limit(max) if max && max > 0
-    players = players.desc(:elo).asc(:_id).to_a
-    rank = 1
-    standings = []
-    players.each_with_index do |player, index|
-      rank += 1 if index > 0 && players[index - 1].elo != player.elo
-      standings << [rank, player]
-    end
-    standings
-  end
-
-  def self.leaderboard(max = 3)
-    stringify_leaderboard(standings(max))
-  end
-
-  def self.rank_section(users)
-    users = find_many_by_slack_mention(users)
-    leaderboard = standings(nil).drop_while { |row| !users.member? row.last }
-    leaderboard = leaderboard.reverse.drop_while { |row| !users.member? row.last }.reverse
-    stringify_leaderboard(leaderboard)
-  end
-
   def self.reset_all!
     User.all.set(wins: 0, losses: 0, elo: 0, tau: 0)
   end
 
   def to_s
     "#{user_name}: #{wins} win#{wins != 1 ? 's' : ''}, #{losses} loss#{losses != 1 ? 'es' : ''} (elo: #{elo})"
   end
-
-  def self.stringify_leaderboard(leaderboard)
-    leaderboard.any? ? leaderboard.map { |a| a.join('. ') }.join("\n") : 'No players.'
-  end
 end

app/slack-gamebot/commands/leaderboard.rb

@@ -9,7 +9,7 @@ def self.call(data, _command, arguments)
         else
           max = Integer(arguments.first)
         end if arguments.any?
-        send_message data.channel, User.leaderboard(max)
+        send_message data.channel, ::Leaderboard.leaderboard(max)
       end
     end
   end

app/slack-gamebot/commands/rank.rb

@@ -3,8 +3,9 @@ module Commands
     class Rank < Base
       def self.call(data, _command, arguments)
         users = arguments
-        users << data.user.slack_mention if arguments.empty?
-        send_message data.channel, User.rank_section(users)
+        users << data.user.user_name if arguments.empty?
+        users = User.find_many_by_slack_mention!(users)
+        send_message data.channel, ::Leaderboard.rank_section(users)
       end
     end
   end

spec/models/leaderboard_spec.rb

@@ -0,0 +1,56 @@
+require 'spec_helper'
+
+describe Leaderboard do
+  context '#standings' do
+    it 'returns an empty list' do
+      expect(Leaderboard.standings).to eq []
+    end
+    it 'ranks incrementally' do
+      user1 = Fabricate(:user, elo: 1, wins: 1, losses: 1)
+      user2 = Fabricate(:user, elo: 2, wins: 1, losses: 1)
+      expect(Leaderboard.standings).to eq [[1, user2], [2, user1]]
+    end
+    it 'ranks players with the same elo equally' do
+      user1 = Fabricate(:user, elo: 1, wins: 1, losses: 1)
+      user2 = Fabricate(:user, elo: 2, wins: 1, losses: 1)
+      user3 = Fabricate(:user, elo: 1, wins: 1, losses: 1)
+      expect(Leaderboard.standings).to eq [[1, user2], [2, user1], [2, user3]]
+    end
+    it 'limits to max' do
+      Fabricate(:user, elo: 1, wins: 1, losses: 1)
+      user2 = Fabricate(:user, elo: 2, wins: 1, losses: 1)
+      Fabricate(:user, elo: 1, wins: 1, losses: 1)
+      expect(Leaderboard.standings(1)).to eq [[1, user2]]
+    end
+    it 'ignores players without wins or losses' do
+      user1 = Fabricate(:user, elo: 1, wins: 1, losses: 1)
+      Fabricate(:user, elo: 2, wins: 0, losses: 0)
+      expect(Leaderboard.standings).to eq [[1, user1]]
+    end
+    it 'first game' do
+      user1 = Fabricate(:user, elo: 48, wins: 1, losses: 0)
+      user2 = Fabricate(:user, elo: -48, wins: 0, losses: 1)
+      expect(Leaderboard.standings).to eq [[1, user1], [2, user2]]
+    end
+  end
+  context '#leaderboard' do
+    it 'returns no players' do
+      expect(Leaderboard.leaderboard).to eq 'No players.'
+    end
+    it 'returns formatted leaderboard' do
+      user1 = Fabricate(:user, elo: 48, wins: 1, losses: 0)
+      user2 = Fabricate(:user, elo: -48, wins: 0, losses: 1)
+      expect(Leaderboard.leaderboard).to eq "1. #{user1}\n2. #{user2}"
+    end
+  end
+  context '#rank_section' do
+    it 'returns no players' do
+      expect(Leaderboard.leaderboard).to eq 'No players.'
+    end
+  end
+  context '#stringify_leaderboard' do
+    it 'joins arrays of arrays' do
+      expect(Leaderboard.stringify_leaderboard([[1, 'foo'], [2, 'bar']])).to eq "1. foo\n2. bar"
+    end
+  end
+end

spec/models/user_spec.rb

@@ -1,15 +1,34 @@
 require 'spec_helper'
 
 describe User do
-  context '#find_by_slack_mention' do
+  context '#find_by_slack_mention!' do
     before do
       @user = Fabricate(:user)
     end
     it 'finds by slack id' do
-      expect(User.find_by_slack_mention("<@#{@user.user_id}>")).to eq @user
+      expect(User.find_by_slack_mention!("<@#{@user.user_id}>")).to eq @user
     end
     it 'finds by username' do
-      expect(User.find_by_slack_mention(@user.user_name)).to eq @user
+      expect(User.find_by_slack_mention!(@user.user_name)).to eq @user
+    end
+    it 'requires a known user' do
+      expect do
+        User.find_by_slack_mention!('<@nobody>')
+      end.to raise_error ArgumentError, "I don't know who <@nobody> is! Ask them to _#{SlackGamebot.config.user} register_."
+    end
+  end
+  context '#find_many_by_slack_mention!' do
+    before do
+      @users = [Fabricate(:user), Fabricate(:user)]
+    end
+    it 'finds by slack_id or slack_mention' do
+      users = User.find_many_by_slack_mention! [@users.first.user_name, @users.last.slack_mention]
+      expect(users).to contain_exactly(*@users)
+    end
+    it 'requires known users' do
+      expect do
+        User.find_many_by_slack_mention! %w(foo bar)
+      end.to raise_error ArgumentError, "I don't know who foo is! Ask them to _#{SlackGamebot.config.user} register_."
     end
   end
   context '#find_create_or_update_by_slack_id!', vcr: { cassette_name: 'user_info' } do
@@ -40,38 +59,6 @@
       end
     end
   end
-  context '#leaderboard' do
-    it 'returns no players' do
-      expect(User.leaderboard).to eq 'No players.'
-    end
-    it 'ranks incrementally' do
-      user1 = Fabricate(:user, elo: 1, wins: 1, losses: 1)
-      user2 = Fabricate(:user, elo: 2, wins: 1, losses: 1)
-      expect(User.leaderboard).to eq "1. #{user2}\n2. #{user1}"
-    end
-    it 'ranks players with the same elo equally' do
-      user1 = Fabricate(:user, elo: 1, wins: 1, losses: 1)
-      user2 = Fabricate(:user, elo: 2, wins: 1, losses: 1)
-      user3 = Fabricate(:user, elo: 1, wins: 1, losses: 1)
-      expect(User.leaderboard).to eq "1. #{user2}\n2. #{user1}\n2. #{user3}"
-    end
-    it 'limits to max' do
-      Fabricate(:user, elo: 1, wins: 1, losses: 1)
-      user2 = Fabricate(:user, elo: 2, wins: 1, losses: 1)
-      Fabricate(:user, elo: 1, wins: 1, losses: 1)
-      expect(User.leaderboard(1)).to eq "1. #{user2}"
-    end
-    it 'ignores players without wins or losses' do
-      user1 = Fabricate(:user, elo: 1, wins: 1, losses: 1)
-      Fabricate(:user, elo: 2, wins: 0, losses: 0)
-      expect(User.leaderboard).to eq "1. #{user1}"
-    end
-    it 'first game' do
-      user1 = Fabricate(:user, elo: 48, wins: 1, losses: 0)
-      user2 = Fabricate(:user, elo: -48, wins: 0, losses: 1)
-      expect(User.leaderboard).to eq "1. #{user1}\n2. #{user2}"
-    end
-  end
   context '#reset_all' do
     it 'resets all user stats' do
       user1 = Fabricate(:user, elo: 48, losses: 1, wins: 2, tau: 0.5)

spec/slack-gamebot/commands/rank_spec.rb

@@ -9,7 +9,7 @@
   it 'ranks the requester if no argument is passed' do
     expect(message: 'gamebot rank', user: user_elo_42).to respond_with_slack_message "3. #{user_elo_42}"
   end
-  it 'ranks one player by name' do
+  it 'ranks one player by slack mention' do
     expect(message: "gamebot rank #{user_elo_42.slack_mention}").to respond_with_slack_message "3. #{user_elo_42}"
   end
   it 'shows the smallest range of ranks for a list of players' do