nathanheffley / laravel-slack-blocks

An extension of the official `laravel/slack-notification-channel` package to add support for Slack Blocks.
MIT License
49 stars 13 forks source link

Support for `actions` block #4

Closed ortonomy closed 4 years ago

ortonomy commented 4 years ago

The official BlockKit has an actions block:

https://api.slack.com/reference/block-kit/blocks#actions

{
  "type": "actions",
  "elements": [
    {
        ...
   }
  ]

but I'm getting an error when I try to use it. This is meant to house buttons.

Here's my example code:

$attachment->block(function($block) use ($url, $self) {
  $block
    ->type('actions')
    ->elements([
      [
        'type' => 'button',
    'text' => [
      'type' => 'plain_text',
      'text' => 'Go to app'
         ],
    'url' => $url
      ]
  ]);
});

Are you going to support this part of the blockkit?

ortonomy commented 4 years ago

I event looked at your test cases and found:

$block
                    ->type('actions')
                    ->elements([
                            'type' => 'button',
                            'text' => [
                                    'type' => 'plain_text',
                                    'text' => 'Cancel',
                            ],
                    ]);

but I get a server error when running it....

nathanheffley commented 4 years ago

What is the error you get?

ortonomy commented 4 years ago

Sorry that I never got back to this - had other work to do.

Basically, I let the server die at the point it was creating the JSON, as we checked the error and it was getting a 400 error back from slack. I can't post the JSON that was sent here, as I'm not running the server.

Weirdly, I cleared the composer cache, and reinstalled packages, and it worked... so I'm worried there is some issue in the autoload process --- how does composer know which version of the /Channels/SlackWebhookChannel.php file to use? (your package overrides the laravel one) but has a dependency on the laravel package.)

Either way, I'd like to draw your attention here: https://api.slack.com/messaging/composing/layouts#attachments where they have deprecated attachments:

In short, we recommend you use Block Kit for as much of your message composition as you can, and avoid using attachments if possible. Blocks have many more visual and interactive capabilities available.

This means that color and title no longer even work for attachments if you send blocks (tried and failed to get a color)... might be time to disable that API and just make it unavailable.

nathanheffley commented 4 years ago

For as long as attachments are supported by Slack I believe I'll keep them in this package. Attachments are available in the original Laravel slack channel and I would hate for somebody who relies on attachments for whatever reason to not be able to use this package. Whoever is using this package needs to be familiar with Slack's message interface. Although adding a @deprecated tag is probably prudent.

As for the autoloading issue, I'll look into it. I have never experienced this issue, and as long as you're using the classes in this package I don't think it would be an issue.

nathanheffley commented 4 years ago

Did you by any chance have your project require both this package and the Laravel Slack package in your composer.json file?

williamjulianvicary commented 4 years ago

@nathanheffley For what it's worth I ran into some composer deploy issues as well.

Locally, installing alongside the Laravel Slack Package worked fine, however when doing a fresh deploy Composer complained that the lock file was out of date (for some reason).

To fix this I removed both extensions (I had the Laravel Slack package installed locally too) and then re-installed your version. This solved the issue but seems odd to have caused the issues I observed! Hopefully that helps!

nathanheffley commented 4 years ago

Yeah, there is some autoload confusion that occurs when requiring both packages. The readme does say to require this package instead of the official one. Maybe that needs to be expanded to say that if you already require the official one you need to remove it first.

I have but been able to recreate the issue when just requiring my package as per the readme.

If anyone knows how to resolve this issue without changing the classname (I want the package to stay as much of a drop in replacement as possible.

nathanheffley commented 4 years ago

There hasn't been any activity on this issue for a while and I've never encountered it personally. If anybody does run into this again and can replicate, please leave a comment to let me know!

ejerskov commented 3 years ago

Hi @nathanheffley Im experiencing this issue now and I cant get it to work with your "test"-example:

return (new SlackMessage)
    ->attachment(function ($attachment) {
        $attachment
            ->block(function ($block) {
                $block
                    ->type('actions')
                    ->elements([
                        'type' => 'button',
                        'text' => [
                            'type' => 'plain_text',
                            'text' => 'Cancel',
                        ],
                    ]);
            });
    });

Laravel throws a bad request with invalid_attachments response.

Its working fine with a text section.

I had the original laravel notification package installed prior to yours, but uninstalled before installing yours. I've tried to clear composer cache, dump auto load and run composer install again.

I still get the error. Any idea on how to get it to work?

EDIT: Okay, so for some reason I can only get it to work with an extra [] around the elements, compared to the test example:

->elements([
    [
        'type' => 'button',
        'text' => [
            'type' => 'plain_text',
            'text' => 'Approve',
            'emoji' => false,
        ],
        'value' => 'click_me_123',
        'action_id' => 'approve_x',
        'style' => 'primary',
    ]
]);