@dblock Show rank section for N players, closes #14. #17
Conversation
| @@ -19,6 +19,14 @@ def self.find_by_slack_mention(user_name) | |||
| User.where(user_name =~ /^<@(.*)>$/ ? { user_id: Regexp.last_match[1] } : { user_name: user_name }).first | |||
| end | |||
|
|
|||
| def self.find_many_by_slack_mention(user_names) | |||
There was a problem hiding this comment.
Since find_by_slack_mention doesn't raise an exception when a user isn't found, find_many_by_slack_mention shouldn't either. You pick: either change find_by_slack_mention to find_by_slack_mention! (bang method) and fail in there (and adjust callers) or return an array where some users may be nil and handle it in the caller. I don't have a preference.
|
The model-level methods introduced like This is looking great! |
| 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: "gamebot rank #{users.join(' ')}").to respond_with_slack_message "2. #{user_elo_67}\n3. #{user_elo_42}\n4. #{user_elo_38}" | ||
| end |
There was a problem hiding this comment.
It should also work with the user slack ID, not just the slack mention, there should be a test like that.
|
Also forgot to mention, README needs to change, the new command should be documented. |
|
And while you're at it, the API could support it as well. |
wrgoldstein
force-pushed
the
master
branch
2 times, most recently
from
f6315ba to
7d51de1
Compare
|
This looks good, it needs to be rebased and squashed, please? There's one more comment re: a spec that you could/should probably address and the API. |
wrgoldstein
force-pushed
the
master
branch
2 times, most recently
from
dd007c3 to
a5f4381
Compare
| @@ -31,4 +31,5 @@ group :test do | |||
| gem 'database_cleaner', '~> 1.4' | |||
| gem 'hyperclient' | |||
| gem 'rack-test', '~> 0.6.2' | |||
| gem 'pry' | |||
There was a problem hiding this comment.
Can you remove pry from here, no need for it.
|
Merged. |
No description provided.