From 17f9c1623953abbe05d0a9acdb0c224c6dc1d30e Mon Sep 17 00:00:00 2001 From: Siarhei Date: Fri, 23 Oct 2026 15:08:38 +0200 Subject: [PATCH] feat!: provenance becomes an ordinary module on a run seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The translation handler wired provenance in by name: four blocks guarded with `provenance?.` resolved the service, built its key, captured fingerprints, computed drift and wrote the receipt. Nothing else could join a translation run without editing that handler again. A run now has two positions any module can occupy — before the translation, and after the document is saved. Provenance is the first to take them, with no privileges. A module returns only its contribution rather than the whole run state; the chain merges `moduleState` by key, so a module cannot drop a slot it does not own, and a module that fails contributes nothing instead of ending the run. Module names are refused at boot when two collide, because a name keys the module's slot in that state. The config-time half follows. `levels` is replaced by `modules` and `components`, separating what wires server routes from what attaches an admin control. The document and collection levels registered the same route bundle — the only reason endpoints were deduplicated at all — and once the controls moved out they were indistinguishable and collapsed into one module. Two further changes ride along: - An address absent from a stored receipt now means "translate" rather than "cannot tell". A field whose source was emptied and later refilled is re-translated instead of being left alone forever. - Route factories and the shared config context no longer accept the absence of an access guard. The guarantee is pinned with `@ts-expect-error`, so a route that forgets one fails the type-check rather than serving unguarded. Closes #198 BREAKING CHANGE: the `levels` plugin option is removed, along with `documentLevel()`, `collectionLevel()`, `fieldLevel()` and the `TranslationLevel` type. Replace `levels: [documentLevel(), collectionLevel()]` with `modules: [documentTranslation()]` and `components: [translateDocumentControl(), bulkTranslationDashboard()]`; `levels: [fieldLevel()]` becomes `modules: [fieldTranslation()]`. Both lists are required and either may be empty — an omitted option used to mean two surfaces silently, and now fails at boot with the replacement in the message. See docs/DEPRECATIONS.md for the full table. --- apps/cms/src/lib/plugins/index.ts | 5 + .../integration/translator/bootTestPayload.ts | 10 +- .../translator/staleness-refresh.int.test.ts | 24 +- apps/dev/src/payload.config.ts | 10 +- packages/payload-plugin-translator/README.md | 50 ++- .../docs/DEPRECATIONS.md | 48 +++ .../2026-10-22-provenance-as-a-module.task.md | 343 ++++++++++++++++++ ...-10-23-the-seam-says-what-it-means.task.md | 206 +++++++++++ .../2026-10-23-tidying-the-module-tree.md | 123 +++++++ .../src/__tests__/plugin.access.test.ts | 2 + .../src/__tests__/plugin.test.ts | 107 +++++- .../api/queries/useDocumentStaleness.ts | 26 +- .../ui/TranslateFieldControl.tsx | 23 +- .../components/bulkTranslationDashboard.ts | 19 + .../src/composition/components/index.ts | 2 + .../components/translateDocumentControl.ts | 20 + .../src/composition/levels/collectionLevel.ts | 29 -- .../src/composition/levels/documentLevel.ts | 34 -- .../src/composition/levels/fieldLevel.test.ts | 37 -- .../src/composition/levels/fieldLevel.ts | 42 --- .../src/composition/levels/index.ts | 3 - .../src/composition/levels/levels.test.ts | 72 ---- .../levels/useDocTranslationApi.ts | 24 -- .../modules/documentTranslation.ts | 25 ++ .../composition/modules/fieldTranslation.ts | 25 ++ .../src/composition/modules/index.ts | 2 + .../domain/provenance/SourceFingerprint.ts | 7 +- .../provenance/__tests__/theWatermark.test.ts | 36 +- .../src/core/domain/provenance/index.ts | 2 +- .../core/domain/provenance/staleness.test.ts | 45 +-- .../src/core/domain/provenance/staleness.ts | 42 +-- .../theLeafThatNeverCameBack.test.ts | 2 +- .../src/field-config.ts | 2 +- .../payload-plugin-translator/src/index.ts | 8 +- .../payload-plugin-translator/src/plugin.ts | 74 +++- .../cancel-by-collection/route.test.ts | 11 +- .../features/cancel-by-collection/route.ts | 2 +- .../src/server/features/cancel/route.test.ts | 6 +- .../src/server/features/cancel/route.ts | 2 +- .../features/createTranslationRoutes.test.ts | 5 +- .../features/createTranslationRoutes.ts | 26 +- .../__tests__/route.test.ts | 6 +- .../features/enqueue-translation/route.ts | 2 +- .../get-collection-status/route.test.ts | 8 +- .../features/get-collection-status/route.ts | 2 +- .../get-document-status/route.test.ts | 11 +- .../features/get-document-status/route.ts | 2 +- .../features/run-translation/route.test.ts | 6 +- .../server/features/run-translation/route.ts | 2 +- .../staleness/dismissStaleness.handler.ts | 4 +- .../staleness/getDocumentStaleness.handler.ts | 6 +- .../src/server/features/staleness/model.ts | 10 +- .../server/features/staleness/route.test.ts | 10 +- .../src/server/features/staleness/route.ts | 4 +- .../features/staleness/staleness.test.ts | 11 +- .../__tests__/handler.access.test.ts | 1 + .../__tests__/handler.enforce.test.ts | 11 +- .../__tests__/handler.moduleFailure.test.ts | 98 +++++ .../__tests__/handler.spend.test.ts | 11 +- .../__tests__/handler.test.ts | 14 +- .../__tests__/handler.transaction.test.ts | 14 +- .../features/translate-document/handler.ts | 57 +-- .../features/translate-document/model.ts | 1 + .../translate-document/wireTranslateRunner.ts | 13 +- .../translate-field/__tests__/route.test.ts | 4 + .../server/features/translate-field/route.ts | 2 +- .../PluginConfigBuilder.test.ts | 5 +- .../PluginConfigBuilder.ts | 42 +-- .../src/server/modules/plugin-config/index.ts | 6 + .../src/server/modules/plugin-config/types.ts | 61 ++++ .../modules/provenance/Provenance.module.ts | 90 +++++ .../modules/provenance/Provenance.wiring.ts | 64 ++-- .../__tests__/Provenance.module.test.ts | 123 +++++++ .../provenance/__tests__/fixtures/inAChain.ts | 15 + .../src/server/modules/provenance/index.ts | 7 +- .../TaskRunnerProvider.interface.ts | 2 + .../PayloadJobsRunnerProvider.ts | 14 +- .../__tests__/attemptCount.test.ts | 63 ++++ .../model/attemptsBefore.ts | 7 + .../payload-jobs-runner/model/index.ts | 1 + .../model/isLastAttempt.ts | 5 +- .../payload-jobs-runner/store/index.ts | 8 +- .../payload-jobs-runner/store/types.ts | 4 +- .../task-runner/sync-runner/SyncTaskRunner.ts | 3 + .../__tests__/SyncTaskRunner.test.ts | 1 + .../modules/translation-levels/index.ts | 6 - .../modules/translation-levels/types.ts | 68 ---- .../TranslationRun.interface.ts | 57 +++ .../__tests__/chainOrder.test.ts | 117 ++++++ .../__tests__/fixtures/failures.ts | 10 + .../translation-run/__tests__/fixtures/run.ts | 36 ++ .../__tests__/moduleFailure.test.ts | 185 ++++++++++ .../__tests__/seamShape.pins.ts | 29 ++ .../server/modules/translation-run/index.ts | 7 + .../modules/translation-run/runChain.ts | 69 ++++ .../__tests__/accessIsNotOptional.pins.ts | 25 ++ .../http/__tests__/withAccessCheck.test.ts | 8 - .../src/server/shared/http/withAccessCheck.ts | 9 +- .../src/types/ConfigModifier.ts | 8 +- 99 files changed, 2345 insertions(+), 701 deletions(-) create mode 100644 packages/payload-plugin-translator/docs/plans/2026-10-22-provenance-as-a-module.task.md create mode 100644 packages/payload-plugin-translator/docs/plans/2026-10-23-the-seam-says-what-it-means.task.md create mode 100644 packages/payload-plugin-translator/docs/plans/2026-10-23-tidying-the-module-tree.md create mode 100644 packages/payload-plugin-translator/src/composition/components/bulkTranslationDashboard.ts create mode 100644 packages/payload-plugin-translator/src/composition/components/index.ts create mode 100644 packages/payload-plugin-translator/src/composition/components/translateDocumentControl.ts delete mode 100644 packages/payload-plugin-translator/src/composition/levels/collectionLevel.ts delete mode 100644 packages/payload-plugin-translator/src/composition/levels/documentLevel.ts delete mode 100644 packages/payload-plugin-translator/src/composition/levels/fieldLevel.test.ts delete mode 100644 packages/payload-plugin-translator/src/composition/levels/fieldLevel.ts delete mode 100644 packages/payload-plugin-translator/src/composition/levels/index.ts delete mode 100644 packages/payload-plugin-translator/src/composition/levels/levels.test.ts delete mode 100644 packages/payload-plugin-translator/src/composition/levels/useDocTranslationApi.ts create mode 100644 packages/payload-plugin-translator/src/composition/modules/documentTranslation.ts create mode 100644 packages/payload-plugin-translator/src/composition/modules/fieldTranslation.ts create mode 100644 packages/payload-plugin-translator/src/composition/modules/index.ts create mode 100644 packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.moduleFailure.test.ts rename packages/payload-plugin-translator/src/server/modules/{translation-levels => plugin-config}/PluginConfigBuilder.test.ts (97%) rename packages/payload-plugin-translator/src/server/modules/{translation-levels => plugin-config}/PluginConfigBuilder.ts (74%) create mode 100644 packages/payload-plugin-translator/src/server/modules/plugin-config/index.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/plugin-config/types.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/provenance/Provenance.module.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/provenance/__tests__/Provenance.module.test.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/provenance/__tests__/fixtures/inAChain.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/__tests__/attemptCount.test.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/model/attemptsBefore.ts delete mode 100644 packages/payload-plugin-translator/src/server/modules/translation-levels/index.ts delete mode 100644 packages/payload-plugin-translator/src/server/modules/translation-levels/types.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/translation-run/TranslationRun.interface.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/chainOrder.test.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/fixtures/failures.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/fixtures/run.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/moduleFailure.test.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/seamShape.pins.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/translation-run/index.ts create mode 100644 packages/payload-plugin-translator/src/server/modules/translation-run/runChain.ts create mode 100644 packages/payload-plugin-translator/src/server/shared/access/__tests__/accessIsNotOptional.pins.ts diff --git a/apps/cms/src/lib/plugins/index.ts b/apps/cms/src/lib/plugins/index.ts index f63d03e65..6f92f70ba 100644 --- a/apps/cms/src/lib/plugins/index.ts +++ b/apps/cms/src/lib/plugins/index.ts @@ -6,6 +6,9 @@ import { schedulePublicationPlugin } from "@focus-reactive/payload-plugin-schedu import { seoPlugin as seoAnalysisPlugin } from "@focus-reactive/payload-plugin-seo"; import { translatorPlugin, + documentTranslation, + translateDocumentControl, + bulkTranslationDashboard, createOpenAIProvider, createSyncRunner, } from "@focus-reactive/payload-plugin-translator"; @@ -347,6 +350,8 @@ export const plugins: Plugin[] = [ ), access: { check: ({ req }) => Boolean(req.user) }, runner: createSyncRunner(), + modules: [documentTranslation()], + components: [translateDocumentControl(), bulkTranslationDashboard()], translationProvider: createOpenAIProvider({ apiKey: process.env.OPENAI_API_KEY!, dryRun: false, diff --git a/apps/dev/src/integration/translator/bootTestPayload.ts b/apps/dev/src/integration/translator/bootTestPayload.ts index dffcce300..10cf4886b 100644 --- a/apps/dev/src/integration/translator/bootTestPayload.ts +++ b/apps/dev/src/integration/translator/bootTestPayload.ts @@ -7,8 +7,9 @@ import { AnyAccessGuard, createTranslationProvider, createSyncRunner, - documentLevel, - fieldLevel, + documentTranslation, + fieldTranslation, + translateDocumentControl, translatorPlugin, withAutoTranslate, } from "@focus-reactive/payload-plugin-translator"; @@ -157,7 +158,10 @@ export async function bootTestPayload(opts?: { access: new AnyAccessGuard(), translationProvider: countingProvider, runner: opts?.runner ?? createSyncRunner(), - levels: opts?.fieldSurface ? [documentLevel(), fieldLevel()] : [documentLevel()], + modules: opts?.fieldSurface + ? [documentTranslation(), fieldTranslation()] + : [documentTranslation()], + components: [translateDocumentControl()], provenance: true, ...(opts?.lifecycle ? { lifecycle: opts.lifecycle } : {}), ...(opts?.failedWindowMs === undefined ? {} : { failedWindowMs: opts.failedWindowMs }), diff --git a/apps/dev/src/integration/translator/staleness-refresh.int.test.ts b/apps/dev/src/integration/translator/staleness-refresh.int.test.ts index 05a291619..6a8438715 100644 --- a/apps/dev/src/integration/translator/staleness-refresh.int.test.ts +++ b/apps/dev/src/integration/translator/staleness-refresh.int.test.ts @@ -109,7 +109,9 @@ describe("a translation refreshes the leaf whose source moved, and only that one await translate(id, "skip_existing"); const de = await german(id); - expect(de.title).toBe("HAND-WRITTEN"); + expect(de.title, "no receipt says nothing moved, so skip_existing keeps its promise").toBe( + "HAND-WRITTEN" + ); expect(de.note, "empty, so it is filled in as it always was").toBe(tr("de", "No receipt note")); }); @@ -201,7 +203,7 @@ describe("a translation refreshes the leaf whose source moved, and only that one ]); }); - it("leaves a filled-in target alone when a receipt exists but has no entry for that leaf", async () => { + it("translates a leaf the receipt has no entry for", async () => { const id = await create({ title: "Partial receipt", note: "" }); await translate(id, "overwrite"); @@ -216,11 +218,25 @@ describe("a translation refreshes the leaf whose source moved, and only that one await translate(id, "skip_existing"); const de = await german(id); - expect(de.note, "the receipt says nothing about this leaf, so nothing is claimed").toBe( - "HAND-WRITTEN NOTE" + expect(de.note, "the receipt never saw this leaf, so it is owed a translation").toBe( + tr("de", "A note exists now") ); }); + it("translates a leaf again after its source was emptied and later refilled", async () => { + const id = await create({ title: "Refilled", note: "The first note" }); + await translate(id, "overwrite"); + expect((await german(id)).note).toBe(tr("de", "The first note")); + + await editSource(id, { title: "Refilled", note: "" }); + await translate(id, "overwrite"); + + await editSource(id, { title: "Refilled", note: "The note is back" }); + await translate(id, "skip_existing"); + + expect((await german(id)).note).toBe(tr("de", "The note is back")); + }); + // Fingerprinting the target instead would tell a human's text from ours, but a Payload round-trip // can rewrite the stored value, so every leaf would read as hand-edited and refreshes would stop // silently. diff --git a/apps/dev/src/payload.config.ts b/apps/dev/src/payload.config.ts index 61b5dc131..2d9d138ab 100644 --- a/apps/dev/src/payload.config.ts +++ b/apps/dev/src/payload.config.ts @@ -7,9 +7,10 @@ import { presetsPlugin } from "@focus-reactive/payload-plugin-presets"; import { schedulePublicationPlugin } from "@focus-reactive/payload-plugin-scheduling"; import { translatorPlugin, - documentLevel, - collectionLevel, - fieldLevel, + documentTranslation, + fieldTranslation, + translateDocumentControl, + bulkTranslationDashboard, withAutoTranslate, } from "@focus-reactive/payload-plugin-translator"; import { analyticsPlugin } from "@focus-reactive/payload-plugin-analytics"; @@ -100,7 +101,8 @@ export default buildConfig({ access: { check: ({ req }) => Boolean(req.user) }, runner: resolveTranslatorRunner(), translationProvider: resolveTranslationProvider(), - levels: [documentLevel(), collectionLevel(), fieldLevel()], + modules: [documentTranslation(), fieldTranslation()], + components: [translateDocumentControl(), bulkTranslationDashboard()], provenance: true, lifecycle: loggingLifecycle, }), diff --git a/packages/payload-plugin-translator/README.md b/packages/payload-plugin-translator/README.md index 4bc45dc74..80d37e106 100644 --- a/packages/payload-plugin-translator/README.md +++ b/packages/payload-plugin-translator/README.md @@ -92,14 +92,14 @@ npx payload generate:importmap Open a `posts` document. A Translate control now sits beside the document controls: choose `en` as the source and `de` as the target, and the German locale is filled in within a minute. > [!TIP] -> Rerun `payload generate:importmap` after adding or removing `documentLevel()` or `collectionLevel()`, or the admin renders without their controls. `fieldLevel()` adds no admin component, so it needs no regeneration. +> Rerun `payload generate:importmap` after changing `components`, or the admin renders without the controls you listed. `modules` adds no admin component, so it needs no regeneration. ## What you can do - **Translate one document** — the control on the edit view queues a job for the target locale you pick, choosing a strategy and whether to publish the locale once it is filled in. - **Translate a whole collection** — a dashboard above the list view runs the selected documents, or all of them, and reports progress per locale. - **Translate into several locales at once** — `targetSelection: 'multi'` turns the target field on both the document and the collection surfaces into a multi-select, and every selected document is translated into every locale you picked. The default is one locale per run. -- **Translate a single field** — a control on the field itself, answering in place without queueing anything. Needs both `fieldLevel()` in `levels` and `withFieldTranslation` on the field. +- **Translate a single field** — a control on the field itself, answering in place without queueing anything. Needs both `fieldTranslation()` in `modules` and `withFieldTranslation` on the field. - **Auto-translate on save** — wrap a collection in `withAutoTranslate(collection, { targets })` and editing the source locale queues its own translations, with no one pressing anything. On a collection with drafts enabled it waits until the document is published; without drafts it fires on every save. Rapid edits are coalesced into one run. - **Keep a field out of translation** — `withFieldTranslation(field, { exclude: true })` is the one way to say a field is never sent to the provider. `skip_existing` is about emptiness and drift, not authorship: it fills an empty target, and — with `provenance` on, for a locale the plugin has translated at least once — refreshes a target whose source has moved since that run, whoever wrote the text that is there. - **Steer how it translates** — `systemPrompt` replaces the instruction sent with every request, so tone, register, a glossary, or brand names that must stay untranslated are yours to set. @@ -155,37 +155,52 @@ translatorPlugin(config: TranslatorPluginConfig) | `collections` | `CollectionConfig[]` | — | **Required.** Original collection configs to make translatable. | | `translationProvider` | `TranslationProvider` | — | **Required.** What actually translates the text. | | `runner` | `TaskRunnerProvider` | — | **Required.** What runs the translation — in the background, or inline. | -| `levels` | `TranslationLevel[]` | `[documentLevel(), collectionLevel()]` | Which translation surfaces to enable. | +| `modules` | `TranslationModule[]` | — | **Required.** Which server surfaces to enable. `[]` is allowed. | +| `components` | `TranslationComponent[]` | — | **Required.** Which admin controls to attach. `[]` is allowed. | | `access` | `AccessGuard` | — | **Required.** Who may call the translation endpoints. `new AnyAccessGuard()` leaves them open on purpose. | | `basePath` | `string` | `'/translate'` | Base path for the plugin's endpoints. | | `targetSelection` | `'single' \| 'multi'` | `'single'` | `'multi'` lets an editor pick several target locales for one run. _Since v0.10.0._ | | `provenance` | `boolean \| { slug?: string }` | `false` | Adds a collection recording what each locale was translated from, so the admin can flag stale ones. Default slug `translator-provenance`. | | `lifecycle` | `TranslationLifecycleCallbacks` | `{}` | Server-side `onQueued` / `onCompleted` / `onFailed` callbacks. | -### Translation levels +### Modules and components -_Since v0.5.0; `fieldLevel()` since v0.6.0._ +_Since v1.0.0; replaces `levels`._ -Levels are the surfaces the plugin adds. List the ones you want; each should appear at most once. +A **module** is server wiring — routes, and the collections behind them. A **component** is an +admin control, which runs in the browser and calls a module. Both lists are required, and `[]` is a +real answer; each entry should appear at most once. -| Level | What it adds | +| Module | What it adds | | --- | --- | -| `documentLevel()` | A Translate popup on the document edit view. | -| `collectionLevel()` | A bulk translation dashboard above the list table. | -| `fieldLevel()` | Turns on the per-field control. Not in the default set. | +| `documentTranslation()` | The job API: queue a run, run it now, cancel it, read its progress. | +| `fieldTranslation()` | The synchronous `POST /field` endpoint. | + +| Component | What it adds | +| --- | --- | +| `translateDocumentControl()` | A Translate popup on the document edit view. | +| `bulkTranslationDashboard()` | A bulk translation dashboard above the list table. | ```ts -import { documentLevel, collectionLevel, fieldLevel } from '@focus-reactive/payload-plugin-translator' +import { + documentTranslation, + fieldTranslation, + translateDocumentControl, + bulkTranslationDashboard, +} from '@focus-reactive/payload-plugin-translator' translatorPlugin({ collections: [Posts], translationProvider, runner, - levels: [documentLevel(), collectionLevel(), fieldLevel()], + modules: [documentTranslation(), fieldTranslation()], + components: [translateDocumentControl(), bulkTranslationDashboard()], }) ``` -`fieldLevel()` on its own shows nothing: the control appears only on the fields you wrap with `withFieldTranslation`. +Both controls call `documentTranslation()`, so listing one without it leaves it calling nothing. +`fieldTranslation()` shows nothing by itself: its control appears only on the fields you wrap with +`withFieldTranslation`. ### Per-field control and exclusion @@ -261,7 +276,7 @@ The source is the source locale's **own current content** — the newer draft wh Two consequences worth knowing before relying on it: - **Publishing publishes the current draft, whatever is in it.** That includes pending edits nobody made for the translation's sake, and non-localized fields, which Payload stores once per document and cannot scope to a locale. Worth remembering before running translate-and-publish over a long list: every unpublished draft among them goes live. -- **`skip_existing` refreshes on drift, and does not ask who wrote the target.** With `provenance` on and a receipt already written for that locale, it leaves a non-empty target alone while its own source text is unchanged, and re-translates it once that source moves — including text a person typed or corrected. Without `provenance`, or before the plugin has translated the locale once, it has nothing to compare against and fills only empty targets. It has no notion of *reviewed*; `withFieldTranslation(field, { exclude: true })` is what keeps a field out of a run altogether. +- **`skip_existing` refreshes on drift, and does not ask who wrote the target.** With `provenance` on and a receipt already written for that locale, it leaves a non-empty target alone while its own source text is unchanged, and re-translates it once that source moves — including text a person typed or corrected. A field the record does not name — one added later, or emptied and since refilled — counts as moved, because a run records the source as it stood and an empty field is not in it. Without `provenance`, or before the plugin has translated the locale once, it has nothing to compare against and fills only empty targets. It has no notion of *reviewed*; `withFieldTranslation(field, { exclude: true })` is what keeps a field out of a run altogether. ## Translation providers @@ -445,7 +460,7 @@ Three endpoints are meant for you. They mount under your Payload API route plus | --- | --- | --- | | `POST` | `/enqueue` | Start translations from your own code — a migration, an import, any script. | | `POST` | `/run/:id` | Run a queued job now. Required wherever cron autorun does not fire, such as Vercel. | -| `POST` | `/field` | Translate one field and get the result back. Mounted only when `levels` includes `fieldLevel()`. | +| `POST` | `/field` | Translate one field and get the result back. Mounted only when `modules` includes `fieldTranslation()`. | `POST /enqueue` takes `source_lng`, `target_lng` (one locale or a list), `collection_slug` and `collection_id[]`, plus optional `select_all`, `strategy` (`'overwrite' | 'skip_existing'`, default `'overwrite'`) and `publish_on_translation` (default `false`). Field names are snake_case. @@ -457,7 +472,8 @@ Three endpoints are meant for you. They mount under your Payload API route plus | Export | Kind | Purpose | | --- | --- | --- | | `translatorPlugin` | function | The plugin itself. | -| `documentLevel`, `collectionLevel`, `fieldLevel` | functions | Translation surfaces for `levels`. | +| `documentTranslation`, `fieldTranslation` | functions | Server surfaces for `modules`. | +| `translateDocumentControl`, `bulkTranslationDashboard` | functions | Admin controls for `components`. | | `withFieldTranslation` | function | Attach a per-field control, or exclude a field. | | `withAutoTranslate` | function | Opt a collection into auto-translation. | | `createTranslationProvider` | function | Build a provider from one `complete` function. | @@ -471,7 +487,7 @@ Three endpoints are meant for you. They mount under your Payload API route plus | `SystemPromptBuilder`, `SystemPromptContext`, `JsonSchemaObject` | types | Prompt and schema customization. | | `OpenAIClientShape`, `OpenAISamplingParams`, `OpenAIStructuredOutput` | types | OpenAI adapter. | | `TaskRunnerProvider`, `PayloadJobsRunnerOptions`, `TaskFilter` | types | Task runners. | -| `TranslationLevel`, `TargetSelectionMode` | types | Surfaces and target selection. | +| `TargetSelectionMode` | type | Single or multi target-language selection. | | `AutoTranslateConfig`, `AutoTranslateStrategy`, `FieldTranslationConfig` | types | Collection and field configuration. | | `TranslationTask`, `TranslationLifecycleCallbacks` | types | Lifecycle callbacks. | | `TranslationProvenanceRecord` | type | One stored provenance record. | diff --git a/packages/payload-plugin-translator/docs/DEPRECATIONS.md b/packages/payload-plugin-translator/docs/DEPRECATIONS.md index 142d097de..265a23e02 100644 --- a/packages/payload-plugin-translator/docs/DEPRECATIONS.md +++ b/packages/payload-plugin-translator/docs/DEPRECATIONS.md @@ -392,3 +392,51 @@ run. Per-document, per-field control arrives with the field picker (#48). taken as *revisit this leaf*, and the first run that translates that leaf records a hash in its place. A leaf whose source is empty is never sent, so its `null` is carried forward and the locale keeps flagging until the source has text again. + +### levels-become-modules-and-components + +- **What:** the `levels` plugin option and the four names it needed — `documentLevel()`, + `collectionLevel()`, `fieldLevel()` and the `TranslationLevel` type. +- **Status:** removed +- **Changed:** 2026-10-22 / #198 +- **Replacement:** two required lists, `modules` and `components`. Either may be `[]`. + + | Was | Becomes | + | --- | --- | + | `levels: [documentLevel()]` | `modules: [documentTranslation()]` · `components: [translateDocumentControl()]` | + | `levels: [collectionLevel()]` | `modules: [documentTranslation()]` · `components: [bulkTranslationDashboard()]` | + | `levels: [fieldLevel()]` | `modules: [fieldTranslation()]` | + | omitted (the old default) | `modules: [documentTranslation()]` · `components: [translateDocumentControl(), bulkTranslationDashboard()]` | + +- **Why:** a level was a server surface and an admin control welded together, and the two have + nothing to do with each other — a control runs in the browser and is loaded by path, while the + routes behind it are server wiring. Separating them is what lets provenance, which contributes + routes and a collection but no control, be listed the same way everything else is. The document + and collection levels registered the *same* route bundle, which is the only reason endpoints are + deduplicated at all; once the controls moved out, the two were indistinguishable and collapsed + into one module. +- **No default:** an omitted option used to mean two surfaces, silently. Both lists are now + required, and omitting either fails at boot with the table above in the message. `[]` is a real + answer: auto-translate runs from a save hook and needs no route. +- **Who this breaks:** every host. A plugin config that sets `levels`, or relies on its default, + does not compile and does not boot. +- **Code refs:** + - `src/plugin.ts` (`modules`, `components`, and the refusal) + - `src/composition/modules/`, `src/composition/components/` + - `src/server/modules/plugin-config/types.ts` (`TranslationModule`, `TranslationComponent`) + +### runner-context-carries-the-attempt-count + +- **What:** `TaskHandlerInput` gains a required `previousAttempts: number`. +- **Status:** removed (the old shape) +- **Changed:** 2026-10-22 / #198 +- **Replacement:** a runner supplies the number of times this locale was tried before the attempt + now running — zero on the first, and zero always for a runner that never retries. +- **Why:** a module joining a translation run may need to behave differently on a replay, and the + count already exists on the job row. Reading it in the runner, where it lives, is cheaper and + more honest than every module guessing. +- **Who this breaks:** only a `TaskRunnerProvider` implemented outside this package. The built-in + runners both supply it; a host that merely configures the plugin sees no change. +- **Code refs:** + - `src/server/modules/task-runner/TaskRunnerProvider.interface.ts` + - `src/server/modules/task-runner/payload-jobs-runner/model/attemptsBefore.ts` diff --git a/packages/payload-plugin-translator/docs/plans/2026-10-22-provenance-as-a-module.task.md b/packages/payload-plugin-translator/docs/plans/2026-10-22-provenance-as-a-module.task.md new file mode 100644 index 000000000..57c19e460 --- /dev/null +++ b/packages/payload-plugin-translator/docs/plans/2026-10-22-provenance-as-a-module.task.md @@ -0,0 +1,343 @@ +# Provenance becomes an ordinary tenant + +**Status:** in progress · **Risk:** high · **Issue:** #198 · **Step 4 of** `2026-10-18-the-rewiring.md` + +## Requirements / Task restatement + +`translate-document/handler.ts:112-151` knows provenance by name. It resolves the service, builds +its key, captures fingerprints, computes the drift map and writes the receipt, with four +`provenance?.` guards for the case where provenance is off. Nothing else can join a translation run: +a glossary that must read from the CMS before the request, or an audit entry after the save, has +nowhere to go without editing that handler again. Issue #109 is a consumer who hit exactly this and +wrapped the whole provider to get an `await`. + +This step replaces those lines with **two positions any module can occupy**, and makes provenance +the first tenant with no privileges (D6). It also dissolves `levels` into `modules` + `components`, +the config-time half of the same move (D7, D12, D13). + +It is a stopping point: nothing is exported, and the seam opens to hosts at step 9 (#202). Every +later step waits on it — #199, #48, #200, #201. + +## What already exists — platform · installed · project · verdict + +Nothing is invented. Every piece has a precedent in this package: + +| Need | What already does it | +|---|---| +| a config-time contribution | `ConfigModifier` (`types/ConfigModifier.ts`), whose docblock already describes the `configure(ctx) → ConfigModifier` arrangement both `ProvenanceModule` and `AutoTranslateModule` implement | +| one sink that writes into the Payload config | `PluginConfigBuilder.applyTo` | +| swallow a module failure, but not one that took the caller's transaction | `swallowOrThrow` (`server/shared/payload/swallowOrThrow.ts`) | +| read the attempt count defensively | `isLastAttempt.ts:10` already reads `totalTried` | +| the two staleness endpoints as separable units | `createGetDocumentStalenessRoute` / `createDismissStalenessRoute` — standalone factories; nothing else in `createTranslationRoutes.ts:45-59` reads them | + +## Decisions + +Adopted from `2026-10-18-the-rewiring.md`: D1, D3–D10, D12–D14, D22. Settled in conversation and +recorded here because they depart from, or narrow, that plan. + +**D-a — two positions, not three.** D2 names three: before the translation, after it but before the +save, after the save. The middle has no tenant at this step and none at step 5 either. +*Rejected:* building it anyway for symmetry — a position with no caller is a field, not a type, and +step 9 is where its first tenant arrives. + +**D-b — no stored run-kind field here.** D21 assembles the built-in chain per trigger, which needs +the trigger to survive the queue as a stored field. At this step nothing turns on it: whether drift +shortens the field list is still decided by `strategy`, and `SkipExisting.strategy.ts:11` is the +only reader of the drift map. Both provenance modules are in every chain, and behaviour is +unchanged. +*Rejected:* adding the field now. It would be a second edit to the stored job schema — step 5 adds +`targets: [{ lng, selection? }]` and the run kind together. +*When it lands it names the kind of run, not who started it.* `TranslationStatusList.tsx:71-81` is a +person pressing a button and asking for the drift-only kind, so a field naming the caller would +break that button. + +**D-c — one provenance module with two positions, not a recorder and a shortener.** *Reversed +during Phase 3, in the open.* The split was recorded first, citing step 5 as its consumer. On +contact with the code it lost: the shortening half needs the fingerprints the recording half +computes, so two chain entries would read each other's note under a foreign key and the document +would be hashed twice. One module captures once at position one, uses it for the drift map there, +and writes it at position three — which is what the hard-wired code already did. +*Rejected:* two entries now. Step 5 splits them where the choice between them is actually made, and +that is a local change inside `server/modules/provenance/`. +*Why the note cannot be dropped instead:* `handler.test.ts:481` pins that the receipt describes the +source the handler **read**, not whatever the pipeline handed back. Re-hashing after the save would +reverse that guard. + +**D-d — the components option is named `components`.** Payload already says `admin.components`. +*Rejected:* a second word (`surfaces`, `controls`) — new vocabulary for a thing the host already +knows by that name. + +**D-e — abilities are not threaded into module setup.** D23's `{ inspect, cancel, rerun, staleness }` +goes to the browser only. No module reads it, and adding the field later is additive. + +**D-f — a module is told which attempt this is**, as `previousAttempts`, zero on the first run. +D11 recorded "a rule, not a flag"; the owner overruled that — the count is information a module may +use, and D11's rule (modules must survive a replay) still binds. The count already exists at +`payload-jobs-runner/store/types.ts:16`. + +**D-g — a module's failure is left to `swallowOrThrow`, unadopted.** *Written one way, then +reversed on evidence. Both halves recorded, because the first was wrong in a way worth remembering.* + +The original decision was that a module's bug must not cost an editor their save, so the chain +adopted any unrecognised failure as a harmless `ModuleFailed` — the same fork `translateOrWrap` +(`handler.ts:196-205`) makes for the translation provider. + +**That was unsound, and a probe proved it.** The provider is given no `Payload` handle; a module is. +`ProvenanceService.record` already calls `swallowOrThrow` itself and *deliberately rethrows* when +the caller has work to lose — and the chain's adoption caught that rethrow and demoted it, so the +run reported success over a write the database had already rolled back. That is F2 in the rewiring +plan, arriving through the door built to prevent it. + +*Chosen:* the chain passes the failure to `swallowOrThrow` unchanged. `mustPropagate`'s own rule +holds — "a foreign error may already have rolled the caller's transaction back, so assume it did" — +which is also what D10's exception says in words. `ModuleFailed` is deleted. +*The cost, accepted:* a module's own bug inside an editor's save does end that save. That is exactly +what provenance does today, so nothing regresses; what this step adds is that a host module gets the +same treatment rather than a weaker one. +*Pinned by:* "does not demote a failure a module raised deliberately" +(`translation-run/__tests__/moduleFailure.test.ts`). + +**D-h — an address a per-leaf receipt does not name means "translate"; a document with no per-leaf +receipt at all still answers for nothing.** *Narrowed during Phase 3, with the owner, on measured +evidence.* + +The rule as first written made no distinction, and that is right for the world after step 5. Applied +now it collapses `skip_existing` into `overwrite` for every document the plugin has not yet +recorded: **ten integration checks failed, including `strategies.int.test.ts:56`, which is the +strategy's own definition.** Narrowed, the same suite fails nothing and the case this task exists +for still passes. +*The deciding argument:* `2026-10-22-the-receipt-stops-guessing.task.md` accepted that a person's +text is overwritten **"once its source moves"**. With no receipt the source has not moved, so the +wider rule is a different trade from the one that was approved — and the editor's replacement for +the lost protection is the field picker in #48, which ships at step 5. +*Rejected:* keeping absence as *cannot tell* in both cases. A field whose source was emptied and +later refilled would then never be re-translated, because a run records the source as it stood and +an empty leaf is absent from it. + +### Placement + +| Piece | Lands in | Why there | +|---|---|---| +| the two state types, the chain's narrow member slice, the chain runner | `src/server/modules/translation-run/` (new) | it is server-side run-time wiring; `server/modules/` is where the other two modules already live | +| the two loops | `translate-document/handler.ts` | replacing the four blocks in place | +| the provenance module | `src/server/modules/provenance/Provenance.module.ts` | the module already owns its config-time wiring | +| the drift rule | `src/core/domain/provenance/staleness.ts` | `changedLeaves` is already there and `recordsEachLeaf` is reused | +| the two staleness routes | `src/server/modules/provenance/` | D8 — a module contributes its own endpoints | +| module and component factories | `src/composition/modules/`, `src/composition/components/` | replaces `src/composition/levels/` | +| `ModuleContext`, `ComponentContext`, `TranslationModule` | `src/server/modules/plugin-config/types.ts` (was `translation-levels/`) | `TranslationContext` loses `provenanceServiceFactory`, which existed only for the staleness routes — that is what removes the import cycle a unified module type would otherwise create | +| `previousAttempts` | `TaskHandlerInput` (`TaskRunnerProvider.interface.ts:10`) | the existing channel from runner to handler | + +### New surface — the shape check + +Fired: a new field on `TaskHandlerInput` (implemented outside this package by any custom runner); +two new state types. + +**`BeforeTranslation` and `AfterSave` are separate types, not one with an extension.** By the save, +`schema`, `sourceData`, `targetData`, `requester` and the drift map are all spent — `AfterSave` +carrying them would be a type whose readers ignore most of its fields. Counted: `AfterSave` has 7 +fields, all read by the recorder; an extension would have 13, of which 6 are dead. +*The second caller for both types is already in the codebase* — the recorder and the shortener. + +**`previousAttempts` is a required `number`, not optional.** Optional would admit "no runner told +us", a state nothing produces: the sync runner answers 0 and the queue runner reads `totalTried`, +defaulting to 0. States admitted 1, states produced 1. +*Breaking for an implementor outside this repo:* a custom `TaskRunnerProvider` must now supply it. +Acceptable on the major line; it gets a `DEPRECATIONS.md` entry alongside the `levels` removal. + +### Written contract + +`swallowOrThrow`'s behaviour on a position-one failure is not in its signature: it returns +`undefined`, and the chain must then keep the state it had rather than adopt `undefined`. Stated in +the chain runner's docblock and pinned by AC5. + +### Escalate to `/sp-architect`? + +No. The design is `2026-10-18-the-rewiring.md`, which is the architecture document; this is one of +its build steps. + +## Acceptance Criteria + +| AC | Criterion | How it is checked | Passes when | +|---|---|---|---| +| AC1 | Staleness reports the same answers it does today | `bunx vitest run src/core/domain/provenance src/server/modules/provenance` | green, with the `changedLeaves` expectations rewritten per AC4 and no other assertion touched | +| AC2 | A field whose source drifted is refreshed while an untouched sibling is left alone | `bun run test:integration -- staleness-refresh` from `apps/dev` | `:64`, `:85`, `:137`, `:173`, `:227` unchanged and green | +| AC3 | The drift map and the receipt are the same values the hard-wired code produced | `bunx vitest run src/server/features/translate-document` | green with no assertion weakened | +| AC4 | **New** — a field whose source was emptied and later refilled is re-translated | new case in `apps/dev/src/integration/translator/staleness-refresh.int.test.ts` | the refilled field is translated, not left alone | +| AC5 | A module's failure is logged, and one that may have taken the caller's transaction is let out rather than reported as success | `handler.moduleFailure.test.ts` + `translation-run/__tests__/moduleFailure.test.ts`; `transaction-rollback.int.test.ts` stays green | inside a transaction it propagates; outside one it is swallowed, the translation is written, and the chain carries on with the state the module was given | +| AC6 | A module sees `previousAttempts: 0` on a first run and `1` on the first retry | new case under `src/server/modules/task-runner/payload-jobs-runner/__tests__/` | both values observed | +| AC7 | Provenance contributes its own two staleness endpoints, and the admin still works without them | `bunx vitest run src/server/features/createTranslationRoutes.test.ts src/__tests__/plugin.test.ts` | the bundle returns 6 endpoints; the two staleness paths appear only with provenance on, and `useDocumentStaleness` reads their absence as "nothing is out of date" | +| AC8 | A config with neither `modules` nor `components` fails at boot naming the replacement | new case beside `src/plugin.test.ts` if one exists, else a new `plugin.modules.test.ts` | the thrown message names `modules` and `components` | +| AC9 | Nothing else moves | `bunx vitest run` · `bun run check-types` · `bunx oxlint .` · `bun run test:integration` from `apps/dev` · `bunx turbo run build --filter='./packages/payload-plugin-translator'` | suite green, 0 type errors, 0 lint errors and ≤44 warnings, 63 integration files green, build green | + +## Pre-flight — each check run against the untouched tree + +| AC | Result on the base | Classification | +|---|---|---| +| AC1 | passing — but four `changedLeaves` checks assert the behaviour AC4 reverses | **mixed**: invariant except the four named below | +| AC2 | passing | **invariant** | +| AC3 | passing | **invariant** | +| AC4 | no such case | **change** — fails now | +| AC5 | no seam exists | **change** | +| AC6 | no such field | **change** | +| AC7 | the two routes are in the shared bundle | **change** | +| AC8 | `levels` defaults silently to two surfaces (`plugin.ts:140`) | **change** | +| AC9 | 180 files / 2405 checks green; baseline for types/lint/build carried from the previous task | **invariant** | + +### What the absent-address rule inverts — named, so Phase 5 cannot gloss it + +Four recorded expectations reverse, all in `staleness.test.ts`. Each is rewritten with its reason, +not deleted. + +| Where | Title today | After | +|---|---|---| +| `staleness.test.ts:144` | "says nothing at all about a leaf the receipt never recorded" | answers `true` | +| `staleness.test.ts:167` | "answers only for the leaves it can answer for" | answers for every leaf the source has now | +| `staleness.test.ts:141` | "says a leaf changed when it is recorded but absent from the document now" | no entry — the map answers about the document as it stands | +| `handler.test.ts:614` | "hands the pipeline the leaves the receipt can answer for, and no others" | every leaf the source has now, an unrecorded one marked changed | + +**Measured, not assumed.** The wider form of D-h — treating "no receipt at all" the same way — +failed ten integration checks across four files, among them `strategies.int.test.ts:56` +("skip_existing keeps a filled target but fills an empty sibling"), which is the strategy's own +definition. The narrowed rule fails none and still satisfies AC4. Both runs are in the Phase 5 +evidence. + +## Risk notes + +**High.** The change moves the provenance write path, changes a public config option, and reverses a +documented protection. + +- **A field the receipt was keeping and no longer names is re-translated**, including a target a + person filled, and without the source having moved in the usual sense. That is D-h's point and the + task's reason; it is bounded to documents the plugin has already recorded leaf by leaf. +- **`levels` is removed outright**, breaking every host that set it or relied on its default. All 63 + integration suites boot through `bootTestPayload.ts:160`, so a mistake there is invisible until + the whole integration run is red. +- **The chain runs inside the editor's transaction** on the sync runner. D-g's wrapping is what + keeps a module's failure from becoming a lost save; AC5 is the only thing proving it. +- `swallowOrThrow` returning `undefined` is easy to adopt into the state by accident, which would + silently blank the run's fingerprints. Pinned by AC5. + +## Human choices + +- **One pass, not two.** The run-time seam and the `levels` → `modules` rename ship together. + *Rejected:* splitting at the config boundary — the owner chose the single pass knowing it touches + the boot path of all 63 integration suites. +- **The middle position is not built.** The owner's words: if nothing needs it, there is no reason + to occupy it, and no interface requires a placeholder. +- **The run-kind field is deferred to step 5.** Proposed, then withdrawn after checking that + `strategy` still decides this at step 4; the owner accepted the withdrawal. +- **The absent-address rule is narrowed to a receipt that exists.** Raised mid-build with both + variants measured against the integration suite. The owner's words: *"Ты предлагаешь уважать + стратегию пока она есть?"* — yes. While `skip_existing` exists, its promise is kept; the wider + rule lands at step 5, where the strategy is deleted and #48 gives the editor the replacement. + +## Open, not solved here + +- A run that overwrites a person's text still says nothing about it (D21's second half) — #48. +- The field-level route writes no receipt at all. +- A run that answered nothing still writes a fresh `translatedAt`. +- The position after the translation and before the save — step 9, with its first tenant. +- **Step 5 must decide what "no per-leaf receipt at all" means, explicitly.** Removing the strategy + does not settle it by itself — it makes it worse. Today `SkipExistingStrategy` reads an absent + answer as *leave it alone*, and that is what protects a filled target. Once the pipeline takes a + ready list of addresses instead, that list comes from `changedLeaves`, which answers for nothing + here — so a first automatic run on an unrecorded document would translate **zero** fields rather + than all of them. The answer there is D-h's wider form, which belongs with #48's field picker. + +## Review log + +### Comment audit over the whole change + +59 blocks collected · 58 verdicts · 1 declined as scheduled for deletion at step S1. Applied: +16 deletions, 27 shortenings, 6 code rewrites, 9 kept with a named reason, 2 gaps closed. + +**Nine false statements it caught in comments written during this build**, each checked against the +code rather than read: + +| Where | Claimed | True | +|---|---|---| +| `plugin.ts` `components` | "controls are not deduplicated, so a duplicate renders twice" | `PluginConfigBuilder` collapses them by `componentKey`; pinned by its own test. Carried over from the old `levels` docblock, where it was already untrue | +| `runChain.ts` | an `APIError` is "the one failure" that escapes | a `TranslatorError` declaring `mustPropagate` escapes too, and a test pins it | +| `staleness.ts` `changedLeaves` | "a receipt that names no leaf at all answers for none" | false for a `fields` receipt holding an empty map — `recordsEachLeaf` accepts it | +| `TranslationRun.interface.ts` `AfterSave` | "carries no document" | it carried `sourceData`, the source document | +| `plugin-config/types.ts` | the list of types deriving from `TranslationContext` | wrong in two directions, and "every handler reads it" is false | +| `plugin-config/types.ts` `TranslationModule` | "not externally implementable" | `setup` is optional, so a host can satisfy it today | +| `Provenance.wiring.ts` | provenance is "a module the plugin lists like any other" and this is "the one place its wiring lives" | it is pinned ahead of every listed module, and its routes live in `Provenance.module.ts` | +| `ModuleNotes` | "the queue takes up to fifty jobs per pass" | 50 is `defaultAutoRun.limit`, which a host overrides; concurrency is the real reason | +| `ConfigModifier.ts` | the `types/` placement prevents a module cycle | that cycle went when `provenanceServiceFactory` left `TranslationContext` | + +**One defect in the code, not the prose:** inserting `NEVER_RETRIED` above `SyncTaskRunner` stranded +the class's docblock on the constant, leaving the class undocumented and the constant with two. + +**Three dead things removed, each found by asking who reads it:** `AfterSave.sourceData` (no +reader), `ResolvedProvenance.serviceFactory` (no reader — `configureProvenance` now returns the +module itself), and a `Record>` that became the named `TaskStatusByLocale`. + +**One owner rule broken and repaired:** `state.notes[PROVENANCE_MODULE] as FieldFingerprints` was a +type assertion to quiet the checker. It is now the type predicate `isFingerprints`, the same idiom +as `recordsEachLeaf`. + +**Declined:** the `sourceChangedByLeaf` docblock — step S1 deletes the field with the strategies. + +### Complexity audit of `staleness.ts`, and what the owner saw in it + +Two changes, the second worth more than the first. + +**`leafChanged` dissolved into its two call sites.** It wrapped one comparison, and its signature +admitted a pair — both arguments absent — that neither caller can produce. Applied at the owner's +request; by this skill's own balance rule it is not a complexity finding, because nothing left the +system: CHANGE 0 → 0, READ unchanged. What did change is that one impossible state stopped being +expressible, and the file dropped two type imports it kept alive for that signature alone. + +**`changedLeaves` stopped taking the whole stored vocabulary.** The owner's observation: the +function works only with a per-leaf map, yet took `SourceFingerprint | null` and narrowed inside, so +two of the three forms it accepted answered with an empty map. It now takes `RecordedFingerprints`, +and `Provenance.module.ts` narrows with `recordsEachLeaf` before calling it. + +*Proof that the guard is now structural rather than a convention:* deleting it from the caller gives +two type errors at the call site — `'previous' is possibly 'null'` and `Property 'hashes' does not +exist on type 'SourceFingerprint'`. Before, the same mistake compiled and returned `{}`. + +*A second state went with it.* `sourceChangedByLeaf` could be `undefined` or `{}`, indistinguishable +to its one consumer. The caller now simply omits the field, so there is one way to say it. + +*Where the decision moved.* "A receipt that cannot answer per leaf tells the pipeline nothing" is a +policy, not a comparison, and it now lives with the module that owns provenance policy — pinned by +`server/modules/provenance/__tests__/Provenance.module.test.ts`, five checks, which the module did +not have before. + +*Why neither audit caught it.* The abstraction audit's honesty measure is exactly the right +instrument (3 forms admitted, 1 used), but the brief that scoped it was written by the author of the +code and described this boundary by its return value. That is the failure its own Step 0 warns +about. The complexity audit cannot see it at all: its counters price a future change, not a +signature that over-claims. + +### Fresh-eyes review of the whole change + +Three findings, one of them the serious kind. + +**Confirmed and fixed — the chain demoted a failure a module raised on purpose.** See D-g. Proved +with a throwaway probe: a module using `swallowOrThrow` itself, exactly as `ProvenanceService.record` +does, rethrows when the caller has work to lose — and the chain's `ModuleFailed` wrapper caught that +rethrow and reported success. The probe failed before the fix and passes after; it is now a standing +check rather than scaffolding. + +**Checked and declined — the fingerprint note is written only after the receipt read succeeds.** +Unreachable: `lastTranslatedFrom` swallows and answers `null` outside a transaction, and inside one +a failure now ends the run, so there is no path where the note is lost while the run continues. + +**Checked and declined — the chain judges a failure against the run's own `scope`, not one a module +returned.** Deliberate, and documented at the call site. + +The reviewer also walked the four shapes a stored fingerprint can take, the `previousAttempts` key +path, and what config-time work moved, and found each sound. + +**Fixed, not carried:** `useDocumentStaleness` requests the staleness route on every document mount, +and that route now exists only while provenance is on — so with it off the admin would have shown an +error where it used to show nothing. The hook reads a 404 as "no locale is out of date", which is +true however the plugin is configured and needs no flag in the browser. Telling the browser what the +plugin can do is D20's ability set, which this step deliberately does not build. + +*Found by following the audit's note into `apps/cms`, which configures the plugin with provenance +off.* The same read also caught that `apps/cms` relied on the removed `levels` default and would not +have booted; it is migrated in this change. diff --git a/packages/payload-plugin-translator/docs/plans/2026-10-23-the-seam-says-what-it-means.task.md b/packages/payload-plugin-translator/docs/plans/2026-10-23-the-seam-says-what-it-means.task.md new file mode 100644 index 000000000..5ccdf1998 --- /dev/null +++ b/packages/payload-plugin-translator/docs/plans/2026-10-23-the-seam-says-what-it-means.task.md @@ -0,0 +1,206 @@ +# The seam says what it means + +**Status:** built and verified, not committed · **Risk:** low · **Follows:** #198, same branch + +## Requirements / Task restatement + +Three findings from today's complexity and abstraction audits over the module seam built in #198. +All three are about the same moment — step 9 (#202), when the seam opens and hosts write modules — +and none of them breaks anything today. + +1. **`beforeTranslation` returns `BeforeTranslation`, but only two of its seven fields are read.** + `handler.ts` takes `moduleState` and `sourceChangedByLeaf`; `runChain.ts` deliberately reads + `scope` from the run's own state; `sourceData`, `target`, `payload` and `previousAttempts` are + read by nobody. A module that changes the source text — the first plausible thing a host module + would do — is ignored with no error and no log line. Measured: 7 expressible / 2 honoured. +2. **`ChainMember.name` keys the module's slot in `moduleState` and must be unique.** The docblock + says so; nothing checks it. Two modules sharing a name share one slot: the second overwrites, and + the first reads back a value that is not its own. +3. **`TranslationModule` and `TranslationComponent` are re-exported from `src/index.ts` for nobody.** + Proven twice: a consumer configuring the plugin without naming either compiles, and the package + compiles without the two exports. `TranslationModule`'s own docblock says "writing your own is + not supported yet", so publishing it invites what it forbids. + +## What already exists — platform · installed · project · verdict + +Nothing is invented. Each fix reuses a pattern the package already has: + +| Need | What already does it | +|---|---| +| refuse at boot with the answer in the message | `plugin.ts` does this twice already — for `access`, and for missing `modules`/`components` | +| pin a compile-time guarantee | `*.pins.ts` with `@ts-expect-error`; `check-types` reads them, the build and vitest do not. Two exist: `claims.pins.ts`, `accessIsNotOptional.pins.ts` | +| merge a module's contribution | the chain already threads state; only what is threaded changes | + +## Decisions + +**D1 — `beforeTranslation` returns a contribution, not the state.** + +```ts +export type RunContribution = { + readonly moduleState?: ModuleState; + readonly sourceChangedByLeaf?: ChangedLeaves; +}; +beforeTranslation?(state: BeforeTranslation): Promise; +``` + +The chain merges: `moduleState` shallowly by key; `sourceChangedByLeaf` is replaced unless the +contribution leaves it out, so no module can clear it. + +*Rejected — have the handler honour the whole returned state.* It cannot: `runChain` deliberately +refuses a module's `scope`, so the type would still lie by one field. Worse, adopting `payload` or +`target` would let a module change which document is being translated half-way through a run. That +makes the type honest by making the behaviour wrong. + +*Rejected — two parameters, the frozen run and the accumulating contribution.* Counted against D1 on +the four shape questions and it buys nothing: with the return type narrowed, a module already cannot +change the run, so the second parameter adds a signature change for a guarantee D1 already gives. + +*Zero option — do nothing and record it for step 9.* Rejected by the owner, who asked for these +fixed now rather than carried. + +**Supersedes D3 of `2026-10-18-the-rewiring.md`,** which reads "The first two take the state and +return it." They take the state and return their contribution. The clause was written when positions +one and two were both planned; position two was never built (D-a of the #198 contract), and the +measured cost of the literal reading is the 5 silently-dropped fields above. + +*A second thing D1 buys, not in the finding:* the chain owns the `moduleState` merge, so a module can +no longer **drop** another module's slot by returning a whole `moduleState` object. It can still +overwrite one by name — the merge protects absence, not ownership. Today provenance spreads +`...state.moduleState` by hand to avoid exactly the dropping. + +**D2 — module names are checked when the chain is assembled, in `plugin.ts`.** +*Rejected:* keying the chain by name in a `Map`. The chain is ordered and D1 of the rewiring plan +makes list order the execution order; a `Map` would make that order incidental. +*Rejected:* checking inside `runChain`. It runs per translation; a configuration mistake belongs at +boot, where the other two refusals already are. + +**D3 — the two types leave `src/index.ts`, and `README.md`'s exports table with them.** +The `modules` / `components` rows of the options table keep naming the types: that table describes +the option's shape, which a reader needs whether or not the name is importable. +*No `DEPRECATIONS.md` entry:* both names were added to `src/index.ts` earlier today, in this same +unmerged change. Nothing shipped with them. + +### Placement + +| Piece | Lands in | Why there | +|---|---|---| +| `RunContribution`, the narrowed signature | `translation-run/TranslationRun.interface.ts` | beside the two state types it completes | +| the merge | `translation-run/runChain.ts` | the chain is what accumulates | +| the name check | `src/plugin.ts` | next to the two refusals already there | +| the unexport | `src/index.ts`, `README.md` | the published surface | +| the pins | `translation-run/__tests__/seamShape.pins.ts` (new) | the package's idiom for compile-time guarantees | + +### New surface — the shape check + +Fired: a new exported type (`RunContribution`), and two optional fields on it. + +| | `RunContribution` (chosen) | `BeforeTranslation` as the return (today) | +|---|---|---| +| cost of the next field of this kind | 1 place | 1 place | +| states admitted / produced | **4 / 4** | **128 / 4** (7 fields a module may return changed; 2 read) | +| second caller already in the repo | the chain merges it; provenance produces it | — | +| breaks for an implementor outside this repo | nothing: the seam is unexported by D3 | — | + +One shape survives. The two rejected alternatives are recorded under D1 with what ruled them out. + +### Written contract + +`runBeforeTranslation` owes its callers two things the signature cannot state: `moduleState` merges +by key rather than replacing, and a module that fails contributes nothing. Both go in its docblock +and both are pinned by a test. + +### Escalate to `/sp-architect`? + +No. One seam, four in-repo consumers, no new dependency, no stored data. + +## Acceptance Criteria + +| AC | Criterion | How it is checked | Passes when | +|---|---|---|---| +| AC1 | A module may return only its contribution, and the chain reads nothing else | `bun run check-types` with `translation-run/__tests__/seamShape.pins.ts`; AC2 covers what the chain reads | both contribution shapes compile — today neither does | +| AC2 | A module that returns only its own note does not wipe a sibling's | new case in `translation-run/__tests__/chainOrder.test.ts` | both keys are present in `moduleState` after the chain | +| AC3 | A module that fails still contributes nothing | `bunx vitest run src/server/modules/translation-run` | the existing failure checks stay green, unweakened | +| AC4 | Two modules with the same name are refused at boot | new case in `src/__tests__/plugin.test.ts` | the thrown message names the duplicated name | +| AC5 | A distinct-name chain still boots | same file | the existing plugin tests stay green | +| AC6 | The two types are not importable from the package entry | `bun run check-types` with the same pins file | `@ts-expect-error` on importing them from `../index.js` is a real error | +| AC7 | Provenance still records what it recorded | `bunx vitest run src/server/modules/provenance src/server/features/translate-document` | green, no assertion weakened | +| AC8 | Nothing else moves | `bunx vitest run` · `bun run check-types` · `bunx oxlint .` · `bunx turbo run build --filter='./packages/payload-plugin-translator'` · `bun run test:integration` from `apps/dev` | 183 files / ≥2443 checks, 0 type errors, ≤44 warnings and 0 errors, build green, 63 integration files green | + +## Pre-flight — each check run against the untouched tree + +Run against the untouched tree. The pins file was written, run, and parked in the scratchpad so the +tree is not left red at the gate. + +| AC | Result on the base | Classification | +|---|---|---| +| AC1, AC6 | **three `@ts-expect-error` directives report "Unused"** — importing both types from the package entry is legal today, and so is returning the whole state | **change** — fails first, for the right reason | +| AC2 | **cannot be written in its final form against the current types**: a contribution-shaped return is `error TS2322: Type '() => Promise<{ sourceChangedByLeaf: {} }>' is not assignable to '(state) => Promise'`. That type error is its pre-flight failure | **change** | +| AC4 | no such refusal; two same-named modules boot silently | **change** | +| AC3, AC5, AC7 | passing | **invariant** | +| AC8 | 183 files / 2439 checks, 0 type errors, 44 warnings, build green, 63 integration files | **invariant** | + +## One correction made while building + +**AC1 overclaimed, and the probe said so.** It promised the type would *refuse* a module that +returns the whole run state. TypeScript cannot: a wider object is assignable to a narrower one, and +excess-property checking does not reach a return position. Probed three ways — returning the state +by reference, an object literal with an extra field, and a literal built by spreading the state — +and none is an error. + +So the narrowed return **states** the contract rather than enforcing it. That is still worth having: +the signature now names the two things the chain reads instead of promising seven, a module written +against it has no reason to touch anything else, and the merge in AC2 is what actually protects a +sibling's slot. The pins file records the limitation in a comment rather than pretending to pin it. + +## Risk notes + +**Low.** Everything changed is internal: after D3 nothing about the seam is published, every consumer +is in this repository and compile-checked, and no stored data or access rule is touched. + +Two things to watch: + +- `chainOrder.test.ts` currently proves threading by having a module return a changed + `previousAttempts`. That field stops being returnable, so the check must be rewritten to prove the + same thing through `moduleState` — a rewrite, not a deletion, and the hygiene gate will see it. +- The `moduleState` merge moves from provenance to the chain. If the chain merged by replacement + instead of by key, a second module would wipe the first's slot and no existing test would see it, + because there is only one module today. AC2 exists for that. + +## Human choices + +- **Fix now rather than record for step 9.** The owner's words: "Я не хочу держать это в голове — + если чиним, то давай сейчас." + +## Open, not solved here + +- The second deleted test flagged by the hygiene gate — `"returns empty when provenance is + disabled"` in `staleness.test.ts` — still has no replacement. A symmetric type pin would close it; + the owner has not decided. +- `previousAttempts` still has no reader (D-f of the #198 contract). + +## Review log + +### Fresh eyes over the twelve changed files — 2026-10-23 + +Verdict: the change itself is correct. The merge keeps a sibling's slot on every path, the name +check runs on the fully assembled chain and nothing else builds one, and provenance writes and reads +what it wrote and read before. The defects were all in what the tests prove, and three of them were +confirmed by mutation before being fixed. + +| Found | Proof it was real | Done | +|---|---|---| +| Nothing covered the `?? state.sourceChangedByLeaf` fallback. A later module contributing only a note would wipe the leaf-change map at step 9 | dropped the fallback; **all 2443 checks passed** | new case in `chainOrder.test.ts`, reddens alone | +| Nothing covered a failure *between* two contributing modules. Resetting to the run's opening state on a failure would wipe an earlier module's slot | `: carried` → `: state`; **all 2443 checks passed** | new case in `moduleFailure.test.ts`, reddens alone | +| `@ts-expect-error` on the two unexport pins is satisfied by "cannot find module", so a path that stops resolving would hide a re-export | broke the path; **`check-types` reported 0 errors** | positive control on `TargetSelectionMode`, verified to redden the broken path | +| Nothing pinned that the check sees the *built-in* module names. Moving it above the chain assembly would let a host module take `"provenance"` | `refuseDuplicateNames(modules)`; the new case was the only one that reddened | new case in `plugin.test.ts` | +| The replacement for the old threading check duplicated `"carries one module's note through to the next"` | no mutation reddens it alone — only alongside two others | removed; the surviving case is the one that discriminates | +| `moduleFailure.test.ts` still returned the whole state from a module | — | returns `{}` | +| The merge rule sat on the private helper, not on the exported function the contract names | — | moved onto `runBeforeTranslation` | + +Two overclaims in this document, corrected above: D1 said a module can no longer **overwrite** +another's slot (the merge protects against **dropping** one, not against overwriting it by name), +and said `sourceChangedByLeaf` merges by replacement (it is replaced only when the contribution +carries one, so no module can clear it). + +Final state: **183 files / 2445 checks**, 0 type errors, 44 lint warnings and 0 errors, build green, +`apps/cms` and `apps/dev` type-check clean, integration **63 files, 236 passed / 8 skipped**. diff --git a/packages/payload-plugin-translator/docs/plans/2026-10-23-tidying-the-module-tree.md b/packages/payload-plugin-translator/docs/plans/2026-10-23-tidying-the-module-tree.md new file mode 100644 index 000000000..20a195bf9 --- /dev/null +++ b/packages/payload-plugin-translator/docs/plans/2026-10-23-tidying-the-module-tree.md @@ -0,0 +1,123 @@ +# Tidying the module tree + +**Status:** proposed · **Follows:** #198, on the same branch · **Decision owed:** which option + +## Why + +`server/modules/` has grown into a flat listing a reader cannot navigate: the folder tells you a +module exists, and nothing about what each file in it is for. The package already declares a +convention for this — `CLAUDE.md`, "Grouping files inside a module": folders named by role, with the +dependency running one way, `model/` → `store/`, and `payload-jobs-runner/` named as the worked +example. One module follows it. The others do not. + +## What the tree actually looks like + +Measured 2026-10-23, after #198. + +| Module | Files in its root | Lines | State | +|---|---|---|---| +| `provenance/` | **9** | 1930 | the sprawl this plan is about | +| `auto-translate/` | 5 | 1058 | at the edge | +| `task-runner/payload-jobs-runner/` | 4 + `model/` + `store/` | ~2300 | the only one following the convention | +| `task-runner/` (root) | 5 | — | the two ports, plus a stray `toTaskFilter.ts` | +| `task-runner/sync-runner/` | 4 | ~200 | small enough to stay flat | +| `translation-run/` | 3 | 430 | reads as it is | +| `plugin-config/` | 4 | 383 | reads as it is | +| `lifecycle/` | 6 | 309 | reads as it is | + +Inside `provenance/` every role tag is mixed into one list: `.collection.ts`, `.module.ts`, +`.service.ts`, `.shapes.ts`, `.store.ts`, `.wiring.ts`, `Cleanup.hook.ts`, `slugGuard.ts`, `index.ts`. + +## Option 1 — extend the convention already written down + +Apply `payload-jobs-runner/`'s shape to every module that has outgrown a flat listing. The +`Provenance.` prefix goes: inside `provenance/` it repeats the folder. + +``` +provenance/ + index.ts + config.ts <- Provenance.wiring.ts resolves the option to a module, or to nothing + module.ts <- Provenance.module.ts the two run positions + setup + model/ + policy.ts <- Provenance.service.ts decides what to record and what reads stale + store/ + collection.ts <- Provenance.collection.ts + payload.ts <- Provenance.store.ts + shapes.ts <- Provenance.shapes.ts + slugGuard.ts + hooks/ + cleanup.ts <- ProvenanceCleanup.hook.ts +``` + +**For:** nothing is invented; the rule and its worked example are already in `CLAUDE.md`, and +`model/` → `store/` is checkable with one grep. +**Against:** `hooks/` is a role the table does not list yet; the dotted tags move from filenames into +folder names, so the "a directory listing reads as an architecture map" claim moves with them. + +## Option 2 — keep the nesting, drop the repeated prefix + +The cheapest change, and it is a subset of Option 1. + +``` +provenance/ + index.ts wiring.ts module.ts service.ts store.ts + collection.ts cleanup.hook.ts shapes.ts slugGuard.ts +``` + +**For:** one pass over every module, no new vocabulary, the listing reads as roles. +**Against:** nine files stay nine. It answers "what is each file for" and not "there are too many of +them in one list". + +## Option 3 — split by when the code runs + +Every module has two halves and the plugin calls them at different times: once while the config is +built, and again on each translation. That is the first question a reader has about an unfamiliar +module. + +``` +provenance/ + index.ts + module.ts the module object, both halves in one place + setup/ runs once, while the config is built + collection.ts cleanup.hook.ts routes.ts slugGuard.ts + run/ runs on every translation + fingerprints.ts receipt.ts + store/ shared by both halves + payload.ts shapes.ts +``` + +**For:** it mirrors the seam #198 built — the folder answers "when". +**Against:** a third vocabulary beside the two that exist; `store/` belongs to neither half and sits +outside the split. + +## Two things to settle alongside, whichever option wins + +**Test placement is inconsistent.** Four modules keep tests in `__tests__/`; `lifecycle/` and +`plugin-config/` keep them beside the source (`LifecycleNotifier.test.ts`, `taskMapping.test.ts`, +`PluginConfigBuilder.test.ts`). Pick one and apply it. + +**The frame sits among its tenants.** `plugin-config/` and `translation-run/` are not modules — they +are what a module plugs into: `TranslationModule`, `ModuleContext`, `PluginConfigBuilder`, and the +chain runner. A listing of `modules/` gives no sign that two of its six entries are the construction +rather than its occupants. Separable independently of the option above: + +``` +server/ + modules/ provenance · auto-translate · task-runner · lifecycle + plugin/ plugin-config + translation-run +``` + +## Recommendation + +**Option 1**, because it is already written down, has a working example, and carries a checkable +dependency rule; inventing a third vocabulary for one sprawling folder is not worth it. Option 2 is +part of Option 1 and is worth doing regardless. The frame/tenant split is cheap and answers the +complaint directly. + +## Open + +- Which option. +- Whether `hooks/` joins the role table in `CLAUDE.md`, or cleanup lives under `store/`. +- What the frame folder is called, if it is split out — `server/plugin/` collides with nothing, but + `src/composition/` already holds the public factories, so the two names should not be confusable. +- Whether `task-runner/toTaskFilter.ts` belongs where it is. diff --git a/packages/payload-plugin-translator/src/__tests__/plugin.access.test.ts b/packages/payload-plugin-translator/src/__tests__/plugin.access.test.ts index bbd59acf0..4dd30e3cc 100644 --- a/packages/payload-plugin-translator/src/__tests__/plugin.access.test.ts +++ b/packages/payload-plugin-translator/src/__tests__/plugin.access.test.ts @@ -23,6 +23,8 @@ const build = (extra: Record) => collections: [posts], translationProvider: provider, runner: createSyncRunner(), + modules: [], + components: [], ...extra, } as never)(baseConfig()); diff --git a/packages/payload-plugin-translator/src/__tests__/plugin.test.ts b/packages/payload-plugin-translator/src/__tests__/plugin.test.ts index 5511c322b..7dfffe19d 100644 --- a/packages/payload-plugin-translator/src/__tests__/plugin.test.ts +++ b/packages/payload-plugin-translator/src/__tests__/plugin.test.ts @@ -8,7 +8,12 @@ import { translatorPlugin } from "../plugin.js"; import { AnyAccessGuard } from "../server/shared/access/AnyAccessGuard.js"; import type { TranslatorPluginConfig } from "../plugin.js"; import { withAutoTranslate } from "../auto-translate-config.js"; -import { documentLevel, fieldLevel } from "../composition/levels/index.js"; +import { documentTranslation, fieldTranslation } from "../composition/modules/index.js"; +import { PROVENANCE_MODULE } from "../server/modules/provenance/index.js"; +import { + bulkTranslationDashboard, + translateDocumentControl, +} from "../composition/components/index.js"; import { TranslateDocumentExport } from "../client/widgets/translate-document/index.js"; import { BulkDocumentTranslationDashboard } from "../client/widgets/bulk-translation-dashboard/ui/BulkTranslationDashboard.export.js"; @@ -20,10 +25,8 @@ vi.mock("../core/translation-pipeline/index.js", () => ({ translateContent: vi.fn().mockResolvedValue(null), })); -// End-to-end behaviour guard for the levels refactor: the default levels must -// reproduce today's wiring (6-route bundle, cache provider, doc popup + bulk -// dashboard on managed collections, runner configured once), and an explicit -// `levels` list must scope the surfaces accordingly. +const STALENESS_PATH = "/translate/stale/:collection_slug/:collection_id"; +const DISMISS_PATH = "/translate/stale/dismiss"; const makeCollection = () => ({ slug: "posts", @@ -44,6 +47,8 @@ async function build(overrides: Partial = {}) { // The plugin refuses to start without an access decision; these fixtures make the open one. access: new AnyAccessGuard(), runner, + modules: [documentTranslation()], + components: [translateDocumentControl(), bulkTranslationDashboard()], ...overrides, } as unknown as TranslatorPluginConfig; @@ -55,11 +60,60 @@ async function build(overrides: Partial = {}) { }; } -describe("translatorPlugin — default levels (behaviour-preserving)", () => { - it("registers the shared route bundle once (document + collection share it)", async () => { +describe("translatorPlugin — the listed surfaces", () => { + it("registers the job-API bundle, and no staleness routes without provenance", async () => { const { result } = await build(); - expect(result.endpoints).toHaveLength(8); // not 16 — deduped across the two doc levels - expect(result.endpoints?.map((e) => e.path)).toContain("/translate/enqueue"); + const paths = result.endpoints?.map((endpoint) => endpoint.path); + + expect(paths).toContain("/translate/enqueue"); + expect(paths).not.toContain(STALENESS_PATH); + expect(paths).not.toContain(DISMISS_PATH); + expect(result.endpoints).toHaveLength(6); + }); + + it("registers the staleness routes once provenance is switched on", async () => { + const { result } = await build({ provenance: true }); + const paths = result.endpoints?.map((endpoint) => endpoint.path); + + expect(paths).toContain(STALENESS_PATH); + expect(paths).toContain(DISMISS_PATH); + expect(result.endpoints).toHaveLength(8); + }); + + it("refuses to start when the surfaces are not listed at all", async () => { + await expect(build({ modules: undefined, components: undefined })).rejects.toThrow( + /`modules` and `components` are both required/u + ); + }); + + it("refuses two modules that share a name", async () => { + const twice = [documentTranslation(), documentTranslation()]; + + await expect(build({ modules: twice })).rejects.toThrow(/two modules are named/u); + }); + + it("names the module that was declared twice", async () => { + const twice = [documentTranslation(), documentTranslation()]; + + await expect(build({ modules: twice })).rejects.toThrow(/"document-translation"/u); + }); + + it("refuses a module that takes the name of a built-in one", async () => { + const collides = [documentTranslation(), { name: PROVENANCE_MODULE }]; + + await expect(build({ provenance: true, modules: collides })).rejects.toThrow(/"provenance"/u); + }); + + it("boots a chain whose modules all differ", async () => { + const { result } = await build({ modules: [documentTranslation(), fieldTranslation()] }); + + expect(result.endpoints).toHaveLength(7); + }); + + it("names the replacement for each level it removed", async () => { + await expect(build({ modules: undefined, components: undefined })).rejects.toThrow( + /documentTranslation\(\)/u + ); }); it("does not duplicate endpoints when the same config is run through the plugin twice", async () => { @@ -70,6 +124,8 @@ describe("translatorPlugin — default levels (behaviour-preserving)", () => { // The plugin refuses to start without an access decision; these fixtures make the open one. access: new AnyAccessGuard(), runner: makeRunner(), + modules: [documentTranslation()], + components: [translateDocumentControl(), bulkTranslationDashboard()], } as unknown as TranslatorPluginConfig; const once = await translatorPlugin(pluginConfig)({ @@ -77,9 +133,7 @@ describe("translatorPlugin — default levels (behaviour-preserving)", () => { } as unknown as Config); const twice = await translatorPlugin(pluginConfig)(once); - // Endpoint seeding dedups against routes already on the config, so a second - // registration on the same config doesn't double the 6-route bundle. - expect(twice.endpoints).toHaveLength(8); + expect(twice.endpoints).toHaveLength(6); }); it("adds the cache provider and configures the runner once", async () => { @@ -106,24 +160,27 @@ describe("translatorPlugin — default levels (behaviour-preserving)", () => { }); }); -describe("translatorPlugin — explicit levels", () => { - it("documentLevel only → bundle present, doc popup yes, bulk dashboard no", async () => { - const { result, posts } = await build({ levels: [documentLevel()] }); - expect(result.endpoints).toHaveLength(8); +describe("translatorPlugin — listing the surfaces one at a time", () => { + it("one control only → bundle present, doc popup yes, bulk dashboard no", async () => { + const { result, posts } = await build({ + modules: [documentTranslation()], + components: [translateDocumentControl()], + }); + expect(result.endpoints).toHaveLength(6); expect(posts.admin.components.edit.beforeDocumentControls).toHaveLength(1); expect(posts.admin.components.beforeListTable).toBeUndefined(); }); - it("fieldLevel only → one /field endpoint, no doc routes, no collection components", async () => { - const { result, posts } = await build({ levels: [fieldLevel()] }); + it("the field module only → one /field endpoint, no doc routes, no collection components", async () => { + const { result, posts } = await build({ modules: [fieldTranslation()], components: [] }); expect(result.endpoints).toHaveLength(1); expect(result.endpoints?.[0].path).toBe("/translate/field"); expect(result.endpoints?.[0].method).toBe("post"); expect(posts.admin).toBeUndefined(); }); - it("empty levels → no endpoints/components, but cache provider + runner config still happen", async () => { - const { result, runner } = await build({ levels: [] }); + it("nothing listed → no endpoints/components, but cache provider + runner config still happen", async () => { + const { result, runner } = await build({ modules: [], components: [] }); expect(result.endpoints ?? []).toHaveLength(0); expect(result.collections?.[0] as Record).not.toHaveProperty("admin"); expect(result.admin?.components?.providers).toHaveLength(1); @@ -190,6 +247,8 @@ describe("translatorPlugin — provenance (opt-in)", () => { // The plugin refuses to start without an access decision; these fixtures make the open one. access: new AnyAccessGuard(), runner: { create: vi.fn(), configure: vi.fn().mockReturnValue((c: Config) => c) }, + modules: [documentTranslation()], + components: [translateDocumentControl(), bulkTranslationDashboard()], provenance: {}, } as unknown as TranslatorPluginConfig; @@ -213,6 +272,8 @@ describe("translatorPlugin — provenance (opt-in)", () => { // The plugin refuses to start without an access decision; these fixtures make the open one. access: new AnyAccessGuard(), runner: { create: vi.fn(), configure: vi.fn().mockReturnValue((c: Config) => c) }, + modules: [documentTranslation()], + components: [translateDocumentControl(), bulkTranslationDashboard()], provenance: { slug }, }) as unknown as TranslatorPluginConfig; @@ -251,6 +312,8 @@ describe("translatorPlugin — provenance (opt-in)", () => { // The plugin refuses to start without an access decision; these fixtures make the open one. access: new AnyAccessGuard(), runner: { create: vi.fn(), configure: vi.fn().mockReturnValue((c: Config) => c) }, + modules: [documentTranslation()], + components: [translateDocumentControl(), bulkTranslationDashboard()], provenance: {}, } as unknown as TranslatorPluginConfig; @@ -279,6 +342,8 @@ describe("translatorPlugin — auto-translate (opt-in wiring)", () => { // The plugin refuses to start without an access decision; these fixtures make the open one. access: new AnyAccessGuard(), runner: makeRunner(), + modules: [documentTranslation()], + components: [translateDocumentControl(), bulkTranslationDashboard()], } as unknown as TranslatorPluginConfig; return translatorPlugin(pluginConfig)({ collections } as unknown as Config); }; @@ -311,6 +376,8 @@ describe("translatorPlugin — auto-translate (opt-in wiring)", () => { // The plugin refuses to start without an access decision; these fixtures make the open one. access: new AnyAccessGuard(), runner: makeRunner(), + modules: [documentTranslation()], + components: [translateDocumentControl(), bulkTranslationDashboard()], } as unknown as TranslatorPluginConfig; const once = await translatorPlugin(pluginConfig)({ collections: [posts], diff --git a/packages/payload-plugin-translator/src/client/entities/translation/api/queries/useDocumentStaleness.ts b/packages/payload-plugin-translator/src/client/entities/translation/api/queries/useDocumentStaleness.ts index 976926722..e11b01509 100644 --- a/packages/payload-plugin-translator/src/client/entities/translation/api/queries/useDocumentStaleness.ts +++ b/packages/payload-plugin-translator/src/client/entities/translation/api/queries/useDocumentStaleness.ts @@ -1,5 +1,5 @@ import { useQuery } from "@tanstack/react-query"; -import { ofetch } from "ofetch"; +import { FetchError, ofetch } from "ofetch"; import type { CollectionSlug } from "payload"; import { useTranslateKitConfig } from "../../../../app/config/index.js"; @@ -19,10 +19,17 @@ export const DOCUMENT_STALENESS_QUERY_KEY = "document-staleness"; const getQueryKey = ({ collection, id }: Props) => [DOCUMENT_STALENESS_QUERY_KEY, collection, id]; +const NOTHING_TO_REPORT: DocumentStaleness = { locales: [] }; + +const isNotFound = (error: unknown) => error instanceof FetchError && error.status === 404; + /** * Reads per-locale staleness for a document (#50). Computed lazily on the server, so there is no * polling — it refetches on mount / window focus and whenever the dismiss mutation invalidates its - * key. Reports nothing when provenance is disabled. + * key. + * + * The route exists only while provenance is switched on, so its absence is an answer — no locale is + * out of date — rather than a failure to show the editor. */ export function useDocumentStaleness({ collection, id }: Props, options?: Options) { const { basePath } = useTranslateKitConfig(); @@ -31,11 +38,16 @@ export function useDocumentStaleness({ collection, id }: Props, options?: Option queryKey: getQueryKey({ collection, id }), queryFn: async ({ signal }) => { return handleNextApiError(async () => { - const response = await ofetch<{ data: DocumentStaleness }>( - `/api${basePath}/stale/${collection}/${id}`, - { method: "get", signal } - ); - return response.data; + try { + const response = await ofetch<{ data: DocumentStaleness }>( + `/api${basePath}/stale/${collection}/${id}`, + { method: "get", signal } + ); + return response.data; + } catch (error) { + if (isNotFound(error)) return NOTHING_TO_REPORT; + throw error; + } }); }, enabled: options?.enabled, diff --git a/packages/payload-plugin-translator/src/client/widgets/translate-field-control/ui/TranslateFieldControl.tsx b/packages/payload-plugin-translator/src/client/widgets/translate-field-control/ui/TranslateFieldControl.tsx index 601d95a85..9c48476a7 100644 --- a/packages/payload-plugin-translator/src/client/widgets/translate-field-control/ui/TranslateFieldControl.tsx +++ b/packages/payload-plugin-translator/src/client/widgets/translate-field-control/ui/TranslateFieldControl.tsx @@ -58,34 +58,21 @@ const TranslateFieldControl = () => { const localeOptions = useLocaleOptions(); const { mutateAsync, isPending } = useTranslateField(); - // `config.localization` is `false | {…}`, so the truthy check is load-bearing (not just a nil guard). const defaultLocale = config.localization ? config.localization.defaultLocale : undefined; - // `useLocale()` is typed `Locale` but actually returns `false | {} | Locale` until the admin's - // locale provider resolves (see the V4 TODO on the hook in @payloadcms/ui), so `code` can be absent. const targetCode = locale?.code ?? ""; const [isOpen, popup] = useToggle(); const [sourceLng, setSourceLng] = useState(() => defaultLocale && defaultLocale !== targetCode ? defaultLocale : "" ); - // `null` = no undo available; an object wraps the value so a legitimately `undefined` field value - // (a real pre-translation state) stays distinguishable from "nothing to undo". - const [undo, setUndo] = useState<{ value: unknown } | null>(null); - // fieldLevel is off while creating a document: from-locale reads the *saved* document's value, - // which doesn't exist yet — so the control is hidden entirely on a new/unsaved doc. (Re-enable - // once translating the unsaved value in place is supported.) All hooks above run unconditionally. + const [undo, setUndo] = useState<{ value: unknown } | null>(null); if (id === undefined || id === null) return null; - // Source options: every locale except the one being edited (it's the fixed target). const sourceLocaleOptions = localeOptions.filter((option) => option.value !== targetCode); const canTranslate = sourceLng !== "" && !isPending; - // One write path for every supported field type: a form UPDATE bumping value AND initialValue. - // A Lexical (richText) editor re-mounts on the initialValue change so it shows the new content; - // text/textarea inputs just re-render from value. setModified keeps the form dirty/savable, since - // value === initialValue would otherwise read as unchanged. const writeFieldValue = (next: unknown) => { dispatchFields({ type: "UPDATE", path, value: next, initialValue: next }); setModified(true); @@ -99,9 +86,9 @@ const TranslateFieldControl = () => { const { data } = await mutateAsync({ collectionSlug, fieldPath: path, - targetLng: targetCode, // always the current edit locale — never derived from the source picker + targetLng: targetCode, sourceLng, - docId: id, // non-null here: the control early-returns on create (no document id) + docId: id, }); if (data.status === "translated") { @@ -130,8 +117,6 @@ const TranslateFieldControl = () => { $side="right" open={isOpen} onOpenChange={popup.setValue} - // Don't auto-focus the first button on open — it's a Tooltip trigger, and Radix tooltips - // open on focus, so autofocus would flash the Translate button's tooltip immediately. onOpenAutoFocus={(event) => event.preventDefault()} $trigger={
@@ -153,8 +138,6 @@ const TranslateFieldControl = () => { >
- {/* The from → to pair as one labeled unit, so a screen reader announces the direction - together (mirrors the TranslationDirection entity used at the doc/collection level). */}
new BulkDocumentTranslationDashboard(ctx.access, ctx.settings.targetSelection) + ); + }, + }; +} diff --git a/packages/payload-plugin-translator/src/composition/components/index.ts b/packages/payload-plugin-translator/src/composition/components/index.ts new file mode 100644 index 000000000..b344b305d --- /dev/null +++ b/packages/payload-plugin-translator/src/composition/components/index.ts @@ -0,0 +1,2 @@ +export { bulkTranslationDashboard } from "./bulkTranslationDashboard.js"; +export { translateDocumentControl } from "./translateDocumentControl.js"; diff --git a/packages/payload-plugin-translator/src/composition/components/translateDocumentControl.ts b/packages/payload-plugin-translator/src/composition/components/translateDocumentControl.ts new file mode 100644 index 000000000..6cdf12605 --- /dev/null +++ b/packages/payload-plugin-translator/src/composition/components/translateDocumentControl.ts @@ -0,0 +1,20 @@ +import { TranslateDocumentExport } from "../../client/widgets/translate-document/index.js"; +import type { TranslationComponent } from "../../server/modules/plugin-config/index.js"; + +/** + * The per-document control on every managed collection's edit view. Calls `documentTranslation()`'s + * routes, so list that module too. + * + * @since 1.0.0 + */ +export function translateDocumentControl(): TranslationComponent { + return { + attach(ctx) { + ctx.addCollectionComponent( + "beforeDocumentControls", + (collection) => + new TranslateDocumentExport(collection, ctx.access, ctx.settings.targetSelection) + ); + }, + }; +} diff --git a/packages/payload-plugin-translator/src/composition/levels/collectionLevel.ts b/packages/payload-plugin-translator/src/composition/levels/collectionLevel.ts deleted file mode 100644 index ce85d4d50..000000000 --- a/packages/payload-plugin-translator/src/composition/levels/collectionLevel.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { BulkDocumentTranslationDashboard } from "../../client/widgets/bulk-translation-dashboard/ui/BulkTranslationDashboard.export.js"; - -import type { TranslationLevel } from "../../server/modules/translation-levels/index.js"; -import { useDocTranslationApi } from "./useDocTranslationApi.js"; - -/** - * Bulk collection translation: a dashboard on the list view (translate many - * documents at once), backed by the same shared document-translation API as - * {@link documentLevel}. - * - * @since 0.5.0 - * @returns An opaque {@link TranslationLevel} to list in `translatorPlugin({ levels })`. - * @example - * ```ts - * // Enable only bulk collection translation (no per-document popup): - * translatorPlugin({ collections: [Posts], translationProvider, runner, levels: [collectionLevel()] }) - * ``` - */ -export function collectionLevel(): TranslationLevel { - return { - extend(ctx) { - useDocTranslationApi(ctx); - ctx.addCollectionComponent( - "beforeListTable", - () => new BulkDocumentTranslationDashboard(ctx.access, ctx.settings.targetSelection) - ); - }, - }; -} diff --git a/packages/payload-plugin-translator/src/composition/levels/documentLevel.ts b/packages/payload-plugin-translator/src/composition/levels/documentLevel.ts deleted file mode 100644 index f6aea0f1a..000000000 --- a/packages/payload-plugin-translator/src/composition/levels/documentLevel.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { TranslateDocumentExport } from "../../client/widgets/translate-document/index.js"; - -import type { TranslationLevel } from "../../server/modules/translation-levels/index.js"; -import { useDocTranslationApi } from "./useDocTranslationApi.js"; - -/** - * Per-document translation: a popup control on the document edit view, backed by - * the shared document-translation API (async/job by default; synchronous if the - * configured `runner` is a sync runner). - * - * @since 0.5.0 - * @returns An opaque {@link TranslationLevel} to list in `translatorPlugin({ levels })`. - * @example - * ```ts - * translatorPlugin({ - * collections: [Posts], - * translationProvider, - * runner: createPayloadJobsRunner(), - * levels: [documentLevel(), collectionLevel()], // this is the default — omit for the same result - * }) - * ``` - */ -export function documentLevel(): TranslationLevel { - return { - extend(ctx) { - useDocTranslationApi(ctx); - ctx.addCollectionComponent( - "beforeDocumentControls", - (collection) => - new TranslateDocumentExport(collection, ctx.access, ctx.settings.targetSelection) - ); - }, - }; -} diff --git a/packages/payload-plugin-translator/src/composition/levels/fieldLevel.test.ts b/packages/payload-plugin-translator/src/composition/levels/fieldLevel.test.ts deleted file mode 100644 index c9371003d..000000000 --- a/packages/payload-plugin-translator/src/composition/levels/fieldLevel.test.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { describe, it, expect, vi } from "vitest"; -import type { CollectionSlug, Field } from "payload"; - -import type { CollectionSchemaMap } from "../../types/CollectionSchemaMap.js"; - -import { resolveSettings } from "../../settings.js"; -import { fieldLevel } from "./fieldLevel.js"; -import type { LevelContext } from "../../server/modules/translation-levels/index.js"; - -// Unit-level: assert which context primitives the level calls. The endpoint's -// path / method / basePath / access wiring is covered by createFieldRoute's own -// contract test (route.test.ts), so we don't re-assert it here. -const makeCtx = (): LevelContext => ({ - collections: [], - settings: resolveSettings({}), - access: undefined, - taskRunnerFactory: { create: vi.fn() }, - schemaMap: new Map([ - ["posts" as CollectionSlug, [{ name: "title", type: "text" }] as Field[]], - ]) as CollectionSchemaMap, - translationProvider: { translate: vi.fn() }, - addEndpoints: vi.fn(), - addCollectionComponent: vi.fn(), -}); - -describe("fieldLevel", () => { - it("contributes the single field-translation endpoint and no collection component", () => { - const ctx = makeCtx(); - fieldLevel().extend(ctx); - - expect(ctx.addEndpoints).toHaveBeenCalledTimes(1); - const [endpoints] = (ctx.addEndpoints as ReturnType).mock.calls[0]; - expect(endpoints).toHaveLength(1); - expect(endpoints[0].path).toBe("/translate/field"); // the field route, not the doc bundle - expect(ctx.addCollectionComponent).not.toHaveBeenCalled(); - }); -}); diff --git a/packages/payload-plugin-translator/src/composition/levels/fieldLevel.ts b/packages/payload-plugin-translator/src/composition/levels/fieldLevel.ts deleted file mode 100644 index 75804f8ee..000000000 --- a/packages/payload-plugin-translator/src/composition/levels/fieldLevel.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { createFieldRoute } from "../../server/features/translate-field/index.js"; - -import type { - LevelContext, - TranslationLevel, -} from "../../server/modules/translation-levels/index.js"; - -/** - * Field-level translation surface: a synchronous `POST {basePath}/field` endpoint that translates - * a single declared field **from a chosen source locale** — it reads the field's value from the - * saved document in that locale and translates it into the active locale. No runner, no queue, no - * persistence. Unlike `documentLevel` / `collectionLevel`, it contributes no admin component: the - * per-field control UI is wired separately at field-declaration time via `withFieldTranslation(field)`. - * - * Not part of the default `levels`, so it stays strictly opt-in. - * - * @returns an opaque {@link TranslationLevel} to list in `translatorPlugin({ levels })` - * @since 0.6.0 - * @example - * ```ts - * translatorPlugin({ - * collections: [Posts], - * translationProvider: createOpenAIProvider({ apiKey: process.env.OPENAI_API_KEY }), - * runner: createPayloadJobsRunner(), - * levels: [documentLevel(), fieldLevel()], - * }); - * ``` - */ -export function fieldLevel(): TranslationLevel { - return { - extend(ctx: LevelContext) { - ctx.addEndpoints([ - createFieldRoute({ - schemaMap: ctx.schemaMap, - translationProvider: ctx.translationProvider, - access: ctx.access, - basePath: ctx.settings.basePath, - }), - ]); - }, - }; -} diff --git a/packages/payload-plugin-translator/src/composition/levels/index.ts b/packages/payload-plugin-translator/src/composition/levels/index.ts deleted file mode 100644 index bcb85e090..000000000 --- a/packages/payload-plugin-translator/src/composition/levels/index.ts +++ /dev/null @@ -1,3 +0,0 @@ -export { documentLevel } from "./documentLevel.js"; -export { collectionLevel } from "./collectionLevel.js"; -export { fieldLevel } from "./fieldLevel.js"; diff --git a/packages/payload-plugin-translator/src/composition/levels/levels.test.ts b/packages/payload-plugin-translator/src/composition/levels/levels.test.ts deleted file mode 100644 index 77497fe56..000000000 --- a/packages/payload-plugin-translator/src/composition/levels/levels.test.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { describe, it, expect, vi } from "vitest"; -import type { CollectionConfig } from "payload"; - -import { TranslateDocumentExport } from "../../client/widgets/translate-document/index.js"; -import { BulkDocumentTranslationDashboard } from "../../client/widgets/bulk-translation-dashboard/ui/BulkTranslationDashboard.export.js"; - -import { resolveSettings } from "../../settings.js"; -import { documentLevel } from "./documentLevel.js"; -import { collectionLevel } from "./collectionLevel.js"; -import type { - CollectionAdminSlot, - LevelContext, -} from "../../server/modules/translation-levels/index.js"; - -const fakeCollection = { - slug: "posts", - fields: [], -} as unknown as CollectionConfig; - -// Pull the make() callback handed to addCollectionComponent for the given slot. -const componentFor = (ctx: LevelContext, slot: CollectionAdminSlot) => { - const call = (ctx.addCollectionComponent as ReturnType).mock.calls.find( - (c) => c[0] === slot - ); - return call?.[1](fakeCollection); -}; - -// Unit-level: a level's extend() should call the expected context primitives. -// The shared route bundle + component construction are covered elsewhere; here -// we only assert the wiring (which primitive, which slot). -const makeCtx = (): LevelContext => ({ - collections: [{ slug: "posts", fields: [] } as unknown as CollectionConfig], - settings: resolveSettings({}), - access: undefined, - taskRunnerFactory: { create: vi.fn() }, - schemaMap: new Map(), - translationProvider: { translate: vi.fn() }, - addEndpoints: vi.fn(), - addCollectionComponent: vi.fn(), -}); - -describe("documentLevel", () => { - it("contributes the document-translation bundle + a beforeDocumentControls component", () => { - const ctx = makeCtx(); - - documentLevel().extend(ctx); - - expect(ctx.addEndpoints).toHaveBeenCalledTimes(1); - expect((ctx.addEndpoints as ReturnType).mock.calls[0][0]).toHaveLength(8); - expect(ctx.addCollectionComponent).toHaveBeenCalledWith( - "beforeDocumentControls", - expect.any(Function) - ); - expect(componentFor(ctx, "beforeDocumentControls")).toBeInstanceOf(TranslateDocumentExport); - }); -}); - -describe("collectionLevel", () => { - it("contributes the document-translation bundle + a beforeListTable component", () => { - const ctx = makeCtx(); - - collectionLevel().extend(ctx); - - expect(ctx.addEndpoints).toHaveBeenCalledTimes(1); - expect((ctx.addEndpoints as ReturnType).mock.calls[0][0]).toHaveLength(8); - expect(ctx.addCollectionComponent).toHaveBeenCalledWith( - "beforeListTable", - expect.any(Function) - ); - expect(componentFor(ctx, "beforeListTable")).toBeInstanceOf(BulkDocumentTranslationDashboard); - }); -}); diff --git a/packages/payload-plugin-translator/src/composition/levels/useDocTranslationApi.ts b/packages/payload-plugin-translator/src/composition/levels/useDocTranslationApi.ts deleted file mode 100644 index 5648f60fc..000000000 --- a/packages/payload-plugin-translator/src/composition/levels/useDocTranslationApi.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { createTranslationRoutes } from "../../server/features/createTranslationRoutes.js"; - -import type { LevelContext } from "../../server/modules/translation-levels/index.js"; - -/** - * Contribute the runner-agnostic document-translation API — the shared route - * bundle, bound to the level context's runner. Both `documentLevel` and - * `collectionLevel` call this; the plugin deduplicates the endpoints by method + - * path, so the bundle registers exactly once. The provenance service factory - * rides along for the staleness endpoints (its schema lives inside the service). - */ -export function useDocTranslationApi(ctx: LevelContext): void { - ctx.addEndpoints( - createTranslationRoutes({ - taskRunnerFactory: ctx.taskRunnerFactory, - collectionConfig: { - availableCollections: new Set(ctx.collections.map((collection) => collection.slug)), - }, - access: ctx.access, - settings: ctx.settings, - provenanceServiceFactory: ctx.provenanceServiceFactory, - }) - ); -} diff --git a/packages/payload-plugin-translator/src/composition/modules/documentTranslation.ts b/packages/payload-plugin-translator/src/composition/modules/documentTranslation.ts new file mode 100644 index 000000000..766133d49 --- /dev/null +++ b/packages/payload-plugin-translator/src/composition/modules/documentTranslation.ts @@ -0,0 +1,25 @@ +import { createTranslationRoutes } from "../../server/features/createTranslationRoutes.js"; +import type { TranslationModule } from "../../server/modules/plugin-config/index.js"; + +/** + * The document-translation job API — queue a run, run it now, cancel it, read its status. + * + * @since 1.0.0 + */ +export function documentTranslation(): TranslationModule { + return { + name: "document-translation", + setup(ctx) { + ctx.addEndpoints( + createTranslationRoutes({ + taskRunnerFactory: ctx.taskRunnerFactory, + collectionConfig: { + availableCollections: new Set(ctx.collections.map((collection) => collection.slug)), + }, + access: ctx.access, + settings: ctx.settings, + }) + ); + }, + }; +} diff --git a/packages/payload-plugin-translator/src/composition/modules/fieldTranslation.ts b/packages/payload-plugin-translator/src/composition/modules/fieldTranslation.ts new file mode 100644 index 000000000..c93598a0f --- /dev/null +++ b/packages/payload-plugin-translator/src/composition/modules/fieldTranslation.ts @@ -0,0 +1,25 @@ +import { createFieldRoute } from "../../server/features/translate-field/index.js"; +import type { TranslationModule } from "../../server/modules/plugin-config/index.js"; + +/** + * A synchronous `POST {basePath}/field`: one declared field, read from the *saved* document — + * nothing queued, nothing persisted. Its control comes from `withFieldTranslation(field)`, not from + * `components`. + * + * @since 1.0.0 + */ +export function fieldTranslation(): TranslationModule { + return { + name: "field-translation", + setup(ctx) { + ctx.addEndpoints([ + createFieldRoute({ + schemaMap: ctx.schemaMap, + translationProvider: ctx.translationProvider, + access: ctx.access, + basePath: ctx.settings.basePath, + }), + ]); + }, + }; +} diff --git a/packages/payload-plugin-translator/src/composition/modules/index.ts b/packages/payload-plugin-translator/src/composition/modules/index.ts new file mode 100644 index 000000000..ae530db06 --- /dev/null +++ b/packages/payload-plugin-translator/src/composition/modules/index.ts @@ -0,0 +1,2 @@ +export { documentTranslation } from "./documentTranslation.js"; +export { fieldTranslation } from "./fieldTranslation.js"; diff --git a/packages/payload-plugin-translator/src/core/domain/provenance/SourceFingerprint.ts b/packages/payload-plugin-translator/src/core/domain/provenance/SourceFingerprint.ts index 165fad3e2..3dd161a50 100644 --- a/packages/payload-plugin-translator/src/core/domain/provenance/SourceFingerprint.ts +++ b/packages/payload-plugin-translator/src/core/domain/provenance/SourceFingerprint.ts @@ -65,5 +65,10 @@ export function parseSourceFingerprint( } export function serializeSourceFingerprint(fingerprint: SourceFingerprint): string { - return fingerprint.kind === "document" ? fingerprint.hash : JSON.stringify(fingerprint.hashes); + switch (fingerprint.kind) { + case "document": + return fingerprint.hash; + case "fields": + return JSON.stringify(fingerprint.hashes); + } } diff --git a/packages/payload-plugin-translator/src/core/domain/provenance/__tests__/theWatermark.test.ts b/packages/payload-plugin-translator/src/core/domain/provenance/__tests__/theWatermark.test.ts index 8cbb217ed..4d8b14953 100644 --- a/packages/payload-plugin-translator/src/core/domain/provenance/__tests__/theWatermark.test.ts +++ b/packages/payload-plugin-translator/src/core/domain/provenance/__tests__/theWatermark.test.ts @@ -6,7 +6,7 @@ import type { CollectionSchemaMap } from "../../../../types/CollectionSchemaMap. import { makeIdPath } from "../../content-projection/idPath.js"; import { SkipExistingStrategy } from "../../../translation-pipeline/strategies/index.js"; import type { CurrentFingerprint } from "../staleness.js"; -import { changedLeaves, isRecordStale } from "../staleness.js"; +import { changedLeaves, isRecordStale, recordsEachLeaf } from "../staleness.js"; import { parseSourceFingerprint, serializeSourceFingerprint, @@ -45,7 +45,9 @@ describe("a leaf the last run saw and skipped", () => { } as const; it("is reported as changed once its source moves", () => { - expect(changedLeaves(watermarked, sourceNow({ title: "ace", body: "cafe" }).fields)).toEqual({ + expect( + changedLeaves(watermarked.hashes, sourceNow({ title: "ace", body: "cafe" }).fields) + ).toEqual({ title: false, body: true, }); @@ -61,7 +63,10 @@ describe("a leaf the last run saw and skipped", () => { }); it("is selected by skip_existing although its target is not empty", () => { - const changed = changedLeaves(watermarked, sourceNow({ title: "ace", body: "cafe" }).fields); + const changed = changedLeaves( + watermarked.hashes, + sourceNow({ title: "ace", body: "cafe" }).fields + ); expect( new SkipExistingStrategy().shouldTranslate({ @@ -73,7 +78,10 @@ describe("a leaf the last run saw and skipped", () => { }); it("is left alone while its source stands still", () => { - const changed = changedLeaves(watermarked, sourceNow({ title: "ace", body: "beef" }).fields); + const changed = changedLeaves( + watermarked.hashes, + sourceNow({ title: "ace", body: "beef" }).fields + ); expect( new SkipExistingStrategy().shouldTranslate({ @@ -94,13 +102,19 @@ describe("a receipt written before the watermark", () => { it("revisits the leaf an old run declined, instead of protecting it for ever", () => { const stored = parseSourceFingerprint(legacy); + if (!recordsEachLeaf(stored)) + throw new Error("the legacy record must still parse leaf by leaf"); - expect(changedLeaves(stored, sourceNow({ title: "ace", body: "beef" }).fields).body).toBe(true); + expect( + changedLeaves(stored.hashes, sourceNow({ title: "ace", body: "beef" }).fields).body + ).toBe(true); }); it("makes that leaf selected by skip_existing over a target it did not write", () => { const stored = parseSourceFingerprint(legacy); - const changed = changedLeaves(stored, sourceNow({ title: "ace", body: "beef" }).fields); + if (!recordsEachLeaf(stored)) + throw new Error("the legacy record must still parse leaf by leaf"); + const changed = changedLeaves(stored.hashes, sourceNow({ title: "ace", body: "beef" }).fields); expect( new SkipExistingStrategy().shouldTranslate({ @@ -181,12 +195,10 @@ describe("the receipt a run leaves behind", () => { it("brings that leaf back on the next run once its source moves", async () => { const stored = await storedAfterRun(null); - expect( - changedLeaves( - { kind: "fields", hashes: stored }, - sourceNow({ title: "ace", body: "cafe" }).fields - ) - ).toEqual({ title: false, body: true }); + expect(changedLeaves(stored, sourceNow({ title: "ace", body: "cafe" }).fields)).toEqual({ + title: false, + body: true, + }); }); it("keeps an older decline marked for revisiting until a run answers it", async () => { diff --git a/packages/payload-plugin-translator/src/core/domain/provenance/index.ts b/packages/payload-plugin-translator/src/core/domain/provenance/index.ts index fe4c6910b..49f32ce95 100644 --- a/packages/payload-plugin-translator/src/core/domain/provenance/index.ts +++ b/packages/payload-plugin-translator/src/core/domain/provenance/index.ts @@ -4,7 +4,7 @@ export type { ProvenanceStore, TranslationProvenanceRecord, } from "./ProvenanceStore.interface.js"; -export type { CurrentFingerprint } from "./staleness.js"; +export type { ChangedLeaves, CurrentFingerprint } from "./staleness.js"; export { changedLeaves, isRecordStale, recordsEachLeaf } from "./staleness.js"; export type { RecordedClaim, SourceFingerprint } from "./SourceFingerprint.js"; export { diff --git a/packages/payload-plugin-translator/src/core/domain/provenance/staleness.test.ts b/packages/payload-plugin-translator/src/core/domain/provenance/staleness.test.ts index fb4566a7f..ab0479a81 100644 --- a/packages/payload-plugin-translator/src/core/domain/provenance/staleness.test.ts +++ b/packages/payload-plugin-translator/src/core/domain/provenance/staleness.test.ts @@ -131,21 +131,24 @@ describe("changedLeaves — the per-leaf answer", () => { const stored = storedFields({ title: "1111", body: "2222" }); it("says a leaf changed when its stored hash differs from now", () => { - expect(changedLeaves(stored, hashed({ title: "1111", body: "9999" })).body).toBe(true); + expect(changedLeaves(stored.hashes, hashed({ title: "1111", body: "9999" })).body).toBe(true); }); it("says a leaf did not change when its stored hash still matches", () => { - expect(changedLeaves(stored, hashed({ title: "1111", body: "2222" })).body).toBe(false); + expect(changedLeaves(stored.hashes, hashed({ title: "1111", body: "2222" })).body).toBe(false); }); - it("says a leaf changed when it is recorded but absent from the document now", () => { - expect(changedLeaves(stored, hashed({ title: "1111" })).body).toBe(true); + it("says nothing about a leaf the document no longer has", () => { + expect(changedLeaves(stored.hashes, hashed({ title: "1111" }))).not.toHaveProperty("body"); }); - it("says nothing at all about a leaf the receipt never recorded", () => { - const answers = changedLeaves(stored, hashed({ title: "1111", body: "2222", sub: "3333" })); + it("says a leaf the receipt never recorded changed, so the next run takes it", () => { + const answers = changedLeaves( + stored.hashes, + hashed({ title: "1111", body: "2222", sub: "3333" }) + ); - expect(answers, "an absent address is cannot-tell, never unchanged").not.toHaveProperty("sub"); + expect(answers.sub, "nothing was recorded for it, so it is owed a translation").toBe(true); }); }); @@ -165,27 +168,17 @@ describe("recordsEachLeaf", () => { describe("changedLeaves", () => { const stored = storedFields({ title: "1111", body: "2222" }); - it("answers only for the leaves it can answer for", () => { + it("answers for every leaf the source has now", () => { expect( - changedLeaves(stored, hashed({ title: "1111", body: "9999", seen: "3333", fresh: "4444" })) - ).toStrictEqual({ title: false, body: true }); - }); - - it("is empty when there is no receipt", () => { - expect(changedLeaves(null, hashed({ title: "1111" }))).toStrictEqual({}); - }); - - it("is empty for a receipt written before per-field fingerprints", () => { - expect( - changedLeaves({ kind: "document", hash: HASH_A }, hashed({ title: "1111" })) - ).toStrictEqual({}); + changedLeaves( + stored.hashes, + hashed({ title: "1111", body: "9999", seen: "3333", fresh: "4444" }) + ) + ).toStrictEqual({ title: false, body: true, seen: true, fresh: true }); }); - it("says a recorded leaf changed when the document no longer has it", () => { - expect(changedLeaves(stored, hashed({ title: "1111" }))).toStrictEqual({ - title: false, - body: true, - }); + it("drops a recorded leaf the document no longer has", () => { + expect(changedLeaves(stored.hashes, hashed({ title: "1111" }))).toStrictEqual({ title: false }); }); }); @@ -202,6 +195,6 @@ describe("a leaf recorded as unanswered", () => { }); it("is reported as changed, so the next run selects the leaf again", () => { - expect(changedLeaves(stored, hashed({ title: "1111", body: "2222" })).body).toBe(true); + expect(changedLeaves(stored.hashes, hashed({ title: "1111", body: "2222" })).body).toBe(true); }); }); diff --git a/packages/payload-plugin-translator/src/core/domain/provenance/staleness.ts b/packages/payload-plugin-translator/src/core/domain/provenance/staleness.ts index 5da20fbf8..ff5e2e259 100644 --- a/packages/payload-plugin-translator/src/core/domain/provenance/staleness.ts +++ b/packages/payload-plugin-translator/src/core/domain/provenance/staleness.ts @@ -1,17 +1,10 @@ -import type { - FieldFingerprints, - SourceHash, -} from "../content-projection/computeFieldFingerprints.js"; +import type { FieldFingerprints } from "../content-projection/computeFieldFingerprints.js"; import type { ProvenanceReceipt } from "./ProvenanceStore.interface.js"; -import type { - RecordedClaim, - RecordedFingerprints, - SourceFingerprint, -} from "./SourceFingerprint.js"; +import type { RecordedFingerprints, SourceFingerprint } from "./SourceFingerprint.js"; -/** One entry per address the receipt recorded, and no others. An address it never saw has no entry. */ +/** One entry per translatable leaf the source has now. */ export type ChangedLeaves = Readonly>; export type CurrentFingerprint = { @@ -19,16 +12,11 @@ export type CurrentFingerprint = { fields: FieldFingerprints; }; -/** `UNANSWERED` can never equal a hash, so one comparison answers for both kinds of claim. */ -const leafChanged = (recorded: RecordedClaim, current: SourceHash | undefined): boolean => { - return recorded !== current; -}; - const sameFields = (stored: RecordedFingerprints, current: FieldFingerprints): boolean => { const addresses = Object.keys(stored); return ( addresses.length === Object.keys(current).length && - addresses.every((address) => !leafChanged(stored[address], current[address])) + addresses.every((address) => stored[address] === current[address]) ); }; @@ -37,9 +25,13 @@ const describesCurrentSource = ( current: CurrentFingerprint ): boolean => { if (fingerprint === null) return false; - return fingerprint.kind === "document" - ? fingerprint.hash === current.document - : sameFields(fingerprint.hashes, current.fields); + + switch (fingerprint.kind) { + case "document": + return fingerprint.hash === current.document; + case "fields": + return sameFields(fingerprint.hashes, current.fields); + } }; /** Out of date unless the receipt — or the editor's dismissal — describes the source as it stands now. */ @@ -56,15 +48,19 @@ export const recordsEachLeaf = ( return fingerprint?.kind === "fields"; }; +/** + * A leaf the receipt never recorded counts as changed: a leaf empty at the last run was not + * recorded, and would otherwise never be translated again. + */ export function changedLeaves( - stored: SourceFingerprint | null, + recorded: RecordedFingerprints, currentFields: FieldFingerprints ): ChangedLeaves { - if (!recordsEachLeaf(stored)) return {}; + const claims = new Map(Object.entries(recorded)); const answers: Record = {}; - for (const [address, recorded] of Object.entries(stored.hashes)) { - answers[address] = leafChanged(recorded, currentFields[address]); + for (const [address, current] of Object.entries(currentFields)) { + answers[address] = claims.get(address) !== current; } return answers; } diff --git a/packages/payload-plugin-translator/src/core/translation-pipeline/__tests__/theLeafThatNeverCameBack.test.ts b/packages/payload-plugin-translator/src/core/translation-pipeline/__tests__/theLeafThatNeverCameBack.test.ts index 52edf904c..d6ebe7d36 100644 --- a/packages/payload-plugin-translator/src/core/translation-pipeline/__tests__/theLeafThatNeverCameBack.test.ts +++ b/packages/payload-plugin-translator/src/core/translation-pipeline/__tests__/theLeafThatNeverCameBack.test.ts @@ -118,7 +118,7 @@ describe("what the unanswered leaf does on the second run", () => { hashes: { title: sourceHashes.title, summary: UNANSWERED }, }; - const changed = changedLeaves(stored, sourceHashes); + const changed = changedLeaves(stored.hashes, sourceHashes); expect(changed.summary).toBe(true); expect( diff --git a/packages/payload-plugin-translator/src/field-config.ts b/packages/payload-plugin-translator/src/field-config.ts index 1e747d0f2..03c839a21 100644 --- a/packages/payload-plugin-translator/src/field-config.ts +++ b/packages/payload-plugin-translator/src/field-config.ts @@ -24,7 +24,7 @@ const activeFieldControlPositioner: FieldControlPositioner = beforeInputPosition * * - **Add a translate control** — wrap a `text` / `textarea` field with no second * argument. A per-field Translate control is positioned on the field (today, just - * above the input). Requires `fieldLevel()` in the plugin's `levels` to register the + * above the input). Requires `fieldTranslation()` in the plugin's `modules` to register the * endpoint. Control-injection behavior is `@since 0.6.0`; field exclusion predates it. * - **Exclude from translation** — pass `{ exclude: true }` (any field type). The field * value is never sent to the translation provider and no control is added. diff --git a/packages/payload-plugin-translator/src/index.ts b/packages/payload-plugin-translator/src/index.ts index cb939c507..ae87fac31 100644 --- a/packages/payload-plugin-translator/src/index.ts +++ b/packages/payload-plugin-translator/src/index.ts @@ -66,9 +66,11 @@ export type { */ export type { RequestScope, Requester } from "./server/shared/payload/RequestScope.shapes.js"; -// Translation levels -export { documentLevel, collectionLevel, fieldLevel } from "./composition/levels/index.js"; -export type { TranslationLevel } from "./server/modules/translation-levels/index.js"; +export { documentTranslation, fieldTranslation } from "./composition/modules/index.js"; +export { + bulkTranslationDashboard, + translateDocumentControl, +} from "./composition/components/index.js"; // Field config export { withFieldTranslation } from "./field-config.js"; diff --git a/packages/payload-plugin-translator/src/plugin.ts b/packages/payload-plugin-translator/src/plugin.ts index 67150cb86..9197c5e7f 100644 --- a/packages/payload-plugin-translator/src/plugin.ts +++ b/packages/payload-plugin-translator/src/plugin.ts @@ -10,9 +10,11 @@ import type { TranslationProvider } from "./core/domain/translation-providers/in import type { TaskRunnerProvider } from "./server/modules/task-runner/index.js"; import { wireTranslateRunner } from "./server/features/translate-document/index.js"; import type { TranslationLifecycleCallbacks } from "./server/modules/lifecycle/index.js"; -import { documentLevel, collectionLevel } from "./composition/levels/index.js"; -import type { TranslationLevel } from "./server/modules/translation-levels/index.js"; -import { PluginConfigBuilder } from "./server/modules/translation-levels/PluginConfigBuilder.js"; +import type { + TranslationComponent, + TranslationModule, +} from "./server/modules/plugin-config/index.js"; +import { PluginConfigBuilder } from "./server/modules/plugin-config/PluginConfigBuilder.js"; import { resolveSettings } from "./settings.js"; import type { TranslatorSettingsOptions } from "./settings.js"; @@ -44,15 +46,21 @@ export type TranslatorPluginConfig = TranslatorSettingsOptions & { */ access: AccessGuard; /** - * Which translation surfaces to enable, as level factories — `documentLevel()`, - * `collectionLevel()`, `fieldLevel()`. Omit for the - * default `[documentLevel(), collectionLevel()]`, which is exactly today's behaviour. - * Each level should appear at most once: endpoints are deduplicated, but admin - * components are not, so a duplicated level renders a duplicated control. - * @default [documentLevel(), collectionLevel()] - * @since 0.5.0 + * Which server modules to enable, as factories — `documentTranslation()`, `fieldTranslation()`. + * Required, and `[]` is a real answer: auto-translate needs no route at all. + * + * @since 1.0.0 — replaces `levels`. + */ + modules: TranslationModule[]; + /** + * Which admin controls to attach, as factories — `translateDocumentControl()`, + * `bulkTranslationDashboard()`. Required, and `[]` is a real answer for a headless setup. + * + * A control calls its module's routes, so list that module in `modules` too. + * + * @since 1.0.0 — split out of `levels`. */ - levels?: TranslationLevel[]; + components: TranslationComponent[]; /** * Opt-in translation provenance — a durable, per-locale record of what source state each * translation was derived from, stored in a plugin-managed sidecar collection. Set `true` (or `{}`) @@ -82,6 +90,23 @@ export type TranslatorPluginConfig = TranslatorSettingsOptions & { lifecycle?: TranslationLifecycleCallbacks; }; +/** + * A module's name keys its slot in the run's `moduleState`, so two modules sharing one would share + * the slot: the second overwrites, and the first reads back a value that is not its own. + */ +function refuseDuplicateNames(chain: readonly TranslationModule[]): void { + const seen = new Set(); + for (const member of chain) { + if (seen.has(member.name)) { + throw new Error( + `payload-plugin-translator: two modules are named "${member.name}". ` + + "Module names must differ — each one keys its own state for the duration of a run." + ); + } + seen.add(member.name); + } +} + /** * Creates a translation plugin for Payload CMS collections. * @@ -108,8 +133,9 @@ export function translatorPlugin( pluginConfig: TranslatorPluginConfig ): (config: Config) => Promise { return async (payloadConfig) => { - const { access, translationProvider, runner, collections, levels, provenance, lifecycle } = + const { access, translationProvider, runner, collections, provenance, lifecycle } = pluginConfig; + const { modules, components } = pluginConfig; const settings = resolveSettings(pluginConfig); // Required by the type, so only a plain-JavaScript caller reaches this. @@ -120,6 +146,17 @@ export function translatorPlugin( ); } + if (!modules || !components) { + throw new Error( + "payload-plugin-translator: `modules` and `components` are both required, and `[]` is a " + + "valid answer for either. `levels` is gone: `documentLevel()` becomes " + + "`modules: [documentTranslation()]` plus `components: [translateDocumentControl()]`, " + + "`collectionLevel()` becomes the same module plus " + + "`components: [bulkTranslationDashboard()]`, and `fieldLevel()` becomes " + + "`modules: [fieldTranslation()]`." + ); + } + const schemaMap: CollectionSchemaMap = new Map( collections.map((col) => [col.slug, projectFieldsToFieldLike(col.fields)]) ); @@ -127,17 +164,21 @@ export function translatorPlugin( const provenanceModule = configureProvenance(provenance, schemaMap); + // Provenance goes first: the staleness route re-hashes the stored source, so a fingerprint + // taken after another module replaced `sourceData` would never match. + const chain: TranslationModule[] = provenanceModule ? [provenanceModule, ...modules] : modules; + refuseDuplicateNames(chain); + const { taskRunnerFactory, configModifier: runnerConfigModifier } = wireTranslateRunner({ translationProvider, schemaMap, - provenanceServiceFactory: provenanceModule.serviceFactory, + modules: chain, runner, lifecycle: lifecycle ?? {}, collections: Array.from(collectionSlugs), }); const autoTranslateModule = configureAutoTranslate(collections, schemaMap, taskRunnerFactory); - const activeLevels = levels ?? [documentLevel(), collectionLevel()]; const builder = new PluginConfigBuilder({ collections, settings, @@ -145,12 +186,11 @@ export function translatorPlugin( taskRunnerFactory, schemaMap, translationProvider, - provenanceServiceFactory: provenanceModule.serviceFactory, }); - for (const level of activeLevels) level.extend(builder); + for (const member of chain) member.setup?.(builder); + for (const component of components) component.attach(builder); builder.addConfigModifier(runnerConfigModifier); - builder.addConfigModifier(provenanceModule.configure(collectionSlugs)); builder.addConfigModifier(autoTranslateModule.configure(collectionSlugs)); builder.addAdminProvider(new CacheProviderExport(settings.basePath)); diff --git a/packages/payload-plugin-translator/src/server/features/cancel-by-collection/route.test.ts b/packages/payload-plugin-translator/src/server/features/cancel-by-collection/route.test.ts index d28d7bf98..85448a5e7 100644 --- a/packages/payload-plugin-translator/src/server/features/cancel-by-collection/route.test.ts +++ b/packages/payload-plugin-translator/src/server/features/cancel-by-collection/route.test.ts @@ -6,7 +6,9 @@ import type { TaskRunnerFactory } from "../../modules/task-runner/index.js"; import { createCancelByCollectionRoute } from "./route.js"; import type { CancelConfig } from "./model.js"; +import { AnyAccessGuard } from "../../shared/access/AnyAccessGuard.js"; +const OPEN_ON_PURPOSE = new AnyAccessGuard(); // Route-contract test: pin path + method + access-guard wiring so the Phase 1 // relocation of route registration stays behaviour-preserving. Handler logic // and the shared wrappers have their own unit tests. @@ -15,13 +17,18 @@ const denyAccess: AccessGuard = { check: vi.fn().mockReturnValue(false) }; describe("createCancelByCollectionRoute (contract)", () => { it("registers DELETE at {basePath}/cancel-by-collection/:collection_slug with the default basePath", () => { - const endpoint = createCancelByCollectionRoute(config, { create: vi.fn() }); + const endpoint = createCancelByCollectionRoute(config, { create: vi.fn() }, OPEN_ON_PURPOSE); expect(endpoint.path).toBe("/translate/cancel-by-collection/:collection_slug"); expect(endpoint.method).toBe("delete"); }); it("honors a custom basePath", () => { - const endpoint = createCancelByCollectionRoute(config, { create: vi.fn() }, undefined, "/i18n"); + const endpoint = createCancelByCollectionRoute( + config, + { create: vi.fn() }, + OPEN_ON_PURPOSE, + "/i18n" + ); expect(endpoint.path).toBe("/i18n/cancel-by-collection/:collection_slug"); }); diff --git a/packages/payload-plugin-translator/src/server/features/cancel-by-collection/route.ts b/packages/payload-plugin-translator/src/server/features/cancel-by-collection/route.ts index fb02c7fbf..8164bc6c3 100644 --- a/packages/payload-plugin-translator/src/server/features/cancel-by-collection/route.ts +++ b/packages/payload-plugin-translator/src/server/features/cancel-by-collection/route.ts @@ -13,7 +13,7 @@ import { CancelByCollectionHandler } from "./handler.js"; export function createCancelByCollectionRoute( config: CancelConfig, taskRunnerFactory: TaskRunnerFactory, - access?: AccessGuard, + access: AccessGuard, basePath = "/translate" ): Endpoint { const handler = new CancelByCollectionHandler(config, taskRunnerFactory); diff --git a/packages/payload-plugin-translator/src/server/features/cancel/route.test.ts b/packages/payload-plugin-translator/src/server/features/cancel/route.test.ts index 6445f8a73..ff2aee16d 100644 --- a/packages/payload-plugin-translator/src/server/features/cancel/route.test.ts +++ b/packages/payload-plugin-translator/src/server/features/cancel/route.test.ts @@ -5,7 +5,9 @@ import type { AccessGuard } from "../../shared/index.js"; import type { TaskRunnerFactory } from "../../modules/task-runner/index.js"; import { createCancelRoute } from "./route.js"; +import { AnyAccessGuard } from "../../shared/access/AnyAccessGuard.js"; +const OPEN_ON_PURPOSE = new AnyAccessGuard(); // Route-contract test: pin path + method + access-guard wiring so the Phase 1 // relocation of route registration stays behaviour-preserving. Handler logic // and the shared wrappers have their own unit tests. @@ -13,13 +15,13 @@ const denyAccess: AccessGuard = { check: vi.fn().mockReturnValue(false) }; describe("createCancelRoute (contract)", () => { it("registers DELETE at {basePath}/cancel with the default basePath", () => { - const endpoint = createCancelRoute({ create: vi.fn() }); + const endpoint = createCancelRoute({ create: vi.fn() }, OPEN_ON_PURPOSE); expect(endpoint.path).toBe("/translate/cancel"); expect(endpoint.method).toBe("delete"); }); it("honors a custom basePath", () => { - const endpoint = createCancelRoute({ create: vi.fn() }, undefined, "/i18n"); + const endpoint = createCancelRoute({ create: vi.fn() }, OPEN_ON_PURPOSE, "/i18n"); expect(endpoint.path).toBe("/i18n/cancel"); }); diff --git a/packages/payload-plugin-translator/src/server/features/cancel/route.ts b/packages/payload-plugin-translator/src/server/features/cancel/route.ts index 5d9cd1b0d..ca8350cc8 100644 --- a/packages/payload-plugin-translator/src/server/features/cancel/route.ts +++ b/packages/payload-plugin-translator/src/server/features/cancel/route.ts @@ -11,7 +11,7 @@ import { CancelHandler } from "./handler.js"; */ export function createCancelRoute( taskRunnerFactory: TaskRunnerFactory, - access?: AccessGuard, + access: AccessGuard, basePath = "/translate" ): Endpoint { const handler = new CancelHandler(taskRunnerFactory); diff --git a/packages/payload-plugin-translator/src/server/features/createTranslationRoutes.test.ts b/packages/payload-plugin-translator/src/server/features/createTranslationRoutes.test.ts index e11188891..517ab99b8 100644 --- a/packages/payload-plugin-translator/src/server/features/createTranslationRoutes.test.ts +++ b/packages/payload-plugin-translator/src/server/features/createTranslationRoutes.test.ts @@ -3,7 +3,9 @@ import { describe, it, expect, vi } from "vitest"; import { resolveSettings } from "../../settings.js"; import { createTranslationRoutes } from "./createTranslationRoutes.js"; import type { TranslationRoutesDeps } from "./createTranslationRoutes.js"; +import { AnyAccessGuard } from "../shared/access/AnyAccessGuard.js"; +const OPEN_ON_PURPOSE = new AnyAccessGuard(); // Bundle-contract test: pins that the shared route bundle stays complete and // ordered, and threads basePath into every endpoint. The bundle registers via // the doc + collection levels (deduped), so "all endpoints, right paths/methods" @@ -11,6 +13,7 @@ import type { TranslationRoutesDeps } from "./createTranslationRoutes.js"; const deps = (basePath?: string): TranslationRoutesDeps => ({ taskRunnerFactory: { create: vi.fn() }, collectionConfig: { availableCollections: new Set() }, + access: OPEN_ON_PURPOSE, settings: resolveSettings({ basePath }), }); @@ -25,8 +28,6 @@ describe("createTranslationRoutes (bundle contract)", () => { "delete /translate/cancel-by-collection/:collection_slug", "get /translate/document/:collection_slug/:collection_id", "get /translate/collection/:collection_slug", - "get /translate/stale/:collection_slug/:collection_id", - "post /translate/stale/dismiss", ]); }); diff --git a/packages/payload-plugin-translator/src/server/features/createTranslationRoutes.ts b/packages/payload-plugin-translator/src/server/features/createTranslationRoutes.ts index cffd11f61..ce12a84a8 100644 --- a/packages/payload-plugin-translator/src/server/features/createTranslationRoutes.ts +++ b/packages/payload-plugin-translator/src/server/features/createTranslationRoutes.ts @@ -1,6 +1,6 @@ import type { CollectionSlug, Endpoint } from "payload"; -import type { TranslationContext } from "../modules/translation-levels/index.js"; +import type { ConfigTimeContext } from "../modules/plugin-config/index.js"; import { createEnqueueRoute } from "./enqueue-translation/index.js"; import { createRunRoute } from "./run-translation/index.js"; @@ -8,40 +8,22 @@ import { createCancelRoute } from "./cancel/index.js"; import { createCancelByCollectionRoute } from "./cancel-by-collection/index.js"; import { createGetDocumentStatusRoute } from "./get-document-status/index.js"; import { createGetCollectionStatusRoute } from "./get-collection-status/index.js"; -import { createGetDocumentStalenessRoute, createDismissStalenessRoute } from "./staleness/index.js"; export type TranslationRoutesDeps = Pick< - TranslationContext, - "taskRunnerFactory" | "access" | "provenanceServiceFactory" | "settings" + ConfigTimeContext, + "taskRunnerFactory" | "access" | "settings" > & { /** Collections the plugin manages — gates which slugs the routes accept. */ collectionConfig: { availableCollections: Set }; }; -/** - * The translator's HTTP surface as one shared bundle: the job-API endpoints - * (enqueue / run / cancel / cancel-by-collection / document-status / - * collection-status) plus the provenance-backed staleness endpoints - * (document-staleness / dismiss), all wired to the same access guard. - * - * Extracted from the plugin's `init()` so registration is a single unit. The - * document and collection levels each contribute this bundle via the level - * context; the plugin deduplicates by method + path, so it registers exactly - * once. The staleness endpoints always register; when provenance is disabled - * (`provenanceServiceFactory` absent) they simply report no staleness. - */ export function createTranslationRoutes({ taskRunnerFactory, collectionConfig, access, settings, - provenanceServiceFactory, }: TranslationRoutesDeps): Endpoint[] { const { basePath, failedWindowMs } = settings; - const stalenessConfig = { - availableCollections: collectionConfig.availableCollections, - provenanceServiceFactory, - }; return [ createEnqueueRoute(taskRunnerFactory, collectionConfig, access, basePath), createRunRoute(taskRunnerFactory, access, basePath), @@ -54,7 +36,5 @@ export function createTranslationRoutes({ access, basePath ), - createGetDocumentStalenessRoute(stalenessConfig, access, basePath), - createDismissStalenessRoute(stalenessConfig, access, basePath), ]; } diff --git a/packages/payload-plugin-translator/src/server/features/enqueue-translation/__tests__/route.test.ts b/packages/payload-plugin-translator/src/server/features/enqueue-translation/__tests__/route.test.ts index e4e30db26..db0826954 100644 --- a/packages/payload-plugin-translator/src/server/features/enqueue-translation/__tests__/route.test.ts +++ b/packages/payload-plugin-translator/src/server/features/enqueue-translation/__tests__/route.test.ts @@ -6,7 +6,9 @@ import type { TaskRunnerFactory } from "../../../modules/task-runner/index.js"; import { createEnqueueRoute } from "../route.js"; import type { EnqueueConfig } from "../model.js"; +import { AnyAccessGuard } from "../../../shared/access/AnyAccessGuard.js"; +const OPEN_ON_PURPOSE = new AnyAccessGuard(); // Route-contract tests: pin path + method + access-guard wiring so the Phase 1 // relocation of route registration stays behaviour-preserving. The handler // logic and the shared wrappers (withAccessCheck / withErrorHandler) are @@ -16,13 +18,13 @@ const denyAccess: AccessGuard = { check: vi.fn().mockReturnValue(false) }; describe("createEnqueueRoute (contract)", () => { it("registers POST at {basePath}/enqueue with the default basePath", () => { - const endpoint = createEnqueueRoute({ create: vi.fn() }, config); + const endpoint = createEnqueueRoute({ create: vi.fn() }, config, OPEN_ON_PURPOSE); expect(endpoint.path).toBe("/translate/enqueue"); expect(endpoint.method).toBe("post"); }); it("honors a custom basePath", () => { - const endpoint = createEnqueueRoute({ create: vi.fn() }, config, undefined, "/i18n"); + const endpoint = createEnqueueRoute({ create: vi.fn() }, config, OPEN_ON_PURPOSE, "/i18n"); expect(endpoint.path).toBe("/i18n/enqueue"); }); diff --git a/packages/payload-plugin-translator/src/server/features/enqueue-translation/route.ts b/packages/payload-plugin-translator/src/server/features/enqueue-translation/route.ts index 7548bb9a0..348a9e813 100644 --- a/packages/payload-plugin-translator/src/server/features/enqueue-translation/route.ts +++ b/packages/payload-plugin-translator/src/server/features/enqueue-translation/route.ts @@ -13,7 +13,7 @@ import { EnqueueTranslationHandler } from "./handler.js"; export function createEnqueueRoute( taskRunnerFactory: TaskRunnerFactory, config: EnqueueConfig, - access?: AccessGuard, + access: AccessGuard, basePath = "/translate" ): Endpoint { const handler = new EnqueueTranslationHandler(config, taskRunnerFactory); diff --git a/packages/payload-plugin-translator/src/server/features/get-collection-status/route.test.ts b/packages/payload-plugin-translator/src/server/features/get-collection-status/route.test.ts index 3c76f01bb..f1309bb2b 100644 --- a/packages/payload-plugin-translator/src/server/features/get-collection-status/route.test.ts +++ b/packages/payload-plugin-translator/src/server/features/get-collection-status/route.test.ts @@ -6,7 +6,9 @@ import type { TaskRunnerFactory } from "../../modules/task-runner/index.js"; import { createGetCollectionStatusRoute } from "./route.js"; import type { GetCollectionStatusConfig } from "./model.js"; +import { AnyAccessGuard } from "../../shared/access/AnyAccessGuard.js"; +const OPEN_ON_PURPOSE = new AnyAccessGuard(); // Route-contract test: pin path + method + access-guard wiring so the Phase 1 // relocation of route registration stays behaviour-preserving. Handler logic // and the shared wrappers have their own unit tests. @@ -15,9 +17,7 @@ const denyAccess: AccessGuard = { check: vi.fn().mockReturnValue(false) }; describe("createGetCollectionStatusRoute (contract)", () => { it("registers GET at {basePath}/collection/:collection_slug with the default basePath", () => { - const endpoint = createGetCollectionStatusRoute(config, { - create: vi.fn(), - }); + const endpoint = createGetCollectionStatusRoute(config, { create: vi.fn() }, OPEN_ON_PURPOSE); expect(endpoint.path).toBe("/translate/collection/:collection_slug"); expect(endpoint.method).toBe("get"); }); @@ -26,7 +26,7 @@ describe("createGetCollectionStatusRoute (contract)", () => { const endpoint = createGetCollectionStatusRoute( config, { create: vi.fn() }, - undefined, + OPEN_ON_PURPOSE, "/i18n" ); expect(endpoint.path).toBe("/i18n/collection/:collection_slug"); diff --git a/packages/payload-plugin-translator/src/server/features/get-collection-status/route.ts b/packages/payload-plugin-translator/src/server/features/get-collection-status/route.ts index fc474f841..a1d565e5c 100644 --- a/packages/payload-plugin-translator/src/server/features/get-collection-status/route.ts +++ b/packages/payload-plugin-translator/src/server/features/get-collection-status/route.ts @@ -13,7 +13,7 @@ import { GetCollectionStatusHandler } from "./handler.js"; export function createGetCollectionStatusRoute( config: GetCollectionStatusConfig, taskRunnerFactory: TaskRunnerFactory, - access?: AccessGuard, + access: AccessGuard, basePath = "/translate" ): Endpoint { const handler = new GetCollectionStatusHandler(config, taskRunnerFactory); diff --git a/packages/payload-plugin-translator/src/server/features/get-document-status/route.test.ts b/packages/payload-plugin-translator/src/server/features/get-document-status/route.test.ts index 470983acd..aa4a1e782 100644 --- a/packages/payload-plugin-translator/src/server/features/get-document-status/route.test.ts +++ b/packages/payload-plugin-translator/src/server/features/get-document-status/route.test.ts @@ -6,7 +6,9 @@ import type { TaskRunnerFactory } from "../../modules/task-runner/index.js"; import { createGetDocumentStatusRoute } from "./route.js"; import type { GetDocumentStatusConfig } from "./model.js"; +import { AnyAccessGuard } from "../../shared/access/AnyAccessGuard.js"; +const OPEN_ON_PURPOSE = new AnyAccessGuard(); // Route-contract test: pin path + method + access-guard wiring so the Phase 1 // relocation of route registration stays behaviour-preserving. Handler logic // and the shared wrappers have their own unit tests. @@ -15,13 +17,18 @@ const denyAccess: AccessGuard = { check: vi.fn().mockReturnValue(false) }; describe("createGetDocumentStatusRoute (contract)", () => { it("registers GET at {basePath}/document/:collection_slug/:collection_id with the default basePath", () => { - const endpoint = createGetDocumentStatusRoute(config, { create: vi.fn() }); + const endpoint = createGetDocumentStatusRoute(config, { create: vi.fn() }, OPEN_ON_PURPOSE); expect(endpoint.path).toBe("/translate/document/:collection_slug/:collection_id"); expect(endpoint.method).toBe("get"); }); it("honors a custom basePath", () => { - const endpoint = createGetDocumentStatusRoute(config, { create: vi.fn() }, undefined, "/i18n"); + const endpoint = createGetDocumentStatusRoute( + config, + { create: vi.fn() }, + OPEN_ON_PURPOSE, + "/i18n" + ); expect(endpoint.path).toBe("/i18n/document/:collection_slug/:collection_id"); }); diff --git a/packages/payload-plugin-translator/src/server/features/get-document-status/route.ts b/packages/payload-plugin-translator/src/server/features/get-document-status/route.ts index 175311f6b..570d52b43 100644 --- a/packages/payload-plugin-translator/src/server/features/get-document-status/route.ts +++ b/packages/payload-plugin-translator/src/server/features/get-document-status/route.ts @@ -13,7 +13,7 @@ import { GetDocumentStatusHandler } from "./handler.js"; export function createGetDocumentStatusRoute( config: GetDocumentStatusConfig, taskRunnerFactory: TaskRunnerFactory, - access?: AccessGuard, + access: AccessGuard, basePath = "/translate" ): Endpoint { const handler = new GetDocumentStatusHandler(config, taskRunnerFactory); diff --git a/packages/payload-plugin-translator/src/server/features/run-translation/route.test.ts b/packages/payload-plugin-translator/src/server/features/run-translation/route.test.ts index f72f3c054..bf1d54fb1 100644 --- a/packages/payload-plugin-translator/src/server/features/run-translation/route.test.ts +++ b/packages/payload-plugin-translator/src/server/features/run-translation/route.test.ts @@ -5,7 +5,9 @@ import type { AccessGuard } from "../../shared/index.js"; import type { TaskRunnerFactory } from "../../modules/task-runner/index.js"; import { createRunRoute } from "./route.js"; +import { AnyAccessGuard } from "../../shared/access/AnyAccessGuard.js"; +const OPEN_ON_PURPOSE = new AnyAccessGuard(); // Route-contract test: pin path + method + access-guard wiring so the Phase 1 // relocation of route registration stays behaviour-preserving. Handler logic // and the shared wrappers have their own unit tests. @@ -13,13 +15,13 @@ const denyAccess: AccessGuard = { check: vi.fn().mockReturnValue(false) }; describe("createRunRoute (contract)", () => { it("registers POST at {basePath}/run/:id with the default basePath", () => { - const endpoint = createRunRoute({ create: vi.fn() }); + const endpoint = createRunRoute({ create: vi.fn() }, OPEN_ON_PURPOSE); expect(endpoint.path).toBe("/translate/run/:id"); expect(endpoint.method).toBe("post"); }); it("honors a custom basePath", () => { - const endpoint = createRunRoute({ create: vi.fn() }, undefined, "/i18n"); + const endpoint = createRunRoute({ create: vi.fn() }, OPEN_ON_PURPOSE, "/i18n"); expect(endpoint.path).toBe("/i18n/run/:id"); }); diff --git a/packages/payload-plugin-translator/src/server/features/run-translation/route.ts b/packages/payload-plugin-translator/src/server/features/run-translation/route.ts index b1a0b909a..c3448da2e 100644 --- a/packages/payload-plugin-translator/src/server/features/run-translation/route.ts +++ b/packages/payload-plugin-translator/src/server/features/run-translation/route.ts @@ -11,7 +11,7 @@ import { RunTranslationHandler } from "./handler.js"; */ export function createRunRoute( taskRunnerFactory: TaskRunnerFactory, - access?: AccessGuard, + access: AccessGuard, basePath = "/translate" ): Endpoint { const handler = new RunTranslationHandler(taskRunnerFactory); diff --git a/packages/payload-plugin-translator/src/server/features/staleness/dismissStaleness.handler.ts b/packages/payload-plugin-translator/src/server/features/staleness/dismissStaleness.handler.ts index 476bd2d69..42fc3e385 100644 --- a/packages/payload-plugin-translator/src/server/features/staleness/dismissStaleness.handler.ts +++ b/packages/payload-plugin-translator/src/server/features/staleness/dismissStaleness.handler.ts @@ -22,8 +22,8 @@ export class DismissStalenessHandler { return ServerResponse.badRequest("Collection not available for translation"); } - const service = this.config.provenanceServiceFactory?.(req.payload); - await service?.dismiss( + const service = this.config.provenanceServiceFactory(req.payload); + await service.dismiss( { collectionSlug: collectionSlug, documentId: collection_id, diff --git a/packages/payload-plugin-translator/src/server/features/staleness/getDocumentStaleness.handler.ts b/packages/payload-plugin-translator/src/server/features/staleness/getDocumentStaleness.handler.ts index 80d96b859..0c0b6d723 100644 --- a/packages/payload-plugin-translator/src/server/features/staleness/getDocumentStaleness.handler.ts +++ b/packages/payload-plugin-translator/src/server/features/staleness/getDocumentStaleness.handler.ts @@ -27,10 +27,8 @@ export class GetDocumentStalenessHandler { } try { - const service = this.config.provenanceServiceFactory?.(req.payload); - const locales = service - ? await service.getStaleness(collectionSlug, collection_id, req.user) - : []; + const service = this.config.provenanceServiceFactory(req.payload); + const locales = await service.getStaleness(collectionSlug, collection_id, req.user); return ServerResponse.success({ locales }); } catch (error) { req.payload.logger.error({ diff --git a/packages/payload-plugin-translator/src/server/features/staleness/model.ts b/packages/payload-plugin-translator/src/server/features/staleness/model.ts index bb14a2ed3..98c6a7cf3 100644 --- a/packages/payload-plugin-translator/src/server/features/staleness/model.ts +++ b/packages/payload-plugin-translator/src/server/features/staleness/model.ts @@ -2,7 +2,7 @@ import { z } from "zod"; import type { CollectionSlug } from "payload"; import { JobIdSchema } from "../../shared/index.js"; -import type { TranslationContext } from "../../modules/translation-levels/index.js"; +import type { ProvenanceServiceFactory } from "../../modules/provenance/index.js"; /** * Route params for reading a document's per-locale staleness. @@ -21,11 +21,7 @@ export const DismissStalenessInputSchema = z.object({ target_lng: z.string().nonempty(), }); -/** - * Handler configuration. `provenanceServiceFactory` is absent when provenance is disabled — the - * handlers then report no staleness (empty), so the endpoint contract stays stable either way. - * The fingerprint policy + schema live inside {@link ProvenanceService}, so no `schemaMap` here. - */ -export type StalenessConfig = Pick & { +export type StalenessConfig = { + provenanceServiceFactory: ProvenanceServiceFactory; availableCollections: Set; }; diff --git a/packages/payload-plugin-translator/src/server/features/staleness/route.test.ts b/packages/payload-plugin-translator/src/server/features/staleness/route.test.ts index 2ad1e6dd1..701fcac9f 100644 --- a/packages/payload-plugin-translator/src/server/features/staleness/route.test.ts +++ b/packages/payload-plugin-translator/src/server/features/staleness/route.test.ts @@ -5,7 +5,9 @@ import type { AccessGuard } from "../../shared/index.js"; import { createGetDocumentStalenessRoute, createDismissStalenessRoute } from "./route.js"; import type { StalenessConfig } from "./model.js"; +import { AnyAccessGuard } from "../../shared/access/AnyAccessGuard.js"; +const OPEN_ON_PURPOSE = new AnyAccessGuard(); // Route-contract test: pin path + method + access-guard wiring so the Phase 1 // relocation of route registration stays behaviour-preserving. Handler logic // and the shared wrappers have their own unit tests. @@ -20,13 +22,13 @@ const makeConfig = (): StalenessConfig & { describe("createGetDocumentStalenessRoute (contract)", () => { it("registers GET at {basePath}/stale/:collection_slug/:collection_id with the default basePath", () => { - const endpoint = createGetDocumentStalenessRoute(makeConfig()); + const endpoint = createGetDocumentStalenessRoute(makeConfig(), OPEN_ON_PURPOSE); expect(endpoint.path).toBe("/translate/stale/:collection_slug/:collection_id"); expect(endpoint.method).toBe("get"); }); it("honors a custom basePath", () => { - const endpoint = createGetDocumentStalenessRoute(makeConfig(), undefined, "/i18n"); + const endpoint = createGetDocumentStalenessRoute(makeConfig(), OPEN_ON_PURPOSE, "/i18n"); expect(endpoint.path).toBe("/i18n/stale/:collection_slug/:collection_id"); }); @@ -44,13 +46,13 @@ describe("createGetDocumentStalenessRoute (contract)", () => { describe("createDismissStalenessRoute (contract)", () => { it("registers POST at {basePath}/stale/dismiss with the default basePath", () => { - const endpoint = createDismissStalenessRoute(makeConfig()); + const endpoint = createDismissStalenessRoute(makeConfig(), OPEN_ON_PURPOSE); expect(endpoint.path).toBe("/translate/stale/dismiss"); expect(endpoint.method).toBe("post"); }); it("honors a custom basePath", () => { - const endpoint = createDismissStalenessRoute(makeConfig(), undefined, "/i18n"); + const endpoint = createDismissStalenessRoute(makeConfig(), OPEN_ON_PURPOSE, "/i18n"); expect(endpoint.path).toBe("/i18n/stale/dismiss"); }); diff --git a/packages/payload-plugin-translator/src/server/features/staleness/route.ts b/packages/payload-plugin-translator/src/server/features/staleness/route.ts index f65bc05f4..adba78662 100644 --- a/packages/payload-plugin-translator/src/server/features/staleness/route.ts +++ b/packages/payload-plugin-translator/src/server/features/staleness/route.ts @@ -10,7 +10,7 @@ import { DismissStalenessHandler } from "./dismissStaleness.handler.js"; /** GET per-locale staleness for one document. */ export function createGetDocumentStalenessRoute( config: StalenessConfig, - access?: AccessGuard, + access: AccessGuard, basePath = "/translate" ): Endpoint { const handler = new GetDocumentStalenessHandler(config); @@ -25,7 +25,7 @@ export function createGetDocumentStalenessRoute( /** POST to dismiss (acknowledge) staleness of one target locale. */ export function createDismissStalenessRoute( config: StalenessConfig, - access?: AccessGuard, + access: AccessGuard, basePath = "/translate" ): Endpoint { const handler = new DismissStalenessHandler(config); diff --git a/packages/payload-plugin-translator/src/server/features/staleness/staleness.test.ts b/packages/payload-plugin-translator/src/server/features/staleness/staleness.test.ts index 888d4c43c..fb859296b 100644 --- a/packages/payload-plugin-translator/src/server/features/staleness/staleness.test.ts +++ b/packages/payload-plugin-translator/src/server/features/staleness/staleness.test.ts @@ -54,12 +54,10 @@ beforeEach(() => { const schemaMap = new Map([["posts", schema]]) as CollectionSchemaMap; -function makeConfig(store: ProvenanceStore | null): StalenessConfig { +function makeConfig(store: ProvenanceStore): StalenessConfig { return { availableCollections: new Set(["posts"]) as StalenessConfig["availableCollections"], - provenanceServiceFactory: store - ? (p) => new ProvenanceService(store, schemaMap, {}, provenanceIo(p)) - : undefined, + provenanceServiceFactory: (p) => new ProvenanceService(store, schemaMap, {}, provenanceIo(p)), }; } @@ -114,11 +112,6 @@ describe("GetDocumentStalenessHandler", () => { expect((await bodyOf(res)).locales).toEqual([]); }); - it("returns empty when provenance is disabled", async () => { - const res = await new GetDocumentStalenessHandler(makeConfig(null)).handle(readReq()); - expect((await bodyOf(res)).locales).toEqual([]); - }); - it("degrades to empty and logs when the store throws (e.g. missing table)", async () => { const store = makeStore({ findByDocument: vi.fn().mockRejectedValue(new Error("relation does not exist")), diff --git a/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.access.test.ts b/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.access.test.ts index 26d520694..e6d4f0230 100644 --- a/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.access.test.ts +++ b/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.access.test.ts @@ -44,6 +44,7 @@ describe("TranslateDocumentHandler — it asks before it writes", () => { targetLng: "de", strategy: "overwrite", publishOnTranslation: false, + previousAttempts: 0, ...over, }); diff --git a/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.enforce.test.ts b/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.enforce.test.ts index 9618d1540..d4d692950 100644 --- a/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.enforce.test.ts +++ b/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.enforce.test.ts @@ -8,12 +8,10 @@ import type { TranslateDocumentInput } from "../model.js"; import { asRequester } from "../../../shared/payload/RequestScope.shapes.js"; vi.mock("../../../../core/translation-pipeline/index.js", () => ({ - translateContent: vi - .fn() - .mockResolvedValue({ - translatedData: { title: "Titel" }, - leaves: [{ idPath: "title", answeredInFull: true }], - }), + translateContent: vi.fn().mockResolvedValue({ + translatedData: { title: "Titel" }, + leaves: [{ idPath: "title", answeredInFull: true }], + }), })); vi.mock("../translationPermission.js", () => ({ @@ -47,6 +45,7 @@ describe("TranslateDocumentHandler — where Payload's own enforcement is afford targetLng: "de", strategy: "overwrite", publishOnTranslation: false, + previousAttempts: 0, }); const writeArgs = () => diff --git a/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.moduleFailure.test.ts b/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.moduleFailure.test.ts new file mode 100644 index 000000000..b4844c9c3 --- /dev/null +++ b/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.moduleFailure.test.ts @@ -0,0 +1,98 @@ +import type { CollectionSlug, Payload } from "payload"; +import { APIError } from "payload"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { TranslationProvider } from "../../../../core/domain/translation-providers/index.js"; +import type { CollectionSchemaMap } from "../../../../types/CollectionSchemaMap.js"; +import type { ChainMember } from "../../../modules/translation-run/index.js"; +import { TranslateDocumentHandler } from "../handler.js"; +import type { TranslateDocumentInput } from "../model.js"; + +vi.mock("../../../../core/translation-pipeline/index.js", () => ({ + translateContent: vi.fn().mockResolvedValue({ + translatedData: { title: "Titel" }, + leaves: [{ idPath: "title", answeredInFull: true }], + }), +})); + +const INSIDE_THE_EDITORS_SAVE = { transactionID: "tx-42" }; + +const input = (): TranslateDocumentInput => ({ + collection: "posts" as CollectionSlug, + collectionId: "doc-123", + sourceLng: "en", + targetLng: "de", + strategy: "overwrite", + publishOnTranslation: false, + previousAttempts: 0, +}); + +const failingAt = (position: "beforeTranslation" | "afterSave", error: unknown): ChainMember => ({ + name: "bad", + [position]: async () => { + throw error; + }, +}); + +describe("a module that fails while the editor's save is still open", () => { + let payload: Payload; + let provider: TranslationProvider; + let schemaMap: CollectionSchemaMap; + + beforeEach(() => { + vi.clearAllMocks(); + provider = { translate: vi.fn().mockResolvedValue({}) }; + schemaMap = new Map([ + ["posts" as CollectionSlug, [{ name: "title", type: "text", localized: true }]], + ]) as CollectionSchemaMap; + payload = { + findByID: vi.fn().mockResolvedValue({ id: "doc-123", title: "Test" }), + update: vi.fn().mockResolvedValue({}), + logger: { error: vi.fn(), info: vi.fn(), warn: vi.fn() }, + collections: { posts: { config: { versions: { drafts: true } } } }, + } as unknown as Payload; + }); + + const handlerWith = (member: ChainMember) => + new TranslateDocumentHandler(provider, schemaMap, [member]); + + it("lets the failure out rather than reporting success over a transaction it may have taken", async () => { + const underTest = handlerWith(failingAt("beforeTranslation", new Error("boom"))); + + await expect(underTest.handle(payload, input(), INSIDE_THE_EDITORS_SAVE)).rejects.toThrow( + "boom" + ); + expect(payload.update, "nothing was written").not.toHaveBeenCalled(); + }); + + it("reports the failure rather than passing over it in silence", async () => { + const underTest = handlerWith(failingAt("beforeTranslation", new Error("boom"))); + + await expect(underTest.handle(payload, input(), INSIDE_THE_EDITORS_SAVE)).rejects.toThrow(); + + expect(payload.logger.error).toHaveBeenCalledWith(expect.objectContaining({ module: "bad" })); + }); + + it("swallows that same failure when there is no transaction to lose", async () => { + const underTest = handlerWith(failingAt("beforeTranslation", new Error("boom"))); + + await expect(underTest.handle(payload, input())).resolves.toEqual({ success: true }); + expect(payload.update).toHaveBeenCalledTimes(1); + }); + + it("keeps the translation when a module fails after the save, outside a transaction", async () => { + const underTest = handlerWith(failingAt("afterSave", new Error("boom"))); + + await expect(underTest.handle(payload, input())).resolves.toEqual({ success: true }); + expect(payload.update).toHaveBeenCalledTimes(1); + }); + + it("lets out a failed Payload operation, which has already ended that transaction", async () => { + const underTest = handlerWith(failingAt("beforeTranslation", new APIError("gone"))); + + await expect( + underTest.handle(payload, input(), INSIDE_THE_EDITORS_SAVE) + ).rejects.toBeInstanceOf(APIError); + expect(payload.update, "nothing was written").not.toHaveBeenCalled(); + }); +}); diff --git a/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.spend.test.ts b/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.spend.test.ts index f00c7497a..3b5803909 100644 --- a/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.spend.test.ts +++ b/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.spend.test.ts @@ -7,12 +7,10 @@ import type { CollectionSchemaMap } from "../../../../types/CollectionSchemaMap. import type { TranslateDocumentInput } from "../model.js"; vi.mock("../../../../core/translation-pipeline/index.js", () => ({ - translateContent: vi - .fn() - .mockResolvedValue({ - translatedData: { title: "Titel" }, - leaves: [{ idPath: "title", answeredInFull: true }], - }), + translateContent: vi.fn().mockResolvedValue({ + translatedData: { title: "Titel" }, + leaves: [{ idPath: "title", answeredInFull: true }], + }), })); vi.mock("../translationPermission.js", () => ({ @@ -42,6 +40,7 @@ describe("TranslateDocumentHandler — a refusal costs nothing at the provider", targetLng: "de", strategy: "overwrite", publishOnTranslation: false, + previousAttempts: 0, }); beforeEach(async () => { diff --git a/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.test.ts b/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.test.ts index 5c106b266..c07d6eb9a 100644 --- a/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.test.ts +++ b/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.test.ts @@ -14,6 +14,7 @@ import type { ProvenanceStore } from "../../../../core/domain/provenance/index.j import { ProvenanceService, provenanceIo } from "../../../modules/provenance/index.js"; import type { ProvenanceServiceFactory } from "../../../modules/provenance/index.js"; import type { TranslateDocumentInput } from "../model.js"; +import { provenanceInAChain } from "../../../modules/provenance/__tests__/fixtures/inAChain.js"; vi.mock("../../../../core/translation-pipeline/index.js", () => ({ translateContent: vi.fn().mockResolvedValue(null), @@ -45,6 +46,7 @@ describe("TranslateDocumentHandler", () => { targetLng: TARGET_LNG, strategy: "overwrite", publishOnTranslation: false, + previousAttempts: 0, ...overrides, }); @@ -402,7 +404,7 @@ describe("TranslateDocumentHandler", () => { new TranslateDocumentHandler( mockTranslationProvider, mockSchemaMap, - serviceFactory as unknown as ProvenanceServiceFactory + provenanceInAChain(serviceFactory as unknown as ProvenanceServiceFactory) ); beforeEach(() => { @@ -597,7 +599,7 @@ describe("TranslateDocumentHandler", () => { const underTest = new TranslateDocumentHandler( mockTranslationProvider, mockSchemaMap, - serviceFactory as unknown as ProvenanceServiceFactory + provenanceInAChain(serviceFactory as unknown as ProvenanceServiceFactory) ); await underTest.handle(mockPayload, createInput({})); @@ -611,7 +613,7 @@ describe("TranslateDocumentHandler", () => { }); }); - it("hands the pipeline the leaves the receipt can answer for, and no others", async () => { + it("hands the pipeline every leaf the source has now, marking an unrecorded one changed", async () => { const store = { upsert: vi.fn(), find: vi.fn().mockResolvedValue({ @@ -649,14 +651,14 @@ describe("TranslateDocumentHandler", () => { const underTest = new TranslateDocumentHandler( mockTranslationProvider, mockSchemaMap, - serviceFactory as unknown as ProvenanceServiceFactory + provenanceInAChain(serviceFactory as unknown as ProvenanceServiceFactory) ); await underTest.handle(mockPayload, createInput({})); const passed = (translateContent as unknown as ReturnType).mock.calls[0][0] as { sourceChangedByLeaf?: Record; }; - expect(passed.sourceChangedByLeaf).toEqual({ title: true }); + expect(passed.sourceChangedByLeaf).toEqual({ title: true, tagline: true }); }); }); @@ -690,7 +692,7 @@ describe("TranslateDocumentHandler", () => { const underTest = new TranslateDocumentHandler( mockTranslationProvider, mockSchemaMap, - serviceFactory as unknown as ProvenanceServiceFactory + provenanceInAChain(serviceFactory as unknown as ProvenanceServiceFactory) ); await expect(underTest.handle(mockPayload, createInput({}))).resolves.toEqual({ diff --git a/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.transaction.test.ts b/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.transaction.test.ts index 47b7175f0..3ae2c24f8 100644 --- a/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.transaction.test.ts +++ b/packages/payload-plugin-translator/src/server/features/translate-document/__tests__/handler.transaction.test.ts @@ -6,14 +6,13 @@ import type { TranslationProvider } from "../../../../core/domain/translation-pr import type { CollectionSchemaMap } from "../../../../types/CollectionSchemaMap.js"; import type { TranslateDocumentInput } from "../model.js"; import type { ProvenanceServiceFactory } from "../../../modules/provenance/index.js"; +import { provenanceInAChain } from "../../../modules/provenance/__tests__/fixtures/inAChain.js"; vi.mock("../../../../core/translation-pipeline/index.js", () => ({ - translateContent: vi - .fn() - .mockResolvedValue({ - translatedData: { title: "Titel" }, - leaves: [{ idPath: "title", answeredInFull: true }], - }), + translateContent: vi.fn().mockResolvedValue({ + translatedData: { title: "Titel" }, + leaves: [{ idPath: "title", answeredInFull: true }], + }), })); const TX = "tx-42"; @@ -29,6 +28,7 @@ describe("TranslateDocumentHandler — the caller's transaction", () => { targetLng: "de", strategy: "overwrite", publishOnTranslation: false, + previousAttempts: 0, ...over, }); @@ -98,7 +98,7 @@ describe("TranslateDocumentHandler — the caller's transaction", () => { new Map([ ["posts" as CollectionSlug, [{ name: "title", type: "text", localized: true }]], ]) as CollectionSchemaMap, - serviceFactory as unknown as ProvenanceServiceFactory + provenanceInAChain(serviceFactory as unknown as ProvenanceServiceFactory) ); await provenanceHandler.handle(payload, input(), { transactionID: TX }); diff --git a/packages/payload-plugin-translator/src/server/features/translate-document/handler.ts b/packages/payload-plugin-translator/src/server/features/translate-document/handler.ts index 39fdca2b1..7de3a1501 100644 --- a/packages/payload-plugin-translator/src/server/features/translate-document/handler.ts +++ b/packages/payload-plugin-translator/src/server/features/translate-document/handler.ts @@ -6,7 +6,8 @@ import { APIError } from "payload"; import type { Handler } from "../../shared/index.js"; import type { TranslationProvider } from "../../../core/domain/translation-providers/index.js"; import { translateContent } from "../../../core/translation-pipeline/index.js"; -import type { ProvenanceServiceFactory } from "../../modules/provenance/index.js"; +import type { ChainMember } from "../../modules/translation-run/index.js"; +import { runAfterSave, runBeforeTranslation } from "../../modules/translation-run/index.js"; import { fetchSourceDocument } from "../../shared/payload/sourceDocument.js"; import type { RequestScope } from "../../shared/payload/RequestScope.shapes.js"; import { freshReq } from "../../shared/payload/RequestScope.shapes.js"; @@ -21,7 +22,6 @@ import { AUTO_TRANSLATE_SKIP_CONTEXT_KEY } from "../../../types/AutoTranslateCon import type { TranslateDocumentInput, TranslateDocumentOutput } from "./model.js"; import { resolveTargetLayer } from "./targetLayer.js"; import type { PublishScope, TargetLayer } from "./targetLayer.js"; -import { changedLeaves } from "../../../core/domain/provenance/index.js"; const translatorWriteContext = () => ({ [AUTO_TRANSLATE_SKIP_CONTEXT_KEY]: true }); @@ -43,16 +43,16 @@ export class TranslateDocumentHandler implements Handler< > { private readonly translationProvider: TranslationProvider; private readonly schemaMap: CollectionSchemaMap; - private readonly provenanceServiceFactory?: ProvenanceServiceFactory; + private readonly modules: readonly ChainMember[]; constructor( translationProvider: TranslationProvider, schemaMap: CollectionSchemaMap, - provenanceServiceFactory?: ProvenanceServiceFactory + modules: readonly ChainMember[] = [] ) { this.translationProvider = translationProvider; this.schemaMap = schemaMap; - this.provenanceServiceFactory = provenanceServiceFactory; + this.modules = modules; } async handle( @@ -60,19 +60,23 @@ export class TranslateDocumentHandler implements Handler< input: TranslateDocumentInput, scope: RequestScope = {} ): Promise { - const { collection, collectionId, sourceLng, targetLng, strategy, publishOnTranslation } = - input; + const { + collection, + collectionId, + sourceLng, + targetLng, + strategy, + publishOnTranslation, + previousAttempts, + } = input; const schema = this.schemaMap.get(collection); if (!schema) { throw new TranslatorConfigError(`Collection "${collection}" not found in schemaMap`); } - const layer = resolveTargetLayer({ - versions: payload.collections[collection].config.versions, - targetLng, - }); - + const versions = payload.collections[collection].config.versions; + const layer = resolveTargetLayer({ versions, targetLng }); const requester = await rebuildRequester(payload, scope); const [sourceData, currentTargetVersion] = await Promise.all([ @@ -109,14 +113,17 @@ export class TranslateDocumentHandler implements Handler< }); if (!allowed) throw new TranslationRefused(collection, targetLng); - const provenance = this.provenanceServiceFactory?.(payload, scope); - const provenanceKey = { - collectionSlug: collection, - documentId: String(collectionId), + const target = { + collection, + documentId: collectionId, + sourceLocale: sourceLng, targetLocale: targetLng, }; - const currentFields = provenance?.captureFingerprint(collection, sourceData) ?? null; - const previous = provenance ? await provenance.lastTranslatedFrom(provenanceKey) : null; + const joined = await runBeforeTranslation( + this.modules, + { payload, target, scope, sourceData, previousAttempts, moduleState: {} }, + payload.logger + ); const translated = await this.translateOrWrap({ schema, @@ -127,7 +134,7 @@ export class TranslateDocumentHandler implements Handler< document: { collection, id: collectionId }, translationProvider: this.translationProvider, strategy, - sourceChangedByLeaf: currentFields ? changedLeaves(previous, currentFields) : undefined, + sourceChangedByLeaf: joined.sourceChangedByLeaf, }); if (translated?.translatedData) { @@ -142,13 +149,11 @@ export class TranslateDocumentHandler implements Handler< requester ); - if (provenance && currentFields !== null) { - await provenance.record( - { ...provenanceKey, sourceLocale: sourceLng }, - currentFields, - leaves - ); - } + await runAfterSave( + this.modules, + { payload, target, scope, moduleState: joined.moduleState, leaves }, + payload.logger + ); } if (publishOnTranslation && layer.kind === "drafts") { diff --git a/packages/payload-plugin-translator/src/server/features/translate-document/model.ts b/packages/payload-plugin-translator/src/server/features/translate-document/model.ts index d419bb22e..5d4dc16c8 100644 --- a/packages/payload-plugin-translator/src/server/features/translate-document/model.ts +++ b/packages/payload-plugin-translator/src/server/features/translate-document/model.ts @@ -12,6 +12,7 @@ export type TranslateDocumentInput = { targetLng: string; strategy: TranslationStrategyName; publishOnTranslation: boolean; + previousAttempts: number; }; /** diff --git a/packages/payload-plugin-translator/src/server/features/translate-document/wireTranslateRunner.ts b/packages/payload-plugin-translator/src/server/features/translate-document/wireTranslateRunner.ts index 7de0b018f..fc590d002 100644 --- a/packages/payload-plugin-translator/src/server/features/translate-document/wireTranslateRunner.ts +++ b/packages/payload-plugin-translator/src/server/features/translate-document/wireTranslateRunner.ts @@ -3,7 +3,7 @@ import type { CollectionSlug } from "payload"; import type { TranslationProvider } from "../../../core/domain/translation-providers/index.js"; import type { CollectionSchemaMap } from "../../../types/CollectionSchemaMap.js"; import type { ConfigModifier } from "../../../types/ConfigModifier.js"; -import type { ProvenanceServiceFactory } from "../../modules/provenance/index.js"; +import type { ChainMember } from "../../modules/translation-run/index.js"; import { LifecycleNotifier, taskFromAssignment } from "../../modules/lifecycle/index.js"; import type { TranslationLifecycleCallbacks } from "../../modules/lifecycle/index.js"; import type { @@ -17,7 +17,7 @@ import { TranslateDocumentHandler } from "./handler.js"; type WireTranslateRunnerParams = { translationProvider: TranslationProvider; schemaMap: CollectionSchemaMap; - provenanceServiceFactory?: ProvenanceServiceFactory; + modules: readonly ChainMember[]; runner: TaskRunnerProvider; lifecycle: TranslationLifecycleCallbacks; collections: CollectionSlug[]; @@ -31,7 +31,7 @@ type WireTranslateRunnerParams = { export function wireTranslateRunner({ translationProvider, schemaMap, - provenanceServiceFactory, + modules, runner, lifecycle, collections, @@ -39,11 +39,7 @@ export function wireTranslateRunner({ taskRunnerFactory: TaskRunnerFactory; configModifier: ConfigModifier; } { - const translateHandler = new TranslateDocumentHandler( - translationProvider, - schemaMap, - provenanceServiceFactory - ); + const translateHandler = new TranslateDocumentHandler(translationProvider, schemaMap, modules); const runnerContext: TaskRunnerContext = { handler: async (payload, input, scope) => { @@ -56,6 +52,7 @@ export function wireTranslateRunner({ targetLng: input.targetLng, strategy: input.strategy, publishOnTranslation: input.publishOnTranslation, + previousAttempts: input.previousAttempts, }, scope ); diff --git a/packages/payload-plugin-translator/src/server/features/translate-field/__tests__/route.test.ts b/packages/payload-plugin-translator/src/server/features/translate-field/__tests__/route.test.ts index a113109f4..fa18eeb72 100644 --- a/packages/payload-plugin-translator/src/server/features/translate-field/__tests__/route.test.ts +++ b/packages/payload-plugin-translator/src/server/features/translate-field/__tests__/route.test.ts @@ -6,7 +6,9 @@ import type { TranslationProvider } from "../../../../core/domain/translation-pr import type { CollectionSchemaMap } from "../../../../types/CollectionSchemaMap.js"; import { createFieldRoute } from "../route.js"; +import { AnyAccessGuard } from "../../../shared/access/AnyAccessGuard.js"; +const OPEN_ON_PURPOSE = new AnyAccessGuard(); // Route-contract tests: pin path + method + access-guard wiring. Handler logic // and the shared wrappers are covered by their own unit tests. const schemaMap = new Map([ @@ -20,6 +22,7 @@ describe("createFieldRoute (contract)", () => { const endpoint = createFieldRoute({ schemaMap, translationProvider: provider, + access: OPEN_ON_PURPOSE, }); expect(endpoint.path).toBe("/translate/field"); expect(endpoint.method).toBe("post"); @@ -29,6 +32,7 @@ describe("createFieldRoute (contract)", () => { const endpoint = createFieldRoute({ schemaMap, translationProvider: provider, + access: OPEN_ON_PURPOSE, basePath: "/i18n", }); expect(endpoint.path).toBe("/i18n/field"); diff --git a/packages/payload-plugin-translator/src/server/features/translate-field/route.ts b/packages/payload-plugin-translator/src/server/features/translate-field/route.ts index dabb2d822..c6a0cf359 100644 --- a/packages/payload-plugin-translator/src/server/features/translate-field/route.ts +++ b/packages/payload-plugin-translator/src/server/features/translate-field/route.ts @@ -7,7 +7,7 @@ import type { FieldTranslationConfig } from "./model.js"; import { TranslateFieldHandler } from "./handler.js"; export type CreateFieldRouteArgs = FieldTranslationConfig & { - access?: AccessGuard; + access: AccessGuard; basePath?: string; }; diff --git a/packages/payload-plugin-translator/src/server/modules/translation-levels/PluginConfigBuilder.test.ts b/packages/payload-plugin-translator/src/server/modules/plugin-config/PluginConfigBuilder.test.ts similarity index 97% rename from packages/payload-plugin-translator/src/server/modules/translation-levels/PluginConfigBuilder.test.ts rename to packages/payload-plugin-translator/src/server/modules/plugin-config/PluginConfigBuilder.test.ts index 8d57ba325..acf6af0fd 100644 --- a/packages/payload-plugin-translator/src/server/modules/translation-levels/PluginConfigBuilder.test.ts +++ b/packages/payload-plugin-translator/src/server/modules/plugin-config/PluginConfigBuilder.test.ts @@ -5,10 +5,12 @@ import type { RawPayloadComponentExport } from "../../../types/PayloadComponentE import { resolveSettings } from "../../../settings.js"; import { PluginConfigBuilder } from "./PluginConfigBuilder.js"; +import { AnyAccessGuard } from "../../shared/access/AnyAccessGuard.js"; const deps = (collections: Array<{ slug: string }> = []) => ({ collections: collections as unknown as CollectionConfig[], settings: resolveSettings({}), + access: new AnyAccessGuard(), taskRunnerFactory: { create: vi.fn() }, schemaMap: new Map(), translationProvider: { translate: vi.fn() }, @@ -63,9 +65,8 @@ describe("PluginConfigBuilder", () => { expect(other.admin).toBeUndefined(); // unmanaged collection untouched }); - it("deduplicates a collection component declared twice for the same slot + path (duplicate level)", () => { + it("deduplicates a collection component declared twice for the same slot + path", () => { const builder = new PluginConfigBuilder(deps([{ slug: "posts" }])); - // Two documentLevel()-style registrations: distinct `make` closures, same slot + component path. builder.addCollectionComponent("beforeDocumentControls", () => component("doc")); builder.addCollectionComponent("beforeDocumentControls", () => component("doc")); diff --git a/packages/payload-plugin-translator/src/server/modules/translation-levels/PluginConfigBuilder.ts b/packages/payload-plugin-translator/src/server/modules/plugin-config/PluginConfigBuilder.ts similarity index 74% rename from packages/payload-plugin-translator/src/server/modules/translation-levels/PluginConfigBuilder.ts rename to packages/payload-plugin-translator/src/server/modules/plugin-config/PluginConfigBuilder.ts index b9318dff1..56e806b5e 100644 --- a/packages/payload-plugin-translator/src/server/modules/translation-levels/PluginConfigBuilder.ts +++ b/packages/payload-plugin-translator/src/server/modules/plugin-config/PluginConfigBuilder.ts @@ -6,26 +6,25 @@ import type { CollectionSchemaMap } from "../../../types/CollectionSchemaMap.js" import type { ConfigModifier } from "../../../types/ConfigModifier.js"; import type { TranslationProvider } from "../../../core/domain/translation-providers/index.js"; import type { TaskRunnerFactory } from "../task-runner/index.js"; -import type { ProvenanceServiceFactory } from "../provenance/index.js"; import type { TranslatorSettings } from "../../../settings.js"; -import type { CollectionAdminSlot, LevelContext, TranslationContext } from "./types.js"; +import type { + CollectionAdminSlot, + ComponentContext, + ModuleContext, + ConfigTimeContext, +} from "./types.js"; type CollectionComponent = { slot: CollectionAdminSlot; make: (collection: CollectionConfig) => RawPayloadComponentExport; }; -/** The builder's construction deps are exactly the shared {@link TranslationContext}. */ -export type PluginConfigBuilderDeps = TranslationContext; +export type PluginConfigBuilderDeps = ConfigTimeContext; const endpointKey = (endpoint: Endpoint): string => `${endpoint.method} ${endpoint.path}`; -/** - * Identity of a collection admin component for dedup: its slot plus its module path (+ export name). - * `serverProps` (per-collection data) are excluded, so the same control registered by a level declared - * twice collapses to one — while genuinely different controls (distinct path or slot) do not. - */ +/** `serverProps` is left out, so the same control registered twice collapses to one. */ const componentKey = (slot: CollectionAdminSlot, component: RawPayloadComponentExport): string => `${slot} ${component.path}#${component.exportName ?? ""}`; @@ -51,29 +50,17 @@ function attachToSlot( return; } - // Exhaustiveness: a new CollectionAdminSlot must be handled above, not fall through. slot satisfies never; } -/** - * The single place that mutates the Payload `config`. Levels (through the narrow - * {@link LevelContext}) and the plugin (through `addAdminProvider` / - * `addConfigModifier`) only *describe* their contributions; `applyTo(config)` is - * the one sink that writes them in. This keeps Payload's in-place-mutation style - * — including the lazy nested-config init — confined here, so `plugin.ts` and the - * levels stay free of direct config mutation. - * - * It implements {@link LevelContext}, so the same instance is what levels receive - * as their context (they see only `addEndpoints` / `addCollectionComponent`). - */ -export class PluginConfigBuilder implements LevelContext { +/** The only place that mutates the Payload `config`; modules and components merely describe contributions. */ +export class PluginConfigBuilder implements ComponentContext, ModuleContext { readonly collections: CollectionConfig[]; readonly settings: TranslatorSettings; - readonly access?: AccessGuard; + readonly access: AccessGuard; readonly taskRunnerFactory: TaskRunnerFactory; readonly schemaMap: CollectionSchemaMap; readonly translationProvider: TranslationProvider; - readonly provenanceServiceFactory?: ProvenanceServiceFactory; private readonly endpoints: Endpoint[] = []; private readonly collectionComponents: CollectionComponent[] = []; @@ -87,7 +74,6 @@ export class PluginConfigBuilder implements LevelContext { this.taskRunnerFactory = deps.taskRunnerFactory; this.schemaMap = deps.schemaMap; this.translationProvider = deps.translationProvider; - this.provenanceServiceFactory = deps.provenanceServiceFactory; } addEndpoints(endpoints: Endpoint[]): void { @@ -135,12 +121,12 @@ export class PluginConfigBuilder implements LevelContext { const managed = new Set(this.collections.map((collection) => collection.slug)); config.collections?.forEach((collection) => { if (!managed.has(collection.slug)) return; - const seen = new Set(); + const seenOnThisCollection = new Set(); for (const { slot, make } of this.collectionComponents) { const component = make(collection); const key = componentKey(slot, component); - if (seen.has(key)) continue; - seen.add(key); + if (seenOnThisCollection.has(key)) continue; + seenOnThisCollection.add(key); attachToSlot(collection, slot, component); } }); diff --git a/packages/payload-plugin-translator/src/server/modules/plugin-config/index.ts b/packages/payload-plugin-translator/src/server/modules/plugin-config/index.ts new file mode 100644 index 000000000..ea93f203a --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/plugin-config/index.ts @@ -0,0 +1,6 @@ +export type { + ModuleContext, + TranslationComponent, + ConfigTimeContext, + TranslationModule, +} from "./types.js"; diff --git a/packages/payload-plugin-translator/src/server/modules/plugin-config/types.ts b/packages/payload-plugin-translator/src/server/modules/plugin-config/types.ts new file mode 100644 index 000000000..c10027ded --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/plugin-config/types.ts @@ -0,0 +1,61 @@ +import type { CollectionConfig, Endpoint } from "payload"; + +import type { AccessGuard } from "../../../types/AccessGuard.js"; +import type { ConfigModifier } from "../../../types/ConfigModifier.js"; +import type { RawPayloadComponentExport } from "../../../types/PayloadComponentExport.js"; +import type { CollectionSchemaMap } from "../../../types/CollectionSchemaMap.js"; +import type { TranslationProvider } from "../../../core/domain/translation-providers/index.js"; +import type { TaskRunnerFactory } from "../task-runner/index.js"; +import type { ChainMember } from "../translation-run/index.js"; +import type { TranslatorSettings } from "../../../settings.js"; + +export type CollectionAdminSlot = "beforeDocumentControls" | "beforeListTable"; + +export type ConfigTimeContext = { + readonly collections: CollectionConfig[]; + readonly settings: TranslatorSettings; + readonly access: AccessGuard; + readonly taskRunnerFactory: TaskRunnerFactory; + readonly schemaMap: CollectionSchemaMap; + readonly translationProvider: TranslationProvider; +}; + +/** + * What a module may contribute while the config is being built. + * @internal + */ +export interface ModuleContext extends ConfigTimeContext { + /** Deduplicated by method + path when applied. */ + addEndpoints(endpoints: Endpoint[]): void; + addConfigModifier(modifier: ConfigModifier): void; +} + +/** + * What an admin component may contribute: a slot on each managed collection, nothing on the server. + * @internal + */ +export interface ComponentContext extends ConfigTimeContext { + addCollectionComponent( + slot: CollectionAdminSlot, + make: (collection: CollectionConfig) => RawPayloadComponentExport + ): void; +} + +/** + * Create one with a built-in factory and list it in `translatorPlugin({ modules })`; writing your own + * is not supported yet. + * + * @since 1.0.0 + */ +export interface TranslationModule extends ChainMember { + setup?(ctx: ModuleContext): void; +} + +/** + * An admin control the plugin attaches to every managed collection. + * + * @since 1.0.0 + */ +export interface TranslationComponent { + attach(ctx: ComponentContext): void; +} diff --git a/packages/payload-plugin-translator/src/server/modules/provenance/Provenance.module.ts b/packages/payload-plugin-translator/src/server/modules/provenance/Provenance.module.ts new file mode 100644 index 000000000..19e2ca499 --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/provenance/Provenance.module.ts @@ -0,0 +1,90 @@ +import type { FieldFingerprints } from "../../../core/domain/content-projection/computeFieldFingerprints.js"; +import { changedLeaves, recordsEachLeaf } from "../../../core/domain/provenance/index.js"; +import type { ProvenanceKey } from "../../../core/domain/provenance/index.js"; +import { + createDismissStalenessRoute, + createGetDocumentStalenessRoute, +} from "../../features/staleness/index.js"; +import type { ConfigModifier } from "../../../types/ConfigModifier.js"; +import type { ModuleContext, TranslationModule } from "../plugin-config/index.js"; +import type { BeforeTranslation, RunTarget } from "../translation-run/index.js"; + +import type { ProvenanceServiceFactory } from "./Provenance.wiring.js"; + +export const PROVENANCE_MODULE = "provenance"; + +const isFingerprints = (recorded: unknown): recorded is FieldFingerprints => { + if (typeof recorded !== "object" || recorded === null) return false; + return Object.values(recorded).every((hash) => typeof hash === "string"); +}; + +const receiptKey = (target: RunTarget): ProvenanceKey => ({ + collectionSlug: target.collection, + documentId: String(target.documentId), + targetLocale: target.targetLocale, +}); + +type ProvenanceModuleDeps = { + serviceFactory: ProvenanceServiceFactory; + refuseACollidingSlug: ConfigModifier; + addTheCollection: ConfigModifier; + cleanUpOnDelete: (managedSlugs: Set) => ConfigModifier; +}; + +/** + * The source is fingerprinted before the provider is called and that value is what the receipt + * records, so a pipeline writing into the document it was handed cannot change the receipt. + */ +export function provenanceModule({ + serviceFactory, + refuseACollidingSlug, + addTheCollection, + cleanUpOnDelete, +}: ProvenanceModuleDeps): TranslationModule { + return { + name: PROVENANCE_MODULE, + + setup(ctx: ModuleContext) { + const managedSlugs = new Set(ctx.collections.map((collection) => collection.slug)); + const config = { + availableCollections: managedSlugs, + provenanceServiceFactory: serviceFactory, + }; + ctx.addEndpoints([ + createGetDocumentStalenessRoute(config, ctx.access, ctx.settings.basePath), + createDismissStalenessRoute(config, ctx.access, ctx.settings.basePath), + ]); + // refuseACollidingSlug first: a collision must leave the host's config untouched. + ctx.addConfigModifier(refuseACollidingSlug); + ctx.addConfigModifier(addTheCollection); + ctx.addConfigModifier(cleanUpOnDelete(managedSlugs)); + }, + + async beforeTranslation(state: BeforeTranslation) { + const service = serviceFactory(state.payload, state.scope); + const currentFields = service.captureFingerprint(state.target.collection, state.sourceData); + if (currentFields === null) return {}; + + const fingerprints = { moduleState: { [PROVENANCE_MODULE]: currentFields } }; + const previous = await service.lastTranslatedFrom(receiptKey(state.target)); + if (!recordsEachLeaf(previous)) return fingerprints; + + return { + ...fingerprints, + sourceChangedByLeaf: changedLeaves(previous.hashes, currentFields), + }; + }, + + async afterSave(state) { + const fingerprints = state.moduleState[PROVENANCE_MODULE]; + if (!isFingerprints(fingerprints)) return; + + const service = serviceFactory(state.payload, state.scope); + await service.record( + { ...receiptKey(state.target), sourceLocale: state.target.sourceLocale }, + fingerprints, + state.leaves + ); + }, + }; +} diff --git a/packages/payload-plugin-translator/src/server/modules/provenance/Provenance.wiring.ts b/packages/payload-plugin-translator/src/server/modules/provenance/Provenance.wiring.ts index dde28442b..225f9acc4 100644 --- a/packages/payload-plugin-translator/src/server/modules/provenance/Provenance.wiring.ts +++ b/packages/payload-plugin-translator/src/server/modules/provenance/Provenance.wiring.ts @@ -2,6 +2,9 @@ import type { Payload } from "payload"; import type { CollectionSchemaMap } from "../../../types/CollectionSchemaMap.js"; import type { ConfigModifier } from "../../../types/ConfigModifier.js"; +import type { TranslationModule } from "../plugin-config/index.js"; + +import { provenanceModule } from "./Provenance.module.js"; import { ProvenanceService } from "./Provenance.service.js"; import { PayloadProvenanceStore } from "./Provenance.store.js"; @@ -30,19 +33,6 @@ function resolveProvenanceSlug(option: ProvenanceOption): string | null { return option.slug || DEFAULT_PROVENANCE_SLUG; } -/** - * Everything the provenance module contributes at config time, in one object (mirrors - * `TaskRunnerProvider.configure`): the request-scoped {@link ProvenanceService} factory used by the - * handlers/routes, and the single {@link ConfigModifier} that registers the sidecar collection and - * the cleanup hook. When provenance is disabled, `serviceFactory` is absent and `configure` is a no-op. - */ -export type ProvenanceModule = { - serviceFactory?: ProvenanceServiceFactory; - configure(managedSlugs: Set): ConfigModifier; -}; - -const NOOP: ConfigModifier = (config) => config; - const OUTSIDE_THE_CALLERS_TRANSACTION = undefined; /** The two things {@link ProvenanceService} needs from Payload, bound to one instance. */ @@ -64,35 +54,47 @@ export type ProvenanceServiceFactory = ( scope?: RequestScope ) => ProvenanceService; -/** - * Turn the opt-in `provenance` option into a self-contained {@link ProvenanceModule}. This is the one - * place provenance's config-time wiring lives — `plugin.ts` only calls `configureProvenance(...)` and - * registers the returned modifier through the shared builder (no raw `config.collections` mutation). - */ export function configureProvenance( option: ProvenanceOption, schemaMap: CollectionSchemaMap -): ProvenanceModule { +): TranslationModule | undefined { const slug = resolveProvenanceSlug(option); - if (!slug) return { configure: () => NOOP }; + if (!slug) return undefined; + + const storeFactory: ProvenanceStoreFactory = (payload, scope) => { + return new PayloadProvenanceStore(payload, slug, scope); + }; + + const serviceFactory: ProvenanceServiceFactory = (payload, scope = {}) => { + const store = storeFactory(payload, scope); + const io = provenanceIo(payload); + return new ProvenanceService(store, schemaMap, scope, io); + }; - const storeFactory: ProvenanceStoreFactory = (payload, scope) => - new PayloadProvenanceStore(payload, slug, scope); + const refuseACollidingSlug: ConfigModifier = (config) => { + const theHostsOwn = (config.collections ?? []).filter( + (collection) => !isProvenanceCollection(collection) + ); + assertProvenanceSlugFree(slug, theHostsOwn); + return config; + }; - const serviceFactory: ProvenanceServiceFactory = (payload, scope = {}) => - new ProvenanceService(storeFactory(payload, scope), schemaMap, scope, provenanceIo(payload)); + const addTheCollection: ConfigModifier = (config) => { + ensureProvenanceCollectionRegistered(config, slug); + return config; + }; - const configure = + const cleanUpOnDelete = (managedSlugs: Set): ConfigModifier => (config) => { - assertProvenanceSlugFree( - slug, - (config.collections ?? []).filter((collection) => !isProvenanceCollection(collection)) - ); - ensureProvenanceCollectionRegistered(config, slug); injectProvenanceCleanup(config, managedSlugs, storeFactory, slug); return config; }; - return { serviceFactory, configure }; + return provenanceModule({ + serviceFactory, + refuseACollidingSlug, + addTheCollection, + cleanUpOnDelete, + }); } diff --git a/packages/payload-plugin-translator/src/server/modules/provenance/__tests__/Provenance.module.test.ts b/packages/payload-plugin-translator/src/server/modules/provenance/__tests__/Provenance.module.test.ts new file mode 100644 index 000000000..f7af6a7c2 --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/provenance/__tests__/Provenance.module.test.ts @@ -0,0 +1,123 @@ +import type { Field, Payload } from "payload"; +import { describe, expect, it, vi } from "vitest"; + +import type { CollectionSchemaMap } from "../../../../types/CollectionSchemaMap.js"; +import type { ConfigModifier } from "../../../../types/ConfigModifier.js"; +import type { + ProvenanceReceipt, + ProvenanceStore, + SourceFingerprint, +} from "../../../../core/domain/provenance/index.js"; +import type { BeforeTranslation, RunContribution } from "../../translation-run/index.js"; +import type { FieldFingerprints } from "../../../../core/domain/content-projection/computeFieldFingerprints.js"; +import { PROVENANCE_MODULE, provenanceModule } from "../Provenance.module.js"; +import { ProvenanceService } from "../Provenance.service.js"; + +const COLLECTION = "posts"; +const NOT_IN_A_TRANSACTION = {}; +const UNCHANGED: ConfigModifier = (config) => config; + +const schema: Field[] = [ + { name: "title", type: "text", localized: true }, + { name: "body", type: "text", localized: true }, +]; + +const receipt = (sourceFingerprint: SourceFingerprint | null): ProvenanceReceipt => ({ + collectionSlug: COLLECTION, + documentId: "doc-1", + targetLocale: "de", + sourceLocale: "en", + sourceFingerprint, + translatedAt: "2026-10-22T00:00:00.000Z", + dismissedFingerprint: null, +}); + +const moduleReading = (stored: SourceFingerprint | null) => { + const store = { + upsert: vi.fn().mockResolvedValue(undefined), + find: vi.fn().mockResolvedValue(stored === null ? null : receipt(stored)), + findByDocument: vi.fn().mockResolvedValue([]), + dismiss: vi.fn().mockResolvedValue(undefined), + deleteByDocument: vi.fn().mockResolvedValue(undefined), + } as unknown as ProvenanceStore; + + return provenanceModule({ + serviceFactory: () => + new ProvenanceService( + store, + new Map([[COLLECTION, schema]]) as CollectionSchemaMap, + NOT_IN_A_TRANSACTION, + { logger: { error: vi.fn() }, readSource: vi.fn() } + ), + refuseACollidingSlug: UNCHANGED, + addTheCollection: UNCHANGED, + cleanUpOnDelete: () => UNCHANGED, + }); +}; + +const run = (over: Partial = {}): BeforeTranslation => ({ + payload: {} as Payload, + target: { + collection: COLLECTION, + documentId: "doc-1", + sourceLocale: "en", + targetLocale: "de", + }, + scope: NOT_IN_A_TRANSACTION, + sourceData: { title: "Hello", body: "World" }, + previousAttempts: 0, + moduleState: {}, + ...over, +}); + +const joinedWith = async ( + stored: SourceFingerprint | null, + over: Partial = {} +) => { + const tenant = moduleReading(stored); + if (!tenant.beforeTranslation) throw new Error("the module must take the first position"); + return tenant.beforeTranslation(run(over)); +}; + +const fingerprintsLeftBy = (contribution: RunContribution): FieldFingerprints => { + const left = contribution.moduleState?.[PROVENANCE_MODULE]; + if (left === undefined) throw new Error("the module must leave its fingerprints for the write"); + return left as FieldFingerprints; +}; + +describe("what provenance tells the pipeline about drift", () => { + it("answers leaf by leaf when the receipt records leaf by leaf", async () => { + const asTranslated = fingerprintsLeftBy(await joinedWith(null)); + + const joined = await joinedWith({ kind: "fields", hashes: asTranslated }); + + expect(joined.sourceChangedByLeaf).toEqual({ title: false, body: false }); + }); + + it("marks the leaf whose source moved since that receipt", async () => { + const asTranslated = fingerprintsLeftBy(await joinedWith(null)); + + const joined = await joinedWith( + { kind: "fields", hashes: asTranslated }, + { sourceData: { title: "Hello", body: "Moved" } } + ); + + expect(joined.sourceChangedByLeaf).toEqual({ title: false, body: true }); + }); + + it("says nothing at all when no receipt exists", async () => { + expect((await joinedWith(null)).sourceChangedByLeaf).toBeUndefined(); + }); + + it("says nothing at all for a receipt written before per-leaf fingerprints", async () => { + const legacy: SourceFingerprint = { kind: "document", hash: "a".repeat(64) }; + + expect((await joinedWith(legacy)).sourceChangedByLeaf).toBeUndefined(); + }); + + it("still leaves the fingerprints behind for the write, whatever the receipt was", async () => { + expect(fingerprintsLeftBy(await joinedWith(null))).toEqual( + expect.objectContaining({ title: expect.any(String), body: expect.any(String) }) + ); + }); +}); diff --git a/packages/payload-plugin-translator/src/server/modules/provenance/__tests__/fixtures/inAChain.ts b/packages/payload-plugin-translator/src/server/modules/provenance/__tests__/fixtures/inAChain.ts new file mode 100644 index 000000000..1b7b26680 --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/provenance/__tests__/fixtures/inAChain.ts @@ -0,0 +1,15 @@ +import type { ConfigModifier } from "../../../../../types/ConfigModifier.js"; +import type { ChainMember } from "../../../translation-run/index.js"; +import { provenanceModule } from "../../Provenance.module.js"; +import type { ProvenanceServiceFactory } from "../../Provenance.wiring.js"; + +const UNCHANGED: ConfigModifier = (config) => config; + +export const provenanceInAChain = (serviceFactory: ProvenanceServiceFactory): ChainMember[] => [ + provenanceModule({ + serviceFactory, + refuseACollidingSlug: UNCHANGED, + addTheCollection: UNCHANGED, + cleanUpOnDelete: () => UNCHANGED, + }), +]; diff --git a/packages/payload-plugin-translator/src/server/modules/provenance/index.ts b/packages/payload-plugin-translator/src/server/modules/provenance/index.ts index 01c8f5231..eaf278b0a 100644 --- a/packages/payload-plugin-translator/src/server/modules/provenance/index.ts +++ b/packages/payload-plugin-translator/src/server/modules/provenance/index.ts @@ -11,9 +11,6 @@ export { injectProvenanceCleanup, makeProvenanceCleanupHook } from "./Provenance export { assertProvenanceSlugFree } from "./slugGuard.js"; export { ProvenanceService } from "./Provenance.service.js"; export type { StalenessLocale } from "./Provenance.service.js"; +export { PROVENANCE_MODULE, provenanceModule } from "./Provenance.module.js"; export { configureProvenance, provenanceIo } from "./Provenance.wiring.js"; -export type { - ProvenanceModule, - ProvenanceOption, - ProvenanceServiceFactory, -} from "./Provenance.wiring.js"; +export type { ProvenanceOption, ProvenanceServiceFactory } from "./Provenance.wiring.js"; diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/TaskRunnerProvider.interface.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/TaskRunnerProvider.interface.ts index cf6a1456e..6be7739bd 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/TaskRunnerProvider.interface.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/TaskRunnerProvider.interface.ts @@ -14,6 +14,8 @@ export type TaskHandlerInput = { targetLng: string; strategy: TranslationStrategyName; publishOnTranslation: boolean; + /** Zero on the first attempt, and on any runner that does not retry. */ + previousAttempts: number; }; /** diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsRunnerProvider.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsRunnerProvider.ts index 14b3b6deb..ea8bd775f 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsRunnerProvider.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsRunnerProvider.ts @@ -15,7 +15,14 @@ import { withLegacyCollection, WorkflowInputSchema, } from "./store/index.js"; -import { deliveredLocales, localesAsTheyStand, owedIfGaveUp, retryLimitOf } from "./model/index.js"; +import type { TaskStatusByLocale } from "./store/index.js"; +import { + attemptsBefore, + deliveredLocales, + localesAsTheyStand, + owedIfGaveUp, + retryLimitOf, +} from "./model/index.js"; import type { TaskRunnerContext, TaskRunnerProvider } from "../TaskRunnerProvider.interface.js"; import type { TaskEvent } from "../types.js"; import type { Requester } from "../../../shared/payload/RequestScope.shapes.js"; @@ -63,7 +70,7 @@ const defaultValues = { */ type TranslateLocaleTask = { req: { payload: Payload }; - job?: { id?: unknown }; + job?: { id?: unknown; taskStatus?: TaskStatusByLocale }; input: StoredTaskInput & { strategy: TranslationStrategyName }; }; @@ -135,6 +142,9 @@ export class PayloadJobsRunnerProvider implements TaskRunnerProvider { targetLng: args.input.target_lng, strategy: args.input.strategy, publishOnTranslation: args.input.publish_on_translation ?? false, + previousAttempts: attemptsBefore( + args.job?.taskStatus?.[taskName]?.[args.input.target_lng] + ), }, { requester: requesterOf(args.input) } ); diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/__tests__/attemptCount.test.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/__tests__/attemptCount.test.ts new file mode 100644 index 000000000..2c7d28e3d --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/__tests__/attemptCount.test.ts @@ -0,0 +1,63 @@ +import type { Config, Payload } from "payload"; +import { describe, expect, it, vi } from "vitest"; + +import { createPayloadJobsRunner } from "../PayloadJobsRunnerProvider.js"; +import type { TaskHandlerInput } from "../../TaskRunnerProvider.interface.js"; +import type { TaskRunnerContext } from "../../TaskRunnerProvider.interface.js"; + +const TASK_SLUG = "translate_document"; + +const registeredTask = (handler: TaskRunnerContext["handler"]) => { + const config = { jobs: {} } as unknown as Config; + const context: TaskRunnerContext = { + handler, + report: vi.fn().mockResolvedValue(undefined), + collections: ["posts"], + }; + createPayloadJobsRunner().configure(context)(config); + const task = config.jobs?.tasks?.find((candidate) => candidate.slug === TASK_SLUG); + if (!task) throw new Error("the translate task was not registered"); + return task; +}; + +const runTheTask = async (job: unknown): Promise => { + const handler = vi.fn().mockResolvedValue(undefined); + const task = registeredTask(handler); + + await (task.handler as (args: unknown) => Promise)({ + req: { payload: {} as Payload }, + job, + input: { + collection_slug: "posts", + collection_id: "doc-1", + source_lng: "en", + target_lng: "de", + strategy: "overwrite", + publish_on_translation: false, + }, + }); + + return handler.mock.calls[0][1] as TaskHandlerInput; +}; + +describe("what a run is told about the attempt it is on", () => { + it("says zero on a first run, where Payload has recorded nothing yet", async () => { + expect((await runTheTask(undefined)).previousAttempts).toBe(0); + }); + + it("says zero when the row exists but this locale has not been tried", async () => { + expect((await runTheTask({ taskStatus: {} })).previousAttempts).toBe(0); + }); + + it("says one on the first retry of that locale", async () => { + const job = { taskStatus: { [TASK_SLUG]: { de: { totalTried: 1 } } } }; + + expect((await runTheTask(job)).previousAttempts).toBe(1); + }); + + it("counts the locale that is running, not a sibling locale of the same run", async () => { + const job = { taskStatus: { [TASK_SLUG]: { fr: { totalTried: 3 } } } }; + + expect((await runTheTask(job)).previousAttempts).toBe(0); + }); +}); diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/model/attemptsBefore.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/model/attemptsBefore.ts new file mode 100644 index 000000000..46e4e9c76 --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/model/attemptsBefore.ts @@ -0,0 +1,7 @@ +/** + * Payload's `totalTried` counts the attempts before the current one and is absent on the first, so a + * first run reads zero (`payload/dist/queues/errors/handleTaskError.js:54`). + */ +export const attemptsBefore = (tried: { totalTried?: unknown } | undefined): number => { + return typeof tried?.totalTried === "number" ? tried.totalTried : 0; +}; diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/model/index.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/model/index.ts index f7c213b8b..8190493bc 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/model/index.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/model/index.ts @@ -3,6 +3,7 @@ export type { EnqueuePlan, RequestShape } from "./planEnqueue.js"; export { alreadyCovered } from "./alreadyCovered.js"; export { retryLimitOf } from "./retryLimitOf.js"; export { isLastAttempt } from "./isLastAttempt.js"; +export { attemptsBefore } from "./attemptsBefore.js"; export { stillOwed } from "./stillOwed.js"; export { owedIfGaveUp } from "./owedIfGaveUp.js"; export { deliveredLocales } from "./deliveredLocales.js"; diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/model/isLastAttempt.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/model/isLastAttempt.ts index 6d429a431..1a63a42a4 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/model/isLastAttempt.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/model/isLastAttempt.ts @@ -1,3 +1,5 @@ +import { attemptsBefore } from "./attemptsBefore.js"; + /** * Whether the attempt that just threw was the run's last for this locale. * @@ -7,6 +9,5 @@ export type IsLastAttempt = (tried: { totalTried?: unknown } | undefined, limit: number) => boolean; export const isLastAttempt: IsLastAttempt = (tried, limit) => { - const before = typeof tried?.totalTried === "number" ? tried.totalTried : 0; - return before >= limit; + return attemptsBefore(tried) >= limit; }; diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/store/index.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/store/index.ts index 980d0e76c..0991d33c1 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/store/index.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/store/index.ts @@ -1,4 +1,10 @@ -export type { PayloadJob, StoredWorkflowInput, StoredTaskInput, JobLogEntry } from "./types.js"; +export type { + PayloadJob, + StoredWorkflowInput, + StoredTaskInput, + JobLogEntry, + TaskStatusByLocale, +} from "./types.js"; export { TaskInputSchema, WorkflowInputSchema } from "./jobInput.schema.js"; export { toPayloadFields, withLegacyCollection } from "./toPayloadFields.js"; export { handleOf } from "./handleOf.js"; diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/store/types.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/store/types.ts index de3843fcf..0b189ee44 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/store/types.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/store/types.ts @@ -7,13 +7,15 @@ import type { StoredInputSchema, WorkflowInputSchema } from "./jobInput.schema.j type TaskSlug = string; type TaskId = string; +export type TaskStatusByLocale = Record>; + export type PayloadJob = { /** Payload sets this once a run's retries are spent, and never clears it. */ hasError?: boolean; log?: JobLogEntry[]; /** How many times the whole run has been executed — Payload's second give-up counter. */ totalTried?: unknown; - taskStatus?: Record>; + taskStatus?: TaskStatusByLocale; id: string | number; completedAt?: string | null; createdAt: string; diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/sync-runner/SyncTaskRunner.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/sync-runner/SyncTaskRunner.ts index 5acea0b5b..3f6beb1cb 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/sync-runner/SyncTaskRunner.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/sync-runner/SyncTaskRunner.ts @@ -8,6 +8,8 @@ import type { LazyMap } from "../../../shared/utils/index.js"; import type { RequestScope } from "../../../shared/payload/RequestScope.shapes.js"; import { swallowOrThrow } from "../../../shared/payload/swallowOrThrow.js"; +const NEVER_RETRIED = 0; + /** Runs each task inline on enqueue; results live only in memory, so status queries see nothing from a previous process. */ export class SyncTaskRunner implements TaskRunner { private readonly payload: Payload; @@ -70,6 +72,7 @@ export class SyncTaskRunner implements TaskRunner { targetLng: input.targetLng, strategy: input.strategy, publishOnTranslation: input.publishOnTranslation, + previousAttempts: NEVER_RETRIED, }, scope ); diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/sync-runner/__tests__/SyncTaskRunner.test.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/sync-runner/__tests__/SyncTaskRunner.test.ts index 17ea5d065..43b7b406f 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/sync-runner/__tests__/SyncTaskRunner.test.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/sync-runner/__tests__/SyncTaskRunner.test.ts @@ -84,6 +84,7 @@ describe("SyncTaskRunner", () => { targetLng: "de", strategy: "overwrite", publishOnTranslation: false, + previousAttempts: 0, }, {} ); diff --git a/packages/payload-plugin-translator/src/server/modules/translation-levels/index.ts b/packages/payload-plugin-translator/src/server/modules/translation-levels/index.ts deleted file mode 100644 index ff987ff8e..000000000 --- a/packages/payload-plugin-translator/src/server/modules/translation-levels/index.ts +++ /dev/null @@ -1,6 +0,0 @@ -export type { - TranslationLevel, - LevelContext, - TranslationContext, - CollectionAdminSlot, -} from "./types.js"; diff --git a/packages/payload-plugin-translator/src/server/modules/translation-levels/types.ts b/packages/payload-plugin-translator/src/server/modules/translation-levels/types.ts deleted file mode 100644 index 0a46e152e..000000000 --- a/packages/payload-plugin-translator/src/server/modules/translation-levels/types.ts +++ /dev/null @@ -1,68 +0,0 @@ -import type { CollectionConfig, Endpoint } from "payload"; - -import type { AccessGuard } from "../../../types/AccessGuard.js"; -import type { RawPayloadComponentExport } from "../../../types/PayloadComponentExport.js"; -import type { CollectionSchemaMap } from "../../../types/CollectionSchemaMap.js"; -import type { TranslationProvider } from "../../../core/domain/translation-providers/index.js"; -import type { TaskRunnerFactory } from "../task-runner/index.js"; -import type { ProvenanceServiceFactory } from "../provenance/index.js"; -import type { TranslatorSettings } from "../../../settings.js"; - -export type CollectionAdminSlot = "beforeDocumentControls" | "beforeListTable"; - -/** - * The one bundle of config-time dependencies every translation surface / route / handler reads — - * the single source of truth that {@link LevelContext}, `PluginConfigBuilderDeps`, `StalenessConfig` - * and `TranslationRoutesDeps` all derive from (via `extends`/`Pick`), so a field can never drift - * between them. - */ -export type TranslationContext = { - readonly collections: CollectionConfig[]; - /** The plugin's resolved settings, relayed whole — see `src/settings.ts`. */ - readonly settings: TranslatorSettings; - readonly access?: AccessGuard; - readonly taskRunnerFactory: TaskRunnerFactory; - /** Deep-cloned localized field schema per managed collection slug. */ - readonly schemaMap: CollectionSchemaMap; - /** The configured translation backend (used by the synchronous field level). */ - readonly translationProvider: TranslationProvider; - /** Builds a provenance service; absent when provenance is disabled (staleness then reports empty). */ - readonly provenanceServiceFactory?: ProvenanceServiceFactory; -}; - -/** - * A composable translation surface (document / collection / field). - * - * Treat it as an **opaque value**: create one with `documentLevel()` / - * `collectionLevel()` (and, later, `fieldLevel()`) and list it in - * `translatorPlugin({ levels })`. It is produced only by those built-in - * factories — not externally implementable yet, since {@link LevelContext} is - * intentionally not exported. Opening it later (export `LevelContext`, drop the - * `@internal` tag) is additive and non-breaking. - * - * @since 0.5.0 - */ -export interface TranslationLevel { - /** - * Not a stable contract yet — produce via the level factories. - * @internal - */ - extend(ctx: LevelContext): void; -} - -/** - * Public-ready shape, kept internal until external levels are needed. - * - * A level contributes via these generic primitives; it never mutates the raw - * Payload config. The plugin deduplicates endpoints by method + path on apply. - * @internal - */ -export interface LevelContext extends TranslationContext { - /** Register endpoints. Deduplicated by method + path when applied. */ - addEndpoints(endpoints: Endpoint[]): void; - /** Attach an admin component to a slot on every managed collection. */ - addCollectionComponent( - slot: CollectionAdminSlot, - make: (collection: CollectionConfig) => RawPayloadComponentExport - ): void; -} diff --git a/packages/payload-plugin-translator/src/server/modules/translation-run/TranslationRun.interface.ts b/packages/payload-plugin-translator/src/server/modules/translation-run/TranslationRun.interface.ts new file mode 100644 index 000000000..aef44a920 --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/translation-run/TranslationRun.interface.ts @@ -0,0 +1,57 @@ +import type { CollectionSlug, Payload } from "payload"; + +import type { ChangedLeaves } from "../../../core/domain/provenance/index.js"; +import type { LeafOutcome } from "../../../core/translation-pipeline/types/index.js"; +import type { ID } from "../task-runner/types.js"; +import type { RequestScope } from "../../shared/payload/RequestScope.shapes.js"; + +/** + * A module's working value for one run, keyed by module name. Per-run state lives here, never on the + * module: one instance serves every concurrent translation. + */ +export type ModuleState = Readonly>; + +export type RunTarget = { + readonly collection: CollectionSlug; + readonly documentId: ID; + readonly sourceLocale: string; + readonly targetLocale: string; +}; + +export type BeforeTranslation = { + readonly payload: Payload; + readonly target: RunTarget; + readonly scope: RequestScope; + /** The source document as the run read it, in the source locale. */ + readonly sourceData: Record; + readonly previousAttempts: number; + readonly moduleState: ModuleState; + readonly sourceChangedByLeaf?: ChangedLeaves; +}; + +export type AfterSave = { + readonly payload: Payload; + readonly target: RunTarget; + readonly scope: RequestScope; + readonly moduleState: ModuleState; + /** What the provider answered for each leaf the run sent. */ + readonly leaves: readonly LeafOutcome[]; +}; + +/** What a module adds to the run; the chain reads nothing else back from it. */ +export type RunContribution = { + readonly moduleState?: ModuleState; + readonly sourceChangedByLeaf?: ChangedLeaves; +}; + +/** A module must survive a replay: a retried job runs `beforeTranslation` again. */ +export type ChainMember = { + /** Names this module's slot in {@link ModuleState}; unique across the chain. */ + readonly name: string; + beforeTranslation?(state: BeforeTranslation): Promise; + afterSave?(state: AfterSave): Promise; +}; + +export type RunLogger = { + error(details: Record): void; +}; diff --git a/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/chainOrder.test.ts b/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/chainOrder.test.ts new file mode 100644 index 000000000..e0541e11f --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/chainOrder.test.ts @@ -0,0 +1,117 @@ +import { describe, expect, it } from "vitest"; + +import type { ChainMember } from "../index.js"; +import { runAfterSave, runBeforeTranslation } from "../index.js"; + +import { afterSave, beforeTranslation, collectingLogger } from "./fixtures/run.js"; + +const silent = collectingLogger(); + +describe("the chain before the translation", () => { + it("returns the state untouched when no module is listed", async () => { + const state = beforeTranslation(); + + expect(await runBeforeTranslation([], state, silent)).toEqual(state); + }); + + it("keeps the leaf-change map when a later module contributes only its own note", async () => { + const changed = { title: true }; + const mapper: ChainMember = { + name: "mapper", + beforeTranslation: async () => ({ sourceChangedByLeaf: changed }), + }; + const noteOnly: ChainMember = { + name: "note-only", + beforeTranslation: async () => ({ moduleState: { noteOnly: 1 } }), + }; + + const joined = await runBeforeTranslation([mapper, noteOnly], beforeTranslation(), silent); + + expect(joined.sourceChangedByLeaf).toEqual(changed); + }); + + it("keeps a sibling's note when a module contributes only its own", async () => { + const first: ChainMember = { + name: "first", + beforeTranslation: async () => ({ moduleState: { first: 1 } }), + }; + const second: ChainMember = { + name: "second", + beforeTranslation: async () => ({ moduleState: { second: 2 } }), + }; + + const joined = await runBeforeTranslation([first, second], beforeTranslation(), silent); + + expect(joined.moduleState).toEqual({ first: 1, second: 2 }); + }); + + it("runs the modules in the order they are listed", async () => { + const order: string[] = []; + const record = (name: string): ChainMember => ({ + name, + beforeTranslation: async () => { + order.push(name); + return {}; + }, + }); + + await runBeforeTranslation( + [record("a"), record("b"), record("c")], + beforeTranslation(), + silent + ); + + expect(order).toEqual(["a", "b", "c"]); + }); + + it("carries one module's note through to the next", async () => { + const writer: ChainMember = { + name: "writer", + beforeTranslation: async () => ({ moduleState: { writer: "left behind" } }), + }; + let read: unknown; + const reader: ChainMember = { + name: "reader", + beforeTranslation: async (state) => { + read = state.moduleState.writer; + return {}; + }, + }; + + await runBeforeTranslation([writer, reader], beforeTranslation(), silent); + + expect(read).toBe("left behind"); + }); + + it("passes over a module that does not take this position", async () => { + const afterOnly: ChainMember = { name: "after-only", afterSave: async () => undefined }; + const state = beforeTranslation(); + + expect(await runBeforeTranslation([afterOnly], state, silent)).toEqual(state); + }); +}); + +describe("the chain after the save", () => { + it("runs every module that takes the position", async () => { + const ran: string[] = []; + const record = (name: string): ChainMember => ({ + name, + afterSave: async () => { + ran.push(name); + }, + }); + + await runAfterSave([record("a"), record("b")], afterSave(), silent); + + expect(ran).toEqual(["a", "b"]); + }); + + it("passes over a module that does not take this position", async () => { + const beforeOnly: ChainMember = { + name: "before-only", + beforeTranslation: async () => ({}), + }; + + await expect(runAfterSave([beforeOnly], afterSave(), silent)).resolves.toBeUndefined(); + }); +}); diff --git a/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/fixtures/failures.ts b/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/fixtures/failures.ts new file mode 100644 index 000000000..8c8a28bcb --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/fixtures/failures.ts @@ -0,0 +1,10 @@ +import { TranslatorError } from "../../../../../core/errors/index.js"; + +export class PluginFailure extends TranslatorError { + readonly mustPropagate: boolean; + + constructor(message: string, mustPropagate: boolean) { + super(message); + this.mustPropagate = mustPropagate; + } +} diff --git a/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/fixtures/run.ts b/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/fixtures/run.ts new file mode 100644 index 000000000..e1cc010ae --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/fixtures/run.ts @@ -0,0 +1,36 @@ +import type { Payload } from "payload"; + +import type { AfterSave, BeforeTranslation, RunLogger } from "../../TranslationRun.interface.js"; + +const NOT_READ_BY_THE_CHAIN = {} as Payload; + +const TARGET = { + collection: "pages", + documentId: "doc-1", + sourceLocale: "en", + targetLocale: "de", +} as const; + +export const beforeTranslation = (over: Partial = {}): BeforeTranslation => ({ + payload: NOT_READ_BY_THE_CHAIN, + target: TARGET, + scope: {}, + sourceData: { title: "Source" }, + previousAttempts: 0, + moduleState: {}, + ...over, +}); + +export const afterSave = (over: Partial = {}): AfterSave => ({ + payload: NOT_READ_BY_THE_CHAIN, + target: TARGET, + scope: {}, + moduleState: {}, + leaves: [], + ...over, +}); + +export const collectingLogger = (): RunLogger & { entries: Record[] } => { + const entries: Record[] = []; + return { entries, error: (details) => entries.push(details) }; +}; diff --git a/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/moduleFailure.test.ts b/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/moduleFailure.test.ts new file mode 100644 index 000000000..b9a18f30b --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/moduleFailure.test.ts @@ -0,0 +1,185 @@ +import { APIError } from "payload"; +import { describe, expect, it } from "vitest"; + +import type { ChainMember } from "../index.js"; +import { runAfterSave, runBeforeTranslation } from "../index.js"; + +import { PluginFailure } from "./fixtures/failures.js"; +import { afterSave, beforeTranslation, collectingLogger } from "./fixtures/run.js"; + +const IN_A_TRANSACTION = { transactionID: "tx-1" }; +const NOT_IN_A_TRANSACTION = {}; + +const throwing = (name: string, error: unknown): ChainMember => ({ + name, + beforeTranslation: async () => { + throw error; + }, + afterSave: async () => { + throw error; + }, +}); + +describe("a module that fails before the translation", () => { + it("leaves the chain carrying the state it was given, never undefined", async () => { + const state = beforeTranslation({ sourceData: { title: "Untouched" } }); + + const out = await runBeforeTranslation( + [throwing("bad", new Error("boom"))], + state, + collectingLogger() + ); + + expect(out).toEqual(state); + }); + + it("does not stop the module listed after it", async () => { + let reached = false; + const later: ChainMember = { + name: "later", + beforeTranslation: async () => { + reached = true; + return {}; + }, + }; + + await runBeforeTranslation( + [throwing("bad", new Error("boom")), later], + beforeTranslation(), + collectingLogger() + ); + + expect(reached).toBe(true); + }); + + it("keeps an earlier module's note when the next one fails", async () => { + const writer: ChainMember = { + name: "writer", + beforeTranslation: async () => ({ moduleState: { writer: "left behind" } }), + }; + let seen: unknown; + const reader: ChainMember = { + name: "reader", + beforeTranslation: async (state) => { + seen = state.moduleState.writer; + return {}; + }, + }; + + await runBeforeTranslation( + [writer, throwing("bad", new Error("boom")), reader], + beforeTranslation(), + collectingLogger() + ); + + expect(seen).toBe("left behind"); + }); + + it("reports the failure under the name of the module that raised it", async () => { + const log = collectingLogger(); + + await runBeforeTranslation([throwing("bad", new Error("boom"))], beforeTranslation(), log); + + expect(log.entries).toHaveLength(1); + expect(log.entries[0]).toMatchObject({ module: "bad" }); + }); + + it("lets an unrecognised failure out of an open transaction, rather than reporting success", async () => { + const state = beforeTranslation({ scope: IN_A_TRANSACTION }); + + await expect( + runBeforeTranslation([throwing("bad", new Error("boom"))], state, collectingLogger()) + ).rejects.toThrow("boom"); + }); + + it("lets a failed Payload operation out, because it already took that transaction", async () => { + const state = beforeTranslation({ scope: IN_A_TRANSACTION }); + + await expect( + runBeforeTranslation([throwing("bad", new APIError("gone"))], state, collectingLogger()) + ).rejects.toBeInstanceOf(APIError); + }); + + it("does not demote a failure a module raised deliberately", async () => { + const escalating: ChainMember = { + name: "escalating", + beforeTranslation: async () => { + throw new PluginFailure("the write is already gone", true); + }, + }; + const state = beforeTranslation({ scope: IN_A_TRANSACTION }); + + await expect( + runBeforeTranslation([escalating], state, collectingLogger()) + ).rejects.toBeInstanceOf(PluginFailure); + }); + + it("swallows even a failed Payload operation when there is no transaction to lose", async () => { + const state = beforeTranslation({ scope: NOT_IN_A_TRANSACTION }); + + await expect( + runBeforeTranslation([throwing("bad", new APIError("gone"))], state, collectingLogger()) + ).resolves.toEqual(state); + }); + + it("lets out a plugin failure that declares it must propagate", async () => { + const state = beforeTranslation({ scope: IN_A_TRANSACTION }); + const lostTheWrite = new PluginFailure("gone", true); + + await expect( + runBeforeTranslation([throwing("bad", lostTheWrite)], state, collectingLogger()) + ).rejects.toBe(lostTheWrite); + }); + + it("swallows a plugin failure that declares itself harmless", async () => { + const state = beforeTranslation({ scope: IN_A_TRANSACTION }); + + await expect( + runBeforeTranslation( + [throwing("bad", new PluginFailure("fine", false))], + state, + collectingLogger() + ) + ).resolves.toEqual(state); + }); +}); + +describe("a module that fails after the save", () => { + it("does not stop the module listed after it", async () => { + let reached = false; + const later: ChainMember = { + name: "later", + afterSave: async () => { + reached = true; + }, + }; + + await runAfterSave( + [throwing("bad", new Error("boom")), later], + afterSave(), + collectingLogger() + ); + + expect(reached).toBe(true); + }); + + it("lets an unrecognised failure out of an open transaction, rather than reporting success", async () => { + await expect( + runAfterSave( + [throwing("bad", new Error("boom"))], + afterSave({ scope: IN_A_TRANSACTION }), + collectingLogger() + ) + ).rejects.toThrow("boom"); + }); + + it("lets a failed Payload operation out, because it already took that transaction", async () => { + await expect( + runAfterSave( + [throwing("bad", new APIError("gone"))], + afterSave({ scope: IN_A_TRANSACTION }), + collectingLogger() + ) + ).rejects.toBeInstanceOf(APIError); + }); +}); diff --git a/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/seamShape.pins.ts b/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/seamShape.pins.ts new file mode 100644 index 000000000..a8ed5b49c --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/translation-run/__tests__/seamShape.pins.ts @@ -0,0 +1,29 @@ +/** + * Never imported and never run: `check-types` fails if any `@ts-expect-error` below stops being an + * error. + * + * What this file cannot pin: a module that returns the whole run state. A wider object is always + * assignable to a narrower one and excess-property checking does not reach a return position, so + * {@link RunContribution} states what the chain reads without being able to refuse more. + */ + +// @ts-expect-error the seam is not part of the published surface +import type { TranslationModule } from "../../../../index.js"; +// @ts-expect-error nor is the component contract +import type { TranslationComponent } from "../../../../index.js"; +// The positive control. Without it a path that stops resolving would satisfy both directives above +// with "cannot find module", and the two types could be published again unnoticed. +import type { TargetSelectionMode } from "../../../../index.js"; +import type { ChainMember } from "../index.js"; + +const _contributesOnlyItsOwn: ChainMember = { + name: "contributes", + beforeTranslation: async () => ({ moduleState: { mine: 1 } }), +}; + +const _contributesNothing: ChainMember = { + name: "contributes-nothing", + beforeTranslation: async () => ({}), +}; + +export type { TargetSelectionMode, TranslationComponent, TranslationModule }; diff --git a/packages/payload-plugin-translator/src/server/modules/translation-run/index.ts b/packages/payload-plugin-translator/src/server/modules/translation-run/index.ts new file mode 100644 index 000000000..312db4f77 --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/translation-run/index.ts @@ -0,0 +1,7 @@ +export type { + BeforeTranslation, + ChainMember, + RunContribution, + RunTarget, +} from "./TranslationRun.interface.js"; +export { runAfterSave, runBeforeTranslation } from "./runChain.js"; diff --git a/packages/payload-plugin-translator/src/server/modules/translation-run/runChain.ts b/packages/payload-plugin-translator/src/server/modules/translation-run/runChain.ts new file mode 100644 index 000000000..1a439a388 --- /dev/null +++ b/packages/payload-plugin-translator/src/server/modules/translation-run/runChain.ts @@ -0,0 +1,69 @@ +import type { RequestScope } from "../../shared/payload/RequestScope.shapes.js"; +import { swallowOrThrow } from "../../shared/payload/swallowOrThrow.js"; + +import type { + AfterSave, + BeforeTranslation, + ChainMember, + RunContribution, + RunLogger, +} from "./TranslationRun.interface.js"; + +/** + * A module is handed `payload` and may query it, so a failure it raises is assumed to have taken + * the caller's transaction — {@link swallowOrThrow} decides, and the chain does not second-guess it. + * Adopting a module's failure as harmless would also demote one a module raised deliberately. + */ +async function callModule( + name: string, + scope: RequestScope, + work: () => Promise, + log: RunLogger +): Promise { + return swallowOrThrow(scope, work, (error) => + log.error({ err: error, module: name, msg: "translator: a translation module failed" }) + ); +} + +const withContribution = ( + state: BeforeTranslation, + contribution: RunContribution +): BeforeTranslation => ({ + ...state, + moduleState: { ...state.moduleState, ...contribution.moduleState }, + sourceChangedByLeaf: contribution.sourceChangedByLeaf ?? state.sourceChangedByLeaf, +}); + +/** + * Each module in list order, owed two things the signature cannot state: `moduleState` merges by + * key, so a module cannot drop a slot it does not own; and a failed module contributes nothing — + * the chain keeps the state that module was given, never the `undefined` {@link swallowOrThrow} + * answers with. + */ +export async function runBeforeTranslation( + modules: readonly ChainMember[], + state: BeforeTranslation, + log: RunLogger +): Promise { + const { scope } = state; + let carried = state; + for (const member of modules) { + const join = member.beforeTranslation?.bind(member); + if (!join) continue; + const contribution = await callModule(member.name, scope, () => join(carried), log); + carried = contribution ? withContribution(carried, contribution) : carried; + } + return carried; +} + +export async function runAfterSave( + modules: readonly ChainMember[], + state: AfterSave, + log: RunLogger +): Promise { + for (const member of modules) { + const join = member.afterSave?.bind(member); + if (!join) continue; + await callModule(member.name, state.scope, () => join(state), log); + } +} diff --git a/packages/payload-plugin-translator/src/server/shared/access/__tests__/accessIsNotOptional.pins.ts b/packages/payload-plugin-translator/src/server/shared/access/__tests__/accessIsNotOptional.pins.ts new file mode 100644 index 000000000..2108525fc --- /dev/null +++ b/packages/payload-plugin-translator/src/server/shared/access/__tests__/accessIsNotOptional.pins.ts @@ -0,0 +1,25 @@ +/** Never imported and never run: `check-types` fails if any `@ts-expect-error` below stops being an error. */ + +import type { PayloadRequest } from "payload"; + +import { createEnqueueRoute } from "../../../features/enqueue-translation/index.js"; +import type { TaskRunnerFactory } from "../../../modules/task-runner/index.js"; +import type { ConfigTimeContext } from "../../../modules/plugin-config/index.js"; +import { withAccessCheck } from "../../http/withAccessCheck.js"; +import type { AccessGuard } from "../types.js"; + +declare const handler: (req: PayloadRequest) => Promise; +declare const runner: TaskRunnerFactory; +declare const guard: AccessGuard; + +// @ts-expect-error every route is guarded: there is no "open by omission" +withAccessCheck(handler); + +// @ts-expect-error a route factory cannot be built without a guard either +createEnqueueRoute(runner, { availableCollections: new Set() }, undefined, "/translate"); + +// @ts-expect-error the shared context carries a guard, never the absence of one +const _contextWithoutAGuard: ConfigTimeContext["access"] = undefined; + +// the positive control: with a guard, the same call compiles +withAccessCheck(handler, guard); diff --git a/packages/payload-plugin-translator/src/server/shared/http/__tests__/withAccessCheck.test.ts b/packages/payload-plugin-translator/src/server/shared/http/__tests__/withAccessCheck.test.ts index 665c81466..8ad60f844 100644 --- a/packages/payload-plugin-translator/src/server/shared/http/__tests__/withAccessCheck.test.ts +++ b/packages/payload-plugin-translator/src/server/shared/http/__tests__/withAccessCheck.test.ts @@ -14,14 +14,6 @@ describe("withAccessCheck", () => { check: vi.fn().mockResolvedValue(allowed), }); - it("returns original handler when no access guard provided", async () => { - const handler = vi.fn().mockResolvedValue(ServerResponse.success()); - - const wrappedHandler = withAccessCheck(handler); - - expect(wrappedHandler).toBe(handler); - }); - it("calls handler when access is allowed", async () => { const expectedResponse = ServerResponse.success({ data: "test" }); const handler = vi.fn().mockResolvedValue(expectedResponse); diff --git a/packages/payload-plugin-translator/src/server/shared/http/withAccessCheck.ts b/packages/payload-plugin-translator/src/server/shared/http/withAccessCheck.ts index fcd1a37d0..ba83235e6 100644 --- a/packages/payload-plugin-translator/src/server/shared/http/withAccessCheck.ts +++ b/packages/payload-plugin-translator/src/server/shared/http/withAccessCheck.ts @@ -3,16 +3,11 @@ import type { PayloadRequest } from "payload"; import type { AccessGuard } from "../access/types.js"; import { ServerResponse } from "./ServerResponse.js"; -/** - * Wraps handler with access check. - * Returns 403 Forbidden if access guard denies the request. - */ +/** Every route is guarded; pass `AnyAccessGuard` to open one on purpose. */ export function withAccessCheck( handler: (req: PayloadRequest) => Promise, - access?: AccessGuard + access: AccessGuard ): (req: PayloadRequest) => Promise { - if (!access) return handler; - return async (req) => { const hasAccess = await access.check({ req }); if (!hasAccess) return ServerResponse.forbidden(); diff --git a/packages/payload-plugin-translator/src/types/ConfigModifier.ts b/packages/payload-plugin-translator/src/types/ConfigModifier.ts index 17fa36743..2fc4fb48a 100644 --- a/packages/payload-plugin-translator/src/types/ConfigModifier.ts +++ b/packages/payload-plugin-translator/src/types/ConfigModifier.ts @@ -1,11 +1,7 @@ import type { Config } from "payload"; /** - * A config-time contribution: takes the Payload `config` and returns it (possibly a fresh object). - * Each translator module exposes a `configure(ctx) → ConfigModifier`; the plugin registers them - * through `PluginConfigBuilder.addConfigModifier`, which is the one place they are applied. - * - * Lives in `types/` (a leaf contract layer) so both `translation-levels` and the `provenance` - * module can import it without creating a module cycle. + * A config-time contribution: takes the Payload `config` and returns it — possibly a fresh object, + * so callers must use the return value. */ export type ConfigModifier = (config: Config) => Config;