diff --git a/plugin/subminer/lifecycle.lua b/plugin/subminer/lifecycle.lua index 6d1ee1b2..2ddd25d5 100644 --- a/plugin/subminer/lifecycle.lua +++ b/plugin/subminer/lifecycle.lua @@ -272,6 +272,7 @@ function M.create(ctx) state.pending_reload_media_identity = nil state.pending_reload_media_title = nil state.pending_reload_reason = nil + state.app_managed_playback_pending = false state.app_managed_playback_active = false if state.overlay_running and reason ~= "quit" then process.hide_visible_overlay() diff --git a/plugin/subminer/process.lua b/plugin/subminer/process.lua index 214d92d9..eae53ff7 100644 --- a/plugin/subminer/process.lua +++ b/plugin/subminer/process.lua @@ -401,7 +401,6 @@ function M.create(ctx) end run_control_command_async = function(action, overrides, callback) - record_visible_overlay_action(action) local args = build_command_args(action, overrides) local command = build_subprocess_command(args) subminer_log("debug", "process", "Control command: " .. table.concat(args, " ")) @@ -414,6 +413,9 @@ function M.create(ctx) capture_stderr = true, }, function(success, result, error) local ok = success and (result == nil or result.status == 0) + if ok then + record_visible_overlay_action(action) + end if callback then callback(ok, result, error) end diff --git a/scripts/test-plugin-process-start-retries.lua b/scripts/test-plugin-process-start-retries.lua index cf5fca17..fb5b1d4e 100644 --- a/scripts/test-plugin-process-start-retries.lua +++ b/scripts/test-plugin-process-start-retries.lua @@ -83,10 +83,16 @@ local process = process_module.create({ return true end, }, - environment = { - detect_backend = function() - return "x11" - end, + environment = { + detect_backend = function() + return "x11" + end, + is_linux = function() + return false + end, + is_subminer_app_running_async = function(callback) + callback(false) + end, }, options_helper = { coerce_bool = function(value, default_value) @@ -125,4 +131,79 @@ for _, timeout_seconds in ipairs(recorded.timeouts) do end assert_true(retry_timeout_seen, "expected shorter bounded retry timeout") +do + local visibility_state = { + binary_path = "/tmp/subminer", + overlay_running = true, + texthooker_running = false, + visible_overlay_requested = false, + } + local visibility_calls = {} + local visibility_mp = {} + + function visibility_mp.command_native_async(command, callback) + visibility_calls[#visibility_calls + 1] = command + if callback then + callback(false, { status = 1, stdout = "", stderr = "failed" }, "failed") + end + end + + local visibility_process = process_module.create({ + mp = visibility_mp, + opts = { + backend = "x11", + socket_path = "/tmp/subminer.sock", + log_level = "debug", + texthooker_enabled = true, + texthooker_port = 5174, + auto_start_visible_overlay = false, + }, + state = visibility_state, + binary = { + ensure_binary_available = function() + return true + end, + }, + environment = { + detect_backend = function() + return "x11" + end, + is_linux = function() + return false + end, + is_subminer_app_running_async = function(callback) + callback(true) + end, + }, + options_helper = { + coerce_bool = function(value, default_value) + if value == true or value == "yes" or value == "true" then + return true + end + if value == false or value == "no" or value == "false" then + return false + end + return default_value + end, + }, + log = { + subminer_log = function(_level, _scope, line) + recorded.logs[#recorded.logs + 1] = line + end, + show_osd = function(_) end, + normalize_log_level = function(value) + return value or "info" + end, + }, + }) + + visibility_process.run_control_command_async("show-visible-overlay") + + assert_true(#visibility_calls == 1, "expected visible overlay command to run") + assert_true( + visibility_state.visible_overlay_requested == false, + "failed visible-overlay command should not update requested visibility state" + ) +end + print("plugin process retry regression tests: OK") diff --git a/scripts/test-plugin-start-gate.lua b/scripts/test-plugin-start-gate.lua index 9ccccf5d..81043e2c 100644 --- a/scripts/test-plugin-start-gate.lua +++ b/scripts/test-plugin-start-gate.lua @@ -645,6 +645,36 @@ do ) end +do + local scenario = { + process_list = "", + option_overrides = { + binary_path = binary_path, + auto_start = "yes", + auto_start_visible_overlay = "yes", + auto_start_pause_until_ready = "yes", + socket_path = "/tmp/subminer-socket", + }, + input_ipc_server = "/tmp/subminer-socket", + path = "/media/aborted-app-managed.m3u8", + media_title = "Aborted App Managed", + files = { + [binary_path] = true, + }, + } + local recorded, err = run_plugin_scenario(scenario) + assert_true(recorded ~= nil, "plugin failed to load for aborted app-managed scenario: " .. tostring(err)) + recorded.script_messages["subminer-managed-subtitles-loading"]() + fire_event(recorded, "end-file", { reason = "error" }) + scenario.path = "/media/next-normal.mkv" + scenario.media_title = "Next Normal" + fire_event(recorded, "file-loaded") + assert_true( + count_start_calls(recorded.async_calls) == 1, + "aborted app-managed playback should not leak pending state into the next item" + ) +end + do local scenario = { process_list = "", diff --git a/src/core/services/overlay-visibility.test.ts b/src/core/services/overlay-visibility.test.ts index 847867d5..b5a4d13a 100644 --- a/src/core/services/overlay-visibility.test.ts +++ b/src/core/services/overlay-visibility.test.ts @@ -197,6 +197,68 @@ test('tracked non-macOS overlay stays hidden while tracker is not ready', () => assert.ok(!calls.includes('osd')); }); +test('non-native passive overlay stays click-through after subsequent visibility updates', () => { + const { window, calls } = createMainWindowRecorder(); + + updateVisibleOverlayVisibility({ + visibleOverlayVisible: true, + mainWindow: window as never, + trackerNotReadyWarningShown: false, + setTrackerNotReadyWarningShown: () => {}, + updateVisibleOverlayBounds: () => { + calls.push('update-bounds'); + }, + ensureOverlayWindowLevel: () => { + calls.push('ensure-level'); + }, + syncPrimaryOverlayWindowLayer: () => { + calls.push('sync-layer'); + }, + enforceOverlayLayerOrder: () => { + calls.push('enforce-order'); + }, + syncOverlayShortcuts: () => { + calls.push('sync-shortcuts'); + }, + isMacOSPlatform: false, + isWindowsPlatform: false, + overlayInteractionActive: false, + showOverlayLoadingOsd: () => {}, + resolveFallbackBounds: () => ({ x: 12, y: 24, width: 640, height: 360 }), + } as never); + calls.length = 0; + + updateVisibleOverlayVisibility({ + visibleOverlayVisible: true, + mainWindow: window as never, + trackerNotReadyWarningShown: false, + setTrackerNotReadyWarningShown: () => {}, + updateVisibleOverlayBounds: () => { + calls.push('update-bounds'); + }, + ensureOverlayWindowLevel: () => { + calls.push('ensure-level'); + }, + syncPrimaryOverlayWindowLayer: () => { + calls.push('sync-layer'); + }, + enforceOverlayLayerOrder: () => { + calls.push('enforce-order'); + }, + syncOverlayShortcuts: () => { + calls.push('sync-shortcuts'); + }, + isMacOSPlatform: false, + isWindowsPlatform: false, + overlayInteractionActive: false, + showOverlayLoadingOsd: () => {}, + resolveFallbackBounds: () => ({ x: 12, y: 24, width: 640, height: 360 }), + } as never); + + assert.equal(calls.includes('mouse-ignore:false:plain'), false); + assert.ok(calls.includes('mouse-ignore:true:forward')); +}); + test('suspended visible overlay hides without refreshing bounds or z-order', () => { const { window, calls } = createMainWindowRecorder(); const tracker: WindowTrackerStub = { diff --git a/src/core/services/overlay-visibility.ts b/src/core/services/overlay-visibility.ts index 7e84828d..ec1785ea 100644 --- a/src/core/services/overlay-visibility.ts +++ b/src/core/services/overlay-visibility.ts @@ -181,12 +181,13 @@ export function updateVisibleOverlayVisibility(args: { !isTrackedWindowsTargetMinimized && (args.windowTracker.isTracking() || args.windowTracker.getGeometry() !== null); const shouldForcePassiveReshow = args.isWindowsPlatform && !wasVisible; + const isNonNativePassiveOverlay = + !args.isWindowsPlatform && !args.isMacOSPlatform && !overlayInteractionActive; const shouldIgnoreMouseEvents = shouldUseMacOSMousePassthrough || forceMousePassthrough || + isNonNativePassiveOverlay || (shouldDefaultToPassthrough && (!isVisibleOverlayFocused || shouldForcePassiveReshow)); - const isNonNativePassiveOverlay = - !args.isWindowsPlatform && !args.isMacOSPlatform && !overlayInteractionActive; const shouldBindTrackedWindowsOverlay = args.isWindowsPlatform && !!args.windowTracker; const shouldKeepTrackedWindowsOverlayTopmost = !args.isWindowsPlatform || diff --git a/src/main.ts b/src/main.ts index 3f3c5862..39851182 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1830,12 +1830,13 @@ async function refreshSubtitleSidebarFromSource( if (!normalizedSourcePath) { return; } - appState.activeParsedSubtitleMediaPath = mediaPath?.trim() || getCurrentAutoplayMediaPath(); + const nextMediaPath = mediaPath?.trim() || getCurrentAutoplayMediaPath(); await subtitlePrefetchInitController.initSubtitlePrefetch( normalizedSourcePath, lastObservedTimePos, normalizedSourcePath, ); + appState.activeParsedSubtitleMediaPath = nextMediaPath; } const refreshSubtitlePrefetchFromActiveTrackHandler = createRefreshSubtitlePrefetchFromActiveTrackHandler({ @@ -6476,8 +6477,8 @@ function setVisibleOverlayVisible(visible: boolean): void { function toggleVisibleOverlay(): void { ensureOverlayWindowsReadyForVisibilityActions(); const nextVisible = !overlayManager.getVisibleOverlayVisible(); - autoplayReadyGate.markCurrentMediaAutoplayReady(); if (!nextVisible) { + autoplayReadyGate.markCurrentMediaAutoplayReady(); cancelPendingLinuxMpvFullscreenOverlayRefreshBurst(); } else { void ensureOverlayMpvSubtitlesHidden(); diff --git a/src/main/main-wiring.test.ts b/src/main/main-wiring.test.ts index 8d83cc9c..08bf882f 100644 --- a/src/main/main-wiring.test.ts +++ b/src/main/main-wiring.test.ts @@ -59,17 +59,33 @@ test('same media path updates do not reset autoplay ready fallback state', () => ); }); -test('manual visible overlay toggles suppress current-media autoplay release', () => { +test('manual visible overlay toggles only release current-media autoplay when hiding', () => { const source = readMainSource(); const actionBlock = source.match( /function toggleVisibleOverlay\(\): void \{(?[\s\S]*?)\n\}/, )?.groups?.body; assert.ok(actionBlock); - assert.match(actionBlock, /autoplayReadyGate\.markCurrentMediaAutoplayReady\(\);/); + assert.match( + actionBlock, + /if \(!nextVisible\) \{\s+autoplayReadyGate\.markCurrentMediaAutoplayReady\(\);\s+cancelPendingLinuxMpvFullscreenOverlayRefreshBurst\(\);/, + ); +}); + +test('subtitle sidebar media path tag is assigned after prefetch succeeds', () => { + const source = readMainSource(); + const actionBlock = source.match( + /async function refreshSubtitleSidebarFromSource\([\s\S]*?\): Promise \{(?[\s\S]*?)\n\}/, + )?.groups?.body; + + assert.ok(actionBlock); + assert.match( + actionBlock, + /const nextMediaPath = mediaPath\?\.trim\(\) \|\| getCurrentAutoplayMediaPath\(\);/, + ); assert.ok( - actionBlock.indexOf('autoplayReadyGate.markCurrentMediaAutoplayReady();') < - actionBlock.indexOf('toggleVisibleOverlayHandler();'), + actionBlock.indexOf('subtitlePrefetchInitController.initSubtitlePrefetch') < + actionBlock.indexOf('appState.activeParsedSubtitleMediaPath = nextMediaPath;'), ); }); diff --git a/src/main/runtime/jellyfin-remote-playback.test.ts b/src/main/runtime/jellyfin-remote-playback.test.ts index a99ce75b..b227f5aa 100644 --- a/src/main/runtime/jellyfin-remote-playback.test.ts +++ b/src/main/runtime/jellyfin-remote-playback.test.ts @@ -290,6 +290,37 @@ test('createReportJellyfinRemoteStoppedHandler reports stop and clears playback' assert.equal(cleared, true); }); +test('createReportJellyfinRemoteStoppedHandler clears aborted playback that never loaded', async () => { + let cleared = false; + const reportStopped = createReportJellyfinRemoteStoppedHandler({ + getActivePlayback: () => ({ + itemId: 'item-2', + mediaSourceId: undefined, + playMethod: 'Transcode', + audioStreamIndex: null, + subtitleStreamIndex: null, + loadedMediaPath: null, + }), + clearActivePlayback: () => { + cleared = true; + }, + getSession: () => ({ + isConnected: () => true, + reportProgress: async () => {}, + reportStopped: async () => { + throw new Error('should not report stopped for unloaded media'); + }, + }), + getMpvClient: () => null, + ticksPerSecond: 10_000_000, + logDebug: () => {}, + }); + + await reportStopped(); + + assert.equal(cleared, true); +}); + test('createReportJellyfinRemoteStoppedHandler reports stop while remote websocket is disconnected', async () => { let cleared = false; let stoppedPayload: { @@ -409,7 +440,7 @@ test('createReportJellyfinRemoteStoppedHandler ignores unloaded active playback' await reportStopped(); assert.equal(stopped, false); - assert.equal(cleared, false); + assert.equal(cleared, true); }); test('createReportJellyfinRemoteProgressHandler caches last nonzero mpv position', async () => { diff --git a/src/main/runtime/jellyfin-remote-playback.ts b/src/main/runtime/jellyfin-remote-playback.ts index b1367105..8ea0c6c9 100644 --- a/src/main/runtime/jellyfin-remote-playback.ts +++ b/src/main/runtime/jellyfin-remote-playback.ts @@ -209,7 +209,10 @@ export function createReportJellyfinRemoteStoppedHandler(deps: JellyfinRemoteSto return async (): Promise => { const playback = deps.getActivePlayback(); if (!playback) return; - if (playback.loadedMediaPath === null) return; + if (playback.loadedMediaPath === null) { + deps.clearActivePlayback(); + return; + } if ( typeof playback.stopReportsAfterMs === 'number' && Number.isFinite(playback.stopReportsAfterMs) && diff --git a/src/main/runtime/jellyfin-subtitle-cache-io.test.ts b/src/main/runtime/jellyfin-subtitle-cache-io.test.ts index f0382a45..ac833ec6 100644 --- a/src/main/runtime/jellyfin-subtitle-cache-io.test.ts +++ b/src/main/runtime/jellyfin-subtitle-cache-io.test.ts @@ -67,3 +67,27 @@ test('jellyfin subtitle cache io removes temp dir when download fails', async () ); assert.deepEqual(removed, ['/tmp/subminer-jellyfin-subtitles-failed']); }); + +test('jellyfin subtitle cache io awaits async temp cleanup when download fails', async () => { + let removed = false; + const cacheIo = createJellyfinSubtitleCacheIo({ + tmpDir: () => '/tmp', + makeTempDir: async () => '/tmp/subminer-jellyfin-subtitles-failed', + writeFile: async () => {}, + removeDir: async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + removed = true; + }, + fetch: async () => ({ + ok: false, + status: 500, + arrayBuffer: async () => new ArrayBuffer(0), + }), + }); + + await assert.rejects( + () => cacheIo.cacheSubtitleTrack({ index: 1, deliveryUrl: 'https://example.test/sub.srt' }), + /HTTP 500/, + ); + assert.equal(removed, true); +}); diff --git a/src/main/runtime/jellyfin-subtitle-cache-io.ts b/src/main/runtime/jellyfin-subtitle-cache-io.ts index d2a0ee98..3361c16a 100644 --- a/src/main/runtime/jellyfin-subtitle-cache-io.ts +++ b/src/main/runtime/jellyfin-subtitle-cache-io.ts @@ -20,7 +20,7 @@ type JellyfinSubtitleCacheIoDeps = { tmpDir: () => string; makeTempDir: (prefix: string) => Promise; writeFile: (filePath: string, bytes: Uint8Array) => Promise; - removeDir: (dir: string, options: { recursive: true; force: true }) => void; + removeDir: (dir: string, options: { recursive: true; force: true }) => void | Promise; fetch: (url: string) => Promise; }; @@ -59,14 +59,16 @@ export function createJellyfinSubtitleCacheIo(deps: JellyfinSubtitleCacheIoDeps) const bytes = new Uint8Array(await response.arrayBuffer()); await deps.writeFile(subtitlePath, bytes); } catch (error) { - deps.removeDir(cacheDir, { recursive: true, force: true }); + try { + await Promise.resolve(deps.removeDir(cacheDir, { recursive: true, force: true })); + } catch {} throw error; } return { path: subtitlePath, cleanupDir: cacheDir }; }, cleanupCachedSubtitles(dirs: string[]): void { for (const dir of dirs) { - deps.removeDir(dir, { recursive: true, force: true }); + void Promise.resolve(deps.removeDir(dir, { recursive: true, force: true })).catch(() => {}); } }, }; diff --git a/src/main/runtime/jellyfin-subtitle-preload.test.ts b/src/main/runtime/jellyfin-subtitle-preload.test.ts index 039d6d97..bc8ae77c 100644 --- a/src/main/runtime/jellyfin-subtitle-preload.test.ts +++ b/src/main/runtime/jellyfin-subtitle-preload.test.ts @@ -73,7 +73,8 @@ function withoutTrackAutoSelectionCommands( (command[1] === 'sid' && command[2] === 'no') || (command[1] === 'secondary-sid' && command[2] === 'no') || (command[1] === 'sub-visibility' && command[2] === 'no') || - (command[1] === 'secondary-sub-visibility' && command[2] === 'no')) + (command[1] === 'secondary-sub-visibility' && command[2] === 'no') || + (command[1] === 'sub-delay' && command[2] === 0)) ), ); } @@ -284,6 +285,25 @@ test('preload jellyfin subtitles waits for delayed external japanese track inste ]); }); +test('preload jellyfin subtitles clears managed delay when no external tracks are available', async () => { + const commands: Array> = []; + const activeDelayKeys: Array = []; + const preload = createPreloadJellyfinExternalSubtitlesHandler( + makeDeps({ + listJellyfinSubtitleTracks: async () => [ + { index: 0, language: 'jpn', title: 'Embedded Japanese' }, + ], + sendMpvCommand: (command) => commands.push(command), + setActiveSubtitleDelayKey: (key) => activeDelayKeys.push(key), + }), + ); + + await preload({ session, clientInfo, itemId: 'item-1' }); + + assert.deepEqual(activeDelayKeys, [null]); + assert.deepEqual(commands, [['set_property', 'sub-delay', 0]]); +}); + test('preload jellyfin subtitles prefers Jellyfin default and embedded japanese sources', async () => { const commands: Array> = []; const preload = createPreloadJellyfinExternalSubtitlesHandler( @@ -953,7 +973,7 @@ test('preload jellyfin subtitles exits quietly when no external tracks', async ( await preload({ session, clientInfo, itemId: 'item-1' }); assert.equal(waited, false); - assert.deepEqual(commands, []); + assert.deepEqual(commands, [['set_property', 'sub-delay', 0]]); }); test('preload jellyfin subtitles logs debug on failure', async () => { diff --git a/src/main/runtime/jellyfin-subtitle-preload.ts b/src/main/runtime/jellyfin-subtitle-preload.ts index 2acf35dd..abcebff8 100644 --- a/src/main/runtime/jellyfin-subtitle-preload.ts +++ b/src/main/runtime/jellyfin-subtitle-preload.ts @@ -326,9 +326,7 @@ export function createPreloadJellyfinExternalSubtitlesHandler(deps: { let preloadQueue: Promise = Promise.resolve(); function resetManagedSubtitleDelay(): void { - if (deps.getSavedSubtitleDelay) { - deps.sendMpvCommand(['set_property', 'sub-delay', 0]); - } + deps.sendMpvCommand(['set_property', 'sub-delay', 0]); } function cleanupActiveCache(): void { @@ -358,6 +356,8 @@ export function createPreloadJellyfinExternalSubtitlesHandler(deps: { ); const externalTracks = tracks.filter((track) => Boolean(track.deliveryUrl)); if (externalTracks.length === 0) { + deps.setActiveSubtitleDelayKey?.(null); + resetManagedSubtitleDelay(); return; } diff --git a/stats/src/components/layout/DeleteConfirmDialog.test.tsx b/stats/src/components/layout/DeleteConfirmDialog.test.tsx new file mode 100644 index 00000000..7a96fecd --- /dev/null +++ b/stats/src/components/layout/DeleteConfirmDialog.test.tsx @@ -0,0 +1,22 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import test from 'node:test'; + +test('delete confirmation dialog swallows Escape before closing', () => { + const source = fs.readFileSync( + path.join(process.cwd(), 'stats/src/components/layout/DeleteConfirmDialog.tsx'), + 'utf8', + ); + const handlerBlock = source.match( + /const onKeyDown = \(event: KeyboardEvent\) => \{(?[\s\S]*?)\n \};/, + )?.groups?.body; + + assert.ok(handlerBlock); + assert.match(handlerBlock, /event\.preventDefault\(\);/); + assert.match(handlerBlock, /event\.stopPropagation\(\);/); + assert.match(handlerBlock, /event\.stopImmediatePropagation\(\);/); + assert.ok( + handlerBlock.indexOf('event.stopPropagation();') < handlerBlock.indexOf('finish(false);'), + ); +}); diff --git a/stats/src/components/layout/DeleteConfirmDialog.tsx b/stats/src/components/layout/DeleteConfirmDialog.tsx index 09da6b5a..e708c8f8 100644 --- a/stats/src/components/layout/DeleteConfirmDialog.tsx +++ b/stats/src/components/layout/DeleteConfirmDialog.tsx @@ -40,6 +40,8 @@ export function DeleteConfirmDialog() { const onKeyDown = (event: KeyboardEvent) => { if (event.key !== 'Escape') return; event.preventDefault(); + event.stopPropagation(); + event.stopImmediatePropagation(); finish(false); }; window.addEventListener('keydown', onKeyDown, true); diff --git a/stats/src/components/library/MediaDetailView.test.tsx b/stats/src/components/library/MediaDetailView.test.tsx index 6a8c47a9..9230f398 100644 --- a/stats/src/components/library/MediaDetailView.test.tsx +++ b/stats/src/components/library/MediaDetailView.test.tsx @@ -125,3 +125,40 @@ test('buildDeleteEpisodeHandler sets error when deleteVideo throws', async () => await handler(); assert.equal(capturedError, 'Network failure'); }); + +test('buildDeleteEpisodeHandler guards duplicate clicks while confirmation is pending', async () => { + const confirmResolvers: Array<(value: boolean) => void> = []; + let confirmCalls = 0; + let deleteCalls = 0; + const isDeletingRef = { current: false }; + + const handler = buildDeleteEpisodeHandler({ + videoId: 42, + title: 'Test Episode', + apiClient: { + deleteVideo: async () => { + deleteCalls += 1; + }, + }, + confirmFn: () => { + confirmCalls += 1; + return new Promise((resolve) => { + confirmResolvers.push(resolve); + }); + }, + onBack: () => {}, + setDeleteError: () => {}, + isDeletingRef, + }); + + const first = handler(); + const second = handler(); + for (const resolveConfirm of confirmResolvers) { + resolveConfirm(true); + } + await Promise.all([first, second]); + + assert.equal(confirmCalls, 1); + assert.equal(deleteCalls, 1); + assert.equal(isDeletingRef.current, false); +}); diff --git a/stats/src/components/library/MediaDetailView.tsx b/stats/src/components/library/MediaDetailView.tsx index d2129f32..3e5d6d8c 100644 --- a/stats/src/components/library/MediaDetailView.tsx +++ b/stats/src/components/library/MediaDetailView.tsx @@ -27,8 +27,19 @@ interface DeleteEpisodeHandlerOptions { export function buildDeleteEpisodeHandler(opts: DeleteEpisodeHandlerOptions): () => Promise { return async () => { if (opts.isDeletingRef?.current) return; - if (!(await opts.confirmFn(opts.title))) return; if (opts.isDeletingRef) opts.isDeletingRef.current = true; + let confirmed = false; + try { + confirmed = await opts.confirmFn(opts.title); + } catch (err) { + if (opts.isDeletingRef) opts.isDeletingRef.current = false; + opts.setDeleteError(err instanceof Error ? err.message : 'Failed to confirm delete.'); + return; + } + if (!confirmed) { + if (opts.isDeletingRef) opts.isDeletingRef.current = false; + return; + } opts.setIsDeleting?.(true); opts.setDeleteError(null); try { @@ -73,6 +84,7 @@ export function MediaDetailView({ const [deletingSessionId, setDeletingSessionId] = useState(null); const [isDeletingEpisode, setIsDeletingEpisode] = useState(false); const isDeletingEpisodeRef = useRef(false); + const isDeletingSessionRef = useRef(false); useEffect(() => { setLocalSessions(data?.sessions ?? null); @@ -101,7 +113,20 @@ export function MediaDetailView({ const relatedCollectionLabel = getRelatedCollectionLabel(detail); const handleDeleteSession = async (session: SessionSummary) => { - if (!(await confirmSessionDelete())) return; + if (isDeletingSessionRef.current) return; + isDeletingSessionRef.current = true; + let confirmed = false; + try { + confirmed = await confirmSessionDelete(); + } catch (err) { + setDeleteError(err instanceof Error ? err.message : 'Failed to confirm delete.'); + isDeletingSessionRef.current = false; + return; + } + if (!confirmed) { + isDeletingSessionRef.current = false; + return; + } setDeleteError(null); setDeletingSessionId(session.sessionId); @@ -114,6 +139,7 @@ export function MediaDetailView({ setDeleteError(err instanceof Error ? err.message : 'Failed to delete session.'); } finally { setDeletingSessionId(null); + isDeletingSessionRef.current = false; } }; diff --git a/stats/src/components/sessions/SessionsTab.test.tsx b/stats/src/components/sessions/SessionsTab.test.tsx index ebf17334..956a49bc 100644 --- a/stats/src/components/sessions/SessionsTab.test.tsx +++ b/stats/src/components/sessions/SessionsTab.test.tsx @@ -125,6 +125,34 @@ test('buildBucketDeleteHandler reports errors via onError without calling onSucc assert.equal(successCalled, false); }); +test('buildBucketDeleteHandler reports confirmation errors via onError', async () => { + let errorMessage: string | null = null; + let deleteCalled = false; + + const bucket = makeBucket([makeSession({ sessionId: 1 }), makeSession({ sessionId: 2 })]); + + const handler = buildBucketDeleteHandler({ + bucket, + apiClient: { + deleteSessions: async () => { + deleteCalled = true; + }, + }, + confirm: async () => { + throw new Error('confirm failed'); + }, + onSuccess: () => {}, + onError: (message) => { + errorMessage = message; + }, + }); + + await handler(); + + assert.equal(errorMessage, 'confirm failed'); + assert.equal(deleteCalled, false); +}); + test('buildBucketDeleteHandler falls back to a generic title when canonicalTitle is null', async () => { let seenTitle: string | null = null; diff --git a/stats/src/components/sessions/SessionsTab.tsx b/stats/src/components/sessions/SessionsTab.tsx index 628359a2..d4cd8593 100644 --- a/stats/src/components/sessions/SessionsTab.tsx +++ b/stats/src/components/sessions/SessionsTab.tsx @@ -43,8 +43,8 @@ export function buildBucketDeleteHandler(deps: BucketDeleteDeps): () => Promise< return async () => { const title = bucket.representativeSession.canonicalTitle ?? 'this episode'; const ids = bucket.sessions.map((s) => s.sessionId); - if (!(await confirm(title, ids.length))) return; try { + if (!(await confirm(title, ids.length))) return; await client.deleteSessions(ids); onSuccess(ids); } catch (err) { @@ -120,7 +120,14 @@ export function SessionsTab({ }; const handleDeleteSession = async (session: SessionSummary) => { - if (!(await confirmSessionDelete())) return; + let confirmed = false; + try { + confirmed = await confirmSessionDelete(); + } catch (err) { + setDeleteError(err instanceof Error ? err.message : 'Failed to confirm delete.'); + return; + } + if (!confirmed) return; setDeleteError(null); setDeletingSessionId(session.sessionId);