Merge pull request 'fix: remove duplicate old() function causing 500 on create page' (#58) from feat/dashboard-aggregated-chart into master

Reviewed-on: #58

AGENTS.md

@@ -38,6 +38,7 @@ Repo-specific guidance for OpenCode sessions. Verified against code.
 - Admin group routes (`/admin/*`) already include `AuthMiddleware` + `RoleMiddleware`; individual routes add `CSRFMiddleware`.
 - Rate limiting on `POST /login` only (via `RateLimitMiddleware`)
 - Every credential access must be logged at `warning` level via `AuditService`
+- Notification read receipts tracked in `notification_reads` table: `(notification_id, user_id)` unique pair, with `read_at` timestamp
 
 ## Permission validation on server create/edit
 - When creating or editing a server, required SSH permissions can be specified and are validated via SSH before saving.
@@ -49,7 +50,7 @@ Repo-specific guidance for OpenCode sessions. Verified against code.
 
 ## Database
 - Migrations run manually: `mysql servermanager < database/migrations/NNN_name.sql`
-- 7 migrations: `001_initial_schema`, `002_server_access`, `003_teams`, `004_soft_deletes`, `005_app_config_and_notifications`, `006_agent_tokens`, `007_server_permission_checks`
+- 8 migrations: `001_initial_schema`, `002_server_access`, `003_teams`, `004_soft_deletes`, `005_app_config_and_notifications`, `006_agent_tokens`, `007_server_permission_checks`, `008_notification_reads`
 - Users table has `role` ENUM: `super_admin`, `admin`, `operator`
 - `api_token` column on users for Bearer token auth
 - `server_permissions` table stores required SSH permissions per server (FK to `servers.id` CASCADE)

database/migrations/008_notification_reads.sql

@@ -0,0 +1,18 @@
+CREATE TABLE IF NOT EXISTS `notification_reads` (
+    `id`              INT UNSIGNED NOT NULL AUTO_INCREMENT,
+    `notification_id` INT UNSIGNED NOT NULL,
+    `user_id`         INT UNSIGNED NOT NULL,
+    `read_at`         DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
+    PRIMARY KEY (`id`),
+    UNIQUE KEY `idx_notification_user` (`notification_id`, `user_id`),
+    KEY `idx_notification_id` (`notification_id`),
+    KEY `idx_user_id` (`user_id`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
+
+-- Migrate existing read receipts for user-targeted notifications
+INSERT IGNORE INTO `notification_reads` (`notification_id`, `user_id`, `read_at`)
+SELECT n.id, u.id, COALESCE(n.read_at, n.created_at)
+FROM notifications n
+JOIN users u ON u.id = n.user_id AND u.status = 'active'
+WHERE n.is_read = 1
+  AND n.user_id IS NOT NULL;

public/assets/css/style.css

@@ -1008,6 +1008,7 @@ textarea.form-input {
 .badge-warning { background: var(--warning-bg); color: var(--warning); }
 .badge-info { background: var(--info-bg); color: var(--info); }
 .badge-purple { background: var(--purple-bg); color: var(--purple); }
+.badge-secondary { background: rgba(156, 163, 175, 0.15); color: #9ca3af; }
 
 /* ==========================================================================
    Mini Progress

src/Controllers/AdminController.php

@@ -255,10 +255,13 @@ class AdminController
         $userModel = new User();
         $users = $userModel->getAll(1, 200);
 
+        $totalActiveUsers = $notificationModel->getTotalActiveUsers();
+
         $this->view->display('admin.notifications', [
             'title' => 'Notifications - ServerManager',
             'notifications' => $allNotifications,
             'users' => $users,
+            'totalActiveUsers' => $totalActiveUsers,
         ]);
     }
 

src/Models/Notification.php

@@ -29,15 +29,29 @@ class Notification
     public function getForUser(int $userId, int $page = 1, int $perPage = 20): array
     {
         $offset = ($page - 1) * $perPage;
+
         $total = $this->db->fetch(
-            "SELECT COUNT(*) as total FROM notifications WHERE user_id IS NULL OR user_id = ?",
+            "SELECT COUNT(*) as total FROM notifications n
+             WHERE (n.user_id IS NULL OR n.user_id = ?)",
             [$userId]
         );
+
         $items = $this->db->fetchAll(
-            "SELECT * FROM notifications WHERE user_id IS NULL OR user_id = ?
+            "SELECT n.*,
-             ORDER BY created_at DESC LIMIT ? OFFSET ?",
+                    CASE
-            [$userId, $perPage, $offset]
+                        WHEN nr.id IS NOT NULL THEN 1
+                        ELSE 0
+                    END as is_read,
+                    nr.read_at as read_at,
+                    (SELECT COUNT(*) FROM notification_reads WHERE notification_id = n.id) as total_reads,
+                    (SELECT COUNT(*) FROM users WHERE status = 'active') as total_users
+             FROM notifications n
+             LEFT JOIN notification_reads nr ON nr.notification_id = n.id AND nr.user_id = ?
+             WHERE n.user_id IS NULL OR n.user_id = ?
+             ORDER BY n.created_at DESC LIMIT ? OFFSET ?",
+            [$userId, $userId, $perPage, $offset]
         );
+
         return [
             'data' => $items,
             'total' => (int) ($total['total'] ?? 0),
@@ -50,15 +64,29 @@ class Notification
     public function getUnreadCount(int $userId): int
     {
         $result = $this->db->fetch(
-            "SELECT COUNT(*) as total FROM notifications
+            "SELECT COUNT(*) as total FROM notifications n
-             WHERE (user_id IS NULL OR user_id = ?) AND is_read = 0",
+             WHERE (n.user_id IS NULL OR n.user_id = ?)
-            [$userId]
+             AND NOT EXISTS (
+                 SELECT 1 FROM notification_reads nr
+                 WHERE nr.notification_id = n.id AND nr.user_id = ?
+             )",
+            [$userId, $userId]
         );
         return (int) ($result['total'] ?? 0);
     }
 
     public function markAsRead(int $id, int $userId): void
     {
+        try {
+            $this->db->insert('notification_reads', [
+                'notification_id' => $id,
+                'user_id' => $userId,
+                'read_at' => date('Y-m-d H:i:s'),
+            ]);
+        } catch (\Throwable $e) {
+            // Already read, ignore
+        }
+
         $this->db->update(
             'notifications',
             ['is_read' => 1, 'read_at' => date('Y-m-d H:i:s')],
@@ -69,9 +97,32 @@ class Notification
 
     public function markAllAsRead(int $userId): void
     {
+        $unread = $this->db->fetchAll(
+            "SELECT n.id FROM notifications n
+             WHERE (n.user_id IS NULL OR n.user_id = ?)
+             AND NOT EXISTS (
+                 SELECT 1 FROM notification_reads nr
+                 WHERE nr.notification_id = n.id AND nr.user_id = ?
+             )",
+            [$userId, $userId]
+        );
+
+        $now = date('Y-m-d H:i:s');
+        foreach ($unread as $note) {
+            try {
+                $this->db->insert('notification_reads', [
+                    'notification_id' => (int) $note['id'],
+                    'user_id' => $userId,
+                    'read_at' => $now,
+                ]);
+            } catch (\Throwable $e) {
+                // Duplicate entry — already read, skip
+            }
+        }
+
         $this->db->update(
             'notifications',
-            ['is_read' => 1, 'read_at' => date('Y-m-d H:i:s')],
+            ['is_read' => 1, 'read_at' => $now],
             '(user_id IS NULL OR user_id = ?) AND is_read = 0',
             [$userId]
         );
@@ -80,14 +131,18 @@ class Notification
     public function getAll(int $page = 1, int $perPage = 20): array
     {
         $offset = ($page - 1) * $perPage;
+
         $total = $this->db->fetch("SELECT COUNT(*) as total FROM notifications");
+
         $items = $this->db->fetchAll(
-            "SELECT n.*, u.username as target_username
+            "SELECT n.*, u.username as target_username,
+                    (SELECT COUNT(*) FROM notification_reads nr WHERE nr.notification_id = n.id) as read_count
              FROM notifications n
              LEFT JOIN users u ON n.user_id = u.id
              ORDER BY n.created_at DESC LIMIT ? OFFSET ?",
             [$perPage, $offset]
         );
+
         return [
             'data' => $items,
             'total' => (int) ($total['total'] ?? 0),
@@ -96,4 +151,10 @@ class Notification
             'total_pages' => (int) ceil((int) ($total['total'] ?? 0) / $perPage),
         ];
     }
+
+    public function getTotalActiveUsers(): int
+    {
+        $result = $this->db->fetch("SELECT COUNT(*) as total FROM users WHERE status = 'active'");
+        return (int) ($result['total'] ?? 0);
+    }
 }

views/admin/notifications.php

@@ -1,6 +1,7 @@
 <?php
 $notifications = $notifications ?? [];
 $users = $users ?? [];
+$totalActiveUsers = $totalActiveUsers ?? 0;
 $data = $notifications['data'] ?? [];
 ?>
 
@@ -27,7 +28,7 @@ $data = $notifications['data'] ?? [];
                         <th>Title</th>
                         <th>Message</th>
                         <th>Target</th>
-                        <th>Status</th>
+                        <th>Read by</th>
                     </tr>
                 </thead>
                 <tbody>
@@ -53,9 +54,16 @@ $data = $notifications['data'] ?? [];
                                 <td><?= htmlspecialchars(mb_substr($note['message'] ?? '', 0, 80)) ?><?= mb_strlen($note['message'] ?? '') > 80 ? '...' : '' ?></td>
                                 <td><?= $note['user_id'] ? htmlspecialchars($note['target_username'] ?? "User #{$note['user_id']}") : '<em>All Users</em>' ?></td>
                                 <td>
-                                    <span class="status-badge <?= $note['is_read'] ? 'status-online' : 'status-offline' ?>">
+                                    <?php if ($note['user_id']): ?>
-                                        <?= $note['is_read'] ? 'Read' : 'Unread' ?>
+                                        <span class="status-badge <?= ($note['read_count'] ?? 0) > 0 ? 'status-online' : 'status-offline' ?>">
-                                    </span>
+                                            <?= ($note['read_count'] ?? 0) > 0 ? 'Read' : 'Unread' ?>
+                                        </span>
+                                    <?php else: ?>
+                                        <?php $rc = (int) ($note['read_count'] ?? 0); ?>
+                                        <span class="badge badge-<?= $rc >= $totalActiveUsers ? 'success' : ($rc > 0 ? 'warning' : 'secondary') ?>">
+                                            <?= $rc ?> / <?= $totalActiveUsers ?>
+                                        </span>
+                                    <?php endif; ?>
                                 </td>
                             </tr>
                         <?php endforeach; ?>

views/servers/create.php

@@ -1,9 +1,6 @@
 <?php
 $groups = $groups ?? [];
 $old = $oldInput ?? [];
-function old(string $key, string $default = ''): string {
-    return htmlspecialchars($GLOBALS['old'][$key] ?? $default);
-}
 ?>
 
 <div class="page-header">
@@ -23,13 +20,13 @@ function old(string $key, string $default = ''): string {
                         <label for="name">Server Name *</label>
                         <input type="text" id="name" name="name" class="form-input"
                                placeholder="e.g., Production Web Server" required
-                               value="<?= old('name') ?>">
+                               value="<?= htmlspecialchars($old['name'] ?? '') ?>">
                     </div>
                     <div class="form-group">
                         <label for="group_name">Group / Category</label>
                         <input type="text" id="group_name" name="group_name" class="form-input"
                                placeholder="e.g., Production, Staging"
-                               list="groupList" value="<?= old('group_name') ?>">
+                               list="groupList"                                value="<?= htmlspecialchars($old['group_name'] ?? '') ?>">
                         <datalist id="groupList">
                             <?php foreach ($groups as $group): ?>
                                 <option value="<?= htmlspecialchars($group) ?>">
@@ -40,7 +37,7 @@ function old(string $key, string $default = ''): string {
                 <div class="form-group">
                     <label for="description">Description</label>
                     <textarea id="description" name="description" class="form-input" rows="3"
-                              placeholder="Optional description of the server"><?= old('description') ?></textarea>
+                              placeholder="Optional description of the server"><?= htmlspecialchars($old['description'] ?? '') ?></textarea>
                 </div>
             </div>
 
@@ -51,18 +48,18 @@ function old(string $key, string $default = ''): string {
                         <label for="ip_address">IP Address / Hostname *</label>
                         <input type="text" id="ip_address" name="ip_address" class="form-input"
                                placeholder="e.g., 192.168.1.100 or server.example.com" required
-                               value="<?= old('ip_address') ?>">
+                               value="<?= htmlspecialchars($old['ip_address'] ?? '') ?>">
                     </div>
                     <div class="form-group">
                         <label for="ssh_port">SSH Port *</label>
                         <input type="number" id="ssh_port" name="ssh_port" class="form-input"
-                               value="<?= old('ssh_port', '22') ?>" min="1" max="65535" required>
+                               value="<?= htmlspecialchars($old['ssh_port'] ?? '22') ?>" min="1" max="65535" required>
                     </div>
                     <div class="form-group">
                         <label for="ssh_user">SSH User *</label>
                         <input type="text" id="ssh_user" name="ssh_user" class="form-input"
                                placeholder="e.g., root or admin" required
-                               value="<?= old('ssh_user') ?>">
+                               value="<?= htmlspecialchars($old['ssh_user'] ?? '') ?>">
                     </div>
                 </div>
             </div>