From 3db124e709f2093c97d227f3a75a07ec6ef26892 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 5 Mar 2025 18:12:23 -0800 Subject: [PATCH 1/5] First pass at updating wording for asset checkout mail --- app/Mail/CheckoutAssetMail.php | 28 +++++- database/factories/AssetFactory.php | 10 +++ database/factories/AssetModelFactory.php | 9 ++ database/factories/CategoryFactory.php | 7 ++ .../mail/markdown/checkout-asset.blade.php | 4 +- tests/Unit/Mail/CheckoutAssetMailTest.php | 88 +++++++++++++++++++ 6 files changed, 142 insertions(+), 4 deletions(-) create mode 100644 tests/Unit/Mail/CheckoutAssetMailTest.php diff --git a/app/Mail/CheckoutAssetMail.php b/app/Mail/CheckoutAssetMail.php index 1447dfd6f5..a810ab8c82 100644 --- a/app/Mail/CheckoutAssetMail.php +++ b/app/Mail/CheckoutAssetMail.php @@ -74,7 +74,7 @@ class CheckoutAssetMail extends Mailable { $this->item->load('assetstatus'); $eula = method_exists($this->item, 'getEula') ? $this->item->getEula() : ''; - $req_accept = method_exists($this->item, 'requireAcceptance') ? $this->item->requireAcceptance() : 0; + $req_accept = $this->requiresAcceptance(); $fields = []; // Check if the item has custom fields associated with it @@ -98,6 +98,7 @@ class CheckoutAssetMail extends Mailable 'accept_url' => $accept_url, 'last_checkout' => $this->last_checkout, 'expected_checkin' => $this->expected_checkin, + 'opening_line' => $this->openingLine(), ], ); } @@ -118,6 +119,29 @@ class CheckoutAssetMail extends Mailable return trans('mail.Asset_Checkout_Notification'); } - return trans('mail.unaccepted_asset_reminder'); + return 'Reminder: ' . trans('mail.unaccepted_asset_reminder'); + } + + private function openingLine(): string + { + if ($this->firstTimeSending && $this->requiresAcceptance()) { + return 'A new item has been checked out under your name that requires acceptance, details are below.'; + } + + if ($this->firstTimeSending && !$this->requiresAcceptance()) { + return 'A new item has been checked out under your name, details are below.'; + } + + if (!$this->firstTimeSending && $this->requiresAcceptance()) { + return 'An item was recently checked out under your name that requires acceptance, details are below.'; + } + + // we shouldn't get here but let's send a default message just in case + return trans('new_item_checked'); + } + + private function requiresAcceptance(): int|bool + { + return method_exists($this->item, 'requireAcceptance') ? $this->item->requireAcceptance() : 0; } } diff --git a/database/factories/AssetFactory.php b/database/factories/AssetFactory.php index 4d6d20651c..63b64364e7 100644 --- a/database/factories/AssetFactory.php +++ b/database/factories/AssetFactory.php @@ -4,6 +4,7 @@ namespace Database\Factories; use App\Models\Asset; use App\Models\AssetModel; +use App\Models\Category; use App\Models\CustomField; use App\Models\Location; use App\Models\Statuslabel; @@ -333,6 +334,15 @@ class AssetFactory extends Factory }); } + public function doesNotRequireAcceptance() + { + return $this->state(function () { + return [ + 'model_id' => AssetModel::factory()->doesNotRequireAcceptance(), + ]; + }); + } + public function deleted() { return $this->state(function () { diff --git a/database/factories/AssetModelFactory.php b/database/factories/AssetModelFactory.php index 8acecd55d7..3adb933e3e 100644 --- a/database/factories/AssetModelFactory.php +++ b/database/factories/AssetModelFactory.php @@ -448,4 +448,13 @@ class AssetModelFactory extends Factory ]; }); } + + public function doesNotRequireAcceptance() + { + return $this->state(function () { + return [ + 'category_id' => Category::factory()->doesNotRequireAcceptance(), + ]; + }); + } } diff --git a/database/factories/CategoryFactory.php b/database/factories/CategoryFactory.php index 9897b396c1..42103f1b76 100644 --- a/database/factories/CategoryFactory.php +++ b/database/factories/CategoryFactory.php @@ -207,4 +207,11 @@ class CategoryFactory extends Factory 'category_type' => 'consumable', ]); } + + public function doesNotRequireAcceptance() + { + return $this->state([ + 'require_acceptance' => false, + ]); + } } diff --git a/resources/views/mail/markdown/checkout-asset.blade.php b/resources/views/mail/markdown/checkout-asset.blade.php index 72e5860e2c..8cb7101abe 100644 --- a/resources/views/mail/markdown/checkout-asset.blade.php +++ b/resources/views/mail/markdown/checkout-asset.blade.php @@ -1,7 +1,7 @@ @component('mail::message') # {{ trans('mail.hello') }} {{ $target->present()->fullName() }}, -{{ trans('mail.new_item_checked') }} +{{ $opening_line }} @if (($snipeSettings->show_images_in_email =='1') && $item->getImageUrl())
Asset
@@ -76,4 +76,4 @@ {{ $snipeSettings->site_name }} -@endcomponent \ No newline at end of file +@endcomponent diff --git a/tests/Unit/Mail/CheckoutAssetMailTest.php b/tests/Unit/Mail/CheckoutAssetMailTest.php new file mode 100644 index 0000000000..45004b4fc0 --- /dev/null +++ b/tests/Unit/Mail/CheckoutAssetMailTest.php @@ -0,0 +1,88 @@ +create(); + $actor = User::factory()->create(); + + $assetRequiringAcceptance = Asset::factory()->requiresAcceptance()->create(); + $assetNotRequiringAcceptance = Asset::factory()->doesNotRequireAcceptance()->create(); + + $acceptance = CheckoutAcceptance::factory()->for($assetRequiringAcceptance, 'checkoutable')->create(); + + (new CheckoutAssetMail( + $assetRequiringAcceptance, + $user, + $actor, + $acceptance, + 'A note goes here', + true, + ))->assertHasSubject('Asset checked out'); + + (new CheckoutAssetMail( + $assetNotRequiringAcceptance, + $user, + $actor, + null, + 'A note goes here', + true, + ))->assertHasSubject('Asset checked out'); + + (new CheckoutAssetMail( + $assetRequiringAcceptance, + $user, + $actor, + $acceptance, + 'A note goes here', + false, + ))->assertHasSubject('Reminder: You have Unaccepted Assets.'); + } + + public function testContent() + { + $user = User::factory()->create(); + $actor = User::factory()->create(); + + $assetRequiringAcceptance = Asset::factory()->requiresAcceptance()->create(); + $assetNotRequiringAcceptance = Asset::factory()->doesNotRequireAcceptance()->create(); + + $acceptance = CheckoutAcceptance::factory()->for($assetRequiringAcceptance, 'checkoutable')->create(); + + (new CheckoutAssetMail( + $assetRequiringAcceptance, + $user, + $actor, + $acceptance, + 'A note goes here', + true, + ))->assertSeeInText('A new item has been checked out under your name that requires acceptance, details are below.'); + + (new CheckoutAssetMail( + $assetNotRequiringAcceptance, + $user, + $actor, + null, + 'A note goes here', + true, + ))->assertSeeInText('A new item has been checked out under your name, details are below.'); + + (new CheckoutAssetMail( + $assetRequiringAcceptance, + $user, + $actor, + $acceptance, + 'A note goes here', + false, + ))->assertSeeInText('An item was recently checked out under your name that requires acceptance, details are below.'); + } +} From 7df636515fdaf050b1fde6fd893bba72b913928a Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 5 Mar 2025 18:20:40 -0800 Subject: [PATCH 2/5] Move to data providers --- tests/Unit/Mail/CheckoutAssetMailTest.php | 115 ++++++++++------------ 1 file changed, 51 insertions(+), 64 deletions(-) diff --git a/tests/Unit/Mail/CheckoutAssetMailTest.php b/tests/Unit/Mail/CheckoutAssetMailTest.php index 45004b4fc0..af198b2cd1 100644 --- a/tests/Unit/Mail/CheckoutAssetMailTest.php +++ b/tests/Unit/Mail/CheckoutAssetMailTest.php @@ -6,83 +6,70 @@ use App\Mail\CheckoutAssetMail; use App\Models\Asset; use App\Models\CheckoutAcceptance; use App\Models\User; +use PHPUnit\Framework\Attributes\DataProvider; use Tests\TestCase; class CheckoutAssetMailTest extends TestCase { - public function testSubjectLine() + public static function data() { - $user = User::factory()->create(); - $actor = User::factory()->create(); + yield 'Asset requiring acceptance' => [ + function () { + $asset = Asset::factory()->requiresAcceptance()->create(); + return [ + 'asset' => $asset, + 'acceptance' => CheckoutAcceptance::factory()->for($asset, 'checkoutable')->create(), + 'first_time_sending' => true, + 'expected_subject' => 'Asset checked out', + 'expected_opening' => 'A new item has been checked out under your name that requires acceptance, details are below.' + ]; + } + ]; - $assetRequiringAcceptance = Asset::factory()->requiresAcceptance()->create(); - $assetNotRequiringAcceptance = Asset::factory()->doesNotRequireAcceptance()->create(); + yield 'Asset not requiring acceptance' => [ + function () { + return [ + 'asset' => Asset::factory()->doesNotRequireAcceptance()->create(), + 'acceptance' => null, + 'first_time_sending' => true, + 'expected_subject' => 'Asset checked out', + 'expected_opening' => 'A new item has been checked out under your name, details are below.' + ]; + } + ]; - $acceptance = CheckoutAcceptance::factory()->for($assetRequiringAcceptance, 'checkoutable')->create(); - - (new CheckoutAssetMail( - $assetRequiringAcceptance, - $user, - $actor, - $acceptance, - 'A note goes here', - true, - ))->assertHasSubject('Asset checked out'); - - (new CheckoutAssetMail( - $assetNotRequiringAcceptance, - $user, - $actor, - null, - 'A note goes here', - true, - ))->assertHasSubject('Asset checked out'); - - (new CheckoutAssetMail( - $assetRequiringAcceptance, - $user, - $actor, - $acceptance, - 'A note goes here', - false, - ))->assertHasSubject('Reminder: You have Unaccepted Assets.'); + yield 'Reminder' => [ + function () { + return [ + 'asset' => Asset::factory()->requiresAcceptance()->create(), + 'acceptance' => CheckoutAcceptance::factory()->create(), + 'first_time_sending' => false, + 'expected_subject' => 'Reminder: You have Unaccepted Assets.', + 'expected_opening' => 'An item was recently checked out under your name that requires acceptance, details are below.' + ]; + } + ]; } - public function testContent() + #[DataProvider('data')] + public function testSubjectLineAndOpening($data) { - $user = User::factory()->create(); - $actor = User::factory()->create(); - - $assetRequiringAcceptance = Asset::factory()->requiresAcceptance()->create(); - $assetNotRequiringAcceptance = Asset::factory()->doesNotRequireAcceptance()->create(); - - $acceptance = CheckoutAcceptance::factory()->for($assetRequiringAcceptance, 'checkoutable')->create(); + [ + 'asset' => $asset, + 'acceptance' => $acceptance, + 'first_time_sending' => $firstTimeSending, + 'expected_subject' => $expectedSubject, + 'expected_opening' => $expectedOpening, + ] = $data(); (new CheckoutAssetMail( - $assetRequiringAcceptance, - $user, - $actor, + $asset, + User::factory()->create(), + User::factory()->create(), $acceptance, 'A note goes here', - true, - ))->assertSeeInText('A new item has been checked out under your name that requires acceptance, details are below.'); - - (new CheckoutAssetMail( - $assetNotRequiringAcceptance, - $user, - $actor, - null, - 'A note goes here', - true, - ))->assertSeeInText('A new item has been checked out under your name, details are below.'); - - (new CheckoutAssetMail( - $assetRequiringAcceptance, - $user, - $actor, - $acceptance, - 'A note goes here', - false, - ))->assertSeeInText('An item was recently checked out under your name that requires acceptance, details are below.'); + $firstTimeSending, + ))->assertHasSubject($expectedSubject) + ->assertSeeInText($expectedOpening); } } From f2028178526d7aed9194bb7af29f8523d67661a9 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 6 Mar 2025 12:43:59 -0800 Subject: [PATCH 3/5] Use translation strings --- app/Mail/CheckoutAssetMail.php | 12 ++++++------ resources/lang/en-US/mail.php | 4 +++- .../views/mail/markdown/checkout-asset.blade.php | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/Mail/CheckoutAssetMail.php b/app/Mail/CheckoutAssetMail.php index a810ab8c82..7ac20861ed 100644 --- a/app/Mail/CheckoutAssetMail.php +++ b/app/Mail/CheckoutAssetMail.php @@ -98,7 +98,7 @@ class CheckoutAssetMail extends Mailable 'accept_url' => $accept_url, 'last_checkout' => $this->last_checkout, 'expected_checkin' => $this->expected_checkin, - 'opening_line' => $this->openingLine(), + 'introduction_line' => $this->introductionLine(), ], ); } @@ -119,21 +119,21 @@ class CheckoutAssetMail extends Mailable return trans('mail.Asset_Checkout_Notification'); } - return 'Reminder: ' . trans('mail.unaccepted_asset_reminder'); + return trans('mail.unaccepted_asset_reminder'); } - private function openingLine(): string + private function introductionLine(): string { if ($this->firstTimeSending && $this->requiresAcceptance()) { - return 'A new item has been checked out under your name that requires acceptance, details are below.'; + return trans('mail.new_item_checked_with_acceptance'); } if ($this->firstTimeSending && !$this->requiresAcceptance()) { - return 'A new item has been checked out under your name, details are below.'; + return trans('mail.new_item_checked'); } if (!$this->firstTimeSending && $this->requiresAcceptance()) { - return 'An item was recently checked out under your name that requires acceptance, details are below.'; + return trans('mail.recent_item_checked'); } // we shouldn't get here but let's send a default message just in case diff --git a/resources/lang/en-US/mail.php b/resources/lang/en-US/mail.php index 7663a0167b..797f735a2b 100644 --- a/resources/lang/en-US/mail.php +++ b/resources/lang/en-US/mail.php @@ -66,6 +66,8 @@ return [ 'min_QTY' => 'Min QTY', 'name' => 'Name', 'new_item_checked' => 'A new item has been checked out under your name, details are below.', + 'new_item_checked_with_acceptance' => 'A new item has been checked out under your name that requires acceptance, details are below.', + 'recent_item_checked' => 'An item was recently checked out under your name that requires acceptance, details are below.', 'notes' => 'Notes', 'password' => 'Password', 'password_reset' => 'Password Reset', @@ -88,7 +90,7 @@ return [ 'upcoming-audits' => 'There is :count asset that is coming up for audit within :threshold days.|There are :count assets that are coming up for audit within :threshold days.', 'user' => 'User', 'username' => 'Username', - 'unaccepted_asset_reminder' => 'You have Unaccepted Assets.', + 'unaccepted_asset_reminder' => 'Reminder: You have Unaccepted Assets.', 'welcome' => 'Welcome :name', 'welcome_to' => 'Welcome to :web!', 'your_assets' => 'View Your Assets', diff --git a/resources/views/mail/markdown/checkout-asset.blade.php b/resources/views/mail/markdown/checkout-asset.blade.php index 8cb7101abe..1bcbe462ba 100644 --- a/resources/views/mail/markdown/checkout-asset.blade.php +++ b/resources/views/mail/markdown/checkout-asset.blade.php @@ -1,7 +1,7 @@ @component('mail::message') # {{ trans('mail.hello') }} {{ $target->present()->fullName() }}, -{{ $opening_line }} +{{ $introduction_line }} @if (($snipeSettings->show_images_in_email =='1') && $item->getImageUrl())
Asset
From b1b248f03d5d28d6a5141ee7c5c87dad1d217776 Mon Sep 17 00:00:00 2001 From: Godfrey M Date: Wed, 2 Apr 2025 11:29:01 -0700 Subject: [PATCH 4/5] removed duplicate class from accessories history table --- resources/views/accessories/view.blade.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/views/accessories/view.blade.php b/resources/views/accessories/view.blade.php index 8042b4ec51..a4f4656bf7 100644 --- a/resources/views/accessories/view.blade.php +++ b/resources/views/accessories/view.blade.php @@ -102,7 +102,7 @@
-
+
Date: Thu, 3 Apr 2025 15:35:07 +0100 Subject: [PATCH 5/5] Paveit had old Doctrine code to list tables; use the new method --- app/Console/Commands/PaveIt.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/Console/Commands/PaveIt.php b/app/Console/Commands/PaveIt.php index 09c846ea68..ef69d25a5b 100644 --- a/app/Console/Commands/PaveIt.php +++ b/app/Console/Commands/PaveIt.php @@ -51,8 +51,7 @@ class PaveIt extends Command } // List all the tables in the database so we don't have to worry about missing some as the app grows - $tables = DB::connection()->getDoctrineSchemaManager()->listTableNames(); - + $tables = Schema::getTables(); $except_tables = [ 'oauth_access_tokens', 'oauth_clients', @@ -74,7 +73,8 @@ class PaveIt extends Command } } - foreach ($tables as $table) { + foreach ($tables as $table_obj) { + $table = $table_obj['name']; if (in_array($table, $except_tables)) { $this->info($table. ' is SKIPPED.'); } else {