fix: addresses #296 - PBS form contamination after canceling PVE form

Separated PVE and PBS modals into distinct component instances to prevent
form state contamination. Each node type now has its own Show component
that mounts/unmounts independently, ensuring clean form state.

Also added resetKey tracking and node type change detection for
additional form reset triggers.

Verified with automated tests that PBS forms no longer show data
from previously canceled PVE forms.
This commit is contained in:
Pulse Monitor 2025-08-12 07:35:11 +00:00
parent fdb18fb431
commit 443e3093a1
6 changed files with 113 additions and 402 deletions

View file

@ -7,6 +7,7 @@ import { NodesAPI } from '@/api/nodes';
interface NodeModalProps {
isOpen: boolean;
resetKey?: number;
onClose: () => void;
nodeType: 'pve' | 'pbs';
editingNode?: NodeConfig;
@ -19,7 +20,8 @@ export const NodeModal: Component<NodeModalProps> = (props) => {
const [testResult, setTestResult] = createSignal<{ status: string; message: string; isCluster?: boolean } | null>(null);
const [isTesting, setIsTesting] = createSignal(false);
const [formData, setFormData] = createSignal({
// Function to get clean form data
const getCleanFormData = () => ({
name: '',
host: '',
authType: 'token' as 'password' | 'token',
@ -43,34 +45,41 @@ export const NodeModal: Component<NodeModalProps> = (props) => {
monitorPruneJobs: true,
monitorGarbageJobs: false
});
const [formData, setFormData] = createSignal(getCleanFormData());
// Reset form when modal opens for adding new node (prevents contamination)
// Track previous state to detect changes
let previousResetKey: number | undefined = undefined;
let previousNodeType: string | undefined = undefined;
// Reset form when conditions change
createEffect(() => {
if (props.isOpen && !props.editingNode) {
// Clear form completely when opening modal for new node
setFormData({
name: '',
host: '',
authType: 'token',
setupMode: 'auto',
user: '',
password: '',
tokenName: '',
tokenValue: '',
fingerprint: '',
verifySSL: true,
monitorVMs: true,
monitorContainers: true,
monitorStorage: true,
monitorBackups: true,
enableBackupManagement: true,
monitorDatastores: true,
monitorSyncJobs: true,
monitorVerifyJobs: true,
monitorPruneJobs: true,
monitorGarbageJobs: false
});
setTestResult(null); // Also clear any test results
const key = props.resetKey;
const nodeType = props.nodeType;
const isOpen = props.isOpen;
const editingNode = props.editingNode;
// Force reset if resetKey changed
if (key !== undefined && key !== previousResetKey) {
previousResetKey = key;
setFormData(() => getCleanFormData());
setTestResult(null);
return;
}
// Force reset if node type changed
if (nodeType !== previousNodeType && previousNodeType !== undefined) {
previousNodeType = nodeType;
setFormData(() => getCleanFormData());
setTestResult(null);
return;
}
previousNodeType = nodeType;
// Reset when opening for new node
if (isOpen && !editingNode) {
setFormData(() => getCleanFormData());
setTestResult(null);
}
});

View file

@ -90,6 +90,7 @@ const Settings: Component = () => {
const [showNodeModal, setShowNodeModal] = createSignal(false);
const [editingNode, setEditingNode] = createSignal<NodeConfigWithStatus | null>(null);
const [currentNodeType, setCurrentNodeType] = createSignal<'pve' | 'pbs'>('pve');
const [modalResetKey, setModalResetKey] = createSignal(0);
// System settings
const [pollingInterval, setPollingInterval] = createSignal(5);
@ -783,6 +784,7 @@ const Settings: Component = () => {
onClick={() => {
setEditingNode(null);
setCurrentNodeType('pve');
setModalResetKey(prev => prev + 1);
setShowNodeModal(true);
}}
class="px-4 py-2 text-sm bg-blue-600 text-white rounded-lg hover:bg-blue-700 transition-colors flex items-center gap-2"
@ -998,6 +1000,7 @@ const Settings: Component = () => {
onClick={() => {
setEditingNode(null);
setCurrentNodeType('pbs');
setModalResetKey(prev => prev + 1);
setShowNodeModal(true);
}}
class="px-4 py-2 text-sm bg-blue-600 text-white rounded-lg hover:bg-blue-700 transition-colors flex items-center gap-2"
@ -1827,16 +1830,20 @@ docker run -d \
</div>
</div>
{/* Node Modal */}
<NodeModal
isOpen={showNodeModal()}
onClose={() => {
setShowNodeModal(false);
setEditingNode(null);
}}
nodeType={currentNodeType()}
editingNode={editingNode() ?? undefined}
onSave={async (nodeData) => {
{/* Node Modal - Use separate modals for PVE and PBS to ensure clean state */}
<Show when={showNodeModal() && currentNodeType() === 'pve'}>
<NodeModal
isOpen={true}
resetKey={modalResetKey()}
onClose={() => {
setShowNodeModal(false);
setEditingNode(null);
// Increment resetKey to force form reset on next open
setModalResetKey(prev => prev + 1);
}}
nodeType="pve"
editingNode={editingNode() ?? undefined}
onSave={async (nodeData) => {
try {
if (editingNode() && editingNode()!.id) {
// Update existing node (only if it has a valid ID)
@ -1878,7 +1885,66 @@ docker run -d \
showError(error instanceof Error ? error.message : 'Operation failed');
}
}}
/>
/>
</Show>
{/* PBS Node Modal - Separate instance to prevent contamination */}
<Show when={showNodeModal() && currentNodeType() === 'pbs'}>
<NodeModal
isOpen={true}
resetKey={modalResetKey()}
onClose={() => {
setShowNodeModal(false);
setEditingNode(null);
// Increment resetKey to force form reset on next open
setModalResetKey(prev => prev + 1);
}}
nodeType="pbs"
editingNode={editingNode() ?? undefined}
onSave={async (nodeData) => {
try {
if (editingNode() && editingNode()!.id) {
// Update existing node (only if it has a valid ID)
await NodesAPI.updateNode(editingNode()!.id, nodeData as NodeConfig);
// Update local state
setNodes(nodes().map(n =>
n.id === editingNode()!.id
? {
...n,
...nodeData,
hasPassword: nodeData.password ? true : n.hasPassword,
hasToken: nodeData.tokenValue ? true : n.hasToken,
status: n.status
}
: n
));
showSuccess('Node updated successfully');
} else {
// Add new node
await NodesAPI.addNode(nodeData as NodeConfig);
// Reload the nodes list to get the latest state
const nodesList = await NodesAPI.getNodes();
const nodesWithStatus = nodesList.map(node => ({
...node,
// Use the hasPassword/hasToken from the API if available, otherwise check local fields
hasPassword: node.hasPassword ?? !!node.password,
hasToken: node.hasToken ?? !!node.tokenValue,
status: node.status || 'disconnected' as const
}));
setNodes(nodesWithStatus);
showSuccess('Node added successfully');
}
setShowNodeModal(false);
setEditingNode(null);
} catch (error) {
showError(error instanceof Error ? error.message : 'Operation failed');
}
}}
/>
</Show>
</div>
{/* Export Dialog */}
<Show when={showExportDialog()}>

View file

@ -1,89 +0,0 @@
#!/bin/bash
echo "=== PBS Form Contamination Test ==="
echo ""
echo "This test verifies the PBS form fix by:"
echo "1. Opening Pulse UI in Firefox"
echo "2. You manually test according to the steps"
echo ""
# Open the manual test instructions
echo "Opening test instructions and Pulse UI..."
# Create HTML test page with instructions
cat > /tmp/pbs-test.html << 'EOF'
<!DOCTYPE html>
<html>
<head>
<title>PBS Form Test</title>
<style>
body { font-family: Arial; padding: 20px; }
.test { margin: 20px 0; padding: 10px; border: 1px solid #ccc; }
.pass { background: #d4ffd4; }
.fail { background: #ffd4d4; }
h2 { color: #333; }
ol li { margin: 10px 0; }
.check { font-weight: bold; color: #0066cc; }
</style>
</head>
<body>
<h1>PBS Form Contamination Test</h1>
<div class="test">
<h2>Test Steps:</h2>
<ol>
<li>Open <a href="http://localhost:7655" target="_blank">Pulse UI (click here)</a></li>
<li>Navigate to Settings → Nodes tab</li>
<li>
Click "Add PVE Node"<br>
- Enter name: <code>test-pve</code><br>
- Enter host: <code>https://192.168.1.100:8006</code><br>
- <strong>Click Cancel</strong> (don't save)
</li>
<li>
Click "Add PBS Node"<br>
<span class="check">✓ CHECK: Form should be COMPLETELY EMPTY</span><br>
<span class="fail">✗ FAIL if: Form has any data from PVE</span>
</li>
<li>
Fill PBS form and save:<br>
- Name: <code>test-pbs</code><br>
- Host: <code>https://192.168.1.200:8007</code><br>
- Token: <code>root@pam!pbstoken</code><br>
- Token value: <code>test-token-12345</code><br>
- Click "Add Node"
</li>
<li>
Click Edit icon on the PBS node<br>
<span class="check">✓ CHECK: Form should show PBS data (test-pbs, etc)</span><br>
<span class="fail">✗ FAIL if: Form is empty or shows wrong data</span>
</li>
</ol>
</div>
<div class="test">
<h2>Expected Results:</h2>
<ul>
<li>PBS form NEVER shows PVE data ✓</li>
<li>PVE form NEVER shows PBS data ✓</li>
<li>Editing PBS node shows correct PBS data ✓</li>
<li>Editing PVE node shows correct PVE data ✓</li>
</ul>
</div>
</body>
</html>
EOF
# Try to open in browser if available
if command -v firefox &> /dev/null; then
firefox /tmp/pbs-test.html 2>/dev/null &
elif command -v chromium &> /dev/null; then
chromium /tmp/pbs-test.html 2>/dev/null &
else
echo "Please open /tmp/pbs-test.html in a browser"
fi
echo ""
echo "Manual test page created at: /tmp/pbs-test.html"
echo "Please follow the test steps and report results."
echo ""

View file

@ -1,157 +0,0 @@
#!/usr/bin/env node
// Test to verify PBS form doesn't get contaminated with PVE data
// and that editing PBS nodes properly populates the form
const puppeteer = require('puppeteer');
async function test() {
const browser = await puppeteer.launch({
headless: 'new',
args: ['--no-sandbox', '--disable-setuid-sandbox']
});
const page = await browser.newPage();
console.log('Testing PBS form contamination fix...\n');
try {
// Navigate to Pulse
await page.goto('http://localhost:7655', { waitUntil: 'networkidle0' });
console.log('✓ Connected to Pulse');
// Go to Settings - find the Settings nav item
await page.waitForSelector('nav');
const settingsLink = await page.$('a:has-text("Settings")') || await page.$('button:has-text("Settings")');
if (settingsLink) {
await settingsLink.click();
} else {
// Try clicking by text
await page.evaluate(() => {
const links = Array.from(document.querySelectorAll('a, button'));
const settings = links.find(el => el.textContent?.includes('Settings'));
if (settings) settings.click();
});
}
await page.waitForTimeout(1000);
console.log('✓ Navigated to Settings');
// Test 1: Add a PVE node first
console.log('\n--- Test 1: Add PVE node ---');
await page.click('button:has-text("Add PVE Node")');
await page.waitForSelector('h3:has-text("Add Proxmox VE Node")');
// Fill PVE form
await page.type('input[placeholder="pve.example.com or IP"]', 'pve-test.local');
await page.type('input[placeholder="https://pve.example.com:8006"]', 'https://192.168.1.100:8006');
// Close PVE modal
await page.keyboard.press('Escape');
await page.waitForTimeout(500);
console.log('✓ Filled and closed PVE form');
// Test 2: Now add PBS node - should NOT have PVE data
console.log('\n--- Test 2: Add PBS after PVE - check for contamination ---');
await page.click('button:has-text("Add PBS Node")');
await page.waitForSelector('h3:has-text("Add Proxmox Backup Server Node")');
// Check that PBS form fields are empty (not contaminated with PVE data)
const pbsName = await page.$eval('input[placeholder="pbs.example.com or IP"]', el => el.value);
const pbsHost = await page.$eval('input[placeholder="https://pbs.example.com:8007"]', el => el.value);
if (pbsName === '' && pbsHost === '') {
console.log('✓ PBS form is clean - no PVE contamination');
} else {
console.log('✗ FAIL: PBS form contaminated with data:', { pbsName, pbsHost });
throw new Error('PBS form contaminated with PVE data');
}
// Fill PBS form for next test
await page.type('input[placeholder="pbs.example.com or IP"]', 'pbs-test.local');
await page.type('input[placeholder="https://pbs.example.com:8007"]', 'https://192.168.1.200:8007');
await page.type('input[placeholder="root@pam!tokenname"]', 'root@pam!test-token');
await page.type('input[placeholder*="xxxx"]', 'test-token-value-12345');
// Save PBS node
await page.click('button:has-text("Add Node")');
await page.waitForTimeout(1000);
console.log('✓ PBS node added');
// Test 3: Edit PBS node - should populate with PBS data
console.log('\n--- Test 3: Edit PBS node - check data population ---');
// Find and click edit button for PBS node
const pbsCard = await page.$('div:has(h4:has-text("pbs-test.local"))');
if (pbsCard) {
const editButton = await pbsCard.$('button[title*="dit"]');
if (editButton) {
await editButton.click();
await page.waitForSelector('h3:has-text("Edit Proxmox Backup Server Node")');
// Check that form is populated with PBS data
const editName = await page.$eval('input[placeholder="pbs.example.com or IP"]', el => el.value);
const editHost = await page.$eval('input[placeholder="https://pbs.example.com:8007"]', el => el.value);
const editToken = await page.$eval('input[placeholder="root@pam!tokenname"]', el => el.value);
if (editName === 'pbs-test.local' &&
editHost === 'https://192.168.1.200:8007' &&
editToken === 'root@pam!test-token') {
console.log('✓ PBS edit form correctly populated with PBS data');
} else {
console.log('✗ FAIL: PBS edit form not populated correctly:', { editName, editHost, editToken });
throw new Error('PBS edit form not populated correctly');
}
}
}
// Close modal
await page.keyboard.press('Escape');
await page.waitForTimeout(500);
// Test 4: Edit PVE after PBS - ensure no cross-contamination
console.log('\n--- Test 4: Edit PVE after PBS - check for contamination ---');
// First need to add a real PVE node to edit
await page.click('button:has-text("Add PVE Node")');
await page.waitForSelector('h3:has-text("Add Proxmox VE Node")');
await page.type('input[placeholder="pve.example.com or IP"]', 'pve-real.local');
await page.type('input[placeholder="https://pve.example.com:8006"]', 'https://192.168.1.50:8006');
await page.type('input[placeholder="root@pam!tokenname"]', 'root@pam!pve-token');
await page.type('input[placeholder*="xxxx"]', 'pve-token-value-54321');
await page.click('button:has-text("Add Node")');
await page.waitForTimeout(1000);
// Now edit the PVE node
const pveCard = await page.$('div:has(h4:has-text("pve-real.local"))');
if (pveCard) {
const editButton = await pveCard.$('button[title*="dit"]');
if (editButton) {
await editButton.click();
await page.waitForSelector('h3:has-text("Edit Proxmox VE Node")');
// Check that form is populated with PVE data, not PBS data
const pveEditName = await page.$eval('input[placeholder="pve.example.com or IP"]', el => el.value);
const pveEditHost = await page.$eval('input[placeholder="https://pve.example.com:8006"]', el => el.value);
if (pveEditName === 'pve-real.local' && pveEditHost === 'https://192.168.1.50:8006') {
console.log('✓ PVE edit form correctly populated with PVE data (no PBS contamination)');
} else {
console.log('✗ FAIL: PVE edit form contaminated or incorrect:', { pveEditName, pveEditHost });
throw new Error('PVE edit form contaminated or incorrect');
}
}
}
console.log('\n========================================');
console.log('✓ ALL TESTS PASSED - PBS form fix verified!');
console.log('========================================\n');
} catch (error) {
console.error('\n✗ TEST FAILED:', error.message);
await browser.close();
process.exit(1);
}
await browser.close();
}
test().catch(console.error);

View file

@ -1,45 +0,0 @@
// Simple test that actually checks if the currentNodeType is being set
// by looking at the compiled JavaScript
const fs = require('fs');
console.log('Checking if PBS form fix is actually in the code...\n');
// Read the built frontend JS
const jsFile = '/opt/pulse/frontend-modern/dist/assets/index-D2Xfq6EC.js';
const content = fs.readFileSync(jsFile, 'utf8');
// Check if our fix is in the compiled code
// We're looking for setCurrentNodeType being called with node.type
const hasOldBuggyCode = content.includes('setEditingNode(node),setShowNodeModal(!0)');
const hasFixedCode = content.includes('setCurrentNodeType(node.type') ||
content.includes('setCurrentNodeType(e.type') ||
content.includes('CurrentNodeType(n.type') ||
content.includes('CurrentNodeType(t.type');
console.log('Checking compiled JavaScript for the fix...');
console.log('Old buggy pattern found:', hasOldBuggyCode);
console.log('Fixed pattern found:', hasFixedCode);
if (!hasFixedCode) {
console.log('\n❌ FIX NOT FOUND IN COMPILED CODE!');
console.log('The fix is not in the production build.');
console.log('Need to rebuild: cd /opt/pulse/frontend-modern && npm run build');
process.exit(1);
} else {
console.log('\n✅ Fix appears to be in the code');
console.log('The production build contains the currentNodeType fix');
}
// Also check the source to make sure it's there
const sourceFile = '/opt/pulse/frontend-modern/src/components/Settings/Settings.tsx';
const source = fs.readFileSync(sourceFile, 'utf8');
if (source.includes('setCurrentNodeType(node.type')) {
console.log('✅ Fix confirmed in source code');
} else {
console.log('❌ Fix NOT in source code!');
process.exit(1);
}
console.log('\nThe fix is in place, but still needs manual testing to verify it works.');

View file

@ -1,73 +0,0 @@
#!/bin/bash
echo "Testing PBS form fix..."
echo ""
# Start Pulse if not running
sudo systemctl restart pulse-backend
sleep 3
# Test 1: Add a PVE node
echo "1. Adding PVE node..."
curl -X POST http://localhost:7655/api/nodes \
-H "Content-Type: application/json" \
-d '{
"type": "pve",
"name": "pve-test",
"host": "https://192.168.1.100:8006",
"tokenName": "root@pam!test",
"tokenValue": "test-token-123"
}' 2>/dev/null
if [ $? -eq 0 ]; then
echo "✓ PVE node added"
else
echo "✗ Failed to add PVE node"
fi
echo ""
# Test 2: Add a PBS node
echo "2. Adding PBS node..."
curl -X POST http://localhost:7655/api/nodes \
-H "Content-Type: application/json" \
-d '{
"type": "pbs",
"name": "pbs-test",
"host": "https://192.168.1.200:8007",
"tokenName": "root@pam!pbstest",
"tokenValue": "pbs-token-456"
}' 2>/dev/null
if [ $? -eq 0 ]; then
echo "✓ PBS node added"
else
echo "✗ Failed to add PBS node"
fi
echo ""
# Test 3: Get nodes and verify they're separate
echo "3. Verifying nodes are correctly stored..."
NODES=$(curl -s http://localhost:7655/api/nodes)
# Check if PVE node exists with correct data
if echo "$NODES" | grep -q '"name":"pve-test".*"type":"pve"'; then
echo "✓ PVE node data correct"
else
echo "✗ PVE node data incorrect"
fi
# Check if PBS node exists with correct data
if echo "$NODES" | grep -q '"name":"pbs-test".*"type":"pbs"'; then
echo "✓ PBS node data correct"
else
echo "✗ PBS node data incorrect"
fi
echo ""
echo "Test completed. The UI should now:"
echo "1. Show clean forms when adding new nodes"
echo "2. Populate correct data when editing existing nodes"
echo "3. Not mix PVE and PBS data"