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

Persist more user data from Slack in the brain #229

Merged
merged 3 commits into from
Oct 20, 2015

Conversation

technicalpickles
Copy link
Contributor

I wanted access to more details about a user in order to write listener middleware for doing things like blocking access for restricted and bot users.

The data is there in hubot-slack, but just not put into hubot's brain. I had considered just adding everything to the user object at the top level, but I think it'd be better to namespace it under slack, so it doesn't seem like that's something a script author should be manipulating.

Here's what it looks like slackbot looks like:

  { id: 'USLACKBOT',
       name: 'slackbot',
       real_name: 'slackbot',
       email_address: '',
       slack: 
        { id: 'USLACKBOT',
          name: 'slackbot',
          deleted: false,
          status: null,
          color: '757575',
          real_name: 'slackbot',
          tz: null,
          tz_label: 'Pacific Daylight Time',
          tz_offset: -25200,
          profile: 
           { first_name: 'slackbot',
             last_name: '',
             image_24: 'https://slack-assets2.s3-us-west-2.amazonaws.com/10068/img/slackbot_24.png',
             image_32: 'https://slack-assets2.s3-us-west-2.amazonaws.com/10068/img/slackbot_32.png',
             image_48: 'https://slack-assets2.s3-us-west-2.amazonaws.com/10068/img/slackbot_48.png',
             image_72: 'https://slack-assets2.s3-us-west-2.amazonaws.com/10068/img/slackbot_72.png',
             image_192: 'https://slack-assets2.s3-us-west-2.amazonaws.com/10068/img/slackbot_192.png',
             real_name: 'slackbot',
             real_name_normalized: 'slackbot',
             email: '' },
          is_admin: false,
          is_owner: false,
          is_primary_owner: false,
          is_restricted: false,
          is_ultra_restricted: false,
          is_bot: false,
          presence: 'active' } } },

@jeffbyrnes
Copy link

This is pretty nifty, though I’d vote for it to not be namespaces under slack, since if you’re using Slack, all of your users will be Slack users.

@technicalpickles
Copy link
Contributor Author

Yeah, I can definitely appreciate that. However, this is what holds me back:

  • There could be hubot scripts out in the wild that use these some of these variable names, leading to unexpected behavior when they overlap
  • The data is coming from Slack, which could add or remove data to the API payload
  • Scripts could come to rely on the data in the brain without realizing it was a Slack-ism

Given that, I think it makes sense to store it under a namespace. That way, it's really easy to update the entire object (just stash the new data on user.slack), and if you are a script author, you are likely to know it's coming from Slack rather than somewhere else. Between that, I think it will help keep scripts somewhat portable.

I will also mention, it's worth keeping email and real_name separate, as there's a (undocumented 😓) convention of adapters that there's at least email in user object.

@jeffbyrnes
Copy link

Makes sense. Definitely safer to namespace; you've convinced me.

email_address: user.profile.email
slack: {}
for key, value of user
# user contains an of the SlackClient, which and contains references to the all the data types (users, channels) plus things like the token, s
Copy link

Choose a reason for hiding this comment

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

I think this line needs an update! I couldn't get it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I edited this a few times, and ended up with word soup 😓 Here's what I think I was trying to say originally:

user contains an instance of the Slack, which contains references to all the data types

I'll take another pass at describing it.

@mjsz mjsz merged commit 9674b78 into slackapi:master Oct 20, 2015
mjsz added a commit that referenced this pull request Oct 20, 2015
Merge pull request #229 from github/persist-more-slack-user-data

Persist more user data from Slack in the brain
@jeffbyrnes
Copy link

Would love to know when this ships in a new version, I can make great use of it in a few scripts of mine!

@mjsz
Copy link
Contributor

mjsz commented Oct 23, 2015

Now live in 3.4.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants