From 2813b7ea58a4cc579eb3de426f4bb783f9157443 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 13 Mar 2023 15:47:50 -0700 Subject: [PATCH 01/36] Scaffold tests for slack notification on checkout --- .../AssetCheckoutSlackNotificationTest.php | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php diff --git a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php new file mode 100644 index 0000000000..75df4a1626 --- /dev/null +++ b/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php @@ -0,0 +1,97 @@ +setSlackWebhook(); + + $asset = $this->createAsset(); + $user = User::factory()->create(); + + $asset->checkOut( + $user, + User::factory()->superuser()->create()->id + ); + + $this->assetSlackNotificationSentTo($user); + } + + public function testNotificationSentToSlackWhenAssetCheckedOutToAssetAndSlackNotificationEnabled() + { + $this->markTestIncomplete(); + + Notification::fake(); + + $this->setSlackWebhook(); + + $assetBeingCheckedOut = $this->createAsset(); + $targetAsset = $this->createAsset(); + + $assetBeingCheckedOut->checkOut( + $targetAsset, + User::factory()->superuser()->create()->id + ); + + $this->assetSlackNotificationSentTo($targetAsset); + } + + public function testNotificationSentToSlackWhenAssetCheckedOutToLocationAndSlackNotificationEnabled() + { + $this->markTestIncomplete(); + + Notification::fake(); + + $this->setSlackWebhook(); + + $asset = $this->createAsset(); + $location = Location::factory()->create(); + + $asset->checkOut( + $location, + User::factory()->superuser()->create()->id + ); + + $this->assetSlackNotificationSentTo($location); + } + + private function setSlackWebhook() + { + Setting::factory()->create([ + 'slack_endpoint' => 'https://hooks.slack.com/services/NZ59O2F54K/Q4465WNLM8/672N8MU5JV15RP436WDHRN58', + ]); + } + + private function createAsset() + { + return Asset::factory()->create([ + 'model_id' => AssetModel::factory()->create([ + 'category_id' => Category::factory()->assetLaptopCategory()->id, + ])->id, + ]); + } + + private function assetSlackNotificationSentTo($target) + { + Notification::assertSentTo( + $target, + function (CheckoutAssetNotification $notification, $channels) { + return in_array('slack', $channels); + } + ); + } +} From 315f5231cd3052ee9b01dfd7f7234cfd73488a0c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 21 Mar 2023 17:03:51 -0700 Subject: [PATCH 02/36] Send slack notification for check outs to assets and locations --- app/Listeners/CheckoutableListener.php | 15 +++ .../AssetCheckoutSlackNotificationTest.php | 102 ++++++++++++------ 2 files changed, 86 insertions(+), 31 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 9d6cd942bd..5a39b5f513 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -32,11 +32,26 @@ class CheckoutableListener public function onCheckedOut($event) { + // @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()->slack_endpoint) { + Notification::route('slack', Setting::getSettings()->slack_endpoint) + ->notify(new CheckoutAssetNotification( + $event->checkoutable, + $event->checkedOutTo, + $event->checkedOutBy, + null, + $event->note) + ); + } + return; } diff --git a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php index 75df4a1626..468d81678a 100644 --- a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php +++ b/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php @@ -9,16 +9,27 @@ use App\Models\Location; use App\Models\Setting; use App\Models\User; use App\Notifications\CheckoutAssetNotification; +use Illuminate\Notifications\AnonymousNotifiable; use Illuminate\Support\Facades\Notification; use Tests\TestCase; class AssetCheckoutSlackNotificationTest extends TestCase { + private Category $assetLaptopCategory; + private string $slackWebhookUrl = 'https://hooks.slack.com/services/NZ59O2F54K/Q4465WNLM8/672N8MU5JV15RP436WDHRN58'; + + protected function setUp(): void + { + parent::setUp(); + + $this->assetLaptopCategory = Category::factory()->assetLaptopCategory(); + } + public function testNotificationSentToSlackWhenAssetCheckedOutToUserAndSlackNotificationEnabled() { Notification::fake(); - $this->setSlackWebhook(); + Setting::factory()->create(['slack_endpoint' => $this->slackWebhookUrl]); $asset = $this->createAsset(); $user = User::factory()->create(); @@ -28,70 +39,99 @@ class AssetCheckoutSlackNotificationTest extends TestCase User::factory()->superuser()->create()->id ); - $this->assetSlackNotificationSentTo($user); + Notification::assertSentTo( + $user, + function (CheckoutAssetNotification $notification, $channels) { + // @todo: is this actually accurate? + return in_array('slack', $channels); + } + ); } public function testNotificationSentToSlackWhenAssetCheckedOutToAssetAndSlackNotificationEnabled() { - $this->markTestIncomplete(); - Notification::fake(); - $this->setSlackWebhook(); + Setting::factory()->create(['slack_endpoint' => $this->slackWebhookUrl]); $assetBeingCheckedOut = $this->createAsset(); - $targetAsset = $this->createAsset(); - $assetBeingCheckedOut->checkOut( - $targetAsset, + $this->createAsset(), User::factory()->superuser()->create()->id ); - $this->assetSlackNotificationSentTo($targetAsset); + // Since the target is not a user with an email address we have + // to check if an AnonymousNotifiable was sent. + Notification::assertSentTo( + new AnonymousNotifiable, + CheckoutAssetNotification::class, + function ($notification, $channels, $notifiable) { + return $notifiable->routes['slack'] === $this->slackWebhookUrl; + } + ); + } + + public function testDoesNotSendNotificationViaSlackIfWebHookEndpointIsNotSetWhenCheckingOutAssetToAsset() + { + Notification::fake(); + + $assetBeingCheckedOut = $this->createAsset(); + $assetBeingCheckedOut->checkOut( + $this->createAsset(), + User::factory()->superuser()->create()->id + ); + + Notification::assertNotSentTo( + new AnonymousNotifiable, + CheckoutAssetNotification::class, + ); } public function testNotificationSentToSlackWhenAssetCheckedOutToLocationAndSlackNotificationEnabled() { - $this->markTestIncomplete(); - Notification::fake(); - $this->setSlackWebhook(); + Setting::factory()->create(['slack_endpoint' => $this->slackWebhookUrl]); $asset = $this->createAsset(); - $location = Location::factory()->create(); - $asset->checkOut( - $location, + Location::factory()->create(), User::factory()->superuser()->create()->id ); - $this->assetSlackNotificationSentTo($location); + // Since the target is not a user with an email address we have + // to check if an AnonymousNotifiable was sent. + Notification::assertSentTo( + new AnonymousNotifiable, + CheckoutAssetNotification::class, + function ($notification, $channels, $notifiable) { + return $notifiable->routes['slack'] === $this->slackWebhookUrl; + } + ); } - private function setSlackWebhook() + public function testtestDoesNotSendNotificationViaSlackIfWebHookEndpointIsNotSetWhenCheckingOutAssetToLocation() { - Setting::factory()->create([ - 'slack_endpoint' => 'https://hooks.slack.com/services/NZ59O2F54K/Q4465WNLM8/672N8MU5JV15RP436WDHRN58', - ]); + Notification::fake(); + + $asset = $this->createAsset(); + $asset->checkOut( + Location::factory()->create(), + User::factory()->superuser()->create()->id + ); + + Notification::assertNotSentTo( + new AnonymousNotifiable, + CheckoutAssetNotification::class, + ); } private function createAsset() { return Asset::factory()->create([ 'model_id' => AssetModel::factory()->create([ - 'category_id' => Category::factory()->assetLaptopCategory()->id, + 'category_id' => $this->assetLaptopCategory->id, ])->id, ]); } - - private function assetSlackNotificationSentTo($target) - { - Notification::assertSentTo( - $target, - function (CheckoutAssetNotification $notification, $channels) { - return in_array('slack', $channels); - } - ); - } } From e303aeadd1de47ebd7265b99bcdb47e793ec620e Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 Mar 2023 11:56:54 -0700 Subject: [PATCH 03/36] Fix test name --- .../Notifications/AssetCheckoutSlackNotificationTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php index 468d81678a..5ec8fe241f 100644 --- a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php +++ b/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php @@ -110,7 +110,7 @@ class AssetCheckoutSlackNotificationTest extends TestCase ); } - public function testtestDoesNotSendNotificationViaSlackIfWebHookEndpointIsNotSetWhenCheckingOutAssetToLocation() + public function testDoesNotSendNotificationViaSlackIfWebHookEndpointIsNotSetWhenCheckingOutAssetToLocation() { Notification::fake(); From 9c4a3ce56a5bebb02c8880daac17ede30ae10fa8 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 Mar 2023 12:27:36 -0700 Subject: [PATCH 04/36] Update slack_endpoint to webhook_endpoint in test and notification --- app/Listeners/CheckoutableListener.php | 4 +- .../CheckoutAssetNotification.php | 2 +- .../AssetCheckoutSlackNotificationTest.php | 39 +++++-------------- 3 files changed, 13 insertions(+), 32 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 5a39b5f513..47dc9c280a 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -41,8 +41,8 @@ class CheckoutableListener if (! $event->checkedOutTo instanceof User) { // @todo: comment - if (Setting::getSettings() && Setting::getSettings()->slack_endpoint) { - Notification::route('slack', Setting::getSettings()->slack_endpoint) + if (Setting::getSettings() && Setting::getSettings()->webhook_endpoint) { + Notification::route('slack', Setting::getSettings()->webhook_endpoint) ->notify(new CheckoutAssetNotification( $event->checkoutable, $event->checkedOutTo, diff --git a/app/Notifications/CheckoutAssetNotification.php b/app/Notifications/CheckoutAssetNotification.php index c56c1a3711..84e0fd1dc7 100644 --- a/app/Notifications/CheckoutAssetNotification.php +++ b/app/Notifications/CheckoutAssetNotification.php @@ -53,7 +53,7 @@ class CheckoutAssetNotification extends Notification { $notifyBy = []; - if ((Setting::getSettings()) && (Setting::getSettings()->slack_endpoint != '')) { + if ((Setting::getSettings()) && (Setting::getSettings()->webhook_endpoint != '')) { \Log::debug('use slack'); $notifyBy[] = 'slack'; } diff --git a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php index 5ec8fe241f..24c588b301 100644 --- a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php +++ b/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php @@ -3,8 +3,6 @@ namespace Tests\Feature\Notifications; use App\Models\Asset; -use App\Models\AssetModel; -use App\Models\Category; use App\Models\Location; use App\Models\Setting; use App\Models\User; @@ -15,23 +13,15 @@ use Tests\TestCase; class AssetCheckoutSlackNotificationTest extends TestCase { - private Category $assetLaptopCategory; private string $slackWebhookUrl = 'https://hooks.slack.com/services/NZ59O2F54K/Q4465WNLM8/672N8MU5JV15RP436WDHRN58'; - protected function setUp(): void - { - parent::setUp(); - - $this->assetLaptopCategory = Category::factory()->assetLaptopCategory(); - } - public function testNotificationSentToSlackWhenAssetCheckedOutToUserAndSlackNotificationEnabled() { Notification::fake(); - Setting::factory()->create(['slack_endpoint' => $this->slackWebhookUrl]); + Setting::factory()->create(['webhook_endpoint' => $this->slackWebhookUrl]); - $asset = $this->createAsset(); + $asset = Asset::factory()->laptopMbp()->create(); $user = User::factory()->create(); $asset->checkOut( @@ -52,11 +42,11 @@ class AssetCheckoutSlackNotificationTest extends TestCase { Notification::fake(); - Setting::factory()->create(['slack_endpoint' => $this->slackWebhookUrl]); + Setting::factory()->create(['webhook_endpoint' => $this->slackWebhookUrl]); - $assetBeingCheckedOut = $this->createAsset(); + $assetBeingCheckedOut = Asset::factory()->laptopMbp()->create(); $assetBeingCheckedOut->checkOut( - $this->createAsset(), + Asset::factory()->laptopMbp()->create(), User::factory()->superuser()->create()->id ); @@ -75,9 +65,9 @@ class AssetCheckoutSlackNotificationTest extends TestCase { Notification::fake(); - $assetBeingCheckedOut = $this->createAsset(); + $assetBeingCheckedOut = Asset::factory()->laptopMbp()->create(); $assetBeingCheckedOut->checkOut( - $this->createAsset(), + Asset::factory()->laptopMbp()->create(), User::factory()->superuser()->create()->id ); @@ -91,9 +81,9 @@ class AssetCheckoutSlackNotificationTest extends TestCase { Notification::fake(); - Setting::factory()->create(['slack_endpoint' => $this->slackWebhookUrl]); + Setting::factory()->create(['webhook_endpoint' => $this->slackWebhookUrl]); - $asset = $this->createAsset(); + $asset = Asset::factory()->laptopMbp()->create(); $asset->checkOut( Location::factory()->create(), User::factory()->superuser()->create()->id @@ -114,7 +104,7 @@ class AssetCheckoutSlackNotificationTest extends TestCase { Notification::fake(); - $asset = $this->createAsset(); + $asset = Asset::factory()->laptopMbp()->create(); $asset->checkOut( Location::factory()->create(), User::factory()->superuser()->create()->id @@ -125,13 +115,4 @@ class AssetCheckoutSlackNotificationTest extends TestCase CheckoutAssetNotification::class, ); } - - private function createAsset() - { - return Asset::factory()->create([ - 'model_id' => AssetModel::factory()->create([ - 'category_id' => $this->assetLaptopCategory->id, - ])->id, - ]); - } } From 66224765ea4970597b20252e451d645d8bbbaba2 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 Mar 2023 12:31:47 -0700 Subject: [PATCH 05/36] Use factory state for webhook settings --- database/factories/SettingFactory.php | 9 +++++++++ .../AssetCheckoutSlackNotificationTest.php | 12 +++++------- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/database/factories/SettingFactory.php b/database/factories/SettingFactory.php index 970d00cd68..acbd6afafa 100644 --- a/database/factories/SettingFactory.php +++ b/database/factories/SettingFactory.php @@ -43,4 +43,13 @@ class SettingFactory extends Factory ]; }); } + + public function withWebhookEnabled() + { + return $this->state(fn() => [ + 'webhook_botname' => 'SnipeBot5000', + 'webhook_endpoint' => 'https://hooks.slack.com/services/NZ59/Q446/672N', + 'webhook_channel' => '#it', + ]); + } } diff --git a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php index 24c588b301..784ffb18d0 100644 --- a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php +++ b/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php @@ -13,13 +13,11 @@ use Tests\TestCase; class AssetCheckoutSlackNotificationTest extends TestCase { - private string $slackWebhookUrl = 'https://hooks.slack.com/services/NZ59O2F54K/Q4465WNLM8/672N8MU5JV15RP436WDHRN58'; - public function testNotificationSentToSlackWhenAssetCheckedOutToUserAndSlackNotificationEnabled() { Notification::fake(); - Setting::factory()->create(['webhook_endpoint' => $this->slackWebhookUrl]); + Setting::factory()->withWebhookEnabled()->create(); $asset = Asset::factory()->laptopMbp()->create(); $user = User::factory()->create(); @@ -42,7 +40,7 @@ class AssetCheckoutSlackNotificationTest extends TestCase { Notification::fake(); - Setting::factory()->create(['webhook_endpoint' => $this->slackWebhookUrl]); + Setting::factory()->withWebhookEnabled()->create(); $assetBeingCheckedOut = Asset::factory()->laptopMbp()->create(); $assetBeingCheckedOut->checkOut( @@ -56,7 +54,7 @@ class AssetCheckoutSlackNotificationTest extends TestCase new AnonymousNotifiable, CheckoutAssetNotification::class, function ($notification, $channels, $notifiable) { - return $notifiable->routes['slack'] === $this->slackWebhookUrl; + return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint; } ); } @@ -81,7 +79,7 @@ class AssetCheckoutSlackNotificationTest extends TestCase { Notification::fake(); - Setting::factory()->create(['webhook_endpoint' => $this->slackWebhookUrl]); + Setting::factory()->withWebhookEnabled()->create(); $asset = Asset::factory()->laptopMbp()->create(); $asset->checkOut( @@ -95,7 +93,7 @@ class AssetCheckoutSlackNotificationTest extends TestCase new AnonymousNotifiable, CheckoutAssetNotification::class, function ($notification, $channels, $notifiable) { - return $notifiable->routes['slack'] === $this->slackWebhookUrl; + return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint; } ); } From 28ced46b9d82745dfcb964c108428ff3d1a1d498 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 Mar 2023 12:38:14 -0700 Subject: [PATCH 06/36] Clean up test code --- .../AssetCheckoutSlackNotificationTest.php | 57 ++++++++++++------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php index 784ffb18d0..ac61d4402a 100644 --- a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php +++ b/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php @@ -17,14 +17,13 @@ class AssetCheckoutSlackNotificationTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookEnabled()->create(); + $this->enableWebhookSettings(); - $asset = Asset::factory()->laptopMbp()->create(); - $user = User::factory()->create(); + $user = $this->createUser(); - $asset->checkOut( + $this->createAsset()->checkOut( $user, - User::factory()->superuser()->create()->id + $this->createSuperUser()->id ); Notification::assertSentTo( @@ -40,12 +39,11 @@ class AssetCheckoutSlackNotificationTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookEnabled()->create(); + $this->enableWebhookSettings(); - $assetBeingCheckedOut = Asset::factory()->laptopMbp()->create(); - $assetBeingCheckedOut->checkOut( - Asset::factory()->laptopMbp()->create(), - User::factory()->superuser()->create()->id + $this->createAsset()->checkOut( + $this->createAsset(), + $this->createSuperUser()->id ); // Since the target is not a user with an email address we have @@ -63,10 +61,9 @@ class AssetCheckoutSlackNotificationTest extends TestCase { Notification::fake(); - $assetBeingCheckedOut = Asset::factory()->laptopMbp()->create(); - $assetBeingCheckedOut->checkOut( - Asset::factory()->laptopMbp()->create(), - User::factory()->superuser()->create()->id + $this->createAsset()->checkOut( + $this->createAsset(), + $this->createSuperUser()->id ); Notification::assertNotSentTo( @@ -79,12 +76,11 @@ class AssetCheckoutSlackNotificationTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookEnabled()->create(); + $this->enableWebhookSettings(); - $asset = Asset::factory()->laptopMbp()->create(); - $asset->checkOut( + $this->createAsset()->checkOut( Location::factory()->create(), - User::factory()->superuser()->create()->id + $this->createSuperUser()->id ); // Since the target is not a user with an email address we have @@ -102,10 +98,9 @@ class AssetCheckoutSlackNotificationTest extends TestCase { Notification::fake(); - $asset = Asset::factory()->laptopMbp()->create(); - $asset->checkOut( + $this->createAsset()->checkOut( Location::factory()->create(), - User::factory()->superuser()->create()->id + $this->createSuperUser()->id ); Notification::assertNotSentTo( @@ -113,4 +108,24 @@ class AssetCheckoutSlackNotificationTest extends TestCase CheckoutAssetNotification::class, ); } + + private function enableWebhookSettings() + { + Setting::factory()->withWebhookEnabled()->create(); + } + + private function createAsset() + { + return Asset::factory()->laptopMbp()->create(); + } + + private function createUser() + { + return User::factory()->create(); + } + + private function createSuperUser() + { + return User::factory()->superuser()->create(); + } } From b396f2bed33fe213f6525f18664f8d1a341a6d9b Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 Mar 2023 12:41:30 -0700 Subject: [PATCH 07/36] Update test case names --- ....php => AssetCheckoutWebhookNotificationTest.php} | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) rename tests/Feature/Notifications/{AssetCheckoutSlackNotificationTest.php => AssetCheckoutWebhookNotificationTest.php} (83%) diff --git a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php similarity index 83% rename from tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php rename to tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php index ac61d4402a..1cbd1a823e 100644 --- a/tests/Feature/Notifications/AssetCheckoutSlackNotificationTest.php +++ b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php @@ -11,9 +11,9 @@ use Illuminate\Notifications\AnonymousNotifiable; use Illuminate\Support\Facades\Notification; use Tests\TestCase; -class AssetCheckoutSlackNotificationTest extends TestCase +class AssetCheckoutWebhookNotificationTest extends TestCase { - public function testNotificationSentToSlackWhenAssetCheckedOutToUserAndSlackNotificationEnabled() + public function testNotificationSentToWebhookWhenAssetCheckedOutToUserAndWebhookNotificationEnabled() { Notification::fake(); @@ -35,7 +35,7 @@ class AssetCheckoutSlackNotificationTest extends TestCase ); } - public function testNotificationSentToSlackWhenAssetCheckedOutToAssetAndSlackNotificationEnabled() + public function testNotificationSentToWebhookWhenAssetCheckedOutToAssetAndWebhookNotificationEnabled() { Notification::fake(); @@ -57,7 +57,7 @@ class AssetCheckoutSlackNotificationTest extends TestCase ); } - public function testDoesNotSendNotificationViaSlackIfWebHookEndpointIsNotSetWhenCheckingOutAssetToAsset() + public function testDoesNotSendNotificationViaWebhookIfWebHookEndpointIsNotSetWhenCheckingOutAssetToAsset() { Notification::fake(); @@ -72,7 +72,7 @@ class AssetCheckoutSlackNotificationTest extends TestCase ); } - public function testNotificationSentToSlackWhenAssetCheckedOutToLocationAndSlackNotificationEnabled() + public function testNotificationSentToWebhookWhenAssetCheckedOutToLocationAndWebhookNotificationEnabled() { Notification::fake(); @@ -94,7 +94,7 @@ class AssetCheckoutSlackNotificationTest extends TestCase ); } - public function testDoesNotSendNotificationViaSlackIfWebHookEndpointIsNotSetWhenCheckingOutAssetToLocation() + public function testDoesNotSendNotificationViaWebhookIfWebHookEndpointIsNotSetWhenCheckingOutAssetToLocation() { Notification::fake(); From 2dcf4e3d16814b303b0fe11029178d640091bea8 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 23 Mar 2023 16:31:40 -0700 Subject: [PATCH 08/36] 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; } ); } From 25e859c8660ca3cd69338511872d31a4a8f09471 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 23 Mar 2023 16:42:21 -0700 Subject: [PATCH 09/36] Simplify test case --- .../AssetCheckoutWebhookNotificationTest.php | 142 +++++------------- 1 file changed, 38 insertions(+), 104 deletions(-) diff --git a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php index 0125289a26..4683d6f306 100644 --- a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php @@ -13,105 +13,48 @@ use Tests\TestCase; class AssetCheckoutWebhookNotificationTest extends TestCase { - public function testNotificationSentToWebhookWhenAssetCheckedOutToUserAndWebhookNotificationEnabled() + + public function checkoutTargets() + { + 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 checkoutTargets */ + public function testWebhookNotificationsAreSentOnAssetCheckoutWhenWebhookSettingEnabled($checkoutTarget) { Notification::fake(); - $this->enableWebhookSettings(); - - $user = $this->createUser(); - - $this->createAsset()->checkOut( - $user, - $this->createSuperUser()->id - ); - - Notification::assertSentTo( - new AnonymousNotifiable, - CheckoutAssetNotification::class, - function ($notification, $channels, $notifiable) { - return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint; - } - ); - } - - public function testNotificationSentToWebhookWhenAssetCheckedOutToAssetAndWebhookNotificationEnabled() - { - Notification::fake(); - - $this->enableWebhookSettings(); - - $this->createAsset()->checkOut( - $this->createAsset(), - $this->createSuperUser()->id - ); - - // Since the target is not a user with an email address we have - // to check if an AnonymousNotifiable was sent. - Notification::assertSentTo( - new AnonymousNotifiable, - CheckoutAssetNotification::class, - function ($notification, $channels, $notifiable) { - return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint; - } - ); - } - - public function testDoesNotSendNotificationViaWebhookIfWebHookEndpointIsNotSetWhenCheckingOutAssetToAsset() - { - Notification::fake(); - - $this->createAsset()->checkOut( - $this->createAsset(), - $this->createSuperUser()->id - ); - - Notification::assertNotSentTo( - new AnonymousNotifiable, - CheckoutAssetNotification::class, - ); - } - - public function testNotificationSentToWebhookWhenAssetCheckedOutToLocationAndWebhookNotificationEnabled() - { - Notification::fake(); - - $this->enableWebhookSettings(); - - $this->createAsset()->checkOut( - Location::factory()->create(), - $this->createSuperUser()->id - ); - - // Since the target is not a user with an email address we have - // to check if an AnonymousNotifiable was sent. - Notification::assertSentTo( - new AnonymousNotifiable, - CheckoutAssetNotification::class, - function ($notification, $channels, $notifiable) { - return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint; - } - ); - } - - public function testDoesNotSendNotificationViaWebhookIfWebHookEndpointIsNotSetWhenCheckingOutAssetToLocation() - { - Notification::fake(); - - $this->createAsset()->checkOut( - Location::factory()->create(), - $this->createSuperUser()->id - ); - - Notification::assertNotSentTo( - new AnonymousNotifiable, - CheckoutAssetNotification::class, - ); - } - - private function enableWebhookSettings() - { Setting::factory()->withWebhookEnabled()->create(); + + $this->createAsset()->checkOut( + $checkoutTarget(), + User::factory()->superuser()->create()->id + ); + + Notification::assertSentTo( + new AnonymousNotifiable, + CheckoutAssetNotification::class, + function ($notification, $channels, $notifiable) { + return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint; + } + ); + } + + /** @dataProvider checkoutTargets */ + public function testWebhookNotificationsAreNotSentOnAssetCheckoutWhenWebhookSettingNotEnabled($checkoutTarget) + { + Notification::fake(); + + $this->createAsset()->checkOut( + $checkoutTarget(), + User::factory()->superuser()->create()->id + ); + + Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutAssetNotification::class); } private function createAsset() @@ -119,13 +62,4 @@ class AssetCheckoutWebhookNotificationTest extends TestCase return Asset::factory()->laptopMbp()->create(); } - private function createUser() - { - return User::factory()->create(); - } - - private function createSuperUser() - { - return User::factory()->superuser()->create(); - } } From fc043a35d9b090785ac1fb47c04dd8dfc9293657 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 23 Mar 2023 17:03:48 -0700 Subject: [PATCH 10/36] Fix checkouts for licenses --- app/Listeners/CheckoutableListener.php | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index a606fbc3bd..2133876789 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -38,13 +38,7 @@ class CheckoutableListener // @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) - ); + ->notify($this->getCheckoutNotification($event)); } /** @@ -53,6 +47,7 @@ class CheckoutableListener $acceptance = $this->getCheckoutAcceptance($event); try { + // @todo: wrap notification above in this try if (! $event->checkedOutTo->locale) { Notification::locale(Setting::getSettings()->locale)->send( $this->getNotifiables($event), @@ -190,10 +185,10 @@ class CheckoutableListener * Get the appropriate notification for the event * * @param CheckoutableCheckedIn $event - * @param CheckoutAcceptance $acceptance + * @param CheckoutAcceptance $acceptance * @return Notification */ - private function getCheckoutNotification($event, $acceptance) + private function getCheckoutNotification($event, $acceptance = null) { $notificationClass = null; From fa69a580ab470e222f89129da5e39e77a74de7f5 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 23 Mar 2023 17:18:33 -0700 Subject: [PATCH 11/36] Start to scaffold license checkout notification test --- app/Models/LicenseSeat.php | 4 +- database/factories/LicenseSeatFactory.php | 21 ++++++++ .../AssetCheckoutWebhookNotificationTest.php | 1 - ...LicenseCheckoutWebhookNotificationTest.php | 53 +++++++++++++++++++ 4 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 database/factories/LicenseSeatFactory.php create mode 100644 tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php diff --git a/app/Models/LicenseSeat.php b/app/Models/LicenseSeat.php index 2207edd02c..d2a99d3c56 100755 --- a/app/Models/LicenseSeat.php +++ b/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; diff --git a/database/factories/LicenseSeatFactory.php b/database/factories/LicenseSeatFactory.php new file mode 100644 index 0000000000..c4d43f89b3 --- /dev/null +++ b/database/factories/LicenseSeatFactory.php @@ -0,0 +1,21 @@ + License::factory(), + ]; + } +} diff --git a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php index 4683d6f306..57a6fcfb98 100644 --- a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php @@ -61,5 +61,4 @@ class AssetCheckoutWebhookNotificationTest extends TestCase { return Asset::factory()->laptopMbp()->create(); } - } diff --git a/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php new file mode 100644 index 0000000000..6fd7d22cfc --- /dev/null +++ b/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php @@ -0,0 +1,53 @@ + [fn() => User::factory()->create()], + 'License checked out to asset' => [fn() => Asset::factory()->laptopMbp()->create()], + ]; + } + + /** @dataProvider checkoutTargets */ + public function testWebhookNotificationsAreSentOnLicenseCheckoutWhenWebhookSettingEnabled($checkoutTarget) + { + Notification::fake(); + + Setting::factory()->withWebhookEnabled()->create(); + + $checkoutTarget = $checkoutTarget(); + + $licenseSeat = LicenseSeat::factory()->create(); + + // @todo: this has to go through the LicenseCheckoutController::store() method + // @todo: to have the CheckoutableCheckedOut fire... + // @todo: either change this to go through controller + // @todo: or move that functionality to the model? + // $licenseSeat->checkOut( + // $checkoutTarget, + // User::factory()->superuser()->create()->id + // ); + + Notification::assertSentTo( + new AnonymousNotifiable, + CheckoutLicenseSeatNotification::class, + function ($notification, $channels, $notifiable) { + return $notifiable->routes['slack'] === Setting::getSettings()->webhook_endpoint; + } + ); + } +} From 2cbc6276f779dbe9e7d92124417ee772523a9d83 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 27 Mar 2023 14:09:21 -0700 Subject: [PATCH 12/36] Implement test for license checkout notification --- .../AssetCheckoutWebhookNotificationTest.php | 1 - .../LicenseCheckoutWebhookNotificationTest.php | 18 ++++++------------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php index 57a6fcfb98..2a2a8367e3 100644 --- a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php @@ -13,7 +13,6 @@ use Tests\TestCase; class AssetCheckoutWebhookNotificationTest extends TestCase { - public function checkoutTargets() { return [ diff --git a/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php index 6fd7d22cfc..e01a2dc62b 100644 --- a/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php @@ -29,18 +29,12 @@ class LicenseCheckoutWebhookNotificationTest extends TestCase Setting::factory()->withWebhookEnabled()->create(); - $checkoutTarget = $checkoutTarget(); - - $licenseSeat = LicenseSeat::factory()->create(); - - // @todo: this has to go through the LicenseCheckoutController::store() method - // @todo: to have the CheckoutableCheckedOut fire... - // @todo: either change this to go through controller - // @todo: or move that functionality to the model? - // $licenseSeat->checkOut( - // $checkoutTarget, - // User::factory()->superuser()->create()->id - // ); + event(new CheckoutableCheckedOut( + LicenseSeat::factory()->create(), + $checkoutTarget(), + User::factory()->superuser()->create(), + '' + )); Notification::assertSentTo( new AnonymousNotifiable, From 3271c9dc9fb40718d114243fc0b1c07052f2d315 Mon Sep 17 00:00:00 2001 From: Dominik Both Date: Wed, 29 Mar 2023 09:56:34 +0200 Subject: [PATCH 13/36] Fix #8208, #8896, #8985, #9789 --- app/Http/Controllers/Accessories/AccessoriesController.php | 4 ++-- app/Http/Controllers/Api/AssetMaintenancesController.php | 4 ++-- app/Http/Controllers/Api/AssetsController.php | 2 +- app/Http/Controllers/AssetMaintenancesController.php | 4 ++-- app/Http/Controllers/Assets/AssetsController.php | 4 ++-- app/Http/Controllers/Assets/BulkAssetsController.php | 2 +- app/Http/Controllers/Components/ComponentsController.php | 4 ++-- app/Http/Controllers/Consumables/ConsumablesController.php | 4 ++-- app/Http/Controllers/Licenses/LicensesController.php | 4 ++-- app/Models/AssetMaintenance.php | 4 ++-- app/Models/SnipeModel.php | 4 ++-- 11 files changed, 20 insertions(+), 20 deletions(-) diff --git a/app/Http/Controllers/Accessories/AccessoriesController.php b/app/Http/Controllers/Accessories/AccessoriesController.php index 7d4e697b98..111cbb3c8b 100755 --- a/app/Http/Controllers/Accessories/AccessoriesController.php +++ b/app/Http/Controllers/Accessories/AccessoriesController.php @@ -77,7 +77,7 @@ class AccessoriesController extends Controller $accessory->manufacturer_id = request('manufacturer_id'); $accessory->model_number = request('model_number'); $accessory->purchase_date = request('purchase_date'); - $accessory->purchase_cost = Helper::ParseCurrency(request('purchase_cost')); + $accessory->purchase_cost = request('purchase_cost'); $accessory->qty = request('qty'); $accessory->user_id = Auth::user()->id; $accessory->supplier_id = request('supplier_id'); @@ -180,7 +180,7 @@ class AccessoriesController extends Controller $accessory->order_number = request('order_number'); $accessory->model_number = request('model_number'); $accessory->purchase_date = request('purchase_date'); - $accessory->purchase_cost = Helper::ParseCurrency(request('purchase_cost')); + $accessory->purchase_cost = request('purchase_cost'); $accessory->qty = request('qty'); $accessory->supplier_id = request('supplier_id'); $accessory->notes = request('notes'); diff --git a/app/Http/Controllers/Api/AssetMaintenancesController.php b/app/Http/Controllers/Api/AssetMaintenancesController.php index e38d5382fa..a5679fe304 100644 --- a/app/Http/Controllers/Api/AssetMaintenancesController.php +++ b/app/Http/Controllers/Api/AssetMaintenancesController.php @@ -121,7 +121,7 @@ class AssetMaintenancesController extends Controller $assetMaintenance = new AssetMaintenance(); $assetMaintenance->supplier_id = $request->input('supplier_id'); $assetMaintenance->is_warranty = $request->input('is_warranty'); - $assetMaintenance->cost = Helper::ParseCurrency($request->input('cost')); + $assetMaintenance->cost = $request->input('cost'); $assetMaintenance->notes = e($request->input('notes')); $asset = Asset::find(e($request->input('asset_id'))); @@ -178,7 +178,7 @@ class AssetMaintenancesController extends Controller $assetMaintenance->supplier_id = e($request->input('supplier_id')); $assetMaintenance->is_warranty = e($request->input('is_warranty')); - $assetMaintenance->cost = Helper::ParseCurrency($request->input('cost')); + $assetMaintenance->cost = $request->input('cost'); $assetMaintenance->notes = e($request->input('notes')); $asset = Asset::find(request('asset_id')); diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index d4ff0092ba..fbd9ae7f84 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -554,7 +554,7 @@ class AssetsController extends Controller $asset->depreciate = '0'; $asset->status_id = $request->get('status_id', 0); $asset->warranty_months = $request->get('warranty_months', null); - $asset->purchase_cost = Helper::ParseCurrency($request->get('purchase_cost')); // this is the API's store method, so I don't know that I want to do this? Confusing. FIXME (or not?!) + $asset->purchase_cost = $request->get('purchase_cost'); // this is the API's store method, so I don't know that I want to do this? Confusing. FIXME (or not?!) $asset->purchase_date = $request->get('purchase_date', null); $asset->assigned_to = $request->get('assigned_to', null); $asset->supplier_id = $request->get('supplier_id'); diff --git a/app/Http/Controllers/AssetMaintenancesController.php b/app/Http/Controllers/AssetMaintenancesController.php index 5f3221d051..dc6bc84347 100644 --- a/app/Http/Controllers/AssetMaintenancesController.php +++ b/app/Http/Controllers/AssetMaintenancesController.php @@ -101,7 +101,7 @@ class AssetMaintenancesController extends Controller $assetMaintenance = new AssetMaintenance(); $assetMaintenance->supplier_id = $request->input('supplier_id'); $assetMaintenance->is_warranty = $request->input('is_warranty'); - $assetMaintenance->cost = Helper::ParseCurrency($request->input('cost')); + $assetMaintenance->cost = $request->input('cost'); $assetMaintenance->notes = $request->input('notes'); $asset = Asset::find($request->input('asset_id')); @@ -211,7 +211,7 @@ class AssetMaintenancesController extends Controller $assetMaintenance->supplier_id = $request->input('supplier_id'); $assetMaintenance->is_warranty = $request->input('is_warranty'); - $assetMaintenance->cost = Helper::ParseCurrency($request->input('cost')); + $assetMaintenance->cost = $request->input('cost'); $assetMaintenance->notes = $request->input('notes'); $asset = Asset::find(request('asset_id')); diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 3b2ff4623f..b385222bd3 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -140,7 +140,7 @@ class AssetsController extends Controller $asset->depreciate = '0'; $asset->status_id = request('status_id'); $asset->warranty_months = request('warranty_months', null); - $asset->purchase_cost = Helper::ParseCurrency($request->get('purchase_cost')); + $asset->purchase_cost = $request->get('purchase_cost'); $asset->purchase_date = request('purchase_date', null); $asset->asset_eol_date = request('asset_eol_date', null); $asset->assigned_to = request('assigned_to', null); @@ -312,7 +312,7 @@ class AssetsController extends Controller $asset->status_id = $request->input('status_id', null); $asset->warranty_months = $request->input('warranty_months', null); - $asset->purchase_cost = Helper::ParseCurrency($request->input('purchase_cost', null)); + $asset->purchase_cost = $request->input('purchase_cost', null); $asset->asset_eol_date = request('asset_eol_date', null); $asset->purchase_date = $request->input('purchase_date', null); diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 7f9e812ab4..5a2a6e7387 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -135,7 +135,7 @@ class BulkAssetsController extends Controller } if ($request->filled('purchase_cost')) { - $this->update_array['purchase_cost'] = Helper::ParseCurrency($request->input('purchase_cost')); + $this->update_array['purchase_cost'] = $request->input('purchase_cost'); } if ($request->filled('company_id')) { diff --git a/app/Http/Controllers/Components/ComponentsController.php b/app/Http/Controllers/Components/ComponentsController.php index f943a71a2e..7912a590a0 100644 --- a/app/Http/Controllers/Components/ComponentsController.php +++ b/app/Http/Controllers/Components/ComponentsController.php @@ -77,7 +77,7 @@ class ComponentsController extends Controller $component->min_amt = $request->input('min_amt', null); $component->serial = $request->input('serial', null); $component->purchase_date = $request->input('purchase_date', null); - $component->purchase_cost = Helper::ParseCurrency($request->input('purchase_cost', null)); + $component->purchase_cost = $request->input('purchase_cost', null); $component->qty = $request->input('qty'); $component->user_id = Auth::id(); $component->notes = $request->input('notes'); @@ -151,7 +151,7 @@ class ComponentsController extends Controller $component->min_amt = $request->input('min_amt'); $component->serial = $request->input('serial'); $component->purchase_date = $request->input('purchase_date'); - $component->purchase_cost = Helper::ParseCurrency(request('purchase_cost')); + $component->purchase_cost = request('purchase_cost'); $component->qty = $request->input('qty'); $component->notes = $request->input('notes'); diff --git a/app/Http/Controllers/Consumables/ConsumablesController.php b/app/Http/Controllers/Consumables/ConsumablesController.php index f068e9868d..dd6667893a 100644 --- a/app/Http/Controllers/Consumables/ConsumablesController.php +++ b/app/Http/Controllers/Consumables/ConsumablesController.php @@ -76,7 +76,7 @@ class ConsumablesController extends Controller $consumable->model_number = $request->input('model_number'); $consumable->item_no = $request->input('item_no'); $consumable->purchase_date = $request->input('purchase_date'); - $consumable->purchase_cost = Helper::ParseCurrency($request->input('purchase_cost')); + $consumable->purchase_cost = $request->input('purchase_cost'); $consumable->qty = $request->input('qty'); $consumable->user_id = Auth::id(); $consumable->notes = $request->input('notes'); @@ -152,7 +152,7 @@ class ConsumablesController extends Controller $consumable->model_number = $request->input('model_number'); $consumable->item_no = $request->input('item_no'); $consumable->purchase_date = $request->input('purchase_date'); - $consumable->purchase_cost = Helper::ParseCurrency($request->input('purchase_cost')); + $consumable->purchase_cost = $request->input('purchase_cost'); $consumable->qty = Helper::ParseFloat($request->input('qty')); $consumable->notes = $request->input('notes'); diff --git a/app/Http/Controllers/Licenses/LicensesController.php b/app/Http/Controllers/Licenses/LicensesController.php index a0467654ca..7e0435cd15 100755 --- a/app/Http/Controllers/Licenses/LicensesController.php +++ b/app/Http/Controllers/Licenses/LicensesController.php @@ -86,7 +86,7 @@ class LicensesController extends Controller $license->name = $request->input('name'); $license->notes = $request->input('notes'); $license->order_number = $request->input('order_number'); - $license->purchase_cost = Helper::ParseCurrency($request->input('purchase_cost')); + $license->purchase_cost = $request->input('purchase_cost'); $license->purchase_date = $request->input('purchase_date'); $license->purchase_order = $request->input('purchase_order'); $license->purchase_order = $request->input('purchase_order'); @@ -164,7 +164,7 @@ class LicensesController extends Controller $license->name = $request->input('name'); $license->notes = $request->input('notes'); $license->order_number = $request->input('order_number'); - $license->purchase_cost = Helper::ParseCurrency($request->input('purchase_cost')); + $license->purchase_cost = $request->input('purchase_cost'); $license->purchase_date = $request->input('purchase_date'); $license->purchase_order = $request->input('purchase_order'); $license->reassignable = $request->input('reassignable', 0); diff --git a/app/Models/AssetMaintenance.php b/app/Models/AssetMaintenance.php index 41ab802579..292e529571 100644 --- a/app/Models/AssetMaintenance.php +++ b/app/Models/AssetMaintenance.php @@ -95,8 +95,8 @@ class AssetMaintenance extends Model implements ICompanyableChild */ public function setCostAttribute($value) { - $value = Helper::ParseFloat($value); - if ($value == '0.0') { + $value = Helper::ParseCurrency($value); + if ($value == 0) { $value = null; } $this->attributes['cost'] = $value; diff --git a/app/Models/SnipeModel.php b/app/Models/SnipeModel.php index e5a039a4e1..af12c3d29b 100644 --- a/app/Models/SnipeModel.php +++ b/app/Models/SnipeModel.php @@ -21,9 +21,9 @@ class SnipeModel extends Model */ public function setPurchaseCostAttribute($value) { - $value = Helper::ParseFloat($value); + $value = Helper::ParseCurrency($value); - if ($value == '0.0') { + if ($value == 0) { $value = null; } $this->attributes['purchase_cost'] = $value; From 158e1544cde10448f1312619f9a8c4617668ad5a Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 30 Mar 2023 15:38:00 -0700 Subject: [PATCH 14/36] Test the event instead of the checkout --- .../AssetCheckoutWebhookNotificationTest.php | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php index 2a2a8367e3..535bd62a3d 100644 --- a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php @@ -2,12 +2,14 @@ namespace Tests\Feature\Notifications; +use App\Events\CheckoutableCheckedOut; use App\Models\Asset; use App\Models\Location; use App\Models\Setting; use App\Models\User; use App\Notifications\CheckoutAssetNotification; use Illuminate\Notifications\AnonymousNotifiable; +use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Notification; use Tests\TestCase; @@ -22,6 +24,15 @@ class AssetCheckoutWebhookNotificationTest extends TestCase ]; } + public function testAssetCheckoutFiresCheckoutEvent() + { + Event::fake([CheckoutableCheckedOut::class]); + + $this->createAsset()->checkOut(User::factory()->create(), User::factory()->create()); + + Event::assertDispatched(CheckoutableCheckedOut::class); + } + /** @dataProvider checkoutTargets */ public function testWebhookNotificationsAreSentOnAssetCheckoutWhenWebhookSettingEnabled($checkoutTarget) { @@ -29,10 +40,12 @@ class AssetCheckoutWebhookNotificationTest extends TestCase Setting::factory()->withWebhookEnabled()->create(); - $this->createAsset()->checkOut( + event(new CheckoutableCheckedOut( + $this->createAsset(), $checkoutTarget(), - User::factory()->superuser()->create()->id - ); + User::factory()->superuser()->create(), + '' + )); Notification::assertSentTo( new AnonymousNotifiable, @@ -48,10 +61,12 @@ class AssetCheckoutWebhookNotificationTest extends TestCase { Notification::fake(); - $this->createAsset()->checkOut( + event(new CheckoutableCheckedOut( + $this->createAsset(), $checkoutTarget(), - User::factory()->superuser()->create()->id - ); + User::factory()->superuser()->create(), + '' + )); Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutAssetNotification::class); } From 134ab631d43480fdeb014130f01538f06827e1ba Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 30 Mar 2023 15:38:22 -0700 Subject: [PATCH 15/36] Add test to ensure notification is not sent for license checkouts if not enabled --- .../LicenseCheckoutWebhookNotificationTest.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php index e01a2dc62b..095e0c436c 100644 --- a/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php @@ -44,4 +44,19 @@ class LicenseCheckoutWebhookNotificationTest extends TestCase } ); } + + /** @dataProvider checkoutTargets */ + public function testWebhookNotificationsAreNotSentOnLicenseCheckoutWhenWebhookSettingNotEnabled($checkoutTarget) + { + Notification::fake(); + + event(new CheckoutableCheckedOut( + LicenseSeat::factory()->create(), + $checkoutTarget(), + User::factory()->superuser()->create(), + '' + )); + + Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutLicenseSeatNotification::class); + } } From aefc53cfcf9e5519b89b724249a559d98e2c35e0 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 30 Mar 2023 16:40:21 -0700 Subject: [PATCH 16/36] Explicitly disable webhook settings in tests --- database/factories/SettingFactory.php | 9 +++++++++ .../AssetCheckoutWebhookNotificationTest.php | 2 ++ .../LicenseCheckoutWebhookNotificationTest.php | 2 ++ 3 files changed, 13 insertions(+) diff --git a/database/factories/SettingFactory.php b/database/factories/SettingFactory.php index acbd6afafa..2e19f831d7 100644 --- a/database/factories/SettingFactory.php +++ b/database/factories/SettingFactory.php @@ -52,4 +52,13 @@ class SettingFactory extends Factory 'webhook_channel' => '#it', ]); } + + public function withWebhookDisabled() + { + return $this->state(fn() => [ + 'webhook_botname' => '', + 'webhook_endpoint' => '', + 'webhook_channel' => '', + ]); + } } diff --git a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php index 535bd62a3d..1523b93c09 100644 --- a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php @@ -61,6 +61,8 @@ class AssetCheckoutWebhookNotificationTest extends TestCase { Notification::fake(); + Setting::factory()->withWebhookDisabled()->create(); + event(new CheckoutableCheckedOut( $this->createAsset(), $checkoutTarget(), diff --git a/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php index 095e0c436c..57948a14b6 100644 --- a/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php @@ -50,6 +50,8 @@ class LicenseCheckoutWebhookNotificationTest extends TestCase { Notification::fake(); + Setting::factory()->withWebhookDisabled()->create(); + event(new CheckoutableCheckedOut( LicenseSeat::factory()->create(), $checkoutTarget(), From b2292db3c84705fdd14fa2c78ba253a51f98350c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 30 Mar 2023 16:40:53 -0700 Subject: [PATCH 17/36] Implement tests for webhook notifications on consumable checkout --- ...sumableCheckoutWebhookNotificationTest.php | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 tests/Feature/Notifications/ConsumableCheckoutWebhookNotificationTest.php diff --git a/tests/Feature/Notifications/ConsumableCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/ConsumableCheckoutWebhookNotificationTest.php new file mode 100644 index 0000000000..c69576007f --- /dev/null +++ b/tests/Feature/Notifications/ConsumableCheckoutWebhookNotificationTest.php @@ -0,0 +1,53 @@ +withWebhookEnabled()->create(); + + 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 testWebhookNotificationsAreNotSentOnConsumableCheckoutWhenWebhookSettingNotEnabled() + { + Notification::fake(); + + Setting::factory()->withWebhookDisabled()->create(); + + event(new CheckoutableCheckedOut( + Consumable::factory()->cardstock()->create(), + User::factory()->create(), + User::factory()->superuser()->create(), + '' + )); + + Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutConsumableNotification::class); + } +} From 524249d4d707a62067e52e503c4dbcbb577da606 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 30 Mar 2023 16:58:16 -0700 Subject: [PATCH 18/36] Implement tests for webhook notifications on accessory checkout --- ...cessoryCheckoutWebhookNotificationTest.php | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 tests/Feature/Notifications/AccessoryCheckoutWebhookNotificationTest.php diff --git a/tests/Feature/Notifications/AccessoryCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/AccessoryCheckoutWebhookNotificationTest.php new file mode 100644 index 0000000000..04bb0f2186 --- /dev/null +++ b/tests/Feature/Notifications/AccessoryCheckoutWebhookNotificationTest.php @@ -0,0 +1,54 @@ +withWebhookEnabled()->create(); + + 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 testWebhookNotificationsAreNotSentOnAccessoryCheckoutWhenWebhookSettingNotEnabled() + { + Notification::fake(); + + Setting::factory()->withWebhookDisabled()->create(); + + event(new CheckoutableCheckedOut( + Accessory::factory()->appleBtKeyboard()->create(), + User::factory()->create(), + User::factory()->superuser()->create(), + '' + )); + + Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutAccessoryNotification::class); + } +} From b41902976bb9a376ea32120f9966c11ca715c89f Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 5 Apr 2023 12:23:11 -0700 Subject: [PATCH 19/36] Improve test class names --- ...outWebhookNotificationTest.php => AccessoryWebhookTest.php} | 3 +-- ...heckoutWebhookNotificationTest.php => AssetWebhookTest.php} | 2 +- ...utWebhookNotificationTest.php => ConsumableWebhookTest.php} | 2 +- ...ckoutWebhookNotificationTest.php => LicenseWebhookTest.php} | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) rename tests/Feature/Notifications/{AccessoryCheckoutWebhookNotificationTest.php => AccessoryWebhookTest.php} (96%) rename tests/Feature/Notifications/{AssetCheckoutWebhookNotificationTest.php => AssetWebhookTest.php} (97%) rename tests/Feature/Notifications/{ConsumableCheckoutWebhookNotificationTest.php => ConsumableWebhookTest.php} (96%) rename tests/Feature/Notifications/{LicenseCheckoutWebhookNotificationTest.php => LicenseWebhookTest.php} (96%) diff --git a/tests/Feature/Notifications/AccessoryCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/AccessoryWebhookTest.php similarity index 96% rename from tests/Feature/Notifications/AccessoryCheckoutWebhookNotificationTest.php rename to tests/Feature/Notifications/AccessoryWebhookTest.php index 04bb0f2186..2350f4e9f7 100644 --- a/tests/Feature/Notifications/AccessoryCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/AccessoryWebhookTest.php @@ -2,7 +2,6 @@ namespace Tests\Feature\Notifications; - use App\Events\CheckoutableCheckedOut; use App\Models\Accessory; use App\Models\Setting; @@ -12,7 +11,7 @@ use Illuminate\Notifications\AnonymousNotifiable; use Illuminate\Support\Facades\Notification; use Tests\TestCase; -class AccessoryCheckoutWebhookNotificationTest extends TestCase +class AccessoryWebhookTest extends TestCase { public function testWebhookNotificationsAreSentOnAccessoryCheckoutWhenWebhookSettingEnabled() { diff --git a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/AssetWebhookTest.php similarity index 97% rename from tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php rename to tests/Feature/Notifications/AssetWebhookTest.php index 1523b93c09..e153de40f6 100644 --- a/tests/Feature/Notifications/AssetCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/AssetWebhookTest.php @@ -13,7 +13,7 @@ use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Notification; use Tests\TestCase; -class AssetCheckoutWebhookNotificationTest extends TestCase +class AssetWebhookTest extends TestCase { public function checkoutTargets() { diff --git a/tests/Feature/Notifications/ConsumableCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/ConsumableWebhookTest.php similarity index 96% rename from tests/Feature/Notifications/ConsumableCheckoutWebhookNotificationTest.php rename to tests/Feature/Notifications/ConsumableWebhookTest.php index c69576007f..9313831f57 100644 --- a/tests/Feature/Notifications/ConsumableCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/ConsumableWebhookTest.php @@ -11,7 +11,7 @@ use Illuminate\Notifications\AnonymousNotifiable; use Illuminate\Support\Facades\Notification; use Tests\TestCase; -class ConsumableCheckoutWebhookNotificationTest extends TestCase +class ConsumableWebhookTest extends TestCase { public function testWebhookNotificationsAreSentOnConsumableCheckoutWhenWebhookSettingEnabled() { diff --git a/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php b/tests/Feature/Notifications/LicenseWebhookTest.php similarity index 96% rename from tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php rename to tests/Feature/Notifications/LicenseWebhookTest.php index 57948a14b6..f20ffc9937 100644 --- a/tests/Feature/Notifications/LicenseCheckoutWebhookNotificationTest.php +++ b/tests/Feature/Notifications/LicenseWebhookTest.php @@ -12,7 +12,7 @@ use Illuminate\Notifications\AnonymousNotifiable; use Illuminate\Support\Facades\Notification; use Tests\TestCase; -class LicenseCheckoutWebhookNotificationTest extends TestCase +class LicenseWebhookTest extends TestCase { public function checkoutTargets() { From 2187310abb51f26e5e66505b52475f8db4889c1e Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 5 Apr 2023 12:27:18 -0700 Subject: [PATCH 20/36] Simplify test case names --- tests/Feature/Notifications/AccessoryWebhookTest.php | 4 ++-- tests/Feature/Notifications/AssetWebhookTest.php | 4 ++-- tests/Feature/Notifications/ConsumableWebhookTest.php | 4 ++-- tests/Feature/Notifications/LicenseWebhookTest.php | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/Feature/Notifications/AccessoryWebhookTest.php b/tests/Feature/Notifications/AccessoryWebhookTest.php index 2350f4e9f7..967faf9c86 100644 --- a/tests/Feature/Notifications/AccessoryWebhookTest.php +++ b/tests/Feature/Notifications/AccessoryWebhookTest.php @@ -13,7 +13,7 @@ use Tests\TestCase; class AccessoryWebhookTest extends TestCase { - public function testWebhookNotificationsAreSentOnAccessoryCheckoutWhenWebhookSettingEnabled() + public function testAccessoryCheckoutSendsWebhookNotificationWhenSettingEnabled() { Notification::fake(); @@ -35,7 +35,7 @@ class AccessoryWebhookTest extends TestCase ); } - public function testWebhookNotificationsAreNotSentOnAccessoryCheckoutWhenWebhookSettingNotEnabled() + public function testAccessoryCheckoutDoesNotSendWebhookNotificationWhenSettingDisabled() { Notification::fake(); diff --git a/tests/Feature/Notifications/AssetWebhookTest.php b/tests/Feature/Notifications/AssetWebhookTest.php index e153de40f6..fad1deb1ce 100644 --- a/tests/Feature/Notifications/AssetWebhookTest.php +++ b/tests/Feature/Notifications/AssetWebhookTest.php @@ -34,7 +34,7 @@ class AssetWebhookTest extends TestCase } /** @dataProvider checkoutTargets */ - public function testWebhookNotificationsAreSentOnAssetCheckoutWhenWebhookSettingEnabled($checkoutTarget) + public function testAssetCheckoutSendsWebhookNotificationWhenSettingEnabled($checkoutTarget) { Notification::fake(); @@ -57,7 +57,7 @@ class AssetWebhookTest extends TestCase } /** @dataProvider checkoutTargets */ - public function testWebhookNotificationsAreNotSentOnAssetCheckoutWhenWebhookSettingNotEnabled($checkoutTarget) + public function testAssetCheckoutDoesNotSendWebhookNotificationWhenSettingDisabled($checkoutTarget) { Notification::fake(); diff --git a/tests/Feature/Notifications/ConsumableWebhookTest.php b/tests/Feature/Notifications/ConsumableWebhookTest.php index 9313831f57..ec3c0045a3 100644 --- a/tests/Feature/Notifications/ConsumableWebhookTest.php +++ b/tests/Feature/Notifications/ConsumableWebhookTest.php @@ -13,7 +13,7 @@ use Tests\TestCase; class ConsumableWebhookTest extends TestCase { - public function testWebhookNotificationsAreSentOnConsumableCheckoutWhenWebhookSettingEnabled() + public function testConsumableCheckoutSendsWebhookNotificationWhenSettingEnabled() { Notification::fake(); @@ -35,7 +35,7 @@ class ConsumableWebhookTest extends TestCase ); } - public function testWebhookNotificationsAreNotSentOnConsumableCheckoutWhenWebhookSettingNotEnabled() + public function testConsumableCheckoutDoesNotSendWebhookNotificationWhenSettingDisabled() { Notification::fake(); diff --git a/tests/Feature/Notifications/LicenseWebhookTest.php b/tests/Feature/Notifications/LicenseWebhookTest.php index f20ffc9937..f12fcceaea 100644 --- a/tests/Feature/Notifications/LicenseWebhookTest.php +++ b/tests/Feature/Notifications/LicenseWebhookTest.php @@ -23,7 +23,7 @@ class LicenseWebhookTest extends TestCase } /** @dataProvider checkoutTargets */ - public function testWebhookNotificationsAreSentOnLicenseCheckoutWhenWebhookSettingEnabled($checkoutTarget) + public function testLicenseCheckoutSendsWebhookNotificationWhenSettingEnabled($checkoutTarget) { Notification::fake(); @@ -46,7 +46,7 @@ class LicenseWebhookTest extends TestCase } /** @dataProvider checkoutTargets */ - public function testWebhookNotificationsAreNotSentOnLicenseCheckoutWhenWebhookSettingNotEnabled($checkoutTarget) + public function testLicenseCheckoutDoesNotSendWebhookNotificationWhenSettingDisabled($checkoutTarget) { Notification::fake(); From 3cc3662992e329320e783084a7815f8bbe2fc576 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 5 Apr 2023 12:36:24 -0700 Subject: [PATCH 21/36] Send webhook notifications for accessory checkins when enabled --- app/Listeners/CheckoutableListener.php | 19 ++++++--- .../Notifications/AccessoryWebhookTest.php | 40 +++++++++++++++++++ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 2133876789..a46c47e0e0 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -36,7 +36,7 @@ class CheckoutableListener // @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) { + if ($this->shouldSendWebhookNotification()) { Notification::route('slack', Setting::getSettings()->webhook_endpoint) ->notify($this->getCheckoutNotification($event)); } @@ -71,11 +71,13 @@ class CheckoutableListener { \Log::debug('onCheckedIn in the Checkoutable listener fired'); - /** - * When the item wasn't checked out to a user, we can't send notifications - */ - if (! $event->checkedOutTo instanceof User) { - return; + // @todo: update docblock + + // @todo: comment...we send this anonymously so that webhook notification still + // @todo: get sent for models that don't have email addresses associated... + if ($this->shouldSendWebhookNotification()) { + Notification::route('slack', Setting::getSettings()->webhook_endpoint) + ->notify($this->getCheckinNotification($event)); } /** @@ -227,4 +229,9 @@ class CheckoutableListener 'App\Listeners\CheckoutableListener@onCheckedOut' ); } + + private function shouldSendWebhookNotification(): bool + { + return Setting::getSettings() && Setting::getSettings()->webhook_endpoint; + } } diff --git a/tests/Feature/Notifications/AccessoryWebhookTest.php b/tests/Feature/Notifications/AccessoryWebhookTest.php index 967faf9c86..8272202d8c 100644 --- a/tests/Feature/Notifications/AccessoryWebhookTest.php +++ b/tests/Feature/Notifications/AccessoryWebhookTest.php @@ -2,10 +2,12 @@ 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; @@ -50,4 +52,42 @@ class AccessoryWebhookTest extends TestCase Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutAccessoryNotification::class); } + + public function testAccessoryCheckinSendsWebhookNotificationWhenSettingEnabled() + { + Notification::fake(); + + Setting::factory()->withWebhookEnabled()->create(); + + 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(); + + Setting::factory()->withWebhookDisabled()->create(); + + event(new CheckoutableCheckedIn( + Accessory::factory()->appleBtKeyboard()->create(), + User::factory()->create(), + User::factory()->superuser()->create(), + '' + )); + + Notification::assertNotSentTo(new AnonymousNotifiable, CheckinAccessoryNotification::class); + } } From 3054d633b0ac85538f5c2361f7663e9f4c30cdfa Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 5 Apr 2023 12:39:41 -0700 Subject: [PATCH 22/36] Improve comments and remove unused imports --- app/Listeners/CheckoutableListener.php | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index a46c47e0e0..c990dc211e 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -9,15 +9,12 @@ 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; @@ -26,16 +23,12 @@ use Log; class CheckoutableListener { /** - * 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 and add a record to the * checkout_requests table. */ public function onCheckedOut($event) { - - // @todo: update docblock - - // @todo: comment...we send this anonymously so that webhook notification still - // @todo: get sent for models that don't have email addresses associated... + // Send an anonymous webhook notification if setting enabled if ($this->shouldSendWebhookNotification()) { Notification::route('slack', Setting::getSettings()->webhook_endpoint) ->notify($this->getCheckoutNotification($event)); @@ -65,16 +58,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'); - // @todo: update docblock - - // @todo: comment...we send this anonymously so that webhook notification still - // @todo: get sent for models that don't have email addresses associated... + // Send an anonymous webhook notification if setting enabled if ($this->shouldSendWebhookNotification()) { Notification::route('slack', Setting::getSettings()->webhook_endpoint) ->notify($this->getCheckinNotification($event)); From 5b4d5cadf4e8ba58c38d8cde5ec21e967b866cfc Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 5 Apr 2023 12:57:53 -0700 Subject: [PATCH 23/36] Add tests for sending webhooks on asset and license seat checkin --- .../Notifications/AssetWebhookTest.php | 44 ++++++++++++++++++- .../Notifications/LicenseWebhookTest.php | 44 ++++++++++++++++++- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Notifications/AssetWebhookTest.php b/tests/Feature/Notifications/AssetWebhookTest.php index fad1deb1ce..0e1ef9f201 100644 --- a/tests/Feature/Notifications/AssetWebhookTest.php +++ b/tests/Feature/Notifications/AssetWebhookTest.php @@ -2,11 +2,13 @@ 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\Event; @@ -15,7 +17,7 @@ use Tests\TestCase; class AssetWebhookTest extends TestCase { - public function checkoutTargets() + public function checkoutTargets(): array { return [ 'Asset checked out to user' => [fn() => User::factory()->create()], @@ -73,6 +75,46 @@ class AssetWebhookTest extends TestCase Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutAssetNotification::class); } + /** @dataProvider checkoutTargets */ + public function testAssetCheckinSendsWebhookNotificationWhenSettingEnabled($checkoutTarget) + { + Notification::fake(); + + Setting::factory()->withWebhookEnabled()->create(); + + 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 checkoutTargets */ + public function testAssetCheckinDoesNotSendWebhookNotificationWhenSettingDisabled($checkoutTarget) + { + Notification::fake(); + + Setting::factory()->withWebhookDisabled()->create(); + + event(new CheckoutableCheckedIn( + $this->createAsset(), + $checkoutTarget(), + User::factory()->superuser()->create(), + '' + )); + + Notification::assertNotSentTo(new AnonymousNotifiable, CheckinAssetNotification::class); + } + private function createAsset() { return Asset::factory()->laptopMbp()->create(); diff --git a/tests/Feature/Notifications/LicenseWebhookTest.php b/tests/Feature/Notifications/LicenseWebhookTest.php index f12fcceaea..9931f80a2c 100644 --- a/tests/Feature/Notifications/LicenseWebhookTest.php +++ b/tests/Feature/Notifications/LicenseWebhookTest.php @@ -2,11 +2,13 @@ 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; @@ -14,7 +16,7 @@ use Tests\TestCase; class LicenseWebhookTest extends TestCase { - public function checkoutTargets() + public function checkoutTargets(): array { return [ 'License checked out to user' => [fn() => User::factory()->create()], @@ -61,4 +63,44 @@ class LicenseWebhookTest extends TestCase Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutLicenseSeatNotification::class); } + + /** @dataProvider checkoutTargets */ + public function testLicenseCheckinSendsWebhookNotificationWhenSettingEnabled($checkoutTarget) + { + Notification::fake(); + + Setting::factory()->withWebhookEnabled()->create(); + + 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 checkoutTargets */ + public function testLicenseCheckinDoesNotSendWebhookNotificationWhenSettingDisabled($checkoutTarget) + { + Notification::fake(); + + Setting::factory()->withWebhookDisabled()->create(); + + event(new CheckoutableCheckedIn( + LicenseSeat::factory()->create(), + $checkoutTarget(), + User::factory()->superuser()->create(), + '' + )); + + Notification::assertNotSentTo(new AnonymousNotifiable, CheckinLicenseSeatNotification::class); + } } From f6cff908297e52b1e17d716f78434054318b7073 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 17 Apr 2023 17:31:12 -0700 Subject: [PATCH 24/36] Migrate to new test settings interface --- .../Notifications/AccessoryWebhookTest.php | 11 +++++++---- .../Feature/Notifications/AssetWebhookTest.php | 11 +++++++---- .../Notifications/ConsumableWebhookTest.php | 7 +++++-- .../Notifications/LicenseWebhookTest.php | 11 +++++++---- tests/Support/Settings.php | 18 ++++++++++++++++++ 5 files changed, 44 insertions(+), 14 deletions(-) diff --git a/tests/Feature/Notifications/AccessoryWebhookTest.php b/tests/Feature/Notifications/AccessoryWebhookTest.php index 8272202d8c..a1db59b985 100644 --- a/tests/Feature/Notifications/AccessoryWebhookTest.php +++ b/tests/Feature/Notifications/AccessoryWebhookTest.php @@ -11,15 +11,18 @@ 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(); - Setting::factory()->withWebhookEnabled()->create(); + $this->settings->enableWebhook(); event(new CheckoutableCheckedOut( Accessory::factory()->appleBtKeyboard()->create(), @@ -41,7 +44,7 @@ class AccessoryWebhookTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookDisabled()->create(); + $this->settings->disableWebhook(); event(new CheckoutableCheckedOut( Accessory::factory()->appleBtKeyboard()->create(), @@ -57,7 +60,7 @@ class AccessoryWebhookTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookEnabled()->create(); + $this->settings->enableWebhook(); event(new CheckoutableCheckedIn( Accessory::factory()->appleBtKeyboard()->create(), @@ -79,7 +82,7 @@ class AccessoryWebhookTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookDisabled()->create(); + $this->settings->disableWebhook(); event(new CheckoutableCheckedIn( Accessory::factory()->appleBtKeyboard()->create(), diff --git a/tests/Feature/Notifications/AssetWebhookTest.php b/tests/Feature/Notifications/AssetWebhookTest.php index 0e1ef9f201..c931b565a0 100644 --- a/tests/Feature/Notifications/AssetWebhookTest.php +++ b/tests/Feature/Notifications/AssetWebhookTest.php @@ -13,10 +13,13 @@ use App\Notifications\CheckoutAssetNotification; use Illuminate\Notifications\AnonymousNotifiable; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Notification; +use Tests\Support\InteractsWithSettings; use Tests\TestCase; class AssetWebhookTest extends TestCase { + use InteractsWithSettings; + public function checkoutTargets(): array { return [ @@ -40,7 +43,7 @@ class AssetWebhookTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookEnabled()->create(); + $this->settings->enableWebhook(); event(new CheckoutableCheckedOut( $this->createAsset(), @@ -63,7 +66,7 @@ class AssetWebhookTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookDisabled()->create(); + $this->settings->disableWebhook(); event(new CheckoutableCheckedOut( $this->createAsset(), @@ -80,7 +83,7 @@ class AssetWebhookTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookEnabled()->create(); + $this->settings->enableWebhook(); event(new CheckoutableCheckedIn( $this->createAsset(), @@ -103,7 +106,7 @@ class AssetWebhookTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookDisabled()->create(); + $this->settings->disableWebhook(); event(new CheckoutableCheckedIn( $this->createAsset(), diff --git a/tests/Feature/Notifications/ConsumableWebhookTest.php b/tests/Feature/Notifications/ConsumableWebhookTest.php index ec3c0045a3..854fdf5341 100644 --- a/tests/Feature/Notifications/ConsumableWebhookTest.php +++ b/tests/Feature/Notifications/ConsumableWebhookTest.php @@ -9,15 +9,18 @@ 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(); - Setting::factory()->withWebhookEnabled()->create(); + $this->settings->enableWebhook(); event(new CheckoutableCheckedOut( Consumable::factory()->cardstock()->create(), @@ -39,7 +42,7 @@ class ConsumableWebhookTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookDisabled()->create(); + $this->settings->disableWebhook(); event(new CheckoutableCheckedOut( Consumable::factory()->cardstock()->create(), diff --git a/tests/Feature/Notifications/LicenseWebhookTest.php b/tests/Feature/Notifications/LicenseWebhookTest.php index 9931f80a2c..da511d880f 100644 --- a/tests/Feature/Notifications/LicenseWebhookTest.php +++ b/tests/Feature/Notifications/LicenseWebhookTest.php @@ -12,10 +12,13 @@ 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 checkoutTargets(): array { return [ @@ -29,7 +32,7 @@ class LicenseWebhookTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookEnabled()->create(); + $this->settings->enableWebhook(); event(new CheckoutableCheckedOut( LicenseSeat::factory()->create(), @@ -52,7 +55,7 @@ class LicenseWebhookTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookDisabled()->create(); + $this->settings->disableWebhook(); event(new CheckoutableCheckedOut( LicenseSeat::factory()->create(), @@ -69,7 +72,7 @@ class LicenseWebhookTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookEnabled()->create(); + $this->settings->enableWebhook(); event(new CheckoutableCheckedIn( LicenseSeat::factory()->create(), @@ -92,7 +95,7 @@ class LicenseWebhookTest extends TestCase { Notification::fake(); - Setting::factory()->withWebhookDisabled()->create(); + $this->settings->disableWebhook(); event(new CheckoutableCheckedIn( LicenseSeat::factory()->create(), diff --git a/tests/Support/Settings.php b/tests/Support/Settings.php index ccf50c3ce3..9d4209da79 100644 --- a/tests/Support/Settings.php +++ b/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. */ From 508660b1df1be099e929a8366be54f3ef7d4f3d9 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 18 Apr 2023 13:07:55 -0700 Subject: [PATCH 25/36] Skip notifications for component checkouts and checkins --- app/Listeners/CheckoutableListener.php | 18 +++++++ .../Notifications/ComponentWebhookTest.php | 50 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 tests/Feature/Notifications/ComponentWebhookTest.php diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index c990dc211e..01d74df85d 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -5,6 +5,7 @@ namespace App\Listeners; 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; @@ -22,12 +23,20 @@ use Log; class CheckoutableListener { + private array $skipNotificationsFor = [ + Component::class, + ]; + /** * Notify the user and post to webhook about the checked out checkoutable and add a record to the * checkout_requests table. */ public function onCheckedOut($event) { + if ($this->shouldNotSendAnyNotifications($event->checkoutable)){ + return; + } + // Send an anonymous webhook notification if setting enabled if ($this->shouldSendWebhookNotification()) { Notification::route('slack', Setting::getSettings()->webhook_endpoint) @@ -64,6 +73,10 @@ class CheckoutableListener { \Log::debug('onCheckedIn in the Checkoutable listener fired'); + if ($this->shouldNotSendAnyNotifications($event->checkoutable)) { + return; + } + // Send an anonymous webhook notification if setting enabled if ($this->shouldSendWebhookNotification()) { Notification::route('slack', Setting::getSettings()->webhook_endpoint) @@ -220,6 +233,11 @@ class CheckoutableListener ); } + private function shouldNotSendAnyNotifications($checkoutable): bool + { + return in_array(get_class($checkoutable), $this->skipNotificationsFor); + } + private function shouldSendWebhookNotification(): bool { return Setting::getSettings() && Setting::getSettings()->webhook_endpoint; diff --git a/tests/Feature/Notifications/ComponentWebhookTest.php b/tests/Feature/Notifications/ComponentWebhookTest.php new file mode 100644 index 0000000000..8f3a51b15d --- /dev/null +++ b/tests/Feature/Notifications/ComponentWebhookTest.php @@ -0,0 +1,50 @@ +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(); + } +} From bad2eead4c441aaad60f9078bb85aa7cbb4f5893 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 18 Apr 2023 13:13:57 -0700 Subject: [PATCH 26/36] Remove test method --- tests/Feature/Notifications/AssetWebhookTest.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/Feature/Notifications/AssetWebhookTest.php b/tests/Feature/Notifications/AssetWebhookTest.php index c931b565a0..41b2598618 100644 --- a/tests/Feature/Notifications/AssetWebhookTest.php +++ b/tests/Feature/Notifications/AssetWebhookTest.php @@ -11,7 +11,6 @@ use App\Models\User; use App\Notifications\CheckinAssetNotification; use App\Notifications\CheckoutAssetNotification; use Illuminate\Notifications\AnonymousNotifiable; -use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Notification; use Tests\Support\InteractsWithSettings; use Tests\TestCase; @@ -29,15 +28,6 @@ class AssetWebhookTest extends TestCase ]; } - public function testAssetCheckoutFiresCheckoutEvent() - { - Event::fake([CheckoutableCheckedOut::class]); - - $this->createAsset()->checkOut(User::factory()->create(), User::factory()->create()); - - Event::assertDispatched(CheckoutableCheckedOut::class); - } - /** @dataProvider checkoutTargets */ public function testAssetCheckoutSendsWebhookNotificationWhenSettingEnabled($checkoutTarget) { From 835f8876c4a7705534205c81892d2ba5bcbed9e8 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 19 Apr 2023 12:26:48 -0700 Subject: [PATCH 27/36] Move notification sending into try catch block --- app/Listeners/CheckoutableListener.php | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 01d74df85d..6099c8f2e3 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -37,19 +37,18 @@ class CheckoutableListener return; } - // Send an anonymous webhook notification if setting enabled - if ($this->shouldSendWebhookNotification()) { - Notification::route('slack', Setting::getSettings()->webhook_endpoint) - ->notify($this->getCheckoutNotification($event)); - } - /** * Make a checkout acceptance and attach it in the notification */ $acceptance = $this->getCheckoutAcceptance($event); try { - // @todo: wrap notification above in this try + // Send an anonymous webhook notification if setting enabled + 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), @@ -77,12 +76,6 @@ class CheckoutableListener return; } - // Send an anonymous webhook notification if setting enabled - if ($this->shouldSendWebhookNotification()) { - Notification::route('slack', Setting::getSettings()->webhook_endpoint) - ->notify($this->getCheckinNotification($event)); - } - /** * Send the appropriate notification */ @@ -97,6 +90,12 @@ class CheckoutableListener } try { + // Send an anonymous webhook notification if setting enabled + 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( From 144382e57a78e0f6446e04ce99038d4d83c65c23 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 19 Apr 2023 12:31:12 -0700 Subject: [PATCH 28/36] Update docblock types --- app/Listeners/CheckoutableListener.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 6099c8f2e3..37e8b7f888 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -2,6 +2,7 @@ namespace App\Listeners; +use App\Events\CheckoutableCheckedOut; use App\Models\Accessory; use App\Models\Asset; use App\Models\CheckoutAcceptance; @@ -188,8 +189,8 @@ class CheckoutableListener /** * Get the appropriate notification for the event * - * @param CheckoutableCheckedIn $event - * @param CheckoutAcceptance $acceptance + * @param CheckoutableCheckedOut $event + * @param CheckoutAcceptance|null $acceptance * @return Notification */ private function getCheckoutNotification($event, $acceptance = null) From e92c1e7beaeccec4554c7461ad7a8b181fff54ff Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 19 Apr 2023 17:22:56 -0700 Subject: [PATCH 29/36] Minor docblock update --- app/Listeners/CheckoutableListener.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 37e8b7f888..38e5819b80 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -29,8 +29,8 @@ class CheckoutableListener ]; /** - * Notify the user and post to webhook about the checked out checkoutable and add a record to the - * checkout_requests table. + * Notify the user and post to webhook about the checked out checkoutable + * and add a record to the checkout_requests table. */ public function onCheckedOut($event) { From dd40798c43d0c4d725eba7db9f10d42b5d5676f4 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 19 Apr 2023 17:49:31 -0700 Subject: [PATCH 30/36] Remove unneeded comments --- app/Listeners/CheckoutableListener.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 38e5819b80..09cb3ae8f2 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -44,7 +44,6 @@ class CheckoutableListener $acceptance = $this->getCheckoutAcceptance($event); try { - // Send an anonymous webhook notification if setting enabled if ($this->shouldSendWebhookNotification()) { Notification::route('slack', Setting::getSettings()->webhook_endpoint) ->notify($this->getCheckoutNotification($event)); @@ -91,7 +90,6 @@ class CheckoutableListener } try { - // Send an anonymous webhook notification if setting enabled if ($this->shouldSendWebhookNotification()) { Notification::route('slack', Setting::getSettings()->webhook_endpoint) ->notify($this->getCheckinNotification($event)); From 645f6ed692bf33a09eccd680a11e3ab685ecafde Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 19 Apr 2023 17:52:44 -0700 Subject: [PATCH 31/36] Remove unneeded doc block --- database/factories/LicenseSeatFactory.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/database/factories/LicenseSeatFactory.php b/database/factories/LicenseSeatFactory.php index c4d43f89b3..3c6cc4246b 100644 --- a/database/factories/LicenseSeatFactory.php +++ b/database/factories/LicenseSeatFactory.php @@ -7,11 +7,6 @@ use Illuminate\Database\Eloquent\Factories\Factory; class LicenseSeatFactory extends Factory { - /** - * Define the model's default state. - * - * @return array - */ public function definition() { return [ From c357d9f01efe7a0ef07d511f6bb40728b84779de Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 19 Apr 2023 18:10:23 -0700 Subject: [PATCH 32/36] Update data provider name --- tests/Feature/Notifications/AssetWebhookTest.php | 10 +++++----- tests/Feature/Notifications/LicenseWebhookTest.php | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/Feature/Notifications/AssetWebhookTest.php b/tests/Feature/Notifications/AssetWebhookTest.php index 41b2598618..ae45a4caa4 100644 --- a/tests/Feature/Notifications/AssetWebhookTest.php +++ b/tests/Feature/Notifications/AssetWebhookTest.php @@ -19,7 +19,7 @@ class AssetWebhookTest extends TestCase { use InteractsWithSettings; - public function checkoutTargets(): array + public function targets(): array { return [ 'Asset checked out to user' => [fn() => User::factory()->create()], @@ -28,7 +28,7 @@ class AssetWebhookTest extends TestCase ]; } - /** @dataProvider checkoutTargets */ + /** @dataProvider targets */ public function testAssetCheckoutSendsWebhookNotificationWhenSettingEnabled($checkoutTarget) { Notification::fake(); @@ -51,7 +51,7 @@ class AssetWebhookTest extends TestCase ); } - /** @dataProvider checkoutTargets */ + /** @dataProvider targets */ public function testAssetCheckoutDoesNotSendWebhookNotificationWhenSettingDisabled($checkoutTarget) { Notification::fake(); @@ -68,7 +68,7 @@ class AssetWebhookTest extends TestCase Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutAssetNotification::class); } - /** @dataProvider checkoutTargets */ + /** @dataProvider targets */ public function testAssetCheckinSendsWebhookNotificationWhenSettingEnabled($checkoutTarget) { Notification::fake(); @@ -91,7 +91,7 @@ class AssetWebhookTest extends TestCase ); } - /** @dataProvider checkoutTargets */ + /** @dataProvider targets */ public function testAssetCheckinDoesNotSendWebhookNotificationWhenSettingDisabled($checkoutTarget) { Notification::fake(); diff --git a/tests/Feature/Notifications/LicenseWebhookTest.php b/tests/Feature/Notifications/LicenseWebhookTest.php index da511d880f..4313ff69d4 100644 --- a/tests/Feature/Notifications/LicenseWebhookTest.php +++ b/tests/Feature/Notifications/LicenseWebhookTest.php @@ -19,7 +19,7 @@ class LicenseWebhookTest extends TestCase { use InteractsWithSettings; - public function checkoutTargets(): array + public function targets(): array { return [ 'License checked out to user' => [fn() => User::factory()->create()], @@ -27,7 +27,7 @@ class LicenseWebhookTest extends TestCase ]; } - /** @dataProvider checkoutTargets */ + /** @dataProvider targets */ public function testLicenseCheckoutSendsWebhookNotificationWhenSettingEnabled($checkoutTarget) { Notification::fake(); @@ -50,7 +50,7 @@ class LicenseWebhookTest extends TestCase ); } - /** @dataProvider checkoutTargets */ + /** @dataProvider targets */ public function testLicenseCheckoutDoesNotSendWebhookNotificationWhenSettingDisabled($checkoutTarget) { Notification::fake(); @@ -67,7 +67,7 @@ class LicenseWebhookTest extends TestCase Notification::assertNotSentTo(new AnonymousNotifiable, CheckoutLicenseSeatNotification::class); } - /** @dataProvider checkoutTargets */ + /** @dataProvider targets */ public function testLicenseCheckinSendsWebhookNotificationWhenSettingEnabled($checkoutTarget) { Notification::fake(); @@ -90,7 +90,7 @@ class LicenseWebhookTest extends TestCase ); } - /** @dataProvider checkoutTargets */ + /** @dataProvider targets */ public function testLicenseCheckinDoesNotSendWebhookNotificationWhenSettingDisabled($checkoutTarget) { Notification::fake(); From b4f162f316d93e4bc3fbc12a1c3454067b8182af Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 24 Apr 2023 17:26:06 -0700 Subject: [PATCH 33/36] Fix operator in where clause --- app/Console/Commands/CheckoutLicenseToAllUsers.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Console/Commands/CheckoutLicenseToAllUsers.php b/app/Console/Commands/CheckoutLicenseToAllUsers.php index c81408442a..801b3d187a 100644 --- a/app/Console/Commands/CheckoutLicenseToAllUsers.php +++ b/app/Console/Commands/CheckoutLicenseToAllUsers.php @@ -56,7 +56,7 @@ class CheckoutLicenseToAllUsers extends Command return false; } - $users = User::whereNull('deleted_at')->where('autoassign_licenses', '==', 1)->with('licenses')->get(); + $users = User::whereNull('deleted_at')->where('autoassign_licenses', '=', 1)->with('licenses')->get(); if ($users->count() > $license->getAvailSeatsCountAttribute()) { $this->info('You do not have enough free seats to complete this task, so we will check out as many as we can. '); From 37b881b906082af6ddb84ec08bb96eed4d10418d Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 25 Apr 2023 22:32:45 -0700 Subject: [PATCH 34/36] Add @dboth as a contributor --- .all-contributorsrc | 9 +++++++++ README.md | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index dd090262eb..b48da51226 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -2889,6 +2889,15 @@ "avatar_url": "https://avatars.githubusercontent.com/u/570639?v=4", "profile": "https://github.com/Mezzle", "contributions": [] + }, + { + "login": "dboth", + "name": "dboth", + "avatar_url": "https://avatars.githubusercontent.com/u/5731963?v=4", + "profile": "http://dboth.de", + "contributions": [ + "code" + ] } ] } diff --git a/README.md b/README.md index 3a0d37998f..7fa5fae3e9 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,5 @@ ![Build Status](https://app.chipperci.com/projects/0e5f8979-31eb-4ee6-9abf-050b76ab0383/status/master) [![Crowdin](https://d322cqt584bo4o.cloudfront.net/snipe-it/localized.svg)](https://crowdin.com/project/snipe-it) [![Docker Pulls](https://img.shields.io/docker/pulls/snipe/snipe-it.svg)](https://hub.docker.com/r/snipe/snipe-it/) [![Twitter Follow](https://img.shields.io/twitter/follow/snipeitapp.svg?style=social)](https://twitter.com/snipeitapp) [![Codacy Badge](https://api.codacy.com/project/badge/Grade/553ce52037fc43ea99149785afcfe641)](https://www.codacy.com/app/snipe/snipe-it?utm_source=github.com&utm_medium=referral&utm_content=snipe/snipe-it&utm_campaign=Badge_Grade) -[![All Contributors](https://img.shields.io/badge/all_contributors-318-orange.svg?style=flat-square)](#contributors) [![Discord](https://badgen.net/badge/icon/discord?icon=discord&label)](https://discord.gg/yZFtShAcKk) [![huntr](https://cdn.huntr.dev/huntr_security_badge_mono.svg)](https://huntr.dev) +[![All Contributors](https://img.shields.io/badge/all_contributors-319-orange.svg?style=flat-square)](#contributors) [![Discord](https://badgen.net/badge/icon/discord?icon=discord&label)](https://discord.gg/yZFtShAcKk) [![huntr](https://cdn.huntr.dev/huntr_security_badge_mono.svg)](https://huntr.dev) ## Snipe-IT - Open Source Asset Management System @@ -144,7 +144,7 @@ Thanks goes to all of these wonderful people ([emoji key](https://github.com/ken | [
Peace](https://github.com/julian-piehl)
[💻](https://github.com/snipe/snipe-it/commits?author=julian-piehl "Code") | [
Kyle Gordon](https://github.com/kylegordon)
[💻](https://github.com/snipe/snipe-it/commits?author=kylegordon "Code") | [
Katharina Drexel](http://www.bfh.ch)
[💻](https://github.com/snipe/snipe-it/commits?author=sunflowerbofh "Code") | [
David Sferruzza](https://david.sferruzza.fr/)
[💻](https://github.com/snipe/snipe-it/commits?author=dsferruzza "Code") | [
Rick Nelson](https://github.com/rnelsonee)
[💻](https://github.com/snipe/snipe-it/commits?author=rnelsonee "Code") | [
BasO12](https://github.com/BasO12)
[💻](https://github.com/snipe/snipe-it/commits?author=BasO12 "Code") | [
Vautia](https://github.com/Vautia)
[💻](https://github.com/snipe/snipe-it/commits?author=Vautia "Code") | | [
Chris Hartjes](http://www.littlehart.net/atthekeyboard)
[💻](https://github.com/snipe/snipe-it/commits?author=chartjes "Code") | [
geo-chen](https://github.com/geo-chen)
[💻](https://github.com/snipe/snipe-it/commits?author=geo-chen "Code") | [
Phan Nguyen](https://github.com/nh314)
[💻](https://github.com/snipe/snipe-it/commits?author=nh314 "Code") | [
Iisakki Jaakkola](https://github.com/StarlessNights)
[💻](https://github.com/snipe/snipe-it/commits?author=StarlessNights "Code") | [
Ikko Ashimine](https://bandism.net/)
[💻](https://github.com/snipe/snipe-it/commits?author=eltociear "Code") | [
Lukas Fehling](https://github.com/lukasfehling)
[💻](https://github.com/snipe/snipe-it/commits?author=lukasfehling "Code") | [
Fernando Almeida](https://github.com/fernando-almeida)
[💻](https://github.com/snipe/snipe-it/commits?author=fernando-almeida "Code") | | [
akemidx](https://github.com/akemidx)
[💻](https://github.com/snipe/snipe-it/commits?author=akemidx "Code") | [
Oguz Bilgic](http://oguz.site)
[💻](https://github.com/snipe/snipe-it/commits?author=oguzbilgic "Code") | [
Scooter Crawford](https://github.com/scoo73r)
[💻](https://github.com/snipe/snipe-it/commits?author=scoo73r "Code") | [
subdriven](https://github.com/subdriven)
[💻](https://github.com/snipe/snipe-it/commits?author=subdriven "Code") | [
Andrew Savinykh](https://github.com/AndrewSav)
[💻](https://github.com/snipe/snipe-it/commits?author=AndrewSav "Code") | [
Tadayuki Onishi](https://kenchan0130.github.io)
[💻](https://github.com/snipe/snipe-it/commits?author=kenchan0130 "Code") | [
Florian](https://github.com/floschoepfer)
[💻](https://github.com/snipe/snipe-it/commits?author=floschoepfer "Code") | -| [
Spencer Long](http://spencerlong.com)
[💻](https://github.com/snipe/snipe-it/commits?author=spencerrlongg "Code") | [
Marcus Moore](https://github.com/marcusmoore)
[💻](https://github.com/snipe/snipe-it/commits?author=marcusmoore "Code") | [
Martin Meredith](https://github.com/Mezzle)
| +| [
Spencer Long](http://spencerlong.com)
[💻](https://github.com/snipe/snipe-it/commits?author=spencerrlongg "Code") | [
Marcus Moore](https://github.com/marcusmoore)
[💻](https://github.com/snipe/snipe-it/commits?author=marcusmoore "Code") | [
Martin Meredith](https://github.com/Mezzle)
| [
dboth](http://dboth.de)
[💻](https://github.com/snipe/snipe-it/commits?author=dboth "Code") | This project follows the [all-contributors](https://github.com/kentcdodds/all-contributors) specification. Contributions of any kind welcome! From 2299771e10354d40aeaa10d0aea79ea6371759d4 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 25 Apr 2023 22:37:17 -0700 Subject: [PATCH 35/36] Add @zacharyfleck as a contributor --- .all-contributorsrc | 9 +++++++++ README.md | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index b48da51226..eb052687eb 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -2898,6 +2898,15 @@ "contributions": [ "code" ] + }, + { + "login": "zacharyfleck", + "name": "Zachary Fleck", + "avatar_url": "https://avatars.githubusercontent.com/u/87536651?v=4", + "profile": "https://github.com/zacharyfleck", + "contributions": [ + "code" + ] } ] } diff --git a/README.md b/README.md index 7fa5fae3e9..040de8e93c 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,5 @@ ![Build Status](https://app.chipperci.com/projects/0e5f8979-31eb-4ee6-9abf-050b76ab0383/status/master) [![Crowdin](https://d322cqt584bo4o.cloudfront.net/snipe-it/localized.svg)](https://crowdin.com/project/snipe-it) [![Docker Pulls](https://img.shields.io/docker/pulls/snipe/snipe-it.svg)](https://hub.docker.com/r/snipe/snipe-it/) [![Twitter Follow](https://img.shields.io/twitter/follow/snipeitapp.svg?style=social)](https://twitter.com/snipeitapp) [![Codacy Badge](https://api.codacy.com/project/badge/Grade/553ce52037fc43ea99149785afcfe641)](https://www.codacy.com/app/snipe/snipe-it?utm_source=github.com&utm_medium=referral&utm_content=snipe/snipe-it&utm_campaign=Badge_Grade) -[![All Contributors](https://img.shields.io/badge/all_contributors-319-orange.svg?style=flat-square)](#contributors) [![Discord](https://badgen.net/badge/icon/discord?icon=discord&label)](https://discord.gg/yZFtShAcKk) [![huntr](https://cdn.huntr.dev/huntr_security_badge_mono.svg)](https://huntr.dev) +[![All Contributors](https://img.shields.io/badge/all_contributors-320-orange.svg?style=flat-square)](#contributors) [![Discord](https://badgen.net/badge/icon/discord?icon=discord&label)](https://discord.gg/yZFtShAcKk) [![huntr](https://cdn.huntr.dev/huntr_security_badge_mono.svg)](https://huntr.dev) ## Snipe-IT - Open Source Asset Management System @@ -144,7 +144,7 @@ Thanks goes to all of these wonderful people ([emoji key](https://github.com/ken | [
Peace](https://github.com/julian-piehl)
[💻](https://github.com/snipe/snipe-it/commits?author=julian-piehl "Code") | [
Kyle Gordon](https://github.com/kylegordon)
[💻](https://github.com/snipe/snipe-it/commits?author=kylegordon "Code") | [
Katharina Drexel](http://www.bfh.ch)
[💻](https://github.com/snipe/snipe-it/commits?author=sunflowerbofh "Code") | [
David Sferruzza](https://david.sferruzza.fr/)
[💻](https://github.com/snipe/snipe-it/commits?author=dsferruzza "Code") | [
Rick Nelson](https://github.com/rnelsonee)
[💻](https://github.com/snipe/snipe-it/commits?author=rnelsonee "Code") | [
BasO12](https://github.com/BasO12)
[💻](https://github.com/snipe/snipe-it/commits?author=BasO12 "Code") | [
Vautia](https://github.com/Vautia)
[💻](https://github.com/snipe/snipe-it/commits?author=Vautia "Code") | | [
Chris Hartjes](http://www.littlehart.net/atthekeyboard)
[💻](https://github.com/snipe/snipe-it/commits?author=chartjes "Code") | [
geo-chen](https://github.com/geo-chen)
[💻](https://github.com/snipe/snipe-it/commits?author=geo-chen "Code") | [
Phan Nguyen](https://github.com/nh314)
[💻](https://github.com/snipe/snipe-it/commits?author=nh314 "Code") | [
Iisakki Jaakkola](https://github.com/StarlessNights)
[💻](https://github.com/snipe/snipe-it/commits?author=StarlessNights "Code") | [
Ikko Ashimine](https://bandism.net/)
[💻](https://github.com/snipe/snipe-it/commits?author=eltociear "Code") | [
Lukas Fehling](https://github.com/lukasfehling)
[💻](https://github.com/snipe/snipe-it/commits?author=lukasfehling "Code") | [
Fernando Almeida](https://github.com/fernando-almeida)
[💻](https://github.com/snipe/snipe-it/commits?author=fernando-almeida "Code") | | [
akemidx](https://github.com/akemidx)
[💻](https://github.com/snipe/snipe-it/commits?author=akemidx "Code") | [
Oguz Bilgic](http://oguz.site)
[💻](https://github.com/snipe/snipe-it/commits?author=oguzbilgic "Code") | [
Scooter Crawford](https://github.com/scoo73r)
[💻](https://github.com/snipe/snipe-it/commits?author=scoo73r "Code") | [
subdriven](https://github.com/subdriven)
[💻](https://github.com/snipe/snipe-it/commits?author=subdriven "Code") | [
Andrew Savinykh](https://github.com/AndrewSav)
[💻](https://github.com/snipe/snipe-it/commits?author=AndrewSav "Code") | [
Tadayuki Onishi](https://kenchan0130.github.io)
[💻](https://github.com/snipe/snipe-it/commits?author=kenchan0130 "Code") | [
Florian](https://github.com/floschoepfer)
[💻](https://github.com/snipe/snipe-it/commits?author=floschoepfer "Code") | -| [
Spencer Long](http://spencerlong.com)
[💻](https://github.com/snipe/snipe-it/commits?author=spencerrlongg "Code") | [
Marcus Moore](https://github.com/marcusmoore)
[💻](https://github.com/snipe/snipe-it/commits?author=marcusmoore "Code") | [
Martin Meredith](https://github.com/Mezzle)
| [
dboth](http://dboth.de)
[💻](https://github.com/snipe/snipe-it/commits?author=dboth "Code") | +| [
Spencer Long](http://spencerlong.com)
[💻](https://github.com/snipe/snipe-it/commits?author=spencerrlongg "Code") | [
Marcus Moore](https://github.com/marcusmoore)
[💻](https://github.com/snipe/snipe-it/commits?author=marcusmoore "Code") | [
Martin Meredith](https://github.com/Mezzle)
| [
dboth](http://dboth.de)
[💻](https://github.com/snipe/snipe-it/commits?author=dboth "Code") | [
Zachary Fleck](https://github.com/zacharyfleck)
[💻](https://github.com/snipe/snipe-it/commits?author=zacharyfleck "Code") | This project follows the [all-contributors](https://github.com/kentcdodds/all-contributors) specification. Contributions of any kind welcome! From 8f0b823fb35b993d3ca6dc1fad772515ee5dce2d Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 25 Apr 2023 23:39:32 -0700 Subject: [PATCH 36/36] Applies #12868 Signed-off-by: snipe --- app/Http/Controllers/Api/AssetsController.php | 4 +++- app/Http/Controllers/Assets/AssetsController.php | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 8e9c395d0c..1816fc9f63 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -550,7 +550,8 @@ class AssetsController extends Controller $asset->depreciate = '0'; $asset->status_id = $request->get('status_id', 0); $asset->warranty_months = $request->get('warranty_months', null); - $asset->purchase_cost = $request->get('purchase_cost'); // this is the API's store method, so I don't know that I want to do this? Confusing. FIXME (or not?!) + $asset->purchase_cost = $request->get('purchase_cost'); + $asset->asset_eol_date = $request->get('asset_eol_date', $asset->present()->eol_date()); $asset->purchase_date = $request->get('purchase_date', null); $asset->assigned_to = $request->get('assigned_to', null); $asset->supplier_id = $request->get('supplier_id'); @@ -558,6 +559,7 @@ class AssetsController extends Controller $asset->rtd_location_id = $request->get('rtd_location_id', null); $asset->location_id = $request->get('rtd_location_id', null); + /** * this is here just legacy reasons. Api\AssetController * used image_source once to allow encoded image uploads. diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index b385222bd3..2a93845507 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -140,9 +140,9 @@ class AssetsController extends Controller $asset->depreciate = '0'; $asset->status_id = request('status_id'); $asset->warranty_months = request('warranty_months', null); - $asset->purchase_cost = $request->get('purchase_cost'); + $asset->purchase_cost = request('purchase_cost'); $asset->purchase_date = request('purchase_date', null); - $asset->asset_eol_date = request('asset_eol_date', null); + $asset->asset_eol_date = request('asset_eol_date', $asset->present()->eol_date()); $asset->assigned_to = request('assigned_to', null); $asset->supplier_id = request('supplier_id', null); $asset->requestable = request('requestable', 0);