NAS-133547 / 25.04 / Fibre Channel QA#2 (#11343)

* NAS-133547: Fibre Channel QA#2 * NAS-133547: PR update * NAS-133547: PR Update
truenas · Jan 20, 2025 · 644e2a8 · 644e2a8
1 parent ddd0812
commit 644e2a8
Show file tree

Hide file tree

Showing 105 changed files with 232 additions and 154 deletions.
diff --git a/src/app/modules/card-expand-collapse/card-expand-collapse.component.ts b/src/app/modules/card-expand-collapse/card-expand-collapse.component.ts
@@ -1,4 +1,5 @@
 import {
+  AfterViewInit,
   ChangeDetectionStrategy,
   Component,
   computed,
@@ -10,9 +11,12 @@ import {
   viewChild,
 } from '@angular/core';
 import { MatButton } from '@angular/material/button';
+import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
 import { TranslateModule } from '@ngx-translate/core';
+import { timer } from 'rxjs';
 import { TestDirective } from 'app/modules/test-id/test.directive';
 
+@UntilDestroy()
 @Component({
   selector: 'ix-card-expand-collapse',
   templateUrl: './card-expand-collapse.component.html',
@@ -25,7 +29,7 @@ import { TestDirective } from 'app/modules/test-id/test.directive';
     TestDirective,
   ],
 })
-export class CardExpandCollapseComponent implements OnChanges {
+export class CardExpandCollapseComponent implements OnChanges, AfterViewInit {
   section = viewChild.required<ElementRef<HTMLElement>>('section');
   maxHeight = input<number>(250);
   height = signal<number>(this.maxHeight());
@@ -48,6 +52,10 @@ export class CardExpandCollapseComponent implements OnChanges {
     this.section().nativeElement.style.maxHeight = `${this.maxHeight()}px`;
   }
 
+  ngAfterViewInit(): void {
+    timer(0).pipe(untilDestroyed(this)).subscribe(() => this.setHeight());
+  }
+
   changeCollapsed(): void {
     this.setHeight();
     this.isCollapsed.set(!this.isCollapsed());

diff --git a/...ages/sharing/iscsi/fibre-channel-ports/fc-ports-controls/fc-ports-controls.component.html b/...ages/sharing/iscsi/fibre-channel-ports/fc-ports-controls/fc-ports-controls.component.html
@@ -1,21 +1,23 @@
 <ix-radio-group
-  [formControl]="isNewControl"
+  [formControl]="optionsControl"
   [options]="isNewOptions$"
 ></ix-radio-group>
 <ng-container [formGroup]="form()">
-  @if(isNewControl.value) {
-    <ix-select
-      formControlName="host_id"
-      [label]="'Choose a new virtual port' | translate"
-      [options]="creatingPortOptions$"
-      [required]="true"
-    ></ix-select>
-  } @else {
-    <ix-select
-      formControlName="port"
-      [label]="'Existing Ports' | translate"
-      [options]="existingPortOptions$"
-      [required]="true"
-    ></ix-select>
+  @if (optionsControl.value !== null) {
+    @if(optionsControl.value) {
+      <ix-select
+        formControlName="host_id"
+        [label]="'Choose a new virtual port' | translate"
+        [options]="creatingPortOptions$"
+        [required]="true"
+      ></ix-select>
+    } @else {
+      <ix-select
+        formControlName="port"
+        [label]="'Existing Ports' | translate"
+        [options]="existingPortOptions$"
+        [required]="true"
+      ></ix-select>
+    }
   }
 </ng-container>
diff --git a/.../pages/sharing/iscsi/fibre-channel-ports/fc-ports-controls/fc-ports-controls.component.ts b/.../pages/sharing/iscsi/fibre-channel-ports/fc-ports-controls/fc-ports-controls.component.ts
@@ -6,7 +6,7 @@ import { FormBuilder } from '@ngneat/reactive-forms';
 import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
 import { TranslateService, TranslateModule } from '@ngx-translate/core';
 import { map, Observable, of } from 'rxjs';
-import { Option, nullOption, skipOption } from 'app/interfaces/option.interface';
+import { Option, skipOption } from 'app/interfaces/option.interface';
 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';
@@ -29,9 +29,10 @@ export class FcPortsControlsComponent implements OnInit {
   form = input.required<TargetFormComponent['fcForm']>();
   isEdit = input(false);
 
-  isNewControl = this.fb.control(false);
+  optionsControl = this.fb.control(null);
 
   readonly isNewOptions$: Observable<Option<boolean>[]> = of([
+    { label: this.translate.instant('Do not connect to a fibre channel port'), value: null },
     { label: this.translate.instant('Use an existing port'), value: false },
     { label: this.translate.instant('Create new virtual port'), value: true },
   ]);
@@ -44,18 +45,7 @@ export class FcPortsControlsComponent implements OnInit {
   }));
 
   readonly existingPortOptions$ = this.api.call('fcport.port_choices', [false]).pipe(map((ports) => {
-    const option = [{
-      label: this.translate.instant('Do not connect to a fibre channel port'),
-      value: nullOption,
-    }];
-
-    if (this.isEdit()) {
-      option.push({
-        label: this.translate.instant('Use current port'),
-        value: skipOption,
-      });
-    }
-    return option.concat(Object.entries(ports).map(([value]) => ({ label: value, value })));
+    return Object.entries(ports).map(([value]) => ({ label: value, value }));
   }));
 
   constructor(
@@ -66,8 +56,18 @@ export class FcPortsControlsComponent implements OnInit {
 
   ngOnInit(): void {
     this.form().controls.host_id.disable();
-    this.isNewControl.valueChanges.pipe(untilDestroyed(this)).subscribe((isNew) => {
-      if (isNew) {
+    this.form().controls.port.setValue(skipOption);
+
+    this.listenToOptionControlChanges();
+  }
+
+  private listenToOptionControlChanges(): void {
+    this.optionsControl.valueChanges.pipe(untilDestroyed(this)).subscribe((isNew) => {
+      if (isNew === null) {
+        this.form().controls.port.setValue(skipOption);
+        this.form().controls.host_id.setValue(null);
+        this.form().controls.host_id.disable();
+      } else if (isNew) {
         this.form().controls.port.disable();
         this.form().controls.host_id.enable();
         this.form().controls.port.setValue(null);

diff --git a/src/app/pages/sharing/iscsi/fibre-channel-ports/fibre-channel-ports.component.html b/src/app/pages/sharing/iscsi/fibre-channel-ports/fibre-channel-ports.component.html
@@ -11,7 +11,6 @@ <h3>{{ 'Fibre Channel Ports' | translate }}</h3>
     </div>
   </mat-toolbar-row>
   <mat-card-content>
-
     <ix-table
       [ixUiSearch]="searchableElements.elements.list"
       [ix-table-empty]="!(dataProvider.currentPageCount$ | async)"

diff --git a/...t/all-targets/target-details/associated-extents-card/associated-extents-card.component.ts b/...t/all-targets/target-details/associated-extents-card/associated-extents-card.component.ts
@@ -1,8 +1,7 @@
 import {
-  ChangeDetectionStrategy, ChangeDetectorRef, Component, computed, input,
+  ChangeDetectionStrategy, ChangeDetectorRef, Component, computed, effect, input,
   signal,
 } from '@angular/core';
-import { toSignal } from '@angular/core/rxjs-interop';
 import { MatButtonModule, MatIconButton } from '@angular/material/button';
 import {
   MatCard, MatCardContent, MatCardHeader, MatCardTitle,
@@ -12,7 +11,9 @@ import { MatTooltip } from '@angular/material/tooltip';
 import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
 import { TranslateModule, TranslateService } from '@ngx-translate/core';
 import { NgxSkeletonLoaderModule } from 'ngx-skeleton-loader';
-import { filter, switchMap, take } from 'rxjs';
+import {
+  filter, finalize, forkJoin, switchMap, take,
+} from 'rxjs';
 import { RequiresRolesDirective } from 'app/directives/requires-roles/requires-roles.directive';
 import { Role } from 'app/enums/role.enum';
 import {
@@ -50,10 +51,9 @@ import { IscsiService } from 'app/services/iscsi.service';
 export class AssociatedExtentsCardComponent {
   readonly target = input.required<IscsiTarget>();
 
+  readonly isLoadingExtents = signal<boolean>(false);
   readonly targetExtents = signal<IscsiTargetExtent[]>([]);
-  readonly isLoadingExtents = signal<boolean>(true);
-
-  readonly extents = toSignal(this.iscsiService.getExtents(), { initialValue: [] });
+  readonly extents = signal<IscsiExtent[]>([]);
 
   readonly unassociatedExtents = computed(() => {
     return this.extents().filter((extent) => {
@@ -84,7 +84,11 @@ export class AssociatedExtentsCardComponent {
     private dialogService: DialogService,
     private translate: TranslateService,
   ) {
-    this.getTargetExtents();
+    effect(() => {
+      if (this.target()) {
+        this.getTargetExtents();
+      }
+    });
   }
 
   associateTarget(): void {
@@ -112,17 +116,22 @@ export class AssociatedExtentsCardComponent {
       filter(Boolean),
       switchMap(() => this.iscsiService.deleteTargetExtent(extent.id).pipe(this.loader.withLoader())),
       untilDestroyed(this),
-    )
-      .subscribe(() => this.getTargetExtents());
+    ).subscribe(() => this.getTargetExtents());
   }
 
   private getTargetExtents(): void {
-    this.iscsiService.getTargetExtents().pipe(
+    this.isLoadingExtents.set(true);
+
+    forkJoin([
+      this.iscsiService.getExtents(),
+      this.iscsiService.getTargetExtents(),
+    ]).pipe(
       take(1),
       untilDestroyed(this),
-    ).subscribe((extents) => {
-      this.targetExtents.set(extents);
-      this.isLoadingExtents.set(false);
+      finalize(() => this.isLoadingExtents.set(false)),
+    ).subscribe(([extents, targetExtents]) => {
+      this.extents.set(extents);
+      this.targetExtents.set(targetExtents);
       this.cdr.markForCheck();
     });
   }

diff --git a/...all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.html b/...all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.html
@@ -5,8 +5,16 @@ <h3 mat-card-title>
     </h3>
   </mat-card-header>
   <mat-card-content>
-    <p>{{ 'Name' | translate }}: {{ port().id }}</p>
-    <p>{{ 'Controller A WWPN' | translate }}: {{ port().wwpn }}</p>
-    <p>{{ 'Controller B WWPN' | translate }}: {{ port().wwpn_b }}</p>
+    @if (isLoading()) {
+      <ngx-skeleton-loader></ngx-skeleton-loader>
+    } @else {
+      @if (port(); as port) {
+        <p>{{ 'Port' | translate }}: {{ port.port }}</p>
+        <p>{{ 'Controller A WWPN' | translate }}: {{ port.wwpn }}</p>
+        <p>{{ 'Controller B WWPN' | translate }}: {{ port.wwpn_b }}</p>
+      } @else {
+        <p>{{ 'No associated Fibre Channel port' | translate }}</p>
+      }
+    }
   </mat-card-content>
 </mat-card>
diff --git a/...-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.spec.ts b/...-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.spec.ts
@@ -13,8 +13,9 @@ describe('FibreChannelPortCardComponent', () => {
   beforeEach(() => {
     spectator = createComponent({
       props: {
+        isLoading: false,
         port: {
-          id: 'Port-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,
@@ -30,8 +31,15 @@ describe('FibreChannelPortCardComponent', () => {
   it('displays port details correctly', () => {
     const content = spectator.queryAll('mat-card-content p');
     expect(content).toHaveLength(3);
-    expect(content[0]).toHaveText('Name: Port-1');
+    expect(content[0]).toHaveText('Port: fc1/5');
     expect(content[1]).toHaveText('Controller A WWPN: 10:00:00:00:c9:20:00:00');
     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);
+    spectator.setInput('isLoading', false);
+    const content = spectator.query('mat-card-content');
+    expect(content).toHaveText('No associated Fibre Channel port');
+  });
 });
diff --git a/...t/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.ts b/...t/all-targets/target-details/fibre-channel-port-card/fibre-channel-port-card.component.ts
@@ -1,8 +1,11 @@
-import { ChangeDetectionStrategy, Component, input } from '@angular/core';
+import {
+  ChangeDetectionStrategy, Component, input,
+} from '@angular/core';
 import {
   MatCard, MatCardContent, MatCardHeader, MatCardTitle,
 } from '@angular/material/card';
 import { TranslateModule } from '@ngx-translate/core';
+import { NgxSkeletonLoaderModule } from 'ngx-skeleton-loader';
 import { FibreChannelPort } from 'app/interfaces/fibre-channel.interface';
 
 @Component({
@@ -17,8 +20,10 @@ import { FibreChannelPort } from 'app/interfaces/fibre-channel.interface';
     MatCardTitle,
     TranslateModule,
     MatCardContent,
+    NgxSkeletonLoaderModule,
   ],
 })
 export class FibreChannelPortCardComponent {
   readonly port = input.required<FibreChannelPort>();
+  readonly isLoading = input.required<boolean>();
 }
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
@@ -5,8 +5,12 @@
     }
 
     @if (hasFibreCards()) {
+      <ix-fibre-channel-port-card
+        [port]="targetPort()"
+        [isLoading]="isLoading()"
+      ></ix-fibre-channel-port-card>
+
       @if (targetPort()) {
-        <ix-fibre-channel-port-card [port]="targetPort()"></ix-fibre-channel-port-card>
         <ix-fibre-channel-connections-card [connections]="connections()"></ix-fibre-channel-connections-card>
       }
     }

diff --git a/...pp/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.spec.ts b/...pp/pages/sharing/iscsi/target/all-targets/target-details/target-details.component.spec.ts
@@ -78,11 +78,11 @@ describe('TargetDetailsComponent', () => {
     expect(spectator.query(FibreChannelPortCardComponent)?.port).toEqual(mockPort);
   });
 
-  it('does not render FibreChannelPortCardComponent if no targetPort is available', () => {
+  it('should render FibreChannelPortCardComponent even if no targetPort is available', () => {
     spectator.component.targetPort.set(null);
     spectator.detectChanges();
 
-    expect(spectator.query(FibreChannelPortCardComponent)).toBeNull();
+    expect(spectator.query(FibreChannelPortCardComponent)).not.toBeNull();
   });
 
   it('calls API to fetch Fibre Channel ports when target ID changes', () => {

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
@@ -4,7 +4,7 @@ import {
 } from '@angular/core';
 import { toSignal } from '@angular/core/rxjs-interop';
 import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
-import { take } from 'rxjs';
+import { finalize, take } from 'rxjs';
 import { IscsiTargetMode } from 'app/enums/iscsi.enum';
 import { FibreChannelPort } from 'app/interfaces/fibre-channel.interface';
 import { IscsiTarget } from 'app/interfaces/iscsi.interface';
@@ -35,6 +35,8 @@ export class TargetDetailsComponent {
   readonly target = input.required<IscsiTarget>();
 
   targetPort = signal<FibreChannelPort | null>(null);
+  isLoading = signal<boolean>(false);
+
   connections = toSignal(this.api.call('fcport.status'));
 
   protected hasIscsiCards = computed(() => [
@@ -61,9 +63,12 @@ export class TargetDetailsComponent {
   }
 
   private getPortByTargetId(id: number): void {
+    this.isLoading.set(true);
+
     this.api.call('fcport.query', [[['target.id', '=', id]]])
       .pipe(
         take(1),
+        finalize(() => this.isLoading.set(false)),
         untilDestroyed(this),
       )
       .subscribe((ports) => {

diff --git a/src/app/pages/sharing/iscsi/target/all-targets/target-list/target-list.component.html b/src/app/pages/sharing/iscsi/target/all-targets/target-list/target-list.component.html
@@ -27,7 +27,15 @@ <h3>{{ 'Targets' | translate }}</h3>
         [dataProvider]="dataProvider()"
         [isLoading]="!!(dataProvider().isLoading$ | async)"
         (expanded)="expanded($event)"
-      ></tbody>
+      >
+        <ng-template
+          ix-table-cell
+          [columnIndex]="3"
+          [dataProvider]="dataProvider()"
+        >
+          <ix-icon name="mdi-chevron-right"></ix-icon>
+        </ng-template>
+      </tbody>
     </ix-table>
     <ix-table-pager [dataProvider]="dataProvider()"></ix-table-pager>
   </mat-card-content>

diff --git a/src/app/pages/sharing/iscsi/target/all-targets/target-list/target-list.component.scss b/src/app/pages/sharing/iscsi/target/all-targets/target-list/target-list.component.scss
@@ -0,0 +1,12 @@
+@import 'scss-imports/variables';
+
+:host ::ng-deep .view-details-column {
+  align-items: center;
+  display: flex;
+  height: inherit;
+  justify-content: flex-end;
+
+  @media (min-width: calc($breakpoint-tablet + 1px)) {
+    display: none;
+  }
+}