slack-ruby-client: translation from channel name to ID is prone to failure

When a channel name is given for a channel parameter, rather than a channel ID, we are using channels.list to look up the ID of the channel and then using the ID in the request instead of the name.

However, channels.list is a legacy method and is no longer guaranteed to return every channel. This leads to a channel_not_found error if you pass a channel name for a channel that is not supported by channels.list.

A couple suggested solutions:

  1. Update channels_id to use conversations.list instead of channels.list. The tricky thing about this is that conversations.list only works with pagination, so it’s not a simple drop-in replacement. It also means that if you do something like chat_postMessage(channel: 'general', text: 'foo') then for a team with many channels you might end up making hundreds of paginated calls to conversations.list before actually hitting chat.postMessage, which is not ideal. If we use this solution I think we should change the readme to discourage this approach and encourage using channel IDs directly.
  2. Remove the translation completely, which would be a breaking change (but not the worst breaking change, since it’s already basically broken). Since using conversations.list is not ideal for making the translation (see above) we could remove support for this altogether, forcing people to confront the issue directly.
  3. Leave it as it is, and update the readme to note that it does not always work and discourage its use.

See #270 for more background.

About this issue

  • Original URL
  • State: open
  • Created 5 years ago
  • Reactions: 1
  • Comments: 15 (2 by maintainers)

Most upvoted comments

in all fairness, chat_postMessage does work given a channel name, but omitting the leading hash mark. in this case no automatic mapping occurs, and Slack does find the channel by name. however, it doesn’t do the same for all the other chat methods (e.g. chat_update). pretty weird inconsistency.

Well, maybe that channel list, once acquired, should be cached, so that the client wouldn’t reload it for every single chat operation. I kind of guess that following channel creations/deletions dynamically from a single client instance should be quite a rarely occuring requirement, in which case I’d suggest a method to invalidate/force redownload the channel list.

Personally, I feel that caching would be beyond the scope of the library. A client can do it if they want. Similarly I do see how doing the lookup could also be beyond the scope of the library 😉