slack-github-action icon indicating copy to clipboard operation
slack-github-action copied to clipboard

Updating threads in multiple channels doesn't work correctly

Open dbasilio opened this issue 2 years ago • 7 comments

Description

#118 introduced the ability to post a message to multiple channels, but we're having issues with updating the threads in multiple channels afterwards. When calling the action you can put multiple channel IDs and and those get posted to correctly. However, the output from the action does not include the channel and thread id for each channel, only the last one to respond.

I'm happy to fix this bug, but want to confirm what the resulting output should be since the change is likely going to be breaking.

What type of issue is this? (place an x in one of the [ ])

  • [x] bug
  • [x] enhancement (feature request)
  • [ ] question
  • [ ] documentation related
  • [ ] example code related
  • [ ] testing related
  • [ ] discussion

Requirements (place an x in each of the [ ])

  • [x] I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • [x] I've read and agree to the Code of Conduct.
  • [x] I've searched for any related issues and avoided creating a duplicate issue.

Bug Report

The issue stems from here it's always overwriting the webResponse variable, so then when you get to the output section it only holds a reference to a single channel and thread id instead of all of them.

In order to fix this, you may need to change the shape of the output (which would be a breaking change). I'd suggest the following structure:

{
    "threads": [
        { "ts": "...", "thread_ts": "...", "channel_id": "..." },
        { "ts": "...", "thread_ts": "...", "channel_id": "..." }
    ]
}

An alternative would be to return a comma-separate list in each of ts, thread_ts, and channel_id, but I think it's safer to do an array to make sure the right thread_ts and channel_id are kept together.

Steps to reproduce:

  1. Call the action with multiple channels
  2. Store the output from the action in a workflow for use later
  3. View the output and see that only a single channel was output.

Expected result:

The output from the action includes the channel and thread ids for all the channels messaged.

Actual result:

The output from the action includes whichever channel/thread was the last to resolve from the API.

Attachments:

Logs showing we called the action with multiple channel IDs image

Log showing the output from the output (stored in an env_var METADATA_JSON) image

dbasilio avatar Jul 21 '23 20:07 dbasilio

Actually as I look at the code more to properly support updating multiple channels the inputs would likely need to change. Right now input-ts is not split, so you can only input a single input-ts value, but if you are trying to update threads in multiple channels that would not work :\

dbasilio avatar Jul 21 '23 20:07 dbasilio

Hi, thanks for writing in. To my understanding, the multi-channel posting feature was added to simply support posting a single message across multiple channels. I don't believe it was designed to spread the same thread discussion across several channel threads, and that remains true today.

As one of the maintainers of this tool, I hesitate to add such complex functionality to the project at this time. However, if many other users express the same need or if other maintainers feel strongly that we should support this use case, I would be open to enhancing it.

In the meantime, if you need this feature immediately, please consider forking this project or starting your own. I'd appreciate it if you could understand this.

seratch avatar Jul 22 '23 00:07 seratch

@seratch Of course! Thanks for the quick reply! It does feel very much like a full breaking change to both inputs and outputs so taking time to consider those effects is well warranted. I think for our use case we're going to move to calling the action multiple times, once for each channel we want to keep up to date. I will keep an eye on the discussion though!

dbasilio avatar Jul 24 '23 14:07 dbasilio

Hi @seratch ,

I wanted to use this feature to trigger e2e tests post new deployments and update team on a team channel and individuals using the bot.

Since e2e tests take some time to finish, I wanted to start by posting a message that tests has started and then later update the same message on both channel and to the individual.

The reason, I want both of these to be updated is, if we only send replies to individuals, team does not know if a person misses it. If we only post it on channel, then it is expected that most people will not watch it since it gets noisy after a while.

So the above use case was a perfect solution (if it could work).

PramodKumarYadav avatar Jul 10 '24 16:07 PramodKumarYadav

@PramodKumarYadav I'm wondering if a matrix with different channel IDs might serve as a workaround for posting and updating the same message at different points in a workflow? 🤔

I plan to explore this a bit soon since the current progress of @v2 doesn't support multiple channel IDs in this input to mirror chat.postMessage, but this seems like a useful case to support at an action level!

zimeg avatar Jul 10 '24 19:07 zimeg

Thanks for the suggestion @zimeg.

In my use case, I am trying to send a message to the github actor whose last commit (and merge to main) triggered the deployment.

I am dynamically getting this value from github context and storing a map of actor name with their slack-ids in github secrets. This is how, I am passing both the "fixed" channel name and "dynamic" slack id of actor.

channel-id: 'A12CBNAS307,${{ secrets[github.event.client_payload.github.actor] }}'

Since matrix strategy expects hard coded values and I can only know about the triggering person at the time of running tests, unfortunately in this use case, I am not able to solve this using matrix strategy.

If you can think or suggest any workarounds, I will be happy to hear.

PramodKumarYadav avatar Jul 10 '24 21:07 PramodKumarYadav

@PramodKumarYadav Woah TIL that matrix values must be hardcoded! It makes sense, but that stumped me a bit...

I know this might not be ideal, even a bit repetitive, but would duplicating the slack-send step for each of the messages being sent work? This should let these steps use a separate id, each with output related to the message that was sent to just the one channel, in following steps? :eyes:

zimeg avatar Aug 28 '24 05:08 zimeg

👋 Hello again! We've just released version @v2.0.0 and I think duplicating the step that sends the payload is a decent approach at the moment for keeping track of individual thread timestamps, so will close this issue with a workaround.

The following steps can be used for the initial message and thread, then these could be repasted as much as you'd like with changing IDs:

- name: Initiate a deployment
  uses: slackapi/[email protected]
  id: deployment_message_1
  with:
    method: chat.postMessage
    token: ${{ secrets.SLACK_BOT_TOKEN }}
    payload: |
      channel: ${{ secrets.SLACK_CHANNEL_ID }}
      text: "Deployment started :eyes:"

# Include duplicates of the shown steps and additional deployment steps here!

- name: Conclude the deployment
  if: ${{ steps.deployment_message_1.outputs.ok }}
  uses: slackapi/[email protected]
  with:
    method: chat.postMessage
    token: ${{ secrets.SLACK_BOT_TOKEN }}
    payload: |
      channel: "${{ steps.deployment_message_1.outputs.channel_id }}"
      thread_ts: "${{ steps.deployment_message_1.outputs.ts }}"
      text: "Deployment finished! :rocket:"

The channel ID can be a conversation of all kind, and user IDs can be used to send DMs from the bot. More info on collecting a user ID in an action might be found in #349 too 🙏

Please note: Sending a message to multiple channels with one step is no longer supported with this update as well!

Feel free to comment if this workaround isn't working well with some of these changes or if you have other suggetions. Also, opening a new issue with other questions and ideas is always encouraged! 💡

zimeg avatar Nov 16 '24 04:11 zimeg