From fdcb891cbbdb42e8999cb27e1dcaa960ded995ba Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Wed, 29 Jan 2025 15:20:56 -0800
Subject: [PATCH 01/17] Improve test case

---
 tests/Unit/NotificationTest.php | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tests/Unit/NotificationTest.php b/tests/Unit/NotificationTest.php
index 3d5b3c5a76..5b420a6753 100644
--- a/tests/Unit/NotificationTest.php
+++ b/tests/Unit/NotificationTest.php
@@ -7,9 +7,7 @@ use App\Models\Asset;
 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 @@ class NotificationTest extends TestCase
 
         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()

From d254a40e0a90ef080b74fb39745c5b24d0993a33 Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Wed, 29 Jan 2025 15:21:10 -0800
Subject: [PATCH 02/17] Scaffold tests

---
 .../factories/CheckoutAcceptanceFactory.php   | 19 ++++
 .../Email/AssetAcceptanceReminderTest.php     | 90 +++++++++++++++++++
 2 files changed, 109 insertions(+)
 create mode 100644 tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php

diff --git a/database/factories/CheckoutAcceptanceFactory.php b/database/factories/CheckoutAcceptanceFactory.php
index 6bb87d18e1..8dbb6712c6 100644
--- a/database/factories/CheckoutAcceptanceFactory.php
+++ b/database/factories/CheckoutAcceptanceFactory.php
@@ -27,6 +27,17 @@ class CheckoutAcceptanceFactory extends Factory
     public function configure(): static
     {
         return $this->afterCreating(function (CheckoutAcceptance $acceptance) {
+            // @todo: add comment
+            if ($acceptance->checkoutable instanceof Asset) {
+                $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,
+                ]);
+            }
+
             if ($acceptance->checkoutable instanceof Asset && $acceptance->assignedTo instanceof User) {
                 $acceptance->checkoutable->update([
                     'assigned_to' => $acceptance->assigned_to_id,
@@ -51,4 +62,12 @@ class CheckoutAcceptanceFactory extends Factory
             'declined_at' => null,
         ]);
     }
+
+    public function accepted()
+    {
+        return $this->state([
+            'accepted_at' => now()->subDay(),
+            'declined_at' => null,
+        ]);
+    }
 }
diff --git a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
new file mode 100644
index 0000000000..3809b826e6
--- /dev/null
+++ b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
@@ -0,0 +1,90 @@
+<?php
+
+namespace Tests\Feature\Notifications\Email;
+
+use App\Mail\CheckoutAssetMail;
+use App\Models\CheckoutAcceptance;
+use App\Models\User;
+use Illuminate\Support\Facades\Mail;
+use Tests\TestCase;
+
+class AssetAcceptanceReminderTest extends TestCase
+{
+    private CheckoutAcceptance $checkoutAcceptance;
+    private User $actor;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        Mail::fake();
+
+        $this->actor = User::factory()->canViewReports()->create();
+        $this->checkoutAcceptance = CheckoutAcceptance::factory()->pending()->create();
+    }
+
+    public function testMustHavePermissionToSendReminder()
+    {
+        $userWithoutPermission = User::factory()->create();
+
+        $this->actingAs($userWithoutPermission)
+            ->post($this->routeFor($this->checkoutAcceptance))
+            ->assertForbidden();
+    }
+
+    public function testReminderNotSentIfAcceptanceDoesNotExist()
+    {
+        $this->actingAs($this->actor)
+            ->post(route('reports/unaccepted_assets_sent_reminder', [
+                'acceptance_id' => 999999,
+            ]));
+
+        Mail::assertNotSent(CheckoutAssetMail::class);
+    }
+
+    public function testReminderNotSentIfAcceptanceAlreadyAccepted()
+    {
+        $checkoutAcceptanceAlreadyAccepted = CheckoutAcceptance::factory()->accepted()->create();
+
+        $this->actingAs($this->actor)
+            ->post($this->routeFor($checkoutAcceptanceAlreadyAccepted));
+
+        Mail::assertNotSent(CheckoutAssetMail::class);
+    }
+
+    public function testUserWithoutEmailAddressHandledGracefully()
+    {
+        $userWithoutEmailAddress = User::factory()->create(['email' => null]);
+
+        $this->checkoutAcceptance->assigned_to_id = $userWithoutEmailAddress->id;
+        $this->checkoutAcceptance->save();
+
+        $this->actingAs($this->actor)
+            ->post($this->routeFor($this->checkoutAcceptance))
+            // check we didn't crash...
+            ->assertRedirect();
+
+        Mail::assertNotSent(CheckoutAssetMail::class);
+    }
+
+    public function testReminderIsSentToUser()
+    {
+        $this->actingAs($this->actor)
+            ->post($this->routeFor($this->checkoutAcceptance))
+            ->assertRedirect(route('reports/unaccepted_assets'));
+
+        Mail::assertSent(CheckoutAssetMail::class, 1);
+        Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) {
+            return $mail->hasTo($this->checkoutAcceptance->assignedTo->email)
+                // @todo:
+                && $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,
+        ]);
+    }
+}

From e2805f40332afce449c72695c0f4fbca232d718e Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Wed, 29 Jan 2025 15:36:45 -0800
Subject: [PATCH 03/17] Add "Reminder" to subject line

---
 app/Http/Controllers/ReportsController.php    |  5 ++-
 app/Mail/CheckoutAssetMail.php                | 15 +++++++--
 .../Email/AssetAcceptanceReminderTest.php     | 31 ++++++++++++++++---
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/app/Http/Controllers/ReportsController.php b/app/Http/Controllers/ReportsController.php
index f0bbfb74a2..33188d0c01 100644
--- a/app/Http/Controllers/ReportsController.php
+++ b/app/Http/Controllers/ReportsController.php
@@ -1177,10 +1177,9 @@ class ReportsController extends Controller
         $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));
-
+            Mail::to($email)->send((new CheckoutAssetMail($assetItem, $assetItem->assignedTo, $logItem->user, $acceptance, $logItem->note, firstTimeSending: false))->locale($locale));
             } elseif ($email) {
-            Mail::to($email)->send((new CheckoutAssetMail($assetItem, $assetItem->assignedTo, $logItem->user, $acceptance, $logItem->note)));
+            Mail::to($email)->send((new CheckoutAssetMail($assetItem, $assetItem->assignedTo, $logItem->user, $acceptance, $logItem->note, firstTimeSending: false)));
             }
 
         if ($email == ''){
diff --git a/app/Mail/CheckoutAssetMail.php b/app/Mail/CheckoutAssetMail.php
index 680014dcd1..fbdaa4fcc0 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 @@ class CheckoutAssetMail extends Mailable
         $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);
@@ -54,9 +58,16 @@ class CheckoutAssetMail extends Mailable
     {
         $from = new Address(config('mail.from.address'), config('mail.from.name'));
 
+        $subject = trans('mail.Asset_Checkout_Notification');
+
+        if (!$this->firstTimeSending) {
+            // @todo: translate
+            $subject = 'Reminder: ' . $subject;
+        }
+
         return new Envelope(
             from: $from,
-            subject: trans('mail.Asset_Checkout_Notification'),
+            subject: $subject,
         );
     }
 
diff --git a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
index 3809b826e6..d84b2f7120 100644
--- a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
+++ b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
@@ -6,6 +6,7 @@ use App\Mail\CheckoutAssetMail;
 use App\Models\CheckoutAcceptance;
 use App\Models\User;
 use Illuminate\Support\Facades\Mail;
+use PHPUnit\Framework\Attributes\DataProvider;
 use Tests\TestCase;
 
 class AssetAcceptanceReminderTest extends TestCase
@@ -67,16 +68,36 @@ class AssetAcceptanceReminderTest extends TestCase
         Mail::assertNotSent(CheckoutAssetMail::class);
     }
 
-    public function testReminderIsSentToUser()
+    public static function users()
     {
+        yield 'User with locale set' => [
+            function () {
+                return CheckoutAcceptance::factory()->pending()->create();
+            }
+        ];
+
+        yield 'User without locale set' => [
+            function () {
+                $checkoutAcceptance = CheckoutAcceptance::factory()->pending()->create();
+                $checkoutAcceptance->assignedTo->update(['locale' => null]);
+
+                return $checkoutAcceptance;
+            }
+        ];
+    }
+
+    #[DataProvider('users')]
+    public function testReminderIsSentToUser($data)
+    {
+        $checkoutAcceptance = $data();
+
         $this->actingAs($this->actor)
-            ->post($this->routeFor($this->checkoutAcceptance))
+            ->post($this->routeFor($checkoutAcceptance))
             ->assertRedirect(route('reports/unaccepted_assets'));
 
         Mail::assertSent(CheckoutAssetMail::class, 1);
-        Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) {
-            return $mail->hasTo($this->checkoutAcceptance->assignedTo->email)
-                // @todo:
+        Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) use ($checkoutAcceptance) {
+            return $mail->hasTo($checkoutAcceptance->assignedTo->email)
                 && $mail->hasSubject('Reminder: ' . trans('mail.Asset_Checkout_Notification'));
         });
     }

From 70aed45bfe6c5d4b5ee4ed964c58b414d2d35718 Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Wed, 29 Jan 2025 15:56:20 -0800
Subject: [PATCH 04/17] Improve naming

---
 .../Notifications/Email/AssetAcceptanceReminderTest.php   | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
index d84b2f7120..7411cfbd3b 100644
--- a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
+++ b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
@@ -68,7 +68,7 @@ class AssetAcceptanceReminderTest extends TestCase
         Mail::assertNotSent(CheckoutAssetMail::class);
     }
 
-    public static function users()
+    public static function acceptances()
     {
         yield 'User with locale set' => [
             function () {
@@ -86,10 +86,10 @@ class AssetAcceptanceReminderTest extends TestCase
         ];
     }
 
-    #[DataProvider('users')]
-    public function testReminderIsSentToUser($data)
+    #[DataProvider('acceptances')]
+    public function testReminderIsSentToUser($callback)
     {
-        $checkoutAcceptance = $data();
+        $checkoutAcceptance = $callback();
 
         $this->actingAs($this->actor)
             ->post($this->routeFor($checkoutAcceptance))

From 4e7c6bd2cfd87901fec2c71cde855f4c63f77c0c Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Wed, 29 Jan 2025 16:14:09 -0800
Subject: [PATCH 05/17] Fix relationship

---
 .../Email/AssetAcceptanceReminderTest.php              | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
index 7411cfbd3b..4898234e10 100644
--- a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
+++ b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
@@ -55,13 +55,13 @@ class AssetAcceptanceReminderTest extends TestCase
 
     public function testUserWithoutEmailAddressHandledGracefully()
     {
-        $userWithoutEmailAddress = User::factory()->create(['email' => null]);
-
-        $this->checkoutAcceptance->assigned_to_id = $userWithoutEmailAddress->id;
-        $this->checkoutAcceptance->save();
+        $checkoutAcceptance = CheckoutAcceptance::factory()
+            ->pending()
+            ->forAssignedTo(['email' => null])
+            ->create();
 
         $this->actingAs($this->actor)
-            ->post($this->routeFor($this->checkoutAcceptance))
+            ->post($this->routeFor($checkoutAcceptance))
             // check we didn't crash...
             ->assertRedirect();
 

From 78f92925553d29c6149fcd7c45b1a75dd03b1b17 Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Wed, 29 Jan 2025 16:15:27 -0800
Subject: [PATCH 06/17] Inline variable

---
 .../Notifications/Email/AssetAcceptanceReminderTest.php      | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
index 4898234e10..34bc935bb7 100644
--- a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
+++ b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
@@ -11,7 +11,6 @@ use Tests\TestCase;
 
 class AssetAcceptanceReminderTest extends TestCase
 {
-    private CheckoutAcceptance $checkoutAcceptance;
     private User $actor;
 
     protected function setUp(): void
@@ -21,15 +20,15 @@ class AssetAcceptanceReminderTest extends TestCase
         Mail::fake();
 
         $this->actor = User::factory()->canViewReports()->create();
-        $this->checkoutAcceptance = CheckoutAcceptance::factory()->pending()->create();
     }
 
     public function testMustHavePermissionToSendReminder()
     {
+        $checkoutAcceptance = CheckoutAcceptance::factory()->pending()->create();
         $userWithoutPermission = User::factory()->create();
 
         $this->actingAs($userWithoutPermission)
-            ->post($this->routeFor($this->checkoutAcceptance))
+            ->post($this->routeFor($checkoutAcceptance))
             ->assertForbidden();
     }
 

From ce31ce477e046ecec0cc6d56a5df7c44ab863c87 Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Wed, 29 Jan 2025 16:16:47 -0800
Subject: [PATCH 07/17] Inline additional variables

---
 .../Email/AssetAcceptanceReminderTest.php            | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
index 34bc935bb7..f163225d1b 100644
--- a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
+++ b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
@@ -11,15 +11,11 @@ use Tests\TestCase;
 
 class AssetAcceptanceReminderTest extends TestCase
 {
-    private User $actor;
-
     protected function setUp(): void
     {
         parent::setUp();
 
         Mail::fake();
-
-        $this->actor = User::factory()->canViewReports()->create();
     }
 
     public function testMustHavePermissionToSendReminder()
@@ -34,7 +30,7 @@ class AssetAcceptanceReminderTest extends TestCase
 
     public function testReminderNotSentIfAcceptanceDoesNotExist()
     {
-        $this->actingAs($this->actor)
+        $this->actingAs(User::factory()->canViewReports()->create())
             ->post(route('reports/unaccepted_assets_sent_reminder', [
                 'acceptance_id' => 999999,
             ]));
@@ -46,7 +42,7 @@ class AssetAcceptanceReminderTest extends TestCase
     {
         $checkoutAcceptanceAlreadyAccepted = CheckoutAcceptance::factory()->accepted()->create();
 
-        $this->actingAs($this->actor)
+        $this->actingAs(User::factory()->canViewReports()->create())
             ->post($this->routeFor($checkoutAcceptanceAlreadyAccepted));
 
         Mail::assertNotSent(CheckoutAssetMail::class);
@@ -59,7 +55,7 @@ class AssetAcceptanceReminderTest extends TestCase
             ->forAssignedTo(['email' => null])
             ->create();
 
-        $this->actingAs($this->actor)
+        $this->actingAs(User::factory()->canViewReports()->create())
             ->post($this->routeFor($checkoutAcceptance))
             // check we didn't crash...
             ->assertRedirect();
@@ -90,7 +86,7 @@ class AssetAcceptanceReminderTest extends TestCase
     {
         $checkoutAcceptance = $callback();
 
-        $this->actingAs($this->actor)
+        $this->actingAs(User::factory()->canViewReports()->create())
             ->post($this->routeFor($checkoutAcceptance))
             ->assertRedirect(route('reports/unaccepted_assets'));
 

From d1197d015c22758bbc49f9f6046f76db9e95a844 Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Wed, 29 Jan 2025 16:24:43 -0800
Subject: [PATCH 08/17] Add another case scenario

---
 .../Email/AssetAcceptanceReminderTest.php     | 29 +++++++++++++++----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
index f163225d1b..d7c89e041e 100644
--- a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
+++ b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
@@ -48,12 +48,31 @@ class AssetAcceptanceReminderTest extends TestCase
         Mail::assertNotSent(CheckoutAssetMail::class);
     }
 
-    public function testUserWithoutEmailAddressHandledGracefully()
+    public static function CheckoutAcceptancesToUsersWithoutEmailAddresses()
     {
-        $checkoutAcceptance = CheckoutAcceptance::factory()
-            ->pending()
-            ->forAssignedTo(['email' => null])
-            ->create();
+        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))

From c15c338ffd29beed180d352897599226f70d10bb Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Wed, 29 Jan 2025 16:25:37 -0800
Subject: [PATCH 09/17] Merge if/else

---
 app/Http/Controllers/ReportsController.php | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/app/Http/Controllers/ReportsController.php b/app/Http/Controllers/ReportsController.php
index 33188d0c01..5ce23138e2 100644
--- a/app/Http/Controllers/ReportsController.php
+++ b/app/Http/Controllers/ReportsController.php
@@ -1176,11 +1176,9 @@ class ReportsController extends Controller
         $email = $assetItem->assignedTo?->email;
         $locale = $assetItem->assignedTo?->locale;
         // Only send notification if assigned
-        if ($locale && $email) {
+        if ($email) {
             Mail::to($email)->send((new CheckoutAssetMail($assetItem, $assetItem->assignedTo, $logItem->user, $acceptance, $logItem->note, firstTimeSending: false))->locale($locale));
-            } elseif ($email) {
-            Mail::to($email)->send((new CheckoutAssetMail($assetItem, $assetItem->assignedTo, $logItem->user, $acceptance, $logItem->note, firstTimeSending: false)));
-            }
+        }
 
         if ($email == ''){
             return redirect()->route('reports/unaccepted_assets')->with('error', trans('general.no_email'));

From ab9e9b66d257d91ca35351ee423901223d7afc1b Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Wed, 29 Jan 2025 16:27:18 -0800
Subject: [PATCH 10/17] Reduce complexity

---
 app/Http/Controllers/ReportsController.php | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/app/Http/Controllers/ReportsController.php b/app/Http/Controllers/ReportsController.php
index 5ce23138e2..fa437ed20e 100644
--- a/app/Http/Controllers/ReportsController.php
+++ b/app/Http/Controllers/ReportsController.php
@@ -1175,15 +1175,13 @@ class ReportsController extends Controller
         }
         $email = $assetItem->assignedTo?->email;
         $locale = $assetItem->assignedTo?->locale;
-        // Only send notification if assigned
-        if ($email) {
-            Mail::to($email)->send((new CheckoutAssetMail($assetItem, $assetItem->assignedTo, $logItem->user, $acceptance, $logItem->note, firstTimeSending: false))->locale($locale));
-        }
 
-        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'));
     }
 

From 6a4a5d13806fead9cc5f19588dad4227956f2231 Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Thu, 30 Jan 2025 10:35:31 -0800
Subject: [PATCH 11/17] Add translation

---
 app/Mail/CheckoutAssetMail.php | 3 +--
 resources/lang/en-US/mail.php  | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/Mail/CheckoutAssetMail.php b/app/Mail/CheckoutAssetMail.php
index fbdaa4fcc0..d6ce123d40 100644
--- a/app/Mail/CheckoutAssetMail.php
+++ b/app/Mail/CheckoutAssetMail.php
@@ -61,8 +61,7 @@ class CheckoutAssetMail extends Mailable
         $subject = trans('mail.Asset_Checkout_Notification');
 
         if (!$this->firstTimeSending) {
-            // @todo: translate
-            $subject = 'Reminder: ' . $subject;
+            $subject = trans('mail.Asset_Checkout_Reminder_Notification');
         }
 
         return new Envelope(
diff --git a/resources/lang/en-US/mail.php b/resources/lang/en-US/mail.php
index 7663a0167b..f7d3794dc3 100644
--- a/resources/lang/en-US/mail.php
+++ b/resources/lang/en-US/mail.php
@@ -6,6 +6,7 @@ return [
     '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',

From e94ee48f741846cd8ee4d51987fdb9848ac744f9 Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Thu, 30 Jan 2025 10:37:11 -0800
Subject: [PATCH 12/17] Extract helper

---
 app/Mail/CheckoutAssetMail.php | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/app/Mail/CheckoutAssetMail.php b/app/Mail/CheckoutAssetMail.php
index d6ce123d40..eb9619217f 100644
--- a/app/Mail/CheckoutAssetMail.php
+++ b/app/Mail/CheckoutAssetMail.php
@@ -58,15 +58,9 @@ class CheckoutAssetMail extends Mailable
     {
         $from = new Address(config('mail.from.address'), config('mail.from.name'));
 
-        $subject = trans('mail.Asset_Checkout_Notification');
-
-        if (!$this->firstTimeSending) {
-            $subject = trans('mail.Asset_Checkout_Reminder_Notification');
-        }
-
         return new Envelope(
             from: $from,
-            subject: $subject,
+            subject: $this->getSubject(),
         );
     }
 
@@ -117,4 +111,13 @@ class CheckoutAssetMail extends Mailable
     {
         return [];
     }
+
+    private function getSubject(): string
+    {
+        if ($this->firstTimeSending) {
+            return trans('mail.Asset_Checkout_Notification');
+        }
+
+        return trans('mail.Asset_Checkout_Reminder_Notification');
+    }
 }

From fc88b2487ff524d6515852b9fa8224263cdba411 Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Thu, 30 Jan 2025 11:44:37 -0800
Subject: [PATCH 13/17] Extract method

---
 .../factories/CheckoutAcceptanceFactory.php   | 20 +++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/database/factories/CheckoutAcceptanceFactory.php b/database/factories/CheckoutAcceptanceFactory.php
index 8dbb6712c6..a15e942a84 100644
--- a/database/factories/CheckoutAcceptanceFactory.php
+++ b/database/factories/CheckoutAcceptanceFactory.php
@@ -27,15 +27,8 @@ class CheckoutAcceptanceFactory extends Factory
     public function configure(): static
     {
         return $this->afterCreating(function (CheckoutAcceptance $acceptance) {
-            // @todo: add comment
             if ($acceptance->checkoutable instanceof Asset) {
-                $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,
-                ]);
+                $this->createdAssociatedActionLogEntry($acceptance);
             }
 
             if ($acceptance->checkoutable instanceof Asset && $acceptance->assignedTo instanceof User) {
@@ -70,4 +63,15 @@ class CheckoutAcceptanceFactory extends Factory
             '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,
+        ]);
+    }
 }

From 7e9c564d0b1398eafaff1fedfc16773e11b5163d Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Thu, 30 Jan 2025 11:47:43 -0800
Subject: [PATCH 14/17] Simplify test

---
 .../Email/AssetAcceptanceReminderTest.php     | 23 ++-----------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
index d7c89e041e..db3de3c54c 100644
--- a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
+++ b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
@@ -82,28 +82,9 @@ class AssetAcceptanceReminderTest extends TestCase
         Mail::assertNotSent(CheckoutAssetMail::class);
     }
 
-    public static function acceptances()
+    public function testReminderIsSentToUser()
     {
-        yield 'User with locale set' => [
-            function () {
-                return CheckoutAcceptance::factory()->pending()->create();
-            }
-        ];
-
-        yield 'User without locale set' => [
-            function () {
-                $checkoutAcceptance = CheckoutAcceptance::factory()->pending()->create();
-                $checkoutAcceptance->assignedTo->update(['locale' => null]);
-
-                return $checkoutAcceptance;
-            }
-        ];
-    }
-
-    #[DataProvider('acceptances')]
-    public function testReminderIsSentToUser($callback)
-    {
-        $checkoutAcceptance = $callback();
+        $checkoutAcceptance = CheckoutAcceptance::factory()->pending()->create();
 
         $this->actingAs(User::factory()->canViewReports()->create())
             ->post($this->routeFor($checkoutAcceptance))

From 7cbb3f7e07f7820f665211e0e0e32a75644281fd Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Thu, 30 Jan 2025 11:48:55 -0800
Subject: [PATCH 15/17] Add assertion

---
 .../Feature/Notifications/Email/AssetAcceptanceReminderTest.php | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
index db3de3c54c..85915352b7 100644
--- a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
+++ b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
@@ -26,6 +26,8 @@ class AssetAcceptanceReminderTest extends TestCase
         $this->actingAs($userWithoutPermission)
             ->post($this->routeFor($checkoutAcceptance))
             ->assertForbidden();
+
+        Mail::assertNotSent(CheckoutAssetMail::class);
     }
 
     public function testReminderNotSentIfAcceptanceDoesNotExist()

From 027c2b36276d5da0a15334c9da11ac9c8cdb9cfb Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Mon, 24 Feb 2025 11:45:23 -0800
Subject: [PATCH 16/17] Change subject to "You have Unaccepted Assets."

---
 app/Mail/CheckoutAssetMail.php                                  | 2 +-
 .../Feature/Notifications/Email/AssetAcceptanceReminderTest.php | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/Mail/CheckoutAssetMail.php b/app/Mail/CheckoutAssetMail.php
index eb9619217f..1447dfd6f5 100644
--- a/app/Mail/CheckoutAssetMail.php
+++ b/app/Mail/CheckoutAssetMail.php
@@ -118,6 +118,6 @@ class CheckoutAssetMail extends Mailable
             return trans('mail.Asset_Checkout_Notification');
         }
 
-        return trans('mail.Asset_Checkout_Reminder_Notification');
+        return trans('mail.unaccepted_asset_reminder');
     }
 }
diff --git a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
index 85915352b7..deb3e07d2c 100644
--- a/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
+++ b/tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
@@ -95,7 +95,7 @@ class AssetAcceptanceReminderTest extends TestCase
         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'));
+                && $mail->hasSubject(trans('mail.unaccepted_asset_reminder'));
         });
     }
 

From f97211f6cd0d312cc83c11d5ffad360a59517e36 Mon Sep 17 00:00:00 2001
From: Marcus Moore <contact@marcusmoore.io>
Date: Mon, 24 Feb 2025 11:45:58 -0800
Subject: [PATCH 17/17] Remove unused language string

---
 resources/lang/en-US/mail.php | 1 -
 1 file changed, 1 deletion(-)

diff --git a/resources/lang/en-US/mail.php b/resources/lang/en-US/mail.php
index f7d3794dc3..7663a0167b 100644
--- a/resources/lang/en-US/mail.php
+++ b/resources/lang/en-US/mail.php
@@ -6,7 +6,6 @@ return [
     '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',