From bb46a707ec753a595c7d91790f6dd3b7c7a0bdba Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 9 Aug 2026 13:46:02 +0200 Subject: [PATCH] fix(settings): route every update-install entry point through one coordinator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The global app-update notification panel called installAppUpdate() directly, bypassing the unload-guard suspension entirely — with a dirty settings form its install was cancelled at the DOM layer and treated as a reload. The whole install choreography (suspend registered unload guards, invoke the updater, restore on non-downloaded reply / rejected IPC / later error push, tolerant of push-vs-reply ordering) now lives in the root AppUpdateInstallService; the settings facade and the panel both delegate to it, and the settings unload guard registers its suspend/resume hooks with it while settings is mounted. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 2 +- ...app-update-notification-panel.component.ts | 14 +- .../app-update-install.service.spec.ts | 179 ++++++++++++++++++ .../services/app-update-install.service.ts | 91 +++++++++ .../settings-app-update.facade.spec.ts | 142 +------------- .../settings/settings-app-update.facade.ts | 58 +----- .../settings-unload-guard.service.spec.ts | 38 ++++ .../settings/settings-unload-guard.service.ts | 15 ++ 8 files changed, 349 insertions(+), 190 deletions(-) create mode 100644 apps/web/src/app/services/app-update-install.service.spec.ts create mode 100644 apps/web/src/app/services/app-update-install.service.ts diff --git a/CLAUDE.md b/CLAUDE.md index 57ef45769..a8a1e5cb6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -514,7 +514,7 @@ See `docs/architecture/m3u-playlist-module.md` for complete documentation. `/workspace/xtreams/:id/downloads/:downloadId` and `/workspace/stalker/:id/downloads/:downloadId`. Focused download details hide the workspace context panel. -- Settings: `/workspace/settings/:section` — one page per section (`general`, `playback`, `epg`, `dashboard`, `remote-control`, `tmdb`, `backup`, `reset`, `about`); `/workspace/settings` redirects to `general`, unknown or capability-gated sections redirect there too, and `/settings` redirects into the workspace. The shared form lives on the parent `SettingsComponent`, so edits survive section switches; a floating unsaved-changes bar (Save/Discard) replaces the old always-visible footer Save button. Leaving the settings AREA with a dirty form triggers `settingsUnsavedChangesGuard` (canDeactivate) and a save/discard/stay dialog — section switches deliberately bypass it, and a failed save cancels the navigation. Non-router exits are covered too: `SettingsUnloadGuardService` (provided by `SettingsComponent`) arms a `beforeunload` handler while the form is dirty (native leave prompt in the PWA) and arms an Electron main-process close guard (`window-close-guard.service.ts`) for the whole settings mount — mount-long on purpose, since arming on the first edit would race the close it protects against. The guard intercepts window close/app quit before `beforeunload` fires and completes the original intent only after the renderer confirms through the same dialog (a pristine form auto-confirms); Electron reloads are cancelled and re-triggered the same way, a failed save always keeps the window open, and installing an app update suspends the whole guard so the updater's quit passes unchallenged +- Settings: `/workspace/settings/:section` — one page per section (`general`, `playback`, `epg`, `dashboard`, `remote-control`, `tmdb`, `backup`, `reset`, `about`); `/workspace/settings` redirects to `general`, unknown or capability-gated sections redirect there too, and `/settings` redirects into the workspace. The shared form lives on the parent `SettingsComponent`, so edits survive section switches; a floating unsaved-changes bar (Save/Discard) replaces the old always-visible footer Save button. Leaving the settings AREA with a dirty form triggers `settingsUnsavedChangesGuard` (canDeactivate) and a save/discard/stay dialog — section switches deliberately bypass it, and a failed save cancels the navigation. Non-router exits are covered too: `SettingsUnloadGuardService` (provided by `SettingsComponent`) arms a `beforeunload` handler while the form is dirty (native leave prompt in the PWA) and arms an Electron main-process close guard (`window-close-guard.service.ts`) for the whole settings mount — mount-long on purpose, since arming on the first edit would race the close it protects against. The guard intercepts window close/app quit before `beforeunload` fires and completes the original intent only after the renderer confirms through the same dialog (a pristine form auto-confirms); Electron reloads are cancelled and re-triggered the same way, a failed save always keeps the window open, and installing an app update suspends the whole guard so the updater's quit passes unchallenged — every install entry point (settings About section and the global update notification panel) must go through the root `AppUpdateInstallService`, which owns that suspend/restore choreography **Service Architecture** (Factory Pattern): diff --git a/apps/web/src/app/app-update-notification-panel.component.ts b/apps/web/src/app/app-update-notification-panel.component.ts index a597e6d94..6045d01dc 100644 --- a/apps/web/src/app/app-update-notification-panel.component.ts +++ b/apps/web/src/app/app-update-notification-panel.component.ts @@ -14,6 +14,7 @@ import { ELECTRON_BRIDGE_APP_UPDATE_STATUSES, ElectronBridgeAppUpdateStatus, } from '@iptvnator/shared/interfaces'; +import { AppUpdateInstallService } from './services/app-update-install.service'; import { AppUpdateReleaseNotesDialogComponent } from './settings/app-update-release-notes-dialog.component'; @Component({ @@ -176,6 +177,7 @@ import { AppUpdateReleaseNotesDialogComponent } from './settings/app-update-rele }) export class AppUpdateNotificationPanelComponent implements OnInit, OnDestroy { private readonly dialog = inject(MatDialog); + private readonly installService = inject(AppUpdateInstallService); private unsubscribeStatus: (() => void) | null = null; readonly appUpdateStatuses = ELECTRON_BRIDGE_APP_UPDATE_STATUSES; @@ -268,11 +270,15 @@ export class AppUpdateNotificationPanelComponent implements OnInit, OnDestroy { } async installUpdate(): Promise { - if (!window.electron?.installAppUpdate) { - return; - } + // Never window.electron.installAppUpdate() directly: the install + // quits the app, and AppUpdateInstallService stands the settings + // unload guard down first — this panel is global, so the settings + // form can be dirty right now. + const status = await this.installService.installAppUpdate(); - this.status.set(await window.electron.installAppUpdate()); + if (status) { + this.status.set(status); + } } dismiss(): void { diff --git a/apps/web/src/app/services/app-update-install.service.spec.ts b/apps/web/src/app/services/app-update-install.service.spec.ts new file mode 100644 index 000000000..44ba7da86 --- /dev/null +++ b/apps/web/src/app/services/app-update-install.service.spec.ts @@ -0,0 +1,179 @@ +import { TestBed } from '@angular/core/testing'; +import { + ELECTRON_BRIDGE_APP_UPDATE_STATUSES, + ElectronBridgeAppUpdateStatus, +} from '@iptvnator/shared/interfaces'; +import { + AppQuitUnloadGuardHooks, + AppUpdateInstallService, +} from './app-update-install.service'; + +const BASE_STATUS: ElectronBridgeAppUpdateStatus = { + currentVersion: '0.23.0', + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Idle, + supportedSelfUpdate: true, +}; + +/** + * The install choreography shared by every entry point (settings About + * section, global update notification panel): suspend the unsaved-settings + * unload guard for the updater-driven quit, and restore it on every path + * that proves the quit is not happening. + */ +describe('AppUpdateInstallService', () => { + const originalElectron = window.electron; + + let pushStatus: (status: ElectronBridgeAppUpdateStatus) => void; + let electronStub: { + installAppUpdate: jest.Mock; + onAppUpdateStatusChange: jest.Mock; + }; + let guard: AppQuitUnloadGuardHooks & { + suspendForAppQuit: jest.Mock; + resumeAfterAbortedAppQuit: jest.Mock; + }; + + beforeEach(() => { + pushStatus = () => undefined; + electronStub = { + installAppUpdate: jest.fn().mockResolvedValue(BASE_STATUS), + onAppUpdateStatusChange: jest.fn( + (callback: (status: ElectronBridgeAppUpdateStatus) => void) => { + pushStatus = callback; + return jest.fn(); + } + ), + }; + window.electron = electronStub as unknown as typeof window.electron; + guard = { + resumeAfterAbortedAppQuit: jest.fn(), + suspendForAppQuit: jest.fn(), + }; + + TestBed.configureTestingModule({}); + }); + + afterEach(() => { + window.electron = originalElectron; + }); + + function createService(): AppUpdateInstallService { + return TestBed.inject(AppUpdateInstallService); + } + + it('returns null and touches no guard without the desktop bridge', async () => { + const service = createService(); + service.registerUnloadGuard(guard); + window.electron = undefined; + + expect(await service.installAppUpdate()).toBeNull(); + expect(guard.suspendForAppQuit).not.toHaveBeenCalled(); + }); + + it('keeps the guard down while a real install quits the app', async () => { + electronStub.installAppUpdate.mockResolvedValue({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded, + }); + const service = createService(); + service.registerUnloadGuard(guard); + + await service.installAppUpdate(); + + expect(guard.suspendForAppQuit).toHaveBeenCalledTimes(1); + expect(guard.resumeAfterAbortedAppQuit).not.toHaveBeenCalled(); + }); + + it('restores the guard when nothing was installed', async () => { + const service = createService(); + service.registerUnloadGuard(guard); + + // BASE_STATUS reply stays Idle: nothing installable, no quit. + await service.installAppUpdate(); + + expect(guard.suspendForAppQuit).toHaveBeenCalledTimes(1); + expect(guard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(1); + }); + + it('restores the guard when the install IPC rejects', async () => { + const failure = new Error('ipc failed'); + electronStub.installAppUpdate.mockRejectedValue(failure); + const service = createService(); + service.registerUnloadGuard(guard); + + await expect(service.installAppUpdate()).rejects.toThrow(failure); + + expect(guard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(1); + }); + + it('restores the guard when an error push follows a quitting install', async () => { + // electron-updater reports install failures as a later 'error' + // status push, not as a rejection of the install IPC. + electronStub.installAppUpdate.mockResolvedValue({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded, + }); + const service = createService(); + service.registerUnloadGuard(guard); + await service.installAppUpdate(); + + pushStatus({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error, + }); + + expect(guard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(1); + + // One-shot: later unrelated pushes must not resume again. + pushStatus(BASE_STATUS); + expect(guard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(1); + }); + + it('survives an error push that beats the install reply', async () => { + // IPC ordering between the invoke reply and status pushes is not + // guaranteed; a stale 'downloaded' reply must not re-suspend the + // protection the failure already restored. + let resolveInstall: ( + status: ElectronBridgeAppUpdateStatus + ) => void = () => undefined; + electronStub.installAppUpdate.mockImplementation( + () => + new Promise((resolve) => { + resolveInstall = resolve; + }) + ); + const service = createService(); + service.registerUnloadGuard(guard); + + const install = service.installAppUpdate(); + pushStatus({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error, + }); + expect(guard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(1); + + resolveInstall({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded, + }); + await install; + + expect(guard.suspendForAppQuit).toHaveBeenCalledTimes(1); + pushStatus({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error, + }); + expect(guard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(1); + }); + + it('stops reaching a guard after it unregisters', async () => { + const service = createService(); + const unregister = service.registerUnloadGuard(guard); + + unregister(); + await service.installAppUpdate(); + + expect(guard.suspendForAppQuit).not.toHaveBeenCalled(); + expect(guard.resumeAfterAbortedAppQuit).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/app/services/app-update-install.service.ts b/apps/web/src/app/services/app-update-install.service.ts new file mode 100644 index 000000000..1916f11c4 --- /dev/null +++ b/apps/web/src/app/services/app-update-install.service.ts @@ -0,0 +1,91 @@ +import { Injectable } from '@angular/core'; +import { + ELECTRON_BRIDGE_APP_UPDATE_STATUSES, + ElectronBridgeAppUpdateStatus, +} from '@iptvnator/shared/interfaces'; + +/** What an unload guard must offer to survive an updater-driven quit. */ +export interface AppQuitUnloadGuardHooks { + suspendForAppQuit(): void; + resumeAfterAbortedAppQuit(): void; +} + +/** + * The one place that installs a downloaded update without the + * unsaved-settings unload guard fighting the quit. Every install entry + * point — the settings About section and the app-wide update notification + * panel — must go through it: installing quits the app and closes the + * window while the settings form may be dirty, and only a root service can + * reach the settings-scoped unload guard from both. + * + * The choreography: suspend every registered guard, invoke the updater, + * and restore the protection whenever the promised quit provably did not + * happen — a non-'downloaded' reply, a rejected IPC, or a later + * error-status push (electron-updater reports install failures that way + * rather than throwing). `installQuitPending` is armed before the IPC + * round trip because the ordering between the reply and status pushes is + * not guaranteed; the 'downloaded' reply never re-arms it, so a failure + * push that wins the race cannot be undone by the stale reply. + */ +@Injectable({ providedIn: 'root' }) +export class AppUpdateInstallService { + private readonly guards = new Set(); + private installQuitPending = false; + + constructor() { + // App-lifetime subscription on purpose: the failure push can arrive + // after the UI that requested the install is gone. + window.electron?.onAppUpdateStatusChange?.((status) => { + if ( + this.installQuitPending && + status.status !== + ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded + ) { + this.abortInstallQuit(); + } + }); + } + + /** + * The settings unload guard registers itself while settings is open. + * @returns the matching unregister + */ + registerUnloadGuard(hooks: AppQuitUnloadGuardHooks): () => void { + this.guards.add(hooks); + return () => this.guards.delete(hooks); + } + + /** + * @returns the updater's reply, or null when the bridge is unavailable + */ + async installAppUpdate(): Promise { + if (!window.electron?.installAppUpdate) { + return null; + } + + this.guards.forEach((guard) => guard.suspendForAppQuit()); + this.installQuitPending = true; + + try { + const status = await window.electron.installAppUpdate(); + + if ( + status?.status !== + ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded + ) { + this.abortInstallQuit(); + } + + return status; + } catch (error) { + this.abortInstallQuit(); + throw error; + } + } + + /** The promised quit is not happening: restore the unload protection. */ + private abortInstallQuit(): void { + this.installQuitPending = false; + this.guards.forEach((guard) => guard.resumeAfterAbortedAppQuit()); + } +} diff --git a/apps/web/src/app/settings/settings-app-update.facade.spec.ts b/apps/web/src/app/settings/settings-app-update.facade.spec.ts index f6e1dd3c7..3feba5a49 100644 --- a/apps/web/src/app/settings/settings-app-update.facade.spec.ts +++ b/apps/web/src/app/settings/settings-app-update.facade.spec.ts @@ -12,7 +12,6 @@ import { ElectronServiceStub } from '../services/electron.service.stub'; import { SettingsService } from '../services/settings.service'; import { AppUpdateReleaseNotesDialogComponent } from './app-update-release-notes-dialog.component'; import { SettingsAppUpdateFacade } from './settings-app-update.facade'; -import { SettingsUnloadGuardService } from './settings-unload-guard.service'; import { createElectronStub, DEFAULT_APP_UPDATE_STATUS, @@ -47,10 +46,6 @@ describe('SettingsAppUpdateFacade', () => { { provide: DataService, useClass: ElectronServiceStub }, MockProvider(MatDialog, { open: jest.fn() }), { provide: SettingsService, useClass: MockSettingsService }, - MockProvider(SettingsUnloadGuardService, { - resumeAfterAbortedAppQuit: jest.fn(), - suspendForAppQuit: jest.fn(), - }), ], imports: [TranslateModule.forRoot()], }); @@ -75,15 +70,17 @@ describe('SettingsAppUpdateFacade', () => { facade.init(); await flush(); - const statusHandler = ( + // The last subscriber is the facade's own — AppUpdateInstallService + // (injected by the facade) also subscribes at construction time. + const statusCalls = ( window.electron.onAppUpdateStatusChange as jest.Mock - ).mock.calls[0][0] as (status: ElectronBridgeAppUpdateStatus) => void; + ).mock.calls; + const statusHandler = statusCalls[statusCalls.length - 1][0] as ( + status: ElectronBridgeAppUpdateStatus + ) => void; statusHandler(pushedStatus); expect(window.electron.getAppUpdateStatus).toHaveBeenCalledTimes(1); - expect(window.electron.onAppUpdateStatusChange).toHaveBeenCalledTimes( - 1 - ); expect(facade.status()).toEqual(pushedStatus); }); @@ -154,131 +151,6 @@ describe('SettingsAppUpdateFacade', () => { expect(window.electron.installAppUpdate).toHaveBeenCalledTimes(1); }); - it('keeps the unload guard down while a real install quits the app', async () => { - // The updater closes the window while the settings form may still be - // dirty; an armed beforeunload would cancel that close at the DOM - // layer and strand the install. - const unloadGuard = TestBed.inject(SettingsUnloadGuardService); - (window.electron.installAppUpdate as jest.Mock).mockResolvedValue({ - ...DEFAULT_APP_UPDATE_STATUS, - status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded, - }); - - await facade.installAppUpdate(); - - expect(unloadGuard.suspendForAppQuit).toHaveBeenCalledTimes(1); - expect(unloadGuard.resumeAfterAbortedAppQuit).not.toHaveBeenCalled(); - }); - - it('restores the unload guard when the install did not quit', async () => { - const unloadGuard = TestBed.inject(SettingsUnloadGuardService); - // The default stub reply stays Idle: nothing installable, no quit. - await facade.installAppUpdate(); - - expect(unloadGuard.suspendForAppQuit).toHaveBeenCalledTimes(1); - expect(unloadGuard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes( - 1 - ); - }); - - it('restores the unload guard when the install IPC rejects', async () => { - const unloadGuard = TestBed.inject(SettingsUnloadGuardService); - const failure = new Error('ipc failed'); - (window.electron.installAppUpdate as jest.Mock).mockRejectedValue( - failure - ); - - await expect(facade.installAppUpdate()).rejects.toThrow(failure); - - expect(unloadGuard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes( - 1 - ); - }); - - it('restores the unload guard when an error push follows a quitting install', async () => { - // electron-updater reports install failures as a later 'error' - // status event, not as a rejection of the install IPC — the app is - // then still running with the protection suspended. - const unloadGuard = TestBed.inject(SettingsUnloadGuardService); - (window.electron.installAppUpdate as jest.Mock).mockResolvedValue({ - ...DEFAULT_APP_UPDATE_STATUS, - status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded, - }); - facade.init(); - const pushStatus = ( - window.electron.onAppUpdateStatusChange as jest.Mock - ).mock.calls[0][0] as ( - status: ElectronBridgeAppUpdateStatus - ) => void; - - await facade.installAppUpdate(); - expect(unloadGuard.resumeAfterAbortedAppQuit).not.toHaveBeenCalled(); - - pushStatus({ - ...DEFAULT_APP_UPDATE_STATUS, - status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error, - }); - - expect(unloadGuard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes( - 1 - ); - - // The resume is one-shot: later unrelated pushes change nothing. - pushStatus(DEFAULT_APP_UPDATE_STATUS); - expect(unloadGuard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes( - 1 - ); - }); - - it('survives an error push that beats the install reply', async () => { - // IPC ordering between the invoke reply and status pushes is not - // guaranteed: the failure can arrive first, and the stale - // 'downloaded' reply must not re-suspend the protection the - // failure already restored. - const unloadGuard = TestBed.inject(SettingsUnloadGuardService); - let resolveInstall: ( - status: ElectronBridgeAppUpdateStatus - ) => void = () => undefined; - (window.electron.installAppUpdate as jest.Mock).mockImplementation( - () => - new Promise((resolve) => { - resolveInstall = resolve; - }) - ); - facade.init(); - const pushStatus = ( - window.electron.onAppUpdateStatusChange as jest.Mock - ).mock.calls[0][0] as ( - status: ElectronBridgeAppUpdateStatus - ) => void; - - const install = facade.installAppUpdate(); - - pushStatus({ - ...DEFAULT_APP_UPDATE_STATUS, - status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error, - }); - expect(unloadGuard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes( - 1 - ); - - resolveInstall({ - ...DEFAULT_APP_UPDATE_STATUS, - status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded, - }); - await install; - - expect(unloadGuard.suspendForAppQuit).toHaveBeenCalledTimes(1); - // A later push must find nothing left to resume. - pushStatus({ - ...DEFAULT_APP_UPDATE_STATUS, - status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error, - }); - expect(unloadGuard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes( - 1 - ); - }); - it('opens the manual release URL from unsupported update status', () => { const openSpy = jest.spyOn(window, 'open').mockReturnValue(null); facade.status.set({ diff --git a/apps/web/src/app/settings/settings-app-update.facade.ts b/apps/web/src/app/settings/settings-app-update.facade.ts index 3a6cdf22c..9c1410759 100644 --- a/apps/web/src/app/settings/settings-app-update.facade.ts +++ b/apps/web/src/app/settings/settings-app-update.facade.ts @@ -7,9 +7,9 @@ import { } from '@iptvnator/shared/interfaces'; import { TranslateService } from '@ngx-translate/core'; import { take } from 'rxjs'; +import { AppUpdateInstallService } from '../services/app-update-install.service'; import { SettingsService } from '../services/settings.service'; import { AppUpdateReleaseNotesDialogComponent } from './app-update-release-notes-dialog.component'; -import { SettingsUnloadGuardService } from './settings-unload-guard.service'; const APP_UPDATE_STATUS_LOAD_ATTEMPTS = 60; const APP_UPDATE_STATUS_LOAD_RETRY_DELAY_MS = 250; @@ -25,7 +25,7 @@ export class SettingsAppUpdateFacade { private readonly runtime = inject(RuntimeCapabilitiesService); private readonly settingsService = inject(SettingsService); private readonly translate = inject(TranslateService); - private readonly unloadGuard = inject(SettingsUnloadGuardService); + private readonly installService = inject(AppUpdateInstallService); /** Latest updater status, polled once and then pushed by the backend */ readonly status = signal(null); @@ -37,8 +37,6 @@ export class SettingsAppUpdateFacade { readonly updateMessage = signal(''); private unsubscribeStatus: (() => void) | null = null; - /** True after an install reply promised a quit that has not happened. */ - private installQuitPending = false; /** Subscribes to status pushes and kicks off the initial status load */ init(): void { @@ -68,49 +66,20 @@ export class SettingsAppUpdateFacade { } async installAppUpdate(): Promise { - if (!this.runtime.isElectron || !window.electron?.installAppUpdate) { + if (!this.runtime.isElectron) { return; } - // Installing quits the app; the unsaved-settings unload guard must - // not fight a quit the user just asked for — its `beforeunload` - // would cancel the updater's window close at the DOM layer and - // strand the install. A 'downloaded' reply means quitAndInstall ran - // and the app is going down; anything else — including a rejected - // IPC — means no quit happened, so the protection comes back. - // electron-updater can also report the failure as a later 'error' - // status push instead (bindStatusEvents handles that). - this.unloadGuard.suspendForAppQuit(); - // Armed before the IPC round trip: the error push can beat the - // invoke reply, and the status listener must already see the - // pending quit to resume the guard. The 'downloaded' reply - // deliberately does not re-arm it — if the push won the race, - // re-arming would re-suspend a protection the failure just - // restored. - this.installQuitPending = true; + // Installing quits the app; AppUpdateInstallService owns the whole + // choreography of standing the unsaved-settings unload guard down + // for that quit and restoring it when the quit provably fails. + const status = await this.installService.installAppUpdate(); - try { - const status = await window.electron.installAppUpdate(); + if (status) { this.status.set(status); - - if ( - status?.status !== - ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded - ) { - this.abortInstallQuit(); - } - } catch (error) { - this.abortInstallQuit(); - throw error; } } - /** The promised quit is not happening: restore the unload protection. */ - private abortInstallQuit(): void { - this.installQuitPending = false; - this.unloadGuard.resumeAfterAbortedAppQuit(); - } - openManualAppUpdate(): void { const manualDownloadUrl = this.status()?.manualDownloadUrl; @@ -206,17 +175,6 @@ export class SettingsAppUpdateFacade { this.unsubscribeStatus = window.electron.onAppUpdateStatusChange( (status) => { this.status.set(status); - - // Any post-install status other than the quitting install's - // own proves the app is still running — the promised quit is - // not coming, so the unload protection returns. - if ( - this.installQuitPending && - status.status !== - ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded - ) { - this.abortInstallQuit(); - } } ); } diff --git a/apps/web/src/app/settings/settings-unload-guard.service.spec.ts b/apps/web/src/app/settings/settings-unload-guard.service.spec.ts index 883862f16..3cfa6d3bb 100644 --- a/apps/web/src/app/settings/settings-unload-guard.service.spec.ts +++ b/apps/web/src/app/settings/settings-unload-guard.service.spec.ts @@ -1,5 +1,6 @@ import { TestBed } from '@angular/core/testing'; import { FormBuilder } from '@angular/forms'; +import { AppUpdateInstallService } from '../services/app-update-install.service'; import { SettingsForm } from './settings-form.utils'; import { SettingsUnloadGuardHost, @@ -49,6 +50,11 @@ describe('SettingsUnloadGuardService', () => { setWindowCloseGuard: jest.fn().mockResolvedValue(undefined), confirmWindowClose: jest.fn().mockResolvedValue(undefined), cancelWindowClose: jest.fn().mockResolvedValue(undefined), + installAppUpdate: jest.fn().mockResolvedValue({ + currentVersion: '0.23.0', + status: 'idle', + supportedSelfUpdate: true, + }), onWindowCloseRequested: jest.fn((callback: () => void) => { closeRequestCallback = callback; return unsubscribeCloseRequests; @@ -165,6 +171,38 @@ describe('SettingsUnloadGuardService', () => { const event = dispatchBeforeUnload(); expect(event.defaultPrevented).toBe(true); }); + + it('stands down for any install entry point via the install service', async () => { + // The global update panel installs through the same root + // service; while settings is mounted, that must reach this + // guard. An Idle reply (nothing installable) suspends and + // restores in one round trip. + const installService = TestBed.inject(AppUpdateInstallService); + activateInElectron(); + form.markAsDirty(); + + await installService.installAppUpdate(); + + expect(electronStub.setWindowCloseGuard).toHaveBeenLastCalledWith( + true + ); + const event = dispatchBeforeUnload(); + expect(event.defaultPrevented).toBe(true); + }); + + it('unregisters from the install service on destroy', async () => { + const installService = TestBed.inject(AppUpdateInstallService); + activateInElectron(); + service.ngOnDestroy(); + const callsAfterDestroy = + electronStub.setWindowCloseGuard.mock.calls.length; + + await installService.installAppUpdate(); + + expect(electronStub.setWindowCloseGuard.mock.calls.length).toBe( + callsAfterDestroy + ); + }); }); describe('intercepted window close (Electron)', () => { diff --git a/apps/web/src/app/settings/settings-unload-guard.service.ts b/apps/web/src/app/settings/settings-unload-guard.service.ts index f53e042ba..b90da2919 100644 --- a/apps/web/src/app/settings/settings-unload-guard.service.ts +++ b/apps/web/src/app/settings/settings-unload-guard.service.ts @@ -1,4 +1,5 @@ import { inject, Injectable, NgZone, OnDestroy } from '@angular/core'; +import { AppUpdateInstallService } from '../services/app-update-install.service'; import { SettingsForm } from './settings-form.utils'; export interface SettingsUnloadGuardHost { @@ -39,8 +40,10 @@ export interface SettingsUnloadGuardHost { @Injectable() export class SettingsUnloadGuardService implements OnDestroy { private readonly zone = inject(NgZone); + private readonly installService = inject(AppUpdateInstallService); private host: SettingsUnloadGuardHost | null = null; private unsubscribeCloseRequests: (() => void) | null = null; + private unregisterFromInstallService: (() => void) | null = null; private confirmationPending = false; /** * Intent of the confirmation currently on screen. A close request @@ -68,6 +71,16 @@ export class SettingsUnloadGuardService implements OnDestroy { this.zone.run(() => void this.handleCloseRequest('close')); }) ?? null; + // Whatever surface installs an app update — the settings About + // section or the app-wide notification panel — must be able to + // stand this guard down before the updater closes the window. + this.unregisterFromInstallService = + this.installService.registerUnloadGuard({ + resumeAfterAbortedAppQuit: () => + this.resumeAfterAbortedAppQuit(), + suspendForAppQuit: () => this.suspendForAppQuit(), + }); + // Armed before the user can possibly stage an edit; a close with a // pristine form auto-confirms through the dialog-less path, so the // only visible effect of mount-long arming is race-free protection. @@ -110,6 +123,8 @@ export class SettingsUnloadGuardService implements OnDestroy { window.removeEventListener('beforeunload', this.beforeUnloadHandler); this.unsubscribeCloseRequests?.(); this.unsubscribeCloseRequests = null; + this.unregisterFromInstallService?.(); + this.unregisterFromInstallService = null; this.suspended = false; this.syncCloseGuard(false); this.host = null;