laravel / slack-notification-channel

Slack Notification Channel for laravel.
https://laravel.com/docs/notifications#slack-notifications
MIT License
864 stars 59 forks source link

Function to() doesn't work #54

Closed ok200paul closed 2 years ago

ok200paul commented 2 years ago

Description:

Specifying an alternative channel to send a slack notification to, does not work. Probably linked to https://github.com/laravel/slack-notification-channel/issues/34 from back in 2020.

Steps To Reproduce:

// The app is set to send to a channel named #test-channel, but I want to send to #general
return (new SlackMessage())
            ->from('Ghost', ':ghost:')
            ->to('#general')
            ->content('This will be sent to #general');

Demo

https://github.com/ok200paul/l8-slack-channel-bug-report

driesvints commented 2 years ago

You mention creating an app but we don't support that atm. Only incoming webhooks.

driesvints commented 2 years ago

Hmm, I'll try to have a look at this to see if we can support apps as well. For now I don't think this will work sorry.

ok200paul commented 2 years ago

Hi Dries - not looking to reopen this, however, the doco mentions that we need to create a slack app for the integration to work, so yeah, we create an app. Does the doco needs to be updated? I might have been following them a little too closely if creating an app is not required! Doco screengrab:

image

The slack documentation indicates that we're (nowadays) unable to send incoming webhooks without setting up a slack app

driesvints commented 2 years ago

Hey @ok200paul, you're correct. It seems we updated those docs incorrectly. I've sent in a PR to revert this as this currently doesn't work: https://github.com/laravel/docs/pull/7651.

Like I said, I'll be looking into modernising the package hopefully soon so Slack Apps are supported as well.

ok200paul commented 2 years ago

Thanks Dries! Ping me when you face back in this direction and I’ll help out where I can.

On Fri, 4 Feb 2022 at 19:07, Dries Vints @.***> wrote:

Hey @ok200paul https://github.com/ok200paul, you're correct. It seems we updated those docs incorrectly. I've sent in a PR to revert this as this currently doesn't work: laravel/docs#7651 https://github.com/laravel/docs/pull/7651.

Like I said, I'll be looking into modernising the package hopefully soon so Slack Apps are supported as well.

— Reply to this email directly, view it on GitHub https://github.com/laravel/slack-notification-channel/issues/54#issuecomment-1029743021, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFK6K63PAEVM4R4GUKVQR6TUZOCMDANCNFSM5NKMD3JA . You are receiving this because you were mentioned.Message ID: @.***>

--

Regards, Paul Grimes OK200 129a Chapel St, Windsor 3181 Melbourne P: 0432 903 145 E: @.** https://ok200.netLooking to set up a meeting?* Check my availability here: https://calendly.com/ok200-paul/30min

driesvints commented 2 years ago

@ok200paul we've decided to leave the current docs in place and just note the methods that don't work with Slack apps. It looks that you can't use from and to with Slack Apps right now. Should you figure out a way on how to do that we'd welcome PR's for it.

ok200paul commented 2 years ago

Will do, thanks Dries!