feat(message): support multiple channel IDs#126
feat(message): support multiple channel IDs#126filmaj merged 6 commits intoslackapi:mainfrom treemmett:multiple-channels
Conversation
|
|
README.md
Outdated
| SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} | ||
| ``` | ||
|
|
||
| Multiple channels can be sent the same message by providing a comma-separated list of ID's: |
There was a problem hiding this comment.
Any thoughts on how to reword this?
There was a problem hiding this comment.
I think I would actually remove this additional section and just add a comment in the specific "Usage" sections above where applicable. E.g. line 90-92 above I would add an extra line of comment that is like:
# You can pass in multiple channels to post to by providing a comma-delimited list of channel IDs.
filmaj
left a comment
There was a problem hiding this comment.
Thanks a lot for the PR @treemmett !
I left a couple of comments, one suggestion on how to integrate the new feature you are adding support for in the README and another one regarding test organization.
Going to approve the github action execution - feel free to at-mention me directly if you'd like for me to take another look!
README.md
Outdated
| SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} | ||
| ``` | ||
|
|
||
| Multiple channels can be sent the same message by providing a comma-separated list of ID's: |
There was a problem hiding this comment.
I think I would actually remove this additional section and just add a comment in the specific "Usage" sections above where applicable. E.g. line 90-92 above I would add an extra line of comment that is like:
# You can pass in multiple channels to post to by providing a comma-delimited list of channel IDs.
| // update message | ||
| webResponse = await web.chat.update({ ts, channel: channelId, text: message, ...(payload || {}) }); | ||
| } else { | ||
| webResponse = await web.chat.update({ ts, channel: channelId.trim(), text: message, ...(payload || {}) }); |
There was a problem hiding this comment.
Nice, good call to trim it here.
test/slack-send-test.js
Outdated
| it('should send a message using the postMessage API', async () => { | ||
| fakeCore.getInput.withArgs('slack-message').returns('who let the dogs out?'); | ||
| fakeCore.getInput.withArgs('channel-id').returns('C123456'); | ||
| fakeCore.getInput.withArgs('channel-id').returns('C123456,C987654'); |
There was a problem hiding this comment.
Thanks for adding the test! Could we actually split this test case up into two test cases? Ideally I'd like separate tests for the single-channel and multi-channel scenarios.
mwbrooks
left a comment
There was a problem hiding this comment.
Beauty, thanks for the contribution @treemmett!
|
@filmaj Thanks for the suggestions! They've been implemented and pushed. |
filmaj
left a comment
There was a problem hiding this comment.
Awesome! Thanks a ton for putting this together. Gonna let the tests run then will merge in!
|
@treemmett Thanks for implementing this feature. We're trying to update the messages sent to multiple channels and we get |
Summary
Adds support for comma-separated channel ID's.
Closes #118
Requirements (place an
xin each[ ])