feat(message): support multiple channel IDs #126
Conversation
|
|
README.md
Outdated
| @@ -124,6 +124,22 @@ Using JSON payload for constructing a message is also available: | |||
| 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.
There was a problem hiding this comment.
Yep, good suggestion @filmaj! ➕
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
| @@ -124,6 +124,22 @@ Using JSON payload for constructing a message is also available: | |||
| 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
| @@ -56,16 +56,19 @@ describe('slack-send', () => { | |||
| describe('happy path', () => { | |||
| 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.
README.md
Outdated
| @@ -124,6 +124,22 @@ Using JSON payload for constructing a message is also available: | |||
| 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.
Yep, good suggestion @filmaj! ➕
|
@filmaj Thanks for the suggestions! They've been implemented and pushed. |
|
@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[ ])