diff --git a/lib/App/Module/Contracts/SessionProvider.php b/lib/App/Module/Contracts/SessionProvider.php index a49706f..c297702 100644 --- a/lib/App/Module/Contracts/SessionProvider.php +++ b/lib/App/Module/Contracts/SessionProvider.php @@ -22,5 +22,5 @@ interface SessionProvider /** * Remove session keys owned by this module (called on logout). */ - public function clear(): void; + public function clear(AppContainer $container): void; } diff --git a/lib/Service/Auth/AuthSessionTenantContextService.php b/lib/Service/Auth/AuthSessionTenantContextService.php index f87afe0..1658692 100644 --- a/lib/Service/Auth/AuthSessionTenantContextService.php +++ b/lib/Service/Auth/AuthSessionTenantContextService.php @@ -54,7 +54,7 @@ class AuthSessionTenantContextService public function clearModuleSessionData(): void { foreach ($this->moduleSessionProviders() as $provider) { - $provider->clear(); + $provider->clear($this->container); } } diff --git a/modules/addressbook/lib/Module/AddressBook/Providers/AddressBookSessionProvider.php b/modules/addressbook/lib/Module/AddressBook/Providers/AddressBookSessionProvider.php index 0a8e525..f09f983 100644 --- a/modules/addressbook/lib/Module/AddressBook/Providers/AddressBookSessionProvider.php +++ b/modules/addressbook/lib/Module/AddressBook/Providers/AddressBookSessionProvider.php @@ -15,15 +15,23 @@ final class AddressBookSessionProvider implements SessionProvider { public function populate(array $user, AppContainer $container): void { - $userId = (int) ($user['id'] ?? 0); - if ($userId <= 0) { - unset($_SESSION['module.addressbook.departments_by_tenant']); + $sessionStore = $container->get(SessionStoreInterface::class); + if (!$sessionStore instanceof SessionStoreInterface) { return; } - $tenantContext = $container->get(UserTenantContextService::class); - $sessionStore = $container->get(SessionStoreInterface::class); - if (!$tenantContext instanceof UserTenantContextService || !$sessionStore instanceof SessionStoreInterface) { + $userId = (int) ($user['id'] ?? 0); + if ($userId <= 0) { + $sessionStore->remove('module.addressbook.departments_by_tenant'); + return; + } + + try { + $tenantContext = $container->get(UserTenantContextService::class); + } catch (\Throwable) { + return; + } + if (!$tenantContext instanceof UserTenantContextService) { return; } @@ -33,8 +41,11 @@ final class AddressBookSessionProvider implements SessionProvider ); } - public function clear(): void + public function clear(AppContainer $container): void { - unset($_SESSION['module.addressbook.departments_by_tenant']); + $sessionStore = $container->get(SessionStoreInterface::class); + if ($sessionStore instanceof SessionStoreInterface) { + $sessionStore->remove('module.addressbook.departments_by_tenant'); + } } } diff --git a/modules/bookmarks/lib/Module/Bookmarks/Providers/BookmarksSessionProvider.php b/modules/bookmarks/lib/Module/Bookmarks/Providers/BookmarksSessionProvider.php index bcbea96..38b772e 100644 --- a/modules/bookmarks/lib/Module/Bookmarks/Providers/BookmarksSessionProvider.php +++ b/modules/bookmarks/lib/Module/Bookmarks/Providers/BookmarksSessionProvider.php @@ -13,23 +13,30 @@ final class BookmarksSessionProvider implements SessionProvider public function populate(array $user, AppContainer $container): void { - $userId = (int) ($user['id'] ?? 0); - if ($userId <= 0) { - unset($_SESSION[self::SESSION_KEY]); + $sessionStore = $container->get(SessionStoreInterface::class); + if (!$sessionStore instanceof SessionStoreInterface) { + return; + } + + $userId = (int) ($user['id'] ?? 0); + if ($userId <= 0) { + $sessionStore->remove(self::SESSION_KEY); return; } - $sessionStore = $container->get(SessionStoreInterface::class); $bookmarkService = $container->get(BookmarkService::class); - if (!$sessionStore instanceof SessionStoreInterface || !$bookmarkService instanceof BookmarkService) { + if (!$bookmarkService instanceof BookmarkService) { return; } $sessionStore->set(self::SESSION_KEY, $bookmarkService->listGroupedForUser($userId)); } - public function clear(): void + public function clear(AppContainer $container): void { - unset($_SESSION[self::SESSION_KEY]); + $sessionStore = $container->get(SessionStoreInterface::class); + if ($sessionStore instanceof SessionStoreInterface) { + $sessionStore->remove(self::SESSION_KEY); + } } } diff --git a/modules/notifications/i18n/default_de.json b/modules/notifications/i18n/default_de.json index c5002ad..1aa9fd7 100644 --- a/modules/notifications/i18n/default_de.json +++ b/modules/notifications/i18n/default_de.json @@ -5,6 +5,8 @@ "Mark as read": "Als gelesen markieren", "Dismiss": "Verwerfen", "Action failed": "Aktion fehlgeschlagen", + "Loading notifications…": "Benachrichtigungen werden geladen…", + "Failed to load notifications": "Benachrichtigungen konnten nicht geladen werden", "New user: %s": "Neuer Benutzer: %s", "User deleted: %s": "Benutzer gelöscht: %s", "User activated: %s": "Benutzer aktiviert: %s", diff --git a/modules/notifications/i18n/default_en.json b/modules/notifications/i18n/default_en.json index 5a2c464..76bcf93 100644 --- a/modules/notifications/i18n/default_en.json +++ b/modules/notifications/i18n/default_en.json @@ -5,6 +5,8 @@ "Mark as read": "Mark as read", "Dismiss": "Dismiss", "Action failed": "Action failed", + "Loading notifications…": "Loading notifications…", + "Failed to load notifications": "Failed to load notifications", "New user: %s": "New user: %s", "User deleted: %s": "User deleted: %s", "User activated: %s": "User activated: %s", diff --git a/modules/notifications/lib/Module/Notifications/Providers/NotificationsSessionProvider.php b/modules/notifications/lib/Module/Notifications/Providers/NotificationsSessionProvider.php index 324823b..f53fa1f 100644 --- a/modules/notifications/lib/Module/Notifications/Providers/NotificationsSessionProvider.php +++ b/modules/notifications/lib/Module/Notifications/Providers/NotificationsSessionProvider.php @@ -13,15 +13,19 @@ final class NotificationsSessionProvider implements SessionProvider public function populate(array $user, AppContainer $container): void { - $userId = (int) ($user['id'] ?? 0); - if ($userId <= 0) { - unset($_SESSION[self::SESSION_KEY]); + $sessionStore = $container->get(SessionStoreInterface::class); + if (!$sessionStore instanceof SessionStoreInterface) { + return; + } + + $userId = (int) ($user['id'] ?? 0); + if ($userId <= 0) { + $sessionStore->remove(self::SESSION_KEY); return; } - $sessionStore = $container->get(SessionStoreInterface::class); $service = $container->get(NotificationService::class); - if (!$sessionStore instanceof SessionStoreInterface || !$service instanceof NotificationService) { + if (!$service instanceof NotificationService) { return; } @@ -30,8 +34,11 @@ final class NotificationsSessionProvider implements SessionProvider $sessionStore->set(self::SESSION_KEY, $service->unreadCount($userId, $tenantId > 0 ? $tenantId : null)); } - public function clear(): void + public function clear(AppContainer $container): void { - unset($_SESSION[self::SESSION_KEY]); + $sessionStore = $container->get(SessionStoreInterface::class); + if ($sessionStore instanceof SessionStoreInterface) { + $sessionStore->remove(self::SESSION_KEY); + } } } diff --git a/modules/notifications/templates/topbar-notification-bell.phtml b/modules/notifications/templates/topbar-notification-bell.phtml index 9edaba0..9b7e588 100644 --- a/modules/notifications/templates/topbar-notification-bell.phtml +++ b/modules/notifications/templates/topbar-notification-bell.phtml @@ -26,6 +26,14 @@ $unreadCount = (int) ($notifNav['unread_count'] ?? 0);
  • + + t('No notifications'), diff --git a/modules/notifications/web/css/components/app-notification-dropdown.css b/modules/notifications/web/css/components/app-notification-dropdown.css index 53cd520..f471e13 100644 --- a/modules/notifications/web/css/components/app-notification-dropdown.css +++ b/modules/notifications/web/css/components/app-notification-dropdown.css @@ -156,4 +156,33 @@ color: var(--app-notification-text); background: var(--app-notification-border); } + + .app-notification-loading, + .app-notification-error { + display: flex; + align-items: center; + justify-content: center; + gap: 8px; + padding: 24px 16px; + font-size: 13px; + color: var(--app-notification-muted); + } + + .app-notification-error { + color: var(--app-danger, #c0392b); + } + + .app-notification-loading__spinner { + display: inline-block; + width: 16px; + height: 16px; + border: 2px solid var(--app-notification-border); + border-top-color: var(--app-primary); + border-radius: 50%; + animation: app-notification-spin 0.6s linear infinite; + } + + @keyframes app-notification-spin { + to { transform: rotate(360deg); } + } } diff --git a/modules/notifications/web/js/components/app-notification-bell.js b/modules/notifications/web/js/components/app-notification-bell.js index d279ebd..6b7bc28 100644 --- a/modules/notifications/web/js/components/app-notification-bell.js +++ b/modules/notifications/web/js/components/app-notification-bell.js @@ -59,6 +59,8 @@ export function initNotificationBell(root = document, config = {}) { const badge = details.querySelector('[data-notification-badge]'); const list = dropdown.querySelector('[data-notification-list]'); const emptyEl = list?.querySelector('.app-empty-state'); + const loadingEl = list?.querySelector('[data-notification-loading]'); + const errorEl = list?.querySelector('[data-notification-error]'); const markAllBtn = dropdown.querySelector('[data-notification-mark-all]'); const ac = new AbortController(); @@ -77,18 +79,25 @@ export function initNotificationBell(root = document, config = {}) { } } + // --- State management --- + function showState(state) { + if (loadingEl) loadingEl.style.display = state === 'loading' ? '' : 'none'; + if (errorEl) errorEl.style.display = state === 'error' ? '' : 'none'; + if (emptyEl) emptyEl.style.display = state === 'empty' ? '' : 'none'; + } + // --- Render --- function renderNotifications(notifications) { if (!list) return; - // Remove rendered items, keep empty state partial + // Remove rendered items, keep state elements list.querySelectorAll('.app-notification-item').forEach(el => el.remove()); if (notifications.length === 0) { - if (emptyEl) emptyEl.style.display = ''; + showState('empty'); return; } - if (emptyEl) emptyEl.style.display = 'none'; + showState('success'); const frag = document.createDocumentFragment(); for (const n of notifications) { @@ -161,14 +170,19 @@ export function initNotificationBell(root = document, config = {}) { // --- API --- async function loadNotifications() { + showState('loading'); + list?.querySelectorAll('.app-notification-item').forEach(el => el.remove()); try { const data = await fetchJson(endpoint('notifications/list-data'), { signal }); if (data.ok) { renderNotifications(data.notifications || []); updateBadge(data.unread_count ?? 0); + } else { + showState('error'); } } catch (err) { if (err instanceof DOMException && err.name === 'AbortError') return; + showState('error'); telemetry.capture('warn_once', { message: 'Notification bell: load failed', meta: { module: 'notifications', component: 'notification-bell', action: 'load' }, diff --git a/tests/Service/Auth/AuthSessionTenantContextServiceTest.php b/tests/Service/Auth/AuthSessionTenantContextServiceTest.php index 34741f1..6575763 100644 --- a/tests/Service/Auth/AuthSessionTenantContextServiceTest.php +++ b/tests/Service/Auth/AuthSessionTenantContextServiceTest.php @@ -144,7 +144,7 @@ final class DummyAuthSessionProvider implements SessionProvider $_SESSION['dummy_provider_user_id'] = (int) ($user['id'] ?? 0); } - public function clear(): void + public function clear(AppContainer $container): void { self::$clearCalls++; unset($_SESSION['dummy_provider_user_id']); diff --git a/tests/Unit/Module/SessionProviderTest.php b/tests/Unit/Module/SessionProviderTest.php index d4e1995..1566d0f 100644 --- a/tests/Unit/Module/SessionProviderTest.php +++ b/tests/Unit/Module/SessionProviderTest.php @@ -3,6 +3,8 @@ namespace MintyPHP\Tests\Unit\Module; use MintyPHP\App\AppContainer; +use MintyPHP\Http\SessionStore; +use MintyPHP\Http\SessionStoreInterface; use MintyPHP\Module\AddressBook\Providers\AddressBookSessionProvider; use PHPUnit\Framework\TestCase; @@ -21,13 +23,11 @@ final class SessionProviderTest extends TestCase $_SESSION = []; } - /** - * Create a real AppContainer that returns null for any get() call. - * AppContainer is final and cannot be mocked. - */ - private function emptyContainer(): AppContainer + private function containerWithSessionStore(): AppContainer { - return new AppContainer(); + $container = new AppContainer(); + $container->set(SessionStoreInterface::class, static fn (): SessionStore => new SessionStore()); + return $container; } public function testPopulateWithInvalidUserClearsSessionKey(): void @@ -35,7 +35,7 @@ final class SessionProviderTest extends TestCase $_SESSION['module.addressbook.departments_by_tenant'] = [['tenant' => 'old']]; $provider = new AddressBookSessionProvider(); - $provider->populate(['id' => 0], $this->emptyContainer()); + $provider->populate(['id' => 0], $this->containerWithSessionStore()); self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION); } @@ -45,22 +45,17 @@ final class SessionProviderTest extends TestCase $_SESSION['module.addressbook.departments_by_tenant'] = [['tenant' => 'old']]; $provider = new AddressBookSessionProvider(); - $provider->populate([], $this->emptyContainer()); + $provider->populate([], $this->containerWithSessionStore()); self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION); } public function testPopulateReturnsEarlyWhenContainerMissesDependencies(): void { + $container = $this->containerWithSessionStore(); $provider = new AddressBookSessionProvider(); - // Container has no bindings → get() will throw, but provider guards with instanceof check - // The provider should handle this gracefully (no session key set) - try { - $provider->populate(['id' => 42], $this->emptyContainer()); - } catch (\Throwable) { - // Provider may throw if container has no binding — that's acceptable; - // in production, bindings are always registered before providers run. - } + // Container has SessionStore but no UserTenantContextService → provider returns early + $provider->populate(['id' => 42], $container); self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION); } @@ -72,7 +67,7 @@ final class SessionProviderTest extends TestCase ]; $provider = new AddressBookSessionProvider(); - $provider->clear(); + $provider->clear($this->containerWithSessionStore()); self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION); } @@ -81,7 +76,7 @@ final class SessionProviderTest extends TestCase { // Key doesn't exist — clear() should not throw $provider = new AddressBookSessionProvider(); - $provider->clear(); + $provider->clear($this->containerWithSessionStore()); self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION); }