Skip to content

Allow Channel.send_message to reply to a thread#162

Merged
Roach merged 1 commit intoslackapi:masterfrom
sd2k:master
Jan 23, 2017
Merged

Allow Channel.send_message to reply to a thread#162
Roach merged 1 commit intoslackapi:masterfrom
sd2k:master

Conversation

@sd2k
Copy link
Contributor

@sd2k sd2k commented Jan 21, 2017

  • 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 been mindful about doing atomic commits, adding documentation to my changes, not refactoring too much.
  • I've a descriptive title and added any useful information for the reviewer. Where appropriate, I've attached a screenshot and/or screencast (gif preferably).
  • I've written tests to cover the new code and functionality included in this PR.
  • I've read, agree to, and signed the Contributor License Agreement (CLA).

PR Summary

This PR adds two keyword arguments to slackclient._channel.Channel.send_message: thread and reply_broadcast. These allow a message sent using this method to be marked as a reply to a thread, and optionally to be posted back to the channel.

This makes it easier to meet Slack's Best Practices for bots (https://api.slack.com/docs/message-threading#best_practices) which states that they should reply to threaded messages in the same thread.

It also fixes the test for send_message by monkeypatching the server attribute, and adds an extra test for the new functionality.

Related Issues

None

Test strategy

e.g. Add tests around whatsit production.

@codecov-io
Copy link

codecov-io commented Jan 21, 2017

Current coverage is 64.16% (diff: 85.71%)

Merging #162 into master will decrease coverage by 1.23%

@@             master       #162   diff @@
==========================================
  Files             9          9          
  Lines           289        293     +4   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            189        188     -1   
- Misses          100        105     +5   
  Partials          0          0          

Powered by Codecov. Last update 4b7f996...ce1cb1b

@Roach
Copy link
Contributor

Roach commented Jan 23, 2017

@sd2k thanks Ben! would you mind updating the method reference in the docs real quick?

https://github.com/slackapi/python-slackclient/blob/master/docs-src/real_time_messaging.rst

Copy link
Contributor

@Roach Roach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!
Pending docs update

This makes it easier to meet Slack's Best Practices for bots
(https://api.slack.com/docs/message-threading#best_practices)
which states that they should reply to threaded messages in
the same thread.
@sd2k
Copy link
Contributor Author

sd2k commented Jan 23, 2017

Thanks! I've updated the docs and actually noticed that I should have made the same changes to SlackClient.rtm_send_message, so I've done that as well.

Copy link
Contributor

@Roach Roach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Roach Roach merged commit fb1fa7a into slackapi:master Jan 23, 2017
c-goosen pushed a commit to c-goosen/python-slackclient that referenced this pull request Jun 18, 2019
This makes it easier to meet Slack's Best Practices for bots
(https://api.slack.com/docs/message-threading#best_practices)
which states that they should reply to threaded messages in
the same thread.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants