Skip to content

Commit 115bb94

Browse files
snipesnipe
authored
Merge pull request #16156 from marcusmoore/acceptance-reminder-subject
Added "Reminder" to subject line of follow up asset checkout emails
2 parents b8799f8 + e88bba5 commit 115bb94

File tree

5 files changed

+151
-14
lines changed

5 files changed

+151
-14
lines changed

‎app/Http/Controllers/ReportsController.php‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,18 +1175,13 @@ public function sentAssetAcceptanceReminder(Request $request) : RedirectResponse
11751175
}
11761176
$email = $assetItem->assignedTo?->email;
11771177
$locale = $assetItem->assignedTo?->locale;
1178-
// Only send notification if assigned
1179-
if ($locale && $email) {
1180-
Mail::to($email)->send((new CheckoutAssetMail($assetItem, $assetItem->assignedTo, $logItem->user, $acceptance, $logItem->note))->locale($locale));
11811178

1182-
} elseif ($email) {
1183-
Mail::to($email)->send((new CheckoutAssetMail($assetItem, $assetItem->assignedTo, $logItem->user, $acceptance, $logItem->note)));
1184-
}
1185-
1186-
if ($email == ''){
1179+
if (is_null($email) || $email === '') {
11871180
return redirect()->route('reports/unaccepted_assets')->with('error', trans('general.no_email'));
11881181
}
11891182

1183+
Mail::to($email)->send((new CheckoutAssetMail($assetItem, $assetItem->assignedTo, $logItem->user, $acceptance, $logItem->note, firstTimeSending: false))->locale($locale));
1184+
11901185
return redirect()->route('reports/unaccepted_assets')->with('success', trans('admin/reports/general.reminder_sent'));
11911186
}
11921187

‎app/Mail/CheckoutAssetMail.php‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@ class CheckoutAssetMail extends Mailable
2020
{
2121
use Queueable, SerializesModels;
2222

23+
private bool $firstTimeSending;
24+
2325
/**
2426
* Create a new message instance.
2527
*/
26-
public function __construct(Asset $asset, $checkedOutTo, User $checkedOutBy, $acceptance, $note)
28+
public function __construct(Asset $asset, $checkedOutTo, User $checkedOutBy, $acceptance, $note, bool $firstTimeSending = true)
2729
{
2830
$this->item = $asset;
2931
$this->admin = $checkedOutBy;
@@ -36,6 +38,8 @@ public function __construct(Asset $asset, $checkedOutTo, User $checkedOutBy, $a
3638
$this->last_checkout = '';
3739
$this->expected_checkin = '';
3840

41+
$this->firstTimeSending = $firstTimeSending;
42+
3943
if ($this->item->last_checkout) {
4044
$this->last_checkout = Helper::getFormattedDateObject($this->item->last_checkout, 'date',
4145
false);
@@ -56,7 +60,7 @@ public function envelope(): Envelope
5660

5761
return new Envelope(
5862
from: $from,
59-
subject: trans('mail.Asset_Checkout_Notification'),
63+
subject: $this->getSubject(),
6064
);
6165
}
6266

@@ -107,4 +111,13 @@ public function attachments(): array
107111
{
108112
return [];
109113
}
114+
115+
private function getSubject(): string
116+
{
117+
if ($this->firstTimeSending) {
118+
return trans('mail.Asset_Checkout_Notification');
119+
}
120+
121+
return trans('mail.unaccepted_asset_reminder');
122+
}
110123
}

‎database/factories/CheckoutAcceptanceFactory.php‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ public function definition()
2727
public function configure(): static
2828
{
2929
return $this->afterCreating(function (CheckoutAcceptance $acceptance) {
30+
if ($acceptance->checkoutable instanceof Asset) {
31+
$this->createdAssociatedActionLogEntry($acceptance);
32+
}
33+
3034
if ($acceptance->checkoutable instanceof Asset && $acceptance->assignedTo instanceof User) {
3135
$acceptance->checkoutable->update([
3236
'assigned_to' => $acceptance->assigned_to_id,
@@ -51,4 +55,23 @@ public function pending()
5155
'declined_at' => null,
5256
]);
5357
}
58+
59+
public function accepted()
60+
{
61+
return $this->state([
62+
'accepted_at' => now()->subDay(),
63+
'declined_at' => null,
64+
]);
65+
}
66+
67+
private function createdAssociatedActionLogEntry(CheckoutAcceptance $acceptance): void
68+
{
69+
$acceptance->checkoutable->assetlog()->create([
70+
'action_type' => 'checkout',
71+
'target_id' => $acceptance->assigned_to_id,
72+
'target_type' => get_class($acceptance->assignedTo),
73+
'item_id' => $acceptance->checkoutable_id,
74+
'item_type' => $acceptance->checkoutable_type,
75+
]);
76+
}
5477
}

‎tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
<?php
2+
3+
namespace Tests\Feature\Notifications\Email;
4+
5+
use App\Mail\CheckoutAssetMail;
6+
use App\Models\CheckoutAcceptance;
7+
use App\Models\User;
8+
use Illuminate\Support\Facades\Mail;
9+
use PHPUnit\Framework\Attributes\DataProvider;
10+
use Tests\TestCase;
11+
12+
class AssetAcceptanceReminderTest extends TestCase
13+
{
14+
protected function setUp(): void
15+
{
16+
parent::setUp();
17+
18+
Mail::fake();
19+
}
20+
21+
public function testMustHavePermissionToSendReminder()
22+
{
23+
$checkoutAcceptance = CheckoutAcceptance::factory()->pending()->create();
24+
$userWithoutPermission = User::factory()->create();
25+
26+
$this->actingAs($userWithoutPermission)
27+
->post($this->routeFor($checkoutAcceptance))
28+
->assertForbidden();
29+
30+
Mail::assertNotSent(CheckoutAssetMail::class);
31+
}
32+
33+
public function testReminderNotSentIfAcceptanceDoesNotExist()
34+
{
35+
$this->actingAs(User::factory()->canViewReports()->create())
36+
->post(route('reports/unaccepted_assets_sent_reminder', [
37+
'acceptance_id' => 999999,
38+
]));
39+
40+
Mail::assertNotSent(CheckoutAssetMail::class);
41+
}
42+
43+
public function testReminderNotSentIfAcceptanceAlreadyAccepted()
44+
{
45+
$checkoutAcceptanceAlreadyAccepted = CheckoutAcceptance::factory()->accepted()->create();
46+
47+
$this->actingAs(User::factory()->canViewReports()->create())
48+
->post($this->routeFor($checkoutAcceptanceAlreadyAccepted));
49+
50+
Mail::assertNotSent(CheckoutAssetMail::class);
51+
}
52+
53+
public static function CheckoutAcceptancesToUsersWithoutEmailAddresses()
54+
{
55+
yield 'User with null email address' => [
56+
function () {
57+
return CheckoutAcceptance::factory()
58+
->pending()
59+
->forAssignedTo(['email' => null])
60+
->create();
61+
}
62+
];
63+
64+
yield 'User with empty string email address' => [
65+
function () {
66+
return CheckoutAcceptance::factory()
67+
->pending()
68+
->forAssignedTo(['email' => ''])
69+
->create();
70+
}
71+
];
72+
}
73+
74+
#[DataProvider('CheckoutAcceptancesToUsersWithoutEmailAddresses')]
75+
public function testUserWithoutEmailAddressHandledGracefully($callback)
76+
{
77+
$checkoutAcceptance = $callback();
78+
79+
$this->actingAs(User::factory()->canViewReports()->create())
80+
->post($this->routeFor($checkoutAcceptance))
81+
// check we didn't crash...
82+
->assertRedirect();
83+
84+
Mail::assertNotSent(CheckoutAssetMail::class);
85+
}
86+
87+
public function testReminderIsSentToUser()
88+
{
89+
$checkoutAcceptance = CheckoutAcceptance::factory()->pending()->create();
90+
91+
$this->actingAs(User::factory()->canViewReports()->create())
92+
->post($this->routeFor($checkoutAcceptance))
93+
->assertRedirect(route('reports/unaccepted_assets'));
94+
95+
Mail::assertSent(CheckoutAssetMail::class, 1);
96+
Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) use ($checkoutAcceptance) {
97+
return $mail->hasTo($checkoutAcceptance->assignedTo->email)
98+
&& $mail->hasSubject(trans('mail.unaccepted_asset_reminder'));
99+
});
100+
}
101+
102+
private function routeFor(CheckoutAcceptance $checkoutAcceptance): string
103+
{
104+
return route('reports/unaccepted_assets_sent_reminder', [
105+
'acceptance_id' => $checkoutAcceptance->id,
106+
]);
107+
}
108+
}

‎tests/Unit/NotificationTest.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77
use App\Models\AssetModel;
88
use App\Models\Category;
99
use Carbon\Carbon;
10-
use App\Notifications\CheckoutAssetNotification;
1110
use Illuminate\Support\Facades\Mail;
12-
use Illuminate\Support\Facades\Notification;
1311
use Tests\TestCase;
1412

1513
class NotificationTest extends TestCase
@@ -33,8 +31,8 @@ public function testAUserIsEmailedIfTheyCheckoutAnAssetWithEULA()
3331

3432
Mail::fake();
3533
$asset->checkOut($user, $admin->id);
36-
Mail::assertSent(CheckoutAssetMail::class, function ($mail) use ($user) {
37-
return $mail->hasTo($user->email);
34+
Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) use ($user) {
35+
return $mail->hasTo($user->email) && $mail->hasSubject(trans('mail.Asset_Checkout_Notification'));
3836
});
3937
}
4038
public function testDefaultEulaIsSentWhenSetInCategory()

0 commit comments

Comments
 (0)