NAS-139101 / 26.04 / Preserve pool name when using Start Over in update mode

[AlexKarpov98]

When adding VDEVs to an existing pool, clicking Start Over now preserves the pool name instead of clearing it. The pool name is read-only in update mode, so it should remain when resetting the form.

Changes:

• Modified resetForm() in general-wizard-step to preserve pool name in update mode
• Start Over button now shows in both create and update modes
• In create mode: clears all fields (original behavior)
• In update mode: preserves pool name, resets other fields

Changes:

Testing:

Downstream

Affects	Reasoning
Documentation
Testing

[bugclerk]

Jira URL: https://ixsystems.atlassian.net/browse/NAS-139101

[bugclerk]

Jira URL: https://ixsystems.atlassian.net/browse/NAS-139101

[claude]

Claude finished @AlexKarpov98's task —— View job

---

Code Review Complete ✅

The implementation looks solid and solves the issue appropriately! 🎉

Positive Aspects

The changes are well-structured, minimal, and correctly preserve the pool name in update mode while maintaining the original behavior for pool creation. The added test coverage and clear comments demonstrate good development practices.

Observations & Suggestions

1. Optional Chaining in showStartOver getter (src/app/pages/storage/modules/pool-manager/components/pool-manager-wizard/steps/9-review-wizard-step/review-wizard-step.component.ts:77-82)

The change from this.state.name to this.state?.name adds optional chaining, which is good defensive programming. However, since state is set in ngOnInit via a subscription and there's no explicit nullable type declaration, this suggests either:

• The property should be explicitly typed as PoolManagerState | undefined if it can be undefined
• Or the optional chaining might be unnecessary if state is always initialized before showStartOver is called

Suggestion: Consider explicitly typing the state property:

state: PoolManagerState | undefined;

This makes the intention clear and aligns with TypeScript strict null checks.

2. Test Coverage Enhancement (src/app/pages/storage/modules/pool-manager/components/pool-manager-wizard/steps/9-review-wizard-step/review-wizard-step.component.spec.ts:211-217)

The new test verifies that the Start Over button appears in update mode, which is good. However, it would be valuable to also test that the pool name is actually preserved after clicking Start Over in update mode.

Suggestion: Consider adding a test in general-wizard-step.component.spec.ts that verifies:

it('preserves pool name when Start Over is clicked in update mode', () => {
  spectator.setInput('isAddingVdevs', true);
  spectator.setInput('pool', { name: 'existing-pool' } as Pool);
  
  store.startOver(); // or trigger the startOver action
  
  expect(spectator.component.form.value.name).toBe('existing-pool');
});

3. Minor: Test Selector (src/app/pages/storage/modules/pool-manager/components/pool-manager-wizard/steps/9-review-wizard-step/review-wizard-step.component.spec.ts:215)

The test uses button[ixTest="start-over"] selector. According to CLAUDE.md, "Never rely on ixTest attributes for locating elements" when writing tests. The harness approach used in other tests would be more consistent.

Suggestion: Use the harness pattern like other tests in the file:

const startOverButton = await loader.getHarness(MatButtonHarness.with({ text: 'Start Over' }));
expect(await startOverButton.host()).toBeTruthy();

Summary

Great work on this PR! The core logic is correct and the implementation properly handles both create and update modes. The minor suggestions above are optional improvements that would enhance code consistency and test robustness.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.69%. Comparing base (1b4fd67) to head (3d59fb1).
⚠️ Report is 4 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #13043      +/-   ##
==========================================
- Coverage   86.69%   86.69%   -0.01%     
==========================================
  Files        1846     1846              
  Lines       69371    69372       +1     
  Branches     8574     8575       +1     
==========================================
- Hits        60143    60142       -1     
- Misses       9228     9230       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.