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

add support for user.id being used instead of user.name for sending help message in private #15

Closed
TobiTenno opened this issue Jun 23, 2016 · 23 comments

Comments

@TobiTenno
Copy link

No description provided.

@iancward
Copy link

iancward commented Aug 11, 2016

I'm running into issues using this with the slack adapter (when using HUBOT_HELP_REPLY_IN_PRIVATE); I think this might be a similar issue.
Instead of using robot.send here: https://github.com/hubot-scripts/hubot-help/blob/master/src/help.coffee#L76, it would probably be better to use robot.messageRoom:

robot.messageRoom msg.message.user.id, emit

@rogerpfaff
Copy link

The way it works now is not compatible to RocketChat because the RoomId will be the same as the users name and so this is not working.

I can see that there is the userID we need in robot.adapter.asteroid.userId which is part of the private chat you can open with hubot.

@iancward
Copy link

iancward commented Aug 12, 2016

@rogerpfaff when replying directly to the user (e.g. when HUBOT_HELP_REPLY_IN_PRIVATE is set), the code uses user.name right now.

I'm using the Slack adapter, and I attempted to modify the code locally on my machine to use robot.messageRoom msg.message.user.id, emit, but I ran into this issue:slackapi/hubot-slack#332

What I have now is this, which works:

    if replyInPrivate and msg.message?.user?.name?
      msg.reply 'replied to you in private!'
      # robot.messageRoom(msg.message.user.id, emit)
      room = robot.adapter.client.rtm.dataStore.getDMByName msg.message.user.name
      robot.messageRoom room.id, emit
    else
      msg.reply emit

@iancward
Copy link

iancward commented Sep 9, 2016

@rogerpfaff so you mean that user.id would not work for RocketChat? Or that the current user.name doesn't work and neither will user.id?

With the slack adapter, msg.message.user.id is the ID of the user, which is the same ID as the DM channel to the user.

@TobiTenno
Copy link
Author

This is the same pattern Discord uses, and It would be a giant help if we could use it there.

@rogerpfaff
Copy link

The DM Channel to the user seems to be both userID. I just took a look and I can see:

RocketChatDriver.asteroid.userId which seems to be the Hubot userId (ABC) and my own userId (XYZ) in msg.message.TextMessage.User.id. When I start a private chat with the bot the user IDs are glued together. First the bot then the user ABCXYZ.

@TobiTenno
Copy link
Author

that's interesting.... that should probably be mapped in your adapter, though...

@rogerpfaff
Copy link

rogerpfaff commented Sep 9, 2016

so robot.adapter.client.rtm.dataStore.getDMByName

should return the exact combination needed.

Am 09.09.2016 7:53 nachm. schrieb "Matej Voboril" <[email protected]

:

that's interesting.... that should probably be mapped in your adapter,
though...

@TobiTenno
Copy link
Author

I think that's an adapter-specific thing that shouldn't necessarily affect global behavior.
In this, I'm more referring to the fact that "name" isn't necessarily a unique identifier, as "id" is, and if the option was there to use it, it would be beneficial.

@jonyeezs
Copy link

jonyeezs commented Oct 5, 2016

Doesn't work for hipchat as well. though i have no idea what it should be using

@rogerpfaff
Copy link

After researching a bit more I found that we live in a little hell and nobody streamlines the handling of private messaging. In this issue RocketChat/hubot-rocketchat#174 I filed a report against the rocketchat adapter because it seems to be their part to keep compatible with the scripts close to hubot.

The slack adapter manages the DM stuff in the send method slackapi/hubot-slack@1156fd6

Rocketchat has implemented a custom sendDirect message and HipChat seems to be based on Jabber and tries - also inside the send() method to decide if this is a public room or a dm room. https://github.com/hipchat/hubot-hipchat/blob/master/src/hipchat.coffee#L26

@TobiTenno
Copy link
Author

Hubot-discord also has logic for handling if the room is a private or open channel. I think it makes more sense to handle what the channel is on the adapter side and to send whatever works best from here.

@TobiTenno
Copy link
Author

I've made a PR, #20, covering this issue.

@reduckted
Copy link

Would love to get that PR from @aliasfalse merged. Replying in private doesn't appear to work with HipChat either, and I'm pretty sure that PR would fix it.

@TobiTenno
Copy link
Author

(I'm trying to figure out who I should bug to get more eyes on it and get it moving)

@jonyeezs
Copy link

I tested it on a hipchat adapter.

Didn't work when i set private = "true" and id = "true"

as well didn't work for private = "true" and no id env var.

@reduckted
Copy link

Yeah, you're right. I was actually just playing with this earlier today and came to the same conclusion. I worked out that msg.message.user.jid needs to be used instead, and when you send the message, you need to use user instead of room.

robot.send { user: msg.message.user.jid }, emit

It would be nice if there was a bit more standardization between adapters :(

@jonyeezs
Copy link

@thirdwaffle i hear you!

@stale stale bot added the stale label Jun 1, 2017
@stale
Copy link

stale bot commented Jun 1, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@reduckted
Copy link

Pretty sure PR #20 would fix this (or at least fix an issue that the HipChat adapter has - see my earlier comments).

@stale
Copy link

stale bot commented Sep 2, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@reduckted
Copy link

Pretty sure PR #20 would fix this (or at least fix an issue that the HipChat adapter has - see my earlier comments).

@stale stale bot removed the stale label Sep 2, 2017
@stale
Copy link

stale bot commented Dec 1, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 1, 2017
@stale stale bot closed this as completed Dec 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants