fa/breadcrumb-the-shire
1
0
Fork 1
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(notifications): use createFromMessage directly for CLI-safe scheduler notifications

Browse Source 
createForTenantAdminUsersFromMessage relies on listActiveUserIdsByTenantId
which returns empty results in CLI context (scheduler). Instead, iterate
admin users directly and call createFromMessage per user per tenant.
Verified end-to-end: notification created, dedup blocks duplicates.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Felix Schneider
2026-04-06 12:38:47 +02:00
parent 34892aff6f
commit fe88e748a3
2 changed files with 52 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
modules/notifications/lib/Module/Notifications/Listeners/SchedulerJobFailedNotificationListener.php
View File

@@ -57,33 +57,18 @@ final class SchedulerJobFailedNotificationListener implements EventListener
]
);
$tenantIds = $this->resolveAdminTenantIds($adminUserIds);
foreach ($tenantIds as $tenantId) {
$this->notificationService->createForTenantAdminUsersFromMessage(
$tenantId,
$message,
null,
$adminSet
);
}
}
/**
* Collect all unique tenant IDs across admin users.
*
* @param list<int> $userIds
* @return list<int>
*/
private function resolveAdminTenantIds(array $userIds): array
{
$tenantIds = [];
foreach ($userIds as $userId) {
foreach ($this->userTenantRepository->listTenantIdsByUserId($userId) as $tenantId) {
$tenantIds[$tenantId] = true;
// Create notifications directly per admin user + tenant.
// This avoids listActiveUserIdsByTenantId which requires a web-context
// DB session — the scheduler runs in CLI where that lookup may fail.
foreach ($adminUserIds as $userId) {
$tenantIds = $this->userTenantRepository->listTenantIdsByUserId($userId);
if ($tenantIds === []) {
$this->notificationService->createFromMessage($userId, null, $message);
continue;
}
foreach ($tenantIds as $tenantId) {
$this->notificationService->createFromMessage($userId, $tenantId, $message);
}
}
return array_keys($tenantIds);
}
}

60
modules/notifications/tests/Module/Notifications/Listeners/SchedulerJobFailedNotificationListenerTest.php
View File

@@ -26,7 +26,7 @@ class SchedulerJobFailedNotificationListenerTest extends TestCase
$this->listener = new SchedulerJobFailedNotificationListener($this->notifService, $this->userRepo, $this->utRepo);
}
public function testHandleCreatesNotificationsForAdminsAcrossTenants(): void
public function testHandleCreatesNotificationsPerAdminPerTenant(): void
{
$this->userRepo->expects($this->any())->method('listPrivilegedUserIdsByPermissionKeys')
->with([PermissionService::JOBS_MANAGE])
@@ -39,31 +39,53 @@ class SchedulerJobFailedNotificationListenerTest extends TestCase
[30, [2, 3]],
]);
$this->notifService->expects($this->exactly(3))
->method('createForTenantAdminUsersFromMessage')
->willReturnCallback(function (int $tenantId, NotificationMessage $message, ?int $excludeUserId, array $adminSet): int {
$this->assertContains($tenantId, [1, 2, 3]);
$calls = [];
$this->notifService->expects($this->exactly(4))
->method('createFromMessage')
->willReturnCallback(function (int $userId, ?int $tenantId, NotificationMessage $message) use (&$calls): int {
$calls[] = ['user' => $userId, 'tenant' => $tenantId];
$this->assertSame('scheduler.job_failed', $message->type());
$this->assertStringContainsString('System audit purge', $message->title());
$this->assertStringContainsString('Test Job', $message->title());
$this->assertSame('admin/scheduled-jobs', $message->link());
$this->assertNull($excludeUserId);
$this->assertArrayHasKey(20, $adminSet);
$this->assertArrayHasKey(30, $adminSet);
$this->assertSame('Scheduled job failed: %s', $message->titleKey());
$data = $message->data();
$this->assertSame('job:system_audit_purge', $data['dedupe_target']);
$this->assertSame('job_not_registered', $data['error_code']);
$this->assertSame('job:test_fail_job', $data['dedupe_target']);
return 1;
});
$this->listener->handle('scheduler.job_failed', [
'job_id' => 5,
'job_key' => 'system_audit_purge',
'label' => 'System audit purge',
'job_key' => 'test_fail_job',
'label' => 'Test Job',
'error_code' => 'job_not_registered',
'error_message' => null,
'trigger_type' => 'scheduler',
]);
// User 20: tenant 1, tenant 2. User 30: tenant 2, tenant 3. = 4 calls total.
$this->assertCount(4, $calls);
$this->assertSame(['user' => 20, 'tenant' => 1], $calls[0]);
$this->assertSame(['user' => 20, 'tenant' => 2], $calls[1]);
$this->assertSame(['user' => 30, 'tenant' => 2], $calls[2]);
$this->assertSame(['user' => 30, 'tenant' => 3], $calls[3]);
}
public function testHandleCreatesGlobalNotificationWhenNoTenants(): void
{
$this->userRepo->expects($this->any())->method('listPrivilegedUserIdsByPermissionKeys')->willReturn([10]);
$this->utRepo->expects($this->any())->method('listTenantIdsByUserId')->willReturn([]);
$this->notifService->expects($this->once())
->method('createFromMessage')
->with(10, null, $this->anything())
->willReturn(1);
$this->listener->handle('scheduler.job_failed', [
'job_id' => 1,
'job_key' => 'some_job',
'label' => 'Some Job',
'error_code' => 'unexpected_error',
]);
}
public function testHandleUsesJobKeyAsLabelWhenLabelEmpty(): void
@@ -72,8 +94,8 @@ class SchedulerJobFailedNotificationListenerTest extends TestCase
$this->utRepo->expects($this->any())->method('listTenantIdsByUserId')->willReturn([1]);
$this->notifService->expects($this->once())
->method('createForTenantAdminUsersFromMessage')
->willReturnCallback(function (int $tenantId, NotificationMessage $message): int {
->method('createFromMessage')
->willReturnCallback(function (int $userId, ?int $tenantId, NotificationMessage $message): int {
$this->assertStringContainsString('my_job_key', $message->title());
return 1;
});
@@ -88,7 +110,7 @@ class SchedulerJobFailedNotificationListenerTest extends TestCase
public function testHandleSkipsEmptyPayload(): void
{
$this->notifService->expects($this->never())->method('createForTenantAdminUsersFromMessage');
$this->notifService->expects($this->never())->method('createFromMessage');
$this->listener->handle('scheduler.job_failed', []);
$this->listener->handle('scheduler.job_failed', ['job_key' => '', 'job_id' => 0]);
@@ -98,7 +120,7 @@ class SchedulerJobFailedNotificationListenerTest extends TestCase
public function testHandleSkipsWhenNoAdmins(): void
{
$this->userRepo->expects($this->any())->method('listPrivilegedUserIdsByPermissionKeys')->willReturn([]);
$this->notifService->expects($this->never())->method('createForTenantAdminUsersFromMessage');
$this->notifService->expects($this->never())->method('createFromMessage');
$this->listener->handle('scheduler.job_failed', [
'job_id' => 5,
@@ -114,8 +136,8 @@ class SchedulerJobFailedNotificationListenerTest extends TestCase
$this->utRepo->expects($this->any())->method('listTenantIdsByUserId')->willReturn([1]);
$this->notifService->expects($this->once())
->method('createForTenantAdminUsersFromMessage')
->willReturnCallback(function (int $tenantId, NotificationMessage $message): int {
->method('createFromMessage')
->willReturnCallback(function (int $userId, ?int $tenantId, NotificationMessage $message): int {
$this->assertNull($message->body());
return 1;
});