-
Notifications
You must be signed in to change notification settings - Fork 613
feat: WIP DON'T REVIEW WIP WIP WIP implement workspace-based URL routing architecture #4008
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f682453
050b281
1016cfd
9f631d0
beaf426
a08462c
bb6d555
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| "use client"; | ||
| import { shortenId } from "@/lib/shorten-id"; | ||
| import { cn } from "@/lib/utils"; | ||
| import { useWorkspace } from "@/providers/workspace-provider"; | ||
| import type { KeysOverviewLog } from "@unkey/clickhouse/src/keys/keys"; | ||
| import { TriangleWarning2 } from "@unkey/icons"; | ||
| import { InfoTooltip, Loading } from "@unkey/ui"; | ||
|
|
@@ -44,11 +45,11 @@ const getWarningMessage = (severity: string, errorRate: number) => { | |
| }; | ||
|
|
||
| export const KeyIdentifierColumn = ({ log, apiId, onNavigate }: KeyIdentifierColumnProps) => { | ||
| const { workspace } = useWorkspace(); | ||
| const router = useRouter(); | ||
| const errorPercentage = getErrorPercentage(log); | ||
| const severity = getErrorSeverity(log); | ||
| const hasErrors = severity !== "none"; | ||
|
|
||
| const [isNavigating, setIsNavigating] = useState(false); | ||
|
|
||
| const handleLinkClick = useCallback( | ||
|
|
@@ -58,9 +59,11 @@ export const KeyIdentifierColumn = ({ log, apiId, onNavigate }: KeyIdentifierCol | |
|
|
||
| onNavigate?.(); | ||
|
|
||
| router.push(`/apis/${apiId}/keys/${log.key_details?.key_auth_id}/${log.key_id}`); | ||
| router.push( | ||
| `/${workspace?.slug}/apis/${apiId}/keys/${log.key_details?.key_auth_id}/${log.key_id}`, | ||
| ); | ||
|
Comment on lines
+62
to
+64
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard against undefined workspace slug/key_auth_id to prevent broken URLs. Optional chaining inside template literals will stringify to "undefined", yielding paths like "/undefined/apis/...". Add a runtime guard and make the link href resilient. Apply these diffs: - router.push(
- `/${workspace?.slug}/apis/${apiId}/keys/${log.key_details?.key_auth_id}/${log.key_id}`,
- );
+ if (!workspace?.slug || !log.key_details?.key_auth_id) {
+ setIsNavigating(false);
+ return;
+ }
+ router.push(
+ `/${workspace.slug}/apis/${apiId}/keys/${log.key_details.key_auth_id}/${log.key_id}`,
+ );- href={`/${workspace?.slug}/apis/${apiId}/keys/${log.key_details?.key_auth_id}/${log.key_id}`}
+ href={
+ workspace?.slug && log.key_details?.key_auth_id
+ ? `/${workspace.slug}/apis/${apiId}/keys/${log.key_details.key_auth_id}/${log.key_id}`
+ : "#"
+ }Also applies to: 89-89 |
||
| }, | ||
| [apiId, log.key_id, log.key_details?.key_auth_id, onNavigate, router.push], | ||
| [apiId, log.key_id, log.key_details?.key_auth_id, onNavigate, router.push, workspace?.slug], | ||
| ); | ||
|
|
||
| return ( | ||
|
|
@@ -83,7 +86,7 @@ export const KeyIdentifierColumn = ({ log, apiId, onNavigate }: KeyIdentifierCol | |
| <Link | ||
| title={`View details for ${log.key_id}`} | ||
| className="font-mono group-hover:underline decoration-dotted" | ||
| href={`/apis/${apiId}/keys/${log.key_details?.key_auth_id}/${log.key_id}`} | ||
| href={`/${workspace?.slug}/apis/${apiId}/keys/${log.key_details?.key_auth_id}/${log.key_id}`} | ||
| onClick={handleLinkClick} | ||
| > | ||
| <div className="font-mono font-medium truncate flex items-center"> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,233 @@ | ||
| "use client"; | ||
|
|
||
| import { QuickNavPopover } from "@/components/navbar-popover"; | ||
| import { NavbarActionButton } from "@/components/navigation/action-button"; | ||
| import { Navbar } from "@/components/navigation/navbar"; | ||
| import { useIsMobile } from "@/hooks/use-mobile"; | ||
| import { trpc } from "@/lib/trpc/client"; | ||
| import { useWorkspace } from "@/providers/workspace-provider"; | ||
| import type { Workspace } from "@unkey/db"; | ||
| import { ChevronExpandY, Nodes, Plus, TaskUnchecked } from "@unkey/icons"; | ||
| import { CreateKeyDialog } from "./_components/create-key"; | ||
|
|
||
| // Types for better type safety | ||
| interface ApiLayoutData { | ||
| currentApi: { | ||
| id: string; | ||
| name: string; | ||
| workspaceId: string; | ||
| keyAuthId: string | null; | ||
| keyspaceDefaults: { | ||
| prefix?: string; | ||
| bytes?: number; | ||
| } | null; | ||
| deleteProtection: boolean | null; | ||
| ipWhitelist: string | null; | ||
| }; | ||
| workspaceApis: Array<{ | ||
| id: string; | ||
| name: string; | ||
| }>; | ||
| keyAuth: { | ||
| id: string; | ||
| defaultPrefix: string | null; | ||
| defaultBytes: number | null; | ||
| sizeApprox: number; | ||
| } | null; | ||
| workspace: { | ||
| id: string; | ||
| ipWhitelist: boolean; | ||
| }; | ||
| } | ||
|
|
||
| interface ApisNavbarProps { | ||
| apiId: string; | ||
| keyspaceId?: string; | ||
| keyId?: string; | ||
| activePage?: { | ||
| href: string; | ||
| text: string; | ||
| }; | ||
| } | ||
|
|
||
| interface LoadingNavbarProps { | ||
| workspace: Workspace | null; | ||
| } | ||
|
|
||
| interface NavbarContentProps { | ||
| apiId: string; | ||
| keyspaceId?: string; | ||
| keyId?: string; | ||
| activePage?: { | ||
| href: string; | ||
| text: string; | ||
| }; | ||
| workspace: Workspace | null; | ||
| isMobile: boolean; | ||
| layoutData: ApiLayoutData; | ||
| } | ||
|
|
||
| // Loading state component | ||
| const LoadingNavbar = ({ workspace }: LoadingNavbarProps) => ( | ||
| <Navbar> | ||
| <Navbar.Breadcrumbs icon={<Nodes />}> | ||
| <Navbar.Breadcrumbs.Link href={`/${workspace?.slug}/apis`}>APIs</Navbar.Breadcrumbs.Link> | ||
| <Navbar.Breadcrumbs.Link href="#" isIdentifier className="group" noop> | ||
| <div className="h-6 w-20 bg-grayA-3 rounded animate-pulse transition-all " /> | ||
| </Navbar.Breadcrumbs.Link> | ||
| <Navbar.Breadcrumbs.Link href="#" noop active> | ||
| <div className="hover:bg-gray-3 rounded-lg flex items-center gap-1 p-1"> | ||
| <div className="h-6 w-16 bg-grayA-3 rounded animate-pulse transition-all " /> | ||
| <ChevronExpandY className="size-4" /> | ||
| </div> | ||
| </Navbar.Breadcrumbs.Link> | ||
| </Navbar.Breadcrumbs> | ||
| <Navbar.Actions> | ||
| <NavbarActionButton disabled> | ||
| <Plus /> | ||
| Create new key | ||
| </NavbarActionButton> | ||
| <div className="h-7 bg-grayA-2 border border-gray-6 rounded-md animate-pulse px-3 flex gap-2 items-center justify-center w-[190px] transition-all "> | ||
| <div className="h-3 w-[190px] bg-grayA-3 rounded" /> | ||
| <div> | ||
| <TaskUnchecked size="md-regular" className="!size-4" /> | ||
| </div> | ||
| </div> | ||
| </Navbar.Actions> | ||
| </Navbar> | ||
| ); | ||
|
Comment on lines
+70
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 🧩 Analysis chainAvoid generating broken links when workspace slug is missing; centralize href building Current code can emit +// Workspace-safe href builder
+const wsHref = (slug?: string, path = "") => (slug ? `/${slug}${path}` : "#");
// Loading state component
const LoadingNavbar = ({ workspace }: LoadingNavbarProps) => (
<Navbar>
<Navbar.Breadcrumbs icon={<Nodes />}>
- <Navbar.Breadcrumbs.Link href={`/${workspace?.slug}/apis`}>APIs</Navbar.Breadcrumbs.Link>
+ <Navbar.Breadcrumbs.Link
+ href={wsHref(workspace?.slug, "/apis")}
+ noop={!workspace}
+ >
+ APIs
+ </Navbar.Breadcrumbs.Link>
<Navbar.Breadcrumbs.Link href="#" isIdentifier className="group" noop>
<div className="h-6 w-20 bg-grayA-3 rounded animate-pulse transition-all " />
</Navbar.Breadcrumbs.Link> const { currentApi } = layoutData;
// Define base path for API navigation
- const base = `/${workspace?.slug}/apis/${currentApi.id}`;
+ const slug = workspace?.slug;
+ const base = wsHref(slug, `/apis/${currentApi.id}`);
// Create navigation items for QuickNavPopover
const navigationItems = [
{
id: "requests",
label: "Requests",
- href: `/${workspace?.slug}/apis/${currentApi.id}`,
+ href: wsHref(slug, `/apis/${currentApi.id}`),
},
];
// Add Keys navigation if keyAuthId exists
if (currentApi.keyAuthId) {
navigationItems.push({
id: "keys",
label: "Keys",
- href: `/${workspace?.slug}/apis/${currentApi.id}/keys/${currentApi.keyAuthId}`,
+ href: wsHref(slug, `/apis/${currentApi.id}/keys/${currentApi.keyAuthId}`),
});
}
// Add Settings navigation
navigationItems.push({
id: "settings",
label: "Settings",
- href: `/${workspace?.slug}/apis/${currentApi.id}/settings`,
+ href: wsHref(slug, `/apis/${currentApi.id}/settings`),
});- <Navbar.Breadcrumbs.Link
- href={`/${workspace?.slug}/apis`}
- className={isMobile ? "hidden" : "max-md:hidden"}
- >
+ <Navbar.Breadcrumbs.Link
+ href={wsHref(slug, "/apis")}
+ className={isMobile ? "hidden" : "max-md:hidden"}
+ noop={!slug}
+ >
APIs
</Navbar.Breadcrumbs.Link>
<Navbar.Breadcrumbs.Link
href={base}
isIdentifier
className={isMobile ? "hidden" : "group max-md:hidden"}
noop
>
<div className="text-accent-10 group-hover:text-accent-12">{currentApi.name}</div>
</Navbar.Breadcrumbs.Link>
- <Navbar.Breadcrumbs.Link href={activePage?.href ?? ""} noop active={!shouldFetchKey}>
+ <Navbar.Breadcrumbs.Link
+ href={activePage?.href ?? wsHref(slug, `/apis/${currentApi.id}`)}
+ noop={!slug}
+ active={!shouldFetchKey}
+ >
<QuickNavPopover items={navigationItems} shortcutKey="M">
<div className="hover:bg-gray-3 rounded-lg flex items-center gap-1 p-1">
{activePage?.text ?? ""}
<ChevronExpandY className="size-4" />
</div>
</QuickNavPopover>
</Navbar.Breadcrumbs.Link>Also applies to: 119-146, 151-173, 165-172 🏁 Script executed: #!/bin/bash
# Find potentially unsafe hrefs using `workspace?.slug` that could yield `/undefined/...`
rg -nP --type=ts --type=tsx -C2 'href\s*=\s*{`/\$\{workspace\?\.\s*slug\}[^`]*`}'Length of output: 27 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Locate the targeted file if present
FILES=$(find . -type f -name 'api-id-navbar.tsx' 2>/dev/null || true)
if [ -n "$FILES" ]; then
for f in $FILES; do
echo "=== FILE: $f ==="
sed -n '1,240p' "$f" || true
done
else
echo "No api-id-navbar.tsx found."
fi
echo "=== Search: occurrences of workspace?.slug ==="
rg -n --hidden -S 'workspace\?\.\s*slug' || true
echo "=== Search: template usages with \${workspace?....} ==="
rg -n --hidden -S '\$\{workspace\?\.' || true
echo "=== Search: href template literals (look for backtick hrefs) ==="
rg -n --hidden -S 'href\s*=\s*{`' || true
rg -n --hidden -S 'href\s*=\s*{`[^`]*\$\{workspace' || true
echo "=== Fallback: any '/${workspace?.' occurrences ==="
rg -n --hidden -S '/\$\{workspace\?\.' || true
echo "=== Done ==="Length of output: 27261 Centralize workspace-safe href builder and mark links noop when slug is missing Template literals like 🤖 Prompt for AI Agents |
||
|
|
||
| // Main navbar content component | ||
| const NavbarContent = ({ | ||
| keyspaceId, | ||
| keyId, | ||
| activePage, | ||
| workspace, | ||
| isMobile, | ||
| layoutData, | ||
| }: NavbarContentProps) => { | ||
| const shouldFetchKey = Boolean(keyspaceId && keyId); | ||
|
|
||
| // If we expected to find a key but this component doesn't handle key details, | ||
| // we should handle this at a higher level or in a different component | ||
| if (shouldFetchKey) { | ||
| console.warn("Key fetching logic should be handled at a higher level"); | ||
| } | ||
|
|
||
| const { currentApi } = layoutData; | ||
|
|
||
| // Define base path for API navigation | ||
| const base = `/${workspace?.slug}/apis/${currentApi.id}`; | ||
|
|
||
| // Create navigation items for QuickNavPopover | ||
| const navigationItems = [ | ||
| { | ||
| id: "requests", | ||
| label: "Requests", | ||
| href: `/${workspace?.slug}/apis/${currentApi.id}`, | ||
| }, | ||
| ]; | ||
|
|
||
| // Add Keys navigation if keyAuthId exists | ||
| if (currentApi.keyAuthId) { | ||
| navigationItems.push({ | ||
| id: "keys", | ||
| label: "Keys", | ||
| href: `/${workspace?.slug}/apis/${currentApi.id}/keys/${currentApi.keyAuthId}`, | ||
| }); | ||
| } | ||
|
|
||
| // Add Settings navigation | ||
| navigationItems.push({ | ||
| id: "settings", | ||
| label: "Settings", | ||
| href: `/${workspace?.slug}/apis/${currentApi.id}/settings`, | ||
| }); | ||
|
|
||
| return ( | ||
| <div className="w-full"> | ||
| <Navbar className="w-full flex justify-between"> | ||
| <Navbar.Breadcrumbs className="flex-1 w-full" icon={<Nodes />}> | ||
| <Navbar.Breadcrumbs.Link | ||
| href={`/${workspace?.slug}/apis`} | ||
| className={isMobile ? "hidden" : "max-md:hidden"} | ||
| > | ||
| APIs | ||
| </Navbar.Breadcrumbs.Link> | ||
| <Navbar.Breadcrumbs.Link | ||
| href={base} | ||
| isIdentifier | ||
| className={isMobile ? "hidden" : "group max-md:hidden"} | ||
| noop | ||
| > | ||
| <div className="text-accent-10 group-hover:text-accent-12">{currentApi.name}</div> | ||
| </Navbar.Breadcrumbs.Link> | ||
| <Navbar.Breadcrumbs.Link href={activePage?.href ?? ""} noop active={!shouldFetchKey}> | ||
| <QuickNavPopover items={navigationItems} shortcutKey="M"> | ||
| <div className="hover:bg-gray-3 rounded-lg flex items-center gap-1 p-1"> | ||
| {activePage?.text ?? ""} | ||
| <ChevronExpandY className="size-4" /> | ||
| </div> | ||
| </QuickNavPopover> | ||
| </Navbar.Breadcrumbs.Link> | ||
| </Navbar.Breadcrumbs> | ||
| {layoutData.keyAuth ? ( | ||
| <CreateKeyDialog | ||
| keyspaceId={layoutData.keyAuth.id} | ||
| apiId={currentApi.id} | ||
| copyIdValue={currentApi.id} | ||
| keyspaceDefaults={currentApi.keyspaceDefaults} | ||
| /> | ||
| ) : ( | ||
| <NavbarActionButton disabled> | ||
| <Plus /> | ||
| Create new key | ||
| </NavbarActionButton> | ||
| )} | ||
| </Navbar> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| // Main component | ||
| export const ApisNavbar = ({ apiId, keyspaceId, keyId, activePage }: ApisNavbarProps) => { | ||
| const { workspace } = useWorkspace(); | ||
| const isMobile = useIsMobile(); | ||
|
|
||
| // Only make the query if we have a valid apiId | ||
| const { | ||
| data: layoutData, | ||
| isLoading, | ||
| error, | ||
| } = trpc.api.queryApiKeyDetails.useQuery( | ||
| { apiId }, | ||
| { | ||
| enabled: Boolean(apiId), // Only run query if apiId exists | ||
| retry: 3, | ||
| retryDelay: 1000, | ||
| }, | ||
| ); | ||
|
|
||
| // Show loading state while fetching data | ||
| if (isLoading || !layoutData) { | ||
| return <LoadingNavbar workspace={workspace} />; | ||
| } | ||
|
|
||
| // Handle error state | ||
| if (error) { | ||
| console.error("Failed to fetch API layout data:", error); | ||
| return <LoadingNavbar workspace={workspace} />; | ||
| } | ||
|
|
||
| return ( | ||
| <NavbarContent | ||
| apiId={apiId} | ||
| keyspaceId={keyspaceId} | ||
| keyId={keyId} | ||
| activePage={activePage} | ||
| workspace={workspace} | ||
| isMobile={isMobile} | ||
| layoutData={layoutData} | ||
| /> | ||
| ); | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Guard against missing workspace slug to avoid navigating to /undefined/...
router.push builds
/${workspace?.slug}/...; if the provider isn't ready, this becomes/undefined/.... Add a fallback from route params and fail gracefully.Apply this diff:
import { useEffect, useRef, useState } from "react"; +import { useParams } from "next/navigation"; @@ - const { workspace } = useWorkspace(); + const { workspace } = useWorkspace(); + const params = useParams<{ workspace: string }>(); @@ - router.push(`/${workspace?.slug}/apis/${apiId}/keys/${keyspaceId}/${keyData.id}`); + { + const slug = workspace?.slug ?? params?.workspace; + if (!slug) { + toast.error("Failed to Navigate", { + description: "Workspace is not loaded yet. Please try again.", + }); + return; + } + router.push( + `/${encodeURIComponent(slug)}/apis/${encodeURIComponent(apiId)}/keys/${encodeURIComponent( + keyspaceId, + )}/${encodeURIComponent(keyData.id)}`, + ); + }Run a quick scan for other potentially unsafe patterns:
Also applies to: 28-28, 94-95
🏁 Script executed:
Length of output: 27
🏁 Script executed:
Length of output: 9171
Guard against missing workspace slug when navigating (must fix)
router.push builds
/${workspace?.slug}/...which becomes/undefined/...if the provider isn't ready — add a fallback to route params and fail gracefully.File: apps/dashboard/app/(app)/[workspace]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx
import { useEffect, useRef, useState } from "react"; +import { useParams } from "next/navigation"; @@ - const { workspace } = useWorkspace(); + const { workspace } = useWorkspace(); + const params = useParams<{ workspace: string }>(); @@ - router.push(`/${workspace?.slug}/apis/${apiId}/keys/${keyspaceId}/${keyData.id}`); + { + const slug = workspace?.slug ?? params?.workspace; + if (!slug) { + toast.error("Failed to Navigate", { + description: "Workspace is not loaded yet. Please try again.", + }); + return; + } + router.push( + `/${encodeURIComponent(slug)}/apis/${encodeURIComponent(apiId)}/keys/${encodeURIComponent( + keyspaceId, + )}/${encodeURIComponent(keyData.id)}`, + ); + }📝 Committable suggestion
🤖 Prompt for AI Agents