Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating threads in multiple channels doesn't work correctly #229

Open
5 of 10 tasks
dbasilio opened this issue Jul 21, 2023 · 7 comments
Open
5 of 10 tasks

Updating threads in multiple channels doesn't work correctly #229

dbasilio opened this issue Jul 21, 2023 · 7 comments

Comments

@dbasilio
Copy link

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 [ ])

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

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

  • 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 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
Copy link
Author

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 :\

@seratch
Copy link
Member

seratch commented Jul 22, 2023

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.

@dbasilio
Copy link
Author

@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!

@PramodKumarYadav
Copy link

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).

@zimeg
Copy link
Member

zimeg commented Jul 10, 2024

@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!

@PramodKumarYadav
Copy link

PramodKumarYadav commented Jul 10, 2024

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.

@zimeg
Copy link
Member

zimeg commented Aug 28, 2024

@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? 👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants