diff --git a/app/Http/Controllers/ReportsController.php b/app/Http/Controllers/ReportsController.php index f0bbfb74a26a..fa437ed20ec3 100644 --- a/app/Http/Controllers/ReportsController.php +++ b/app/Http/Controllers/ReportsController.php @@ -1175,18 +1175,13 @@ public function sentAssetAcceptanceReminder(Request $request) : RedirectResponse } $email = $assetItem->assignedTo?->email; $locale = $assetItem->assignedTo?->locale; - // Only send notification if assigned - if ($locale && $email) { - Mail::to($email)->send((new CheckoutAssetMail($assetItem, $assetItem->assignedTo, $logItem->user, $acceptance, $logItem->note))->locale($locale)); - } elseif ($email) { - Mail::to($email)->send((new CheckoutAssetMail($assetItem, $assetItem->assignedTo, $logItem->user, $acceptance, $logItem->note))); - } - - if ($email == ''){ + if (is_null($email) || $email === '') { return redirect()->route('reports/unaccepted_assets')->with('error', trans('general.no_email')); } + Mail::to($email)->send((new CheckoutAssetMail($assetItem, $assetItem->assignedTo, $logItem->user, $acceptance, $logItem->note, firstTimeSending: false))->locale($locale)); + return redirect()->route('reports/unaccepted_assets')->with('success', trans('admin/reports/general.reminder_sent')); } diff --git a/app/Mail/CheckoutAssetMail.php b/app/Mail/CheckoutAssetMail.php index 680014dcd189..eb9619217fdf 100644 --- a/app/Mail/CheckoutAssetMail.php +++ b/app/Mail/CheckoutAssetMail.php @@ -20,10 +20,12 @@ class CheckoutAssetMail extends Mailable { use Queueable, SerializesModels; + private bool $firstTimeSending; + /** * Create a new message instance. */ - public function __construct(Asset $asset, $checkedOutTo, User $checkedOutBy, $acceptance, $note) + public function __construct(Asset $asset, $checkedOutTo, User $checkedOutBy, $acceptance, $note, bool $firstTimeSending = true) { $this->item = $asset; $this->admin = $checkedOutBy; @@ -36,6 +38,8 @@ public function __construct(Asset $asset, $checkedOutTo, User $checkedOutBy, $a $this->last_checkout = ''; $this->expected_checkin = ''; + $this->firstTimeSending = $firstTimeSending; + if ($this->item->last_checkout) { $this->last_checkout = Helper::getFormattedDateObject($this->item->last_checkout, 'date', false); @@ -56,7 +60,7 @@ public function envelope(): Envelope return new Envelope( from: $from, - subject: trans('mail.Asset_Checkout_Notification'), + subject: $this->getSubject(), ); } @@ -107,4 +111,13 @@ public function attachments(): array { return []; } + + private function getSubject(): string + { + if ($this->firstTimeSending) { + return trans('mail.Asset_Checkout_Notification'); + } + + return trans('mail.Asset_Checkout_Reminder_Notification'); + } } diff --git a/database/factories/CheckoutAcceptanceFactory.php b/database/factories/CheckoutAcceptanceFactory.php index 6bb87d18e11d..a15e942a8473 100644 --- a/database/factories/CheckoutAcceptanceFactory.php +++ b/database/factories/CheckoutAcceptanceFactory.php @@ -27,6 +27,10 @@ public function definition() public function configure(): static { return $this->afterCreating(function (CheckoutAcceptance $acceptance) { + if ($acceptance->checkoutable instanceof Asset) { + $this->createdAssociatedActionLogEntry($acceptance); + } + if ($acceptance->checkoutable instanceof Asset && $acceptance->assignedTo instanceof User) { $acceptance->checkoutable->update([ 'assigned_to' => $acceptance->assigned_to_id, @@ -51,4 +55,23 @@ public function pending() 'declined_at' => null, ]); } + + public function accepted() + { + return $this->state([ + 'accepted_at' => now()->subDay(), + 'declined_at' => null, + ]); + } + + private function createdAssociatedActionLogEntry(CheckoutAcceptance $acceptance): void + { + $acceptance->checkoutable->assetlog()->create([ + 'action_type' => 'checkout', + 'target_id' => $acceptance->assigned_to_id, + 'target_type' => get_class($acceptance->assignedTo), + 'item_id' => $acceptance->checkoutable_id, + 'item_type' => $acceptance->checkoutable_type, + ]); + } } diff --git a/resources/lang/en-US/mail.php b/resources/lang/en-US/mail.php index 7663a0167bb6..f7d3794dc35a 100644 --- a/resources/lang/en-US/mail.php +++ b/resources/lang/en-US/mail.php @@ -6,6 +6,7 @@ 'Accessory_Checkout_Notification' => 'Accessory checked out', 'Asset_Checkin_Notification' => 'Asset checked in', 'Asset_Checkout_Notification' => 'Asset checked out', + 'Asset_Checkout_Reminder_Notification' => 'Reminder: Asset checked out', 'Confirm_Accessory_Checkin' => 'Accessory checkin confirmation', 'Confirm_Asset_Checkin' => 'Asset checkin confirmation', 'Confirm_accessory_delivery' => 'Accessory delivery confirmation', diff --git a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php new file mode 100644 index 000000000000..85915352b78d --- /dev/null +++ b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php @@ -0,0 +1,108 @@ +pending()->create(); + $userWithoutPermission = User::factory()->create(); + + $this->actingAs($userWithoutPermission) + ->post($this->routeFor($checkoutAcceptance)) + ->assertForbidden(); + + Mail::assertNotSent(CheckoutAssetMail::class); + } + + public function testReminderNotSentIfAcceptanceDoesNotExist() + { + $this->actingAs(User::factory()->canViewReports()->create()) + ->post(route('reports/unaccepted_assets_sent_reminder', [ + 'acceptance_id' => 999999, + ])); + + Mail::assertNotSent(CheckoutAssetMail::class); + } + + public function testReminderNotSentIfAcceptanceAlreadyAccepted() + { + $checkoutAcceptanceAlreadyAccepted = CheckoutAcceptance::factory()->accepted()->create(); + + $this->actingAs(User::factory()->canViewReports()->create()) + ->post($this->routeFor($checkoutAcceptanceAlreadyAccepted)); + + Mail::assertNotSent(CheckoutAssetMail::class); + } + + public static function CheckoutAcceptancesToUsersWithoutEmailAddresses() + { + yield 'User with null email address' => [ + function () { + return CheckoutAcceptance::factory() + ->pending() + ->forAssignedTo(['email' => null]) + ->create(); + } + ]; + + yield 'User with empty string email address' => [ + function () { + return CheckoutAcceptance::factory() + ->pending() + ->forAssignedTo(['email' => '']) + ->create(); + } + ]; + } + + #[DataProvider('CheckoutAcceptancesToUsersWithoutEmailAddresses')] + public function testUserWithoutEmailAddressHandledGracefully($callback) + { + $checkoutAcceptance = $callback(); + + $this->actingAs(User::factory()->canViewReports()->create()) + ->post($this->routeFor($checkoutAcceptance)) + // check we didn't crash... + ->assertRedirect(); + + Mail::assertNotSent(CheckoutAssetMail::class); + } + + public function testReminderIsSentToUser() + { + $checkoutAcceptance = CheckoutAcceptance::factory()->pending()->create(); + + $this->actingAs(User::factory()->canViewReports()->create()) + ->post($this->routeFor($checkoutAcceptance)) + ->assertRedirect(route('reports/unaccepted_assets')); + + Mail::assertSent(CheckoutAssetMail::class, 1); + Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) use ($checkoutAcceptance) { + return $mail->hasTo($checkoutAcceptance->assignedTo->email) + && $mail->hasSubject('Reminder: ' . trans('mail.Asset_Checkout_Notification')); + }); + } + + private function routeFor(CheckoutAcceptance $checkoutAcceptance): string + { + return route('reports/unaccepted_assets_sent_reminder', [ + 'acceptance_id' => $checkoutAcceptance->id, + ]); + } +} diff --git a/tests/Unit/NotificationTest.php b/tests/Unit/NotificationTest.php index 3d5b3c5a7651..5b420a675385 100644 --- a/tests/Unit/NotificationTest.php +++ b/tests/Unit/NotificationTest.php @@ -7,9 +7,7 @@ use App\Models\AssetModel; use App\Models\Category; use Carbon\Carbon; -use App\Notifications\CheckoutAssetNotification; use Illuminate\Support\Facades\Mail; -use Illuminate\Support\Facades\Notification; use Tests\TestCase; class NotificationTest extends TestCase @@ -33,8 +31,8 @@ public function testAUserIsEmailedIfTheyCheckoutAnAssetWithEULA() Mail::fake(); $asset->checkOut($user, $admin->id); - Mail::assertSent(CheckoutAssetMail::class, function ($mail) use ($user) { - return $mail->hasTo($user->email); + Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) use ($user) { + return $mail->hasTo($user->email) && $mail->hasSubject(trans('mail.Asset_Checkout_Notification')); }); } public function testDefaultEulaIsSentWhenSetInCategory()