Improve dashboard navigation performance#350
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR migrates the CMS dashboard from client-side to server-side data fetching by introducing workspace-scoped query abstractions, converting dashboard pages to async server components that prefetch data, hydrating client components via props, delegating API routes to new queries, and updating navigation to use layout segments for active state detection. ChangesDashboard Server-Side Rendering Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/cms/src/app/(main)/[workspace]/(dashboard)/authors/page-client.tsx (1)
18-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRethrow fetch failures so query state can fail correctly.
The catch path toasts but returns implicitly, so the query resolves with
undefinedinstead of entering error state. That can hide failures and replace hydrated data with empty state.Proposed fix
queryFn: async () => { try { const response = await fetch("/api/authors"); if (!response.ok) { throw new Error("Failed to fetch authors"); } const data: Author[] = await response.json(); return data; } catch (error) { toast.error( error instanceof Error ? error.message : "Failed to fetch authors" ); + throw error instanceof Error + ? error + : new Error("Failed to fetch authors"); } },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cms/src/app/`(main)/[workspace]/(dashboard)/authors/page-client.tsx around lines 18 - 38, The queryFn for useQuery (in the authors hook that returns { data: authors, isLoading }) currently catches fetch errors, toasts them, and returns implicitly causing useQuery to resolve with undefined; change the catch block in that queryFn to rethrow the error (or return Promise.reject(error)) after calling toast.error so that useQuery enters the error state instead of silently returning undefined—look for the queryFn inside the useQuery call that uses QUERY_KEYS.AUTHORS(workspaceId), workspaceId, isFetchingWorkspace, and initialAuthors and modify the catch to rethrow the caught Error.apps/cms/src/app/(main)/[workspace]/(dashboard)/categories/page-client.tsx (1)
34-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t swallow category fetch errors in
queryFn.This catch block emits a toast but resolves successfully, which bypasses query error state and can render misleading empty data.
Proposed fix
queryFn: async () => { try { const res = await fetch("/api/categories"); if (!res.ok) { throw new Error( `Failed to fetch categories: ${res.status} ${res.statusText}` ); } const data: Category[] = await res.json(); return data; } catch (error) { toast.error( error instanceof Error ? error.message : "Failed to fetch categories" ); + throw error instanceof Error + ? error + : new Error("Failed to fetch categories"); } },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cms/src/app/`(main)/[workspace]/(dashboard)/categories/page-client.tsx around lines 34 - 52, The queryFn for the categories useQuery swallows errors by catching them and only calling toast.error, preventing react-query from entering an error state; update the queryFn (the async function that calls fetch("/api/categories")) to re-throw the caught error (or return Promise.reject(error)) after showing toast.error (or omit the try/catch and let react-query handle it) so that useQuery's error handling and states work correctly instead of returning undefined successful resolution.apps/cms/src/app/(main)/[workspace]/(dashboard)/media/page-client.tsx (1)
34-39:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftScope
initialDatato only the server-matched media query key
PageClientalways passesinitialData: initialMedia(frommedia/page.tsx) while theuseQueryqueryKeychanges with{ page, perPage, search, sort, type }. With TanStack Query, when those key parts change and a new cache entry is created,initialDatais applied to that new key too—so the previous page’s data can be shown whilestaleTime: 5mkeeps it “fresh” longer than expected.Suggested direction
function PageClient({ initialMedia, + initialMediaKey, }: { initialMedia?: MediaPaginatedListResponse; + initialMediaKey?: string; }) { + const currentMediaKey = JSON.stringify({ page, perPage, search, sort, type: normalizedType }); + const shouldUseInitialData = initialMediaKey === currentMediaKey; + const { data, error, isError, isLoading, isFetching } = useQuery({ queryKey: [ ...QUERY_KEYS.MEDIA(workspaceId!), { page, perPage, search, sort, type: normalizedType }, ], @@ - initialData: initialMedia, + initialData: shouldUseInitialData ? initialMedia : undefined,Compute/pass
initialMediaKeyfrommedia/page.tsxusing the same key params that the client puts intoqueryKey(especiallytype: toMediaType(filters.type)andsearchas theuseMediaPageFiltersstring).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cms/src/app/`(main)/[workspace]/(dashboard)/media/page-client.tsx around lines 34 - 39, The initialData (initialMedia) is being passed to useQuery in PageClient without scoping it to the exact server-side query key, causing previous-page data to be reused when client queryKey changes; compute an initialMediaKey in media/page.tsx using the same key parts the client uses (i.e. use the same QUERY_KEYS.MEDIA(...) shape and the same conversions: toMediaType(filters.type) and the string from useMediaPageFilters for search/page/perPage/sort/type), then pass initialData together with that exact key (initialMediaKey) to the client-side useQuery in PageClient so initialData only applies to the matching cache entry rather than every client queryKey variant.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cms/src/app/`(main)/[workspace]/(dashboard)/posts/page-client.tsx:
- Line 76: The useQuery call is seeding new queries with the original
initialPosts because initialData: initialPosts is unconditional; change it to
only supply initialPosts for the initial hydration matching the original filters
(e.g., compare current apiFilters/queryKey to the initialFilters/queryKey
captured at mount or use an isFirstHydration flag) so that when apiFilters
changes the new query does not get the stale initialPosts; update the useQuery
call (the initialData property) to return initialPosts only when the current
queryKey equals the initial queryKey (or when isFirstHydration is true),
otherwise leave initialData undefined and continue relying on placeholderData:
keepPreviousData to preserve UI during transitions.
In `@apps/cms/src/lib/dashboard-prefetch.ts`:
- Around line 69-75: The queryKey for the media prefetch uses
defaultMediaFilters (which contains search: "") but the actual fetch uses
{...defaultMediaFilters, search: null}, causing cache split; update the queryKey
(used with QUERY_KEYS.MEDIA and defaultMediaFilters) to use the exact same
normalized filters as the request (e.g., spread defaultMediaFilters and set
search: null) so the key matches the fetch used by fetchJson/getMediaApiUrl.
In `@apps/cms/src/lib/queries/dashboard.ts`:
- Around line 286-307: The dashboard currently forces requestCount to 0 in the
return mapper, so update getDashboardApiKeys to return the real usage count:
include requestCount in the Prisma/select projection (add requestCount to the
select block) and remove the hardcoded requestCount: 0 in the keys.map so you
map the actual key.requestCount value (while keeping the existing casts for type
and scopes on the keys variable).
In `@apps/cms/src/lib/queries/workspace.ts`:
- Line 213: Replace the direct console.error call used to log "Error fetching
workspace layout data:" with the project's structured logger (e.g., replace
console.error(...) with processLogger.error(...) or the module's
logger.error(...)), pass the error object as a second argument so structured
fields are preserved, and add/import the appropriate logger instance if it's not
available in this module; ensure the log message and error are recorded via the
same logging API used elsewhere in workspace-related functions (match other
usages such as processLogger or logger in this file).
---
Outside diff comments:
In `@apps/cms/src/app/`(main)/[workspace]/(dashboard)/authors/page-client.tsx:
- Around line 18-38: The queryFn for useQuery (in the authors hook that returns
{ data: authors, isLoading }) currently catches fetch errors, toasts them, and
returns implicitly causing useQuery to resolve with undefined; change the catch
block in that queryFn to rethrow the error (or return Promise.reject(error))
after calling toast.error so that useQuery enters the error state instead of
silently returning undefined—look for the queryFn inside the useQuery call that
uses QUERY_KEYS.AUTHORS(workspaceId), workspaceId, isFetchingWorkspace, and
initialAuthors and modify the catch to rethrow the caught Error.
In `@apps/cms/src/app/`(main)/[workspace]/(dashboard)/categories/page-client.tsx:
- Around line 34-52: The queryFn for the categories useQuery swallows errors by
catching them and only calling toast.error, preventing react-query from entering
an error state; update the queryFn (the async function that calls
fetch("/api/categories")) to re-throw the caught error (or return
Promise.reject(error)) after showing toast.error (or omit the try/catch and let
react-query handle it) so that useQuery's error handling and states work
correctly instead of returning undefined successful resolution.
In `@apps/cms/src/app/`(main)/[workspace]/(dashboard)/media/page-client.tsx:
- Around line 34-39: The initialData (initialMedia) is being passed to useQuery
in PageClient without scoping it to the exact server-side query key, causing
previous-page data to be reused when client queryKey changes; compute an
initialMediaKey in media/page.tsx using the same key parts the client uses (i.e.
use the same QUERY_KEYS.MEDIA(...) shape and the same conversions:
toMediaType(filters.type) and the string from useMediaPageFilters for
search/page/perPage/sort/type), then pass initialData together with that exact
key (initialMediaKey) to the client-side useQuery in PageClient so initialData
only applies to the matching cache entry rather than every client queryKey
variant.
🪄 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: fcac1a51-af16-4f37-aa9d-2a08180f344c
📒 Files selected for processing (38)
apps/cms/next.config.tsapps/cms/src/app/(main)/[workspace]/(dashboard)/(home)/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/(home)/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/authors/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/authors/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/categories/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/categories/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/loading.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/media/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/media/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/fields/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/fields/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/keys/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/keys/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/webhooks/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/webhooks/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/tags/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/tags/page.tsxapps/cms/src/app/(main)/[workspace]/layout.tsxapps/cms/src/app/api/authors/route.tsapps/cms/src/app/api/categories/route.tsapps/cms/src/app/api/fields/route.tsapps/cms/src/app/api/keys/route.tsapps/cms/src/app/api/media/route.tsapps/cms/src/app/api/metrics/usage/route.tsapps/cms/src/app/api/posts/route.tsapps/cms/src/app/api/tags/route.tsapps/cms/src/app/api/webhooks/route.tsapps/cms/src/components/nav/app-sidebar.tsxapps/cms/src/components/nav/nav-main.tsxapps/cms/src/components/nav/nav-settings.tsxapps/cms/src/lib/constants.tsapps/cms/src/lib/dashboard-prefetch.tsapps/cms/src/lib/queries/dashboard.tsapps/cms/src/lib/queries/workspace.tsapps/cms/src/lib/search-params.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/cms/src/lib/queries/workspace.ts (1)
213-213:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRoute error logging through the structured logger.
console.erroris used here; please use the project's structured logger and pass the error as a structured field.As per coding guidelines, "Don't use console".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cms/src/lib/queries/workspace.ts` at line 213, Replace the raw console.error call that logs the "Error fetching initial workspace data:" and the error variable with the project's structured logger: import/use the module logger instance (replace console.error) and call its error method, passing a clear message and the error as a structured field (e.g., message plus { error } or error: error.stack) so the error is recorded in the structured logs; update the statement where console.error is used to use the logger instance and include the error object rather than printing to console.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@apps/cms/src/lib/queries/workspace.ts`:
- Line 213: Replace the raw console.error call that logs the "Error fetching
initial workspace data:" and the error variable with the project's structured
logger: import/use the module logger instance (replace console.error) and call
its error method, passing a clear message and the error as a structured field
(e.g., message plus { error } or error: error.stack) so the error is recorded in
the structured logs; update the statement where console.error is used to use the
logger instance and include the error object rather than printing to console.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 58e1e0e0-d40d-4860-ab41-d4a5f3e1408f
📒 Files selected for processing (12)
apps/cms/src/app/(main)/[workspace]/(dashboard)/authors/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/categories/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/media/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/media/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/keys/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/webhooks/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/tags/page-client.tsxapps/cms/src/lib/dashboard-prefetch.tsapps/cms/src/lib/queries/dashboard.tsapps/cms/src/lib/queries/workspace.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page-client.tsx
- apps/cms/src/app/(main)/[workspace]/(dashboard)/categories/page-client.tsx
- apps/cms/src/lib/dashboard-prefetch.ts
- apps/cms/src/app/(main)/[workspace]/(dashboard)/authors/page-client.tsx
- apps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page.tsx
- apps/cms/src/app/(main)/[workspace]/(dashboard)/media/page.tsx
- apps/cms/src/app/(main)/[workspace]/(dashboard)/settings/webhooks/page-client.tsx
- apps/cms/src/lib/queries/dashboard.ts
Replace the single generic table skeleton with skeletons that mirror each route's real layout (home metrics cards, posts and media tables), and trim the shared fallback to the simple bordered-table pages.
Contributor
|
Actionable comments posted: 0 |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
marble-api | 12a73f8 | Commit Preview URL Branch Preview URL |
May 29 2026, 07:28 PM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
marble-jobs | 12a73f8 | Commit Preview URL Branch Preview URL |
May 29 2026, 07:28 PM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
marble-mcp | 12a73f8 | Commit Preview URL Branch Preview URL |
May 29 2026, 07:29 PM |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/cms/src/lib/queries/dashboard/usage.ts (1)
18-156: ⚡ Quick winOptional: the two
Promise.allbatches run sequentially.The second batch (Lines 75-156) doesn't depend on
apiEvents/counts from the first (Lines 18-43); the in-memory bucket build is the only thing between them. Awaiting them as two separate batches adds a DB round-trip of latency. Since this PR targets navigation performance, you could merge both into a singlePromise.alland buildchartBucketsafterward, provided the connection pool comfortably handles the added concurrency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cms/src/lib/queries/dashboard/usage.ts` around lines 18 - 156, The two sequential Promise.all batches cause an extra DB round-trip; merge the first array (producing apiEvents, apiPrevPeriodCount, apiTotalCount) with the second larger array (producing webhookTotal, webhookWeek, webhookDay, webhookTopEndpoint, webhookEvents, mediaTotals, mediaLast30, mediaLastUpload, recentMediaUploads) into a single Promise.all call, then build chartBuckets and compute apiChart/apiLastPeriodCount/apiChange after that single await; locate the arrays passed to Promise.all that populate variables named apiEvents, apiPrevPeriodCount, apiTotalCount and the later variables webhookTotal, webhookWeek, webhookDay, webhookTopEndpoint, webhookEvents, mediaTotals, mediaLast30, mediaLastUpload, recentMediaUploads and combine them into one concurrent request (ensuring your DB connection pool can handle the increased concurrency).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cms/src/lib/queries/dashboard/posts.ts`:
- Around line 86-88: The ordering currently uses a single dynamic key ({
[field]: direction }) in getDashboardPosts which can yield nondeterministic
paging when values tie; update the orderBy in getDashboardPosts to include a
stable secondary key (e.g., include id with the same direction) so it becomes {
[field]: direction, id: direction } to match the approach used in
getDashboardMedia and ensure deterministic pagination.
In `@apps/cms/src/lib/queries/dashboard/settings.ts`:
- Around line 38-49: getDashboardWebhooks currently returns full WebhookEndpoint
records (including the sensitive WebhookEndpoint.secret) to the client; fix by
changing getDashboardWebhooks to explicitly select only the non-sensitive fields
used by the UI (e.g. id, url, name, isActive, createdAt, etc.) instead of
returning the whole record, and ensure any codepaths that currently expect a
Webhook type are updated so the client-side Webhook type does not include secret
(keep secret server-only and only expose it in tightly-scoped create/update
responses or admin endpoints). Locate the function getDashboardWebhooks and the
Webhook/UI type and add an explicit select projection or mapping to omit
WebhookEndpoint.secret, then update associated types and callers (e.g., the API
route GET /api/webhooks and initialWebhooks usage) to match the reduced shape.
In `@apps/cms/src/lib/queries/dashboard/usage.ts`:
- Around line 193-197: The media.totalSize value is incorrectly named because it
sums only the 10-record recentMediaUploads; change the field name to something
explicit (e.g. media.recentUploadsSize) where the usage object is constructed
(replace totalSize: recentMediaUploads.reduce(...) with recentUploadsSize: ...),
update any related type/interface for that usage object, and update the UI
consumer (the MediaUsageCard / media-usage-card component) to read
recentUploadsSize instead of totalSize so the label/prop accurately reflects
that this is the size of recent uploads rather than total storage.
---
Nitpick comments:
In `@apps/cms/src/lib/queries/dashboard/usage.ts`:
- Around line 18-156: The two sequential Promise.all batches cause an extra DB
round-trip; merge the first array (producing apiEvents, apiPrevPeriodCount,
apiTotalCount) with the second larger array (producing webhookTotal,
webhookWeek, webhookDay, webhookTopEndpoint, webhookEvents, mediaTotals,
mediaLast30, mediaLastUpload, recentMediaUploads) into a single Promise.all
call, then build chartBuckets and compute apiChart/apiLastPeriodCount/apiChange
after that single await; locate the arrays passed to Promise.all that populate
variables named apiEvents, apiPrevPeriodCount, apiTotalCount and the later
variables webhookTotal, webhookWeek, webhookDay, webhookTopEndpoint,
webhookEvents, mediaTotals, mediaLast30, mediaLastUpload, recentMediaUploads and
combine them into one concurrent request (ensuring your DB connection pool can
handle the increased concurrency).
🪄 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: 78b92e79-75a7-4dbd-b948-d4409213efed
📒 Files selected for processing (40)
apps/cms/next.config.tsapps/cms/src/app/(main)/[workspace]/(dashboard)/(home)/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/authors/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/categories/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/media/loading.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/media/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/posts/loading.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/fields/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/keys/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/webhooks/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/tags/page.tsxapps/cms/src/app/api/authors/route.tsapps/cms/src/app/api/categories/route.tsapps/cms/src/app/api/fields/route.tsapps/cms/src/app/api/keys/route.tsapps/cms/src/app/api/media/route.tsapps/cms/src/app/api/metrics/usage/route.tsapps/cms/src/app/api/posts/route.tsapps/cms/src/app/api/tags/route.tsapps/cms/src/app/api/webhooks/route.tsapps/cms/src/components/categories/columns.tsxapps/cms/src/components/keys/columns.tsxapps/cms/src/components/layout/wrapper.tsxapps/cms/src/components/nav/nav-main.tsxapps/cms/src/components/nav/nav-settings.tsxapps/cms/src/components/posts/columns.tsxapps/cms/src/components/shared/page-loader.tsxapps/cms/src/components/tags/columns.tsxapps/cms/src/components/ui/activity-indicator.module.cssapps/cms/src/components/ui/activity-indicator.tsxapps/cms/src/lib/queries/dashboard/authors.tsapps/cms/src/lib/queries/dashboard/media.tsapps/cms/src/lib/queries/dashboard/posts.tsapps/cms/src/lib/queries/dashboard/settings.tsapps/cms/src/lib/queries/dashboard/taxonomy.tsapps/cms/src/lib/queries/dashboard/usage.tsapps/cms/src/lib/queries/dashboard/workspace.tsapps/cms/src/types/dashboard.ts
✅ Files skipped from review due to trivial changes (1)
- apps/cms/src/components/ui/activity-indicator.module.css
🚧 Files skipped from review as they are similar to previous changes (16)
- apps/cms/src/app/api/tags/route.ts
- apps/cms/src/app/(main)/[workspace]/(dashboard)/media/loading.tsx
- apps/cms/src/app/api/webhooks/route.ts
- apps/cms/src/app/api/categories/route.ts
- apps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page-client.tsx
- apps/cms/src/app/(main)/[workspace]/(dashboard)/(home)/page.tsx
- apps/cms/src/app/api/metrics/usage/route.ts
- apps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page.tsx
- apps/cms/src/app/(main)/[workspace]/(dashboard)/tags/page.tsx
- apps/cms/src/app/api/fields/route.ts
- apps/cms/src/app/(main)/[workspace]/(dashboard)/settings/keys/page.tsx
- apps/cms/src/app/(main)/[workspace]/(dashboard)/authors/page.tsx
- apps/cms/src/app/api/media/route.ts
- apps/cms/src/app/api/posts/route.ts
- apps/cms/src/app/(main)/[workspace]/(dashboard)/categories/page.tsx
- apps/cms/src/app/(main)/[workspace]/(dashboard)/media/page.tsx
Comment on lines
+38
to
+49
| export async function getDashboardWebhooks( | ||
| workspaceId: string | ||
| ): Promise<Webhook[]> { | ||
| return db.webhookEndpoint.findMany({ | ||
| where: { | ||
| workspaceId, | ||
| }, | ||
| orderBy: { | ||
| createdAt: "desc", | ||
| }, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1) Does the webhookEndpoint model expose a secret/signing field?
rg -nP -C2 'model\s+WebhookEndpoint' --glob '**/*.prisma'
rg -nP -C1 '\b(secret|signingSecret|signing_secret)\b' --glob '**/*.prisma'
# 2) What fields does the Webhook type declare?
fd -t f 'webhook.ts' -p 'types' --exec cat {}
# 3) Is the webhooks result passed straight to a client component?
fd -t f 'page-client.tsx' -p 'settings/webhooks' --exec rg -nP -C2 'initialWebhooks' {}Repository: usemarble/marble
Length of output: 768
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate getDashboardWebhooks usage
rg -n "getDashboardWebhooks\s*\(" -S .
# Find where initialWebhooks is created/passed
rg -n "initialWebhooks" -S apps/cms/src
# Locate the client component mentioned in the original request
fd -t f "page-client.tsx" apps/cms/src --exec echo
# Locate webhook type definition
fd -t f "webhook.ts" . --exec wc -l {} \; --max-depth 6 2>/dev/null || true
fd -t f "*webhook*.*ts" apps/cms/src --max-depth 6 --exec echo {} \;
# If we can find a likely type file, print it (trimmed)
typefile="$(fd -t f "webhook.ts" . | head -n 1 || true)"
if [ -n "${typefile}" ]; then
echo "=== Showing: $typefile ==="
sed -n '1,200p' "$typefile"
fi
# Print the getDashboardWebhooks implementation context
sed -n '1,120p' apps/cms/src/lib/queries/dashboard/settings.ts
# If API route exists, inspect quickly (trimmed)
if [ -f "apps/cms/src/app/api/webhooks/route.ts" ]; then
echo "=== Showing: apps/cms/src/app/api/webhooks/route.ts ==="
sed -n '1,200p' apps/cms/src/app/api/webhooks/route.ts
fiRepository: usemarble/marble
Length of output: 2978
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focused check: confirm prisma schema has secret field and where webhookEndpoint is projected in code.
rg -n "model WebhookEndpoint|secret\s+String" packages/db/prisma/schema.prisma
rg -n "webhookEndpoint\.findMany\(" -S apps/cms/src/lib/queries/dashboard/settings.ts
rg -n "webhookEndpoint" -S apps/cms/src/lib/queries/dashboard/settings.tsRepository: usemarble/marble
Length of output: 197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== apps/cms/src/lib/queries/dashboard/settings.ts (getDashboardWebhooks) ==="
sed -n '1,120p' apps/cms/src/lib/queries/dashboard/settings.ts
echo "=== apps/cms/src/app/api/webhooks/route.ts ==="
sed -n '1,120p' apps/cms/src/app/api/webhooks/route.ts
echo "=== apps/cms/src/app/(main)/[workspace]/(dashboard)/settings/webhooks/page.tsx ==="
sed -n '1,120p' "apps/cms/src/app/(main)/[workspace]/(dashboard)/settings/webhooks/page.tsx"
echo "=== apps/cms/src/app/(main)/[workspace]/(dashboard)/settings/webhooks/page-client.tsx ==="
sed -n '1,220p' "apps/cms/src/app/(main)/[workspace]/(dashboard)/settings/webhooks/page-client.tsx"
echo "=== apps/cms/src/types/webhook.ts ==="
sed -n '1,200p' apps/cms/src/types/webhook.tsRepository: usemarble/marble
Length of output: 8334
Restrict webhook fields: WebhookEndpoint.secret is exposed to the client
packages/db/prisma/schema.prisma defines WebhookEndpoint.secret: String, and apps/cms/src/lib/queries/dashboard/settings.ts#getDashboardWebhooks returns db.webhookEndpoint.findMany(...) without select/omit, so secret is included in:
apps/cms/src/app/api/webhooks/route.ts(GET /api/webhooksreturns it directly)apps/cms/src/app/(main)/[workspace]/(dashboard)/settings/webhooks/page.tsxviainitialWebhooks(andapps/cms/src/types/webhook.tsmodelssecreton theWebhooktype)
Add an explicit select projection (or map/redact) for only the non-sensitive fields needed by the UI, and update the Webhook UI type/client to not include secret (keep secret server-only, or only return it in narrowly authorized create/update flows).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/cms/src/lib/queries/dashboard/settings.ts` around lines 38 - 49,
getDashboardWebhooks currently returns full WebhookEndpoint records (including
the sensitive WebhookEndpoint.secret) to the client; fix by changing
getDashboardWebhooks to explicitly select only the non-sensitive fields used by
the UI (e.g. id, url, name, isActive, createdAt, etc.) instead of returning the
whole record, and ensure any codepaths that currently expect a Webhook type are
updated so the client-side Webhook type does not include secret (keep secret
server-only and only expose it in tightly-scoped create/update responses or
admin endpoints). Locate the function getDashboardWebhooks and the Webhook/UI
type and add an explicit select projection or mapping to omit
WebhookEndpoint.secret, then update associated types and callers (e.g., the API
route GET /api/webhooks and initialWebhooks usage) to match the reduced shape.
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
2 participants
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.
Description
Improves CMS dashboard navigation performance by removing server/client data waterfalls and reducing repeat-navigation loading states.
Motivation and Context
Dashboard navigation was slow because page transitions mounted client pages that then fetched data through internal API routes. Repeated back-and-forth navigation could also show the route-level skeleton even when React Query already had data, because the dynamic RSC payload was refetched before client cache mounted.
This change follows the Next.js best-practice guidance to fetch internal reads in Server Components, avoid waterfalls, prefetch likely navigation targets, and keep route loading states stable.
How to Test
Checks run:
Targeted TypeScript checks for the changed files showed no new errors. Full
tsc --noEmitis still blocked by the existing Zod/react-hook-form resolver version mismatch in unrelated files.Screenshots (if applicable)
N/A
Video Demo (if applicable)
N/A
Types of Changes
Summary by CodeRabbit
Release Notes
New Features
Performance
Bug Fixes