symfony · fabpot · Aug 16, 2020 · Jun 8, 2020
diff --git a/src/Symfony/Component/Notifier/Bridge/Slack/SlackTransport.php b/src/Symfony/Component/Notifier/Bridge/Slack/SlackTransport.php
@@ -21,44 +21,41 @@
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 
 /**
- * Send messages via Slack using Slack Incoming Webhooks.
- *
  * @author Fabien Potencier <fabien@symfony.com>
- * @author Daniel Stancu <birkof@birkof.ro>
  *
  * @internal
  *
- * @see https://api.slack.com/messaging/webhooks
- *
  * @experimental in 5.1
  */
 final class SlackTransport extends AbstractTransport
 {
-    protected const HOST = 'hooks.slack.com';
+    protected const HOST = 'slack.com';
 
-    private $id;
+    private $accessToken;
+    private $chatChannel;
 
-    /**
-     * @param string $id The hook id (anything after https://hooks.slack.com/services/)
-     */
-    public function __construct(string $id, HttpClientInterface $client = null, EventDispatcherInterface $dispatcher = null)
+    public function __construct(string $accessToken, string $channel = null, HttpClientInterface $client = null, EventDispatcherInterface $dispatcher = null)
     {
-        $this->id = $id;
+        $this->accessToken = $accessToken;
+        $this->chatChannel = $channel;
         $this->client = $client;
 
         parent::__construct($client, $dispatcher);
     }
 
     public function __toString(): string
     {
-        return sprintf('slack://%s/%s', $this->getEndpoint(), $this->id);
+        return sprintf('slack://%s?channel=%s', $this->getEndpoint(), urlencode($this->chatChannel));
     }
 
     public function supports(MessageInterface $message): bool
     {
         return $message instanceof ChatMessage && (null === $message->getOptions() || $message->getOptions() instanceof SlackOptions);
     }
 
+    /**
+     * @see https://api.slack.com/methods/chat.postMessage
+     */
     protected function doSend(MessageInterface $message): SentMessage
     {
         if (!$message instanceof ChatMessage) {
@@ -73,19 +70,22 @@ protected function doSend(MessageInterface $message): SentMessage
         }
 
         $options = $opts ? $opts->toArray() : [];
-        $id = $message->getRecipientId() ?: $this->id;
+        if (!isset($options['channel'])) {
+            $options['channel'] = $message->getRecipientId() ?: $this->chatChannel;
+        }
         $options['text'] = $message->getSubject();
-        $response = $this->client->request('POST', sprintf('https://%s/services/%s', $this->getEndpoint(), $id), [
+        $response = $this->client->request('POST', 'https://'.$this->getEndpoint().'/api/chat.postMessage', [
             'json' => array_filter($options),
+            'auth_bearer' => $this->accessToken,
         ]);
 
         if (200 !== $response->getStatusCode()) {
-            throw new TransportException('Unable to post the Slack message: '.$response->getContent(false), $response);
+            throw new TransportException(sprintf('Unable to post the Slack message: "%s".', $response->getContent(false)), $response);
         }
 
-        $result = $response->getContent(false);
-        if ('ok' !== $result) {
-            throw new TransportException('Unable to post the Slack message: '.$result, $response);
+        $result = $response->toArray(false);
+        if (!$result['ok']) {
+            throw new TransportException(sprintf('Unable to post the Slack message: "%s".', $result['error']), $response);
         }
 
         return new SentMessage($message, (string) $this);

diff --git a/src/Symfony/Component/Notifier/Bridge/Slack/SlackTransportFactory.php b/src/Symfony/Component/Notifier/Bridge/Slack/SlackTransportFactory.php
@@ -11,7 +11,6 @@
 
 namespace Symfony\Component\Notifier\Bridge\Slack;
 
-use Symfony\Component\Notifier\Exception\IncompleteDsnException;
 use Symfony\Component\Notifier\Exception\UnsupportedSchemeException;
 use Symfony\Component\Notifier\Transport\AbstractTransportFactory;
 use Symfony\Component\Notifier\Transport\Dsn;
@@ -30,16 +29,13 @@ final class SlackTransportFactory extends AbstractTransportFactory
     public function create(Dsn $dsn): TransportInterface
     {
         $scheme = $dsn->getScheme();
-        $id = ltrim($dsn->getPath(), '/');
+        $accessToken = $this->getUser($dsn);
+        $channel = $dsn->getOption('channel');
         $host = 'default' === $dsn->getHost() ? null : $dsn->getHost();
         $port = $dsn->getPort();
 
-        if (!$id) {
-            throw new IncompleteDsnException('Missing path (maybe you haven\'t update the DSN when upgrading from 5.0).', $dsn->getOriginalDsn());
-        }
-
         if ('slack' === $scheme) {
-            return (new SlackTransport($id, $this->client, $this->dispatcher))->setHost($host)->setPort($port);
+            return (new SlackTransport($accessToken, $channel, $this->client, $this->dispatcher))->setHost($host)->setPort($port);
         }
 
         throw new UnsupportedSchemeException($dsn, 'slack', $this->getSupportedSchemes());

diff --git a/src/Symfony/Component/Notifier/Bridge/Slack/Tests/SlackTransportFactoryTest.php b/src/Symfony/Component/Notifier/Bridge/Slack/Tests/SlackTransportFactoryTest.php
@@ -13,6 +13,7 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Notifier\Bridge\Slack\SlackTransportFactory;
+use Symfony\Component\Notifier\Exception\IncompleteDsnException;
 use Symfony\Component\Notifier\Exception\UnsupportedSchemeException;
 use Symfony\Component\Notifier\Transport\Dsn;
 
@@ -23,18 +24,26 @@ public function testCreateWithDsn(): void
         $factory = new SlackTransportFactory();
 
         $host = 'testHost';
-        $path = 'testPath';
-        $transport = $factory->create(Dsn::fromString(sprintf('slack://%s/%s', $host, $path)));
+        $channel = 'testChannel';
+        $transport = $factory->create(Dsn::fromString(sprintf('slack://testUser@%s/?channel=%s', $host, $channel)));
 
-        $this->assertSame(sprintf('slack://%s/%s', $host, $path), (string) $transport);
+        $this->assertSame(sprintf('slack://%s?channel=%s', $host, $channel), (string) $transport);
+    }
+
+    public function testCreateWithNoTokenThrowsMalformed(): void
+    {
+        $factory = new SlackTransportFactory();
+
+        $this->expectException(IncompleteDsnException::class);
+        $factory->create(Dsn::fromString(sprintf('slack://%s/?channel=%s', 'testHost', 'testChannel')));
     }
 
     public function testSupportsSlackScheme(): void
     {
         $factory = new SlackTransportFactory();
 
-        $this->assertTrue($factory->supports(Dsn::fromString('slack://host/path')));
-        $this->assertFalse($factory->supports(Dsn::fromString('somethingElse://host/path')));
+        $this->assertTrue($factory->supports(Dsn::fromString('slack://host/?channel=testChannel')));
+        $this->assertFalse($factory->supports(Dsn::fromString('somethingElse://host/?channel=testChannel')));
     }
 
     public function testNonSlackSchemeThrows(): void
@@ -43,6 +52,6 @@ public function testNonSlackSchemeThrows(): void
 
         $this->expectException(UnsupportedSchemeException::class);
 
-        $factory->create(Dsn::fromString('somethingElse://host/path'));
+        $factory->create(Dsn::fromString('somethingElse://user:pwd@host/?channel=testChannel'));
     }
 }
diff --git a/src/Symfony/Component/Notifier/Bridge/Slack/Tests/SlackTransportTest.php b/src/Symfony/Component/Notifier/Bridge/Slack/Tests/SlackTransportTest.php
@@ -29,17 +29,17 @@ final class SlackTransportTest extends TestCase
     public function testToStringContainsProperties(): void
     {
         $host = 'testHost';
-        $path = 'testPath';
+        $channel = 'test Channel'; // invalid channel name to test url encoding of the channel
 
-        $transport = new SlackTransport($path, $this->createMock(HttpClientInterface::class));
+        $transport = new SlackTransport('testToken', $channel, $this->createMock(HttpClientInterface::class));
         $transport->setHost('testHost');
 
-        $this->assertSame(sprintf('slack://%s/%s', $host, $path), (string) $transport);
+        $this->assertSame(sprintf('slack://%s?channel=%s', $host, urlencode($channel)), (string) $transport);
     }
 
     public function testSupportsChatMessage(): void
     {
-        $transport = new SlackTransport('testPath', $this->createMock(HttpClientInterface::class));
+        $transport = new SlackTransport('testToken', 'testChannel', $this->createMock(HttpClientInterface::class));
 
         $this->assertTrue($transport->supports(new ChatMessage('testChatMessage')));
         $this->assertFalse($transport->supports($this->createMock(MessageInterface::class)));
@@ -49,7 +49,7 @@ public function testSendNonChatMessageThrows(): void
     {
         $this->expectException(LogicException::class);
 
-        $transport = new SlackTransport('testPath', $this->createMock(HttpClientInterface::class));
+        $transport = new SlackTransport('testToken', 'testChannel', $this->createMock(HttpClientInterface::class));
 
         $transport->send($this->createMock(MessageInterface::class));
     }
@@ -70,15 +70,15 @@ public function testSendWithEmptyArrayResponseThrows(): void
             return $response;
         });
 
-        $transport = new SlackTransport('testPath', $client);
+        $transport = new SlackTransport('testToken', 'testChannel', $client);
 
         $transport->send(new ChatMessage('testMessage'));
     }
 
     public function testSendWithErrorResponseThrows(): void
     {
         $this->expectException(TransportException::class);
-        $this->expectExceptionMessage('testErrorCode');
+        $this->expectExceptionMessageRegExp('/testErrorCode/');
 
         $response = $this->createMock(ResponseInterface::class);
         $response->expects($this->exactly(2))
@@ -87,20 +87,21 @@ public function testSendWithErrorResponseThrows(): void
 
         $response->expects($this->once())
             ->method('getContent')
-            ->willReturn('testErrorCode');
+            ->willReturn(json_encode(['error' => 'testErrorCode']));
 
         $client = new MockHttpClient(static function () use ($response): ResponseInterface {
             return $response;
         });
 
-        $transport = new SlackTransport('testPath', $client);
A93C
+        $transport = new SlackTransport('testToken', 'testChannel', $client);
 
         $transport->send(new ChatMessage('testMessage'));
     }
 
     public function testSendWithOptions(): void
     {
-        $path = 'testPath';
+        $token = 'testToken';
+        $channel = 'testChannel';
         $message = 'testMessage';
 
         $response = $this->createMock(ResponseInterface::class);
@@ -111,24 +112,25 @@ public function testSendWithOptions(): void
 
         $response->expects($this->once())
             ->method('getContent')
-            ->willReturn('ok');
+            ->willReturn(json_encode(['ok' => true]));
 
-        $expectedBody = json_encode(['text' => $message]);
+        $expectedBody = json_encode(['channel' => $channel, 'text' => $message]);
 
         $client = new MockHttpClient(function (string $method, string $url, array $options = []) use ($response, $expectedBody): ResponseInterface {
-            $this->assertSame($expectedBody, $options['body']);
+            $this->assertJsonStringEqualsJsonString($expectedBody, $options['body']);
 
             return $response;
         });
 
-        $transport = new SlackTransport($path, $client);
+        $transport = new SlackTransport($token, $channel, $client);
 
         $transport->send(new ChatMessage('testMessage'));
     }
 
     public function testSendWithNotification(): void
     {
-        $host = 'testHost';
+        $token = 'testToken';
+        $channel = 'testChannel';
         $message = 'testMessage';
 
         $response = $this->createMock(ResponseInterface::class);
@@ -139,24 +141,25 @@ public function testSendWithNotification(): void
 
         $response->expects($this->once())
             ->method('getContent')
-            ->willReturn('ok');
+            ->willReturn(json_encode(['ok' => true]));
 
         $notification = new Notification($message);
         $chatMessage = ChatMessage::fromNotification($notification);
         $options = SlackOptions::fromNotification($notification);
 
         $expectedBody = json_encode([
             'blocks' => $options->toArray()['blocks'],
+            'channel' => $channel,
             'text' => $message,
         ]);
 
         $client = new MockHttpClient(function (string $m
CE70
ethod, string $url, array $options = []) use ($response, $expectedBody): ResponseInterface {
-            $this->assertSame($expectedBody, $options['body']);
+            $this->assertJsonStringEqualsJsonString($expectedBody, $options['body']);
 
             return $response;
         });
 
-        $transport = new SlackTransport($host, $client);
+        $transport = new SlackTransport($token, $channel, $client);
 
         $transport->send($chatMessage);
     }
@@ -169,14 +172,15 @@ public function testSendWithInvalidOptions(): void
             return $this->createMock(ResponseInterface::class);
         });
 
-        $transport = new SlackTransport('testHost', $client);
+        $transport = new SlackTransport('testToken', 'testChannel', $client);
 
         $transport->send(new ChatMessage('testMessage', $this->createMock(MessageOptionsInterface::class)));
     }
 
     public function testSendWith200ResponseButNotOk(): void
     {
-        $host = 'testChannel';
+        $token = 'testToken';
+        $channel = 'testChannel';
         $message = 'testMessage';
 
         $this->expectException(TransportException::class);
@@ -189,17 +193,17 @@ public function testSendWith200ResponseButNotOk(): void
 
         $response->expects($this->once())
             ->method('getContent')
-            ->willReturn('testErrorCode');
+            ->willReturn(json_encode(['ok' => false, 'error' => 'testErrorCode']));
 
-        $expectedBody = json_encode(['text' => $message]);
+        $expectedBody = json_encode(['channel' => $channel, 'text' => $message]);
 
         $client = new MockHttpClient(function (string $method, string $url, array $options = []) use ($response, $expectedBody): ResponseInterface {
-            $this->assertSame($expectedBody, $options['body']);
+            $this->assertJsonStringEqualsJsonString($expectedBody, $options['body']);
 
             return $response;
         });
 
-        $transport = new SlackTransport($host, $client);
+        $transport = new SlackTransport($token, $channel, $client);
 
         $transport->send(new ChatMessage('testMessage'));
     }