Show Versions#540
Conversation
📝 WalkthroughWalkthroughA version tracking system is added to the dashboard. The GitHub workflow extracts version information from repository tags, makes it available during the Next.js build, and a new VersionInfo component displays current versus available versions in the navigation UI with update availability indicators. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 4
🤖 Fix all issues with AI agents
In `@src/components/VersionInfo.tsx`:
- Around line 19-33: The compareVersions function fails for versions with a
leading "v" or pre-release/build suffixes; update compareVersions to normalize
both inputs before splitting and parsing by (1) trimming whitespace, removing a
leading "v" or "V", and stripping anything after a hyphen or plus
(pre-release/build metadata), (2) then splitting on "." and parsing numeric
parts with parseInt (defaulting to 0) and (3) preserving the existing
loop/comparison logic; reference the compareVersions function and variables
currentParts/latestParts to locate where to apply the normalization.
In `@src/modules/users/UserInviteModal.tsx`:
- Line 326: The h2 in UserInviteModal.tsx has a malformed className string
("text-lg my-0 leading-[1.5 text-center") — the leading utility is missing a
closing bracket; update the className for the <h2> (the element rendering
getTitle()) to include the closing bracket so it reads like "text-lg my-0
leading-[1.5] text-center" (or equivalent valid Tailwind syntax) to fix the CSS
syntax error.
In `@src/modules/users/UserInvitesTable.tsx`:
- Around line 411-422: The default sort state in the useLocalStorage call
(variables sorting, setSorting) uses an invalid field id "is_current" which
doesn't exist on UserInviteListItem; update the initial array passed to
useLocalStorage("netbird-table-sort-invites"+path, [...]) to remove or replace
the first object { id: "is_current", desc: true } with a real property name from
UserInviteListItem (for example "status", "isCurrent", "created_at", or another
existing key), so the table sorting works against a valid field.
In `@src/utils/unauthenticatedApi.ts`:
- Around line 57-69: The invite token is interpolated directly into the path in
fetchInviteInfo and acceptInvite which can break routing for tokens containing
reserved characters; before calling unauthenticatedRequest, URL-encode the token
(e.g., via encodeURIComponent) and use the encoded value in the path
`/users/invites/${encodedToken}` and `/users/invites/${encodedToken}/accept` so
the routes are safe and not treated as extra path segments.
🧹 Nitpick comments (7)
src/interfaces/User.ts (1)
20-64: PreferRoleenum for invite role fields.
Keeps role values consistent across the app and avoids stringly-typed roles.♻️ Suggested refactor
export interface UserInviteCreateRequest { email: string; name: string; - role: string; + role: Role; auto_groups: string[]; expires_in?: number; } export interface UserInviteCreateResponse { id: string; email: string; name: string; - role: string; + role: Role; auto_groups: string[]; status: string; invite_link: string; invite_expires_at: string; } @@ export interface UserInviteListItem { id: string; email: string; name: string; - role: string; + role: Role; auto_groups: string[]; expires_at: string; created_at: string; expired: boolean; }src/app/invite/page.tsx (2)
74-75: Special character validation may reject common characters.The regex
[!@#$%^&*(),.?":{}|<>]excludes common special characters like-,_,/,\,[,],~, and+. Consider expanding the set or aligning the hint text (line 263) with the actual validation.♻️ Suggested improvement
- const hasSpecialChar = /[!@#$%^&*(),.?":{}|<>]/.test(password); + const hasSpecialChar = /[!@#$%^&*(),.?":{}|<>\-_\[\]\\\/;'`~=+]/.test(password);And update the hint accordingly:
- <PasswordRule met={hasSpecialChar} text="One special character (!@#$%^&*)" /> + <PasswordRule met={hasSpecialChar} text="One special character" />
95-98: Redundant client-side expiration check.The
isExpiredcomputation duplicates the server'svalidfield check. SinceinviteInfo.validalready accounts for expiration server-side, the client-side date comparison is redundant and could cause false negatives if the user's clock is skewed.Consider relying solely on
inviteInfo.valid:♻️ Suggested simplification
- const isExpired = useMemo(() => { - if (!inviteInfo) return false; - return new Date(inviteInfo.expires_at) < new Date(); - }, [inviteInfo]); - // ... - if (isExpired || !inviteInfo?.valid) { + if (!inviteInfo?.valid) {src/modules/users/UserInviteModal.tsx (1)
231-231: Redundant null check on required callback.
onUserCreatedis a required prop inModalProps, so the&&guard is unnecessary.♻️ Optional cleanup
- onUserCreated && onUserCreated(user); + onUserCreated(user);Same applies to line 253:
- onInviteCreated && onInviteCreated(invite); + onInviteCreated(invite);src/modules/users/UserInvitesTable.tsx (2)
503-532: Duplicate InviteUserButton component.This component is duplicated from
src/modules/users/UsersTable.tsx(lines 297-326). Consider extracting it to a shared module to avoid maintenance burden.#!/bin/bash # Verify the duplication rg -n -A 35 'export const InviteUserButton' src/modules/users/
479-481: Use strict equality.Prefer
===over==for consistency.♻️ Suggested fix
- <DataTableRowsPerPage table={table} disabled={invites?.length == 0} /> + <DataTableRowsPerPage table={table} disabled={invites?.length === 0} /> <DataTableRefreshButton - isDisabled={invites?.length == 0} + isDisabled={invites?.length === 0}src/modules/users/UsersTable.tsx (1)
146-152: Confirm invite-toggle eligibility and guardaccount.settingsaccess.If
settingscan be undefined,account?.settings.embedded_idp_enabledwill throw. Also, please confirm whether cloud tenants should be able to view invites; the current condition hides the toggle on cloud.♻️ Suggested null-safety tweak
- const embeddedIdpEnabled = account?.settings.embedded_idp_enabled; + const embeddedIdpEnabled = account?.settings?.embedded_idp_enabled ?? false;
| function compareVersions(current: string, latest: string): boolean { | ||
| // Returns true if latest is newer than current | ||
| if (!current || !latest) return false; | ||
| if (current === "development") return false; | ||
|
|
||
| const currentParts = current.split(".").map((p) => parseInt(p, 10) || 0); | ||
| const latestParts = latest.split(".").map((p) => parseInt(p, 10) || 0); | ||
|
|
||
| for (let i = 0; i < Math.max(currentParts.length, latestParts.length); i++) { | ||
| const c = currentParts[i] || 0; | ||
| const l = latestParts[i] || 0; | ||
| if (l > c) return true; | ||
| if (l < c) return false; | ||
| } | ||
| return false; |
There was a problem hiding this comment.
Fix version comparison for “vX.Y.Z” / pre-release strings.
parseInt("v1") yields NaN, so current logic can miss updates or show false positives when API returns versions with a leading “v” or suffixes. Normalize before parsing.
🛠️ Suggested fix
-function compareVersions(current: string, latest: string): boolean {
+function compareVersions(current: string, latest: string): boolean {
// Returns true if latest is newer than current
if (!current || !latest) return false;
if (current === "development") return false;
- const currentParts = current.split(".").map((p) => parseInt(p, 10) || 0);
- const latestParts = latest.split(".").map((p) => parseInt(p, 10) || 0);
+ const normalize = (v: string) =>
+ v
+ .trim()
+ .replace(/^v/i, "")
+ .split(/[^\d]+/)
+ .filter(Boolean)
+ .map((p) => parseInt(p, 10));
+
+ const currentParts = normalize(current);
+ const latestParts = normalize(latest);
+ if (!currentParts.length || !latestParts.length) return false;
for (let i = 0; i < Math.max(currentParts.length, latestParts.length); i++) {
const c = currentParts[i] || 0;
const l = latestParts[i] || 0;
if (l > c) return true;
if (l < c) return false;
}
return false;
}📝 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.
| function compareVersions(current: string, latest: string): boolean { | |
| // Returns true if latest is newer than current | |
| if (!current || !latest) return false; | |
| if (current === "development") return false; | |
| const currentParts = current.split(".").map((p) => parseInt(p, 10) || 0); | |
| const latestParts = latest.split(".").map((p) => parseInt(p, 10) || 0); | |
| for (let i = 0; i < Math.max(currentParts.length, latestParts.length); i++) { | |
| const c = currentParts[i] || 0; | |
| const l = latestParts[i] || 0; | |
| if (l > c) return true; | |
| if (l < c) return false; | |
| } | |
| return false; | |
| function compareVersions(current: string, latest: string): boolean { | |
| // Returns true if latest is newer than current | |
| if (!current || !latest) return false; | |
| if (current === "development") return false; | |
| const normalize = (v: string) => | |
| v | |
| .trim() | |
| .replace(/^v/i, "") | |
| .split(/[^\d]+/) | |
| .filter(Boolean) | |
| .map((p) => parseInt(p, 10)); | |
| const currentParts = normalize(current); | |
| const latestParts = normalize(latest); | |
| if (!currentParts.length || !latestParts.length) return false; | |
| for (let i = 0; i < Math.max(currentParts.length, latestParts.length); i++) { | |
| const c = currentParts[i] || 0; | |
| const l = latestParts[i] || 0; | |
| if (l > c) return true; | |
| if (l < c) return false; | |
| } | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In `@src/components/VersionInfo.tsx` around lines 19 - 33, The compareVersions
function fails for versions with a leading "v" or pre-release/build suffixes;
update compareVersions to normalize both inputs before splitting and parsing by
(1) trimming whitespace, removing a leading "v" or "V", and stripping anything
after a hyphen or plus (pre-release/build metadata), (2) then splitting on "."
and parsing numeric parts with parseInt (defaulting to 0) and (3) preserving the
existing loop/comparison logic; reference the compareVersions function and
variables currentParts/latestParts to locate where to apply the normalization.
| <h2 className={"text-lg my-0 leading-[1.5 text-center]"}> | ||
| {isNetBirdHosted() ? "Invite User" : "Create User"} | ||
| </h2> | ||
| <h2 className={"text-lg my-0 leading-[1.5 text-center]"}>{getTitle()}</h2> |
There was a problem hiding this comment.
Fix CSS class syntax error.
There's a typo in the className - missing closing bracket for the leading value.
🐛 Proposed fix
- <h2 className={"text-lg my-0 leading-[1.5 text-center]"}>{getTitle()}</h2>
+ <h2 className={"text-lg my-0 leading-[1.5] text-center"}>{getTitle()}</h2>📝 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.
| <h2 className={"text-lg my-0 leading-[1.5 text-center]"}>{getTitle()}</h2> | |
| <h2 className={"text-lg my-0 leading-[1.5] text-center"}>{getTitle()}</h2> |
🤖 Prompt for AI Agents
In `@src/modules/users/UserInviteModal.tsx` at line 326, The h2 in
UserInviteModal.tsx has a malformed className string ("text-lg my-0 leading-[1.5
text-center") — the leading utility is missing a closing bracket; update the
className for the <h2> (the element rendering getTitle()) to include the closing
bracket so it reads like "text-lg my-0 leading-[1.5] text-center" (or equivalent
valid Tailwind syntax) to fix the CSS syntax error.
| const [sorting, setSorting] = useLocalStorage<SortingState>( | ||
| "netbird-table-sort-invites" + path, | ||
| [ | ||
| { | ||
| id: "is_current", | ||
| desc: true, | ||
| }, | ||
| { | ||
| id: "name", | ||
| desc: true, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Invalid default sort field.
The default sorting references is_current which doesn't exist in UserInviteListItem. This will cause the sort to have no effect or potentially error.
🐛 Proposed fix
const [sorting, setSorting] = useLocalStorage<SortingState>(
"netbird-table-sort-invites" + path,
[
- {
- id: "is_current",
- desc: true,
- },
{
id: "name",
desc: true,
},
+ {
+ id: "expires_at",
+ desc: false,
+ },
],
);📝 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.
| const [sorting, setSorting] = useLocalStorage<SortingState>( | |
| "netbird-table-sort-invites" + path, | |
| [ | |
| { | |
| id: "is_current", | |
| desc: true, | |
| }, | |
| { | |
| id: "name", | |
| desc: true, | |
| }, | |
| ], | |
| const [sorting, setSorting] = useLocalStorage<SortingState>( | |
| "netbird-table-sort-invites" + path, | |
| [ | |
| { | |
| id: "name", | |
| desc: true, | |
| }, | |
| { | |
| id: "expires_at", | |
| desc: false, | |
| }, | |
| ], | |
| ); |
🤖 Prompt for AI Agents
In `@src/modules/users/UserInvitesTable.tsx` around lines 411 - 422, The default
sort state in the useLocalStorage call (variables sorting, setSorting) uses an
invalid field id "is_current" which doesn't exist on UserInviteListItem; update
the initial array passed to useLocalStorage("netbird-table-sort-invites"+path,
[...]) to remove or replace the first object { id: "is_current", desc: true }
with a real property name from UserInviteListItem (for example "status",
"isCurrent", "created_at", or another existing key), so the table sorting works
against a valid field.
| export async function fetchInviteInfo(token: string): Promise<UserInviteInfo> { | ||
| return unauthenticatedRequest<UserInviteInfo>("GET", `/users/invites/${token}`); | ||
| } | ||
|
|
||
| export async function acceptInvite( | ||
| token: string, | ||
| password: string, | ||
| ): Promise<UserInviteAcceptResponse> { | ||
| return unauthenticatedRequest<UserInviteAcceptResponse>( | ||
| "POST", | ||
| `/users/invites/${token}/accept`, | ||
| { password }, | ||
| ); |
There was a problem hiding this comment.
URL-encode invite tokens before path interpolation.
Unencoded tokens can break routing or be interpreted as extra path segments.
🛠️ Suggested fix
export async function fetchInviteInfo(token: string): Promise<UserInviteInfo> {
- return unauthenticatedRequest<UserInviteInfo>("GET", `/users/invites/${token}`);
+ const encodedToken = encodeURIComponent(token);
+ return unauthenticatedRequest<UserInviteInfo>(
+ "GET",
+ `/users/invites/${encodedToken}`,
+ );
}
export async function acceptInvite(
token: string,
password: string,
): Promise<UserInviteAcceptResponse> {
+ const encodedToken = encodeURIComponent(token);
return unauthenticatedRequest<UserInviteAcceptResponse>(
"POST",
- `/users/invites/${token}/accept`,
+ `/users/invites/${encodedToken}/accept`,
{ password },
);
}🤖 Prompt for AI Agents
In `@src/utils/unauthenticatedApi.ts` around lines 57 - 69, The invite token is
interpolated directly into the path in fetchInviteInfo and acceptInvite which
can break routing for tokens containing reserved characters; before calling
unauthenticatedRequest, URL-encode the token (e.g., via encodeURIComponent) and
use the encoded value in the path `/users/invites/${encodedToken}` and
`/users/invites/${encodedToken}/accept` so the routes are safe and not treated
as extra path segments.
|
@CodeRabbit, review this Pr and update description again now that it is pointing to the right target |
|
✅ Actions performedSummary regeneration triggered. |
2 participants
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.