From 7ed2da9c9705c0525537d18fa330f9e9998c3878 Mon Sep 17 00:00:00 2001 From: antonkomarev Date: Sat, 22 Feb 2020 10:53:40 +0300 Subject: [PATCH 1/7] Refactor public API --- README.md | 8 ++--- src/RocketChat.php | 20 ++++++------ src/RocketChatAttachment.php | 44 ++++++++++++-------------- src/RocketChatMessage.php | 2 +- src/RocketChatWebhookChannel.php | 2 +- tests/RocketChatAttachmentTest.php | 2 +- tests/RocketChatMessageTest.php | 16 +++++----- tests/RocketChatWebhookChannelTest.php | 6 ++-- 8 files changed, 49 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index 0ef16c8..4dbca24 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,7 @@ class TaskCompleted extends Notification public function toRocketChat($notifiable): RocketChatMessage { - return RocketChatMessage::create('Test message') + return RocketChatMessage::make('Test message') ->to('channel_name') // optional if set in config ->from('webhook_token'); // optional if set in config } @@ -118,12 +118,12 @@ There are several ways to add one ore more attachments to a message ```php public function toRocketChat($notifiable) { - return RocketChatMessage::create("Test message") + return RocketChatMessage::make("Test message") ->to('channel_name') // optional if set in config ->from('webhook_token') // optional if set in config ->attachments([ - RocketChatAttachment::create()->imageUrl('test'), - RocketChatAttachment::create(['image_url' => 'test']), + RocketChatAttachment::make()->imageUrl('test'), + RocketChatAttachment::make(['image_url' => 'test']), new RocketChatAttachment(['image_url' => 'test']), [ 'image_url' => 'test' diff --git a/src/RocketChat.php b/src/RocketChat.php index ae39b8c..47791dd 100644 --- a/src/RocketChat.php +++ b/src/RocketChat.php @@ -36,16 +36,6 @@ public function __construct(HttpClient $http, string $url, string $token, ?strin $this->defaultChannel = $defaultChannel; } - /** - * Returns default channel id or name. - * - * @return string|null - */ - public function channel(): ?string - { - return $this->defaultChannel; - } - /** * Returns RocketChat base url. * @@ -66,6 +56,16 @@ public function token(): string return $this->token; } + /** + * Returns default channel id or name. + * + * @return string|null + */ + public function defaultChannel(): ?string + { + return $this->defaultChannel; + } + /** * Send a message. * diff --git a/src/RocketChatAttachment.php b/src/RocketChatAttachment.php index 05a6f9d..58badb7 100644 --- a/src/RocketChatAttachment.php +++ b/src/RocketChatAttachment.php @@ -62,42 +62,24 @@ class RocketChatAttachment /** * RocketChatAttachment constructor. * - * @param array|null $config + * @param array $config */ - public function __construct(array $config = null) + public function __construct(array $config = []) { - if ($config != null) { - $this->setFromArray($config); - } + $this->setFromArray($config); } /** * Create a new instance of RocketChatAttachment. * - * @param array|null $config + * @param array $config * @return RocketChatAttachment */ - public static function create(array $config = null) + public static function make(array $config = []) { return new self($config); } - /** - * Set attachment data form array. - * - * @param array $data - */ - protected function setFromArray(array $data) - { - foreach ($data as $key => $value) { - $method = Str::camel($key); - if (! method_exists($this, $method)) { - continue; - } - $this->{$method}($value); - } - } - /** * @param string $color * @return RocketChatAttachment @@ -323,4 +305,20 @@ public function toArray(): array 'fields' => $this->fields, ]); } + + /** + * Set attachment data form array. + * + * @param array $data + */ + protected function setFromArray(array $data) + { + foreach ($data as $key => $value) { + $method = Str::camel($key); + if (! method_exists($this, $method)) { + continue; + } + $this->{$method}($value); + } + } } diff --git a/src/RocketChatMessage.php b/src/RocketChatMessage.php index 2556ee4..ff2ee0e 100644 --- a/src/RocketChatMessage.php +++ b/src/RocketChatMessage.php @@ -33,7 +33,7 @@ class RocketChatMessage * @param string $content * @return static */ - public static function create(string $content = ''): self + public static function make(string $content = ''): self { return new static($content); } diff --git a/src/RocketChatWebhookChannel.php b/src/RocketChatWebhookChannel.php index b11604e..407679a 100644 --- a/src/RocketChatWebhookChannel.php +++ b/src/RocketChatWebhookChannel.php @@ -41,7 +41,7 @@ public function send($notifiable, Notification $notification): void $message = $notification->toRocketChat($notifiable); $to = $message->channel ?: $notifiable->routeNotificationFor('RocketChat'); - if (! $to = $to ?: $this->rocketChat->channel()) { + if (! $to = $to ?: $this->rocketChat->defaultChannel()) { throw CouldNotSendNotification::missingTo(); } diff --git a/tests/RocketChatAttachmentTest.php b/tests/RocketChatAttachmentTest.php index dd8061f..4dc9b9b 100644 --- a/tests/RocketChatAttachmentTest.php +++ b/tests/RocketChatAttachmentTest.php @@ -20,7 +20,7 @@ public function it_can_accept_a_config_when_constructing_an_attachment(): void /** @test */ public function it_can_accept_a_config_when_creating_an_attachment(): void { - $attachment = RocketChatAttachment::create(['title' => 'test123']); + $attachment = RocketChatAttachment::make(['title' => 'test123']); $this->assertEquals(['title' => 'test123'], $attachment->toArray()); } diff --git a/tests/RocketChatMessageTest.php b/tests/RocketChatMessageTest.php index 4e48ab6..5915c3c 100644 --- a/tests/RocketChatMessageTest.php +++ b/tests/RocketChatMessageTest.php @@ -21,7 +21,7 @@ public function it_can_accept_a_content_when_constructing_a_message(): void /** @test */ public function it_can_accept_a_content_when_creating_a_message(): void { - $message = RocketChatMessage::create('hello'); + $message = RocketChatMessage::make('hello'); $this->assertEquals('hello', $message->content); } @@ -77,7 +77,7 @@ public function it_can_set_the_avatar(): void /** @test */ public function it_can_set_attachment(): void { - $attachment = RocketChatAttachment::create(['title' => 'test']); + $attachment = RocketChatAttachment::make(['title' => 'test']); $message = (new RocketChatMessage())->attachment($attachment); $this->assertSame($attachment, $message->attachments[0]); @@ -94,9 +94,9 @@ public function it_can_set_attachment_as_array(): void public function it_can_set_multiple_attachments(): void { $message = (new RocketChatMessage())->attachments([ - RocketChatAttachment::create(), - RocketChatAttachment::create(), - RocketChatAttachment::create(), + RocketChatAttachment::make(), + RocketChatAttachment::make(), + RocketChatAttachment::make(), ]); $this->assertInstanceOf(RocketChatAttachment::class, $message->attachments[0]); $this->assertCount(3, $message->attachments); @@ -106,9 +106,9 @@ public function it_can_set_multiple_attachments(): void public function it_can_clear_attachments(): void { $message = (new RocketChatMessage())->attachments([ - RocketChatAttachment::create(), - RocketChatAttachment::create(), - RocketChatAttachment::create(), + RocketChatAttachment::make(), + RocketChatAttachment::make(), + RocketChatAttachment::make(), ]); $message->clearAttachments(); diff --git a/tests/RocketChatWebhookChannelTest.php b/tests/RocketChatWebhookChannelTest.php index a5dffbf..1ddc984 100644 --- a/tests/RocketChatWebhookChannelTest.php +++ b/tests/RocketChatWebhookChannelTest.php @@ -106,7 +106,7 @@ class TestNotification extends Notification { public function toRocketChat(): RocketChatMessage { - return RocketChatMessage::create('hello')->from(':token')->to(':channel'); + return RocketChatMessage::make('hello')->from(':token')->to(':channel'); } } @@ -114,7 +114,7 @@ class TestNotificationWithMissedChannel extends Notification { public function toRocketChat(): RocketChatMessage { - return RocketChatMessage::create('hello')->from(':token'); + return RocketChatMessage::make('hello')->from(':token'); } } @@ -122,6 +122,6 @@ class TestNotificationWithMissedFrom extends Notification { public function toRocketChat(): RocketChatMessage { - return RocketChatMessage::create('hello')->to(':channel'); + return RocketChatMessage::make('hello')->to(':channel'); } } From 2b753e868b9274a0a4cae2ed9725e3c90136523a Mon Sep 17 00:00:00 2001 From: antonkomarev Date: Sat, 22 Feb 2020 11:28:45 +0300 Subject: [PATCH 2/7] Rename setFromArray method name to more obvious equivalent --- src/RocketChatAttachment.php | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/RocketChatAttachment.php b/src/RocketChatAttachment.php index 58badb7..6324dd6 100644 --- a/src/RocketChatAttachment.php +++ b/src/RocketChatAttachment.php @@ -62,22 +62,22 @@ class RocketChatAttachment /** * RocketChatAttachment constructor. * - * @param array $config + * @param array $data */ - public function __construct(array $config = []) + public function __construct(array $data = []) { - $this->setFromArray($config); + $this->setPropertiesFromArray($data); } /** * Create a new instance of RocketChatAttachment. * - * @param array $config + * @param array $data * @return RocketChatAttachment */ - public static function make(array $config = []) + public static function make(array $data = []) { - return new self($config); + return new self($data); } /** @@ -307,18 +307,18 @@ public function toArray(): array } /** - * Set attachment data form array. + * Set attachment data from array. * * @param array $data */ - protected function setFromArray(array $data) + private function setPropertiesFromArray(array $data) { foreach ($data as $key => $value) { - $method = Str::camel($key); - if (! method_exists($this, $method)) { + $methodName = Str::camel($key); + if (! method_exists($this, $methodName)) { continue; } - $this->{$method}($value); + $this->{$methodName}($value); } } } From 5ff5ee5bb314bbfaaf46df7cfd585df681d193d0 Mon Sep 17 00:00:00 2001 From: antonkomarev Date: Sat, 22 Feb 2020 12:38:26 +0300 Subject: [PATCH 3/7] Protect class properties --- src/RocketChatMessage.php | 28 +++++++++++++++------- src/RocketChatWebhookChannel.php | 4 ++-- tests/RocketChatMessageTest.php | 40 +++++++++++++++++--------------- 3 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/RocketChatMessage.php b/src/RocketChatMessage.php index ff2ee0e..8482728 100644 --- a/src/RocketChatMessage.php +++ b/src/RocketChatMessage.php @@ -6,26 +6,26 @@ class RocketChatMessage { - /** @var string RocketChat channel id. */ - public $channel = ''; + /** @var string|null RocketChat channel id. */ + protected $channel = null; - /** @var string A user or app access token. */ - public $from = ''; + /** @var string|null A user or app access token. */ + protected $from = null; /** @var string The text content of the message. */ - public $content = ''; + protected $content; /** @var string The alias name of the message. */ - public $alias = ''; + protected $alias; /** @var string The avatar emoji of the message. */ - public $emoji = ''; + protected $emoji; /** @var string The avatar image of the message. */ - public $avatar = ''; + protected $avatar; /** @var \NotificationChannels\RocketChat\RocketChatAttachment[] Attachments of the message. */ - public $attachments = []; + protected $attachments = []; /** * Create a new instance of RocketChatMessage. @@ -48,6 +48,16 @@ public function __construct(string $content = '') $this->content($content); } + public function getChannel(): ?string + { + return $this->channel; + } + + public function getFrom(): ?string + { + return $this->from; + } + /** * Set the sender's access token. * diff --git a/src/RocketChatWebhookChannel.php b/src/RocketChatWebhookChannel.php index 407679a..12f171a 100644 --- a/src/RocketChatWebhookChannel.php +++ b/src/RocketChatWebhookChannel.php @@ -40,12 +40,12 @@ public function send($notifiable, Notification $notification): void /** @var \NotificationChannels\RocketChat\RocketChatMessage $message */ $message = $notification->toRocketChat($notifiable); - $to = $message->channel ?: $notifiable->routeNotificationFor('RocketChat'); + $to = $message->getChannel() ?: $notifiable->routeNotificationFor('RocketChat'); if (! $to = $to ?: $this->rocketChat->defaultChannel()) { throw CouldNotSendNotification::missingTo(); } - if (! $from = $message->from ?: $this->rocketChat->token()) { + if (! $from = $message->getFrom() ?: $this->rocketChat->token()) { throw CouldNotSendNotification::missingFrom(); } diff --git a/tests/RocketChatMessageTest.php b/tests/RocketChatMessageTest.php index 5915c3c..1b0cd79 100644 --- a/tests/RocketChatMessageTest.php +++ b/tests/RocketChatMessageTest.php @@ -13,49 +13,49 @@ final class RocketChatMessageTest extends TestCase /** @test */ public function it_can_accept_a_content_when_constructing_a_message(): void { - $message = new RocketChatMessage('hello'); + $message = new RocketChatMessage('test-content'); - $this->assertEquals('hello', $message->content); + $this->assertSame(['text' => 'test-content'], $message->toArray()); } /** @test */ public function it_can_accept_a_content_when_creating_a_message(): void { - $message = RocketChatMessage::make('hello'); + $message = RocketChatMessage::make('test-content'); - $this->assertEquals('hello', $message->content); + $this->assertSame(['text' => 'test-content'], $message->toArray()); } /** @test */ public function it_can_set_the_content(): void { - $message = (new RocketChatMessage())->content('hello'); + $message = (new RocketChatMessage())->content('test-content'); - $this->assertEquals('hello', $message->content); + $this->assertSame(['text' => 'test-content'], $message->toArray()); } /** @test */ public function it_can_set_the_channel(): void { - $message = (new RocketChatMessage())->to('channel'); + $message = (new RocketChatMessage())->to('test-channel'); - $this->assertEquals('channel', $message->channel); + $this->assertSame('test-channel', $message->getChannel()); } /** @test */ public function it_can_set_the_from(): void { - $message = (new RocketChatMessage())->from('token'); + $message = (new RocketChatMessage())->from('test-token'); - $this->assertEquals('token', $message->from); + $this->assertSame('test-token', $message->getFrom()); } /** @test */ public function it_can_set_the_alias(): void { - $message = (new RocketChatMessage())->alias('alias'); + $message = (new RocketChatMessage())->alias('test-alias'); - $this->assertEquals('alias', $message->alias); + $this->assertSame(['alias' => 'test-alias'], $message->toArray()); } /** @test */ @@ -63,7 +63,7 @@ public function it_can_set_the_emoji(): void { $message = (new RocketChatMessage())->emoji(':emoji:'); - $this->assertEquals(':emoji:', $message->emoji); + $this->assertSame(['emoji' => ':emoji:'], $message->toArray()); } /** @test */ @@ -71,7 +71,7 @@ public function it_can_set_the_avatar(): void { $message = (new RocketChatMessage())->avatar('avatar_img'); - $this->assertEquals('avatar_img', $message->avatar); + $this->assertSame(['avatar' => 'avatar_img'], $message->toArray()); } /** @test */ @@ -80,14 +80,15 @@ public function it_can_set_attachment(): void $attachment = RocketChatAttachment::make(['title' => 'test']); $message = (new RocketChatMessage())->attachment($attachment); - $this->assertSame($attachment, $message->attachments[0]); + $this->assertSame($attachment->toArray(), $message->toArray()['attachments'][0]); } /** @test */ public function it_can_set_attachment_as_array(): void { $message = (new RocketChatMessage())->attachment(['title' => 'test']); - $this->assertInstanceOf(RocketChatAttachment::class, $message->attachments[0]); + + $this->assertSame(['title' => 'test'], $message->toArray()['attachments'][0]); } /** @test */ @@ -98,8 +99,8 @@ public function it_can_set_multiple_attachments(): void RocketChatAttachment::make(), RocketChatAttachment::make(), ]); - $this->assertInstanceOf(RocketChatAttachment::class, $message->attachments[0]); - $this->assertCount(3, $message->attachments); + + $this->assertCount(3, $message->toArray()['attachments']); } /** @test */ @@ -112,6 +113,7 @@ public function it_can_clear_attachments(): void ]); $message->clearAttachments(); - $this->assertCount(0, $message->attachments); + + $this->assertArrayNotHasKey('attachments', $message->toArray()); } } From f5ad1b1bc132a19853591122004e6f7d6b698f64 Mon Sep 17 00:00:00 2001 From: antonkomarev Date: Sat, 22 Feb 2020 13:01:20 +0300 Subject: [PATCH 4/7] Don't return ResponseInterface to higher levels, add get prefix to accessor methods, remove url accessor --- README.md | 4 +- src/RocketChat.php | 27 ++++--------- src/RocketChatAttachment.php | 56 +++++++++++++------------- src/RocketChatWebhookChannel.php | 11 +++-- tests/RocketChatWebhookChannelTest.php | 10 ++--- 5 files changed, 48 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 4dbca24..2962d6c 100644 --- a/README.md +++ b/README.md @@ -101,7 +101,7 @@ public function routeNotificationForRocketChat(): string `alias()`: This will cause the message’s name to appear as the given alias, but your username will still display. -`emoji()`: This will make the avatar on this message be an emoji. (e.g. ":see_no_evil:") +`emoji()`: This will make the avatar on this message be an emoji. (e.g. ':see_no_evil:') `avatar()`: This will make the avatar use the provided image url. @@ -118,7 +118,7 @@ There are several ways to add one ore more attachments to a message ```php public function toRocketChat($notifiable) { - return RocketChatMessage::make("Test message") + return RocketChatMessage::make('Test message') ->to('channel_name') // optional if set in config ->from('webhook_token') // optional if set in config ->attachments([ diff --git a/src/RocketChat.php b/src/RocketChat.php index 47791dd..c4a1453 100644 --- a/src/RocketChat.php +++ b/src/RocketChat.php @@ -5,7 +5,6 @@ namespace NotificationChannels\RocketChat; use GuzzleHttp\Client as HttpClient; -use Psr\Http\Message\ResponseInterface; final class RocketChat { @@ -36,22 +35,12 @@ public function __construct(HttpClient $http, string $url, string $token, ?strin $this->defaultChannel = $defaultChannel; } - /** - * Returns RocketChat base url. - * - * @return string - */ - public function url(): string - { - return $this->url; - } - /** * Returns RocketChat token. * * @return string */ - public function token(): string + public function getToken(): string { return $this->token; } @@ -61,7 +50,7 @@ public function token(): string * * @return string|null */ - public function defaultChannel(): ?string + public function getDefaultChannel(): ?string { return $this->defaultChannel; } @@ -71,13 +60,13 @@ public function defaultChannel(): ?string * * @param string $to * @param array $message - * @return \Psr\Http\Message\ResponseInterface + * @return void */ - public function sendMessage(string $to, array $message): ResponseInterface + public function sendMessage(string $to, array $message): void { $url = sprintf('%s/hooks/%s', $this->url, $this->token); - return $this->post($url, [ + $this->post($url, [ 'json' => array_merge($message, [ 'channel' => $to, ]), @@ -89,10 +78,10 @@ public function sendMessage(string $to, array $message): ResponseInterface * * @param string $url * @param array $options - * @return \Psr\Http\Message\ResponseInterface + * @return void */ - private function post(string $url, array $options): ResponseInterface + private function post(string $url, array $options): void { - return $this->http->post($url, $options); + $this->http->post($url, $options); } } diff --git a/src/RocketChatAttachment.php b/src/RocketChatAttachment.php index 6324dd6..08bb399 100644 --- a/src/RocketChatAttachment.php +++ b/src/RocketChatAttachment.php @@ -11,50 +11,50 @@ class RocketChatAttachment { - /** @var string The color you want the order on the left side to be, any value background-css supports. */ - protected $color = ''; + /** @var string|null The color you want the order on the left side to be, any value background-css supports. */ + protected $color; - /** @var string The text to display for this attachment, it is different than the message’s text. */ - protected $text = ''; + /** @var string|null The text to display for this attachment, it is different than the message’s text. */ + protected $text; - /** @var string Displays the time next to the text portion. */ - protected $timestamp = ''; + /** @var string|null Displays the time next to the text portion. */ + protected $timestamp; - /** @var string An image that displays to the left of the text, looks better when this is relatively small. */ - protected $thumbnailUrl = ''; + /** @var string|null An image that displays to the left of the text, looks better when this is relatively small. */ + protected $thumbnailUrl; - /** @var string Only applicable if the ts is provided, as it makes the time clickable to this link. */ - protected $messageLink = ''; + /** @var string|null Only applicable if the ts is provided, as it makes the time clickable to this link. */ + protected $messageLink; /** @var bool Causes the image, audio, and video sections to be hiding when collapsed is true. */ protected $collapsed = false; - /** @var string Name of the author. */ - protected $authorName = ''; + /** @var string|null Name of the author. */ + protected $authorName; - /** @var string Providing this makes the author name clickable and points to this link. */ - protected $authorLink = ''; + /** @var string|null Providing this makes the author name clickable and points to this link. */ + protected $authorLink; - /** @var string Displays a tiny icon to the left of the Author’s name. */ - protected $authorIcon = ''; + /** @var string|null Displays a tiny icon to the left of the Author’s name. */ + protected $authorIcon; - /** @var string Title to display for this attachment, displays under the author. */ - protected $title = ''; + /** @var string|null Title to display for this attachment, displays under the author. */ + protected $title; - /** @var string Providing this makes the title clickable, pointing to this link. */ - protected $titleLink = ''; + /** @var string|null Providing this makes the title clickable, pointing to this link. */ + protected $titleLink; /** @var bool When this is true, a download icon appears and clicking this saves the link to file. */ protected $titleLinkDownload = false; - /** @var string The image to display, will be “big” and easy to see. */ - protected $imageUrl = ''; + /** @var string|null The image to display, will be “big” and easy to see. */ + protected $imageUrl; - /** @var string Audio file to play, only supports what html audio does. */ - protected $audioUrl = ''; + /** @var string|null Audio file to play, only supports what html audio does. */ + protected $audioUrl; - /** @var string Video file to play, only supports what html video does. */ - protected $videoUrl = ''; + /** @var string|null Video file to play, only supports what html video does. */ + protected $videoUrl; /** @var array An array of Attachment Field Objects. */ protected $fields = []; @@ -73,7 +73,7 @@ public function __construct(array $data = []) * Create a new instance of RocketChatAttachment. * * @param array $data - * @return RocketChatAttachment + * @return \NotificationChannels\RocketChat\RocketChatAttachment */ public static function make(array $data = []) { @@ -82,7 +82,7 @@ public static function make(array $data = []) /** * @param string $color - * @return RocketChatAttachment + * @return \NotificationChannels\RocketChat\RocketChatAttachment */ public function color(string $color): self { diff --git a/src/RocketChatWebhookChannel.php b/src/RocketChatWebhookChannel.php index 12f171a..e08eab7 100644 --- a/src/RocketChatWebhookChannel.php +++ b/src/RocketChatWebhookChannel.php @@ -8,7 +8,6 @@ use GuzzleHttp\Exception\ClientException; use Illuminate\Notifications\Notification; use NotificationChannels\RocketChat\Exceptions\CouldNotSendNotification; -use Psr\Http\Message\ResponseInterface; final class RocketChatWebhookChannel { @@ -41,11 +40,11 @@ public function send($notifiable, Notification $notification): void $message = $notification->toRocketChat($notifiable); $to = $message->getChannel() ?: $notifiable->routeNotificationFor('RocketChat'); - if (! $to = $to ?: $this->rocketChat->defaultChannel()) { + if (! $to = $to ?: $this->rocketChat->getDefaultChannel()) { throw CouldNotSendNotification::missingTo(); } - if (! $from = $message->getFrom() ?: $this->rocketChat->token()) { + if (! $from = $message->getFrom() ?: $this->rocketChat->getToken()) { throw CouldNotSendNotification::missingFrom(); } @@ -61,10 +60,10 @@ public function send($notifiable, Notification $notification): void /** * @param string $to * @param \NotificationChannels\RocketChat\RocketChatMessage $message - * @return \Psr\Http\Message\ResponseInterface + * @return void */ - private function sendMessage(string $to, RocketChatMessage $message): ResponseInterface + private function sendMessage(string $to, RocketChatMessage $message): void { - return $this->rocketChat->sendMessage($to, $message->toArray()); + $this->rocketChat->sendMessage($to, $message->toArray()); } } diff --git a/tests/RocketChatWebhookChannelTest.php b/tests/RocketChatWebhookChannelTest.php index 1ddc984..ccc20b3 100644 --- a/tests/RocketChatWebhookChannelTest.php +++ b/tests/RocketChatWebhookChannelTest.php @@ -4,7 +4,7 @@ namespace NotificationChannels\RocketChat\Test; -use GuzzleHttp\Client; +use GuzzleHttp\Client as GuzzleHttpClient; use GuzzleHttp\Psr7\Response; use Illuminate\Notifications\Notifiable; use Illuminate\Notifications\Notification; @@ -23,7 +23,7 @@ final class RocketChatWebhookChannelTest extends TestCase /** @test */ public function it_can_send_a_notification(): void { - $client = Mockery::mock(Client::class); + $client = Mockery::mock(GuzzleHttpClient::class); $apiBaseUrl = 'http://localhost:3000'; $token = ':token'; @@ -48,7 +48,7 @@ public function it_can_send_a_notification(): void /** @test */ public function it_handles_generic_errors(): void { - $client = Mockery::mock(Client::class); + $client = Mockery::mock(GuzzleHttpClient::class); $this->expectException(CouldNotSendNotification::class); $apiBaseUrl = 'http://localhost:3000'; @@ -76,7 +76,7 @@ public function it_does_not_send_a_message_when_channel_missed(): void { $this->expectException(CouldNotSendNotification::class); - $rocketChat = new RocketChat(new Client(), '', '', ''); + $rocketChat = new RocketChat(new GuzzleHttpClient(), '', '', ''); $channel = new RocketChatWebhookChannel($rocketChat); $channel->send(new TestNotifiable(), new TestNotificationWithMissedChannel()); } @@ -86,7 +86,7 @@ public function it_does_not_send_a_message_when_from_missed(): void { $this->expectException(CouldNotSendNotification::class); - $rocketChat = new RocketChat(new Client(), '', '', ''); + $rocketChat = new RocketChat(new GuzzleHttpClient(), '', '', ''); $channel = new RocketChatWebhookChannel($rocketChat); $channel->send(new TestNotifiable(), new TestNotificationWithMissedFrom()); } From 184b05521896d81bbd5fda67c3264a2fc66d3960 Mon Sep 17 00:00:00 2001 From: antonkomarev Date: Sat, 22 Feb 2020 13:32:49 +0300 Subject: [PATCH 5/7] Make exceptions message more explicit --- src/Exceptions/CouldNotSendNotification.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Exceptions/CouldNotSendNotification.php b/src/Exceptions/CouldNotSendNotification.php index 06f01f0..f90bd7f 100644 --- a/src/Exceptions/CouldNotSendNotification.php +++ b/src/Exceptions/CouldNotSendNotification.php @@ -17,7 +17,7 @@ final class CouldNotSendNotification extends RuntimeException */ public static function missingTo(): self { - return new static('Notification was not sent. Channel identifier is missing.'); + return new static('RocketChat notification was not sent. Channel identifier is missing.'); } /** @@ -27,7 +27,7 @@ public static function missingTo(): self */ public static function missingFrom(): self { - return new static('Notification was not sent. Access token is missing.'); + return new static('RocketChat notification was not sent. Access token is missing.'); } /** From b3cc1fa2f59c7940356e8e104ee125fedb389d4d Mon Sep 17 00:00:00 2001 From: antonkomarev Date: Sat, 22 Feb 2020 14:27:14 +0300 Subject: [PATCH 6/7] Add missing return type --- src/RocketChatAttachment.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/RocketChatAttachment.php b/src/RocketChatAttachment.php index 08bb399..5ae7da8 100644 --- a/src/RocketChatAttachment.php +++ b/src/RocketChatAttachment.php @@ -310,8 +310,9 @@ public function toArray(): array * Set attachment data from array. * * @param array $data + * @return void */ - private function setPropertiesFromArray(array $data) + private function setPropertiesFromArray(array $data): void { foreach ($data as $key => $value) { $methodName = Str::camel($key); From 2868f7c08ca52452395db9aa245691ce6d0d4366 Mon Sep 17 00:00:00 2001 From: antonkomarev Date: Sat, 22 Feb 2020 14:40:28 +0300 Subject: [PATCH 7/7] Code style fix --- src/RocketChatAttachment.php | 3 +++ src/RocketChatWebhookChannel.php | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/RocketChatAttachment.php b/src/RocketChatAttachment.php index 5ae7da8..75db712 100644 --- a/src/RocketChatAttachment.php +++ b/src/RocketChatAttachment.php @@ -116,6 +116,7 @@ public function timestamp($timestamp): self $date = clone $timestamp; $timestamp = $date->setTimezone(new DateTimeZone('UTC'))->format('Y-m-d\TH:i:s.v\Z'); } + $this->timestamp = $timestamp; return $this; @@ -316,9 +317,11 @@ private function setPropertiesFromArray(array $data): void { foreach ($data as $key => $value) { $methodName = Str::camel($key); + if (! method_exists($this, $methodName)) { continue; } + $this->{$methodName}($value); } } diff --git a/src/RocketChatWebhookChannel.php b/src/RocketChatWebhookChannel.php index e08eab7..a11e41a 100644 --- a/src/RocketChatWebhookChannel.php +++ b/src/RocketChatWebhookChannel.php @@ -40,11 +40,13 @@ public function send($notifiable, Notification $notification): void $message = $notification->toRocketChat($notifiable); $to = $message->getChannel() ?: $notifiable->routeNotificationFor('RocketChat'); - if (! $to = $to ?: $this->rocketChat->getDefaultChannel()) { + $to = $to ?: $this->rocketChat->getDefaultChannel(); + if ($to === null) { throw CouldNotSendNotification::missingTo(); } - if (! $from = $message->getFrom() ?: $this->rocketChat->getToken()) { + $from = $message->getFrom() ?: $this->rocketChat->getToken(); + if (! $from) { throw CouldNotSendNotification::missingFrom(); }