diff --git a/kits/firestore-translate-text/CHANGELOG.md b/kits/firestore-translate-text/CHANGELOG.md index bc1c5b451..293213e86 100644 --- a/kits/firestore-translate-text/CHANGELOG.md +++ b/kits/firestore-translate-text/CHANGELOG.md @@ -1,3 +1,4 @@ +- fix: a `null` input on update now fails the same way as the extension (a `TypeError` from `translateMultiple`, one error event, no write), instead of being routed to `translateSingle`. - fix: `onStart` and `onCompletion` are now published on every invocation, including a write event that arrives without change data; that path used to return before either event was recorded, so subscribers missed a lifecycle pair the extension always emitted. - Construct the translation client once per process instead of on every invocation, matching the legacy extension. This changes the exported `HandlerContext` type from `{ firestore, config, googleAiApiKey? }` to `{ config, service }`: `handleDocumentWrite` no longer builds the `TranslationService` itself and instead expects it on the context (build one with `createTranslationService`) - Initial release of kit, see README for differences between the legacy extension and this kit diff --git a/kits/firestore-translate-text/src/translate/translateDocument.ts b/kits/firestore-translate-text/src/translate/translateDocument.ts index 36e23bd50..78c5887e2 100644 --- a/kits/firestore-translate-text/src/translate/translateDocument.ts +++ b/kits/firestore-translate-text/src/translate/translateDocument.ts @@ -41,7 +41,7 @@ export const translateDocument = async ( return; } - if (typeof input === "object" && input !== null) { + if (typeof input === "object") { return translateMultiple( input as Record, languages, diff --git a/kits/firestore-translate-text/tests/handlers.test.ts b/kits/firestore-translate-text/tests/handlers.test.ts index 659e158f7..80d0c031c 100644 --- a/kits/firestore-translate-text/tests/handlers.test.ts +++ b/kits/firestore-translate-text/tests/handlers.test.ts @@ -342,6 +342,32 @@ describe("handleDocumentWrite", () => { expect(translateClassMethod).not.toHaveBeenCalled(); }); + test("fails without writing when an update sets the input to null", async () => { + const after = makeSnapshot({ input: null }); + + await expect( + handleDocumentWrite( + makeEvent(makeSnapshot({ input: "hello" }), after), + context() + ) + ).resolves.toBeUndefined(); + + expect(logger.error).toHaveBeenCalledWith( + ...messages.error(expect.any(TypeError)) + ); + expect(logger.error).toHaveBeenCalledTimes(1); + expect(events.recordErrorEvent).toHaveBeenCalledWith(expect.any(TypeError)); + expect(events.recordErrorEvent).toHaveBeenCalledTimes(1); + expect(logger.log).not.toHaveBeenCalledWith( + messages.translateInputStringToAllLanguages( + null as never, + defaultLanguages + ) + ); + expect(translateClassMethod).not.toHaveBeenCalled(); + expect(firestore.update).not.toHaveBeenCalled(); + }); + test("skips processing if there is no input on the before and after snapshots", async () => { const snapshot = makeSnapshot({ notTheInput: "hello" }); diff --git a/kits/firestore-translate-text/tests/translate-document.test.ts b/kits/firestore-translate-text/tests/translate-document.test.ts index b668ef92b..f3b5bf052 100644 --- a/kits/firestore-translate-text/tests/translate-document.test.ts +++ b/kits/firestore-translate-text/tests/translate-document.test.ts @@ -18,11 +18,18 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; vi.mock("firebase-functions", () => import("./mocks/firebase-functions")); vi.mock("../src/events"); +vi.mock("../src/translate/translateMultiple", async (importOriginal) => { + const mod = await importOriginal< + typeof import("../src/translate/translateMultiple") + >(); + return { ...mod, translateMultiple: vi.fn(mod.translateMultiple) }; +}); import * as events from "../src/events"; import { messages } from "../src/logs/messages"; import type { TranslationService } from "../src/translate"; import { translateDocument } from "../src/translate/translateDocument"; +import { translateMultiple } from "../src/translate/translateMultiple"; import { translateSingle } from "../src/translate/translateSingle"; import { defaultLanguages, @@ -104,16 +111,22 @@ describe("translateDocument", () => { ); }); - test("treats a null input as a single translation, uncoerced", async () => { + test("routes a null input through translateMultiple, as the extension does", async () => { const snapshot = makeSnapshot({ input: null }); + const service = makeService({ extractLanguages: vi.fn(() => ["en"]) }); - await translateDocument( + await expect( + translateDocument(snapshot, service, makeConfig()) + ).rejects.toThrow(TypeError); + + expect(translateMultiple).toHaveBeenCalledWith( + null, + ["en"], snapshot, - makeService({ extractLanguages: vi.fn(() => ["en"]) }), - makeConfig() + service ); - - expect(translateString).toHaveBeenCalledWith(null, "en"); + expect(translateString).not.toHaveBeenCalled(); + expect(updateTranslations).not.toHaveBeenCalled(); }); test("exits early when the input field is a translation output path", async () => {