From 97788e3d085d53e3c1b507efe389527a3dceded6 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 15:40:00 +0300 Subject: [PATCH 01/16] fix(nextly): take the live row's verdict only for a field the promotion drops --- ...verdict-does-not-refuse-a-valid-publish.md | 32 +++++++++++++++++++ .../lib/__tests__/denied-change.test.ts | 17 ++++++++++ .../nextly/src/shared/lib/denied-change.ts | 18 ++++++++--- 3 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 .changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md diff --git a/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md b/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md new file mode 100644 index 0000000000..56671bfa9b --- /dev/null +++ b/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md @@ -0,0 +1,32 @@ +--- +"nextly": patch +"create-nextly-app": patch +"@nextlyhq/admin": patch +"@nextlyhq/admin-css": patch +"@nextlyhq/blocks-engine": patch +"@nextlyhq/blocks-react": patch +"@nextlyhq/ui": patch +"@nextlyhq/adapter-drizzle": patch +"@nextlyhq/adapter-postgres": patch +"@nextlyhq/adapter-mysql": patch +"@nextlyhq/adapter-sqlite": patch +"@nextlyhq/storage-s3": patch +"@nextlyhq/storage-uploadthing": patch +"@nextlyhq/storage-vercel-blob": patch +"@nextlyhq/plugin-form-builder": patch +"@nextlyhq/plugin-mcp": patch +"@nextlyhq/plugin-page-builder": patch +"@nextlyhq/plugin-seo": patch +"@nextlyhq/plugin-sdk": patch +"@nextlyhq/eslint-config": patch +"@nextlyhq/eslint-plugin": patch +"@nextlyhq/prettier-config": patch +"@nextlyhq/telemetry": patch +"@nextlyhq/tsconfig": patch +"@nextlyhq/builder": patch +"@nextlyhq/module-specifiers": patch +--- + +A publish is no longer refused over a field whose rule the pending change itself satisfies. + +The gate consults the live row as well as the promoted document, because a rule is never asked about a key that is absent and a field the pending change removes outright would otherwise be judged nowhere. It took the live row's verdict for every field, though, and a rule reads its siblings: where the live row says `kind: "private"`, which denies `guarded`, and the pending change sets `kind` to `public` and edits `guarded` legitimately, the stale verdict refused a publish that is perfectly valid. The live row now speaks only for the fields the promotion no longer carries. diff --git a/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts b/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts index 9bd45c0eca..94e198a6cf 100644 --- a/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts +++ b/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts @@ -154,6 +154,23 @@ describe("resolvePromotedDocument", () => { }); }); + it("does not import a stale live verdict for a field the promotion still holds", async () => { + // A rule reads its siblings. Live says `kind: "private"`, which denies + // `guarded`; the pending change sets `kind` to `public` and edits `guarded` + // legitimately. The promoted document is the one that has the right of it, + // so taking live's verdict too would refuse a valid publish. + const rulesByKind = (document: Record): Promise => { + if (document.kind === "private") delete document.guarded; + return Promise.resolve(); + }; + const out = await resolve({ + before: { kind: "public", guarded: "edited" }, + live: { kind: "private", guarded: "live" }, + applyRules: rulesByKind, + }); + expect(out).toEqual({ kind: "public", guarded: "edited" }); + }); + it("keeps an own __proto__ key instead of invoking the prototype setter", async () => { const before: Record = { guarded: "live" }; Object.defineProperty(before, "__proto__", { diff --git a/packages/nextly/src/shared/lib/denied-change.ts b/packages/nextly/src/shared/lib/denied-change.ts index de3ee22de6..4f2f32f530 100644 --- a/packages/nextly/src/shared/lib/denied-change.ts +++ b/packages/nextly/src/shared/lib/denied-change.ts @@ -104,10 +104,20 @@ export async function resolvePromotedDocument( const permittedLive = detachData(input.live); await input.applyRules(permittedLive); - const denied = new Set([ - ...deniedPaths(input.before, permittedBefore, ""), - ...deniedPaths(input.live, permittedLive, ""), - ]); + // The promoted document's own verdict, plus — from the live row — ONLY the + // fields the promotion no longer carries. + // + // Live is consulted for one reason: a rule is never asked about a key that is + // absent, so a field the pending change removes outright is judged nowhere. + // Taking live's verdict for a field the promotion still holds would import a + // stale answer instead, because a rule reads its siblings: where live says + // `kind: "private"` denies `guarded`, and the pending change sets `kind` to + // `public` and edits `guarded` legitimately, the promoted document is the one + // that has the right of it. + const denied = new Set(deniedPaths(input.before, permittedBefore, "")); + for (const path of deniedPaths(input.live, permittedLive, "")) { + if (!pathExists(input.before, path)) denied.add(path); + } const refusals: string[] = []; for (const path of denied) { From d20fc1a6280c7ff35eb227af5ff902a9cd1777ba Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 15:52:50 +0300 Subject: [PATCH 02/16] fix(nextly): collect declared names through named containers, and give them to Singles too --- ...verdict-does-not-refuse-a-valid-publish.md | 2 ++ .../services/collection-mutation-service.ts | 11 ++++---- .../domains/singles/services/promote-gate.ts | 8 +++++- .../lib/__tests__/denied-change.test.ts | 25 ++++++++++++++++- .../nextly/src/shared/lib/denied-change.ts | 28 +++++++++++++++++++ 5 files changed, 66 insertions(+), 8 deletions(-) diff --git a/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md b/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md index 56671bfa9b..338bf265b9 100644 --- a/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md +++ b/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md @@ -30,3 +30,5 @@ A publish is no longer refused over a field whose rule the pending change itself satisfies. The gate consults the live row as well as the promoted document, because a rule is never asked about a key that is absent and a field the pending change removes outright would otherwise be judged nowhere. It took the live row's verdict for every field, though, and a rule reads its siblings: where the live row says `kind: "private"`, which denies `guarded`, and the pending change sets `kind` to `public` and edits `guarded` legitimately, the stale verdict refused a publish that is perfectly valid. The live row now speaks only for the fields the promotion no longer carries. + +A field declared inside a group or a repeater counts as content too. The names the promotion gate defers to were collected with `addressableFields`, which pushes a named field and stops, so the set held the top level and nothing else and a nested field named like one of the store's own columns was still skipped. The walk that collects them descends every container now, and a Single's publish hands its declared names over as a collection's does. diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index d9abbc0e2f..20cf7aed67 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -78,7 +78,10 @@ import { rehydrateSystemTimestamps, SYSTEM_TIMESTAMP_KEYS, } from "../../../shared/lib/case-conversion"; -import { resolvePromotedDocument } from "../../../shared/lib/denied-change"; +import { + declaredFieldNames, + resolvePromotedDocument, +} from "../../../shared/lib/denied-change"; import { detachData } from "../../../shared/lib/detach"; import { validateEntryData } from "../../../shared/lib/entry-validation"; import { applyFieldDefaults } from "../../../shared/lib/field-defaults"; @@ -6821,11 +6824,7 @@ export class CollectionMutationService extends BaseService { }), // What the schema declares, so a field named like one of the // store's own columns is still judged as the content it is. - authoredFieldNames: new Set( - addressableFields(fields, { descendInto: () => true }) - .map(entry => entry.name) - .filter((name): name is string => typeof name === "string") - ), + authoredFieldNames: declaredFieldNames(fields), slug: params.collectionName, locale: draftLocaleKey ?? params.locale ?? null, }); diff --git a/packages/nextly/src/domains/singles/services/promote-gate.ts b/packages/nextly/src/domains/singles/services/promote-gate.ts index fedfa2995e..d6bef30e4d 100644 --- a/packages/nextly/src/domains/singles/services/promote-gate.ts +++ b/packages/nextly/src/domains/singles/services/promote-gate.ts @@ -30,7 +30,10 @@ import type { AuthenticatedScope } from "../../../auth/authenticated-scope"; import type { FieldConfig } from "../../../collections/fields/types"; import { NextlyError } from "../../../errors"; -import { resolvePromotedDocument } from "../../../shared/lib/denied-change"; +import { + declaredFieldNames, + resolvePromotedDocument, +} from "../../../shared/lib/denied-change"; import { validateEntryData } from "../../../shared/lib/entry-validation"; import { applyFieldWriteAccess, @@ -222,6 +225,9 @@ async function resolveForLocale( grants: ctx.grants, id: ctx.entryId, }), + // What this Single declares, so a field named like one of the store's own + // columns is judged as the content it is rather than skipped as metadata. + authoredFieldNames: declaredFieldNames(ctx.fields), slug: ctx.slug, locale, }); diff --git a/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts b/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts index 94e198a6cf..baacc65cd0 100644 --- a/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts +++ b/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts @@ -13,7 +13,7 @@ */ import { describe, expect, it } from "vitest"; -import { resolvePromotedDocument } from "../denied-change"; +import { declaredFieldNames, resolvePromotedDocument } from "../denied-change"; /** Removes the named top-level or nested paths, as the field rules would. */ function denies(...paths: string[]) { @@ -218,6 +218,29 @@ describe("resolvePromotedDocument", () => { expect(out).toBeDefined(); }); + it("collects a name declared inside a NAMED container", async () => { + // `addressableFields` pushes a named field and stops, so a set built from + // it holds the top level only, and the nested name the metadata list is + // meant to defer to is exactly the one it misses. + const names = declaredFieldNames([ + { name: "title", type: "text" }, + { name: "meta", type: "group", fields: [{ name: "id", type: "text" }] }, + { + type: "row", + fields: [ + { + name: "rows", + type: "repeater", + fields: [{ name: "updatedAt", type: "text" }], + }, + ], + }, + ]); + expect(names.has("meta")).toBe(true); + expect(names.has("id")).toBe(true); + expect(names.has("updatedAt")).toBe(true); + }); + it("leaves an allowed deletion deleted", async () => { const out = await resolve({ before: { title: "new" }, diff --git a/packages/nextly/src/shared/lib/denied-change.ts b/packages/nextly/src/shared/lib/denied-change.ts index 4f2f32f530..41faf02219 100644 --- a/packages/nextly/src/shared/lib/denied-change.ts +++ b/packages/nextly/src/shared/lib/denied-change.ts @@ -37,6 +37,34 @@ import { NextlyError } from "../../errors"; import { detachData } from "./detach"; +/** + * Every field name a schema declares, at any depth. + * + * Its own walk rather than `addressableFields`, which pushes a NAMED field and + * stops: `descendInto` reaches the children of unnamed containers only, so a + * set built from it holds the top level and nothing else, and the nested name + * this exists to protect is exactly the one it would miss. + * + * Names, not paths, because the caller compares the last segment of a path: a + * field is content wherever it is declared, and the store's own columns are + * what the name list is for. + */ +export function declaredFieldNames(fields: unknown): Set { + const names = new Set(); + const seen = new WeakSet(); + const pending: unknown[] = Array.isArray(fields) ? [...fields] : []; + while (pending.length > 0) { + const field = pending.pop(); + if (typeof field !== "object" || field === null) continue; + if (seen.has(field)) continue; + seen.add(field); + const record = field as { name?: unknown; fields?: unknown }; + if (typeof record.name === "string") names.add(record.name); + if (Array.isArray(record.fields)) pending.push(...record.fields); + } + return names; +} + /** What deciding one promotion needs from the service performing it. */ export interface PromotionAccessInput { /** The document the write would persist, before any rule has run. */ From 003a340937924ecff575e3c67d239480bf9cf2bd Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 15:59:19 +0300 Subject: [PATCH 03/16] fix(nextly): expand a live-side denied container to its leaves before filtering --- .../lib/__tests__/denied-change.test.ts | 20 +++++++++++++++++++ .../nextly/src/shared/lib/denied-change.ts | 8 +++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts b/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts index baacc65cd0..63ea9d1122 100644 --- a/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts +++ b/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts @@ -171,6 +171,26 @@ describe("resolvePromotedDocument", () => { expect(out).toEqual({ kind: "public", guarded: "edited" }); }); + it("refuses a deleted child even when the live rule denies its whole container", async () => { + // The live rule removes `seo` entirely, so the live-side denial names the + // container. The promotion keeps `seo` and drops `secret` from inside it, + // so a filter applied at the container asks the wrong question and the + // deletion goes through unjudged. + const rulesByKind = (document: Record): Promise => { + if (document.kind === "private") delete document.seo; + return Promise.resolve(); + }; + await expect( + resolve({ + before: { kind: "public", seo: { title: "new" } }, + live: { kind: "private", seo: { title: "old", secret: "live" } }, + applyRules: rulesByKind, + }) + ).rejects.toMatchObject({ + publicData: { errors: [{ path: "seo.secret" }] }, + }); + }); + it("keeps an own __proto__ key instead of invoking the prototype setter", async () => { const before: Record = { guarded: "live" }; Object.defineProperty(before, "__proto__", { diff --git a/packages/nextly/src/shared/lib/denied-change.ts b/packages/nextly/src/shared/lib/denied-change.ts index 41faf02219..c0e3a311fc 100644 --- a/packages/nextly/src/shared/lib/denied-change.ts +++ b/packages/nextly/src/shared/lib/denied-change.ts @@ -144,7 +144,13 @@ export async function resolvePromotedDocument( // that has the right of it. const denied = new Set(deniedPaths(input.before, permittedBefore, "")); for (const path of deniedPaths(input.live, permittedLive, "")) { - if (!pathExists(input.before, path)) denied.add(path); + // Expanded to leaves BEFORE the filter, because a rule denies a container + // whole. Filtering at the container asks "does the promotion still carry + // `seo`", which is yes even when it has dropped `seo.secret` from inside + // it, and the dropped child is what the live side was consulted for. + for (const leaf of leafPaths(valueAt(input.live, path), path)) { + if (!pathExists(input.before, leaf)) denied.add(leaf); + } } const refusals: string[] = []; From 9315be950b3fcf5998e750d7be94f1748bc70da4 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 16:17:59 +0300 Subject: [PATCH 04/16] refactor(nextly): split the promotion resolver so each function answers one question --- .../nextly/src/shared/lib/denied-change.ts | 240 +++++++++++------- 1 file changed, 146 insertions(+), 94 deletions(-) diff --git a/packages/nextly/src/shared/lib/denied-change.ts b/packages/nextly/src/shared/lib/denied-change.ts index c0e3a311fc..00e3340324 100644 --- a/packages/nextly/src/shared/lib/denied-change.ts +++ b/packages/nextly/src/shared/lib/denied-change.ts @@ -132,16 +132,35 @@ export async function resolvePromotedDocument( const permittedLive = detachData(input.live); await input.applyRules(permittedLive); - // The promoted document's own verdict, plus — from the live row — ONLY the - // fields the promotion no longer carries. - // - // Live is consulted for one reason: a rule is never asked about a key that is - // absent, so a field the pending change removes outright is judged nowhere. - // Taking live's verdict for a field the promotion still holds would import a - // stale answer instead, because a rule reads its siblings: where live says - // `kind: "private"` denies `guarded`, and the pending change sets `kind` to - // `public` and edits `guarded` legitimately, the promoted document is the one - // that has the right of it. + const denied = collectDenied(input, permittedBefore, permittedLive); + const refusals = [...denied].flatMap(path => refusedLeaves(input, path)); + if (refusals.length > 0) refuse(input, refusals); + + return restoreDenied( + input.before, + permittedBefore, + input.live, + permittedLive + ) as Record; +} + +/** + * Every path the rules deny: the promoted document's own verdict, plus from the + * live row ONLY the fields the promotion no longer carries. + * + * Live is consulted for one reason: a rule is never asked about a key that is + * absent, so a field the pending change removes outright is judged nowhere. + * Taking live's verdict for a field the promotion still holds would import a + * stale answer instead, because a rule reads its siblings: where live says + * `kind: "private"` denies `guarded`, and the pending change sets `kind` to + * `public` and edits `guarded` legitimately, the promoted document is the one + * that has the right of it. + */ +function collectDenied( + input: PromotionAccessInput, + permittedBefore: Record, + permittedLive: Record +): Set { const denied = new Set(deniedPaths(input.before, permittedBefore, "")); for (const path of deniedPaths(input.live, permittedLive, "")) { // Expanded to leaves BEFORE the filter, because a rule denies a container @@ -152,54 +171,52 @@ export async function resolvePromotedDocument( if (!pathExists(input.before, leaf)) denied.add(leaf); } } + return denied; +} - const refusals: string[] = []; - for (const path of denied) { - // Leaf by leaf, over the UNION of both sides. The rules delete a denied - // container whole, so the removal names the container while its contents - // can have two authors, and a property present only on the live side is one - // this document deletes. - const leaves = new Set([ - ...leafPaths(valueAt(input.before, path), path), - ...leafPaths(valueAt(input.live, path), path), - ]); - for (const leaf of leaves) { - if (isStoreBookkeeping(leaf, input.authoredFieldNames)) continue; - if ( - sameStoredValue(valueAt(input.before, leaf), valueAt(input.live, leaf)) - ) { - continue; - } - // The caller's own edit is dropped back to live below, not refused. - if (pathExists(input.callerSupplied, leaf)) continue; - refusals.push(leaf); - } - } +/** + * The leaves under one denied path that the pending change would change. + * + * Leaf by leaf, over the UNION of both sides. The rules delete a denied + * container whole, so the removal names the container while its contents can + * have two authors, and a property present only on the live side is one this + * document deletes. + */ +function refusedLeaves(input: PromotionAccessInput, path: string): string[] { + const leaves = new Set([ + ...leafPaths(valueAt(input.before, path), path), + ...leafPaths(valueAt(input.live, path), path), + ]); + return [...leaves].filter(leaf => isRefusal(input, leaf)); +} - if (refusals.length > 0) { - const fields = [...new Set(refusals)].sort(); - throw NextlyError.validation({ - errors: fields.map(path => ({ - path, - code: "FORBIDDEN", - message: - "The pending change edits this field and you do not have permission to write it, so it cannot be published. The change is kept.", - })), - logContext: { - cause: "promote-denied-field", - slug: input.slug, - locale: input.locale ?? null, - fields, - }, - }); +/** Whether one denied leaf is a change the pending change makes. */ +function isRefusal(input: PromotionAccessInput, leaf: string): boolean { + if (isStoreBookkeeping(leaf, input.authoredFieldNames)) return false; + if (sameStoredValue(valueAt(input.before, leaf), valueAt(input.live, leaf))) { + return false; } + // The caller's own edit is dropped back to live, not refused. + return !pathExists(input.callerSupplied, leaf); +} - return restoreDenied( - input.before, - permittedBefore, - input.live, - permittedLive - ) as Record; +/** Refuse the promotion, naming every field at fault, in a stable order. */ +function refuse(input: PromotionAccessInput, refusals: string[]): never { + const fields = [...new Set(refusals)].sort(); + throw NextlyError.validation({ + errors: fields.map(path => ({ + path, + code: "FORBIDDEN", + message: + "The pending change edits this field and you do not have permission to write it, so it cannot be published. The change is kept.", + })), + logContext: { + cause: "promote-denied-field", + slug: input.slug, + locale: input.locale ?? null, + fields, + }, + }); } /** @@ -217,58 +234,93 @@ function restoreDenied( permittedLive: unknown ): unknown { if (Array.isArray(before)) { - if (!Array.isArray(permitted)) return live; - return before.map((row, index) => - restoreDenied( - row, - permitted[index], - Array.isArray(live) ? live[index] : undefined, - Array.isArray(permittedLive) ? permittedLive[index] : undefined - ) - ); + return restoreArray(before, permitted, live, permittedLive); } if (!isRecord(before)) return before; // The rules removed this whole level, so the row keeps what it has. if (!isRecord(permitted)) return live; + return restoreRecord( + before, + permitted, + recordOrUndefined(live), + recordOrUndefined(permittedLive) + ); +} - const liveRecord = isRecord(live) ? live : undefined; - const permittedLiveRecord = isRecord(permittedLive) - ? permittedLive - : undefined; - const out: Record = {}; +function restoreArray( + before: unknown[], + permitted: unknown, + live: unknown, + permittedLive: unknown +): unknown { + // The rules removed the whole list, so the row keeps what it has. + if (!Array.isArray(permitted)) return live; + return before.map((row, index) => + restoreDenied( + row, + permitted[index], + itemAt(live, index), + itemAt(permittedLive, index) + ) + ); +} +function restoreRecord( + before: Record, + permitted: Record, + live: Record | undefined, + permittedLive: Record | undefined +): Record { + const out: Record = {}; for (const key of Object.keys(before)) { - if (!hasOwn(permitted, key)) { - if (liveRecord && hasOwn(liveRecord, key)) { - assign(out, key, liveRecord[key]); - } - continue; + if (hasOwn(permitted, key)) { + assign( + out, + key, + restoreDenied( + before[key], + permitted[key], + live?.[key], + permittedLive?.[key] + ) + ); + } else if (live && hasOwn(live, key)) { + // Denied: the row keeps what it has. + assign(out, key, live[key]); } - assign( - out, - key, - restoreDenied( - before[key], - permitted[key], - liveRecord?.[key], - permittedLiveRecord?.[key] - ) - ); } + keepDeniedLiveOnlyKeys(out, before, live, permittedLive); + return out; +} - // A key the row holds that this document drops. Allowed, that is a deletion - // the promotion is entitled to make; denied, it is one the caller may not, - // so the value stays. Anything the pending change was deleting has already - // been refused above, so what reaches here is the caller's own. - if (liveRecord) { - for (const key of Object.keys(liveRecord)) { - if (hasOwn(before, key)) continue; - if (permittedLiveRecord && hasOwn(permittedLiveRecord, key)) continue; - assign(out, key, liveRecord[key]); - } +/** + * A key the row holds that this document drops. Allowed, that is a deletion the + * promotion is entitled to make; denied, it is one the caller may not, so the + * value stays. Anything the pending change was deleting has already been + * refused, so what reaches here is the caller's own. + */ +function keepDeniedLiveOnlyKeys( + out: Record, + before: Record, + live: Record | undefined, + permittedLive: Record | undefined +): void { + if (!live) return; + for (const key of Object.keys(live)) { + if (hasOwn(before, key)) continue; + if (permittedLive && hasOwn(permittedLive, key)) continue; + assign(out, key, live[key]); } +} - return out; +function recordOrUndefined( + value: unknown +): Record | undefined { + return isRecord(value) ? value : undefined; +} + +function itemAt(value: unknown, index: number): unknown { + return Array.isArray(value) ? value[index] : undefined; } /** From da32ad702deb973f9ba84bbd7785a9817de57036 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 16:31:49 +0300 Subject: [PATCH 05/16] fix(nextly): a publish may carry a group edit, and provenance is what the caller sent --- ...verdict-does-not-refuse-a-valid-publish.md | 6 + ...e-judges-the-publisher.integration.test.ts | 133 ++++++++++++++++++ .../services/collection-mutation-service.ts | 77 ++++++---- 3 files changed, 186 insertions(+), 30 deletions(-) diff --git a/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md b/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md index 338bf265b9..5f4c56fef8 100644 --- a/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md +++ b/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md @@ -32,3 +32,9 @@ A publish is no longer refused over a field whose rule the pending change itself The gate consults the live row as well as the promoted document, because a rule is never asked about a key that is absent and a field the pending change removes outright would otherwise be judged nowhere. It took the live row's verdict for every field, though, and a rule reads its siblings: where the live row says `kind: "private"`, which denies `guarded`, and the pending change sets `kind` to `public` and edits `guarded` legitimately, the stale verdict refused a publish that is perfectly valid. The live row now speaks only for the fields the promotion no longer carries. A field declared inside a group or a repeater counts as content too. The names the promotion gate defers to were collected with `addressableFields`, which pushes a named field and stops, so the set held the top level and nothing else and a nested field named like one of the store's own columns was still skipped. The walk that collects them descends every container now, and a Single's publish hands its declared names over as a collection's does. + +A deletion inside a container a rule denies whole is judged. The live row's denial names the container, and the promotion can keep that container while dropping a protected child from inside it: asked at the container, "does the promotion still carry `seo`" is yes, and the dropped `seo.secret` went through unjudged. Each live-side denial is expanded to its leaves before it is compared. + +A publish that also edits a group, repeater or JSON field no longer fails validation. The ordinary write encodes those fields to their column strings before the promotion runs, and the check on the promoted document then read a group as text and refused it as "must be an object", so an editor who published their pending change together with any edit to such a field could not publish at all. Both promotion checks now read the document in its logical shape, through the same conversion that reads the live row, and the write encodes it once. + +Whether the caller supplied a value is read from what they sent. It was read from their payload after the field rules had already removed what they may not write, and after that payload's containers had been encoded, so a protected value the caller sent unchanged inside a group looked unsent and the publish was refused as a deletion they never made. diff --git a/packages/nextly/src/domains/collections/__tests__/collection-promote-judges-the-publisher.integration.test.ts b/packages/nextly/src/domains/collections/__tests__/collection-promote-judges-the-publisher.integration.test.ts index 37d0c5f214..26cc5e8056 100644 --- a/packages/nextly/src/domains/collections/__tests__/collection-promote-judges-the-publisher.integration.test.ts +++ b/packages/nextly/src/domains/collections/__tests__/collection-promote-judges-the-publisher.integration.test.ts @@ -444,6 +444,139 @@ describe("a collection publish re-judges the draft it promotes", () => { expect(doc?.guarded).toBe("live-value"); }); + it("publishes a group edit sent with the status while a change is pending", async () => { + // No field rules at all, so nothing here is about access. The ordinary + // write encodes a group to its column string before the promotion runs, and + // the check that validates the promoted document read that string where it + // expects an object: a publish that also edited any group, repeater or JSON + // field was refused as "ops must be an object" whenever a pending change + // existed, which is the ordinary shape of an editor publishing their edit. + const slug = "grouppublish"; + current = await createTestNextly({ + collections: [ + defineCollection({ + slug, + status: true, + versions: { drafts: true }, + access: { + read: () => true, + update: () => true, + publish: () => true, + unpublish: () => true, + }, + fields: [ + text({ name: "body" }), + group({ name: "ops", fields: [text({ name: "note" })] }), + ], + }), + ], + }); + const h = handlerOf(current); + + const created = await h.createEntry( + { collectionName: slug, overrideAccess: true }, + { body: "live", ops: { note: "live-note" }, status: "published" } + ); + const id = (created.data as { id?: string }).id as string; + + await h.updateEntry( + { collectionName: slug, entryId: id, routeAuthorized: true, user: CLERK }, + { body: "edited" } + ); + + const published = await h.updateEntry( + { collectionName: slug, entryId: id, routeAuthorized: true, user: CLERK }, + { status: "published", ops: { note: "published-note" } } + ); + + expect(published.success, JSON.stringify(published)).toBe(true); + const doc = (await current.nextly.findByID({ + collection: slug as never, + id, + overrideAccess: true, + status: "all", + } as never)) as Record | null; + expect(doc?.body).toBe("edited"); + expect(doc?.ops).toEqual({ note: "published-note" }); + }); + + it("publishes a caller's allowed edit beside a protected value they also sent", async () => { + // The ordinary field gate removes the protected value from the caller's + // payload before the promotion runs. Judged from that filtered payload, the + // caller looks as though they never sent it, the live row still holds it, + // and the missing leaf reads as a deletion: the publish is refused over a + // value the caller supplied unchanged. What the caller sent is the fact + // that decides, so it is read before any gate touches it. + const slug = "provenance"; + current = await createTestNextly({ + collections: [ + defineCollection({ + slug, + status: true, + versions: { drafts: true }, + access: { + read: () => true, + update: () => true, + publish: () => true, + unpublish: () => true, + }, + fields: [ + text({ name: "body" }), + group({ + name: "ops", + fields: [ + text({ name: "note" }), + text({ + name: "runbook", + access: { + update: ({ req }) => req.user?.email === BOSS.email, + }, + }), + ], + }), + ], + }), + ], + }); + const h = handlerOf(current); + + const created = await h.createEntry( + { collectionName: slug, overrideAccess: true }, + { + body: "live", + ops: { note: "live-note", runbook: "live-runbook" }, + status: "published", + } + ); + const id = (created.data as { id?: string }).id as string; + + // A pending change exists, so the publish below promotes one. + await h.updateEntry( + { collectionName: slug, entryId: id, routeAuthorized: true, user: CLERK }, + { body: "edited" } + ); + + // CLERK sends the whole group: an allowed edit to `note`, and `runbook` at + // the value it already holds, which the gate will strip from their payload. + const published = await h.updateEntry( + { collectionName: slug, entryId: id, routeAuthorized: true, user: CLERK }, + { + status: "published", + ops: { note: "clerk-note", runbook: "live-runbook" }, + } + ); + + expect(published.success, JSON.stringify(published)).toBe(true); + const doc = (await current.nextly.findByID({ + collection: slug as never, + id, + overrideAccess: true, + status: "all", + } as never)) as Record | null; + expect(doc?.body).toBe("edited"); + expect(doc?.ops).toEqual({ note: "clerk-note", runbook: "live-runbook" }); + }); + it("still promotes the change for a publisher who MAY write it", async () => { const t = await boot(); const h = handlerOf(t); diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 20cf7aed67..57e05d1771 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -5543,6 +5543,12 @@ export class CollectionMutationService extends BaseService { // unlocalized write (`locale: undefined`): the main row's `status` moves and // is NOT stripped the way a non-default locale's write strips it. The only // thing the wildcard adds is the companion sweep at the write itself. + // What the caller SENT, taken before anything can change it. Hooks may + // rewrite the payload, and the field gate deletes the caller's denied keys in + // place, from `body` itself whenever no hook returned a fresh object: by the + // time a promotion asks whether the caller supplied a value, `body` has + // already lost the answer. Deep, because that gate walks nested containers. + const callerSentBody = detachData(body); const sweepAllLocales = rawParams.locale === EVERY_LOCALE; const params = sweepAllLocales ? { ...rawParams, locale: undefined } @@ -6276,14 +6282,24 @@ export class CollectionMutationService extends BaseService { // group/repeater/component) and covers component and m2m fields, not only // columns. The authoritative pass runs again on the locked draft in the // transaction (see the promote block). - const merged = this.assemblePromotedDocument( - draftInput, - finalData, - componentFieldData, - manyToManyData, - fields, - manyToManyFields, - splitComponentSchemas + // In the LOGICAL shape the validator reads. `finalData` has already + // been through `shapeWriteParts`, which encodes every JSON-backed field + // to its column string, so a group the caller sent with the publish + // arrives here as text and is refused as "must be an object": a + // publish carrying any group, repeater or JSON value failed whenever a + // pending change existed. Parsed by the same function that builds the + // live row, so the two sides are one representation. + const merged = this.deserializeJsonFieldsForSnapshot( + this.assemblePromotedDocument( + draftInput, + finalData, + componentFieldData, + manyToManyData, + fields, + manyToManyFields, + splitComponentSchemas + ), + fields ); // Validated as ASSEMBLED, with nothing removed. A field this // publisher may not write is not a schema violation, and the gate @@ -6778,29 +6794,30 @@ export class CollectionMutationService extends BaseService { // Re-extracting the write parts from the FILTERED document keeps a // denied component/m2m value out of the persisted parts, which the // earlier after-access merge would have restored. - const mergedPromoteData = this.assemblePromotedDocument( - draftInput, - finalData, - componentFieldData, - manyToManyData, - fields, - manyToManyFields, - splitComponentSchemas - ); - // The caller's own contribution, assembled by the SAME function - // with no draft behind it, so it lands in the same shape as the - // document it is compared against. A denied value of theirs is - // dropped back to live as on any other write; only the pending - // change's own values are worth refusing over. - const callerContribution = this.assemblePromotedDocument( - {}, - finalData, - componentFieldData, - manyToManyData, - fields, - manyToManyFields, - splitComponentSchemas + // Logical, for the same reason as the check before the + // transaction: `finalData` is already column-encoded, and a group + // left as its string is one leaf to the resolver, so a denied field + // inside it is never found and an untouched group compares unequal + // to the parsed live row. The write below encodes it again, once. + const mergedPromoteData = this.deserializeJsonFieldsForSnapshot( + this.assemblePromotedDocument( + draftInput, + finalData, + componentFieldData, + manyToManyData, + fields, + manyToManyFields, + splitComponentSchemas + ), + fields ); + // What the caller SENT, as the provenance the resolver reads. It + // asks only whether a path is present, so the raw payload is the + // answer as it stands. Shaping it first is what lost the answer: + // `shapeWriteParts` encodes a group to its column string, and a + // string has no children, so a protected value the caller sent + // inside a group looked unsent and was refused as a deletion. + const callerContribution = callerSentBody; // One call decides the refusal AND returns what to write, so the // document that was judged is the document that lands. A denied // field comes back at its LIVE value rather than removed: taking From 8c6bce70858d19c228b2c777d57c821f75c69e3f Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 16:57:55 +0300 Subject: [PATCH 06/16] fix(nextly): return promotion checks to fail-closed where the looser version lost data --- ...verdict-does-not-refuse-a-valid-publish.md | 12 +-- ...e-judges-the-publisher.integration.test.ts | 72 +++++++++++++--- .../services/collection-mutation-service.ts | 35 +++++--- .../lib/__tests__/denied-change.test.ts | 82 ++++++++++++++----- .../nextly/src/shared/lib/denied-change.ts | 36 ++++---- 5 files changed, 165 insertions(+), 72 deletions(-) diff --git a/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md b/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md index 5f4c56fef8..64c73c0479 100644 --- a/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md +++ b/.changeset/a-stale-live-verdict-does-not-refuse-a-valid-publish.md @@ -27,14 +27,10 @@ "@nextlyhq/module-specifiers": patch --- -A publish is no longer refused over a field whose rule the pending change itself satisfies. +A publish that also edits a group, repeater or JSON field no longer fails validation. -The gate consults the live row as well as the promoted document, because a rule is never asked about a key that is absent and a field the pending change removes outright would otherwise be judged nowhere. It took the live row's verdict for every field, though, and a rule reads its siblings: where the live row says `kind: "private"`, which denies `guarded`, and the pending change sets `kind` to `public` and edits `guarded` legitimately, the stale verdict refused a publish that is perfectly valid. The live row now speaks only for the fields the promotion no longer carries. +When a pending change existed, publishing it together with an edit to any group, repeater or JSON field was refused as "must be an object", with no field rules involved at all. The ordinary write encodes those fields to their column strings before the publish is judged, and the check on the document being published then read a group as text. Both checks now read the document in its logical shape, through the same conversion that reads the live row, and the write encodes it once. -A field declared inside a group or a repeater counts as content too. The names the promotion gate defers to were collected with `addressableFields`, which pushes a named field and stops, so the set held the top level and nothing else and a nested field named like one of the store's own columns was still skipped. The walk that collects them descends every container now, and a Single's publish hands its declared names over as a collection's does. +A field declared inside a group or a repeater is judged as content, however it is named. -A deletion inside a container a rule denies whole is judged. The live row's denial names the container, and the promotion can keep that container while dropping a protected child from inside it: asked at the container, "does the promotion still carry `seo`" is yes, and the dropped `seo.secret` went through unjudged. Each live-side denial is expanded to its leaves before it is compared. - -A publish that also edits a group, repeater or JSON field no longer fails validation. The ordinary write encodes those fields to their column strings before the promotion runs, and the check on the promoted document then read a group as text and refused it as "must be an object", so an editor who published their pending change together with any edit to such a field could not publish at all. Both promotion checks now read the document in its logical shape, through the same conversion that reads the live row, and the write encodes it once. - -Whether the caller supplied a value is read from what they sent. It was read from their payload after the field rules had already removed what they may not write, and after that payload's containers had been encoded, so a protected value the caller sent unchanged inside a group looked unsent and the publish was refused as a deletion they never made. +The publish gate skips the store's own bookkeeping columns, such as `id` and `updatedAt`, and defers to the field names the schema declares so that a real field with one of those names is still judged. Those names were collected in a way that stopped at the first named container, so a declared field nested inside a group or repeater was still skipped, and a Single's publish was not given the names at all. The names are collected at every depth now, for collections and Singles alike. diff --git a/packages/nextly/src/domains/collections/__tests__/collection-promote-judges-the-publisher.integration.test.ts b/packages/nextly/src/domains/collections/__tests__/collection-promote-judges-the-publisher.integration.test.ts index 26cc5e8056..7844eb7890 100644 --- a/packages/nextly/src/domains/collections/__tests__/collection-promote-judges-the-publisher.integration.test.ts +++ b/packages/nextly/src/domains/collections/__tests__/collection-promote-judges-the-publisher.integration.test.ts @@ -500,13 +500,16 @@ describe("a collection publish re-judges the draft it promotes", () => { expect(doc?.ops).toEqual({ note: "published-note" }); }); - it("publishes a caller's allowed edit beside a protected value they also sent", async () => { - // The ordinary field gate removes the protected value from the caller's - // payload before the promotion runs. Judged from that filtered payload, the - // caller looks as though they never sent it, the live row still holds it, - // and the missing leaf reads as a deletion: the publish is refused over a - // value the caller supplied unchanged. What the caller sent is the fact - // that decides, so it is read before any gate touches it. + it("refuses, and keeps the draft, when a group the caller sends drops a protected child", async () => { + // A KNOWN over-refusal, pinned so it cannot change unnoticed. A group the + // caller sends replaces the pending change's group whole, and the field gate + // has already stripped the protected child the caller included, so the + // promoted group lacks it and it reads as a deletion. Crediting the caller + // from their raw request would allow it, but that same reading lost a + // pending edit outright when a caller echoed a protected path, so this + // refuses instead. It fails closed: the publish is refused and the pending + // change is kept. Judging each leaf by the source that won the merge is what + // would allow it. const slug = "provenance"; current = await createTestNextly({ collections: [ @@ -566,15 +569,64 @@ describe("a collection publish re-judges the draft it promotes", () => { } ); - expect(published.success, JSON.stringify(published)).toBe(true); + expect(published.success).toBe(false); + const issues = ( + published as { + publicData?: { errors?: Array<{ path: string; code: string }> }; + } + ).publicData?.errors; + expect(issues?.map(i => i.path)).toEqual(["ops.runbook"]); + expect(issues?.[0]?.code).toBe("FORBIDDEN"); + // Nothing published, and the pending change is still there. const doc = (await current.nextly.findByID({ collection: slug as never, id, overrideAccess: true, status: "all", } as never)) as Record | null; - expect(doc?.body).toBe("edited"); - expect(doc?.ops).toEqual({ note: "clerk-note", runbook: "live-runbook" }); + expect(doc?.body).toBe("live"); + expect(doc?.ops).toEqual({ note: "live-note", runbook: "live-runbook" }); + expect(await pendingDrafts(current, id)).toHaveLength(1); + }); + + it("refuses, and keeps the draft, when the publisher echoes a protected path the pending change edited", async () => { + // A full form resubmits every field, so a publisher routinely sends a + // protected value back unchanged. The field gate strips it, which leaves the + // pending change's edit in the promoted document. Credited to the publisher + // because the path appeared in their request, that edit was restored to live + // and the successful publish deleted the draft: measured before this was + // fixed, success with no pending change left and the author's edit gone. + const t = await boot(); + const h = handlerOf(t); + + const created = await h.createEntry( + { collectionName: SLUG, overrideAccess: true }, + { body: "live", guarded: "live-value", status: "published" } + ); + const id = (created.data as { id?: string }).id as string; + + await h.updateEntry( + { collectionName: SLUG, entryId: id, routeAuthorized: true, user: BOSS }, + { guarded: "boss-secret" } + ); + expect(JSON.stringify(await pendingDrafts(t, id))).toContain("boss-secret"); + + const published = await h.updateEntry( + { collectionName: SLUG, entryId: id, routeAuthorized: true, user: CLERK }, + { status: "published", guarded: "live-value" } + ); + + expect(published.success).toBe(false); + const issues = ( + published as { + publicData?: { errors?: Array<{ path: string; code: string }> }; + } + ).publicData?.errors; + expect(issues?.map(i => i.path)).toEqual(["guarded"]); + expect(issues?.[0]?.code).toBe("FORBIDDEN"); + const live = await liveDoc(t, id); + expect(live.guarded).toBe("live-value"); + expect(JSON.stringify(await pendingDrafts(t, id))).toContain("boss-secret"); }); it("still promotes the change for a publisher who MAY write it", async () => { diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 57e05d1771..7960121545 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -5543,12 +5543,6 @@ export class CollectionMutationService extends BaseService { // unlocalized write (`locale: undefined`): the main row's `status` moves and // is NOT stripped the way a non-default locale's write strips it. The only // thing the wildcard adds is the companion sweep at the write itself. - // What the caller SENT, taken before anything can change it. Hooks may - // rewrite the payload, and the field gate deletes the caller's denied keys in - // place, from `body` itself whenever no hook returned a fresh object: by the - // time a promotion asks whether the caller supplied a value, `body` has - // already lost the answer. Deep, because that gate walks nested containers. - const callerSentBody = detachData(body); const sweepAllLocales = rawParams.locale === EVERY_LOCALE; const params = sweepAllLocales ? { ...rawParams, locale: undefined } @@ -6811,13 +6805,28 @@ export class CollectionMutationService extends BaseService { ), fields ); - // What the caller SENT, as the provenance the resolver reads. It - // asks only whether a path is present, so the raw payload is the - // answer as it stands. Shaping it first is what lost the answer: - // `shapeWriteParts` encodes a group to its column string, and a - // string has no children, so a protected value the caller sent - // inside a group looked unsent and was refused as a deletion. - const callerContribution = callerSentBody; + // The caller's own contribution, from the payload the field gate + // has ALREADY filtered, so it holds only what this publisher was + // allowed to send. Read from the raw request instead, it credited + // the publisher with the pending change's edit whenever they echoed + // the same path: the gate stripped their value, the merged document + // still held the draft's, the resolver treated that edit as theirs + // and restored live, and the publish consumed the draft. Filtered, + // an echoed protected value is simply absent, so the draft's edit + // is refused rather than lost. Logical, through the same conversion + // as the document it is compared against. + const callerContribution = this.deserializeJsonFieldsForSnapshot( + this.assemblePromotedDocument( + {}, + finalData, + componentFieldData, + manyToManyData, + fields, + manyToManyFields, + splitComponentSchemas + ), + fields + ); // One call decides the refusal AND returns what to write, so the // document that was judged is the document that lands. A denied // field comes back at its LIVE value rather than removed: taking diff --git a/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts b/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts index 63ea9d1122..cb6f5a5fce 100644 --- a/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts +++ b/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts @@ -154,41 +154,81 @@ describe("resolvePromotedDocument", () => { }); }); - it("does not import a stale live verdict for a field the promotion still holds", async () => { - // A rule reads its siblings. Live says `kind: "private"`, which denies - // `guarded`; the pending change sets `kind` to `public` and edits `guarded` - // legitimately. The promoted document is the one that has the right of it, - // so taking live's verdict too would refuse a valid publish. + it("refuses when a stale live verdict denies a field the final document allows", async () => { + // A KNOWN over-refusal, pinned so it cannot change unnoticed. Live says + // `kind: "private"`, which denies `guarded`; the pending change sets `kind` + // to `public` and edits `guarded`, which the final document would allow. + // The live verdict is kept whole because filtering it by path lost data when + // repeater rows shifted, so this refuses. It fails closed: nothing is lost, + // the publish is refused and the pending change is kept. Judging rows by + // identity and siblings on the final document is what would allow it. const rulesByKind = (document: Record): Promise => { if (document.kind === "private") delete document.guarded; return Promise.resolve(); }; - const out = await resolve({ - before: { kind: "public", guarded: "edited" }, - live: { kind: "private", guarded: "live" }, - applyRules: rulesByKind, - }); - expect(out).toEqual({ kind: "public", guarded: "edited" }); + await expect( + resolve({ + before: { kind: "public", guarded: "edited" }, + live: { kind: "private", guarded: "live" }, + applyRules: rulesByKind, + }) + ).rejects.toMatchObject({ code: "VALIDATION_ERROR" }); }); it("refuses a deleted child even when the live rule denies its whole container", async () => { - // The live rule removes `seo` entirely, so the live-side denial names the - // container. The promotion keeps `seo` and drops `secret` from inside it, - // so a filter applied at the container asks the wrong question and the - // deletion goes through unjudged. + // The live rule removes `seo` entirely, so the live denial names the + // container while the promotion keeps `seo` and drops `secret` from it. The + // dropped child must be judged. Its sibling `title` is refused as well, since + // the whole-container verdict is not narrowed by what the final document + // would allow, which fails closed; this asserts the child it exists for. const rulesByKind = (document: Record): Promise => { if (document.kind === "private") delete document.seo; return Promise.resolve(); }; + let paths: string[] = []; + await resolve({ + before: { kind: "public", seo: { title: "new" } }, + live: { kind: "private", seo: { title: "old", secret: "live" } }, + applyRules: rulesByKind, + }).catch((error: { publicData?: { errors?: Array<{ path: string }> } }) => { + paths = (error.publicData?.errors ?? []).map(issue => issue.path); + }); + expect(paths).toContain("seo.secret"); + }); + + it("refuses deleting a protected repeater row when a later row takes its index", async () => { + // Paths are positions. The pending change deletes live row 0, which is + // private and so denies `secret`, and the public row shifts into index 0. A + // check that asked whether `rows[0].secret` still exists in the promotion + // answered yes, and the protected row was deleted without refusal. Measured + // before this was fixed: resolved, with only the public row left. + const rowRule = (document: Record): Promise => { + const rows = document.rows; + if (Array.isArray(rows)) { + for (const row of rows) { + if ( + row && + typeof row === "object" && + (row as Record).kind === "private" + ) { + delete (row as Record).secret; + } + } + } + return Promise.resolve(); + }; await expect( resolve({ - before: { kind: "public", seo: { title: "new" } }, - live: { kind: "private", seo: { title: "old", secret: "live" } }, - applyRules: rulesByKind, + before: { rows: [{ kind: "public", secret: "s1" }] }, + live: { + rows: [ + { kind: "private", secret: "s0" }, + { kind: "public", secret: "s1" }, + ], + }, + applyRules: rowRule, }) - ).rejects.toMatchObject({ - publicData: { errors: [{ path: "seo.secret" }] }, - }); + ).rejects.toMatchObject({ code: "VALIDATION_ERROR" }); }); it("keeps an own __proto__ key instead of invoking the prototype setter", async () => { diff --git a/packages/nextly/src/shared/lib/denied-change.ts b/packages/nextly/src/shared/lib/denied-change.ts index 00e3340324..94aaf8afef 100644 --- a/packages/nextly/src/shared/lib/denied-change.ts +++ b/packages/nextly/src/shared/lib/denied-change.ts @@ -145,33 +145,29 @@ export async function resolvePromotedDocument( } /** - * Every path the rules deny: the promoted document's own verdict, plus from the - * live row ONLY the fields the promotion no longer carries. + * Every path the rules deny, on the promoted document AND on the live row, + * both kept whole. * - * Live is consulted for one reason: a rule is never asked about a key that is - * absent, so a field the pending change removes outright is judged nowhere. - * Taking live's verdict for a field the promotion still holds would import a - * stale answer instead, because a rule reads its siblings: where live says - * `kind: "private"` denies `guarded`, and the pending change sets `kind` to - * `public` and edits `guarded` legitimately, the promoted document is the one - * that has the right of it. + * The live row is consulted because a rule is never asked about a key that is + * absent, so a field the pending change removes outright is judged nowhere else. + * An earlier revision filtered the live verdict down to the paths the promotion + * no longer carries, and that filter made the check unsafe: a path is a + * position, so when a pending change deletes a repeater row the row after it + * takes its index, the protected row's path still exists, and the deletion went + * through unjudged. Kept whole, a stale live verdict can refuse a publish the + * final document would allow, which fails closed; filtered, it failed open and + * lost data. Judging rows by identity rather than position is what would make + * this precise in both directions. */ function collectDenied( input: PromotionAccessInput, permittedBefore: Record, permittedLive: Record ): Set { - const denied = new Set(deniedPaths(input.before, permittedBefore, "")); - for (const path of deniedPaths(input.live, permittedLive, "")) { - // Expanded to leaves BEFORE the filter, because a rule denies a container - // whole. Filtering at the container asks "does the promotion still carry - // `seo`", which is yes even when it has dropped `seo.secret` from inside - // it, and the dropped child is what the live side was consulted for. - for (const leaf of leafPaths(valueAt(input.live, path), path)) { - if (!pathExists(input.before, leaf)) denied.add(leaf); - } - } - return denied; + return new Set([ + ...deniedPaths(input.before, permittedBefore, ""), + ...deniedPaths(input.live, permittedLive, ""), + ]); } /** From cfb2a55a90917f0ec34e8e644165f1c877debce9 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 18:04:57 +0300 Subject: [PATCH 07/16] test(nextly): pin the refusal of a deleted field the final document would allow The resolver refuses a publish that deletes a field whose live rule denies it, even when the promoted siblings would allow it. The edited variant was already pinned. The deleted one is decided by the live row's verdict alone, so it gets its own test, and both share one rule helper. --- .../lib/__tests__/denied-change.test.ts | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts b/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts index cb6f5a5fce..bc7dd1ab17 100644 --- a/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts +++ b/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts @@ -37,6 +37,14 @@ function denies(...paths: string[]) { const allow = (): Promise => Promise.resolve(); +/** Denies `guarded` while `kind` is private: a rule that reads a sibling. */ +function deniesGuardedWhilePrivate( + document: Record +): Promise { + if (document.kind === "private") delete document.guarded; + return Promise.resolve(); +} + function resolve( input: Partial[0]> & { before: Record; @@ -162,19 +170,34 @@ describe("resolvePromotedDocument", () => { // repeater rows shifted, so this refuses. It fails closed: nothing is lost, // the publish is refused and the pending change is kept. Judging rows by // identity and siblings on the final document is what would allow it. - const rulesByKind = (document: Record): Promise => { - if (document.kind === "private") delete document.guarded; - return Promise.resolve(); - }; await expect( resolve({ before: { kind: "public", guarded: "edited" }, live: { kind: "private", guarded: "live" }, - applyRules: rulesByKind, + applyRules: deniesGuardedWhilePrivate, }) ).rejects.toMatchObject({ code: "VALIDATION_ERROR" }); }); + it("refuses deleting a field the final document's siblings would allow", async () => { + // A KNOWN over-refusal, pinned so it cannot change unnoticed: the same rule, + // with the pending change deleting `guarded` rather than editing it. A + // deleted field is absent from the promotion, so the live row's verdict is + // the only one that names it, and that verdict reads the live siblings. It + // fails closed: the publish is refused and the pending change is kept. + // Judging a deleted field against the promoted siblings is what would allow + // it. + await expect( + resolve({ + before: { kind: "public" }, + live: { kind: "private", guarded: "live" }, + applyRules: deniesGuardedWhilePrivate, + }) + ).rejects.toMatchObject({ + publicData: { errors: [{ path: "guarded" }] }, + }); + }); + it("refuses a deleted child even when the live rule denies its whole container", async () => { // The live rule removes `seo` entirely, so the live denial names the // container while the promotion keeps `seo` and drops `secret` from it. The From f68a8e474f98800eefb5db028727348068fb6634 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 21:08:05 +0300 Subject: [PATCH 08/16] docs(nextly): state the promotion resolver's rules, not how they came to be The resolver's comments and the tests that pin it described what earlier versions did and what was measured before a fix. They now state the rule each piece of code holds, and why, in the present tense. The module header also drops the comparison to another product. --- ...e-judges-the-publisher.integration.test.ts | 45 ++++++++--------- .../services/collection-mutation-service.ts | 49 +++++++++---------- .../lib/__tests__/denied-change.test.ts | 25 +++++----- .../nextly/src/shared/lib/denied-change.ts | 48 +++++++++--------- 4 files changed, 79 insertions(+), 88 deletions(-) diff --git a/packages/nextly/src/domains/collections/__tests__/collection-promote-judges-the-publisher.integration.test.ts b/packages/nextly/src/domains/collections/__tests__/collection-promote-judges-the-publisher.integration.test.ts index 7844eb7890..2bfcbf79e6 100644 --- a/packages/nextly/src/domains/collections/__tests__/collection-promote-judges-the-publisher.integration.test.ts +++ b/packages/nextly/src/domains/collections/__tests__/collection-promote-judges-the-publisher.integration.test.ts @@ -228,9 +228,8 @@ describe("a collection publish re-judges the draft it promotes", () => { // it was serialised to while the live row comes back from the driver as a // `Date`. Compared as they arrive, a date-bearing field the publisher may // not write reads as an edit and refuses a publish that touches nothing. - // Every type is covered rather than the one that broke: measured, text, - // number, boolean, JSON and group agreed and only the date did not, and a - // suite that checked one of them would not have said so. + // Every type is covered: only the date arrives in two representations, and + // a suite that checked one type would not show which. const onlyBoss = { update: ({ req }: { req?: { user?: { email?: string } } }) => req?.user?.email === BOSS.email, @@ -339,12 +338,11 @@ describe("a collection publish re-judges the draft it promotes", () => { }); it("refuses a pending change that CLEARS a field it may not write", async () => { - // A GUARD, not a demonstration: this passes against the previous revision - // too. Clearing a field sends `null`, which is a present key and so is - // judged like any other value. The case the live-side pass exists for is a - // key ABSENT from the promoted document entirely, which no route through - // the public API was found to produce, since a collection snapshot is a - // full copy of the row. That half stays defensive. + // A GUARD, not a demonstration. Clearing a field sends `null`, which is a + // present key and so is judged like any other value. The case the live-side + // pass exists for is a key ABSENT from the promoted document entirely, which + // no known route through the public API produces, since a collection + // snapshot is a full copy of the row, so that half is guarded defensively. const t = await boot(); const h = handlerOf(t); @@ -381,8 +379,8 @@ describe("a collection publish re-judges the draft it promotes", () => { // The shaping pass coerces a caller's date to a `Date` before the resolver // sees it, and a `Date` is an object with no enumerable keys: a rebuild // that treats every object as a container returns `{}` and the driver then - // refuses the write outright. Measured before the fix: the publish failed - // with "value.getTime is not a function" and nothing went live. + // refuses that write with "value.getTime is not a function", so nothing + // goes live. const slug = "dated"; current = await createTestNextly({ collections: [ @@ -446,11 +444,11 @@ describe("a collection publish re-judges the draft it promotes", () => { it("publishes a group edit sent with the status while a change is pending", async () => { // No field rules at all, so nothing here is about access. The ordinary - // write encodes a group to its column string before the promotion runs, and - // the check that validates the promoted document read that string where it - // expects an object: a publish that also edited any group, repeater or JSON - // field was refused as "ops must be an object" whenever a pending change - // existed, which is the ordinary shape of an editor publishing their edit. + // write encodes a group to its column string before the promotion runs, so + // the promoted document has to be validated in its logical shape: read as + // the column string, any group, repeater or JSON field is refused as "ops + // must be an object", and publishing alongside an edit to one is the + // ordinary shape of an editor publishing their work. const slug = "grouppublish"; current = await createTestNextly({ collections: [ @@ -505,11 +503,10 @@ describe("a collection publish re-judges the draft it promotes", () => { // caller sends replaces the pending change's group whole, and the field gate // has already stripped the protected child the caller included, so the // promoted group lacks it and it reads as a deletion. Crediting the caller - // from their raw request would allow it, but that same reading lost a - // pending edit outright when a caller echoed a protected path, so this - // refuses instead. It fails closed: the publish is refused and the pending - // change is kept. Judging each leaf by the source that won the merge is what - // would allow it. + // from their raw request would allow it, and would also credit a caller who + // echoes a protected path with the pending change's edit, so this refuses. + // It fails closed: the publish is refused and the pending change is kept. + // Judging each leaf by the source that won the merge is what would allow it. const slug = "provenance"; current = await createTestNextly({ collections: [ @@ -593,9 +590,9 @@ describe("a collection publish re-judges the draft it promotes", () => { // A full form resubmits every field, so a publisher routinely sends a // protected value back unchanged. The field gate strips it, which leaves the // pending change's edit in the promoted document. Credited to the publisher - // because the path appeared in their request, that edit was restored to live - // and the successful publish deleted the draft: measured before this was - // fixed, success with no pending change left and the author's edit gone. + // because the path appears in their request, that edit would be restored to + // live while the successful publish deletes the draft, so the author's edit + // would be gone with success reported. const t = await boot(); const h = handlerOf(t); diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 7960121545..101f0d3ccc 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -6207,8 +6207,8 @@ export class CollectionMutationService extends BaseService { // alone, so a collection whose registered functions are validators, // defaults, hooks or READ rules never reaches a lookup. Without this // every publish, unpublish and republish on a draft-enabled collection - // paid for the roles and permissions queries to answer a question - // nothing would ask. Both tests are map reads. + // would pay for the roles and permissions queries to answer a question + // nothing asks. Both tests are map reads. const promoteRulesCouldRun = promotePossible && params.overrideAccess !== true && @@ -6279,10 +6279,9 @@ export class CollectionMutationService extends BaseService { // In the LOGICAL shape the validator reads. `finalData` has already // been through `shapeWriteParts`, which encodes every JSON-backed field // to its column string, so a group the caller sent with the publish - // arrives here as text and is refused as "must be an object": a - // publish carrying any group, repeater or JSON value failed whenever a - // pending change existed. Parsed by the same function that builds the - // live row, so the two sides are one representation. + // arrives here as text, and validated as text it is refused as "must + // be an object". Parsed by the same function that builds the live row, + // so the two sides are one representation. const merged = this.deserializeJsonFieldsForSnapshot( this.assemblePromotedDocument( draftInput, @@ -6296,12 +6295,11 @@ export class CollectionMutationService extends BaseService { fields ); // Validated as ASSEMBLED, with nothing removed. A field this - // publisher may not write is not a schema violation, and the gate - // that judges permission now refuses the promotion outright rather - // than dropping the value, under the row lock where it can compare - // against what is live. Filtering here would validate a document - // that is never written either way, and report a denied required - // field as missing rather than as forbidden. + // publisher may not write is not a schema violation: the gate that + // judges permission refuses a promotion that changes one, under the + // row lock where it can compare against what is live. Filtering here + // would validate a document that is never written either way, and + // report a denied required field as missing rather than as forbidden. const localeCtx = await this.localizedRequiredContext( params.collectionName, params.locale @@ -6780,14 +6778,13 @@ export class CollectionMutationService extends BaseService { // Assemble the full document the promotion persists (the locked // draft, the caller's scalars overlaid, the caller's single-component // patches merged onto the draft's components, and the caller's m2m), - // then filter it through the current field-level write access. A rule + // then judge it against the current field-level write access. A rule // that depends on a sibling the publish patch supplies (e.g. a field // writable only when `approved` is true, where the publish sets it - // false) is judged on the real final values, and a denied value is - // dropped at any depth for column, component, and m2m fields alike. - // Re-extracting the write parts from the FILTERED document keeps a - // denied component/m2m value out of the persisted parts, which the - // earlier after-access merge would have restored. + // false) is judged on the real final values, at any depth, for column, + // component, and m2m fields alike. The write parts are re-extracted + // from the RESOLVED document, so a denied component or m2m value is + // written at what the row already holds. // Logical, for the same reason as the check before the // transaction: `finalData` is already column-encoded, and a group // left as its string is one leaf to the resolver, so a denied field @@ -6807,14 +6804,14 @@ export class CollectionMutationService extends BaseService { ); // The caller's own contribution, from the payload the field gate // has ALREADY filtered, so it holds only what this publisher was - // allowed to send. Read from the raw request instead, it credited - // the publisher with the pending change's edit whenever they echoed - // the same path: the gate stripped their value, the merged document - // still held the draft's, the resolver treated that edit as theirs - // and restored live, and the publish consumed the draft. Filtered, - // an echoed protected value is simply absent, so the draft's edit - // is refused rather than lost. Logical, through the same conversion - // as the document it is compared against. + // allowed to send. Read from the raw request, it would credit the + // publisher with the pending change's edit whenever they echo the + // same path: the gate strips their value, the merged document still + // holds the draft's, the resolver treats that edit as theirs and + // restores live, and the publish consumes the draft. Filtered, an + // echoed protected value is simply absent, so the draft's edit is + // refused rather than lost. Logical, through the same conversion as + // the document it is compared against. const callerContribution = this.deserializeJsonFieldsForSnapshot( this.assemblePromotedDocument( {}, diff --git a/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts b/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts index bc7dd1ab17..8f3638d01c 100644 --- a/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts +++ b/packages/nextly/src/shared/lib/__tests__/denied-change.test.ts @@ -3,11 +3,10 @@ * * It is a pure function answering an intricate question over four documents, * and the integration suites reach it only through a whole publish, one shape - * per test at considerable cost. Every defect it has had so far was a shape - * question the caller could not see: a `Date` rebuilt as `{}`, a container - * exempted because the caller supplied one field of it, a property deleted on - * the live side that no traversal enumerated. Those belong here, where a shape - * costs three lines. + * per test at considerable cost. The hard cases are shapes a caller cannot + * see: a `Date` that must not be rebuilt as `{}`, a container whose contents + * have two authors, a property present only on the live side. Those belong + * here, where a shape costs three lines. * * @module shared/lib/__tests__/denied-change.test */ @@ -166,10 +165,10 @@ describe("resolvePromotedDocument", () => { // A KNOWN over-refusal, pinned so it cannot change unnoticed. Live says // `kind: "private"`, which denies `guarded`; the pending change sets `kind` // to `public` and edits `guarded`, which the final document would allow. - // The live verdict is kept whole because filtering it by path lost data when - // repeater rows shifted, so this refuses. It fails closed: nothing is lost, - // the publish is refused and the pending change is kept. Judging rows by - // identity and siblings on the final document is what would allow it. + // The live verdict is kept whole because a path is a position, so this + // refuses. It fails closed: nothing is lost, the publish is refused and the + // pending change is kept. Judging rows by identity and siblings on the final + // document is what would allow it. await expect( resolve({ before: { kind: "public", guarded: "edited" }, @@ -221,10 +220,10 @@ describe("resolvePromotedDocument", () => { it("refuses deleting a protected repeater row when a later row takes its index", async () => { // Paths are positions. The pending change deletes live row 0, which is - // private and so denies `secret`, and the public row shifts into index 0. A - // check that asked whether `rows[0].secret` still exists in the promotion - // answered yes, and the protected row was deleted without refusal. Measured - // before this was fixed: resolved, with only the public row left. + // private and so denies `secret`, and the public row shifts into index 0. + // `rows[0].secret` still exists in the promotion, so a check asking only + // whether a denied path survived would let the protected row's deletion + // through; the refusal comes from the live verdict kept whole. const rowRule = (document: Record): Promise => { const rows = document.rows; if (Array.isArray(rows)) { diff --git a/packages/nextly/src/shared/lib/denied-change.ts b/packages/nextly/src/shared/lib/denied-change.ts index 94aaf8afef..15d15de225 100644 --- a/packages/nextly/src/shared/lib/denied-change.ts +++ b/packages/nextly/src/shared/lib/denied-change.ts @@ -9,16 +9,15 @@ * question and deserves one answer. * * ONE function answers it AND returns the document to write, rather than a - * caller applying the rules and interpreting the result for itself. Splitting - * those apart is what went wrong in an earlier revision: the rules DELETE a - * denied value, the refusal was judged from that deletion, and the same - * stripped document was then handed to the write, so a protected value nobody - * had touched was cleared by an unrelated publish. + * caller applying the rules and interpreting the result for itself. The rules + * DELETE a denied value, so a caller that judged the refusal from that deletion + * and then wrote the same stripped document would clear a protected value + * nobody touched; returning the document from the judgment keeps the two from + * disagreeing. * - * A denied field keeps its LIVE value. That is what an update means, the caller - * may not write the field so the field does not change, and it is the answer - * Payload gives to the same question. Removing it instead writes an absence - * nobody asked for. + * A denied field keeps its LIVE value. That is what an update means: the caller + * may not write the field, so the field does not change. Removing it instead + * writes an absence nobody asked for. * * A refusal is reserved for a change the PENDING CHANGE makes. A denied value * the caller sent with the publish is their own input, and dropping it back to @@ -150,14 +149,13 @@ export async function resolvePromotedDocument( * * The live row is consulted because a rule is never asked about a key that is * absent, so a field the pending change removes outright is judged nowhere else. - * An earlier revision filtered the live verdict down to the paths the promotion - * no longer carries, and that filter made the check unsafe: a path is a - * position, so when a pending change deletes a repeater row the row after it - * takes its index, the protected row's path still exists, and the deletion went - * through unjudged. Kept whole, a stale live verdict can refuse a publish the - * final document would allow, which fails closed; filtered, it failed open and - * lost data. Judging rows by identity rather than position is what would make - * this precise in both directions. + * Both verdicts stay whole because a path is a position: when a pending change + * deletes a repeater row, the row after it takes its index, so the protected + * row's path still exists in the promotion and a verdict narrowed to missing + * paths would let the deletion through unjudged. Kept whole, a stale live + * verdict can refuse a publish the final document would allow, which fails + * closed. Judging rows by identity rather than position is what would make this + * precise in both directions. */ function collectDenied( input: PromotionAccessInput, @@ -417,8 +415,8 @@ function leafPaths(value: unknown, prefix: string): string[] { * A pending change is JSON, so a timestamp reaches here as the ISO string it * was serialised to, while the live row comes back from the driver as a `Date`. * Compared as they are, every date-bearing field the publisher may not write - * reads as an edit and refuses a publish that touches nothing. Measured: of - * text, number, boolean, JSON, group and date, only the date diverged. + * reads as an edit and refuses a publish that touches nothing. Of text, number, + * boolean, JSON, group and date, only the date arrives in two representations. */ function sameStoredValue(a: unknown, b: unknown): boolean { return isDeepStrictEqual(asComparable(a), asComparable(b)); @@ -486,12 +484,12 @@ function hasOwn(record: object, key: string): boolean { * A PLAIN record, and the distinction is load-bearing. * * A `Date` is an object with no enumerable keys of its own, so a walk that - * treats every object as a container rebuilds one as `{}` and the driver then - * refuses it: measured, a caller who supplied a date with the publish got - * `value.getTime is not a function` and no publish at all. The same is true of - * anything else the store round-trips as a value rather than a shape, a - * `Buffer` or a `RegExp` among them. Only an object made from `{}` or from a - * null prototype carries children worth descending into. + * treats every object as a container rebuilds one as `{}`, and the driver + * refuses that with `value.getTime is not a function`: a publish carrying a + * date would fail outright. The same is true of anything else the store + * round-trips as a value rather than a shape, a `Buffer` or a `RegExp` among + * them. Only an object made from `{}` or from a null prototype carries + * children worth descending into. */ function isRecord(value: unknown): value is Record { if (typeof value !== "object" || value === null || Array.isArray(value)) { From 3cb2f37e11253e6fe9251c52283b0e201bda38e1 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 14:07:48 +0300 Subject: [PATCH 09/16] fix(admin): show why publishing all languages failed, not one fixed message --- .../admin/src/hooks/queries/usePublishAllLocales.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/admin/src/hooks/queries/usePublishAllLocales.ts b/packages/admin/src/hooks/queries/usePublishAllLocales.ts index fb63776608..9c00b87a01 100644 --- a/packages/admin/src/hooks/queries/usePublishAllLocales.ts +++ b/packages/admin/src/hooks/queries/usePublishAllLocales.ts @@ -38,8 +38,17 @@ export function usePublishAllLocales({ }); if (!silent) toast.success("All languages published."); }, - onError: () => { - if (!silent) toast.error("Couldn't publish all languages."); + onError: (error: unknown) => { + // The server's own message, which is the only thing that says WHY. This + // call can refuse for reasons an editor can act on, and a fixed string + // turns every one of them into the same unexplained failure. + if (!silent) { + toast.error( + error instanceof Error && error.message + ? error.message + : "Couldn't publish all languages." + ); + } }, }); } From c6fe70b1682b128f828d34e44672f0d66f00ad8f Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 19:09:30 +0300 Subject: [PATCH 10/16] feat(nextly): fold every language's pending change by field, not by save A pending change stores the whole document as it was when saved, so a shared value in it is either an edit or a stale copy of what was live. The rule takes a shared value only where it differs from live, oldest save first, and each language's translations from its own change, matched on instance id inside components. --- .../__tests__/pending-change-merge.test.ts | 182 +++++++++++ .../services/pending-change-merge.ts | 284 ++++++++++++++++++ 2 files changed, 466 insertions(+) create mode 100644 packages/nextly/src/domains/collections/services/__tests__/pending-change-merge.test.ts create mode 100644 packages/nextly/src/domains/collections/services/pending-change-merge.ts diff --git a/packages/nextly/src/domains/collections/services/__tests__/pending-change-merge.test.ts b/packages/nextly/src/domains/collections/services/__tests__/pending-change-merge.test.ts new file mode 100644 index 0000000000..379e1efa24 --- /dev/null +++ b/packages/nextly/src/domains/collections/services/__tests__/pending-change-merge.test.ts @@ -0,0 +1,182 @@ +/** + * The rule that folds every language's pending change into one write. + * + * @module domains/collections/services/__tests__/pending-change-merge.test + */ +import { describe, expect, it } from "vitest"; + +import { + changedKeys, + languageTarget, + pendingChangesToApply, + sameContent, + withTranslationsFrom, + withoutTranslations, + type ComponentValueShape, +} from "../pending-change-merge"; + +const FLAT: ComponentValueShape = { + translatableKeys: () => new Set(["heading"]), + nested: () => undefined, +}; + +describe("pendingChangesToApply", () => { + it("applies the oldest save first and skips languages the app does not configure", () => { + const out = pendingChangesToApply( + [ + { locale: "de", snapshot: {}, updatedAt: "2026-01-02T00:00:00.000Z" }, + { locale: "fr", snapshot: {}, updatedAt: "2026-01-01T00:00:00.000Z" }, + { locale: null, snapshot: {}, updatedAt: "2026-01-01T00:00:00.000Z" }, + { + locale: "en", + snapshot: {}, + updatedAt: new Date("2026-01-01T00:00:00.000Z"), + }, + ], + new Set(["en", "de"]) + ); + expect(out.map(change => change.locale)).toEqual(["en", "de"]); + }); + + it("orders two saves at the same instant by language, so the result never depends on the database", () => { + const at = "2026-01-01T00:00:00.000Z"; + const out = pendingChangesToApply( + [ + { locale: "en", snapshot: {}, updatedAt: at }, + { locale: "de", snapshot: {}, updatedAt: at }, + ], + new Set(["en", "de"]) + ); + expect(out.map(change => change.locale)).toEqual(["de", "en"]); + }); +}); + +describe("sameContent", () => { + it("treats an instant and its ISO string as the same value", () => { + expect( + sameContent( + new Date("2026-01-02T03:04:05.000Z"), + "2026-01-02T03:04:05.000Z" + ) + ).toBe(true); + }); + + it("counts a declared field named like a row timestamp as content", () => { + // Both sides arrive schema-shaped, so a key left in them is a field the + // author declared, and an edit to it must not read as unchanged. + expect( + sameContent( + { meta: { updatedAt: "edited" } }, + { meta: { updatedAt: "live" } } + ) + ).toBe(false); + }); + + it("still tells two instance ids apart", () => { + expect(sameContent([{ id: "r1" }], [{ id: "r2" }])).toBe(false); + }); +}); + +describe("withoutTranslations and withTranslationsFrom", () => { + it("drops translatable keys and keeps the structure", () => { + expect( + withoutTranslations( + [{ id: "r1", heading: "Hallo", variant: "wide" }], + FLAT + ) + ).toEqual([{ id: "r1", variant: "wide" }]); + }); + + it("takes one language's translations onto the current instances by id", () => { + const out = withTranslationsFrom({ + current: [ + { id: "r1", heading: "old", variant: "wide" }, + { id: "r2", heading: "added elsewhere", variant: "narrow" }, + ], + pending: [{ id: "r1", heading: "Hallo", variant: "stale" }], + shape: FLAT, + }); + expect(out).toEqual([ + { id: "r1", heading: "Hallo", variant: "wide" }, + { id: "r2", heading: "added elsewhere", variant: "narrow" }, + ]); + }); +}); + +describe("languageTarget", () => { + const base = { + localizedFieldNames: new Set(["title"]), + componentFields: new Map([["blocks", FLAT]]), + }; + + it("keeps a shared edit made earlier in the write when this change never touched it", () => { + const out = languageTarget({ + ...base, + live: { title: "DE", note: "live note" }, + current: { title: "DE", note: "EN edited note" }, + pending: { title: "DE v2", note: "live note" }, + }); + expect(out).toEqual({ title: "DE v2", note: "EN edited note" }); + }); + + it("takes a shared value this change edited", () => { + const out = languageTarget({ + ...base, + live: { note: "live note" }, + current: { note: "live note" }, + pending: { note: "DE edited note" }, + }); + expect(out.note).toBe("DE edited note"); + }); + + it("keeps a field the pending change does not hold", () => { + const out = languageTarget({ + ...base, + live: { note: "live", added: "x" }, + current: { note: "live", added: "x" }, + pending: { note: "live" }, + }); + expect(out.added).toBe("x"); + }); + + it("keeps a block another language added when this change only translated", () => { + const out = languageTarget({ + ...base, + live: { blocks: [{ id: "r1", heading: "EN", variant: "wide" }] }, + current: { + blocks: [ + { id: "r1", heading: "EN", variant: "wide" }, + { id: "r2", heading: "EN new", variant: "narrow" }, + ], + }, + pending: { blocks: [{ id: "r1", heading: "DE", variant: "wide" }] }, + }); + expect(out.blocks).toEqual([ + { id: "r1", heading: "DE", variant: "wide" }, + { id: "r2", heading: "EN new", variant: "narrow" }, + ]); + }); + + it("takes this change's blocks when it changed their structure", () => { + const pendingBlocks = [{ id: "r1", heading: "DE", variant: "narrow" }]; + const out = languageTarget({ + ...base, + live: { blocks: [{ id: "r1", heading: "EN", variant: "wide" }] }, + current: { blocks: [{ id: "r1", heading: "EN", variant: "wide" }] }, + pending: { blocks: pendingBlocks }, + }); + expect(out.blocks).toEqual(pendingBlocks); + }); +}); + +describe("changedKeys", () => { + it("names only what differs from the document as it stands", () => { + expect([ + ...changedKeys( + { a: 1, b: "2026-01-01T00:00:00.000Z" }, + { a: 1, b: new Date("2026-01-01T00:00:00.000Z") } + ), + ]).toEqual([]); + expect([...changedKeys({ a: 2 }, { a: 1 })]).toEqual(["a"]); + }); +}); diff --git a/packages/nextly/src/domains/collections/services/pending-change-merge.ts b/packages/nextly/src/domains/collections/services/pending-change-merge.ts new file mode 100644 index 0000000000..9dc9a0557d --- /dev/null +++ b/packages/nextly/src/domains/collections/services/pending-change-merge.ts @@ -0,0 +1,284 @@ +/** + * Folding every language's pending change into one write. + * + * A pending change stores the whole document as it looked when its author saved + * it, not only the fields they touched, so a shared value inside one is either + * an edit or a stale copy of what was live, and the two look identical. Applied + * one after another, a later save would put back a shared value another + * language's author had changed. A shared value is therefore taken from a + * pending change only where it differs from the live row, oldest save first, so + * a later save wins only when two languages changed the same value. Each + * language's translations come from its own pending change. + * + * Every value compared here is schema-shaped, the way a pending change is + * restored, so a key present on either side is a field the schema declares. + * + * @module domains/collections/services/pending-change-merge + */ + +import { isDeepStrictEqual } from "node:util"; + +/** One language's pending change, ready to apply. */ +export interface PendingLanguageChange { + locale: string; + snapshot: unknown; + updatedAt: Date; +} + +/** + * How one component field's instances hold translations. + * + * Asked per instance, because a dynamic zone mixes component types and each + * type declares its own translatable fields. + */ +export interface ComponentValueShape { + translatableKeys(instance: Record): ReadonlySet; + nested( + instance: Record, + key: string + ): ComponentValueShape | undefined; +} + +/** What building one language's write needs. */ +export interface LanguageTargetInput { + /** The pending change. */ + pending: Record; + /** The document as it was live before this write began. */ + live: Record; + /** The document as it stands now inside this write. */ + current: Record; + /** The document's own translatable fields. */ + localizedFieldNames: ReadonlySet; + /** Component fields by name. */ + componentFields: ReadonlyMap; +} + +/** + * The pending changes a whole-document write applies, oldest save first. + * + * Only languages the app still configures: a change held for a removed language + * cannot be read or written through the API, so it stays where it is rather + * than being published or deleted as a side effect. Ties break on the language + * code, so the order never depends on what the database happens to return. + */ +export function pendingChangesToApply( + drafts: ReadonlyArray<{ + locale: string | null; + snapshot: unknown; + updatedAt: Date | string; + }>, + configuredLocales: ReadonlySet +): PendingLanguageChange[] { + const out: PendingLanguageChange[] = []; + for (const draft of drafts) { + if (draft.locale === null || !configuredLocales.has(draft.locale)) continue; + out.push({ + locale: draft.locale, + snapshot: draft.snapshot, + updatedAt: new Date(draft.updatedAt), + }); + } + return out.sort( + (a, b) => + a.updatedAt.getTime() - b.updatedAt.getTime() || + a.locale.localeCompare(b.locale) + ); +} + +/** + * Whether two values hold the same content. + * + * An instant compares by the moment it names: a pending change is JSON, so it + * carries the ISO string, while the row comes back from the driver as a `Date`. + */ +export function sameContent(a: unknown, b: unknown): boolean { + return isDeepStrictEqual(comparable(a), comparable(b)); +} + +/** The keys of `target` whose value differs from `current`. */ +export function changedKeys( + target: Record, + current: Record +): Set { + const out = new Set(); + for (const key of Object.keys(target)) { + if (!sameContent(target[key], current[key])) out.add(key); + } + return out; +} + +/** A component value with every translatable key removed, at every depth. */ +export function withoutTranslations( + value: unknown, + shape: ComponentValueShape +): unknown { + if (Array.isArray(value)) { + return value.map(instance => withoutTranslations(instance, shape)); + } + if (!isPlainRecord(value)) return value; + const translatable = shape.translatableKeys(value); + const out: Record = {}; + for (const [key, child] of Object.entries(value)) { + if (translatable.has(key)) continue; + const nested = shape.nested(value, key); + assign(out, key, nested ? withoutTranslations(child, nested) : child); + } + return out; +} + +/** + * The current component value carrying one language's translations from its + * pending change, matched instance by instance on `id`. + * + * Structure stays as it currently stands: which instances exist, their order, + * and their shared values. An instance the pending change does not hold keeps + * the translations it has. + */ +export function withTranslationsFrom(args: { + current: unknown; + pending: unknown; + shape: ComponentValueShape; +}): unknown { + const sources = instancesById(args.pending); + const overlay = (instance: unknown): unknown => { + if (!isPlainRecord(instance) || typeof instance.id !== "string") { + return instance; + } + const source = sources.get(instance.id); + return source ? overlayInstance(instance, source, args.shape) : instance; + }; + return Array.isArray(args.current) + ? args.current.map(overlay) + : overlay(args.current); +} + +/** The document one language's promotion writes. */ +export function languageTarget( + input: LanguageTargetInput +): Record { + const out: Record = {}; + const keys = new Set([ + ...Object.keys(input.current), + ...Object.keys(input.pending), + ]); + for (const key of keys) { + assign(out, key, valueForKey(input, key)); + } + return out; +} + +function valueForKey(input: LanguageTargetInput, key: string): unknown { + const { pending, live, current } = input; + // A field the pending change does not hold, one the schema gained after it + // was saved, is a field it says nothing about. + if (!hasOwn(pending, key)) return current[key]; + if (input.localizedFieldNames.has(key)) return pending[key]; + const shape = input.componentFields.get(key); + if (shape) + return componentValue(pending[key], live[key], current[key], shape); + return sameContent(pending[key], live[key]) ? current[key] : pending[key]; +} + +/** + * A component field's value: this change's own instances when it changed their + * structure or shared values, otherwise the current instances carrying this + * language's translations. + */ +function componentValue( + pending: unknown, + live: unknown, + current: unknown, + shape: ComponentValueShape +): unknown { + const structureChanged = !sameContent( + withoutTranslations(pending, shape), + withoutTranslations(live, shape) + ); + return structureChanged + ? pending + : withTranslationsFrom({ current, pending, shape }); +} + +function overlayInstance( + instance: Record, + source: Record, + shape: ComponentValueShape +): Record { + const out: Record = {}; + const translatable = shape.translatableKeys(instance); + for (const [key, child] of Object.entries(instance)) { + const nested = shape.nested(instance, key); + if (!hasOwn(source, key)) { + assign(out, key, child); + } else if (translatable.has(key)) { + assign(out, key, source[key]); + } else if (nested) { + assign( + out, + key, + withTranslationsFrom({ + current: child, + pending: source[key], + shape: nested, + }) + ); + } else { + assign(out, key, child); + } + } + return out; +} + +function comparable(value: unknown): unknown { + if (value instanceof Date) return value.toISOString(); + if (Array.isArray(value)) return value.map(comparable); + if (!isPlainRecord(value)) return value; + const out: Record = {}; + for (const [key, child] of Object.entries(value)) { + assign(out, key, comparable(child)); + } + return out; +} + +function instancesById(value: unknown): Map> { + const out = new Map>(); + const add = (instance: unknown): void => { + if (isPlainRecord(instance) && typeof instance.id === "string") { + out.set(instance.id, instance); + } + }; + if (Array.isArray(value)) value.forEach(add); + else add(value); + return out; +} + +/** + * A plain record, as opposed to a value the store round-trips whole: a `Date` + * is an object with no own keys, and walking it as a container rebuilds it as + * an empty object. + */ +function isPlainRecord(value: unknown): value is Record { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return false; + } + const proto = Object.getPrototypeOf(value) as object | null; + return proto === Object.prototype || proto === null; +} + +function hasOwn(record: object, key: string): boolean { + return Object.prototype.hasOwnProperty.call(record, key); +} + +/** Assign a key that may be `__proto__` without invoking the prototype setter. */ +function assign( + out: Record, + key: string, + value: unknown +): void { + Object.defineProperty(out, key, { + value, + enumerable: true, + writable: true, + configurable: true, + }); +} From 47d7d69f4bf0d121fcd362c037d89d83b90fe1ae Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 19:23:56 +0300 Subject: [PATCH 11/16] fix(nextly): a whole-document publish applies every language's pending change The wildcard locale refused with 409 whenever another language held a pending change. It now applies every configured language's change, oldest save first: a shared value is taken only where a change differs from the live row, and each language's translations come from its own change. Each language is judged and validated on the document it will read, and one refusal rolls back the whole write with every pending change kept. A change held for a language the app no longer configures is left in place. --- ...publish-every-language.integration.test.ts | 239 +++++++ ...ldcard-locale-contract.integration.test.ts | 29 +- .../services/collection-mutation-service.ts | 633 ++++++++++++++---- 3 files changed, 769 insertions(+), 132 deletions(-) create mode 100644 packages/nextly/src/domains/collections/__tests__/publish-every-language.integration.test.ts diff --git a/packages/nextly/src/domains/collections/__tests__/publish-every-language.integration.test.ts b/packages/nextly/src/domains/collections/__tests__/publish-every-language.integration.test.ts new file mode 100644 index 0000000000..2f0f798c7c --- /dev/null +++ b/packages/nextly/src/domains/collections/__tests__/publish-every-language.integration.test.ts @@ -0,0 +1,239 @@ +/** + * Moving a whole document's lifecycle applies every language's pending change. + * + * Each case pairs what must land with what must survive, because a write that + * applies nothing satisfies every "unchanged" assertion on its own. + * + * @module domains/collections/__tests__/publish-every-language.integration.test + */ +import { afterEach, describe, expect, it } from "vitest"; + +import { defineCollection, text } from "../../../config"; +import { + createTestNextly, + getConfiguredTestDialects, + type TestDialect, + type TestNextly, +} from "../../../plugins/test-nextly"; +import type { CollectionsHandler } from "../../../services/collections-handler"; + +let current: TestNextly | undefined; +afterEach(async () => { + await current?.destroy(); + current = undefined; +}); + +const SLUG = "pages"; +const GUARDED_SLUG = "guardedpages"; + +const OPEN_ACCESS = { + read: () => true, + update: () => true, + publish: () => true, + unpublish: () => true, +}; + +async function boot(dialect: TestDialect): Promise { + current = await createTestNextly({ + dialect, + collections: [ + defineCollection({ + slug: SLUG, + localized: true, + status: true, + versions: { drafts: true }, + access: OPEN_ACCESS, + fields: [ + text({ name: "title", localized: true }), + text({ name: "note", localized: false }), + ], + }), + defineCollection({ + slug: GUARDED_SLUG, + localized: true, + status: true, + versions: { drafts: true }, + access: OPEN_ACCESS, + fields: [ + text({ name: "title", localized: true }), + text({ + name: "note", + localized: false, + access: { update: () => false }, + }), + ], + }), + ], + localization: { locales: ["en", "de"], defaultLocale: "en" }, + }); + return current; +} + +const handlerOf = (t: TestNextly): CollectionsHandler => + t.getService("collectionsHandler") as CollectionsHandler; + +/** A document published in English and German. */ +async function publishedInBoth(t: TestNextly, slug = SLUG): Promise { + const created = await handlerOf(t).createEntry( + { collectionName: slug, overrideAccess: true, locale: "en" }, + { title: "EN v1", note: "live note", status: "published" } + ); + const id = (created.data as { id?: string } | undefined)?.id; + if (typeof id !== "string") throw new Error("no id from create"); + await handlerOf(t).updateEntry( + { collectionName: slug, entryId: id, overrideAccess: true, locale: "de" }, + { title: "DE v1", status: "published" } + ); + return id; +} + +/** Hold an edit as that language's pending change. */ +async function holdEdit( + t: TestNextly, + id: string, + locale: string, + data: Record, + slug = SLUG +): Promise { + const res = await handlerOf(t).updateEntry( + { collectionName: slug, entryId: id, overrideAccess: true, locale }, + data + ); + if (!res.success) throw new Error(`hold failed: ${JSON.stringify(res)}`); +} + +/** + * Date a language's pending change, so which save is later does not depend on + * two writes landing in different milliseconds, or seconds on MySQL. + */ +async function dateChange( + t: TestNextly, + id: string, + locale: string, + iso: string +): Promise { + await t.adapter.update( + "nextly_versions", + { updatedAt: new Date(iso) }, + { + and: [ + { column: "entryId", op: "=", value: id }, + { column: "locale", op: "=", value: locale }, + { column: "versionNo", op: "IS NULL" }, + ], + } + ); +} + +/** What a reader gets for one language, at every status. */ +async function live( + t: TestNextly, + id: string, + locale: string, + slug = SLUG +): Promise> { + const doc = (await t.nextly.findByID({ + collection: slug as never, + id, + locale, + overrideAccess: true, + status: "all", + } as never)) as Record | null; + return doc ?? {}; +} + +/** The languages this document still holds a pending change for. */ +async function pendingLocales(t: TestNextly, id: string): Promise { + // `nextly_versions` is a mapped system table: its rows come back camelCased. + const rows = await t.adapter.select<{ + entryId?: unknown; + versionNo?: unknown; + locale?: unknown; + }>("nextly_versions", {}); + return rows + .filter(r => String(r.entryId) === id && r.versionNo === null) + .map(r => String(r.locale)) + .sort(); +} + +async function publishEveryLanguage(t: TestNextly, id: string, slug = SLUG) { + return handlerOf(t).updateEntry( + { collectionName: slug, entryId: id, overrideAccess: true, locale: "*" }, + { status: "published" } + ); +} + +describe.each(getConfiguredTestDialects())( + "a whole-document publish and every language's pending change (%s)", + dialect => { + it("publishes every language's pending change and consumes each one", async () => { + const t = await boot(dialect); + const id = await publishedInBoth(t); + await holdEdit(t, id, "en", { title: "EN v2" }); + await holdEdit(t, id, "de", { title: "DE v2" }); + + const res = await publishEveryLanguage(t, id); + + expect(res.success, JSON.stringify(res)).toBe(true); + expect((await live(t, id, "en")).title).toBe("EN v2"); + expect((await live(t, id, "de")).title).toBe("DE v2"); + expect(await pendingLocales(t, id)).toEqual([]); + }); + + it("keeps a shared edit that a later translation-only change never touched", async () => { + const t = await boot(dialect); + const id = await publishedInBoth(t); + await holdEdit(t, id, "en", { note: "EN edited note" }); + await holdEdit(t, id, "de", { title: "DE v2" }); + // German saved later, holding the note as it was live. + await dateChange(t, id, "en", "2026-01-01T00:00:00.000Z"); + await dateChange(t, id, "de", "2026-01-02T00:00:00.000Z"); + + const res = await publishEveryLanguage(t, id); + + expect(res.success, JSON.stringify(res)).toBe(true); + expect((await live(t, id, "de")).title).toBe("DE v2"); + expect((await live(t, id, "en")).note).toBe("EN edited note"); + }); + + it("lets the later save win when two languages edited the same shared value", async () => { + const t = await boot(dialect); + const id = await publishedInBoth(t); + await holdEdit(t, id, "en", { note: "EN note" }); + await holdEdit(t, id, "de", { note: "DE note" }); + await dateChange(t, id, "de", "2026-01-01T00:00:00.000Z"); + await dateChange(t, id, "en", "2026-01-02T00:00:00.000Z"); + + const res = await publishEveryLanguage(t, id); + + expect(res.success, JSON.stringify(res)).toBe(true); + expect((await live(t, id, "de")).note).toBe("EN note"); + }); + + it("refuses the whole write, and keeps every pending change, when one edits a field the publisher may not", async () => { + const t = await boot(dialect); + const id = await publishedInBoth(t, GUARDED_SLUG); + await holdEdit(t, id, "en", { title: "EN v2" }, GUARDED_SLUG); + await holdEdit(t, id, "de", { note: "forbidden" }, GUARDED_SLUG); + await dateChange(t, id, "en", "2026-01-01T00:00:00.000Z"); + await dateChange(t, id, "de", "2026-01-02T00:00:00.000Z"); + + const res = await handlerOf(t).updateEntry( + { + collectionName: GUARDED_SLUG, + entryId: id, + locale: "*", + routeAuthorized: true, + user: { id: "editor-1", isActive: true } as never, + }, + { status: "published" } + ); + + expect(res.success, JSON.stringify(res)).toBe(false); + // English was applied first and must not survive the refusal. + expect((await live(t, id, "en", GUARDED_SLUG)).title).toBe("EN v1"); + expect((await live(t, id, "en", GUARDED_SLUG)).note).toBe("live note"); + expect(await pendingLocales(t, id)).toEqual(["de", "en"]); + }); + } +); diff --git a/packages/nextly/src/domains/collections/__tests__/wildcard-locale-contract.integration.test.ts b/packages/nextly/src/domains/collections/__tests__/wildcard-locale-contract.integration.test.ts index 4a32941e2f..f7768c199f 100644 --- a/packages/nextly/src/domains/collections/__tests__/wildcard-locale-contract.integration.test.ts +++ b/packages/nextly/src/domains/collections/__tests__/wildcard-locale-contract.integration.test.ts @@ -460,7 +460,7 @@ describe.each(getConfiguredTestDialects())( expect(afterRow?.first_published_at ?? null).toBeNull(); }); - it("REFUSES rather than decide the fate of unreleased work (collection)", async () => { + it("publishes the unreleased work of every language (collection)", async () => { const t = await boot(dialect); const created = await handlerOf(t).createEntry( { collectionName: DRAFTS_SLUG, overrideAccess: true }, @@ -497,7 +497,7 @@ describe.each(getConfiguredTestDialects())( }; expect(await liveTitle("de")).toBe("DE v1"); - const refused = await handlerOf(t).updateEntry( + const published = await handlerOf(t).updateEntry( { collectionName: DRAFTS_SLUG, entryId: id, @@ -507,10 +507,8 @@ describe.each(getConfiguredTestDialects())( { status: "published" } ); - expect(refused.success).toBe(false); - expect(refused.statusCode).toBe(409); - expect(refused.message).toContain("de"); - expect(await liveTitle("de")).toBe("DE v1"); + expect(published.success, JSON.stringify(published)).toBe(true); + expect(await liveTitle("de")).toBe("DE v2"); }); it("REFUSES the wildcard on a Single with no lifecycle to move", async () => { @@ -721,10 +719,23 @@ describe.each(getConfiguredTestDialects())( { status: "draft" } ); - // It runs. A configured language holding work still blocks — that case is - // covered by the refusal test above, which is what keeps this from - // passing on a guard that stopped blocking entirely. + // It runs, and the work held for the removed language is neither + // published nor deleted: nothing can read or write that language, and + // the row is the only record that the work existed. expect(result.success).toBe(true); + const held = await t.adapter.select<{ + entryId?: unknown; + locale?: unknown; + versionNo?: unknown; + }>("nextly_versions", {}); + expect( + held.some( + r => + String(r.entryId) === id && + r.locale === "fr" && + r.versionNo === null + ) + ).toBe(true); }); it("refuses a lifecycle-less wildcard BEFORE any hook runs", async () => { diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 101f0d3ccc..f450f4ab7a 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -120,6 +120,7 @@ import { extractFieldGroupReferences, } from "../../field-groups/storage/field-group-field-type"; import { readFieldGroupType } from "../../field-groups/storage/field-group-type-key"; +import { resolveLocalizedFieldNames } from "../../i18n/classify-fields"; import { COMPANION_LOCALE_COLUMN, COMPANION_PARENT_COLUMN, @@ -207,6 +208,13 @@ import { getTableName, generateSlug, } from "./collection-utils"; +import { + changedKeys, + languageTarget, + pendingChangesToApply, + type ComponentValueShape, + type PendingLanguageChange, +} from "./pending-change-merge"; /** The Drizzle executor shape the companion-join readers accept (a transaction * handle's `getDrizzle()` result, or the pooled `this.db`). */ @@ -531,6 +539,127 @@ interface CreateEntryWriteOptions { failureMessage: string; } +/** A write's values split between the main row and one language's companion row. */ +interface LocalizedWriteSplit { + companionTableName: string; + writeLocale: string; + companionData: Record; + /** + * The same written localized values, keyed by FIELD NAME (companionData is + * keyed by snake_case column). A version snapshot merges these onto the + * parent so the read-shape snapshot carries this locale's translatable + * values instead of dropping them. + */ + localizedFieldValues: Record; + /** + * Whether the companion carries a per-locale `_status` column. Reading that + * column on a collection without it fails the whole write, so every read of + * it must be gated on this. + */ + hasStatus: boolean; +} + +/** What validating one language's document needs to know about translations. */ +interface LocaleValidationContext { + localizedFieldNames: ReadonlySet; + enforceLocalizedRequired: boolean; +} + +/** What applying every language's pending change needs from the write in progress. */ +interface EveryLanguagePromotion { + collectionName: string; + entryId: string; + user?: UserContext; + authenticatedScope?: AuthenticatedScope; + overrideAccess?: boolean; + collection: unknown; + /** The collection's table, as `loadDynamicSchema` returns it. */ + schema: Awaited>; + tableName: string; + fields: FieldDefinition[]; + manyToManyFields: FieldDefinition[]; + componentSchemas: ComponentSchemas | null; + restoreCtx: RestoreSchemaContext; + grants: Parameters[0]["grants"]; + /** Validation inputs per language, resolved before the transaction opened. */ + localeContexts: ReadonlyMap; +} + +/** + * Whether a promoted translation is written to its language's row. + * + * An existing row takes the authored write unconditionally, clears included: + * refusing one leaves the old translation live while the promotion consumes the + * change that replaced it. An absent row is created only by content that is + * genuinely translated, since a pending change holds every localized field and + * an untranslated language would otherwise gain a row with no content. + */ +function shouldWritePromotedTranslation(args: { + rowExists: boolean; + localizedFieldValues: Record; +}): boolean { + return ( + args.rowExists || + Object.values(args.localizedFieldValues).some(value => !isBlank(value)) + ); +} + +const NO_TRANSLATABLE_KEYS: ReadonlySet = new Set(); + +/** The fields of a list that carry a name, the ones a document can hold. */ +function namedFields( + fields: readonly FieldConfig[] +): Array { + return fields.filter( + (field): field is FieldConfig & { name: string } => + typeof (field as { name?: unknown }).name === "string" + ); +} + +/** + * How a component field's instances hold translations, read from the schemas + * of the components it references. + * + * A dynamic zone names each instance's component on the instance; a field that + * references one component needs no marker. A component whose schema did not + * resolve is treated as holding no translations, so its value is compared whole + * rather than split on a guess. + */ +function componentValueShape( + field: unknown, + schemas: ComponentSchemas | null +): ComponentValueShape { + const slugs = fieldGroupSlugList(field); + const schemaOf = (instance: Record) => { + const named = readFieldGroupType(instance); + const slug = + named !== undefined && slugs.includes(named) + ? named + : slugs.length === 1 + ? slugs[0] + : undefined; + const schema = slug === undefined ? undefined : schemas?.get(slug); + return schema?.resolved ? schema : undefined; + }; + return { + translatableKeys: instance => { + const schema = schemaOf(instance); + return schema?.localized + ? new Set(resolveLocalizedFieldNames(namedFields(schema.fields), true)) + : NO_TRANSLATABLE_KEYS; + }, + nested: (instance, key) => { + const schema = schemaOf(instance); + const child = schema + ? namedFields(schema.fields).find( + candidate => candidate.name === key && isFieldGroupField(candidate) + ) + : undefined; + return child ? componentValueShape(child, schemas) : undefined; + }, + }; +} + interface WorkingDraftWriteContext { collection: unknown; collectionHasStatus: boolean; @@ -1381,20 +1510,7 @@ export class CollectionMutationService extends BaseService { * pool. */ executor?: unknown - ): Promise<{ - companionTableName: string; - writeLocale: string; - companionData: Record; - // The same written localized values, keyed by FIELD NAME (companionData is - // keyed by snake_case column). A version snapshot merges these onto the - // parent so the read-shape snapshot carries this locale's translatable - // values instead of dropping them. - localizedFieldValues: Record; - // Whether the companion carries a per-locale `_status` column. Reading that - // column on a collection without it fails the whole write, so every read of - // it must be gated on this. - hasStatus: boolean; - } | null> { + ): Promise { if (!this.localization) return null; const companion = await this.fileManager.loadCompanionSchema( collectionName, @@ -5490,6 +5606,336 @@ export class CollectionMutationService extends BaseService { return { workingDraftDocument, priorWorkingDraftDocument }; } + /** Replace the junction rows of each many-to-many field this write names. */ + private async replaceManyToManyInTx( + tx: TransactionContext, + collectionName: string, + entryId: string, + manyToManyFields: FieldDefinition[], + data: Record + ): Promise { + const executor = tx.getDrizzle(); + for (const field of manyToManyFields) { + const relatedIds = data[field.name]; + if (relatedIds === undefined) continue; + await this.relationshipService.deleteManyToManyRelations( + collectionName, + entryId, + field, + executor + ); + if (relatedIds.length > 0) { + await this.relationshipService.insertManyToManyRelations( + collectionName, + entryId, + field, + relatedIds, + executor + ); + } + } + } + + /** How each component field's instances hold translations, by field name. */ + private componentValueShapes( + fields: FieldDefinition[], + schemas: ComponentSchemas | null + ): Map { + const shapes = new Map(); + for (const field of fields) { + if (!isFieldGroupField(field)) continue; + shapes.set(field.name, componentValueShape(field, schemas)); + } + return shapes; + } + + /** + * Apply every configured language's pending change, for a write that moves + * the whole document's lifecycle. + * + * Every language's live document is read before anything is written, because + * that is what each pending change is measured against. Each language is then + * judged on the document it will read once the write lands, validated, and + * written, oldest save first; a refusal rolls the transaction back with every + * pending change kept. + */ + private async promoteEveryLanguageInTx( + tx: TransactionContext, + ctx: EveryLanguagePromotion + ): Promise { + const repo = new VersionsRepository(tx); + const ref = { + scopeKind: "collection" as const, + scopeSlug: ctx.collectionName, + entryId: ctx.entryId, + }; + const changes = pendingChangesToApply( + await repo.findAllWorkingDrafts(ref), + new Set(this.localization?.locales.map(locale => locale.code) ?? []) + ); + const liveByLocale = new Map>(); + for (const change of changes) { + liveByLocale.set( + change.locale, + await this.restoreShapedInTx(tx, ctx, change.locale) + ); + } + for (const change of changes) { + await this.applyLanguageChangeInTx( + tx, + ctx, + change, + liveByLocale.get(change.locale) ?? {} + ); + await repo.deleteWorkingDraft(ref, change.locale); + } + return changes.map(change => change.locale); + } + + /** One language's document, in the shape a pending change is stored in. */ + private async restoreShapedInTx( + tx: TransactionContext, + ctx: EveryLanguagePromotion, + locale: string + ): Promise> { + const [row] = (await tx + .getDrizzle() + .select() + .from(ctx.schema) + .where(eq(ctx.schema.id, ctx.entryId)) + .limit(1)) as (Record | undefined)[]; + const translations = await this.readCompanionLocalizedValues( + tx, + ctx.collectionName, + ctx.entryId, + locale + ); + const parentRow = this.deserializeJsonFieldsForSnapshot( + { ...convertTimestampsToCamelCase({ ...(row ?? {}) }), ...translations }, + ctx.fields + ); + stripPasswordFieldValues(parentRow, ctx.fields); + stripSystemOwnerField(parentRow); + const { components, manyToMany } = await this.buildFullSnapshotRelations( + tx, + ctx.entryId, + ctx.collectionName, + ctx.tableName, + ctx.fields, + ctx.manyToManyFields, + locale + ); + const document = assembleDocument( + await this.snapshotPartsFor( + { parentRow, components, manyToMany }, + ctx.fields, + tx + ) + ); + return buildRestorePayload( + document, + ctx.fields as unknown as FieldConfig[], + ctx.restoreCtx + ).payload; + } + + /** Build, judge and write one language's pending change. */ + private async applyLanguageChangeInTx( + tx: TransactionContext, + ctx: EveryLanguagePromotion, + change: PendingLanguageChange, + live: Record + ): Promise { + const pending = buildRestorePayload( + change.snapshot, + ctx.fields as unknown as FieldConfig[], + ctx.restoreCtx + ).payload; + const current = await this.restoreShapedInTx(tx, ctx, change.locale); + const target = languageTarget({ + pending, + live, + current, + localizedFieldNames: + ctx.localeContexts.get(change.locale)?.localizedFieldNames ?? + NO_TRANSLATABLE_KEYS, + componentFields: this.componentValueShapes( + ctx.fields, + ctx.componentSchemas + ), + }); + // The statuses this write sets are the ones that count. + const liveContent = { ...live }; + for (const document of [target, current, liveContent]) { + delete document.status; + } + const judged = await this.judgeLanguageChange( + ctx, + change.locale, + target, + liveContent + ); + const changed = changedKeys(judged, current); + if (changed.size === 0) return; + await this.writeLanguageChangeInTx( + tx, + ctx, + change.locale, + Object.fromEntries( + Object.entries(judged).filter(([key]) => changed.has(key)) + ) + ); + } + + /** The document one language may write, or a refusal. */ + private async judgeLanguageChange( + ctx: EveryLanguagePromotion, + locale: string, + target: Record, + live: Record + ): Promise> { + const logical = (document: Record) => + this.deserializeJsonFieldsForSnapshot( + this.assemblePromotedDocument( + document, + {}, + {}, + {}, + ctx.fields, + ctx.manyToManyFields, + ctx.componentSchemas + ), + ctx.fields + ); + const judged = await resolvePromotedDocument({ + before: logical(target), + live: logical(live), + applyRules: document => + applyFieldWriteAccess({ + kind: "collection", + slug: ctx.collectionName, + data: document, + operation: "update", + user: ctx.user, + authenticatedScope: ctx.authenticatedScope, + overrideAccess: ctx.overrideAccess, + grants: ctx.grants, + id: ctx.entryId, + }), + authoredFieldNames: declaredFieldNames(ctx.fields), + slug: ctx.collectionName, + locale, + }); + const issues = await validateEntryData( + this.validationView(judged, ctx.fields), + attachFieldValidators("collection", ctx.collectionName, ctx.fields), + { + mode: "update", + req: ctx.user ? { user: ctx.user } : {}, + ...(ctx.localeContexts.get(locale) ?? {}), + } + ); + if (issues.length > 0) { + throw NextlyError.validation({ errors: issues }); + } + return judged; + } + + /** Write one language's changed values: shared columns, its translation, components, relations. */ + private async writeLanguageChangeInTx( + tx: TransactionContext, + ctx: EveryLanguagePromotion, + locale: string, + document: Record + ): Promise { + const parts = this.shapeWriteParts( + document, + ctx.fields, + ctx.manyToManyFields, + ctx.collection + ); + // Split BEFORE the main columns are taken: the split moves translated + // values out of the document, and the main table has no column for them. + const translation = await this.splitLocalizedWriteData( + ctx.collectionName, + document, + locale, + false, + tx.getDrizzle() + ); + const columns: Record = {}; + for (const [key, value] of Object.entries( + stripImmutableSystemFields(document, "collection") + )) { + columns[toSnakeCase(key)] = value; + } + if (Object.keys(columns).length > 0) { + await tx.update(ctx.tableName, columns, this.whereEq("id", ctx.entryId)); + } + await this.writePromotedTranslationInTx(tx, ctx, locale, translation); + if ( + this.fieldGroupDataService && + Object.keys(parts.componentFieldData).length > 0 + ) { + await this.fieldGroupDataService.saveComponentDataInTransaction(tx, { + parentId: ctx.entryId, + parentTable: ctx.tableName, + fields: ctx.fields as unknown as FieldConfig[], + // Cloned because the save mutates what it is given. + data: structuredClone(parts.componentFieldData), + locale, + req: ctx.user ? { user: ctx.user } : {}, + }); + } + await this.replaceManyToManyInTx( + tx, + ctx.collectionName, + ctx.entryId, + ctx.manyToManyFields, + parts.manyToManyData + ); + } + + /** One language's translated values, written to its row when they belong there. */ + private async writePromotedTranslationInTx( + tx: TransactionContext, + ctx: EveryLanguagePromotion, + locale: string, + translation: LocalizedWriteSplit | null + ): Promise { + if (!translation || Object.keys(translation.companionData).length === 0) { + return; + } + const companion = await this.fileManager.loadCompanionSchema( + ctx.collectionName, + tx.getDrizzle() + ); + const rowExists = companion + ? await companionRowExists( + tx.getDrizzle[0]>(), + companion.table, + ctx.entryId, + locale, + cachedCompanionReadiness(this.adapter, companion.companionTableName) + ) + : false; + if ( + !shouldWritePromotedTranslation({ + rowExists, + localizedFieldValues: translation.localizedFieldValues, + }) + ) { + return; + } + await upsertCompanionRow( + companionWriteVia(tx, this.dialect), + translation.companionTableName, + ctx.entryId, + locale, + translation.companionData + ); + } + async updateEntry( // Named `rawParams` because the body must not read it: the wildcard locale // is resolved away into `params` at the top of the method. See there. @@ -6190,6 +6636,22 @@ export class CollectionMutationService extends BaseService { const isRestoreWrite = params.sourceVersionNo !== undefined && params.sourceVersionNo !== null; const promotePossible = splitEnabled && !namesNoStatus && !isRestoreWrite; + // A write that moves the whole document's lifecycle applies every + // language's pending change, not only the one its default locale keys. + const promoteEveryLanguage = + sweepAllLocales && promotePossible && documentLocalized; + // Resolved here, before the transaction opens: it reads the companion + // schema on the pool, which a transaction holding its own connection + // must not wait on. + const everyLanguageContexts = new Map(); + if (promoteEveryLanguage) { + for (const { code } of this.localization?.locales ?? []) { + everyLanguageContexts.set( + code, + await this.localizedRequiredContext(params.collectionName, code) + ); + } + } // The caller's grants, resolved HERE, before the write transaction opens. // // The promotion gate runs under the row lock, on the draft that @@ -6250,7 +6712,7 @@ export class CollectionMutationService extends BaseService { // pool — and validate the SAME merged shape the in-transaction fold // persists (`buildRestorePayload` output with the caller's fields on top). // The fold below stays authoritative for the write; this only gates it. - if (promotePossible && promoteRestoreCtx) { + if (promotePossible && promoteRestoreCtx && !promoteEveryLanguage) { const advisoryDraft = await new VersionsRepository( this.adapter ).findWorkingDraft( @@ -6403,77 +6865,6 @@ export class CollectionMutationService extends BaseService { // does not exist. await tx.lockRow(tableName, params.entryId); - // A wildcard must not decide the fate of work somebody saved and has - // not released yet — asked UNDER THE LOCK, because the answer changes. - // - // Another language may be holding a pending edit, and moving the whole - // document's lifecycle would publish that language while its edit - // stayed unreleased — marking a translation live against values its - // author had already replaced. Releasing those edits here instead is - // not the smaller problem it looks: a pending edit stores the whole - // document as it looked when it was saved, not the fields its author - // touched, so two languages' edits cannot be merged without inventing - // a rule for which shared value wins — and every such rule silently - // discards somebody's work in some ordering. - // - // Asked here rather than before the transaction because a draft save - // serialises on this same parent lock: a check that ran earlier can be - // overtaken by a save that commits first, and the release would then - // proceed over work it never saw. The refusal travels out on the - // same out-of-band result the transition gate uses, since the adapter - // re-wraps a thrown sentinel before the catch can identify it. - const configuredLocalesForHold = new Set( - this.localization?.locales.map(l => l.code) ?? [] - ); - if ( - sweepAllLocales && - (collection as { versions?: { drafts?: { enabled?: boolean } } }) - .versions?.drafts?.enabled === true - ) { - const heldBy = ( - await new VersionsRepository(tx).findAllWorkingDrafts({ - scopeKind: "collection", - scopeSlug: params.collectionName, - entryId: params.entryId, - }) - ) - .map(draft => draft.locale) - .filter( - (locale): locale is string => - locale !== null && locale !== draftLocaleKey - ) - // Only a language the app still configures can block this write. - // - // A draft left behind by a language that was REMOVED from the - // configuration would otherwise refuse every wildcard publish and - // takedown forever, and the remedy this refusal recommends — - // publish or discard it — cannot be carried out, because reads and - // writes reject that locale. A scheduled takedown would then leave - // the document live indefinitely with no route out through the - // API: a refusal that cannot be satisfied is worse than the - // ambiguity it was added to avoid. - // - // The stale draft is left where it is rather than cleaned up here. - // A write asked to move a lifecycle has no business deleting - // somebody's stored work as a side effect, and the row is the only - // record that the work existed. - .filter(locale => configuredLocalesForHold.has(locale)); - if (heldBy.length > 0) { - transitionDeniedResult = { - success: false, - statusCode: 409, - message: - `This document has unpublished changes in ${heldBy.join(", ")}. ` + - `Publish or discard them first, or publish each language on ` + - `its own — locale '${EVERY_LOCALE}' moves every language's ` + - `status and will not decide what happens to work that has ` + - `not been released.`, - data: null, - }; - throw new StatusTransitionDeniedError(); - } - } - // Read the committed state before this attempt's UPDATE. Nothing read // after the write can serve as prior state: the UPDATE below, the // companion upsert, and the many-to-many rewrite have all run by then. @@ -6727,7 +7118,26 @@ export class CollectionMutationService extends BaseService { // draft is deleted in the same transaction below, so promote is atomic: // any failure rolls back the live write and leaves the draft intact. let promotedDraft = false; - if (promotePossible && promoteRestoreCtx) { + if (promoteEveryLanguage && promoteRestoreCtx) { + // Every configured language's pending change is applied here, and + // the write below moves only the lifecycle. + await this.promoteEveryLanguageInTx(tx, { + collectionName: params.collectionName, + entryId: params.entryId, + user: params.user, + authenticatedScope: params.authenticatedScope, + overrideAccess: params.overrideAccess, + collection, + schema, + tableName, + fields, + manyToManyFields, + componentSchemas: splitComponentSchemas, + restoreCtx: promoteRestoreCtx, + grants: promoteGrants, + localeContexts: everyLanguageContexts, + }); + } else if (promotePossible && promoteRestoreCtx) { const workingDraft = await new VersionsRepository( tx // The split is non-localized only, so the working draft is keyed @@ -7061,17 +7471,10 @@ export class CollectionMutationService extends BaseService { // for a language nobody wrote. (!sweepAllLocales || (promotedDraft && - // An EXISTING row takes its authored write unconditionally, - // clears included: refusing one leaves the old translation live - // while the promotion deletes the draft that asked for it. - // An ABSENT row is only brought into being by content that is - // genuinely translated — `isBlank` is the shared definition of - // that, and an empty string means "not translated" under it, so - // clearing a translation that never existed creates nothing. - (promotedLocaleRowExists || - Object.values(localizedUpdate.localizedFieldValues).some( - v => !isBlank(v) - )))) && + shouldWritePromotedTranslation({ + rowExists: promotedLocaleRowExists, + localizedFieldValues: localizedUpdate.localizedFieldValues, + }))) && Object.keys(localizedUpdate.companionData).length > 0 ) { await upsertCompanionRow( @@ -7150,31 +7553,15 @@ export class CollectionMutationService extends BaseService { // Replace many-to-many junction rows inside the transaction so a // junction failure rolls back the update (atomic write). The entry is - // already known to exist (validated before the transaction). The - // tx-scoped Drizzle handle binds the junction writes to this tx. - const txExecutor = tx.getDrizzle(); - for (const field of manyToManyFields) { - if ( - !storeAsWorkingDraft && - manyToManyData[field.name] !== undefined - ) { - await this.relationshipService.deleteManyToManyRelations( - params.collectionName, - params.entryId, - field, - txExecutor - ); - const relatedIds = manyToManyData[field.name]; - if (relatedIds.length > 0) { - await this.relationshipService.insertManyToManyRelations( - params.collectionName, - params.entryId, - field, - relatedIds, - txExecutor - ); - } - } + // already known to exist (validated before the transaction). + if (!storeAsWorkingDraft) { + await this.replaceManyToManyInTx( + tx, + params.collectionName, + params.entryId, + manyToManyFields, + manyToManyData + ); } // Capture a version snapshot of the post-update document atomically From 887e82a9936155aee3365ee6e3fd91aa0d0a039a Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 19:29:20 +0300 Subject: [PATCH 12/16] fix(nextly): a language's publish writes only the component translations it holds Promoting every language saved each language's components with that language's values. A block another language added in the same write came back with no translation for it, and saving that blank stored an empty translation. The write now carries translations only for instances the language's pending change holds; an added instance keeps what its author gave it. Tested with the translation saved before and after the new block. --- ...publish-every-language.integration.test.ts | 150 +++++++++++++++++- .../__tests__/pending-change-merge.test.ts | 26 +++ .../services/collection-mutation-service.ts | 31 ++-- .../services/pending-change-merge.ts | 51 ++++++ 4 files changed, 246 insertions(+), 12 deletions(-) diff --git a/packages/nextly/src/domains/collections/__tests__/publish-every-language.integration.test.ts b/packages/nextly/src/domains/collections/__tests__/publish-every-language.integration.test.ts index 2f0f798c7c..ab856ad7b1 100644 --- a/packages/nextly/src/domains/collections/__tests__/publish-every-language.integration.test.ts +++ b/packages/nextly/src/domains/collections/__tests__/publish-every-language.integration.test.ts @@ -8,7 +8,12 @@ */ import { afterEach, describe, expect, it } from "vitest"; -import { defineCollection, text } from "../../../config"; +import { + defineCollection, + defineFieldGroup, + fieldGroup, + text, +} from "../../../config"; import { createTestNextly, getConfiguredTestDialects, @@ -25,6 +30,7 @@ afterEach(async () => { const SLUG = "pages"; const GUARDED_SLUG = "guardedpages"; +const BLOCKS_SLUG = "blockpages"; const OPEN_ACCESS = { read: () => true, @@ -36,7 +42,28 @@ const OPEN_ACCESS = { async function boot(dialect: TestDialect): Promise { current = await createTestNextly({ dialect, + fieldGroups: [ + defineFieldGroup({ + slug: "hero", + localized: true, + fields: [ + text({ name: "heading", localized: true }), + text({ name: "variant", localized: false }), + ], + }), + ], collections: [ + defineCollection({ + slug: BLOCKS_SLUG, + localized: true, + status: true, + versions: { drafts: true }, + access: OPEN_ACCESS, + fields: [ + text({ name: "title", localized: true }), + fieldGroup({ name: "blocks", component: "hero", repeatable: true }), + ], + }), defineCollection({ slug: SLUG, localized: true, @@ -156,6 +183,103 @@ async function pendingLocales(t: TestNextly, id: string): Promise { .sort(); } +type Block = { id: string; heading?: unknown; variant?: unknown }; + +/** A document with one block, published and translated in both languages. */ +async function publishedWithOneBlock( + t: TestNextly +): Promise<{ id: string; firstBlockId: string }> { + const created = await handlerOf(t).createEntry( + { collectionName: BLOCKS_SLUG, overrideAccess: true, locale: "en" }, + { + title: "EN", + blocks: [{ heading: "EN one", variant: "wide" }], + status: "published", + } + ); + const id = (created.data as { id?: string } | undefined)?.id; + if (typeof id !== "string") throw new Error("no id from create"); + const blocks = (await live(t, id, "en", BLOCKS_SLUG)).blocks as Block[]; + const firstBlockId = blocks[0].id; + await handlerOf(t).updateEntry( + { + collectionName: BLOCKS_SLUG, + entryId: id, + overrideAccess: true, + locale: "de", + }, + { + title: "DE", + blocks: [{ id: firstBlockId, heading: "DE eins", variant: "wide" }], + status: "published", + } + ); + return { id, firstBlockId }; +} + +/** Whether a block holds a stored translation for one language. */ +async function hasTranslation( + t: TestNextly, + blockId: string, + locale: string +): Promise { + const rows = await t.adapter.select>( + "comp_hero_locales", + { + where: { + and: [ + { column: "_parent", op: "=", value: blockId }, + { column: "_locale", op: "=", value: locale }, + ], + }, + } + ); + return rows.length > 0; +} + +/** + * English adds a block while German translates the existing one; `germanLater` + * decides which pending change was saved last. + */ +async function blockAddedBesideATranslation( + t: TestNextly, + germanLater: boolean +): Promise { + const { id, firstBlockId } = await publishedWithOneBlock(t); + await holdEdit( + t, + id, + "de", + { blocks: [{ id: firstBlockId, heading: "DE eins v2", variant: "wide" }] }, + BLOCKS_SLUG + ); + await holdEdit( + t, + id, + "en", + { + blocks: [ + { id: firstBlockId, heading: "EN one", variant: "wide" }, + { heading: "EN two", variant: "narrow" }, + ], + }, + BLOCKS_SLUG + ); + await dateChange( + t, + id, + "de", + germanLater ? "2026-01-02T00:00:00.000Z" : "2026-01-01T00:00:00.000Z" + ); + await dateChange( + t, + id, + "en", + germanLater ? "2026-01-01T00:00:00.000Z" : "2026-01-02T00:00:00.000Z" + ); + return id; +} + async function publishEveryLanguage(t: TestNextly, id: string, slug = SLUG) { return handlerOf(t).updateEntry( { collectionName: slug, entryId: id, overrideAccess: true, locale: "*" }, @@ -210,6 +334,30 @@ describe.each(getConfiguredTestDialects())( expect((await live(t, id, "de")).note).toBe("EN note"); }); + it.each([ + ["saved before", false], + ["saved after", true], + ])( + "keeps a block English added when the German translation was %s it", + async (_label, germanLater) => { + const t = await boot(dialect); + const id = await blockAddedBesideATranslation(t, germanLater); + + const res = await publishEveryLanguage(t, id, BLOCKS_SLUG); + + expect(res.success, JSON.stringify(res)).toBe(true); + const en = (await live(t, id, "en", BLOCKS_SLUG)).blocks as Block[]; + const de = (await live(t, id, "de", BLOCKS_SLUG)).blocks as Block[]; + expect(en.map(block => block.heading)).toEqual(["EN one", "EN two"]); + expect(de.map(block => block.variant)).toEqual(["wide", "narrow"]); + expect(de[0].heading).toBe("DE eins v2"); + // German never translated the new block, so no German value is stored + // for it: a read falling back to English is not a translation. + expect(await hasTranslation(t, en[1].id, "de")).toBe(false); + expect(await pendingLocales(t, id)).toEqual([]); + } + ); + it("refuses the whole write, and keeps every pending change, when one edits a field the publisher may not", async () => { const t = await boot(dialect); const id = await publishedInBoth(t, GUARDED_SLUG); diff --git a/packages/nextly/src/domains/collections/services/__tests__/pending-change-merge.test.ts b/packages/nextly/src/domains/collections/services/__tests__/pending-change-merge.test.ts index 379e1efa24..b4b96d9a53 100644 --- a/packages/nextly/src/domains/collections/services/__tests__/pending-change-merge.test.ts +++ b/packages/nextly/src/domains/collections/services/__tests__/pending-change-merge.test.ts @@ -10,6 +10,7 @@ import { languageTarget, pendingChangesToApply, sameContent, + translationsHeldBy, withTranslationsFrom, withoutTranslations, type ComponentValueShape, @@ -103,6 +104,31 @@ describe("withoutTranslations and withTranslationsFrom", () => { }); }); +describe("translationsHeldBy", () => { + it("leaves out the translations of an instance the pending change does not hold", () => { + expect( + translationsHeldBy({ + value: [ + { id: "r1", heading: "DE", variant: "wide" }, + { id: "r2", heading: null, variant: "narrow" }, + ], + pending: [{ id: "r1", heading: "DE", variant: "wide" }], + shape: FLAT, + }) + ).toEqual([ + { id: "r1", heading: "DE", variant: "wide" }, + { id: "r2", variant: "narrow" }, + ]); + }); + + it("keeps an instance the change added, which has no id yet", () => { + const added = { heading: "EN two", variant: "narrow" }; + expect( + translationsHeldBy({ value: [added], pending: [added], shape: FLAT }) + ).toEqual([added]); + }); +}); + describe("languageTarget", () => { const base = { localizedFieldNames: new Set(["title"]), diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index f450f4ab7a..24aa8e6b82 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -212,6 +212,7 @@ import { changedKeys, languageTarget, pendingChangesToApply, + translationsHeldBy, type ComponentValueShape, type PendingLanguageChange, } from "./pending-change-merge"; @@ -5752,6 +5753,7 @@ export class CollectionMutationService extends BaseService { ctx.restoreCtx ).payload; const current = await this.restoreShapedInTx(tx, ctx, change.locale); + const shapes = this.componentValueShapes(ctx.fields, ctx.componentSchemas); const target = languageTarget({ pending, live, @@ -5759,10 +5761,7 @@ export class CollectionMutationService extends BaseService { localizedFieldNames: ctx.localeContexts.get(change.locale)?.localizedFieldNames ?? NO_TRANSLATABLE_KEYS, - componentFields: this.componentValueShapes( - ctx.fields, - ctx.componentSchemas - ), + componentFields: shapes, }); // The statuses this write sets are the ones that count. const liveContent = { ...live }; @@ -5777,14 +5776,24 @@ export class CollectionMutationService extends BaseService { ); const changed = changedKeys(judged, current); if (changed.size === 0) return; - await this.writeLanguageChangeInTx( - tx, - ctx, - change.locale, - Object.fromEntries( - Object.entries(judged).filter(([key]) => changed.has(key)) - ) + // Judged whole, written narrow: a component carries only the translations + // this language's pending change holds. + const writable = Object.fromEntries( + [...changed].map(key => { + const shape = shapes.get(key); + return [ + key, + shape + ? translationsHeldBy({ + value: judged[key], + pending: pending[key], + shape, + }) + : judged[key], + ]; + }) ); + await this.writeLanguageChangeInTx(tx, ctx, change.locale, writable); } /** The document one language may write, or a refusal. */ diff --git a/packages/nextly/src/domains/collections/services/pending-change-merge.ts b/packages/nextly/src/domains/collections/services/pending-change-merge.ts index 9dc9a0557d..36d8fe9f12 100644 --- a/packages/nextly/src/domains/collections/services/pending-change-merge.ts +++ b/packages/nextly/src/domains/collections/services/pending-change-merge.ts @@ -152,6 +152,34 @@ export function withTranslationsFrom(args: { : overlay(args.current); } +/** + * A component value carrying translations only for the instances a language's + * pending change holds. + * + * An instance the change does not hold keeps whatever translations it has in + * storage, so its translatable keys are left out of the write: the value a + * read returned for it is not something this language authored, and writing it + * back would store a blank translation nobody wrote. An instance with no id is + * one the change itself added. + */ +export function translationsHeldBy(args: { + value: unknown; + pending: unknown; + shape: ComponentValueShape; +}): unknown { + const held = instancesById(args.pending); + const keep = (instance: unknown): unknown => { + if (!isPlainRecord(instance) || typeof instance.id !== "string") { + return instance; + } + const source = held.get(instance.id); + return source + ? keepHeldNested(instance, source, args.shape) + : withoutTranslations(instance, args.shape); + }; + return Array.isArray(args.value) ? args.value.map(keep) : keep(args.value); +} + /** The document one language's promotion writes. */ export function languageTarget( input: LanguageTargetInput @@ -229,6 +257,29 @@ function overlayInstance( return out; } +function keepHeldNested( + instance: Record, + source: Record, + shape: ComponentValueShape +): Record { + const out: Record = {}; + for (const [key, child] of Object.entries(instance)) { + const nested = shape.nested(instance, key); + assign( + out, + key, + nested + ? translationsHeldBy({ + value: child, + pending: source[key], + shape: nested, + }) + : child + ); + } + return out; +} + function comparable(value: unknown): unknown { if (value instanceof Date) return value.toISOString(); if (Array.isArray(value)) return value.map(comparable); From df8bfae1fd8effebec581e45ce3559ddf6c15917 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 19:45:41 +0300 Subject: [PATCH 13/16] fix(nextly): publish all languages runs through the one update path, hooks included publishAllLocales and unpublishAllLocales keep their pre-checks and move every language with a status patch under the wildcard locale, so hooks, field rules, validation and each language's pending change come from updateEntry. The request reaches the hooks from the route. Two gaps in the wildcard path surfaced and are fixed there, which also serves scheduled releases: the revalidation intent carries every language's slug, and an in-process status event is not repeated untagged when the write locale's companion records the same transition, the rule the durable events already follow. --- .../handlers/collection-dispatcher.ts | 4 +- ...publish-every-language.integration.test.ts | 60 ++++++++ .../services/all-locales-lifecycle.ts | 4 + .../services/collection-mutation-service.ts | 132 +++++++++++++++++- .../src/services/collections-handler.ts | 4 + .../collections/collection-entry-service.ts | 4 + 6 files changed, 200 insertions(+), 8 deletions(-) diff --git a/packages/nextly/src/dispatcher/handlers/collection-dispatcher.ts b/packages/nextly/src/dispatcher/handlers/collection-dispatcher.ts index 520531ccf5..93a91e416c 100644 --- a/packages/nextly/src/dispatcher/handlers/collection-dispatcher.ts +++ b/packages/nextly/src/dispatcher/handlers/collection-dispatcher.ts @@ -191,7 +191,7 @@ function allLocalesLifecycle( successMessage: string ): MethodHandler { return { - execute: async (svc, p) => { + execute: async (svc, p, _body, request) => { // `requireParam` rather than an inline throw: a missing route parameter is // caller-fixable, and a bare `Error` here would surface it as a 500. const collectionName = requireParam(p, "collectionName"); @@ -212,6 +212,8 @@ function allLocalesLifecycle( userRoles: readAuthenticatedRoles(p), routeAuthorized: true, authenticatedScope: readAuthenticatedScope(p), + // The request this operation's hooks are told about. + request, }); const entry = unwrapServiceResult(result, { collectionName, entryId }); return respondMutation(result.message ?? successMessage, entry); diff --git a/packages/nextly/src/domains/collections/__tests__/publish-every-language.integration.test.ts b/packages/nextly/src/domains/collections/__tests__/publish-every-language.integration.test.ts index ab856ad7b1..ff023efa39 100644 --- a/packages/nextly/src/domains/collections/__tests__/publish-every-language.integration.test.ts +++ b/packages/nextly/src/domains/collections/__tests__/publish-every-language.integration.test.ts @@ -14,6 +14,7 @@ import { fieldGroup, text, } from "../../../config"; +import { registerHook, unregisterHook } from "../../../hooks"; import { createTestNextly, getConfiguredTestDialects, @@ -358,6 +359,65 @@ describe.each(getConfiguredTestDialects())( } ); + it("runs the update hooks once for the whole document when every language is published", async () => { + const t = await boot(dialect); + const id = await publishedInBoth(t); + await holdEdit(t, id, "de", { title: "DE v2" }); + const seen: string[] = []; + const before = (ctx: { req?: { http?: { method?: string } } }) => { + seen.push(`before:${ctx.req?.http?.method ?? "none"}`); + return undefined; + }; + const after = () => { + seen.push("after"); + return undefined; + }; + // Registered after boot: creating the instance resets the hook registry. + registerHook("beforeUpdate", SLUG, before as never); + registerHook("afterUpdate", SLUG, after as never); + try { + const res = await handlerOf(t).publishAllLocales({ + collectionName: SLUG, + entryId: id, + overrideAccess: true, + request: new Request("http://localhost/api/collections", { + method: "POST", + }), + }); + + expect(res.success, JSON.stringify(res)).toBe(true); + expect(seen).toEqual(["before:POST", "after"]); + expect((await live(t, id, "de")).title).toBe("DE v2"); + expect(await pendingLocales(t, id)).toEqual([]); + } finally { + unregisterHook("beforeUpdate", SLUG, before as never); + unregisterHook("afterUpdate", SLUG, after as never); + } + }); + + it("applies every language's pending change before taking the document down", async () => { + // A pending change sits over a published row. Withdrawn, the row is a + // draft and nothing accumulates onto it any more, so each language's + // pending change becomes the content, as a single-language unpublish does. + const t = await boot(dialect); + const id = await publishedInBoth(t); + await holdEdit(t, id, "en", { title: "EN v2" }); + await holdEdit(t, id, "de", { title: "DE v2" }); + + const res = await handlerOf(t).unpublishAllLocales({ + collectionName: SLUG, + entryId: id, + overrideAccess: true, + }); + + expect(res.success, JSON.stringify(res)).toBe(true); + const en = await live(t, id, "en"); + const de = await live(t, id, "de"); + expect([en.status, de.status]).toEqual(["draft", "draft"]); + expect([en.title, de.title]).toEqual(["EN v2", "DE v2"]); + expect(await pendingLocales(t, id)).toEqual([]); + }); + it("refuses the whole write, and keeps every pending change, when one edits a field the publisher may not", async () => { const t = await boot(dialect); const id = await publishedInBoth(t, GUARDED_SLUG); diff --git a/packages/nextly/src/domains/collections/services/all-locales-lifecycle.ts b/packages/nextly/src/domains/collections/services/all-locales-lifecycle.ts index 432bb6e146..39270d5b32 100644 --- a/packages/nextly/src/domains/collections/services/all-locales-lifecycle.ts +++ b/packages/nextly/src/domains/collections/services/all-locales-lifecycle.ts @@ -107,4 +107,8 @@ export interface AllLocalesLifecycleParams { authenticatedScope?: AuthenticatedScope; /** Who performed the transition, recorded on the events and the trail. */ actor?: RequestActor; + /** The request this operation's hooks are told about. */ + request?: Request; + /** Values shared between this operation's hooks. */ + context?: Record; } diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 24aa8e6b82..7507815d75 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -5023,16 +5023,16 @@ export class CollectionMutationService extends BaseService { } /** - * Publish ALL languages of an entry at once (i18n M7, spec §10). + * Publish every language of an entry at once. * - * Unchanged in behaviour and in signature: the route, the dispatcher and the - * admin hooks that call this keep working. What moved is where the work is - * stated — see {@link LifecycleDirection}. + * An ordinary update under the wildcard locale, so hooks, field rules, + * validation, the publish permission, every language's pending change, the + * version and the events all come from the one write path. */ async publishAllLocales( params: AllLocalesLifecycleParams ): Promise { - return this.setLifecycleAllLocales(PUBLISH_ALL_LOCALES, params); + return this.moveEveryLanguage(PUBLISH_ALL_LOCALES, params); } /** @@ -5103,7 +5103,104 @@ export class CollectionMutationService extends BaseService { data: null, }; } - return this.setLifecycleAllLocales(WITHDRAW_ALL_LOCALES, params); + return this.moveEveryLanguage(WITHDRAW_ALL_LOCALES, params); + } + + /** + * Move every language of a document through the one write path. + * + * A status patch under the wildcard locale is an ordinary update: hooks, + * field rules, validation, the lifecycle permission, every language's pending + * change, the version and the events all come from `updateEntry`. + */ + private async moveEveryLanguage( + direction: LifecycleDirection, + params: AllLocalesLifecycleParams + ): Promise { + let nothingToDo: CollectionServiceResult | null; + try { + nothingToDo = await this.lifecycleNothingToDo(direction, params); + } catch (error) { + return { + success: false, + statusCode: 500, + message: + error instanceof Error + ? error.message + : "Failed to change the status of every language", + data: null, + // A typed error keeps its own status and code. + ...errorEnvelopeFields(error), + }; + } + if (nothingToDo) return nothingToDo; + const result = await this.updateEntry( + { + collectionName: params.collectionName, + entryId: params.entryId, + user: params.user, + actor: params.actor, + overrideAccess: params.overrideAccess, + routeAuthorized: params.routeAuthorized, + authenticatedScope: params.authenticatedScope, + request: params.request, + context: params.context, + locale: EVERY_LOCALE, + }, + { status: direction.nextStatus } + ); + if (!result.success) return result; + return { + ...result, + message: direction.successMessage, + data: { id: params.entryId, status: direction.nextStatus }, + }; + } + + /** + * The answer for a collection with no draft/published lifecycle: the entry + * exists, the caller may update it, and there is nothing to move. `null` when + * the collection has a lifecycle, so the move goes ahead. + */ + private async lifecycleNothingToDo( + direction: LifecycleDirection, + params: AllLocalesLifecycleParams + ): Promise { + const collection = await this.collectionService.getCollection( + params.collectionName + ); + if ((collection as { status?: boolean }).status === true) return null; + const schema = await this.fileManager.loadDynamicSchema( + params.collectionName + ); + const [existing] = await this.db + .select() + .from(schema) + .where(eq(schema.id, params.entryId)) + .limit(1); + if (!existing) { + return { + success: false, + statusCode: 404, + message: "Entry not found", + data: null, + }; + } + const denied = await this.accessService.checkCollectionAccess({ + collectionName: params.collectionName, + operation: "update", + user: params.overrideAccess ? undefined : params.user, + overrideAccess: params.overrideAccess, + routeAuthorized: params.routeAuthorized, + authenticatedScope: params.authenticatedScope, + }); + if (denied) return denied; + return { + success: true, + statusCode: 200, + message: direction.nothingToDoMessage, + data: { id: params.entryId }, + }; } /** @@ -6081,6 +6178,10 @@ export class CollectionMutationService extends BaseService { from: string | null; data: Record; }[] = []; + // Whether the write locale's companion recorded the same transition as the + // main row, set where the durable status events are routed and read after + // the commit so the in-process events follow the same rule. + let mainTransitionEncodedByCompanion = false; try { // reject an unknown write locale before doing anything else. const badLocale = this.rejectInvalidWriteLocale(params.locale); @@ -6836,6 +6937,7 @@ export class CollectionMutationService extends BaseService { await withVersionConflictRetry(() => this.adapter.transaction(async tx => { recorded = false; + mainTransitionEncodedByCompanion = false; // Reset the payloads the promote fold rebinds, so a retried attempt // re-decides the split from the caller's input; and clear the pending // draft document, so a stale one from a promoted attempt cannot suppress @@ -7956,6 +8058,7 @@ export class CollectionMutationService extends BaseService { companionStatusWritten && localizedPreviousStatus === mainFrom && companionNext === mainTo; + mainTransitionEncodedByCompanion = companionEncodesMainTransition; // The main-row event must describe the main `status` column's // transition, but `updatedDocument`/`previousDocument` carry the // write-locale companion status overlaid — and for a default-locale @@ -8145,6 +8248,14 @@ export class CollectionMutationService extends BaseService { slug: readStringField(updated as Record, "slug"), previousSlug, locale: localizedUpdate?.writeLocale, + // Moving every language makes every language's address current. + localizedSlugs: sweepAllLocales + ? await this.readCompanionSlugsAllLocales( + this.db, + params.collectionName, + params.entryId + ) + : undefined, } ); } @@ -8207,7 +8318,14 @@ export class CollectionMutationService extends BaseService { | undefined) ?? null; const nextStatus = (updated as { status?: unknown }).status; - if (typeof nextStatus === "string" && nextStatus !== previousStatus) { + // Not reported untagged when the write locale's companion records this + // same transition: the locale-tagged event below reports it, as the + // durable events do, so a subscriber hears one publish rather than two. + if ( + typeof nextStatus === "string" && + nextStatus !== previousStatus && + !mainTransitionEncodedByCompanion + ) { this.transitionStatus({ collection: params.collectionName, id: (updated as { id?: unknown }).id, diff --git a/packages/nextly/src/services/collections-handler.ts b/packages/nextly/src/services/collections-handler.ts index dbdadf1a64..cb4a1f7acf 100644 --- a/packages/nextly/src/services/collections-handler.ts +++ b/packages/nextly/src/services/collections-handler.ts @@ -946,6 +946,8 @@ export class CollectionsHandler { authenticatedScope?: AuthenticatedScope; /** Acting identity from the transport, forwarded to the recorded event. */ actor?: RequestActor; + /** The request this operation's hooks are told about. */ + request?: Request; }) { return this.entryService.publishAllLocales(this.resolveUserParam(params)); } @@ -986,6 +988,8 @@ export class CollectionsHandler { authenticatedScope?: AuthenticatedScope; /** Acting identity from the transport, forwarded to the recorded event. */ actor?: RequestActor; + /** The request this operation's hooks are told about. */ + request?: Request; }) { return this.entryService.unpublishAllLocales(this.resolveUserParam(params)); } diff --git a/packages/nextly/src/services/collections/collection-entry-service.ts b/packages/nextly/src/services/collections/collection-entry-service.ts index 8724ee08bf..6ba2ade698 100644 --- a/packages/nextly/src/services/collections/collection-entry-service.ts +++ b/packages/nextly/src/services/collections/collection-entry-service.ts @@ -611,6 +611,8 @@ export class CollectionEntryService extends BaseService { routeAuthorized?: boolean; /** API-key scope; gates the unconditional publish check. */ authenticatedScope?: AuthenticatedScope; + /** The request this operation's hooks are told about. */ + request?: Request; }) { const result = await this.mutationService.publishAllLocales(params); await this.afterWriteIfRecorded(result, params.disableRevalidate); @@ -648,6 +650,8 @@ export class CollectionEntryService extends BaseService { routeAuthorized?: boolean; /** API-key scope; gates the unconditional unpublish check. */ authenticatedScope?: AuthenticatedScope; + /** The request this operation's hooks are told about. */ + request?: Request; }) { const result = await this.mutationService.unpublishAllLocales(params); await this.afterWriteIfRecorded(result, params.disableRevalidate); From b90eb370ca10e15da68c2d6fe3d20932a761a8a4 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 19:51:53 +0300 Subject: [PATCH 14/16] refactor(nextly): remove the second publish-all write path setLifecycleAllLocales stated the lifecycle gate, the row lock, the companion sweep, the version capture, the events and the cache flush a second time beside updateEntry. Both entry points now move every language through updateEntry, so it goes, along with the direction fields only it read. --- .../services/all-locales-lifecycle.ts | 47 +- .../services/collection-mutation-service.ts | 752 ------------------ 2 files changed, 6 insertions(+), 793 deletions(-) diff --git a/packages/nextly/src/domains/collections/services/all-locales-lifecycle.ts b/packages/nextly/src/domains/collections/services/all-locales-lifecycle.ts index 39270d5b32..e735166aec 100644 --- a/packages/nextly/src/domains/collections/services/all-locales-lifecycle.ts +++ b/packages/nextly/src/domains/collections/services/all-locales-lifecycle.ts @@ -1,25 +1,11 @@ /** - * Moving a document's whole lifecycle — every language at once. + * Moving a document's whole lifecycle: every language at once. * - * ## Why a direction rather than a method per verb - * - * "Set this document's status across its locales" is ONE operation with a - * parameter, not two operations that happen to look alike. The codebase reached - * for the second shape first: `publishAllLocales` stated the access gate, the - * row lock, the companion sweep, the version capture, the event fan-out and the - * cache flush for publishing, and a withdrawal written beside it would have - * stated all of them again — 745 of its 783 lines carry no direction at all. - * A third lifecycle verb would have stated them a third time. - * - * So the direction is data. A verb picks a target status, an access action and - * whether it establishes first publication, and inherits every guarantee the - * other direction already proved. - * - * Prior art agrees. Strapi's document service exposes `publish`/`unpublish` - * taking `locale: '*'` rather than four scope-specific methods; Payload's - * per-locale status is a flag on one write path, not a parallel one. Directus - * has no built-in per-language lifecycle at all, and its users hand-roll the - * asymmetry this module exists to avoid. + * A direction names the status every language ends up in and what to report, + * and nothing more. The move itself is an ordinary update under the wildcard + * locale, so the lifecycle permission, hooks, field rules, validation, each + * language's pending change, the version and the events come from the one + * write path rather than from a second copy of it. * * @module domains/collections/services/all-locales-lifecycle */ @@ -33,22 +19,6 @@ import type { UserContext } from "./collection-types"; export interface LifecycleDirection { /** The status every locale ends up in. */ nextStatus: "published" | "draft"; - /** - * The access rule kind this transition is judged against, checked ON TOP of - * `update`. Publishing and withdrawing are separate capabilities: someone - * trusted to put content live is not automatically trusted to take the whole - * site's translations of it down, and the reverse is likelier still. - */ - accessAction: "publish" | "unpublish"; - /** - * Whether this direction can ESTABLISH first publication. - * - * True for publishing only. `first_published_at` records when a document - * first became reachable, which withdrawing it does not change — re-dating or - * clearing it would make a later republish report a first publication that - * had already happened. - */ - stampsFirstPublished: boolean; /** * What to say when the collection has no lifecycle at all, so there is * nothing for this transition to move. @@ -71,8 +41,6 @@ export interface LifecycleDirection { /** Put every language of a document live. */ export const PUBLISH_ALL_LOCALES: LifecycleDirection = { nextStatus: "published", - accessAction: "publish", - stampsFirstPublished: true, nothingToDoMessage: "Nothing to publish (collection has no status).", successMessage: "All languages published.", }; @@ -80,9 +48,6 @@ export const PUBLISH_ALL_LOCALES: LifecycleDirection = { /** Take every language of a document down. */ export const WITHDRAW_ALL_LOCALES: LifecycleDirection = { nextStatus: "draft", - accessAction: "unpublish", - // A withdrawal never establishes first publication; see the field's note. - stampsFirstPublished: false, nothingToDoMessage: "Nothing to unpublish (collection has no status).", successMessage: "All languages unpublished.", }; diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 7507815d75..51b5086791 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -4270,758 +4270,6 @@ export class CollectionMutationService extends BaseService { ); } - /** - * Publish ALL languages of an entry at once (i18n M7, spec §10). Atomically sets the main - * `status` to 'published' and — when the collection has per-locale status (M6) — every companion - * row's `_status` to 'published', in a single transaction. For a non-localized / no-status - * collection it is a plain publish of the single row. Only touches status columns (no field - * values), so it needs none of the localized-write machinery. - */ - private async setLifecycleAllLocales( - direction: LifecycleDirection, - params: AllLocalesLifecycleParams - ): Promise { - // Set when the in-transaction document-rule re-check refuses the publish - // against the row-locked document. Declared out here so the catch can read - // it: the adapter re-wraps the thrown sentinel in a DatabaseError as the - // transaction rolls back, so `instanceof` no longer identifies it. - let publishDocDenied: CollectionServiceResult | undefined; - // The tags this publish invalidates, set before the success return so a - // publish busts the entry's cached reads. Publishing every locale, so no - // single locale tag — the locale-less id tag covers them all. - let revalidationIntent: RevalidationIntent | undefined; - try { - const accessUser = params.overrideAccess ? undefined : params.user; - const schema = await this.fileManager.loadDynamicSchema( - params.collectionName - ); - - const [existingEntry] = await this.db - .select() - .from(schema) - .where(eq(schema.id, params.entryId)) - .limit(1); - if (!existingEntry) { - return { - success: false, - statusCode: 404, - message: "Entry not found", - data: null, - }; - } - - const accessDenied = await this.accessService.checkCollectionAccess({ - collectionName: params.collectionName, - operation: "update", - user: accessUser, - overrideAccess: params.overrideAccess, - // The route already ran the `update` gate (against the API key's scope, - // when applicable), so skip the redundant RBAC re-check here; the publish - // gate below still runs. - routeAuthorized: params.routeAuthorized, - authenticatedScope: params.authenticatedScope, - }); - if (accessDenied) return accessDenied; - - // The draft/published lifecycle flag on the collection config, NOT the - // mere presence of a `status` column: a collection that defines an - // ordinary user field named `status` has the column but no lifecycle, so - // it is not publishable and must not demand the publish permission here. - // Resolved through the collection so a custom tableName/dbName override is - // honored below, matching every other mutation. - const publishCollection = await this.collectionService.getCollection( - params.collectionName - ); - const hasMainStatus = - (publishCollection as { status?: boolean }).status === true; - const companion = await this.fileManager.loadCompanionSchema( - params.collectionName - ); - const companionPublishable = - !!companion && - companion.hasStatus && - // Only `ready` matters: a companion that is not there has no per-locale publish - // lifecycle, and why it is not there changes nothing about that. - (await isCompanionReady(this.adapter, companion.companionTableName)); - - if (!hasMainStatus && !companionPublishable) { - // Nothing to publish — the collection has no status concept. Returned - // before the publish permission check so a collection with no lifecycle - // does not demand `publish-` for a call that changes nothing. - return { - success: true, - statusCode: 200, - message: direction.nothingToDoMessage, - data: { id: params.entryId }, - }; - } - - // This method exists to publish every locale, so it is unconditionally a - // publish and needs the publish permission on top of update — checked - // directly rather than via a transition, since it publishes companion - // locales even when the main row is already published. Runs only once - // there is actually something publishable. - - // Readiness for this collection AND for every field-group type it can hold, resolved on the - // pool before the transaction opens. The snapshot built inside it reads all of them, and - // there it can only READ a verdict — resolving issues a query, and a query against a missing - // relation aborts the whole transaction on PostgreSQL. A publish is a plausible first act on - // a fresh worker, and an unresolved verdict reads as unusable, so every translated value - // would be missing from the durable event. - await this.warmCompanionReadiness(params.collectionName); - await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ - fields: (publishCollection as { fields?: FieldConfig[] }).fields ?? [], - // Nothing is being written, so nothing is judged: this call is here purely for the - // verdicts it leaves behind. - data: {}, - locale: undefined, - }); - - const publishDenied = await this.accessService.checkCollectionAccess({ - collectionName: params.collectionName, - operation: direction.accessAction, - user: accessUser, - overrideAccess: params.overrideAccess, - // Not route-authorized as publish: the POST was authorized as `update`, - // so the publish permission is checked here. - routeAuthorized: false, - // Judge a scoped API key on its own `publish-` grant. - authenticatedScope: params.authenticatedScope, - }); - if (publishDenied) return publishDenied; - - // `publishCollection` (loaded above for the lifecycle flag) also resolves a - // custom tableName/dbName override, matching every other mutation; - // getTableName would hardcode the default dc_ and target the wrong - // table for a renamed collection. - const tableName = this.resolveTableName( - publishCollection, - params.collectionName - ); - - // Resolved versioning config + field set for the in-transaction capture. - const versionsConfig = (publishCollection as Record) - .versions as ResolvedVersionsConfig | null | undefined; - const fields = ((publishCollection as { fields?: unknown }).fields ?? - []) as FieldDefinition[]; - const manyToManyFields = fields.filter( - f => - f.type === "relationship" && f.options?.relationType === "manyToMany" - ); - const previousStatusRaw = (existingEntry as { status?: unknown }).status; - const previousStatus = - typeof previousStatusRaw === "string" ? previousStatusRaw : null; - - // The parent row for both the snapshot and the status-change event is - // re-read fresh inside the transaction (not the pre-transaction - // `existingEntry`), mirroring updateEntry: a conflict retry re-runs this - // closure, and any concurrent write committed before the tx began is then - // reflected, so neither the recorded snapshot nor the emitted event - // payload exposes a stale pre-image of the non-status columns. The closure - // sets it; it is read once after commit for the event. - let publishedParentRow: Record | undefined; - // Set inside the transaction when the publish records its outbox events, so - // the caller can flush the drain after IT commits. - let eventRecorded = false; - // Set inside the transaction when the row is gone by the time the lock is - // taken (deleted between the pre-transaction read and the lock), so the - // publish records nothing and the caller answers not-found rather than - // reporting a publish of content that no longer exists. - let entryVanished = false; - // The main row's status read UNDER the transaction lock, so the publish - // transition is judged against the committed value this publish overwrites - // rather than the stale pre-transaction read. The closure sets it; the - // post-commit transition event reads it. Defaults to the pre-read value so - // a companion-only (no main status) publish still has a sane fallback. - let lockedPreviousStatus = previousStatus; - // The first-publication marker this publish committed, or undefined when it recorded none. - // The event payload, version snapshot and workflow reaction are all built from the - // PRE-update row with the new status overlaid, so without carrying this across they would - // report the marker absent on the very publication that establishes it. Reset per attempt - // by the closure, so a retry after a concurrent winner does not reuse a stale value. - let publishFirstPublishedAt: Date | undefined; - // The per-locale publish transitions recorded to the outbox inside the - // transaction, replayed to the in-process workflow subscribers after it - // commits — the durable event and the reaction event must not diverge on - // which locales published. The closure rebuilds it each attempt. - let perLocaleTransitions: { - locale: string; - from: string | null; - data: Record; - }[] = []; - // Whether the default locale's companion row transitions to published, so - // the post-commit workflow replay suppresses the untagged main transition - // (the default's locale-tagged replay stands in for it). Set in the tx. - let defaultCompanionTransitions = false; - // Read the fresh post-publish parent whenever the collection captures - // versions or carries a status: the pre-transaction `previousStatus` can - // be stale (a concurrent writer may commit between it and the lock), so a - // transition detected only under the lock still needs the committed row - // for its event payload rather than falling back to the stale pre-read. - const needsFreshParent = !!versionsConfig?.enabled || hasMainStatus; - - // Retry the whole publish+capture transaction on a version_no allocation - // race, mirroring updateEntry. - await withVersionConflictRetry(() => - this.adapter.transaction(async tx => { - // Lock the main row up front. One read serves three needs: it is the - // liveness check (a row deleted between the pre-transaction read and - // this lock is gone here, so the publish writes and records nothing); - // it carries the committed status the publish transition is judged - // against; and it is the document a deferred publish rule re-checks. - // Reset per attempt because the conflict retry re-runs this closure. - entryVanished = false; - lockedPreviousStatus = previousStatus; - publishFirstPublishedAt = undefined; - perLocaleTransitions = []; - defaultCompanionTransitions = false; - const lockedRow = await tx.selectOne>( - tableName, - { where: this.whereEq("id", params.entryId), forUpdate: true } - ); - if (!lockedRow) { - // Nothing to publish — record nothing and roll back an empty tx. - entryVanished = true; - return; - } - const lockedStatusRaw = (lockedRow as { status?: unknown }).status; - lockedPreviousStatus = - typeof lockedStatusRaw === "string" ? lockedStatusRaw : null; - - // The committed pre-publish row in the main-table schema shape, read on - // the TRANSACTION connection (never the pool: the transaction already - // holds a connection, and a pooled read would wait on itself against a - // one-connection pool and deadlock). This is the concurrency-correct - // event pre-image; publishing changes only status, so its non-status - // columns are the post-publish columns too. Read after the lock, so on - // a retry it reflects the concurrent winner just like the pre-read did. - const [lockedSchemaRow] = (await tx - .getDrizzle() - .select() - .from(schema) - .where(eq(schema.id, params.entryId)) - .limit(1)) as (Record | undefined)[]; - const preImageRow = lockedSchemaRow ?? existingEntry; - - // No under-lock re-check: the transition gate judges the caller and - // the operation, not the row, so re-reading the row-locked document - // could not change its answer. - // Each locale's committed per-locale status BEFORE the bulk companion - // flip below, so a real draft->published transition can be told from a - // locale that was already live. Read under the lock and inside the - // retry so it reflects the state this publish actually overwrites. - let priorCompanionStatuses: Map; - try { - priorCompanionStatuses = - companion && companionPublishable - ? await readCompanionLocaleStatusAll( - tx.getDrizzle< - Parameters[0] - >(), - companion.table, - params.entryId, - cachedCompanionReadiness( - this.adapter, - companion.companionTableName - ) - ) - : new Map(); - } catch (err) { - // The helper already tolerates a missing companion table; any error - // here is a real database failure. Normalize it to the canonical - // internal error so the raw driver message (which can carry schema - // or connection details) does not reach the API caller through the - // service's failure result. - throw NextlyError.internal({ - cause: err instanceof Error ? err : undefined, - logContext: { - reason: "publish-all-companion-status-scan", - collection: params.collectionName, - }, - }); - } - - if (hasMainStatus) { - // The marker this publish records, if any. Decided from the row read under the lock - // above, so an already-published row records nothing — which matters most for rows - // published before this column existed, whose marker is null precisely because their - // history was never captured. Dating those today would report a publication that - // never happened. - const publishNow = new Date(); - const lockedMarker = ( - lockedRow as { first_published_at?: unknown } | undefined - )?.first_published_at; - // Publish-all can find a document in a mixed state: a draft main row alongside a - // translation that has been live since before this column existed. The main row's own - // transition then reads as a first publication when the document was already - // reachable, so the same document-level question is asked here. No locale is excluded - // — this write publishes all of them, so any already-published one predates it. - // Only a PUBLICATION can establish first publication. A withdrawal - // leaves the marker untouched: it records when the document first - // became reachable, which taking it down does not change, and - // re-dating or clearing it would make a later republish report a - // first publication that had already happened years earlier. - // - // Nothing below needs a branch for that — `firstPublishedStamp` - // stays undefined for a withdrawal, and every use of it is already a - // conditional spread. - if (direction.stampsFirstPublished) { - const alreadyPublicBeforeThisWrite = - lockedMarker == null - ? await this.isDocumentAlreadyPublic( - tx, - params.collectionName, - params.entryId, - lockedPreviousStatus, - undefined - ) - : false; - publishFirstPublishedAt = resolveFirstPublishedStamp({ - hasStatus: true, - previousStatus: alreadyPublicBeforeThisWrite - ? "published" - : lockedPreviousStatus, - nextStatus: "published", - existingMarker: lockedMarker, - now: publishNow, - }); - } - // Through the adapter's Drizzle layer rather than an interpolated statement. That - // also removes the reason the previous version needed a SQL `now()` expression: a - // `Date` bound as a raw parameter stores wrong against SQLite's integer timestamps, - // while Drizzle converts it per dialect. - await tx.update( - tableName, - { - status: direction.nextStatus, - updated_at: publishNow, - ...(publishFirstPublishedAt - ? { first_published_at: publishFirstPublishedAt } - : {}), - }, - this.whereEq("id", params.entryId) - ); - } - if (companion && companionPublishable) { - await this.writeCompanionStatus(tx, { - companionTableName: companion.companionTableName, - parentId: params.entryId, - status: direction.nextStatus, - locale: EVERY_LOCALE, - }); - } - - if (needsFreshParent) { - // The committed post-publish parent, built from the pre-image read on - // the transaction connection above: publish only mutates status, so - // its non-status columns are already the post-publish ones — overlay - // the new status rather than taking a second pooled connection while - // this transaction holds one (which would deadlock a one-connection - // pool). Undefined only if the row vanished, which the lock above - // already rules out. - // The marker is overlaid alongside the status for the same reason: it was written by - // the UPDATE above and so is not on the pre-image this row is built from. Without it - // the publication event and the captured version would both report no first - // publication for the write that just established one. - publishedParentRow = lockedSchemaRow - ? { - ...lockedSchemaRow, - status: direction.nextStatus, - ...(publishFirstPublishedAt - ? { first_published_at: publishFirstPublishedAt } - : {}), - } - : undefined; - - // Record a version snapshot for the publish: publishing changes the - // document's status, so history/audit should capture that state. - // Components + m2m are read from the transaction (read-your-writes). - // Status/owner/password handling matches the other capture paths. If - // the row was deleted concurrently, skip — nothing committed to - // snapshot. - if (versionsConfig?.enabled && publishedParentRow) { - const parentRow = convertTimestampsToCamelCase( - this.deserializeJsonFieldsForSnapshot( - { ...publishedParentRow }, - fields - ) - ); - stripPasswordFieldValues(parentRow, fields); - stripSystemOwnerField(parentRow); - const { - components: snapshotComponents, - manyToMany: snapshotM2M, - } = await this.buildFullSnapshotRelations( - tx, - params.entryId, - params.collectionName, - tableName, - fields, - manyToManyFields - ); - await captureInTx(tx, this.versionCapture, { - ref: { - scopeKind: "collection", - scopeSlug: params.collectionName, - entryId: params.entryId, - }, - contentStatus: direction.nextStatus, - // Tagged like every other capture: a snapshot records which - // component its values came from, whichever path produced it. - parts: await this.snapshotPartsFor( - { - parentRow, - components: snapshotComponents, - manyToMany: snapshotM2M, - }, - fields, - tx - ), - createdBy: params.user?.id ?? null, - // Left unlabelled deliberately. Publishing spans every locale, - // and this snapshot is the main row alone — on a migrated - // collection the localized columns live only in the companion, - // so it holds no locale's translatable values. Claiming one - // would tell a restore to write content it never captured. - locale: null, - maxPerDoc: versionsConfig.maxPerDoc, - }); - } - } - - // Append the base `entry.updated` outbox event for the publish write, - // then its publish lifecycle event, both on this transaction so they - // commit with the status write and never survive a rollback. Built from - // the post-publish document (the fresh in-tx row overlaid with the new - // status, or the pre-read row so the event still fires when no fresh - // read was needed). The publish event is gated on a real main-row - // transition, matching the post-commit `transitionStatus` below. - const publishedDocument = this.readShapeEventDocument( - { - ...(publishedParentRow ?? preImageRow), - status: direction.nextStatus, - }, - fields - ); - // Overlay the committed publish instant AFTER camelCasing: the source - // rows carry the pre-publish `updatedAt` (the pooled/pre-read excludes - // this tx's own `SET updated_at = now()`), and a snake-case overlay - // would be dropped because `convertTimestampsToCamelCase` keeps the - // existing camelCase value. Without this the event reports the stale - // timestamp and omits `updatedAt` from changedFields. - publishedDocument.updatedAt = new Date(); - // Built from the pre-image read under the lock, so a concurrent write - // committed between the pre-transaction read and the lock is reflected - // rather than the stale pre-read (its non-status fields and its own - // prior status). - const previousDocument = this.readShapeEventDocument( - preImageRow as Record, - fields - ); - const publishEventFields = await this.webhookFieldTreeIfRecording( - params.collectionName, - fields, - tx.getDrizzle() - ); - const publishActor = actorForWrite(params.actor, params.user); - const baseRecorded = await recordMutationEvent(tx, { - type: "entry.updated", - resource: { - kind: "entry", - collection: params.collectionName, - id: params.entryId, - }, - data: publishedDocument, - previous: previousDocument, - fields: publishEventFields, - actor: publishActor, - }); - eventRecorded = baseRecorded || eventRecorded; - // Whether the default locale's own companion row transitions to - // published here. When it does, the per-locale loop below emits the - // default locale's transition tagged `locale: ` — matching - // the ordinary localized update path, where a default-locale status - // that rides the companion is emitted locale-tagged and the untagged - // main-row event is suppressed (its companion event already encodes - // the transition). So a consumer routing the default language by its - // locale still sees the default translation go live. - const defaultLocale = this.localization?.defaultLocale; - defaultCompanionTransitions = - defaultLocale !== undefined && - priorCompanionStatuses.has(defaultLocale) && - priorCompanionStatuses.get(defaultLocale) !== direction.nextStatus; - // The document-wide (main-row) publish transition, WITHOUT a locale - // tag. Emitted only when a default-companion event does not already - // encode it (a non-localized collection, or a default locale whose - // status lives only on the main row) — otherwise the locale-tagged - // default event below stands in, avoiding a duplicate. Gated on the - // status read under the lock so a concurrent unpublish/publish is - // judged correctly. - if ( - hasMainStatus && - lockedPreviousStatus !== direction.nextStatus && - !defaultCompanionTransitions - ) { - const statusRecorded = await this.recordStatusEvents(tx, { - collection: params.collectionName, - id: params.entryId, - from: lockedPreviousStatus, - to: direction.nextStatus, - isCreate: false, - data: publishedDocument, - previous: previousDocument, - fields: publishEventFields, - actor: publishActor, - }); - eventRecorded = statusRecorded || eventRecorded; - } - // Per-locale publish transitions. The bulk flip above moved every - // companion locale to published in one statement, but a subscriber - // watching a single language needs its own `entry.published` — so each - // companion locale that actually transitioned gets a locale-tagged - // event, with the locale's own prior `_status` as the transition - // `from`. The default locale is included: its companion event replaces - // the untagged main event suppressed above. - // Only locales the app still configures get an event: a locale - // removed from configuration can leave stale companion rows behind, - // and a publish event tagged with a locale that normal reads/writes - // reject would mislead locale-routed consumers. - const configuredLocales = new Set( - this.localization?.locales.map(l => l.code) ?? [] - ); - for (const [locale, priorLocaleStatus] of priorCompanionStatuses) { - if (configuredLocales.size > 0 && !configuredLocales.has(locale)) - continue; - if (priorLocaleStatus === direction.nextStatus) continue; - // Build this locale's own before/after documents. Publishing changes - // only status, so the locale's translatable values AND its component - // subtrees are identical on both sides — read them at this locale and - // assemble them onto the main-row event shape so a `locale`-tagged - // event carries that language's full content (fields and localized - // components) and its own prior status, matching the ordinary - // localized update path. Read on the transaction (read-your-writes). - let rawLocaleValues: Record; - try { - // These values feed a durable locale-tagged event; a real - // companion read failure must abort rather than commit a publish - // event missing this locale's translated fields. - rawLocaleValues = await this.readCompanionLocalizedValues( - tx, - params.collectionName, - params.entryId, - locale - ); - } catch (err) { - // Normalize the raw driver error the same way the status scan - // above does, so a schema/permission failure returns the canonical - // internal error instead of leaking the driver's message through - // the service failure result. - throw NextlyError.internal({ - cause: err instanceof Error ? err : undefined, - logContext: { - reason: "publish-all-locale-values-read", - collection: params.collectionName, - locale, - }, - }); - } - const localeValues = this.deserializeJsonFieldsForSnapshot( - rawLocaleValues, - fields - ); - const { components: localeComponents, manyToMany: localeM2M } = - await this.buildFullSnapshotRelations( - tx, - params.entryId, - params.collectionName, - tableName, - fields, - manyToManyFields, - locale - ); - // Strip parent-level password fields before assembling: a localized - // group/repeater can carry a nested password hash the overlay - // reintroduces after `publishedDocument` was already redacted. The - // durable outbox re-strips while building its envelope, but the - // in-process `transitionStatus` replay receives this document - // unchanged, so strip here to protect both paths. Component subtrees - // are already password-stripped by the snapshot read. - const localeDataParent = { - ...publishedDocument, - ...localeValues, - status: direction.nextStatus, - }; - stripPasswordFieldValues(localeDataParent, fields); - const localePrevParent = { - ...previousDocument, - ...localeValues, - status: priorLocaleStatus, - }; - stripPasswordFieldValues(localePrevParent, fields); - const localeData = assembleDocument({ - parentRow: localeDataParent, - components: localeComponents, - manyToMany: localeM2M, - }); - const localePrevious = assembleDocument({ - parentRow: localePrevParent, - components: localeComponents, - manyToMany: localeM2M, - }); - const localeRecorded = await this.recordStatusEvents(tx, { - collection: params.collectionName, - id: params.entryId, - locale, - from: priorLocaleStatus, - to: direction.nextStatus, - isCreate: false, - data: localeData, - previous: localePrevious, - fields: publishEventFields, - actor: publishActor, - }); - eventRecorded = localeRecorded || eventRecorded; - // Remember the transition so the same locale is replayed to the - // in-process workflow subscribers post-commit, the way the localized - // update path does — otherwise `statusTransition`/`published` - // listeners never observe a companion locale going live. - perLocaleTransitions.push({ - locale, - from: priorLocaleStatus, - data: localeData, - }); - } - }) - ); - - // The entry was deleted out from under the publish: nothing was written or - // recorded, so answer not-found rather than a success for absent content. - if (entryVanished) { - return { - success: false, - statusCode: 404, - message: "Entry not found", - data: null, - }; - } - - // Post-commit status events: publishing is a real status transition, so - // workflow subscribers on statusTransition/published must see it (this - // path previously changed status without emitting anything). Skip when the - // main row was already published (no transition, judged on the status read - // under the lock), matching updateEntry. Prefer the fresh in-tx row; fall - // back to the pre-read only if the row vanished mid-publish. - if ( - hasMainStatus && - lockedPreviousStatus !== direction.nextStatus && - !defaultCompanionTransitions - ) { - this.transitionStatus({ - collection: params.collectionName, - id: params.entryId, - data: publishedParentRow ?? { - ...(existingEntry as Record), - status: direction.nextStatus, - }, - user: params.user, - previousStatus: lockedPreviousStatus, - status: direction.nextStatus, - emitStatusChanged: true, - }); - } - // Each companion locale that went live, replayed to the in-process - // workflow subscribers with its own `locale` — mirroring the localized - // update path — so `statusTransition`/`statusChanged`/`published` - // listeners observe every published translation, not only the main row. - for (const transition of perLocaleTransitions) { - this.transitionStatus({ - collection: params.collectionName, - id: params.entryId, - data: transition.data, - user: params.user, - previousStatus: transition.from, - status: direction.nextStatus, - emitStatusChanged: true, - locale: transition.locale, - }); - } - - // emit the post-commit "updated" reaction event so cache - // revalidation / webhooks fire, matching a single-locale publish. Best-effort: a - // reaction failure must not fail the already-committed publish. - try { - const [updated] = await this.db - .select() - .from(schema) - .where(eq(schema.id, params.entryId)) - .limit(1); - if (updated) { - emitCollectionEvent( - "updated", - params.collectionName, - updated as Record, - params.user - ); - } - } catch { - // Reaction/event emission is non-critical; the publish already committed. - } - - // Publishing all locales makes every locale's slug public at once, so bust - // each localized slug's tag (read post-commit on the pool — publish only - // flips status, so the committed companion slugs are stable here). - const publishedLocalizedSlugs = await this.readCompanionSlugsAllLocales( - this.db, - params.collectionName, - params.entryId - ); - revalidationIntent = buildEntryRevalidationIntent( - params.collectionName, - readRevalidateConfig(publishCollection), - { - id: params.entryId, - // Prefer the committed post-publish row's slug so a concurrent rename - // that landed after the pre-read still busts the actually-published - // URL; fall back to the pre-read only when no fresh row was - // reconstructed. - slug: readStringField( - (publishedParentRow ?? existingEntry) as Record, - "slug" - ), - localizedSlugs: publishedLocalizedSlugs, - } - ); - - return { - success: true, - statusCode: 200, - message: direction.successMessage, - data: { id: params.entryId, status: direction.nextStatus }, - eventRecorded, - revalidationIntent, - }; - } catch (error) { - // A publish refused by the under-lock document-rule re-check aborts the - // transaction; return the 403 it resolved, not a 500. - if (publishDocDenied) { - return publishDocDenied; - } - return { - success: false, - statusCode: 500, - message: - error instanceof Error - ? error.message - : "Failed to publish all languages", - data: null, - // A typed error keeps its own status and code. Hardcoding 500 reported - // a hook's refusal or rate limit as a server fault, and left a boundary - // nothing to rebuild it from. - ...errorEnvelopeFields(error), - }; - } - } - /** * Publish every language of an entry at once. * From 4713e55c655c5cf6281407ee0f0ea87bce52d5f5 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 21:20:59 +0300 Subject: [PATCH 15/16] chore(nextly): describe publish all languages on one write path for the release notes --- ...anguages-publishes-every-pending-change.md | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 .changeset/publish-all-languages-publishes-every-pending-change.md diff --git a/.changeset/publish-all-languages-publishes-every-pending-change.md b/.changeset/publish-all-languages-publishes-every-pending-change.md new file mode 100644 index 0000000000..a57c6c772a --- /dev/null +++ b/.changeset/publish-all-languages-publishes-every-pending-change.md @@ -0,0 +1,34 @@ +--- +"nextly": patch +"create-nextly-app": patch +"@nextlyhq/admin": patch +"@nextlyhq/admin-css": patch +"@nextlyhq/blocks-engine": patch +"@nextlyhq/blocks-react": patch +"@nextlyhq/ui": patch +"@nextlyhq/adapter-drizzle": patch +"@nextlyhq/adapter-postgres": patch +"@nextlyhq/adapter-mysql": patch +"@nextlyhq/adapter-sqlite": patch +"@nextlyhq/storage-s3": patch +"@nextlyhq/storage-uploadthing": patch +"@nextlyhq/storage-vercel-blob": patch +"@nextlyhq/plugin-form-builder": patch +"@nextlyhq/plugin-mcp": patch +"@nextlyhq/plugin-page-builder": patch +"@nextlyhq/plugin-seo": patch +"@nextlyhq/plugin-sdk": patch +"@nextlyhq/eslint-config": patch +"@nextlyhq/eslint-plugin": patch +"@nextlyhq/prettier-config": patch +"@nextlyhq/telemetry": patch +"@nextlyhq/tsconfig": patch +"@nextlyhq/builder": patch +"@nextlyhq/module-specifiers": patch +--- + +"Publish all languages" and "Unpublish all languages" on a collection entry now run through the same update path as any other edit, so update hooks, field rules and validation apply to them. + +Moving every language at once, from the button or from a scheduled release, now applies every language's pending change instead of refusing when another language holds one. A shared field keeps the edit of the language that changed it, and when two languages changed the same shared field the later save wins. Each language's translations, including those inside components, come from its own pending change. A pending change held for a language the app no longer configures is left in place. + +When publishing every language fails, the admin now shows the server's reason instead of one fixed message. From 4274860506b21702b96d92b1cd55b83227491309 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 00:36:58 +0000 Subject: [PATCH 16/16] chore(root): lower the comment allowlist to what collection-mutation-service holds Deleting setLifecycleAllLocales removed two recorded offences from collection-mutation-service.ts, leaving 17 where the allowlist recorded 19. The allowlist may only shrink, so the entry is lowered to 17 and the two digests whose comments no longer exist are removed; the pinned total follows to 505. The entry count is unchanged, since the file still holds offences. --- scripts/check-comment-convention.test.mjs | 2 +- scripts/comment-convention-allowlist.json | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/scripts/check-comment-convention.test.mjs b/scripts/check-comment-convention.test.mjs index 1ba6495e51..061585815f 100644 --- a/scripts/check-comment-convention.test.mjs +++ b/scripts/check-comment-convention.test.mjs @@ -221,7 +221,7 @@ describe("the allowlist", () => { // more when patterns began reading normalised text and a label wrapped across lines became // visible. A raise for any other reason is the silencing this guards against. const EXPECTED_ENTRIES = 228; - const EXPECTED_TOTAL = 507; + const EXPECTED_TOTAL = 505; it("matches its pinned size exactly", () => { expect(readAllowlist().size).toBe(EXPECTED_ENTRIES); diff --git a/scripts/comment-convention-allowlist.json b/scripts/comment-convention-allowlist.json index 7688a2988a..df495e2234 100644 --- a/scripts/comment-convention-allowlist.json +++ b/scripts/comment-convention-allowlist.json @@ -436,16 +436,14 @@ "digests": ["d5997ab8d4023315"] }, "packages/nextly/src/domains/collections/services/collection-mutation-service.ts": { - "count": 19, + "count": 17, "digests": [ "03f5b83d5dd2242f", - "0524236cf96cf39b", "09e9b38cef55f0e2", "0cf6c1f6791c6d67", "21185c2d7cba5586", "3334c2660c4178e0", "4cc0a776deb9205d", - "606368adea554209", "6b8890b87c096b79", "6e6b410ca569dace", "782632344c273aa0",