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

feat: Support thread_ts parameter to post the new message as a thread #303

Closed

Conversation

ArnaudRinquin
Copy link

Closes #187

Summary

Describe the goal of this PR. Mention any related Issue numbers.

Requirements (place an x in each [ ])

@WilliamBergamin
Copy link
Contributor

Thanks for the contribution @ArnaudRinquin 💯

Before I move forward with this would you mind testing out what happens if the thread_ts field is passed in the payload?

Based on the behavior from this line it seems like the spread operator should support thread_ts being placed in the payload argument

- name: Send GitHub trigger payload to Slack Workflow Builder
  id: slack
  uses: slackapi/[email protected]
  with:
    channel-id: 'SLACK_CHANNEL_ID' # ID of Slack Channel you want to post to
    payload: |
      {
        "blocks": [
          { "type": "divider" },
          {
            "type": "image",
            "title": {
              "type": "plain_text",
              "text": "Slack Slack Slack",
              "emoji": true
            },
            "image_url": "https://media.makeameme.org/created/a-slack-this.jpg",
            "alt_text": "marg"
          },
          {
            "type": "actions",
            "elements": [
              {
                "type": "button",
                "text": {
                  "type": "plain_text",
                  "text": "${{ github.sha }}"
                },
                "url": "https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
              }
            ]
          }
        ],
        "thread_ts": "TIMESTAMP_OF_A_MESSAGE"
      }
  env:
    SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }}

@ArnaudRinquin
Copy link
Author

@WilliamBergamin I've added 2 test cases ensuring payload provided thread_ts is still used + overrides input passed thread-ts (I'm not sure which should be used in priority but seems better this way).

@WilliamBergamin
Copy link
Contributor

@ArnaudRinquin these are awesome changes 💯 but implementing the simplest solution for this problem should create a more maintainable project

It would be awesome if this PR could include the unit tests that ensures payload provided thread_ts is valid and documentation on how to use payload provided thread_ts. With this we will be able to update and close #187

@ArnaudRinquin
Copy link
Author

@WilliamBergamin

ensures payload provided thread_ts is valid

I'd be happy to validate the thread_ts value but I can't find any spec on the expected shape. Even in official doc the type looks like it can be a simple string. I'll had more tests if you can provide me some hints on how to validate.

CleanShot 2024-04-26 at 18 16 14@2x

I've updated README.md example of specifying thread_ts from payload.

@fleytman
Copy link

fleytman commented May 24, 2024

@WilliamBergamin, what is blocking this pr at the moment?

@WilliamBergamin
Copy link
Contributor

Closing in favor of #309

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

Successfully merging this pull request may close these issues.

Support thread_ts for threaded updates to a single topic
3 participants