-
Notifications
You must be signed in to change notification settings - Fork 395
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
Add user property #110
base: develop
Are you sure you want to change the base?
Add user property #110
Conversation
* develop: Stop recommending ``sudo pip install``
user = self.webapi.users.info(userid) | ||
if not user.successful: | ||
raise RuntimeError(user.error) | ||
return user.body["user"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about caching the user info in a dict to avoid getting it back from slack web api for each time? We can add an expiration time to avoid out of date user info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea—I'll see if I can improve it. By the way, any suggestions for how to write a test case for this? I haven't completely understood how you've set up the test environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jtatum has written the detailed steps to run the tests in a dedicated slack team, you can read it here: https://github.com/lins05/slackbot/blob/develop/CONTRIBUTING.md#configure-tests
Thanks! These are great ideas! There should be a better way to load some non-Slack information that belongs exclusively to the user in a Team so the bot can provide her/him with ad-hoc data. Example: a user in a team wants to know if her holiday application was approved by the company.
At the moment 'message.user' seems a bit counter-intuitive. What do you think? |
This change looks cool but I think |
Rather than using the API to poll users, it would be cool if it used the profile information captured during rtm.start and kept it updated using the mechanism from #117. Once it's merged, we'd need to add a handler for user.change as well to keep the stored profile data current. Then there's no need to make api calls for data that slack has already told us about. |
ping @nikitaborisov |
Recent changes to bot code should keep the _client.users dict updated with current user data, including slack profile info. Using the API shouldn't be required. Please rebase changes against latest code, then remove usage of the web api to retrieve user info. |
I wanted to get the name of the user who sent me a message so that I could, for example, say "Hello, FirstName", so I created a
user
property forMessage
. Now this code works: