From 34593c571b2850b881b41e4eefd080ed01eeaeff Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Wed, 17 Dec 2025 14:11:31 -0600 Subject: [PATCH 1/7] NAS-138487: fix: prevent iSCSI target details from closing on navigation --- .../target/all-targets/all-targets.component.html | 2 +- .../iscsi/target/all-targets/all-targets.component.ts | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.html b/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.html index 1c5a29587ba..c29dada5797 100644 --- a/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.html +++ b/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.html @@ -5,7 +5,7 @@ #masterDetailView="masterDetailViewContext" [selectedItem]="target" [showDetails]="showDetails" - (mobileDetailsClosed)="dataProvider.expandedRow = null" + (mobileDetailsClosed)="onMobileDetailsClosed()" > Date: Thu, 18 Dec 2025 12:46:04 -0600 Subject: [PATCH 2/7] NAS-138487: test: introduce minimal tests for `all-targets` component --- .../all-targets/all-targets.component.spec.ts | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts diff --git a/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts b/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts new file mode 100644 index 00000000000..8fe8355bdda --- /dev/null +++ b/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts @@ -0,0 +1,77 @@ +import { signal } from '@angular/core'; +import { MatDialog } from '@angular/material/dialog'; +import { createComponentFactory, mockProvider, Spectator } from '@ngneat/spectator/jest'; +import { of } from 'rxjs'; +import { IscsiTarget } from 'app/interfaces/iscsi.interface'; +import { AsyncDataProvider } from 'app/modules/ix-table/classes/async-data-provider/async-data-provider'; +import { SlideIn } from 'app/modules/slide-ins/slide-in'; +import { AllTargetsComponent } from 'app/pages/sharing/iscsi/target/all-targets/all-targets.component'; +import { IscsiService } from 'app/services/iscsi.service'; + +// this test suite is pretty minimal, since this component doesn't have a lot of user-facing +// functionality we need to test. regardless, since we introduced `onMobileDetailsClosed`, +// we need to test it to prevent regressions. +describe('AllTargetsComponent', () => { + let spectator: Spectator; + const createComponent = createComponentFactory({ + component: AllTargetsComponent, + providers: [ + mockProvider(IscsiService, { + getTargets: jest.fn(() => of([])), + listenForDataRefresh: jest.fn(() => of(null)), + }), + mockProvider(MatDialog), + mockProvider(SlideIn), + ], + }); + + beforeEach(() => { + spectator = createComponent(); + }); + + it('should create', () => { + expect(spectator.component).toBeTruthy(); + }); + + describe('onMobileDetailsClosed', () => { + it('clears the selected target when in mobile view', () => { + // create a mock data provider and rig it to the component + // we have to use `Object.defineProperty` here since they're protected values, and we don't + // have a way to mock them otherwise. this is generally ill-advised, but in such a small case it's permissible. + const mockTarget = { id: 1, name: 'iSCSI-1' } as IscsiTarget; + const dataProvider = new AsyncDataProvider(of([mockTarget])); + dataProvider.expandedRow = mockTarget; + Object.defineProperty(spectator.component, 'dataProvider', { + value: dataProvider, + }); + + // and create a mock master detail view and also add it to the component + const mockMasterDetailView = signal({ isMobileView: () => true }); + Object.defineProperty(spectator.component, 'masterDetailView', { + value: mockMasterDetailView, + }); + + // ensure the component nullifies the expanded row when we're in the mobile view + spectator.component.onMobileDetailsClosed(); + expect(dataProvider.expandedRow).toBeNull(); + }); + + it('keeps the selected target when not in mobile view', () => { + const mockTarget = { id: 1, name: 'iSCSI-1' } as IscsiTarget; + const dataProvider = new AsyncDataProvider(of([mockTarget])); + dataProvider.expandedRow = mockTarget; + Object.defineProperty(spectator.component, 'dataProvider', { + value: dataProvider, + }); + + const mockMasterDetailView = signal({ isMobileView: () => false }); + Object.defineProperty(spectator.component, 'masterDetailView', { + value: mockMasterDetailView, + }); + + spectator.component.onMobileDetailsClosed(); + expect(dataProvider.expandedRow).not.toBeNull(); + expect(dataProvider.expandedRow).toBe(mockTarget); + }); + }); +}); From ac2485f21d80c67c9b2a809ee7cda59437f9fcd0 Mon Sep 17 00:00:00 2001 From: Aaron Ervin Date: Thu, 18 Dec 2025 12:58:22 -0600 Subject: [PATCH 3/7] NAS-138487: NAS-000000: use correct FCP_SHARE purpose name in smb-form test (#13012) Co-authored-by: Aaron Ervin --- .../smb/smb-form/smb-form.component.spec.ts | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/src/app/pages/sharing/smb/smb-form/smb-form.component.spec.ts b/src/app/pages/sharing/smb/smb-form/smb-form.component.spec.ts index 1a483e5ca55..ff147f4695c 100644 --- a/src/app/pages/sharing/smb/smb-form/smb-form.component.spec.ts +++ b/src/app/pages/sharing/smb/smb-form/smb-form.component.spec.ts @@ -1383,6 +1383,158 @@ describe('SmbFormComponent', () => { }); }); + describe('Submit button behavior with Apple SMB2/3 extensions', () => { + beforeEach(async () => { + spectator = createComponent(); + api = spectator.inject(ApiService); + loader = TestbedHarnessEnvironment.loader(spectator.fixture); + form = await loader.getHarness(IxFormHarness); + mockStore$ = spectator.inject(MockStore); + + jest.spyOn(api, 'call').mockImplementation((method) => { + if (method === 'smb.config') { + return of({ aapl_extensions: false } as SmbConfig); + } + return of(null); + }); + }); + + it('should disable submit button when extensions warning is shown', async () => { + // Select Time Machine Share (requires Apple SMB2/3 extensions) + await form.fillForm({ + Purpose: 'Time Machine Share', + Path: '/mnt/pool/timemachine', + Name: 'timemachine', + }); + + // Manually set the smbConfig signal to trigger the warning + // eslint-disable-next-line @typescript-eslint/dot-notation + spectator.component['smbConfig'].set({ aapl_extensions: false } as SmbConfig); + // eslint-disable-next-line @typescript-eslint/dot-notation + spectator.component['updateExtensionsWarning'](); + spectator.detectChanges(); + + // Verify warning is shown + // eslint-disable-next-line @typescript-eslint/dot-notation + expect(spectator.component['showExtensionsWarning']()).toBe(true); + + // Verify submit button is disabled + const saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save' })); + expect(await saveButton.isDisabled()).toBe(true); + + // Verify warning component is shown with correct id + const warningElement = spectator.query('#apple-extensions-warning'); + expect(warningElement).toBeTruthy(); + }); + + it('should enable submit button after extensions are enabled', async () => { + // Select Time Machine Share (requires Apple SMB2/3 extensions) + await form.fillForm({ + Purpose: 'Time Machine Share', + Path: '/mnt/pool/timemachine', + Name: 'timemachine', + }); + + // Set config to show warning + // eslint-disable-next-line @typescript-eslint/dot-notation + spectator.component['smbConfig'].set({ aapl_extensions: false } as SmbConfig); + // eslint-disable-next-line @typescript-eslint/dot-notation + spectator.component['updateExtensionsWarning'](); + spectator.detectChanges(); + + // Verify button is disabled + const saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save' })); + expect(await saveButton.isDisabled()).toBe(true); + + // Enable extensions + // eslint-disable-next-line @typescript-eslint/dot-notation + spectator.component['extensionsEnabled'](); + spectator.detectChanges(); + + // Verify warning is gone and button is enabled + // eslint-disable-next-line @typescript-eslint/dot-notation + expect(spectator.component['showExtensionsWarning']()).toBe(false); + expect(await saveButton.isDisabled()).toBe(false); + + // Verify warning component is no longer shown + const warningElement = spectator.query('#apple-extensions-warning'); + expect(warningElement).toBeFalsy(); + }); + + it('should show warning reactively when switching between purposes requiring extensions', async () => { + // Start with Default Share (no warning) + await form.fillForm({ + Purpose: 'Default Share', + Path: '/mnt/pool/default', + Name: 'default', + }); + + // eslint-disable-next-line @typescript-eslint/dot-notation + spectator.component['smbConfig'].set({ aapl_extensions: false } as SmbConfig); + // eslint-disable-next-line @typescript-eslint/dot-notation + spectator.component['updateExtensionsWarning'](); + spectator.detectChanges(); + + // Verify no warning for Default Share + // eslint-disable-next-line @typescript-eslint/dot-notation + expect(spectator.component['showExtensionsWarning']()).toBe(false); + let saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save' })); + expect(await saveButton.isDisabled()).toBe(false); + + // Switch to Time Machine Share (warning should appear) + await form.fillForm({ + Purpose: 'Time Machine Share', + }); + // eslint-disable-next-line @typescript-eslint/dot-notation + spectator.component['updateExtensionsWarning'](); + spectator.detectChanges(); + + // Verify warning is shown and button is disabled + // eslint-disable-next-line @typescript-eslint/dot-notation + expect(spectator.component['showExtensionsWarning']()).toBe(true); + expect(await saveButton.isDisabled()).toBe(true); + + // Enable extensions + // eslint-disable-next-line @typescript-eslint/dot-notation + spectator.component['extensionsEnabled'](); + spectator.detectChanges(); + + // Verify button is enabled + // eslint-disable-next-line @typescript-eslint/dot-notation + expect(spectator.component['showExtensionsWarning']()).toBe(false); + expect(await saveButton.isDisabled()).toBe(false); + + // Switch to FCP Share (another purpose requiring extensions with extensions disabled) + // eslint-disable-next-line @typescript-eslint/dot-notation + spectator.component['smbConfig'].set({ aapl_extensions: false } as SmbConfig); + await form.fillForm({ + Purpose: 'Final Cut Pro Storage Share', + }); + // eslint-disable-next-line @typescript-eslint/dot-notation + spectator.component['updateExtensionsWarning'](); + spectator.detectChanges(); + + // Verify warning appears again for FCP share + // eslint-disable-next-line @typescript-eslint/dot-notation + expect(spectator.component['showExtensionsWarning']()).toBe(true); + saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save' })); + expect(await saveButton.isDisabled()).toBe(true); + + // Switch back to Default Share (warning should disappear) + await form.fillForm({ + Purpose: 'Default Share', + }); + // eslint-disable-next-line @typescript-eslint/dot-notation + spectator.component['updateExtensionsWarning'](); + spectator.detectChanges(); + + // Verify warning is gone + // eslint-disable-next-line @typescript-eslint/dot-notation + expect(spectator.component['showExtensionsWarning']()).toBe(false); + expect(await saveButton.isDisabled()).toBe(false); + }); + }); + describe('grace_period validation', () => { beforeEach(async () => { await setupTest(); From de2dc714897772bf47016b32433632e71bda3690 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Thu, 18 Dec 2025 14:42:39 -0600 Subject: [PATCH 4/7] NAS-138487: test: make mobile view detection more testable --- .../target/all-targets/all-targets.component.spec.ts | 9 +++------ .../iscsi/target/all-targets/all-targets.component.ts | 6 +++++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts b/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts index 8fe8355bdda..07e7168fe84 100644 --- a/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts +++ b/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts @@ -35,6 +35,9 @@ describe('AllTargetsComponent', () => { describe('onMobileDetailsClosed', () => { it('clears the selected target when in mobile view', () => { + // pretend we're in mobile view + jest.spyOn(spectator.component, 'isMobileView').mockReturnValue(true); + // create a mock data provider and rig it to the component // we have to use `Object.defineProperty` here since they're protected values, and we don't // have a way to mock them otherwise. this is generally ill-advised, but in such a small case it's permissible. @@ -45,12 +48,6 @@ describe('AllTargetsComponent', () => { value: dataProvider, }); - // and create a mock master detail view and also add it to the component - const mockMasterDetailView = signal({ isMobileView: () => true }); - Object.defineProperty(spectator.component, 'masterDetailView', { - value: mockMasterDetailView, - }); - // ensure the component nullifies the expanded row when we're in the mobile view spectator.component.onMobileDetailsClosed(); expect(dataProvider.expandedRow).toBeNull(); diff --git a/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.ts b/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.ts index bca920756e9..d3d24f552b3 100644 --- a/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.ts +++ b/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.ts @@ -75,13 +75,17 @@ export class AllTargetsComponent implements OnInit { this.dataProvider = new AsyncDataProvider(targets$); } + isMobileView(): boolean { + return this.masterDetailView().isMobileView(); + } + onMobileDetailsClosed(): void { // we only want to clear the current `expandedRow` if we're *actually* in mobile view. // // this statement was previously running when the `ix-master-detail-view` in the template // manually nullified the `expandedRow`, even on desktop. // for this component we need to check and make sure that we're actually in mobile view before clearing anything. - if (this.masterDetailView().isMobileView()) { + if (this.isMobileView()) { this.dataProvider.expandedRow = null; } } From 6414f203e8935e47d227557457a6c16fa7acabd1 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Thu, 18 Dec 2025 14:51:12 -0600 Subject: [PATCH 5/7] NAS-138487: test: more fixes for recent update --- .../target/all-targets/all-targets.component.spec.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts b/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts index 07e7168fe84..bb6f8a50dce 100644 --- a/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts +++ b/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts @@ -1,4 +1,3 @@ -import { signal } from '@angular/core'; import { MatDialog } from '@angular/material/dialog'; import { createComponentFactory, mockProvider, Spectator } from '@ngneat/spectator/jest'; import { of } from 'rxjs'; @@ -54,6 +53,8 @@ describe('AllTargetsComponent', () => { }); it('keeps the selected target when not in mobile view', () => { + jest.spyOn(spectator.component, 'isMobileView').mockReturnValue(false); + const mockTarget = { id: 1, name: 'iSCSI-1' } as IscsiTarget; const dataProvider = new AsyncDataProvider(of([mockTarget])); dataProvider.expandedRow = mockTarget; @@ -61,11 +62,6 @@ describe('AllTargetsComponent', () => { value: dataProvider, }); - const mockMasterDetailView = signal({ isMobileView: () => false }); - Object.defineProperty(spectator.component, 'masterDetailView', { - value: mockMasterDetailView, - }); - spectator.component.onMobileDetailsClosed(); expect(dataProvider.expandedRow).not.toBeNull(); expect(dataProvider.expandedRow).toBe(mockTarget); From 3ad12686e025acda68b9e1b17c562c42f56f786f Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Thu, 18 Dec 2025 15:09:14 -0600 Subject: [PATCH 6/7] NAS-138487: test: add test for `isMobileView` --- .../iscsi/target/all-targets/all-targets.component.spec.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts b/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts index bb6f8a50dce..eed26e73037 100644 --- a/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts +++ b/src/app/pages/sharing/iscsi/target/all-targets/all-targets.component.spec.ts @@ -32,6 +32,10 @@ describe('AllTargetsComponent', () => { expect(spectator.component).toBeTruthy(); }); + it('should not be in mobile view by default', () => { + expect(spectator.component.isMobileView()).toBe(false); + }); + describe('onMobileDetailsClosed', () => { it('clears the selected target when in mobile view', () => { // pretend we're in mobile view From 9e8b7af0154353b9c21cb6c71374c73840ffa960 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Fri, 19 Dec 2025 12:46:56 -0600 Subject: [PATCH 7/7] NAS-138487: test: remove unrelated SMB tests from prior merge --- .../smb/smb-form/smb-form.component.spec.ts | 152 ------------------ 1 file changed, 152 deletions(-) diff --git a/src/app/pages/sharing/smb/smb-form/smb-form.component.spec.ts b/src/app/pages/sharing/smb/smb-form/smb-form.component.spec.ts index ff147f4695c..1a483e5ca55 100644 --- a/src/app/pages/sharing/smb/smb-form/smb-form.component.spec.ts +++ b/src/app/pages/sharing/smb/smb-form/smb-form.component.spec.ts @@ -1383,158 +1383,6 @@ describe('SmbFormComponent', () => { }); }); - describe('Submit button behavior with Apple SMB2/3 extensions', () => { - beforeEach(async () => { - spectator = createComponent(); - api = spectator.inject(ApiService); - loader = TestbedHarnessEnvironment.loader(spectator.fixture); - form = await loader.getHarness(IxFormHarness); - mockStore$ = spectator.inject(MockStore); - - jest.spyOn(api, 'call').mockImplementation((method) => { - if (method === 'smb.config') { - return of({ aapl_extensions: false } as SmbConfig); - } - return of(null); - }); - }); - - it('should disable submit button when extensions warning is shown', async () => { - // Select Time Machine Share (requires Apple SMB2/3 extensions) - await form.fillForm({ - Purpose: 'Time Machine Share', - Path: '/mnt/pool/timemachine', - Name: 'timemachine', - }); - - // Manually set the smbConfig signal to trigger the warning - // eslint-disable-next-line @typescript-eslint/dot-notation - spectator.component['smbConfig'].set({ aapl_extensions: false } as SmbConfig); - // eslint-disable-next-line @typescript-eslint/dot-notation - spectator.component['updateExtensionsWarning'](); - spectator.detectChanges(); - - // Verify warning is shown - // eslint-disable-next-line @typescript-eslint/dot-notation - expect(spectator.component['showExtensionsWarning']()).toBe(true); - - // Verify submit button is disabled - const saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save' })); - expect(await saveButton.isDisabled()).toBe(true); - - // Verify warning component is shown with correct id - const warningElement = spectator.query('#apple-extensions-warning'); - expect(warningElement).toBeTruthy(); - }); - - it('should enable submit button after extensions are enabled', async () => { - // Select Time Machine Share (requires Apple SMB2/3 extensions) - await form.fillForm({ - Purpose: 'Time Machine Share', - Path: '/mnt/pool/timemachine', - Name: 'timemachine', - }); - - // Set config to show warning - // eslint-disable-next-line @typescript-eslint/dot-notation - spectator.component['smbConfig'].set({ aapl_extensions: false } as SmbConfig); - // eslint-disable-next-line @typescript-eslint/dot-notation - spectator.component['updateExtensionsWarning'](); - spectator.detectChanges(); - - // Verify button is disabled - const saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save' })); - expect(await saveButton.isDisabled()).toBe(true); - - // Enable extensions - // eslint-disable-next-line @typescript-eslint/dot-notation - spectator.component['extensionsEnabled'](); - spectator.detectChanges(); - - // Verify warning is gone and button is enabled - // eslint-disable-next-line @typescript-eslint/dot-notation - expect(spectator.component['showExtensionsWarning']()).toBe(false); - expect(await saveButton.isDisabled()).toBe(false); - - // Verify warning component is no longer shown - const warningElement = spectator.query('#apple-extensions-warning'); - expect(warningElement).toBeFalsy(); - }); - - it('should show warning reactively when switching between purposes requiring extensions', async () => { - // Start with Default Share (no warning) - await form.fillForm({ - Purpose: 'Default Share', - Path: '/mnt/pool/default', - Name: 'default', - }); - - // eslint-disable-next-line @typescript-eslint/dot-notation - spectator.component['smbConfig'].set({ aapl_extensions: false } as SmbConfig); - // eslint-disable-next-line @typescript-eslint/dot-notation - spectator.component['updateExtensionsWarning'](); - spectator.detectChanges(); - - // Verify no warning for Default Share - // eslint-disable-next-line @typescript-eslint/dot-notation - expect(spectator.component['showExtensionsWarning']()).toBe(false); - let saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save' })); - expect(await saveButton.isDisabled()).toBe(false); - - // Switch to Time Machine Share (warning should appear) - await form.fillForm({ - Purpose: 'Time Machine Share', - }); - // eslint-disable-next-line @typescript-eslint/dot-notation - spectator.component['updateExtensionsWarning'](); - spectator.detectChanges(); - - // Verify warning is shown and button is disabled - // eslint-disable-next-line @typescript-eslint/dot-notation - expect(spectator.component['showExtensionsWarning']()).toBe(true); - expect(await saveButton.isDisabled()).toBe(true); - - // Enable extensions - // eslint-disable-next-line @typescript-eslint/dot-notation - spectator.component['extensionsEnabled'](); - spectator.detectChanges(); - - // Verify button is enabled - // eslint-disable-next-line @typescript-eslint/dot-notation - expect(spectator.component['showExtensionsWarning']()).toBe(false); - expect(await saveButton.isDisabled()).toBe(false); - - // Switch to FCP Share (another purpose requiring extensions with extensions disabled) - // eslint-disable-next-line @typescript-eslint/dot-notation - spectator.component['smbConfig'].set({ aapl_extensions: false } as SmbConfig); - await form.fillForm({ - Purpose: 'Final Cut Pro Storage Share', - }); - // eslint-disable-next-line @typescript-eslint/dot-notation - spectator.component['updateExtensionsWarning'](); - spectator.detectChanges(); - - // Verify warning appears again for FCP share - // eslint-disable-next-line @typescript-eslint/dot-notation - expect(spectator.component['showExtensionsWarning']()).toBe(true); - saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save' })); - expect(await saveButton.isDisabled()).toBe(true); - - // Switch back to Default Share (warning should disappear) - await form.fillForm({ - Purpose: 'Default Share', - }); - // eslint-disable-next-line @typescript-eslint/dot-notation - spectator.component['updateExtensionsWarning'](); - spectator.detectChanges(); - - // Verify warning is gone - // eslint-disable-next-line @typescript-eslint/dot-notation - expect(spectator.component['showExtensionsWarning']()).toBe(false); - expect(await saveButton.isDisabled()).toBe(false); - }); - }); - describe('grace_period validation', () => { beforeEach(async () => { await setupTest();