From 4f5d4a09843135af12ba3bb80ad73199a3fdc8cf Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 2 Jun 2025 16:25:06 -0700 Subject: [PATCH 01/22] Scaffold settings page changes --- resources/views/settings/alerts.blade.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/resources/views/settings/alerts.blade.php b/resources/views/settings/alerts.blade.php index 9f31206452..6e5ab2c486 100644 --- a/resources/views/settings/alerts.blade.php +++ b/resources/views/settings/alerts.blade.php @@ -96,8 +96,18 @@ {!! $errors->first('admin_cc_email', '
') !!}

{{ trans('admin/settings/general.admin_cc_email_help') }}

- - + + +
+
+ +
From 367ab8ddd5fb275eaf640c9fdc5dd4016746ead9 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 2 Jun 2025 16:27:04 -0700 Subject: [PATCH 02/22] Add help text --- resources/views/settings/alerts.blade.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/resources/views/settings/alerts.blade.php b/resources/views/settings/alerts.blade.php index 6e5ab2c486..6b28a5f3a5 100644 --- a/resources/views/settings/alerts.blade.php +++ b/resources/views/settings/alerts.blade.php @@ -108,6 +108,9 @@ Only send if acceptance required +

+ "Always send copy to CC email" requires valid CC Email to be set. +

From 054ff425474570e93cf8aa30c6a4f59c00d4724f Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 2 Jun 2025 17:03:14 -0700 Subject: [PATCH 03/22] Add migration for admin_cc_always --- ..._add_admin_cc_always_to_settings_table.php | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 database/migrations/2025_06_02_233556_add_admin_cc_always_to_settings_table.php diff --git a/database/migrations/2025_06_02_233556_add_admin_cc_always_to_settings_table.php b/database/migrations/2025_06_02_233556_add_admin_cc_always_to_settings_table.php new file mode 100644 index 0000000000..d28115a6d2 --- /dev/null +++ b/database/migrations/2025_06_02_233556_add_admin_cc_always_to_settings_table.php @@ -0,0 +1,27 @@ +boolean('admin_cc_always')->after('admin_cc_email')->default(1); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('settings', function (Blueprint $table) { + $table->dropColumn('admin_cc_always'); + }); + } +}; From 6bc3209333fc053b58507f9cd9dfc2220652268e Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 2 Jun 2025 17:05:00 -0700 Subject: [PATCH 04/22] Use @checked for inputs --- resources/views/settings/alerts.blade.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/resources/views/settings/alerts.blade.php b/resources/views/settings/alerts.blade.php index 6b28a5f3a5..53a85ea7a2 100644 --- a/resources/views/settings/alerts.blade.php +++ b/resources/views/settings/alerts.blade.php @@ -101,11 +101,21 @@

From 9e4aab7165dcace335dfc26d8cdefdbcf888af4c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 2 Jun 2025 17:05:18 -0700 Subject: [PATCH 05/22] Scaffold tests --- tests/Feature/Settings/AlertsSettingTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/Feature/Settings/AlertsSettingTest.php b/tests/Feature/Settings/AlertsSettingTest.php index d79bd1cf21..1281547c67 100644 --- a/tests/Feature/Settings/AlertsSettingTest.php +++ b/tests/Feature/Settings/AlertsSettingTest.php @@ -26,4 +26,18 @@ class AlertsSettingTest extends TestCase $this->followRedirects($response)->assertSee('alert-success'); } + public function testCannotUpdateAdminCcAwaysWithoutAdminCcEmail() + { + $this->markTestIncomplete(); + } + + public function test_can_update_admin_cc_always_to_true() + { + $this->markTestIncomplete(); + } + + public function test_can_update_admin_cc_always_to_false() + { + $this->markTestIncomplete(); + } } From d75120000aaf08665d4c34957e054f6255b102f4 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 2 Jun 2025 17:11:18 -0700 Subject: [PATCH 06/22] Add failing tests --- tests/Feature/Settings/AlertsSettingTest.php | 28 +++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/tests/Feature/Settings/AlertsSettingTest.php b/tests/Feature/Settings/AlertsSettingTest.php index 1281547c67..6dae959648 100644 --- a/tests/Feature/Settings/AlertsSettingTest.php +++ b/tests/Feature/Settings/AlertsSettingTest.php @@ -26,18 +26,32 @@ class AlertsSettingTest extends TestCase $this->followRedirects($response)->assertSee('alert-success'); } - public function testCannotUpdateAdminCcAwaysWithoutAdminCcEmail() - { - $this->markTestIncomplete(); - } - public function test_can_update_admin_cc_always_to_true() { - $this->markTestIncomplete(); + $this->settings->disableAdminCCAlways(); + + $this->actingAs(User::factory()->superuser()->create()) + ->post(route('settings.alerts.save', ['admin_cc_always' => '1'])); + + $this->assertDatabaseHas('settings', ['admin_cc_always' => '1']); + } + + public function test_cannot_update_admin_cc_always_without_admin_cc_email() + { + $this->settings->disableAdminCCAlways(); + + $this->actingAs(User::factory()->superuser()->create()) + ->post(route('settings.alerts.save', ['admin_cc_always' => '1'])) + ->assertSessionHasErrors('admin_cc_always'); } public function test_can_update_admin_cc_always_to_false() { - $this->markTestIncomplete(); + $this->settings->enableAdminCCAlways(); + + $this->actingAs(User::factory()->superuser()->create()) + ->post(route('settings.alerts.save', ['admin_cc_always' => '0'])); + + $this->assertDatabaseHas('settings', ['admin_cc_always' => '0']); } } From 6e37f945ac081493d1db188bb7d773e86414bc12 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 2 Jun 2025 17:13:37 -0700 Subject: [PATCH 07/22] Add test helpers --- tests/Feature/Settings/AlertsSettingTest.php | 2 +- tests/Support/Settings.php | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/Feature/Settings/AlertsSettingTest.php b/tests/Feature/Settings/AlertsSettingTest.php index 6dae959648..42974d6640 100644 --- a/tests/Feature/Settings/AlertsSettingTest.php +++ b/tests/Feature/Settings/AlertsSettingTest.php @@ -47,7 +47,7 @@ class AlertsSettingTest extends TestCase public function test_can_update_admin_cc_always_to_false() { - $this->settings->enableAdminCCAlways(); + $this->settings->enableAdminCC()->enableAdminCCAlways(); $this->actingAs(User::factory()->superuser()->create()) ->post(route('settings.alerts.save', ['admin_cc_always' => '0'])); diff --git a/tests/Support/Settings.php b/tests/Support/Settings.php index bcbd83d8b6..d33970e683 100644 --- a/tests/Support/Settings.php +++ b/tests/Support/Settings.php @@ -4,6 +4,7 @@ namespace Tests\Support; use App\Models\Setting; use Illuminate\Support\Facades\Crypt; +use RuntimeException; class Settings { @@ -60,6 +61,24 @@ class Settings ]); } + public function enableAdminCCAlways(): Settings + { + if (is_null($this->setting->admin_cc_email) || $this->setting->admin_cc_email == 0) { + throw new RuntimeException('admin_cc_email requires admin_cc_email to be set.'); + } + + return $this->update([ + 'admin_cc_always' => 1, + ]); + } + + public function disableAdminCCAlways(): Settings + { + return $this->update([ + 'admin_cc_always' => 0, + ]); + } + public function enableMultipleFullCompanySupport(): Settings { return $this->update(['full_multiple_companies_support' => 1]); From 12dc33244da092213fad045ed99782be557147ea Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 2 Jun 2025 17:20:01 -0700 Subject: [PATCH 08/22] Start storing admin_cc_always --- app/Http/Controllers/SettingsController.php | 1 + app/Http/Requests/StoreNotificationSettings.php | 4 ++++ tests/Feature/Settings/AlertsSettingTest.php | 5 ++++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/SettingsController.php b/app/Http/Controllers/SettingsController.php index 747f7b7284..b652455a57 100644 --- a/app/Http/Controllers/SettingsController.php +++ b/app/Http/Controllers/SettingsController.php @@ -650,6 +650,7 @@ class SettingsController extends Controller $setting->alert_email = $alert_email; $setting->admin_cc_email = $admin_cc_email; + $setting->admin_cc_always = $request->validated('admin_cc_always'); $setting->alerts_enabled = $request->input('alerts_enabled', '0'); $setting->alert_interval = $request->input('alert_interval'); $setting->alert_threshold = $request->input('alert_threshold'); diff --git a/app/Http/Requests/StoreNotificationSettings.php b/app/Http/Requests/StoreNotificationSettings.php index bf5f5b3d4e..f58d014c76 100644 --- a/app/Http/Requests/StoreNotificationSettings.php +++ b/app/Http/Requests/StoreNotificationSettings.php @@ -5,6 +5,7 @@ namespace App\Http\Requests; use App\Models\Accessory; use Illuminate\Foundation\Http\FormRequest; use Illuminate\Support\Facades\Gate; +use Illuminate\Validation\Rule; class StoreNotificationSettings extends FormRequest { @@ -26,6 +27,9 @@ class StoreNotificationSettings extends FormRequest return [ 'alert_email' => 'email_array|nullable', 'admin_cc_email' => 'email_array|nullable', + 'admin_cc_always' => [ + Rule::in('0', '1'), + ], 'alert_threshold' => 'numeric|nullable', 'alert_interval' => 'numeric|nullable|gt:0', 'audit_warning_days' => 'numeric|nullable', diff --git a/tests/Feature/Settings/AlertsSettingTest.php b/tests/Feature/Settings/AlertsSettingTest.php index 42974d6640..7812d0eb33 100644 --- a/tests/Feature/Settings/AlertsSettingTest.php +++ b/tests/Feature/Settings/AlertsSettingTest.php @@ -18,7 +18,10 @@ class AlertsSettingTest extends TestCase public function testAdminCCEmailArrayCanBeSaved() { $response = $this->actingAs(User::factory()->superuser()->create()) - ->post(route('settings.alerts.save', ['alert_email' => 'me@example.com,you@example.com'])) + ->post(route('settings.alerts.save', [ + 'alert_email' => 'me@example.com,you@example.com', + 'admin_cc_always' => '1', + ])) ->assertStatus(302) ->assertValid('alert_email') ->assertRedirect(route('settings.index')) From 79e00c1191ef6c42579c000d2e50c6c3bac2eccc Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 2 Jun 2025 17:21:58 -0700 Subject: [PATCH 09/22] Require admin_cc_email if admin_cc_always is true --- app/Http/Requests/StoreNotificationSettings.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/Http/Requests/StoreNotificationSettings.php b/app/Http/Requests/StoreNotificationSettings.php index f58d014c76..6b2b1f5aec 100644 --- a/app/Http/Requests/StoreNotificationSettings.php +++ b/app/Http/Requests/StoreNotificationSettings.php @@ -26,7 +26,11 @@ class StoreNotificationSettings extends FormRequest { return [ 'alert_email' => 'email_array|nullable', - 'admin_cc_email' => 'email_array|nullable', + 'admin_cc_email' => [ + 'email_array', + 'nullable', + 'required_if_accepted:admin_cc_always', + ], 'admin_cc_always' => [ Rule::in('0', '1'), ], From dec3c4aff380f0d1819ceaa5e54c04db0ea73e6c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 2 Jun 2025 17:45:08 -0700 Subject: [PATCH 10/22] Improve wording --- resources/views/settings/alerts.blade.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/views/settings/alerts.blade.php b/resources/views/settings/alerts.blade.php index 53a85ea7a2..3a472fbe20 100644 --- a/resources/views/settings/alerts.blade.php +++ b/resources/views/settings/alerts.blade.php @@ -116,7 +116,7 @@ value="0" @checked($setting->admin_cc_always == 0) > - Only send if acceptance required + Only send copy if acceptance required

"Always send copy to CC email" requires valid CC Email to be set. From ea3364ab6815b0821908ce14cac6555383b11690 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 3 Jun 2025 13:19:44 -0700 Subject: [PATCH 11/22] Split test case --- ...ficationsToAdminAlertEmailUponCheckout.php | 89 +++++++++++++++++++ ...ilNotificationsToUserUponCheckoutTest.php} | 40 +-------- 2 files changed, 90 insertions(+), 39 deletions(-) create mode 100644 tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckout.php rename tests/Feature/Notifications/Email/{EmailNotificationsUponCheckoutTest.php => EmailNotificationsToUserUponCheckoutTest.php} (68%) diff --git a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckout.php b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckout.php new file mode 100644 index 0000000000..930a1388f0 --- /dev/null +++ b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckout.php @@ -0,0 +1,89 @@ +category = Category::factory()->create([ + 'checkin_email' => false, + 'eula_text' => null, + 'require_acceptance' => false, + 'use_default_eula' => false, + ]); + + $this->assetModel = AssetModel::factory()->for($this->category)->create(); + $this->asset = Asset::factory()->for($this->assetModel, 'model')->create(); + + $this->user = User::factory()->create(); + } + + public function test_admin_alert_email_sends() + { + $this->settings->enableAdminCC('cc@example.com'); + + $this->category->update(['checkin_email' => true]); + + $this->fireCheckoutEvent(); + + Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) { + return $mail->hasCc('cc@example.com'); + }); + } + + public function test_admin_alert_email_still_sent_when_category_is_not_set_to_send_email_to_user() + { + $this->settings->enableAdminCC('cc@example.com'); + + $this->fireCheckoutEvent(); + + Mail::assertSent(CheckoutAssetMail::class, function ($mail) { + return $mail->hasTo('cc@example.com'); + }); + } + + public function test_admin_alert_email_still_sent_when_user_has_no_email_address() + { + $this->settings->enableAdminCC('cc@example.com'); + + $this->category->update(['checkin_email' => true]); + $this->user->update(['email' => null]); + + $this->fireCheckoutEvent(); + + Mail::assertSent(CheckoutAssetMail::class, function ($mail) { + return $mail->hasTo('cc@example.com'); + }); + } + + private function fireCheckoutEvent(): void + { + event(new CheckoutableCheckedOut( + $this->asset, + $this->user, + User::factory()->superuser()->create(), + '', + )); + } +} diff --git a/tests/Feature/Notifications/Email/EmailNotificationsUponCheckoutTest.php b/tests/Feature/Notifications/Email/EmailNotificationsToUserUponCheckoutTest.php similarity index 68% rename from tests/Feature/Notifications/Email/EmailNotificationsUponCheckoutTest.php rename to tests/Feature/Notifications/Email/EmailNotificationsToUserUponCheckoutTest.php index e5a1a66cb6..e5741da8ec 100644 --- a/tests/Feature/Notifications/Email/EmailNotificationsUponCheckoutTest.php +++ b/tests/Feature/Notifications/Email/EmailNotificationsToUserUponCheckoutTest.php @@ -13,7 +13,7 @@ use PHPUnit\Framework\Attributes\Group; use Tests\TestCase; #[Group('notifications')] -class EmailNotificationsUponCheckoutTest extends TestCase +class EmailNotificationsToUserUponCheckoutTest extends TestCase { private Asset $asset; private AssetModel $assetModel; @@ -87,44 +87,6 @@ class EmailNotificationsUponCheckoutTest extends TestCase Mail::assertNothingSent(); } - public function test_admin_alert_email_sends() - { - $this->settings->enableAdminCC('cc@example.com'); - - $this->category->update(['checkin_email' => true]); - - $this->fireCheckoutEvent(); - - Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) { - return $mail->hasCc('cc@example.com'); - }); - } - - public function test_admin_alert_email_still_sent_when_category_is_not_set_to_send_email_to_user() - { - $this->settings->enableAdminCC('cc@example.com'); - - $this->fireCheckoutEvent(); - - Mail::assertSent(CheckoutAssetMail::class, function ($mail) { - return $mail->hasTo('cc@example.com'); - }); - } - - public function test_admin_alert_email_still_sent_when_user_has_no_email_address() - { - $this->settings->enableAdminCC('cc@example.com'); - - $this->category->update(['checkin_email' => true]); - $this->user->update(['email' => null]); - - $this->fireCheckoutEvent(); - - Mail::assertSent(CheckoutAssetMail::class, function ($mail) { - return $mail->hasTo('cc@example.com'); - }); - } - private function fireCheckoutEvent(): void { event(new CheckoutableCheckedOut( From 51479c8bbc3a5cc76ed7db79435e15c65930dfb9 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 3 Jun 2025 13:28:40 -0700 Subject: [PATCH 12/22] Scaffold failing test --- ...NotificationsToAdminAlertEmailUponCheckout.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckout.php b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckout.php index 930a1388f0..9493e30b90 100644 --- a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckout.php +++ b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckout.php @@ -77,6 +77,21 @@ class EmailNotificationsToAdminAlertEmailUponCheckout extends TestCase }); } + public function test_admin_alert_email_not_sent_when_always_send_is_false_and_asset_does_not_require_acceptance() + { + $this->settings + ->enableAdminCC('cc@example.com') + ->disableAdminCCAlways(); + + $this->category->update(['checkin_email' => false]); + + $this->fireCheckoutEvent(); + + Mail::assertNotSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) { + return $mail->hasTo('cc@example.com') || $mail->hasCc('cc@example.com'); + }); + } + private function fireCheckoutEvent(): void { event(new CheckoutableCheckedOut( From 3942489d21aeb8538c8084acb5537162727720fa Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 3 Jun 2025 14:13:52 -0700 Subject: [PATCH 13/22] Add Test suffix and scaffold test --- ...ationsToAdminAlertEmailUponCheckoutTest.php} | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) rename tests/Feature/Notifications/Email/{EmailNotificationsToAdminAlertEmailUponCheckout.php => EmailNotificationsToAdminAlertEmailUponCheckoutTest.php} (83%) diff --git a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckout.php b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckoutTest.php similarity index 83% rename from tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckout.php rename to tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckoutTest.php index 9493e30b90..d881ae7dab 100644 --- a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckout.php +++ b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckoutTest.php @@ -13,7 +13,7 @@ use PHPUnit\Framework\Attributes\Group; use Tests\TestCase; #[Group('notifications')] -class EmailNotificationsToAdminAlertEmailUponCheckout extends TestCase +class EmailNotificationsToAdminAlertEmailUponCheckoutTest extends TestCase { private Asset $asset; private AssetModel $assetModel; @@ -77,6 +77,21 @@ class EmailNotificationsToAdminAlertEmailUponCheckout extends TestCase }); } + public function test_admin_alert_email_sent_when_always_send_is_true_and_asset_does_not_require_acceptance() + { + $this->settings + ->enableAdminCC('cc@example.com') + ->enableAdminCCAlways(); + + $this->category->update(['checkin_email' => false]); + + $this->fireCheckoutEvent(); + + Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) { + return $mail->hasTo('cc@example.com') || $mail->hasCc('cc@example.com'); + }); + } + public function test_admin_alert_email_not_sent_when_always_send_is_false_and_asset_does_not_require_acceptance() { $this->settings From d01f7cf3177e7cc948c55adee9806986bf939bba Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 3 Jun 2025 15:32:11 -0700 Subject: [PATCH 14/22] Adhere to admin_cc_always setting --- app/Listeners/CheckoutableListener.php | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index e647bcd9e0..ec843b980b 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -69,16 +69,16 @@ class CheckoutableListener return; } + $acceptance = $this->getCheckoutAcceptance($event); + $shouldSendEmailToUser = $this->shouldSendCheckoutEmailToUser($event->checkoutable); - $shouldSendEmailToAlertAddress = $this->shouldSendEmailToAlertAddress(); + $shouldSendEmailToAlertAddress = $this->shouldSendEmailToAlertAddress($acceptance); $shouldSendWebhookNotification = $this->shouldSendWebhookNotification(); if (!$shouldSendEmailToUser && !$shouldSendEmailToAlertAddress && !$shouldSendWebhookNotification) { return; } - $acceptance = $this->getCheckoutAcceptance($event); - if ($shouldSendEmailToUser || $shouldSendEmailToAlertAddress) { $mailable = $this->getCheckoutMailType($event, $acceptance); $notifiable = $this->getNotifiableUser($event); @@ -419,9 +419,19 @@ class CheckoutableListener return false; } - private function shouldSendEmailToAlertAddress(): bool + private function shouldSendEmailToAlertAddress($acceptance = null): bool { - return Setting::getSettings() && Setting::getSettings()->admin_cc_email; + $setting = Setting::getSettings(); + + if (!$setting) { + return false; + } + + if (is_null($acceptance) && !$setting->admin_cc_always) { + return false; + } + + return (bool) $setting->admin_cc_email; } private function getFormattedAlertAddresses(): array From 8bc57f98a5a8ab25497e128038b9ee0ee1f43d6e Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 4 Jun 2025 11:51:29 -0700 Subject: [PATCH 15/22] Split test case --- ...ationsToAdminAlertEmailUponCheckinTest.php | 85 +++++++++++++++++++ ...ailNotificationsToUserUponCheckinTest.php} | 55 +----------- 2 files changed, 86 insertions(+), 54 deletions(-) create mode 100644 tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php rename tests/Feature/Notifications/Email/{EmailNotificationsUponCheckinTest.php => EmailNotificationsToUserUponCheckinTest.php} (64%) diff --git a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php new file mode 100644 index 0000000000..ec2481b59d --- /dev/null +++ b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php @@ -0,0 +1,85 @@ +settings->enableAdminCC('cc@example.com'); + + $user = User::factory()->create(); + $asset = Asset::factory()->assignedToUser($user)->create(); + + $asset->model->category->update(['checkin_email' => true]); + + $this->fireCheckInEvent($asset, $user); + + Mail::assertSent(CheckinAssetMail::class, function ($mail) use ($user) { + return $mail->hasTo($user->email) && $mail->hasCc('cc@example.com'); + }); + } + + public function test_admin_alert_email_still_sent_when_category_email_is_not_set_to_send_email_to_user() + { + $this->settings->enableAdminCC('cc@example.com'); + + $category = Category::factory()->create([ + 'checkin_email' => false, + 'eula_text' => null, + 'use_default_eula' => false, + ]); + $assetModel = AssetModel::factory()->create(['category_id' => $category->id]); + $asset = Asset::factory()->create(['model_id' => $assetModel->id]); + + $this->fireCheckInEvent($asset, User::factory()->create()); + + Mail::assertSent(CheckinAssetMail::class, function ($mail) { + return $mail->hasTo('cc@example.com'); + }); + } + + public function test_admin_alert_email_still_sent_when_user_has_no_email_address() + { + $this->settings->enableAdminCC('cc@example.com'); + + $user = User::factory()->create(['email' => null]); + $asset = Asset::factory()->assignedToUser($user)->create(); + + $asset->model->category->update(['checkin_email' => true]); + + $this->fireCheckInEvent($asset, $user); + + Mail::assertSent(CheckinAssetMail::class, function ($mail) { + return $mail->hasTo('cc@example.com'); + }); + } + + private function fireCheckInEvent($asset, $user): void + { + event(new CheckoutableCheckedIn( + $asset, + $user, + User::factory()->checkinAssets()->create(), + '' + )); + } +} diff --git a/tests/Feature/Notifications/Email/EmailNotificationsUponCheckinTest.php b/tests/Feature/Notifications/Email/EmailNotificationsToUserUponCheckinTest.php similarity index 64% rename from tests/Feature/Notifications/Email/EmailNotificationsUponCheckinTest.php rename to tests/Feature/Notifications/Email/EmailNotificationsToUserUponCheckinTest.php index 254e2e09ca..432ce88b4b 100644 --- a/tests/Feature/Notifications/Email/EmailNotificationsUponCheckinTest.php +++ b/tests/Feature/Notifications/Email/EmailNotificationsToUserUponCheckinTest.php @@ -4,8 +4,6 @@ namespace Tests\Feature\Notifications\Email; use App\Mail\CheckinAssetMail; use App\Models\Accessory; -use App\Models\AssetModel; -use App\Models\Category; use App\Models\Consumable; use App\Models\LicenseSeat; use Illuminate\Support\Facades\Mail; @@ -16,7 +14,7 @@ use App\Models\User; use Tests\TestCase; #[Group('notifications')] -class EmailNotificationsUponCheckinTest extends TestCase +class EmailNotificationsToUserUponCheckinTest extends TestCase { protected function setUp(): void { @@ -101,57 +99,6 @@ class EmailNotificationsUponCheckinTest extends TestCase Mail::assertNothingSent(); } - public function test_admin_alert_email_sends() - { - $this->settings->enableAdminCC('cc@example.com'); - - $user = User::factory()->create(); - $asset = Asset::factory()->assignedToUser($user)->create(); - - $asset->model->category->update(['checkin_email' => true]); - - $this->fireCheckInEvent($asset, $user); - - Mail::assertSent(CheckinAssetMail::class, function ($mail) use ($user) { - return $mail->hasTo($user->email) && $mail->hasCc('cc@example.com'); - }); - } - - public function test_admin_alert_email_still_sent_when_category_email_is_not_set_to_send_email_to_user() - { - $this->settings->enableAdminCC('cc@example.com'); - - $category = Category::factory()->create([ - 'checkin_email' => false, - 'eula_text' => null, - 'use_default_eula' => false, - ]); - $assetModel = AssetModel::factory()->create(['category_id' => $category->id]); - $asset = Asset::factory()->create(['model_id' => $assetModel->id]); - - $this->fireCheckInEvent($asset, User::factory()->create()); - - Mail::assertSent(CheckinAssetMail::class, function ($mail) { - return $mail->hasTo('cc@example.com'); - }); - } - - public function test_admin_alert_email_still_sent_when_user_has_no_email_address() - { - $this->settings->enableAdminCC('cc@example.com'); - - $user = User::factory()->create(['email' => null]); - $asset = Asset::factory()->assignedToUser($user)->create(); - - $asset->model->category->update(['checkin_email' => true]); - - $this->fireCheckInEvent($asset, $user); - - Mail::assertSent(CheckinAssetMail::class, function ($mail) { - return $mail->hasTo('cc@example.com'); - }); - } - private function fireCheckInEvent($asset, $user): void { event(new CheckoutableCheckedIn( From 8e70ff135a65d52edf2034b8777af1ec727aa96b Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 4 Jun 2025 12:44:55 -0700 Subject: [PATCH 16/22] Scaffold tests --- ...ilNotificationsToAdminAlertEmailUponCheckinTest.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php index ec2481b59d..47cfb2e492 100644 --- a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php +++ b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php @@ -73,6 +73,16 @@ class EmailNotificationsToAdminAlertEmailUponCheckinTest extends TestCase }); } + public function test_admin_alert_email_sent_when_always_send_is_true_and_asset_does_not_require_acceptance() + { + $this->markTestIncomplete(); + } + + public function test_admin_alert_email_not_sent_when_always_send_is_false_and_asset_does_not_require_acceptance() + { + $this->markTestIncomplete(); + } + private function fireCheckInEvent($asset, $user): void { event(new CheckoutableCheckedIn( From 31db86abd3aa45fcee6546003195f5be4659c4ee Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 4 Jun 2025 12:51:48 -0700 Subject: [PATCH 17/22] Simplify test --- ...ationsToAdminAlertEmailUponCheckinTest.php | 50 ++++++++++++------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php index 47cfb2e492..a65baa1a5a 100644 --- a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php +++ b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php @@ -15,26 +15,45 @@ use Tests\TestCase; #[Group('notifications')] class EmailNotificationsToAdminAlertEmailUponCheckinTest extends TestCase { + private Asset $asset; + private AssetModel $assetModel; + private Category $category; + private User $user; + protected function setUp(): void { parent::setUp(); Mail::fake(); + + $this->user = User::factory()->create(); + + $this->category = Category::factory()->create([ + 'checkin_email' => false, + 'eula_text' => null, + 'require_acceptance' => false, + 'use_default_eula' => false, + ]); + + $this->assetModel = AssetModel::factory()->for($this->category)->create(); + + $this->asset = Asset::factory() + ->for($this->assetModel, 'model') + ->assignedToUser($this->user) + ->create(); + } public function test_admin_alert_email_sends() { $this->settings->enableAdminCC('cc@example.com'); - $user = User::factory()->create(); - $asset = Asset::factory()->assignedToUser($user)->create(); + $this->category->update(['checkin_email' => true]); - $asset->model->category->update(['checkin_email' => true]); + $this->fireCheckInEvent($this->asset, $this->user); - $this->fireCheckInEvent($asset, $user); - - Mail::assertSent(CheckinAssetMail::class, function ($mail) use ($user) { - return $mail->hasTo($user->email) && $mail->hasCc('cc@example.com'); + Mail::assertSent(CheckinAssetMail::class, function ($mail) { + return $mail->hasTo($this->user->email) && $mail->hasCc('cc@example.com'); }); } @@ -42,15 +61,9 @@ class EmailNotificationsToAdminAlertEmailUponCheckinTest extends TestCase { $this->settings->enableAdminCC('cc@example.com'); - $category = Category::factory()->create([ - 'checkin_email' => false, - 'eula_text' => null, - 'use_default_eula' => false, - ]); - $assetModel = AssetModel::factory()->create(['category_id' => $category->id]); - $asset = Asset::factory()->create(['model_id' => $assetModel->id]); + $this->category->update(['checkin_email' => false]); - $this->fireCheckInEvent($asset, User::factory()->create()); + $this->fireCheckInEvent($this->asset, $this->user); Mail::assertSent(CheckinAssetMail::class, function ($mail) { return $mail->hasTo('cc@example.com'); @@ -61,12 +74,11 @@ class EmailNotificationsToAdminAlertEmailUponCheckinTest extends TestCase { $this->settings->enableAdminCC('cc@example.com'); - $user = User::factory()->create(['email' => null]); - $asset = Asset::factory()->assignedToUser($user)->create(); + $this->user->update(['email' => null]); - $asset->model->category->update(['checkin_email' => true]); + $this->category->update(['checkin_email' => true]); - $this->fireCheckInEvent($asset, $user); + $this->fireCheckInEvent($this->asset, $this->user); Mail::assertSent(CheckinAssetMail::class, function ($mail) { return $mail->hasTo('cc@example.com'); From 92e22eead55b1e9d3da970ed95ea139731ba820c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 4 Jun 2025 12:51:59 -0700 Subject: [PATCH 18/22] Formatting --- .../Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php index a65baa1a5a..f2ce26e712 100644 --- a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php +++ b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php @@ -41,7 +41,6 @@ class EmailNotificationsToAdminAlertEmailUponCheckinTest extends TestCase ->for($this->assetModel, 'model') ->assignedToUser($this->user) ->create(); - } public function test_admin_alert_email_sends() From 10be434c137fdfad4718fa2e6ce7dfd0d688009b Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 4 Jun 2025 12:58:49 -0700 Subject: [PATCH 19/22] Populate tests --- ...ationsToAdminAlertEmailUponCheckinTest.php | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php index f2ce26e712..1bdbf0b673 100644 --- a/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php +++ b/tests/Feature/Notifications/Email/EmailNotificationsToAdminAlertEmailUponCheckinTest.php @@ -86,12 +86,32 @@ class EmailNotificationsToAdminAlertEmailUponCheckinTest extends TestCase public function test_admin_alert_email_sent_when_always_send_is_true_and_asset_does_not_require_acceptance() { - $this->markTestIncomplete(); + $this->settings + ->enableAdminCC('cc@example.com') + ->enableAdminCCAlways(); + + $this->category->update(['checkin_email' => false]); + + $this->fireCheckInEvent($this->asset, $this->user); + + Mail::assertSent(CheckinAssetMail::class, function ($mail) { + return $mail->hasTo('cc@example.com') || $mail->hasCc('cc@example.com'); + }); } public function test_admin_alert_email_not_sent_when_always_send_is_false_and_asset_does_not_require_acceptance() { - $this->markTestIncomplete(); + $this->settings + ->enableAdminCC('cc@example.com') + ->disableAdminCCAlways(); + + $this->category->update(['checkin_email' => false]); + + $this->fireCheckInEvent($this->asset, $this->user); + + Mail::assertNotSent(CheckinAssetMail::class, function ($mail) { + return $mail->hasTo('cc@example.com') || $mail->hasCc('cc@example.com'); + }); } private function fireCheckInEvent($asset, $user): void From c1505de8d6915623acf2545309040f4b0ca0769c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 4 Jun 2025 13:02:04 -0700 Subject: [PATCH 20/22] Update wording --- resources/views/settings/alerts.blade.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/resources/views/settings/alerts.blade.php b/resources/views/settings/alerts.blade.php index 3a472fbe20..59c1888d2f 100644 --- a/resources/views/settings/alerts.blade.php +++ b/resources/views/settings/alerts.blade.php @@ -107,7 +107,7 @@ value="1" @checked($setting->admin_cc_always == 1) > - Always send copy to CC email + Always send copy upon checkin/checkout -

- "Always send copy to CC email" requires valid CC Email to be set. -

From 088e6af0b5cd696966f889066c4cb18a3f17ab51 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 5 Jun 2025 12:07:14 -0700 Subject: [PATCH 21/22] Remove admin_cc_email validation for admin_cc_always --- app/Http/Requests/StoreNotificationSettings.php | 6 +----- tests/Feature/Settings/AlertsSettingTest.php | 9 --------- tests/Support/Settings.php | 5 ----- 3 files changed, 1 insertion(+), 19 deletions(-) diff --git a/app/Http/Requests/StoreNotificationSettings.php b/app/Http/Requests/StoreNotificationSettings.php index 6b2b1f5aec..f58d014c76 100644 --- a/app/Http/Requests/StoreNotificationSettings.php +++ b/app/Http/Requests/StoreNotificationSettings.php @@ -26,11 +26,7 @@ class StoreNotificationSettings extends FormRequest { return [ 'alert_email' => 'email_array|nullable', - 'admin_cc_email' => [ - 'email_array', - 'nullable', - 'required_if_accepted:admin_cc_always', - ], + 'admin_cc_email' => 'email_array|nullable', 'admin_cc_always' => [ Rule::in('0', '1'), ], diff --git a/tests/Feature/Settings/AlertsSettingTest.php b/tests/Feature/Settings/AlertsSettingTest.php index 7812d0eb33..f8829d3825 100644 --- a/tests/Feature/Settings/AlertsSettingTest.php +++ b/tests/Feature/Settings/AlertsSettingTest.php @@ -39,15 +39,6 @@ class AlertsSettingTest extends TestCase $this->assertDatabaseHas('settings', ['admin_cc_always' => '1']); } - public function test_cannot_update_admin_cc_always_without_admin_cc_email() - { - $this->settings->disableAdminCCAlways(); - - $this->actingAs(User::factory()->superuser()->create()) - ->post(route('settings.alerts.save', ['admin_cc_always' => '1'])) - ->assertSessionHasErrors('admin_cc_always'); - } - public function test_can_update_admin_cc_always_to_false() { $this->settings->enableAdminCC()->enableAdminCCAlways(); diff --git a/tests/Support/Settings.php b/tests/Support/Settings.php index d33970e683..8ff8ba6945 100644 --- a/tests/Support/Settings.php +++ b/tests/Support/Settings.php @@ -4,7 +4,6 @@ namespace Tests\Support; use App\Models\Setting; use Illuminate\Support\Facades\Crypt; -use RuntimeException; class Settings { @@ -63,10 +62,6 @@ class Settings public function enableAdminCCAlways(): Settings { - if (is_null($this->setting->admin_cc_email) || $this->setting->admin_cc_email == 0) { - throw new RuntimeException('admin_cc_email requires admin_cc_email to be set.'); - } - return $this->update([ 'admin_cc_always' => 1, ]); From 77234f6580215f6b7a4c75b8baca07cd742ec03f Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 5 Jun 2025 12:24:46 -0700 Subject: [PATCH 22/22] Extract translation strings --- resources/lang/en-US/admin/settings/general.php | 2 ++ resources/views/settings/alerts.blade.php | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/resources/lang/en-US/admin/settings/general.php b/resources/lang/en-US/admin/settings/general.php index 5da86537d6..9dafacd0e7 100644 --- a/resources/lang/en-US/admin/settings/general.php +++ b/resources/lang/en-US/admin/settings/general.php @@ -9,6 +9,8 @@ return [ 'ad_append_domain_help' => 'User isn\'t required to write "username@domain.local", they can just type "username".', 'admin_cc_email' => 'CC Email', 'admin_cc_email_help' => 'Send a copy of checkin/checkout emails to this address.', + 'admin_cc_always' => 'Always send copy upon checkin/checkout', + 'admin_cc_when_acceptance_required' => 'Only send copy upon checkout if acceptance is required', 'admin_settings' => 'Admin Settings', 'is_ad' => 'This is an Active Directory server', 'alerts' => 'Alerts', diff --git a/resources/views/settings/alerts.blade.php b/resources/views/settings/alerts.blade.php index 59c1888d2f..c861cc63bf 100644 --- a/resources/views/settings/alerts.blade.php +++ b/resources/views/settings/alerts.blade.php @@ -107,7 +107,7 @@ value="1" @checked($setting->admin_cc_always == 1) > - Always send copy upon checkin/checkout + {{ trans('admin/settings/general.admin_cc_always') }}