Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load users to robot.brain.users #381

Merged
merged 9 commits into from
Dec 31, 2016
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion src/bot.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ class SlackBot extends Adapter
@client.on 'reaction_added', @reaction
@client.on 'reaction_removed', @reaction
@client.on 'authenticated', @authenticated
@client.on 'user_change', @userChange

@client.web.users.list @loadUsers
@robot.brain.on 'loaded', () =>
@client.web.users.list @loadUsers

# Start logging in
@client.connect()
Expand Down Expand Up @@ -182,7 +187,7 @@ class SlackBot extends Adapter
@robot.logger.debug "#{user.name} set the topic in #{channel.name} to #{topic}"
@receive new TopicMessage user, message.topic, message.ts

else
else
@robot.logger.debug "Received message: '#{text}' in channel: #{channel.name}, subtype: #{subtype}"
message.user = user
@receive new CatchAllMessage(message)
Expand All @@ -199,4 +204,32 @@ class SlackBot extends Adapter
item_user = @client.rtm.dataStore.getUserById(item_user)
@receive new ReactionMessage(type, user, reaction, item_user, item, event_ts)

loadUsers: (err, res) =>
if err || !res.ok
@robot.logger.error "Can't fetch users"
return

@userChange member for member in res.members

userChange: (user) =>
return unless user
newUser =
id: user.id
name: user.name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we store id: user.id in here as well? if the idea that Slack-specific properties should go inside the slack property, then this is already not working since the ID that the user will be identified with in the brain is the Slack ID.

real_name: user.real_name
email_address: user.profile.email
slack: {}
for key, value of user
# don't store the SlackClient, because it'd cause a circular reference
# (it contains users and channels), and because it has sensitive information like the token
continue if value instanceof SlackClient
newUser.slack[key] = value

if user.id of @robot.brain.data.users
for key, value of @robot.brain.data.users[user.id]
unless key of newUser
newUser[key] = value
delete @robot.brain.data.users[user.id]
@robot.brain.userForId user.id, newUser

module.exports = SlackBot
68 changes: 68 additions & 0 deletions test/bot.coffee
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
should = require 'should'
{Adapter, TextMessage, EnterMessage, LeaveMessage, TopicMessage, Message, CatchAllMessage, Robot, Listener} = require.main.require 'hubot'
ReactionMessage = require '../src/reaction-message'
SlackClient = require '../src/client'

describe 'Adapter', ->
it 'Should initialize with a robot', ->
Expand Down Expand Up @@ -254,3 +255,70 @@ describe 'Robot.react', ->
@slackbot.robot.react matcher, @handleReaction
listener = @slackbot.robot.listeners.shift()
listener.matcher(@reactionMessage).should.be.false

describe 'Users data', ->
it 'Should add a user data', ->
@slackbot.userChange(@stubs.user)

user = @slackbot.robot.brain.data.users[@stubs.user.id]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of accesing the brain data directly, wouldn't it make more sense to use the brain.userForId() method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brain.userForId() create User instance and return it when brain have no user.id.
I'm concerned about that behavior.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call 👍

should.equal user.id, @stubs.user.id
should.equal user.name, @stubs.user.name
should.equal user.real_name, @stubs.user.real_name
should.equal user.email_address, @stubs.user.profile.email
should.equal user.slack.misc, @stubs.user.misc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add id to this


it 'Should modify a user data', ->
@slackbot.userChange(@stubs.user)

user = @slackbot.robot.brain.data.users[@stubs.user.id]
should.equal user.id, @stubs.user.id
should.equal user.name, @stubs.user.name
should.equal user.real_name, @stubs.user.real_name
should.equal user.email_address, @stubs.user.profile.email
should.equal user.slack.misc, @stubs.user.misc

client = new SlackClient {token: 'xoxb-faketoken'}, @stubs.robot

modified_user =
id: @stubs.user.id
name: 'modified_name'
real_name: @stubs.user.real_name
profile:
email: @stubs.user.profile.email
client:
client

@slackbot.userChange(modified_user)

user = @slackbot.robot.brain.data.users[@stubs.user.id]
should.equal user.id, @stubs.user.id
should.equal user.name, modified_user.name
should.equal user.real_name, @stubs.user.real_name
should.equal user.email_address, @stubs.user.profile.email
should.equal user.slack.misc, undefined
should.equal user.slack.client, undefined

it 'Should ignore user data which is undefined', ->
@slackbot.userChange(undefined)
users = @slackbot.robot.brain.data.users
should.equal Object.keys(users).length, 0

it 'Should load users data from web api', ->
@slackbot.loadUsers(null, @stubs.responseUsersList)

user = @slackbot.robot.brain.data.users[@stubs.user.id]
should.equal user.id, @stubs.user.id
should.equal user.name, @stubs.user.name
should.equal user.real_name, @stubs.user.real_name
should.equal user.email_address, @stubs.user.profile.email
should.equal user.slack.misc, @stubs.user.misc

userperiod = @slackbot.robot.brain.data.users[@stubs.userperiod.id]
should.equal userperiod.id, @stubs.userperiod.id
should.equal userperiod.name, @stubs.userperiod.name
should.equal userperiod.real_name, @stubs.userperiod.real_name
should.equal userperiod.email_address, @stubs.userperiod.profile.email

it 'Should detect wrong response from web api', ->
@slackbot.loadUsers(null, @stubs.wrongResponseUsersList)
should.equal @slackbot.robot.brain.data.users[@stubs.user.id], undefined
8 changes: 8 additions & 0 deletions test/stubs.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ beforeEach ->
getType: -> 'group'
@stubs.user =
name: 'name'
real_name: 'real_name'
id: 'U123'
profile:
email: 'email@example.com'
misc: 'misc'
@stubs.bot =
name: 'testbot'
id: 'B123'
Expand Down Expand Up @@ -129,6 +131,12 @@ beforeEach ->
@stubs.channelsMock =
setTopic: (id, topic) =>
@stubs._topic = topic

@stubs.responseUsersList =
ok: true
members: [@stubs.user, @stubs.userperiod]
@stubs.wrongResponseUsersList =
ok: false
# Hubot.Robot instance
@stubs.robot = do ->
robot = new EventEmitter
Expand Down