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

explicitly track user list sync, preference for which users' presence to subscribe #479

Open
3 tasks done
aoberoi opened this issue Apr 21, 2018 · 0 comments
Open
3 tasks done
Labels
enhancement M-T: A feature request for new functionality

Comments

@aoberoi
Copy link
Contributor

aoberoi commented Apr 21, 2018

Description

in the current implementation, SlackClient#loadUsers() will typically be called twice each time Hubot starts up. the first time, immediately. the second time, after the brain's storage connection is completed and the 'loaded' event is fired. this is wasteful.

the second problem is that subscribing to user presence is tied to the brain 'loaded' event, which seems incorrect. will this ever fire if the brain doesn't have any backing storage (just using memory)? will it only subscribe to a partial list of users (those who were in the brain when it loaded but not those who are modified after SlackClient#loadUsers() completes)?

with the introduction of the Bluebird dependency, it seems like we can use Promises to create a better solution. here's some pseudo-code.

# in SlackBot constructor add `subscribeAllUserPresence` boolean option default to true

# in SlackBot#run()
loadUsers = new Promise (resolve, reject) =>
  @client.loadUsers (error, res) =>
    if (error)
      return reject(error)
    resolve(res)

brainHasStorage = true # no idea how to get this value
initialBrainLoad = not brainHasStorage ? Promise.resolve() : new Promise (resolve, reject) =>
  loaded = false
  @robot.brain.on 'loaded', () ->
    if not loaded
      loaded = true
      resolve()

Promise.all([loadUsers, initialBrainLoad])
  .then ([res]) =>
    @usersLoaded(null, res)
    @presenceSub()
  .catch console.error # TODO

# in SlackBot#presenceSub()
if @options.subscribeAllUserPresence
  # ... current implementation here

# in SlackBot#updateUserInBrain()
if @options.subscribeAllUserPresence && userNotInBrain
  # add a new presence subscription for a single user

should we be handling team_joinevents too?

should we centralize the two places where @robot.brain.userForId are called?

Requirements (place an x in each of the [ ])

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.
@aoberoi aoberoi added the enhancement M-T: A feature request for new functionality label Jun 12, 2018
@aoberoi aoberoi mentioned this issue Oct 23, 2018
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement M-T: A feature request for new functionality
Projects
None yet
Development

No branches or pull requests

1 participant