NAS-137713: Convert Containers pages to use new API

[AlexKarpov98]

WIP

Testing: see ticket & story.

not: API Limitations (for now API is limited and not fully created) Available endpoints, which I used for current refactoring :

container.create
container.delete
container.get_instance
container.migrate (not used in this PR)
container.query
container.start
container.stop
container.update
container.pool_choices

and as well config endpoints:

lxc.bridge_choices
lxc.config
lxc.update

As well:

• Removed VM-specific features: TPM, PCI passthrough, boot disk management, VNC configuration
• Deleted unused components: instance-edit-form (now only instance wizard used), change-root-disk-setup, change-boot-from-disk, pci-passthrough-dialog, import-iso-dialog, upload-iso-button, virtualization-state
• Refactored to use MapValuePipe instead of component methods for value transformation
• Remove unnecessary computed properties (isLocked, needToSetupPool, virtualizationState)
• Cleaned up unused API definitions from api-call-directory, api-job-directory, api-event-directory
• Fixed all test failures and removed skipped tests

Preview:

[bugclerk]

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

[claude[bot]]

Claude finished @AlexKarpov98's task —— View job

---

PR Review Complete ✅

Great work on this substantial API migration! 🎉 The architecture is clean and the modern Angular patterns are well-applied. The latest commit addresses the two main issues mentioned by william-gr. Here's my comprehensive feedback:

---

✅ What's Working Well

The PR demonstrates solid architectural decisions with NgRx ComponentStore for state management, proper separation of concerns, and comprehensive error handling. The use of modern Angular patterns like signals, inject(), and reactive forms is excellent.

---

🔍 Issues Found

1. Shell Button Reactivity - Now Fixed ✅

The latest commit (f8dc79c) properly addresses this by injecting the store directly in instance-tools.component.ts:33. The shell button should now properly react to container status changes.

2. USB Device Empty Entry - Now Fixed ✅

The latest commit adds proper validation in add-usb-device-menu.component.ts:73-75 to filter out invalid USB entries missing description or product_id. This should resolve the empty entry issue shown in the review screenshot.

3. @ngneat/until-destroy Usage - Should Migrate ⚠️

Per CLAUDE.md guidelines, the codebase should use takeUntilDestroyed(this.destroyRef) from @angular/core/rxjs-interop instead of @ngneat/until-destroy. Found 23 files in the instances module still using the old pattern:

• instance-form.component.ts
• device-actions-menu.component.ts
• add-usb-device-menu.component.ts
• And 20 more files

Recommendation: Create a follow-up task to migrate these to the modern Angular pattern. This isn't blocking but should be addressed for consistency.

4. @HostListener Usage in instance-form.component.ts ⚠️

Found @HostListener in instance-form.component.ts:2. Per CLAUDE.md, should use host: { '(window:beforeunload)': '...' } instead of the @HostListener decorator.

// Current (line 2):
import { HostListener } from '@angular/core';

// Should be in component metadata:
@Component({
  host: {
    '(window:beforeunload)': 'handleBeforeUnload($event)'
  }
})

5. Constructor Injection Pattern ℹ️

Found 6 files still using constructor injection instead of the preferred inject() function. While not critical, standardizing on inject() improves consistency:

• select-image-dialog.component.ts
• instance-filesystem-device-form.component.ts
• stop-options-dialog.component.ts
• instance-list.component.ts
• global-config-form.component.ts
• instance-form.component.ts

6. TODO Comments Indicate API Limitations ℹ️

Found 2 TODO comments in container-instances.store.ts:

• Line 107: Workaround for API sending instance name instead of ID for status-only updates
• Line 192: API endpoint under review (virt.instance.metrics vs container.metrics)

These are well-documented and have clear explanations. Monitor the middleware changes mentioned in the PR description (GPU and Metrics support) to remove these workarounds.

---

💡 Minor Observations

1. Test Coverage: The codecov report shows 86.26% coverage with 122 lines missing. Key areas needing coverage:

   • instance-form.component.ts (36 lines)
   • device-actions-menu.component.ts (17 lines)
   • instance-nic-form-dialog.component.ts (13 lines)

2. Error Handling: Excellent use of catchError and ErrorHandlerService throughout the codebase.

3. Store Pattern: The ComponentStore usage is solid, with proper initialization patterns and reactivity.

4. Type Safety: Good TypeScript practices with proper interfaces and type safety throughout.

---

📋 Recommendations

High Priority:

• ✅ USB device filtering - Fixed in latest commit
• ✅ Shell button reactivity - Fixed in latest commit

Medium Priority:

• Migrate from @ngneat/until-destroy to takeUntilDestroyed() (23 files)
• Replace @HostListener with host metadata in instance-form

Low Priority:

• Standardize on inject() function over constructor injection
• Increase test coverage for key components

---

🎯 Overall Assessment

This is a well-executed API migration with clean architecture and proper use of modern Angular patterns. The two issues raised by william-gr have been addressed in the latest commit. The remaining items are mostly stylistic consistency issues that can be addressed in follow-up work.

[codecov[bot]]

Codecov Report

:x: Patch coverage is 86.51452% with 130 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 86.25%. Comparing base (c1ad331) to head (f8dc79c). :white_check_mark: All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...omponents/instance-form/instance-form.component.ts	85.60%	36 Missing :warning:
...vice-actions-menu/device-actions-menu.component.ts	66.66%	17 Missing :warning:
...-form-dialog/instance-nic-form-dialog.component.ts	80.59%	13 Missing :warning:
...ages/instances/stores/container-instances.store.ts	63.33%	11 Missing :warning:
...lect-image-dialog/select-image-dialog.component.ts	76.19%	10 Missing :warning:
...lk-actions/instance-list-bulk-actions.component.ts	56.25%	7 Missing :warning:
...d-usb-device-menu/add-usb-device-menu.component.ts	89.28%	6 Missing :warning:
...pages/instances/utils/storage-device-icon.utils.ts	80.00%	5 Missing :warning:
.../pages/instances/stores/container-devices.store.ts	89.74%	4 Missing :warning:
...nents/common/utils/get-device-description.utils.ts	85.71%	3 Missing :warning:
... and 9 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #12620      +/-   ##
==========================================
- Coverage   86.43%   86.25%   -0.18%     
==========================================
  Files        1825     1812      -13     
  Lines       67985    67334     -651     
  Branches     8231     8259      +28     
==========================================
- Hits        58761    58080     -681     
- Misses       9224     9254      +30     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

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

[AlexKarpov98]

@william-gr I made quick updates to move to SlideIn instead of Wizard. Will address comments later. 🙌

[william-gr]

@william-gr I made quick updates to move to SlideIn instead of Wizard. Will address comments later. 🙌

at least Virtual CPUs and memory we should have in the default view, I think, but I like this way better.

[AlexKarpov98]

@william-gr comments addressed :) Let's wait for API changes now.

[AlexKarpov98]

@william-gr good catches 👍 (fixed)

[william-gr]

Thanks! For the next set of fixes I believe you can create another PR that merges into this branch, as we discussed.

[william-gr]

One more issue I found If you have the container stopped and then start it, shell will remain disabled:

[AlexKarpov98]

@william-gr did some updates, could you check it now, please? I mock API for USBs:

USB + SHELL:

https://github.com/user-attachments/assets/6cb2a5ae-3614-4d61-9563-a23904fe8b92