fix(firestore-translate-text): emit start and completion events on every write

kits/firestore-translate-text/CHANGELOG.md

@@ -1,3 +1,4 @@
+- 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
 - The Google AI API key (and any other secret-shaped config value) is now masked as `<omitted>` in the config logged at startup and on each invocation; the legacy extension logs it in cleartext

kits/firestore-translate-text/src/handlers.ts

@@ -54,15 +54,16 @@ export async function handleDocumentWrite(
   event: TranslateWriteEvent,
   ctx: HandlerContext
 ): Promise<void> {
-  if (!event.data) {
-    return;
-  }
-
   const { config, service } = ctx;
 
   logs.start(config);
   await events.recordStartEvent({ data: event.data, params: event.params });
 
+  if (!event.data) {
+    await events.recordCompletionEvent({ params: event.params });
+    return;
+  }
+
   const { languages, inputFieldName, outputFieldName } = config;
 
   if (validators.fieldNamesMatch(inputFieldName, outputFieldName)) {

kits/firestore-translate-text/tests/handlers.test.ts

@@ -83,13 +83,21 @@ describe("handleDocumentWrite", () => {
   });
 
   test("skips events without change data", async () => {
+    const event = makeEvent(undefined, undefined);
+
     await expect(
-      handleDocumentWrite(makeEvent(undefined, undefined), context())
+      handleDocumentWrite(event, context())
     ).resolves.toBeUndefined();
 
     expect(translateClassMethod).not.toHaveBeenCalled();
     expect(firestore.update).not.toHaveBeenCalled();
-    expect(events.recordStartEvent).not.toHaveBeenCalled();
+    expect(events.recordStartEvent).toHaveBeenCalledWith({
+      data: undefined,
+      params: event.params,
+    });
+    expect(events.recordCompletionEvent).toHaveBeenCalledWith({
+      params: event.params,
+    });
   });
 
   test("records start and completion events", async () => {
@@ -391,4 +399,183 @@ describe("handleDocumentWrite", () => {
     );
     expect(JSON.stringify(logger.log.mock.calls)).not.toContain("super-secret");
   });
+
+  /**
+   * Every invocation of the extension's `fstranslate` publishes `onStart`
+   * first and `onCompletion` last, whichever branch it takes in between; the
+   * early returns are not exempt. Each test here pins the full event sequence
+   * for one branch.
+   */
+  describe("lifecycle events", () => {
+    const EVENT_SPIES = {
+      start: events.recordStartEvent,
+      success: events.recordSuccessEvent,
+      error: events.recordErrorEvent,
+      completion: events.recordCompletionEvent,
+    } as const;
+
+    function publishedEvents(): Array<keyof typeof EVENT_SPIES> {
+      return Object.entries(EVENT_SPIES)
+        .flatMap(([name, spy]) =>
+          vi
+            .mocked(spy)
+            .mock.invocationCallOrder.map(
+              (order) => [name as keyof typeof EVENT_SPIES, order] as const
+            )
+        )
+        .sort(([, a], [, b]) => a - b)
+        .map(([name]) => name);
+    }
+
+    test("start then completion when the event carries no change", async () => {
+      await handleDocumentWrite(makeEvent(undefined, undefined), context());
+
+      expect(publishedEvents()).toEqual(["start", "completion"]);
+    });
+
+    test("start then completion when the input and output fields match", async () => {
+      await handleDocumentWrite(
+        makeEvent(makeSnapshot(), makeSnapshot({ input: "hello" })),
+        context({ inputFieldName: "input", outputFieldName: "input" })
+      );
+
+      expect(publishedEvents()).toEqual(["start", "completion"]);
+    });
+
+    test("start then completion when the input field is a configured translation path", async () => {
+      await handleDocumentWrite(
+        makeEvent(makeSnapshot(), makeSnapshot({ input: "hello" })),
+        context({
+          inputFieldName: "translated.en",
+          outputFieldName: "translated",
+        })
+      );
+
+      expect(publishedEvents()).toEqual(["start", "completion"]);
+    });
+
+    test("start then completion when the document's languages make the input field a translation path", async () => {
+      await handleDocumentWrite(
+        makeEvent(
+          makeSnapshot(),
+          makeSnapshot({ translated: { fr: "hello" }, langs: ["fr"] })
+        ),
+        context({
+          inputFieldName: "translated.fr",
+          outputFieldName: "translated",
+          languages: "en",
+        })
+      );
+
+      expect(publishedEvents()).toEqual(["start", "completion"]);
+      expect(firestore.update).not.toHaveBeenCalled();
+    });
+
+    test("start, success, completion when a document is created with input", async () => {
+      await handleDocumentWrite(
+        makeEvent(makeSnapshot(), makeSnapshot({ input: "hello" })),
+        context()
+      );
+
+      expect(publishedEvents()).toEqual(["start", "success", "completion"]);
+    });
+
+    test("start then completion when a document is created without input", async () => {
+      await handleDocumentWrite(
+        makeEvent(makeSnapshot(), makeSnapshot({ changed: 123 })),
+        context()
+      );
+
+      expect(publishedEvents()).toEqual(["start", "completion"]);
+    });
+
+    test("start then completion when a document is deleted", async () => {
+      await handleDocumentWrite(
+        makeEvent(
+          makeSnapshot({ input: "hello" }),
+          makeSnapshot({ input: "hello" }, { exists: false })
+        ),
+        context()
+      );
+
+      expect(publishedEvents()).toEqual(["start", "completion"]);
+    });
+
+    test("start then completion when neither snapshot has input", async () => {
+      const snapshot = makeSnapshot({ notTheInput: "hello" });
+
+      await handleDocumentWrite(makeEvent(snapshot, snapshot), context());
+
+      expect(publishedEvents()).toEqual(["start", "completion"]);
+    });
+
+    test("start, success, completion when the input field is removed", async () => {
+      await handleDocumentWrite(
+        makeEvent(
+          makeSnapshot({ input: "hello" }),
+          makeSnapshot({}, { exists: true })
+        ),
+        context()
+      );
+
+      expect(publishedEvents()).toEqual(["start", "success", "completion"]);
+    });
+
+    test("start then completion when the input is unchanged", async () => {
+      await handleDocumentWrite(
+        makeEvent(
+          makeSnapshot({ input: "hello" }),
+          makeSnapshot({ input: "hello", changed: 123 })
+        ),
+        context()
+      );
+
+      expect(publishedEvents()).toEqual(["start", "completion"]);
+    });
+
+    test("start, success, completion when the input changes", async () => {
+      await handleDocumentWrite(
+        makeEvent(
+          makeSnapshot({ input: "goodbye" }),
+          makeSnapshot({ input: "hello" })
+        ),
+        context()
+      );
+
+      expect(publishedEvents()).toEqual(["start", "success", "completion"]);
+    });
+
+    test("start, one error per layer, completion when a string translation fails", async () => {
+      translateClassMethod.mockRejectedValueOnce(new Error("boom"));
+
+      await handleDocumentWrite(
+        makeEvent(makeSnapshot(), makeSnapshot({ input: "hello" })),
+        context()
+      );
+
+      expect(publishedEvents()).toEqual([
+        "start",
+        "error",
+        "error",
+        "error",
+        "completion",
+      ]);
+    });
+
+    test("start, one error per layer, completion when a map translation fails", async () => {
+      translateClassMethod.mockRejectedValueOnce(new Error("boom"));
+
+      await handleDocumentWrite(
+        makeEvent(makeSnapshot(), makeSnapshot({ input: { one: "hello" } })),
+        context()
+      );
+
+      expect(publishedEvents()).toEqual([
+        "start",
+        "error",
+        "error",
+        "completion",
+      ]);
+    });
+  });
 });