Add support for response.sendPrivate

[faridnsh]

• 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.

Description

It'd be nice to have response.sendPrivate which basically sends a direct message to the given user that initiated that response. It would be a shortcut for this mainly:

robot.messageRoom msg.message.user.name, "Hello!"

This is supported by hubot-irc, hubot-test-helper and hubot-rocketchat. Probably it would be also easier for users to figure out sending private messages quick and easy.(Check #159 and #299)

I'd like to contribute this, but I'm not really sure how to write to test it with the stubs and whatever we have in place.

[aoberoi]

thanks for pointing this hole in functionality out @alFReD-NSH! looking through those older issues really underscores how this adapter is really under serving users who want to simply send a DM.

although the snippet you posted works, its really not preferred and may break in the future. the slack platform is moving away from any guarantees of uniqueness for user names, ever since the introduction of Shared Channels. you can see the details in this changelog entry. the future is about only operating on user IDs. this means that the internal implementation wouldn't just be calling Web API method chat.postMessage (which is what SlackClient.send() uses currently) with the username in the channel argument. the right way to do this would be to first open a new conversation using conversations.open passing the user ID in the users argument. this call would return a channel ID, which can subsequently be used in chat.postMessage.

i'd love a contribution! let's talk a little about how this should be done. first of all, we should make sure the implementation does not depend on the RTMClient DataStore (see #385) since it is being removed. as i previously mentioned, it should not depend on the user name either, only the ID. one idea (that might not be a good one, but worth considering) is we can keep the functionality decoupled from the Adapter code by implementing it as a response middleware and then registering it on adapter initialization. i'm happy to hear your thoughts on an implementation and even collaborate if you'd like.

[drdamour]

it seems like robot.messageRoom msg.message.user.name, "Hello!" is already broken i have to use robot.messageRoom msg.message.user.id, "Hello!" for things to work.

this unfortunately breaks the reply in private of hubot-help

[drdamour]

what's weird is robot.messageRoom msg.message.user.name works in the old hubot-slack, v 3.4.2, but in v4 it does not, why is that?

[aoberoi]

@drdamour the difference is because the old hubot-slack (v3.4.2) purely uses the RTM API to send messages (which also means it cannot handle attachments and other nice things). in the new version, those messages are sent over the Web API (using chat.postMessage). these two techniques have different behavior.

i still think this would be a good feature to have, so adding the good first issue label for anyone that feels enthusiastic.

[drdamour]

ok that makes sense...but it is a shame because a fair amount of hubot scripts rely on sendMessage room: user.name and this pass through nature straight to the service layer that can no longer handle that makes them ineffective. trying to conceive if there's a way in the slack adapter to build up a map as messages come in of username -> userId and to intercept messages targeting a user.name room and switch it to id.

[aoberoi]

@drdamour heh, that map is what the data store that we just deprecated contains (amongst other things).

i won't go into all the problems with the data store (see: slackapi/node-slack-sdk#330), but since the platform stopped guaranteeing the uniqueness of usernames its design hasn't worked as well. we could probably do better for just the username -> user ID map situation! we could build a cache where if the lookup failed, the web API would be used to find the correct mapping. the tricky part is cache invalidation. i'd have to think more about that.

open to any ideas and thoughts!

[drdamour]

yeah makes sense to kill it in core slack...but want to do it in the hubot-slack layer to "polyfill" the hubot contract.