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

Expose private File Upload V2 methods to support multiple file uploads in a single message #1376

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

YutoKashiwagi
Copy link
Contributor

@YutoKashiwagi YutoKashiwagi commented Feb 19, 2025

Hi, thank you for maintaining slack-go!

Currently, UploadFileV2Context works by sequentially calling three private methods in the following flow:

  1. getUploadURLExternal (retrieves the upload URL)
  2. uploadToURL (uploads the file to that URL)
  3. completeUploadExternal (finalizes the upload)

Because these methods are private, developers can only upload one file per message via UploadFileV2Context. However, the Slack API itself supports uploading multiple files in a single message by running these steps for each file and then calling completeUploadURLExternal for all file IDs together.

To unlock this functionality while still using slack-go, I’ve made the following changes in this PR:

  • Made getUploadURLExternal and completeUploadExternal public
    • This allows developers to orchestrate multiple file uploads (Steps 1–3 repeated for each file) and finalize them all in a single message.
  • uploadToURL remains private for now. This method is simply a regular POST request handler, not part of Slack’s official API. Since making getUploadURLExternal and completeUploadExternal public already addresses the main use case of uploading multiple files, this might be sufficient.
    • However, if you feel uploadToURL should also be made public for consistency, please let me know and I can include that change in this PR.

Thank you for considering this proposal. Being able to handle multi-file uploads in a single Slack message through slack-go would be extremely helpful. If you have any suggestions or an alternative approach, I’d love to hear them!

Related Issues

Pull Request Guidelines

These are recommendations for pull requests.
They are strictly guidelines to help manage expectations.

PR preparation

Run make pr-prep from the root of the repository to run formatting, linting and tests.

Should this be an issue instead
  • is it a convenience method? (no new functionality, streamlines some use case)
  • exposes a previously private type, const, method, etc.
  • is it application specific (caching, retry logic, rate limiting, etc)
  • is it performance related.
API changes

Since API changes have to be maintained they undergo a more detailed review and are more likely to require changes.

  • no tests, if you're adding to the API include at least a single test of the happy case.
  • If you can accomplish your goal without changing the API, then do so.
  • dependency changes. updates are okay. adding/removing need justification.
Examples of API changes that do not meet guidelines:
  • in library cache for users. caches are use case specific.
  • Convenience methods for Sending Messages, update, post, ephemeral, etc. consider opening an issue instead.

Comment on lines +288 to +302
type mockGetUploadURLExternalHttpClient struct {
ResponseStatus int
ResponseBody []byte
}

func (m *mockGetUploadURLExternalHttpClient) Do(req *http.Request) (*http.Response, error) {
if req.URL.Path != "files.getUploadURLExternal" {
return nil, fmt.Errorf("invalid path: %s", req.URL.Path)
}

return &http.Response{
StatusCode: m.ResponseStatus,
Body: io.NopCloser(bytes.NewBuffer(m.ResponseBody)),
}, nil
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered following the existing once.Do(startServer) approach for testing, but found that simply mocking Client.httpclient would be sufficient. Therefore, I decided to mock it instead.

Comment on lines +627 to +630
Files: []FileSummary{{
ID: u.FileID,
Title: params.Title,
}},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For CompleteUploadURLExternal, I've not only made it public but also updated its signature so that multiple fileID parameters (with IDs and titles) can be specified, aligning it more closely with Slack’s official API.

@sheldonhull
Copy link

thanks for doing this. I just was revisiting a way to grab the urls so I could publish a single "gallery style" message and this probably will fix much of that by providing back the upload urls back. cheers 🙇

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.

2 participants