Skip to content

Enable file uploads inside threads #188

Merged
damienalexandre merged 6 commits intojolicode:mainfrom
TheIrritainer:main
Mar 10, 2026
Merged

Enable file uploads inside threads #188
damienalexandre merged 6 commits intojolicode:mainfrom
TheIrritainer:main

Conversation

@TheIrritainer
Copy link
Copy Markdown
Contributor

This PR will allow file uploads inside threads, useful when you have a bot in a thread returning a file.
see slack documentation: https://docs.slack.dev/reference/methods/files.completeUploadExternal/#:~:text=Example%3A%20C0NF841BK-,thread_ts,-string

Tested on a dev slack app

image

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for uploading files into a Slack thread by extending the Client::filesUploadV2() helper to accept an optional thread_ts, and wires this through in the integration test configuration.

Changes:

  • Extend Client::filesUploadV2() with an optional $threadTs parameter and pass it as thread_ts to filesCompleteUploadExternal.
  • Update the existing external upload integration test to pass a thread timestamp.
  • Document new Slack test env var(s) in phpunit.xml.dist.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/Client.php Adds optional thread_ts support to filesUploadV2() and updates its docblock.
tests/WritingTest.php Uses SLACK_TEST_THREAD_TS and passes it to filesUploadV2() for thread uploads.
phpunit.xml.dist Documents SLACK_TEST_THREAD_TS and clarifies SLACK_TEST_CHANNEL expectations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@damienalexandre
Copy link
Copy Markdown
Member

Thanks for this contribution, looks good. Copilot made some good suggestion as well.

Looking futher into this subject, it appear that more than one parameter is missing.

The official Python implementation has a lot:

https://github.com/slackapi/python-slack-sdk/blob/72e915a7d9b2e5c15586abd3062369bb4825586b/slack_sdk/web/client.py#L3994-L4012

Whould you have the ressources and time to propose something or should we open a new issue and just to the thread thing now?

@TheIrritainer
Copy link
Copy Markdown
Contributor Author

Thanks for this contribution, looks good. Copilot made some good suggestion as well.

Looking futher into this subject, it appear that more than one parameter is missing.

The official Python implementation has a lot:

https://github.com/slackapi/python-slack-sdk/blob/72e915a7d9b2e5c15586abd3062369bb4825586b/slack_sdk/web/client.py#L3994-L4012

Whould you have the ressources and time to propose something or should we open a new issue and just to the thread thing now?

The suggestions of copilot are good indeed, will incorporate.
Currently no resources unfortunately; I've been using my fork for quite some time but now i'm nearing a release of an open source app I'd like to get rid of the fork.

@damienalexandre
Copy link
Copy Markdown
Member

Thanks a lot for pushing your changes upstream 🙏

@damienalexandre damienalexandre merged commit 1bf31dc into jolicode:main Mar 10, 2026
@damienalexandre
Copy link
Copy Markdown
Member

Version v4.9.0 has been released with your change, thanks again 👍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants