From e746a2e8741256fdc68e16e4d31eb264fa01c8f9 Mon Sep 17 00:00:00 2001 From: fs Date: Thu, 26 Mar 2026 12:01:11 +0100 Subject: [PATCH] refactor(notifications): remove factory indirection, interface, and listener duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove NotificationRepositoryFactory, NotificationServicesFactory, and NotificationRepositoryInterface — all single-consumer abstractions with no polymorphism benefit. Simplify container registrar to wire services directly. Extract shared listener logic (sanitizeTenantIds, resolveDisplayName, resolveUuid) into NotificationListenerTrait, eliminating duplication across 4 listeners. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../Listeners/NotificationListenerTrait.php | 56 +++++++++++++++++++ .../UserActivatedNotificationListener.php | 32 ++--------- ...rAssignmentChangedNotificationListener.php | 32 ++--------- .../UserCreatedNotificationListener.php | 8 +-- .../UserDeactivatedNotificationListener.php | 32 ++--------- .../UserDeletedNotificationListener.php | 2 + .../NotificationsContainerRegistrar.php | 11 ++-- .../Repository/NotificationRepository.php | 2 +- .../NotificationRepositoryInterface.php | 26 --------- .../Service/NotificationRepositoryFactory.php | 16 ------ .../Service/NotificationService.php | 4 +- .../Service/NotificationServicesFactory.php | 23 -------- .../Service/NotificationServiceTest.php | 6 +- 13 files changed, 83 insertions(+), 167 deletions(-) create mode 100644 modules/notifications/lib/Module/Notifications/Listeners/NotificationListenerTrait.php delete mode 100644 modules/notifications/lib/Module/Notifications/Repository/NotificationRepositoryInterface.php delete mode 100644 modules/notifications/lib/Module/Notifications/Service/NotificationRepositoryFactory.php delete mode 100644 modules/notifications/lib/Module/Notifications/Service/NotificationServicesFactory.php diff --git a/modules/notifications/lib/Module/Notifications/Listeners/NotificationListenerTrait.php b/modules/notifications/lib/Module/Notifications/Listeners/NotificationListenerTrait.php new file mode 100644 index 0000000..03b1714 --- /dev/null +++ b/modules/notifications/lib/Module/Notifications/Listeners/NotificationListenerTrait.php @@ -0,0 +1,56 @@ + + */ + protected function sanitizeTenantIds(mixed $tenantIds): array + { + if (!is_array($tenantIds)) { + return []; + } + + $ids = array_values(array_unique(array_map('intval', $tenantIds))); + return array_values(array_filter($ids, static fn (int $tenantId): bool => $tenantId > 0)); + } +} diff --git a/modules/notifications/lib/Module/Notifications/Listeners/UserActivatedNotificationListener.php b/modules/notifications/lib/Module/Notifications/Listeners/UserActivatedNotificationListener.php index 630864a..5f4b560 100644 --- a/modules/notifications/lib/Module/Notifications/Listeners/UserActivatedNotificationListener.php +++ b/modules/notifications/lib/Module/Notifications/Listeners/UserActivatedNotificationListener.php @@ -11,6 +11,8 @@ use MintyPHP\Service\Access\PermissionService; final class UserActivatedNotificationListener implements EventListener { + use NotificationListenerTrait; + public function __construct( private readonly NotificationService $notificationService, private readonly UserReadRepositoryInterface $userReadRepository, @@ -26,21 +28,8 @@ final class UserActivatedNotificationListener implements EventListener } $user = $this->userReadRepository->find($userId); - $displayName = trim((string) ($payload['display_name'] ?? '')); - if ($displayName === '' && is_array($user)) { - $displayName = trim((string) ($user['display_name'] ?? '')); - if ($displayName === '') { - $displayName = trim((string) (($user['first_name'] ?? '') . ' ' . ($user['last_name'] ?? ''))); - } - } - if ($displayName === '') { - $displayName = '#' . $userId; - } - - $uuid = trim((string) ($payload['uuid'] ?? '')); - if ($uuid === '' && is_array($user)) { - $uuid = trim((string) ($user['uuid'] ?? '')); - } + $displayName = $this->resolveDisplayName($payload, $user, $userId); + $uuid = $this->resolveUuid($payload, $user); $tenantIds = $this->sanitizeTenantIds($payload['tenant_ids'] ?? []); if ($tenantIds === []) { @@ -79,17 +68,4 @@ final class UserActivatedNotificationListener implements EventListener ); } } - - /** - * @return list - */ - private function sanitizeTenantIds(mixed $tenantIds): array - { - if (!is_array($tenantIds)) { - return []; - } - - $ids = array_values(array_unique(array_map('intval', $tenantIds))); - return array_values(array_filter($ids, static fn (int $tenantId): bool => $tenantId > 0)); - } } diff --git a/modules/notifications/lib/Module/Notifications/Listeners/UserAssignmentChangedNotificationListener.php b/modules/notifications/lib/Module/Notifications/Listeners/UserAssignmentChangedNotificationListener.php index 989f487..ab9e187 100644 --- a/modules/notifications/lib/Module/Notifications/Listeners/UserAssignmentChangedNotificationListener.php +++ b/modules/notifications/lib/Module/Notifications/Listeners/UserAssignmentChangedNotificationListener.php @@ -10,6 +10,8 @@ use MintyPHP\Service\Access\PermissionService; final class UserAssignmentChangedNotificationListener implements EventListener { + use NotificationListenerTrait; + public function __construct( private readonly NotificationService $notificationService, private readonly UserReadRepositoryInterface $userReadRepository @@ -25,21 +27,8 @@ final class UserAssignmentChangedNotificationListener implements EventListener $actorUserId = isset($payload['actor_user_id']) ? (int) $payload['actor_user_id'] : null; $user = $this->userReadRepository->find($userId); - $displayName = trim((string) ($payload['display_name'] ?? '')); - if ($displayName === '' && is_array($user)) { - $displayName = trim((string) ($user['display_name'] ?? '')); - if ($displayName === '') { - $displayName = trim((string) (($user['first_name'] ?? '') . ' ' . ($user['last_name'] ?? ''))); - } - } - if ($displayName === '') { - $displayName = '#' . $userId; - } - - $uuid = trim((string) ($payload['uuid'] ?? '')); - if ($uuid === '' && is_array($user)) { - $uuid = trim((string) ($user['uuid'] ?? '')); - } + $displayName = $this->resolveDisplayName($payload, $user, $userId); + $uuid = $this->resolveUuid($payload, $user); $tenantIds = $this->sanitizeTenantIds($payload['tenant_ids'] ?? []); $changes = is_array($payload['changes'] ?? null) ? $payload['changes'] : []; @@ -87,17 +76,4 @@ final class UserAssignmentChangedNotificationListener implements EventListener ); } } - - /** - * @return list - */ - private function sanitizeTenantIds(mixed $tenantIds): array - { - if (!is_array($tenantIds)) { - return []; - } - - $ids = array_values(array_unique(array_map('intval', $tenantIds))); - return array_values(array_filter($ids, static fn (int $tenantId): bool => $tenantId > 0)); - } } diff --git a/modules/notifications/lib/Module/Notifications/Listeners/UserCreatedNotificationListener.php b/modules/notifications/lib/Module/Notifications/Listeners/UserCreatedNotificationListener.php index cec5baa..8639335 100644 --- a/modules/notifications/lib/Module/Notifications/Listeners/UserCreatedNotificationListener.php +++ b/modules/notifications/lib/Module/Notifications/Listeners/UserCreatedNotificationListener.php @@ -11,6 +11,8 @@ use MintyPHP\Service\Access\PermissionService; final class UserCreatedNotificationListener implements EventListener { + use NotificationListenerTrait; + public function __construct( private readonly NotificationService $notificationService, private readonly UserReadRepositoryInterface $userReadRepository, @@ -33,17 +35,13 @@ final class UserCreatedNotificationListener implements EventListener return; } - $displayName = trim((string) ($user['display_name'] ?? '')); - if ($displayName === '') { - $displayName = trim(($user['first_name'] ?? '') . ' ' . ($user['last_name'] ?? '')); - } + $displayName = $this->resolveDisplayName($payload, $user, $userId); $tenantIds = $this->userTenantRepository->listTenantIdsByUserId($userId); if ($tenantIds === []) { return; } - // Only notify users who have the users.create permission (admins) $adminUserIds = $this->userReadRepository->listPrivilegedUserIdsByPermissionKeys( [PermissionService::USERS_CREATE] ); diff --git a/modules/notifications/lib/Module/Notifications/Listeners/UserDeactivatedNotificationListener.php b/modules/notifications/lib/Module/Notifications/Listeners/UserDeactivatedNotificationListener.php index e8a5f8b..e26c754 100644 --- a/modules/notifications/lib/Module/Notifications/Listeners/UserDeactivatedNotificationListener.php +++ b/modules/notifications/lib/Module/Notifications/Listeners/UserDeactivatedNotificationListener.php @@ -11,6 +11,8 @@ use MintyPHP\Service\Access\PermissionService; final class UserDeactivatedNotificationListener implements EventListener { + use NotificationListenerTrait; + public function __construct( private readonly NotificationService $notificationService, private readonly UserReadRepositoryInterface $userReadRepository, @@ -26,21 +28,8 @@ final class UserDeactivatedNotificationListener implements EventListener } $user = $this->userReadRepository->find($userId); - $displayName = trim((string) ($payload['display_name'] ?? '')); - if ($displayName === '' && is_array($user)) { - $displayName = trim((string) ($user['display_name'] ?? '')); - if ($displayName === '') { - $displayName = trim((string) (($user['first_name'] ?? '') . ' ' . ($user['last_name'] ?? ''))); - } - } - if ($displayName === '') { - $displayName = '#' . $userId; - } - - $uuid = trim((string) ($payload['uuid'] ?? '')); - if ($uuid === '' && is_array($user)) { - $uuid = trim((string) ($user['uuid'] ?? '')); - } + $displayName = $this->resolveDisplayName($payload, $user, $userId); + $uuid = $this->resolveUuid($payload, $user); $tenantIds = $this->sanitizeTenantIds($payload['tenant_ids'] ?? []); if ($tenantIds === []) { @@ -79,17 +68,4 @@ final class UserDeactivatedNotificationListener implements EventListener ); } } - - /** - * @return list - */ - private function sanitizeTenantIds(mixed $tenantIds): array - { - if (!is_array($tenantIds)) { - return []; - } - - $ids = array_values(array_unique(array_map('intval', $tenantIds))); - return array_values(array_filter($ids, static fn (int $tenantId): bool => $tenantId > 0)); - } } diff --git a/modules/notifications/lib/Module/Notifications/Listeners/UserDeletedNotificationListener.php b/modules/notifications/lib/Module/Notifications/Listeners/UserDeletedNotificationListener.php index b4c7397..052650e 100644 --- a/modules/notifications/lib/Module/Notifications/Listeners/UserDeletedNotificationListener.php +++ b/modules/notifications/lib/Module/Notifications/Listeners/UserDeletedNotificationListener.php @@ -10,6 +10,8 @@ use MintyPHP\Service\Access\PermissionService; final class UserDeletedNotificationListener implements EventListener { + use NotificationListenerTrait; + public function __construct( private readonly NotificationService $notificationService, private readonly UserReadRepositoryInterface $userReadRepository diff --git a/modules/notifications/lib/Module/Notifications/NotificationsContainerRegistrar.php b/modules/notifications/lib/Module/Notifications/NotificationsContainerRegistrar.php index c642263..dfeebbb 100644 --- a/modules/notifications/lib/Module/Notifications/NotificationsContainerRegistrar.php +++ b/modules/notifications/lib/Module/Notifications/NotificationsContainerRegistrar.php @@ -10,9 +10,8 @@ use MintyPHP\Module\Notifications\Listeners\UserAssignmentChangedNotificationLis use MintyPHP\Module\Notifications\Listeners\UserCreatedNotificationListener; use MintyPHP\Module\Notifications\Listeners\UserDeactivatedNotificationListener; use MintyPHP\Module\Notifications\Listeners\UserDeletedNotificationListener; -use MintyPHP\Module\Notifications\Service\NotificationRepositoryFactory; +use MintyPHP\Module\Notifications\Repository\NotificationRepository; use MintyPHP\Module\Notifications\Service\NotificationService; -use MintyPHP\Module\Notifications\Service\NotificationServicesFactory; use MintyPHP\Repository\Tenant\UserTenantRepository; use MintyPHP\Repository\User\UserReadRepository; use MintyPHP\Service\Access\PermissionService; @@ -24,13 +23,11 @@ final class NotificationsContainerRegistrar implements ContainerRegistrar $container->set(NotificationsAuthorizationPolicy::class, static fn (AppContainer $c): NotificationsAuthorizationPolicy => new NotificationsAuthorizationPolicy( $c->get(PermissionService::class) )); - $container->set(NotificationRepositoryFactory::class, static fn (): NotificationRepositoryFactory => new NotificationRepositoryFactory()); - $container->set(NotificationServicesFactory::class, static fn (AppContainer $c): NotificationServicesFactory => new NotificationServicesFactory( - $c->get(NotificationRepositoryFactory::class) - )); + $container->set(NotificationRepository::class, static fn (): NotificationRepository => new NotificationRepository()); - $container->set(NotificationService::class, static fn (AppContainer $c): NotificationService => $c->get(NotificationServicesFactory::class)->createNotificationService( + $container->set(NotificationService::class, static fn (AppContainer $c): NotificationService => new NotificationService( + $c->get(NotificationRepository::class), $c->get(UserTenantRepository::class) )); diff --git a/modules/notifications/lib/Module/Notifications/Repository/NotificationRepository.php b/modules/notifications/lib/Module/Notifications/Repository/NotificationRepository.php index 853dbfd..fbdf497 100644 --- a/modules/notifications/lib/Module/Notifications/Repository/NotificationRepository.php +++ b/modules/notifications/lib/Module/Notifications/Repository/NotificationRepository.php @@ -5,7 +5,7 @@ namespace MintyPHP\Module\Notifications\Repository; use MintyPHP\DB; use MintyPHP\Repository\Support\RepositoryArrayHelper; -class NotificationRepository implements NotificationRepositoryInterface +class NotificationRepository { private const DEDUPE_WINDOW_SECONDS = 1800; diff --git a/modules/notifications/lib/Module/Notifications/Repository/NotificationRepositoryInterface.php b/modules/notifications/lib/Module/Notifications/Repository/NotificationRepositoryInterface.php deleted file mode 100644 index 1f4d7b8..0000000 --- a/modules/notifications/lib/Module/Notifications/Repository/NotificationRepositoryInterface.php +++ /dev/null @@ -1,26 +0,0 @@ -> - */ - public function listByUser(int $userId, ?int $tenantId, int $limit, int $offset): array; - - public function countUnreadByUser(int $userId, ?int $tenantId): int; - - public function markRead(int $id, int $userId, ?int $tenantId): bool; - - public function markAllReadByUser(int $userId, ?int $tenantId): int; - - public function delete(int $id, int $userId, ?int $tenantId): bool; - - public function deleteAllByUser(int $userId): int; - - public function purgeReadOlderThanDays(int $days): int; -} diff --git a/modules/notifications/lib/Module/Notifications/Service/NotificationRepositoryFactory.php b/modules/notifications/lib/Module/Notifications/Service/NotificationRepositoryFactory.php deleted file mode 100644 index 32948f7..0000000 --- a/modules/notifications/lib/Module/Notifications/Service/NotificationRepositoryFactory.php +++ /dev/null @@ -1,16 +0,0 @@ -notificationRepository ??= new NotificationRepository(); - } -} diff --git a/modules/notifications/lib/Module/Notifications/Service/NotificationService.php b/modules/notifications/lib/Module/Notifications/Service/NotificationService.php index 3330df6..d59d96a 100644 --- a/modules/notifications/lib/Module/Notifications/Service/NotificationService.php +++ b/modules/notifications/lib/Module/Notifications/Service/NotificationService.php @@ -2,7 +2,7 @@ namespace MintyPHP\Module\Notifications\Service; -use MintyPHP\Module\Notifications\Repository\NotificationRepositoryInterface; +use MintyPHP\Module\Notifications\Repository\NotificationRepository; use MintyPHP\Repository\Tenant\UserTenantRepositoryInterface; class NotificationService @@ -18,7 +18,7 @@ class NotificationService ]; public function __construct( - private readonly NotificationRepositoryInterface $notificationRepository, + private readonly NotificationRepository $notificationRepository, private readonly UserTenantRepositoryInterface $userTenantRepository ) { } diff --git a/modules/notifications/lib/Module/Notifications/Service/NotificationServicesFactory.php b/modules/notifications/lib/Module/Notifications/Service/NotificationServicesFactory.php deleted file mode 100644 index 31db294..0000000 --- a/modules/notifications/lib/Module/Notifications/Service/NotificationServicesFactory.php +++ /dev/null @@ -1,23 +0,0 @@ -notificationService ??= new NotificationService( - $this->notificationRepositoryFactory->createNotificationRepository(), - $userTenantRepository - ); - } -} diff --git a/modules/notifications/tests/Module/Notifications/Service/NotificationServiceTest.php b/modules/notifications/tests/Module/Notifications/Service/NotificationServiceTest.php index 0039333..11a3861 100644 --- a/modules/notifications/tests/Module/Notifications/Service/NotificationServiceTest.php +++ b/modules/notifications/tests/Module/Notifications/Service/NotificationServiceTest.php @@ -2,7 +2,7 @@ namespace MintyPHP\Tests\Module\Notifications\Service; -use MintyPHP\Module\Notifications\Repository\NotificationRepositoryInterface; +use MintyPHP\Module\Notifications\Repository\NotificationRepository; use MintyPHP\Module\Notifications\Service\NotificationMessage; use MintyPHP\Module\Notifications\Service\NotificationService; use MintyPHP\Repository\Tenant\UserTenantRepositoryInterface; @@ -11,13 +11,13 @@ use PHPUnit\Framework\TestCase; class NotificationServiceTest extends TestCase { - private NotificationRepositoryInterface&MockObject $notifRepo; + private NotificationRepository&MockObject $notifRepo; private UserTenantRepositoryInterface&MockObject $utRepo; private NotificationService $service; protected function setUp(): void { - $this->notifRepo = $this->createMock(NotificationRepositoryInterface::class); + $this->notifRepo = $this->createMock(NotificationRepository::class); $this->utRepo = $this->createMock(UserTenantRepositoryInterface::class); $this->service = new NotificationService($this->notifRepo, $this->utRepo); }