Skip to content

Commit bb614c2

Browse files
xavierbriandfabpot
authored andcommitted
[Notifier][Slack] Use Slack Web API chat.postMessage instead of WebHooks
1 parent df57119 commit bb614c2

File tree

4 files changed

+65
-56
lines changed

4 files changed

+65
-56
lines changed

src/Symfony/Component/Notifier/Bridge/Slack/SlackTransport.php

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,44 +21,41 @@
2121
use Symfony\Contracts\HttpClient\HttpClientInterface;
2222

2323
/**
24-
* Send messages via Slack using Slack Incoming Webhooks.
25-
*
2624
* @author Fabien Potencier <fabien@symfony.com>
27-
* @author Daniel Stancu <birkof@birkof.ro>
2825
*
2926
* @internal
3027
*
31-
* @see https://api.slack.com/messaging/webhooks
32-
*
3328
* @experimental in 5.1
3429
*/
3530
final class SlackTransport extends AbstractTransport
3631
{
37-
protected const HOST = 'hooks.slack.com';
32+
protected const HOST = 'slack.com';
3833

39-
private $id;
34+
private $accessToken;
35+
private $chatChannel;
4036

41-
/**
42-
* @param string $id The hook id (anything after https://hooks.slack.com/services/)
43-
*/
44-
public function __construct(string $id, HttpClientInterface $client = null, EventDispatcherInterface $dispatcher = null)
37+
public function __construct(string $accessToken, string $channel = null, HttpClientInterface $client = null, EventDispatcherInterface $dispatcher = null)
4538
{
46-
$this->id = $id;
39+
$this->accessToken = $accessToken;
40+
$this->chatChannel = $channel;
4741
$this->client = $client;
4842

4943
parent::__construct($client, $dispatcher);
5044
}
5145

5246
public function __toString(): string
5347
{
54-
return sprintf('slack://%s/%s', $this->getEndpoint(), $this->id);
48+
return sprintf('slack://%s?channel=%s', $this->getEndpoint(), urlencode($this->chatChannel));
5549
}
5650

5751
public function supports(MessageInterface $message): bool
5852
{
5953
return $message instanceof ChatMessage && (null === $message->getOptions() || $message->getOptions() instanceof SlackOptions);
6054
}
6155

56+
/**
57+
* @see https://api.slack.com/methods/chat.postMessage
58+
*/
6259
protected function doSend(MessageInterface $message): SentMessage
6360
{
6461
if (!$message instanceof ChatMessage) {
@@ -73,19 +70,22 @@ protected function doSend(MessageInterface $message): SentMessage
7370
}
7471

7572
$options = $opts ? $opts->toArray() : [];
76-
$id = $message->getRecipientId() ?: $this->id;
73+
if (!isset($options['channel'])) {
74+
$options['channel'] = $message->getRecipientId() ?: $this->chatChannel;
75+
}
7776
$options['text'] = $message->getSubject();
78-
$response = $this->client->request('POST', sprintf('https://%s/services/%s', $this->getEndpoint(), $id), [
77+
$response = $this->client->request('POST', 'https://'.$this->getEndpoint().'/api/chat.postMessage', [
7978
'json' => array_filter($options),
79+
'auth_bearer' => $this->accessToken,
8080
]);
8181

8282
if (200 !== $response->getStatusCode()) {
83-
throw new TransportException('Unable to post the Slack message: '.$response->getContent(false), $response);
83+
throw new TransportException(sprintf('Unable to post the Slack message: "%s".', $response->getContent(false)), $response);
8484
}
8585

86-
$result = $response->getContent(false);
87-
if ('ok' !== $result) {
88-
throw new TransportException('Unable to post the Slack message: '.$result, $response);
86+
$result = $response->toArray(false);
87+
if (!$result['ok']) {
88+
throw new TransportException(sprintf('Unable to post the Slack message: "%s".', $result['error']), $response);
8989
}
9090

9191
return new SentMessage($message, (string) $this);

src/Symfony/Component/Notifier/Bridge/Slack/SlackTransportFactory.php

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
namespace Symfony\Component\Notifier\Bridge\Slack;
1313

14-
use Symfony\Component\Notifier\Exception\IncompleteDsnException;
1514
use Symfony\Component\Notifier\Exception\UnsupportedSchemeException;
1615
use Symfony\Component\Notifier\Transport\AbstractTransportFactory;
1716
use Symfony\Component\Notifier\Transport\Dsn;
@@ -30,16 +29,13 @@ final class SlackTransportFactory extends AbstractTransportFactory
3029
public function create(Dsn $dsn): TransportInterface
3130
{
3231
$scheme = $dsn->getScheme();
33-
$id = ltrim($dsn->getPath(), '/');
32+
$accessToken = $this->getUser($dsn);
33+
$channel = $dsn->getOption('channel');
3434
$host = 'default' === $dsn->getHost() ? null : $dsn->getHost();
3535
$port = $dsn->getPort();
3636

37-
if (!$id) {
38-
throw new IncompleteDsnException('Missing path (maybe you haven\'t update the DSN when upgrading from 5.0).', $dsn->getOriginalDsn());
39-
}
40-
4137
if ('slack' === $scheme) {
42-
return (new SlackTransport($id, $this->client, $this->dispatcher))->setHost($host)->setPort($port);
38+
return (new SlackTransport($accessToken, $channel, $this->client, $this->dispatcher))->setHost($host)->setPort($port);
4339
}
4440

4541
throw new UnsupportedSchemeException($dsn, 'slack', $this->getSupportedSchemes());

src/Symfony/Component/Notifier/Bridge/Slack/Tests/SlackTransportFactoryTest.php

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Notifier\Bridge\Slack\SlackTransportFactory;
16+
use Symfony\Component\Notifier\Exception\IncompleteDsnException;
1617
use Symfony\Component\Notifier\Exception\UnsupportedSchemeException;
1718
use Symfony\Component\Notifier\Transport\Dsn;
1819

@@ -23,18 +24,26 @@ public function testCreateWithDsn(): void
2324
$factory = new SlackTransportFactory();
2425

2526
$host = 'testHost';
26-
$path = 'testPath';
27-
$transport = $factory->create(Dsn::fromString(sprintf('slack://%s/%s', $host, $path)));
27+
$channel = 'testChannel';
28+
$transport = $factory->create(Dsn::fromString(sprintf('slack://testUser@%s/?channel=%s', $host, $channel)));
2829

29-
$this->assertSame(sprintf('slack://%s/%s', $host, $path), (string) $transport);
30+
$this->assertSame(sprintf('slack://%s?channel=%s', $host, $channel), (string) $transport);
31+
}
32+
33+
public function testCreateWithNoTokenThrowsMalformed(): void
34+
{
35+
$factory = new SlackTransportFactory();
36+
37+
$this->expectException(IncompleteDsnException::class);
38+
$factory->create(Dsn::fromString(sprintf('slack://%s/?channel=%s', 'testHost', 'testChannel')));
3039
}
3140

3241
public function testSupportsSlackScheme(): void
3342
{
3443
$factory = new SlackTransportFactory();
3544

36-
$this->assertTrue($factory->supports(Dsn::fromString('slack://host/path')));
37-
$this->assertFalse($factory->supports(Dsn::fromString('somethingElse://host/path')));
45+
$this->assertTrue($factory->supports(Dsn::fromString('slack://host/?channel=testChannel')));
46+
$this->assertFalse($factory->supports(Dsn::fromString('somethingElse://host/?channel=testChannel')));
3847
}
3948

4049
public function testNonSlackSchemeThrows(): void
@@ -43,6 +52,6 @@ public function testNonSlackSchemeThrows(): void
4352

4453
$this->expectException(UnsupportedSchemeException::class);
4554

46-
$factory->create(Dsn::fromString('somethingElse://host/path'));
55+
$factory->create(Dsn::fromString('somethingElse://user:pwd@host/?channel=testChannel'));
4756
}
4857
}

src/Symfony/Component/Notifier/Bridge/Slack/Tests/SlackTransportTest.php

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,17 @@ final class SlackTransportTest extends TestCase
2929
public function testToStringContainsProperties(): void
3030
{
3131
$host = 'testHost';
32-
$path = 'testPath';
32+
$channel = 'test Channel'; // invalid channel name to test url encoding of the channel
3333

34-
$transport = new SlackTransport($path, $this->createMock(HttpClientInterface::class));
34+
$transport = new SlackTransport('testToken', $channel, $this->createMock(HttpClientInterface::class));
3535
$transport->setHost('testHost');
3636

37-
$this->assertSame(sprintf('slack://%s/%s', $host, $path), (string) $transport);
37+
$this->assertSame(sprintf('slack://%s?channel=%s', $host, urlencode($channel)), (string) $transport);
3838
}
3939

4040
public function testSupportsChatMessage(): void
4141
{
42-
$transport = new SlackTransport('testPath', $this->createMock(HttpClientInterface::class));
42+
$transport = new SlackTransport('testToken', 'testChannel', $this->createMock(HttpClientInterface::class));
4343

4444
$this->assertTrue($transport->supports(new ChatMessage('testChatMessage')));
4545
$this->assertFalse($transport->supports($this->createMock(MessageInterface::class)));
@@ -49,7 +49,7 @@ public function testSendNonChatMessageThrows(): void
4949
{
5050
$this->expectException(LogicException::class);
5151

52-
$transport = new SlackTransport('testPath', $this->createMock(HttpClientInterface::class));
52+
$transport = new SlackTransport('testToken', 'testChannel', $this->createMock(HttpClientInterface::class));
5353

5454
$transport->send($this->createMock(MessageInterface::class));
5555
}
@@ -70,15 +70,15 @@ public function testSendWithEmptyArrayResponseThrows(): void
7070
return $response;
7171
});
7272

73-
$transport = new SlackTransport('testPath', $client);
73+
$transport = new SlackTransport('testToken', 'testChannel', $client);
7474

7575
$transport->send(new ChatMessage('testMessage'));
7676
}
7777

7878
public function testSendWithErrorResponseThrows(): void
7979
{
8080
$this->expectException(TransportException::class);
81-
$this->expectExceptionMessage('testErrorCode');
81+
$this->expectExceptionMessageRegExp('/testErrorCode/');
8282

8383
$response = $this->createMock(ResponseInterface::class);
8484
$response->expects($this->exactly(2))
@@ -87,20 +87,21 @@ public function testSendWithErrorResponseThrows(): void
8787

8888
$response->expects($this->once())
8989
->method('getContent')
90-
->willReturn('testErrorCode');
90+
->willReturn(json_encode(['error' => 'testErrorCode']));
9191

9292
$client = new MockHttpClient(static function () use ($response): ResponseInterface {
9393
return $response;
9494
});
9595

96-
$transport = new SlackTransport('testPath', $client);
96+
$transport = new SlackTransport('testToken', 'testChannel', $client);
9797

9898
$transport->send(new ChatMessage('testMessage'));
9999
}
100100

101101
public function testSendWithOptions(): void
102102
{
103-
$path = 'testPath';
103+
$token = 'testToken';
104+
$channel = 'testChannel';
104105
$message = 'testMessage';
105106

106107
$response = $this->createMock(ResponseInterface::class);
@@ -111,24 +112,25 @@ public function testSendWithOptions(): void
111112

112113
$response->expects($this->once())
113114
->method('getContent')
114-
->willReturn('ok');
115+
->willReturn(json_encode(['ok' => true]));
115116

116-
$expectedBody = json_encode(['text' => $message]);
117+
$expectedBody = json_encode(['channel' => $channel, 'text' => $message]);
117118

118119
$client = new MockHttpClient(function (string $method, string $url, array $options = []) use ($response, $expectedBody): ResponseInterface {
119-
$this->assertSame($expectedBody, $options['body']);
120+
$this->assertJsonStringEqualsJsonString($expectedBody, $options['body']);
120121

121122
return $response;
122123
});
123124

124-
$transport = new SlackTransport($path, $client);
125+
$transport = new SlackTransport($token, $channel, $client);
125126

126127
$transport->send(new ChatMessage('testMessage'));
127128
}
128129

129130
public function testSendWithNotification(): void
130131
{
131-
$host = 'testHost';
132+
$token = 'testToken';
133+
$channel = 'testChannel';
132134
$message = 'testMessage';
133135

134136
$response = $this->createMock(ResponseInterface::class);
@@ -139,24 +141,25 @@ public function testSendWithNotification(): void
139141

140142
$response->expects($this->once())
141143
->method('getContent')
142-
->willReturn('ok');
144+
->willReturn(json_encode(['ok' => true]));
143145

144146
$notification = new Notification($message);
145147
$chatMessage = ChatMessage::fromNotification($notification);
146148
$options = SlackOptions::fromNotification($notification);
147149

148150
$expectedBody = json_encode([
149151
'blocks' => $options->toArray()['blocks'],
152+
'channel' => $channel,
150153
'text' => $message,
151154
]);
152155

153156
$client = new MockHttpClient(function (string $method, string $url, array $options = []) use ($response, $expectedBody): ResponseInterface {
154-
$this->assertSame($expectedBody, $options['body']);
157+
$this->assertJsonStringEqualsJsonString($expectedBody, $options['body']);
155158

156159
return $response;
157160
});
158161

159-
$transport = new SlackTransport($host, $client);
162+
$transport = new SlackTransport($token, $channel, $client);
160163

161164
$transport->send($chatMessage);
162165
}
@@ -169,14 +172,15 @@ public function testSendWithInvalidOptions(): void
169172
return $this->createMock(ResponseInterface::class);
170173
});
171174

172-
$transport = new SlackTransport('testHost', $client);
175+
$transport = new SlackTransport('testToken', 'testChannel', $client);
173176

174177
$transport->send(new ChatMessage('testMessage', $this->createMock(MessageOptionsInterface::class)));
175178
}
176179

177180
public function testSendWith200ResponseButNotOk(): void
178181
{
179-
$host = 'testChannel';
182+
$token = 'testToken';
183+
$channel = 'testChannel';
180184
$message = 'testMessage';
181185

182186
$this->expectException(TransportException::class);
@@ -189,17 +193,17 @@ public function testSendWith200ResponseButNotOk(): void
189193

190194
$response->expects($this->once())
191195
->method('getContent')
192-
->willReturn('testErrorCode');
196+
->willReturn(json_encode(['ok' => false, 'error' => 'testErrorCode']));
193197

194-
$expectedBody = json_encode(['text' => $message]);
198+
$expectedBody = json_encode(['channel' => $channel, 'text' => $message]);
195199

196200
$client = new MockHttpClient(function (string $method, string $url, array $options = []) use ($response, $expectedBody): ResponseInterface {
197-
$this->assertSame($expectedBody, $options['body']);
201+
$this->assertJsonStringEqualsJsonString($expectedBody, $options['body']);
198202

199203
return $response;
200204
});
201205

202-
$transport = new SlackTransport($host, $client);
206+
$transport = new SlackTransport($token, $channel, $client);
203207

204208
$transport->send(new ChatMessage('testMessage'));
205209
}

0 commit comments

Comments
 (0)