From 15cf4672b233a9a3fbe0c55bccdbaba9544b2240 Mon Sep 17 00:00:00 2001
From: Pulse Monitor <pulse@pulse-monitor.dev>
Date: Fri, 29 Aug 2025 21:41:15 +0000
Subject: [PATCH] fix: restore backup table columns and fix PBS storage node
 display

- restored Node, Owner, and Location columns to backups table
- fixed issue where PBS backups via Proxmox storage showed storage name in Node column
- now correctly shows actual Proxmox node in Node column for all backup types
- Location column shows storage/datastore name without duplication
---
 .../src/components/Backups/UnifiedBackups.tsx | 55 +++++++++++++------
 internal/monitoring/monitor.go                |  6 +-
 2 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/frontend-modern/src/components/Backups/UnifiedBackups.tsx b/frontend-modern/src/components/Backups/UnifiedBackups.tsx
index da73d26ee..59ed15df1 100644
--- a/frontend-modern/src/components/Backups/UnifiedBackups.tsx
+++ b/frontend-modern/src/components/Backups/UnifiedBackups.tsx
@@ -254,20 +254,22 @@ const UnifiedBackups: Component = () => {
         displayType = 'LXC'; // Default fallback (most people have more containers than VMs)
       }
       
+      // For PBS backups through storage: show Proxmox node in Node column, PBS storage in Location
+      // For regular backups: show Proxmox node in Node column, local storage in Location
       unified.push({
         backupType: backupType,
         vmid: backup.vmid || 0,
         name: backup.notes || backup.volid?.split('/').pop() || '',
         type: displayType,
-        node: backup.node || '',
+        node: backup.node || '',  // Proxmox node that has access to this backup
         backupTime: backup.ctime || 0,
         backupName: backup.volid?.split('/').pop() || '',
         description: backup.notes || '', // Use notes field for PBS backup descriptions
         status: backup.verified ? 'verified' : 'unverified',
         size: backup.size || null,
-        storage: backup.storage || null,
-        datastore: backup.isPBS ? backup.storage : null,
-        namespace: backup.isPBS ? 'root' : null,
+        storage: backup.storage || null,  // Storage name (PBS storage or local storage)
+        datastore: null,  // Only set for direct PBS API backups
+        namespace: null,  // Only set for direct PBS API backups
         verified: backup.verified || false,
         protected: backup.protected || false,
         encrypted: backup.encryption ? true : false  // Check encryption field from Proxmox API
@@ -1603,6 +1605,22 @@ const UnifiedBackups: Component = () => {
                 >
                   Name {sortKey() === 'name' && (sortDirection() === 'asc' ? '▲' : '▼')}
                 </th>
+                <th
+                  class="px-2 py-1.5 text-left text-[11px] sm:text-xs font-medium uppercase tracking-wider cursor-pointer hover:bg-gray-200 dark:hover:bg-gray-600"
+                  onClick={() => handleSort('node')}
+                  style="width: 100px;"
+                >
+                  Node {sortKey() === 'node' && (sortDirection() === 'asc' ? '▲' : '▼')}
+                </th>
+                <Show when={backupTypeFilter() === 'all' || backupTypeFilter() === 'remote'}>
+                  <th
+                    class="px-2 py-1.5 text-left text-[11px] sm:text-xs font-medium uppercase tracking-wider cursor-pointer hover:bg-gray-200 dark:hover:bg-gray-600"
+                    onClick={() => handleSort('owner')}
+                    style="width: 80px;"
+                  >
+                    Owner {sortKey() === 'owner' && (sortDirection() === 'asc' ? '▲' : '▼')}
+                  </th>
+                </Show>
                 <th
                   class="px-2 py-1.5 text-left text-[11px] sm:text-xs font-medium uppercase tracking-wider cursor-pointer hover:bg-gray-200 dark:hover:bg-gray-600"
                   onClick={() => handleSort('backupTime')}
@@ -1655,7 +1673,8 @@ const UnifiedBackups: Component = () => {
                   <>
                     <tr class="bg-gray-50/50 dark:bg-gray-700/30">
                       <td colspan={(() => {
-                        let cols = 6; // Base columns: Name, Type, VMID, Time, Backup, Details
+                        let cols = 7; // Base columns: VMID, Type, Name, Node, Time, Backup, Details
+                        if (backupTypeFilter() === 'all' || backupTypeFilter() === 'remote') cols++; // Add Owner column
                         if (backupTypeFilter() !== 'snapshot') cols++; // Add Size column
                         if (backupTypeFilter() === 'all' || backupTypeFilter() === 'remote') cols++; // Add Verified column
                         if (backupTypeFilter() !== 'snapshot') cols++; // Add Location column
@@ -1682,6 +1701,14 @@ const UnifiedBackups: Component = () => {
                           <td class="p-0.5 px-1.5 text-sm align-middle">
                             {item.name || '-'}
                           </td>
+                          <td class="p-0.5 px-1.5 text-sm align-middle">
+                            {item.node}
+                          </td>
+                          <Show when={backupTypeFilter() === 'all' || backupTypeFilter() === 'remote'}>
+                            <td class="p-0.5 px-1.5 text-xs align-middle text-gray-500 dark:text-gray-400">
+                              {item.owner ? item.owner.split('@')[0] : '-'}
+                            </td>
+                          </Show>
                           <td class={`p-0.5 px-1.5 text-xs align-middle ${getAgeColorClass(item.backupTime)}`}>
                             {formatTime(item.backupTime * 1000)}
                           </td>
@@ -1719,19 +1746,11 @@ const UnifiedBackups: Component = () => {
                           </td>
                           <Show when={backupTypeFilter() !== 'snapshot'}>
                             <td class="p-0.5 px-1.5 text-sm align-middle">
-                              {(() => {
-                                let location = item.storage || (item.datastore && (
-                                  item.namespace && item.namespace !== 'root'
-                                    ? `${item.datastore}/${item.namespace}`
-                                    : item.datastore
-                                )) || '-';
-                                
-                                // Prepend node name for clarity (unless it's already part of the location)
-                                if (item.node && location !== '-' && !location.includes(item.node)) {
-                                  return `${item.node}:${location}`;
-                                }
-                                return location;
-                              })()}
+                              {item.storage || (item.datastore && (
+                                item.namespace && item.namespace !== 'root'
+                                  ? `${item.datastore}/${item.namespace}`
+                                  : item.datastore
+                              )) || '-'}
                             </td>
                           </Show>
                           <Show when={backupTypeFilter() === 'all' || backupTypeFilter() === 'remote'}>
diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go
index ebc579e4c..7222f92cb 100644
--- a/internal/monitoring/monitor.go
+++ b/internal/monitoring/monitor.go
@@ -2289,13 +2289,9 @@ func (m *Monitor) pollStorageBackupsWithNodes(ctx context.Context, instanceName
 					backupType = "host"
 				}
 
-				// For shared storage (like PBS), use the storage name as node
-				// to avoid confusion about which node the backup is on
+				// Always use the actual node name
 				backupNode := node.Node
 				isPBSStorage := strings.HasPrefix(storage.Storage, "pbs-") || storage.Type == "pbs"
-				if isPBSStorage || storage.Shared == 1 {
-					backupNode = storage.Storage // Use storage name for shared storage
-				}
 
 				// Check verification status for PBS backups
 				verified := false