feat(admin): show org slug in detail URLs instead of UUID#1763
feat(admin): show org slug in detail URLs instead of UUID#1763Shreyag02 wants to merge 8 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughOrganization navigation now separates URL slugs from organization IDs carried in router state. Organization details resolve IDs from navigation state or URL lookup, while audit-log links propagate organization state through client-side navigation. ChangesOrganization-aware navigation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/sdk/admin/views/organizations/details/layout/navbar.tsx (2)
241-243: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFallback to
organizationIdif the parsed segment is empty.While
currentPathusually includes the organization slug, adding an explicit fallback protects against edge cases where the extracted segment evaluates to an empty string (e.g., if the path exactly matchesorgPrefixwithout a trailing segment).♻️ Proposed refactor
const orgSegment = currentPath.startsWith(orgPrefix) - ? currentPath.slice(orgPrefix.length).split("/")[0] + ? currentPath.slice(orgPrefix.length).split("/")[0] || organizationId : organizationId;
319-337: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract navigation paths to reduce duplication.
You can extract the string literals for the paths into variables to avoid duplicating them between the
hrefandonNavigatecallbacks.♻️ Proposed refactor
@@ -311,27 +311,28 @@ search.onChange(value); } + const listPath = `/${adminPaths.organizations}`; + const detailsPath = `${listPath}/${organization?.name || organization?.id}`; + return ( <nav className={styles.navbar}> <Flex gap={4} align="center"> <Breadcrumb size="small"> <Breadcrumb.Item - href={`/${adminPaths.organizations}`} + href={listPath} onClick={(e) => { e.preventDefault(); - onNavigate(`/${adminPaths.organizations}`); + onNavigate(listPath); }} leadingIcon={<OrganizationIcon />} > {t.organization({ plural: true, case: "capital" })} </Breadcrumb.Item> <Breadcrumb.Separator /> {/* Navigate client-side so we don't full-reload and lose the held org id. */} <Breadcrumb.Item - href={`/${adminPaths.organizations}/${organization?.name || organization?.id}`} + href={detailsPath} onClick={(e) => { e.preventDefault(); - onNavigate( - `/${adminPaths.organizations}/${organization?.name || organization?.id}`, - ); + onNavigate(detailsPath); }} leadingIcon={
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 95db9072-ec2e-4afc-8448-2a4967c46c75
📒 Files selected for processing (13)
web/apps/admin/src/pages/admins/AdminsPage.tsxweb/apps/admin/src/pages/audit-logs/AuditLogsPage.tsxweb/apps/admin/src/pages/organizations/details/index.tsxweb/apps/admin/src/pages/organizations/list/index.tsxweb/sdk/admin/hooks/useOrganizationLookup.tsweb/sdk/admin/views/admins/columns.tsxweb/sdk/admin/views/admins/index.tsxweb/sdk/admin/views/audit-logs/index.tsxweb/sdk/admin/views/audit-logs/sidepanel-details.tsxweb/sdk/admin/views/audit-logs/sidepanel-list-link.tsxweb/sdk/admin/views/organizations/details/layout/navbar.tsxweb/sdk/admin/views/organizations/list/create.tsxweb/sdk/admin/views/organizations/list/index.tsx
Coverage Report for CI Build 29817588770Coverage remained the same at 46.201%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c0c0b34e-4e9d-45b3-8cc3-956eb05b542a
📒 Files selected for processing (1)
web/apps/admin/src/pages/organizations/details/index.tsx
| useEffect(() => { | ||
| if (paramIsId && org?.name && org.state !== 'disabled') { | ||
| const rest = location.pathname.slice( | ||
| location.pathname.indexOf(urlParam!) + urlParam!.length, | ||
| ); | ||
| navigate(`/${paths.organizations}/${org.name}${rest}`, { | ||
| replace: true, | ||
| state: { orgId }, | ||
| }); | ||
| } | ||
| }, [ | ||
| paramIsId, | ||
| org?.name, | ||
| org?.state, | ||
| orgId, | ||
| urlParam, | ||
| location.pathname, | ||
| navigate, | ||
| paths.organizations, | ||
| ]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Canonical-URL rewrite drops the query string and hash.
rest is sliced from location.pathname only, so a UUID bookmark like /organizations/{id}/members?page=2 rewrites to /organizations/{slug}/members, silently losing ?page=2. Safe fallback (page still loads), but any tab relying on query-string state (filters, pagination) loses it on this one-time redirect.
Proposed fix
useEffect(() => {
if (paramIsId && org?.name && org.state !== 'disabled') {
- const rest = location.pathname.slice(
- location.pathname.indexOf(urlParam!) + urlParam!.length,
- );
- navigate(`/${paths.organizations}/${org.name}${rest}`, {
+ const restPath = location.pathname.slice(
+ location.pathname.indexOf(urlParam!) + urlParam!.length,
+ );
+ navigate(
+ `/${paths.organizations}/${org.name}${restPath}${location.search}${location.hash}`,
+ {
replace: true,
state: { orgId },
- });
+ },
+ );
}
}, [
paramIsId,
org?.name,
org?.state,
orgId,
urlParam,
location.pathname,
+ location.search,
+ location.hash,
navigate,
paths.organizations,
]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (paramIsId && org?.name && org.state !== 'disabled') { | |
| const rest = location.pathname.slice( | |
| location.pathname.indexOf(urlParam!) + urlParam!.length, | |
| ); | |
| navigate(`/${paths.organizations}/${org.name}${rest}`, { | |
| replace: true, | |
| state: { orgId }, | |
| }); | |
| } | |
| }, [ | |
| paramIsId, | |
| org?.name, | |
| org?.state, | |
| orgId, | |
| urlParam, | |
| location.pathname, | |
| navigate, | |
| paths.organizations, | |
| ]); | |
| useEffect(() => { | |
| if (paramIsId && org?.name && org.state !== 'disabled') { | |
| const restPath = location.pathname.slice( | |
| location.pathname.indexOf(urlParam!) + urlParam!.length, | |
| ); | |
| navigate( | |
| `/${paths.organizations}/${org.name}${restPath}${location.search}${location.hash}`, | |
| { | |
| replace: true, | |
| state: { orgId }, | |
| }, | |
| ); | |
| } | |
| }, [ | |
| paramIsId, | |
| org?.name, | |
| org?.state, | |
| orgId, | |
| urlParam, | |
| location.pathname, | |
| location.search, | |
| location.hash, | |
| navigate, | |
| paths.organizations, | |
| ]); |
2 participants
Summary
Admin organization detail pages now show the org's readable slug (its
name) in the URL instead of the raw UUID(e.g.
/organizations/acme-corpinstead of/organizations/6f3a…).All API calls still run on the org id — the slug is display-only — so the change is purely about the URL, not the data layer.
Changes
/organizations/<uuid>links keep working and are rewritten in place to the canonical slug URL once resolved (disabled orgs stay on the id URL).Technical Details
The id is the org's stable primary key and rides along with in-app navigation via router state, so every org RPC (
getOrganization, KYC, members, roles, billing) uses the id directly and never pays a slug→id lookup. The slug is display-only.Navigators pass
{ state: { orgId } }alongside the slug URL. The detail page holds that id across tab switches (which clear router state).With no router state, the page resolves the URL segment via a single
getOrganizationcall. The server'sGetRawaccepts either form — a UUID (GetByID) or a slug (GetByName) — and returns disabled orgs too, so ids, enabled slugs, and disabled slugs all resolve in one request; only a genuinely unknown segment 404s and redirects to the list. Resolution runs in parallel with the id-scoped RPCs, so there's no per-load waterfall.An old UUID URL is
replace-navigated to its slug once resolved (preserving the active sub-tab), so each org has a single live URL and the back-button stays sane. Disabled orgs are left on the id URL — the id is always a valid address for them.useOrganizationLookup) to display its title and build the slug link.Note: an org's
name(slug) is globally unique and is never freed — disabling only flips the org'sstate— so a slug never gets reassigned to a different org and bookmarks stay pointed at the same org.Test Plan
tscclean; SDK rebuilt)SQL Safety (if your PR touches
*_repository.goorgoqu.*)N/A