From 17a1df8ec887d36d2d4a296210af530c551d6305 Mon Sep 17 00:00:00 2001
From: "Joseph.Rawlings" <root0091@gmail.com>
Date: Thu, 4 Dec 2025 14:03:38 -0500
Subject: [PATCH] refactor: enhance error handling and streamline rule
 management in Proxmox firewall component for improved user experience

---
 .../Servers/Proxmox/Livewire/Firewall.php     |  58 ++++---
 .../Servers/Proxmox/views/firewall.blade.php  | 162 +++++-------------
 2 files changed, 76 insertions(+), 144 deletions(-)

diff --git a/extensions/Servers/Proxmox/Livewire/Firewall.php b/extensions/Servers/Proxmox/Livewire/Firewall.php
index 5f4ef593b..a2ca45e3c 100644
--- a/extensions/Servers/Proxmox/Livewire/Firewall.php
+++ b/extensions/Servers/Proxmox/Livewire/Firewall.php
@@ -245,22 +245,27 @@ class Firewall extends Component
 
     public function startEditRule(int $index): void
     {
-        if (isset($this->rules[$index])) {
-            $rule = $this->rules[$index];
-            $this->editingRulePos = $rule['pos'];
-            $this->showRuleForm = true;
-            $this->ruleAction = $rule['action'] ?? 'ACCEPT';
-            $this->ruleSource = $rule['source'] ?? '';
-            $this->ruleDest = $rule['dest'] ?? '';
-            $this->ruleProto = $rule['proto'] ?? '';
-            $this->ruleDport = $rule['dport'] ?? '';
-            $this->ruleSport = $rule['sport'] ?? '';
-            $this->ruleComment = $rule['comment'] ?? '';
-            $this->ruleEnable = !isset($rule['enable']) || $rule['enable'] == 1;
-            $this->ruleMacro = $rule['macro'] ?? '';
-            $this->ruleIface = $rule['iface'] ?? '';
-            $this->ruleLog = $rule['log'] ?? 'nolog';
+        if (!isset($this->rules[$index])) {
+            $this->error = 'Rule not found';
+            $this->showErrorModal = true;
+            return;
         }
+
+        $rule = $this->rules[$index];
+        $this->editingRulePos = $index;
+        $this->showRuleForm = true;
+
+        $this->ruleAction = $rule['action'] ?? 'ACCEPT';
+        $this->ruleSource = $rule['source'] ?? '';
+        $this->ruleDest = $rule['dest'] ?? '';
+        $this->ruleProto = $rule['proto'] ?? 'tcp';
+        $this->ruleDport = $rule['dport'] ?? '';
+        $this->ruleSport = $rule['sport'] ?? '';
+        $this->ruleComment = $rule['comment'] ?? '';
+        $this->ruleEnable = !isset($rule['enable']) || $rule['enable'] != 0;
+        $this->ruleMacro = $rule['macro'] ?? '';
+        $this->ruleIface = $rule['iface'] ?? '';
+        $this->ruleLog = $rule['log'] ?? 'nolog';
     }
 
     public function resetRuleForm(): void
@@ -322,10 +327,12 @@ class Firewall extends Component
                 $payload['log'] = trim($this->ruleLog);
             }
 
-            if ($this->editingRulePos !== null) {
+            if ($this->editingRulePos !== null && isset($this->rules[$this->editingRulePos])) {
                 // Update existing rule
+                $rule = $this->rules[$this->editingRulePos];
+                $pos = $rule['pos'];
                 $response = $this->firewallRequest(
-                    '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules/' . $this->editingRulePos,
+                    '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules/' . $pos,
                     'put',
                     $payload
                 );
@@ -437,10 +444,14 @@ class Firewall extends Component
 
     public function confirmDeleteRule(int $index): void
     {
-        if (isset($this->rules[$index])) {
-            $this->deletingRulePos = $this->rules[$index]['pos'];
-            $this->showDeleteModal = true;
+        if (!isset($this->rules[$index])) {
+            $this->error = 'Rule not found';
+            $this->showErrorModal = true;
+            return;
         }
+
+        $this->deletingRulePos = $index;
+        $this->showDeleteModal = true;
     }
 
     public function cancelDeleteRule(): void
@@ -451,14 +462,15 @@ class Firewall extends Component
 
     public function deleteRule(): void
     {
-        if ($this->deletingRulePos === null) {
+        if ($this->deletingRulePos === null || !isset($this->rules[$this->deletingRulePos])) {
             return;
         }
 
-        $pos = $this->deletingRulePos;
+        $rule = $this->rules[$this->deletingRulePos];
+        $pos = $rule['pos'];
         $this->loading = true;
         $this->showDeleteModal = false;
-        
+
         try {
             $response = $this->firewallRequest(
                 '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules/' . $pos,
diff --git a/extensions/Servers/Proxmox/views/firewall.blade.php b/extensions/Servers/Proxmox/views/firewall.blade.php
index 58ab1f6cb..ef9c0d67e 100644
--- a/extensions/Servers/Proxmox/views/firewall.blade.php
+++ b/extensions/Servers/Proxmox/views/firewall.blade.php
@@ -506,87 +506,7 @@
 
     @if($firewallEnabled)
         <!-- Firewall Rules Card -->
-        <div class="bg-background border border-neutral rounded-lg p-6"
-            x-data="{
-                editingPositions: false,
-                rules: @js($rules),
-                draggedIndex: null,
-                dragOverIndex: null,
-                init() {
-                    // Listen for Livewire updates to sync the rules array
-                    $wire.on('rulesUpdated', () => {
-                        this.rules = @js($rules);
-                    });
-                },
-                        
-                        startDrag(event, index) {
-                            if (!this.editingPositions) {
-                                event.preventDefault();
-                                return;
-                            }
-                            this.draggedIndex = index;
-                            event.target.closest('tr').classList.add('dragging');
-                            event.dataTransfer.effectAllowed = 'move';
-                            event.dataTransfer.setData('text/html', event.target.closest('tr').innerHTML);
-                        },
-                        
-                        endDrag(event) {
-                            event.target.closest('tr').classList.remove('dragging');
-                            this.draggedIndex = null;
-                            this.dragOverIndex = null;
-                            // Remove all drag-over classes
-                            document.querySelectorAll('.drag-over').forEach(el => el.classList.remove('drag-over'));
-                        },
-                        
-                        dragOver(event, index) {
-                            event.preventDefault();
-                            if (this.draggedIndex !== null && this.draggedIndex !== index) {
-                                this.dragOverIndex = index;
-                            }
-                        },
-                        
-                        dragLeave(event) {
-                            event.target.closest('tr')?.classList.remove('drag-over');
-                        },
-                        
-                        async drop(event, targetIndex) {
-                            event.preventDefault();
-                            event.target.closest('tr')?.classList.remove('drag-over');
-                            
-                            if (this.draggedIndex === null || this.draggedIndex === targetIndex) {
-                                return;
-                            }
-                            
-                            // Capture ORIGINAL positions before modifying the array
-                            const originalDraggedPos = this.rules[this.draggedIndex].pos;
-                            const originalTargetPos = this.rules[targetIndex].pos;
-                            
-                            // Optimistically update UI
-                            const movedRule = this.rules.splice(this.draggedIndex, 1)[0];
-                            this.rules.splice(targetIndex, 0, movedRule);
-                            
-                            // Update positions in the array
-                            this.rules.forEach((rule, idx) => {
-                                rule.pos = idx;
-                            });
-                            
-                            // Call Livewire to persist the change using ORIGINAL positions
-                            try {
-                                await @this.reorderRules(originalDraggedPos, originalTargetPos);
-                            } catch (error) {
-                                console.error('Failed to reorder rules:', error);
-                                // Reload rules on error
-                                window.location.reload();
-                            }
-                        },
-                        
-                        getActionClass(action) {
-                            if (action === 'ACCEPT') return 'bg-success/20 text-success';
-                            if (action === 'REJECT') return 'bg-warning/20 text-warning';
-                            if (action === 'DROP') return 'bg-error/20 text-error';
-                            return '';
-                        }
-                    }">
+        <div class="bg-background border border-neutral rounded-lg p-6">
             <div class="flex items-center justify-between mb-4">
                 <div class="flex items-center gap-3">
                     <div class="flex items-center justify-center w-10 h-10 rounded-lg bg-primary/10">
@@ -623,7 +543,6 @@
                     <table class="w-full border-collapse">
                         <thead>
                             <tr class="border-b border-neutral">
-                                <th class="text-left py-3 px-2 font-semibold text-sm w-8"></th>
                                 <th class="text-left py-3 px-4 font-semibold text-sm">Order</th>
                                 <th class="text-left py-3 px-4 font-semibold text-sm">Action</th>
                                 <th class="text-left py-3 px-4 font-semibold text-sm">Source</th>
@@ -635,36 +554,32 @@
                             </tr>
                         </thead>
                         <tbody>
-                            <template x-for="(rule, index) in rules" :key="rule.pos">
-                                <tr class="rule-row border-b border-neutral/20 hover:bg-background-secondary/50"
-                                    :draggable="editingPositions"
-                                    @dragstart="startDrag($event, index)"
-                                    @dragend="endDrag($event)"
-                                    @dragover="dragOver($event, index)"
-                                    @dragleave="dragLeave($event)"
-                                    @drop="drop($event, index)"
-                                    :class="{ 'drag-over': dragOverIndex === index }">
-                                    <td class="py-3 px-2 text-sm">
-                                        <div x-show="editingPositions" class="drag-handle" title="Drag to reorder">
-                                            <x-ri-menu-line class="size-4" />
-                                        </div>
-                                    </td>
-                                    <td class="py-3 px-4 text-sm" x-text="rule.pos ?? '-'"></td>
+                            @foreach($rules as $index => $rule)
+                                <tr class="border-b border-neutral/20 hover:bg-background-secondary/50">
+                                    <td class="py-3 px-4 text-sm">{{ $rule['pos'] ?? '-' }}</td>
                                     <td class="py-3 px-4 text-sm">
-                                        <span class="px-2 py-1 rounded text-xs font-medium"
-                                              :class="getActionClass(rule.action)"
-                                              x-text="rule.action ?? '-'"></span>
+                                        @php
+                                            $actionClass = match($rule['action'] ?? '') {
+                                                'ACCEPT' => 'bg-success/20 text-success',
+                                                'REJECT' => 'bg-warning/20 text-warning',
+                                                'DROP' => 'bg-error/20 text-error',
+                                                default => ''
+                                            };
+                                        @endphp
+                                        <span class="px-2 py-1 rounded text-xs font-medium {{ $actionClass }}">
+                                            {{ $rule['action'] ?? '-' }}
+                                        </span>
                                     </td>
                                     <td class="py-3 px-4 text-sm">
                                         <div class="flex items-center gap-1">
-                                            <span class="font-mono text-xs" x-text="rule.source ?? '-'"></span>
-                                            <template x-if="rule.source && rule.source !== '-' && window.isPublicIP(rule.source)">
+                                            <span class="font-mono text-xs">{{ $rule['source'] ?? '-' }}</span>
+                                            @if($rule['source'] && $rule['source'] !== '-' && window.isPublicIP($rule['source']))
                                                 <div class="relative" x-data="{ open: false, loading: false, ipData: null }" x-cloak>
                                                     <button type="button" @click.stop="
                                                         if (!ipData && !loading) {
                                                             loading = true;
                                                             open = true;
-                                                            window.fetchIPLocation(rule.source).then(data => {
+                                                            window.fetchIPLocation('{{ $rule['source'] }}').then(data => {
                                                                 ipData = data;
                                                                 loading = false;
                                                             }).catch(err => {
@@ -754,8 +669,8 @@
                                     </td>
                                     <td class="py-3 px-4 text-sm">
                                         <div class="flex items-center gap-1">
-                                            <span class="font-mono text-xs" x-text="rule.dest ?? '-'"></span>
-                                            <template x-if="rule.dest && rule.dest !== '-' && window.isPublicIP(rule.dest)">
+                                            <span class="font-mono text-xs">{{ $rule['dest'] ?? '-' }}</span>
+                                            @if($rule['dest'] && $rule['dest'] !== '-' && window.isPublicIP($rule['dest']))
                                                 <div class="relative" x-data="{ open: false, loading: false, ipData: null }" x-cloak>
                                                     <button type="button" @click.stop="
                                                         if (!ipData && !loading) {
@@ -849,37 +764,42 @@
                                             </template>
                                         </div>
                                     </td>
-                                    <td class="py-3 px-4 text-sm" x-text="(rule.proto ?? 'any').toUpperCase()"></td>
-                                    <td class="py-3 px-4 text-sm" x-text="rule.dport ?? rule.sport ?? '-'"></td>
+                                    <td class="py-3 px-4 text-sm">{{ strtoupper($rule['proto'] ?? 'any') }}</td>
+                                    <td class="py-3 px-4 text-sm">{{ $rule['dport'] ?? $rule['sport'] ?? '-' }}</td>
                                     <td class="py-3 px-4 text-sm">
-                                        <span x-show="rule.enable == 0" class="px-2 py-1 rounded text-xs font-medium bg-base/20 text-base/60">Disabled</span>
-                                        <span x-show="rule.enable != 0" class="px-2 py-1 rounded text-xs font-medium bg-success/20 text-success">Enabled</span>
+                                        @if(isset($rule['enable']) && $rule['enable'] == 0)
+                                            <span class="px-2 py-1 rounded text-xs font-medium bg-base/20 text-base/60">Disabled</span>
+                                        @else
+                                            <span class="px-2 py-1 rounded text-xs font-medium bg-success/20 text-success">Enabled</span>
+                                        @endif
                                     </td>
                                     <td class="py-3 px-4 text-sm">
                                         <div class="flex gap-2">
                                             <button
                                                 type="button"
-                                                wire:click="toggleRule(index)"
-                                                :wire:key="'toggle-' + index"
+                                                wire:click="toggleRule({{ $index }})"
+                                                wire:key="toggle-rule-{{ $index }}"
                                                 wire:loading.attr="disabled"
-                                                class="transition-colors"
-                                                :class="rule.enable == 0 ? 'text-warning hover:text-warning/80' : 'text-success hover:text-success/80'"
-                                                :title="rule.enable == 0 ? 'Enable rule' : 'Disable rule'">
-                                                <x-ri-toggle-line x-show="rule.enable == 0" class="size-4" />
-                                                <x-ri-toggle-fill x-show="rule.enable != 0" class="size-4" />
+                                                class="transition-colors {{ isset($rule['enable']) && $rule['enable'] == 0 ? 'text-warning hover:text-warning/80' : 'text-success hover:text-success/80' }}"
+                                                title="{{ isset($rule['enable']) && $rule['enable'] == 0 ? 'Enable rule' : 'Disable rule' }}">
+                                                @if(isset($rule['enable']) && $rule['enable'] == 0)
+                                                    <x-ri-toggle-line class="size-4" />
+                                                @else
+                                                    <x-ri-toggle-fill class="size-4" />
+                                                @endif
                                             </button>
                                             <button
                                                 type="button"
-                                                wire:click="startEditRule(index)"
-                                                :wire:key="'edit-' + index"
+                                                wire:click="startEditRule({{ $index }})"
+                                                wire:key="edit-rule-{{ $index }}"
                                                 class="text-primary hover:text-primary/80 transition-colors"
                                                 title="Edit rule">
                                                 <x-ri-edit-line class="size-4" />
                                             </button>
                                             <button
                                                 type="button"
-                                                wire:click="confirmDeleteRule(index)"
-                                                :wire:key="'delete-' + index"
+                                                wire:click="confirmDeleteRule({{ $index }})"
+                                                wire:key="delete-rule-{{ $index }}"
                                                 class="text-error hover:text-error/80 transition-colors"
                                                 title="Delete rule">
                                                 <x-ri-delete-bin-line class="size-4" />
@@ -887,7 +807,7 @@
                                         </div>
                                     </td>
                                 </tr>
-                            </template>
+                            @endforeach
                         </tbody>
                     </table>
                 </div>