Slack Button Fix and Threaded Messaging #6020
Conversation
…nto slack-enhancements
|
oh i didn't mean to rush you i was just checking whether ella had checked with you, thank you very much! |
Co-authored-by: Christina Koß <c.koss@rasa.com>
Co-authored-by: Christina Koß <c.koss@rasa.com>
Co-authored-by: Christina Koß <c.koss@rasa.com>
|
@b-quachtran let me know when you're ready for me to take another look at this |
…nto slack-enhancements
|
@akelad Yep, just finished pushing some changes if you can take a look. |
Co-authored-by: Christina Koß <c.koss@rasa.com>
|
looks good to me, Christina already approved so you don't need a second approval. But some build stages seem to be failing.. once those are fixed feel free to add the ready to merge label @b-quachtran |
| @@ -297,13 +316,18 @@ async def process_message( | |||
|
|
|||
| if metadata is not None: | |||
| output_channel = metadata.get("out_channel") | |||
| if self.use_threads: | |||
| thread_id = metadata.get("ts") | |||
There was a problem hiding this comment.
@b-quachtran This line confuses me - shouldn't it be metadata.get("thread_id") now? If I'm not mistaken, can you create a PR with a quick fix?
There was a problem hiding this comment.
And then we should probably also extend the tests to include a case that would have caught this (e.g. a test where we get a message event, post a reply to the same thread, and assert that everything went as expected).
If you have time, this could go into the PR with the fix, otherwise let's just create an issue to add such a test.
Proposed changes:
Status (please check what you already did):
black(please check Readme for instructions)