Merge pull request #18542 from marcusmoore/fixes/fd-53577-reminder-subject-2

Updated subject line of acceptance reminder emails
This commit is contained in:
snipe
2026-02-12 21:22:55 +00:00
committed by GitHub
11 changed files with 399 additions and 170 deletions

View File

@@ -1228,12 +1228,14 @@ class ReportsController extends Controller
];
$mailable= $lookup[get_class($acceptance->checkoutable)];
return new $mailable($acceptance->checkoutable,
return new $mailable(
$acceptance->checkoutable,
$acceptance->checkedOutTo ?? $acceptance->assignedTo,
$logItem->adminuser,
$acceptance,
$acceptance->note);
$acceptance->note,
firstTimeSending: false,
);
}
/**
* sentAssetAcceptanceReminder

View File

@@ -18,10 +18,12 @@ class CheckoutAccessoryMail extends BaseMailable
{
use Queueable, SerializesModels;
private bool $firstTimeSending;
/**
* Create a new message instance.
*/
public function __construct(Accessory $accessory, $checkedOutTo, User $checkedOutBy, $acceptance, $note)
public function __construct(Accessory $accessory, $checkedOutTo, User $checkedOutBy, $acceptance, $note, bool $firstTimeSending = true)
{
$this->item = $accessory;
$this->admin = $checkedOutBy;
@@ -29,6 +31,7 @@ class CheckoutAccessoryMail extends BaseMailable
$this->checkout_qty = $accessory->checkout_qty;
$this->target = $checkedOutTo;
$this->acceptance = $acceptance;
$this->firstTimeSending = $firstTimeSending;
$this->settings = Setting::getSettings();
}
@@ -41,7 +44,7 @@ class CheckoutAccessoryMail extends BaseMailable
return new Envelope(
from: $from,
subject: trans_choice('mail.Accessory_Checkout_Notification', $this->checkout_qty),
subject: $this->getSubject(),
);
}
@@ -88,14 +91,18 @@ class CheckoutAccessoryMail extends BaseMailable
return trans_choice('mail.new_item_checked_location', $this->checkout_qty, ['location' => $this->target->name]);
}
if ($this->requiresAcceptance()) {
if ($this->firstTimeSending && $this->requiresAcceptance()) {
return trans_choice('mail.new_item_checked_with_acceptance', $this->checkout_qty);
}
if (!$this->requiresAcceptance()) {
if ($this->firstTimeSending && !$this->requiresAcceptance()) {
return trans_choice('mail.new_item_checked', $this->checkout_qty);
}
if (!$this->firstTimeSending && $this->requiresAcceptance()) {
return trans('mail.recent_item_checked');
}
// we shouldn't get here but let's send a default message just in case
return trans('new_item_checked');
}
@@ -113,4 +120,13 @@ class CheckoutAccessoryMail extends BaseMailable
{
return [];
}
private function getSubject(): string
{
if ($this->firstTimeSending) {
return trans_choice('mail.Accessory_Checkout_Notification', $this->checkout_qty);
}
return trans('mail.unaccepted_asset_reminder');
}
}

View File

@@ -15,10 +15,12 @@ class CheckoutConsumableMail extends BaseMailable
{
use Queueable, SerializesModels;
private bool $firstTimeSending;
/**
* Create a new message instance.
*/
public function __construct(Consumable $consumable, $checkedOutTo, User $checkedOutBy, $acceptance, $note)
public function __construct(Consumable $consumable, $checkedOutTo, User $checkedOutBy, $acceptance, $note, bool $firstTimeSending = true)
{
$this->item = $consumable;
$this->admin = $checkedOutBy;
@@ -26,6 +28,7 @@ class CheckoutConsumableMail extends BaseMailable
$this->target = $checkedOutTo;
$this->acceptance = $acceptance;
$this->qty = $consumable->checkout_qty;
$this->firstTimeSending = $firstTimeSending;
$this->settings = Setting::getSettings();
}
@@ -39,7 +42,7 @@ class CheckoutConsumableMail extends BaseMailable
return new Envelope(
from: $from,
subject: trans('mail.Confirm_consumable_delivery'),
subject: $this->getSubject(),
);
}
@@ -65,6 +68,7 @@ class CheckoutConsumableMail extends BaseMailable
'req_accept' => $req_accept,
'accept_url' => $accept_url,
'qty' => $this->qty,
'introduction_line' => $this->introductionLine(),
]
);
}
@@ -78,4 +82,36 @@ class CheckoutConsumableMail extends BaseMailable
{
return [];
}
private function getSubject(): string
{
if ($this->firstTimeSending) {
return trans('mail.Confirm_consumable_delivery');
}
return trans('mail.unaccepted_asset_reminder');
}
private function introductionLine()
{
if ($this->firstTimeSending && $this->requiresAcceptance()) {
return trans_choice('mail.new_item_checked_with_acceptance', $this->qty);
}
if ($this->firstTimeSending && !$this->requiresAcceptance()) {
return trans_choice('mail.new_item_checked', $this->qty);
}
if (!$this->firstTimeSending && $this->requiresAcceptance()) {
return trans('mail.recent_item_checked');
}
// 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;
}
}

View File

@@ -16,10 +16,12 @@ class CheckoutLicenseMail extends BaseMailable
{
use Queueable, SerializesModels;
private bool $firstTimeSending;
/**
* Create a new message instance.
*/
public function __construct(LicenseSeat $licenseSeat, $checkedOutTo, User $checkedOutBy, $acceptance, $note)
public function __construct(LicenseSeat $licenseSeat, $checkedOutTo, User $checkedOutBy, $acceptance, $note, bool $firstTimeSending = true)
{
$this->item = $licenseSeat;
$this->admin = $checkedOutBy;
@@ -27,6 +29,7 @@ class CheckoutLicenseMail extends BaseMailable
$this->acceptance = $acceptance;
$this->settings = Setting::getSettings();
$this->target = $checkedOutTo;
$this->firstTimeSending = $firstTimeSending;
if($this->target instanceof User){
$this->target = $this->target->display_name;
@@ -45,7 +48,7 @@ class CheckoutLicenseMail extends BaseMailable
return new Envelope(
from: $from,
subject: trans('mail.Confirm_license_delivery'),
subject: $this->getSubject(),
);
}
@@ -69,6 +72,7 @@ class CheckoutLicenseMail extends BaseMailable
'eula' => $eula,
'req_accept' => $req_accept,
'accept_url' => $accept_url,
'introduction_line' => $this->introductionLine(),
]
);
}
@@ -82,4 +86,36 @@ class CheckoutLicenseMail extends BaseMailable
{
return [];
}
private function getSubject(): string
{
if ($this->firstTimeSending) {
return trans('mail.Confirm_license_delivery');
}
return trans('mail.unaccepted_asset_reminder');
}
private function introductionLine(): string
{
if ($this->firstTimeSending && $this->requiresAcceptance()) {
return trans_choice('mail.new_item_checked_with_acceptance', 1);
}
if ($this->firstTimeSending && !$this->requiresAcceptance()) {
return trans_choice('mail.new_item_checked', 1);
}
if (!$this->firstTimeSending && $this->requiresAcceptance()) {
return trans('mail.recent_item_checked');
}
// 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;
}
}

View File

@@ -20,6 +20,7 @@ use Watson\Validating\ValidatingTrait;
*/
class AccessoryCheckout extends Model
{
use HasFactory;
use Searchable;
protected $fillable = [
@@ -41,7 +42,7 @@ class AccessoryCheckout extends Model
*/
public function accessory()
{
return $this->hasOne(Accessory::class, 'id', 'accessory_id');
return $this->belongsTo(Accessory::class);
}
public function accessories()
@@ -57,7 +58,7 @@ class AccessoryCheckout extends Model
*/
public function adminuser()
{
return $this->hasOne(\App\Models\User::class, 'id', 'created_by');
return $this->belongsTo(User::class, 'created_by');
}
/**

View File

@@ -0,0 +1,23 @@
<?php
namespace Database\Factories;
use App\Models\Accessory;
use App\Models\AccessoryCheckout;
use App\Models\User;
use Illuminate\Database\Eloquent\Factories\Factory;
class AccessoryCheckoutFactory extends Factory
{
protected $model = AccessoryCheckout::class;
public function definition(): array
{
return [
'created_by' => User::factory(),
'accessory_id' => Accessory::factory(),
'assigned_to' => User::factory(),
'assigned_type' => User::class,
];
}
}

View File

@@ -49,4 +49,11 @@ class LicenseSeatFactory extends Factory
$seat->license->update(['reassignable' => false]);
});
}
public function requiringAcceptance()
{
return $this->afterCreating(function ($seat) {
$seat->license->category->update(['require_acceptance' => 1]);
});
}
}

View File

@@ -1,7 +1,7 @@
@component('mail::message')
# {{ trans('mail.hello') }} {{ $target->display_name }},
{{ trans_choice('mail.new_item_checked', $qty) }}
{{ $introduction_line }}
@component('mail::table')

View File

@@ -1,7 +1,7 @@
@component('mail::message')
# {{ trans('mail.hello').' '.$target.','}}
{{ trans_choice('mail.new_item_checked', 1) }}
{{ $introduction_line }}
@component('mail::table')
| | |

View File

@@ -0,0 +1,263 @@
<?php
namespace Tests\Feature\Notifications\Email;
use App\Mail\CheckoutAccessoryMail;
use App\Mail\CheckoutAssetMail;
use App\Mail\CheckoutConsumableMail;
use App\Mail\CheckoutLicenseMail;
use App\Models\Accessory;
use App\Models\AccessoryCheckout;
use App\Models\Actionlog;
use App\Models\Asset;
use App\Models\CheckoutAcceptance;
use App\Models\Consumable;
use App\Models\License;
use App\Models\LicenseSeat;
use App\Models\User;
use Generator;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Support\Facades\Mail;
use PHPUnit\Framework\Attributes\DataProvider;
use Tests\TestCase;
class AcceptanceReminderTest extends TestCase
{
private User $admin;
private User $assignee;
protected function setUp(): void
{
parent::setUp();
Mail::fake();
$this->admin = User::factory()->canViewReports()->create();
$this->assignee = User::factory()->create();
}
public function testMustHavePermissionToSendReminder()
{
$checkoutAcceptance = CheckoutAcceptance::factory()->pending()->create();
$userWithoutPermission = User::factory()->create();
$this->actingAs($userWithoutPermission)
->post(route('reports/unaccepted_assets_sent_reminder', [
'acceptance_id' => $checkoutAcceptance->id,
]))
->assertForbidden();
Mail::assertNotSent(CheckoutAssetMail::class);
}
public function testReminderNotSentIfAcceptanceDoesNotExist()
{
$this->actingAs($this->admin)
->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->admin)
->post(route('reports/unaccepted_assets_sent_reminder', [
'acceptance_id' => $checkoutAcceptanceAlreadyAccepted->id,
]));
Mail::assertNotSent(CheckoutAssetMail::class);
}
public static function checkoutAcceptancesToUsersWithoutEmailAddresses(): Generator
{
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($this->admin)
->post(route('reports/unaccepted_assets_sent_reminder', [
'acceptance_id' => $checkoutAcceptance->id,
]))
// check we didn't crash...
->assertRedirect();
Mail::assertNotSent(CheckoutAssetMail::class);
}
public function testReminderIsSentToUserForAccessory()
{
$accessory = Accessory::factory()->requiringAcceptance()->create();
$acceptance = $this->createCheckoutAcceptance($accessory, $this->assignee);
$this->createActionLogEntry($accessory, $this->admin, $this->assignee, $acceptance);
AccessoryCheckout::factory()
->for($this->admin, 'adminuser')
->for($accessory)
->for($this->assignee, 'assignedTo')
->create();
$this->actingAs($this->admin)
->post(route('reports/unaccepted_assets_sent_reminder', [
'acceptance_id' => $acceptance->id,
]))
->assertRedirect(route('reports/unaccepted_assets'));
Mail::assertSent(CheckoutAccessoryMail::class, 1);
Mail::assertSent(CheckoutAccessoryMail::class, function (CheckoutAccessoryMail $mail) {
return $mail->hasTo($this->assignee);
});
Mail::assertSent(CheckoutAccessoryMail::class, function (CheckoutAccessoryMail $mail) {
return $mail->hasSubject(trans('mail.unaccepted_asset_reminder'));
});
Mail::assertSent(CheckoutAccessoryMail::class, function (CheckoutAccessoryMail $mail) {
return str_contains($mail->render(), trans('mail.recent_item_checked'));
});
}
public function testReminderIsSentToUserForAsset()
{
$asset = Asset::factory()->requiresAcceptance()->create();
$acceptance = $this->createCheckoutAcceptance($asset, $this->assignee);
$this->createActionLogEntry($asset, $this->admin, $this->assignee, $acceptance);
$this->actingAs($this->admin)
->post(route('reports/unaccepted_assets_sent_reminder', [
'acceptance_id' => $acceptance->id,
]))
->assertRedirect(route('reports/unaccepted_assets'));
Mail::assertSent(CheckoutAssetMail::class, 1);
Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) {
return $mail->hasTo($this->assignee);
});
Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) {
return $mail->hasSubject(trans('mail.unaccepted_asset_reminder'));
});
Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) {
return str_contains($mail->render(), trans('mail.recent_item_checked'));
});
}
public function testReminderIsSentToUserForConsumable()
{
$consumable = Consumable::factory()->requiringAcceptance()->create();
$acceptance = $this->createCheckoutAcceptance($consumable, $this->assignee);
$this->createActionLogEntry($consumable, $this->admin, $this->assignee, $acceptance);
$this->actingAs($this->admin)
->post(route('reports/unaccepted_assets_sent_reminder', [
'acceptance_id' => $acceptance->id,
]))
->assertRedirect(route('reports/unaccepted_assets'));
Mail::assertSent(CheckoutConsumableMail::class, 1);
Mail::assertSent(CheckoutConsumableMail::class, function (CheckoutConsumableMail $mail) {
return $mail->hasTo($this->assignee);
});
Mail::assertSent(CheckoutConsumableMail::class, function (CheckoutConsumableMail $mail) {
return $mail->hasSubject(trans('mail.unaccepted_asset_reminder'));
});
Mail::assertSent(CheckoutConsumableMail::class, function (CheckoutConsumableMail $mail) {
return str_contains($mail->render(), trans('mail.recent_item_checked'));
});
}
public function testReminderIsSentToUserForLicenseSeat()
{
$licenseSeat = LicenseSeat::factory()->requiringAcceptance()->create();
$acceptance = $this->createCheckoutAcceptance($licenseSeat, $this->assignee);
$this->createActionLogEntry($licenseSeat, $this->admin, $this->assignee, $acceptance);
$this->actingAs($this->admin)
->post(route('reports/unaccepted_assets_sent_reminder', [
'acceptance_id' => $acceptance->id,
]))
->assertRedirect(route('reports/unaccepted_assets'));
Mail::assertSent(CheckoutLicenseMail::class, 1);
Mail::assertSent(CheckoutLicenseMail::class, function (CheckoutLicenseMail $mail) {
return $mail->hasTo($this->assignee);
});
Mail::assertSent(CheckoutLicenseMail::class, function (CheckoutLicenseMail $mail) {
return $mail->hasSubject(trans('mail.unaccepted_asset_reminder'));
});
Mail::assertSent(CheckoutLicenseMail::class, function (CheckoutLicenseMail $mail) {
return str_contains($mail->render(), trans('mail.recent_item_checked'));
});
}
private function createCheckoutAcceptance(Model $item, Model $assignee): CheckoutAcceptance
{
return CheckoutAcceptance::factory()
->for($item, 'checkoutable')
->for($assignee, 'assignedTo')
->withoutActionLog()
->pending()
->create();
}
private function createActionLogEntry(Model $item, Model $admin, Model $assignee, CheckoutAcceptance $acceptance): Actionlog
{
$itemId = $item->id;
$itemType = get_class($item);
if (get_class($item) === LicenseSeat::class) {
$itemId = $item->license->id;
$itemType = License::class;
}
return Actionlog::factory()
->for($admin, 'adminuser')
->for($assignee, 'target')
->create([
'action_type' => 'checkout',
'item_id' => $itemId,
'item_type' => $itemType,
'created_at' => $acceptance->created_at,
]);
}
}

View File

@@ -1,155 +0,0 @@
<?php
namespace Tests\Feature\Notifications\Email;
use App\Mail\CheckoutAccessoryMail;
use App\Mail\CheckoutAssetMail;
use App\Mail\CheckoutComponentMail;
use App\Mail\CheckoutConsumableMail;
use App\Mail\CheckoutLicenseMail;
use App\Models\Accessory;
use App\Models\Actionlog;
use App\Models\Asset;
use App\Models\CheckoutAcceptance;
use App\Models\Component;
use App\Models\Consumable;
use App\Models\License;
use App\Models\LicenseSeat;
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()
{
$checkedOutBy = User::factory()->canViewReports()->create();
$checkoutTypes = [
Asset::class => CheckoutAssetMail::class,
Accessory::class => CheckoutAccessoryMail::class,
LicenseSeat::class => CheckoutLicenseMail::class,
Consumable::class => CheckoutConsumableMail::class,
//for the future its setup for components, but we dont send reminders for components at the moment.
// Component::class => CheckoutComponentMail::class,
];
$assignee = User::factory()->create(['email' => 'test@example.com']);
foreach ($checkoutTypes as $modelClass => $mailable) {
$item = $modelClass::factory()->create();
$acceptance = CheckoutAcceptance::factory()->withoutActionLog()->pending()->create([
'checkoutable_id' => $item->id,
'checkoutable_type' => $modelClass,
'assigned_to_id' => $assignee->id,
]);
if ($modelClass === LicenseSeat::class) {
$logType = License::class;
$logId = $item->license->id;
} else {
$logType = $modelClass;
$logId = $item->id;
}
Actionlog::factory()->create([
'action_type' => 'checkout',
'created_by' => $checkedOutBy->id,
'target_id' => $assignee->id,
'item_type' => $logType,
'item_id' => $logId,
'created_at' => $acceptance->created_at,
]);
$this->actingAs($checkedOutBy)
->post($this->routeFor($acceptance))
->assertRedirect(route('reports/unaccepted_assets'));
}
Mail::assertSent($mailable, 1);
Mail::assertSent($mailable, function ($mail) use ($assignee) {
return $mail->hasTo($assignee->email);
});
}
private function routeFor(CheckoutAcceptance $checkoutAcceptance): string
{
return route('reports/unaccepted_assets_sent_reminder', [
'acceptance_id' => $checkoutAcceptance->id,
]);
}
}