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

Slack direct messages not working #369

Open
pastorhudson opened this issue Jul 21, 2018 · 14 comments
Open

Slack direct messages not working #369

pastorhudson opened this issue Jul 21, 2018 · 14 comments

Comments

@pastorhudson
Copy link
Contributor

It seems sending to a user ID in slack doesn't work like I used to?
If I try:
self.say("Test message", channel = self.message.data.sender.id)
I get back:
Fri, 20 Jul 2018 23:43:29 [ERROR] I was asked to post to the slack U08FPSYTW channel, but it doesn't exist.
When I send a direct message to the bot and:
print(self.message.data.channel.id)
It prints DBH45HA59. I can't figure out how to get DBH45HA59 from the user U08FPSYTW.
@Ashex Any ideas on how to send DM through the slack backend?

@pastorhudson
Copy link
Contributor Author

If I look in
self.load('slack_channel_cache', {})
This is what's stored for DM channels.

This is the key: DBH45HA59
This is the content: {'id': 'DBH45HA59', 'name': 'DBH45HA59', 'source': {'id': 'DBH45HA59', 'members': [], 'name': 'DBH45HA59'}, 'members': {}}

If someone could point me toward where this is built I could perhaps figure out how to get name to be stored as U08FPSYTW

@Ashex
Copy link
Collaborator

Ashex commented Jul 23, 2018

You want to pass the message content back with say so that Will knows it's a private message and handles it appropriately.

self.say("Test message", message=self.message)

@pastorhudson
Copy link
Contributor Author

@Ashex Thanks! Does this work for initiating dm's too? The event comes in in the #bot channel and I want to then send a dm to the person who sent the message to the bot channel. I'll give it a try!

@pastorhudson
Copy link
Contributor Author

It didn't work.
will.say('"%s" channel added to the whitelist.' % will.message.data.channel.name.title(), message=will.message)
Sends to the same channel the command was recieved from. Unless I'm missing something?

@Ashex
Copy link
Collaborator

Ashex commented Jul 23, 2018

Ah so you want to take a message from a channel and act on it by sending a direct message, that's a bit different.

You're sending a message directly to a user so you want to actually look inside slack_people_cache to get their user ID for sending messages (unless it's a group chat).

I haven't actually done what you're trying to accomplish but it's on my list of enhancements to make. The message should already include their user ID so you either want to override portions of the message dictionary in the response or add a new function that will send direct messages.

@pastorhudson
Copy link
Contributor Author

pastorhudson commented Jul 23, 2018

Right! So I tried that and I get a user ID 'U08FPSYTW' and I try to send a message to that user. channel='U08FPSYTW' and I get Mon, 23 Jul 2018 08:20:56 [ERROR] I was asked to post to the slack U08FPSYTW channel, but it doesn't exist. When I look at the slack api it seems the DM channel has it's own name seperate from the user name. I can't find that DM channel ID anywhere in slack_people_cache. The way to get it is to make an api call to "im.open", user="U08FPSYTW"
This returns something like this:

{
    "ok": true,
    "no_op": true,
    "already_open": true,
    "channel": {
        "id": "D19U4244U"
    }
}

So where should we add this call? Is this something we should cache in slack_people.cache? Or is a better way to have channel try an im.open when it can't find a channel?

@pastorhudson
Copy link
Contributor Author

pastorhudson commented Jul 23, 2018

I found a fix that works for me. I can do a PR if the solution is acceptable.
I added:

def get_im(self, user_id):
        return self.client.api_call(
            "im.open",
            user=user_id)['channel']['id']

And added "channel" to Person:

        for k, v in self.client.server.users.items():
            user_timezone = None
            if v.tz:
                user_timezone = v.tz
            people[k] = Person(
                id=v.id,
                mention_handle="<@%s>" % v.id,
                handle=v.name,
                source=clean_for_pickling(v),
                name=v.real_name,
                channel=self.get_im(v.id)
            )
            if v.name == self.handle:
                self.me = Person(
                    id=v.id,
                    mention_handle="<@%s>" % v.id,
                    handle=v.name,
                    source=clean_for_pickling(v),
                    name=v.real_name,
                    channel=self.get_im(v.id)
                )

I called it channel because that's the first thing I tried. You can now DM anybody using:
say("Some DM message", channel=self.message.data.sender.channel)
Channel is also populated with the DM channel in slack_people_cache.

Is this an acceptable solution?

@pastorhudson
Copy link
Contributor Author

Another option would be to put it in the say function where it would catch a slack user ID inputted as a channel and convert it to the IM channel. But I like having all the dm channels in slack_people_cache

@Ashex
Copy link
Collaborator

Ashex commented Jul 24, 2018

I'm just a contributor so it's ultimately up to @skoczen how this should be integrated. I'm a bit worried about storing the DM channels in the cache as this can become resource intensive on large slack workspaces and potentially cause startup delays while it makes the im.open call for each user as it populates the cache. We can reduce the impact with a condition where it only adds/updates where the channel key is missing.

I like the second option better, perhaps adding an argument to the say function that would explicitly call it a direct user message but the user ID always starts with U so that's probably superfluous.

Edit That argument may actually be necessary since say is used for other services besides slack and we don't know how the user ids are constructed. It'd be best to simply have an argument that is added to the event kwargs dictionary so that it is processed appropriately within handle_outgoing_event inside the slack io_adapter.

Another Edit Forget about the argument and just do a pattern match against the channel and update the value with the direct channel.

@pastorhudson
Copy link
Contributor Author

That makes a lot of sense. I'll work it up and make a pull request. @skoczen can take a look at the pr.

@pastorhudson
Copy link
Contributor Author

Submitted solution here #375

@pastorhudson
Copy link
Contributor Author

I'm not sure if this is needed anymore. I'm merging in the new slack update and I'll report back.

@Ashex
Copy link
Collaborator

Ashex commented Mar 19, 2021

I had to do some extra condition checks in the slack backend for channel lookups and also set is_direct=True so it would know how to process the channel lookup. With the slack update it can reply to DMs but unsolicited (i.e responding to a message in a channel by sending a DM) doesn't work for some reason even though logging shows a success so I was going to dive deeper as I did sort it out in my code base about 6 months ago.

I opened #429 for that specifically so if you sort it with your PR we can close that out.

Edit: My issue was specifically with file uploads but I believe there's overlap.

@pastorhudson
Copy link
Contributor Author

I just got contract for another project that will be consuming cycles for a couple weeks. So if you have the time go for it!

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

3 participants