Skip to content

#1603 : Cannot revert to the preserved (original) IP if the user first enters a new IP and applies the changes #1640

Open
AbhijeetThakur wants to merge 5 commits intomainfrom
1603-cannot-revert-to-the-preserved-original-ip-if-the-user-first-enters-a-new-ip-and-applies-the-changes
Open

#1603 : Cannot revert to the preserved (original) IP if the user first enters a new IP and applies the changes #1640
AbhijeetThakur wants to merge 5 commits intomainfrom
1603-cannot-revert-to-the-preserved-original-ip-if-the-user-first-enters-a-new-ip-and-applies-the-changes

Conversation

@AbhijeetThakur
Copy link
Collaborator

@AbhijeetThakur AbhijeetThakur commented Mar 6, 2026

What this PR does / why we need it

  • Track original IP addresses for network interface preservation

Which issue(s) this PR fixes

fixes #1603

Testing done

Screencast.from.06-03-26.03.08.51.PM.IST.webm

Note

Medium Risk
Moderate risk: changes bulk IP assignment state/validation logic, which could alter what IP updates are applied for selected VMs/interfaces. No auth/security impact, but regressions could prevent correctly reverting or applying IPs during migration setup.

Overview
Fixes bulk IP assignment so users can revert back to the original preserved IP after previously applying a different IP.

The UI now tracks per-VM/interface originalIPsPerVM separately from the current displayed IP (bulkCurrentIPs), initializes the bulk editor using the original value when Preserve IP is enabled, and updates apply logic to only submit changes when the typed IP differs from the current IP. The migration template network interface model is extended with optional originalIpAddress to support carrying this data.

Written by Cursor Bugbot for commit e8bf236. This will update automatically on new commits. Configure here.

Add originalIpAddress field to VmNetworkInterface model and implement tracking of original IPs across VM network interfaces. Introduce originalIPsPerVM and bulkCurrentIPs state to distinguish between discovered IPs and user-modified IPs. Update bulk IP edit logic to compare against current IPs instead of existing IPs to prevent unnecessary reapplication. Initialize original IPs on component mount and preserve them when applying bulk IP
Copy link
Contributor

@windsurf-bot windsurf-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other comments (3)
  • ui/src/features/migration/RollingMigrationForm.tsx (2543-2556) In handleApplyBulkIPs, the originalIPsPerVM update logic only adds entries if they don't already exist (`if (next[vmId][idx] !== undefined) return`). This might prevent updating original IPs in certain scenarios. Consider whether this is the intended behavior or if original IPs should be updated in some cases.
  • ui/src/features/migration/VmsSelectionStep.tsx (1592-1597) This nested ternary expression is difficult to read and maintain. Consider refactoring to use more explicit conditional logic:
    let originalIp = '';
    if (originalIPsPerVM?.[vmName]?.[0] !== undefined) {
      originalIp = originalIPsPerVM[vmName][0];
    } else if (vm.ipAddress && vm.ipAddress !== '—') {
      originalIp = vm.ipAddress;
    }
    
  • ui/src/features/migration/VmsSelectionStep.tsx (1610-1612) This complex ternary expression for setting validation status is difficult to read. Consider simplifying:
    const ipToCheck = effectivePreserveIp ? originalIp : currentIp;
    initialValidationStatus[vmName][0] = ipToCheck ? 'valid' : 'empty';
    

💡 To request another review, post a new comment with "/windsurf-review".

Copy link
Contributor

@sanya-pf9 sanya-pf9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM1

…-ip-if-the-user-first-enters-a-new-ip-and-applies-the-changes
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Free Tier Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Toggling preserve IP off keeps original IP, causing unintended revert
    • Added a preserve-off branch in handleBulkPreserveIpChange that resets the edited IP to bulkCurrentIPs and clears validation state so Apply no longer reverts to the original IP unintentionally.
  • ✅ Fixed: Unused originalIpAddress field added to model interface
    • Removed the unused originalIpAddress property from VmNetworkInterface since original IP tracking is handled by component state and the field was never read or populated.

Create PR

Or push these changes by commenting:

@cursor push dde0dabca1
Preview (dde0dabca1)
diff --git a/ui/src/features/migration/VmsSelectionStep.tsx b/ui/src/features/migration/VmsSelectionStep.tsx
--- a/ui/src/features/migration/VmsSelectionStep.tsx
+++ b/ui/src/features/migration/VmsSelectionStep.tsx
@@ -971,6 +971,20 @@
           [vmName]: { ...prev[vmName], [interfaceIndex]: '' }
         }))
       }
+    } else {
+      const currentIp = bulkCurrentIPs?.[vmName]?.[interfaceIndex] || ''
+      setBulkEditIPs((prev) => ({
+        ...prev,
+        [vmName]: { ...prev[vmName], [interfaceIndex]: currentIp }
+      }))
+      setBulkValidationStatus((prev) => ({
+        ...prev,
+        [vmName]: { ...prev[vmName], [interfaceIndex]: currentIp.trim() ? 'valid' : 'empty' }
+      }))
+      setBulkValidationMessages((prev) => ({
+        ...prev,
+        [vmName]: { ...prev[vmName], [interfaceIndex]: '' }
+      }))
     }
   }
 

diff --git a/ui/src/features/migration/api/migration-templates/model.ts b/ui/src/features/migration/api/migration-templates/model.ts
--- a/ui/src/features/migration/api/migration-templates/model.ts
+++ b/ui/src/features/migration/api/migration-templates/model.ts
@@ -86,7 +86,6 @@
   mac: string
   network: string
   ipAddress: string
-  originalIpAddress?: string
   preserveIP?: boolean
   preserveMAC?: boolean
 }

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

@cursor
Copy link

cursor bot commented Mar 12, 2026

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on April 11.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

…-ip-if-the-user-first-enters-a-new-ip-and-applies-the-changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot revert to the preserved (original) IP if the user first enters a new IP and applies the changes

2 participants