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

Add support for files.getUploadURLExternal and files.completeUploadExternal #176

Open
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

maybe-Baylie
Copy link

This pull request introduces support for two new endpoints:

  1. files.getUploadURLExternal docs
  2. files.completeUploadExternal docs

Additionally, A few errors were corrected in the command classes to allow the tools to run

@maybe-Baylie
Copy link
Author

I can add the string literal to class string conversions from Jane as well if desired, will add 448 file changes in generated/normalizer/.

@@ -16828,6 +16828,130 @@
]
}
},
"/files.completeUploadExternal": {
Copy link
Member

Choose a reason for hiding this comment

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

The proper way to add stuffs in this file is described here: https://github.com/jolicode/slack-php-api/blob/main/docs/4-updating-sdk.md#generating-a-new-patch

You need to run a command to update the .patch.

Copy link
Author

@maybe-Baylie maybe-Baylie Feb 27, 2025

Choose a reason for hiding this comment

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

I rebased the branch and followed the instructions in the section and confirmed the patch was correctly applied. Should I write unit tests in the WritingTest.php file?

*edit: I think I answered my own question for the test

@damienalexandre
Copy link
Member

Also, can you rebase your PR? main has been updated.

@maybe-Baylie
Copy link
Author

@damienalexandre Is there a preferred way for sending a post request with the client that isn't to a normal endpoint? To complete the test for FilesCompleteUploadExternal, I need to post the file to the link returned from filesGetUploadURLExternal. I would like to use the client since the auth is already attached to it but I'm not super familiar with how I would go about that. Mocking an endpoint maybe?

@damienalexandre
Copy link
Member

Actually, if we must POST to a custom URL, we should create a new customizable Endpoint in the lib, inspired by the existing upload one (https://github.com/jolicode/slack-php-api/blob/main/generated/Endpoint/FilesUpload.php).

class NewFileUpload extends \JoliCode\Slack\Api\Runtime\Client\BaseEndpoint implements \JoliCode\Slack\Api\Runtime\Client\Endpoint
{
    use \JoliCode\Slack\Api\Runtime\Client\EndpointTrait;
   
    private $uri;

    public function __construct(string $uri, array $queryParameters = [], array $headerParameters = [])
    {
        $this->queryParameters = $queryParameters;
        $this->headerParameters = $headerParameters;
    }

    public function getMethod(): string
    {
        return 'POST';
    }

    public function getUri(): string
    {
        return $this->uri;
    }

    public function getBody(\Symfony\Component\Serializer\SerializerInterface $serializer, $streamFactory = null): array
    {
        return $this->getMultipartBody($streamFactory);
    }

    public function getExtraHeaders(): array
    {
        return ['Accept' => ['application/json']];
    }

    public function getAuthenticationScopes(): array
    {
        return ['slackAuth'];
    }

    protected function getQueryOptionsResolver(): \Symfony\Component\OptionsResolver\OptionsResolver
    {
        $optionsResolver = parent::getQueryOptionsResolver();

        // todo

        return $optionsResolver;
    }

    protected function getHeadersOptionsResolver(): \Symfony\Component\OptionsResolver\OptionsResolver
    {
        $optionsResolver = parent::getHeadersOptionsResolver();
        // todo

        return $optionsResolver;
    }

    protected function transformResponseBody(\Psr\Http\Message\ResponseInterface $response, \Symfony\Component\Serializer\SerializerInterface $serializer, ?string $contentType = null)
    {
        // todo
    }
}

Then it can be used like this in the Client:

    public function newfileUpload(string $uploadUri, array $formParameters = [], string $fetch = self::FETCH_OBJECT)
    {
        return $this->executeEndpoint(new Endpoint\NewFileUpload($uploadUri, $formParameters), $fetch);
    }

Would that be an option? Do you think it's the way to go? I didn't study the new upload behavior yet but if you can to POST to a custom URI, that's the way to go I think.

@maybe-Baylie
Copy link
Author

That seems like a fine approach to me. Based on slacks own implementation in the WebAPI, they ended up defining the filesUploadV2 method as a convenience wrapper around the two new endpoints with an internal post call in between. I'll get started on it and see what I can do

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