fix: properly use app router layouts & loading boundaries#313
Open
fix: properly use app router layouts & loading boundaries#313
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a new (authed) route layout that validates cached sessions and redirects unauthenticated users to /login, removes legacy middleware and server-user helpers, consolidates pages under the (authed) group, adds route-specific loading components, and updates navigation and several UI/error messages. Changes
Sequence DiagramsequenceDiagram
participant Client
participant AuthedLayout as AuthedLayout
participant SessionUtil as getCachedSession
participant AuthProvider as AuthedPageProvider
participant Page as PageComponent
Client->>AuthedLayout: Request /(authed)/* route
AuthedLayout->>SessionUtil: call getCachedSession()
SessionUtil->>SessionUtil: React cache lookup / fetch from cookies
alt cache hit
SessionUtil-->>AuthedLayout: return cached session
else cache miss
SessionUtil-->>AuthedLayout: fetch session, cache result
end
alt session contains user
AuthedLayout->>AuthProvider: initialize with { user }
AuthProvider->>Page: render child page with context
Page-->>Client: respond with authenticated UI
else no user
AuthedLayout-->>Client: redirect to /login
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend-v2/src/app/`(authed)/coaching/[id]/page.tsx:
- Around line 5-22: The EditCoachingPage currently renders EventForm without
prefetched data causing EventForm's guard (the check that throws when eventId
exists but eventData is undefined in event-form.tsx) to error; fix by
prefetching the event within EditCoachingPage (resolve params to eventId, fetch
the event record, and provide it to EventForm via React Query hydration or as a
prop) OR remove/relax the strict guard inside EventForm so it can load
client-side when only eventId is present; update the EditCoachingPage export
(function EditCoachingPage) to perform the server fetch and pass the prefetched
data/hydration to EventForm, or modify EventForm’s validation logic to allow
async loading when eventData is not yet available.
In `@frontend-v2/src/app/`(authed)/coaching/page.tsx:
- Around line 4-11: The CoachingPage function is declared async but contains no
awaits; remove the unnecessary async keyword by changing the declaration from
"export default async function CoachingPage()" to "export default function
CoachingPage()", keeping the JSX return exactly the same (ContentWrapper, h1,
EventForm) so behavior is unchanged; verify there are no awaiting calls or
reliance on async behavior inside CoachingPage or from EventForm before
committing.
In `@frontend-v2/src/app/`(authed)/loading.tsx:
- Around line 4-9: Extract a reusable shell component (e.g., AuthedLoadingShell)
to encapsulate the repeated ContentWrapper + optional heading + inline Loading
UI, then replace the current AuthedLoading component with a simple call to that
shell. Specifically, create a new component (AuthedLoadingShell) that accepts
props for size, optional heading/children, and renders <ContentWrapper
size={...} className="gap-6"> with <Loading inline /> (and optional heading
slot), then update AuthedLoading to return <AuthedLoadingShell size="xl" /> (or
include heading if needed) and update other identical loading files to use the
same AuthedLoadingShell to remove duplication and ensure consistent
spacing/typography.
In `@frontend-v2/src/app/`(authed)/users/[id]/loading.tsx:
- Line 7: The heading in the loading UI is too generic ("User"); update the
route-specific heading to reflect the edit flow by changing the h1 text in the
[id]/loading.tsx component to something like "Edit user" (or localised
equivalent) so users see "Edit user" while the edit route is loading; locate the
h1 element rendered in the loading component and replace its inner text from
"User" to "Edit user".
In `@frontend-v2/src/app/`(authed)/users/new/loading.tsx:
- Line 7: Change the ambiguous heading text in the loading UI from "User" to an
explicit create-flow label such as "New user" so users know they are in the
create route; locate the heading JSX in loading.tsx (the <h1 className="text-2xl
font-semibold"> element) and update its inner text accordingly.
In `@frontend-v2/src/components/nav.tsx`:
- Around line 100-105: Normalize v2 paths explicitly: replace the loose
startsWith('/v2') logic with checks for exact '/v2' and '/v2/' prefix so you
don't produce an empty navPath or accidentally match '/v20'. Specifically, in
the navPath computation that uses innerItem.href and the isActive check that
compares to pathname, handle three cases: if innerItem.href === '/v2' map
navPath to '/' (or whatever root you want), if innerItem.href startsWith('/v2/')
strip the leading '/v2' into the intended subpath, otherwise leave navPath null;
then use that sanitized navPath in isActive (pathname === navPath ||
pathname.startsWith(navPath + '/')). Apply the same change to the other
occurrence of this logic around the later block that computes href/active state.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (55)
frontend-v2/src/app/(authed)/activists/activist-filters.tsxfrontend-v2/src/app/(authed)/activists/activists-page.tsxfrontend-v2/src/app/(authed)/activists/activists-table.tsxfrontend-v2/src/app/(authed)/activists/column-definitions.tsfrontend-v2/src/app/(authed)/activists/column-selector.tsxfrontend-v2/src/app/(authed)/activists/last-event-filter.tsxfrontend-v2/src/app/(authed)/activists/loading.tsxfrontend-v2/src/app/(authed)/activists/page.tsxfrontend-v2/src/app/(authed)/activists/query-utils.tsfrontend-v2/src/app/(authed)/activists/sort-selector.tsxfrontend-v2/src/app/(authed)/coaching/[id]/page.tsxfrontend-v2/src/app/(authed)/coaching/loading.tsxfrontend-v2/src/app/(authed)/coaching/page.tsxfrontend-v2/src/app/(authed)/event/[id]/page.tsxfrontend-v2/src/app/(authed)/event/activist-registry.tsfrontend-v2/src/app/(authed)/event/activist-storage.tsfrontend-v2/src/app/(authed)/event/attendee-input-field.tsxfrontend-v2/src/app/(authed)/event/event-form.tsxfrontend-v2/src/app/(authed)/event/loading.tsxfrontend-v2/src/app/(authed)/event/page.tsxfrontend-v2/src/app/(authed)/event/useActivistRegistry.tsfrontend-v2/src/app/(authed)/interest/generator/generator-form.tsxfrontend-v2/src/app/(authed)/interest/generator/loading.tsxfrontend-v2/src/app/(authed)/interest/generator/page.tsxfrontend-v2/src/app/(authed)/layout.tsxfrontend-v2/src/app/(authed)/loading.tsxfrontend-v2/src/app/(authed)/users/[id]/loading.tsxfrontend-v2/src/app/(authed)/users/[id]/page.tsxfrontend-v2/src/app/(authed)/users/loading.tsxfrontend-v2/src/app/(authed)/users/new/loading.tsxfrontend-v2/src/app/(authed)/users/new/page.tsxfrontend-v2/src/app/(authed)/users/page.tsxfrontend-v2/src/app/(authed)/users/user-form.tsxfrontend-v2/src/app/(authed)/users/user-table.tsxfrontend-v2/src/app/(authed)/users/users-page.tsxfrontend-v2/src/app/activists/page.tsxfrontend-v2/src/app/authed-page-layout.tsxfrontend-v2/src/app/authed-page-provider.tsxfrontend-v2/src/app/coaching/[id]/page.tsxfrontend-v2/src/app/coaching/page.tsxfrontend-v2/src/app/event/[id]/page.tsxfrontend-v2/src/app/event/page.tsxfrontend-v2/src/app/example-server-component/activists-section.tsxfrontend-v2/src/app/example-server-component/activists.tsxfrontend-v2/src/app/example-server-component/page.tsxfrontend-v2/src/app/interest/generator/page.tsxfrontend-v2/src/app/session.tsfrontend-v2/src/app/users/[id]/page.tsxfrontend-v2/src/app/users/loading.tsxfrontend-v2/src/app/users/new/page.tsxfrontend-v2/src/app/users/page.tsxfrontend-v2/src/components/nav.tsxfrontend-v2/src/lib/server-user.tsfrontend-v2/src/middleware.tsshared/nav.json
💤 Files with no reviewable changes (18)
- frontend-v2/src/app/coaching/page.tsx
- frontend-v2/src/app/authed-page-provider.tsx
- frontend-v2/src/app/interest/generator/page.tsx
- frontend-v2/src/app/example-server-component/activists-section.tsx
- frontend-v2/src/app/event/[id]/page.tsx
- frontend-v2/src/app/users/new/page.tsx
- frontend-v2/src/lib/server-user.ts
- frontend-v2/src/app/event/page.tsx
- frontend-v2/src/middleware.ts
- frontend-v2/src/app/example-server-component/activists.tsx
- frontend-v2/src/app/coaching/[id]/page.tsx
- shared/nav.json
- frontend-v2/src/app/authed-page-layout.tsx
- frontend-v2/src/app/users/[id]/page.tsx
- frontend-v2/src/app/users/page.tsx
- frontend-v2/src/app/users/loading.tsx
- frontend-v2/src/app/activists/page.tsx
- frontend-v2/src/app/example-server-component/page.tsx
There was a problem hiding this comment.
Pull request overview
This PR restructures the Next.js App Router setup to rely on segment layouts and route-level loading.tsx boundaries for authenticated areas, removing the prior middleware/header-based approach and migrating several pages into an (authed) route group.
Changes:
- Replace edge middleware +
x-user-dataheader auth with an(authed)/layout.tsxthat loads the session server-side and redirects to/login. - Move authenticated routes (users, activists, event/coaching, interest generator) under
app/(authed)/...and add granularloading.tsxboundaries. - Introduce/expand client-side pages for Users/Activists and add the Event form + activist registry (with IndexedDB caching utilities).
Reviewed changes
Copilot reviewed 38 out of 55 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| shared/nav.json | Removes the “Test Page” nav item. |
| frontend-v2/src/middleware.ts | Removes edge middleware previously responsible for auth + user header injection. |
| frontend-v2/src/lib/server-user.ts | Removes server-user header parsing helper (no longer needed without middleware). |
| frontend-v2/src/components/nav.tsx | Switches active-nav logic to pathname-based detection. |
| frontend-v2/src/app/users/page.tsx | Removes legacy users route (migrated under (authed)). |
| frontend-v2/src/app/users/new/page.tsx | Removes legacy new-user route (migrated under (authed)). |
| frontend-v2/src/app/users/loading.tsx | Removes legacy users segment loading boundary. |
| frontend-v2/src/app/users/[id]/page.tsx | Removes legacy edit-user route (migrated under (authed)). |
| frontend-v2/src/app/session.ts | Adds getCachedSession() for server-side session retrieval via React cache(). |
| frontend-v2/src/app/interest/generator/page.tsx | Removes legacy interest generator route (migrated under (authed)). |
| frontend-v2/src/app/example-server-component/page.tsx | Removes example server-component page. |
| frontend-v2/src/app/example-server-component/activists.tsx | Removes example client component used by the deleted example page. |
| frontend-v2/src/app/example-server-component/activists-section.tsx | Removes example SSR-prefetch wrapper used by the deleted example page. |
| frontend-v2/src/app/event/page.tsx | Removes legacy event route (migrated under (authed)). |
| frontend-v2/src/app/event/[id]/page.tsx | Removes legacy edit-event route (migrated under (authed)). |
| frontend-v2/src/app/coaching/page.tsx | Removes legacy coaching route (migrated under (authed)). |
| frontend-v2/src/app/coaching/[id]/page.tsx | Removes legacy edit-coaching route (migrated under (authed)). |
| frontend-v2/src/app/authed-page-provider.tsx | Removes pageName from the authed page context shape. |
| frontend-v2/src/app/authed-page-layout.tsx | Removes old per-page “authed layout” wrapper (replaced by (authed)/layout.tsx). |
| frontend-v2/src/app/activists/page.tsx | Removes legacy activists route (migrated under (authed)). |
| frontend-v2/src/app/(authed)/users/users-page.tsx | Adds client-side users list UI and fetching. |
| frontend-v2/src/app/(authed)/users/user-table.tsx | Adds responsive users table (desktop table + mobile cards). |
| frontend-v2/src/app/(authed)/users/user-form.tsx | Adds client-side create/edit user form with validation + mutation. |
| frontend-v2/src/app/(authed)/users/page.tsx | Adds server page wrapper for users list under (authed). |
| frontend-v2/src/app/(authed)/users/new/page.tsx | Adds server page wrapper for create-user under (authed). |
| frontend-v2/src/app/(authed)/users/new/loading.tsx | Adds loading boundary for /users/new. |
| frontend-v2/src/app/(authed)/users/loading.tsx | Adds loading boundary for /users. |
| frontend-v2/src/app/(authed)/users/[id]/page.tsx | Adds server page wrapper for edit-user under (authed). |
| frontend-v2/src/app/(authed)/users/[id]/loading.tsx | Adds loading boundary for /users/[id]. |
| frontend-v2/src/app/(authed)/loading.tsx | Adds loading boundary for the (authed) group. |
| frontend-v2/src/app/(authed)/layout.tsx | Introduces authenticated layout: loads session and renders navbar + provider. |
| frontend-v2/src/app/(authed)/interest/generator/page.tsx | Adds interest generator page under (authed). |
| frontend-v2/src/app/(authed)/interest/generator/loading.tsx | Adds loading boundary for interest generator. |
| frontend-v2/src/app/(authed)/interest/generator/generator-form.tsx | Adds client-side interest URL generator form under (authed). |
| frontend-v2/src/app/(authed)/event/useActivistRegistry.ts | Adds hook to manage cached activist registry + background sync. |
| frontend-v2/src/app/(authed)/event/page.tsx | Adds event page under (authed). |
| frontend-v2/src/app/(authed)/event/loading.tsx | Adds loading boundary for event page. |
| frontend-v2/src/app/(authed)/event/event-form.tsx | Adds new event/coaching form implementation using TanStack Form + registry. |
| frontend-v2/src/app/(authed)/event/attendee-input-field.tsx | Adds attendee input with suggestions popover and status indicators. |
| frontend-v2/src/app/(authed)/event/activist-storage.ts | Adds IndexedDB storage implementation for activist caching + metadata. |
| frontend-v2/src/app/(authed)/event/activist-registry.ts | Adds in-memory registry with write-through persistence and suggestions. |
| frontend-v2/src/app/(authed)/event/[id]/page.tsx | Adds edit-event route under (authed). |
| frontend-v2/src/app/(authed)/coaching/page.tsx | Adds coaching page under (authed) using shared EventForm. |
| frontend-v2/src/app/(authed)/coaching/loading.tsx | Adds loading boundary for coaching page. |
| frontend-v2/src/app/(authed)/coaching/[id]/page.tsx | Adds edit-coaching route under (authed). |
| frontend-v2/src/app/(authed)/activists/sort-selector.tsx | Adds sort selector UI for activists list. |
| frontend-v2/src/app/(authed)/activists/query-utils.ts | Adds URL param parsing + query option builders for activists search. |
| frontend-v2/src/app/(authed)/activists/page.tsx | Adds activists page wrapper under (authed). |
| frontend-v2/src/app/(authed)/activists/loading.tsx | Adds loading boundary for activists page. |
| frontend-v2/src/app/(authed)/activists/last-event-filter.tsx | Adds last-event date range filter control. |
| frontend-v2/src/app/(authed)/activists/column-selector.tsx | Adds column picker UI for activists table. |
| frontend-v2/src/app/(authed)/activists/column-definitions.ts | Defines activists columns, categories, defaults, normalization helpers. |
| frontend-v2/src/app/(authed)/activists/activists-table.tsx | Adds responsive activists table rendering with sort UI integration. |
| frontend-v2/src/app/(authed)/activists/activists-page.tsx | Adds main activists page client logic (filters/URL sync/infinite query). |
| frontend-v2/src/app/(authed)/activists/activist-filters.tsx | Adds filter bar UI wrapper for activists list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend-v2/src/app/(authed)/event/event-form.tsx (1)
103-107:⚠️ Potential issue | 🟠 MajorAdd error handling for failed event fetch in edit mode.
When editing an event and the fetch fails, the component renders the form with empty defaults, allowing users to unintentionally submit from an edit URL. Destructure
isErrorfrom theuseQueryhook and add an error guard before rendering the form.Proposed fix
- const { data: eventData, isLoading: isEventLoading } = useQuery({ + const { + data: eventData, + isLoading: isEventLoading, + isError: isEventError, + } = useQuery({ queryKey: [API_PATH.EVENT_GET, eventId], queryFn: () => apiClient.getEvent(Number(eventId)), enabled: !!eventId, })Then add an error guard after the loading check:
+ if (eventId && isEventError) { + return ( + <div className="flex items-center justify-center p-8"> + <p className="text-red-600">Failed to load event. Please refresh and try again.</p> + </div> + ) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend-v2/src/app/`(authed)/event/event-form.tsx around lines 103 - 107, The query for loading an event lacks error handling: destructure isError from the useQuery call (alongside data and isLoading) that calls apiClient.getEvent with eventId, then add a guard in the EventForm render (after the isEventLoading check) to return an error state or null when isError is true so the form is not rendered with empty defaults in edit mode; update references to eventData, isEventLoading, isError, useQuery, and apiClient.getEvent accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend-v2/src/app/`(authed)/event/event-form.tsx:
- Around line 103-107: The query for loading an event lacks error handling:
destructure isError from the useQuery call (alongside data and isLoading) that
calls apiClient.getEvent with eventId, then add a guard in the EventForm render
(after the isEventLoading check) to return an error state or null when isError
is true so the form is not rendered with empty defaults in edit mode; update
references to eventData, isEventLoading, isError, useQuery, and
apiClient.getEvent accordingly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
frontend-v2/src/app/(authed)/coaching/page.tsxfrontend-v2/src/app/(authed)/event/event-form.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend-v2/src/app/`(authed)/event/event-form.tsx:
- Around line 339-356: Loading guard currently runs before the event error
guard, which can mask a failed event fetch; move the event error check (the if
that tests eventId && isEventError and returns the "Failed to load..." UI) above
the loading guard so that the component first renders the event error state when
eventId exists and isEventError is true; keep the existing conditions
(isLoadingActivists, isEventLoading, activistRegistry) intact but after the
event error check to preserve the original loading behavior.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
frontend-v2/src/app/(authed)/event/event-form.tsxfrontend-v2/src/app/(authed)/users/user-form.tsxfrontend-v2/src/app/(authed)/users/users-page.tsx
jakehobbs
commented
Mar 1, 2026
alexsapps
reviewed
Mar 2, 2026
Collaborator
There was a problem hiding this comment.
curious why some of the loading pages use <div> and <p> and others use <h1> but i'm not i'm not too worried about it
| @@ -1,88 +0,0 @@ | |||
| import { NextResponse } from 'next/server' | |||
Collaborator
There was a problem hiding this comment.
that's awesome we can get rid of all of this!
alexsapps
reviewed
Mar 2, 2026
Collaborator
There was a problem hiding this comment.
wdyt about calling this (user) or (crm) instead of (authed) to make room for future types of authed pages like the members portal?
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
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 by CodeRabbit
New Features
Bug Fixes
Refactor
UI/UX Improvements