Conversation
ogzhanolguncu
requested review from
chronark,
mcstepp and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthroughRoutes and navigation were made workspace-slug aware: components now use useWorkspaceNavigation to build base paths/hrefs, several imports updated to slug-aware paths, sidebar auto-open logic matches against item.href, and minor UI tweaks added (hoverable project/namespace displays with ChevronExpandY). Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Component (Nav/Links)
participant WS as useWorkspaceNavigation
participant Router as Router
UI->>WS: request workspace
WS-->>UI: { slug }
UI->>UI: basePath = "/" + slug
UI->>Router: navigate to `${basePath}/projects/...`
Router-->>UI: render matching route
note right of UI: workspace-aware hrefs replace static paths
sequenceDiagram
participant Pop as DeploymentActionsPopover
participant WS as useWorkspaceNavigation
participant Router as Router
Pop->>WS: request workspace
WS-->>Pop: { slug }
Pop->>Pop: logsHref = `/${slug}/projects/${projectId}/deployments/${deploymentId}/gateway-logs`
Pop->>Router: push(logsHref)
Router-->>Pop: route to Gateway Logs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
49-50: Use segments.at(1) for workspace-scoped nav items
segments.at(0) is still used for these workspace-scoped routes, causing wrong active highlighting—change to segments.at(1):
• APIs (line 49)
• Rate limits (64)
• Audit Log (91)
• Logs (104)
• Identities (118)
• Settings (125)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-navigation.tsx (1)
32-41: Potential crash: useLiveQuery .data may be undefined on first renderUsing
.data.at(0)can throw when data hasn’t arrived yet. Guard it.Apply this minimal fix:
- ).data.at(0); + ).data?.at(0);
🧹 Nitpick comments (4)
apps/dashboard/app/(app)/[workspaceSlug]/projects/_components/list/projects-card.tsx (1)
34-48: LGTM: project links now workspace-scoped; optional UX tweakGreat update to projectPath. Optional: also set the loading state when clicking the name link for consistent feedback.
Apply this small tweak:
<InfoTooltip content={name} asChild position={{ align: "start", side: "top" }}> <Link - href={projectPath} + href={projectPath} + onClick={handleLinkClick} className="font-medium text-sm leading-[14px] text-accent-12 truncate hover:underline" > {name} </Link> </InfoTooltip>Also applies to: 65-66
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-navigation.tsx (2)
44-55: Avoid premature “Project not found” by handling activeProject loadingActive project is queried separately but its loading state isn’t considered; users may briefly see “not found” before data arrives.
Apply this refactor to track and respect the active project’s loading state:
- const activeProject = useLiveQuery((q) => - q - .from({ project: collection.projects }) - .where(({ project }) => eq(project.id, projectId)) - .select(({ project }) => ({ - id: project.id, - name: project.name, - gitRepositoryUrl: project.gitRepositoryUrl, - })), - ).data.at(0); + const activeProjectQuery = useLiveQuery((q) => + q + .from({ project: collection.projects }) + .where(({ project }) => eq(project.id, projectId)) + .select(({ project }) => ({ + id: project.id, + name: project.name, + gitRepositoryUrl: project.gitRepositoryUrl, + })), + ); + const activeProject = activeProjectQuery.data?.at(0); - if (projects.isLoading) { + if (projects.isLoading || activeProjectQuery.isLoading) { return ( <Navbar> <Navbar.Breadcrumbs icon={<Cube />}> - <Navbar.Breadcrumbs.Link href={basePath}>Projects</Navbar.Breadcrumbs.Link> + <Navbar.Breadcrumbs.Link href={basePath}>Projects</Navbar.Breadcrumbs.Link> <Navbar.Breadcrumbs.Link href="#" isIdentifier className="group max-md:hidden" noop> <div className="h-6 w-24 bg-grayA-3 rounded animate-pulse transition-all" /> </Navbar.Breadcrumbs.Link> </Navbar.Breadcrumbs> </Navbar> ); }
73-77: Null-safe map over projects for resilienceDefensive guard avoids rare NPEs if data is temporarily undefined.
- items={projects.data.map((project) => ({ + items={(projects.data ?? []).map((project) => ({ id: project.id, label: project.name, href: `${basePath}/${project.id}`, }))}apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx (1)
134-149: Reset collapse preferences should key off href (workspace-aware), not slugified labelSlugifying labels to derive a section path can desync from workspace-aware hrefs and prevent proper resets.
- useLayoutEffect(() => { - if (!pathname || typeof item.label !== "string") { - return; - } - const itemPath = `/${slugify(item.label, { - lower: true, - replacement: "-", - })}`; - // If we've navigated away from this section entirely, reset user preferences - if (!pathname.startsWith(itemPath)) { - setUserManuallyCollapsed(false); - setChildrenUserManuallyCollapsed(false); - } - }, [pathname, item.label]); + useLayoutEffect(() => { + if (!pathname) return; + // Prefer href (workspace-aware); fall back to slug label if href is missing + const refPath = + item.href ?? + (typeof item.label === "string" + ? `/${slugify(item.label, { lower: true, replacement: "-" })}` + : undefined); + if (!refPath) return; + const isWithin = + pathname === refPath || pathname.startsWith(`${refPath}/`); + if (!isWithin) { + setUserManuallyCollapsed(false); + setChildrenUserManuallyCollapsed(false); + } + }, [pathname, item.href, item.label]);If you remove the fallback, also drop the slugify import.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/deployments/components/controls/components/deployment-list-filters/components/environment-filter.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/deployments/components/table/components/actions/deployment-list-table-action.popover.constants.tsx(3 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-navigation.tsx(5 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-sub-navigation.tsx(3 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/_components/list/projects-card.tsx(3 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/navigation.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/ratelimits/[namespaceId]/namespace-navbar.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/ratelimits/navigation.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx(4 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx(1 hunks)apps/dashboard/lib/trpc/routers/deploy/deployment/llm-search/utils.ts(1 hunks)apps/dashboard/lib/trpc/routers/deploy/envs/list.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: perkinsjr
PR: unkeyed/unkey#4009
File: apps/dashboard/app/(app)/[workspace]/apis/[apiId]/_overview/components/table/components/log-details/index.tsx:4-5
Timestamp: 2025-09-22T18:44:56.279Z
Learning: In the Unkey dashboard, the workspace hook (useWorkspace) provides security validation by checking database access and user authorization to the workspace, with 10-minute caching for performance. Using URL params (useParams) for workspace slug would bypass this security validation and allow unauthorized access attempts. Always use the workspace hook for workspace-scoped navigation and handle loading states properly rather than switching to URL parameters.
📚 Learning: 2025-09-22T18:44:56.279Z
Learnt from: perkinsjr
PR: unkeyed/unkey#4009
File: apps/dashboard/app/(app)/[workspace]/apis/[apiId]/_overview/components/table/components/log-details/index.tsx:4-5
Timestamp: 2025-09-22T18:44:56.279Z
Learning: In the Unkey dashboard, the workspace hook (useWorkspace) provides security validation by checking database access and user authorization to the workspace, with 10-minute caching for performance. Using URL params (useParams) for workspace slug would bypass this security validation and allow unauthorized access attempts. Always use the workspace hook for workspace-scoped navigation and handle loading states properly rather than switching to URL parameters.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/ratelimits/navigation.tsxapps/dashboard/app/(app)/[workspaceSlug]/projects/_components/list/projects-card.tsxapps/dashboard/app/(app)/[workspaceSlug]/projects/navigation.tsxapps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-sub-navigation.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx
📚 Learning: 2025-09-23T17:39:59.820Z
Learnt from: perkinsjr
PR: unkeyed/unkey#4009
File: apps/dashboard/app/(app)/[workspace]/apis/[apiId]/_overview/components/table/components/override-indicator.tsx:88-97
Timestamp: 2025-09-23T17:39:59.820Z
Learning: The useWorkspaceNavigation hook in the Unkey dashboard guarantees that a workspace exists. If no workspace is found, the hook redirects the user to create a new workspace. Users cannot be logged in without a workspace, and new users must create one to continue. Therefore, workspace will never be null when using this hook.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/ratelimits/navigation.tsxapps/dashboard/app/(app)/[workspaceSlug]/projects/_components/list/projects-card.tsxapps/dashboard/app/(app)/[workspaceSlug]/projects/navigation.tsxapps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-navigation.tsxapps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-sub-navigation.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx
📚 Learning: 2025-09-23T17:40:44.944Z
Learnt from: perkinsjr
PR: unkeyed/unkey#4009
File: apps/dashboard/app/(app)/[workspace]/authorization/roles/navigation.tsx:26-40
Timestamp: 2025-09-23T17:40:44.944Z
Learning: In the Unkey dashboard authorization section navigation components, the maintainer prefers to wrap entire navbars in Suspense rather than scoping Suspense to individual components, even if it blocks the whole navigation during loading.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/ratelimits/navigation.tsx
📚 Learning: 2024-10-04T20:47:34.791Z
Learnt from: chronark
PR: unkeyed/unkey#2180
File: apps/dashboard/lib/constants/workspace-navigations.tsx:85-88
Timestamp: 2024-10-04T20:47:34.791Z
Learning: In navigation code (e.g., `apps/dashboard/lib/constants/workspace-navigations.tsx`), prefer using `segments.at(0)` over `segments[0]` to handle cases when the `segments` array might be empty.
Applied to files:
apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsxapps/dashboard/components/navigation/sidebar/workspace-navigations.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx
📚 Learning: 2025-06-24T13:29:10.129Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3401
File: apps/dashboard/app/(app)/logs/filters.query-params.ts:10-0
Timestamp: 2025-06-24T13:29:10.129Z
Learning: The `buildQueryParams` function in `apps/dashboard/app/(app)/logs/filters.query-params.ts` calls `useFilters()` hook inside it, but this is valid because the function is only called from within other React hooks, maintaining the Rules of Hooks compliance.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/deployments/components/controls/components/deployment-list-filters/components/environment-filter.tsx
📚 Learning: 2025-08-31T19:30:38.171Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3853
File: apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts:75-87
Timestamp: 2025-08-31T19:30:38.171Z
Learning: In apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts, the getEnvs procedure currently uses mock data (VARIABLES) and ignores the projectId input parameter. This is intentional temporary behavior - the user ogzhanolguncu indicated they plan to hook this up to the database later.
Applied to files:
apps/dashboard/lib/trpc/routers/deploy/envs/list.ts
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Applied to files:
apps/dashboard/lib/trpc/routers/deploy/envs/list.tsapps/dashboard/lib/trpc/routers/deploy/deployment/llm-search/utils.tsapps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/deployments/components/table/components/actions/deployment-list-table-action.popover.constants.tsx
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/projects/navigation.tsx
📚 Learning: 2025-08-25T13:46:34.441Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx:4-4
Timestamp: 2025-08-25T13:46:34.441Z
Learning: The namespace list refresh component (apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx) intentionally uses the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than a namespace-specific hook. This cross-coupling between namespace list components and overview hooks is an architectural design decision.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/ratelimits/[namespaceId]/namespace-navbar.tsx
📚 Learning: 2025-08-25T13:46:08.303Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx:1-1
Timestamp: 2025-08-25T13:46:08.303Z
Learning: The NamespaceListDateTime component in apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx is intentionally designed to use the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than the namespace list hook, as clarified by ogzhanolguncu. This coupling is by design, not an architectural issue.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/ratelimits/[namespaceId]/namespace-navbar.tsx
📚 Learning: 2025-08-18T10:28:47.391Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3797
File: apps/dashboard/app/(app)/projects/[projectId]/deployments/components/control-cloud/index.tsx:1-4
Timestamp: 2025-08-18T10:28:47.391Z
Learning: In Next.js App Router, components that use React hooks don't need their own "use client" directive if they are rendered within a client component that already has the directive. The client boundary propagates to child components.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-sub-navigation.tsxapps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/deployments/components/table/components/actions/deployment-list-table-action.popover.constants.tsx
🧬 Code graph analysis (1)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx (2)
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
NavItem(16-29)apps/dashboard/lib/collections/index.ts (1)
collection(72-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Go API Local / Test
🔇 Additional comments (14)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/deployments/components/controls/components/deployment-list-filters/components/environment-filter.tsx (1)
1-1: Workspace-aware layout import looks goodPointing
useProjectLayoutat the[workspaceSlug]route keeps this filter wired into the slug-scoped provider. No concerns.apps/dashboard/lib/trpc/routers/deploy/envs/list.ts (1)
1-1: Slug-aware import LGTMThe import now points at the workspace-aware route tree, matching the slugged
/[workspaceSlug]/projects/...structure introduced in this PR. Looks consistent with the navigation updates elsewhere.apps/dashboard/lib/trpc/routers/deploy/deployment/llm-search/utils.ts (1)
1-4: Slug-aware filters import looks goodUpdating the schema import to the
/[workspaceSlug]/projectspath keeps the LLM helpers in sync with the new route hierarchy. Nice catch.apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/deployments/components/table/components/actions/deployment-list-table-action.popover.constants.tsx (1)
2-5: Good move to workspace-aware imports and hook usageSwitching to the workspace-scoped layout provider and adding useWorkspaceNavigation aligns routing with slug-aware paths. No concerns.
Based on learnings
Also applies to: 24-26
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-sub-navigation.tsx (2)
27-27: LGTM: using workspace hook for slugUsing useWorkspaceNavigation ensures validated workspace context rather than URL params. Good.
Based on learnings
56-75: LGTM: basePath and tab hrefs updated to slug-aware pathsPaths correctly prefixed with /{workspaceSlug}/projects/{projectId}[...].
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx (1)
17-18: LGTM: basePath use and parent item mergeDeriving basePath from workspace.slug and matching the parent by href is correct and future-proof.
Also applies to: 39-50
apps/dashboard/app/(app)/[workspaceSlug]/projects/navigation.tsx (1)
8-13: LGTM: breadcrumb now workspace-awareBreadcrumb link correctly points to /{workspaceSlug}/projects.
apps/dashboard/app/(app)/[workspaceSlug]/ratelimits/navigation.tsx (1)
9-15: LGTM: ratelimits breadcrumb is slug-awareLink now correctly points to /{workspaceSlug}/ratelimits.
apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx (1)
59-63: Confirm segment index assumptions for solo mode detectionSwitching to segments.at(1) assumes the slug occupies .at(0). Ensure this matches how useSelectedLayoutSegments is called here; otherwise solo mode may never activate.
If segments here already exclude the workspace slug (common when called within the [workspaceSlug] layout), revert to at(0). Please verify locally by logging segments for a few routes (apis, projects, ratelimits).
apps/dashboard/app/(app)/[workspaceSlug]/ratelimits/[namespaceId]/namespace-navbar.tsx (1)
89-92: LGTM: clearer, larger click target for namespace switcherThe hoverable wrapper + Chevron icon improves affordance. Workspace-aware hrefs and popover items look correct.
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-navigation.tsx (2)
80-83: LGTM: improved affordance for active project selectorThe hover state and Chevron icon match the updated pattern in other navigations.
24-24: Hard-coded “/projects” in Quickstart docs
QUICKSTART-DEPLOY.md:35 still links tohttp://localhost:3000/projects; update it to include the workspace slug (e.g.http://localhost:3000/{workspaceSlug}/projects).⛔ Skipped due to learnings
Learnt from: perkinsjr PR: unkeyed/unkey#4009 File: apps/dashboard/app/(app)/[workspace]/apis/[apiId]/_overview/components/table/components/override-indicator.tsx:88-97 Timestamp: 2025-09-23T17:39:59.820Z Learning: The useWorkspaceNavigation hook in the Unkey dashboard guarantees that a workspace exists. If no workspace is found, the hook redirects the user to create a new workspace. Users cannot be logged in without a workspace, and new users must create one to continue. Therefore, workspace will never be null when using this hook.Learnt from: perkinsjr PR: unkeyed/unkey#4009 File: apps/dashboard/app/(app)/[workspace]/apis/[apiId]/_overview/components/table/components/log-details/index.tsx:4-5 Timestamp: 2025-09-22T18:44:56.279Z Learning: In the Unkey dashboard, the workspace hook (useWorkspace) provides security validation by checking database access and user authorization to the workspace, with 10-minute caching for performance. Using URL params (useParams) for workspace slug would bypass this security validation and allow unauthorized access attempts. Always use the workspace hook for workspace-scoped navigation and handle loading states properly rather than switching to URL parameters.Learnt from: chronark PR: unkeyed/unkey#2180 File: apps/dashboard/lib/constants/workspace-navigations.tsx:56-118 Timestamp: 2024-10-04T20:44:38.489Z Learning: When typing the `workspace` parameter in functions like `createWorkspaceNavigation`, prefer importing the `Workspace` type from the database module and picking the necessary keys (e.g., `features`) instead of redefining the interface.apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx (1)
64-67: LGTM: child match + Boolean normalizationDirect href equality for child detection and Boolean() normalization are clear and correct.
Also applies to: 71-72
...ments/components/table/components/actions/deployment-list-table-action.popover.constants.tsx
Show resolved
Hide resolved
...dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx
Show resolved
Hide resolved
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsx (1)
34-59: Fix sub-item active checks to follow the dynamic “apis” indexWe now locate the
"apis"segment dynamically, but the sub-item active checks still readsegments.at(3). On any route where"apis"sits deeper (e.g./[workspace]/projects/[projectId]/apis/...), the Requests/Settings/Keys items will never mark active, breaking sidebar feedback for those pages. Please key these checks off the same dynamic index.- const currentApiActive = aIndex !== -1 && segments.at(aIndex + 1) === api.id; + const currentApiActive = aIndex !== -1 && segments.at(aIndex + 1) === api.id; + const currentApiSection = + aIndex !== -1 ? segments.at(aIndex + 2) : undefined; @@ - active: currentApiActive && segments.at(3) === "settings", + active: currentApiActive && currentApiSection === "settings", @@ - active: currentApiActive && !segments.at(3), + active: currentApiActive && !currentApiSection, @@ - active: currentApiActive && segments.at(3) === "keys", + active: currentApiActive && currentApiSection === "keys",
🧹 Nitpick comments (4)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx (1)
40-40: Harden parent item match against trailing slashesDirect equality can fail if one side has a trailing slash. Normalize both sides.
- const projectsItemIndex = items.findIndex((item) => item.href === basePath); + const normalize = (s: string) => s.replace(/\/$/, ""); + const projectsItemIndex = items.findIndex( + (item) => normalize(item.href) === normalize(basePath), + );apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (3)
94-99: Monitors uses a root path; align with slug routing or mark as externalIf Monitors is workspace-scoped, make it slug-aware; if global, mark it external and adjust active logic accordingly.
Option A (workspace-scoped):
- href: "/monitors/verifications", + href: `${basePath}/monitors/verifications`,Option B (global):
icon: Grid, - href: "/monitors/verifications", + href: "/monitors/verifications", label: "Monitors", - active: segments.at(1) === "verifications", + active: segments.at(0) === "monitors" || segments.at(1) === "verifications", + external: true,
106-113: Success uses a root path; align with slug routing or mark as externalSame consideration as Monitors; ensure UX stays within workspace or explicitly exits it.
Option A (workspace-scoped):
- href: "/success", + href: `${basePath}/success`,Option B (global):
icon: Sparkle3, - href: "/success", + href: "/success", label: "Success", - active: segments.at(1) === "success", + active: segments.at(0) === "success", + external: true,
42-45: Optional: reduce repetition by caching the top-level segmentMicro clean-up to avoid repeated at(1) calls.
export const createWorkspaceNavigation = (segments: string[], workspace: Workspace) => { const basePath = `/${workspace.slug}`; + const top = segments.at(1); return [ ... - active: segments.at(1) === "apis", + active: top === "apis",(Apply similarly for projects, ratelimits, audit, verifications, logs, success, identities, settings.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsx(2 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx(3 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: perkinsjr
PR: unkeyed/unkey#4009
File: apps/dashboard/app/(app)/[workspace]/apis/[apiId]/_overview/components/table/components/log-details/index.tsx:4-5
Timestamp: 2025-09-22T18:44:56.279Z
Learning: In the Unkey dashboard, the workspace hook (useWorkspace) provides security validation by checking database access and user authorization to the workspace, with 10-minute caching for performance. Using URL params (useParams) for workspace slug would bypass this security validation and allow unauthorized access attempts. Always use the workspace hook for workspace-scoped navigation and handle loading states properly rather than switching to URL parameters.
Learnt from: perkinsjr
PR: unkeyed/unkey#4009
File: apps/dashboard/app/(app)/[workspace]/authorization/roles/navigation.tsx:26-40
Timestamp: 2025-09-23T17:46:49.043Z
Learning: In the Unkey dashboard, there is no overview page at /${workspace.slug}/authorization. The roles page at /${workspace.slug}/authorization/roles serves as the default/primary page for the authorization section, so breadcrumb navigation appropriately points both "Authorization" and "Roles" breadcrumbs to the roles page.
📚 Learning: 2024-10-04T20:47:34.791Z
Learnt from: chronark
PR: unkeyed/unkey#2180
File: apps/dashboard/lib/constants/workspace-navigations.tsx:85-88
Timestamp: 2024-10-04T20:47:34.791Z
Learning: In navigation code (e.g., `apps/dashboard/lib/constants/workspace-navigations.tsx`), prefer using `segments.at(0)` over `segments[0]` to handle cases when the `segments` array might be empty.
Applied to files:
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsxapps/dashboard/components/navigation/sidebar/workspace-navigations.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.
Applied to files:
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx
📚 Learning: 2025-09-23T17:39:59.820Z
Learnt from: perkinsjr
PR: unkeyed/unkey#4009
File: apps/dashboard/app/(app)/[workspace]/apis/[apiId]/_overview/components/table/components/override-indicator.tsx:88-97
Timestamp: 2025-09-23T17:39:59.820Z
Learning: The useWorkspaceNavigation hook in the Unkey dashboard guarantees that a workspace exists. If no workspace is found, the hook redirects the user to create a new workspace. Users cannot be logged in without a workspace, and new users must create one to continue. Therefore, workspace will never be null when using this hook.
Applied to files:
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx
📚 Learning: 2025-08-25T13:46:34.441Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx:4-4
Timestamp: 2025-08-25T13:46:34.441Z
Learning: The namespace list refresh component (apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx) intentionally uses the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than a namespace-specific hook. This cross-coupling between namespace list components and overview hooks is an architectural design decision.
Applied to files:
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx
📚 Learning: 2025-08-25T13:46:08.303Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx:1-1
Timestamp: 2025-08-25T13:46:08.303Z
Learning: The NamespaceListDateTime component in apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx is intentionally designed to use the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than the namespace list hook, as clarified by ogzhanolguncu. This coupling is by design, not an architectural issue.
Applied to files:
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx
🧬 Code graph analysis (3)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsx (1)
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
NavItem(16-29)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx (2)
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
NavItem(16-29)apps/dashboard/lib/collections/index.ts (1)
collection(72-76)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx (1)
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
NavItem(16-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Go API Local / Test
- GitHub Check: Build / Build
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx (6)
3-11: Good: using useWorkspaceNavigation for slug-aware, authorized navigationCorrect hook choice; avoids bypassing workspace auth and guarantees non-null workspace.
Based on learnings
17-17: Base path computed correctlySlug-aware base path aligns with new routing.
23-24: Active-state logic fixed for slugged routesDynamic index of "projects" resolves the off-by-one introduced by the workspace slug.
Optional micro-optimization: compute pIndex once outside the map to avoid repeated scans.
- return data.map((project) => { - const pIndex = segments.findIndex((s) => s === "projects"); - const currentProjectActive = pIndex !== -1 && segments.at(pIndex + 1) === project.id; + const pIndex = segments.findIndex((s) => s === "projects"); + return data.map((project) => { + const currentProjectActive = + pIndex !== -1 && segments.at(pIndex + 1) === project.id;
27-27: Correct: hrefs use the slug-aware basePathLinks will route to /:workspaceSlug/projects/:projectId as intended.
36-36: Dependencies updated appropriatelyIncluding basePath ensures recomputation on workspace changes.
51-51: Memo deps look goodTracks baseNavItems, derived items, and basePath correctly.
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (3)
54-56: Projects route moved under workspace slug: LGTMSlug-aware href and active check look correct.
42-44: All createWorkspaceNavigation callers obtain workspace via useWorkspaceNavigation
Only AppSidebar invokes createWorkspaceNavigation with the workspace returned by useWorkspaceNavigation(), not from URL params.
49-49: segments.at(1) correctly targets section after workspace slug
useSelectedLayoutSegments returns[workspaceSlug,…], sosegments.at(1)accurately checks the API/project/etc. segment. No changes needed.
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx
Show resolved
Hide resolved
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (09/26/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
3 participants
What does this PR do?
This PR changes
/projectsroutes to/[workspaceSlug]/projects. Adds chevron icon to show list ofprojectsandratelimitssee the screenshot below.Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
/projectsroutes workChecklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated