[3.0] Support BlockKit / WebAPI-based bot notifications

[claudiodekker]

Re-submit of #63 using a clean commit history, as requested.

My reasoning for removing the v2 API & considering this a full v3 is pretty simple: Although Incoming Webhooks still work today, they have been marked as deprecated in favor of the WebAPI that this PR (in part) implements: https://api.slack.com/legacy/custom-integrations/messaging/webhooks

Since this constitutes a breaking change, I've also taken the opportunity to remove deprecated/no-longer-supported Laravel and PHP versions. Those who still rely on the now-deprecated legacy webhooks can lock the composer dependency to ^2.0 until this API is eventually removed by Slack altogether.

[taylorotwell]

Should be a separate, new channel to avoid breaking change.

[driesvints]

@taylorotwell can't we instruct people to use this package's v2 version if they need the old version?

[taylorotwell]

I don't see any reason to not just make it a new channel. It's just so easy to avoid the breaking change entirely by doing so. We already take a similar approach to our SES and SES 2.0 drivers for mail.

[claudiodekker]

Sounds good to me & shouldn't be too difficult.

I'll probably pick it up tomorrow morning, as I don't have time for it today. I'll ping you when it's done. 👍

[taylorotwell]

Thanks! Just mark as ready for review when done and I'll take a look.

[taylorotwell]

Can you copy of your usage examples into this PR and update them as necessary since it is a new channel? Thanks.

[claudiodekker]

@taylorotwell The usage remains unchanged!

The internal channel (Webhook vs. WebAPI) is determined based on whether the routeNotificationFor method is an URL string/PSR URL instance. If it is, it'll be routed to a Webhook, and if it's not, it'll be routed to the new WebAPI channel.

Part of the reason for this, has to do with the fact that the v2 version uses the Illuminate\Notifications namespace directly. With this "v3" version, I had placed everything in the Illuminate\Notifications\Slack namespace, but since we want to maintain backwards compatibility, this simply meant placing the new version in a subfolder/sub-namespace.

The reason for this has to do with that I wasn't sure which one deserves the "slack" channel type more, and only one channel (I think?) can have it at a time. With the webhooks being legacy, changing them would break backwards compatibility.

[jbrooksuk]

I'm not sure what the precedence is in other notification channels, but while testing I spent a long time not receiving notifications until I realised I needed to add the Slack app into each channel I wanted to receive notifications for.

I wonder whether we should throw an exception depending on the response?

[claudiodekker]

Yeah, that actually makes a lot of sense. Fixed. 👍

It'll now throw on non-200's, as well as when the response itself gets checked, and if a Slack "non-ok" response is given, it'll throw an exception that includes the "error" property from the error. This can range from permission issues, to non-existent channels and even invalid authentication tokens.

[jbrooksuk]

If you're not sending a token / channel then perhaps we should throw an exception so that the developer can figure out what is missing? As per-below, I'm not sure what the situation is across other channels, but it's very easy to not receive notifications and have no direction as to how to resolve it.

[alies-dev]

@claudiodekker
Thanks for this great PR!

I started to use the new major version on my small project and had some issues with tests that I solved. I just wanty to share my experience ti save someones time.

SLACK_WEBHOOK_URL .env variable needed to route Slack webhook-based notifications to SlackWebhookChannel (previous implementation of slack notification channel that co-exist with a new one),
see \Illuminate\Notifications\SlackNotificationRouterChannel::determineChannel() for details.
But non-empty SLACK_WEBHOOK_URL also enables SlackWebhookChannel to use a real HTTP client
that sends a real HTTP request to Slack (not what we need from tests, right?).
To avoid it, you need to mock or fake something, e.g., SlackWebhookChannel:

$this->app->singleton(SlackWebhookChannel::class, function () {
    $httpClientHandler = new MockHandler([
        new \GuzzleHttp\Psr7\Response(200),
    ]);
    $handlerStack = HandlerStack::create($httpClientHandler);
    return new SlackWebhookChannel(new \GuzzleHttp\Client(['handler' => $handlerStack]));
});

But in my case it was even simpler: I just needed to fake Event dispatcher because I send Slack notification from an Event Listener:

$fakeEvent = \Illuminate\Support\Facades\Event::fake();
...
$fakeEvent->assertDispatched(OrderPaid::class);

[jameshulse]

The reason for this has to do with that I wasn't sure which one deserves the "slack" channel type more, and only one channel (I think?) can have it at a time. With the webhooks being legacy, changing them would break backwards compatibility.

@claudiodekker, great PR. Unfortunately I think backwards compatibility was broken anyway - or at least the upgrade path isn't totally clear. We are returning Illuminate\Notifications\Messages\SlackMessage from our toSlack function, but after the 3.x upgrade we get:

Illuminate\Notifications\Slack\SlackChannel::buildJsonPayload(): Argument #1 ($message) must be of type Illuminate\Notifications\Slack\SlackMessage, Illuminate\Notifications\Messages\SlackMessage given

It looks like buildJsonPayload expects the new Slack\SlackMessage. Could some other part of our configuration be incorrect which means our 2.x objects are being passed to the 3.x behaviour?

For now we are down-grading to 2.5 to ensure this is working until we can commit to the conversion to the block API.