Skip to content

Add Support For files.getUploadURLExternal & files.completeUploadExternal #182

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

Merged
merged 8 commits into from
Apr 1, 2025

Conversation

pmoorani
Copy link
Contributor

@pmoorani pmoorani commented Mar 30, 2025

Slack deprecated https://api.slack.com/methods/files.upload and introduced a new flow for file uploads, which comprises of the following steps:

  1. https://api.slack.com/methods/files.getUploadURLExternal
  2. https://api.slack.com/methods/files.completeUploadExternal

Fixes #168

@pmoorani pmoorani marked this pull request as ready for review March 31, 2025 00:01
Copy link
Member

@damienalexandre damienalexandre left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this implementation and the new helper function in the client ! I just have one question.

src/Client.php Outdated
);
}

private function uploadToSlackUrlWithCurl(string $uploadUrl, string $filePath): void
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to use the http client user in the actual api client (Jane).

Would that be possible?

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 tried it out but couldn't make it work. Ended up moving from cURL and using GuzzleClient instead. Let me know if this is not ok for you.

I got following error with generated jane api client:

1) JoliCode\Slack\Tests\WritingTest::testItCanUploadFilesViaFilesUploadV2
Http\Client\Common\Exception\ClientErrorException: Not Found

/Users/john.doe/repositories/slack-php-api/vendor/php-http/client-common/src/Plugin/ErrorPlugin.php:80
/Users/john.doe/repositories/slack-php-api/vendor/php-http/client-common/src/Plugin/ErrorPlugin.php:62
/Users/john.doe/repositories/slack-php-api/vendor/php-http/httplug/src/Promise/HttpFulfilledPromise.php:28
/Users/john.doe/repositories/slack-php-api/vendor/php-http/client-common/src/Plugin/ErrorPlugin.php:63
/Users/john.doe/repositories/slack-php-api/vendor/php-http/client-common/src/PluginChain.php:44
/Users/john.doe/repositories/slack-php-api/vendor/php-http/client-common/src/PluginChain.php:59
/Users/john.doe/repositories/slack-php-api/vendor/php-http/client-common/src/PluginClient.php:84
/Users/john.doe/repositories/slack-php-api/src/Client.php:193
/Users/john.doe/repositories/slack-php-api/src/Client.php:151
/Users/john.doe/repositories/slack-php-api/tests/WritingTest.php:264

Copy link
Member

Choose a reason for hiding this comment

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

That was discussed here too: #176

I like what you did yes 👍 much more closer to the actual API client that way. I will suggest a modification tho.

src/Client.php Outdated
Comment on lines 186 to 187
$client = Psr18ClientDiscovery::find();
$response = $client->sendRequest($request);
Copy link
Member

Choose a reason for hiding this comment

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

We must use the Client provided by the developer, so I think this should be:

Suggested change
$client = Psr18ClientDiscovery::find();
$response = $client->sendRequest($request);
$response = $this->httpClient->sendRequest($request);

The HttpClient is from here:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's exactly what I did, it doesn't work at least when I run my test :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@damienalexandre it doesn't work because generated client has base uri set as slack.com and what getUploadUrlExternal returns is a different url i.e., https://files.slack.com/upload/v1/CwABAAAAYwoAAdZbBYEretOHCgACGDH-HcmojZAMAAMLAAEAAAALRTA3UEI5R1QzTU0LAAIAAAALVTA4S1YyNjkwOEsLAAMAAAALRjA4S1kyQVRFUTYACgAEAAAAAAAAAJoIAAcAAAAEAAsAAgAAABTBhf2hsqHZcysRfVuQ1zXp61dVZAA and that's why I am getting 404s

Copy link
Member

Choose a reason for hiding this comment

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

Ok I see, I just tested your branch, and found a way to keep the "files.slack.com" URI when using the internal HTTP Client. I just pushed a new commit in your PR to add this. Is it ok for you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, looks good! 👍

@pmoorani pmoorani requested a review from damienalexandre April 1, 2025 08:42
@damienalexandre damienalexandre merged commit d863b8a into jolicode:main Apr 1, 2025
6 checks passed
@damienalexandre
Copy link
Member

Thanks a lot! This was an highly requested feature! 👏

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.

Method files.upload is no longer supported by Slack for new Applications
2 participants