feat: add PAT tab to admin organization details#1576
feat: add PAT tab to admin organization details#1576rohanchkrabrty wants to merge 14 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds an Organization Personal Access Tokens (PAT) management feature: new nested org route, exported view, navbar link, PAT list columns, styles, a details dialog component, and the OrganizationPatView with server-mode table and data fetching. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Review rate limit: 0/1 reviews remaining, refill in 55 minutes and 21 seconds.Comment |
Coverage Report for CI Build 25108823581Coverage remained the same at 42.353%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
web/sdk/admin/views/organizations/details/pat/pat.module.css (2)
19-23: Preferoverflow: autooveroverflow: scroll.
overflow: scrollalways renders scrollbars (both axes) even when content fits, which is visually noisy on macOS/Linux when the table is small.autoonly shows them when needed.♻️ Proposed fix
.table-wrapper { /* Navbar Height + Toolbar height */ max-height: calc(100vh - 90px); - overflow: scroll; + overflow: auto; }
20-21: Magic number for header/toolbar offset.
calc(100vh - 90px)hardcodes the combined navbar + toolbar height. If either control's size changes (zoom, density, theme), the table either over- or under-scrolls. Consider deriving the value via a CSS custom property or measuring with a ref/ResizeObserver.web/sdk/admin/views/organizations/details/pat/components/pat-details-dialog.tsx (2)
33-33: Dialogopenflips on every render after close.
const open = pat !== null;is fine, but combined withenabled: openonuseQueryanddefaultValue="projects"on<Tabs>, opening a different PAT after closing one preserves nothing — that's intentional. Just flagging thatroleEntries(line 72) is recomputed on every render rather than memoized; not a correctness issue, but consideruseMemofor symmetry withscopeProjects/rolesMap.
172-172: Nit: stray space in closing tag.
</Dialog >— minor, but inconsistent with the rest of the file.♻️ Proposed fix
- </Dialog > + </Dialog>web/sdk/admin/views/organizations/details/pat/columns.tsx (1)
8-21: Module-leveldayjs.extend(relativeTime)is fine but global.Calling
dayjs.extend(relativeTime)at module scope mutates the shared dayjs instance. If another part of the app already extends it (or expects vanilla dayjs), this is a no-op / harmless — just noting that side-effectful module init can surprise consumers of this barrel. No change required if dayjs is already centrally extended elsewhere.web/sdk/admin/views/organizations/details/pat/index.tsx (1)
152-153: Memoizedatato avoid prop-identity churn.
infiniteData?.pages?.flatMap(...) ?? []produces a new array every render, which causesDataTableto re-receive a newdatareference each render even when content is unchanged. Wrap inuseMemokeyed oninfiniteData?.pages.♻️ Proposed fix
- const data = - infiniteData?.pages?.flatMap((page) => page?.organizationPats ?? []) ?? []; + const data = useMemo( + () => infiniteData?.pages?.flatMap((page) => page?.organizationPats ?? []) ?? [], + [infiniteData?.pages], + );
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 99248006-e1cb-4420-9d55-384ca65d31b0
📒 Files selected for processing (8)
web/apps/admin/src/routes.tsxweb/sdk/admin/index.tsweb/sdk/admin/views/organizations/details/layout/navbar.tsxweb/sdk/admin/views/organizations/details/pat/columns.tsxweb/sdk/admin/views/organizations/details/pat/components/pat-details-dialog.module.cssweb/sdk/admin/views/organizations/details/pat/components/pat-details-dialog.tsxweb/sdk/admin/views/organizations/details/pat/index.tsxweb/sdk/admin/views/organizations/details/pat/pat.module.css
| { | ||
| accessorKey: "scopes", | ||
| header: "Project", | ||
| classNames: { cell: styles["truncate-cell"] }, | ||
| enableSorting: false, | ||
| cell: ({ row }) => { | ||
| const projectScope = row.original.scopes?.find( | ||
| (scope) => scope.resourceType === SCOPES.PROJECT, | ||
| ); | ||
| const resourceIds = projectScope?.resourceIds ?? []; | ||
| if (resourceIds.length === 0) { | ||
| return <Text className={styles["truncate-text"]}>-</Text>; | ||
| } | ||
| const projectNamesText = resourceIds.map( | ||
| (id) => projectsMap[id]?.title || projectsMap[id]?.name || id, | ||
| ).join(", "); | ||
| return <Text className={styles["truncate-text"]}>{projectNamesText}</Text>; | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Missing tooltip for overflowing project names.
The PR description specifies that the Project column should show overflow names in a 600px-wide tooltip, but the current implementation only truncates via CSS — hovering reveals nothing. When a PAT is scoped to multiple projects, users can't see the full list. Consider wrapping the truncated text in a Tooltip showing the full comma-separated list (constrained to ~600px).
♻️ Sketch
- return <Text className={styles["truncate-text"]}>{projectNamesText}</Text>;
+ return (
+ <Tooltip message={projectNamesText} side="top">
+ <Text className={styles["truncate-text"]}>{projectNamesText}</Text>
+ </Tooltip>
+ );| <Tabs.Content value="projects" className={styles["tab-content"]}> | ||
| {scopeProjects.length === 0 ? null : ( | ||
| <Flex direction="column"> | ||
| {scopeProjects.map((project) => ( | ||
| <div key={project.id} className={styles["list-item"]}> | ||
| <Text | ||
| size="small" | ||
| weight="medium" | ||
| className={styles["list-item-text"]} | ||
| > | ||
| {project.title || project.name} | ||
| </Text> | ||
| </div> | ||
| ))} | ||
| </Flex> | ||
| )} | ||
| </Tabs.Content> | ||
| <Tabs.Content value="roles" className={styles["tab-content"]}> | ||
| {isRolesLoading ? ( | ||
| <Skeleton | ||
| containerClassName={styles["skeleton"]} | ||
| height={20} | ||
| count={4} | ||
| /> | ||
| ) : roleEntries.length === 0 ? ( | ||
| null |
There was a problem hiding this comment.
Empty Projects / Roles tabs render nothing.
When scopeProjects.length === 0 (Projects tab) or roleEntries.length === 0 (Roles tab, post-load), the tab body is null — the user sees a tab labeled "Projects (0)" / "Roles (0)" with a blank panel. EmptyState is already imported; rendering it (or a minimal <Text>No projects</Text>) gives clearer feedback.
♻️ Proposed fix
- <Tabs.Content value="projects" className={styles["tab-content"]}>
- {scopeProjects.length === 0 ? null : (
+ <Tabs.Content value="projects" className={styles["tab-content"]}>
+ {scopeProjects.length === 0 ? (
+ <EmptyState
+ classNames={{ container: styles["empty-state"] }}
+ heading="No projects"
+ />
+ ) : (
<Flex direction="column">
...
</Flex>
)}
</Tabs.Content>
|
|
||
| const fetchMore = async () => { | ||
| if (hasNextPage && !isFetchingNextPage && !isError) { | ||
| await fetchNextPage(); |
There was a problem hiding this comment.
Lets handle the error here with try catch
There was a problem hiding this comment.
try/catch around await fetchNextPage() wouldn't catch anything in normal failure cases, useInfiniteQuery catches the error if any rather than rejecting the promise.
That same error is displayed as ErrorState in DataTable. This is a common pattern across all other pages
4 participants
Summary
PATtab under the admin organization details page that lists the current admin user's personal access tokens for the org.OrganizationPatViewinsdk/adminusingFrontierService.searchCurrentUserPATs(read-only — no create/edit/revoke actions per the design).orgMembersMap), Created On / Expiry Date (DD MMM YYYY), Last used (relative time via dayjs).patroute inapps/admin.