Merge pull request #10 from laravel-notification-channels/dev

Add PHP 8.0, 8.1 support; Improve error logging with additional metadata
laravel-notification-channels · Jan 4, 2022 · e327a73 · e327a73
2 parents 42e0bac + 395f266
commit e327a73
Show file tree

Hide file tree

Showing 11 changed files with 133 additions and 33 deletions.
diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml
@@ -0,0 +1,32 @@
+name: Unit test coverage
+
+on:
+  - push
+  - pull_request
+
+jobs:
+  tests:
+    runs-on: ubuntu-latest
+    name: Unit test coverage
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v2
+
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: 7.4
+          tools: composer:v2
+          coverage: pcov
+
+      - name: Install dependencies
+        run: composer update --prefer-stable --prefer-dist --no-interaction --no-progress
+
+      - name: Execute tests
+        run: vendor/bin/phpunit --verbose --coverage-text --coverage-clover=coverage.clover
+
+      - name: Send coverage to Scrutinizer
+        run: |
+          wget https://scrutinizer-ci.com/ocular.phar
+          php ocular.phar code-coverage:upload --format=php-clover coverage.clover
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -0,0 +1,32 @@
+name: PHPUnit tests
+
+on:
+  - push
+  - pull_request
+
+jobs:
+  tests:
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: true
+      matrix:
+        php: [7.2, 7.3, 7.4, 8.0, 8.1]
+
+    name: Tests on PHP ${{ matrix.php }} - ${{ matrix.stability }}
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v2
+
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php }}
+          tools: composer:v2
+          coverage: none
+
+      - name: Install dependencies
+        run: composer update --prefer-source --no-interaction --no-progress
+
+      - name: Execute tests
+        run: vendor/bin/phpunit --verbose
diff --git a/.travis.yml b/.travis.yml
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,15 @@
 # Changelog
 
-All notable changes to the `InterFAX notification channel` will be documented in this file
+All notable changes to the `InterFAX notification channel` will be documented in this file.
+
+## 2.1.1 - 2021-05-12
+
+- Prevent API limit requests with updates to status polling
+- Update exception handling for better error messages from InterFAX client
+
+## 2.1.0 - 2020-11-10
+
+- Add Laravel 8 support
 
 ## 2.0.0 - 2020-07-15
 

diff --git a/README.md b/README.md
@@ -110,6 +110,7 @@ public function routeNotificationForInterfax($notification)
 `file(string $file)` : Accepts the full path to a single file (full list of supported file types [found here](https://www.interfax.net/en/help/supported_file_types)).  
 `files(array $array)` : Accepts an array of file paths.  
 `stream(Filestream $stream, string $name)` : Accepts a file stream.
+`addMetadata(array $data)`: Add metadata for logging purposes in case of an error.
 
 ## Changelog
 

diff --git a/composer.json b/composer.json
@@ -12,7 +12,7 @@
         }
     ],
     "require": {
-        "php": "^7.2",
+        "php": "^7.2|^8.0",
         "illuminate/notifications": "^6.0|^7.0|^8.0",
         "illuminate/support": "^6.0|^7.0|^8.0",
         "interfax/interfax": "^1.1"

diff --git a/src/Exceptions/CouldNotSendNotification.php b/src/Exceptions/CouldNotSendNotification.php
@@ -28,6 +28,11 @@ public function getUser()
         return $this->interfaxMessage->user;
     }
 
+    public function getMetadata()
+    {
+        return $this->interfaxMessage->metadata;
+    }
+
     public function getAttributes()
     {
         return $this->responseAttributes;

diff --git a/src/InterfaxChannel.php b/src/InterfaxChannel.php
@@ -9,6 +9,7 @@
 class InterfaxChannel
 {
     protected $client;
+    protected $fax;
 
     public function __construct(Client $client)
     {
@@ -18,8 +19,8 @@ public function __construct(Client $client)
     /**
      * Send the given notification.
      *
-     * @param mixed $notifiable
-     * @param \Illuminate\Notifications\Notification $notification
+     * @param  mixed  $notifiable
+     * @param  \Illuminate\Notifications\Notification  $notification
      *
      * @throws \NotificationChannels\Interfax\Exceptions\CouldNotSendNotification
      */
@@ -32,25 +33,27 @@ public function send($notifiable, Notification $notification)
         $message = $notification->toInterfax($notifiable);
 
         try {
-            $fax = $this->client->deliver([
+            $this->fax = $this->client->deliver([
                 'faxNumber' => $faxNumber,
                 'files' => $message->makeFiles(),
             ]);
 
             if ($message->shouldCheckStatus()) {
                 $message->sleep();
 
-                while ($fax->refresh()->status < 0) {
+                while ($this->fax->refresh()->status < 0) {
                     $message->sleep();
                 }
 
-                if ($fax->refresh()->status > 0) {
-                    throw CouldNotSendNotification::serviceRespondedWithAnError($message, $fax->refresh()->attributes());
+                if ($this->fax->refresh()->status > 0) {
+                    throw CouldNotSendNotification::serviceRespondedWithAnError($message, $this->fax->attributes());
                 }
             }
         } catch (\Interfax\Exception\RequestException $e) {
             $exceptionMessage = $e->getMessage().': '.($e->getWrappedException())->getMessage();
-            throw CouldNotSendNotification::serviceRespondedWithAnError($message, $fax->refresh()->attributes(), $exceptionMessage);
+            $attributes = $this->fax ? $this->fax->attributes() : ['status' => $e->getStatusCode()];
+
+            throw CouldNotSendNotification::serviceRespondedWithAnError($message, $attributes, $exceptionMessage);
         }
     }
 }
diff --git a/src/InterfaxMessage.php b/src/InterfaxMessage.php
@@ -12,6 +12,7 @@ class InterfaxMessage
     protected $method;
     protected $statusCheck = false;
     public $user;
+    public $metadata = [];
 
     const FILES = 'files';
     const STREAM = 'stream';
@@ -58,7 +59,8 @@ public function shouldCheckStatus(): bool
 
     /**
      * Set a user who can be notified in case the fax fails to send.
-     * @param  mixed $notifiable [description]
+     *
+     * @param  mixed  $notifiable  The user to notify
      * @return InterfaxMessage
      */
     public function user($notifiable)
@@ -68,6 +70,21 @@ public function user($notifiable)
         return $this;
     }
 
+    /**
+     * Add metadata to the message for logging purposes.
+     *
+     * @param  array  $data  The data to add to the metadata array
+     * @return InterfaxMessage
+     */
+    public function addMetadata(array $data)
+    {
+        foreach ($data as $key => $value) {
+            $this->metadata[$key] = $value;
+        }
+
+        return $this;
+    }
+
     public function makeFiles(): array
     {
         if ($this->method === static::STREAM) {

diff --git a/tests/CouldNotSendNotificationExceptionTest.php b/tests/CouldNotSendNotificationExceptionTest.php
@@ -13,6 +13,7 @@ public function setUp(): void
     {
         parent::setUp();
         $this->message = (new InterfaxMessage)
+                            ->addMetadata(['key' => 'Some sample metadata.'])
                             ->checkStatus()
                             ->user(new TestNotifiable)
                             ->file('test-file.pdf');
@@ -29,6 +30,17 @@ public function it_can_get_the_exception_user()
         $this->assertInstanceOf(TestNotifiable::class, $exception->getUser());
     }
 
+    /** @test */
+    public function it_can_get_the_exception_metadata()
+    {
+        $exception = CouldNotSendNotification::serviceRespondedWithAnError($this->message, [
+            'status' => 500,
+            'message' => 'Failed to send.',
+        ]);
+
+        $this->assertSame('Some sample metadata.', $exception->getMetadata()['key']);
+    }
+
     /** @test */
     public function it_can_get_the_default_exception_message()
     {

diff --git a/tests/InterfaxChannelTest.php b/tests/InterfaxChannelTest.php
@@ -152,9 +152,15 @@ public function it_can_throw_the_exception()
     {
         $this->expectException(CouldNotSendNotification::class);
 
+        $testResource = new TestResource;
+
         $this->resource
              ->shouldReceive('refresh')
-             ->andReturn(new TestResource);
+             ->andReturn($testResource);
+
+        $this->resource
+             ->shouldReceive('attributes')
+             ->andReturn($testResource->attributes());
 
         $this->interfax
              ->shouldReceive('deliver')
@@ -169,6 +175,7 @@ class TestNotificationWithSingleFile extends Notification
     /**
      * @param $notifiable
      * @return InterfaxMessage
+     *
      * @throws CouldNotSendNotification
      */
     public function toInterfax($notifiable)
@@ -184,6 +191,7 @@ class TestNotificationWithFiles extends Notification
     /**
      * @param $notifiable
      * @return InterfaxMessage
+     *
      * @throws CouldNotSendNotification
      */
     public function toInterfax($notifiable)
@@ -199,6 +207,7 @@ class TestNotificationAsStreamPdf extends Notification
     /**
      * @param $notifiable
      * @return InterfaxMessage
+     *
      * @throws CouldNotSendNotification
      */
     public function toInterfax($notifiable)
@@ -216,6 +225,7 @@ class TestNotificationAsStreamHtml extends Notification
     /**
      * @param $notifiable
      * @return InterfaxMessage
+     *
      * @throws CouldNotSendNotification
      */
     public function toInterfax($notifiable)
@@ -233,6 +243,7 @@ class TestNotificationWithRefresh extends Notification
     /**
      * @param $notifiable
      * @return InterfaxMessage
+     *
      * @throws CouldNotSendNotification
      */
     public function toInterfax($notifiable)