Repository navigation
feat(content): ✨ scaffold visibility, duplication and id strategy contracts - #851
Conversation
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. |
|
React Doctor found 6 new issues in 5 files · 6 warnings · score 82 / 100 (Needs work) · 0 fixed · vs 6 warnings
Reviewed by React Doctor for commit |
![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: 9058d9171c
ℹ️ 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".
| * or `bigint` (both strings in every JSON-facing API). Changing it on an | ||
| * existing content type is a data migration, not a config change. | ||
| */ | ||
| idStrategy?: TIdStrategy; |
There was a problem hiding this comment.
When a content type selects uuid or bigint, this public option promises string identifiers, but buildSystemColumns() still creates id with serial(), while buildContentSchemas() still validates response IDs and route parameters as numbers. The definition therefore compiles with string-typed IDs but creates numeric rows and rejects the promised UUID/bigint URLs; reject these strategies until storage, schemas, services, and routes are implemented, or plumb the selected strategy through all of them.
Useful? React with 👍 / 👎.
| | "duplicated" | ||
| | "hidden" |
There was a problem hiding this comment.
The commit adds public duplicated, hidden, and unhidden event contracts and payloads, but the repository-wide event reference still contains no entry for any of them. Consumers therefore cannot discover their triggers, payload differences, or the duplicate event ordering described only in source comments; add them to apps/web/content/docs/dev/events/built-in-events.mdx as required for built-in events.
AGENTS.md reference: AGENTS.md:L57-L57
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 291435416e
ℹ️ 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".
| // `id` is a decimal string: `example.event` uses the `bigint` id strategy, and | ||
| // an identifier above 2^53 would not survive a round trip through a number. | ||
| const zodEvent = z.object({ | ||
| body: z.string(), |
There was a problem hiding this comment.
The event definition declares body as nullable and exposes it through the public API, so publishing an event without a body legitimately returns body: null. This schema rejects that response and makes loadEvent throw instead of rendering the event page; accept null here and handle it when rendering EditorContent.
Useful? React with 👍 / 👎.
| const detail = await fetcher({ | ||
| plugin: CONFIG_PLUGIN.pluginId, | ||
| method: "get", | ||
| module: "content/events", | ||
| path: "/{slug}", |
There was a problem hiding this comment.
When a published event's slug changes, its content type has delivery.redirects enabled, but this loader only calls the current-slug public endpoint. Requests to the historical /events/:slug therefore become not-found instead of receiving the configured permanent redirect, and the configured SEO/hreflang metadata is likewise never applied; fetch /delivery/resolve/{slug} and pass it through the delivery-page helper as the article page does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44d340623b
ℹ️ 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".
| for (const index of definition.indexes) { | ||
| if (!index.unique) continue; | ||
| if (index.on.some(column => regenerated.has(column))) continue; | ||
| if (!index.on.every(column => column in storage)) continue; |
There was a problem hiding this comment.
When a duplicable type declares a unique composite index such as on: ["status", "code"], this condition skips it because status is not in storage. Duplicating a draft then copies both the draft status and code, so PostgreSQL rejects the insert with a generic 409 instead of the documented CONTENT_DUPLICATE_UNIQUE_REQUIRED 422 that identifies code and allows the caller to retry with an override. Include applicable generated values such as status/version when determining whether the copy preserves a unique index.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…tracts Adds the definition-level configuration and resolved shapes for record visibility, duplication and the primary key strategy, the central content ID helpers, the can_hide permission and the duplicated/hidden/unhidden event contracts. Behaviour lands in follow-up commits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
Adds `duplicate(sourceId, options)` to the plain and editorial services of a
content type with `duplication: { enabled: true }`, orchestrated once in
`content/server/duplicate.ts` on top of the existing create pipeline: one
transaction, the source locked FOR SHARE, every collection and translation
copied as a draft with its own create revision, fresh slugs per locale
(live rows and redirect history, bounded candidates, savepoint retry on a
concurrent 23505), a localized title suffix and a 422 for unique fields
that need an override.
`POST /{id}/duplicate` (can_create + can_view) runs the standard create
effects once after commit, then the `duplicated` event. Includes a typed
AdminCP client helper and Postgres-backed tests.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
Content types with `visibility: { enabled: true }` get generated `hiddenAt` /
`hiddenBy` columns (FK core_users, ON DELETE SET NULL, indexed), and a hidden
record is unavailable on every public read path regardless of publication:
the SQL `publishedCondition` ANDs `hiddenAt IS NULL` and the JS
`isContentPubliclyVisible` refuses a set `hiddenAt`, so list, detail,
localized reads and fallback, delivery resolve and redirects, sitemap,
hreflang alternates, public locales, search and diagnostics all agree.
Previews keep working.
- Plain and editorial `hide` / `unhide` services (idempotent, optional
`expectedVersion`, `hide`/`unhide` revisions, delivery bookkeeping),
present only when visibility is enabled and typed with `ContentIdOf`.
- `POST /{id}/hide` and `/{id}/unhide` behind `can_hide`, effects after the
commit: hidden/unhidden events, private/public search documents, delivery
events, and front-end revalidation via `dispatchContentRevalidation`.
- Admin list `visibility=hidden|visible` filter next to `status`.
- Restore never changes visibility; publish/unpublish and scheduled
transitions keep a hidden record hidden and now report the real
before-state (the schedule executor no longer assumes an unpublish came
from a public row).
- Typed AdminCP client helpers `setContentVisibilityInBrowser` and
`setContentHiddenInBrowser`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
AdminCP (generic Content Engine screens): - Row actions "Duplicate" (can_create + can_view, duplication-enabled types) and "Hide"/"Unhide" (can_hide, visibility-enabled types), each behind an accessible confirmation; sonner toasts with the record's title; list invalidation; a duplicate opens the copy's edit page (page mode) or edit dialog (dialog mode). Duplicate refusals name the field: slug conflict and unique-field-required map to field-specific messages. - "Hidden" badge (icon + text) beside the status in the list and the editor. - Status and Visibility single-value filters in the list toolbar (All / Visible / Hidden), wired to ?status= and ?visibility=; malformed values in the URL read as "all" instead of a 400. - Editor: Hide/Unhide control sent with the form's expectedVersion (409 opens the conflict banner), hidden notice in the status area, and "Publishing a hidden record does not make it public" in publish confirmations. - Content form transport gains optional setHidden; the form dialog slot can be opened without a trigger (open/onOpenChange). Blog: blog.post enables duplication and visibility; can_hide permission label; the article editor shows the hidden notice and inherits the badge and the Hide/Unhide control; the legacy event bridge documents why duplicated, hidden and unhidden are not re-emitted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
…he engine core Thread ContentIdOf through types, schemas, services, routes, effects, search, delivery, previews, schedules and the AdminCP; key the shared revisions/schedules/slug-history/search tables by contentIdKey. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
…search item keys Adds example.tag (uuid) and example.event (bigint) to the example plugin with a uuid tags relation on advanced-article, full admin wiring and locales. Covers id strategies with Postgres integration tests, the shared itemId varchar migration, type tests and parsing tests. Elasticsearch documents gain an itemKey keyword so non-numeric ids round-trip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
example.event publishes /events/:slug, and the boot guard refuses to start the app when no page route serves a content type's delivery path. Adds the event page: the rich text body through EditorContent and its sessions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
The AdminCP list, item, mutation and option responses were still parsed with numeric id schemas, so a uuid or bigint content type's list failed to load and its to-many pickers showed raw keys instead of labels. Adds contentAnyIdSchema for code that reads rows of any content type. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
…rategy Canary's live editing (field locks, shared drafts, collaborative documents) and AI translation sources stored a content record's itemId as an integer. They now store contentIdKey(id) in varchar(64), like the other shared content tables, and read it back under the content type's own strategy. Socket rooms accept any strategy's id and the server re-parses it before it reaches Postgres; the live routes parse ids like every other generated route; the AdminCP live editor and translation freshness carry ContentId. Also drops a table test that asserted a column's SQL type. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
Canary stores field.richText as a JSON document, so the bigint event page renders it with RichTextContent, and the blog article editor takes the ContentId the form layout now passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
Adds blog_posts.hiddenAt/hiddenBy (FK core_users ON DELETE SET NULL, indexed); stores the content item key of every shared content table (revisions, schedules, slug history, search index, live editing locks, drafts and documents, AI translation sources) as varchar(64), cast in place so serial keys keep their digits; and adds the example plugin's uuid tags and bigint events. No other blog changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
Removes Drizzle column-type assertions, re-assertions of the id parser through wrappers, static constant and option-value checks, duplicated render tests whose logic has a pure-function test, and thin wrappers over the shared confirm dialog; collapses near-identical cases into it.each tables and moves the filter selection tests next to their function. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
The shared data table's row id widened to number | string for uuid and bigint content, which broke onReorder callbacks typed number[]. The ids are the rows' own, so they now carry the row's id type and numeric tables keep number[]. Also formats content/types.ts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
5bc8342 to
98a6adf
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98a6adf49f
ℹ️ 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".
| * How the primary key is generated: `serial` (the default, a number), `uuid` | ||
| * or `bigint` (both strings in every JSON-facing API). Changing it on an | ||
| * existing content type is a data migration, not a config change. | ||
| */ | ||
| idStrategy?: TIdStrategy; |
There was a problem hiding this comment.
The public idStrategy option changes database column generation, JSON identifier types, relation storage, URLs, and migration requirements, but this commit does not update any Content Engine documentation and a repo-wide search of apps/web/content/docs finds no idStrategy reference. Add guidance covering all three strategies—especially the required data migration for an existing type—so plugin authors do not treat this as a safe configuration-only change.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
|
|
||
| return { | ||
| id: source.itemId, | ||
| id: typeof itemId === "number" ? itemId : 0, |
There was a problem hiding this comment.
When Elasticsearch returns a UUID- or bigint-keyed content document, itemId is a string and this assigns the literal 0 to the hit's public id. Consequently every string-keyed result exposes the same hit identifier, so API consumers that use SearchHit.id for selection, normalization, or caching collide even though the records differ; return a stable per-hit identity instead of a shared sentinel.
Useful? React with 👍 / 👎.
…gies Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf244dcec1
ℹ️ 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".
| ...(locales === undefined ? {} : { locales }), | ||
| // A hidden page must stop being served now, not after its next revalidation. | ||
| mode: "immediate", | ||
| slugs: typeof slug === "string" && slug !== "" ? [slug] : [], |
There was a problem hiding this comment.
When redirects are enabled and a front end caches /delivery/resolve/{slug} by contentDeliveryRedirectTag, hiding changes every historical slug from a redirect to not_found, but this revalidation request expires only the current slug. Previously cached old slugs can therefore keep redirecting while the record is hidden (and cached not_found results can survive an unhide); include the record's active historical slugs in the invalidation or make those reads share an item-level tag.
AGENTS.md reference: AGENTS.md:L54-L55
Useful? React with 👍 / 👎.
feat(server): ✨ implement hidden access control for content visibility feat(admin): ✨ create publication label component for content status display feat(admin): ✨ add save menu for content actions including publication and visibility
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11e05b6747
ℹ️ 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".
| const values = row as Record<string, unknown>; | ||
| const itemId = parseContentId(definition.idStrategy, values.id); | ||
| if (itemId === null) return null; |
There was a problem hiding this comment.
For a visibility-enabled type whose publicApi.fields does not include id, findBySlug returns the projected row without an identifier, so this parse always fails and returns null before canViewHiddenContent checks author or editor access. This is a valid configuration because resolveVisibility requires publication but does not require exposing id; consequently even staff with can_edit receive a 404 for hidden detail and delivery pages. Retain the internal ID alongside the projection or resolve authorization from an unprojected lookup.
Useful? React with 👍 / 👎.
| const isReachabilityOperation = (operation: ContentSearchOperation): boolean => | ||
| operation === "publish" || | ||
| operation === "unpublish" || | ||
| operation === "hide" || | ||
| operation === "unhide"; |
There was a problem hiding this comment.
With an external provider such as Elasticsearch, a transient provider failure during hide is caught by syncContentSearch, so the hide request still succeeds while the external document remains public. A repeated hide then returns changed: false, and treating hide as a reachability no-op makes synchronization skip permanently; the hidden record can therefore remain visible in site search until a manual reindex. Queue or otherwise retry failed hide/unhide synchronization instead of relying on a later state transition.
Useful? React with 👍 / 👎.
…g/unhiding content - Introduced `canHide` permission check in `ContentListTable` for managing visibility. - Updated `ContentRowActionsMenu` to include visibility actions with new `VisibilityRowPanel`. - Enhanced bulk actions to support `hide` and `unhide` functionalities. - Refactored `contentBulkActions` to accommodate new visibility actions. - Added `AdminSidebarToggle` component for improved sidebar interaction. - Updated UI components to reflect changes in visibility management.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| }); | ||
|
|
||
| for (const translation of input.outcome.translations) { | ||
| await contentTranslationEffects(c, definition, translation, { |
There was a problem hiding this comment.
React Doctor · react-doctor/async-await-in-loop (warning)
This for…of loop waits before starting the next iteration. If iterations perform independent asynchronous work, consider bounded concurrency; await syntax alone does not establish a speedup.
Fix → Consider concurrent calls only for independent asynchronous work. Shared queues or synchronous work may not benefit. Preserve resource limits, transaction ordering, and failure/cancellation semantics; observe all callback promises.
| } | ||
|
|
||
| for (const translation of input.result.translations) { | ||
| await contentTranslationEffects( |
There was a problem hiding this comment.
React Doctor · react-doctor/async-await-in-loop (warning)
This for…of loop waits before starting the next iteration. If iterations perform independent asynchronous work, consider bounded concurrency; await syntax alone does not establish a speedup.
Fix → Consider concurrent calls only for independent asynchronous work. Shared queues or synchronous work may not benefit. Preserve resource limits, transaction ordering, and failure/cancellation semantics; observe all callback promises.
| for (let attempt = 1; ; attempt += 1) { | ||
| const slugs: Record<string, string> = {}; | ||
| for (const plan of plans) { | ||
| slugs[plan.field] = await chooseSlug( |
There was a problem hiding this comment.
React Doctor · react-doctor/async-await-in-loop (warning)
This for…of loop waits before starting the next iteration. If iterations perform independent asynchronous work, consider bounded concurrency; await syntax alone does not establish a speedup.
Fix → Consider concurrent calls only for independent asynchronous work. Shared queues or synchronous work may not benefit. Preserve resource limits, transaction ordering, and failure/cancellation semantics; observe all callback promises.
| version, | ||
| }); | ||
|
|
||
| const delivery = await applyDelivery(tx, { |
There was a problem hiding this comment.
React Doctor · react-doctor/server-sequential-independent-await (warning)
This awaited initializer does not read the previous result. If the operations are independent asynchronous work, they may overlap; await syntax alone does not establish a speedup.
Fix → Consider Promise.all([...]) only for independent asynchronous work. Shared queues or synchronous work may not benefit. Preserve resource limits, transaction ordering, and failure/cancellation semantics.
| pluginId, | ||
| }); | ||
|
|
||
| const revalidation = await dispatchContentRevalidation( |
There was a problem hiding this comment.
React Doctor · react-doctor/server-sequential-independent-await (warning)
This awaited initializer does not read the previous result. If the operations are independent asynchronous work, they may overlap; await syntax alone does not establish a speedup.
Fix → Consider Promise.all([...]) only for independent asynchronous work. Shared queues or synchronous work may not benefit. Preserve resource limits, transaction ordering, and failure/cancellation semantics.
| const target = contentApiTarget(definition, pluginId); | ||
| const groups = CONTENT_BULK_GROUPS.map(group => ({ | ||
| ...group, | ||
| actions: group.actions.filter(action => actions.includes(action)), |
There was a problem hiding this comment.
React Doctor · react-doctor/js-set-map-lookups (warning)
This scales poorly because array.includes() inside a loop scans the whole list every time. Use a Set for constant-time lookups.
Fix → Use a Set or Map when you check for the same items over and over. Array.includes/find scans the whole list each time
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32406c4f75
ℹ️ 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".
| .slice(0, Math.max(maxLength - tail.length, 0)) | ||
| .replace(/-+$/, ""); | ||
|
|
||
| return head === "" ? tail.slice(1) : `${head}${tail}`; |
There was a problem hiding this comment.
When a slug has maxLength: 1 and the first nine generated candidates are already occupied, position 10 returns "10" because tail.slice(1) is not capped to maxLength. The duplicate then fails schema validation instead of continuing safely or returning CONTENT_DUPLICATE_SLUG_CONFLICT, even though this is a valid field configuration. Ensure every fallback candidate respects maxLength and discard candidates that cannot fit uniquely.
Useful? React with 👍 / 👎.
2 participants
Adds the definition-level configuration and resolved shapes for record
visibility, duplication and the primary key strategy, the central content
ID helpers, the can_hide permission and the duplicated/hidden/unhidden
event contracts. Behaviour lands in follow-up commits.
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01VpG3D33ZqXEYjN2N6eNcMK