From 8d11f674cffef6fca44b1ec9d42e7dba66bb43cd Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 27 Sep 2026 15:53:41 +0200 Subject: [PATCH] fix(settings): keep the parental lock switch on the saved state and roll back to the recovered value The Settings switch snaps back to the saved state when clicked and follows it once the PIN action succeeds, so a cancelled or refused PIN no longer leaves it showing the opposite state. A failed switch write is undone to the value read after the settings retry instead of the hard-coded inverse. Co-Authored-By: Claude Opus 5.5 --- .../settings-parental-section.component.html | 2 +- ...ettings-parental-section.component.spec.ts | 46 +++++++++++++++++ .../settings-parental-section.component.ts | 17 ++++++- docs/architecture/parental-lock.md | 6 ++- .../parental-lock-settings-writer.spec.ts | 51 +++++++++++++++++++ .../parental-lock-settings-writer.ts | 12 ++++- 6 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 apps/web/src/app/settings/settings-parental-section.component.spec.ts create mode 100644 libs/services/src/lib/parental-lock/parental-lock-settings-writer.spec.ts diff --git a/apps/web/src/app/settings/settings-parental-section.component.html b/apps/web/src/app/settings/settings-parental-section.component.html index 6f7118038..00264f3ce 100644 --- a/apps/web/src/app/settings/settings-parental-section.component.html +++ b/apps/web/src/app/settings/settings-parental-section.component.html @@ -19,7 +19,7 @@ data-test-id="parental-lock-enabled" [checked]="enabled()" [disabled]="busy()" - (change)="toggleEnabled.emit($event.checked)" + (change)="onToggleEnabled($event)" [attr.aria-label]="'SETTINGS.PARENTAL_LOCK.ENABLE' | translate" > diff --git a/apps/web/src/app/settings/settings-parental-section.component.spec.ts b/apps/web/src/app/settings/settings-parental-section.component.spec.ts new file mode 100644 index 000000000..6bb0d987c --- /dev/null +++ b/apps/web/src/app/settings/settings-parental-section.component.spec.ts @@ -0,0 +1,46 @@ +import { TestBed } from '@angular/core/testing'; +import { MatSlideToggle } from '@angular/material/slide-toggle'; +import { By } from '@angular/platform-browser'; +import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import { TranslateModule } from '@ngx-translate/core'; +import { SettingsParentalLockSectionComponent } from './settings-parental-section.component'; + +describe('SettingsParentalLockSectionComponent', () => { + it('keeps the enable switch on the saved state until the PIN action succeeds', async () => { + await TestBed.configureTestingModule({ + imports: [ + SettingsParentalLockSectionComponent, + NoopAnimationsModule, + TranslateModule.forRoot(), + ], + }).compileComponents(); + const fixture = TestBed.createComponent( + SettingsParentalLockSectionComponent + ); + fixture.componentRef.setInput('enabled', false); + fixture.componentRef.setInput('unlocked', false); + fixture.componentRef.setInput('hasPin', false); + fixture.componentRef.setInput('relockMinutes', 15); + fixture.componentRef.setInput('relockOptions', [0, 15]); + fixture.detectChanges(); + const requested = jest.fn(); + fixture.componentInstance.toggleEnabled.subscribe(requested); + const toggle = fixture.debugElement.query(By.directive(MatSlideToggle)) + .componentInstance as MatSlideToggle; + const button = fixture.nativeElement.querySelector( + '[data-test-id="parental-lock-enabled"] button' + ) as HTMLButtonElement; + + // The set-PIN prompt is then cancelled: `enabled` never changes. + button.click(); + fixture.detectChanges(); + + expect(requested).toHaveBeenCalledWith(true); + expect(toggle.checked).toBe(false); + + // The PIN was set: the switch follows the saved state. + fixture.componentRef.setInput('enabled', true); + fixture.detectChanges(); + expect(toggle.checked).toBe(true); + }); +}); diff --git a/apps/web/src/app/settings/settings-parental-section.component.ts b/apps/web/src/app/settings/settings-parental-section.component.ts index 95a33ec71..7e874c010 100644 --- a/apps/web/src/app/settings/settings-parental-section.component.ts +++ b/apps/web/src/app/settings/settings-parental-section.component.ts @@ -3,7 +3,10 @@ import { MatButtonModule } from '@angular/material/button'; import { MatFormFieldModule } from '@angular/material/form-field'; import { MatIconModule } from '@angular/material/icon'; import { MatSelectModule } from '@angular/material/select'; -import { MatSlideToggleModule } from '@angular/material/slide-toggle'; +import { + MatSlideToggleChange, + MatSlideToggleModule, +} from '@angular/material/slide-toggle'; import { TranslateModule } from '@ngx-translate/core'; import { ParentalLockRelockMinutes } from '@iptvnator/shared/interfaces'; @@ -36,6 +39,18 @@ export class SettingsParentalLockSectionComponent { readonly lockNow = output(); readonly unlock = output(); + /** + * The switch only REQUESTS the change: enabling and disabling both go + * through the PIN, which may be cancelled. It is put back to the saved + * state at once and follows `enabled()` when the action succeeds; + * otherwise the slide toggle would keep showing the state it flipped to + * (`[checked]` is not re-applied while the bound value is unchanged). + */ + onToggleEnabled(event: MatSlideToggleChange): void { + event.source.checked = this.enabled(); + this.toggleEnabled.emit(event.checked); + } + relockLabelKey(minutes: ParentalLockRelockMinutes): string { return minutes === 0 ? 'SETTINGS.PARENTAL_LOCK.RELOCK_NEVER' diff --git a/docs/architecture/parental-lock.md b/docs/architecture/parental-lock.md index 0dcb736d3..d1fa5e231 100644 --- a/docs/architecture/parental-lock.md +++ b/docs/architecture/parental-lock.md @@ -173,7 +173,11 @@ settings switch BEFORE the PIN is stored, since `enabled` follows `hasPin` while the switch is unknown); the Electron mirror write is awaited next, and a mirror that cannot be written undoes the settings write the same way — the toggle never shows a state the next launch will not have, -on either side. +on either side. The undo restores the value read AFTER the settings retry +(not the hard-coded inverse, which a recovered read may already hold). The +Settings switch itself only requests the change: it snaps back to the +saved state at once and follows `enabled()` when the PIN action succeeds, +so a cancelled or refused PIN leaves it showing the real state. ### In-memory catalogs diff --git a/libs/services/src/lib/parental-lock/parental-lock-settings-writer.spec.ts b/libs/services/src/lib/parental-lock/parental-lock-settings-writer.spec.ts new file mode 100644 index 000000000..0ce31026b --- /dev/null +++ b/libs/services/src/lib/parental-lock/parental-lock-settings-writer.spec.ts @@ -0,0 +1,51 @@ +import { persistParentalLockEnabled } from './parental-lock-settings-writer'; + +describe('persistParentalLockEnabled', () => { + it('rolls a failed write back to the value recovered by the settings retry', async () => { + jest.spyOn(console, 'error').mockImplementation(() => undefined); + let failure: 'load' | 'save' | null = 'load'; + let stored: boolean | undefined = undefined; + const settings = { + storageFailure: () => failure, + parentalLockEnabled: () => stored, + // The retried read recovers: the switch was already off. + loadSettings: jest.fn(async () => { + failure = null; + stored = false; + }), + updateSettings: jest + .fn() + .mockRejectedValueOnce(new Error('disk full')) + .mockResolvedValue(undefined), + }; + + await expect(persistParentalLockEnabled(settings, false)).resolves.toBe( + false + ); + + // Undone to the recovered `false`, not the hard-coded inverse. + expect(settings.updateSettings).toHaveBeenLastCalledWith({ + parentalLockEnabled: false, + }); + }); + + it('undoes to the inverse when the switch was never persisted', async () => { + jest.spyOn(console, 'error').mockImplementation(() => undefined); + const settings = { + storageFailure: () => null, + loadSettings: jest.fn(), + updateSettings: jest + .fn() + .mockRejectedValueOnce(new Error('disk full')) + .mockResolvedValue(undefined), + }; + + await expect(persistParentalLockEnabled(settings, true)).resolves.toBe( + false + ); + + expect(settings.updateSettings).toHaveBeenLastCalledWith({ + parentalLockEnabled: false, + }); + }); +}); diff --git a/libs/services/src/lib/parental-lock/parental-lock-settings-writer.ts b/libs/services/src/lib/parental-lock/parental-lock-settings-writer.ts index 3feddcbf6..47db2cde1 100644 --- a/libs/services/src/lib/parental-lock/parental-lock-settings-writer.ts +++ b/libs/services/src/lib/parental-lock/parental-lock-settings-writer.ts @@ -5,7 +5,10 @@ import { mirrorParentalLockEnabledSetting } from './parental-lock-bridge'; type SettingsWriter = Pick< InstanceType, 'updateSettings' | 'loadSettings' -> & { storageFailure?: () => 'load' | 'save' | null }; +> & { + storageFailure?: () => 'load' | 'save' | null; + parentalLockEnabled?: () => boolean | undefined; +}; /** * `updateSettings` writes the WHOLE settings object. After a failed startup @@ -39,9 +42,14 @@ export async function persistParentalLockEnabled( if (!(await ensureParentalLockSettingsReadable(settings))) { return false; } + // Read AFTER the retry, as for the relock timeout: a recovered read may + // already hold `enabled` (e.g. disable() entered while the unknown + // switch followed the stored PIN), and undoing to the inverse would + // then write the opposite of what was persisted. + const previous = settings.parentalLockEnabled?.() ?? !enabled; const undo = () => settings - .updateSettings({ parentalLockEnabled: !enabled }) + .updateSettings({ parentalLockEnabled: previous }) .catch(() => undefined); try { await settings.updateSettings({ parentalLockEnabled: enabled });