From 88d3e79dfa517847f3bdf0138f8346ee94191547 Mon Sep 17 00:00:00 2001 From: vedanth Date: Tue, 27 Oct 2026 08:32:40 +0000 Subject: [PATCH] Fix caption translation request ownership --- .github/workflows/ci.yml | 1 + .../caption-translation-ownership.test.ts | 165 ++++++++++++++++++ .../[videoId]/_components/CaptionContext.tsx | 76 +++++--- 3 files changed, 217 insertions(+), 25 deletions(-) create mode 100644 apps/web/__tests__/unit/caption-translation-ownership.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 70bb14815be..429be8b6947 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -131,6 +131,7 @@ jobs: __tests__/unit/analytics-track-route.test.ts __tests__/unit/caps-analytics-round-trips.test.ts __tests__/unit/caps-keyboard.test.ts + __tests__/unit/caption-translation-ownership.test.ts __tests__/unit/comment-composer-composition.test.ts __tests__/unit/dashboard-analytics-cap-access.test.ts __tests__/unit/dashboard-analytics-space-access.test.ts diff --git a/apps/web/__tests__/unit/caption-translation-ownership.test.ts b/apps/web/__tests__/unit/caption-translation-ownership.test.ts new file mode 100644 index 00000000000..7fa918d9866 --- /dev/null +++ b/apps/web/__tests__/unit/caption-translation-ownership.test.ts @@ -0,0 +1,165 @@ +// @vitest-environment jsdom + +import type { Video } from "@cap/web-domain"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { act, createElement, StrictMode } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { + CaptionProvider, + useCaptionContext, +} from "@/app/s/[videoId]/_components/CaptionContext"; + +const translate = vi.hoisted(() => vi.fn()); +vi.mock("@/actions/videos/translate-transcript", () => ({ + translateTranscript: translate, +})); +vi.mock("@/actions/videos/get-available-translations", () => ({ + getAvailableTranslations: vi.fn(async () => ({ + hasOriginal: true, + translations: [], + })), +})); +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} +let root: Root; +let container: HTMLDivElement; +let client: QueryClient; +let captions: ReturnType; +function Consumer() { + captions = useCaptionContext(); + return null; +} +function tree(id: string) { + return createElement( + StrictMode, + null, + createElement( + QueryClientProvider, + { client }, + createElement( + CaptionProvider, + { + videoId: id as Video.VideoId, + transcriptionStatus: "COMPLETE", + } as Parameters[0], + createElement(Consumer), + ), + ), + ); +} +beforeEach(async () => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + translate.mockReset(); + client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); + await act(async () => root.render(tree("video-a"))); +}); +afterEach(async () => { + await act(async () => root.unmount()); + client.clear(); + container.remove(); + vi.unstubAllGlobals(); +}); +it("deduplicates concurrent requests for the same language in Strict Mode", async () => { + const gate = deferred<{ success: boolean; translatedVtt: string }>(); + translate.mockReturnValue(gate.promise); + let first!: Promise; + let second!: Promise; + await act(async () => { + first = captions.requestTranslation("es"); + second = captions.requestTranslation("es"); + }); + expect(translate).toHaveBeenCalledTimes(1); + expect(captions.isTranslating).toBe(true); + await act(async () => { + gate.resolve({ success: true, translatedVtt: "Spanish" }); + expect(await first).toBe("Spanish"); + expect(await second).toBe("Spanish"); + }); + expect(captions.translatedVttContent.get("es")).toBe("Spanish"); + await act(async () => { + expect(await captions.requestTranslation("es")).toBe("Spanish"); + }); + expect(translate).toHaveBeenCalledTimes(1); +}); +it("keeps the loading indicator until all language requests settle", async () => { + const spanish = deferred<{ success: boolean; translatedVtt: string }>(); + const french = deferred<{ success: boolean; translatedVtt: string }>(); + translate.mockImplementation((_id: string, language: string) => + language === "es" ? spanish.promise : french.promise, + ); + let first!: Promise; + let second!: Promise; + await act(async () => { + first = captions.requestTranslation("es"); + second = captions.requestTranslation("fr"); + }); + await act(async () => { + spanish.resolve({ success: true, translatedVtt: "Spanish" }); + await first; + }); + expect(captions.isTranslating).toBe(true); + await act(async () => { + french.resolve({ success: true, translatedVtt: "French" }); + await second; + }); + expect(captions.isTranslating).toBe(false); + expect(captions.translatedVttContent.get("fr")).toBe("French"); +}); +it("does not reuse or apply a previous video's translation after a video switch", async () => { + const old = deferred<{ success: boolean; translatedVtt: string }>(); + translate.mockImplementation((id: string) => + id === "video-a" + ? old.promise + : Promise.resolve({ success: true, translatedVtt: "New video" }), + ); + let oldRequest!: Promise; + await act(async () => { + oldRequest = captions.requestTranslation("es"); + }); + await act(async () => root.render(tree("video-b"))); + await act(async () => { + old.resolve({ success: true, translatedVtt: "Old video" }); + await oldRequest; + }); + expect(captions.translatedVttContent.has("es")).toBe(false); + expect(captions.isTranslating).toBe(false); + await act(async () => { + expect(await captions.requestTranslation("es")).toBe("New video"); + }); + expect(translate).toHaveBeenCalledWith("video-b", "es"); +}); +it("settles a rejected request and allows a retry", async () => { + translate + .mockRejectedValueOnce(new Error("connection lost")) + .mockResolvedValueOnce({ success: true, translatedVtt: "Spanish" }); + await act(async () => { + expect(await captions.requestTranslation("es")).toBeNull(); + }); + expect(captions.isTranslating).toBe(false); + await act(async () => { + expect(await captions.requestTranslation("es")).toBe("Spanish"); + }); + expect(translate).toHaveBeenCalledTimes(2); +}); +it("does not cache an unsuccessful response", async () => { + translate + .mockResolvedValueOnce({ success: false }) + .mockResolvedValueOnce({ success: true, translatedVtt: "Spanish" }); + await act(async () => { + expect(await captions.requestTranslation("es")).toBeNull(); + }); + expect(captions.isTranslating).toBe(false); + await act(async () => { + expect(await captions.requestTranslation("es")).toBe("Spanish"); + }); + expect(translate).toHaveBeenCalledTimes(2); +}); diff --git a/apps/web/app/s/[videoId]/_components/CaptionContext.tsx b/apps/web/app/s/[videoId]/_components/CaptionContext.tsx index 32de6183d1f..71bac842cde 100644 --- a/apps/web/app/s/[videoId]/_components/CaptionContext.tsx +++ b/apps/web/app/s/[videoId]/_components/CaptionContext.tsx @@ -7,7 +7,9 @@ import { type ReactNode, useCallback, useContext, + useEffect, useMemo, + useRef, useState, } from "react"; import { getAvailableTranslations } from "@/actions/videos/get-available-translations"; @@ -49,6 +51,22 @@ export function CaptionProvider({ children, videoId, transcriptionStatus, +}: CaptionProviderProps) { + return ( + + {children} + + ); +} + +function CaptionProviderForVideo({ + children, + videoId, + transcriptionStatus, }: CaptionProviderProps) { const [selectedLanguage, setSelectedLanguage] = useState("original"); @@ -59,6 +77,16 @@ export function CaptionProvider({ const [originalVttContent, setOriginalVttContent] = useState( null, ); + const mounted = useRef(true); + const translations = useRef(new Map()); + const pending = useRef(new Map>()); + + useEffect(() => { + mounted.current = true; + return () => { + mounted.current = false; + }; + }, []); const { data: availableData, isLoading: isLoadingAvailable } = useQuery({ queryKey: ["availableTranslations", videoId], @@ -81,33 +109,31 @@ export function CaptionProvider({ const hasOriginal = availableData?.hasOriginal ?? false; const requestTranslation = useCallback( - async (language: LanguageCode): Promise => { - return new Promise((resolve) => { - setTranslatedVttContent((prev) => { - const cached = prev.get(language); - if (cached) { - resolve(cached); - return prev; + (language: LanguageCode): Promise => { + const cached = translations.current.get(language); + if (cached) return Promise.resolve(cached); + const existing = pending.current.get(language); + if (existing) return existing; + + const request = Promise.resolve() + .then(() => translateTranscript(videoId, language)) + .then((result) => { + if (!result.success || !result.translatedVtt) return null; + if (mounted.current) { + translations.current.set(language, result.translatedVtt); + setTranslatedVttContent(new Map(translations.current)); } - - setIsTranslating(true); - translateTranscript(videoId, language) - .then((result) => { - if (result.success && result.translatedVtt) { - const vtt = result.translatedVtt; - setTranslatedVttContent((p) => new Map(p).set(language, vtt)); - resolve(vtt); - } else { - resolve(null); - } - }) - .finally(() => { - setIsTranslating(false); - }); - - return prev; + return result.translatedVtt; + }) + .catch(() => null) + .finally(() => { + if (pending.current.get(language) === request) + pending.current.delete(language); + if (mounted.current) setIsTranslating(pending.current.size > 0); }); - }); + pending.current.set(language, request); + if (mounted.current) setIsTranslating(true); + return request; }, [videoId], );