diff --git a/.gitignore b/.gitignore index d614f007a46..370d536a893 100644 --- a/.gitignore +++ b/.gitignore @@ -110,3 +110,4 @@ src/assets/scripts/ie-support/ie-polyfills.min.js # Sentry Config File .sentryclirc +src/app/testing-configs/* diff --git a/src/app/helpers/form-array-snapshot.helper.spec.ts b/src/app/helpers/form-array-snapshot.helper.spec.ts new file mode 100644 index 00000000000..1af582a2a7f --- /dev/null +++ b/src/app/helpers/form-array-snapshot.helper.spec.ts @@ -0,0 +1,258 @@ +import { DestroyRef } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { FormBuilder } from '@ngneat/reactive-forms'; +import { createFormArraySnapshot } from './form-array-snapshot.helper'; + +describe('createFormArraySnapshot', () => { + let fb: FormBuilder; + let destroyRef: DestroyRef; + + beforeEach(() => { + fb = new FormBuilder(); + destroyRef = TestBed.inject(DestroyRef); + }); + + it('initializes with form array current value', () => { + const formArray = fb.array([ + fb.control('item1'), + fb.control('item2'), + ]); + + const snapshot = createFormArraySnapshot(formArray, destroyRef); + + expect(snapshot()).toEqual(['item1', 'item2']); + }); + + it('initializes with custom initial value when provided', () => { + const formArray = fb.array([ + fb.control('item1'), + ]); + + const snapshot = createFormArraySnapshot(formArray, destroyRef, ['custom']); + + expect(snapshot()).toEqual(['custom']); + }); + + it('updates snapshot when form array value changes', () => { + const formArray = fb.array([ + fb.control('item1'), + ]); + + const snapshot = createFormArraySnapshot(formArray, destroyRef); + + expect(snapshot()).toEqual(['item1']); + + // Add item + formArray.push(fb.control('item2')); + expect(snapshot()).toEqual(['item1', 'item2']); + + // Update item + formArray.at(0).setValue('updated'); + expect(snapshot()).toEqual(['updated', 'item2']); + + // Remove item + formArray.removeAt(1); + expect(snapshot()).toEqual(['updated']); + }); + + it('handles form array with objects', () => { + interface Item { + name: string; + value: number; + } + + const formArray = fb.array([ + fb.group({ name: fb.control('test'), value: fb.control(1) }), + ]); + + const snapshot = createFormArraySnapshot(formArray, destroyRef); + + expect(snapshot()).toEqual([{ name: 'test', value: 1 }]); + + formArray.push(fb.group({ name: fb.control('test2'), value: fb.control(2) })); + expect(snapshot()).toEqual([ + { name: 'test', value: 1 }, + { name: 'test2', value: 2 }, + ]); + }); + + it('handles empty form array', () => { + const formArray = fb.array([]); + + const snapshot = createFormArraySnapshot(formArray, destroyRef); + + expect(snapshot()).toEqual([]); + + formArray.push(fb.control('new')); + expect(snapshot()).toEqual(['new']); + }); + + it('captures disabled control values with getRawValue', () => { + const formArray = fb.array([ + fb.control({ value: 'enabled', disabled: false }), + fb.control({ value: 'disabled', disabled: true }), + ]); + + const snapshot = createFormArraySnapshot(formArray, destroyRef); + + // getRawValue includes disabled controls + expect(snapshot()).toEqual(['enabled', 'disabled']); + }); + + it('updates snapshot when form array is cleared', () => { + const formArray = fb.array([ + fb.control('item1'), + fb.control('item2'), + fb.control('item3'), + ]); + + const snapshot = createFormArraySnapshot(formArray, destroyRef); + + expect(snapshot()).toEqual(['item1', 'item2', 'item3']); + + formArray.clear(); + expect(snapshot()).toEqual([]); + }); + + it('updates snapshot when form array values are patched', () => { + const formArray = fb.array([ + fb.group({ name: fb.control('test1'), value: fb.control(1) }), + fb.group({ name: fb.control('test2'), value: fb.control(2) }), + ]); + + const snapshot = createFormArraySnapshot(formArray, destroyRef); + + expect(snapshot()).toEqual([ + { name: 'test1', value: 1 }, + { name: 'test2', value: 2 }, + ]); + + // Patch individual item + formArray.at(0).patchValue({ name: 'updated' }); + expect(snapshot()).toEqual([ + { name: 'updated', value: 1 }, + { name: 'test2', value: 2 }, + ]); + }); + + it('updates snapshot when form array is reset', () => { + const formArray = fb.array([ + fb.control('item1'), + fb.control('item2'), + ]); + + const snapshot = createFormArraySnapshot(formArray, destroyRef); + + // Make changes + formArray.at(0).setValue('changed'); + expect(snapshot()).toEqual(['changed', 'item2']); + + // Reset + formArray.reset(); + expect(snapshot()).toEqual([null, null]); + }); + + it('handles multiple rapid changes correctly', () => { + const formArray = fb.array([fb.control('initial')]); + const snapshot = createFormArraySnapshot(formArray, destroyRef); + + // Multiple rapid changes + formArray.push(fb.control('second')); + formArray.push(fb.control('third')); + formArray.removeAt(0); + formArray.at(0).setValue('modified'); + + // Should capture the final state + expect(snapshot()).toEqual(['modified', 'third']); + }); + + it('handles nested form groups with complex structures', () => { + interface ComplexItem { + port: string | null; + host_id: number | null; + } + + const formArray = fb.array([ + fb.group({ + port: fb.control('fc0'), + host_id: fb.control(null), + }), + fb.group({ + port: fb.control(null), + host_id: fb.control(123), + }), + ]); + + const snapshot = createFormArraySnapshot(formArray, destroyRef); + + expect(snapshot()).toEqual([ + { port: 'fc0', host_id: null }, + { port: null, host_id: 123 }, + ]); + + // Modify nested values + formArray.at(0).patchValue({ port: 'fc1' }); + expect(snapshot()).toEqual([ + { port: 'fc1', host_id: null }, + { port: null, host_id: 123 }, + ]); + }); + + it('works correctly with mixed disabled/enabled controls', () => { + const formArray = fb.array([ + fb.group({ + port: fb.control({ value: 'fc0', disabled: false }), + host_id: fb.control({ value: null, disabled: true }), + }), + fb.group({ + port: fb.control({ value: null, disabled: true }), + host_id: fb.control({ value: 456, disabled: false }), + }), + ]); + + const snapshot = createFormArraySnapshot(formArray, destroyRef); + + // getRawValue should include all values regardless of disabled state + expect(snapshot()).toEqual([ + { port: 'fc0', host_id: null }, + { port: null, host_id: 456 }, + ]); + + // Enable a disabled control and change value + formArray.at(0).controls.host_id.enable(); + formArray.at(0).controls.host_id.setValue(789); + + expect(snapshot()).toEqual([ + { port: 'fc0', host_id: 789 }, + { port: null, host_id: 456 }, + ]); + }); + + it('maintains separate snapshots for different form arrays', () => { + const formArray1 = fb.array([fb.control('array1-item')]); + const formArray2 = fb.array([fb.control('array2-item')]); + + const snapshot1 = createFormArraySnapshot(formArray1, destroyRef); + const snapshot2 = createFormArraySnapshot(formArray2, destroyRef); + + expect(snapshot1()).toEqual(['array1-item']); + expect(snapshot2()).toEqual(['array2-item']); + + // Modify one, other should be unchanged + formArray1.push(fb.control('array1-new')); + expect(snapshot1()).toEqual(['array1-item', 'array1-new']); + expect(snapshot2()).toEqual(['array2-item']); // Unchanged + }); + + it('handles form arrays with null and undefined values', () => { + const formArray = fb.array([ + fb.control(null), + fb.control(), + fb.control('valid'), + ]); + + const snapshot = createFormArraySnapshot(formArray, destroyRef); + + expect(snapshot()).toEqual([null, null, 'valid']); // undefined becomes null in forms + }); +}); diff --git a/src/app/helpers/form-array-snapshot.helper.ts b/src/app/helpers/form-array-snapshot.helper.ts new file mode 100644 index 00000000000..e2c82d64fa6 --- /dev/null +++ b/src/app/helpers/form-array-snapshot.helper.ts @@ -0,0 +1,56 @@ +import { DestroyRef, signal, WritableSignal } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; +import { FormArray } from '@ngneat/reactive-forms'; + +/** + * Creates a signal that tracks form array changes to prevent + * infinite change detection loops when using signals with reactive forms. + * + * This helper solves a common problem when mixing signals and reactive forms: + * - Reading form array values directly in a computed signal causes circular reactivity + * - Changes trigger re-computation, which can cause infinite loops + * - The snapshot pattern breaks the cycle by updating only on valueChanges emissions + * + * @param formArray The FormArray to track + * @param destroyRef DestroyRef for automatic cleanup + * @param initialValue Optional initial value (defaults to formArray.getRawValue()) + * @returns A signal that updates whenever the form array changes + * + * @example + * ```typescript + * class MyComponent { + * private destroyRef = inject(DestroyRef); + * + * form = this.fb.group({ + * items: this.fb.array([]) + * }); + * + * // Create snapshot that tracks form changes + * itemsSnapshot = createFormArraySnapshot( + * this.form.controls.items, + * this.destroyRef + * ); + * + * // Use in computed signals safely + * processedItems = computed(() => { + * return this.itemsSnapshot().map(item => transform(item)); + * }); + * } + * ``` + */ +export function createFormArraySnapshot( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + formArray: FormArray, + destroyRef: DestroyRef, + initialValue?: T[], +): WritableSignal { + const snapshot = signal(initialValue ?? formArray.getRawValue() as T[]); + + formArray.valueChanges.pipe( + takeUntilDestroyed(destroyRef), + ).subscribe(() => { + snapshot.set(formArray.getRawValue() as T[]); + }); + + return snapshot; +} diff --git a/src/app/interfaces/fibre-channel.interface.ts b/src/app/interfaces/fibre-channel.interface.ts index ec23ff676b0..126b51239b1 100644 --- a/src/app/interfaces/fibre-channel.interface.ts +++ b/src/app/interfaces/fibre-channel.interface.ts @@ -48,3 +48,8 @@ export interface FibreChannelHost { wwpn_b: string; npiv: number; } + +export interface FcPortFormValue { + port: string | null; + host_id: number | null; +} diff --git a/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component.html b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component.html new file mode 100644 index 00000000000..5f62fc3419e --- /dev/null +++ b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component.html @@ -0,0 +1,11 @@ +
+
+
+ + {{ 'MPIO Configuration' | translate }} +
+

+ {{ 'MPIO is supported. Each Fibre Channel port must use a unique physical port.' | translate }} +

+
+
diff --git a/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component.scss b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component.scss new file mode 100644 index 00000000000..9b64a2d0bd0 --- /dev/null +++ b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component.scss @@ -0,0 +1,32 @@ +.mpio-info-banner { + margin-top: 16px; + margin-bottom: 16px; +} + +.info-box { + background-color: var(--alt-bg1); + border: 1px solid var(--lines); + border-left: 4px solid var(--primary); + border-radius: 4px; + padding: 16px; +} + +.info-header { + align-items: center; + color: var(--primary); + display: flex; + font-size: 16px; + font-weight: 600; + gap: 8px; + margin-bottom: 8px; + + ix-icon { + font-size: 20px; + } +} + +.info-message { + color: var(--fg2); + line-height: 1.5; + margin: 0; +} diff --git a/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component.spec.ts b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component.spec.ts new file mode 100644 index 00000000000..a03b23208f6 --- /dev/null +++ b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component.spec.ts @@ -0,0 +1,33 @@ +import { createComponentFactory, Spectator } from '@ngneat/spectator/jest'; +import { FcMpioInfoBannerComponent } from './fc-mpio-info-banner.component'; + +describe('FcMpioInfoBannerComponent', () => { + let spectator: Spectator; + const createComponent = createComponentFactory({ + component: FcMpioInfoBannerComponent, + }); + + beforeEach(() => { + spectator = createComponent(); + }); + + it('displays the MPIO information banner', () => { + expect(spectator.query('.mpio-info-banner')).toExist(); + }); + + it('displays the information icon', () => { + const icon = spectator.query('ix-icon'); + expect(icon).toExist(); + expect(icon.getAttribute('name')).toBe('mdi-information-outline'); + }); + + it('displays the MPIO configuration title', () => { + expect(spectator.query('.info-header')).toContainText('MPIO Configuration'); + }); + + it('displays the information message', () => { + const message = spectator.query('.info-message'); + expect(message).toContainText('MPIO'); + expect(message).toContainText('unique physical port'); + }); +}); diff --git a/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component.ts b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component.ts new file mode 100644 index 00000000000..3f404affaa7 --- /dev/null +++ b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component.ts @@ -0,0 +1,17 @@ +import { + ChangeDetectionStrategy, Component, +} from '@angular/core'; +import { TranslateModule } from '@ngx-translate/core'; +import { IxIconComponent } from 'app/modules/ix-icon/ix-icon.component'; + +@Component({ + selector: 'ix-fc-mpio-info-banner', + templateUrl: './fc-mpio-info-banner.component.html', + styleUrls: ['./fc-mpio-info-banner.component.scss'], + changeDetection: ChangeDetectionStrategy.OnPush, + imports: [ + IxIconComponent, + TranslateModule, + ], +}) +export class FcMpioInfoBannerComponent {} diff --git a/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-port-item-controls/fc-port-item-controls.component.html b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-port-item-controls/fc-port-item-controls.component.html new file mode 100644 index 00000000000..8ba658d3a0b --- /dev/null +++ b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-port-item-controls/fc-port-item-controls.component.html @@ -0,0 +1,24 @@ + + +@if (modeControl.value === 'new') { + +} @else if (modeControl.value === 'existing') { + +} diff --git a/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-port-item-controls/fc-port-item-controls.component.spec.ts b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-port-item-controls/fc-port-item-controls.component.spec.ts new file mode 100644 index 00000000000..3fcc328e71a --- /dev/null +++ b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-port-item-controls/fc-port-item-controls.component.spec.ts @@ -0,0 +1,443 @@ +import { HarnessLoader } from '@angular/cdk/testing'; +import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed'; +import { FormBuilder, FormControl, FormGroup } from '@ngneat/reactive-forms'; +import { createComponentFactory, Spectator } from '@ngneat/spectator/jest'; +import { mockApi, mockCall } from 'app/core/testing/utils/mock-api.utils'; +import { FibreChannelHost, FibreChannelPortChoices } from 'app/interfaces/fibre-channel.interface'; +import { IxSelectHarness } from 'app/modules/forms/ix-forms/components/ix-select/ix-select.harness'; +import { ApiService } from 'app/modules/websocket/api.service'; +import { FcPortItemControlsComponent } from './fc-port-item-controls.component'; + +describe('FcPortItemControlsComponent', () => { + let spectator: Spectator; + let loader: HarnessLoader; + let mockForm: FormGroup<{ + port: FormControl; + host_id: FormControl; + }>; + + const mockPortChoices: FibreChannelPortChoices = { + fc0: { wwpn: '10:00:00:00:c9:20:00:00', wwpn_b: '10:00:00:00:c9:20:00:01' }, + fc1: { wwpn: '10:00:00:00:c9:30:00:00', wwpn_b: '10:00:00:00:c9:30:00:01' }, + }; + + const mockHosts: FibreChannelHost[] = [ + { + id: 1, alias: 'fc', npiv: 1, wwpn: '10:00:00:00:c9:20:00:00', wwpn_b: '10:00:00:00:c9:20:00:01', + }, + { + id: 2, alias: 'fc1', npiv: 0, wwpn: '10:00:00:00:c9:30:00:00', wwpn_b: '10:00:00:00:c9:30:00:01', + }, + ]; + + const createComponent = createComponentFactory({ + component: FcPortItemControlsComponent, + providers: [ + FormBuilder, + mockApi([ + mockCall('fcport.port_choices', mockPortChoices), + mockCall('fc.fc_host.query', mockHosts), + ]), + ], + }); + + beforeEach(() => { + const fb = new FormBuilder(); + mockForm = fb.group({ + port: fb.control(null), + host_id: fb.control(null), + }); + + spectator = createComponent({ + props: { + form: mockForm, + isEdit: false, + currentPort: null, + usedPhysicalPorts: [], + availablePorts: ['fc0', 'fc1', 'fc2'], + }, + }); + loader = TestbedHarnessEnvironment.loader(spectator.fixture); + }); + + describe('initialization', () => { + it('creates component successfully', () => { + expect(spectator.component).toBeTruthy(); + }); + + it('defaults to "existing" mode and shows existing port selector', async () => { + const modeSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Port Mode' })); + expect(await modeSelect.getValue()).toBe('Use existing port'); + + // Verify existing port selector is visible + const portSelect = await loader.getHarnessOrNull(IxSelectHarness.with({ label: 'Existing Port' })); + expect(portSelect).toBeTruthy(); + + // Verify new port selector is NOT visible + const hostSelect = await loader.getHarnessOrNull(IxSelectHarness.with({ label: 'Choose Host for New Virtual Port' })); + expect(hostSelect).toBeNull(); + }); + + it('loads existing port options correctly', async () => { + const portSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Existing Port' })); + const options = await portSelect.getOptionLabels(); + + expect(options).toContain('fc0'); + expect(options).toContain('fc1'); + }); + }); + + describe('mode switching', () => { + it('switches to "new" mode and shows host selector when user selects create new virtual port', async () => { + const modeSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Port Mode' })); + await modeSelect.setValue('Create new virtual port'); + + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + // Verify new port selector is now visible + const hostSelect = await loader.getHarnessOrNull(IxSelectHarness.with({ label: 'Choose Host for New Virtual Port' })); + expect(hostSelect).toBeTruthy(); + + // Verify existing port selector is hidden + const portSelect = await loader.getHarnessOrNull(IxSelectHarness.with({ label: 'Existing Port' })); + expect(portSelect).toBeNull(); + }); + + it('loads host options correctly in new mode', async () => { + const modeSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Port Mode' })); + await modeSelect.setValue('Create new virtual port'); + + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + const hostSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Choose Host for New Virtual Port' })); + const options = await hostSelect.getOptionLabels(); + + expect(options).toContain('fc/2'); + expect(options).toContain('fc1/1'); + }); + + it('switches back to "existing" mode and shows port selector', async () => { + const modeSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Port Mode' })); + + // Switch to new mode first + await modeSelect.setValue('Create new virtual port'); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + // Switch back to existing + await modeSelect.setValue('Use existing port'); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + // Verify existing port selector is visible again + const portSelect = await loader.getHarnessOrNull(IxSelectHarness.with({ label: 'Existing Port' })); + expect(portSelect).toBeTruthy(); + + // Verify new port selector is hidden + const hostSelect = await loader.getHarnessOrNull(IxSelectHarness.with({ label: 'Choose Host for New Virtual Port' })); + expect(hostSelect).toBeNull(); + }); + + it('clears port value when switching to "new" mode', async () => { + // First select a port in existing mode + const portSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Existing Port' })); + await portSelect.setValue('fc0'); + expect(mockForm.controls.port.value).toBe('fc0'); + + // Switch to new mode + const modeSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Port Mode' })); + await modeSelect.setValue('Create new virtual port'); + spectator.detectChanges(); + + // Port value should be cleared + expect(mockForm.controls.port.value).toBeNull(); + }); + + it('clears host_id value when switching to "existing" mode', async () => { + // Switch to new mode and select a host + const modeSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Port Mode' })); + await modeSelect.setValue('Create new virtual port'); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + const hostSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Choose Host for New Virtual Port' })); + await hostSelect.setValue('fc/2'); + expect(mockForm.controls.host_id.value).toBe(1); + + // Switch to existing mode + await modeSelect.setValue('Use existing port'); + spectator.detectChanges(); + + // Host_id value should be cleared + expect(mockForm.controls.host_id.value).toBeNull(); + }); + }); + + describe('edit mode', () => { + beforeEach(async () => { + const fb = new FormBuilder(); + mockForm = fb.group({ + port: fb.control(null), + host_id: fb.control(null), + }); + + spectator = createComponent({ + props: { + form: mockForm, + isEdit: true, + currentPort: 'fc0', + usedPhysicalPorts: [], + availablePorts: ['fc0', 'fc1', 'fc2'], + }, + }); + loader = TestbedHarnessEnvironment.loader(spectator.fixture); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + }); + + it('prefills port control with currentPort value', async () => { + expect(mockForm.controls.port.value).toBe('fc0'); + + const portSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Existing Port' })); + expect(await portSelect.getValue()).toBe('fc0'); + }); + + it('defaults to "existing" mode in edit mode', async () => { + const modeSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Port Mode' })); + expect(await modeSelect.getValue()).toBe('Use existing port'); + }); + + it('shows existing port selector in edit mode', async () => { + const portSelect = await loader.getHarnessOrNull(IxSelectHarness.with({ label: 'Existing Port' })); + expect(portSelect).toBeTruthy(); + }); + }); + + describe('form integration', () => { + it('updates parent form when user selects a port in existing mode', async () => { + const portSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Existing Port' })); + await portSelect.setValue('fc1'); + + expect(mockForm.controls.port.value).toBe('fc1'); + }); + + it('updates parent form when user selects a host in new mode', async () => { + const modeSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Port Mode' })); + await modeSelect.setValue('Create new virtual port'); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + const hostSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Choose Host for New Virtual Port' })); + await hostSelect.setValue('fc/2'); + + expect(mockForm.controls.host_id.value).toBe(1); + }); + }); + + describe('validation', () => { + it('form is invalid when mode is "existing" and no port selected', () => { + // Port is null by default in existing mode + expect(mockForm.invalid).toBe(true); + expect(mockForm.controls.port.hasError('required')).toBe(true); + }); + + it('form is valid when mode is "existing" and port selected', async () => { + const portSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Existing Port' })); + await portSelect.setValue('fc0'); + + expect(mockForm.valid).toBe(true); + }); + + it('form is invalid when mode is "new" and no host_id selected', async () => { + const modeSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Port Mode' })); + await modeSelect.setValue('Create new virtual port'); + spectator.detectChanges(); + + expect(mockForm.invalid).toBe(true); + expect(mockForm.controls.host_id.hasError('required')).toBe(true); + }); + + it('form is valid when mode is "new" and host_id selected', async () => { + const modeSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Port Mode' })); + await modeSelect.setValue('Create new virtual port'); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + const hostSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Choose Host for New Virtual Port' })); + await hostSelect.setValue('fc/2'); + + expect(mockForm.valid).toBe(true); + }); + }); + + describe('API data loading', () => { + it('calls fc.fc_host.query', () => { + const apiService = spectator.inject(ApiService); + expect(apiService.call).toHaveBeenCalledWith('fc.fc_host.query'); + }); + }); + + describe('validator management', () => { + it('does not accumulate validators on repeated mode switches', async () => { + const modeSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Port Mode' })); + + // Switch modes multiple times + await modeSelect.setValue('Create new virtual port'); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + await modeSelect.setValue('Use existing port'); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + await modeSelect.setValue('Create new virtual port'); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + await modeSelect.setValue('Use existing port'); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + // Verify form validity works correctly after multiple switches + expect(mockForm.invalid).toBe(true); // port is required but null + + const portSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Existing Port' })); + await portSelect.setValue('fc0'); + + expect(mockForm.valid).toBe(true); // Should be valid with one port selected + }); + }); + + describe('port filtering behavior', () => { + it('filters out ports with same physical port prefix as usedPhysicalPorts', async () => { + spectator.setInput('availablePorts', ['fc0', 'fc0/1', 'fc1', 'fc2']); + spectator.setInput('usedPhysicalPorts', ['fc0']); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + const portSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Existing Port' })); + const options = await portSelect.getOptionLabels(); + + // Should NOT include fc0 or fc0/1 (same physical port) + expect(options).not.toContain('fc0'); + expect(options).not.toContain('fc0/1'); + + // Should include fc1 and fc2 (different physical ports) + expect(options).toContain('fc1'); + expect(options).toContain('fc2'); + }); + + it('filters out virtual ports sharing physical port with used ports', async () => { + spectator.setInput('availablePorts', ['fc0', 'fc0/1', 'fc0/2', 'fc1']); + spectator.setInput('usedPhysicalPorts', ['fc0']); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + const portSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Existing Port' })); + const options = await portSelect.getOptionLabels(); + + // Should NOT include any fc0 variants + expect(options).not.toContain('fc0'); + expect(options).not.toContain('fc0/1'); + expect(options).not.toContain('fc0/2'); + + // Should only include fc1 + expect(options).toContain('fc1'); + expect(options).toHaveLength(1); + }); + + it('shows all ports when usedPhysicalPorts is empty', async () => { + spectator.setInput('availablePorts', ['fc0', 'fc1', 'fc2']); + spectator.setInput('usedPhysicalPorts', []); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + const portSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Existing Port' })); + const options = await portSelect.getOptionLabels(); + + // Should include all available ports + expect(options).toContain('fc0'); + expect(options).toContain('fc1'); + expect(options).toContain('fc2'); + }); + + it('always includes currentPort in edit mode even if filtered', async () => { + spectator.setInput('availablePorts', ['fc0', 'fc1', 'fc2']); + spectator.setInput('usedPhysicalPorts', ['fc0']); // Would normally filter fc0 + spectator.setInput('currentPort', 'fc0'); + spectator.setInput('isEdit', true); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + const portSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Existing Port' })); + const options = await portSelect.getOptionLabels(); + + // Should include fc0 (current port) even though it's in usedPhysicalPorts + expect(options).toContain('fc0'); + + // Should also include other non-conflicting ports + expect(options).toContain('fc1'); + expect(options).toContain('fc2'); + }); + }); + + describe('reactivity to input changes', () => { + it('updates dropdown options when usedPhysicalPorts changes', async () => { + // Initial state: fc0 is used + spectator.setInput('availablePorts', ['fc0', 'fc1', 'fc2', 'fc3']); + spectator.setInput('usedPhysicalPorts', ['fc0']); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + const portSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Existing Port' })); + let options = await portSelect.getOptionLabels(); + + // Should not include fc0 + expect(options).not.toContain('fc0'); + expect(options).toContain('fc1'); + expect(options).toContain('fc2'); + + // Change to: fc2 is used instead + spectator.setInput('usedPhysicalPorts', ['fc2']); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + options = await portSelect.getOptionLabels(); + + // Should now exclude fc2 and show fc0 + expect(options).toContain('fc0'); + expect(options).toContain('fc1'); + expect(options).not.toContain('fc2'); + expect(options).toContain('fc3'); + }); + + it('updates dropdown options when availablePorts changes', async () => { + // Initial state: limited ports + spectator.setInput('availablePorts', ['fc0', 'fc1']); + spectator.setInput('usedPhysicalPorts', []); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + const portSelect = await loader.getHarness(IxSelectHarness.with({ label: 'Existing Port' })); + let options = await portSelect.getOptionLabels(); + + expect(options).toContain('fc0'); + expect(options).toContain('fc1'); + expect(options).not.toContain('fc2'); + + // Add fc2 to available ports + spectator.setInput('availablePorts', ['fc0', 'fc1', 'fc2']); + spectator.detectChanges(); + await spectator.fixture.whenStable(); + + options = await portSelect.getOptionLabels(); + + // Should now include fc2 + expect(options).toContain('fc0'); + expect(options).toContain('fc1'); + expect(options).toContain('fc2'); + }); + }); +}); diff --git a/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-port-item-controls/fc-port-item-controls.component.ts b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-port-item-controls/fc-port-item-controls.component.ts new file mode 100644 index 00000000000..5589e3f3252 --- /dev/null +++ b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-port-item-controls/fc-port-item-controls.component.ts @@ -0,0 +1,101 @@ +import { + ChangeDetectionStrategy, Component, computed, DestroyRef, effect, inject, input, OnInit, +} from '@angular/core'; +import { takeUntilDestroyed, toObservable } from '@angular/core/rxjs-interop'; +import { ReactiveFormsModule } from '@angular/forms'; +import { FormBuilder, FormControl, FormGroup } from '@ngneat/reactive-forms'; +import { TranslateModule, TranslateService } from '@ngx-translate/core'; +import { map, of } from 'rxjs'; +import { Option } from 'app/interfaces/option.interface'; +import { IxSelectComponent } from 'app/modules/forms/ix-forms/components/ix-select/ix-select.component'; +import { ApiService } from 'app/modules/websocket/api.service'; +import { configurePortControlsForMode } from 'app/pages/sharing/iscsi/fibre-channel-ports/helpers/port-mode-control.helper'; + +@Component({ + selector: 'ix-fc-port-item-controls', + templateUrl: './fc-port-item-controls.component.html', + changeDetection: ChangeDetectionStrategy.OnPush, + imports: [ + ReactiveFormsModule, + IxSelectComponent, + TranslateModule, + ], +}) +export class FcPortItemControlsComponent implements OnInit { + private api = inject(ApiService); + private fb = inject(FormBuilder); + private destroyRef = inject(DestroyRef); + private translate = inject(TranslateService); + + // Inputs from parent + readonly form = input.required; + host_id: FormControl; + }>>(); + + readonly isEdit = input(false); + readonly currentPort = input(null); + readonly usedPhysicalPorts = input.required(); + readonly availablePorts = input.required(); + + // Local mode control (not part of parent form) + modeControl = this.fb.control<'existing' | 'new'>('existing'); + + // Mode options for dropdown + readonly modeOptions$ = of([ + { label: this.translate.instant('Use existing port'), value: 'existing' }, + { label: this.translate.instant('Create new virtual port'), value: 'new' }, + ] as Option[]); + + // Data sources (computed signal converted to observable for ix-select compatibility) + readonly existingPortOptions = computed(() => { + const availablePorts = this.availablePorts(); + const usedPhysicalPorts = this.usedPhysicalPorts(); + const currentPort = this.currentPort(); + + // Filter out ports that share physical port prefix with OTHER selections + let options = availablePorts + .filter((port) => { + const portPhysicalPrefix = port.split('/')[0]; + return !usedPhysicalPorts.includes(portPhysicalPrefix); + }) + .map((value) => ({ label: value, value } as Option)); + + // Add current port in edit mode (always show current selection) + if (this.isEdit() && currentPort && !options.some((option) => option.value === currentPort)) { + options = [{ label: currentPort, value: currentPort }, ...options]; + } + + return options; + }); + + readonly existingPortOptions$ = toObservable(this.existingPortOptions); + + readonly creatingPortOptions$ = this.api.call('fc.fc_host.query').pipe( + map((hosts) => hosts.map((host) => ({ + label: `${host.alias}/${host.npiv + 1}`, + value: host.id, + } as Option))), + ); + + constructor() { + // Initialize mode based on edit state + effect(() => { + const currentPortValue = this.currentPort(); + if (this.isEdit() && currentPortValue) { + this.modeControl.setValue('existing'); + this.form().controls.port.setValue(currentPortValue); + this.form().controls.host_id.setValue(null); + } + }); + } + + ngOnInit(): void { + // Handle mode switching with helper + this.modeControl.valueChanges.pipe( + takeUntilDestroyed(this.destroyRef), + ).subscribe((mode) => { + configurePortControlsForMode(mode, this.form().controls); + }); + } +} diff --git a/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-ports-controls/fc-ports-controls.component.html b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-ports-controls/fc-ports-controls.component.html deleted file mode 100644 index 442ad9a40d5..00000000000 --- a/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-ports-controls/fc-ports-controls.component.html +++ /dev/null @@ -1,21 +0,0 @@ - - - @if (optionsControl.value === FibrePortOption.CreateNewPort) { - - } @else if (optionsControl.value === FibrePortOption.UseExistingPort) { - - } - diff --git a/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-ports-controls/fc-ports-controls.component.ts b/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-ports-controls/fc-ports-controls.component.ts deleted file mode 100644 index 4f00b05b832..00000000000 --- a/src/app/pages/sharing/iscsi/fibre-channel-ports/fc-ports-controls/fc-ports-controls.component.ts +++ /dev/null @@ -1,107 +0,0 @@ -import { ChangeDetectionStrategy, Component, computed, effect, input, OnInit, inject } from '@angular/core'; -import { ReactiveFormsModule } from '@angular/forms'; -import { FormBuilder } from '@ngneat/reactive-forms'; -import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy'; -import { TranslateService, TranslateModule } from '@ngx-translate/core'; -import { map, of } from 'rxjs'; -import { IxRadioGroupComponent } from 'app/modules/forms/ix-forms/components/ix-radio-group/ix-radio-group.component'; -import { IxSelectComponent } from 'app/modules/forms/ix-forms/components/ix-select/ix-select.component'; -import { ApiService } from 'app/modules/websocket/api.service'; -import { TargetFormComponent } from 'app/pages/sharing/iscsi/target/target-form/target-form.component'; - -export enum FibrePortOption { - DoNotConnect = 1, - UseExistingPort = 2, - CreateNewPort = 3, - KeepCurrentPort = 4, -} - -@UntilDestroy() -@Component({ - selector: 'ix-fc-ports-controls', - templateUrl: './fc-ports-controls.component.html', - changeDetection: ChangeDetectionStrategy.OnPush, - imports: [ - ReactiveFormsModule, - IxSelectComponent, - IxRadioGroupComponent, - TranslateModule, - ], -}) -export class FcPortsControlsComponent implements OnInit { - private fb = inject(FormBuilder); - private api = inject(ApiService); - private translate = inject(TranslateService); - - form = input.required(); - isEdit = input(false); - currentPort = input(null); - - optionsControl = this.fb.control(FibrePortOption.DoNotConnect); - - protected readonly FibrePortOption = FibrePortOption; - - readonly portOptions = computed(() => { - const baseOptions = [ - { label: this.translate.instant('Do not connect to a fibre channel port'), value: FibrePortOption.DoNotConnect }, - { label: this.translate.instant('Use an existing port'), value: FibrePortOption.UseExistingPort }, - { label: this.translate.instant('Create new virtual port'), value: FibrePortOption.CreateNewPort }, - ]; - - if (this.isEdit() && this.currentPort()) { - return of([ - { - label: this.translate.instant('Keep current port {port}', { port: this.form().controls.port.value }), - value: FibrePortOption.KeepCurrentPort, - }, - ...baseOptions, - ]); - } - - return of(baseOptions); - }); - - readonly creatingPortOptions$ = this.api.call('fc.fc_host.query').pipe(map((hosts) => { - return hosts.map((host) => ({ - label: this.translate.instant('Create {port}', { port: `${host.alias}/${host.npiv + 1}` }), - value: host.id, - })); - })); - - readonly existingPortOptions$ = this.api.call('fcport.port_choices', [false]).pipe(map((ports) => { - return Object.entries(ports).map(([value]) => ({ label: value, value })); - })); - - constructor() { - effect(() => { - if (this.isEdit() && this.currentPort()) { - this.optionsControl.setValue(FibrePortOption.KeepCurrentPort); - this.form().controls.port.setValue(this.currentPort()); - } - }); - } - - ngOnInit(): void { - this.form().controls.host_id.disable(); - - this.listenToOptionControlChanges(); - } - - private listenToOptionControlChanges(): void { - this.optionsControl.valueChanges.pipe(untilDestroyed(this)).subscribe((option) => { - this.form().controls.port.disable(); - this.form().controls.host_id.disable(); - - if (option === FibrePortOption.DoNotConnect) { - this.form().controls.port.setValue(null); - this.form().controls.host_id.setValue(null); - } else if (option === FibrePortOption.CreateNewPort) { - this.form().controls.host_id.enable(); - this.form().controls.port.setValue(null); - } else { - this.form().controls.port.enable(); - this.form().controls.host_id.setValue(null); - } - }); - } -} diff --git a/src/app/pages/sharing/iscsi/fibre-channel-ports/helpers/port-mode-control.helper.spec.ts b/src/app/pages/sharing/iscsi/fibre-channel-ports/helpers/port-mode-control.helper.spec.ts new file mode 100644 index 00000000000..fa3ce3f84d0 --- /dev/null +++ b/src/app/pages/sharing/iscsi/fibre-channel-ports/helpers/port-mode-control.helper.spec.ts @@ -0,0 +1,196 @@ +import { FormBuilder } from '@ngneat/reactive-forms'; +import { configurePortControlsForMode, PortModeControls } from './port-mode-control.helper'; + +describe('configurePortControlsForMode', () => { + let fb: FormBuilder; + let controls: PortModeControls; + + beforeEach(() => { + fb = new FormBuilder(); + controls = { + port: fb.control(null), + host_id: fb.control(null), + }; + }); + + describe('existing mode', () => { + it('enables port control and disables host_id control', () => { + configurePortControlsForMode('existing', controls); + + expect(controls.port.enabled).toBe(true); + expect(controls.host_id.disabled).toBe(true); + }); + + it('sets required validator on port control', () => { + configurePortControlsForMode('existing', controls); + + controls.port.setValue(null); + expect(controls.port.hasError('required')).toBe(true); + + controls.port.setValue('fc0'); + expect(controls.port.hasError('required')).toBe(false); + }); + + it('clears validators from host_id control', () => { + // First set validators on host_id + configurePortControlsForMode('new', controls); + controls.host_id.setValue(null); + expect(controls.host_id.hasError('required')).toBe(true); + + // Switch to existing mode + configurePortControlsForMode('existing', controls); + controls.host_id.setValue(null); + expect(controls.host_id.hasError('required')).toBe(false); + }); + + it('clears host_id value', () => { + controls.host_id.setValue(123); + + configurePortControlsForMode('existing', controls); + + expect(controls.host_id.value).toBeNull(); + }); + + it('calls updateValueAndValidity on both controls', () => { + const portSpy = jest.spyOn(controls.port, 'updateValueAndValidity'); + const hostSpy = jest.spyOn(controls.host_id, 'updateValueAndValidity'); + + configurePortControlsForMode('existing', controls); + + expect(portSpy).toHaveBeenCalled(); + expect(hostSpy).toHaveBeenCalled(); + }); + }); + + describe('new mode', () => { + it('enables host_id control and disables port control', () => { + configurePortControlsForMode('new', controls); + + expect(controls.host_id.enabled).toBe(true); + expect(controls.port.disabled).toBe(true); + }); + + it('sets required validator on host_id control', () => { + configurePortControlsForMode('new', controls); + + controls.host_id.setValue(null); + expect(controls.host_id.hasError('required')).toBe(true); + + controls.host_id.setValue(1); + expect(controls.host_id.hasError('required')).toBe(false); + }); + + it('clears validators from port control', () => { + // First set validators on port + configurePortControlsForMode('existing', controls); + controls.port.setValue(null); + expect(controls.port.hasError('required')).toBe(true); + + // Switch to new mode + configurePortControlsForMode('new', controls); + controls.port.setValue(null); + expect(controls.port.hasError('required')).toBe(false); + }); + + it('clears port value', () => { + controls.port.setValue('fc0'); + + configurePortControlsForMode('new', controls); + + expect(controls.port.value).toBeNull(); + }); + + it('calls updateValueAndValidity on both controls', () => { + const portSpy = jest.spyOn(controls.port, 'updateValueAndValidity'); + const hostSpy = jest.spyOn(controls.host_id, 'updateValueAndValidity'); + + configurePortControlsForMode('new', controls); + + expect(portSpy).toHaveBeenCalled(); + expect(hostSpy).toHaveBeenCalled(); + }); + }); + + describe('mode switching', () => { + it('correctly transitions from existing to new mode', () => { + // Start in existing mode + configurePortControlsForMode('existing', controls); + controls.port.setValue('fc0'); + expect(controls.port.value).toBe('fc0'); + expect(controls.port.enabled).toBe(true); + expect(controls.host_id.disabled).toBe(true); + + // Switch to new mode + configurePortControlsForMode('new', controls); + expect(controls.port.value).toBeNull(); // Cleared + expect(controls.port.disabled).toBe(true); + expect(controls.host_id.enabled).toBe(true); + }); + + it('correctly transitions from new to existing mode', () => { + // Start in new mode + configurePortControlsForMode('new', controls); + controls.host_id.setValue(123); + expect(controls.host_id.value).toBe(123); + expect(controls.host_id.enabled).toBe(true); + expect(controls.port.disabled).toBe(true); + + // Switch to existing mode + configurePortControlsForMode('existing', controls); + expect(controls.host_id.value).toBeNull(); // Cleared + expect(controls.host_id.disabled).toBe(true); + expect(controls.port.enabled).toBe(true); + }); + + it('handles multiple mode switches without validator accumulation', () => { + // Switch modes multiple times + configurePortControlsForMode('existing', controls); + configurePortControlsForMode('new', controls); + configurePortControlsForMode('existing', controls); + configurePortControlsForMode('new', controls); + configurePortControlsForMode('existing', controls); + + // Verify validation still works correctly (no accumulated validators) + controls.port.setValue(null); + expect(controls.port.hasError('required')).toBe(true); + + controls.port.setValue('fc0'); + expect(controls.port.hasError('required')).toBe(false); + expect(controls.port.valid).toBe(true); + }); + }); + + describe('form integration', () => { + it('integrates correctly with form group', () => { + const form = fb.group(controls); + + configurePortControlsForMode('existing', controls); + expect(form.invalid).toBe(true); // port is required but null + + controls.port.setValue('fc0'); + expect(form.valid).toBe(true); + + configurePortControlsForMode('new', controls); + expect(form.invalid).toBe(true); // host_id is required but null + + controls.host_id.setValue(1); + expect(form.valid).toBe(true); + }); + + it('returns correct getRawValue after mode switch', () => { + const form = fb.group(controls); + + configurePortControlsForMode('existing', controls); + controls.port.setValue('fc0'); + + let rawValue = form.getRawValue(); + expect(rawValue).toEqual({ port: 'fc0', host_id: null }); + + configurePortControlsForMode('new', controls); + controls.host_id.setValue(123); + + rawValue = form.getRawValue(); + expect(rawValue).toEqual({ port: null, host_id: 123 }); + }); + }); +}); diff --git a/src/app/pages/sharing/iscsi/fibre-channel-ports/helpers/port-mode-control.helper.ts b/src/app/pages/sharing/iscsi/fibre-channel-ports/helpers/port-mode-control.helper.ts new file mode 100644 index 00000000000..eb7d393e6fe --- /dev/null +++ b/src/app/pages/sharing/iscsi/fibre-channel-ports/helpers/port-mode-control.helper.ts @@ -0,0 +1,65 @@ +import { Validators } from '@angular/forms'; +import { FormControl } from '@ngneat/reactive-forms'; + +export type PortMode = 'existing' | 'new'; + +export interface PortModeControls { + port: FormControl; + host_id: FormControl; +} + +/** + * Configures validators and enabled state for port/host_id controls + * based on the selected mode (existing port vs new virtual port). + * + * This helper centralizes the logic for switching between two mutually exclusive + * form control configurations in FC MPIO port selection: + * + * - **"existing" mode**: Uses the `port` control (string), disables `host_id` + * - **"new" mode**: Uses the `host_id` control (number), disables `port` + * + * The function handles: + * - Clearing validators from the disabled control + * - Setting required validator on the enabled control + * - Disabling/enabling controls appropriately + * - Clearing values from disabled controls + * - Calling updateValueAndValidity() to trigger validation + * + * @param mode The selected mode ('existing' to use existing port, 'new' to create virtual port) + * @param controls Object containing both port and host_id form controls + * + * @example + * ```typescript + * // In component with mode switching: + * this.modeControl.valueChanges.pipe( + * takeUntilDestroyed(this.destroyRef), + * ).subscribe((mode) => { + * configurePortControlsForMode(mode, this.form().controls); + * }); + * ``` + */ +export function configurePortControlsForMode( + mode: PortMode, + controls: PortModeControls, +): void { + if (mode === 'new') { + // Creating new virtual port - use host_id + controls.port.clearValidators(); + controls.port.disable(); + controls.port.setValue(null); + + controls.host_id.enable(); + controls.host_id.setValidators([Validators.required]); + } else { + // Using existing port - use port string + controls.host_id.clearValidators(); + controls.host_id.disable(); + controls.host_id.setValue(null); + + controls.port.enable(); + controls.port.setValidators([Validators.required]); + } + + controls.port.updateValueAndValidity(); + controls.host_id.updateValueAndValidity(); +} diff --git a/src/app/pages/sharing/iscsi/iscsi-wizard/iscsi-wizard.component.html b/src/app/pages/sharing/iscsi/iscsi-wizard/iscsi-wizard.component.html index 1f98c7815c0..4d2684ba044 100644 --- a/src/app/pages/sharing/iscsi/iscsi-wizard/iscsi-wizard.component.html +++ b/src/app/pages/sharing/iscsi/iscsi-wizard/iscsi-wizard.component.html @@ -62,7 +62,18 @@ + + @if (isFibreChannelMode && validateFcPorts().length > 0) { + + {{ validateFcPorts().join(', ') }} + + } +
diff --git a/src/app/pages/sharing/iscsi/iscsi-wizard/iscsi-wizard.component.spec.ts b/src/app/pages/sharing/iscsi/iscsi-wizard/iscsi-wizard.component.spec.ts index 05721770bf0..93f11d7497d 100644 --- a/src/app/pages/sharing/iscsi/iscsi-wizard/iscsi-wizard.component.spec.ts +++ b/src/app/pages/sharing/iscsi/iscsi-wizard/iscsi-wizard.component.spec.ts @@ -15,6 +15,7 @@ import { LicenseFeature } from 'app/enums/license-feature.enum'; import { ServiceName } from 'app/enums/service-name.enum'; import { ServiceStatus } from 'app/enums/service-status.enum'; import { Dataset } from 'app/interfaces/dataset.interface'; +import { FibreChannelHost, FibreChannelPortChoices } from 'app/interfaces/fibre-channel.interface'; import { IscsiGlobalSession } from 'app/interfaces/iscsi-global-config.interface'; import { IscsiAuthAccess, IscsiExtent, IscsiInitiatorGroup, IscsiPortal, IscsiTarget, IscsiTargetExtent, @@ -87,6 +88,22 @@ describe('IscsiWizardComponent', () => { mockCall('iscsi.initiator.create', { id: 14 } as IscsiInitiatorGroup), mockCall('iscsi.target.create', { id: 15 } as IscsiTarget), mockCall('iscsi.targetextent.create', { id: 16 } as IscsiTargetExtent), + mockCall('fcport.port_choices', { + fc0: { wwpn: '10:00:00:00:c9:20:00:00', wwpn_b: '10:00:00:00:c9:20:00:01' }, + fc1: { wwpn: '10:00:00:00:c9:30:00:00', wwpn_b: '10:00:00:00:c9:30:00:01' }, + 'fc0/1': { wwpn: '10:00:00:00:c9:20:01:00', wwpn_b: '10:00:00:00:c9:20:01:01' }, + } as FibreChannelPortChoices), + mockCall('fc.fc_host.query', [ + { + id: 1, alias: 'fc0', npiv: 1, wwpn: '10:00:00:00:c9:20:00:00', wwpn_b: '10:00:00:00:c9:20:00:01', + }, + { + id: 2, alias: 'fc1', npiv: 0, wwpn: '10:00:00:00:c9:30:00:00', wwpn_b: '10:00:00:00:c9:30:00:01', + }, + ] as FibreChannelHost[]), + mockCall('fcport.query', []), + mockCall('fcport.create'), + mockCall('fc.fc_host.update'), ]), provideMockStore({ selectors: [ @@ -265,4 +282,120 @@ describe('IscsiWizardComponent', () => { expect(spectator.inject(SlideInRef).close).toHaveBeenCalled(); }); + + describe('FC MPIO validation', () => { + beforeEach(async () => { + // Fill target step with FC mode + await form.fillForm({ + Name: 'test-fc-target', + Mode: 'Fibre Channel', + }); + + // Move to extent step + await form.fillForm({ + Device: 'Create New', + 'Pool/Dataset': '/mnt/new_pool', + Size: 1024, + }); + + spectator.detectChanges(); + }); + + it('allows valid MPIO configuration with ports on different physical ports', async () => { + // Add first port on fc0 + const fcPortsList = await loader.getHarness(IxListHarness.with({ label: 'Fibre Channel Ports' })); + await fcPortsList.pressAddButton(); + + await form.fillForm({ + 'Port Mode': 'Use existing port', + 'Existing Port': 'fc0', + }); + + // Add second port on fc1 (different physical port) + await fcPortsList.pressAddButton(); + + await form.fillForm({ + 'Port Mode': 'Use existing port', + 'Existing Port': 'fc1', + }); + + spectator.detectChanges(); + + // Verify no FC port validation errors displayed + const errorElement = spectator.query('mat-error'); + expect(errorElement).toBeFalsy(); + + // Note: Button may still be disabled due to other form validation, + // but FC port validation specifically should pass + }); + + + it('blocks when two NPIV virtual ports share the same physical port', async () => { + // Add two virtual ports on the same physical port + const fcPortsList = await loader.getHarness(IxListHarness.with({ label: 'Fibre Channel Ports' })); + + // First NPIV port on fc0 + await fcPortsList.pressAddButton(); + await form.fillForm({ + 'Port Mode': 'Create new virtual port', + 'Choose Host for New Virtual Port': 'fc0/2', + }); + + // Second NPIV port on fc0 (should fail validation) + await fcPortsList.pressAddButton(); + await form.fillForm({ + 'Port Mode': 'Create new virtual port', + 'Choose Host for New Virtual Port': 'fc0/2', + }); + + spectator.detectChanges(); + + // Verify submit button is disabled due to validation failure + const saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save' })); + expect(await saveButton.isDisabled()).toBe(true); + + // Note: Error detection for NPIV ports would require backend resolution + // of host_id to port string, so validation happens at a different level + }); + + it('allows valid MPIO with mix of physical and NPIV ports on different ports', async () => { + // Add physical port on fc0 + const fcPortsList = await loader.getHarness(IxListHarness.with({ label: 'Fibre Channel Ports' })); + await fcPortsList.pressAddButton(); + + await form.fillForm({ + 'Port Mode': 'Use existing port', + 'Existing Port': 'fc0', + }); + + // Add NPIV port on fc1 (different physical port) + await fcPortsList.pressAddButton(); + + await form.fillForm({ + 'Port Mode': 'Create new virtual port', + 'Choose Host for New Virtual Port': 'fc1/1', + }); + + spectator.detectChanges(); + + // Verify no FC port validation error displayed + const errorElement = spectator.query('mat-error'); + expect(errorElement).toBeFalsy(); + + // Note: Mixed mode (physical + NPIV) should be valid as long as they're on different physical ports + }); + + + it('allows empty FC ports array in FC mode when no ports added', () => { + // No ports added - should still be valid (empty is acceptable) + spectator.detectChanges(); + + // No error message should be shown for empty ports + const errorElement = spectator.query('mat-error'); + expect(errorElement).toBeFalsy(); + + // Note: The form may still be invalid due to other required fields + // but FC port validation specifically should pass + }); + }); }); diff --git a/src/app/pages/sharing/iscsi/iscsi-wizard/iscsi-wizard.component.ts b/src/app/pages/sharing/iscsi/iscsi-wizard/iscsi-wizard.component.ts index 243abd8bbfe..a4cab2d63df 100644 --- a/src/app/pages/sharing/iscsi/iscsi-wizard/iscsi-wizard.component.ts +++ b/src/app/pages/sharing/iscsi/iscsi-wizard/iscsi-wizard.component.ts @@ -1,7 +1,9 @@ -import { ChangeDetectionStrategy, Component, OnInit, signal, inject } from '@angular/core'; +import { ChangeDetectionStrategy, Component, computed, DestroyRef, OnInit, signal, inject } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { Validators, ReactiveFormsModule } from '@angular/forms'; import { MatButton } from '@angular/material/button'; import { MatCard } from '@angular/material/card'; +import { MatError } from '@angular/material/form-field'; import { MatStepper, MatStep, MatStepLabel, MatStepperNext, MatStepperPrevious, } from '@angular/material/stepper'; @@ -13,6 +15,7 @@ import { lastValueFrom, forkJoin, of, } from 'rxjs'; +import { switchMap } from 'rxjs/operators'; import { patterns } from 'app/constants/name-patterns.constant'; import { RequiresRolesDirective } from 'app/directives/requires-roles/requires-roles.directive'; import { DatasetType } from 'app/enums/dataset.enum'; @@ -26,8 +29,8 @@ import { import { mntPath } from 'app/enums/mnt-path.enum'; import { Role } from 'app/enums/role.enum'; import { ServiceName } from 'app/enums/service-name.enum'; +import { createFormArraySnapshot } from 'app/helpers/form-array-snapshot.helper'; import { Dataset, DatasetCreate } from 'app/interfaces/dataset.interface'; -import { FibreChannelPort } from 'app/interfaces/fibre-channel.interface'; import { IscsiExtent, IscsiExtentUpdate, @@ -70,6 +73,7 @@ import { ExtentWizardStepComponent } from './steps/extent-wizard-step/extent-wiz imports: [ ModalHeaderComponent, MatCard, + MatError, ReactiveFormsModule, MatStepper, MatStep, @@ -95,11 +99,14 @@ export class IscsiWizardComponent implements OnInit { private translate = inject(TranslateService); private loader = inject(LoaderService); private store$ = inject>(Store); + private destroyRef = inject(DestroyRef); slideInRef = inject>(SlideInRef); isLoading = signal(false); toStop = signal(false); namesInUse = signal([]); + fcHosts = signal<{ id: number; alias: string }[]>([]); + availableFcPorts = signal([]); createdZvol: Dataset | undefined; createdExtent: IscsiExtent | undefined; @@ -132,10 +139,10 @@ export class IscsiWizardComponent implements OnInit { portal: new FormControl(null as typeof newOption | number | null, [Validators.required]), listen: this.fb.array([]), initiators: [[] as string[]], - fcport: this.fb.group({ - port: [null as string | null], - host_id: new FormControl(null as number | null, [Validators.required]), - }), + fcPorts: this.fb.array<{ + port: FormControl; + host_id: FormControl; + }>([]), }), }, { validators: [ @@ -147,6 +154,12 @@ export class IscsiWizardComponent implements OnInit { ], }); + // Reactive snapshot of FC ports form array for use in computed signals + protected fcPortsSnapshot = createFormArraySnapshot<{ port: string | null; host_id: number | null }>( + this.form.controls.options.controls.fcPorts, + this.destroyRef, + ); + protected readonly requiredRoles = [ Role.SharingIscsiTargetWrite, Role.SharingIscsiWrite, @@ -267,6 +280,53 @@ export class IscsiWizardComponent implements OnInit { }); } + protected addFcPort(): void { + this.form.controls.options.controls.fcPorts.push( + this.fb.group({ + port: new FormControl(null as string | null), + host_id: new FormControl(null as number | null), + }), + ); + } + + protected deleteFcPort(index: number): void { + this.form.controls.options.controls.fcPorts.removeAt(index); + } + + protected validateFcPorts(): string[] { + const ports = this.form.controls.options.controls.fcPorts.getRawValue(); + const validation = this.fcService.validatePhysicalPortUniqueness(ports, this.fcHosts()); + + if (!validation.valid) { + return validation.duplicates.map((port) => this.translate.instant( + 'Physical port {port} is used multiple times. Each target port must use a different physical FC port.', + { port }, + )); + } + + return []; + } + + protected areFcPortsValid(): boolean { + return this.form.controls.options.controls.fcPorts.valid && this.validateFcPorts().length === 0; + } + + // Array of used physical ports for each index (excluding that index) + protected usedPhysicalPortsByIndex = computed(() => { + const ports = this.fcPortsSnapshot(); + const hosts = this.fcHosts(); + + return ports.map((_item, currentIndex) => ports + .filter((_port, idx) => idx !== currentIndex) + .map((portForm) => this.fcService.getPhysicalPort(portForm, hosts)) + .filter((port): port is string => port !== null)); + }); + + // Function wrapper for child components that expect function interface + protected getUsedPhysicalPortsFn = (): ((index: number) => string[]) => { + return (index: number) => this.usedPhysicalPortsByIndex()[index] || []; + }; + ngOnInit(): void { this.form.controls.extent.controls.path.disable(); this.form.controls.extent.controls.filesize.disable(); @@ -284,16 +344,38 @@ export class IscsiWizardComponent implements OnInit { } }); - this.form.controls.target.controls.mode.valueChanges.pipe(untilDestroyed(this)).subscribe((mode) => { + this.form.controls.target.controls.mode.valueChanges.pipe( + takeUntilDestroyed(this.destroyRef), + ).subscribe((mode) => { if (mode === IscsiTargetMode.Iscsi) { this.form.controls.options.controls.portal.enable(); this.form.controls.options.controls.initiators.enable(); - this.form.controls.options.controls.fcport.disable(); + this.form.controls.options.controls.fcPorts.disable(); } else { + // FC or Both mode this.form.controls.options.controls.portal.setValue(null); this.form.controls.options.controls.portal.disable(); this.form.controls.options.controls.initiators.disable(); - this.form.controls.options.controls.fcport.enable(); + this.form.controls.options.controls.fcPorts.enable(); + + // Load FC hosts for validation + this.api.call('fc.fc_host.query').pipe( + takeUntilDestroyed(this.destroyRef), + ).subscribe((hosts) => { + this.fcHosts.set(hosts.map((host) => ({ id: host.id, alias: host.alias }))); + }); + + // Load available FC port choices + this.api.call('fcport.port_choices', [false]).pipe( + takeUntilDestroyed(this.destroyRef), + ).subscribe((portsData) => { + this.availableFcPorts.set(Object.keys(portsData)); + }); + + // Auto-add first port when switching to FC mode (UX improvement) + if (this.form.controls.options.controls.fcPorts.length === 0) { + this.addFcPort(); + } } }); } @@ -318,12 +400,11 @@ export class IscsiWizardComponent implements OnInit { return lastValueFrom(this.api.call('iscsi.target.create', [payload])); } - private createTargetFiberChannel( - targetId: number, - port: string, - hostId: number, - ): Promise { - return lastValueFrom(this.fcService.linkFiberChannelToTarget(targetId, port, hostId)); + private createTargetFiberChannel(targetId: number): Promise { + const fcPortValues = this.form.controls.options.controls.fcPorts.getRawValue(); + return lastValueFrom( + this.fcService.linkFiberChannelPortsToTarget(targetId, fcPortValues), + ); } private createTargetExtent(payload: IscsiTargetExtentUpdate): Promise { @@ -355,6 +436,20 @@ export class IscsiWizardComponent implements OnInit { requests.push(this.api.call('iscsi.target.delete', [this.createdTarget.id])); } + // CRITICAL: Cleanup FC ports if wizard fails after creating them + if (this.createdTarget && this.isFibreChannelMode) { + requests.push( + this.fcService.loadTargetPorts(this.createdTarget.id).pipe( + switchMap((ports) => { + if (ports.length === 0) return of(null); + return forkJoin( + ports.map((port) => this.api.call('fcport.delete', [port.id])), + ); + }), + ), + ); + } + if (this.createdTargetExtent) { requests.push(this.api.call('iscsi.targetextent.delete', [this.createdTargetExtent.id])); } @@ -453,11 +548,8 @@ export class IscsiWizardComponent implements OnInit { } if (this.isNewTarget && this.isFibreChannelMode) { - await this.createTargetFiberChannel( - this.createdTarget.id, - this.form.value.options.fcport.port, - this.form.value.options.fcport.host_id, - ).then(() => {}, (err: unknown) => this.handleError(err)); + await this.createTargetFiberChannel(this.createdTarget.id) + .then(() => {}, (err: unknown) => this.handleError(err)); } if (this.toStop()) { diff --git a/src/app/pages/sharing/iscsi/iscsi-wizard/steps/protocol-options-wizard-step/protocol-options-wizard-step.component.html b/src/app/pages/sharing/iscsi/iscsi-wizard/steps/protocol-options-wizard-step/protocol-options-wizard-step.component.html index 03704f44960..9f7e870285d 100644 --- a/src/app/pages/sharing/iscsi/iscsi-wizard/steps/protocol-options-wizard-step/protocol-options-wizard-step.component.html +++ b/src/app/pages/sharing/iscsi/iscsi-wizard/steps/protocol-options-wizard-step/protocol-options-wizard-step.component.html @@ -1,6 +1,28 @@ @if(isFibreChannelMode()) { - + + @if (form().controls.fcPorts.controls.length >= 2) { + + } + + @for (entry of form().controls.fcPorts.controls; track entry; let i = $index) { + + + + } + } @else { (); isFibreChannelMode = input(false); + addFcPort = input.required<() => void>(); + deleteFcPort = input.required<(index: number) => void>(); + getUsedPhysicalPorts = input.required<(excludeIndex: number) => string[]>(); + availablePorts = input.required(); readonly helptextSharingIscsi = helptextIscsi; diff --git a/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.html b/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.html index 47f05be45eb..d350e9d70bc 100644 --- a/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.html +++ b/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.html @@ -1,26 +1,30 @@

- {{ 'Fibre Channel Port' | translate }} + {{ 'Fibre Channel Ports' | translate }}

@if (isLoading()) { } @else { - @if (port(); as port) { -

{{ 'Port' | translate }}: {{ port.port }}

+ @if (ports().length > 0) { + @for (port of ports(); track port.id) { +
+

{{ 'Port' | translate }}: {{ port.port }}

- -

{{ 'Controller A WWPN' | translate }}: {{ port.wwpn }}

-

{{ 'Controller B WWPN' | translate }}: {{ port.wwpn_b }}

-
+ +

{{ 'Controller A WWPN' | translate }}: {{ port.wwpn }}

+

{{ 'Controller B WWPN' | translate }}: {{ port.wwpn_b }}

+
- -

{{ 'WWPN' | translate }}: {{ port.wwpn }}

-
+ +

{{ 'WWPN' | translate }}: {{ port.wwpn }}

+
+
+ } } @else { -

{{ 'No associated Fibre Channel port.' | translate }}

+

{{ 'No associated Fibre Channel ports.' | translate }}

} }
diff --git a/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.scss b/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.scss index bf620acc1b0..99e422f4e49 100644 --- a/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.scss +++ b/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.scss @@ -1,3 +1,11 @@ mat-card-content p { margin: 0 0 6px; } + +.port-group { + &:not(:last-child) { + margin-bottom: 16px; + padding-bottom: 16px; + border-bottom: 1px solid var(--lines); + } +} diff --git a/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.spec.ts b/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.spec.ts index a92fd94f61b..b0ca792f0b8 100644 --- a/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.spec.ts +++ b/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.spec.ts @@ -26,18 +26,19 @@ describe('FibreChannelPortCardComponent', () => { spectator = createComponent({ props: { isLoading: false, - port: { + ports: [{ + id: 1, port: 'fc1/5', wwpn: '10:00:00:00:c9:20:00:00', wwpn_b: '10:00:00:00:c9:20:00:01', - } as unknown as FibreChannelPort, + } as FibreChannelPort], }, }); }); - it('renders Fibre Channel Port title', () => { + it('renders Fibre Channel Ports title', () => { const title = spectator.query('h3[mat-card-title]'); - expect(title).toHaveText('Fibre Channel Port'); + expect(title).toHaveText('Fibre Channel Ports'); }); it('displays port details correctly', () => { @@ -48,11 +49,35 @@ describe('FibreChannelPortCardComponent', () => { expect(content[2]).toHaveText('Controller B WWPN: 10:00:00:00:c9:20:00:01'); }); - it('displays "No associated Fibre Channel port" message', () => { - spectator.setInput('port', null); + it('displays "No associated Fibre Channel ports" message', () => { + spectator.setInput('ports', []); spectator.setInput('isLoading', false); const content = spectator.query('mat-card-content'); - expect(content).toHaveText('No associated Fibre Channel port'); + expect(content).toHaveText('No associated Fibre Channel ports'); + }); + + it('displays multiple ports correctly', () => { + spectator.setInput('ports', [ + { + id: 1, + port: 'fc0', + wwpn: '10:00:00:00:c9:20:00:00', + wwpn_b: '10:00:00:00:c9:20:00:01', + } as FibreChannelPort, + { + id: 2, + port: 'fc1', + wwpn: '10:00:00:00:c9:30:00:00', + wwpn_b: '10:00:00:00:c9:30:00:01', + } as FibreChannelPort, + ]); + + const portGroups = spectator.queryAll('.port-group'); + expect(portGroups).toHaveLength(2); + + const allParagraphs = spectator.queryAll('mat-card-content p'); + expect(allParagraphs[0]).toHaveText('Port: fc0'); + expect(allParagraphs[3]).toHaveText('Port: fc1'); }); }); @@ -77,10 +102,11 @@ describe('FibreChannelPortCardComponent not HA', () => { spectator = createComponent({ props: { isLoading: false, - port: { + ports: [{ + id: 1, port: 'fc1/5', wwpn: '10:00:00:00:c9:20:00:00', - } as unknown as FibreChannelPort, + } as FibreChannelPort], }, }); }); diff --git a/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.ts b/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.ts index 4cdbc9e7420..714c72bec4c 100644 --- a/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.ts +++ b/src/app/pages/sharing/iscsi/target/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.ts @@ -25,6 +25,6 @@ import { FibreChannelPort } from 'app/interfaces/fibre-channel.interface'; ], }) export class FibreChannelPortCardComponent { - readonly port = input.required(); + readonly ports = input.required(); readonly isLoading = input.required(); } diff --git a/src/app/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.html b/src/app/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.html index 1782b2ef31a..bf80643049b 100644 --- a/src/app/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.html +++ b/src/app/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.html @@ -7,11 +7,11 @@ @if (hasFibreCards()) { - @if (targetPort()) { + @if (targetPorts().length > 0) { } } diff --git a/src/app/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.spec.ts b/src/app/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.spec.ts index 8782ecc8c81..d22a358363a 100644 --- a/src/app/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.spec.ts +++ b/src/app/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.spec.ts @@ -80,14 +80,14 @@ describe('TargetDetailsComponent', () => { expect(spectator.query(IscsiGroupsCardComponent)).toExist(); }); - it('renders FibreChannelPortCardComponent if targetPort is set', () => { + it('renders FibreChannelPortCardComponent if targetPorts are set', () => { spectator.detectChanges(); expect(spectator.query(FibreChannelPortCardComponent)).toExist(); - expect(spectator.query(FibreChannelPortCardComponent)?.port).toEqual(mockPort); + expect(spectator.query(FibreChannelPortCardComponent)?.ports).toEqual([mockPort]); }); - it('should render FibreChannelPortCardComponent even if no targetPort is available', () => { - spectator.component.targetPort.set(null); + it('should render FibreChannelPortCardComponent even if no targetPorts are available', () => { + spectator.component.targetPorts.set([]); spectator.detectChanges(); expect(spectator.query(FibreChannelPortCardComponent)).not.toBeNull(); diff --git a/src/app/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.ts b/src/app/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.ts index 2b841fcc561..0f58f7c6ec8 100644 --- a/src/app/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.ts +++ b/src/app/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.ts @@ -34,7 +34,7 @@ export class TargetDetailsComponent { readonly target = input.required(); - targetPort = signal(null); + targetPorts = signal([]); isLoading = signal(false); connections = toSignal(this.api.call('fcport.status'), { initialValue: [] }); @@ -52,15 +52,15 @@ export class TargetDetailsComponent { constructor() { effect(() => { const targetId = this.target().id; - this.targetPort.set(null); + this.targetPorts.set([]); if (targetId) { - this.getPortByTargetId(targetId); + this.getPortsByTargetId(targetId); } }); } - private getPortByTargetId(id: number): void { + private getPortsByTargetId(id: number): void { this.isLoading.set(true); this.api.call('fcport.query', [[['target.id', '=', id]]]) @@ -70,7 +70,7 @@ export class TargetDetailsComponent { untilDestroyed(this), ) .subscribe((ports) => { - this.targetPort.set(ports[0] || null); + this.targetPorts.set(ports); }); } } diff --git a/src/app/pages/sharing/iscsi/target/target-form/target-form.component.html b/src/app/pages/sharing/iscsi/target/target-form/target-form.component.html index 2289fbacb69..e193d64e726 100644 --- a/src/app/pages/sharing/iscsi/target/target-form/target-form.component.html +++ b/src/app/pages/sharing/iscsi/target/target-form/target-form.component.html @@ -55,11 +55,38 @@ @if (showPortControls) { - + + @if (form.controls.fcPorts.length >= 2) { + + } + + @for (entry of form.controls.fcPorts.controls; track entry; let i = $index) { + + + + } + + + @if (fcPortValidationErrors().length > 0) { + + {{ fcPortValidationErrors().join(', ') }} + + } } @@ -121,7 +148,7 @@ type="submit" color="primary" ixTest="save" - [disabled]="form.invalid || (showPortControls && fcForm.invalid) || isLoading() || isAsyncValidatorPending" + [disabled]="form.invalid || (showPortControls && !areFcPortsValid()) || isLoading() || isAsyncValidatorPending" > {{ 'Save' | translate }} diff --git a/src/app/pages/sharing/iscsi/target/target-form/target-form.component.spec.ts b/src/app/pages/sharing/iscsi/target/target-form/target-form.component.spec.ts index 601f2889a1d..ecf3cd9defb 100644 --- a/src/app/pages/sharing/iscsi/target/target-form/target-form.component.spec.ts +++ b/src/app/pages/sharing/iscsi/target/target-form/target-form.component.spec.ts @@ -23,6 +23,9 @@ import { IxFormHarness } from 'app/modules/forms/ix-forms/testing/ix-form.harnes import { SlideIn } from 'app/modules/slide-ins/slide-in'; import { SlideInRef } from 'app/modules/slide-ins/slide-in-ref'; import { ApiService } from 'app/modules/websocket/api.service'; +import { + FcMpioInfoBannerComponent, +} from 'app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component'; import { TargetFormComponent } from 'app/pages/sharing/iscsi/target/target-form/target-form.component'; import { FibreChannelService } from 'app/services/fibre-channel.service'; import { selectSystemInfo } from 'app/store/system-info/system-info.selectors'; @@ -64,6 +67,7 @@ describe('TargetFormComponent', () => { imports: [ ReactiveFormsModule, IxIpInputWithNetmaskComponent, + FcMpioInfoBannerComponent, ], providers: [ provideMockStore({ @@ -82,8 +86,9 @@ describe('TargetFormComponent', () => { mockProvider(SlideIn), mockProvider(DialogService), mockProvider(FibreChannelService, { - loadTargetPort: jest.fn(() => of(null)), - linkFiberChannelToTarget: jest.fn(() => of(null)), + loadTargetPorts: jest.fn(() => of([])), + linkFiberChannelPortsToTarget: jest.fn(() => of(null)), + validatePhysicalPortUniqueness: jest.fn(() => ({ valid: true, duplicates: [] as string[] })), }), mockProvider(SlideInRef, slideInRef), mockApi([ @@ -142,12 +147,16 @@ describe('TargetFormComponent', () => { }); it('add new target when form is submitted', async () => { + // Click Add buttons to create FormArray items: + // addButtons[0] = Add button for groups (click twice for 2 groups) + // addButtons[1] = Add button for auth_networks (click twice for 2 networks) const addButtons = await loader.getAllHarnesses(MatButtonHarness.with({ text: 'Add' })); await addButtons[0].click(); await addButtons[0].click(); await addButtons[1].click(); await addButtons[1].click(); + // Use patchValue to set nested FormArray values (simpler than harness for complex nested structures) spectator.component.form.patchValue({ name: 'name_new', alias: 'alias_new', @@ -244,10 +253,9 @@ describe('TargetFormComponent', () => { }, ], ); - expect(spectator.inject(FibreChannelService).linkFiberChannelToTarget).toHaveBeenCalledWith( + expect(spectator.inject(FibreChannelService).linkFiberChannelPortsToTarget).toHaveBeenCalledWith( 123, - null, - undefined, + [], ); expect(spectator.inject(SlideInRef).close).toHaveBeenCalled(); }); @@ -287,12 +295,6 @@ describe('TargetFormComponent', () => { }); describe('validation error handling', () => { - beforeEach(async () => { - spectator = createComponent(); - loader = TestbedHarnessEnvironment.loader(spectator.fixture); - form = await loader.getHarness(IxFormHarness); - }); - beforeEach(async () => { spectator = createComponent(); api = spectator.inject(ApiService); @@ -315,4 +317,22 @@ describe('TargetFormComponent', () => { expect(await nameControl.getErrorText()).toBe('Target with this name already exists'); }); }); + + describe('MPIO info banner conditional display', () => { + beforeEach(async () => { + spectator = createComponent(); + loader = TestbedHarnessEnvironment.loader(spectator.fixture); + form = await loader.getHarness(IxFormHarness); + }); + + it('should not display banner when there are 0 FC ports', async () => { + await form.fillForm({ + Mode: 'Fibre Channel', + }); + spectator.detectChanges(); + + const banner = spectator.query('ix-fc-mpio-info-banner'); + expect(banner).not.toExist(); + }); + }); }); diff --git a/src/app/pages/sharing/iscsi/target/target-form/target-form.component.ts b/src/app/pages/sharing/iscsi/target/target-form/target-form.component.ts index 0922d0c3ad8..619de014560 100644 --- a/src/app/pages/sharing/iscsi/target/target-form/target-form.component.ts +++ b/src/app/pages/sharing/iscsi/target/target-form/target-form.component.ts @@ -1,8 +1,11 @@ -import { ChangeDetectionStrategy, ChangeDetectorRef, Component, OnInit, signal, inject } from '@angular/core'; +import { + ChangeDetectionStrategy, ChangeDetectorRef, Component, computed, DestroyRef, OnInit, signal, inject, +} from '@angular/core'; import { toSignal } from '@angular/core/rxjs-interop'; import { ReactiveFormsModule, Validators } from '@angular/forms'; import { MatButton } from '@angular/material/button'; import { MatCard, MatCardContent } from '@angular/material/card'; +import { MatError } from '@angular/material/form-field'; import { FormBuilder, FormControl } from '@ngneat/reactive-forms'; import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy'; import { TranslateModule, TranslateService } from '@ngx-translate/core'; @@ -12,6 +15,7 @@ import { map, switchMap } from 'rxjs/operators'; import { RequiresRolesDirective } from 'app/directives/requires-roles/requires-roles.directive'; import { IscsiAuthMethod, IscsiTargetMode, iscsiTargetModeNames } from 'app/enums/iscsi.enum'; import { Role } from 'app/enums/role.enum'; +import { createFormArraySnapshot } from 'app/helpers/form-array-snapshot.helper'; import { mapToOptions } from 'app/helpers/options.helper'; import { helptextIscsi } from 'app/helptext/sharing'; import { IscsiTarget, IscsiTargetGroup } from 'app/interfaces/iscsi.interface'; @@ -34,8 +38,11 @@ import { TranslateOptionsPipe } from 'app/modules/translate/translate-options/tr import { ignoreTranslation, TranslatedString } from 'app/modules/translate/translate.helper'; import { ApiService } from 'app/modules/websocket/api.service'; import { - FcPortsControlsComponent, -} from 'app/pages/sharing/iscsi/fibre-channel-ports/fc-ports-controls/fc-ports-controls.component'; + FcMpioInfoBannerComponent, +} from 'app/pages/sharing/iscsi/fibre-channel-ports/fc-mpio-info-banner/fc-mpio-info-banner.component'; +import { + FcPortItemControlsComponent, +} from 'app/pages/sharing/iscsi/fibre-channel-ports/fc-port-item-controls/fc-port-item-controls.component'; import { TargetNameValidationService } from 'app/pages/sharing/iscsi/target/target-name-validation.service'; import { FibreChannelService } from 'app/services/fibre-channel.service'; import { IscsiService } from 'app/services/iscsi.service'; @@ -51,6 +58,7 @@ import { LicenseService } from 'app/services/license.service'; ModalHeaderComponent, MatCard, MatCardContent, + MatError, ReactiveFormsModule, IxFieldsetComponent, IxInputComponent, @@ -59,7 +67,8 @@ import { LicenseService } from 'app/services/license.service'; IxIpInputWithNetmaskComponent, IxSelectComponent, FormActionsComponent, - FcPortsControlsComponent, + FcPortItemControlsComponent, + FcMpioInfoBannerComponent, RequiresRolesDirective, MatButton, TestDirective, @@ -78,6 +87,7 @@ export class TargetFormComponent implements OnInit { private fcService = inject(FibreChannelService); private license = inject(LicenseService); private targetNameValidationService = inject(TargetNameValidationService); + private destroyRef = inject(DestroyRef); slideInRef = inject>(SlideInRef); get isNew(): boolean { @@ -148,7 +158,8 @@ export class TargetFormComponent implements OnInit { protected isLoading = signal(false); protected editingTarget: IscsiTarget | undefined = undefined; - protected editingTargetPort: string | undefined = undefined; + protected fcHosts = signal<{ id: number; alias: string }[]>([]); + protected availableFcPorts = signal([]); form = this.formBuilder.group({ name: [ @@ -159,12 +170,20 @@ export class TargetFormComponent implements OnInit { mode: [IscsiTargetMode.Iscsi], groups: this.formBuilder.array([]), auth_networks: this.formBuilder.array([]), + fcPorts: this.formBuilder.array<{ + port: FormControl; + host_id: FormControl; + }>([]), }); - fcForm = this.formBuilder.group({ - port: new FormControl(null as string | null), - host_id: new FormControl(null as number | null, [Validators.required]), - }); + // Reactive snapshot of FC ports form array for use in computed signals + protected fcPortsSnapshot = createFormArraySnapshot<{ port: string | null; host_id: number | null }>( + this.form.controls.fcPorts, + this.destroyRef, + ); + + // Computed signal for current port values (used in edit mode) + protected currentPorts = computed(() => this.fcPortsSnapshot().map((form) => form.port).filter(Boolean) as string[]); constructor() { const slideInRef = this.slideInRef; @@ -181,11 +200,25 @@ export class TargetFormComponent implements OnInit { } ngOnInit(): void { + // Load FC hosts for validation + this.api.call('fc.fc_host.query').pipe( + untilDestroyed(this), + ).subscribe((hosts) => { + this.fcHosts.set(hosts.map((host) => ({ id: host.id, alias: host.alias }))); + }); + + // Load available FC port choices + this.api.call('fcport.port_choices', [false]).pipe( + untilDestroyed(this), + ).subscribe((portsData) => { + this.availableFcPorts.set(Object.keys(portsData)); + }); + if (this.editingTarget) { this.setTargetForEdit(this.editingTarget); if ([IscsiTargetMode.Fc, IscsiTargetMode.Both].includes(this.editingTarget.mode)) { - this.loadFibreChannelPort(); + this.loadFibreChannelPorts(); } } } @@ -200,7 +233,7 @@ export class TargetFormComponent implements OnInit { } protected onSubmit(): void { - const values = this.form.getRawValue(); + const { fcPorts, ...values } = this.form.getRawValue(); this.isLoading.set(true); let request$: Observable; @@ -216,10 +249,10 @@ export class TargetFormComponent implements OnInit { return of(target); } - return this.fcService.linkFiberChannelToTarget( + const fcPortValues = this.form.controls.fcPorts.getRawValue(); + return this.fcService.linkFiberChannelPortsToTarget( target.id, - this.fcForm.value.port, - this.fcForm.value.host_id || undefined, + fcPortValues, ).pipe(map(() => target)); }), untilDestroyed(this), @@ -260,14 +293,65 @@ export class TargetFormComponent implements OnInit { this.form.controls.auth_networks.removeAt(index); } - private loadFibreChannelPort(): void { - this.fcService.loadTargetPort(this.editingTarget.id).pipe( + protected addFcPort(): void { + this.form.controls.fcPorts.push( + this.formBuilder.group({ + port: new FormControl(null as string | null), + host_id: new FormControl(null as number | null), + }), + ); + } + + protected deleteFcPort(index: number): void { + this.form.controls.fcPorts.removeAt(index); + } + + protected validateFcPorts(): string[] { + const ports = this.form.controls.fcPorts.getRawValue(); + const validation = this.fcService.validatePhysicalPortUniqueness(ports, this.fcHosts()); + + if (!validation.valid) { + return validation.duplicates.map((port) => this.translate.instant( + 'Physical port {port} is used multiple times. Each target port must use a different physical FC port.', + { port }, + )); + } + + return []; + } + + protected areFcPortsValid(): boolean { + return this.form.controls.fcPorts.valid && this.validateFcPorts().length === 0; + } + + protected fcPortValidationErrors = (): string[] => { + return this.validateFcPorts(); + }; + + // Array of used physical ports for each index (excluding that index) + protected usedPhysicalPortsByIndex = computed(() => { + const ports = this.fcPortsSnapshot(); + const hosts = this.fcHosts(); + + return ports.map((_item, currentIndex) => ports + .filter((_port, idx) => idx !== currentIndex) + .map((portForm) => this.fcService.getPhysicalPort(portForm, hosts)) + .filter((port): port is string => port !== null)); + }); + + private loadFibreChannelPorts(): void { + this.fcService.loadTargetPorts(this.editingTarget.id).pipe( untilDestroyed(this), - ).subscribe((port) => { - if (port) { - this.editingTargetPort = port.port; - this.cdr.markForCheck(); - } + ).subscribe((ports) => { + this.form.controls.fcPorts.clear(); + + ports.forEach((port) => { + this.addFcPort(); + const index = this.form.controls.fcPorts.length - 1; + this.form.controls.fcPorts.at(index).patchValue({ port: port.port, host_id: null }); + }); + + this.cdr.markForCheck(); }); } } diff --git a/src/app/services/fibre-channel.service.spec.ts b/src/app/services/fibre-channel.service.spec.ts index aca11090d29..0f309548802 100644 --- a/src/app/services/fibre-channel.service.spec.ts +++ b/src/app/services/fibre-channel.service.spec.ts @@ -1,7 +1,7 @@ import { createServiceFactory, SpectatorService } from '@ngneat/spectator/jest'; import { lastValueFrom, of } from 'rxjs'; import { mockCall, mockApi } from 'app/core/testing/utils/mock-api.utils'; -import { FibreChannelHost, FibreChannelPort } from 'app/interfaces/fibre-channel.interface'; +import { FcPortFormValue, FibreChannelHost, FibreChannelPort } from 'app/interfaces/fibre-channel.interface'; import { ApiService } from 'app/modules/websocket/api.service'; import { FibreChannelService } from 'app/services/fibre-channel.service'; @@ -10,7 +10,6 @@ describe('FibreChannelService', () => { const fakeTargetId = 11; const fakeHostId = 22; const fakePortId = 33; - const fakePort = 'fc/1'; const createService = createServiceFactory({ service: FibreChannelService, @@ -28,60 +27,387 @@ describe('FibreChannelService', () => { beforeEach(() => spectator = createService()); - describe('linkFiberChannelToTarget', () => { - it('creates link', async () => { - jest.spyOn(spectator.inject(ApiService), 'call').mockImplementationOnce((method) => { + describe('loadTargetPorts', () => { + it('returns empty array when target has no ports', async () => { + jest.spyOn(spectator.inject(ApiService), 'call').mockReturnValue(of([])); + + const result = await lastValueFrom(spectator.service.loadTargetPorts(fakeTargetId)); + + expect(result).toEqual([]); + expect(spectator.inject(ApiService).call).toHaveBeenCalledWith( + 'fcport.query', + [[['target.id', '=', fakeTargetId]]], + ); + }); + + it('returns array with single port', async () => { + const port = { id: 1, port: 'fc0' } as FibreChannelPort; + jest.spyOn(spectator.inject(ApiService), 'call').mockReturnValue(of([port])); + + const result = await lastValueFrom(spectator.service.loadTargetPorts(fakeTargetId)); + + expect(result).toEqual([port]); + }); + + it('returns array with multiple ports', async () => { + const ports = [ + { id: 1, port: 'fc0' }, + { id: 2, port: 'fc1' }, + { id: 3, port: 'fc0/1' }, + ] as FibreChannelPort[]; + jest.spyOn(spectator.inject(ApiService), 'call').mockReturnValue(of(ports)); + + const result = await lastValueFrom(spectator.service.loadTargetPorts(fakeTargetId)); + + expect(result).toEqual(ports); + }); + }); + + describe('validatePhysicalPortUniqueness', () => { + it('returns valid when no ports provided', () => { + const result = spectator.service.validatePhysicalPortUniqueness([]); + + expect(result).toEqual({ valid: true, duplicates: [] }); + }); + + it('returns valid when all ports have null port string', () => { + const ports: FcPortFormValue[] = [ + { port: null, host_id: 1 }, + { port: null, host_id: 2 }, + ]; + + const result = spectator.service.validatePhysicalPortUniqueness(ports); + + expect(result).toEqual({ valid: true, duplicates: [] }); + }); + + it('returns valid when ports use different physical ports', () => { + const ports: FcPortFormValue[] = [ + { port: 'fc0', host_id: null }, + { port: 'fc1', host_id: null }, + { port: 'fc2/1', host_id: null }, + ]; + + const result = spectator.service.validatePhysicalPortUniqueness(ports); + + expect(result).toEqual({ valid: true, duplicates: [] }); + }); + + it('returns invalid when same physical port used twice (basic)', () => { + const ports: FcPortFormValue[] = [ + { port: 'fc0', host_id: null }, + { port: 'fc0/1', host_id: null }, + ]; + + const result = spectator.service.validatePhysicalPortUniqueness(ports); + + expect(result).toEqual({ valid: false, duplicates: ['fc0'] }); + }); + + it('returns invalid when same physical port used twice (NPIV ports)', () => { + const ports: FcPortFormValue[] = [ + { port: 'fc1/2', host_id: null }, + { port: 'fc1/3', host_id: null }, + ]; + + const result = spectator.service.validatePhysicalPortUniqueness(ports); + + expect(result).toEqual({ valid: false, duplicates: ['fc1'] }); + }); + + it('returns invalid with multiple duplicate ports', () => { + const ports: FcPortFormValue[] = [ + { port: 'fc0', host_id: null }, + { port: 'fc0/1', host_id: null }, + { port: 'fc1/2', host_id: null }, + { port: 'fc1/3', host_id: null }, + ]; + + const result = spectator.service.validatePhysicalPortUniqueness(ports); + + expect(result.valid).toBe(false); + expect(result.duplicates).toContain('fc0'); + expect(result.duplicates).toContain('fc1'); + }); + + it('returns invalid when mixing host_id and port on same physical port', () => { + const hosts = [ + { id: 1, alias: 'fc0' }, + { id: 2, alias: 'fc1' }, + ]; + const ports: FcPortFormValue[] = [ + { port: null, host_id: 1 }, // New virtual port on fc0 + { port: 'fc0/1', host_id: null }, // Existing port on fc0 + ]; + + const result = spectator.service.validatePhysicalPortUniqueness(ports, hosts); + + expect(result).toEqual({ valid: false, duplicates: ['fc0'] }); + }); + + it('returns invalid when using multiple host_ids for same physical port', () => { + const hosts = [ + { id: 1, alias: 'fc0' }, + { id: 2, alias: 'fc1' }, + ]; + const ports: FcPortFormValue[] = [ + { port: null, host_id: 1 }, // New virtual port on fc0 + { port: null, host_id: 1 }, // Another new virtual port on fc0 + ]; + + const result = spectator.service.validatePhysicalPortUniqueness(ports, hosts); + + expect(result).toEqual({ valid: false, duplicates: ['fc0'] }); + }); + + it('returns valid when mixing host_id and port on different physical ports', () => { + const hosts = [ + { id: 1, alias: 'fc0' }, + { id: 2, alias: 'fc1' }, + ]; + const ports: FcPortFormValue[] = [ + { port: null, host_id: 1 }, // New virtual port on fc0 + { port: 'fc1/1', host_id: null }, // Existing port on fc1 + ]; + + const result = spectator.service.validatePhysicalPortUniqueness(ports, hosts); + + expect(result).toEqual({ valid: true, duplicates: [] }); + }); + + it('handles missing host_id gracefully', () => { + const hosts = [ + { id: 1, alias: 'fc0' }, + ]; + const ports: FcPortFormValue[] = [ + { port: null, host_id: 999 }, // host_id not in hosts array + { port: 'fc1', host_id: null }, + ]; + + const result = spectator.service.validatePhysicalPortUniqueness(ports, hosts); + + // Should skip the entry with unknown host_id and only count fc1 + expect(result).toEqual({ valid: true, duplicates: [] }); + }); + }); + + describe('linkFiberChannelPortsToTarget', () => { + it('handles 0→0 transition (no existing, no desired)', async () => { + jest.spyOn(spectator.inject(ApiService), 'call').mockImplementation((method) => { if (method === 'fcport.query') { return of([]); } return of(null); }); - await lastValueFrom(spectator.service.linkFiberChannelToTarget(fakeTargetId, fakePort)); + const result = await lastValueFrom( + spectator.service.linkFiberChannelPortsToTarget(fakeTargetId, []), + ); + expect(result).toBe(true); expect(spectator.inject(ApiService).call).toHaveBeenCalledWith( - 'fcport.create', - [{ target_id: fakeTargetId, port: fakePort }], + 'fcport.query', + [[['target.id', '=', fakeTargetId]]], ); + // Should not call create or delete + expect(spectator.inject(ApiService).call).toHaveBeenCalledTimes(1); }); - it('updates link', async () => { - await lastValueFrom(spectator.service.linkFiberChannelToTarget(fakeTargetId, fakePort)); + it('handles 0→1 transition (create single port)', async () => { + jest.spyOn(spectator.inject(ApiService), 'call').mockImplementation((method) => { + if (method === 'fcport.query') { + return of([]); + } + if (method === 'fcport.create') { + return of({ id: 1, port: 'fc0' }); + } + return of(null); + }); + + await lastValueFrom( + spectator.service.linkFiberChannelPortsToTarget(fakeTargetId, [ + { port: 'fc0', host_id: null }, + ]), + ); expect(spectator.inject(ApiService).call).toHaveBeenCalledWith( - 'fcport.update', - [fakePortId, { target_id: fakeTargetId, port: fakePort }], + 'fcport.create', + [{ port: 'fc0', target_id: fakeTargetId }], ); }); - it('deletes link when new port is null', async () => { - await lastValueFrom(spectator.service.linkFiberChannelToTarget(fakeTargetId, null)); + it('handles 1→0 transition (delete existing port)', async () => { + const existingPort = { id: 1, port: 'fc0' } as FibreChannelPort; + jest.spyOn(spectator.inject(ApiService), 'call').mockImplementation((method) => { + if (method === 'fcport.query') { + return of([existingPort]); + } + if (method === 'fcport.delete') { + return of(true); + } + return of(null); + }); - expect(spectator.inject(ApiService).call).toHaveBeenCalledWith('fcport.delete', [fakePortId]); + await lastValueFrom( + spectator.service.linkFiberChannelPortsToTarget(fakeTargetId, []), + ); + + expect(spectator.inject(ApiService).call).toHaveBeenCalledWith('fcport.delete', [1]); }); - it('skips all operations when new port is the same', async () => { - await lastValueFrom(spectator.service.linkFiberChannelToTarget(fakeTargetId, 'fc/2')); + it('handles 1→1 transition (no change, same port)', async () => { + const existingPort = { id: 1, port: 'fc0' } as FibreChannelPort; + jest.spyOn(spectator.inject(ApiService), 'call').mockImplementation((method) => { + if (method === 'fcport.query') { + return of([existingPort]); + } + return of(null); + }); - expect(spectator.inject(ApiService).call).toHaveBeenCalledWith( - 'fcport.query', - [[['target.id', '=', fakeTargetId]]], + const result = await lastValueFrom( + spectator.service.linkFiberChannelPortsToTarget(fakeTargetId, [ + { port: 'fc0', host_id: null }, + ]), ); + + expect(result).toBe(true); + // Should only call query, no create or delete expect(spectator.inject(ApiService).call).toHaveBeenCalledTimes(1); }); - it('creates new port and updates link', async () => { - await lastValueFrom(spectator.service.linkFiberChannelToTarget(fakeTargetId, '', fakeHostId)); + it('handles 1→1 transition (change to different port)', async () => { + const existingPort = { id: 1, port: 'fc0' } as FibreChannelPort; + jest.spyOn(spectator.inject(ApiService), 'call').mockImplementation((method) => { + if (method === 'fcport.query') { + return of([existingPort]); + } + if (method === 'fcport.delete') { + return of(true); + } + if (method === 'fcport.create') { + return of({ id: 2, port: 'fc1' }); + } + return of(null); + }); + + await lastValueFrom( + spectator.service.linkFiberChannelPortsToTarget(fakeTargetId, [ + { port: 'fc1', host_id: null }, + ]), + ); + + expect(spectator.inject(ApiService).call).toHaveBeenCalledWith('fcport.delete', [1]); + expect(spectator.inject(ApiService).call).toHaveBeenCalledWith( + 'fcport.create', + [{ port: 'fc1', target_id: fakeTargetId }], + ); + }); + + it('handles 1→N transition (add more ports)', async () => { + const existingPort = { id: 1, port: 'fc0' } as FibreChannelPort; + jest.spyOn(spectator.inject(ApiService), 'call').mockImplementation((method) => { + if (method === 'fcport.query') { + return of([existingPort]); + } + if (method === 'fcport.create') { + return of({ id: 2 }); + } + return of(null); + }); + + await lastValueFrom( + spectator.service.linkFiberChannelPortsToTarget(fakeTargetId, [ + { port: 'fc0', host_id: null }, + { port: 'fc1', host_id: null }, + { port: 'fc2', host_id: null }, + ]), + ); + + // Should not delete fc0 (still desired) + expect(spectator.inject(ApiService).call).not.toHaveBeenCalledWith('fcport.delete', expect.anything()); + // Should create fc1 and fc2 + expect(spectator.inject(ApiService).call).toHaveBeenCalledWith( + 'fcport.create', + [{ port: 'fc1', target_id: fakeTargetId }], + ); + expect(spectator.inject(ApiService).call).toHaveBeenCalledWith( + 'fcport.create', + [{ port: 'fc2', target_id: fakeTargetId }], + ); + }); + + it('creates virtual port when host_id provided', async () => { + const host = { id: fakeHostId, alias: 'fc', npiv: 1 } as FibreChannelHost; + jest.spyOn(spectator.inject(ApiService), 'call').mockImplementation((method) => { + if (method === 'fcport.query') { + return of([]); + } + if (method === 'fc.fc_host.query') { + return of([host]); + } + if (method === 'fc.fc_host.update') { + return of({ ...host, npiv: 2 }); + } + if (method === 'fcport.create') { + return of({ id: 1 }); + } + return of(null); + }); + + await lastValueFrom( + spectator.service.linkFiberChannelPortsToTarget(fakeTargetId, [ + { port: null, host_id: fakeHostId }, + ]), + ); expect(spectator.inject(ApiService).call).toHaveBeenCalledWith( 'fc.fc_host.update', [fakeHostId, { npiv: 2 }], ); + expect(spectator.inject(ApiService).call).toHaveBeenCalledWith( + 'fcport.create', + [{ port: 'fc/2', target_id: fakeTargetId }], + ); + }); - expect(spectator.inject(ApiService).call).toHaveBeenLastCalledWith( - 'fcport.update', - [fakePortId, { target_id: fakeTargetId, port: 'fc/2' }], + it('handles mixed desired ports (some existing, some new, some to delete)', async () => { + const existingPorts = [ + { id: 1, port: 'fc0' }, + { id: 2, port: 'fc1' }, + { id: 3, port: 'fc2' }, + ] as FibreChannelPort[]; + + jest.spyOn(spectator.inject(ApiService), 'call').mockImplementation((method) => { + if (method === 'fcport.query') { + return of(existingPorts); + } + if (method === 'fcport.delete') { + return of(true); + } + if (method === 'fcport.create') { + return of({ id: 4 }); + } + return of(null); + }); + + await lastValueFrom( + spectator.service.linkFiberChannelPortsToTarget(fakeTargetId, [ + { port: 'fc0', host_id: null }, // Keep + { port: 'fc3', host_id: null }, // Create + // fc1 and fc2 should be deleted + ]), + ); + + // Should delete fc1 and fc2 + expect(spectator.inject(ApiService).call).toHaveBeenCalledWith('fcport.delete', [2]); + expect(spectator.inject(ApiService).call).toHaveBeenCalledWith('fcport.delete', [3]); + // Should create fc3 + expect(spectator.inject(ApiService).call).toHaveBeenCalledWith( + 'fcport.create', + [{ port: 'fc3', target_id: fakeTargetId }], ); + // Should not touch fc0 + expect(spectator.inject(ApiService).call).not.toHaveBeenCalledWith('fcport.delete', [1]); }); }); }); diff --git a/src/app/services/fibre-channel.service.ts b/src/app/services/fibre-channel.service.ts index 211fc85db68..a53b5dd4382 100644 --- a/src/app/services/fibre-channel.service.ts +++ b/src/app/services/fibre-channel.service.ts @@ -2,7 +2,7 @@ import { Injectable, inject } from '@angular/core'; import { forkJoin, map, Observable, of, switchMap, } from 'rxjs'; -import { FibreChannelPort, FibreChannelPortUpdate } from 'app/interfaces/fibre-channel.interface'; +import { FcPortFormValue, FibreChannelPort } from 'app/interfaces/fibre-channel.interface'; import { ApiService } from 'app/modules/websocket/api.service'; @Injectable({ providedIn: 'root' }) @@ -10,42 +10,135 @@ export class FibreChannelService { private api = inject(ApiService); - loadTargetPort(targetId: number): Observable { - return this.api.call('fcport.query', [[['target.id', '=', targetId]]]).pipe( - map((ports) => ports[0]), - ); + /** + * Load all FC ports associated with a target. + * @param targetId Target ID. + * @returns Observable of FibreChannelPort array. + */ + loadTargetPorts(targetId: number): Observable { + return this.api.call('fcport.query', [[['target.id', '=', targetId]]]); + } + + /** + * Extracts the physical port identifier from a port form value. + * @param portForm The port form value containing either a port string or host_id + * @param hosts Array of FC hosts for looking up host aliases when creating virtual ports + * @returns The physical port identifier (e.g., "fc0" from "fc0/1"), or null if undetermined + */ + getPhysicalPort( + portForm: FcPortFormValue, + hosts: { id: number; alias: string }[] = [], + ): string | null { + if (portForm.port) { + // Existing port: extract physical port from port string + // Example: "fc0/1" → "fc0", "fc1" → "fc1" + return portForm.port.split('/')[0]; + } + + if (portForm.host_id) { + // New virtual port: look up physical port from host_id + const hostMap = new Map(hosts.map((host) => [host.id, host.alias])); + return hostMap.get(portForm.host_id) || null; + } + + return null; + } + + /** + * Validates that all ports in the array use different physical ports. + * @param ports Array of port form values to validate. + * @param hosts Array of FC hosts (required for validating host_id entries). + * @returns Validation result with list of duplicate ports if any. + */ + validatePhysicalPortUniqueness( + ports: FcPortFormValue[], + hosts: { id: number; alias: string }[] = [], + ): { valid: boolean; duplicates: string[] } { + const physicalPorts = new Set(); + const duplicates = new Set(); + + ports.forEach((portForm) => { + const physicalPort = this.getPhysicalPort(portForm, hosts); + + if (!physicalPort) { + return; // Skip if we couldn't determine the physical port + } + + if (physicalPorts.has(physicalPort)) { + duplicates.add(physicalPort); + } else { + physicalPorts.add(physicalPort); + } + }); + + return { + valid: duplicates.size === 0, + duplicates: Array.from(duplicates), + }; } /** - * Specifies the association between target and fiber channel. + * Links multiple FC ports to a target for MPIO support. + * Compares desired ports with existing ports and performs create/delete operations as needed. * @param targetId Target ID. - * @param port Fiber channel port. May not be specified when hostId is present. - * @param hostId Host ID. Must be specified to create a new virtual port when port is not specified. + * @param desiredPorts Array of port form values (port string or host_id for virtual port creation). + * @returns Observable that completes when all port operations are done. */ - linkFiberChannelToTarget( + linkFiberChannelPortsToTarget( targetId: number, - port: string | null, - hostId?: number, - ): Observable { - const fcPort$ = hostId ? this.createNewPort(hostId) : of(port); - - return forkJoin([ - fcPort$, - this.loadTargetPort(targetId), - ]).pipe( - switchMap(([desiredPort, existingPort]) => { - const existingPortId = existingPort?.id || null; - if (port === (existingPort?.port || null)) { - return of(null); - } - if (port === null && existingPortId) { - return this.api.call('fcport.delete', [existingPortId]); - } - - const payload = { port: desiredPort, target_id: targetId } as FibreChannelPortUpdate; - return existingPortId - ? this.api.call('fcport.update', [existingPortId, payload]) - : this.api.call('fcport.create', [payload]); + desiredPorts: FcPortFormValue[], + ): Observable { + return this.loadTargetPorts(targetId).pipe( + switchMap((existingPorts) => { + const operations: Observable[] = []; + + // Build map of existing ports by port string + const existingMap = new Map( + existingPorts.map((port) => [port.port, port.id]), + ); + + // Resolve desired ports (handle host_id → port string conversion) + const resolvedPorts$ = forkJoin( + desiredPorts.length > 0 + ? desiredPorts.map((portForm) => ( + portForm.host_id ? this.createNewPort(portForm.host_id) : of(portForm.port) + )) + : [of(null)], + ); + + return resolvedPorts$.pipe( + switchMap((resolvedPorts) => { + const desiredSet = new Set(resolvedPorts.filter((portString) => portString !== null) as string[]); + + // Delete ports that are no longer desired + existingPorts.forEach((existing) => { + if (!desiredSet.has(existing.port)) { + operations.push( + this.api.call('fcport.delete', [existing.id]), + ); + } + }); + + // Create new ports + resolvedPorts.forEach((port) => { + if (port && !existingMap.has(port)) { + operations.push( + this.api.call('fcport.create', [{ + port, + target_id: targetId, + }]), + ); + } + }); + + // If no operations, return success + if (operations.length === 0) { + return of(true); + } + + return forkJoin(operations); + }), + ); }), ); }