Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added "Reminder" to subject line of follow up asset checkout emails #16156

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
11 changes: 3 additions & 8 deletions app/Http/Controllers/ReportsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}

Expand Down
17 changes: 15 additions & 2 deletions app/Mail/CheckoutAssetMail.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -56,7 +60,7 @@ public function envelope(): Envelope

return new Envelope(
from: $from,
subject: trans('mail.Asset_Checkout_Notification'),
subject: $this->getSubject(),
);
}

Expand Down Expand Up @@ -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');
}
}
23 changes: 23 additions & 0 deletions database/factories/CheckoutAcceptanceFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
]);
}
}
1 change: 1 addition & 0 deletions resources/lang/en-US/mail.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
108 changes: 108 additions & 0 deletions tests/Feature/Notifications/Email/AssetAcceptanceReminderTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
<?php

namespace Tests\Feature\Notifications\Email;

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
{
protected function setUp(): void
{
parent::setUp();

Mail::fake();
}

public function testMustHavePermissionToSendReminder()
{
$checkoutAcceptance = CheckoutAcceptance::factory()->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,
]);
}
}
6 changes: 2 additions & 4 deletions tests/Unit/NotificationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down
Loading