diff --git a/CHANGELOG.md b/CHANGELOG.md index f21f93f2..05a52c14 100644 --- a/CHANGELOG.md +++ b/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). diff --git a/slack-gamebot/commands/rank.rb b/slack-gamebot/commands/rank.rb index 3a1a4d00..65c61786 100644 --- a/slack-gamebot/commands/rank.rb +++ b/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) diff --git a/slack-gamebot/models/user.rb b/slack-gamebot/models/user.rb index 6da6f12b..ae9c818d 100644 --- a/slack-gamebot/models/user.rb +++ b/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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8cfdd541..590e77d1 100644 --- a/spec/models/user_spec.rb +++ b/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 diff --git a/spec/slack-gamebot/commands/rank_spec.rb b/spec/slack-gamebot/commands/rank_spec.rb index 65b7f0ce..ea0e650e 100644 --- a/spec/slack-gamebot/commands/rank_spec.rb +++ b/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