fix(nextly): publish all languages runs hooks and applies every language's pending change - #1883
mobeenabdullah wants to merge 17 commits into
Conversation
…e them to Singles too
…version lost data
…ould 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.
… 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.
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.
…g 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.
…ons 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.
…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.
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.
|
@codex review |
|
@coderabbitai review |
|
|
Warning Review limit reachedNext included review available in 34 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4713e55c65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (promoteEveryLanguage && promoteRestoreCtx) { | ||
| // Every configured language's pending change is applied here, and | ||
| // the write below moves only the lifecycle. | ||
| await this.promoteEveryLanguageInTx(tx, { |
There was a problem hiding this comment.
When a non-default locale already has published status and holds a pending edit—the normal state in which a working draft exists—this call applies and deletes that locale's draft, but the subsequent entry.updated event is emitted only for the normalized default locale, while recordSweptLocaleStatusEvents emits nothing because the locale's status did not change. Webhook and indexing consumers therefore never learn that the non-default locale's live content changed; preserve the promoted locales returned by this helper and record a locale-tagged update for each one independently of lifecycle transitions.
Useful? React with 👍 / 👎.
| if (promoteEveryLanguage && promoteRestoreCtx) { | ||
| // Every configured language's pending change is applied here, and | ||
| // the write below moves only the lifecycle. | ||
| await this.promoteEveryLanguageInTx(tx, { |
There was a problem hiding this comment.
When multiple localized working drafts are promoted here, the helper writes and deletes all of them, but the ordinary capture later in updateEntry runs only once and uses localizedUpdate.writeLocale (the normalized default locale). Consequently a German, French, etc. pending snapshot is consumed without ever becoming a numbered version for that locale, so locale-filtered history cannot display or restore the content that was just released. Capture the post-promotion state of every affected locale before deleting its working draft.
Useful? React with 👍 / 👎.
| message: direction.successMessage, | ||
| data: { id: params.entryId, status: direction.nextStatus }, |
There was a problem hiding this comment.
If a beforeOperation, beforeUpdate, or beforeChange hook replaces the requested status with the opposite valid lifecycle status, updateEntry accepts that value and sweeps every locale to it, but this wrapper unconditionally reports the original direction. For example, a publish-all whose hook changes status to draft can successfully unpublish the document while returning “All languages published” and { status: "published" }; either reject a post-hook status that differs from the requested direction or return the committed status.
Useful? React with 👍 / 👎.
| for (const [key, child] of Object.entries(instance)) { | ||
| const nested = shape.nested(instance, key); | ||
| assign( |
There was a problem hiding this comment.
When a pending draft predates a newly added localized field inside an existing component instance, withTranslationsFrom keeps the current fallback value because the draft source lacks that key, and this loop then copies every non-nested child into the locale write without checking whether the source actually held it. Publishing the draft therefore stores another locale's fallback as an authored translation, so later edits to the fallback language stop propagating. Omit translatable keys that are absent from the source instance when constructing the promoted component write.
Useful? React with 👍 / 👎.
| a.updatedAt.getTime() - b.updatedAt.getTime() || | ||
| a.locale.localeCompare(b.locale) |
There was a problem hiding this comment.
On SQLite, nextly_versions.updated_at is stored with whole-second resolution, so edits to the same shared field in two locales within one second reach this comparator with identical timestamps; MySQL can likewise collide within its millisecond precision. Falling back to the locale code then makes alphabetical order—not the actual later save—decide which shared value is published, silently violating the advertised last-save-wins behavior. Persist and sort by a cross-locale monotonic ordering value rather than using the locale as the tie-breaker.
Useful? React with 👍 / 👎.
github-actions
Bot
added
scope: core
…hooks-and-promotes-every-draft
@nextlyhq/adapter-drizzle
@nextlyhq/adapter-mysql
@nextlyhq/adapter-postgres
@nextlyhq/adapter-sqlite
@nextlyhq/admin
@nextlyhq/admin-css
@nextlyhq/blocks-engine
@nextlyhq/blocks-react
@nextlyhq/builder
create-nextly-app
@nextlyhq/eslint-plugin
nextly
@nextlyhq/plugin-form-builder
@nextlyhq/plugin-mcp
@nextlyhq/plugin-page-builder
@nextlyhq/plugin-sdk
@nextlyhq/plugin-seo
@nextlyhq/storage-s3
@nextlyhq/storage-uploadthing
@nextlyhq/storage-vercel-blob
@nextlyhq/ui
commit: |
…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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4274860506
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await this.applyLanguageChangeInTx( | ||
| tx, | ||
| ctx, | ||
| change, | ||
| liveByLocale.get(change.locale) ?? {} |
There was a problem hiding this comment.
When two pending drafts exist and the later one changes a shared field, this loop validates the earlier locale before that shared value is written and never revisits it. For example, a custom validator can allow note: "bad" for the German document but reject it when data.title is English; German's later draft passes, writes the shared note, and leaves the final English document invalid even though publish-all succeeds. Fold the final shared state first, then validate every locale against that state before applying any draft.
AGENTS.md reference: AGENTS.md:L303-L311
Useful? React with 👍 / 👎.
| if (promoteEveryLanguage && promoteRestoreCtx) { | ||
| // Every configured language's pending change is applied here, and | ||
| // the write below moves only the lifecycle. | ||
| await this.promoteEveryLanguageInTx(tx, { |
There was a problem hiding this comment.
When a default-locale pending draft contains localized fields, this helper writes those values directly to the companion row, but the status-only caller ignores the result and leaves localizedUpdate containing only _status. The post-commit responseSource is then built from the main row—which deliberately lacks localized fields—and only overlays that status-only split, so the newly enabled afterUpdate and afterChange hooks receive stale or absent translated fields even though they were just published; hook-driven indexing or synchronization can therefore publish an incomplete document. Rebuild the resolved default-locale document after promotion before invoking the hooks.
Useful? React with 👍 / 👎.
| const structureChanged = !sameContent( | ||
| withoutTranslations(pending, shape), | ||
| withoutTranslations(live, shape) | ||
| ); | ||
| return structureChanged | ||
| ? pending | ||
| : withTranslationsFrom({ current, pending, shape }); |
There was a problem hiding this comment.
When two locale drafts edit different non-localized children of the same component instance, any shared difference makes this branch return the later draft's entire component snapshot. For example, an older English draft can change variant while a later German draft changes url; the German snapshot still contains the live variant, so applying it second silently reverts the English edit even though German never touched that child. Merge only the shared component paths that differ from that draft's live baseline onto current, reserving whole-instance replacement for actual structural changes.
Useful? React with 👍 / 👎.
2 participants
Stacked on #1859: the base branch is
fix/a-stale-live-verdict-does-not-refuse-a-valid-publish. Once #1859 merges, this rebases ontomain.What changes for editors
beforeUpdateorafterUpdate, no field rules, no validation. They now send a status patch throughupdateEntryunder the wildcard locale, the path a single-language publish and a scheduled release already use.Rulings implemented:
decision:bulk-publish-and-the-pending-draft(B) anddecision:all-languages-publish-uses-one-write-path(one write path).How
pending-change-merge.tsholds the rule as pure functions (order, content equality, shared values, component translations), unit-tested.updateEntry, the wildcard's held-draft 409 is replaced bypromoteEveryLanguageInTx. Every language's live document is read before anything is written. Each language is then built, judged byresolvePromotedDocument(fail closed, no caller payload), validated with contexts resolved before the transaction, and written. 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.publishAllLocalesandunpublishAllLocaleskeep their pre-checks (a collection with no lifecycle still answers "nothing to do"; unpublish still refuses a companion with no_statuscolumn) and delegate. The route passes the request, so hooks see it.setLifecycleAllLocales(752 lines) is deleted.Behaviour changes worth knowing
wildcard-locale-contract's collection test that asserted the 409 now asserts promotion, per the ruling. Its removed-language case now also asserts the held work is kept.document.publishedhears a default-language publish once, tagged with the language, rather than once untagged and once tagged.Not in this PR
SinglePublishAllServicestill runs no hooks and the Singles wildcard still refuses. Filed astask:singles-publish-all-on-one-write-path.findAllWorkingDraftshas no ORDER BY, so the Singles publish-all shared-value winner depends on the database:finding:find-all-working-drafts-has-no-order. The collections path orders explicitly.Verification
Each new test has a control that fails for the reason it names:
expected 'live note' to be 'EN edited note'expected 'DE note' to be 'EN note'unpublished changes in deexpected true to be false: an empty German row storedexpected [] to deeply equal [ 'before:POST', 'after' ]expected [ 'EN v1', 'DE v1' ] to deeply equal [ 'EN v2', 'DE v2' ]nextly:locposts:slug:bonjour;[ undefined, 'en', 'de' ]Unit:
pending-change-merge15 of 15, with a naive last-save merge and a structure-only component merge each failing their case.Integration, SQLite: the new suite 8 of 8; the wildcard contract, promoted-draft-loss, per-language drafts and localized release suites 32 of 32; every suite that calls publish all (i18n publish and unpublish, cache revalidation, first-published marker, publish RBAC, status transition events, webhook outbox, write-path events matrix, plugin document events) passing after the two wildcard fixes.
A local run of the whole integration config was taken while the machine ran at a load of about 150 on 14 cores; it reported 9 failing files, 6 of them unrelated to this change. The same 9 files re-run in isolation at normal load pass: 9 files, 78 tests. CI runs the whole config on SQLite, PostgreSQL and MySQL.
pnpm --filter nextly check-typesexit 0 after the rebase onto #1859's head. eslint (--max-warnings 0),pnpm check:commentsandpnpm fallow:auditexit 0; they ran before that rebase, which changed only comments in #1859's lines.