Conversation
- Added new API endpoints for managing teams, including fetching, creating, updating, and deleting teams. - Updated the site retrieval logic to incorporate team-based access control, allowing users to filter sites based on team membership. - Enhanced the frontend to support team selection in site filtering, improving user experience in managing site access. - Introduced new database schema for teams and their relationships with members and sites, ensuring robust data handling.
- Added functionality to allow team assignments when inviting new members to an organization. - Updated the invitation API to handle team IDs, ensuring that team memberships are validated and stored correctly. - Enhanced the frontend to support team selection during the invitation process, improving user experience. - Modified database schema to include team IDs in the invitation records, facilitating better access control and organization management.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughIntroduces team management functionality across the application, including new database tables for teams and team-site access, REST API endpoints for CRUD operations, client-side hooks and components for managing teams, team-based site access restrictions for members, and team selection during member invitations. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Member)
participant Client as Client App
participant Server as Server
participant DB as Database
User->>Client: Request sites list
Client->>Server: GET /organizations/{orgId}/sites
Server->>DB: Query user's member roles for orgs
DB-->>Server: Member role records
Server->>DB: Query all team-gated site IDs in user's orgs
DB-->>Server: Team-gated site IDs
Server->>DB: Query teams user belongs to (teamMember)
DB-->>Server: User's team IDs
Server->>DB: Query sites team access grants (teamSiteAccess)
DB-->>Server: Team-accessible site IDs
Server->>Server: Filter sites: exclude team-gated unless user's teams grant access
Server->>DB: Fetch team metadata for returned sites
DB-->>Server: Team associations (id, name)
Server-->>Client: Sites with teams field (filtered by access)
Client->>Client: Render sites with team filter dropdown
User->>Client: Select team filter
Client->>Client: Update filteredSites (match selected team or unassigned)
Client-->>User: Display filtered sites
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
client/src/app/page.tsx (1)
63-81:⚠️ Potential issue | 🟠 MajorClear invalid team filters after switching organizations.
selectedTeamFilterpersists across org switches. If it still points at a team from the previous org,filteredSitesbecomes empty; and when the new org has no teams the select is hidden, so there's no visible way to recover from the stale filter.Also applies to: 83-94, 151-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/page.tsx` around lines 63 - 81, selectedTeamFilter can remain pointing at a team from the previous organization and produce empty filteredSites after org switch; add an effect that resets selectedTeamFilter to "all" when the organization or its teams change (watch activeOrganization?.id and/or teams from useTeams) so stale team IDs are cleared; use setSelectedTeamFilter("all") in that effect and ensure the UI still treats "all" as the default when no teams exist.server/src/lib/auth.ts (1)
220-285:⚠️ Potential issue | 🔴 CriticalDon't swallow access-copy failures after acceptance.
This runs after the invitation has already been accepted. If either the site-access copy or the new team-membership insert fails (for example because a selected team was deleted or an insert hits a constraint), the catch only logs it and the user can remain in the organization without the intended restrictions/team assignments. This needs to be part of the same acceptance transaction, or followed by a compensating rollback of the newly created membership.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/lib/auth.ts` around lines 220 - 285, The invitation-to-member access-copy currently runs in its own try/catch and only logs errors, which can leave a newly created membership without intended site restrictions or team assignments; move the access-copy logic into the same database transaction that creates the membership (use the same db.transaction scope and tx used to persist the member) so failures abort the whole acceptance, or if you must keep it separate, perform a compensating rollback by deleting the newly created member and any partial inserts on error; update the code paths around invitationRecord/userRecord and replace the separate db.transaction block (and its tx usage that inserts into memberSiteAccess and teamMember and updates member) so they execute inside the original acceptance transaction that creates member (referencing invitation, invitationRecord, userRecord, memberRecord, memberSiteAccess, teamMember, and tx) to ensure atomicity.
🧹 Nitpick comments (4)
server/src/db/postgres/schema.ts (1)
216-221: Inconsistent table naming convention.The
teamMembertable uses camelCase ("teamMember") while other tables in this file use snake_case (e.g.,"team_site_access","member_site_access"). This inconsistency could cause confusion when writing raw SQL queries or debugging.🔧 Suggested fix for consistency
-export const teamMember = pgTable("teamMember", { +export const teamMember = pgTable("team_member", { id: text("id").primaryKey(), teamId: text("team_id").notNull().references(() => team.id, { onDelete: "cascade" }), userId: text("user_id").notNull().references(() => user.id, { onDelete: "cascade" }), createdAt: timestamp("created_at", { mode: "string" }), });Note: If you proceed with this change, ensure the migration SQL is updated accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/db/postgres/schema.ts` around lines 216 - 221, The table name string for the teamMember pgTable is inconsistent (uses "teamMember"); change the pgTable invocation so the database table name is snake_case (e.g., "team_member") to match the rest of the schema, update any raw SQL/migration SQL that refers to the old name, and verify references to the exported constant teamMember (and any foreign-key usages) still work after renaming; locate the instantiation via the teamMember variable and the pgTable("teamMember", ...) call to make the change.server/src/api/sites/getSitesFromOrg.ts (1)
53-91: Duplicate team-based filtering logic with auth-utils.ts.This team filtering duplicates logic already present in
getSitesUserHasAccessTo(auth-utils.ts:146-207), which runs before this handler is called. Both perform the same filtering: finding team-gated sites, checking user team memberships, and removing inaccessible sites.While the duplication produces correct results, it introduces:
- Extra database queries on every request
- Maintenance burden if filtering rules need to change (must update both locations)
Consider consolidating the team filtering in one location—either fully in
auth-utils.ts(for consistency with other access checks) or fully here (if org-specific context is needed).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/api/sites/getSitesFromOrg.ts` around lines 53 - 91, This handler duplicates team-based filtering already implemented in getSitesUserHasAccessTo (auth-utils.ts); remove the entire memberRecord role==="member" block that queries teamSiteAccess/team and teamMember and instead rely on (or call) getSitesUserHasAccessTo to filter sites for the current user/organization, or extract the shared logic into a single helper used by both; ensure references to memberRecord, teamSiteAccess, teamMember, team, and sitesData are cleaned up so no redundant DB queries remain and the handler uses the centralized access check.server/src/api/teams/listTeams.ts (1)
20-40: Consider returning 403 for unauthorized org access instead of empty list.If a user is not a member of the organization, the handler returns an empty teams list rather than a proper authorization error. While this doesn't leak data, it provides inconsistent UX—users might think there are simply no teams rather than understanding they lack access.
💡 Suggested improvement
const memberRecord = userId ? await db .select({ id: member.id, role: member.role }) .from(member) .where( and( eq(member.organizationId, organizationId), eq(member.userId, userId) ) ) .limit(1) : []; + if (memberRecord.length === 0) { + return reply.status(403).send({ error: "Not authorized to access this organization" }); + } + const isAdminOrOwner = memberRecord.length > 0 && (memberRecord[0].role === "admin" || memberRecord[0].role === "owner");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/api/teams/listTeams.ts` around lines 20 - 40, The handler currently treats a missing membership (memberRecord) as an empty teams response; update the logic in listTeams.ts to return a 403 Unauthorized when the requesting user is not a member of the organization. Specifically, after computing memberRecord and isAdminOrOwner (using request.params.organizationId, request.user?.id / getUserIdFromRequest, and the db query against member), if memberRecord.length === 0 (i.e., user has no membership) respond with a 403 error (or reply.code(403).send) instead of continuing to fetch/return an empty teams list; preserve existing behavior for admins/owners (isAdminOrOwner === true) and for any other allowed flows.server/src/api/teams/updateTeam.ts (1)
91-96: Consider validating non-empty name on server side.If
nameis provided but trims to an empty string, this will update the team with an empty name. The client validates this, but server-side validation provides defense in depth.🛡️ Proposed fix
const updates: Record<string, string> = { updatedAt: now }; if (name !== undefined) { + if (!name.trim()) { + return reply.status(400).send({ error: "Team name cannot be empty" }); + } updates.name = name.trim(); }Note: This validation should occur before the transaction starts, around lines 88-89.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/api/teams/updateTeam.ts` around lines 91 - 96, The code allows a provided name to be trimmed to an empty string and saved; before starting the transaction (i.e. before building updates/await tx.update(team).set(...)), validate the incoming name: if name !== undefined then compute const trimmed = name.trim(); if trimmed === "" return/throw a 400-style error (or BadRequest) and do not proceed; otherwise assign updates.name = trimmed. Apply this check near where now and updates are prepared so teamId, updates, and tx.update(team).set(...) never run with an empty name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/messages/cs.json`:
- Around line 667-669: Several Czech localization keys are empty (e.g.,
"WyooVh", "DoskyC", "t6nrnE" and other keys referenced around the same change
set), causing missing UI text for Czech users; update the JSON entries by
providing appropriate Czech translations for those keys (team filter labels,
team management dialogs, member/site selection strings) or, if you cannot
translate now, add clear TODO comments and fallback English values for each key
so the UI remains readable; ensure you modify the same keys (WyooVh, DoskyC,
t6nrnE, plus the other empty keys noted near lines ~769, ~811-812, ~844-869) in
the locale JSON to include completed translations or temporary fallbacks.
In `@client/messages/es.json`:
- Around line 667-669: The JSON file contains new translation keys ("WyooVh",
"DoskyC", "t6nrnE" and other empty keys in the ranges noted) with empty string
values causing Spanish users to see blanks; update client/messages/es.json by
providing Spanish translations for each empty key (replace the empty values with
the correct Spanish strings for the corresponding team management UI labels),
ensure keys match exactly and keep JSON valid, and if you cannot supply final
translations now, add a clear placeholder comment or track these keys in a
follow-up task so they're not released untranslated.
In `@client/messages/fr.json`:
- Around line 667-669: Several newly added French translation keys (e.g.,
"WyooVh", "DoskyC", "t6nrnE" and other empty keys around the same hunk) are
blank; please provide proper French strings for each missing Teams/invitation
key so the locale isn't shipped partially untranslated—open the same JSON file
and replace the empty values for the listed keys (and the other empty entries
noted near lines 769, 811, and 844–869) with the correct French translations
matching the English source keys.
In `@client/messages/ja.json`:
- Around line 667-669: The ja.json contains empty translation values for new
Teams keys (e.g., "WyooVh", "DoskyC", "t6nrnE") which will produce blank UI; fix
by populating these keys with either proper Japanese translations or English
placeholder strings, and implement a fallback merge in the i18n loader: update
getRequestConfig to merge locale messages with the English messages (default
locale) so missing keys fall back to English (use next-intl recommended merge
pattern). Ensure all other affected locale files (de.json, es.json, fr.json,
it.json, ko.json, pl.json, pt.json, zh.json, cs.json) are either filled or rely
on the new fallback behavior.
In `@client/messages/ko.json`:
- Around line 667-669: Several new localization keys (e.g., "WyooVh", "DoskyC",
"t6nrnE" and other empty entries around the same change set) were committed with
empty strings; fill each empty value with the proper Korean translation for the
label/dialog/filter they represent, following the project's translation
guidelines for files in messages/; update the keys listed in this diff and the
other empty keys noted (other empty entries near the same commit ranges) with
accurate Korean copy, run the repository's i18n/translation validation or
linting to ensure no empty values remain, and submit the updated ko.json entries
in the same format as other locales.
In `@client/messages/pl.json`:
- Around line 667-669: Backfill the missing Polish translations for the new
Teams/invitation keys by replacing the empty values for the keys "WyooVh",
"DoskyC", and "t6nrnE" (and any other empty keys in the same locale block noted
around the ranges mentioned) with appropriate Polish strings; ensure the
translations match the intent of the English originals and preserve any
interpolation/placeholders, then run a quick locale validation to ensure no
empty values remain in the Teams/invitation section.
In `@client/messages/pt.json`:
- Around line 667-669: Backfill the missing Portuguese translations for the new
Teams strings by supplying Portuguese text for the empty keys (e.g., "WyooVh",
"DoskyC", "t6nrnE") and replace the English site-count label "{count} sites"
with a proper Portuguese pluralized string (e.g., "{count} sites" -> "{count}
sites" or locale-appropriate variant such as "{count} sites" translated to
Portuguese) so the Teams/invitation flow is fully localized; update all other
newly added empty keys in the same file ranges referenced in the comment (the
additional empty entries around the noted ranges) to ensure no untranslated
placeholders remain.
In `@client/messages/zh.json`:
- Around line 667-669: The new locale keys "WyooVh", "DoskyC", and "t6nrnE" in
client/messages/zh.json are empty, causing blank UI text; open the corresponding
English keys in client/messages/en.json (and the other empty ranges around lines
noted: 769, 811, 844-869) and provide proper Chinese translations for each key
(copy the EN string and translate it into Chinese, preserving punctuation and
placeholders), update the zh.json entries with those translated values, and run
a quick linter/JSON validator to ensure no syntax errors after inserting the
strings.
In `@client/src/api/admin/endpoints/teams.ts`:
- Around line 47-53: The createTeam client currently returns
authedFetch<Team>(...) but the server returns members: string[] and siteIds:
number[] (and uses the field name siteIds), which doesn't match the Team shape
(members: TeamMember[], sites: TeamSite[]), causing runtime errors; update
createTeam (or the useCreateTeam hook) to either (A) request the actual server
response type and transform it into the client Team shape by mapping members
string[] -> TeamMember[] (e.g., { userId: id, userName: "", userEmail: "" } or
fill from user lookup) and map siteIds number[] -> TeamSite[] with { siteId: id,
domain: "", name: "" } and rename siteIds -> sites, or (B) change the TypeScript
return type to match the server response (e.g., use an explicit
ServerCreateTeamResponse type) so callers handle the flat arrays; implement the
chosen approach inside createTeam (the authedFetch call) or immediately after
its result so downstream code expecting Team (e.g., useCreateTeam consumers)
receives the correct shape.
In
`@client/src/app/settings/organization/members/components/InviteMemberDialog.tsx`:
- Around line 51-58: When organizationId (or the teams data returned by
useTeams) changes you must clear stale selections so invites don’t submit
invalid team IDs: add a useEffect watching organizationId or teams (from
teamsData) and call setSelectedTeamIds([]) (and also clear selectedSiteIds via
setSelectedSiteIds([]) if site lists change) to reset selections; update the
logic around selectedTeamIds/selectedSiteIds (used by the invite mutation in
InviteMemberDialog) so the arrays cannot persist across org switches and cause
server validation errors.
In
`@client/src/app/settings/organization/teams/components/CreateEditTeamDialog.tsx`:
- Line 3: Replace the incorrect next-intl import and usages: change the import
named useExtracted to useTranslations (i.e., import { useTranslations } from
"next-intl") and update every call site in CreateEditTeamDialog (e.g., calls
like useExtracted("...") on line 41) to useTranslations("...") instead; ensure
any destructured values or variable names derived from the hook are updated
accordingly so the component uses useTranslations consistently.
In `@client/src/app/settings/organization/teams/components/DeleteTeamDialog.tsx`:
- Line 3: The DeleteTeamDialog component imports the wrong i18n hook; replace
the invalid useExtracted import from next-intl with useTranslations and update
any calls to useExtracted(...) (e.g., at the useExtracted(...) call around line
27 inside DeleteTeamDialog) to use useTranslations(...), keeping the same
translation namespace/key string and variable name where used so existing t(...)
calls continue to work.
In `@client/src/app/settings/organization/teams/page.tsx`:
- Around line 23-25: The page title is hard-coded in TeamsPage; replace the
literal "Organization Teams" by using next-intl's useTranslations and pass the
localized string into useSetPageTitle. In TeamsPage, import useTranslations from
'next-intl', call const t = useTranslations('<namespace>') (or the existing
translation namespace used by other strings on this page) and then call
useSetPageTitle(t('organizationTeams') or the chosen key); also add the
organizationTeams key to your messages/*.json translation files (en.json,
de.json, etc.) so the title is localized.
- Around line 28-30: The UI currently conflates a failed fetch with an empty
team list because it only checks teamsData and teamsLoading; update the render
logic that uses useTeams (symbols: useTeams, teamsData, teamsLoading) to also
check React Query's isError and error from the hook and handle errors explicitly
(e.g., render an error message or rethrow so the error boundary surfaces the
failure) before falling back to the "No teams yet" empty state; ensure the flow
is: if (teamsLoading) show spinner, else if (isError) show error UI or surface
error, else if (teamsData?.length === 0) show empty state. This change should be
applied wherever the empty-state rendering occurs (the blocks currently using
teamsData/teamsLoading).
In `@server/drizzle/0002_short_bishop.sql`:
- Around line 9-14: The teamMember table allows duplicate memberships for the
same team/user pair; add a uniqueness constraint on the (team_id, user_id) pair
to enforce one row per membership. Modify the CREATE TABLE for "teamMember" (or
add an ALTER TABLE) to include a UNIQUE(team_id, user_id) constraint (or replace
the current "id" primary key with a composite primary key on team_id and user_id
if you prefer) so inserts for the same team_id and user_id will be rejected at
the DB level.
In `@server/src/api/teams/createTeam.ts`:
- Around line 13-17: Validate and normalize the incoming CreateTeamBody
(currently represented by the CreateTeamBody interface and accessed via
request.body) using Zod before any DB work: define a Zod schema for name
(string), memberUserIds (optional array of strings) and siteIds (optional array
of numbers), parse request.body and return a 400 on failure, then normalize by
filtering out falsy values and deduplicating memberUserIds and siteIds (so
duplicates won't create duplicate teamMember rows or trigger the
team_site_access_unique DB constraint). Replace direct usage of request.body in
the create-team handler with the validated result and use the deduped arrays
when inserting into teamMember and team_site_access tables to prevent DB errors
and membership corruption.
---
Outside diff comments:
In `@client/src/app/page.tsx`:
- Around line 63-81: selectedTeamFilter can remain pointing at a team from the
previous organization and produce empty filteredSites after org switch; add an
effect that resets selectedTeamFilter to "all" when the organization or its
teams change (watch activeOrganization?.id and/or teams from useTeams) so stale
team IDs are cleared; use setSelectedTeamFilter("all") in that effect and ensure
the UI still treats "all" as the default when no teams exist.
In `@server/src/lib/auth.ts`:
- Around line 220-285: The invitation-to-member access-copy currently runs in
its own try/catch and only logs errors, which can leave a newly created
membership without intended site restrictions or team assignments; move the
access-copy logic into the same database transaction that creates the membership
(use the same db.transaction scope and tx used to persist the member) so
failures abort the whole acceptance, or if you must keep it separate, perform a
compensating rollback by deleting the newly created member and any partial
inserts on error; update the code paths around invitationRecord/userRecord and
replace the separate db.transaction block (and its tx usage that inserts into
memberSiteAccess and teamMember and updates member) so they execute inside the
original acceptance transaction that creates member (referencing invitation,
invitationRecord, userRecord, memberRecord, memberSiteAccess, teamMember, and
tx) to ensure atomicity.
---
Nitpick comments:
In `@server/src/api/sites/getSitesFromOrg.ts`:
- Around line 53-91: This handler duplicates team-based filtering already
implemented in getSitesUserHasAccessTo (auth-utils.ts); remove the entire
memberRecord role==="member" block that queries teamSiteAccess/team and
teamMember and instead rely on (or call) getSitesUserHasAccessTo to filter sites
for the current user/organization, or extract the shared logic into a single
helper used by both; ensure references to memberRecord, teamSiteAccess,
teamMember, team, and sitesData are cleaned up so no redundant DB queries remain
and the handler uses the centralized access check.
In `@server/src/api/teams/listTeams.ts`:
- Around line 20-40: The handler currently treats a missing membership
(memberRecord) as an empty teams response; update the logic in listTeams.ts to
return a 403 Unauthorized when the requesting user is not a member of the
organization. Specifically, after computing memberRecord and isAdminOrOwner
(using request.params.organizationId, request.user?.id / getUserIdFromRequest,
and the db query against member), if memberRecord.length === 0 (i.e., user has
no membership) respond with a 403 error (or reply.code(403).send) instead of
continuing to fetch/return an empty teams list; preserve existing behavior for
admins/owners (isAdminOrOwner === true) and for any other allowed flows.
In `@server/src/api/teams/updateTeam.ts`:
- Around line 91-96: The code allows a provided name to be trimmed to an empty
string and saved; before starting the transaction (i.e. before building
updates/await tx.update(team).set(...)), validate the incoming name: if name !==
undefined then compute const trimmed = name.trim(); if trimmed === ""
return/throw a 400-style error (or BadRequest) and do not proceed; otherwise
assign updates.name = trimmed. Apply this check near where now and updates are
prepared so teamId, updates, and tx.update(team).set(...) never run with an
empty name.
In `@server/src/db/postgres/schema.ts`:
- Around line 216-221: The table name string for the teamMember pgTable is
inconsistent (uses "teamMember"); change the pgTable invocation so the database
table name is snake_case (e.g., "team_member") to match the rest of the schema,
update any raw SQL/migration SQL that refers to the old name, and verify
references to the exported constant teamMember (and any foreign-key usages)
still work after renaming; locate the instantiation via the teamMember variable
and the pgTable("teamMember", ...) call to make the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db53c9cb-6849-4503-9aea-83da4fae1d9d
📒 Files selected for processing (36)
client/messages/cs.jsonclient/messages/de.jsonclient/messages/en.jsonclient/messages/es.jsonclient/messages/fr.jsonclient/messages/it.jsonclient/messages/ja.jsonclient/messages/ko.jsonclient/messages/pl.jsonclient/messages/pt.jsonclient/messages/zh.jsonclient/src/api/admin/endpoints/index.tsclient/src/api/admin/endpoints/sites.tsclient/src/api/admin/endpoints/teams.tsclient/src/api/admin/hooks/useTeams.tsclient/src/app/page.tsxclient/src/app/settings/components/SettingsSidebar.tsxclient/src/app/settings/organization/layout.tsxclient/src/app/settings/organization/members/components/InviteMemberDialog.tsxclient/src/app/settings/organization/teams/components/CreateEditTeamDialog.tsxclient/src/app/settings/organization/teams/components/DeleteTeamDialog.tsxclient/src/app/settings/organization/teams/page.tsxserver/drizzle/0002_short_bishop.sqlserver/drizzle/meta/0002_snapshot.jsonserver/drizzle/meta/_journal.jsonserver/src/api/memberAccess/updateInvitationSiteAccess.tsserver/src/api/sites/getSitesFromOrg.tsserver/src/api/teams/createTeam.tsserver/src/api/teams/deleteTeam.tsserver/src/api/teams/index.tsserver/src/api/teams/listTeams.tsserver/src/api/teams/updateTeam.tsserver/src/db/postgres/schema.tsserver/src/index.tsserver/src/lib/auth-utils.tsserver/src/lib/auth.ts
Comment on lines
+667
to
+669
| "WyooVh": "", | ||
| "DoskyC": "", | ||
| "t6nrnE": "", |
Contributor
There was a problem hiding this comment.
Missing Czech translations for team-related strings.
Multiple new localization keys have empty values (""), which will result in blank or missing text for Czech-speaking users. These include team filter labels, team management dialogs, and member/site selection strings.
Consider adding the Czech translations before merging, or track this as a follow-up task to ensure complete localization.
Also applies to: 769-769, 811-812, 844-869
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/messages/cs.json` around lines 667 - 669, Several Czech localization
keys are empty (e.g., "WyooVh", "DoskyC", "t6nrnE" and other keys referenced
around the same change set), causing missing UI text for Czech users; update the
JSON entries by providing appropriate Czech translations for those keys (team
filter labels, team management dialogs, member/site selection strings) or, if
you cannot translate now, add clear TODO comments and fallback English values
for each key so the UI remains readable; ensure you modify the same keys
(WyooVh, DoskyC, t6nrnE, plus the other empty keys noted near lines ~769,
~811-812, ~844-869) in the locale JSON to include completed translations or
temporary fallbacks.
Comment on lines
+667
to
+669
| "WyooVh": "", | ||
| "DoskyC": "", | ||
| "t6nrnE": "", |
Contributor
There was a problem hiding this comment.
Missing Spanish translations for team management features.
Multiple new translation keys have been added with empty string values. Spanish-speaking users will see either blank text or English fallbacks for these team-related strings until translations are provided.
Consider adding the Spanish translations before release, or tracking this as a follow-up task.
Also applies to: 769-769, 811-812, 844-869
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/messages/es.json` around lines 667 - 669, The JSON file contains new
translation keys ("WyooVh", "DoskyC", "t6nrnE" and other empty keys in the
ranges noted) with empty string values causing Spanish users to see blanks;
update client/messages/es.json by providing Spanish translations for each empty
key (replace the empty values with the correct Spanish strings for the
corresponding team management UI labels), ensure keys match exactly and keep
JSON valid, and if you cannot supply final translations now, add a clear
placeholder comment or track these keys in a follow-up task so they're not
released untranslated.
Comment on lines
+667
to
+669
| "WyooVh": "", | ||
| "DoskyC": "", | ||
| "t6nrnE": "", |
Contributor
There was a problem hiding this comment.
Backfill the new French team strings before release.
Several of the new Teams/invitation keys added in this hunk are still empty, so this locale will ship partially untranslated.
Also applies to: 769-769, 811-811, 844-869
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/messages/fr.json` around lines 667 - 669, Several newly added French
translation keys (e.g., "WyooVh", "DoskyC", "t6nrnE" and other empty keys around
the same hunk) are blank; please provide proper French strings for each missing
Teams/invitation key so the locale isn't shipped partially untranslated—open the
same JSON file and replace the empty values for the listed keys (and the other
empty entries noted near lines 769, 811, and 844–869) with the correct French
translations matching the English source keys.
Comment on lines
+667
to
+669
| "WyooVh": "", | ||
| "DoskyC": "", | ||
| "t6nrnE": "", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
next-intl fallback locale configuration
💡 Result:
In next-intl, there is no direct "fallbackLocale" configuration option. Instead, fallback behavior for locales and messages is handled in two primary ways: 1. Routing/Middleware Fallback (defaultLocale): Configure defaultLocale in your routing setup (e.g., defineRouting({ locales: ['en', 'de'], defaultLocale: 'en' })). The middleware falls back to this locale if no matching locale is detected from path, cookie, or Accept-Language header. See official docs on routing configuration and middleware. 2. Messages Fallback (Merge in getRequestConfig): For missing translations in a locale, merge messages from a fallback locale (typically the default) in i18n/request.ts before returning them: import {getRequestConfig} from 'next-intl/server'; export default getRequestConfig(async ({locale}) => { // Load primary locale messages let messages = (await import(./messages/${locale}.json)).default; // Merge with default locale as fallback (deep merge required) if (locale !== 'en') { const defaultMessages = (await import('./messages/en.json')).default; messages = merge(defaultMessages, messages); // Use deepmerge or similar } return { messages }; }); This ensures missing keys fall back to the default locale's values at runtime. Official docs explicitly recommend this approach for expected missing messages, as getMessageFallback is for error handling, not routine fallbacks. For client components, NextIntlClientProvider does not automatically inherit/apply server-side merges; pass merged messages explicitly. Use libraries like deepmerge for merging nested message objects.
Citations:
- 1: https://next-intl.dev/docs/usage/configuration
- 2: Fallback to
defaultLocaleif message with current locale throw an error amannn/next-intl#2077 - 3: https://next-intl.dev/docs/usage/plugin
- 4: https://next-intl.dev/blog/next-intl-4-0
- 5: https://www.thisdot.co/blog/internationalization-in-next-js-with-next-intl
🏁 Script executed:
# First, let's check the size and content of ja.json around the specified lines
wc -l client/messages/ja.jsonRepository: rybbit-io/rybbit
Length of output: 88
🏁 Script executed:
# Read ja.json around the lines mentioned (667-669, 769, 811-812, 844-869)
cat -n client/messages/ja.json | sed -n '665,671p'
cat -n client/messages/ja.json | sed -n '767,771p'
cat -n client/messages/ja.json | sed -n '809,814p'
cat -n client/messages/ja.json | sed -n '842,871p'Repository: rybbit-io/rybbit
Length of output: 1348
🏁 Script executed:
# Check the i18n configuration setup
fd -e ts -e tsx -e js . client/src -x grep -l "getRequestConfig\|NextIntlClientProvider" {}Repository: rybbit-io/rybbit
Length of output: 112
🏁 Script executed:
# Look for i18n related config files
fd -e ts -e js -path "*/i18n/*" .
ls -la client/src/ | grep -i i18nRepository: rybbit-io/rybbit
Length of output: 300
🏁 Script executed:
# Check next.config.ts for next-intl configuration
cat -n next.config.ts 2>/dev/null || echo "next.config.ts not found"Repository: rybbit-io/rybbit
Length of output: 84
🏁 Script executed:
# Count total empty string values in ja.json
rg ':\s*"",$' client/messages/ja.json | wc -lRepository: rybbit-io/rybbit
Length of output: 62
🏁 Script executed:
# Compare with en.json to see if those keys have values
rg '"WyooVh"|"DoskyC"|"t6nrnE"' client/messages/en.jsonRepository: rybbit-io/rybbit
Length of output: 140
🏁 Script executed:
# Check the i18n/request.ts configuration
cat -n client/src/i18n/request.tsRepository: rybbit-io/rybbit
Length of output: 1990
🏁 Script executed:
# Check it.json mentioned in the review for same empty strings
rg '"WyooVh"|"DoskyC"|"t6nrnE"' client/messages/it.jsonRepository: rybbit-io/rybbit
Length of output: 107
🏁 Script executed:
# List all locale files and count empty strings in each
fd -e json client/messages/ -x sh -c 'echo "File: {}"; rg ":\s*\"\",$" {} | wc -l'Repository: rybbit-io/rybbit
Length of output: 429
🏁 Script executed:
# Check if there's a package.json or next.config.* in root
fd 'next\.config|tsconfig\.json|package\.json' -x cat -n {} \; | head -100Repository: rybbit-io/rybbit
Length of output: 3822
🏁 Script executed:
# Check all locale files for the same empty string keys
for locale_file in client/messages/*.json; do
locale=$(basename "$locale_file" .json)
count=$(rg ':\s*"",$' "$locale_file" | wc -l)
# Check if the specific keys exist
has_empty=$(rg '"WyooVh"|"DoskyC"|"t6nrnE"' "$locale_file" | grep '""' | wc -l)
echo "$locale: $count empty strings (sample keys empty: $has_empty)"
doneRepository: rybbit-io/rybbit
Length of output: 542
Empty translation strings will cause blank UI text across 10+ languages without fallback configuration.
All new Teams-related translation keys have empty values (""). The i18n configuration in client/src/i18n/request.ts loads only locale-specific messages without implementing a fallback mechanism, so users will see blank text instead of English defaults.
This affects:
- ja.json, de.json, es.json, fr.json, it.json, ko.json, pl.json, pt.json, zh.json: 31 empty strings each
- cs.json: 60 empty strings
- Critical UI elements like team names, actions, and labels
Resolve by:
- Adding proper translations for all locales before merge
- Implementing message merging with English fallback in
getRequestConfig(as documented in next-intl) - Using English strings as temporary placeholders until translations are complete
Lines with empty values in ja.json
"WyooVh": "",
"DoskyC": "",
"t6nrnE": "",
Also affects: 769, 811-812, 844-869
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/messages/ja.json` around lines 667 - 669, The ja.json contains empty
translation values for new Teams keys (e.g., "WyooVh", "DoskyC", "t6nrnE") which
will produce blank UI; fix by populating these keys with either proper Japanese
translations or English placeholder strings, and implement a fallback merge in
the i18n loader: update getRequestConfig to merge locale messages with the
English messages (default locale) so missing keys fall back to English (use
next-intl recommended merge pattern). Ensure all other affected locale files
(de.json, es.json, fr.json, it.json, ko.json, pl.json, pt.json, zh.json,
cs.json) are either filled or rely on the new fallback behavior.
Comment on lines
+667
to
+669
| "WyooVh": "", | ||
| "DoskyC": "", | ||
| "t6nrnE": "", |
Contributor
There was a problem hiding this comment.
Complete these new Korean translations before release.
The added keys are all empty strings, so the localized teams/invitations flows will have missing copy for the affected labels, dialogs, and filters.
As per coding guidelines, "maintain translation files in messages/ directory (en.json, de.json, etc.)".
Also applies to: 769-769, 811-811, 844-869
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/messages/ko.json` around lines 667 - 669, Several new localization
keys (e.g., "WyooVh", "DoskyC", "t6nrnE" and other empty entries around the same
change set) were committed with empty strings; fill each empty value with the
proper Korean translation for the label/dialog/filter they represent, following
the project's translation guidelines for files in messages/; update the keys
listed in this diff and the other empty keys noted (other empty entries near the
same commit ranges) with accurate Korean copy, run the repository's
i18n/translation validation or linting to ensure no empty values remain, and
submit the updated ko.json entries in the same format as other locales.
| @@ -0,0 +1,82 @@ | |||
| "use client"; | |||
|
|
|||
| import { useExtracted } from "next-intl"; | |||
Contributor
There was a problem hiding this comment.
Incorrect i18n hook import - useExtracted is not a valid next-intl export.
The import uses useExtracted from next-intl, but the correct hook is useTranslations. This will cause a runtime error.
As per coding guidelines: "Use next-intl's 'useTranslations()' hook for i18n".
🐛 Proposed fix
-import { useExtracted } from "next-intl";
+import { useTranslations } from "next-intl";And on line 27:
- const t = useExtracted();
+ const t = useTranslations();Also applies to: 27-27
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/settings/organization/teams/components/DeleteTeamDialog.tsx`
at line 3, The DeleteTeamDialog component imports the wrong i18n hook; replace
the invalid useExtracted import from next-intl with useTranslations and update
any calls to useExtracted(...) (e.g., at the useExtracted(...) call around line
27 inside DeleteTeamDialog) to use useTranslations(...), keeping the same
translation namespace/key string and variable name where used so existing t(...)
calls continue to work.
Comment on lines
+23
to
+25
| export default function TeamsPage() { | ||
| useSetPageTitle("Organization Teams"); | ||
| const t = useExtracted(); |
Contributor
There was a problem hiding this comment.
Localize the page title too.
The page/tab title is still hard-coded in English even though the rest of the screen is translated, so non-English locales will keep showing an English title here.
💡 Suggested fix
export default function TeamsPage() {
- useSetPageTitle("Organization Teams");
const t = useExtracted();
+ useSetPageTitle(t("Organization Teams"));As per coding guidelines, "client/src/**/*.{ts,tsx}: Use next-intl's 'useTranslations()' hook for i18n; maintain translation files in messages/ directory (en.json, de.json, etc.)".
📝 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.
Suggested change
| export default function TeamsPage() { | |
| useSetPageTitle("Organization Teams"); | |
| const t = useExtracted(); | |
| export default function TeamsPage() { | |
| const t = useExtracted(); | |
| useSetPageTitle(t("Organization Teams")); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/settings/organization/teams/page.tsx` around lines 23 - 25,
The page title is hard-coded in TeamsPage; replace the literal "Organization
Teams" by using next-intl's useTranslations and pass the localized string into
useSetPageTitle. In TeamsPage, import useTranslations from 'next-intl', call
const t = useTranslations('<namespace>') (or the existing translation namespace
used by other strings on this page) and then call
useSetPageTitle(t('organizationTeams') or the chosen key); also add the
organizationTeams key to your messages/*.json translation files (en.json,
de.json, etc.) so the title is localized.
Comment on lines
+28
to
+30
| const { data: teamsData, isLoading: teamsLoading } = useTeams( | ||
| activeOrganization?.id | ||
| ); |
Contributor
There was a problem hiding this comment.
Don't collapse failed team fetches into the empty state.
If useTeams fails, teamsData stays undefined, teamsLoading flips to false, and this path renders “No teams yet”. That hides auth/server failures as an empty org and will mislead admins into recreating teams that still exist.
💡 Suggested fix
- const { data: teamsData, isLoading: teamsLoading } = useTeams(
+ const {
+ data: teamsData,
+ isLoading: teamsLoading,
+ isError: isTeamsError,
+ } = useTeams(
activeOrganization?.id
);
@@
- const teams = teamsData?.teams || [];
+ if (isTeamsError) {
+ return (
+ <Card>
+ <CardContent className="py-8 text-sm text-muted-foreground">
+ {t("Failed to load teams. Please refresh and try again.")}
+ </CardContent>
+ </Card>
+ );
+ }
+
+ const teams = teamsData?.teams ?? [];As per coding guidelines, "Use try/catch for error handling with specific error types; let React Query surface API errors".
Also applies to: 52-52, 78-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/settings/organization/teams/page.tsx` around lines 28 - 30,
The UI currently conflates a failed fetch with an empty team list because it
only checks teamsData and teamsLoading; update the render logic that uses
useTeams (symbols: useTeams, teamsData, teamsLoading) to also check React
Query's isError and error from the hook and handle errors explicitly (e.g.,
render an error message or rethrow so the error boundary surfaces the failure)
before falling back to the "No teams yet" empty state; ensure the flow is: if
(teamsLoading) show spinner, else if (isError) show error UI or surface error,
else if (teamsData?.length === 0) show empty state. This change should be
applied wherever the empty-state rendering occurs (the blocks currently using
teamsData/teamsLoading).
Comment on lines
+9
to
+14
| CREATE TABLE "teamMember" ( | ||
| "id" text PRIMARY KEY NOT NULL, | ||
| "team_id" text NOT NULL, | ||
| "user_id" text NOT NULL, | ||
| "created_at" timestamp | ||
| ); |
Contributor
There was a problem hiding this comment.
Enforce one membership row per team/user pair.
teamMember currently allows the same user to be inserted into the same team multiple times. That will double-count members and makes membership/access checks depend on application-side deduping.
💡 Suggested fix
CREATE TABLE "teamMember" (
"id" text PRIMARY KEY NOT NULL,
"team_id" text NOT NULL,
"user_id" text NOT NULL,
- "created_at" timestamp
+ "created_at" timestamp,
+ CONSTRAINT "teamMember_team_user_unique" UNIQUE("team_id","user_id")
);
--> statement-breakpoint
+CREATE INDEX "teamMember_team_idx" ON "teamMember" USING btree ("team_id");--> statement-breakpoint
+CREATE INDEX "teamMember_user_idx" ON "teamMember" USING btree ("user_id");--> statement-breakpoint📝 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.
Suggested change
| CREATE TABLE "teamMember" ( | |
| "id" text PRIMARY KEY NOT NULL, | |
| "team_id" text NOT NULL, | |
| "user_id" text NOT NULL, | |
| "created_at" timestamp | |
| ); | |
| CREATE TABLE "teamMember" ( | |
| "id" text PRIMARY KEY NOT NULL, | |
| "team_id" text NOT NULL, | |
| "user_id" text NOT NULL, | |
| "created_at" timestamp, | |
| CONSTRAINT "teamMember_team_user_unique" UNIQUE("team_id","user_id") | |
| ); | |
| --> statement-breakpoint | |
| CREATE INDEX "teamMember_team_idx" ON "teamMember" USING btree ("team_id"); | |
| --> statement-breakpoint | |
| CREATE INDEX "teamMember_user_idx" ON "teamMember" USING btree ("user_id"); | |
| --> statement-breakpoint |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/drizzle/0002_short_bishop.sql` around lines 9 - 14, The teamMember
table allows duplicate memberships for the same team/user pair; add a uniqueness
constraint on the (team_id, user_id) pair to enforce one row per membership.
Modify the CREATE TABLE for "teamMember" (or add an ALTER TABLE) to include a
UNIQUE(team_id, user_id) constraint (or replace the current "id" primary key
with a composite primary key on team_id and user_id if you prefer) so inserts
for the same team_id and user_id will be rejected at the DB level.
Comment on lines
+13
to
+17
| interface CreateTeamBody { | ||
| name: string; | ||
| memberUserIds?: string[]; | ||
| siteIds?: number[]; | ||
| } |
Contributor
There was a problem hiding this comment.
Validate and dedupe the request body before the DB work.
request.body is untrusted here. A payload like {siteIds: [1, 1]} will hit team_site_access_unique and come back as a 500, while duplicate memberUserIds will currently create duplicate teamMember rows. Parse with Zod and normalize the arrays up front so bad input returns a 400 instead of tripping DB errors or corrupting membership data.
💡 Suggested fix
import { eq, and, inArray } from "drizzle-orm";
import { FastifyReply, FastifyRequest } from "fastify";
+import { z } from "zod";
import { db } from "../../db/postgres/postgres.js";
@@
-interface CreateTeamBody {
- name: string;
- memberUserIds?: string[];
- siteIds?: number[];
-}
+const createTeamBodySchema = z.object({
+ name: z.string().trim().min(1, "Team name is required"),
+ memberUserIds: z
+ .array(z.string().min(1))
+ .default([])
+ .transform((ids) => [...new Set(ids)]),
+ siteIds: z
+ .array(z.number().int().positive())
+ .default([])
+ .transform((ids) => [...new Set(ids)]),
+});
@@
export async function createTeam(
request: FastifyRequest<{
Params: { organizationId: string };
- Body: CreateTeamBody;
+ Body: unknown;
}>,
reply: FastifyReply
) {
const { organizationId } = request.params;
- const { name, memberUserIds, siteIds } = request.body;
-
- if (!name || !name.trim()) {
- return reply.status(400).send({ error: "Team name is required" });
+ const parsedBody = createTeamBodySchema.safeParse(request.body);
+ if (!parsedBody.success) {
+ return reply.status(400).send({ error: parsedBody.error.flatten() });
}
+ const { name, memberUserIds, siteIds } = parsedBody.data;Based on learnings, "Backend: Use Fastify, Drizzle ORM with Postgres, ClickHouse, and Zod".
Also applies to: 27-30, 35-72, 87-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/api/teams/createTeam.ts` around lines 13 - 17, Validate and
normalize the incoming CreateTeamBody (currently represented by the
CreateTeamBody interface and accessed via request.body) using Zod before any DB
work: define a Zod schema for name (string), memberUserIds (optional array of
strings) and siteIds (optional array of numbers), parse request.body and return
a 400 on failure, then normalize by filtering out falsy values and deduplicating
memberUserIds and siteIds (so duplicates won't create duplicate teamMember rows
or trigger the team_site_access_unique DB constraint). Replace direct usage of
request.body in the create-team handler with the validated result and use the
deduped arrays when inserting into teamMember and team_site_access tables to
prevent DB errors and membership corruption.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
New Features
Localization