Merge pull request #12903 from marcusmoore/bug/sc-15034

Fixes sending webhook notifications for checkout and checkin

app/Listeners/CheckoutableListener.php

@@ -2,22 +2,21 @@
 
 namespace App\Listeners;
 
+use App\Events\CheckoutableCheckedOut;
 use App\Models\Accessory;
 use App\Models\Asset;
 use App\Models\CheckoutAcceptance;
+use App\Models\Component;
 use App\Models\Consumable;
 use App\Models\LicenseSeat;
 use App\Models\Recipients\AdminRecipient;
 use App\Models\Setting;
-use App\Models\User;
 use App\Notifications\CheckinAccessoryNotification;
 use App\Notifications\CheckinAssetNotification;
-use App\Notifications\CheckinLicenseNotification;
 use App\Notifications\CheckinLicenseSeatNotification;
 use App\Notifications\CheckoutAccessoryNotification;
 use App\Notifications\CheckoutAssetNotification;
 use App\Notifications\CheckoutConsumableNotification;
-use App\Notifications\CheckoutLicenseNotification;
 use App\Notifications\CheckoutLicenseSeatNotification;
 use Illuminate\Support\Facades\Notification;
 use Exception;
@@ -25,18 +24,17 @@ use Log;
 
 class CheckoutableListener
 {
+    private array $skipNotificationsFor = [
+        Component::class,
+    ];
+
     /**
-     * Notify the user about the checked out checkoutable and add a record to the
+     * Notify the user and post to webhook about the checked out checkoutable
-     * checkout_requests table.
+     * and add a record to the checkout_requests table.
      */
     public function onCheckedOut($event)
     {
-
+        if ($this->shouldNotSendAnyNotifications($event->checkoutable)){
-
-        /**
-         * When the item wasn't checked out to a user, we can't send notifications
-         */
-        if (! $event->checkedOutTo instanceof User) {
             return;
         }
 
@@ -46,6 +44,11 @@ class CheckoutableListener
         $acceptance = $this->getCheckoutAcceptance($event);       
 
         try {
+            if ($this->shouldSendWebhookNotification()) {
+                Notification::route('slack', Setting::getSettings()->webhook_endpoint)
+                    ->notify($this->getCheckoutNotification($event));
+            }
+
             if (! $event->checkedOutTo->locale) {
                 Notification::locale(Setting::getSettings()->locale)->send(
                     $this->getNotifiables($event),
@@ -63,16 +66,13 @@ class CheckoutableListener
     }
 
     /**
-     * Notify the user about the checked in checkoutable
+     * Notify the user and post to webhook about the checked in checkoutable
      */    
     public function onCheckedIn($event)
     {
         \Log::debug('onCheckedIn in the Checkoutable listener fired');
 
-        /**
+        if ($this->shouldNotSendAnyNotifications($event->checkoutable)) {
-         * When the item wasn't checked out to a user, we can't send notifications
-         */
-        if (! $event->checkedOutTo instanceof User) {
             return;
         }
 
@@ -90,6 +90,11 @@ class CheckoutableListener
         }
 
         try {
+            if ($this->shouldSendWebhookNotification()) {
+                Notification::route('slack', Setting::getSettings()->webhook_endpoint)
+                    ->notify($this->getCheckinNotification($event));
+            }
+
             // Use default locale
             if (! $event->checkedOutTo->locale) {
                 Notification::locale(Setting::getSettings()->locale)->send(
@@ -182,11 +187,11 @@ class CheckoutableListener
     /**
      * Get the appropriate notification for the event
      * 
-     * @param  CheckoutableCheckedIn $event 
+     * @param  CheckoutableCheckedOut $event
-     * @param  CheckoutAcceptance $acceptance 
+     * @param  CheckoutAcceptance|null $acceptance
      * @return Notification
      */
-    private function getCheckoutNotification($event, $acceptance)
+    private function getCheckoutNotification($event, $acceptance = null)
     {
         $notificationClass = null;
 
@@ -225,4 +230,14 @@ class CheckoutableListener
             'App\Listeners\CheckoutableListener@onCheckedOut'
         ); 
     }
+
+    private function shouldNotSendAnyNotifications($checkoutable): bool
+    {
+        return in_array(get_class($checkoutable), $this->skipNotificationsFor);
+    }
+
+    private function shouldSendWebhookNotification(): bool
+    {
+        return Setting::getSettings() && Setting::getSettings()->webhook_endpoint;
+    }
 }

app/Models/LicenseSeat.php

@@ -6,13 +6,15 @@ use App\Models\Traits\Acceptable;
 use App\Notifications\CheckinLicenseNotification;
 use App\Notifications\CheckoutLicenseNotification;
 use App\Presenters\Presentable;
+use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\SoftDeletes;
 
 class LicenseSeat extends SnipeModel implements ICompanyableChild
 {
     use CompanyableChildTrait;
-    use SoftDeletes;
+    use HasFactory;
     use Loggable;
+    use SoftDeletes;
 
     protected $presenter = \App\Presenters\LicenseSeatPresenter::class;
     use Presentable;

app/Models/User.php

@@ -259,20 +259,6 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
         return $this->last_name.', '.$this->first_name.' ('.$this->username.')';
     }
 
-    /**
-     * The url for slack notifications.
-     * Used by Notifiable trait.
-     * @return mixed
-     */
-    public function routeNotificationForSlack()
-    {
-        // At this point the endpoint is the same for everything.
-        //  In the future this may want to be adapted for individual notifications.
-        $this->endpoint = \App\Models\Setting::getSettings()->webhook_endpoint;
-
-        return $this->endpoint;
-    }
-
 
     /**
      * Establishes the user -> assets relationship

database/factories/LicenseSeatFactory.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace Database\Factories;
+
+use App\Models\License;
+use Illuminate\Database\Eloquent\Factories\Factory;
+
+class LicenseSeatFactory extends Factory
+{
+    public function definition()
+    {
+        return [
+            'license_id' => License::factory(),
+        ];
+    }
+}

tests/Feature/Notifications/AccessoryWebhookTest.php

@@ -0,0 +1,96 @@
+<?php
+
+namespace Tests\Feature\Notifications;
+
+use App\Events\CheckoutableCheckedIn;
+use App\Events\CheckoutableCheckedOut;
+use App\Models\Accessory;
+use App\Models\Setting;
+use App\Models\User;
+use App\Notifications\CheckinAccessoryNotification;
+use App\Notifications\CheckoutAccessoryNotification;
+use Illuminate\Notifications\AnonymousNotifiable;
+use Illuminate\Support\Facades\Notification;
+use Tests\Support\InteractsWithSettings;
+use Tests\TestCase;
+
+class AccessoryWebhookTest extends TestCase
+{
+    use InteractsWithSettings;
+
+    public function testAccessoryCheckoutSendsWebhookNotificationWhenSettingEnabled()
+    {
+        Notification::fake();
+
+        $this->settings->enableWebhook();
+
+        event(new CheckoutableCheckedOut(
+            Accessory::factory()->appleBtKeyboard()->create(),
+            User::factory()->create(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertSentTo(
+            new AnonymousNotifiable,
+            CheckoutAccessoryNotification::class,
+            function ($notification, $channels, $notifiable) {
+                return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint;
+            }
+        );
+    }
+
+    public function testAccessoryCheckoutDoesNotSendWebhookNotificationWhenSettingDisabled()
+    {
+        Notification::fake();
+
+        $this->settings->disableWebhook();
+
+        event(new CheckoutableCheckedOut(
+            Accessory::factory()->appleBtKeyboard()->create(),
+            User::factory()->create(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutAccessoryNotification::class);
+    }
+
+    public function testAccessoryCheckinSendsWebhookNotificationWhenSettingEnabled()
+    {
+        Notification::fake();
+
+        $this->settings->enableWebhook();
+
+        event(new CheckoutableCheckedIn(
+            Accessory::factory()->appleBtKeyboard()->create(),
+            User::factory()->create(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertSentTo(
+            new AnonymousNotifiable,
+            CheckinAccessoryNotification::class,
+            function ($notification, $channels, $notifiable) {
+                return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint;
+            }
+        );
+    }
+
+    public function testAccessoryCheckinDoesNotSendWebhookNotificationWhenSettingDisabled()
+    {
+        Notification::fake();
+
+        $this->settings->disableWebhook();
+
+        event(new CheckoutableCheckedIn(
+            Accessory::factory()->appleBtKeyboard()->create(),
+            User::factory()->create(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertNotSentTo(new AnonymousNotifiable, CheckinAccessoryNotification::class);
+    }
+}

tests/Feature/Notifications/AssetWebhookTest.php

@@ -0,0 +1,115 @@
+<?php
+
+namespace Tests\Feature\Notifications;
+
+use App\Events\CheckoutableCheckedIn;
+use App\Events\CheckoutableCheckedOut;
+use App\Models\Asset;
+use App\Models\Location;
+use App\Models\Setting;
+use App\Models\User;
+use App\Notifications\CheckinAssetNotification;
+use App\Notifications\CheckoutAssetNotification;
+use Illuminate\Notifications\AnonymousNotifiable;
+use Illuminate\Support\Facades\Notification;
+use Tests\Support\InteractsWithSettings;
+use Tests\TestCase;
+
+class AssetWebhookTest extends TestCase
+{
+    use InteractsWithSettings;
+
+    public function targets(): array
+    {
+        return [
+            'Asset checked out to user' => [fn() => User::factory()->create()],
+            'Asset checked out to asset' => [fn() => $this->createAsset()],
+            'Asset checked out to location' => [fn() => Location::factory()->create()],
+        ];
+    }
+
+    /** @dataProvider targets */
+    public function testAssetCheckoutSendsWebhookNotificationWhenSettingEnabled($checkoutTarget)
+    {
+        Notification::fake();
+
+        $this->settings->enableWebhook();
+
+        event(new CheckoutableCheckedOut(
+            $this->createAsset(),
+            $checkoutTarget(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertSentTo(
+            new AnonymousNotifiable,
+            CheckoutAssetNotification::class,
+            function ($notification, $channels, $notifiable) {
+                return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint;
+            }
+        );
+    }
+
+    /** @dataProvider targets */
+    public function testAssetCheckoutDoesNotSendWebhookNotificationWhenSettingDisabled($checkoutTarget)
+    {
+        Notification::fake();
+
+        $this->settings->disableWebhook();
+
+        event(new CheckoutableCheckedOut(
+            $this->createAsset(),
+            $checkoutTarget(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutAssetNotification::class);
+    }
+
+    /** @dataProvider targets */
+    public function testAssetCheckinSendsWebhookNotificationWhenSettingEnabled($checkoutTarget)
+    {
+        Notification::fake();
+
+        $this->settings->enableWebhook();
+
+        event(new CheckoutableCheckedIn(
+            $this->createAsset(),
+            $checkoutTarget(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertSentTo(
+            new AnonymousNotifiable,
+            CheckinAssetNotification::class,
+            function ($notification, $channels, $notifiable) {
+                return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint;
+            }
+        );
+    }
+
+    /** @dataProvider targets */
+    public function testAssetCheckinDoesNotSendWebhookNotificationWhenSettingDisabled($checkoutTarget)
+    {
+        Notification::fake();
+
+        $this->settings->disableWebhook();
+
+        event(new CheckoutableCheckedIn(
+            $this->createAsset(),
+            $checkoutTarget(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertNotSentTo(new AnonymousNotifiable, CheckinAssetNotification::class);
+    }
+
+    private function createAsset()
+    {
+        return Asset::factory()->laptopMbp()->create();
+    }
+}

tests/Feature/Notifications/ComponentWebhookTest.php

@@ -0,0 +1,50 @@
+<?php
+
+namespace Tests\Feature\Notifications;
+
+use App\Events\CheckoutableCheckedIn;
+use App\Events\CheckoutableCheckedOut;
+use App\Models\Asset;
+use App\Models\Component;
+use App\Models\User;
+use Illuminate\Support\Facades\Notification;
+use Tests\Support\InteractsWithSettings;
+use Tests\TestCase;
+
+class ComponentWebhookTest extends TestCase
+{
+
+    use InteractsWithSettings;
+
+    public function testComponentCheckoutDoesNotSendWebhookNotification()
+    {
+        Notification::fake();
+
+        $this->settings->enableWebhook();
+
+        event(new CheckoutableCheckedOut(
+            Component::factory()->ramCrucial8()->create(),
+            Asset::factory()->laptopMbp()->create(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertNothingSent();
+    }
+
+    public function testComponentCheckinDoesNotSendWebhookNotification()
+    {
+        Notification::fake();
+
+        $this->settings->enableWebhook();
+
+        event(new CheckoutableCheckedIn(
+            Component::factory()->ramCrucial8()->create(),
+            Asset::factory()->laptopMbp()->create(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertNothingSent();
+    }
+}

tests/Feature/Notifications/ConsumableWebhookTest.php

@@ -0,0 +1,56 @@
+<?php
+
+namespace Tests\Feature\Notifications;
+
+use App\Events\CheckoutableCheckedOut;
+use App\Models\Consumable;
+use App\Models\Setting;
+use App\Models\User;
+use App\Notifications\CheckoutConsumableNotification;
+use Illuminate\Notifications\AnonymousNotifiable;
+use Illuminate\Support\Facades\Notification;
+use Tests\Support\InteractsWithSettings;
+use Tests\TestCase;
+
+class ConsumableWebhookTest extends TestCase
+{
+    use InteractsWithSettings;
+
+    public function testConsumableCheckoutSendsWebhookNotificationWhenSettingEnabled()
+    {
+        Notification::fake();
+
+        $this->settings->enableWebhook();
+
+        event(new CheckoutableCheckedOut(
+            Consumable::factory()->cardstock()->create(),
+            User::factory()->create(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertSentTo(
+            new AnonymousNotifiable,
+            CheckoutConsumableNotification::class,
+            function ($notification, $channels, $notifiable) {
+                return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint;
+            }
+        );
+    }
+
+    public function testConsumableCheckoutDoesNotSendWebhookNotificationWhenSettingDisabled()
+    {
+        Notification::fake();
+
+        $this->settings->disableWebhook();
+
+        event(new CheckoutableCheckedOut(
+            Consumable::factory()->cardstock()->create(),
+            User::factory()->create(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutConsumableNotification::class);
+    }
+}

tests/Feature/Notifications/LicenseWebhookTest.php

@@ -0,0 +1,109 @@
+<?php
+
+namespace Tests\Feature\Notifications;
+
+use App\Events\CheckoutableCheckedIn;
+use App\Events\CheckoutableCheckedOut;
+use App\Models\Asset;
+use App\Models\LicenseSeat;
+use App\Models\Setting;
+use App\Models\User;
+use App\Notifications\CheckinLicenseSeatNotification;
+use App\Notifications\CheckoutLicenseSeatNotification;
+use Illuminate\Notifications\AnonymousNotifiable;
+use Illuminate\Support\Facades\Notification;
+use Tests\Support\InteractsWithSettings;
+use Tests\TestCase;
+
+class LicenseWebhookTest extends TestCase
+{
+    use InteractsWithSettings;
+
+    public function targets(): array
+    {
+        return [
+            'License checked out to user' => [fn() => User::factory()->create()],
+            'License checked out to asset' => [fn() => Asset::factory()->laptopMbp()->create()],
+        ];
+    }
+
+    /** @dataProvider targets */
+    public function testLicenseCheckoutSendsWebhookNotificationWhenSettingEnabled($checkoutTarget)
+    {
+        Notification::fake();
+
+        $this->settings->enableWebhook();
+
+        event(new CheckoutableCheckedOut(
+            LicenseSeat::factory()->create(),
+            $checkoutTarget(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertSentTo(
+            new AnonymousNotifiable,
+            CheckoutLicenseSeatNotification::class,
+            function ($notification, $channels, $notifiable) {
+                return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint;
+            }
+        );
+    }
+
+    /** @dataProvider targets */
+    public function testLicenseCheckoutDoesNotSendWebhookNotificationWhenSettingDisabled($checkoutTarget)
+    {
+        Notification::fake();
+
+        $this->settings->disableWebhook();
+
+        event(new CheckoutableCheckedOut(
+            LicenseSeat::factory()->create(),
+            $checkoutTarget(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutLicenseSeatNotification::class);
+    }
+
+    /** @dataProvider targets */
+    public function testLicenseCheckinSendsWebhookNotificationWhenSettingEnabled($checkoutTarget)
+    {
+        Notification::fake();
+
+        $this->settings->enableWebhook();
+
+        event(new CheckoutableCheckedIn(
+            LicenseSeat::factory()->create(),
+            $checkoutTarget(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertSentTo(
+            new AnonymousNotifiable,
+            CheckinLicenseSeatNotification::class,
+            function ($notification, $channels, $notifiable) {
+                return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint;
+            }
+        );
+    }
+
+    /** @dataProvider targets */
+    public function testLicenseCheckinDoesNotSendWebhookNotificationWhenSettingDisabled($checkoutTarget)
+    {
+        Notification::fake();
+
+        $this->settings->disableWebhook();
+
+        event(new CheckoutableCheckedIn(
+            LicenseSeat::factory()->create(),
+            $checkoutTarget(),
+            User::factory()->superuser()->create(),
+            ''
+        ));
+
+        Notification::assertNotSentTo(new AnonymousNotifiable, CheckinLicenseSeatNotification::class);
+    }
+}

tests/Support/Settings.php

@@ -23,6 +23,24 @@ class Settings
         return $this->update(['full_multiple_companies_support' => 1]);
     }
 
+    public function enableWebhook(): Settings
+    {
+        return $this->update([
+            'webhook_botname' => 'SnipeBot5000',
+            'webhook_endpoint' => 'https://hooks.slack.com/services/NZ59/Q446/672N',
+            'webhook_channel' => '#it',
+        ]);
+    }
+
+    public function disableWebhook(): Settings
+    {
+        return $this->update([
+            'webhook_botname' => '',
+            'webhook_endpoint' => '',
+            'webhook_channel' => '',
+        ]);
+    }
+
     /**
      * @param array $attributes Attributes to modify in the application's settings.
      */