Fix #104: rank leaks users from other teams.

CHANGELOG.md

@@ -1,5 +1,6 @@
 ### Changelog
 
+* [#104](https://github.com/dblock/slack-gamebot/issues/104): Fix: `rank` leaks users from other teams - [@dblock](https://github.com/dblock).
 * [#102](https://github.com/dblock/slack-gamebot/issues/102), [#101](https://github.com/dblock/slack-gamebot/issues/101): Unable to set options with existing challenges - [@dblock](https://github.com/dblock).
 * [#100](https://github.com/dblock/slack-gamebot/issues/100): Error re-registering and de-activating a team multiple times - [@dblock](https://github.com/dblock).
 * [#98](https://github.com/dblock/slack-gamebot/issues/98): Reactivating a team doesn't update auth token - [@dblock](https://github.com/dblock).

slack-gamebot/commands/rank.rb

@@ -9,7 +9,7 @@ def self.call(client, data, match)
         else
           users << ::User.find_create_or_update_by_slack_id!(client, data.user)
         end
-        message = User.rank_section(users).map do |user|
+        message = User.rank_section(client.owner, users).map do |user|
           user.rank ? "#{user.rank}. #{user}" : "#{user.user_name}: not ranked"
         end.join("\n")
         client.say(channel: data.channel, text: message)

slack-gamebot/models/user.rb

@@ -88,8 +88,8 @@ def self.rank!(team)
     end
   end
 
-  def self.rank_section(users)
+  def self.rank_section(team, users)
     ranks = users.map(&:rank)
-    where(:rank.gte => ranks.min, :rank.lte => ranks.max).asc(:rank).asc(:wins).asc(:ties)
+    where(team: team, :rank.gte => ranks.min, :rank.lte => ranks.max).asc(:rank).asc(:wins).asc(:ties)
   end
 end

spec/models/user_spec.rb

@@ -146,15 +146,20 @@
     end
   end
   context '.rank_section' do
+    let(:team) { Fabricate(:team) }
     it 'returns a section' do
-      user1 = Fabricate(:user, elo: 100, wins: 4, losses: 0)
-      user2 = Fabricate(:user, elo: 40, wins: 1, losses: 1)
-      user3 = Fabricate(:user, elo: 60, wins: 2, losses: 0)
-      user4 = Fabricate(:user, elo: 80, wins: 3, losses: 0)
+      user1 = Fabricate(:user, team: team, elo: 100, wins: 4, losses: 0)
+      user2 = Fabricate(:user, team: team, elo: 40, wins: 1, losses: 1)
+      user3 = Fabricate(:user, team: team, elo: 60, wins: 2, losses: 0)
+      user4 = Fabricate(:user, team: team, elo: 80, wins: 3, losses: 0)
       [user1, user2, user3, user4].each(&:reload)
-      expect(User.rank_section([user1])).to eq [user1]
-      expect(User.rank_section([user1, user3])).to eq [user1, user4, user3]
-      expect(User.rank_section([user1, user3, user4])).to eq [user1, user4, user3]
+      expect(User.rank_section(team, [user1])).to eq [user1]
+      expect(User.rank_section(team, [user1, user3])).to eq [user1, user4, user3]
+      expect(User.rank_section(team, [user1, user3, user4])).to eq [user1, user4, user3]
+    end
+    it 'limits by team' do
+      user = Fabricate(:user, elo: 100, wins: 4, losses: 0)
+      expect(User.rank_section(Fabricate(:team), [user])).to eq []
     end
   end
 end

spec/slack-gamebot/commands/rank_spec.rb

@@ -3,25 +3,34 @@
 describe SlackGamebot::Commands::Rank, vcr: { cassette_name: 'user_info' } do
   let!(:team) { Fabricate(:team) }
   let(:app) { SlackGamebot::Server.new(team: team) }
-  let!(:user_elo_12) { Fabricate(:user, elo: 12, wins: 0, losses: 25) }
-  let!(:user_elo_38) { Fabricate(:user, elo: 38, wins: 3, losses: 3) }
-  let!(:user_elo_42) { Fabricate(:user, elo: 42, wins: 3, losses: 2) }
-  let!(:user_elo_67) { Fabricate(:user, elo: 67, wins: 5, losses: 2) }
-  let!(:user_elo_98) { Fabricate(:user, elo: 98, wins: 7, losses: 0) }
-  it 'ranks the requester if no argument is passed' do
-    expect(message: "#{SlackRubyBot.config.user} rank", user: user_elo_42.user_id).to respond_with_slack_message '3. username: 3 wins, 2 losses (elo: 42)'
+  shared_examples_for 'rank' do
+    let!(:user_elo_12) { Fabricate(:user, elo: 12, wins: 0, losses: 25) }
+    let!(:user_elo_38) { Fabricate(:user, elo: 38, wins: 3, losses: 3) }
+    let!(:user_elo_42) { Fabricate(:user, elo: 42, wins: 3, losses: 2) }
+    let!(:user_elo_67) { Fabricate(:user, elo: 67, wins: 5, losses: 2) }
+    let!(:user_elo_98) { Fabricate(:user, elo: 98, wins: 7, losses: 0) }
+    it 'ranks the requester if no argument is passed' do
+      expect(message: "#{SlackRubyBot.config.user} rank", user: user_elo_42.user_id).to respond_with_slack_message '3. username: 3 wins, 2 losses (elo: 42)'
+    end
+    it 'ranks someone who is not ranked' do
+      expect(message: "#{SlackRubyBot.config.user} rank", user: Fabricate(:user).user_id).to respond_with_slack_message 'username: not ranked'
+    end
+    it 'ranks one player by slack mention' do
+      expect(message: "#{SlackRubyBot.config.user} rank #{user_elo_42.slack_mention}").to respond_with_slack_message "3. #{user_elo_42}"
+    end
+    it 'ranks one player by user_id' do
+      expect(message: "#{SlackRubyBot.config.user} rank <@#{user_elo_42.user_id}>").to respond_with_slack_message "3. #{user_elo_42}"
+    end
+    it 'shows the smallest range of ranks for a list of players' do
+      users = [user_elo_38, user_elo_67].map(&:slack_mention)
+      expect(message: "#{SlackRubyBot.config.user} rank #{users.join(' ')}").to respond_with_slack_message "2. #{user_elo_67}\n3. #{user_elo_42}\n4. #{user_elo_38}"
+    end
   end
-  it 'ranks someone who is not ranked' do
-    expect(message: "#{SlackRubyBot.config.user} rank", user: Fabricate(:user).user_id).to respond_with_slack_message 'username: not ranked'
-  end
-  it 'ranks one player by slack mention' do
-    expect(message: "#{SlackRubyBot.config.user} rank #{user_elo_42.slack_mention}").to respond_with_slack_message "3. #{user_elo_42}"
-  end
-  it 'ranks one player by user_id' do
-    expect(message: "#{SlackRubyBot.config.user} rank <@#{user_elo_42.user_id}>").to respond_with_slack_message "3. #{user_elo_42}"
-  end
-  it 'shows the smallest range of ranks for a list of players' do
-    users = [user_elo_38, user_elo_67].map(&:slack_mention)
-    expect(message: "#{SlackRubyBot.config.user} rank #{users.join(' ')}").to respond_with_slack_message "2. #{user_elo_67}\n3. #{user_elo_42}\n4. #{user_elo_38}"
+  it_behaves_like 'rank'
+  context 'with another team' do
+    let!(:team2) { Fabricate(:team) }
+    let!(:user2_elo_42) { Fabricate(:user, team: team2, elo: 42, wins: 3, losses: 2) }
+    let!(:user2_elo_48) { Fabricate(:user, team: team2, elo: 48, wins: 2, losses: 3) }
+    it_behaves_like 'rank'
   end
 end