Skip to content

Commit 2bfcf12

Browse files
bugclerkwilliam-gr
andcommitted
NAS-141590: Hide deduplication on zvol form when enterprise without dedup license (#13751)
Co-authored-by: William Grzybowski <56250+william-gr@users.noreply.github.com> (cherry picked from commit 5f7dd0d)
1 parent 1d3f872 commit 2bfcf12

7 files changed

Lines changed: 171 additions & 67 deletions

File tree

src/app/pages/datasets/components/dataset-form/sections/other-options-section/other-options-section.component.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
[tooltip]="helptext.atimeTooltip | translate"
3030
></ix-select>
3131

32-
@if (hasDeduplication) {
32+
@if (hasDeduplication()) {
3333
<ix-select
3434
formControlName="deduplication"
3535
[label]="'ZFS Deduplication' | translate"

src/app/pages/datasets/components/dataset-form/sections/other-options-section/other-options-section.component.spec.ts

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ import { IxSelectHarness } from 'app/modules/forms/ix-forms/components/ix-select
3030
import {
3131
OtherOptionsSectionComponent,
3232
} from 'app/pages/datasets/components/dataset-form/sections/other-options-section/other-options-section.component';
33-
import { SystemGeneralService } from 'app/services/system-general.service';
34-
import { selectSystemInfo } from 'app/store/system-info/system-info.selectors';
33+
import { selectIsEnterprise, selectSystemInfo } from 'app/store/system-info/system-info.selectors';
3534

3635
describe('OtherOptionsSectionComponent', () => {
3736
let spectator: Spectator<OtherOptionsSectionComponent>;
@@ -199,19 +198,18 @@ describe('OtherOptionsSectionComponent', () => {
199198
mockCall('pool.dataset.recordsize_choices', ['1K', '64K']),
200199
mockCall('pool.dataset.recommended_zvol_blocksize', '256K' as DatasetRecordSize),
201200
]),
202-
mockProvider(SystemGeneralService, {
203-
getProductType: jest.fn(() => ProductType.CommunityEdition),
204-
}),
205201
mockProvider(DialogService, {
206202
confirm: jest.fn(() => of(true)),
207203
}),
208204
provideMockStore({
209-
selectors: [
210-
{
211-
selector: selectSystemInfo,
212-
value: {} as SystemInfo,
205+
initialState: {
206+
systemInfo: {
207+
productType: ProductType.CommunityEdition,
208+
systemInfo: {
209+
license: { features: [] },
210+
} as SystemInfo,
213211
},
214-
],
212+
},
215213
}),
216214
],
217215
});
@@ -414,25 +412,31 @@ describe('OtherOptionsSectionComponent', () => {
414412
});
415413

416414
it('does not show deduplication field on Enterprise systems that do not have a dedup license', async () => {
417-
const systemGeneralService = spectator.inject(SystemGeneralService);
418-
jest.spyOn(systemGeneralService, 'getProductType').mockReturnValue(ProductType.Enterprise);
419415
const store$ = spectator.inject(MockStore);
416+
store$.overrideSelector(selectIsEnterprise, true);
420417
store$.overrideSelector(selectSystemInfo, {
421418
license: {
422419
features: [],
423420
},
424-
});
425-
spectator.component.ngOnInit();
421+
} as SystemInfo);
422+
store$.refreshState();
423+
spectator.detectChanges();
424+
426425
expect(await form.getLabels()).not.toContain('ZFS Deduplication');
427426

428427
store$.overrideSelector(selectSystemInfo, {
429428
license: {
430429
features: [LicenseFeature.Dedup],
431430
},
432-
});
433-
spectator.component.ngOnInit();
431+
} as SystemInfo);
432+
store$.refreshState();
433+
spectator.detectChanges();
434434

435435
expect(await form.getLabels()).toContain('ZFS Deduplication');
436+
437+
// overrideSelector mutates the module-singleton selectors, so reset them
438+
// to avoid leaking the enterprise/license state into other tests.
439+
store$.resetSelectors();
436440
});
437441
});
438442

src/app/pages/datasets/components/dataset-form/sections/other-options-section/other-options-section.component.ts

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { ChangeDetectionStrategy, ChangeDetectorRef, Component, input, OnChanges, OnInit, output, inject } from '@angular/core';
2+
import { toSignal } from '@angular/core/rxjs-interop';
23
import { NonNullableFormBuilder, ReactiveFormsModule } from '@angular/forms';
34
import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
4-
import { Store } from '@ngrx/store';
55
import { TranslateModule, TranslateService } from '@ngx-translate/core';
66
import {
77
combineLatest, Observable, of, take,
@@ -23,9 +23,7 @@ import {
2323
datasetSyncLabels,
2424
} from 'app/enums/dataset.enum';
2525
import { DeduplicationSetting, deduplicationSettingLabels } from 'app/enums/deduplication-setting.enum';
26-
import { LicenseFeature } from 'app/enums/license-feature.enum';
2726
import { OnOff, onOffLabels } from 'app/enums/on-off.enum';
28-
import { ProductType } from 'app/enums/product-type.enum';
2927
import { inherit, WithInherit } from 'app/enums/with-inherit.enum';
3028
import { ZfsPropertySource } from 'app/enums/zfs-property-source.enum';
3129
import { buildNormalizedFileSize } from 'app/helpers/file-size.utils';
@@ -48,9 +46,7 @@ import {
4846
} from 'app/pages/datasets/components/dataset-form/utils/special-small-block-size-options.constant';
4947
import { getFieldValue } from 'app/pages/datasets/components/dataset-form/utils/zfs-property.utils';
5048
import { getUserProperty } from 'app/pages/datasets/utils/dataset.utils';
51-
import { SystemGeneralService } from 'app/services/system-general.service';
52-
import { AppState } from 'app/store';
53-
import { waitForSystemInfo } from 'app/store/system-info/system-info.selectors';
49+
import { LicenseService } from 'app/services/license.service';
5450

5551
@UntilDestroy()
5652
@Component({
@@ -70,9 +66,8 @@ import { waitForSystemInfo } from 'app/store/system-info/system-info.selectors';
7066
export class OtherOptionsSectionComponent implements OnInit, OnChanges {
7167
private formBuilder = inject(NonNullableFormBuilder);
7268
private translate = inject(TranslateService);
73-
private store$ = inject<Store<AppState>>(Store);
69+
private licenseService = inject(LicenseService);
7470
private cdr = inject(ChangeDetectorRef);
75-
private systemGeneralService = inject(SystemGeneralService);
7671
private dialogService = inject(DialogService);
7772
private formatter = inject(IxFormatterService);
7873
private api = inject(ApiService);
@@ -86,7 +81,7 @@ export class OtherOptionsSectionComponent implements OnInit, OnChanges {
8681
readonly advancedModeChange = output();
8782
readonly formValidityChange = output<boolean>();
8883

89-
hasDeduplication = false;
84+
protected readonly hasDeduplication = toSignal(this.licenseService.hasDedup$, { initialValue: false });
9085
hasRecordsizeWarning = false;
9186
wasDedupChecksumWarningShown = false;
9287
minimumRecommendedRecordsize = '128K' as DatasetRecordSize;
@@ -183,8 +178,6 @@ export class OtherOptionsSectionComponent implements OnInit, OnChanges {
183178
}
184179

185180
ngOnInit(): void {
186-
this.checkIfDedupIsSupported();
187-
188181
this.form.controls.acltype.valueChanges.pipe(untilDestroyed(this)).subscribe(() => {
189182
this.updateAclMode();
190183
});
@@ -214,27 +207,6 @@ export class OtherOptionsSectionComponent implements OnInit, OnChanges {
214207
return payload as Partial<DatasetCreate> | Partial<DatasetUpdate>;
215208
}
216209

217-
private checkIfDedupIsSupported(): void {
218-
this.hasDeduplication = false;
219-
this.cdr.markForCheck();
220-
221-
if (this.systemGeneralService.getProductType() !== ProductType.Enterprise) {
222-
this.hasDeduplication = true;
223-
this.cdr.markForCheck();
224-
return;
225-
}
226-
227-
this.store$.pipe(waitForSystemInfo, untilDestroyed(this)).subscribe((systemInfo) => {
228-
// eslint-disable-next-line @typescript-eslint/prefer-optional-chain
229-
if (!systemInfo.license || !systemInfo.license.features.includes(LicenseFeature.Dedup)) {
230-
return;
231-
}
232-
233-
this.hasDeduplication = true;
234-
this.cdr.markForCheck();
235-
});
236-
}
237-
238210
private setFormValues(): void {
239211
const existing = this.existing();
240212
if (!existing) {

src/app/pages/datasets/components/zvol-form/zvol-form.component.html

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -87,22 +87,24 @@
8787
</div>
8888
</ix-editable>
8989
</ix-details-item>
90-
<ix-details-item
91-
[label]="helptext.deduplicationLabel | translate"
92-
[tooltip]="helptext.deduplicationTooltip | translate"
93-
>
94-
<ix-editable>
95-
<div view>{{ getOptionLabel(deduplicationOptions, form.controls.deduplication.value) }}</div>
96-
<div edit>
97-
<ix-select
98-
formControlName="deduplication"
99-
[options]="deduplicationOptions$"
100-
[attr.aria-label]="helptext.deduplicationLabel | translate"
101-
[required]="true"
102-
></ix-select>
103-
</div>
104-
</ix-editable>
105-
</ix-details-item>
90+
@if (hasDeduplication) {
91+
<ix-details-item
92+
[label]="helptext.deduplicationLabel | translate"
93+
[tooltip]="helptext.deduplicationTooltip | translate"
94+
>
95+
<ix-editable>
96+
<div view>{{ getOptionLabel(deduplicationOptions, form.controls.deduplication.value) }}</div>
97+
<div edit>
98+
<ix-select
99+
formControlName="deduplication"
100+
[options]="deduplicationOptions$"
101+
[attr.aria-label]="helptext.deduplicationLabel | translate"
102+
[required]="true"
103+
></ix-select>
104+
</div>
105+
</ix-editable>
106+
</ix-details-item>
107+
}
106108
<ix-details-item
107109
[label]="helptext.readonlyLabel | translate"
108110
[tooltip]="helptext.readonlyTooltip | translate"

src/app/pages/datasets/components/zvol-form/zvol-form.component.spec.ts

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,31 @@
11
import { HarnessLoader } from '@angular/cdk/testing';
22
import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed';
3-
import { ReactiveFormsModule } from '@angular/forms';
3+
import { ReactiveFormsModule, Validators } from '@angular/forms';
44
import { MatButtonHarness } from '@angular/material/button/testing';
55
import { createComponentFactory, mockProvider, Spectator } from '@ngneat/spectator/jest';
6+
import { MockStore, provideMockStore } from '@ngrx/store/testing';
67
import { mockApi, mockCall } from 'app/core/testing/utils/mock-api.utils';
78
import { mockAuth } from 'app/core/testing/utils/mock-auth.utils';
89
import {
910
DatasetRecordSize, DatasetSnapdev, DatasetSync, DatasetType,
1011
} from 'app/enums/dataset.enum';
1112
import { DeduplicationSetting } from 'app/enums/deduplication-setting.enum';
1213
import { EncryptionKeyFormat } from 'app/enums/encryption-key-format.enum';
14+
import { LicenseFeature } from 'app/enums/license-feature.enum';
1315
import { OnOff } from 'app/enums/on-off.enum';
16+
import { ProductType } from 'app/enums/product-type.enum';
1417
import { inherit } from 'app/enums/with-inherit.enum';
1518
import { ZfsPropertySource } from 'app/enums/zfs-property-source.enum';
1619
import { Dataset } from 'app/interfaces/dataset.interface';
1720
import { QueryFilter } from 'app/interfaces/query-api.interface';
21+
import { SystemInfo } from 'app/interfaces/system-info.interface';
1822
import { DetailsTableHarness } from 'app/modules/details-table/details-table.harness';
1923
import { DialogService } from 'app/modules/dialog/dialog.service';
2024
import { IxFormHarness } from 'app/modules/forms/ix-forms/testing/ix-form.harness';
2125
import { SlideInRef } from 'app/modules/slide-ins/slide-in-ref';
2226
import { ApiService } from 'app/modules/websocket/api.service';
2327
import { ZvolFormComponent } from 'app/pages/datasets/components/zvol-form/zvol-form.component';
28+
import { selectIsEnterprise, selectSystemInfo } from 'app/store/system-info/system-info.selectors';
2429

2530
describe('ZvolFormComponent', () => {
2631
let loader: HarnessLoader;
@@ -135,6 +140,16 @@ describe('ZvolFormComponent', () => {
135140
mockProvider(DialogService),
136141
mockProvider(SlideInRef, slideInRef),
137142
mockAuth(),
143+
provideMockStore({
144+
initialState: {
145+
systemInfo: {
146+
productType: ProductType.CommunityEdition,
147+
systemInfo: {
148+
license: { features: [] },
149+
} as SystemInfo,
150+
},
151+
},
152+
}),
138153
],
139154
});
140155

@@ -207,6 +222,69 @@ describe('ZvolFormComponent', () => {
207222
});
208223
});
209224

225+
describe('deduplication visibility', () => {
226+
// overrideSelector mutates the module-singleton selectors, so reset them
227+
// afterwards to avoid leaking the enterprise/license state into other tests.
228+
afterEach(() => {
229+
spectator.inject(MockStore).resetSelectors();
230+
});
231+
232+
async function setupVisibilityTest(isEnterprise: boolean, hasDedupLicense: boolean): Promise<void> {
233+
spectator = createComponent({
234+
providers: [
235+
mockProvider(SlideInRef, {
236+
...slideInRef,
237+
getData: jest.fn(() => ({ isNew: true, parentOrZvolId: 'parentId' })),
238+
}),
239+
],
240+
});
241+
const store$ = spectator.inject(MockStore);
242+
store$.overrideSelector(selectIsEnterprise, isEnterprise);
243+
store$.overrideSelector(selectSystemInfo, {
244+
license: {
245+
features: hasDedupLicense ? [LicenseFeature.Dedup] : [],
246+
},
247+
} as SystemInfo);
248+
store$.refreshState();
249+
loader = TestbedHarnessEnvironment.loader(spectator.fixture);
250+
await spectator.fixture.whenStable();
251+
mainDetails = await loader.getHarness(DetailsTableHarness);
252+
}
253+
254+
it('shows deduplication when not enterprise', async () => {
255+
await setupVisibilityTest(false, false);
256+
expect(Object.keys(await mainDetails.getValues())).toContain('ZFS Deduplication');
257+
expect(spectator.component.form.controls.deduplication.hasValidator(Validators.required)).toBe(true);
258+
});
259+
260+
it('shows deduplication when enterprise with dedup license', async () => {
261+
await setupVisibilityTest(true, true);
262+
expect(Object.keys(await mainDetails.getValues())).toContain('ZFS Deduplication');
263+
expect(spectator.component.form.controls.deduplication.hasValidator(Validators.required)).toBe(true);
264+
});
265+
266+
it('hides deduplication when enterprise without dedup license', async () => {
267+
await setupVisibilityTest(true, false);
268+
expect(Object.keys(await mainDetails.getValues())).not.toContain('ZFS Deduplication');
269+
// Hidden control drops its required validator so it never blocks submission.
270+
expect(spectator.component.form.controls.deduplication.hasValidator(Validators.required)).toBe(false);
271+
});
272+
273+
it('omits deduplication from the create payload when hidden', async () => {
274+
await setupVisibilityTest(true, false);
275+
form = await loader.getHarness(IxFormHarness);
276+
await form.fillForm({ Name: 'new zvol', Size: '1 GiB' });
277+
278+
const saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save' }));
279+
await saveButton.click();
280+
281+
expect(spectator.inject(ApiService).call).toHaveBeenLastCalledWith(
282+
'pool.dataset.create',
283+
[expect.not.objectContaining({ deduplication: expect.anything() })],
284+
);
285+
});
286+
});
287+
210288
describe('adds a new zvol with encrypted parent', () => {
211289
let encryptedLoader: HarnessLoader;
212290
let encryptedSpectator: Spectator<ZvolFormComponent>;
@@ -303,6 +381,7 @@ describe('ZvolFormComponent', () => {
303381
});
304382

305383
loader = TestbedHarnessEnvironment.loader(spectator.fixture);
384+
await spectator.fixture.whenStable();
306385
form = await loader.getHarness(IxFormHarness);
307386
mainDetails = await loader.getHarness(DetailsTableHarness);
308387
});

0 commit comments

Comments
 (0)