Skip to content

Added metadata definition for chat.postMessage and history#156

Merged
damienalexandre merged 6 commits intojolicode:mainfrom
tchapuis:main
Nov 10, 2023
Merged

Added metadata definition for chat.postMessage and history#156
damienalexandre merged 6 commits intojolicode:mainfrom
tchapuis:main

Conversation

@tchapuis
Copy link
Copy Markdown
Contributor

The Slack documentation says that we can include metadata in chat.postMessage arguments. See : https://api.slack.com/metadata

OpenApi spec for this argument doesn't seem to be up to date.

So I added it to the patched version for message definition and chat.postMessage method. I also added the include_all_metadata argument for the conversations.history method

@yngc0der
Copy link
Copy Markdown

The temporary solution is create a custom endpoint class, and use this class to call api method:

<?php

use JoliCode\Slack\Api\Endpoint\ChatPostMessage;

final class FixedChatPostMessageEndpoint extends ChatPostMessage
{
    protected function getFormOptionsResolver(): \Symfony\Component\OptionsResolver\OptionsResolver
    {
        $optionsResolver = parent::getFormOptionsResolver();
        $optionsResolver->setDefined(['metadata']);
        $optionsResolver->setAllowedTypes('metadata', ['string']);
        return $optionsResolver;
    }
}


$slack->executeEndpoint(new FixedChatPostMessageEndpoint([
    'channel' => $channel,
    'text' => $text,
    'metadata' => $metadata,
]));

Copy link
Copy Markdown
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.

Hello, sorry for the very long delay on the review.

I fear the ".patch" file is wrong, it includes A LOT of changes that have nothing to do with the metadata definition 🤔 Maybe it's just because Slack updated their definition, but we should then split this in two PR.

Thank you for your understanding 🙏

@tchapuis
Copy link
Copy Markdown
Contributor Author

Hello,
The patch file was a pain to get it right 😅

@damienalexandre
Copy link
Copy Markdown
Member

Thanks for working on this!

Tests on main were red, so I fixed them in #162 ;
could you please:

  • rm -rf generated/
  • vendor/bin/jane-openapi generate --config-file=.jane-openapi.php

And push?

@damienalexandre damienalexandre merged commit 9b923e4 into jolicode:main Nov 10, 2023
@damienalexandre
Copy link
Copy Markdown
Member

Awesome, thank you so much!

@damienalexandre
Copy link
Copy Markdown
Member

Version v4.6.0 is published!

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