From 2dcf4e3d16814b303b0fe11029178d640091bea8 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 23 Mar 2023 16:31:40 -0700 Subject: [PATCH] Standardize on sending anonymous notifications for asset checkouts --- app/Listeners/CheckoutableListener.php | 30 +++++++------------ app/Models/User.php | 14 --------- .../AssetCheckoutWebhookNotificationTest.php | 8 ++--- 3 files changed, 15 insertions(+), 37 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 47dc9c280a..a606fbc3bd 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -34,25 +34,17 @@ class CheckoutableListener // @todo: update docblock - /** - * When the item wasn't checked out to a user, we can't send notifications - */ - // @todo: update comment - if (! $event->checkedOutTo instanceof User) { - - // @todo: comment - if (Setting::getSettings() && Setting::getSettings()->webhook_endpoint) { - Notification::route('slack', Setting::getSettings()->webhook_endpoint) - ->notify(new CheckoutAssetNotification( - $event->checkoutable, - $event->checkedOutTo, - $event->checkedOutBy, - null, - $event->note) - ); - } - - return; + // @todo: comment...we send this anonymously so that webhook notification still + // @todo: get sent for models that don't have email addresses associated... + if (Setting::getSettings() && Setting::getSettings()->webhook_endpoint) { + Notification::route('slack', Setting::getSettings()->webhook_endpoint) + ->notify(new CheckoutAssetNotification( + $event->checkoutable, + $event->checkedOutTo, + $event->checkedOutBy, + null, + $event->note) + ); } /** diff --git a/app/Models/User.php b/app/Models/User.php index bf40982db3..de6bcd38df 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -263,20 +263,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 diff --git a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php index 1cbd1a823e..0125289a26 100644 --- a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php @@ -27,10 +27,10 @@ class AssetCheckoutWebhookNotificationTest extends TestCase ); Notification::assertSentTo( - $user, - function (CheckoutAssetNotification $notification, $channels) { - // @todo: is this actually accurate? - return in_array('slack', $channels); + new AnonymousNotifiable, + CheckoutAssetNotification::class, + function ($notification, $channels, $notifiable) { + return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint; } ); }