Conversation
Holocraft
added
enhancement
Contributor
📝 WalkthroughWalkthroughThis PR adds API token expiration date tracking functionality to the UI. It extends the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 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 docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.test.tsx`:
- Around line 132-245: Tests in this block are double-mounting
UserTokenManagementDialog because the parent beforeEach already renders it and
each test calls render(...) again after server.use, causing assertions against
two dialogs; fix by installing handlers first then rendering once per test
(remove or move the parent render in the beforeEach), or if you prefer to keep
the parent render, unmount it before calling render again; alternatively scope
assertions to the new render by capturing its return value (e.g., const {
container } = render(...)) and use within(container) for queries instead of
global screen so each test targets only the freshly rendered dialog (references:
UserTokenManagementDialog, beforeEach, server.use, render, screen,
waitForElementToBeRemoved).
In
`@packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsx`:
- Around line 133-155: The rendering in UserTokenManagementDialog's tokens.map
block can output "Invalid DateTime" when row.expires_at is malformed; update the
logic in the tokens.map callback (where expiresAt, date, daysUntilExpiry,
isExpired, isExpiringSoon are computed) to short-circuit on !date?.isValid
before calling date.toFormat or date.diff — either skip rendering the
date/expiry UI and show an empty/placeholder value or call the same formatter
used by UsersTable.getAPIExpirationDate to centralize validation; ensure any use
of DateTime.fromISO(row.expires_at), date.toFormat('yyyy-MM-dd') and
date.diff(DateTime.now(), 'days') is guarded by date?.isValid to avoid rendering
"Invalid DateTime".
- Line 196: The dialog text in UserTokenManagementDialog still states tokens
“never expire and will remain valid until revoked” even when
apiTokenExpirationEnabled is true; update the explanatory copy in the component
so it conditionally renders different text based on apiTokenExpirationEnabled
(e.g., when true say tokens will expire per the configured expiration date and
are valid until that date or revoked; when false keep the existing “never
expire” wording). Locate the JSX that renders the explanatory paragraph in
UserTokenManagementDialog and replace it with a conditional expression that
outputs the correct message depending on apiTokenExpirationEnabled, ensuring the
new text aligns with the added "API Expiration Date" column.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 9b9ee3b0-f3df-4df9-8116-ac7268e76af1
📒 Files selected for processing (4)
packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.test.tsxpackages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsxpackages/javascript/bh-shared-ui/src/views/Users/UsersTable.tsxpackages/javascript/js-client-library/src/responses.ts
...ipt/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.test.tsx
Outdated
Show resolved
Hide resolved
...vascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsx
Show resolved
Hide resolved
...vascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsx
Show resolved
Hide resolved
Contributor
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)
packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsx (1)
114-120:⚠️ Potential issue | 🟡 MinorApply dynamic
colSpanto the error row for table consistency.Line 117 should span all columns (including optional expiration column), same as loading/empty states.
Suggested fix
} else if (error) { return ( <TableRow> - <TableCell> + <TableCell colSpan={colSpan} align='center'> <div>Error loading user tokens</div> </TableCell> </TableRow> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsx` around lines 114 - 120, The error row in UserTokenManagementDialog currently renders a TableCell without a colSpan; update the error branch to use the same dynamic colSpan used by the loading/empty states so the error message spans all columns (including the optional expiration column). Locate the place where loading/empty rows compute the span (e.g., a variable like colSpan, columnsCount, or a conditional using hasExpiration/includeExpiration) and apply that value to the error TableCell (TableCell colSpan={colSpan} or equivalent) so the error row matches table column layout.
♻️ Duplicate comments (2)
packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsx (2)
183-186:⚠️ Potential issue | 🟠 MajorUpdate dialog copy when token expiration is enabled.
Line 184-185 says tokens never expire, which contradicts the new expiration column and can mislead admins.
Suggested fix
<DialogContentText> - Permanent Authentication Tokens are used for authenticating API calls. Tokens never expire and - will remain valid until revoked. Ensure tokens are stored securely. + {apiTokenExpirationEnabled + ? 'Authentication Tokens are used for authenticating API calls. Tokens may expire based on the configured API token expiration policy or be revoked. Ensure tokens are stored securely.' + : 'Permanent Authentication Tokens are used for authenticating API calls. Tokens never expire and will remain valid until revoked. Ensure tokens are stored securely.'} </DialogContentText>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsx` around lines 183 - 186, The dialog copy in UserTokenManagementDialog currently states tokens never expire; update the DialogContentText inside the UserTokenManagementDialog component to conditionally render two variants based on the token expiration feature (e.g., a prop/flag like isTokenExpirationEnabled or a selector): if expiration is enabled, show copy reflecting that tokens can expire and refer to the Expiration column (e.g., "Tokens may expire based on configured policies; see the Expiration column for details."), otherwise keep the existing "never expire" wording; ensure the conditional targets the DialogContentText element so admins see the correct message.
133-155:⚠️ Potential issue | 🟡 MinorGuard invalid expiration timestamps before rendering the date.
At Line 152, malformed
expires_atcan still renderInvalid DateTimebecause onlydiffis guarded, nottoFormat.Suggested fix
return tokens.map((row) => { const expiresAt = row.expires_at; const date = expiresAt ? DateTime.fromISO(expiresAt) : null; - const daysUntilExpiry = date?.isValid ? date.diff(DateTime.now(), 'days').days : null; + const isValidDate = !!date?.isValid; + const daysUntilExpiry = isValidDate ? date.diff(DateTime.now(), 'days').days : null; const isExpired = daysUntilExpiry !== null && daysUntilExpiry < 0; const isExpiringSoon = daysUntilExpiry !== null && daysUntilExpiry >= 0 && daysUntilExpiry < 10; return ( @@ {apiTokenExpirationEnabled && ( <TableCell> - {isExpired ? ( + {!isValidDate ? null : isExpired ? ( <span className='text-error'>Expired</span> ) : ( <> - <span>{date?.toFormat('yyyy-MM-dd')}</span> + <span>{date.toFormat('yyyy-MM-dd')}</span> {isExpiringSoon && <div className='text-error'><10 Days to Expire</div>} </> )} </TableCell> )}#!/bin/bash # Verify all expiration render paths in both views guard invalid Luxon instances before toFormat. rg -n -C3 "DateTime\\.fromISO|toFormat\\('yyyy-MM-dd'\\)|diff\\(DateTime\\.now\\(\\), 'days'\\)" \ packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsx \ packages/javascript/bh-shared-ui/src/views/Users/UsersTable.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsx` around lines 133 - 155, The expires_at DateTime instance created in UserTokenManagementDialog (inside tokens.map using DateTime.fromISO) is only guarded for diff-based checks but not before calling toFormat; update the rendering logic where date?.toFormat('yyyy-MM-dd') is used (and any other toFormat calls for expires_at) to first verify the Luxon instance is valid (date && date.isValid) and render a safe fallback (e.g., empty string or "Invalid date") when invalid; keep existing isExpired/isExpiringSoon logic that uses daysUntilExpiry but ensure all display paths for expires_at use the validity check before toFormat.
🧹 Nitpick comments (1)
packages/javascript/bh-shared-ui/src/views/Users/UsersTable.tsx (1)
103-116: Extract shared expiration rendering logic to avoid drift.This helper now duplicates behavior implemented in
UserTokenManagementDialog(format, expired label,<10 Days to Expire). Consider centralizing in a shared formatter/component.Refactor direction
- const getAPIExpirationDate = (expiresAt?: string): React.ReactNode => { - if (!expiresAt) return null; - const date = DateTime.fromISO(expiresAt); - if (!date.isValid) return null; - const daysUntilExpiry = date.diff(DateTime.now(), 'days').days; - if (daysUntilExpiry < 0) return <span className='text-error'>Expired</span>; - const isExpiringSoon = daysUntilExpiry < 10; - return ( - <> - <span>{date.toFormat('yyyy-MM-dd')}</span> - {isExpiringSoon && <div className='text-error'><10 Days to Expire</div>} - </> - ); - }; + // Suggestion: move expiration formatting/rendering to a shared utility or small shared component + // and reuse it in both UsersTable and UserTokenManagementDialog.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/Users/UsersTable.tsx` around lines 103 - 116, The getAPIExpirationDate helper in UsersTable duplicates expiration formatting/labels that exist in UserTokenManagementDialog; extract the shared rendering/formatting into a common utility or small presentational component (e.g., export a shared formatExpirationDate function or ExpirationLabel component) and replace the local getAPIExpirationDate implementation with a call to that shared utility in UsersTable and use the same utility from UserTokenManagementDialog so both consume the single source of truth for date parsing (DateTime.fromISO), "Expired" label, date format 'yyyy-MM-dd', and the "<10 Days to Expire" warning.
🤖 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
`@packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsx`:
- Around line 114-120: The error row in UserTokenManagementDialog currently
renders a TableCell without a colSpan; update the error branch to use the same
dynamic colSpan used by the loading/empty states so the error message spans all
columns (including the optional expiration column). Locate the place where
loading/empty rows compute the span (e.g., a variable like colSpan,
columnsCount, or a conditional using hasExpiration/includeExpiration) and apply
that value to the error TableCell (TableCell colSpan={colSpan} or equivalent) so
the error row matches table column layout.
---
Duplicate comments:
In
`@packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsx`:
- Around line 183-186: The dialog copy in UserTokenManagementDialog currently
states tokens never expire; update the DialogContentText inside the
UserTokenManagementDialog component to conditionally render two variants based
on the token expiration feature (e.g., a prop/flag like isTokenExpirationEnabled
or a selector): if expiration is enabled, show copy reflecting that tokens can
expire and refer to the Expiration column (e.g., "Tokens may expire based on
configured policies; see the Expiration column for details."), otherwise keep
the existing "never expire" wording; ensure the conditional targets the
DialogContentText element so admins see the correct message.
- Around line 133-155: The expires_at DateTime instance created in
UserTokenManagementDialog (inside tokens.map using DateTime.fromISO) is only
guarded for diff-based checks but not before calling toFormat; update the
rendering logic where date?.toFormat('yyyy-MM-dd') is used (and any other
toFormat calls for expires_at) to first verify the Luxon instance is valid (date
&& date.isValid) and render a safe fallback (e.g., empty string or "Invalid
date") when invalid; keep existing isExpired/isExpiringSoon logic that uses
daysUntilExpiry but ensure all display paths for expires_at use the validity
check before toFormat.
---
Nitpick comments:
In `@packages/javascript/bh-shared-ui/src/views/Users/UsersTable.tsx`:
- Around line 103-116: The getAPIExpirationDate helper in UsersTable duplicates
expiration formatting/labels that exist in UserTokenManagementDialog; extract
the shared rendering/formatting into a common utility or small presentational
component (e.g., export a shared formatExpirationDate function or
ExpirationLabel component) and replace the local getAPIExpirationDate
implementation with a call to that shared utility in UsersTable and use the same
utility from UserTokenManagementDialog so both consume the single source of
truth for date parsing (DateTime.fromISO), "Expired" label, date format
'yyyy-MM-dd', and the "<10 Days to Expire" warning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: be8b0b86-c883-47e9-886e-6885bb73aa83
📒 Files selected for processing (4)
packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.test.tsxpackages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.tsxpackages/javascript/bh-shared-ui/src/views/Users/UsersTable.tsxpackages/javascript/js-client-library/src/responses.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/javascript/js-client-library/src/responses.ts
- packages/javascript/bh-shared-ui/src/components/UserTokenManagementDialog/UserTokenManagementDialog.test.tsx
| </TableRow> | ||
| )); | ||
| return tokens.map((row) => { | ||
| const expiresAt = row.expires_at; |
Contributor
There was a problem hiding this comment.
I'm seeing a linting error on this row. 'Property 'expires_at' does not exist on type 'AuthToken'.'
Contributor
Author
There was a problem hiding this comment.
This is in the js-client-library so you might have to do a build there to get rid of the error.
Contributor
There was a problem hiding this comment.
Building the js-client-library did it. Thanks!
jvacca-specterops
approved these changes
Mar 23, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Description
This changeset adds columns for the API key expiration date to the Manage Clients and Manage Users tables in Administration.
Motivation and Context
Resolves BED-7504
How Has This Been Tested?
Unit tests were added as well as testing manually
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Tests