CMS-131: finish content overview integration#28
Merged
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a metadata-only content overview endpoint ( Changes
Sequence Diagram(s)sequenceDiagram
participant Studio as Studio UI
participant ClientApi as Studio Overview API
participant Server as Content API Server
participant Auth as Authorization
participant Store as Content Store
participant DB as Database
Studio->>ClientApi: request overview for types[]
ClientApi->>Server: GET /api/v1/content/overview?type=...
Server->>Auth: authorize requiredScope: content:read
Auth-->>Server: authorized
Server->>Store: getOverviewCounts(scope, { types })
Store->>DB: query documents grouped by schemaType (exclude deleted, filter scope)
DB-->>Store: grouped counts (total, published, drafts)
Store-->>Server: per-type counts (ordered)
Server-->>ClientApi: 200 { data: [...] }
ClientApi-->>Studio: return counts
Studio->>Studio: load state, render overview UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
packages/studio/src/lib/content-list-api.test.ts (1)
119-142: LGTM with optional enhancement.The test correctly verifies that
draft: trueis serialized as"true"in the query string, following the existing test patterns.Consider adding a test case for
draft: falseto ensure explicit false values are also correctly serialized (since the implementation uses!== undefinedto distinguish between omitted and explicitly-false values):await api.list({ type: "BlogPost", draft: false, limit: 1 }); const url = new URL(String(calls[0]?.input)); assert.equal(url.searchParams.get("draft"), "false");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/studio/src/lib/content-list-api.test.ts` around lines 119 - 142, Add a test case to verify that an explicit false draft value is serialized as "false" in the query string: in the same test file use createApi and call api.list with { type: "BlogPost", draft: false, limit: 1 } (similar to the existing test that uses draft: true), capture the fetcher call (calls array) and assert that new URL(String(calls[0]?.input)).searchParams.get("draft") === "false"; this ensures the api.list serialization logic (the code path around draft handling) correctly distinguishes explicit false from undefined.apps/server/src/lib/content-api/in-memory-store.ts (1)
476-500: Consider single-pass aggregation to reduce repeated filtering cost.The current implementation repeatedly scans
scopedDocumentsper type and per status. A one-pass map keeps behavior identical with better scaling.♻️ Suggested refactor
async getOverviewCounts(scope, input) { const requestedTypes = [ ...new Set(input.types.map((type) => type.trim())), ]; - const scopedDocuments = [...getScopeStore(scope).values()].filter( - (document) => !document.isDeleted, - ); - - return requestedTypes.map((type) => { - const matchingDocuments = scopedDocuments.filter( - (document) => document.type === type, - ); - - return { - type, - total: matchingDocuments.length, - published: matchingDocuments.filter( - (document) => document.publishedVersion !== null, - ).length, - drafts: matchingDocuments.filter( - (document) => document.publishedVersion === null, - ).length, - }; - }); + const countsByType = new Map< + string, + { total: number; published: number; drafts: number } + >(); + + for (const document of getScopeStore(scope).values()) { + if (document.isDeleted) { + continue; + } + + const counts = countsByType.get(document.type) ?? { + total: 0, + published: 0, + drafts: 0, + }; + counts.total += 1; + if (document.publishedVersion === null) { + counts.drafts += 1; + } else { + counts.published += 1; + } + countsByType.set(document.type, counts); + } + + return requestedTypes.map((type) => { + const counts = countsByType.get(type); + return { + type, + total: counts?.total ?? 0, + published: counts?.published ?? 0, + drafts: counts?.drafts ?? 0, + }; + }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/lib/content-api/in-memory-store.ts` around lines 476 - 500, The getOverviewCounts function currently filters scopedDocuments multiple times per type; refactor to a single-pass aggregation by iterating once over scopedDocuments and updating a map keyed by document.type that tracks total, published, and drafts counts. Use the existing requestedTypes array to ensure only requested types are reported (initialize zero counts for missing types), then transform the map entries into the same return shape. Update references to getScopeStore(scope), requestedTypes, and scopedDocuments in this function to implement the one-pass counting logic.apps/server/src/lib/content-api/types.ts (1)
123-128: Prefer aliasing the shared overview-count contract to avoid drift.
ContentOverviewCountscurrently mirrors the shared response shape. Reusing the shared type would reduce maintenance risk.♻️ Suggested change
import type { ContentDocumentResponse, + ContentOverviewCountsResponse, ContentVersionDocumentResponse, ContentVersionSummaryResponse, SchemaRegistryTypeSnapshot, } from "@mdcms/shared"; @@ -export type ContentOverviewCounts = { - type: string; - total: number; - published: number; - drafts: number; -}; +export type ContentOverviewCounts = ContentOverviewCountsResponse;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/lib/content-api/types.ts` around lines 123 - 128, ContentOverviewCounts currently duplicates the shared response shape; replace the inline definition by importing and aliasing the shared overview-count type (e.g., OverviewCounts) from the shared contract module and export it as ContentOverviewCounts so the service reuses the single source of truth; update the import to reference the shared package and remove the local type literal in apps/server/src/lib/content-api/types.ts, keeping the exported symbol name ContentOverviewCounts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/studio/package.json`:
- Line 107: Update lucide-react icon imports to the v1.x naming (remove the
"Icon" suffix): in select.tsx replace CheckIcon → Check, ChevronDownIcon →
ChevronDown, ChevronUpIcon → ChevronUp; in dialog.tsx replace XIcon → X; in
checkbox.tsx replace CheckIcon → Check; and in dropdown-menu.tsx replace
CheckIcon → Check, ChevronRightIcon → ChevronRight, CircleIcon → Circle; keep
the existing LucideIcon type import in coming-soon.tsx as-is.
In `@packages/studio/src/lib/content-overview-api.ts`:
- Around line 100-107: The current code block treats a malformed 200 response
(when !isRecord(payload) or payload.data is not an array) as valid and returns
zero counts for each type; instead, update the logic in the function that
processes the response (the block using isRecord, payload, and input.types) to
throw or return an explicit error when the payload shape is invalid so callers
can surface a deterministic error state; locate the check that currently returns
input.types.map(...) and replace it with an error throw or rejected Promise that
includes context (e.g., indicate unexpected payload shape and include the raw
payload) so backend contract drift is not silently masked.
In `@packages/studio/src/lib/content-overview-state.ts`:
- Around line 295-299: In the content.readDraft fallback, the call to
input.contentApi.list({ type: input.type, published: true, limit: 1 }) drops
draft mode and can 403 for principals without content.read; change the request
inside the content.readDraft path (where readDraft is invoked) to include draft:
true (e.g., input.contentApi.list with draft: true instead of published: true)
so the draft-only fallback remains in draft mode and avoids publishing-only
permission checks.
In `@packages/studio/src/lib/runtime-ui/adapters/next-themes.tsx`:
- Around line 189-195: The effect is persisting the resolved theme on first
mount (overwriting fallback behavior); update the effect that calls
persistStoredThemePreference so it skips the initial run (e.g., use a mounted
useRef or check for an existing stored preference) and only writes to
window.localStorage on subsequent theme changes (so reference
persistStoredThemePreference, the theme variable, and the useEffect surrounding
them to implement this change).
In `@packages/studio/src/lib/runtime-ui/pages/content-page.tsx`:
- Around line 129-133: Replace the hard-coded "/admin" segment when building the
Link around card so navigation uses the configured Studio mount path: instead of
href={`/admin/content/${entry.type}`} build the href from the Studio base path
configuration (use the runtime config/provider that exposes the mount/base path
used by the app) and append `/content/${entry.type}`; update the Link
construction (around the existing Link, entry.type and card symbols) to derive
the base path from the studio runtime config/provider rather than using the
literal "/admin".
In `@packages/studio/src/lib/studio-loader.ts`:
- Around line 243-250: The try/catch around parseMdcmsConfig currently swallows
all errors; change the catch to capture the error (e.g., catch (err)) and only
swallow known/expected config validation failures (for example check instanceof
ConfigValidationError or err.name === 'ValidationError' / expected error
sentinel) and set supportedLocales = undefined in that case, but rethrow any
other unexpected errors so they surface; keep references to parseMdcmsConfig and
the supportedLocales assignment so you update that exact block.
---
Nitpick comments:
In `@apps/server/src/lib/content-api/in-memory-store.ts`:
- Around line 476-500: The getOverviewCounts function currently filters
scopedDocuments multiple times per type; refactor to a single-pass aggregation
by iterating once over scopedDocuments and updating a map keyed by document.type
that tracks total, published, and drafts counts. Use the existing requestedTypes
array to ensure only requested types are reported (initialize zero counts for
missing types), then transform the map entries into the same return shape.
Update references to getScopeStore(scope), requestedTypes, and scopedDocuments
in this function to implement the one-pass counting logic.
In `@apps/server/src/lib/content-api/types.ts`:
- Around line 123-128: ContentOverviewCounts currently duplicates the shared
response shape; replace the inline definition by importing and aliasing the
shared overview-count type (e.g., OverviewCounts) from the shared contract
module and export it as ContentOverviewCounts so the service reuses the single
source of truth; update the import to reference the shared package and remove
the local type literal in apps/server/src/lib/content-api/types.ts, keeping the
exported symbol name ContentOverviewCounts.
In `@packages/studio/src/lib/content-list-api.test.ts`:
- Around line 119-142: Add a test case to verify that an explicit false draft
value is serialized as "false" in the query string: in the same test file use
createApi and call api.list with { type: "BlogPost", draft: false, limit: 1 }
(similar to the existing test that uses draft: true), capture the fetcher call
(calls array) and assert that new
URL(String(calls[0]?.input)).searchParams.get("draft") === "false"; this ensures
the api.list serialization logic (the code path around draft handling) correctly
distinguishes explicit false from undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 082f9cbb-9998-4b74-ab54-7c9f7d667911
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
apps/server/README.mdapps/server/src/bin/demo-seed-api-key.tsapps/server/src/lib/content-api.integration.test.tsapps/server/src/lib/content-api.test.tsapps/server/src/lib/content-api/database-store.tsapps/server/src/lib/content-api/in-memory-store.tsapps/server/src/lib/content-api/routes.tsapps/server/src/lib/content-api/types.tsapps/server/src/lib/demo-seed.test.tsapps/studio-example/README.mdapps/studio-example/mdcms.config.test.tsapps/studio-example/mdcms.config.tsdocs/specs/SPEC-003-content-storage-versioning-and-migrations.mddocs/specs/SPEC-006-studio-runtime-and-ui.mdpackages/shared/src/lib/contracts/content-api.tspackages/shared/src/lib/contracts/extensibility.test.tspackages/shared/src/lib/contracts/extensibility.tspackages/studio/package.jsonpackages/studio/src/lib/content-list-api.test.tspackages/studio/src/lib/content-list-api.tspackages/studio/src/lib/content-overview-api.test.tspackages/studio/src/lib/content-overview-api.tspackages/studio/src/lib/content-overview-state.test.tspackages/studio/src/lib/content-overview-state.tspackages/studio/src/lib/remote-studio-app.tsxpackages/studio/src/lib/runtime-ui/adapters/next-themes.test.tspackages/studio/src/lib/runtime-ui/adapters/next-themes.tsxpackages/studio/src/lib/runtime-ui/pages/content-page.test.tsxpackages/studio/src/lib/runtime-ui/pages/content-page.tsxpackages/studio/src/lib/studio-loader.test.tspackages/studio/src/lib/studio-loader.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/studio/src/lib/runtime-ui/adapters/next-themes.tsx (2)
89-112: UnusedsystemPrefersDarkparameter in function signature.The
systemPrefersDarkfield is declared in the input type but never referenced in the function body. This appears to be leftover from a refactor sinceresolveThemePreferencereturnsTheme(which includes"system"), not the resolvedAppliedTheme.♻️ Suggested cleanup
export function resolveThemePreference(input: { storedTheme: Theme | null; defaultTheme: Theme | undefined; enableSystem: boolean | undefined; - systemPrefersDark: boolean; }): Theme {Then update the call site in
resolveInitialTheme(lines 133-138) to remove thesystemPrefersDarkargument.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/studio/src/lib/runtime-ui/adapters/next-themes.tsx` around lines 89 - 112, The input.type includes an unused systemPrefersDark field and the resolveThemePreference function never uses it; remove systemPrefersDark from the input parameter type and the function signature for resolveThemePreference, and update the call site in resolveInitialTheme to stop passing systemPrefersDark (remove that argument) so the types and calls stay consistent.
15-20:disableTransitionOnChangeprop is declared but not implemented.The prop type includes
disableTransitionOnChange(line 19) but the component never destructures or uses it (lines 141-146). Remove it to avoid suggesting unsupported functionality to consumers, or add a// TODOcomment if planned for future implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/studio/src/lib/runtime-ui/adapters/next-themes.tsx` around lines 15 - 20, The ThemeProviderProps type declares disableTransitionOnChange but the ThemeProvider component never destructures or implements it; either remove disableTransitionOnChange from the ThemeProviderProps type so the prop is not advertised, or keep it and add a clear TODO comment inside the ThemeProvider component (near its props destructuring/handling, e.g., where ThemeProviderProps is used) indicating planned support and that the prop is currently a no-op; if removing, also update any places that import or pass disableTransitionOnChange so types and callers stay consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/studio/src/lib/runtime-ui/adapters/next-themes.tsx`:
- Around line 89-112: The input.type includes an unused systemPrefersDark field
and the resolveThemePreference function never uses it; remove systemPrefersDark
from the input parameter type and the function signature for
resolveThemePreference, and update the call site in resolveInitialTheme to stop
passing systemPrefersDark (remove that argument) so the types and calls stay
consistent.
- Around line 15-20: The ThemeProviderProps type declares
disableTransitionOnChange but the ThemeProvider component never destructures or
implements it; either remove disableTransitionOnChange from the
ThemeProviderProps type so the prop is not advertised, or keep it and add a
clear TODO comment inside the ThemeProvider component (near its props
destructuring/handling, e.g., where ThemeProviderProps is used) indicating
planned support and that the prop is currently a no-op; if removing, also update
any places that import or pass disableTransitionOnChange so types and callers
stay consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd37ef1e-780b-4939-bdc2-5f21e0339eee
📒 Files selected for processing (17)
apps/server/src/lib/content-api/in-memory-store.tsapps/server/src/lib/content-api/types.tspackages/studio/src/lib/content-list-api.test.tspackages/studio/src/lib/content-overview-api.test.tspackages/studio/src/lib/content-overview-api.tspackages/studio/src/lib/content-overview-state.test.tspackages/studio/src/lib/content-overview-state.tspackages/studio/src/lib/runtime-ui/adapters/next-themes.test.tspackages/studio/src/lib/runtime-ui/adapters/next-themes.tsxpackages/studio/src/lib/runtime-ui/components/ui/calendar.tsxpackages/studio/src/lib/runtime-ui/components/ui/checkbox.tsxpackages/studio/src/lib/runtime-ui/components/ui/dialog.tsxpackages/studio/src/lib/runtime-ui/components/ui/dropdown-menu.tsxpackages/studio/src/lib/runtime-ui/components/ui/pagination.tsxpackages/studio/src/lib/runtime-ui/components/ui/select.tsxpackages/studio/src/lib/studio-loader.test.tspackages/studio/src/lib/studio-loader.ts
✅ Files skipped from review due to trivial changes (7)
- packages/studio/src/lib/runtime-ui/components/ui/dialog.tsx
- packages/studio/src/lib/runtime-ui/components/ui/dropdown-menu.tsx
- packages/studio/src/lib/runtime-ui/components/ui/select.tsx
- packages/studio/src/lib/runtime-ui/components/ui/checkbox.tsx
- packages/studio/src/lib/runtime-ui/components/ui/pagination.tsx
- packages/studio/src/lib/runtime-ui/components/ui/calendar.tsx
- packages/studio/src/lib/content-overview-state.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/studio/src/lib/content-list-api.test.ts
- packages/studio/src/lib/studio-loader.ts
- apps/server/src/lib/content-api/in-memory-store.ts
- packages/studio/src/lib/runtime-ui/adapters/next-themes.test.ts
- packages/studio/src/lib/content-overview-api.test.ts
- packages/studio/src/lib/content-overview-api.ts
- packages/studio/src/lib/studio-loader.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/studio/src/lib/runtime-ui/pages/content-page.tsx (1)
312-319: Missing explicitsupportedLocalesdependency in effect array.The
createContentPageLoadInputfunction usescontext.documentRoute?.supportedLocalesat line 168, but this value is not included in the dependency array at lines 312–319. WhilesupportedLocalesis implicitly covered by theprojectandenvironmentdependencies (since it's computed as part of the context setup), adding it explicitly improves clarity and follows React's dependency rules.Suggested fix
}, [ context.apiBaseUrl, context.auth.mode, context.auth.mode === "token" ? context.auth.token : undefined, context.documentRoute?.environment, context.documentRoute?.project, + context.documentRoute?.supportedLocales, loadState, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/studio/src/lib/runtime-ui/pages/content-page.tsx` around lines 312 - 319, The effect's dependency array is missing context.documentRoute?.supportedLocales which is used inside createContentPageLoadInput; update the dependency array in the useEffect that invokes createContentPageLoadInput to include context.documentRoute?.supportedLocales so React correctly re-runs the effect when supportedLocales changes (refer to createContentPageLoadInput and the useEffect dependency array currently listing context.documentRoute?.environment and context.documentRoute?.project).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/studio/src/lib/runtime-ui/pages/content-page.tsx`:
- Around line 312-319: The effect's dependency array is missing
context.documentRoute?.supportedLocales which is used inside
createContentPageLoadInput; update the dependency array in the useEffect that
invokes createContentPageLoadInput to include
context.documentRoute?.supportedLocales so React correctly re-runs the effect
when supportedLocales changes (refer to createContentPageLoadInput and the
useEffect dependency array currently listing context.documentRoute?.environment
and context.documentRoute?.project).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4eb87f9-3d61-4b7f-9667-41206379ff29
📒 Files selected for processing (2)
packages/studio/src/lib/runtime-ui/pages/content-page.test.tsxpackages/studio/src/lib/runtime-ui/pages/content-page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/studio/src/lib/runtime-ui/pages/content-page.test.tsx
This was referenced Apr 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/admin/contentmock behavior with live schema-driven overview state and a dedicated metadata-only/api/v1/content/overviewcount contracttotal,published,drafts, and locale badges, plus localized demo content to exercise that pathVerification
bun test apps/server/src/lib/content-api.test.ts packages/studio/src/lib/content-overview-api.test.ts packages/studio/src/lib/content-overview-state.test.ts packages/studio/src/lib/runtime-ui/pages/content-page.test.tsx packages/studio/src/lib/runtime-ui/adapters/next-themes.test.ts packages/studio/src/lib/studio-loader.test.ts packages/shared/src/lib/contracts/extensibility.test.ts packages/studio/src/lib/content-list-api.test.tsbun run format:checkbun run checkNotes
docker-compose.dev.ymland local-only planning files is intentionally left out of the PRSummary by CodeRabbit
New Features
Studio
Demo Content
Theme
Tests
Documentation
Style